Re: [spi-devel-general] Re: SPI

[dmitry pervushin]

On Mon, 2005-09-26 at 10:20 -0600, Grant Likely wrote:
> Would "SPI slave" or "SPI slave device" be better terminology than
> "SPI device"?  That way the terminology matches how SPI hardware docs
> are usually written.  (not a big deal, just thought I'd ask)
The term "SPI slave device" looks correct.. I am correcting the doc :)
> > +       err = spi_bus_populate( the_spi_bus,
> > +                       "Dev1 0 1 2\0" "Dev2 2 1 0\0",
> > +                       extract_name )
> In my mind, this is not ideal.  For example, the MPC5200 has 4 PSC
> ports which can be in SPI mode.  The SPI bus driver should/will not
> know what devices are attached to it.  It should be the responsibility
> of the board setup code to populate the bus.... on the other hand,
> perhaps the bus driver should look to it's platform_device structure
> to find a table of attached devices.  Generic platform_device parsing
> code could be used by all SPI bus drivers.
The spi_bus_populate is not the only way to populate the bus; the bus
driver can discover SPI devices on his own and directly call
spi_device_add, isn't it ?
> > +In this example, function like extract_name would put the '\0' on the
> > +1st space of device's name, so names will become just "Dev1", "Dev2",
> > +and the rest of string will become parameters of device.
> I don't think it's wise to use '\0' as a delimiter.  Sure it makes
> parsing really simple when the string passed in is formed correctly,
> but if someone misses the last '\0' you have no way to know where the
> string ends.  It also makes it difficult support passing a device
> string from the kernel command line.
You're right. Using spi_populate_bus is the simplest way, that may lead
to errors... From the other hand, if we used another char to delimit
device name and its parameters, there would be person who would want
this character in device name... I think that we can add another
approach to populate the bus ? 
> 
> > +4. SPI functions are structures reference
> > +-----------------------------------------
> > +This section describes structures and functions that listed
> > +in include/linux/spi.h
> I would like to see this function and structure reference in the spi.h
> file itself rather than here.  Better chance of it being kept up to
> date that way.
Yes; but I personally prefer to look to the only place instead of
spi.h/spi-core.c. I'll try to keep the things consistent :)

> > +This structure represents the message that SPI device driver sends to the
> > +SPI bus driver to handle.
> Is there any way for the SPI device to constrain the clock rate for a
> transfer?  For example, if the devices maximum speed is lower than the
> bus maximum speed.
Thank you for this comment; the `clock' field is initially intended to
do this. Device driver might set the field to maximum speed, and bus
driver would analyze the field in its xfer function and send the message
on lower speed. Moreover, there is the `set_clock' callback in
spi_bus_driver. If msg specifies its own clock value, the bus driver's
set_clock will be called just before transferring the message.
> Overall, I like.  It looks like it does what I need it to.  If I get a
> chance this week I'll port my SPI drivers to it and try it out on my
> MPC5200 board.
Thank you! If your drivers are going to open source, could you also sent
them to spi mailing list, to prepare the consolidated patch ? I hope if
there is no significant troubles, the current core will go to the
mainstream kernel :)


-
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/