Re: Handle pwm with sysfs with recent kernel

Re: Handle pwm with sysfs with recent kernel

[Uwe Kleine-König]

[Cc: += linux-pwm list]

Hello Arthur,

On Tue, Jan 08, 2019 at 11:07:39AM +0100, Arthur LAMBERT wrote:
> I am trying to update my kernel bsp from v4.1.X to v4.19.8 but I have some regressions
> when I try to use pwm through sysfs from userspace. I have three pwm declared as
> follow in my device tree :
> 
> 	leds {
> 	     compatible = "pwm-leds";
> 	     ledred {
> 		pwms = <&pwm1>;
> 		label = "red";
> 		linux,default-trigger = "none";
> 		default-state = "on";
> 	     };
> 
> 	     ledblue {
> 	     	pwms = <&pwm5>;
> 		label = "blue";
> 		linux,default-trigger = "none";
> 		default-state = "on";
> 	     };
> 
> 	     ledgreen {
> 	     	pwms = <&pwm6>;
> 		label = "green";
> 		linux,default-trigger = "none";
> 		default-state = "on";
> 	     };
> 	};
> 
> First I have an issue with duplicate name :
> sysfs: cannot create duplicate filename '/class/pwm/pwm0'
> CPU: 0 PID: 653 Comm: XXXX
> Hardware name: Freescale i.MX6 Ultralite (Device Tree)
> [<8010dda8>] (unwind_backtrace) from [<8010b468>] (show_stack+0x10/0x14)
> [<8010b468>] (show_stack) from [<8025ef0c>] (sysfs_warn_dup+0x58/0x64)
> [<8025ef0c>] (sysfs_warn_dup) from [<8025f200>] (sysfs_do_create_link_sd+0xd4/0xd8)
> [<8025f200>] (sysfs_do_create_link_sd) from [<80458d6c>] (device_add+0x1ec/0x5d4)
> [<80458d6c>] (device_add) from [<8040b5a8>] (export_store+0xfc/0x17c)
> [<8040b5a8>] (export_store) from [<80456904>] (dev_attr_store+0x18/0x24)
> [<80456904>] (dev_attr_store) from [<8025e604>] (sysfs_kf_write+0x44/0x48)
> [<8025e604>] (sysfs_kf_write) from [<8025dcb0>] (kernfs_fop_write+0xe8/0x1d0)
> [<8025dcb0>] (kernfs_fop_write) from [<801eaaf8>] (__vfs_write+0x28/0x144)
> [<801eaaf8>] (__vfs_write) from [<801ead88>] (vfs_write+0xa4/0x164)
> [<801ead88>] (vfs_write) from [<801eaf54>] (ksys_write+0x40/0x94)
> [<801eaf54>] (ksys_write) from [<80101000>] (ret_fast_syscall+0x0
> 
> I was able to fix it by revert a patch : https://patchwork.kernel.org/patch/9971821/
> It is normal that this revert patch is not currently apply on last 4.19.X tag (last one
> seems to be 4.19.13)
> 
> Then I have another issue when I try to enable my pwm :
> 
> # ls /sys/class/pwm/
> pwmchip0  pwmchip2  pwmchip4  pwmchip6
> pwmchip1  pwmchip3  pwmchip5  pwmchip7
> # echo 0 > /sys/class/pwm/pwmchip0/export
> # echo 1 > /sys/class/pwm/pwmchip0/pwm0/enable
> -sh: write error: Invalid argument

Just a guess: did you check that /sys/class/pwm/pwmchip0/pwm0/enable
actually exists?

> Is there a new way to handle pwm in recent kernel ? My old process is
> now deprecated or something ?

Not sure what you want to achive and if pwmchip0/pwm0 is one of the pwms
used in your dts snipped above. If the pwm is in use, don't expect it to
be controllable via sysfs.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: Handle pwm with sysfs with recent kernel

[Lothar Waßmann]

Hi,

On Tue, 8 Jan 2019 23:21:02 +0100 Uwe Kleine-König wrote:
> [Cc: += linux-pwm list]
> 
> Hello Arthur,
> 
> On Tue, Jan 08, 2019 at 11:07:39AM +0100, Arthur LAMBERT wrote:
> > I am trying to update my kernel bsp from v4.1.X to v4.19.8 but I have some regressions
> > when I try to use pwm through sysfs from userspace. I have three pwm declared as
> > follow in my device tree :
> > 
> > 	leds {
> > 	     compatible = "pwm-leds";
> > 	     ledred {
> > 		pwms = <&pwm1>;
> > 		label = "red";
> > 		linux,default-trigger = "none";
> > 		default-state = "on";
> > 	     };
> > 
> > 	     ledblue {
> > 	     	pwms = <&pwm5>;
> > 		label = "blue";
> > 		linux,default-trigger = "none";
> > 		default-state = "on";
> > 	     };
> > 
> > 	     ledgreen {
> > 	     	pwms = <&pwm6>;
> > 		label = "green";
> > 		linux,default-trigger = "none";
> > 		default-state = "on";
> > 	     };
> > 	};
> > 
> > First I have an issue with duplicate name :
> > sysfs: cannot create duplicate filename '/class/pwm/pwm0'
> > CPU: 0 PID: 653 Comm: XXXX
> > Hardware name: Freescale i.MX6 Ultralite (Device Tree)
> > [<8010dda8>] (unwind_backtrace) from [<8010b468>] (show_stack+0x10/0x14)
> > [<8010b468>] (show_stack) from [<8025ef0c>] (sysfs_warn_dup+0x58/0x64)
> > [<8025ef0c>] (sysfs_warn_dup) from [<8025f200>] (sysfs_do_create_link_sd+0xd4/0xd8)
> > [<8025f200>] (sysfs_do_create_link_sd) from [<80458d6c>] (device_add+0x1ec/0x5d4)
> > [<80458d6c>] (device_add) from [<8040b5a8>] (export_store+0xfc/0x17c)
> > [<8040b5a8>] (export_store) from [<80456904>] (dev_attr_store+0x18/0x24)
> > [<80456904>] (dev_attr_store) from [<8025e604>] (sysfs_kf_write+0x44/0x48)
> > [<8025e604>] (sysfs_kf_write) from [<8025dcb0>] (kernfs_fop_write+0xe8/0x1d0)
> > [<8025dcb0>] (kernfs_fop_write) from [<801eaaf8>] (__vfs_write+0x28/0x144)
> > [<801eaaf8>] (__vfs_write) from [<801ead88>] (vfs_write+0xa4/0x164)
> > [<801ead88>] (vfs_write) from [<801eaf54>] (ksys_write+0x40/0x94)
> > [<801eaf54>] (ksys_write) from [<80101000>] (ret_fast_syscall+0x0
> > 
> > I was able to fix it by revert a patch : https://patchwork.kernel.org/patch/9971821/
> > It is normal that this revert patch is not currently apply on last 4.19.X tag (last one
> > seems to be 4.19.13)
> > 
> > Then I have another issue when I try to enable my pwm :
> > 
> > # ls /sys/class/pwm/
> > pwmchip0  pwmchip2  pwmchip4  pwmchip6
> > pwmchip1  pwmchip3  pwmchip5  pwmchip7
> > # echo 0 > /sys/class/pwm/pwmchip0/export
> > # echo 1 > /sys/class/pwm/pwmchip0/pwm0/enable
> > -sh: write error: Invalid argument  
> 
> Just a guess: did you check that /sys/class/pwm/pwmchip0/pwm0/enable
> actually exists?
> 
If not, the error message would be 'No such file or directory' rather
than "Invalid argument".

> > Is there a new way to handle pwm in recent kernel ? My old process is
> > now deprecated or something ?  
> 
> Not sure what you want to achive and if pwmchip0/pwm0 is one of the pwms
> used in your dts snipped above. If the pwm is in use, don't expect it to
> be controllable via sysfs.
> 
If it were in use, he should get a 'Device or resource busy' error when
trying to export it.


Lothar Waßmann

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

Re: Handle pwm with sysfs with recent kernel

[Uwe Kleine-König]

Hello,

On Wed, Jan 09, 2019 at 09:25:31AM +0100, Lothar Waßmann wrote:
> Hi,
> 
> On Tue, 8 Jan 2019 23:21:02 +0100 Uwe Kleine-König wrote:
> > On Tue, Jan 08, 2019 at 11:07:39AM +0100, Arthur LAMBERT wrote:
> > > Then I have another issue when I try to enable my pwm :
> > > 
> > > # ls /sys/class/pwm/
> > > pwmchip0  pwmchip2  pwmchip4  pwmchip6
> > > pwmchip1  pwmchip3  pwmchip5  pwmchip7
> > > # echo 0 > /sys/class/pwm/pwmchip0/export
> > > # echo 1 > /sys/class/pwm/pwmchip0/pwm0/enable
> > > -sh: write error: Invalid argument  
> > 
> > Just a guess: did you check that /sys/class/pwm/pwmchip0/pwm0/enable
> > actually exists?
> > 
> If not, the error message would be 'No such file or directory' rather
> than "Invalid argument".

Then "Invalid argument" means that either the "1" couldn't be parsed as
an integer 1 or 0 (no) or that pwm_apply_state returned -EINVAL. What
type of pwm are we talking about (i.e. which driver is responsible for
pwmchip0)? Hmm, that machine is an imx6ul, looking at the pwm-imx driver
there doesn't seem to be a path through the .apply callback that returns
-EINVAL. Would you care to add a few printk in
drivers/pwm/sysfs.c:enable_store() that show where the -EINVAL comes
from?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: Handle pwm with sysfs with recent kernel

[Arthur LAMBERT]

It is an error due to new check from this commit :

[arthur * dreem-linux-4.19] git show ef2bf4997f7da6efa8540d9cf726c44bf2b863af
commit ef2bf4997f7da6efa8540d9cf726c44bf2b863af
Author: Brian Norris <briannorris@chromium.org>
Date:   Fri May 27 09:45:49 2016 -0700

    pwm: Improve args checking in pwm_apply_state()

    It seems like in the process of refactoring pwm_config() to utilize the
    newly-introduced pwm_apply_state() API, some args/bounds checking was
    dropped.
(...)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index dba3843..ed337a8c 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -457,7 +457,8 @@ int pwm_apply_state(struct pwm_device *pwm, struct pwm_state *state)
 {
        int err;

-       if (!pwm)
+       if (!pwm || !state || !state->period ||
+           state->duty_cycle > state->period)
                return -EINVAL;

(...)

I guess that I have to configure the period and the duty cycle before
enabling the pwm to be compatible with this kernek. I will update my
userspace code.