[U-Boot] [PATCH v3 05/29] dm: spi: Add a uclass for SPI

[Jagan Teki]

Hi Simon,

On 10 October 2014 23:35, Simon Glass <sjg at chromium.org> wrote:
> Hi Jagan,
>
> On 10 October 2014 11:31, Jagan Teki <jagannadh.teki at 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 at 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 at 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 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.

BTW: in tegra20_sflash.c, how does tegra20_sflash_cs_info() called?

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

thanks!
-- 
Jagan.