Hi Simon, On 11 October 2014 04:56, Simon Glass <s...@chromium.org> wrote: > 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).
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! -- Jagan. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
