Hi Jagan, On 10 October 2014 12:37, Jagan Teki <jagannadh.t...@gmail.com> wrote: > Hi Simon, > > On 10 October 2014 23:35, Simon Glass <s...@chromium.org> wrote: >> Hi Jagan, >> >> On 10 October 2014 11:31, Jagan Teki <jagannadh.t...@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 <s...@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 <s...@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). 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. > > 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. > > 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. > >> >>> >>>> - 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 Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
