Hello Jagan, > From: Jagan Teki [mailto:jagannadh.t...@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@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.lan...@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! Please look up similar discussions on the linux-mtd and linux-spi mailing lists! > -- > Thanks, > Jagan. Best Regards, Thomas _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot