| Message ID | 1492689397-4704-3-git-send-email-clg@kaod.org |
|---|---|
| State | Changes Requested |
| Delegated to: | Cyrille Pitchen |
| Headers | show |
On 04/20/2017 01:56 PM, Cédric Le Goater wrote: > From: Robert Lippert <roblip@gmail.com> > > Implements support for the dual IO read mode on aspeed SMC/FMC > controllers which uses both MISO and MOSI lines for data during a read > to double the read bandwidth. > > Signed-off-by: Robert Lippert <rlippert@google.com> > [clg: adapted to mainline driver ] > Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Marek Vasut <marek.vasut@gmail.com>
Hi Cédric, Le 20/04/2017 à 13:56, Cédric Le Goater a écrit : > From: Robert Lippert <roblip@gmail.com> > > Implements support for the dual IO read mode on aspeed SMC/FMC > controllers which uses both MISO and MOSI lines for data during a read > to double the read bandwidth. > > Signed-off-by: Robert Lippert <rlippert@google.com> > [clg: adapted to mainline driver ] > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > drivers/mtd/spi-nor/aspeed-smc.c | 19 ++++++++++++++----- > 1 file changed, 14 insertions(+), 5 deletions(-) > > diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c > index 2940f2098420..183d950e621b 100644 > --- a/drivers/mtd/spi-nor/aspeed-smc.c > +++ b/drivers/mtd/spi-nor/aspeed-smc.c > @@ -372,12 +372,20 @@ static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from, > struct aspeed_smc_chip *chip = nor->priv; > int i; > u8 dummy = 0xFF; > + u32 ctl; > > aspeed_smc_start_user(nor); > aspeed_smc_send_cmd_addr(nor, nor->read_opcode, from); > for (i = 0; i < chip->nor.read_dummy / 8; i++) > aspeed_smc_write_to_ahb(chip->ahb_base, &dummy, sizeof(dummy)); > > + if (chip->nor.flash_read == SPI_NOR_DUAL) { > + /* Switch to dual I/O mode for data cycle */ > + ctl = readl(chip->ctl) & ~CONTROL_IO_MODE_MASK; > + ctl |= CONTROL_IO_DUAL_DATA; > + writel(ctl, chip->ctl); > + } > + As expected this patch doesn't apply as is to the spi-nor/next branch of l2-mtd because flash_read and SPI_NOR_{DUAL|QUAD} enums no longer exists. Besides it looks like the Aspeed controller can support SPI 1-2-2 as well as SPI 1-1-2. I see: #define CONTROL_IO_DUAL_DATA BIT(29) #define CONTROL_IO_DUAL_ADDR_DATA (BIT(29) | BIT(28)) Now spi_nor_scan() makes the difference between these 2 SPI protocols. Besides, you may also want to use the compatible DT string to make the difference between Aspeed controllers also supporting SPI 1-1-4 and SPI 1-4-4 and other controllers supporting only Dual SPI protocols so you could fill the hwcaps parameter of spi_nor_scan() as needed. So I think we should spend a little more time to rework this patch. Best regards, Cyrille > aspeed_smc_read_from_ahb(read_buf, chip->ahb_base, len); > aspeed_smc_stop_user(nor); > return len; > @@ -591,6 +599,9 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) > case SPI_NOR_FAST: > cmd = CONTROL_COMMAND_MODE_FREAD; > break; > + case SPI_NOR_DUAL: > + cmd = CONTROL_COMMAND_MODE_FREAD | CONTROL_IO_DUAL_DATA; > + break; > default: > dev_err(chip->nor.dev, "unsupported SPI read mode\n"); > return -EINVAL; > @@ -599,7 +610,7 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) > chip->ctl_val[smc_read] |= cmd | > CONTROL_IO_DUMMY_SET(chip->nor.read_dummy / 8); > > - dev_dbg(controller->dev, "base control register: %08x\n", > + dev_dbg(controller->dev, "read control register: %08x\n", > chip->ctl_val[smc_read]); > return 0; > } > @@ -670,11 +681,9 @@ static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller, > break; > > /* > - * TODO: Add support for SPI_NOR_QUAD and SPI_NOR_DUAL > - * attach when board support is present as determined > - * by of property. > + * Aspeed SoCs only support Dual mode > */ > - ret = spi_nor_scan(nor, NULL, SPI_NOR_NORMAL); > + ret = spi_nor_scan(nor, NULL, SPI_NOR_DUAL); > if (ret) > break; > >
On 06/21/2017 12:50 AM, Cyrille Pitchen wrote: > Hi Cédric, > > > Le 20/04/2017 à 13:56, Cédric Le Goater a écrit : >> From: Robert Lippert <roblip@gmail.com> >> >> Implements support for the dual IO read mode on aspeed SMC/FMC >> controllers which uses both MISO and MOSI lines for data during a read >> to double the read bandwidth. >> >> Signed-off-by: Robert Lippert <rlippert@google.com> >> [clg: adapted to mainline driver ] >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> drivers/mtd/spi-nor/aspeed-smc.c | 19 ++++++++++++++----- >> 1 file changed, 14 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c >> index 2940f2098420..183d950e621b 100644 >> --- a/drivers/mtd/spi-nor/aspeed-smc.c >> +++ b/drivers/mtd/spi-nor/aspeed-smc.c >> @@ -372,12 +372,20 @@ static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from, >> struct aspeed_smc_chip *chip = nor->priv; >> int i; >> u8 dummy = 0xFF; >> + u32 ctl; >> >> aspeed_smc_start_user(nor); >> aspeed_smc_send_cmd_addr(nor, nor->read_opcode, from); >> for (i = 0; i < chip->nor.read_dummy / 8; i++) >> aspeed_smc_write_to_ahb(chip->ahb_base, &dummy, sizeof(dummy)); >> >> + if (chip->nor.flash_read == SPI_NOR_DUAL) { >> + /* Switch to dual I/O mode for data cycle */ >> + ctl = readl(chip->ctl) & ~CONTROL_IO_MODE_MASK; >> + ctl |= CONTROL_IO_DUAL_DATA; >> + writel(ctl, chip->ctl); >> + } >> + > > As expected this patch doesn't apply as is to the spi-nor/next branch of > l2-mtd because flash_read and SPI_NOR_{DUAL|QUAD} enums no longer exists. > > Besides it looks like the Aspeed controller can support SPI 1-2-2 as > well as SPI 1-1-2. > > I see: > #define CONTROL_IO_DUAL_DATA BIT(29) > #define CONTROL_IO_DUAL_ADDR_DATA (BIT(29) | BIT(28)) > > Now spi_nor_scan() makes the difference between these 2 SPI protocols. > > Besides, you may also want to use the compatible DT string to make the > difference between Aspeed controllers also supporting SPI 1-1-4 and SPI > 1-4-4 and other controllers supporting only Dual SPI protocols so you > could fill the hwcaps parameter of spi_nor_scan() as needed. > > So I think we should spend a little more time to rework this patch. OK. I will take a look. Please consider taking : [PATCH 4/4] mtd: spi-nor: aspeed: use command mode for reads while I reconcile my patchset with the spi-nor tree. Thanks, C. > >> aspeed_smc_read_from_ahb(read_buf, chip->ahb_base, len); >> aspeed_smc_stop_user(nor); >> return len; >> @@ -591,6 +599,9 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) >> case SPI_NOR_FAST: >> cmd = CONTROL_COMMAND_MODE_FREAD; >> break; >> + case SPI_NOR_DUAL: >> + cmd = CONTROL_COMMAND_MODE_FREAD | CONTROL_IO_DUAL_DATA; >> + break; >> default: >> dev_err(chip->nor.dev, "unsupported SPI read mode\n"); >> return -EINVAL; >> @@ -599,7 +610,7 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) >> chip->ctl_val[smc_read] |= cmd | >> CONTROL_IO_DUMMY_SET(chip->nor.read_dummy / 8); >> >> - dev_dbg(controller->dev, "base control register: %08x\n", >> + dev_dbg(controller->dev, "read control register: %08x\n", >> chip->ctl_val[smc_read]); >> return 0; >> } >> @@ -670,11 +681,9 @@ static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller, >> break; >> >> /* >> - * TODO: Add support for SPI_NOR_QUAD and SPI_NOR_DUAL >> - * attach when board support is present as determined >> - * by of property. >> + * Aspeed SoCs only support Dual mode >> */ >> - ret = spi_nor_scan(nor, NULL, SPI_NOR_NORMAL); >> + ret = spi_nor_scan(nor, NULL, SPI_NOR_DUAL); >> if (ret) >> break; >> >> >
On 06/21/2017 09:32 AM, Cédric Le Goater wrote: > On 06/21/2017 12:50 AM, Cyrille Pitchen wrote: >> Hi Cédric, >> >> >> Le 20/04/2017 à 13:56, Cédric Le Goater a écrit : >>> From: Robert Lippert <roblip@gmail.com> >>> >>> Implements support for the dual IO read mode on aspeed SMC/FMC >>> controllers which uses both MISO and MOSI lines for data during a read >>> to double the read bandwidth. >>> >>> Signed-off-by: Robert Lippert <rlippert@google.com> >>> [clg: adapted to mainline driver ] >>> Signed-off-by: Cédric Le Goater <clg@kaod.org> >>> --- >>> drivers/mtd/spi-nor/aspeed-smc.c | 19 ++++++++++++++----- >>> 1 file changed, 14 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c >>> index 2940f2098420..183d950e621b 100644 >>> --- a/drivers/mtd/spi-nor/aspeed-smc.c >>> +++ b/drivers/mtd/spi-nor/aspeed-smc.c >>> @@ -372,12 +372,20 @@ static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from, >>> struct aspeed_smc_chip *chip = nor->priv; >>> int i; >>> u8 dummy = 0xFF; >>> + u32 ctl; >>> >>> aspeed_smc_start_user(nor); >>> aspeed_smc_send_cmd_addr(nor, nor->read_opcode, from); >>> for (i = 0; i < chip->nor.read_dummy / 8; i++) >>> aspeed_smc_write_to_ahb(chip->ahb_base, &dummy, sizeof(dummy)); >>> >>> + if (chip->nor.flash_read == SPI_NOR_DUAL) { >>> + /* Switch to dual I/O mode for data cycle */ >>> + ctl = readl(chip->ctl) & ~CONTROL_IO_MODE_MASK; >>> + ctl |= CONTROL_IO_DUAL_DATA; >>> + writel(ctl, chip->ctl); >>> + } >>> + >> >> As expected this patch doesn't apply as is to the spi-nor/next branch of >> l2-mtd because flash_read and SPI_NOR_{DUAL|QUAD} enums no longer exists. >> >> Besides it looks like the Aspeed controller can support SPI 1-2-2 as >> well as SPI 1-1-2. >> >> I see: >> #define CONTROL_IO_DUAL_DATA BIT(29) >> #define CONTROL_IO_DUAL_ADDR_DATA (BIT(29) | BIT(28)) >> >> Now spi_nor_scan() makes the difference between these 2 SPI protocols. >> >> Besides, you may also want to use the compatible DT string to make the >> difference between Aspeed controllers also supporting SPI 1-1-4 and SPI >> 1-4-4 and other controllers supporting only Dual SPI protocols so you >> could fill the hwcaps parameter of spi_nor_scan() as needed. >> >> So I think we should spend a little more time to rework this patch. > > OK. I will take a look. > > Please consider taking : > > [PATCH 4/4] mtd: spi-nor: aspeed: use command mode for reads Forget it. I am having issues with this patch on newer kernels. I will cook a patchset after the merge window. Thanks, C. > while I reconcile my patchset with the spi-nor tree. > > Thanks, > > C. > >> >>> aspeed_smc_read_from_ahb(read_buf, chip->ahb_base, len); >>> aspeed_smc_stop_user(nor); >>> return len; >>> @@ -591,6 +599,9 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) >>> case SPI_NOR_FAST: >>> cmd = CONTROL_COMMAND_MODE_FREAD; >>> break; >>> + case SPI_NOR_DUAL: >>> + cmd = CONTROL_COMMAND_MODE_FREAD | CONTROL_IO_DUAL_DATA; >>> + break; >>> default: >>> dev_err(chip->nor.dev, "unsupported SPI read mode\n"); >>> return -EINVAL; >>> @@ -599,7 +610,7 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) >>> chip->ctl_val[smc_read] |= cmd | >>> CONTROL_IO_DUMMY_SET(chip->nor.read_dummy / 8); >>> >>> - dev_dbg(controller->dev, "base control register: %08x\n", >>> + dev_dbg(controller->dev, "read control register: %08x\n", >>> chip->ctl_val[smc_read]); >>> return 0; >>> } >>> @@ -670,11 +681,9 @@ static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller, >>> break; >>> >>> /* >>> - * TODO: Add support for SPI_NOR_QUAD and SPI_NOR_DUAL >>> - * attach when board support is present as determined >>> - * by of property. >>> + * Aspeed SoCs only support Dual mode >>> */ >>> - ret = spi_nor_scan(nor, NULL, SPI_NOR_NORMAL); >>> + ret = spi_nor_scan(nor, NULL, SPI_NOR_DUAL); >>> if (ret) >>> break; >>> >>> >> >
Le 21/06/2017 à 14:25, Cédric Le Goater a écrit : > On 06/21/2017 09:32 AM, Cédric Le Goater wrote: >> On 06/21/2017 12:50 AM, Cyrille Pitchen wrote: >>> Hi Cédric, >>> >>> >>> Le 20/04/2017 à 13:56, Cédric Le Goater a écrit : >>>> From: Robert Lippert <roblip@gmail.com> >>>> >>>> Implements support for the dual IO read mode on aspeed SMC/FMC >>>> controllers which uses both MISO and MOSI lines for data during a read >>>> to double the read bandwidth. >>>> >>>> Signed-off-by: Robert Lippert <rlippert@google.com> >>>> [clg: adapted to mainline driver ] >>>> Signed-off-by: Cédric Le Goater <clg@kaod.org> >>>> --- >>>> drivers/mtd/spi-nor/aspeed-smc.c | 19 ++++++++++++++----- >>>> 1 file changed, 14 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c >>>> index 2940f2098420..183d950e621b 100644 >>>> --- a/drivers/mtd/spi-nor/aspeed-smc.c >>>> +++ b/drivers/mtd/spi-nor/aspeed-smc.c >>>> @@ -372,12 +372,20 @@ static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from, >>>> struct aspeed_smc_chip *chip = nor->priv; >>>> int i; >>>> u8 dummy = 0xFF; >>>> + u32 ctl; >>>> >>>> aspeed_smc_start_user(nor); >>>> aspeed_smc_send_cmd_addr(nor, nor->read_opcode, from); >>>> for (i = 0; i < chip->nor.read_dummy / 8; i++) >>>> aspeed_smc_write_to_ahb(chip->ahb_base, &dummy, sizeof(dummy)); >>>> >>>> + if (chip->nor.flash_read == SPI_NOR_DUAL) { >>>> + /* Switch to dual I/O mode for data cycle */ >>>> + ctl = readl(chip->ctl) & ~CONTROL_IO_MODE_MASK; >>>> + ctl |= CONTROL_IO_DUAL_DATA; >>>> + writel(ctl, chip->ctl); >>>> + } >>>> + >>> >>> As expected this patch doesn't apply as is to the spi-nor/next branch of >>> l2-mtd because flash_read and SPI_NOR_{DUAL|QUAD} enums no longer exists. >>> >>> Besides it looks like the Aspeed controller can support SPI 1-2-2 as >>> well as SPI 1-1-2. >>> >>> I see: >>> #define CONTROL_IO_DUAL_DATA BIT(29) >>> #define CONTROL_IO_DUAL_ADDR_DATA (BIT(29) | BIT(28)) >>> >>> Now spi_nor_scan() makes the difference between these 2 SPI protocols. >>> >>> Besides, you may also want to use the compatible DT string to make the >>> difference between Aspeed controllers also supporting SPI 1-1-4 and SPI >>> 1-4-4 and other controllers supporting only Dual SPI protocols so you >>> could fill the hwcaps parameter of spi_nor_scan() as needed. >>> >>> So I think we should spend a little more time to rework this patch. >> >> OK. I will take a look. >> >> Please consider taking : >> >> [PATCH 4/4] mtd: spi-nor: aspeed: use command mode for reads > > Forget it. I am having issues with this patch on newer kernels. I will > cook a patchset after the merge window. OK, so I've updated the state of patch 4 to "rejected" in patchwork. Best regards, Cyrille > > Thanks, > > C. > > >> while I reconcile my patchset with the spi-nor tree. >> >> Thanks, >> >> C. >> >>> >>>> aspeed_smc_read_from_ahb(read_buf, chip->ahb_base, len); >>>> aspeed_smc_stop_user(nor); >>>> return len; >>>> @@ -591,6 +599,9 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) >>>> case SPI_NOR_FAST: >>>> cmd = CONTROL_COMMAND_MODE_FREAD; >>>> break; >>>> + case SPI_NOR_DUAL: >>>> + cmd = CONTROL_COMMAND_MODE_FREAD | CONTROL_IO_DUAL_DATA; >>>> + break; >>>> default: >>>> dev_err(chip->nor.dev, "unsupported SPI read mode\n"); >>>> return -EINVAL; >>>> @@ -599,7 +610,7 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) >>>> chip->ctl_val[smc_read] |= cmd | >>>> CONTROL_IO_DUMMY_SET(chip->nor.read_dummy / 8); >>>> >>>> - dev_dbg(controller->dev, "base control register: %08x\n", >>>> + dev_dbg(controller->dev, "read control register: %08x\n", >>>> chip->ctl_val[smc_read]); >>>> return 0; >>>> } >>>> @@ -670,11 +681,9 @@ static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller, >>>> break; >>>> >>>> /* >>>> - * TODO: Add support for SPI_NOR_QUAD and SPI_NOR_DUAL >>>> - * attach when board support is present as determined >>>> - * by of property. >>>> + * Aspeed SoCs only support Dual mode >>>> */ >>>> - ret = spi_nor_scan(nor, NULL, SPI_NOR_NORMAL); >>>> + ret = spi_nor_scan(nor, NULL, SPI_NOR_DUAL); >>>> if (ret) >>>> break; >>>> >>>> >>> >> > >
diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c index 2940f2098420..183d950e621b 100644 --- a/drivers/mtd/spi-nor/aspeed-smc.c +++ b/drivers/mtd/spi-nor/aspeed-smc.c @@ -372,12 +372,20 @@ static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from, struct aspeed_smc_chip *chip = nor->priv; int i; u8 dummy = 0xFF; + u32 ctl; aspeed_smc_start_user(nor); aspeed_smc_send_cmd_addr(nor, nor->read_opcode, from); for (i = 0; i < chip->nor.read_dummy / 8; i++) aspeed_smc_write_to_ahb(chip->ahb_base, &dummy, sizeof(dummy)); + if (chip->nor.flash_read == SPI_NOR_DUAL) { + /* Switch to dual I/O mode for data cycle */ + ctl = readl(chip->ctl) & ~CONTROL_IO_MODE_MASK; + ctl |= CONTROL_IO_DUAL_DATA; + writel(ctl, chip->ctl); + } + aspeed_smc_read_from_ahb(read_buf, chip->ahb_base, len); aspeed_smc_stop_user(nor); return len; @@ -591,6 +599,9 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) case SPI_NOR_FAST: cmd = CONTROL_COMMAND_MODE_FREAD; break; + case SPI_NOR_DUAL: + cmd = CONTROL_COMMAND_MODE_FREAD | CONTROL_IO_DUAL_DATA; + break; default: dev_err(chip->nor.dev, "unsupported SPI read mode\n"); return -EINVAL; @@ -599,7 +610,7 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip) chip->ctl_val[smc_read] |= cmd | CONTROL_IO_DUMMY_SET(chip->nor.read_dummy / 8); - dev_dbg(controller->dev, "base control register: %08x\n", + dev_dbg(controller->dev, "read control register: %08x\n", chip->ctl_val[smc_read]); return 0; } @@ -670,11 +681,9 @@ static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller, break; /* - * TODO: Add support for SPI_NOR_QUAD and SPI_NOR_DUAL - * attach when board support is present as determined - * by of property. + * Aspeed SoCs only support Dual mode */ - ret = spi_nor_scan(nor, NULL, SPI_NOR_NORMAL); + ret = spi_nor_scan(nor, NULL, SPI_NOR_DUAL); if (ret) break;