Hi Simon,
On 11 October 2014 04:56, Simon Glass sjg@chromium.org wrote:
Hi Jagan,
On 10 October 2014 12:37, Jagan Teki jagannadh.teki@gmail.com wrote:
Hi Simon,
On 10 October 2014 23:35, Simon Glass sjg@chromium.org wrote:
Hi Jagan,
On 10 October 2014 11:31, Jagan Teki jagannadh.teki@gmail.com wrote:
Hi Simon,
Can you please see my comments below, I'm relating more about cs and bus valid scenario which is available on current drivers wrt dm-spi.
On 30 September 2014 01:05, Simon Glass sjg@chromium.org 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@chromium.org
Changes in v3:
- Add a cs_info() method to the driver model SPI API
I think this gives the enough info regarding cs on particular controller what about the bus usage - which is also specific to controller it self, right?
Say - spi controller on a particular soc, have spi0, spi1 means two buses and 4 cs lines, cs0-cs3. How is this works in that case?
And also I saw tegra20_sflash.c validates only cs
int tegra20_sflash_cs_info(struct udevice *bus, unsigned int cs, struct spi_cs_info *info)
What about bus number check this particular tegra spi controller is concern.
The bus device is passed in as a pointer. The bus number gets looked up to find the device for that bus number. See spi_get_bus_and_cs() for that.
This is good because drivers no longer need to validate the bus number.
Agreed, but for controlling "sf probe bus:cs " options bus number should get from the respective driver.
I wonder if the issue here is a difference of understanding about bus and chip select. In my mind, bus is just a device. We use the bus number to specify which device we mean, but it is just a a convenience. If there were some other way of finding the device we would use it (as we do with device tree, for example).
Yes, I do believe that by mentioning bus spi0, spi1 on the devicetree so the respective bus get's bind'ed till that fine. But as we use runtime probe using "sf probe bus:cs" where I would mention # sf probe 2:0 selecting spi2 which is of third bus on the same controller driver will that spi_cs_is_valid() will show spi2 is invalid for this particular driver?
Similarly, chip select is just a device. We look at the bus device, find the appropriate chip select number, and end up with a child device which is referred to by that chip select.
Just like validating supported cs, through cs_info I assume the same works for bus as well as this is a pure driver specific theory.
I have a suggestion, like in spi_find_bus_and_cs() or spi_get_bus_and_cs() we have a function calls for uclass_get_device_by_seq() and spi_find_bus_and_cs() along with that ops->cs_info(bus, cs, info) by adding bus number check for the driver.
So-that there is no requirement to call spi_cs_is_valid from any other location as bus and cs validates through commands itself.
At present spi_cs_info() calls spi_find_chip_select(). I think I see what you are getting at - you don't want a device to be created unless cs_info() allows it, right? I will take a look.
Yes ie my point, along with that instead of a separate caller for spi_cs_info() why can't we call in the probe time, either from command or at the time of bus_cs binding.
Once the bus_cs binding done, and validate the mentioned bus and cs from command or devicetree and return.
Does this make sense, please comment?
BTW: in tegra20_sflash.c, how does tegra20_sflash_cs_info() called?
Only if someone calls spi_cs_info(). I don't think there are any callers at present. With tegra using device tree I'm not sure it would have any purpose - the chip selects and buses are all known.
Yes devicetree will help to take but validating should be require as driver only knows how many bus's with respective supported chip-selects based on this command user may verify the same. what do think?
- Add a uclass for a generic SPI device (for use with the 'sspi' command)
- Add missing comments to spi.h
- Correct typo where 'slave' should say 'bus'
- Fix two comment typos
- Put the cs member back into spi_slave
- Use an explicit chip select value instead of reusing device sequence number
Changes in v2:
- Add missing comments for struct spi_slave
- Fix code nits from Daniel Schwierzeck
- Use 'bus' instead of 'dev' to make the API clearer
common/exports.c | 4 +- drivers/spi/Makefile | 4 + drivers/spi/spi-uclass.c | 390 +++++++++++++++++++++++++++++++++++++++++++++++ include/dm/uclass-id.h | 2 + include/spi.h | 254 +++++++++++++++++++++++++++++- 5 files changed, 650 insertions(+), 4 deletions(-) create mode 100644 drivers/spi/spi-uclass.c
+int spi_cs_is_valid(unsigned int busnum, unsigned int cs)
Who is the caller for this?
This can be called by boards, or anywhere really. It replaces the function with the same purpose in the old SPI framework.
Please see above.
OK
thanks!