Re: [RFC PATCH 1/2] picoxcell-otp: add support for picoxcell OTPdevices
From: Jamie Iles
Date: Thu Mar 24 2011 - 05:30:57 EST
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@xxxxxxxxxxxxx>
> > > +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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/