Messages in this thread Patch in this message | | | Date | Thu, 24 Mar 2011 09:30:36 +0000 | From | Jamie Iles <> | Subject | Re: [RFC PATCH 1/2] picoxcell-otp: add support for picoxcell OTP devices |
| |
Replying to myself...
On Wed, Mar 23, 2011 at 03:12:42PM +0000, Jamie Iles wrote: > On Wed, Mar 23, 2011 at 07:42:30AM -0700, Greg KH wrote: > > On Wed, Mar 23, 2011 at 12:16:58PM +0000, Jamie Iles wrote: > > > +What: /sys/bus/picoxcell-otp/devices/.../size > > > +Date: March 2011 > > > +KernelVersion: 2.6.40+ > > > +Contact: Jamie Iles <jamie@jamieiles.com> > > > +Description: > > > + The effective storage size of the region. This is the amount > > > + of data that a user can store in the region taking into > > > + account the number of regions and the redundancy format of the > > > + region itself. > > > diff --git a/Documentation/ABI/testing/sysfs-platform-picoxcell-otp b/Documentation/ABI/testing/sysfs-platform-picoxcell-otp > > > new file mode 100644 > > > index 0000000..e5ee711 > > > --- /dev/null > > > +++ b/Documentation/ABI/testing/sysfs-platform-picoxcell-otp > > > @@ -0,0 +1,39 @@ > > > +What: /sys/devices/platform/picoxcell-otp*/write_enable > > Why are these in a platform subdirectory? Shouldn't they be the devices > > listed above in the previous file? > > So the way I have it is that there is the real OTP device which can be > split into a number of regions. These attributes affect the physical > device by programming the number of regions and write enable. > > Each region is a virtual device to provide the character device and the > redundancy/size attributes but you can't split these regions down again.
[...]
> > > +/* > > > + * Add all of the device entries to sysfs. This also includes creating the > > > + * region device nodes and sysfs entries. > > > + */ > > > +static int otp_sysfs_add(struct device *dev) > > > +{ > > > + int err; > > > + > > > + err = device_create_file(dev, &dev_attr_write_enable); > > > + if (err) > > > + goto out; > > > + > > > + err = device_create_file(dev, &dev_attr_num_regions); > > > + if (err) > > > + goto num_regions_fail; > > > + > > > + err = device_create_file(dev, &dev_attr_bad_words); > > > + if (err) > > > + goto bad_words_fail; > > > + > > > + err = device_create_file(dev, &dev_attr_strict_programming); > > > + if (err) > > > + goto strict_programming_fail; > > > + > > > > Shouldn't all of these be in an attribute group like the other sysfs > > files are in this driver? That way you add/remove them all at once. > > I did look at doing this but I couldn't see a way to add an attribute > group to an existing device in a single step, or is this just the > wrong approach all together?
So the crucial thing I was missing was the device_type part of the driver model. I'm now creating a virtual "otpa" device that is in the "otp" bus and has the regions has virtual child devices. Both of these virtual devices have different device_types that have different attributes so that fits in very nicely.
Jamie
| |