| Message ID | 1559577023-558-3-git-send-email-suzuki.poulose@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC,01/57] drivers: s390/cio: Use driver_for_each_device | expand |
On Mon, Jun 03, 2019 at 04:49:28PM +0100, Suzuki K Poulose wrote: > Drop the reference to a device found via bus_find_device() This change is correct, but it probably doesn't belong in this series. Would you like me to take it as a stand-alone change? -corey > > Cc: Corey Minyard <minyard@acm.org> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > --- > drivers/char/ipmi/ipmi_si_platform.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/char/ipmi/ipmi_si_platform.c b/drivers/char/ipmi/ipmi_si_platform.c > index f2a91c4..ff82353 100644 > --- a/drivers/char/ipmi/ipmi_si_platform.c > +++ b/drivers/char/ipmi/ipmi_si_platform.c > @@ -442,6 +442,7 @@ void ipmi_remove_platform_device_by_name(char *name) > pdev_match_name))) { > struct platform_device *pdev = to_platform_device(dev); > > + put_device(dev); > platform_device_unregister(pdev); > } > } > -- > 2.7.4 >
Hi On 03/06/2019 18:01, Corey Minyard wrote: > On Mon, Jun 03, 2019 at 04:49:28PM +0100, Suzuki K Poulose wrote: >> Drop the reference to a device found via bus_find_device() > > This change is correct, but it probably doesn't belong in this > series. Would you like me to take it as a stand-alone change? > Sure, please go ahead and I can drop it. Thanks Suzuki
On Mon, Jun 03, 2019 at 04:49:28PM +0100, Suzuki K Poulose wrote: > Drop the reference to a device found via bus_find_device() > > Cc: Corey Minyard <minyard@acm.org> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > --- > drivers/char/ipmi/ipmi_si_platform.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/char/ipmi/ipmi_si_platform.c b/drivers/char/ipmi/ipmi_si_platform.c > index f2a91c4..ff82353 100644 > --- a/drivers/char/ipmi/ipmi_si_platform.c > +++ b/drivers/char/ipmi/ipmi_si_platform.c > @@ -442,6 +442,7 @@ void ipmi_remove_platform_device_by_name(char *name) > pdev_match_name))) { > struct platform_device *pdev = to_platform_device(dev); > > + put_device(dev); > platform_device_unregister(pdev); So you drop the reference, and then actually use the pointer? The drop needs to be _after_ the call to platform_device_unregister(). thanks, greg k-h
On Mon, Jun 03, 2019 at 09:09:21PM +0200, Greg KH wrote: > On Mon, Jun 03, 2019 at 04:49:28PM +0100, Suzuki K Poulose wrote: > > Drop the reference to a device found via bus_find_device() > > > > Cc: Corey Minyard <minyard@acm.org> > > Cc: Arnd Bergmann <arnd@arndb.de> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > > --- > > drivers/char/ipmi/ipmi_si_platform.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/char/ipmi/ipmi_si_platform.c b/drivers/char/ipmi/ipmi_si_platform.c > > index f2a91c4..ff82353 100644 > > --- a/drivers/char/ipmi/ipmi_si_platform.c > > +++ b/drivers/char/ipmi/ipmi_si_platform.c > > @@ -442,6 +442,7 @@ void ipmi_remove_platform_device_by_name(char *name) > > pdev_match_name))) { > > struct platform_device *pdev = to_platform_device(dev); > > > > + put_device(dev); > > platform_device_unregister(pdev); > > So you drop the reference, and then actually use the pointer? I did think about this, and in this case you can, I think. platform_device_unregister() does a put_device() at the end of it's processing, right? But it is better style to do it the other way, so I can change it. -corey > > The drop needs to be _after_ the call to platform_device_unregister(). > > thanks, > > greg k-h
On Mon, Jun 03, 2019 at 02:59:27PM -0500, Corey Minyard wrote: > On Mon, Jun 03, 2019 at 09:09:21PM +0200, Greg KH wrote: > > On Mon, Jun 03, 2019 at 04:49:28PM +0100, Suzuki K Poulose wrote: > > > Drop the reference to a device found via bus_find_device() > > > > > > Cc: Corey Minyard <minyard@acm.org> > > > Cc: Arnd Bergmann <arnd@arndb.de> > > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > > > --- > > > drivers/char/ipmi/ipmi_si_platform.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/drivers/char/ipmi/ipmi_si_platform.c b/drivers/char/ipmi/ipmi_si_platform.c > > > index f2a91c4..ff82353 100644 > > > --- a/drivers/char/ipmi/ipmi_si_platform.c > > > +++ b/drivers/char/ipmi/ipmi_si_platform.c > > > @@ -442,6 +442,7 @@ void ipmi_remove_platform_device_by_name(char *name) > > > pdev_match_name))) { > > > struct platform_device *pdev = to_platform_device(dev); > > > > > > + put_device(dev); > > > platform_device_unregister(pdev); > > > > So you drop the reference, and then actually use the pointer? > > I did think about this, and in this case you can, I think. > platform_device_unregister() does a put_device() at the end of it's > processing, right? Yes, but that is the reference of the counter that was originally initialized, not the reference that was just grabbed here. It's really all the same :) > But it is better style to do it the other way, so I can change it. Please do, thanks. greg k-h
diff --git a/drivers/char/ipmi/ipmi_si_platform.c b/drivers/char/ipmi/ipmi_si_platform.c index f2a91c4..ff82353 100644 --- a/drivers/char/ipmi/ipmi_si_platform.c +++ b/drivers/char/ipmi/ipmi_si_platform.c @@ -442,6 +442,7 @@ void ipmi_remove_platform_device_by_name(char *name) pdev_match_name))) { struct platform_device *pdev = to_platform_device(dev); + put_device(dev); platform_device_unregister(pdev); } }
Drop the reference to a device found via bus_find_device() Cc: Corey Minyard <minyard@acm.org> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> --- drivers/char/ipmi/ipmi_si_platform.c | 1 + 1 file changed, 1 insertion(+) -- 2.7.4