Re: [PATCH 1/2] Add c2 port support.

[Greg KH]

On Wed, Oct 22, 2008 at 01:58:10PM -0700, Andrew Morton wrote:
> On Wed, 22 Oct 2008 22:48:01 +0200
> Rodolfo Giometti <giometti@xxxxxxxxxxxx> wrote:
> 
> > > > > What I haven't yet got my head around is this:
> > > > > 
> > > > >   Currently this code supports only flash programming through sysfs
> > > > >   interface but extensions shoud be easy to add.
> > > > > 
> > > > > is that really what we want to use sysfs for?  As the prime interface
> > > > > to a device driver (or is it a bus driver?) Didn't we used to use
> > > > > device nodes for that sort of thing?
> > > > 
> > > > I decided to use this interface since the C2 ports are simple and very
> > > > easy to manage with such files abstraction.
> > > 
> > > well..  what _is_ the implemented interface?  What alternatives were
> > > considered and why were they judged inappropriate?  What interfaces do
> > > any similar in-tree drivers use?
> > 
> > Well... main reasons in using sysfs support are:
> > 
> > * no more char devices and new ioctls to define,
> > 
> 
> Why is that good?  We have hundreds and hundreds of drivers which use a
> device node interface and they work OK.
> 
> sysfs was never intended for use as the sole interface between device
> drivers and userspace.  Using it that way is somewhat unconventional. 
> At the very least I do think it needs more justification and
> explanation than we're being provded here.

For firmware download to devices, using sysfs is the "normal" interface
these days, that is what the firmware class is for.

Which reminds me, Rodolfo, why didn't you just use that interface for
your firmware downloading instead of rolling your own here?

> Device nodes and ioctls and such do have a number of problems, and
> perhaps we should sit down and invent a new way of wiring drivers up to
> userspace.  But whether that would end up being identical to the
> present sysfs code is not obvious.
> 
> > * the microcontrollers on-board flash can be easily programmed by
> > using only the "echo" and "cat" commands, so no more tools are needed
> > and even simply busybox can be used.
> 
> A system which is using busybox might want to set CONFIG_SYSFS=n for
> memory savings.  Now what?

My daughter's phone (the #1 selling handset last year in the US) has
sysfs enabled, I think this argument can be put to rest as not a valid
one.

thanks,

greg k-h
--
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/