| Message ID | a6fce535-312a-4f67-924b-a829d2b2053d@DB9EHSMHS010.ehs.local |
|---|---|
| State | Superseded |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
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! > } > > return ptr; > diff --git a/include/spi.h b/include/spi.h > index c0dab57..093847e 100644 > --- a/include/spi.h > +++ b/include/spi.h > @@ -40,11 +40,13 @@ > * cs: ID of the chip select connected to the slave. > * max_write_size: If non-zero, the maximum number of bytes which can > * be written at once, excluding command bytes. > + * rd_cmd: Read command. > */ > struct spi_slave { > unsigned int bus; > unsigned int cs; > unsigned int max_write_size; > + u8 rd_cmd; > }; > > /*----------------------------------------------------------------------- Best Regards, Thomas
On Wed, Sep 25, 2013 at 1:40 AM, <thomas.langer@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. > >> } >> >> return ptr; >> diff --git a/include/spi.h b/include/spi.h >> index c0dab57..093847e 100644 >> --- a/include/spi.h >> +++ b/include/spi.h >> @@ -40,11 +40,13 @@ >> * cs: ID of the chip select connected to the slave. >> * max_write_size: If non-zero, the maximum number of bytes which can >> * be written at once, excluding command bytes. >> + * rd_cmd: Read command. >> */ >> struct spi_slave { >> unsigned int bus; >> unsigned int cs; >> unsigned int max_write_size; >> + u8 rd_cmd; >> }; >> >> /*----------------------------------------------------------------------- > > Best Regards, > Thomas > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot
Hello Jagan, > From: Jagan Teki [mailto:jagannadh.teki@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.langer@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
On Wed, Sep 25, 2013 at 3:14 PM, <thomas.langer@lantiq.com> wrote: > Hello Jagan, > >> From: Jagan Teki [mailto:jagannadh.teki@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.langer@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.
Hello Jagan, Jagan Teki wrote on 2013-09-25: > On Wed, Sep 25, 2013 at 3:14 PM, <thomas.langer@lantiq.com> wrote: >> Hello Jagan, >> >>> From: Jagan Teki [mailto:jagannadh.teki@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.langer@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). Maybe your specific controller behaves in another way, but this is not the standard and you should not force this into the interface. And another doubt: The might be different commands for dual/quad read/write, depending on the flash manufacturer. With your solution, the controller drivers would need these details in advance! Or at least need to be updated on each new command, which needs to be supported. > -- > Thanks, > Jagan. Best Regards, Thomas
On Wed, Sep 25, 2013 at 3:34 PM, <thomas.langer@lantiq.com> wrote: > Hello Jagan, > > Jagan Teki wrote on 2013-09-25: >> On Wed, Sep 25, 2013 at 3:14 PM, <thomas.langer@lantiq.com> wrote: >>> Hello Jagan, >>> >>>> From: Jagan Teki [mailto:jagannadh.teki@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.langer@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." From SPI controller: ---------------------------- spi_setup_slave() { spi = spi_alloc_slave(struct zynq_spi_slave, bus, cs); spi->slave.rd_cmd = RD_CMD_FULL; spi->slave.wr_cmd = WR_CMD_FULL; } From SPI FLASH: (drivers/mtd/spi/spi_flash_probe.c) ----------------------------------------------------------------------------- /* Look for the fastest read cmd */ cmd = fls(params->rd_cmd & flash->spi->rd_cmd); if (cmd) { cmd = spi_read_cmds_array[cmd - 1]; flash->read_cmd = cmd; } else { /* Go for controller supported command */ flash->read_cmd = CMD_READ_ARRAY_FAST; } /* Look for the fastest write cmd */ cmd = fls(params->wr_cmd & flash->spi->wr_cmd); if (cmd) { cmd = spi_write_cmds_array[cmd - 1]; flash->write_cmd = cmd; } else { /* Go for controller supported command */ flash->write_cmd = CMD_PAGE_PROGRAM; } include/spi_flash.h: --------------------------- /* Supported write cmds enum list */ enum spi_write_cmds { PAGE_PROGRAM = 1 << 0, QUAD_PAGE_PROGRAM = 1 << 1, }; #define WR_CMD_FULL PAGE_PROGRAM | QUAD_PAGE_PROGRAM /* Supported read cmds enum list */ enum spi_read_cmds { ARRAY_SLOW = 1 << 0, ARRAY_FAST = 1 << 1, DUAL_OUTPUT_FAST = 1 << 2, DUAL_IO_FAST = 1 << 3, QUAD_OUTPUT_FAST = 1 << 4, }; #define RD_CMD_FULL ARRAY_SLOW | ARRAY_FAST | DUAL_OUTPUT_FAST | \ DUAL_IO_FAST | QUAD_OUTPUT_FAST If the controller is not filling slave.rd_cmd and slave.wr_cmd if it's not supporting even if connected flash supports, fill the read/write commands to default ones. Controller is a master to decide which command is to use, and flash is slave to transfer the discovered command through spi_xfer(), so the controller will take care of the command processing. This is well detailed explanation, I hope you understand. > > Maybe your specific controller behaves in another way, but this is not the standard > and you should not force this into the interface. > > And another doubt: The might be different commands for dual/quad read/write, > depending on the flash manufacturer. With your solution, the controller drivers > would need these details in advance! Or at least need to be updated on each new > command, which needs to be supported. > >> -- >> Thanks, >> Jagan. > > Best Regards, > Thomas > >
Hello Jagan, Jagan Teki wrote on 2013-09-25: > On Wed, Sep 25, 2013 at 3:34 PM, <thomas.langer@lantiq.com> wrote: >> Hello Jagan, >> >> Jagan Teki wrote on 2013-09-25: >>> On Wed, Sep 25, 2013 at 3:14 PM, <thomas.langer@lantiq.com> wrote: >>>> Hello Jagan, >>>> >>>>> From: Jagan Teki [mailto:jagannadh.teki@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.langer@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." > > From SPI controller: > ---------------------------- > spi_setup_slave() { > spi = spi_alloc_slave(struct zynq_spi_slave, bus, cs); > > spi->slave.rd_cmd = RD_CMD_FULL; > spi->slave.wr_cmd = WR_CMD_FULL; > } These are already FLASH definitions! Please define it the other way round: Add properties for number of rx and tx lines to the spi-slave. (Do you notice the different naming?) > > > From SPI FLASH: (drivers/mtd/spi/spi_flash_probe.c) > ----------------------------------------------------------------------------- > /* Look for the fastest read cmd */ > cmd = fls(params->rd_cmd & flash->spi->rd_cmd); > if (cmd) { > cmd = spi_read_cmds_array[cmd - 1]; flash->read_cmd = > cmd; } else { /* Go for controller supported command */ > flash->read_cmd = CMD_READ_ARRAY_FAST; > } > > /* Look for the fastest write cmd */ > cmd = fls(params->wr_cmd & flash->spi->wr_cmd); > if (cmd) { > cmd = spi_write_cmds_array[cmd - 1]; flash->write_cmd = > cmd; } else { /* Go for controller supported command */ > flash->write_cmd = CMD_PAGE_PROGRAM; > } > include/spi_flash.h: > --------------------------- > /* Supported write cmds enum list */ > enum spi_write_cmds { > PAGE_PROGRAM = 1 << 0, > QUAD_PAGE_PROGRAM = 1 << 1, > }; > #define WR_CMD_FULL PAGE_PROGRAM | QUAD_PAGE_PROGRAM > > /* Supported read cmds enum list */ > enum spi_read_cmds { > ARRAY_SLOW = 1 << 0, > ARRAY_FAST = 1 << 1, > DUAL_OUTPUT_FAST = 1 << 2, > DUAL_IO_FAST = 1 << 3, > QUAD_OUTPUT_FAST = 1 << 4, > }; > #define RD_CMD_FULL ARRAY_SLOW | ARRAY_FAST | > DUAL_OUTPUT_FAST | \ > DUAL_IO_FAST | QUAD_OUTPUT_FAST > > If the controller is not filling slave.rd_cmd and slave.wr_cmd if it's > not supporting even if connected flash > supports, fill the read/write commands to default ones. > > Controller is a master to decide which command is to use, and flash is > slave to transfer the discovered command through spi_xfer(), so the > controller will take care of the command processing. > > This is well detailed explanation, I hope you understand. I understand, but need to repeat: All this is flash specific and should not be part of the generic spi interface layer! For spi, you have number of data lines, which is independent of the cmd to transfer! And what I am missing is the information about the lines to use when calling spi_xfer(). In your implementation, the controller itself needs to know this from decoding the opcodes. How do you intend to handle this in the future? Let's assume you want to add support for a new flash, which needs a different opcode and/or has different requirements for dummy cycles. Then you add this to the spi_flash driver and to ALL controller driver, which have been integrated until then? I think, all this should be handled only in one place, which is the spi_flash! If you rely on a table with flash opcodes in the controller driver, you have a too strong dependency! I want to avoid this from the beginning, that's why I am so persistent on this topic! Please try to do a step back and ignore the internal details of your zync spi controller. Then think about the details the spi_flash should provide to the spi controller for a specific transfer: Example read for S25FL129P (no preference, just the first I found): For QOR (Quad Output Read Mode) - send opcode, address and dummy byte on SI / IO0 only - receive data on IO0-IO3 in parallel and for QIOR (Quad I/O High Performance Read Mode) - send opcode on SI / IO0 - send address and dummy byte on IO0-IO3 in parallel - receive data on IO0-IO3 With your approach, the controller needs to know both opcodes and the details, when to switch from single to quad mode. I suggest this: - spi_xfer for cmd with SPI_XFER_BEGIN - spi_xfer for addr and dummy with 0 or "SPI_XFER_QUAD", depending on opcode above - spi_xfer for data with SPI_XFER_QUAD (and maybe SPI_XFER_END) The same new flags (SPI_XFER_QUAD, SPI_XFER_DUAL) could be used to indicate the features of the spi controller. This allows to provide every information of the transfer from spi_flash and don't rely on additional information in the controller part. > >> >> Maybe your specific controller behaves in another way, but this is not >> the standard and you should not force this into the interface. >> >> And another doubt: The might be different commands for dual/quad >> read/write, depending on the flash manufacturer. With your solution, >> the controller drivers would need these details in advance! Or at least >> need to be updated on each new command, which needs to be supported. >> > -- > Thanks, > Jagan. Best Regards, Thomas
Hello Jagan, it seems an almost ready patch for m25p80 driver in the kernel was posted today: "[PATCHv2] drivers: mtd: devices: Add quad read support." http://thread.gmane.org/gmane.linux.drivers.mtd/48552/focus=48557 Please see how there in the function "m25p80_quad_read" the "rx_nbits" in the transfer structure is set. Best Regards, Thomas
diff --git a/doc/SPI/status.txt b/doc/SPI/status.txt index be5aebd..cca3ae4 100644 --- a/doc/SPI/status.txt +++ b/doc/SPI/status.txt @@ -10,12 +10,12 @@ SPI FLASH (drivers/mtd/spi): - spi_flash_ops.c: SPI flash operations code. - spi_flash.c: SPI flash interface, which interacts controller driver. - Common probe support for all supported flash vendors except, ramtron. +- Extended read commands support(Array slow/fast, dual output fast, dual IO fast) SPI DRIVERS (drivers/spi): - TODO: -- Extended read commands support(dual read, dual IO read) - Quad Page Program support. - Quad Read support(quad fast read, quad IO read) - Dual flash connection topology support(accessing two spi flash memories with single cs) diff --git a/drivers/mtd/spi/spi_flash_ops.c b/drivers/mtd/spi/spi_flash_ops.c index b4e1c40..9fd1d6b 100644 --- a/drivers/mtd/spi/spi_flash_ops.c +++ b/drivers/mtd/spi/spi_flash_ops.c @@ -273,7 +273,7 @@ int spi_flash_cmd_read_fast(struct spi_flash *flash, u32 offset, return 0; } - cmd[0] = CMD_READ_ARRAY_FAST; + cmd[0] = flash->read_cmd; cmd[4] = 0x00; while (len) { diff --git a/drivers/mtd/spi/spi_flash_probe.c b/drivers/mtd/spi/spi_flash_probe.c index daf53ac..e08d66e 100644 --- a/drivers/mtd/spi/spi_flash_probe.c +++ b/drivers/mtd/spi/spi_flash_probe.c @@ -18,6 +18,13 @@ DECLARE_GLOBAL_DATA_PTR; +static const u32 spi_read_cmds_array[] = { + CMD_READ_ARRAY_SLOW, + CMD_READ_ARRAY_FAST, + CMD_READ_DUAL_OUTPUT_FAST, + CMD_READ_DUAL_IO_FAST, +}; + /* * struct spi_flash_params - SPI/QSPI flash device params structure * @@ -26,6 +33,7 @@ DECLARE_GLOBAL_DATA_PTR; * @ext_jedec: Device ext_jedec ID * @sector_size: Sector size of this device * @nr_sectors: No.of sectors on this device + * @rd_cmd: Read command * @flags: Importent param, for flash specific behaviour */ struct spi_flash_params { @@ -34,107 +42,104 @@ struct spi_flash_params { u16 ext_jedec; u32 sector_size; u32 nr_sectors; + u8 rd_cmd; u16 flags; }; static const struct spi_flash_params spi_flash_params_table[] = { #ifdef CONFIG_SPI_FLASH_ATMEL /* ATMEL */ - {"AT45DB011D", 0x1f2200, 0x0, 64 * 1024, 4, SECT_4K}, - {"AT45DB021D", 0x1f2300, 0x0, 64 * 1024, 8, SECT_4K}, - {"AT45DB041D", 0x1f2400, 0x0, 64 * 1024, 8, SECT_4K}, - {"AT45DB081D", 0x1f2500, 0x0, 64 * 1024, 16, SECT_4K}, - {"AT45DB161D", 0x1f2600, 0x0, 64 * 1024, 32, SECT_4K}, - {"AT45DB321D", 0x1f2700, 0x0, 64 * 1024, 64, SECT_4K}, - {"AT45DB641D", 0x1f2800, 0x0, 64 * 1024, 128, SECT_4K}, + {"AT45DB011D", 0x1f2200, 0x0, 64 * 1024, 4, 0, SECT_4K}, + {"AT45DB021D", 0x1f2300, 0x0, 64 * 1024, 8, 0, SECT_4K}, + {"AT45DB041D", 0x1f2400, 0x0, 64 * 1024, 8, 0, SECT_4K}, + {"AT45DB081D", 0x1f2500, 0x0, 64 * 1024, 16, 0, SECT_4K}, + {"AT45DB161D", 0x1f2600, 0x0, 64 * 1024, 32, 0, SECT_4K}, + {"AT45DB321D", 0x1f2700, 0x0, 64 * 1024, 64, 0, SECT_4K}, + {"AT45DB641D", 0x1f2800, 0x0, 64 * 1024, 128, 0, SECT_4K}, #endif #ifdef CONFIG_SPI_FLASH_EON /* EON */ - {"EN25Q32B", 0x1c3016, 0x0, 64 * 1024, 64, 0}, - {"EN25Q64", 0x1c3017, 0x0, 64 * 1024, 128, SECT_4K}, - {"EN25Q128B", 0x1c3018, 0x0, 64 * 1024, 256, 0}, + {"EN25Q32B", 0x1c3016, 0x0, 64 * 1024, 64, 0, 0}, + {"EN25Q64", 0x1c3017, 0x0, 64 * 1024, 128, 0, SECT_4K}, + {"EN25Q128B", 0x1c3018, 0x0, 64 * 1024, 256, 0, 0}, #endif #ifdef CONFIG_SPI_FLASH_GIGADEVICE /* GIGADEVICE */ - {"GD25Q64B", 0xc84017, 0x0, 64 * 1024, 128, SECT_4K}, - {"GD25LQ32", 0xc86016, 0x0, 64 * 1024, 64, SECT_4K}, + {"GD25Q64B", 0xc84017, 0x0, 64 * 1024, 128, 0, SECT_4K}, + {"GD25LQ32", 0xc86016, 0x0, 64 * 1024, 64, 0, SECT_4K}, #endif #ifdef CONFIG_SPI_FLASH_MACRONIX /* MACRONIX */ - {"MX25L4005", 0xc22013, 0x0, 64 * 1024, 8, 0}, - {"MX25L8005", 0xc22014, 0x0, 64 * 1024, 16, 0}, - {"MX25L1605D", 0xc22015, 0x0, 64 * 1024, 32, 0}, - {"MX25L3205D", 0xc22016, 0x0, 64 * 1024, 64, 0}, - {"MX25L6405D", 0xc22017, 0x0, 64 * 1024, 128, 0}, - {"MX25L12805", 0xc22018, 0x0, 64 * 1024, 256, 0}, - {"MX25L25635F", 0xc22019, 0x0, 64 * 1024, 512, 0}, - {"MX25L51235F", 0xc2201A, 0x0, 64 * 1024, 1024, 0}, - {"MX25L12855E", 0xc22618, 0x0, 64 * 1024, 256, 0}, + {"MX25L4005", 0xc22013, 0x0, 64 * 1024, 8, 0, 0}, + {"MX25L8005", 0xc22014, 0x0, 64 * 1024, 16, 0, 0}, + {"MX25L1605D", 0xc22015, 0x0, 64 * 1024, 32, 0, 0}, + {"MX25L3205D", 0xc22016, 0x0, 64 * 1024, 64, 0, 0}, + {"MX25L6405D", 0xc22017, 0x0, 64 * 1024, 128, 0, 0}, + {"MX25L12805", 0xc22018, 0x0, 64 * 1024, 256, 0, 0}, + {"MX25L12855E", 0xc22618, 0x0, 64 * 1024, 256, 0, 0}, #endif #ifdef CONFIG_SPI_FLASH_SPANSION /* SPANSION */ - {"S25FL008A", 0x010213, 0x0, 64 * 1024, 16, 0}, - {"S25FL016A", 0x010214, 0x0, 64 * 1024, 32, 0}, - {"S25FL032A", 0x010215, 0x0, 64 * 1024, 64, 0}, - {"S25FL064A", 0x010216, 0x0, 64 * 1024, 128, 0}, - {"S25FL128P_256K", 0x012018, 0x0300, 256 * 1024, 64, 0}, - {"S25FL128P_64K", 0x012018, 0x0301, 64 * 1024, 256, 0}, - {"S25FL032P", 0x010215, 0x4d00, 64 * 1024, 64, 0}, - {"S25FL064P", 0x010216, 0x4d00, 64 * 1024, 128, 0}, - {"S25FL128S_64K", 0x012018, 0x4d01, 64 * 1024, 256, 0}, - {"S25FL256S_256K", 0x010219, 0x4d00, 64 * 1024, 512, 0}, - {"S25FL256S_64K", 0x010219, 0x4d01, 64 * 1024, 512, 0}, - {"S25FL512S_256K", 0x010220, 0x4d00, 64 * 1024, 1024, 0}, - {"S25FL512S_64K", 0x010220, 0x4d01, 64 * 1024, 1024, 0}, + {"S25FL008A", 0x010213, 0x0, 64 * 1024, 16, 0, 0}, + {"S25FL016A", 0x010214, 0x0, 64 * 1024, 32, 0, 0}, + {"S25FL032A", 0x010215, 0x0, 64 * 1024, 64, 0, 0}, + {"S25FL064A", 0x010216, 0x0, 64 * 1024, 128, 0, 0}, + {"S25FL128P_256K", 0x012018, 0x0300, 256 * 1024, 64, RD_CMD_FULL, 0}, + {"S25FL128P_64K", 0x012018, 0x0301, 64 * 1024, 256, RD_CMD_FULL, 0}, + {"S25FL032P", 0x010215, 0x4d00, 64 * 1024, 64, RD_CMD_FULL, 0}, + {"S25FL064P", 0x010216, 0x4d00, 64 * 1024, 128, RD_CMD_FULL, 0}, + {"S25FL128S_64K", 0x012018, 0x4d01, 64 * 1024, 256, RD_CMD_FULL, 0}, + {"S25FL256S_64K", 0x010219, 0x4d01, 64 * 1024, 512, RD_CMD_FULL, 0}, + {"S25FL512S_64K", 0x010220, 0x4d01, 64 * 1024, 1024, RD_CMD_FULL, 0}, #endif #ifdef CONFIG_SPI_FLASH_STMICRO /* STMICRO */ - {"M25P10", 0x202011, 0x0, 32 * 1024, 4, 0}, - {"M25P20", 0x202012, 0x0, 64 * 1024, 4, 0}, - {"M25P40", 0x202013, 0x0, 64 * 1024, 8, 0}, - {"M25P80", 0x202014, 0x0, 64 * 1024, 16, 0}, - {"M25P16", 0x202015, 0x0, 64 * 1024, 32, 0}, - {"M25P32", 0x202016, 0x0, 64 * 1024, 64, 0}, - {"M25P64", 0x202017, 0x0, 64 * 1024, 128, 0}, - {"M25P128", 0x202018, 0x0, 256 * 1024, 64, 0}, - {"N25Q32", 0x20ba16, 0x0, 64 * 1024, 64, SECT_4K}, - {"N25Q32A", 0x20bb16, 0x0, 64 * 1024, 64, SECT_4K}, - {"N25Q64", 0x20ba17, 0x0, 64 * 1024, 128, SECT_4K}, - {"N25Q64A", 0x20bb17, 0x0, 64 * 1024, 128, SECT_4K}, - {"N25Q128", 0x20ba18, 0x0, 64 * 1024, 256, SECT_4K}, - {"N25Q128A", 0x20bb18, 0x0, 64 * 1024, 256, SECT_4K}, - {"N25Q256", 0x20ba19, 0x0, 64 * 1024, 512, SECT_4K}, - {"N25Q256A", 0x20bb19, 0x0, 64 * 1024, 512, SECT_4K}, - {"N25Q512", 0x20ba20, 0x0, 64 * 1024, 1024, E_FSR | SECT_4K}, - {"N25Q512A", 0x20bb20, 0x0, 64 * 1024, 1024, E_FSR | SECT_4K}, - {"N25Q1024", 0x20ba21, 0x0, 64 * 1024, 2048, E_FSR | SECT_4K}, - {"N25Q1024A", 0x20bb21, 0x0, 64 * 1024, 2048, E_FSR | SECT_4K}, + {"M25P10", 0x202011, 0x0, 32 * 1024, 4, 0, 0}, + {"M25P20", 0x202012, 0x0, 64 * 1024, 4, 0, 0}, + {"M25P40", 0x202013, 0x0, 64 * 1024, 8, 0, 0}, + {"M25P80", 0x202014, 0x0, 64 * 1024, 16, 0, 0}, + {"M25P16", 0x202015, 0x0, 64 * 1024, 32, 0, 0}, + {"M25P32", 0x202016, 0x0, 64 * 1024, 64, 0, 0}, + {"M25P64", 0x202017, 0x0, 64 * 1024, 128, 0, 0}, + {"M25P128", 0x202018, 0x0, 256 * 1024, 64, 0, 0}, + {"N25Q32", 0x20ba16, 0x0, 64 * 1024, 64, RD_CMD_FULL, SECT_4K}, + {"N25Q32A", 0x20bb16, 0x0, 64 * 1024, 64, RD_CMD_FULL, SECT_4K}, + {"N25Q64", 0x20ba17, 0x0, 64 * 1024, 128, RD_CMD_FULL, SECT_4K}, + {"N25Q64A", 0x20bb17, 0x0, 64 * 1024, 128, RD_CMD_FULL, SECT_4K}, + {"N25Q128", 0x20ba18, 0x0, 64 * 1024, 256, RD_CMD_FULL, SECT_4K}, + {"N25Q128A", 0x20bb18, 0x0, 64 * 1024, 256, RD_CMD_FULL, SECT_4K}, + {"N25Q256", 0x20ba19, 0x0, 64 * 1024, 512, RD_CMD_FULL, SECT_4K}, + {"N25Q256A", 0x20bb19, 0x0, 64 * 1024, 512, RD_CMD_FULL, SECT_4K}, + {"N25Q512", 0x20ba20, 0x0, 64 * 1024, 1024, RD_CMD_FULL, E_FSR | SECT_4K}, + {"N25Q512A", 0x20bb20, 0x0, 64 * 1024, 1024, RD_CMD_FULL, E_FSR | SECT_4K}, + {"N25Q1024", 0x20ba21, 0x0, 64 * 1024, 2048, RD_CMD_FULL, E_FSR | SECT_4K}, + {"N25Q1024A", 0x20bb21, 0x0, 64 * 1024, 2048, RD_CMD_FULL, E_FSR | SECT_4K}, #endif #ifdef CONFIG_SPI_FLASH_SST /* SST */ - {"SST25VF040B", 0xbf258d, 0x0, 64 * 1024, 8, SECT_4K | SST_WP}, - {"SST25VF080B", 0xbf258e, 0x0, 64 * 1024, 16, SECT_4K | SST_WP}, - {"SST25VF016B", 0xbf2541, 0x0, 64 * 1024, 32, SECT_4K | SST_WP}, - {"SST25VF032B", 0xbf254a, 0x0, 64 * 1024, 64, SECT_4K | SST_WP}, - {"SST25VF064C", 0xbf254b, 0x0, 64 * 1024, 128, SECT_4K}, - {"SST25WF512", 0xbf2501, 0x0, 64 * 1024, 1, SECT_4K | SST_WP}, - {"SST25WF010", 0xbf2502, 0x0, 64 * 1024, 2, SECT_4K | SST_WP}, - {"SST25WF020", 0xbf2503, 0x0, 64 * 1024, 4, SECT_4K | SST_WP}, - {"SST25WF040", 0xbf2504, 0x0, 64 * 1024, 8, SECT_4K | SST_WP}, - {"SST25WF080", 0xbf2505, 0x0, 64 * 1024, 16, SECT_4K | SST_WP}, + {"SST25VF040B", 0xbf258d, 0x0, 64 * 1024, 8, 0, SECT_4K | SST_WP}, + {"SST25VF080B", 0xbf258e, 0x0, 64 * 1024, 16, 0, SECT_4K | SST_WP}, + {"SST25VF016B", 0xbf2541, 0x0, 64 * 1024, 32, 0, SECT_4K | SST_WP}, + {"SST25VF032B", 0xbf254a, 0x0, 64 * 1024, 64, 0, SECT_4K | SST_WP}, + {"SST25VF064C", 0xbf254b, 0x0, 64 * 1024, 128, 0 SECT_4K}, + {"SST25WF512", 0xbf2501, 0x0, 64 * 1024, 1, 0, SECT_4K | SST_WP}, + {"SST25WF010", 0xbf2502, 0x0, 64 * 1024, 2, 0, SECT_4K | SST_WP}, + {"SST25WF020", 0xbf2503, 0x0, 64 * 1024, 4, 0, SECT_4K | SST_WP}, + {"SST25WF040", 0xbf2504, 0x0, 64 * 1024, 8, 0, SECT_4K | SST_WP}, + {"SST25WF080", 0xbf2505, 0x0, 64 * 1024, 16, 0, SECT_4K | SST_WP}, #endif #ifdef CONFIG_SPI_FLASH_WINBOND /* WINBOND */ - {"W25P80", 0xef2014, 0x0, 64 * 1024, 16, 0}, - {"W25P16", 0xef2015, 0x0, 64 * 1024, 32, 0}, - {"W25P32", 0xef2016, 0x0, 64 * 1024, 64, 0}, - {"W25X40", 0xef3013, 0x0, 4 * 1024, 128, SECT_4K}, - {"W25X16", 0xef3015, 0x0, 4 * 1024, 512, SECT_4K}, - {"W25X32", 0xef3016, 0x0, 4 * 1024, 1024, SECT_4K}, - {"W25X64", 0xef3017, 0x0, 4 * 1024, 2048, SECT_4K}, - {"W25Q80BL", 0xef4014, 0x0, 4 * 1024, 256, SECT_4K}, - {"W25Q16CL", 0xef4015, 0x0, 4 * 1024, 512, SECT_4K}, - {"W25Q32BV", 0xef4016, 0x0, 4 * 1024, 1024, SECT_4K}, - {"W25Q64CV", 0xef4017, 0x0, 4 * 1024, 2048, SECT_4K}, - {"W25Q128BV", 0xef4018, 0x0, 4 * 1024, 4096, SECT_4K}, - {"W25Q256", 0xef4019, 0x0, 4 * 1024, 8192, SECT_4K}, - {"W25Q80BW", 0xef5014, 0x0, 4 * 1024, 256, SECT_4K}, - {"W25Q16DW", 0xef6015, 0x0, 4 * 1024, 512, SECT_4K}, - {"W25Q32DW", 0xef6016, 0x0, 4 * 1024, 1024, SECT_4K}, - {"W25Q64DW", 0xef6017, 0x0, 4 * 1024, 2048, SECT_4K}, - {"W25Q128FW", 0xef6018, 0x0, 4 * 1024, 4096, SECT_4K}, + {"W25P80", 0xef2014, 0x0, 64 * 1024, 16, 0, 0}, + {"W25P16", 0xef2015, 0x0, 64 * 1024, 32, 0, 0}, + {"W25P32", 0xef2016, 0x0, 64 * 1024, 64, 0, 0}, + {"W25X40", 0xef3013, 0x0, 4 * 1024, 128, 0, SECT_4K}, + {"W25X16", 0xef3015, 0x0, 4 * 1024, 512, 0, SECT_4K}, + {"W25X32", 0xef3016, 0x0, 4 * 1024, 1024, 0, SECT_4K}, + {"W25X64", 0xef3017, 0x0, 4 * 1024, 2048, 0, SECT_4K}, + {"W25Q80BL", 0xef4014, 0x0, 4 * 1024, 256, RD_CMD_FULL, SECT_4K}, + {"W25Q16CL", 0xef4015, 0x0, 4 * 1024, 512, RD_CMD_FULL, SECT_4K}, + {"W25Q32BV", 0xef4016, 0x0, 4 * 1024, 1024, RD_CMD_FULL, SECT_4K}, + {"W25Q64CV", 0xef4017, 0x0, 4 * 1024, 2048, RD_CMD_FULL, SECT_4K}, + {"W25Q128BV", 0xef4018, 0x0, 4 * 1024, 4096, RD_CMD_FULL, SECT_4K}, + {"W25Q256", 0xef4019, 0x0, 4 * 1024, 8192, RD_CMD_FULL, SECT_4K}, + {"W25Q80BW", 0xef5014, 0x0, 4 * 1024, 256, RD_CMD_FULL, SECT_4K}, + {"W25Q16DW", 0xef6015, 0x0, 4 * 1024, 512, RD_CMD_FULL, SECT_4K}, + {"W25Q32DW", 0xef6016, 0x0, 4 * 1024, 1024, RD_CMD_FULL, SECT_4K}, + {"W25Q64DW", 0xef6017, 0x0, 4 * 1024, 2048, RD_CMD_FULL, SECT_4K}, + {"W25Q128FW", 0xef6018, 0x0, 4 * 1024, 4096, RD_CMD_FULL, SECT_4K}, #endif /* * Note: @@ -158,6 +163,7 @@ struct spi_flash *spi_flash_validate_ids(struct spi_slave *spi, u8 *idcode) int i; u16 jedec = idcode[1] << 8 | idcode[2]; u16 ext_jedec = idcode[3] << 8 | idcode[4]; + u8 cmd; /* Get the flash id (jedec = manuf_id + dev_id, ext_jedec) */ for (i = 0; i < ARRAY_SIZE(spi_flash_params_table); i++) { @@ -203,6 +209,16 @@ struct spi_flash *spi_flash_validate_ids(struct spi_slave *spi, u8 *idcode) flash->sector_size = params->sector_size; flash->size = flash->sector_size * params->nr_sectors; + /* Look for the fastest read cmd */ + cmd = fls(params->rd_cmd & flash->spi->rd_cmd); + if (cmd) { + cmd = spi_read_cmds_array[cmd - 1]; + flash->read_cmd = cmd; + } else { + /* Go for controller supported command */ + flash->read_cmd = flash->spi->rd_cmd; + } + /* Compute erase sector and command */ if (params->flags & SECT_4K) { flash->erase_cmd = CMD_ERASE_4K; 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; } return ptr; diff --git a/include/spi.h b/include/spi.h index c0dab57..093847e 100644 --- a/include/spi.h +++ b/include/spi.h @@ -40,11 +40,13 @@ * cs: ID of the chip select connected to the slave. * max_write_size: If non-zero, the maximum number of bytes which can * be written at once, excluding command bytes. + * rd_cmd: Read command. */ struct spi_slave { unsigned int bus; unsigned int cs; unsigned int max_write_size; + u8 rd_cmd; }; /*----------------------------------------------------------------------- diff --git a/include/spi_flash.h b/include/spi_flash.h index 2b1151d..e630913 100644 --- a/include/spi_flash.h +++ b/include/spi_flash.h @@ -27,6 +27,22 @@ #define SECT_32K (1 << 1) #define E_FSR (1 << 2) +/* Read commands */ +#define CMD_READ_ARRAY_SLOW 0x03 +#define CMD_READ_ARRAY_FAST 0x0b +#define CMD_READ_DUAL_OUTPUT_FAST 0x3b +#define CMD_READ_DUAL_IO_FAST 0xbb + +enum spi_read_cmds { + ARRAY_SLOW = 1 << 0, + ARRAY_FAST = 1 << 1, + DUAL_OUTPUT_FAST = 1 << 2, + DUAL_IO_FAST = 1 << 3, +}; + +#define RD_CMD_FULL ARRAY_SLOW | ARRAY_FAST | DUAL_OUTPUT_FAST | \ + DUAL_IO_FAST + /* SST specific macros */ #ifdef CONFIG_SPI_FLASH_SST # define SST_WP 0x01 /* Supports AAI word program */ @@ -48,6 +64,7 @@ * @bank_curr: Current flash bank * @poll_cmd: Poll cmd - for flash erase/program * @erase_cmd: Erase cmd 4K, 32K, 64K + * @read_cmd: Read cmd SA, FA, DOF, DIOF * @memory_map: Address of read-only SPI flash access * @read: Flash read ops * @write: Flash write ops @@ -68,6 +85,7 @@ struct spi_flash { #endif u8 poll_cmd; u8 erase_cmd; + u8 read_cmd; void *memory_map; int (*read)(struct spi_flash *flash, u32 offset, size_t len, void *buf);
Current sf uses FAST_READ command, this patch adds support to use the different/extended read command. This implementation will determine the fastest command by taking the supported commands from the flash and the controller, controller is always been a priority. Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com> --- Changes for v4: - none Changes for v3: - none Changes for v2: - none doc/SPI/status.txt | 2 +- drivers/mtd/spi/spi_flash_ops.c | 2 +- drivers/mtd/spi/spi_flash_probe.c | 180 +++++++++++++++++++++----------------- drivers/spi/spi.c | 2 + include/spi.h | 2 + include/spi_flash.h | 18 ++++ 6 files changed, 122 insertions(+), 84 deletions(-)