Re: [PATCH] pwm: Create device class for pwm channels

From: David Hsu
Date: Mon Jul 11 2016 - 21:25:35 EST


On Mon, Jul 11, 2016 at 2:39 AM, Thierry Reding
<thierry.reding@xxxxxxxxx> wrote:
> On Wed, Jun 15, 2016 at 04:52:56PM -0700, David Hsu wrote:
>> On Wed, Jun 15, 2016 at 7:37 AM, Thierry Reding
>> <thierry.reding@xxxxxxxxx> wrote:
>> > On Tue, Jun 14, 2016 at 07:12:04PM -0700, Greg KH wrote:
>> >> From: David Hsu <davidhsu@xxxxxxxxxx>
>> >>
>> >> Pwm channels don't send uevents when exported, this change adds the
>> >> channels to a pwm class and set their device type to pwm_channel so
>> >> uevents are sent.
>> >>
>> >> To do this properly, the device names need to change to uniquely
>> >> identify a channel. This change is from pwmN to pwm-(chip->base):N
>> >>
>> >> Signed-off-by: David Hsu <davidhsu@xxxxxxxxxx>
>> >> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
>> >> ---
>> >> Documentation/pwm.txt | 6 ++++--
>> >> drivers/pwm/sysfs.c | 15 ++++++++++++---
>> >> 2 files changed, 16 insertions(+), 5 deletions(-)
>> >>
>> >> Note, this patch came from David with his work on a system that has
>> >> dynamic PWM devices and channels, and we needed some way to tell
>> >> userspace what is going on when they are added or removed. If anyone
>> >> knows any other way of doing this that does not involve changing the pwm
>> >> names, please let us know.
>> >
>> > Is it truly PWM channels that dynamically appear and disappear? I'd be
>> > interested in how that's achieved, because there are probably other
>> > issues that will manifest if you do that. Do you have a pointer to the
>> > work that David's been undertaking? Generally some more context on the
>> > use-case would be helpful here.
>>
>> Only PWM devices are dynamic, the number of channels exposed by
>> devices do not change after they've been added to the system.
>
> In that case, would it not be enough to use the uevents generated by the
> addition and removal of the PWM chip devices to/from sysfs?

We need channel level granularity to modify permissions for sysfs nodes
that are created when the channels are exported.

>
>> > Also I'd prefer if this avoided using chip->base here, because it exists
>> > purely for legacy purposes and is supposed to go away eventually.
>> >
>> > Thierry
>>
>> Would using dev_name(parent) be an acceptable alternative?
>
> Yes, that sounds like a more sensible choice to me.

Thanks Thierry, I'll send out a v2 shortly.

It also occurred to me that this change could affect apps that have the previous
name hard coded. I can create a link pointing to the new name but wanted to see
if there's a better recommendation or if it's required.

David


>
> Thierry