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

[Jagan Teki]

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.

-- 
Thanks,
Jagan.
--------
Jagannadha Sutradharudu Teki,
E: jagannadh.teki at gmail.com, P: +91-9676773388
Engineer - System Software Hacker
U-boot - SPI Custodian and Zynq APSOC
Ln: http://www.linkedin.com/in/jaganteki