* [PATCH v2 0/2] pwm: sysfs: fix exporting PWM channel
@ 2018-10-01 13:23 ` Fabrice Gasnier
0 siblings, 0 replies; 27+ messages in thread
From: Fabrice Gasnier @ 2018-10-01 13:23 UTC (permalink / raw)
To: linux-arm-kernel
Since commit 7e5d1fd75c3d ("pwm: Set class for exported channels in sysfs")
- it's not possible to export more than one PWM channel
- ABI has changed, as a side effect. It may cause bad behavior as pwmchip
attributes are wrongly added to pwm channels and report wrong values.
See [1] and [2].
One purpose of the original patch is to send uevents to udev, when exporting a
PWM channel through the sysfs. This series:
- Reverts the original patch.
- Proposes a new way to send notifications to be used by udev rules.
- With this series:
$ echo 0 > /sys/class/pwm/pwmchip0/export
$ ls /sys/class/pwm
pwmchip0 pwmchip4
$ ls /sys/class/pwm/pwmchip0/pwm0/
capture enable polarity uevent
duty_cycle period power
- Without this series:
$ echo 0 > /sys/class/pwm/pwmchip0/export
$ ls /sys/class/pwm
pwm0 pwmchip0 pwmchip4
$ ls /sys/class/pwm/pwmchip0/pwm0/
capture duty_cycle export period power uevent
device enable npwm polarity subsystem unexport
- Backtrace when exporting a 2nd channel (0) on a separate pwmchip device:
$ echo 0 > /sys/class/pwm/pwmchip4/export
[ 95.286558] sysfs: cannot create duplicate filename '/class/pwm/pwm0'
[ 95.293630] CPU: 0 PID: 54 Comm: sh Not tainted 4.19.0-rc6-00013-g00b49b0 #151
[ 95.301344] Hardware name: STM32 (Device Tree Support)
[ 95.306833] [<0000c155>] (unwind_backtrace) from [<0000b273>] (show_stack+0xb/0xc)
[ 95.315136] [<0000b273>] (show_stack) from [<00092455>] (sysfs_warn_dup+0x31/0x48)
[ 95.323247] [<00092455>] (sysfs_warn_dup) from [<00092635>] (sysfs_do_create_link_sd+0x75/0x88)
[ 95.332539] [<00092635>] (sysfs_do_create_link_sd) from [<00125823>] (device_add+0x133/0x3b0)
[ 95.341694] [<00125823>] (device_add) from [<001059ed>] (export_store+0xb5/0x12c)
[ 95.349761] [<001059ed>] (export_store) from [<00091911>] (kernfs_fop_write+0x87/0xda)
[ 95.358150] [<00091911>] (kernfs_fop_write) from [<0005beb1>] (__vfs_write+0x1d/0xe0)
[ 95.366295] [<0005beb1>] (__vfs_write) from [<0005bfe7>] (vfs_write+0x4f/0x7c)
[ 95.374053] [<0005bfe7>] (vfs_write) from [<0005c0bf>] (ksys_write+0x33/0x70)
[ 95.381708] [<0005c0bf>] (ksys_write) from [<00009001>] (ret_fast_syscall+0x1/0x58)
[ 95.389682] Exception stack(0x01bcffa8 to 0x01bcfff0)
[ 95.394946] ffa0: 00000000 00c4883c 00000001 00c4e590 00000002 00000004
[ 95.403639] ffc0: 00000000 00c4883c 00c4cbe8 00000004 00000002 00000020 00000000 00c4d008
[ 95.412223] ffe0: 00c29151 00c4cbe8 00c17833 00c13c0c
-sh: write error: File exists
[1] https://lkml.org/lkml/2018/9/25/713
[2] https://lkml.org/lkml/2018/9/25/447
---
Changes in v2:
- update revert commit message
- new patch 2/2 to propose uevent notification (change) on pwmchip
Fabrice Gasnier (2):
Revert "pwm: Set class for exported channels in sysfs"
pwm: send a uevent on the pwmchip device upon channel sysfs (un)export
drivers/pwm/sysfs.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
--
1.9.1
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH v2 0/2] pwm: sysfs: fix exporting PWM channel
@ 2018-10-01 13:23 ` Fabrice Gasnier
0 siblings, 0 replies; 27+ messages in thread
From: Fabrice Gasnier @ 2018-10-01 13:23 UTC (permalink / raw)
To: thierry.reding, gottfried.haider
Cc: stefan.wahren, hsweeten, loic.pallardy, broonie, fabrice.gasnier,
gohai, michal.vokac, linux-stm32, linux-arm-kernel,
linux-rpi-kernel, linux-kernel, linux-pwm
Since commit 7e5d1fd75c3d ("pwm: Set class for exported channels in sysfs")
- it's not possible to export more than one PWM channel
- ABI has changed, as a side effect. It may cause bad behavior as pwmchip
attributes are wrongly added to pwm channels and report wrong values.
See [1] and [2].
One purpose of the original patch is to send uevents to udev, when exporting a
PWM channel through the sysfs. This series:
- Reverts the original patch.
- Proposes a new way to send notifications to be used by udev rules.
- With this series:
$ echo 0 > /sys/class/pwm/pwmchip0/export
$ ls /sys/class/pwm
pwmchip0 pwmchip4
$ ls /sys/class/pwm/pwmchip0/pwm0/
capture enable polarity uevent
duty_cycle period power
- Without this series:
$ echo 0 > /sys/class/pwm/pwmchip0/export
$ ls /sys/class/pwm
pwm0 pwmchip0 pwmchip4
$ ls /sys/class/pwm/pwmchip0/pwm0/
capture duty_cycle export period power uevent
device enable npwm polarity subsystem unexport
- Backtrace when exporting a 2nd channel (0) on a separate pwmchip device:
$ echo 0 > /sys/class/pwm/pwmchip4/export
[ 95.286558] sysfs: cannot create duplicate filename '/class/pwm/pwm0'
[ 95.293630] CPU: 0 PID: 54 Comm: sh Not tainted 4.19.0-rc6-00013-g00b49b0 #151
[ 95.301344] Hardware name: STM32 (Device Tree Support)
[ 95.306833] [<0000c155>] (unwind_backtrace) from [<0000b273>] (show_stack+0xb/0xc)
[ 95.315136] [<0000b273>] (show_stack) from [<00092455>] (sysfs_warn_dup+0x31/0x48)
[ 95.323247] [<00092455>] (sysfs_warn_dup) from [<00092635>] (sysfs_do_create_link_sd+0x75/0x88)
[ 95.332539] [<00092635>] (sysfs_do_create_link_sd) from [<00125823>] (device_add+0x133/0x3b0)
[ 95.341694] [<00125823>] (device_add) from [<001059ed>] (export_store+0xb5/0x12c)
[ 95.349761] [<001059ed>] (export_store) from [<00091911>] (kernfs_fop_write+0x87/0xda)
[ 95.358150] [<00091911>] (kernfs_fop_write) from [<0005beb1>] (__vfs_write+0x1d/0xe0)
[ 95.366295] [<0005beb1>] (__vfs_write) from [<0005bfe7>] (vfs_write+0x4f/0x7c)
[ 95.374053] [<0005bfe7>] (vfs_write) from [<0005c0bf>] (ksys_write+0x33/0x70)
[ 95.381708] [<0005c0bf>] (ksys_write) from [<00009001>] (ret_fast_syscall+0x1/0x58)
[ 95.389682] Exception stack(0x01bcffa8 to 0x01bcfff0)
[ 95.394946] ffa0: 00000000 00c4883c 00000001 00c4e590 00000002 00000004
[ 95.403639] ffc0: 00000000 00c4883c 00c4cbe8 00000004 00000002 00000020 00000000 00c4d008
[ 95.412223] ffe0: 00c29151 00c4cbe8 00c17833 00c13c0c
-sh: write error: File exists
[1] https://lkml.org/lkml/2018/9/25/713
[2] https://lkml.org/lkml/2018/9/25/447
---
Changes in v2:
- update revert commit message
- new patch 2/2 to propose uevent notification (change) on pwmchip
Fabrice Gasnier (2):
Revert "pwm: Set class for exported channels in sysfs"
pwm: send a uevent on the pwmchip device upon channel sysfs (un)export
drivers/pwm/sysfs.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
--
1.9.1
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH v2 1/2] Revert "pwm: Set class for exported channels in sysfs"
2018-10-01 13:23 ` Fabrice Gasnier
(?)
@ 2018-10-01 13:23 ` Fabrice Gasnier
-1 siblings, 0 replies; 27+ messages in thread
From: Fabrice Gasnier @ 2018-10-01 13:23 UTC (permalink / raw)
To: thierry.reding, gottfried.haider
Cc: stefan.wahren, hsweeten, loic.pallardy, broonie, fabrice.gasnier,
gohai, michal.vokac, linux-stm32, linux-arm-kernel,
linux-rpi-kernel, linux-kernel, linux-pwm
This reverts commit 7e5d1fd75c3dde9fc10c4472b9368089d1b81d00 ("pwm: Set
class for exported channels in sysfs") as it causes regression with
multiple pwm chip[1], when exporting a pwm channel (echo X > export):
- ABI (Documentation/ABI/testing/sysfs-class-pwm) states pwmX should be
created in /sys/class/pwm/pwmchipN/pwmX
- Reverted patch causes new entry to be also created directly in
/sys/class/pwm/pwmX
- 1st time, exporting pwmX will create an entry in /sys/class/pwm/pwmX
- class attributes are added under pwmX folder, such as export, unexport
npwm, symlinks. This is wrong as it belongs to pwmchipN. It may cause
bad behavior and report wrong values.
- when another export happens on another pwmchip, it can't be created
(e.g. -EEXIST). This is causing the issue with multiple pwmchip.
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'
...Exception stack follows...
This is also seen on other platform [2]
[1] https://lkml.org/lkml/2018/9/25/713
[2] https://lkml.org/lkml/2018/9/25/447
Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Tested-by: Gottfried Haider <gottfried.haider@gmail.com>
---
drivers/pwm/sysfs.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
index 7c71cdb..4726d43 100644
--- a/drivers/pwm/sysfs.c
+++ b/drivers/pwm/sysfs.c
@@ -263,7 +263,6 @@ 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);
--
1.9.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v2 1/2] Revert "pwm: Set class for exported channels in sysfs"
@ 2018-10-01 13:23 ` Fabrice Gasnier
0 siblings, 0 replies; 27+ messages in thread
From: Fabrice Gasnier @ 2018-10-01 13:23 UTC (permalink / raw)
To: linux-arm-kernel
This reverts commit 7e5d1fd75c3dde9fc10c4472b9368089d1b81d00 ("pwm: Set
class for exported channels in sysfs") as it causes regression with
multiple pwm chip[1], when exporting a pwm channel (echo X > export):
- ABI (Documentation/ABI/testing/sysfs-class-pwm) states pwmX should be
created in /sys/class/pwm/pwmchipN/pwmX
- Reverted patch causes new entry to be also created directly in
/sys/class/pwm/pwmX
- 1st time, exporting pwmX will create an entry in /sys/class/pwm/pwmX
- class attributes are added under pwmX folder, such as export, unexport
npwm, symlinks. This is wrong as it belongs to pwmchipN. It may cause
bad behavior and report wrong values.
- when another export happens on another pwmchip, it can't be created
(e.g. -EEXIST). This is causing the issue with multiple pwmchip.
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'
...Exception stack follows...
This is also seen on other platform [2]
[1] https://lkml.org/lkml/2018/9/25/713
[2] https://lkml.org/lkml/2018/9/25/447
Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Tested-by: Gottfried Haider <gottfried.haider@gmail.com>
---
drivers/pwm/sysfs.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
index 7c71cdb..4726d43 100644
--- a/drivers/pwm/sysfs.c
+++ b/drivers/pwm/sysfs.c
@@ -263,7 +263,6 @@ 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);
--
1.9.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v2 1/2] Revert "pwm: Set class for exported channels in sysfs"
@ 2018-10-01 13:23 ` Fabrice Gasnier
0 siblings, 0 replies; 27+ messages in thread
From: Fabrice Gasnier @ 2018-10-01 13:23 UTC (permalink / raw)
To: thierry.reding, gottfried.haider
Cc: stefan.wahren, hsweeten, loic.pallardy, broonie, fabrice.gasnier,
gohai, michal.vokac, linux-stm32, linux-arm-kernel,
linux-rpi-kernel, linux-kernel, linux-pwm
This reverts commit 7e5d1fd75c3dde9fc10c4472b9368089d1b81d00 ("pwm: Set
class for exported channels in sysfs") as it causes regression with
multiple pwm chip[1], when exporting a pwm channel (echo X > export):
- ABI (Documentation/ABI/testing/sysfs-class-pwm) states pwmX should be
created in /sys/class/pwm/pwmchipN/pwmX
- Reverted patch causes new entry to be also created directly in
/sys/class/pwm/pwmX
- 1st time, exporting pwmX will create an entry in /sys/class/pwm/pwmX
- class attributes are added under pwmX folder, such as export, unexport
npwm, symlinks. This is wrong as it belongs to pwmchipN. It may cause
bad behavior and report wrong values.
- when another export happens on another pwmchip, it can't be created
(e.g. -EEXIST). This is causing the issue with multiple pwmchip.
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'
...Exception stack follows...
This is also seen on other platform [2]
[1] https://lkml.org/lkml/2018/9/25/713
[2] https://lkml.org/lkml/2018/9/25/447
Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Tested-by: Gottfried Haider <gottfried.haider@gmail.com>
---
drivers/pwm/sysfs.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
index 7c71cdb..4726d43 100644
--- a/drivers/pwm/sysfs.c
+++ b/drivers/pwm/sysfs.c
@@ -263,7 +263,6 @@ 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);
--
1.9.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v2 1/2] Revert "pwm: Set class for exported channels in sysfs"
2018-10-01 13:23 ` Fabrice Gasnier
@ 2018-10-01 16:27 ` Michal Vokáč
-1 siblings, 0 replies; 27+ messages in thread
From: Michal Vokáč @ 2018-10-01 16:27 UTC (permalink / raw)
To: Fabrice Gasnier, thierry.reding, gottfried.haider
Cc: stefan.wahren, hsweeten, loic.pallardy, broonie, gohai,
linux-stm32, linux-arm-kernel, linux-rpi-kernel, linux-kernel,
linux-pwm
On 1.10.2018 15:23, Fabrice Gasnier wrote:
> This reverts commit 7e5d1fd75c3dde9fc10c4472b9368089d1b81d00 ("pwm: Set
> class for exported channels in sysfs") as it causes regression with
> multiple pwm chip[1], when exporting a pwm channel (echo X > export):
>
> - ABI (Documentation/ABI/testing/sysfs-class-pwm) states pwmX should be
> created in /sys/class/pwm/pwmchipN/pwmX
> - Reverted patch causes new entry to be also created directly in
> /sys/class/pwm/pwmX
> - 1st time, exporting pwmX will create an entry in /sys/class/pwm/pwmX
> - class attributes are added under pwmX folder, such as export, unexport
> npwm, symlinks. This is wrong as it belongs to pwmchipN. It may cause
> bad behavior and report wrong values.
> - when another export happens on another pwmchip, it can't be created
> (e.g. -EEXIST). This is causing the issue with multiple pwmchip.
>
> 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'
> ...Exception stack follows...
>
> This is also seen on other platform [2]
>
> [1] https://lkml.org/lkml/2018/9/25/713
> [2] https://lkml.org/lkml/2018/9/25/447
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Tested-by: Gottfried Haider <gottfried.haider@gmail.com>
Tested-by: Michal Vokáč <michal.vokac@ysoft.com>
> ---
> drivers/pwm/sysfs.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
> index 7c71cdb..4726d43 100644
> --- a/drivers/pwm/sysfs.c
> +++ b/drivers/pwm/sysfs.c
> @@ -263,7 +263,6 @@ 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 [flat|nested] 27+ messages in thread* [PATCH v2 1/2] Revert "pwm: Set class for exported channels in sysfs"
@ 2018-10-01 16:27 ` Michal Vokáč
0 siblings, 0 replies; 27+ messages in thread
From: Michal Vokáč @ 2018-10-01 16:27 UTC (permalink / raw)
To: linux-arm-kernel
On 1.10.2018 15:23, Fabrice Gasnier wrote:
> This reverts commit 7e5d1fd75c3dde9fc10c4472b9368089d1b81d00 ("pwm: Set
> class for exported channels in sysfs") as it causes regression with
> multiple pwm chip[1], when exporting a pwm channel (echo X > export):
>
> - ABI (Documentation/ABI/testing/sysfs-class-pwm) states pwmX should be
> created in /sys/class/pwm/pwmchipN/pwmX
> - Reverted patch causes new entry to be also created directly in
> /sys/class/pwm/pwmX
> - 1st time, exporting pwmX will create an entry in /sys/class/pwm/pwmX
> - class attributes are added under pwmX folder, such as export, unexport
> npwm, symlinks. This is wrong as it belongs to pwmchipN. It may cause
> bad behavior and report wrong values.
> - when another export happens on another pwmchip, it can't be created
> (e.g. -EEXIST). This is causing the issue with multiple pwmchip.
>
> 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'
> ...Exception stack follows...
>
> This is also seen on other platform [2]
>
> [1] https://lkml.org/lkml/2018/9/25/713
> [2] https://lkml.org/lkml/2018/9/25/447
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Tested-by: Gottfried Haider <gottfried.haider@gmail.com>
Tested-by: Michal Vok?? <michal.vokac@ysoft.com>
> ---
> drivers/pwm/sysfs.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
> index 7c71cdb..4726d43 100644
> --- a/drivers/pwm/sysfs.c
> +++ b/drivers/pwm/sysfs.c
> @@ -263,7 +263,6 @@ 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 [flat|nested] 27+ messages in thread
* [PATCH v2 2/2] pwm: send a uevent on the pwmchip device upon channel sysfs (un)export
2018-10-01 13:23 ` Fabrice Gasnier
(?)
@ 2018-10-01 13:23 ` Fabrice Gasnier
-1 siblings, 0 replies; 27+ messages in thread
From: Fabrice Gasnier @ 2018-10-01 13:23 UTC (permalink / raw)
To: thierry.reding, gottfried.haider
Cc: stefan.wahren, hsweeten, loic.pallardy, broonie, fabrice.gasnier,
gohai, michal.vokac, linux-stm32, linux-arm-kernel,
linux-rpi-kernel, linux-kernel, linux-pwm
This patch sends a uevent (KOBJ_CHANGE) on the pwmchipN device, everytime
a pwmX channel has been exported/unexported via sysfs.
This allows udev to implement rules on such events, like:
SUBSYSTEM=="pwm*", PROGRAM="/bin/sh -c '\
chown -R root:gpio /sys/class/pwm && chmod -R 770 /sys/class/pwm;\
chown -R root:gpio
/sys/devices/platform/soc/*.pwm/pwm/pwmchip* && chmod -R 770
/sys/devices/platform/soc/*.pwm/pwm/pwmchip*\
'"
This is a replacement patch for commit 7e5d1fd75c3d ("pwm: Set class for
exported channels in sysfs"), see [1].
basic testing:
$ udevadm monitor --environment &
$ echo 0 > /sys/class/pwm/pwmchip0/export
KERNEL[197.321736] change /devices/.../pwm/pwmchip0 (pwm)
ACTION=change
DEVPATH=/devices/.../pwm/pwmchip0
EXPORT=pwm0
SEQNUM=2045
SUBSYSTEM=pwm
[1] https://lkml.org/lkml/2018/9/25/713
Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Tested-by: Gottfried Haider <gottfried.haider@gmail.com>
---
drivers/pwm/sysfs.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
index 4726d43..ceb233d 100644
--- a/drivers/pwm/sysfs.c
+++ b/drivers/pwm/sysfs.c
@@ -249,6 +249,7 @@ static void pwm_export_release(struct device *child)
static int pwm_export_child(struct device *parent, struct pwm_device *pwm)
{
struct pwm_export *export;
+ char *pwm_prop[2];
int ret;
if (test_and_set_bit(PWMF_EXPORTED, &pwm->flags))
@@ -276,6 +277,10 @@ static int pwm_export_child(struct device *parent, struct pwm_device *pwm)
export = NULL;
return ret;
}
+ pwm_prop[0] = kasprintf(GFP_KERNEL, "EXPORT=pwm%u", pwm->hwpwm);
+ pwm_prop[1] = NULL;
+ kobject_uevent_env(&parent->kobj, KOBJ_CHANGE, pwm_prop);
+ kfree(pwm_prop[0]);
return 0;
}
@@ -288,6 +293,7 @@ static int pwm_unexport_match(struct device *child, void *data)
static int pwm_unexport_child(struct device *parent, struct pwm_device *pwm)
{
struct device *child;
+ char *pwm_prop[2];
if (!test_and_clear_bit(PWMF_EXPORTED, &pwm->flags))
return -ENODEV;
@@ -296,6 +302,11 @@ static int pwm_unexport_child(struct device *parent, struct pwm_device *pwm)
if (!child)
return -ENODEV;
+ pwm_prop[0] = kasprintf(GFP_KERNEL, "UNEXPORT=pwm%u", pwm->hwpwm);
+ pwm_prop[1] = NULL;
+ kobject_uevent_env(&parent->kobj, KOBJ_CHANGE, pwm_prop);
+ kfree(pwm_prop[0]);
+
/* for device_find_child() */
put_device(child);
device_unregister(child);
--
1.9.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v2 2/2] pwm: send a uevent on the pwmchip device upon channel sysfs (un)export
@ 2018-10-01 13:23 ` Fabrice Gasnier
0 siblings, 0 replies; 27+ messages in thread
From: Fabrice Gasnier @ 2018-10-01 13:23 UTC (permalink / raw)
To: linux-arm-kernel
This patch sends a uevent (KOBJ_CHANGE) on the pwmchipN device, everytime
a pwmX channel has been exported/unexported via sysfs.
This allows udev to implement rules on such events, like:
SUBSYSTEM=="pwm*", PROGRAM="/bin/sh -c '\
chown -R root:gpio /sys/class/pwm && chmod -R 770 /sys/class/pwm;\
chown -R root:gpio
/sys/devices/platform/soc/*.pwm/pwm/pwmchip* && chmod -R 770
/sys/devices/platform/soc/*.pwm/pwm/pwmchip*\
'"
This is a replacement patch for commit 7e5d1fd75c3d ("pwm: Set class for
exported channels in sysfs"), see [1].
basic testing:
$ udevadm monitor --environment &
$ echo 0 > /sys/class/pwm/pwmchip0/export
KERNEL[197.321736] change /devices/.../pwm/pwmchip0 (pwm)
ACTION=change
DEVPATH=/devices/.../pwm/pwmchip0
EXPORT=pwm0
SEQNUM=2045
SUBSYSTEM=pwm
[1] https://lkml.org/lkml/2018/9/25/713
Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Tested-by: Gottfried Haider <gottfried.haider@gmail.com>
---
drivers/pwm/sysfs.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
index 4726d43..ceb233d 100644
--- a/drivers/pwm/sysfs.c
+++ b/drivers/pwm/sysfs.c
@@ -249,6 +249,7 @@ static void pwm_export_release(struct device *child)
static int pwm_export_child(struct device *parent, struct pwm_device *pwm)
{
struct pwm_export *export;
+ char *pwm_prop[2];
int ret;
if (test_and_set_bit(PWMF_EXPORTED, &pwm->flags))
@@ -276,6 +277,10 @@ static int pwm_export_child(struct device *parent, struct pwm_device *pwm)
export = NULL;
return ret;
}
+ pwm_prop[0] = kasprintf(GFP_KERNEL, "EXPORT=pwm%u", pwm->hwpwm);
+ pwm_prop[1] = NULL;
+ kobject_uevent_env(&parent->kobj, KOBJ_CHANGE, pwm_prop);
+ kfree(pwm_prop[0]);
return 0;
}
@@ -288,6 +293,7 @@ static int pwm_unexport_match(struct device *child, void *data)
static int pwm_unexport_child(struct device *parent, struct pwm_device *pwm)
{
struct device *child;
+ char *pwm_prop[2];
if (!test_and_clear_bit(PWMF_EXPORTED, &pwm->flags))
return -ENODEV;
@@ -296,6 +302,11 @@ static int pwm_unexport_child(struct device *parent, struct pwm_device *pwm)
if (!child)
return -ENODEV;
+ pwm_prop[0] = kasprintf(GFP_KERNEL, "UNEXPORT=pwm%u", pwm->hwpwm);
+ pwm_prop[1] = NULL;
+ kobject_uevent_env(&parent->kobj, KOBJ_CHANGE, pwm_prop);
+ kfree(pwm_prop[0]);
+
/* for device_find_child() */
put_device(child);
device_unregister(child);
--
1.9.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v2 2/2] pwm: send a uevent on the pwmchip device upon channel sysfs (un)export
@ 2018-10-01 13:23 ` Fabrice Gasnier
0 siblings, 0 replies; 27+ messages in thread
From: Fabrice Gasnier @ 2018-10-01 13:23 UTC (permalink / raw)
To: thierry.reding, gottfried.haider
Cc: stefan.wahren, linux-pwm, gohai, michal.vokac, loic.pallardy,
linux-kernel, hsweeten, broonie, linux-rpi-kernel,
fabrice.gasnier, linux-stm32, linux-arm-kernel
This patch sends a uevent (KOBJ_CHANGE) on the pwmchipN device, everytime
a pwmX channel has been exported/unexported via sysfs.
This allows udev to implement rules on such events, like:
SUBSYSTEM=="pwm*", PROGRAM="/bin/sh -c '\
chown -R root:gpio /sys/class/pwm && chmod -R 770 /sys/class/pwm;\
chown -R root:gpio
/sys/devices/platform/soc/*.pwm/pwm/pwmchip* && chmod -R 770
/sys/devices/platform/soc/*.pwm/pwm/pwmchip*\
'"
This is a replacement patch for commit 7e5d1fd75c3d ("pwm: Set class for
exported channels in sysfs"), see [1].
basic testing:
$ udevadm monitor --environment &
$ echo 0 > /sys/class/pwm/pwmchip0/export
KERNEL[197.321736] change /devices/.../pwm/pwmchip0 (pwm)
ACTION=change
DEVPATH=/devices/.../pwm/pwmchip0
EXPORT=pwm0
SEQNUM=2045
SUBSYSTEM=pwm
[1] https://lkml.org/lkml/2018/9/25/713
Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Tested-by: Gottfried Haider <gottfried.haider@gmail.com>
---
drivers/pwm/sysfs.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c
index 4726d43..ceb233d 100644
--- a/drivers/pwm/sysfs.c
+++ b/drivers/pwm/sysfs.c
@@ -249,6 +249,7 @@ static void pwm_export_release(struct device *child)
static int pwm_export_child(struct device *parent, struct pwm_device *pwm)
{
struct pwm_export *export;
+ char *pwm_prop[2];
int ret;
if (test_and_set_bit(PWMF_EXPORTED, &pwm->flags))
@@ -276,6 +277,10 @@ static int pwm_export_child(struct device *parent, struct pwm_device *pwm)
export = NULL;
return ret;
}
+ pwm_prop[0] = kasprintf(GFP_KERNEL, "EXPORT=pwm%u", pwm->hwpwm);
+ pwm_prop[1] = NULL;
+ kobject_uevent_env(&parent->kobj, KOBJ_CHANGE, pwm_prop);
+ kfree(pwm_prop[0]);
return 0;
}
@@ -288,6 +293,7 @@ static int pwm_unexport_match(struct device *child, void *data)
static int pwm_unexport_child(struct device *parent, struct pwm_device *pwm)
{
struct device *child;
+ char *pwm_prop[2];
if (!test_and_clear_bit(PWMF_EXPORTED, &pwm->flags))
return -ENODEV;
@@ -296,6 +302,11 @@ static int pwm_unexport_child(struct device *parent, struct pwm_device *pwm)
if (!child)
return -ENODEV;
+ pwm_prop[0] = kasprintf(GFP_KERNEL, "UNEXPORT=pwm%u", pwm->hwpwm);
+ pwm_prop[1] = NULL;
+ kobject_uevent_env(&parent->kobj, KOBJ_CHANGE, pwm_prop);
+ kfree(pwm_prop[0]);
+
/* for device_find_child() */
put_device(child);
device_unregister(child);
--
1.9.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v2 2/2] pwm: send a uevent on the pwmchip device upon channel sysfs (un)export
2018-10-01 13:23 ` Fabrice Gasnier
(?)
@ 2018-10-01 16:29 ` Michal Vokáč
-1 siblings, 0 replies; 27+ messages in thread
From: Michal Vokáč @ 2018-10-01 16:29 UTC (permalink / raw)
To: Fabrice Gasnier, thierry.reding, gottfried.haider
Cc: stefan.wahren, hsweeten, loic.pallardy, broonie, gohai,
linux-stm32, linux-arm-kernel, linux-rpi-kernel, linux-kernel,
linux-pwm
On 1.10.2018 15:23, Fabrice Gasnier wrote:
> This patch sends a uevent (KOBJ_CHANGE) on the pwmchipN device, everytime
> a pwmX channel has been exported/unexported via sysfs.
> This allows udev to implement rules on such events, like:
>
> SUBSYSTEM=="pwm*", PROGRAM="/bin/sh -c '\
> chown -R root:gpio /sys/class/pwm && chmod -R 770 /sys/class/pwm;\
> chown -R root:gpio
> /sys/devices/platform/soc/*.pwm/pwm/pwmchip* && chmod -R 770
> /sys/devices/platform/soc/*.pwm/pwm/pwmchip*\
> '"
>
> This is a replacement patch for commit 7e5d1fd75c3d ("pwm: Set class for
> exported channels in sysfs"), see [1].
>
> basic testing:
> $ udevadm monitor --environment &
> $ echo 0 > /sys/class/pwm/pwmchip0/export
> KERNEL[197.321736] change /devices/.../pwm/pwmchip0 (pwm)
> ACTION=change
> DEVPATH=/devices/.../pwm/pwmchip0
> EXPORT=pwm0
> SEQNUM=2045
> SUBSYSTEM=pwm
>
> [1] https://lkml.org/lkml/2018/9/25/713
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Tested-by: Gottfried Haider <gottfried.haider@gmail.com>
Tested-by: Michal Vokáč <michal.vokac@ysoft.com>
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH v2 2/2] pwm: send a uevent on the pwmchip device upon channel sysfs (un)export
@ 2018-10-01 16:29 ` Michal Vokáč
0 siblings, 0 replies; 27+ messages in thread
From: Michal Vokáč @ 2018-10-01 16:29 UTC (permalink / raw)
To: linux-arm-kernel
On 1.10.2018 15:23, Fabrice Gasnier wrote:
> This patch sends a uevent (KOBJ_CHANGE) on the pwmchipN device, everytime
> a pwmX channel has been exported/unexported via sysfs.
> This allows udev to implement rules on such events, like:
>
> SUBSYSTEM=="pwm*", PROGRAM="/bin/sh -c '\
> chown -R root:gpio /sys/class/pwm && chmod -R 770 /sys/class/pwm;\
> chown -R root:gpio
> /sys/devices/platform/soc/*.pwm/pwm/pwmchip* && chmod -R 770
> /sys/devices/platform/soc/*.pwm/pwm/pwmchip*\
> '"
>
> This is a replacement patch for commit 7e5d1fd75c3d ("pwm: Set class for
> exported channels in sysfs"), see [1].
>
> basic testing:
> $ udevadm monitor --environment &
> $ echo 0 > /sys/class/pwm/pwmchip0/export
> KERNEL[197.321736] change /devices/.../pwm/pwmchip0 (pwm)
> ACTION=change
> DEVPATH=/devices/.../pwm/pwmchip0
> EXPORT=pwm0
> SEQNUM=2045
> SUBSYSTEM=pwm
>
> [1] https://lkml.org/lkml/2018/9/25/713
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Tested-by: Gottfried Haider <gottfried.haider@gmail.com>
Tested-by: Michal Vok?? <michal.vokac@ysoft.com>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 2/2] pwm: send a uevent on the pwmchip device upon channel sysfs (un)export
@ 2018-10-01 16:29 ` Michal Vokáč
0 siblings, 0 replies; 27+ messages in thread
From: Michal Vokáč @ 2018-10-01 16:29 UTC (permalink / raw)
To: Fabrice Gasnier, thierry.reding, gottfried.haider
Cc: stefan.wahren, linux-pwm, loic.pallardy, linux-kernel, hsweeten,
broonie, linux-rpi-kernel, gohai, linux-stm32, linux-arm-kernel
On 1.10.2018 15:23, Fabrice Gasnier wrote:
> This patch sends a uevent (KOBJ_CHANGE) on the pwmchipN device, everytime
> a pwmX channel has been exported/unexported via sysfs.
> This allows udev to implement rules on such events, like:
>
> SUBSYSTEM=="pwm*", PROGRAM="/bin/sh -c '\
> chown -R root:gpio /sys/class/pwm && chmod -R 770 /sys/class/pwm;\
> chown -R root:gpio
> /sys/devices/platform/soc/*.pwm/pwm/pwmchip* && chmod -R 770
> /sys/devices/platform/soc/*.pwm/pwm/pwmchip*\
> '"
>
> This is a replacement patch for commit 7e5d1fd75c3d ("pwm: Set class for
> exported channels in sysfs"), see [1].
>
> basic testing:
> $ udevadm monitor --environment &
> $ echo 0 > /sys/class/pwm/pwmchip0/export
> KERNEL[197.321736] change /devices/.../pwm/pwmchip0 (pwm)
> ACTION=change
> DEVPATH=/devices/.../pwm/pwmchip0
> EXPORT=pwm0
> SEQNUM=2045
> SUBSYSTEM=pwm
>
> [1] https://lkml.org/lkml/2018/9/25/713
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Tested-by: Gottfried Haider <gottfried.haider@gmail.com>
Tested-by: Michal Vokáč <michal.vokac@ysoft.com>
_______________________________________________
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] 27+ messages in thread
* Re: [PATCH v2 0/2] pwm: sysfs: fix exporting PWM channel
2018-10-01 13:23 ` Fabrice Gasnier
@ 2018-10-01 16:24 ` Michal Vokáč
-1 siblings, 0 replies; 27+ messages in thread
From: Michal Vokáč @ 2018-10-01 16:24 UTC (permalink / raw)
To: Fabrice Gasnier, thierry.reding, gottfried.haider
Cc: stefan.wahren, hsweeten, loic.pallardy, broonie, gohai,
linux-stm32, linux-arm-kernel, linux-rpi-kernel, linux-kernel,
linux-pwm
On 1.10.2018 15:23, Fabrice Gasnier wrote:
> Since commit 7e5d1fd75c3d ("pwm: Set class for exported channels in sysfs")
> - it's not possible to export more than one PWM channel
> - ABI has changed, as a side effect. It may cause bad behavior as pwmchip
> attributes are wrongly added to pwm channels and report wrong values.
> See [1] and [2].
>
> One purpose of the original patch is to send uevents to udev, when exporting a
> PWM channel through the sysfs. This series:
> - Reverts the original patch.
> - Proposes a new way to send notifications to be used by udev rules.
>
> - With this series:
> $ echo 0 > /sys/class/pwm/pwmchip0/export
> $ ls /sys/class/pwm
> pwmchip0 pwmchip4
>
> $ ls /sys/class/pwm/pwmchip0/pwm0/
> capture enable polarity uevent
> duty_cycle period power
>
> - Without this series:
> $ echo 0 > /sys/class/pwm/pwmchip0/export
> $ ls /sys/class/pwm
> pwm0 pwmchip0 pwmchip4
>
> $ ls /sys/class/pwm/pwmchip0/pwm0/
> capture duty_cycle export period power uevent
> device enable npwm polarity subsystem unexport
>
> - Backtrace when exporting a 2nd channel (0) on a separate pwmchip device:
> $ echo 0 > /sys/class/pwm/pwmchip4/export
> [ 95.286558] sysfs: cannot create duplicate filename '/class/pwm/pwm0'
> [ 95.293630] CPU: 0 PID: 54 Comm: sh Not tainted 4.19.0-rc6-00013-g00b49b0 #151
> [ 95.301344] Hardware name: STM32 (Device Tree Support)
> [ 95.306833] [<0000c155>] (unwind_backtrace) from [<0000b273>] (show_stack+0xb/0xc)
> [ 95.315136] [<0000b273>] (show_stack) from [<00092455>] (sysfs_warn_dup+0x31/0x48)
> [ 95.323247] [<00092455>] (sysfs_warn_dup) from [<00092635>] (sysfs_do_create_link_sd+0x75/0x88)
> [ 95.332539] [<00092635>] (sysfs_do_create_link_sd) from [<00125823>] (device_add+0x133/0x3b0)
> [ 95.341694] [<00125823>] (device_add) from [<001059ed>] (export_store+0xb5/0x12c)
> [ 95.349761] [<001059ed>] (export_store) from [<00091911>] (kernfs_fop_write+0x87/0xda)
> [ 95.358150] [<00091911>] (kernfs_fop_write) from [<0005beb1>] (__vfs_write+0x1d/0xe0)
> [ 95.366295] [<0005beb1>] (__vfs_write) from [<0005bfe7>] (vfs_write+0x4f/0x7c)
> [ 95.374053] [<0005bfe7>] (vfs_write) from [<0005c0bf>] (ksys_write+0x33/0x70)
> [ 95.381708] [<0005c0bf>] (ksys_write) from [<00009001>] (ret_fast_syscall+0x1/0x58)
> [ 95.389682] Exception stack(0x01bcffa8 to 0x01bcfff0)
> [ 95.394946] ffa0: 00000000 00c4883c 00000001 00c4e590 00000002 00000004
> [ 95.403639] ffc0: 00000000 00c4883c 00c4cbe8 00000004 00000002 00000020 00000000 00c4d008
> [ 95.412223] ffe0: 00c29151 00c4cbe8 00c17833 00c13c0c
> -sh: write error: File exists
>
> [1] https://lkml.org/lkml/2018/9/25/713
> [2] https://lkml.org/lkml/2018/9/25/447
The [2] report came from me. I tested both patches on my i.MX6 boards and
it works just fine. Thanks for the fix Fabrice!
Michal
>
> ---
> Changes in v2:
> - update revert commit message
> - new patch 2/2 to propose uevent notification (change) on pwmchip
>
> Fabrice Gasnier (2):
> Revert "pwm: Set class for exported channels in sysfs"
> pwm: send a uevent on the pwmchip device upon channel sysfs (un)export
>
> drivers/pwm/sysfs.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH v2 0/2] pwm: sysfs: fix exporting PWM channel
@ 2018-10-01 16:24 ` Michal Vokáč
0 siblings, 0 replies; 27+ messages in thread
From: Michal Vokáč @ 2018-10-01 16:24 UTC (permalink / raw)
To: linux-arm-kernel
On 1.10.2018 15:23, Fabrice Gasnier wrote:
> Since commit 7e5d1fd75c3d ("pwm: Set class for exported channels in sysfs")
> - it's not possible to export more than one PWM channel
> - ABI has changed, as a side effect. It may cause bad behavior as pwmchip
> attributes are wrongly added to pwm channels and report wrong values.
> See [1] and [2].
>
> One purpose of the original patch is to send uevents to udev, when exporting a
> PWM channel through the sysfs. This series:
> - Reverts the original patch.
> - Proposes a new way to send notifications to be used by udev rules.
>
> - With this series:
> $ echo 0 > /sys/class/pwm/pwmchip0/export
> $ ls /sys/class/pwm
> pwmchip0 pwmchip4
>
> $ ls /sys/class/pwm/pwmchip0/pwm0/
> capture enable polarity uevent
> duty_cycle period power
>
> - Without this series:
> $ echo 0 > /sys/class/pwm/pwmchip0/export
> $ ls /sys/class/pwm
> pwm0 pwmchip0 pwmchip4
>
> $ ls /sys/class/pwm/pwmchip0/pwm0/
> capture duty_cycle export period power uevent
> device enable npwm polarity subsystem unexport
>
> - Backtrace when exporting a 2nd channel (0) on a separate pwmchip device:
> $ echo 0 > /sys/class/pwm/pwmchip4/export
> [ 95.286558] sysfs: cannot create duplicate filename '/class/pwm/pwm0'
> [ 95.293630] CPU: 0 PID: 54 Comm: sh Not tainted 4.19.0-rc6-00013-g00b49b0 #151
> [ 95.301344] Hardware name: STM32 (Device Tree Support)
> [ 95.306833] [<0000c155>] (unwind_backtrace) from [<0000b273>] (show_stack+0xb/0xc)
> [ 95.315136] [<0000b273>] (show_stack) from [<00092455>] (sysfs_warn_dup+0x31/0x48)
> [ 95.323247] [<00092455>] (sysfs_warn_dup) from [<00092635>] (sysfs_do_create_link_sd+0x75/0x88)
> [ 95.332539] [<00092635>] (sysfs_do_create_link_sd) from [<00125823>] (device_add+0x133/0x3b0)
> [ 95.341694] [<00125823>] (device_add) from [<001059ed>] (export_store+0xb5/0x12c)
> [ 95.349761] [<001059ed>] (export_store) from [<00091911>] (kernfs_fop_write+0x87/0xda)
> [ 95.358150] [<00091911>] (kernfs_fop_write) from [<0005beb1>] (__vfs_write+0x1d/0xe0)
> [ 95.366295] [<0005beb1>] (__vfs_write) from [<0005bfe7>] (vfs_write+0x4f/0x7c)
> [ 95.374053] [<0005bfe7>] (vfs_write) from [<0005c0bf>] (ksys_write+0x33/0x70)
> [ 95.381708] [<0005c0bf>] (ksys_write) from [<00009001>] (ret_fast_syscall+0x1/0x58)
> [ 95.389682] Exception stack(0x01bcffa8 to 0x01bcfff0)
> [ 95.394946] ffa0: 00000000 00c4883c 00000001 00c4e590 00000002 00000004
> [ 95.403639] ffc0: 00000000 00c4883c 00c4cbe8 00000004 00000002 00000020 00000000 00c4d008
> [ 95.412223] ffe0: 00c29151 00c4cbe8 00c17833 00c13c0c
> -sh: write error: File exists
>
> [1] https://lkml.org/lkml/2018/9/25/713
> [2] https://lkml.org/lkml/2018/9/25/447
The [2] report came from me. I tested both patches on my i.MX6 boards and
it works just fine. Thanks for the fix Fabrice!
Michal
>
> ---
> Changes in v2:
> - update revert commit message
> - new patch 2/2 to propose uevent notification (change) on pwmchip
>
> Fabrice Gasnier (2):
> Revert "pwm: Set class for exported channels in sysfs"
> pwm: send a uevent on the pwmchip device upon channel sysfs (un)export
>
> drivers/pwm/sysfs.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 0/2] pwm: sysfs: fix exporting PWM channel
2018-10-01 13:23 ` Fabrice Gasnier
@ 2018-10-12 11:55 ` Thierry Reding
-1 siblings, 0 replies; 27+ messages in thread
From: Thierry Reding @ 2018-10-12 11:55 UTC (permalink / raw)
To: Fabrice Gasnier
Cc: gottfried.haider, stefan.wahren, hsweeten, loic.pallardy,
broonie, gohai, michal.vokac, linux-stm32, linux-arm-kernel,
linux-rpi-kernel, linux-kernel, linux-pwm
[-- Attachment #1: Type: text/plain, Size: 3710 bytes --]
On Mon, Oct 01, 2018 at 03:23:55PM +0200, Fabrice Gasnier wrote:
> Since commit 7e5d1fd75c3d ("pwm: Set class for exported channels in sysfs")
> - it's not possible to export more than one PWM channel
> - ABI has changed, as a side effect. It may cause bad behavior as pwmchip
> attributes are wrongly added to pwm channels and report wrong values.
> See [1] and [2].
>
> One purpose of the original patch is to send uevents to udev, when exporting a
> PWM channel through the sysfs. This series:
> - Reverts the original patch.
> - Proposes a new way to send notifications to be used by udev rules.
>
> - With this series:
> $ echo 0 > /sys/class/pwm/pwmchip0/export
> $ ls /sys/class/pwm
> pwmchip0 pwmchip4
>
> $ ls /sys/class/pwm/pwmchip0/pwm0/
> capture enable polarity uevent
> duty_cycle period power
>
> - Without this series:
> $ echo 0 > /sys/class/pwm/pwmchip0/export
> $ ls /sys/class/pwm
> pwm0 pwmchip0 pwmchip4
>
> $ ls /sys/class/pwm/pwmchip0/pwm0/
> capture duty_cycle export period power uevent
> device enable npwm polarity subsystem unexport
>
> - Backtrace when exporting a 2nd channel (0) on a separate pwmchip device:
> $ echo 0 > /sys/class/pwm/pwmchip4/export
> [ 95.286558] sysfs: cannot create duplicate filename '/class/pwm/pwm0'
> [ 95.293630] CPU: 0 PID: 54 Comm: sh Not tainted 4.19.0-rc6-00013-g00b49b0 #151
> [ 95.301344] Hardware name: STM32 (Device Tree Support)
> [ 95.306833] [<0000c155>] (unwind_backtrace) from [<0000b273>] (show_stack+0xb/0xc)
> [ 95.315136] [<0000b273>] (show_stack) from [<00092455>] (sysfs_warn_dup+0x31/0x48)
> [ 95.323247] [<00092455>] (sysfs_warn_dup) from [<00092635>] (sysfs_do_create_link_sd+0x75/0x88)
> [ 95.332539] [<00092635>] (sysfs_do_create_link_sd) from [<00125823>] (device_add+0x133/0x3b0)
> [ 95.341694] [<00125823>] (device_add) from [<001059ed>] (export_store+0xb5/0x12c)
> [ 95.349761] [<001059ed>] (export_store) from [<00091911>] (kernfs_fop_write+0x87/0xda)
> [ 95.358150] [<00091911>] (kernfs_fop_write) from [<0005beb1>] (__vfs_write+0x1d/0xe0)
> [ 95.366295] [<0005beb1>] (__vfs_write) from [<0005bfe7>] (vfs_write+0x4f/0x7c)
> [ 95.374053] [<0005bfe7>] (vfs_write) from [<0005c0bf>] (ksys_write+0x33/0x70)
> [ 95.381708] [<0005c0bf>] (ksys_write) from [<00009001>] (ret_fast_syscall+0x1/0x58)
> [ 95.389682] Exception stack(0x01bcffa8 to 0x01bcfff0)
> [ 95.394946] ffa0: 00000000 00c4883c 00000001 00c4e590 00000002 00000004
> [ 95.403639] ffc0: 00000000 00c4883c 00c4cbe8 00000004 00000002 00000020 00000000 00c4d008
> [ 95.412223] ffe0: 00c29151 00c4cbe8 00c17833 00c13c0c
> -sh: write error: File exists
>
> [1] https://lkml.org/lkml/2018/9/25/713
> [2] https://lkml.org/lkml/2018/9/25/447
>
> ---
> Changes in v2:
> - update revert commit message
> - new patch 2/2 to propose uevent notification (change) on pwmchip
>
> Fabrice Gasnier (2):
> Revert "pwm: Set class for exported channels in sysfs"
> pwm: send a uevent on the pwmchip device upon channel sysfs (un)export
>
> drivers/pwm/sysfs.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
Both patches applied, thanks. What do you think would be the importance
of getting this into stable kernels? We can't get one of the patches in
without the other, so they'd both have to be backported. The second one
is still fairly small, so would qualify for stable, I think.
However, given that it's taken a long time for somebody to notice this,
I'm not sure if this is something that people care about too much in the
stable kernels.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH v2 0/2] pwm: sysfs: fix exporting PWM channel
@ 2018-10-12 11:55 ` Thierry Reding
0 siblings, 0 replies; 27+ messages in thread
From: Thierry Reding @ 2018-10-12 11:55 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Oct 01, 2018 at 03:23:55PM +0200, Fabrice Gasnier wrote:
> Since commit 7e5d1fd75c3d ("pwm: Set class for exported channels in sysfs")
> - it's not possible to export more than one PWM channel
> - ABI has changed, as a side effect. It may cause bad behavior as pwmchip
> attributes are wrongly added to pwm channels and report wrong values.
> See [1] and [2].
>
> One purpose of the original patch is to send uevents to udev, when exporting a
> PWM channel through the sysfs. This series:
> - Reverts the original patch.
> - Proposes a new way to send notifications to be used by udev rules.
>
> - With this series:
> $ echo 0 > /sys/class/pwm/pwmchip0/export
> $ ls /sys/class/pwm
> pwmchip0 pwmchip4
>
> $ ls /sys/class/pwm/pwmchip0/pwm0/
> capture enable polarity uevent
> duty_cycle period power
>
> - Without this series:
> $ echo 0 > /sys/class/pwm/pwmchip0/export
> $ ls /sys/class/pwm
> pwm0 pwmchip0 pwmchip4
>
> $ ls /sys/class/pwm/pwmchip0/pwm0/
> capture duty_cycle export period power uevent
> device enable npwm polarity subsystem unexport
>
> - Backtrace when exporting a 2nd channel (0) on a separate pwmchip device:
> $ echo 0 > /sys/class/pwm/pwmchip4/export
> [ 95.286558] sysfs: cannot create duplicate filename '/class/pwm/pwm0'
> [ 95.293630] CPU: 0 PID: 54 Comm: sh Not tainted 4.19.0-rc6-00013-g00b49b0 #151
> [ 95.301344] Hardware name: STM32 (Device Tree Support)
> [ 95.306833] [<0000c155>] (unwind_backtrace) from [<0000b273>] (show_stack+0xb/0xc)
> [ 95.315136] [<0000b273>] (show_stack) from [<00092455>] (sysfs_warn_dup+0x31/0x48)
> [ 95.323247] [<00092455>] (sysfs_warn_dup) from [<00092635>] (sysfs_do_create_link_sd+0x75/0x88)
> [ 95.332539] [<00092635>] (sysfs_do_create_link_sd) from [<00125823>] (device_add+0x133/0x3b0)
> [ 95.341694] [<00125823>] (device_add) from [<001059ed>] (export_store+0xb5/0x12c)
> [ 95.349761] [<001059ed>] (export_store) from [<00091911>] (kernfs_fop_write+0x87/0xda)
> [ 95.358150] [<00091911>] (kernfs_fop_write) from [<0005beb1>] (__vfs_write+0x1d/0xe0)
> [ 95.366295] [<0005beb1>] (__vfs_write) from [<0005bfe7>] (vfs_write+0x4f/0x7c)
> [ 95.374053] [<0005bfe7>] (vfs_write) from [<0005c0bf>] (ksys_write+0x33/0x70)
> [ 95.381708] [<0005c0bf>] (ksys_write) from [<00009001>] (ret_fast_syscall+0x1/0x58)
> [ 95.389682] Exception stack(0x01bcffa8 to 0x01bcfff0)
> [ 95.394946] ffa0: 00000000 00c4883c 00000001 00c4e590 00000002 00000004
> [ 95.403639] ffc0: 00000000 00c4883c 00c4cbe8 00000004 00000002 00000020 00000000 00c4d008
> [ 95.412223] ffe0: 00c29151 00c4cbe8 00c17833 00c13c0c
> -sh: write error: File exists
>
> [1] https://lkml.org/lkml/2018/9/25/713
> [2] https://lkml.org/lkml/2018/9/25/447
>
> ---
> Changes in v2:
> - update revert commit message
> - new patch 2/2 to propose uevent notification (change) on pwmchip
>
> Fabrice Gasnier (2):
> Revert "pwm: Set class for exported channels in sysfs"
> pwm: send a uevent on the pwmchip device upon channel sysfs (un)export
>
> drivers/pwm/sysfs.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
Both patches applied, thanks. What do you think would be the importance
of getting this into stable kernels? We can't get one of the patches in
without the other, so they'd both have to be backported. The second one
is still fairly small, so would qualify for stable, I think.
However, given that it's taken a long time for somebody to notice this,
I'm not sure if this is something that people care about too much in the
stable kernels.
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/20181012/717dc4bc/attachment-0001.sig>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 0/2] pwm: sysfs: fix exporting PWM channel
2018-10-12 11:55 ` Thierry Reding
(?)
@ 2018-10-12 12:15 ` Stefan Wahren
-1 siblings, 0 replies; 27+ messages in thread
From: Stefan Wahren @ 2018-10-12 12:15 UTC (permalink / raw)
To: Thierry Reding, Fabrice Gasnier
Cc: gottfried.haider, michal.vokac, loic.pallardy, linux-pwm,
linux-kernel, hsweeten, broonie, linux-rpi-kernel, gohai,
linux-stm32, linux-arm-kernel
Am 12.10.2018 um 13:55 schrieb Thierry Reding:
> On Mon, Oct 01, 2018 at 03:23:55PM +0200, Fabrice Gasnier wrote:
>> Since commit 7e5d1fd75c3d ("pwm: Set class for exported channels in sysfs")
>> - it's not possible to export more than one PWM channel
>> - ABI has changed, as a side effect. It may cause bad behavior as pwmchip
>> attributes are wrongly added to pwm channels and report wrong values.
>> See [1] and [2].
>>
>> One purpose of the original patch is to send uevents to udev, when exporting a
>> PWM channel through the sysfs. This series:
>> - Reverts the original patch.
>> - Proposes a new way to send notifications to be used by udev rules.
>>
>> - With this series:
>> $ echo 0 > /sys/class/pwm/pwmchip0/export
>> $ ls /sys/class/pwm
>> pwmchip0 pwmchip4
>>
>> $ ls /sys/class/pwm/pwmchip0/pwm0/
>> capture enable polarity uevent
>> duty_cycle period power
>>
>> - Without this series:
>> $ echo 0 > /sys/class/pwm/pwmchip0/export
>> $ ls /sys/class/pwm
>> pwm0 pwmchip0 pwmchip4
>>
>> $ ls /sys/class/pwm/pwmchip0/pwm0/
>> capture duty_cycle export period power uevent
>> device enable npwm polarity subsystem unexport
>>
>> - Backtrace when exporting a 2nd channel (0) on a separate pwmchip device:
>> $ echo 0 > /sys/class/pwm/pwmchip4/export
>> [ 95.286558] sysfs: cannot create duplicate filename '/class/pwm/pwm0'
>> [ 95.293630] CPU: 0 PID: 54 Comm: sh Not tainted 4.19.0-rc6-00013-g00b49b0 #151
>> [ 95.301344] Hardware name: STM32 (Device Tree Support)
>> [ 95.306833] [<0000c155>] (unwind_backtrace) from [<0000b273>] (show_stack+0xb/0xc)
>> [ 95.315136] [<0000b273>] (show_stack) from [<00092455>] (sysfs_warn_dup+0x31/0x48)
>> [ 95.323247] [<00092455>] (sysfs_warn_dup) from [<00092635>] (sysfs_do_create_link_sd+0x75/0x88)
>> [ 95.332539] [<00092635>] (sysfs_do_create_link_sd) from [<00125823>] (device_add+0x133/0x3b0)
>> [ 95.341694] [<00125823>] (device_add) from [<001059ed>] (export_store+0xb5/0x12c)
>> [ 95.349761] [<001059ed>] (export_store) from [<00091911>] (kernfs_fop_write+0x87/0xda)
>> [ 95.358150] [<00091911>] (kernfs_fop_write) from [<0005beb1>] (__vfs_write+0x1d/0xe0)
>> [ 95.366295] [<0005beb1>] (__vfs_write) from [<0005bfe7>] (vfs_write+0x4f/0x7c)
>> [ 95.374053] [<0005bfe7>] (vfs_write) from [<0005c0bf>] (ksys_write+0x33/0x70)
>> [ 95.381708] [<0005c0bf>] (ksys_write) from [<00009001>] (ret_fast_syscall+0x1/0x58)
>> [ 95.389682] Exception stack(0x01bcffa8 to 0x01bcfff0)
>> [ 95.394946] ffa0: 00000000 00c4883c 00000001 00c4e590 00000002 00000004
>> [ 95.403639] ffc0: 00000000 00c4883c 00c4cbe8 00000004 00000002 00000020 00000000 00c4d008
>> [ 95.412223] ffe0: 00c29151 00c4cbe8 00c17833 00c13c0c
>> -sh: write error: File exists
>>
>> [1] https://lkml.org/lkml/2018/9/25/713
>> [2] https://lkml.org/lkml/2018/9/25/447
>>
>> ---
>> Changes in v2:
>> - update revert commit message
>> - new patch 2/2 to propose uevent notification (change) on pwmchip
>>
>> Fabrice Gasnier (2):
>> Revert "pwm: Set class for exported channels in sysfs"
>> pwm: send a uevent on the pwmchip device upon channel sysfs (un)export
>>
>> drivers/pwm/sysfs.c | 12 +++++++++++-
>> 1 file changed, 11 insertions(+), 1 deletion(-)
> Both patches applied, thanks. What do you think would be the importance
> of getting this into stable kernels? We can't get one of the patches in
> without the other, so they'd both have to be backported. The second one
> is still fairly small, so would qualify for stable, I think.
I think the revert patch should go to stable, because it fixes a regression.
Thanks
>
> However, given that it's taken a long time for somebody to notice this,
> I'm not sure if this is something that people care about too much in the
> stable kernels.
>
> Thierry
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH v2 0/2] pwm: sysfs: fix exporting PWM channel
@ 2018-10-12 12:15 ` Stefan Wahren
0 siblings, 0 replies; 27+ messages in thread
From: Stefan Wahren @ 2018-10-12 12:15 UTC (permalink / raw)
To: linux-arm-kernel
Am 12.10.2018 um 13:55 schrieb Thierry Reding:
> On Mon, Oct 01, 2018 at 03:23:55PM +0200, Fabrice Gasnier wrote:
>> Since commit 7e5d1fd75c3d ("pwm: Set class for exported channels in sysfs")
>> - it's not possible to export more than one PWM channel
>> - ABI has changed, as a side effect. It may cause bad behavior as pwmchip
>> attributes are wrongly added to pwm channels and report wrong values.
>> See [1] and [2].
>>
>> One purpose of the original patch is to send uevents to udev, when exporting a
>> PWM channel through the sysfs. This series:
>> - Reverts the original patch.
>> - Proposes a new way to send notifications to be used by udev rules.
>>
>> - With this series:
>> $ echo 0 > /sys/class/pwm/pwmchip0/export
>> $ ls /sys/class/pwm
>> pwmchip0 pwmchip4
>>
>> $ ls /sys/class/pwm/pwmchip0/pwm0/
>> capture enable polarity uevent
>> duty_cycle period power
>>
>> - Without this series:
>> $ echo 0 > /sys/class/pwm/pwmchip0/export
>> $ ls /sys/class/pwm
>> pwm0 pwmchip0 pwmchip4
>>
>> $ ls /sys/class/pwm/pwmchip0/pwm0/
>> capture duty_cycle export period power uevent
>> device enable npwm polarity subsystem unexport
>>
>> - Backtrace when exporting a 2nd channel (0) on a separate pwmchip device:
>> $ echo 0 > /sys/class/pwm/pwmchip4/export
>> [ 95.286558] sysfs: cannot create duplicate filename '/class/pwm/pwm0'
>> [ 95.293630] CPU: 0 PID: 54 Comm: sh Not tainted 4.19.0-rc6-00013-g00b49b0 #151
>> [ 95.301344] Hardware name: STM32 (Device Tree Support)
>> [ 95.306833] [<0000c155>] (unwind_backtrace) from [<0000b273>] (show_stack+0xb/0xc)
>> [ 95.315136] [<0000b273>] (show_stack) from [<00092455>] (sysfs_warn_dup+0x31/0x48)
>> [ 95.323247] [<00092455>] (sysfs_warn_dup) from [<00092635>] (sysfs_do_create_link_sd+0x75/0x88)
>> [ 95.332539] [<00092635>] (sysfs_do_create_link_sd) from [<00125823>] (device_add+0x133/0x3b0)
>> [ 95.341694] [<00125823>] (device_add) from [<001059ed>] (export_store+0xb5/0x12c)
>> [ 95.349761] [<001059ed>] (export_store) from [<00091911>] (kernfs_fop_write+0x87/0xda)
>> [ 95.358150] [<00091911>] (kernfs_fop_write) from [<0005beb1>] (__vfs_write+0x1d/0xe0)
>> [ 95.366295] [<0005beb1>] (__vfs_write) from [<0005bfe7>] (vfs_write+0x4f/0x7c)
>> [ 95.374053] [<0005bfe7>] (vfs_write) from [<0005c0bf>] (ksys_write+0x33/0x70)
>> [ 95.381708] [<0005c0bf>] (ksys_write) from [<00009001>] (ret_fast_syscall+0x1/0x58)
>> [ 95.389682] Exception stack(0x01bcffa8 to 0x01bcfff0)
>> [ 95.394946] ffa0: 00000000 00c4883c 00000001 00c4e590 00000002 00000004
>> [ 95.403639] ffc0: 00000000 00c4883c 00c4cbe8 00000004 00000002 00000020 00000000 00c4d008
>> [ 95.412223] ffe0: 00c29151 00c4cbe8 00c17833 00c13c0c
>> -sh: write error: File exists
>>
>> [1] https://lkml.org/lkml/2018/9/25/713
>> [2] https://lkml.org/lkml/2018/9/25/447
>>
>> ---
>> Changes in v2:
>> - update revert commit message
>> - new patch 2/2 to propose uevent notification (change) on pwmchip
>>
>> Fabrice Gasnier (2):
>> Revert "pwm: Set class for exported channels in sysfs"
>> pwm: send a uevent on the pwmchip device upon channel sysfs (un)export
>>
>> drivers/pwm/sysfs.c | 12 +++++++++++-
>> 1 file changed, 11 insertions(+), 1 deletion(-)
> Both patches applied, thanks. What do you think would be the importance
> of getting this into stable kernels? We can't get one of the patches in
> without the other, so they'd both have to be backported. The second one
> is still fairly small, so would qualify for stable, I think.
I think the revert patch should go to stable, because it fixes a regression.
Thanks
>
> However, given that it's taken a long time for somebody to notice this,
> I'm not sure if this is something that people care about too much in the
> stable kernels.
>
> Thierry
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 0/2] pwm: sysfs: fix exporting PWM channel
@ 2018-10-12 12:15 ` Stefan Wahren
0 siblings, 0 replies; 27+ messages in thread
From: Stefan Wahren @ 2018-10-12 12:15 UTC (permalink / raw)
To: Thierry Reding, Fabrice Gasnier
Cc: gottfried.haider, linux-pwm, michal.vokac, loic.pallardy,
linux-kernel, hsweeten, broonie, linux-rpi-kernel, gohai,
linux-stm32, linux-arm-kernel
Am 12.10.2018 um 13:55 schrieb Thierry Reding:
> On Mon, Oct 01, 2018 at 03:23:55PM +0200, Fabrice Gasnier wrote:
>> Since commit 7e5d1fd75c3d ("pwm: Set class for exported channels in sysfs")
>> - it's not possible to export more than one PWM channel
>> - ABI has changed, as a side effect. It may cause bad behavior as pwmchip
>> attributes are wrongly added to pwm channels and report wrong values.
>> See [1] and [2].
>>
>> One purpose of the original patch is to send uevents to udev, when exporting a
>> PWM channel through the sysfs. This series:
>> - Reverts the original patch.
>> - Proposes a new way to send notifications to be used by udev rules.
>>
>> - With this series:
>> $ echo 0 > /sys/class/pwm/pwmchip0/export
>> $ ls /sys/class/pwm
>> pwmchip0 pwmchip4
>>
>> $ ls /sys/class/pwm/pwmchip0/pwm0/
>> capture enable polarity uevent
>> duty_cycle period power
>>
>> - Without this series:
>> $ echo 0 > /sys/class/pwm/pwmchip0/export
>> $ ls /sys/class/pwm
>> pwm0 pwmchip0 pwmchip4
>>
>> $ ls /sys/class/pwm/pwmchip0/pwm0/
>> capture duty_cycle export period power uevent
>> device enable npwm polarity subsystem unexport
>>
>> - Backtrace when exporting a 2nd channel (0) on a separate pwmchip device:
>> $ echo 0 > /sys/class/pwm/pwmchip4/export
>> [ 95.286558] sysfs: cannot create duplicate filename '/class/pwm/pwm0'
>> [ 95.293630] CPU: 0 PID: 54 Comm: sh Not tainted 4.19.0-rc6-00013-g00b49b0 #151
>> [ 95.301344] Hardware name: STM32 (Device Tree Support)
>> [ 95.306833] [<0000c155>] (unwind_backtrace) from [<0000b273>] (show_stack+0xb/0xc)
>> [ 95.315136] [<0000b273>] (show_stack) from [<00092455>] (sysfs_warn_dup+0x31/0x48)
>> [ 95.323247] [<00092455>] (sysfs_warn_dup) from [<00092635>] (sysfs_do_create_link_sd+0x75/0x88)
>> [ 95.332539] [<00092635>] (sysfs_do_create_link_sd) from [<00125823>] (device_add+0x133/0x3b0)
>> [ 95.341694] [<00125823>] (device_add) from [<001059ed>] (export_store+0xb5/0x12c)
>> [ 95.349761] [<001059ed>] (export_store) from [<00091911>] (kernfs_fop_write+0x87/0xda)
>> [ 95.358150] [<00091911>] (kernfs_fop_write) from [<0005beb1>] (__vfs_write+0x1d/0xe0)
>> [ 95.366295] [<0005beb1>] (__vfs_write) from [<0005bfe7>] (vfs_write+0x4f/0x7c)
>> [ 95.374053] [<0005bfe7>] (vfs_write) from [<0005c0bf>] (ksys_write+0x33/0x70)
>> [ 95.381708] [<0005c0bf>] (ksys_write) from [<00009001>] (ret_fast_syscall+0x1/0x58)
>> [ 95.389682] Exception stack(0x01bcffa8 to 0x01bcfff0)
>> [ 95.394946] ffa0: 00000000 00c4883c 00000001 00c4e590 00000002 00000004
>> [ 95.403639] ffc0: 00000000 00c4883c 00c4cbe8 00000004 00000002 00000020 00000000 00c4d008
>> [ 95.412223] ffe0: 00c29151 00c4cbe8 00c17833 00c13c0c
>> -sh: write error: File exists
>>
>> [1] https://lkml.org/lkml/2018/9/25/713
>> [2] https://lkml.org/lkml/2018/9/25/447
>>
>> ---
>> Changes in v2:
>> - update revert commit message
>> - new patch 2/2 to propose uevent notification (change) on pwmchip
>>
>> Fabrice Gasnier (2):
>> Revert "pwm: Set class for exported channels in sysfs"
>> pwm: send a uevent on the pwmchip device upon channel sysfs (un)export
>>
>> drivers/pwm/sysfs.c | 12 +++++++++++-
>> 1 file changed, 11 insertions(+), 1 deletion(-)
> Both patches applied, thanks. What do you think would be the importance
> of getting this into stable kernels? We can't get one of the patches in
> without the other, so they'd both have to be backported. The second one
> is still fairly small, so would qualify for stable, I think.
I think the revert patch should go to stable, because it fixes a regression.
Thanks
>
> However, given that it's taken a long time for somebody to notice this,
> I'm not sure if this is something that people care about too much in the
> stable kernels.
>
> Thierry
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 0/2] pwm: sysfs: fix exporting PWM channel
2018-10-12 12:15 ` Stefan Wahren
(?)
@ 2018-10-12 12:36 ` Fabrice Gasnier
-1 siblings, 0 replies; 27+ messages in thread
From: Fabrice Gasnier @ 2018-10-12 12:36 UTC (permalink / raw)
To: Stefan Wahren, Thierry Reding
Cc: gottfried.haider, michal.vokac, loic.pallardy, linux-pwm,
linux-kernel, hsweeten, broonie, linux-rpi-kernel, gohai,
linux-stm32, linux-arm-kernel
On 10/12/2018 02:15 PM, Stefan Wahren wrote:
> Am 12.10.2018 um 13:55 schrieb Thierry Reding:
>> On Mon, Oct 01, 2018 at 03:23:55PM +0200, Fabrice Gasnier wrote:
>>> Since commit 7e5d1fd75c3d ("pwm: Set class for exported channels in sysfs")
>>> - it's not possible to export more than one PWM channel
>>> - ABI has changed, as a side effect. It may cause bad behavior as pwmchip
>>> attributes are wrongly added to pwm channels and report wrong values.
>>> See [1] and [2].
>>>
>>> One purpose of the original patch is to send uevents to udev, when exporting a
>>> PWM channel through the sysfs. This series:
>>> - Reverts the original patch.
>>> - Proposes a new way to send notifications to be used by udev rules.
>>>
>>> - With this series:
>>> $ echo 0 > /sys/class/pwm/pwmchip0/export
>>> $ ls /sys/class/pwm
>>> pwmchip0 pwmchip4
>>>
>>> $ ls /sys/class/pwm/pwmchip0/pwm0/
>>> capture enable polarity uevent
>>> duty_cycle period power
>>>
>>> - Without this series:
>>> $ echo 0 > /sys/class/pwm/pwmchip0/export
>>> $ ls /sys/class/pwm
>>> pwm0 pwmchip0 pwmchip4
>>>
>>> $ ls /sys/class/pwm/pwmchip0/pwm0/
>>> capture duty_cycle export period power uevent
>>> device enable npwm polarity subsystem unexport
>>>
>>> - Backtrace when exporting a 2nd channel (0) on a separate pwmchip device:
>>> $ echo 0 > /sys/class/pwm/pwmchip4/export
>>> [ 95.286558] sysfs: cannot create duplicate filename '/class/pwm/pwm0'
>>> [ 95.293630] CPU: 0 PID: 54 Comm: sh Not tainted 4.19.0-rc6-00013-g00b49b0 #151
>>> [ 95.301344] Hardware name: STM32 (Device Tree Support)
>>> [ 95.306833] [<0000c155>] (unwind_backtrace) from [<0000b273>] (show_stack+0xb/0xc)
>>> [ 95.315136] [<0000b273>] (show_stack) from [<00092455>] (sysfs_warn_dup+0x31/0x48)
>>> [ 95.323247] [<00092455>] (sysfs_warn_dup) from [<00092635>] (sysfs_do_create_link_sd+0x75/0x88)
>>> [ 95.332539] [<00092635>] (sysfs_do_create_link_sd) from [<00125823>] (device_add+0x133/0x3b0)
>>> [ 95.341694] [<00125823>] (device_add) from [<001059ed>] (export_store+0xb5/0x12c)
>>> [ 95.349761] [<001059ed>] (export_store) from [<00091911>] (kernfs_fop_write+0x87/0xda)
>>> [ 95.358150] [<00091911>] (kernfs_fop_write) from [<0005beb1>] (__vfs_write+0x1d/0xe0)
>>> [ 95.366295] [<0005beb1>] (__vfs_write) from [<0005bfe7>] (vfs_write+0x4f/0x7c)
>>> [ 95.374053] [<0005bfe7>] (vfs_write) from [<0005c0bf>] (ksys_write+0x33/0x70)
>>> [ 95.381708] [<0005c0bf>] (ksys_write) from [<00009001>] (ret_fast_syscall+0x1/0x58)
>>> [ 95.389682] Exception stack(0x01bcffa8 to 0x01bcfff0)
>>> [ 95.394946] ffa0: 00000000 00c4883c 00000001 00c4e590 00000002 00000004
>>> [ 95.403639] ffc0: 00000000 00c4883c 00c4cbe8 00000004 00000002 00000020 00000000 00c4d008
>>> [ 95.412223] ffe0: 00c29151 00c4cbe8 00c17833 00c13c0c
>>> -sh: write error: File exists
>>>
>>> [1] https://lkml.org/lkml/2018/9/25/713
>>> [2] https://lkml.org/lkml/2018/9/25/447
>>>
>>> ---
>>> Changes in v2:
>>> - update revert commit message
>>> - new patch 2/2 to propose uevent notification (change) on pwmchip
>>>
>>> Fabrice Gasnier (2):
>>> Revert "pwm: Set class for exported channels in sysfs"
>>> pwm: send a uevent on the pwmchip device upon channel sysfs (un)export
>>>
>>> drivers/pwm/sysfs.c | 12 +++++++++++-
>>> 1 file changed, 11 insertions(+), 1 deletion(-)
>> Both patches applied, thanks. What do you think would be the importance
>> of getting this into stable kernels? We can't get one of the patches in
>> without the other, so they'd both have to be backported. The second one
>> is still fairly small, so would qualify for stable, I think.
> I think the revert patch should go to stable, because it fixes a regression.
>
Hi,
Thierry, Thanks for taking these.
I also think at least the 1st patch (revert) should be backported in
stable branch. Not taking the second one may lead to another issue for
the users that now expect uevents. This is replacement patch to the
original one. So, I'd advise to push both: revert + replacement patch.
Fabrice
> Thanks
>>
>> However, given that it's taken a long time for somebody to notice this,
>> I'm not sure if this is something that people care about too much in the
>> stable kernels>>
>> Thierry
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH v2 0/2] pwm: sysfs: fix exporting PWM channel
@ 2018-10-12 12:36 ` Fabrice Gasnier
0 siblings, 0 replies; 27+ messages in thread
From: Fabrice Gasnier @ 2018-10-12 12:36 UTC (permalink / raw)
To: linux-arm-kernel
On 10/12/2018 02:15 PM, Stefan Wahren wrote:
> Am 12.10.2018 um 13:55 schrieb Thierry Reding:
>> On Mon, Oct 01, 2018 at 03:23:55PM +0200, Fabrice Gasnier wrote:
>>> Since commit 7e5d1fd75c3d ("pwm: Set class for exported channels in sysfs")
>>> - it's not possible to export more than one PWM channel
>>> - ABI has changed, as a side effect. It may cause bad behavior as pwmchip
>>> attributes are wrongly added to pwm channels and report wrong values.
>>> See [1] and [2].
>>>
>>> One purpose of the original patch is to send uevents to udev, when exporting a
>>> PWM channel through the sysfs. This series:
>>> - Reverts the original patch.
>>> - Proposes a new way to send notifications to be used by udev rules.
>>>
>>> - With this series:
>>> $ echo 0 > /sys/class/pwm/pwmchip0/export
>>> $ ls /sys/class/pwm
>>> pwmchip0 pwmchip4
>>>
>>> $ ls /sys/class/pwm/pwmchip0/pwm0/
>>> capture enable polarity uevent
>>> duty_cycle period power
>>>
>>> - Without this series:
>>> $ echo 0 > /sys/class/pwm/pwmchip0/export
>>> $ ls /sys/class/pwm
>>> pwm0 pwmchip0 pwmchip4
>>>
>>> $ ls /sys/class/pwm/pwmchip0/pwm0/
>>> capture duty_cycle export period power uevent
>>> device enable npwm polarity subsystem unexport
>>>
>>> - Backtrace when exporting a 2nd channel (0) on a separate pwmchip device:
>>> $ echo 0 > /sys/class/pwm/pwmchip4/export
>>> [ 95.286558] sysfs: cannot create duplicate filename '/class/pwm/pwm0'
>>> [ 95.293630] CPU: 0 PID: 54 Comm: sh Not tainted 4.19.0-rc6-00013-g00b49b0 #151
>>> [ 95.301344] Hardware name: STM32 (Device Tree Support)
>>> [ 95.306833] [<0000c155>] (unwind_backtrace) from [<0000b273>] (show_stack+0xb/0xc)
>>> [ 95.315136] [<0000b273>] (show_stack) from [<00092455>] (sysfs_warn_dup+0x31/0x48)
>>> [ 95.323247] [<00092455>] (sysfs_warn_dup) from [<00092635>] (sysfs_do_create_link_sd+0x75/0x88)
>>> [ 95.332539] [<00092635>] (sysfs_do_create_link_sd) from [<00125823>] (device_add+0x133/0x3b0)
>>> [ 95.341694] [<00125823>] (device_add) from [<001059ed>] (export_store+0xb5/0x12c)
>>> [ 95.349761] [<001059ed>] (export_store) from [<00091911>] (kernfs_fop_write+0x87/0xda)
>>> [ 95.358150] [<00091911>] (kernfs_fop_write) from [<0005beb1>] (__vfs_write+0x1d/0xe0)
>>> [ 95.366295] [<0005beb1>] (__vfs_write) from [<0005bfe7>] (vfs_write+0x4f/0x7c)
>>> [ 95.374053] [<0005bfe7>] (vfs_write) from [<0005c0bf>] (ksys_write+0x33/0x70)
>>> [ 95.381708] [<0005c0bf>] (ksys_write) from [<00009001>] (ret_fast_syscall+0x1/0x58)
>>> [ 95.389682] Exception stack(0x01bcffa8 to 0x01bcfff0)
>>> [ 95.394946] ffa0: 00000000 00c4883c 00000001 00c4e590 00000002 00000004
>>> [ 95.403639] ffc0: 00000000 00c4883c 00c4cbe8 00000004 00000002 00000020 00000000 00c4d008
>>> [ 95.412223] ffe0: 00c29151 00c4cbe8 00c17833 00c13c0c
>>> -sh: write error: File exists
>>>
>>> [1] https://lkml.org/lkml/2018/9/25/713
>>> [2] https://lkml.org/lkml/2018/9/25/447
>>>
>>> ---
>>> Changes in v2:
>>> - update revert commit message
>>> - new patch 2/2 to propose uevent notification (change) on pwmchip
>>>
>>> Fabrice Gasnier (2):
>>> Revert "pwm: Set class for exported channels in sysfs"
>>> pwm: send a uevent on the pwmchip device upon channel sysfs (un)export
>>>
>>> drivers/pwm/sysfs.c | 12 +++++++++++-
>>> 1 file changed, 11 insertions(+), 1 deletion(-)
>> Both patches applied, thanks. What do you think would be the importance
>> of getting this into stable kernels? We can't get one of the patches in
>> without the other, so they'd both have to be backported. The second one
>> is still fairly small, so would qualify for stable, I think.
> I think the revert patch should go to stable, because it fixes a regression.
>
Hi,
Thierry, Thanks for taking these.
I also think at least the 1st patch (revert) should be backported in
stable branch. Not taking the second one may lead to another issue for
the users that now expect uevents. This is replacement patch to the
original one. So, I'd advise to push both: revert + replacement patch.
Fabrice
> Thanks
>>
>> However, given that it's taken a long time for somebody to notice this,
>> I'm not sure if this is something that people care about too much in the
>> stable kernels>>
>> Thierry
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 0/2] pwm: sysfs: fix exporting PWM channel
@ 2018-10-12 12:36 ` Fabrice Gasnier
0 siblings, 0 replies; 27+ messages in thread
From: Fabrice Gasnier @ 2018-10-12 12:36 UTC (permalink / raw)
To: Stefan Wahren, Thierry Reding
Cc: gottfried.haider, linux-pwm, michal.vokac, loic.pallardy,
linux-kernel, hsweeten, broonie, linux-rpi-kernel, gohai,
linux-stm32, linux-arm-kernel
On 10/12/2018 02:15 PM, Stefan Wahren wrote:
> Am 12.10.2018 um 13:55 schrieb Thierry Reding:
>> On Mon, Oct 01, 2018 at 03:23:55PM +0200, Fabrice Gasnier wrote:
>>> Since commit 7e5d1fd75c3d ("pwm: Set class for exported channels in sysfs")
>>> - it's not possible to export more than one PWM channel
>>> - ABI has changed, as a side effect. It may cause bad behavior as pwmchip
>>> attributes are wrongly added to pwm channels and report wrong values.
>>> See [1] and [2].
>>>
>>> One purpose of the original patch is to send uevents to udev, when exporting a
>>> PWM channel through the sysfs. This series:
>>> - Reverts the original patch.
>>> - Proposes a new way to send notifications to be used by udev rules.
>>>
>>> - With this series:
>>> $ echo 0 > /sys/class/pwm/pwmchip0/export
>>> $ ls /sys/class/pwm
>>> pwmchip0 pwmchip4
>>>
>>> $ ls /sys/class/pwm/pwmchip0/pwm0/
>>> capture enable polarity uevent
>>> duty_cycle period power
>>>
>>> - Without this series:
>>> $ echo 0 > /sys/class/pwm/pwmchip0/export
>>> $ ls /sys/class/pwm
>>> pwm0 pwmchip0 pwmchip4
>>>
>>> $ ls /sys/class/pwm/pwmchip0/pwm0/
>>> capture duty_cycle export period power uevent
>>> device enable npwm polarity subsystem unexport
>>>
>>> - Backtrace when exporting a 2nd channel (0) on a separate pwmchip device:
>>> $ echo 0 > /sys/class/pwm/pwmchip4/export
>>> [ 95.286558] sysfs: cannot create duplicate filename '/class/pwm/pwm0'
>>> [ 95.293630] CPU: 0 PID: 54 Comm: sh Not tainted 4.19.0-rc6-00013-g00b49b0 #151
>>> [ 95.301344] Hardware name: STM32 (Device Tree Support)
>>> [ 95.306833] [<0000c155>] (unwind_backtrace) from [<0000b273>] (show_stack+0xb/0xc)
>>> [ 95.315136] [<0000b273>] (show_stack) from [<00092455>] (sysfs_warn_dup+0x31/0x48)
>>> [ 95.323247] [<00092455>] (sysfs_warn_dup) from [<00092635>] (sysfs_do_create_link_sd+0x75/0x88)
>>> [ 95.332539] [<00092635>] (sysfs_do_create_link_sd) from [<00125823>] (device_add+0x133/0x3b0)
>>> [ 95.341694] [<00125823>] (device_add) from [<001059ed>] (export_store+0xb5/0x12c)
>>> [ 95.349761] [<001059ed>] (export_store) from [<00091911>] (kernfs_fop_write+0x87/0xda)
>>> [ 95.358150] [<00091911>] (kernfs_fop_write) from [<0005beb1>] (__vfs_write+0x1d/0xe0)
>>> [ 95.366295] [<0005beb1>] (__vfs_write) from [<0005bfe7>] (vfs_write+0x4f/0x7c)
>>> [ 95.374053] [<0005bfe7>] (vfs_write) from [<0005c0bf>] (ksys_write+0x33/0x70)
>>> [ 95.381708] [<0005c0bf>] (ksys_write) from [<00009001>] (ret_fast_syscall+0x1/0x58)
>>> [ 95.389682] Exception stack(0x01bcffa8 to 0x01bcfff0)
>>> [ 95.394946] ffa0: 00000000 00c4883c 00000001 00c4e590 00000002 00000004
>>> [ 95.403639] ffc0: 00000000 00c4883c 00c4cbe8 00000004 00000002 00000020 00000000 00c4d008
>>> [ 95.412223] ffe0: 00c29151 00c4cbe8 00c17833 00c13c0c
>>> -sh: write error: File exists
>>>
>>> [1] https://lkml.org/lkml/2018/9/25/713
>>> [2] https://lkml.org/lkml/2018/9/25/447
>>>
>>> ---
>>> Changes in v2:
>>> - update revert commit message
>>> - new patch 2/2 to propose uevent notification (change) on pwmchip
>>>
>>> Fabrice Gasnier (2):
>>> Revert "pwm: Set class for exported channels in sysfs"
>>> pwm: send a uevent on the pwmchip device upon channel sysfs (un)export
>>>
>>> drivers/pwm/sysfs.c | 12 +++++++++++-
>>> 1 file changed, 11 insertions(+), 1 deletion(-)
>> Both patches applied, thanks. What do you think would be the importance
>> of getting this into stable kernels? We can't get one of the patches in
>> without the other, so they'd both have to be backported. The second one
>> is still fairly small, so would qualify for stable, I think.
> I think the revert patch should go to stable, because it fixes a regression.
>
Hi,
Thierry, Thanks for taking these.
I also think at least the 1st patch (revert) should be backported in
stable branch. Not taking the second one may lead to another issue for
the users that now expect uevents. This is replacement patch to the
original one. So, I'd advise to push both: revert + replacement patch.
Fabrice
> Thanks
>>
>> However, given that it's taken a long time for somebody to notice this,
>> I'm not sure if this is something that people care about too much in the
>> stable kernels>>
>> Thierry
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 0/2] pwm: sysfs: fix exporting PWM channel
2018-10-12 12:36 ` Fabrice Gasnier
(?)
@ 2018-10-12 12:44 ` Vokáč Michal
-1 siblings, 0 replies; 27+ messages in thread
From: Vokáč Michal @ 2018-10-12 12:44 UTC (permalink / raw)
To: Fabrice Gasnier, Stefan Wahren, Thierry Reding
Cc: gottfried.haider, loic.pallardy, linux-pwm, linux-kernel,
hsweeten, broonie, linux-rpi-kernel, gohai, linux-stm32,
linux-arm-kernel
On 12.10.2018 14:36, Fabrice Gasnier wrote:
> On 10/12/2018 02:15 PM, Stefan Wahren wrote:
>> Am 12.10.2018 um 13:55 schrieb Thierry Reding:
>>> On Mon, Oct 01, 2018 at 03:23:55PM +0200, Fabrice Gasnier wrote:
>>>> Since commit 7e5d1fd75c3d ("pwm: Set class for exported channels in sysfs")
>>>> - it's not possible to export more than one PWM channel
>>>> - ABI has changed, as a side effect. It may cause bad behavior as pwmchip
>>>> attributes are wrongly added to pwm channels and report wrong values.
>>>> See [1] and [2].
>>>>
>>>> One purpose of the original patch is to send uevents to udev, when exporting a
>>>> PWM channel through the sysfs. This series:
>>>> - Reverts the original patch.
>>>> - Proposes a new way to send notifications to be used by udev rules.
>>>>
>>>> - With this series:
>>>> $ echo 0 > /sys/class/pwm/pwmchip0/export
>>>> $ ls /sys/class/pwm
>>>> pwmchip0 pwmchip4
>>>>
>>>> $ ls /sys/class/pwm/pwmchip0/pwm0/
>>>> capture enable polarity uevent
>>>> duty_cycle period power
>>>>
>>>> - Without this series:
>>>> $ echo 0 > /sys/class/pwm/pwmchip0/export
>>>> $ ls /sys/class/pwm
>>>> pwm0 pwmchip0 pwmchip4
>>>>
>>>> $ ls /sys/class/pwm/pwmchip0/pwm0/
>>>> capture duty_cycle export period power uevent
>>>> device enable npwm polarity subsystem unexport
>>>>
>>>> - Backtrace when exporting a 2nd channel (0) on a separate pwmchip device:
>>>> $ echo 0 > /sys/class/pwm/pwmchip4/export
>>>> [ 95.286558] sysfs: cannot create duplicate filename '/class/pwm/pwm0'
>>>> [ 95.293630] CPU: 0 PID: 54 Comm: sh Not tainted 4.19.0-rc6-00013-g00b49b0 #151
>>>> [ 95.301344] Hardware name: STM32 (Device Tree Support)
>>>> [ 95.306833] [<0000c155>] (unwind_backtrace) from [<0000b273>] (show_stack+0xb/0xc)
>>>> [ 95.315136] [<0000b273>] (show_stack) from [<00092455>] (sysfs_warn_dup+0x31/0x48)
>>>> [ 95.323247] [<00092455>] (sysfs_warn_dup) from [<00092635>] (sysfs_do_create_link_sd+0x75/0x88)
>>>> [ 95.332539] [<00092635>] (sysfs_do_create_link_sd) from [<00125823>] (device_add+0x133/0x3b0)
>>>> [ 95.341694] [<00125823>] (device_add) from [<001059ed>] (export_store+0xb5/0x12c)
>>>> [ 95.349761] [<001059ed>] (export_store) from [<00091911>] (kernfs_fop_write+0x87/0xda)
>>>> [ 95.358150] [<00091911>] (kernfs_fop_write) from [<0005beb1>] (__vfs_write+0x1d/0xe0)
>>>> [ 95.366295] [<0005beb1>] (__vfs_write) from [<0005bfe7>] (vfs_write+0x4f/0x7c)
>>>> [ 95.374053] [<0005bfe7>] (vfs_write) from [<0005c0bf>] (ksys_write+0x33/0x70)
>>>> [ 95.381708] [<0005c0bf>] (ksys_write) from [<00009001>] (ret_fast_syscall+0x1/0x58)
>>>> [ 95.389682] Exception stack(0x01bcffa8 to 0x01bcfff0)
>>>> [ 95.394946] ffa0: 00000000 00c4883c 00000001 00c4e590 00000002 00000004
>>>> [ 95.403639] ffc0: 00000000 00c4883c 00c4cbe8 00000004 00000002 00000020 00000000 00c4d008
>>>> [ 95.412223] ffe0: 00c29151 00c4cbe8 00c17833 00c13c0c
>>>> -sh: write error: File exists
>>>>
>>>> [1] https://lkml.org/lkml/2018/9/25/713
>>>> [2] https://lkml.org/lkml/2018/9/25/447
>>>>
>>>> ---
>>>> Changes in v2:
>>>> - update revert commit message
>>>> - new patch 2/2 to propose uevent notification (change) on pwmchip
>>>>
>>>> Fabrice Gasnier (2):
>>>> Revert "pwm: Set class for exported channels in sysfs"
>>>> pwm: send a uevent on the pwmchip device upon channel sysfs (un)export
>>>>
>>>> drivers/pwm/sysfs.c | 12 +++++++++++-
>>>> 1 file changed, 11 insertions(+), 1 deletion(-)
>>> Both patches applied, thanks. What do you think would be the importance
>>> of getting this into stable kernels? We can't get one of the patches in
>>> without the other, so they'd both have to be backported. The second one
>>> is still fairly small, so would qualify for stable, I think.
>> I think the revert patch should go to stable, because it fixes a regression.
>>
>
> Hi,
>
> Thierry, Thanks for taking these.
> I also think at least the 1st patch (revert) should be backported in
> stable branch. Not taking the second one may lead to another issue for
> the users that now expect uevents. This is replacement patch to the
> original one. So, I'd advise to push both: revert + replacement patch.
I also vote for pushing both.
M.
>>> However, given that it's taken a long time for somebody to notice this,
>>> I'm not sure if this is something that people care about too much in the
>>> stable kernels>>
>>> Thierry
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH v2 0/2] pwm: sysfs: fix exporting PWM channel
@ 2018-10-12 12:44 ` Vokáč Michal
0 siblings, 0 replies; 27+ messages in thread
From: Vokáč Michal @ 2018-10-12 12:44 UTC (permalink / raw)
To: linux-arm-kernel
On 12.10.2018 14:36, Fabrice Gasnier wrote:
> On 10/12/2018 02:15 PM, Stefan Wahren wrote:
>> Am 12.10.2018 um 13:55 schrieb Thierry Reding:
>>> On Mon, Oct 01, 2018 at 03:23:55PM +0200, Fabrice Gasnier wrote:
>>>> Since commit 7e5d1fd75c3d ("pwm: Set class for exported channels in sysfs")
>>>> - it's not possible to export more than one PWM channel
>>>> - ABI has changed, as a side effect. It may cause bad behavior as pwmchip
>>>> attributes are wrongly added to pwm channels and report wrong values.
>>>> See [1] and [2].
>>>>
>>>> One purpose of the original patch is to send uevents to udev, when exporting a
>>>> PWM channel through the sysfs. This series:
>>>> - Reverts the original patch.
>>>> - Proposes a new way to send notifications to be used by udev rules.
>>>>
>>>> - With this series:
>>>> $ echo 0 > /sys/class/pwm/pwmchip0/export
>>>> $ ls /sys/class/pwm
>>>> pwmchip0 pwmchip4
>>>>
>>>> $ ls /sys/class/pwm/pwmchip0/pwm0/
>>>> capture enable polarity uevent
>>>> duty_cycle period power
>>>>
>>>> - Without this series:
>>>> $ echo 0 > /sys/class/pwm/pwmchip0/export
>>>> $ ls /sys/class/pwm
>>>> pwm0 pwmchip0 pwmchip4
>>>>
>>>> $ ls /sys/class/pwm/pwmchip0/pwm0/
>>>> capture duty_cycle export period power uevent
>>>> device enable npwm polarity subsystem unexport
>>>>
>>>> - Backtrace when exporting a 2nd channel (0) on a separate pwmchip device:
>>>> $ echo 0 > /sys/class/pwm/pwmchip4/export
>>>> [ 95.286558] sysfs: cannot create duplicate filename '/class/pwm/pwm0'
>>>> [ 95.293630] CPU: 0 PID: 54 Comm: sh Not tainted 4.19.0-rc6-00013-g00b49b0 #151
>>>> [ 95.301344] Hardware name: STM32 (Device Tree Support)
>>>> [ 95.306833] [<0000c155>] (unwind_backtrace) from [<0000b273>] (show_stack+0xb/0xc)
>>>> [ 95.315136] [<0000b273>] (show_stack) from [<00092455>] (sysfs_warn_dup+0x31/0x48)
>>>> [ 95.323247] [<00092455>] (sysfs_warn_dup) from [<00092635>] (sysfs_do_create_link_sd+0x75/0x88)
>>>> [ 95.332539] [<00092635>] (sysfs_do_create_link_sd) from [<00125823>] (device_add+0x133/0x3b0)
>>>> [ 95.341694] [<00125823>] (device_add) from [<001059ed>] (export_store+0xb5/0x12c)
>>>> [ 95.349761] [<001059ed>] (export_store) from [<00091911>] (kernfs_fop_write+0x87/0xda)
>>>> [ 95.358150] [<00091911>] (kernfs_fop_write) from [<0005beb1>] (__vfs_write+0x1d/0xe0)
>>>> [ 95.366295] [<0005beb1>] (__vfs_write) from [<0005bfe7>] (vfs_write+0x4f/0x7c)
>>>> [ 95.374053] [<0005bfe7>] (vfs_write) from [<0005c0bf>] (ksys_write+0x33/0x70)
>>>> [ 95.381708] [<0005c0bf>] (ksys_write) from [<00009001>] (ret_fast_syscall+0x1/0x58)
>>>> [ 95.389682] Exception stack(0x01bcffa8 to 0x01bcfff0)
>>>> [ 95.394946] ffa0: 00000000 00c4883c 00000001 00c4e590 00000002 00000004
>>>> [ 95.403639] ffc0: 00000000 00c4883c 00c4cbe8 00000004 00000002 00000020 00000000 00c4d008
>>>> [ 95.412223] ffe0: 00c29151 00c4cbe8 00c17833 00c13c0c
>>>> -sh: write error: File exists
>>>>
>>>> [1] https://lkml.org/lkml/2018/9/25/713
>>>> [2] https://lkml.org/lkml/2018/9/25/447
>>>>
>>>> ---
>>>> Changes in v2:
>>>> - update revert commit message
>>>> - new patch 2/2 to propose uevent notification (change) on pwmchip
>>>>
>>>> Fabrice Gasnier (2):
>>>> Revert "pwm: Set class for exported channels in sysfs"
>>>> pwm: send a uevent on the pwmchip device upon channel sysfs (un)export
>>>>
>>>> drivers/pwm/sysfs.c | 12 +++++++++++-
>>>> 1 file changed, 11 insertions(+), 1 deletion(-)
>>> Both patches applied, thanks. What do you think would be the importance
>>> of getting this into stable kernels? We can't get one of the patches in
>>> without the other, so they'd both have to be backported. The second one
>>> is still fairly small, so would qualify for stable, I think.
>> I think the revert patch should go to stable, because it fixes a regression.
>>
>
> Hi,
>
> Thierry, Thanks for taking these.
> I also think at least the 1st patch (revert) should be backported in
> stable branch. Not taking the second one may lead to another issue for
> the users that now expect uevents. This is replacement patch to the
> original one. So, I'd advise to push both: revert + replacement patch.
I also vote for pushing both.
M.
>>> However, given that it's taken a long time for somebody to notice this,
>>> I'm not sure if this is something that people care about too much in the
>>> stable kernels>>
>>> Thierry
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 0/2] pwm: sysfs: fix exporting PWM channel
@ 2018-10-12 12:44 ` Vokáč Michal
0 siblings, 0 replies; 27+ messages in thread
From: Vokáč Michal @ 2018-10-12 12:44 UTC (permalink / raw)
To: Fabrice Gasnier, Stefan Wahren, Thierry Reding
Cc: gottfried.haider, loic.pallardy, linux-pwm, linux-kernel,
hsweeten, broonie, linux-rpi-kernel, gohai, linux-stm32,
linux-arm-kernel
On 12.10.2018 14:36, Fabrice Gasnier wrote:
> On 10/12/2018 02:15 PM, Stefan Wahren wrote:
>> Am 12.10.2018 um 13:55 schrieb Thierry Reding:
>>> On Mon, Oct 01, 2018 at 03:23:55PM +0200, Fabrice Gasnier wrote:
>>>> Since commit 7e5d1fd75c3d ("pwm: Set class for exported channels in sysfs")
>>>> - it's not possible to export more than one PWM channel
>>>> - ABI has changed, as a side effect. It may cause bad behavior as pwmchip
>>>> attributes are wrongly added to pwm channels and report wrong values.
>>>> See [1] and [2].
>>>>
>>>> One purpose of the original patch is to send uevents to udev, when exporting a
>>>> PWM channel through the sysfs. This series:
>>>> - Reverts the original patch.
>>>> - Proposes a new way to send notifications to be used by udev rules.
>>>>
>>>> - With this series:
>>>> $ echo 0 > /sys/class/pwm/pwmchip0/export
>>>> $ ls /sys/class/pwm
>>>> pwmchip0 pwmchip4
>>>>
>>>> $ ls /sys/class/pwm/pwmchip0/pwm0/
>>>> capture enable polarity uevent
>>>> duty_cycle period power
>>>>
>>>> - Without this series:
>>>> $ echo 0 > /sys/class/pwm/pwmchip0/export
>>>> $ ls /sys/class/pwm
>>>> pwm0 pwmchip0 pwmchip4
>>>>
>>>> $ ls /sys/class/pwm/pwmchip0/pwm0/
>>>> capture duty_cycle export period power uevent
>>>> device enable npwm polarity subsystem unexport
>>>>
>>>> - Backtrace when exporting a 2nd channel (0) on a separate pwmchip device:
>>>> $ echo 0 > /sys/class/pwm/pwmchip4/export
>>>> [ 95.286558] sysfs: cannot create duplicate filename '/class/pwm/pwm0'
>>>> [ 95.293630] CPU: 0 PID: 54 Comm: sh Not tainted 4.19.0-rc6-00013-g00b49b0 #151
>>>> [ 95.301344] Hardware name: STM32 (Device Tree Support)
>>>> [ 95.306833] [<0000c155>] (unwind_backtrace) from [<0000b273>] (show_stack+0xb/0xc)
>>>> [ 95.315136] [<0000b273>] (show_stack) from [<00092455>] (sysfs_warn_dup+0x31/0x48)
>>>> [ 95.323247] [<00092455>] (sysfs_warn_dup) from [<00092635>] (sysfs_do_create_link_sd+0x75/0x88)
>>>> [ 95.332539] [<00092635>] (sysfs_do_create_link_sd) from [<00125823>] (device_add+0x133/0x3b0)
>>>> [ 95.341694] [<00125823>] (device_add) from [<001059ed>] (export_store+0xb5/0x12c)
>>>> [ 95.349761] [<001059ed>] (export_store) from [<00091911>] (kernfs_fop_write+0x87/0xda)
>>>> [ 95.358150] [<00091911>] (kernfs_fop_write) from [<0005beb1>] (__vfs_write+0x1d/0xe0)
>>>> [ 95.366295] [<0005beb1>] (__vfs_write) from [<0005bfe7>] (vfs_write+0x4f/0x7c)
>>>> [ 95.374053] [<0005bfe7>] (vfs_write) from [<0005c0bf>] (ksys_write+0x33/0x70)
>>>> [ 95.381708] [<0005c0bf>] (ksys_write) from [<00009001>] (ret_fast_syscall+0x1/0x58)
>>>> [ 95.389682] Exception stack(0x01bcffa8 to 0x01bcfff0)
>>>> [ 95.394946] ffa0: 00000000 00c4883c 00000001 00c4e590 00000002 00000004
>>>> [ 95.403639] ffc0: 00000000 00c4883c 00c4cbe8 00000004 00000002 00000020 00000000 00c4d008
>>>> [ 95.412223] ffe0: 00c29151 00c4cbe8 00c17833 00c13c0c
>>>> -sh: write error: File exists
>>>>
>>>> [1] https://lkml.org/lkml/2018/9/25/713
>>>> [2] https://lkml.org/lkml/2018/9/25/447
>>>>
>>>> ---
>>>> Changes in v2:
>>>> - update revert commit message
>>>> - new patch 2/2 to propose uevent notification (change) on pwmchip
>>>>
>>>> Fabrice Gasnier (2):
>>>> Revert "pwm: Set class for exported channels in sysfs"
>>>> pwm: send a uevent on the pwmchip device upon channel sysfs (un)export
>>>>
>>>> drivers/pwm/sysfs.c | 12 +++++++++++-
>>>> 1 file changed, 11 insertions(+), 1 deletion(-)
>>> Both patches applied, thanks. What do you think would be the importance
>>> of getting this into stable kernels? We can't get one of the patches in
>>> without the other, so they'd both have to be backported. The second one
>>> is still fairly small, so would qualify for stable, I think.
>> I think the revert patch should go to stable, because it fixes a regression.
>>
>
> Hi,
>
> Thierry, Thanks for taking these.
> I also think at least the 1st patch (revert) should be backported in
> stable branch. Not taking the second one may lead to another issue for
> the users that now expect uevents. This is replacement patch to the
> original one. So, I'd advise to push both: revert + replacement patch.
I also vote for pushing both.
M.
>>> However, given that it's taken a long time for somebody to notice this,
>>> I'm not sure if this is something that people care about too much in the
>>> stable kernels>>
>>> Thierry
^ permalink raw reply [flat|nested] 27+ messages in thread