[U-Boot] [PATCH v4 31/36] sf: Add extended read commands support

[Jagan Teki]

On Wed, Sep 25, 2013 at 3:34 PM,  <thomas.langer at lantiq.com> wrote:
> Hello Jagan,
>
> Jagan Teki wrote on 2013-09-25:
>> On Wed, Sep 25, 2013 at 3:14 PM,  <thomas.langer at lantiq.com> wrote:
>>> Hello Jagan,
>>>
>>>> From: Jagan Teki [mailto:jagannadh.teki at gmail.com] Sent: Wednesday,
>>>> September 25, 2013 11:36 AM To: Langer Thomas (LQDE RD ST PON SW) Cc:
>>>> Jagannadha Sutradharudu Teki; Tom Rini; jaganna; u-
>>>> boot at lists.denx.de; Todd Legler (tlegler); Willis Max; Syed Hussain;
>>>> Sascha Silbe Subject: Re: [U-Boot] [PATCH v4 31/36] sf: Add extended
>>>> read commands support
>>>>
>>>> On Wed, Sep 25, 2013 at 1:40 AM,  <thomas.langer at lantiq.com> wrote:
>>>>> Hello Jagan,
>>>>>
>>>>> Am 24.09.2013 20:36, schrieb Jagannadha Sutradharudu Teki:
>>>>>> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
>>>>>> index ea39d1a..0ac9fab 100644
>>>>>> --- a/drivers/spi/spi.c
>>>>>> +++ b/drivers/spi/spi.c
>>>>>> @@ -7,6 +7,7 @@
>>>>>>  #include <common.h>
>>>>>>  #include <malloc.h>
>>>>>>  #include <spi.h>
>>>>>> +#include <spi_flash.h>
>>>>>>
>>>>>>  void *spi_do_alloc_slave(int offset, int size, unsigned int bus,
>>>>>>                        unsigned int cs)
>>>>>> @@ -20,6 +21,7 @@ void *spi_do_alloc_slave(int offset, int size,
>>>> unsigned int bus,
>>>>>>               slave = (struct spi_slave *)(ptr + offset);
>>>>>>               slave->bus = bus;
>>>>>>               slave->cs = cs;
>>>>>> +             slave->rd_cmd = CMD_READ_ARRAY_FAST;
>>>>>
>>>>> This is SPI code, not SF! The spi layer should not know such details of
>>>>> the slave!
>>>>> What about EEPROMs or other SPI slaves, which are NOT spi_flash???
>>>>> Examples (just searched for includes of "spi.h"):
>>>>> board/bf527-ezkit/video.c
>>>>> drivers/net/enc28j60.c
>>>>>
>>>>> Please ensure that the layers are not mixed up!
>>>>> SPI is an interface type and SF is ONLY ONE user of this interface!
>>>>
>>>> I understand your concern, but here the point is for discovering the
>>>> command set.
>>>> slave->rd_cmd = CMD_READ_ARRAY_FAST;
>>>> is a default controller supported fast read.
>>>>
>>>> spi_flash layer will discover the respective rd_cmd based slave and
>>>> flash, if slave doesn't have any commands to list, means not support
>>>> extended/quad then these fields are filled in spi.c
>>>>
>>>> there is nothing harm for respective drivers or code.
>>>
>>> But I think this is the wrong approach!
>>> Why should the spi controller care about, what devices type is connected?
>>> And the "CMD" is a SF specific thing!
>>> I agree, that the controller should provide information about his "features",
>>> but this should only be something like "tx_width=4" and "rx_width=4",
>>> which would tell the SF layer that quad-io is possible!
>>>
>>> No details of serial-flashes are necessary for this!
>> Yes, no issues.
>> I can directly assign to flash side while discovering commends.
>
> I don't understand this sentence. Do you mean "commands"?
> Who will "discover" the commands?
>
> The SPI controller does not know about the meaning of commands!
> The controller only knows "I must send this block of data and split it on 1/2/4 lines"
> (or similar for other transfers).

"implementation will discover the fastest command by taking the supported
commands from flash and a controller. Controller supported commands
will always been a priority."