[U-Boot] [PATCH v3] dm: spi: Read default speed and mode values from DT

[Vignesh R]

On 04/20/2016 03:26 PM, Qianyu Gong wrote:
> Hi Vignesh,
> 
>  
> 
>> Date: Wed, 13 Apr 2016 15:40:53 +0530
> 
>> From: Vignesh R <vigneshr at ti.com <mailto:vigneshr at ti.com>>
> 
>> To: Jagan Teki <jteki at openedev.com <mailto:jteki at openedev.com>>, Tom
> Rini <trini at konsulko.com <mailto:trini at konsulko.com>>
> 
>> Cc: u-boot at lists.denx.de <mailto:u-boot at lists.denx.de>, Stefan Roese
> <sr at denx.de <mailto:sr at denx.de>>
> 
>> Subject: [U-Boot] [PATCH v3] dm: spi: Read default speed and mode
> 
>>             values    from DT
> 
>> Message-ID: <1460542253-10580-1-git-send-email-vigneshr at ti.com
> <mailto:1460542253-10580-1-git-send-email-vigneshr at ti.com>>
> 
>> Content-Type: text/plain
> 
>>
> 
>> In case of DT boot, don't read default speed and mode for SPI from
> 
>> CONFIG_*, instead read from DT node. This will make sure that boards
> 
>> with multiple SPI/QSPI controllers can be probed at different
> 
>> bus frequencies and SPI modes.
> 
>>
> 
>> Signed-off-by: Vignesh R <vigneshr at ti.com <mailto:vigneshr at ti.com>>
> 
>> ---
> 
>>
> 
>> v3: Update commit message to mention SPI mode changes
> 
>>
> 
>> v2: Initialize speed, mode to 0 instead of -1
> 
>>
> 
>>  cmd/sf.c                 | 2 ++
> 
>>  drivers/spi/spi-uclass.c | 8 ++++++--
> 
>>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
>>
> 
>> diff --git a/cmd/sf.c b/cmd/sf.c
> 
>> index 42862d9d921a..286906c3a151 100644
> 
>> --- a/cmd/sf.c
> 
>> +++ b/cmd/sf.c
> 
>> @@ -88,6 +88,8 @@ static int do_spi_flash_probe(int argc, char * const
> argv[])
> 
>>  #ifdef CONFIG_DM_SPI_FLASH
> 
>>             struct udevice *new, *bus_dev;
> 
>>             int ret;
> 
>> +          /* In DM mode defaults will be taken from DT */
> 
>> +          speed = 0, mode = 0;
> 
>>  #else
> 
>>             struct spi_flash *new;
> 
>>  #endif
> 
>> diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c
> 
>> index 5561f36762f9..5fb5630e2981 100644
> 
>> --- a/drivers/spi/spi-uclass.c
> 
>> +++ b/drivers/spi/spi-uclass.c
> 
>> @@ -264,6 +264,7 @@ int spi_get_bus_and_cs(int busnum, int cs, int
> speed, int
> 
>> mode,
> 
>>                                   struct udevice **busp, struct
> spi_slave **devp)
> 
>>  {
> 
>>             struct udevice *bus, *dev;
> 
>> +          struct dm_spi_slave_platdata *plat;
> 
>>             bool created = false;
> 
>>             int ret;
> 
>>
> 
>> @@ -280,8 +281,6 @@ int spi_get_bus_and_cs(int busnum, int cs, int
> speed, int
> 
>> mode,
> 
>>              * SPI flash chip - we will bind to the correct driver.
> 
>>              */
> 
>>             if (ret == -ENODEV && drv_name) {
> 
>> -                         struct dm_spi_slave_platdata *plat;
> 
>> -
> 
>>                            debug("%s: Binding new device '%s',
> busnum=%d, cs=%d,
> 
>> driver=%s\n",
> 
>>                                  __func__, dev_name, busnum, cs,
> drv_name);
> 
>>                            ret = device_bind_driver(bus, drv_name,
> dev_name, &dev);
> 
>> @@ -308,6 +307,11 @@ int spi_get_bus_and_cs(int busnum, int cs, int
> speed, int
> 
>> mode,
> 
>>                            slave->dev = dev;
> 
>>             }
> 
>>
> 
>> +          plat = dev_get_parent_platdata(dev);
> 
>> +          if (!speed) {
> 
>> +                         speed = plat->max_hz;
> 
>> +                         mode = plat->mode;
> 
>> +          }
> 
>>             ret = spi_set_speed_mode(bus, speed, mode);
> 
>>             if (ret)
> 
>>                            goto err;
> 
>> --
> 
>  
> 
> I just doubt if spi_set_speed_mode() has really made a difference to
> 
> the actual transfer.
> 

It does (see below)...

> Seems that if the device is inactive, calling device_probe() would also
> call
> 
> spi_set_speed_mode() and do the data transfer. Even if it’s active, setting
> 
> speed and mode for it again would not be necessary.


Yes, spi_set_speed_mode() is called from
spi_flash_probe_slave()->spi_claim_bus() as part of device_probe().
spi_claim_bus() in spi-uclass.c speed & mode are appropriately passed
based on DT data to spi_set_speed_mode(). But that's not the issue.

But in spi_get_bus_and_cs() (called from sf probe) there is a call to
spi_set_speed_mode() after device_probe() for inactive devices. This
call is to _override_ the speed set via DT with those passed as cmd line
args of sf probe. But, if no args are passed to sf probe, speed and mode
default to CONFIG_SF_DEFAULT_SPEED/MODE (see do_spi_flash_probe() in
cmd/sf.c) instead of using DT inputs.

-- 
Regards
Vignesh