All of lore.kernel.org
 help / color / mirror / Atom feed
* [RESEND][PATCH] pwm: Set class for exported channels in sysfs
@ 2017-09-26 11:59 ` gohai at sukzessiv.net
  0 siblings, 0 replies; 12+ messages in thread
From: gohai @ 2017-09-26 11:59 UTC (permalink / raw)
  To: Thierry Reding
  Cc: H Hartley Sweeten, linux-pwm, linux-arm-kernel, linux-rpi-kernel,
	Gottfried Haider

Notifications for devices without bus or class set get dropped by
dev_uevent_filter. Adding the class to the exported child matches
what the gpio subsystem is doing.

With this change exporting a channel triggers a udev event, which
gives userspace a chance to fixup permissions and makes it possible
for non-root users to make use of the pwm subsystem.

Signed-off-by: Gottfried Haider <gottfried.haider@gmail.com>
CC: Thierry Reding <thierry.reding@gmail.com>
CC: H Hartley Sweeten <hsweeten@visionengravers.com>
CC: linux-pwm@vger.kernel.org
CC: linux-arm-kernel@lists.infradead.org
CC: linux-rpi-kernel@lists.infradead.org
---
 drivers/pwm/sysfs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
index a813239..83f2b0b 100644
--- a/drivers/pwm/sysfs.c
+++ b/drivers/pwm/sysfs.c
@@ -263,6 +263,7 @@ static int pwm_export_child(struct device *parent, struct pwm_device *pwm)
        export->pwm = pwm;
        mutex_init(&export->lock);

+       export->child.class = parent->class;
        export->child.release = pwm_export_release;
        export->child.parent = parent;
        export->child.devt = MKDEV(0, 0);

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RESEND][PATCH] pwm: Set class for exported channels in sysfs
@ 2017-09-26 11:59 ` gohai at sukzessiv.net
  0 siblings, 0 replies; 12+ messages in thread
From: gohai at sukzessiv.net @ 2017-09-26 11:59 UTC (permalink / raw)
  To: linux-arm-kernel

Notifications for devices without bus or class set get dropped by
dev_uevent_filter. Adding the class to the exported child matches
what the gpio subsystem is doing.

With this change exporting a channel triggers a udev event, which
gives userspace a chance to fixup permissions and makes it possible
for non-root users to make use of the pwm subsystem.

Signed-off-by: Gottfried Haider <gottfried.haider@gmail.com>
CC: Thierry Reding <thierry.reding@gmail.com>
CC: H Hartley Sweeten <hsweeten@visionengravers.com>
CC: linux-pwm at vger.kernel.org
CC: linux-arm-kernel at lists.infradead.org
CC: linux-rpi-kernel at lists.infradead.org
---
 drivers/pwm/sysfs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
index a813239..83f2b0b 100644
--- a/drivers/pwm/sysfs.c
+++ b/drivers/pwm/sysfs.c
@@ -263,6 +263,7 @@ static int pwm_export_child(struct device *parent, struct pwm_device *pwm)
        export->pwm = pwm;
        mutex_init(&export->lock);

+       export->child.class = parent->class;
        export->child.release = pwm_export_release;
        export->child.parent = parent;
        export->child.devt = MKDEV(0, 0);

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [RESEND][PATCH] pwm: Set class for exported channels in sysfs
  2017-09-26 11:59 ` gohai at sukzessiv.net
@ 2017-10-12 17:33     ` Stefan Wahren
  -1 siblings, 0 replies; 12+ messages in thread
From: Stefan Wahren @ 2017-10-12 17:33 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Gottfried Haider, linux-pwm-u79uwXL29TY76Z2rM5mHXA,
	H Hartley Sweeten,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	gohai-SJMGYJWzoarE6SqAqhcbZQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r


> gohai-SJMGYJWzoarE6SqAqhcbZQ@public.gmane.org hat am 26. September 2017 um 13:59 geschrieben:
> 
> 
> Notifications for devices without bus or class set get dropped by
> dev_uevent_filter. Adding the class to the exported child matches
> what the gpio subsystem is doing.
> 
> With this change exporting a channel triggers a udev event, which
> gives userspace a chance to fixup permissions and makes it possible
> for non-root users to make use of the pwm subsystem.
> 
> Signed-off-by: Gottfried Haider <gottfried.haider-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> CC: Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> CC: H Hartley Sweeten <hsweeten-3FF4nKcrg1dE2c76skzGb0EOCMrvLtNR@public.gmane.org>
> CC: linux-pwm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> CC: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> CC: linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org

gentle ping ...

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RESEND][PATCH] pwm: Set class for exported channels in sysfs
@ 2017-10-12 17:33     ` Stefan Wahren
  0 siblings, 0 replies; 12+ messages in thread
From: Stefan Wahren @ 2017-10-12 17:33 UTC (permalink / raw)
  To: linux-arm-kernel


> gohai at sukzessiv.net hat am 26. September 2017 um 13:59 geschrieben:
> 
> 
> Notifications for devices without bus or class set get dropped by
> dev_uevent_filter. Adding the class to the exported child matches
> what the gpio subsystem is doing.
> 
> With this change exporting a channel triggers a udev event, which
> gives userspace a chance to fixup permissions and makes it possible
> for non-root users to make use of the pwm subsystem.
> 
> Signed-off-by: Gottfried Haider <gottfried.haider@gmail.com>
> CC: Thierry Reding <thierry.reding@gmail.com>
> CC: H Hartley Sweeten <hsweeten@visionengravers.com>
> CC: linux-pwm at vger.kernel.org
> CC: linux-arm-kernel at lists.infradead.org
> CC: linux-rpi-kernel at lists.infradead.org

gentle ping ...

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RESEND][PATCH] pwm: Set class for exported channels in sysfs
  2017-09-26 11:59 ` gohai at sukzessiv.net
@ 2018-03-30 12:40     ` Fabrice Gasnier
  -1 siblings, 0 replies; 12+ messages in thread
From: Fabrice Gasnier @ 2018-03-30 12:40 UTC (permalink / raw)
  To: gohai-SJMGYJWzoarE6SqAqhcbZQ, Thierry Reding
  Cc: Gottfried Haider, linux-pwm-u79uwXL29TY76Z2rM5mHXA,
	Loic PALLARDY, H Hartley Sweeten, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On 09/26/2017 01:59 PM, gohai-SJMGYJWzoarE6SqAqhcbZQ@public.gmane.org wrote:
> Notifications for devices without bus or class set get dropped by
> dev_uevent_filter. Adding the class to the exported child matches
> what the gpio subsystem is doing.
> 
> With this change exporting a channel triggers a udev event, which
> gives userspace a chance to fixup permissions and makes it possible
> for non-root users to make use of the pwm subsystem.
> 
> Signed-off-by: Gottfried Haider <gottfried.haider-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> CC: Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> CC: H Hartley Sweeten <hsweeten-3FF4nKcrg1dE2c76skzGb0EOCMrvLtNR@public.gmane.org>
> CC: linux-pwm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> CC: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> CC: linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> ---
>  drivers/pwm/sysfs.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
> index a813239..83f2b0b 100644
> --- a/drivers/pwm/sysfs.c
> +++ b/drivers/pwm/sysfs.c
> @@ -263,6 +263,7 @@ static int pwm_export_child(struct device *parent, struct pwm_device *pwm)
>         export->pwm = pwm;
>         mutex_init(&export->lock);
> 
> +       export->child.class = parent->class;

Hi all,

Sorry to raise this old mail thread. I just figured out this patch is
causing *regression* on v4.16-rcs.

This patch has side effect at my end, with multiple pwm chip. It creates
a new entry in '/sys/class/pwm' every time a 'pwmX' is exported:
- echo X > export

This breaks pwm on platforms that have multiple pwmchip:
- 1st time export will create a /sys/class/pwm/pwmX
- when another export happens on another pwmchip, it can't be created
(e.g. -EEXIST)

I looked at /Documentation/ABI/testing/sysfs-class-pwm:
- pmwX should be there: /sys/class/pwm/pwmchipN/pwmX (only ?)

With this patch:
- pwmX symlink is created in addition, directly under /sys/class/pwm

Example on stm32 (stm32429i-eval) platform:
---
$ ls /sys/class/pwm
pwmchip0 pwmchip4

$ cd /sys/class/pwm/pwmchip0/
$ echo 0 > export

$ ls /sys/class/pwm
pwm0 pwmchip0 pwmchip4

$ cd /sys/class/pwm/pwmchip4/
$ echo 0 > export

sysfs: cannot create duplicate filename '/class/pwm/pwm0'
CPU: 0 PID: 50 Comm: sh Not tainted 4.16.0-rc7-00020-g3361545 #1682
Hardware name: STM32 (Device Tree Support)
[<0000c0f1>] (unwind_backtrace) from [<0000b23b>] (show_stack+0xb/0xc)
[<0000b23b>] (show_stack) from [<0008d2f1>] (sysfs_warn_dup+0x31/0x48)
[<0008d2f1>] (sysfs_warn_dup) from [<0008d4a5>]
(sysfs_do_create_link_sd+0x75/0x88)
[<0008d4a5>] (sysfs_do_create_link_sd) from [<000e8e91>]
(device_add+0x111/0x374)
[<000e8e91>] (device_add) from [<000ca795>] (export_store+0xb5/0x12c)
[<000ca795>] (export_store) from [<0008c899>] (kernfs_fop_write+0x87/0xda)
[<0008c899>] (kernfs_fop_write) from [<0005a0a5>] (__vfs_write+0x1d/0xcc)
[<0005a0a5>] (__vfs_write) from [<0005a1c7>] (vfs_write+0x4f/0x7c)
[<0005a1c7>] (vfs_write) from [<0005a29b>] (SyS_write+0x33/0x70)
[<0005a29b>] (SyS_write) from [<00009001>] (ret_fast_syscall+0x1/0x58)
Exception stack...
-sh: write error: File exists

Not sure what the best fix would be thought :-(

probably pwmX should be named also according with pwmchipN ?
- dev_set_name(&export->child, "pwm%u", pwm->hwpwm);
+ dev_set_name(&export->child, "pwmchip%d-pwm%u", chip->base, pwm->hwpwm);
BUT I think this would break existing ABI...

Also this is quite late in the cycle. Maybe a revert would be wise for now ?

Please advise,
Best Regards,
Fabrice

>         export->child.release = pwm_export_release;
>         export->child.parent = parent;
>         export->child.devt = MKDEV(0, 0);
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RESEND][PATCH] pwm: Set class for exported channels in sysfs
@ 2018-03-30 12:40     ` Fabrice Gasnier
  0 siblings, 0 replies; 12+ messages in thread
From: Fabrice Gasnier @ 2018-03-30 12:40 UTC (permalink / raw)
  To: linux-arm-kernel

On 09/26/2017 01:59 PM, gohai at sukzessiv.net wrote:
> Notifications for devices without bus or class set get dropped by
> dev_uevent_filter. Adding the class to the exported child matches
> what the gpio subsystem is doing.
> 
> With this change exporting a channel triggers a udev event, which
> gives userspace a chance to fixup permissions and makes it possible
> for non-root users to make use of the pwm subsystem.
> 
> Signed-off-by: Gottfried Haider <gottfried.haider@gmail.com>
> CC: Thierry Reding <thierry.reding@gmail.com>
> CC: H Hartley Sweeten <hsweeten@visionengravers.com>
> CC: linux-pwm at vger.kernel.org
> CC: linux-arm-kernel at lists.infradead.org
> CC: linux-rpi-kernel at lists.infradead.org
> ---
>  drivers/pwm/sysfs.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
> index a813239..83f2b0b 100644
> --- a/drivers/pwm/sysfs.c
> +++ b/drivers/pwm/sysfs.c
> @@ -263,6 +263,7 @@ static int pwm_export_child(struct device *parent, struct pwm_device *pwm)
>         export->pwm = pwm;
>         mutex_init(&export->lock);
> 
> +       export->child.class = parent->class;

Hi all,

Sorry to raise this old mail thread. I just figured out this patch is
causing *regression* on v4.16-rcs.

This patch has side effect at my end, with multiple pwm chip. It creates
a new entry in '/sys/class/pwm' every time a 'pwmX' is exported:
- echo X > export

This breaks pwm on platforms that have multiple pwmchip:
- 1st time export will create a /sys/class/pwm/pwmX
- when another export happens on another pwmchip, it can't be created
(e.g. -EEXIST)

I looked at /Documentation/ABI/testing/sysfs-class-pwm:
- pmwX should be there: /sys/class/pwm/pwmchipN/pwmX (only ?)

With this patch:
- pwmX symlink is created in addition, directly under /sys/class/pwm

Example on stm32 (stm32429i-eval) platform:
---
$ ls /sys/class/pwm
pwmchip0 pwmchip4

$ cd /sys/class/pwm/pwmchip0/
$ echo 0 > export

$ ls /sys/class/pwm
pwm0 pwmchip0 pwmchip4

$ cd /sys/class/pwm/pwmchip4/
$ echo 0 > export

sysfs: cannot create duplicate filename '/class/pwm/pwm0'
CPU: 0 PID: 50 Comm: sh Not tainted 4.16.0-rc7-00020-g3361545 #1682
Hardware name: STM32 (Device Tree Support)
[<0000c0f1>] (unwind_backtrace) from [<0000b23b>] (show_stack+0xb/0xc)
[<0000b23b>] (show_stack) from [<0008d2f1>] (sysfs_warn_dup+0x31/0x48)
[<0008d2f1>] (sysfs_warn_dup) from [<0008d4a5>]
(sysfs_do_create_link_sd+0x75/0x88)
[<0008d4a5>] (sysfs_do_create_link_sd) from [<000e8e91>]
(device_add+0x111/0x374)
[<000e8e91>] (device_add) from [<000ca795>] (export_store+0xb5/0x12c)
[<000ca795>] (export_store) from [<0008c899>] (kernfs_fop_write+0x87/0xda)
[<0008c899>] (kernfs_fop_write) from [<0005a0a5>] (__vfs_write+0x1d/0xcc)
[<0005a0a5>] (__vfs_write) from [<0005a1c7>] (vfs_write+0x4f/0x7c)
[<0005a1c7>] (vfs_write) from [<0005a29b>] (SyS_write+0x33/0x70)
[<0005a29b>] (SyS_write) from [<00009001>] (ret_fast_syscall+0x1/0x58)
Exception stack...
-sh: write error: File exists

Not sure what the best fix would be thought :-(

probably pwmX should be named also according with pwmchipN ?
- dev_set_name(&export->child, "pwm%u", pwm->hwpwm);
+ dev_set_name(&export->child, "pwmchip%d-pwm%u", chip->base, pwm->hwpwm);
BUT I think this would break existing ABI...

Also this is quite late in the cycle. Maybe a revert would be wise for now ?

Please advise,
Best Regards,
Fabrice

>         export->child.release = pwm_export_release;
>         export->child.parent = parent;
>         export->child.devt = MKDEV(0, 0);
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RESEND][PATCH] pwm: Set class for exported channels in sysfs
  2018-03-30 12:40     ` Fabrice Gasnier
@ 2018-05-19 11:50       ` Stefan Wahren
  -1 siblings, 0 replies; 12+ messages in thread
From: Stefan Wahren @ 2018-05-19 11:50 UTC (permalink / raw)
  To: gohai, Thierry Reding, Fabrice Gasnier
  Cc: Gottfried Haider, linux-pwm, Loic PALLARDY, H Hartley Sweeten,
	broonie, linux-rpi-kernel, linux-arm-kernel

Hello Fabrice,

> Fabrice Gasnier <fabrice.gasnier@st.com> hat am 30. März 2018 um 14:40 geschrieben:
> 
> 
> On 09/26/2017 01:59 PM, gohai@sukzessiv.net wrote:
> > Notifications for devices without bus or class set get dropped by
> > dev_uevent_filter. Adding the class to the exported child matches
> > what the gpio subsystem is doing.
> > 
> > With this change exporting a channel triggers a udev event, which
> > gives userspace a chance to fixup permissions and makes it possible
> > for non-root users to make use of the pwm subsystem.
> > 
> > Signed-off-by: Gottfried Haider <gottfried.haider@gmail.com>
> > CC: Thierry Reding <thierry.reding@gmail.com>
> > CC: H Hartley Sweeten <hsweeten@visionengravers.com>
> > CC: linux-pwm@vger.kernel.org
> > CC: linux-arm-kernel@lists.infradead.org
> > CC: linux-rpi-kernel@lists.infradead.org
> > ---
> >  drivers/pwm/sysfs.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
> > index a813239..83f2b0b 100644
> > --- a/drivers/pwm/sysfs.c
> > +++ b/drivers/pwm/sysfs.c
> > @@ -263,6 +263,7 @@ static int pwm_export_child(struct device *parent, struct pwm_device *pwm)
> >         export->pwm = pwm;
> >         mutex_init(&export->lock);
> > 
> > +       export->child.class = parent->class;
> 
> Hi all,
> 
> Sorry to raise this old mail thread. I just figured out this patch is
> causing *regression* on v4.16-rcs.
> 
> This patch has side effect at my end, with multiple pwm chip. It creates
> a new entry in '/sys/class/pwm' every time a 'pwmX' is exported:
> - echo X > export
> 
> This breaks pwm on platforms that have multiple pwmchip:
> - 1st time export will create a /sys/class/pwm/pwmX
> - when another export happens on another pwmchip, it can't be created
> (e.g. -EEXIST)
> 
> I looked at /Documentation/ABI/testing/sysfs-class-pwm:
> - pmwX should be there: /sys/class/pwm/pwmchipN/pwmX (only ?)
> 
> With this patch:
> - pwmX symlink is created in addition, directly under /sys/class/pwm
> 
> Example on stm32 (stm32429i-eval) platform:
> ---
> $ ls /sys/class/pwm
> pwmchip0 pwmchip4
> 
> $ cd /sys/class/pwm/pwmchip0/
> $ echo 0 > export
> 
> $ ls /sys/class/pwm
> pwm0 pwmchip0 pwmchip4
> 
> $ cd /sys/class/pwm/pwmchip4/
> $ echo 0 > export
> 
> sysfs: cannot create duplicate filename '/class/pwm/pwm0'
> CPU: 0 PID: 50 Comm: sh Not tainted 4.16.0-rc7-00020-g3361545 #1682
> Hardware name: STM32 (Device Tree Support)
> [<0000c0f1>] (unwind_backtrace) from [<0000b23b>] (show_stack+0xb/0xc)
> [<0000b23b>] (show_stack) from [<0008d2f1>] (sysfs_warn_dup+0x31/0x48)
> [<0008d2f1>] (sysfs_warn_dup) from [<0008d4a5>]
> (sysfs_do_create_link_sd+0x75/0x88)
> [<0008d4a5>] (sysfs_do_create_link_sd) from [<000e8e91>]
> (device_add+0x111/0x374)
> [<000e8e91>] (device_add) from [<000ca795>] (export_store+0xb5/0x12c)
> [<000ca795>] (export_store) from [<0008c899>] (kernfs_fop_write+0x87/0xda)
> [<0008c899>] (kernfs_fop_write) from [<0005a0a5>] (__vfs_write+0x1d/0xcc)
> [<0005a0a5>] (__vfs_write) from [<0005a1c7>] (vfs_write+0x4f/0x7c)
> [<0005a1c7>] (vfs_write) from [<0005a29b>] (SyS_write+0x33/0x70)
> [<0005a29b>] (SyS_write) from [<00009001>] (ret_fast_syscall+0x1/0x58)
> Exception stack...
> -sh: write error: File exists
> 
> Not sure what the best fix would be thought :-(
> 
> probably pwmX should be named also according with pwmchipN ?
> - dev_set_name(&export->child, "pwm%u", pwm->hwpwm);
> + dev_set_name(&export->child, "pwmchip%d-pwm%u", chip->base, pwm->hwpwm);
> BUT I think this would break existing ABI...
> 
> Also this is quite late in the cycle. Maybe a revert would be wise for now ?

sorry i didn't noticed your mail before. Could you please prepare a revert patch?

Thanks
Stefan

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RESEND][PATCH] pwm: Set class for exported channels in sysfs
@ 2018-05-19 11:50       ` Stefan Wahren
  0 siblings, 0 replies; 12+ messages in thread
From: Stefan Wahren @ 2018-05-19 11:50 UTC (permalink / raw)
  To: linux-arm-kernel

Hello Fabrice,

> Fabrice Gasnier <fabrice.gasnier@st.com> hat am 30. M?rz 2018 um 14:40 geschrieben:
> 
> 
> On 09/26/2017 01:59 PM, gohai at sukzessiv.net wrote:
> > Notifications for devices without bus or class set get dropped by
> > dev_uevent_filter. Adding the class to the exported child matches
> > what the gpio subsystem is doing.
> > 
> > With this change exporting a channel triggers a udev event, which
> > gives userspace a chance to fixup permissions and makes it possible
> > for non-root users to make use of the pwm subsystem.
> > 
> > Signed-off-by: Gottfried Haider <gottfried.haider@gmail.com>
> > CC: Thierry Reding <thierry.reding@gmail.com>
> > CC: H Hartley Sweeten <hsweeten@visionengravers.com>
> > CC: linux-pwm at vger.kernel.org
> > CC: linux-arm-kernel at lists.infradead.org
> > CC: linux-rpi-kernel at lists.infradead.org
> > ---
> >  drivers/pwm/sysfs.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
> > index a813239..83f2b0b 100644
> > --- a/drivers/pwm/sysfs.c
> > +++ b/drivers/pwm/sysfs.c
> > @@ -263,6 +263,7 @@ static int pwm_export_child(struct device *parent, struct pwm_device *pwm)
> >         export->pwm = pwm;
> >         mutex_init(&export->lock);
> > 
> > +       export->child.class = parent->class;
> 
> Hi all,
> 
> Sorry to raise this old mail thread. I just figured out this patch is
> causing *regression* on v4.16-rcs.
> 
> This patch has side effect at my end, with multiple pwm chip. It creates
> a new entry in '/sys/class/pwm' every time a 'pwmX' is exported:
> - echo X > export
> 
> This breaks pwm on platforms that have multiple pwmchip:
> - 1st time export will create a /sys/class/pwm/pwmX
> - when another export happens on another pwmchip, it can't be created
> (e.g. -EEXIST)
> 
> I looked at /Documentation/ABI/testing/sysfs-class-pwm:
> - pmwX should be there: /sys/class/pwm/pwmchipN/pwmX (only ?)
> 
> With this patch:
> - pwmX symlink is created in addition, directly under /sys/class/pwm
> 
> Example on stm32 (stm32429i-eval) platform:
> ---
> $ ls /sys/class/pwm
> pwmchip0 pwmchip4
> 
> $ cd /sys/class/pwm/pwmchip0/
> $ echo 0 > export
> 
> $ ls /sys/class/pwm
> pwm0 pwmchip0 pwmchip4
> 
> $ cd /sys/class/pwm/pwmchip4/
> $ echo 0 > export
> 
> sysfs: cannot create duplicate filename '/class/pwm/pwm0'
> CPU: 0 PID: 50 Comm: sh Not tainted 4.16.0-rc7-00020-g3361545 #1682
> Hardware name: STM32 (Device Tree Support)
> [<0000c0f1>] (unwind_backtrace) from [<0000b23b>] (show_stack+0xb/0xc)
> [<0000b23b>] (show_stack) from [<0008d2f1>] (sysfs_warn_dup+0x31/0x48)
> [<0008d2f1>] (sysfs_warn_dup) from [<0008d4a5>]
> (sysfs_do_create_link_sd+0x75/0x88)
> [<0008d4a5>] (sysfs_do_create_link_sd) from [<000e8e91>]
> (device_add+0x111/0x374)
> [<000e8e91>] (device_add) from [<000ca795>] (export_store+0xb5/0x12c)
> [<000ca795>] (export_store) from [<0008c899>] (kernfs_fop_write+0x87/0xda)
> [<0008c899>] (kernfs_fop_write) from [<0005a0a5>] (__vfs_write+0x1d/0xcc)
> [<0005a0a5>] (__vfs_write) from [<0005a1c7>] (vfs_write+0x4f/0x7c)
> [<0005a1c7>] (vfs_write) from [<0005a29b>] (SyS_write+0x33/0x70)
> [<0005a29b>] (SyS_write) from [<00009001>] (ret_fast_syscall+0x1/0x58)
> Exception stack...
> -sh: write error: File exists
> 
> Not sure what the best fix would be thought :-(
> 
> probably pwmX should be named also according with pwmchipN ?
> - dev_set_name(&export->child, "pwm%u", pwm->hwpwm);
> + dev_set_name(&export->child, "pwmchip%d-pwm%u", chip->base, pwm->hwpwm);
> BUT I think this would break existing ABI...
> 
> Also this is quite late in the cycle. Maybe a revert would be wise for now ?

sorry i didn't noticed your mail before. Could you please prepare a revert patch?

Thanks
Stefan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RESEND][PATCH] pwm: Set class for exported channels in sysfs
  2017-06-02 10:05 ` Gottfried Haider
@ 2017-12-05  8:27   ` Thierry Reding
  -1 siblings, 0 replies; 12+ messages in thread
From: Thierry Reding @ 2017-12-05  8:27 UTC (permalink / raw)
  To: Gottfried Haider
  Cc: H Hartley Sweeten, linux-pwm, linux-arm-kernel, linux-rpi-kernel

[-- Attachment #1: Type: text/plain, Size: 882 bytes --]

On Fri, Jun 02, 2017 at 10:05:21AM +0000, Gottfried Haider wrote:
> Notifications for devices without bus or class set get dropped by
> dev_uevent_filter. Adding the class to the exported child matches
> what the gpio subsystem is doing.
> 
> With this change exporting a channel triggers a udev event, which
> gives userspace a chance to fixup permissions and makes it possible
> for non-root users to make use of the pwm subsystem.
> 
> Signed-off-by: Gottfried Haider <gottfried.haider@gmail.com>
> CC: Thierry Reding <thierry.reding@gmail.com>
> CC: H Hartley Sweeten <hsweeten@visionengravers.com>
> CC: linux-pwm@vger.kernel.org
> CC: linux-arm-kernel@lists.infradead.org
> CC: linux-rpi-kernel@lists.infradead.org
> ---
>  drivers/pwm/sysfs.c | 1 +
>  1 file changed, 1 insertion(+)

Sorry for the long delay, applied to for-next now.

Thanks,
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RESEND][PATCH] pwm: Set class for exported channels in sysfs
@ 2017-12-05  8:27   ` Thierry Reding
  0 siblings, 0 replies; 12+ messages in thread
From: Thierry Reding @ 2017-12-05  8:27 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jun 02, 2017 at 10:05:21AM +0000, Gottfried Haider wrote:
> Notifications for devices without bus or class set get dropped by
> dev_uevent_filter. Adding the class to the exported child matches
> what the gpio subsystem is doing.
> 
> With this change exporting a channel triggers a udev event, which
> gives userspace a chance to fixup permissions and makes it possible
> for non-root users to make use of the pwm subsystem.
> 
> Signed-off-by: Gottfried Haider <gottfried.haider@gmail.com>
> CC: Thierry Reding <thierry.reding@gmail.com>
> CC: H Hartley Sweeten <hsweeten@visionengravers.com>
> CC: linux-pwm at vger.kernel.org
> CC: linux-arm-kernel at lists.infradead.org
> CC: linux-rpi-kernel at lists.infradead.org
> ---
>  drivers/pwm/sysfs.c | 1 +
>  1 file changed, 1 insertion(+)

Sorry for the long delay, applied to for-next now.

Thanks,
Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20171205/5ba41b60/attachment-0001.sig>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RESEND][PATCH] pwm: Set class for exported channels in sysfs
@ 2017-06-02 10:05 ` Gottfried Haider
  0 siblings, 0 replies; 12+ messages in thread
From: Gottfried Haider @ 2017-06-02 10:05 UTC (permalink / raw)
  To: Gottfried Haider, Thierry Reding
  Cc: H Hartley Sweeten, linux-pwm, linux-arm-kernel, linux-rpi-kernel

Notifications for devices without bus or class set get dropped by
dev_uevent_filter. Adding the class to the exported child matches
what the gpio subsystem is doing.

With this change exporting a channel triggers a udev event, which
gives userspace a chance to fixup permissions and makes it possible
for non-root users to make use of the pwm subsystem.

Signed-off-by: Gottfried Haider <gottfried.haider@gmail.com>
CC: Thierry Reding <thierry.reding@gmail.com>
CC: H Hartley Sweeten <hsweeten@visionengravers.com>
CC: linux-pwm@vger.kernel.org
CC: linux-arm-kernel@lists.infradead.org
CC: linux-rpi-kernel@lists.infradead.org
---
 drivers/pwm/sysfs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
index a813239..83f2b0b 100644
--- a/drivers/pwm/sysfs.c
+++ b/drivers/pwm/sysfs.c
@@ -263,6 +263,7 @@ static int pwm_export_child(struct device *parent, struct pwm_device *pwm)
 	export->pwm = pwm;
 	mutex_init(&export->lock);
 
+	export->child.class = parent->class;
 	export->child.release = pwm_export_release;
 	export->child.parent = parent;
 	export->child.devt = MKDEV(0, 0);
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RESEND][PATCH] pwm: Set class for exported channels in sysfs
@ 2017-06-02 10:05 ` Gottfried Haider
  0 siblings, 0 replies; 12+ messages in thread
From: Gottfried Haider @ 2017-06-02 10:05 UTC (permalink / raw)
  To: linux-arm-kernel

Notifications for devices without bus or class set get dropped by
dev_uevent_filter. Adding the class to the exported child matches
what the gpio subsystem is doing.

With this change exporting a channel triggers a udev event, which
gives userspace a chance to fixup permissions and makes it possible
for non-root users to make use of the pwm subsystem.

Signed-off-by: Gottfried Haider <gottfried.haider@gmail.com>
CC: Thierry Reding <thierry.reding@gmail.com>
CC: H Hartley Sweeten <hsweeten@visionengravers.com>
CC: linux-pwm at vger.kernel.org
CC: linux-arm-kernel at lists.infradead.org
CC: linux-rpi-kernel at lists.infradead.org
---
 drivers/pwm/sysfs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
index a813239..83f2b0b 100644
--- a/drivers/pwm/sysfs.c
+++ b/drivers/pwm/sysfs.c
@@ -263,6 +263,7 @@ static int pwm_export_child(struct device *parent, struct pwm_device *pwm)
 	export->pwm = pwm;
 	mutex_init(&export->lock);
 
+	export->child.class = parent->class;
 	export->child.release = pwm_export_release;
 	export->child.parent = parent;
 	export->child.devt = MKDEV(0, 0);
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2018-05-19 11:50 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-26 11:59 [RESEND][PATCH] pwm: Set class for exported channels in sysfs gohai
2017-09-26 11:59 ` gohai at sukzessiv.net
     [not found] ` <20170926115951.GA29367-SJMGYJWzoarE6SqAqhcbZQ@public.gmane.org>
2017-10-12 17:33   ` Stefan Wahren
2017-10-12 17:33     ` Stefan Wahren
2018-03-30 12:40   ` Fabrice Gasnier
2018-03-30 12:40     ` Fabrice Gasnier
2018-05-19 11:50     ` Stefan Wahren
2018-05-19 11:50       ` Stefan Wahren
  -- strict thread matches above, loose matches on Subject: below --
2017-06-02 10:05 Gottfried Haider
2017-06-02 10:05 ` Gottfried Haider
2017-12-05  8:27 ` Thierry Reding
2017-12-05  8:27   ` Thierry Reding

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.