2013-03-05 14:45:37

by Lars Poeschel

[permalink] [raw]
Subject: Re: [PATCH RFC] pwm: add sysfs interface

At first: Thanks for your review!

On Monday 25 February 2013 at 05:00:25, Greg KH wrote:
> On Tue, Feb 19, 2013 at 03:27:41PM +0100, Lars Poeschel wrote:
> > +static int pwmchip_export(struct pwm_chip *chip)
> > +{
> > + int status;
> > + struct device *dev;
> > +
> > + mutex_lock(&sysfs_lock);
> > + dev = device_create(&pwm_class, chip->dev, MKDEV(0, 0), chip,
> > + "pwmchip%d", chip->base);
> > + if (!IS_ERR(dev))
> > + status = sysfs_create_group(&dev->kobj, &pwmchip_attr_group);
> > + else
> > + status = PTR_ERR(dev);
>
> You can't create sysfs files after the device has been exposed to
> userspace. Please use the default group functionality for the class,
> which fixes this problem.

I don't understand, what is wrong here. This is the same that gpio sysfs
does. Could you please be a bit more specific ?
I create a new device using device_create and create a new group with it with
some attributes. Isn't it exposed to userspace after that ?
Or am I not allowed to use pwm_class on device creation ? What to use then ?
What do you mean by default group functionality for the class ? I want to use
my own "pwm" class ? Don't I ?

Regards,
Lars


2013-03-05 21:33:58

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH RFC] pwm: add sysfs interface

On Tue, Mar 05, 2013 at 03:46:13PM +0100, Lars Poeschel wrote:
> At first: Thanks for your review!
>
> On Monday 25 February 2013 at 05:00:25, Greg KH wrote:
> > On Tue, Feb 19, 2013 at 03:27:41PM +0100, Lars Poeschel wrote:
> > > +static int pwmchip_export(struct pwm_chip *chip)
> > > +{
> > > + int status;
> > > + struct device *dev;
> > > +
> > > + mutex_lock(&sysfs_lock);
> > > + dev = device_create(&pwm_class, chip->dev, MKDEV(0, 0), chip,
> > > + "pwmchip%d", chip->base);
> > > + if (!IS_ERR(dev))
> > > + status = sysfs_create_group(&dev->kobj, &pwmchip_attr_group);
> > > + else
> > > + status = PTR_ERR(dev);
> >
> > You can't create sysfs files after the device has been exposed to
> > userspace. Please use the default group functionality for the class,
> > which fixes this problem.
>
> I don't understand, what is wrong here. This is the same that gpio sysfs
> does. Could you please be a bit more specific ?
> I create a new device using device_create and create a new group with it with
> some attributes. Isn't it exposed to userspace after that ?

The device was exposed to userspace when you called device_create(), so
any program is then free to start looking for attributes. It will not
find them because you haven't created them yet.

To prevent this race from happening, use the default attribute group for
the class to create the attributes before the device is announced to
userspace by the driver core.

That is what the dev_attrs pointer is for in 'struct class'.

Hope this helps,

greg k-h