[U-Boot] [PATCH 06/25] dm: spi: Add a uclass for SPI

[Simon Glass]

Hi Pavel,


On 15 July 2014 02:26, Pavel Herrmann <morpheus.ibis at gmail.com> wrote:
>
> Hi
>
> On Monday 14 of July 2014 18:56:13 Simon Glass wrote:
> > Add a uclass which provides access to SPI buses and includes operations
> > required by SPI.
> >
> > For a time driver model will need to co-exist with the legacy SPI interface
> > so some parts of the header file are changed depending on which is in use.
> > The exports are adjusted also since some functions are not available with
> > driver model.
> >
> > Boards must define CONFIG_DM_SPI to use driver model for SPI.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> > ...
> > +int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
> > +          const void *dout, void *din, unsigned long flags)
> > +{
> > +     struct udevice *dev = slave->dev;
> > +     struct udevice *bus = dev->parent;
>
> is this the best interface here?
> I think it would be cleaner if bus drivers had interfaces which follow a
> certain template, such as
> bus_ops(struct udevice *bus, struct udevice *child, ...)

Thanks for your comments.

Well I thought about that long and hard. Clearly in a pure
driver-model work we would pass a udevice, not a spi_slave.

>
> struct spi_slave would be a prime candidate to have in child->parentdata
> (which should only be accessed by the parent IIUC)

Yes, it is. In other words, 'struct spi_slave' is the child's parent
data. The only reason that spi_xfer() is passed a spi_slave rather
than a udevice is to maintain compatibility with the existing SPI
system. I tried various other approachs, such as '#define spi_slave
udevice' and the like. At some point we can change it, but it is
really painful to have two completely different APIs co-existing in
U-Boot.

This way, driver model becomes a fairly soft transition, the amount of
rewriting needed is reduced, and I think it is much more likely that
people will use it. As an example, one of the reasons that the generic
board change has been relatively painless is that people can just
define CONFIG_SYS_GENERIC_BOARD in their board header file, and in
most cases it just works. If we require people to rewrite things it
will take longer, etc. There is already enough rewriting required in
individual drivers to keep people busy. It will be a relatively easy
change to do in the future if we want to.

Re passing both the bus and the device, really, what's the point? The
only valid bus to pass to the function is dev->parent. If you pass
anything else it is an error. It is redundant and therefore just
introduces the possibility of error.

Regards,
Simon