Re: [PATCH v2] Add GPIO-based MMC/SD driver

Greg KH
2008-07-18T23:02:52Z

On Sat, Jul 19, 2008 at 12:38:07AM +0200, Michael Buesch wrote:
> Thanks for the comments.
>
> On Saturday 19 July 2008 00:10:37 Randy Dunlap wrote:
> > > +To add a new device, simply echo the configuration string to the "add" file.
> > > +The config string is composed out of the following elements:
> > > +
> > > +DEVNAME DIpin DOpin CLKpin CSpin SPIMODE MAXBUSSPEED NO_SPI_DELAY CSACTIVELOW
> > > +
> > > +DEVNAME is a unique name string for the device.
> > > +DIpin is the SPI DI GPIO pin.
> > > +DOpin is the SPI DO GPIO pin.
> > > +CLKpin is the SPI CLOCK GPIO pin.
> > > +CSpin is the SPI CHIPSELECT GPIO pin.
> > > +SPIMODE is the hardware mode the device will run at. Can be 0-3.
> > > +MAXBUSSPEED is the maximum bus speed in Hertz.
> > > +NO_SPI_DELAY can be 1 or 0. If it is 1, then the lowlevel SPI delay
> > > +will not be performed. This is not standards compliant, but may be required
> > > +to gain reasonable speeds on embedded hardware.
> > > +CSACTIVELOW can be 1 or 0. If it is 1, the chip is considered to be selected, if CS
> > > +is at a logical 0.
> > > +
> >
> > Would this be better done via configfs? sysfs files are supposed to be
> > single-value files.
>
>
> Well, I really want to avoid over-engineering this thing.
> I thought about using configfs, but that would require to keep lots of state
> information across the operations.
> So one would have to allocate a device with mkdir. Then configure the parameters.
> And then somehow tell the kernel to register it. State has to be maintained over
> this time and I'm not sure how that "register" operation would look like.
> Writing a "1" to a "register" file? So why not write all config parameters
> to an "add" file and be done with all the stuff. ;)
>
> It all depends on how you define "one thing per file", IMO.
> This "add" file does one thing. It creates a device. We must, of course, pass
> some configuration parameters, too.

We have precidence with this for the device id being "added" to a driver
at run time in PCI and USB. Not that I really like this, I do think
configfs would be better to do this with here instead of sysfs.

That being said, as this is a new sysfs file, please break this
documentation up into a file in the Documentation/ABI directory instead
of here.

thanks,

greg k-h