Messages in this thread | | | Date | Mon, 5 Sep 2016 17:20:25 +0200 | From | Thierry Reding <> | Subject | Re: [PATCH v2] pwm: Create device class for pwm channels |
| |
On Mon, Jul 18, 2016 at 03:14:50PM -0700, David Hsu wrote: > 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 pwmchipM:N > > Signed-off-by: David Hsu <davidhsu@google.com> > --- > v2: Use parent name instead of chip->base for channel naming. > > Documentation/pwm.txt | 6 ++++-- > drivers/pwm/sysfs.c | 18 +++++++++++++----- > 2 files changed, 17 insertions(+), 7 deletions(-)
Forgot to mention one other thing that had come to my mind...
> diff --git a/drivers/pwm/sysfs.c b/drivers/pwm/sysfs.c [...] > @@ -248,9 +254,11 @@ static int pwm_export_child(struct device *parent, struct pwm_device *pwm) > > export->child.release = pwm_export_release; > export->child.parent = parent; > + export->child.type = &pwm_channel_type; > export->child.devt = MKDEV(0, 0); > + export->child.class = &pwm_class; > export->child.groups = pwm_groups; > - dev_set_name(&export->child, "pwm%u", pwm->hwpwm); > + dev_set_name(&export->child, "%s:%u", dev_name(parent), pwm->hwpwm);
Using the colon as separator could possibly prevent the use of these files in paths lists. Consider the case where somebody wanted to give a list of PWM sysfs references:
PWM_DEVICES=/sys/class/pwm/pwmchip0:0:/sys/class/pwm/pwmchip0:1
which might get parsed as four entries:
- /sys/class/pwm/pwmchip0 - 0 - /sys/class/pwm/pwmchip0 - 1
none of which point to a valid PWM. There are a couple of other separators that might be possible, such as '-' or '.'.
Then again, looking at existing usage in sysfs it seems like the colon is a fairly popular separator, so maybe I'm just being too paranoid.
Thierry [unhandled content-type:application/pgp-signature] | |