[PATCH] gpio: gpio-tb10x: remove incorrect __exit markup

[PATCH] gpio: gpio-tb10x: remove incorrect __exit markup

[Dmitry Torokhov]

Even if bus is not hot-pluggable, the devices can be unbound from the
driver via sysfs, so we should not be using __exit annotations on
remove() methods. The only exception is drivers registered with
platform_driver_probe() which specifically disables sysfs bind/unbind
attributes.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/gpio/gpio-tb10x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-tb10x.c b/drivers/gpio/gpio-tb10x.c
index 62ab9f4..46b8961 100644
--- a/drivers/gpio/gpio-tb10x.c
+++ b/drivers/gpio/gpio-tb10x.c
@@ -283,7 +283,7 @@ fail_ioremap:
 	return ret;
 }
 
-static int __exit tb10x_gpio_remove(struct platform_device *pdev)
+static int tb10x_gpio_remove(struct platform_device *pdev)
 {
 	struct tb10x_gpio *tb10x_gpio = platform_get_drvdata(pdev);
 
-- 
2.2.0.rc0.207.ga3a616c


-- 
Dmitry

Re: [PATCH] gpio: gpio-tb10x: remove incorrect __exit markup

[Linus Walleij]

On Mon, Mar 9, 2015 at 7:04 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:

> Even if bus is not hot-pluggable, the devices can be unbound from the
> driver via sysfs, so we should not be using __exit annotations on
> remove() methods. The only exception is drivers registered with
> platform_driver_probe() which specifically disables sysfs bind/unbind
> attributes.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

I prefer to have this fixed by setting
.suppress_bind_attrs = true
in the struct device_driver .drv portion of the device driver,
so the driver cannot be removed from sysfs.

So platform_driver_probe() isn't really the only exception,
there is a way to do the same supression on ordinary drivers
if we know we won't fiddle with them from sysfs.

Can you make a patch as per above (alternatively tell me
how wrong I am...)

Yours,
Linus Walleij

Re: [PATCH] gpio: gpio-tb10x: remove incorrect __exit markup

[Dmitry Torokhov]

On Tue, Mar 17, 2015 at 01:30:15PM +0100, Linus Walleij wrote:
> On Mon, Mar 9, 2015 at 7:04 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> 
> > Even if bus is not hot-pluggable, the devices can be unbound from the
> > driver via sysfs, so we should not be using __exit annotations on
> > remove() methods. The only exception is drivers registered with
> > platform_driver_probe() which specifically disables sysfs bind/unbind
> > attributes.
> >
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> 
> I prefer to have this fixed by setting
> .suppress_bind_attrs = true
> in the struct device_driver .drv portion of the device driver,
> so the driver cannot be removed from sysfs.
> 
> So platform_driver_probe() isn't really the only exception,
> there is a way to do the same supression on ordinary drivers
> if we know we won't fiddle with them from sysfs.

Yes, you are right, setting suppress_bind_attrs will work too.

> 
> Can you make a patch as per above (alternatively tell me
> how wrong I am...)

Unfortunately I won't be able to do that as I can't provide
justification for such change (i.e. I do not know why you want to
disable unbinding while still keeping the remove() implementation.

Thanks.

-- 
Dmitry

Re: [PATCH] gpio: gpio-tb10x: remove incorrect __exit markup

[Linus Walleij]

On Wed, Mar 18, 2015 at 5:55 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Tue, Mar 17, 2015 at 01:30:15PM +0100, Linus Walleij wrote:
>> On Mon, Mar 9, 2015 at 7:04 PM, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> wrote:
>>
>> > Even if bus is not hot-pluggable, the devices can be unbound from the
>> > driver via sysfs, so we should not be using __exit annotations on
>> > remove() methods. The only exception is drivers registered with
>> > platform_driver_probe() which specifically disables sysfs bind/unbind
>> > attributes.
>> >
>> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>>
>> I prefer to have this fixed by setting
>> .suppress_bind_attrs = true
>> in the struct device_driver .drv portion of the device driver,
>> so the driver cannot be removed from sysfs.
>>
>> So platform_driver_probe() isn't really the only exception,
>> there is a way to do the same supression on ordinary drivers
>> if we know we won't fiddle with them from sysfs.
>
> Yes, you are right, setting suppress_bind_attrs will work too.
>
>>
>> Can you make a patch as per above (alternatively tell me
>> how wrong I am...)
>
> Unfortunately I won't be able to do that as I can't provide
> justification for such change (i.e. I do not know why you want to
> disable unbinding while still keeping the remove() implementation.

You are right, I want a patch deleting the remove() implementation
and setting the .suppress_bind_attrs = true at the same time.

For a bool driver of this type (typically compiled in and probed
at boot) that makes most sense to me.

Well I guess there are a ton of drivers that should be done like
that ... just wanna start somewhere.

Yours,
Linus Walleij

Re: [PATCH] gpio: gpio-tb10x: remove incorrect __exit markup

[Dmitry Torokhov]

On Wed, Mar 25, 2015 at 04:32:13PM +0100, Linus Walleij wrote:
> On Wed, Mar 18, 2015 at 5:55 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Tue, Mar 17, 2015 at 01:30:15PM +0100, Linus Walleij wrote:
> >> On Mon, Mar 9, 2015 at 7:04 PM, Dmitry Torokhov
> >> <dmitry.torokhov@gmail.com> wrote:
> >>
> >> > Even if bus is not hot-pluggable, the devices can be unbound from the
> >> > driver via sysfs, so we should not be using __exit annotations on
> >> > remove() methods. The only exception is drivers registered with
> >> > platform_driver_probe() which specifically disables sysfs bind/unbind
> >> > attributes.
> >> >
> >> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> >>
> >> I prefer to have this fixed by setting
> >> .suppress_bind_attrs = true
> >> in the struct device_driver .drv portion of the device driver,
> >> so the driver cannot be removed from sysfs.
> >>
> >> So platform_driver_probe() isn't really the only exception,
> >> there is a way to do the same supression on ordinary drivers
> >> if we know we won't fiddle with them from sysfs.
> >
> > Yes, you are right, setting suppress_bind_attrs will work too.
> >
> >>
> >> Can you make a patch as per above (alternatively tell me
> >> how wrong I am...)
> >
> > Unfortunately I won't be able to do that as I can't provide
> > justification for such change (i.e. I do not know why you want to
> > disable unbinding while still keeping the remove() implementation.
> 
> You are right, I want a patch deleting the remove() implementation
> and setting the .suppress_bind_attrs = true at the same time.
> 
> For a bool driver of this type (typically compiled in and probed
> at boot) that makes most sense to me.

Hmm, I do not think this is a good justification for disabling
unbinding. However, as far as I can see, removing gpiochip will succeed
even though there are users of gpios in the system and that might
justify the change. But does this mean that we give up on making
gpiochip clean up properly and we should schedule gpiochip_remove() and
the rest of gpio cleanup infrastructure for removal and declare that
gpio drivers can not ever be made modules?

Seems like wrong direction to move to...

Thanks.

-- 
Dmitry

Re: [PATCH] gpio: gpio-tb10x: remove incorrect __exit markup

[Linus Walleij]

On Wed, Mar 25, 2015 at 6:14 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Wed, Mar 25, 2015 at 04:32:13PM +0100, Linus Walleij wrote:

>> For a bool driver of this type (typically compiled in and probed
>> at boot) that makes most sense to me.
>
> Hmm, I do not think this is a good justification for disabling
> unbinding. However, as far as I can see, removing gpiochip will succeed
> even though there are users of gpios in the system and that might
> justify the change. But does this mean that we give up on making
> gpiochip clean up properly and we should schedule gpiochip_remove() and
> the rest of gpio cleanup infrastructure for removal and declare that
> gpio drivers can not ever be made modules?

No. Johan Hovold is looking into use cases for GPIO chips that come
and go, think GPIO on USB. These obviously need remove() semantics.

What I'm thinking is that the SoC-embedded GPIO controllers (I don't know
how many these are) should probably be bool in Kconfig and also
patched not to have remove() functions and drop the sysfs attribs
for unbinding.

But now I think I'll just apply the patch anyways because it sure does
not make a difference wrt to the bigger problem, so I'm just bitching
around about details and thinking aloud, sorry..

Yours,
Linus Walleij

Re: [PATCH] gpio: gpio-tb10x: remove incorrect __exit markup

[Linus Walleij]

On Mon, Mar 9, 2015 at 7:04 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:

> Even if bus is not hot-pluggable, the devices can be unbound from the
> driver via sysfs, so we should not be using __exit annotations on
> remove() methods. The only exception is drivers registered with
> platform_driver_probe() which specifically disables sysfs bind/unbind
> attributes.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Patch applied.

Yours,
Linus Walleij