* [PATCH] spi: Clean up probe and remove functions
@ 2014-02-13 14:28 Jean Delvare
[not found] ` <20140213152841.390de00f-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>
0 siblings, 1 reply; 5+ messages in thread
From: Jean Delvare @ 2014-02-13 14:28 UTC (permalink / raw)
To: linux-spi-u79uwXL29TY76Z2rM5mHXA
Cc: Mika Westerberg, Mark Brown, Rafael J. Wysocki
While backporting 33cf00e5 ("spi: attach/detach SPI device to the ACPI
power domain"), I noticed that the code changes were suboptimal:
* Why use &spi->dev when we have dev at hand?
* After fixing the above, spi is used only once, so we don't really
need a local variable for it.
This results in the following clean-up.
Signed-off-by: Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org>
Cc: Mika Westerberg <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Cc: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Rafael J. Wysocki <rafael.j.wysocki-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
Untested, no hardware.
drivers/spi/spi.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
--- linux-3.14-rc2.orig/drivers/spi/spi.c 2014-02-13 14:42:09.309512009 +0100
+++ linux-3.14-rc2/drivers/spi/spi.c 2014-02-13 14:53:36.204366691 +0100
@@ -255,13 +255,12 @@ EXPORT_SYMBOL_GPL(spi_bus_type);
static int spi_drv_probe(struct device *dev)
{
const struct spi_driver *sdrv = to_spi_driver(dev->driver);
- struct spi_device *spi = to_spi_device(dev);
int ret;
- acpi_dev_pm_attach(&spi->dev, true);
- ret = sdrv->probe(spi);
+ acpi_dev_pm_attach(dev, true);
+ ret = sdrv->probe(to_spi_device(dev));
if (ret)
- acpi_dev_pm_detach(&spi->dev, true);
+ acpi_dev_pm_detach(dev, true);
return ret;
}
@@ -269,11 +268,10 @@ static int spi_drv_probe(struct device *
static int spi_drv_remove(struct device *dev)
{
const struct spi_driver *sdrv = to_spi_driver(dev->driver);
- struct spi_device *spi = to_spi_device(dev);
int ret;
- ret = sdrv->remove(spi);
- acpi_dev_pm_detach(&spi->dev, true);
+ ret = sdrv->remove(to_spi_device(dev));
+ acpi_dev_pm_detach(dev, true);
return ret;
}
--
Jean Delvare
Suse L3 Support
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] spi: Clean up probe and remove functions
[not found] ` <20140213152841.390de00f-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>
@ 2014-02-13 15:14 ` Rafael J. Wysocki
2014-02-14 15:03 ` Mark Brown
1 sibling, 0 replies; 5+ messages in thread
From: Rafael J. Wysocki @ 2014-02-13 15:14 UTC (permalink / raw)
To: Jean Delvare, linux-spi-u79uwXL29TY76Z2rM5mHXA
Cc: Mika Westerberg, Mark Brown
On 2/13/2014 3:28 PM, Jean Delvare wrote:
> While backporting 33cf00e5 ("spi: attach/detach SPI device to the ACPI
> power domain"), I noticed that the code changes were suboptimal:
>
> * Why use &spi->dev when we have dev at hand?
>
> * After fixing the above, spi is used only once, so we don't really
> need a local variable for it.
>
> This results in the following clean-up.
>
> Signed-off-by: Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org>
> Cc: Mika Westerberg <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Cc: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
> Untested, no hardware.
>
> drivers/spi/spi.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> --- linux-3.14-rc2.orig/drivers/spi/spi.c 2014-02-13 14:42:09.309512009 +0100
> +++ linux-3.14-rc2/drivers/spi/spi.c 2014-02-13 14:53:36.204366691 +0100
> @@ -255,13 +255,12 @@ EXPORT_SYMBOL_GPL(spi_bus_type);
> static int spi_drv_probe(struct device *dev)
> {
> const struct spi_driver *sdrv = to_spi_driver(dev->driver);
> - struct spi_device *spi = to_spi_device(dev);
> int ret;
>
> - acpi_dev_pm_attach(&spi->dev, true);
> - ret = sdrv->probe(spi);
> + acpi_dev_pm_attach(dev, true);
> + ret = sdrv->probe(to_spi_device(dev));
> if (ret)
> - acpi_dev_pm_detach(&spi->dev, true);
> + acpi_dev_pm_detach(dev, true);
>
> return ret;
> }
> @@ -269,11 +268,10 @@ static int spi_drv_probe(struct device *
> static int spi_drv_remove(struct device *dev)
> {
> const struct spi_driver *sdrv = to_spi_driver(dev->driver);
> - struct spi_device *spi = to_spi_device(dev);
> int ret;
>
> - ret = sdrv->remove(spi);
> - acpi_dev_pm_detach(&spi->dev, true);
> + ret = sdrv->remove(to_spi_device(dev));
> + acpi_dev_pm_detach(dev, true);
>
> return ret;
> }
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] spi: Clean up probe and remove functions
[not found] ` <20140213152841.390de00f-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>
2014-02-13 15:14 ` Rafael J. Wysocki
@ 2014-02-14 15:03 ` Mark Brown
[not found] ` <20140214150341.GC4451-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
1 sibling, 1 reply; 5+ messages in thread
From: Mark Brown @ 2014-02-14 15:03 UTC (permalink / raw)
To: Jean Delvare
Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA, Mika Westerberg, Rafael J. Wysocki
[-- Attachment #1: Type: text/plain, Size: 654 bytes --]
On Thu, Feb 13, 2014 at 03:28:41PM +0100, Jean Delvare wrote:
> While backporting 33cf00e5 ("spi: attach/detach SPI device to the ACPI
> power domain"), I noticed that the code changes were suboptimal:
>
> * Why use &spi->dev when we have dev at hand?
>
> * After fixing the above, spi is used only once, so we don't really
> need a local variable for it.
I've applied this but I have to say it's a bit marginal as a cleanup -
for example while we do only use the SPI device once it's not entirely
idiomatic to use to_spi_device() in the middle of the code rather than
at the top of the function so it's a small speed bump to see that.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] spi: Clean up probe and remove functions
[not found] ` <20140214150341.GC4451-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
@ 2014-02-14 18:22 ` Jean Delvare
[not found] ` <1392402129.4365.51.camel-H7Kp9ZFCxt/N0uC3ymp8PA@public.gmane.org>
0 siblings, 1 reply; 5+ messages in thread
From: Jean Delvare @ 2014-02-14 18:22 UTC (permalink / raw)
To: Mark Brown
Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA, Mika Westerberg, Rafael J. Wysocki
Hi Mark,
Le Friday 14 February 2014 à 15:03 +0000, Mark Brown a écrit :
> On Thu, Feb 13, 2014 at 03:28:41PM +0100, Jean Delvare wrote:
> > While backporting 33cf00e5 ("spi: attach/detach SPI device to the ACPI
> > power domain"), I noticed that the code changes were suboptimal:
> >
> > * Why use &spi->dev when we have dev at hand?
> >
> > * After fixing the above, spi is used only once, so we don't really
> > need a local variable for it.
>
> I've applied this but I have to say it's a bit marginal as a cleanup -
> for example while we do only use the SPI device once it's not entirely
> idiomatic to use to_spi_device() in the middle of the code rather than
> at the top of the function so it's a small speed bump to see that.
I don't disagree. The rationale for the change is that I simply reverted
to how the code was before 33cf00e5, assuming that the introduction of
spi as a local variable was caused by it being used more than once. If
you believe this was a good change on its own and would prefer to keep
it that way, I could send a patch replacing this one and only changing
&spi->dev to dev. Let me know.
And yes, this is not the most groundbreaking patch either way,
granted ;-)
--
Jean Delvare
Suse L3 Support
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] spi: Clean up probe and remove functions
[not found] ` <1392402129.4365.51.camel-H7Kp9ZFCxt/N0uC3ymp8PA@public.gmane.org>
@ 2014-02-16 0:40 ` Mark Brown
0 siblings, 0 replies; 5+ messages in thread
From: Mark Brown @ 2014-02-16 0:40 UTC (permalink / raw)
To: Jean Delvare
Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA, Mika Westerberg, Rafael J. Wysocki
[-- Attachment #1: Type: text/plain, Size: 614 bytes --]
On Fri, Feb 14, 2014 at 07:22:09PM +0100, Jean Delvare wrote:
> I don't disagree. The rationale for the change is that I simply reverted
> to how the code was before 33cf00e5, assuming that the introduction of
> spi as a local variable was caused by it being used more than once. If
> you believe this was a good change on its own and would prefer to keep
> it that way, I could send a patch replacing this one and only changing
> &spi->dev to dev. Let me know.
I do have a small preference for it but I don't care enough to suggest
you actually send that patch - if it annoyed me enough I'd just fix it
myself.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-02-16 0:40 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-13 14:28 [PATCH] spi: Clean up probe and remove functions Jean Delvare
[not found] ` <20140213152841.390de00f-R0o5gVi9kd7kN2dkZ6Wm7A@public.gmane.org>
2014-02-13 15:14 ` Rafael J. Wysocki
2014-02-14 15:03 ` Mark Brown
[not found] ` <20140214150341.GC4451-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2014-02-14 18:22 ` Jean Delvare
[not found] ` <1392402129.4365.51.camel-H7Kp9ZFCxt/N0uC3ymp8PA@public.gmane.org>
2014-02-16 0:40 ` Mark Brown
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.