Re: [PATCH v2] pwm: Create device class for pwm channels
From: Thierry Reding
Date: Mon Sep 05 2016 - 11:20:55 EST
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@xxxxxxxxxx>
> ---
> 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
Attachment:
signature.asc
Description: PGP signature