[PATCH v2 0/2] mtd: spi-nor: aspeed: add dual read and command mode support

[PATCH v2 0/2] mtd: spi-nor: aspeed: add dual read and command mode support

[Cédric Le Goater]

Hello,

Here is a short series adding support for Dual read and for the
controller mode "Command". The later is to perform direct memory
access of the flash contents on the AHB Bus. A prereq to use this mode
is to configure the segment registers of the controller which define
the mapping window of the chips on the AHB bus. 

Quad read is only available on the AST2400 SPI controller and not on
any of the AST2500 controllers, I am pondering on the need for the
support. It needs some testing anyhow first.

Thanks,

C.

Changes since v1:

 - reintroduced the setting of the SPI NOR command in the control
   register, which was forgotten in the previous rebase of the patch.
 - reworked dual io support to fit the new spi-nor hwcaps
 - added dual address and data IO
 - took ownership due to the amount of rewritten code.

Cédric Le Goater (2):
  mtd: spi-nor: aspeed: add support for SPI dual IO read mode
  mtd: spi-nor: aspeed: use command mode for reads

 drivers/mtd/spi-nor/aspeed-smc.c | 80 ++++++++++++++++++++++++++++++++++------
 1 file changed, 68 insertions(+), 12 deletions(-)

-- 
2.7.5

[PATCH v2 1/2] mtd: spi-nor: aspeed: add support for SPI dual IO read mode

[Cédric Le Goater]

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.

Based on work from Robert Lippert <roblip@gmail.com>

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Cc: Robert Lippert <roblip@gmail.com>
---

 Changes since v1:
 
 - reworked the patch to fit the new spi-nor hwcaps
 - added dual address and data IO
 - took ownership due to the amount of rewritten code.
 
 drivers/mtd/spi-nor/aspeed-smc.c | 52 +++++++++++++++++++++++++++++++---------
 1 file changed, 41 insertions(+), 11 deletions(-)

diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c
index 0106357421bd..93ca2ee65f51 100644
--- a/drivers/mtd/spi-nor/aspeed-smc.c
+++ b/drivers/mtd/spi-nor/aspeed-smc.c
@@ -373,6 +373,33 @@ static void aspeed_smc_send_cmd_addr(struct spi_nor *nor, u8 cmd, u32 addr)
 	}
 }
 
+static int aspeed_smc_get_io_mode(struct aspeed_smc_chip *chip)
+{
+	switch (chip->nor.read_proto) {
+	case SNOR_PROTO_1_1_1:
+		return 0;
+	case SNOR_PROTO_1_1_2:
+		return CONTROL_IO_DUAL_DATA;
+	case SNOR_PROTO_1_2_2:
+		return CONTROL_IO_DUAL_ADDR_DATA;
+	default:
+		dev_err(chip->nor.dev, "unsupported SPI read mode\n");
+		return -EINVAL;
+	}
+}
+
+static void aspeed_smc_set_io_mode(struct aspeed_smc_chip *chip)
+{
+	u32 io_mode = aspeed_smc_get_io_mode(chip);
+	u32 ctl;
+
+	if (io_mode > 0) {
+		ctl = readl(chip->ctl) & ~CONTROL_IO_MODE_MASK;
+		ctl |= io_mode;
+		writel(ctl, chip->ctl);
+	}
+}
+
 static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from,
 				    size_t len, u_char *read_buf)
 {
@@ -385,6 +412,7 @@ static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from,
 	for (i = 0; i < chip->nor.read_dummy / 8; i++)
 		aspeed_smc_write_to_ahb(chip->ahb_base, &dummy, sizeof(dummy));
 
+	aspeed_smc_set_io_mode(chip);
 	aspeed_smc_read_from_ahb(read_buf, chip->ahb_base, len);
 	aspeed_smc_stop_user(nor);
 	return len;
@@ -711,6 +739,7 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip)
 {
 	struct aspeed_smc_controller *controller = chip->controller;
 	const struct aspeed_smc_info *info = controller->info;
+	u32 io_mode;
 	u32 cmd;
 
 	if (chip->nor.addr_width == 4 && info->set_4b)
@@ -733,20 +762,19 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip)
 	 * TODO: Adjust clocks if fast read is supported and interpret
 	 * SPI-NOR flags to adjust controller settings.
 	 */
-	if (chip->nor.read_proto == SNOR_PROTO_1_1_1) {
-		if (chip->nor.read_dummy == 0)
-			cmd = CONTROL_COMMAND_MODE_NORMAL;
-		else
-			cmd = CONTROL_COMMAND_MODE_FREAD;
-	} else {
-		dev_err(chip->nor.dev, "unsupported SPI read mode\n");
-		return -EINVAL;
-	}
+	io_mode = aspeed_smc_get_io_mode(chip);
+	if (io_mode < 0)
+		return io_mode;
+
+	if (chip->nor.read_dummy == 0)
+		cmd = CONTROL_COMMAND_MODE_NORMAL;
+	else
+		cmd = CONTROL_COMMAND_MODE_FREAD;
 
-	chip->ctl_val[smc_read] |= cmd |
+	chip->ctl_val[smc_read] |= cmd | io_mode |
 		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;
 }
@@ -757,6 +785,8 @@ static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller,
 	const struct spi_nor_hwcaps hwcaps = {
 		.mask = SNOR_HWCAPS_READ |
 			SNOR_HWCAPS_READ_FAST |
+			SNOR_HWCAPS_READ_1_1_2 |
+			SNOR_HWCAPS_READ_1_2_2 |
 			SNOR_HWCAPS_PP,
 	};
 	const struct aspeed_smc_info *info = controller->info;
-- 
2.7.5

Re: [PATCH v2 1/2] mtd: spi-nor: aspeed: add support for SPI dual IO read mode

[Rob Lippert]

On Thu, Jun 22, 2017 at 12:18 AM, Cédric Le Goater <clg@kaod.org> wrote:
> 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.
>
> Based on work from Robert Lippert <roblip@gmail.com>
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Cc: Robert Lippert <roblip@gmail.com>
> ---
>
>  Changes since v1:
>
>  - reworked the patch to fit the new spi-nor hwcaps
>  - added dual address and data IO
>  - took ownership due to the amount of rewritten code.
>
>  drivers/mtd/spi-nor/aspeed-smc.c | 52 +++++++++++++++++++++++++++++++---------
>  1 file changed, 41 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c
> index 0106357421bd..93ca2ee65f51 100644
> --- a/drivers/mtd/spi-nor/aspeed-smc.c
> +++ b/drivers/mtd/spi-nor/aspeed-smc.c
> @@ -373,6 +373,33 @@ static void aspeed_smc_send_cmd_addr(struct spi_nor *nor, u8 cmd, u32 addr)
>         }
>  }
>
> +static int aspeed_smc_get_io_mode(struct aspeed_smc_chip *chip)
> +{
> +       switch (chip->nor.read_proto) {
> +       case SNOR_PROTO_1_1_1:
> +               return 0;
> +       case SNOR_PROTO_1_1_2:
> +               return CONTROL_IO_DUAL_DATA;
> +       case SNOR_PROTO_1_2_2:
> +               return CONTROL_IO_DUAL_ADDR_DATA;
> +       default:
> +               dev_err(chip->nor.dev, "unsupported SPI read mode\n");
> +               return -EINVAL;
> +       }
> +}
> +
> +static void aspeed_smc_set_io_mode(struct aspeed_smc_chip *chip)
> +{
> +       u32 io_mode = aspeed_smc_get_io_mode(chip);
> +       u32 ctl;
> +
> +       if (io_mode > 0) {
> +               ctl = readl(chip->ctl) & ~CONTROL_IO_MODE_MASK;
> +               ctl |= io_mode;
> +               writel(ctl, chip->ctl);
> +       }
> +}
> +
>  static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from,
>                                     size_t len, u_char *read_buf)
>  {
> @@ -385,6 +412,7 @@ static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from,
>         for (i = 0; i < chip->nor.read_dummy / 8; i++)
>                 aspeed_smc_write_to_ahb(chip->ahb_base, &dummy, sizeof(dummy));
>
> +       aspeed_smc_set_io_mode(chip);

Does this actually work for 1_2_2 mode?  I figured you would need to
set it to dual mode before sending the address (but after the command)
a few lines up from here.

>         aspeed_smc_read_from_ahb(read_buf, chip->ahb_base, len);
>         aspeed_smc_stop_user(nor);
>         return len;
> @@ -711,6 +739,7 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip)
>  {
>         struct aspeed_smc_controller *controller = chip->controller;
>         const struct aspeed_smc_info *info = controller->info;
> +       u32 io_mode;
>         u32 cmd;
>
>         if (chip->nor.addr_width == 4 && info->set_4b)
> @@ -733,20 +762,19 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip)
>          * TODO: Adjust clocks if fast read is supported and interpret
>          * SPI-NOR flags to adjust controller settings.
>          */
> -       if (chip->nor.read_proto == SNOR_PROTO_1_1_1) {
> -               if (chip->nor.read_dummy == 0)
> -                       cmd = CONTROL_COMMAND_MODE_NORMAL;
> -               else
> -                       cmd = CONTROL_COMMAND_MODE_FREAD;
> -       } else {
> -               dev_err(chip->nor.dev, "unsupported SPI read mode\n");
> -               return -EINVAL;
> -       }
> +       io_mode = aspeed_smc_get_io_mode(chip);
> +       if (io_mode < 0)
> +               return io_mode;
> +
> +       if (chip->nor.read_dummy == 0)
> +               cmd = CONTROL_COMMAND_MODE_NORMAL;
> +       else
> +               cmd = CONTROL_COMMAND_MODE_FREAD;
>
> -       chip->ctl_val[smc_read] |= cmd |
> +       chip->ctl_val[smc_read] |= cmd | io_mode |
>                 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;
>  }
> @@ -757,6 +785,8 @@ static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller,
>         const struct spi_nor_hwcaps hwcaps = {
>                 .mask = SNOR_HWCAPS_READ |
>                         SNOR_HWCAPS_READ_FAST |
> +                       SNOR_HWCAPS_READ_1_1_2 |
> +                       SNOR_HWCAPS_READ_1_2_2 |
>                         SNOR_HWCAPS_PP,
>         };
>         const struct aspeed_smc_info *info = controller->info;
> --
> 2.7.5
>

Re: [PATCH v2 1/2] mtd: spi-nor: aspeed: add support for SPI dual IO read mode

[Cédric Le Goater]

On 06/22/2017 07:17 PM, Rob Lippert wrote:
> On Thu, Jun 22, 2017 at 12:18 AM, Cédric Le Goater <clg@kaod.org> wrote:
>> 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.
>>
>> Based on work from Robert Lippert <roblip@gmail.com>
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> Cc: Robert Lippert <roblip@gmail.com>
>> ---
>>
>>  Changes since v1:
>>
>>  - reworked the patch to fit the new spi-nor hwcaps
>>  - added dual address and data IO
>>  - took ownership due to the amount of rewritten code.
>>
>>  drivers/mtd/spi-nor/aspeed-smc.c | 52 +++++++++++++++++++++++++++++++---------
>>  1 file changed, 41 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c
>> index 0106357421bd..93ca2ee65f51 100644
>> --- a/drivers/mtd/spi-nor/aspeed-smc.c
>> +++ b/drivers/mtd/spi-nor/aspeed-smc.c
>> @@ -373,6 +373,33 @@ static void aspeed_smc_send_cmd_addr(struct spi_nor *nor, u8 cmd, u32 addr)
>>         }
>>  }
>>
>> +static int aspeed_smc_get_io_mode(struct aspeed_smc_chip *chip)
>> +{
>> +       switch (chip->nor.read_proto) {
>> +       case SNOR_PROTO_1_1_1:
>> +               return 0;
>> +       case SNOR_PROTO_1_1_2:
>> +               return CONTROL_IO_DUAL_DATA;
>> +       case SNOR_PROTO_1_2_2:
>> +               return CONTROL_IO_DUAL_ADDR_DATA;
>> +       default:
>> +               dev_err(chip->nor.dev, "unsupported SPI read mode\n");
>> +               return -EINVAL;
>> +       }
>> +}
>> +
>> +static void aspeed_smc_set_io_mode(struct aspeed_smc_chip *chip)
>> +{
>> +       u32 io_mode = aspeed_smc_get_io_mode(chip);
>> +       u32 ctl;
>> +
>> +       if (io_mode > 0) {
>> +               ctl = readl(chip->ctl) & ~CONTROL_IO_MODE_MASK;
>> +               ctl |= io_mode;
>> +               writel(ctl, chip->ctl);
>> +       }
>> +}
>> +
>>  static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from,
>>                                     size_t len, u_char *read_buf)
>>  {
>> @@ -385,6 +412,7 @@ static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from,
>>         for (i = 0; i < chip->nor.read_dummy / 8; i++)
>>                 aspeed_smc_write_to_ahb(chip->ahb_base, &dummy, sizeof(dummy));
>>
>> +       aspeed_smc_set_io_mode(chip);
> 
> Does this actually work for 1_2_2 mode?  I figured you would need to
> set it to dual mode before sending the address (but after the command)
> a few lines up from here.

yes. This needs a fix for CONTROL_IO_DUAL_ADDR_DATA. 

Thanks,

C.

> 
>>         aspeed_smc_read_from_ahb(read_buf, chip->ahb_base, len);
>>         aspeed_smc_stop_user(nor);
>>         return len;
>> @@ -711,6 +739,7 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip)
>>  {
>>         struct aspeed_smc_controller *controller = chip->controller;
>>         const struct aspeed_smc_info *info = controller->info;
>> +       u32 io_mode;
>>         u32 cmd;
>>
>>         if (chip->nor.addr_width == 4 && info->set_4b)
>> @@ -733,20 +762,19 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip)
>>          * TODO: Adjust clocks if fast read is supported and interpret
>>          * SPI-NOR flags to adjust controller settings.
>>          */
>> -       if (chip->nor.read_proto == SNOR_PROTO_1_1_1) {
>> -               if (chip->nor.read_dummy == 0)
>> -                       cmd = CONTROL_COMMAND_MODE_NORMAL;
>> -               else
>> -                       cmd = CONTROL_COMMAND_MODE_FREAD;
>> -       } else {
>> -               dev_err(chip->nor.dev, "unsupported SPI read mode\n");
>> -               return -EINVAL;
>> -       }
>> +       io_mode = aspeed_smc_get_io_mode(chip);
>> +       if (io_mode < 0)
>> +               return io_mode;
>> +
>> +       if (chip->nor.read_dummy == 0)
>> +               cmd = CONTROL_COMMAND_MODE_NORMAL;
>> +       else
>> +               cmd = CONTROL_COMMAND_MODE_FREAD;
>>
>> -       chip->ctl_val[smc_read] |= cmd |
>> +       chip->ctl_val[smc_read] |= cmd | io_mode |
>>                 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;
>>  }
>> @@ -757,6 +785,8 @@ static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller,
>>         const struct spi_nor_hwcaps hwcaps = {
>>                 .mask = SNOR_HWCAPS_READ |
>>                         SNOR_HWCAPS_READ_FAST |
>> +                       SNOR_HWCAPS_READ_1_1_2 |
>> +                       SNOR_HWCAPS_READ_1_2_2 |
>>                         SNOR_HWCAPS_PP,
>>         };
>>         const struct aspeed_smc_info *info = controller->info;
>> --
>> 2.7.5
>>

Re: [PATCH v2 1/2] mtd: spi-nor: aspeed: add support for SPI dual IO read mode

[Cédric Le Goater]

On 06/23/2017 08:35 AM, Cédric Le Goater wrote:
> On 06/22/2017 07:17 PM, Rob Lippert wrote:
>> On Thu, Jun 22, 2017 at 12:18 AM, Cédric Le Goater <clg@kaod.org> wrote:
>>> 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.
>>>
>>> Based on work from Robert Lippert <roblip@gmail.com>
>>>
>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>> Cc: Robert Lippert <roblip@gmail.com>
>>> ---
>>>
>>>  Changes since v1:
>>>
>>>  - reworked the patch to fit the new spi-nor hwcaps
>>>  - added dual address and data IO
>>>  - took ownership due to the amount of rewritten code.
>>>
>>>  drivers/mtd/spi-nor/aspeed-smc.c | 52 +++++++++++++++++++++++++++++++---------
>>>  1 file changed, 41 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c
>>> index 0106357421bd..93ca2ee65f51 100644
>>> --- a/drivers/mtd/spi-nor/aspeed-smc.c
>>> +++ b/drivers/mtd/spi-nor/aspeed-smc.c
>>> @@ -373,6 +373,33 @@ static void aspeed_smc_send_cmd_addr(struct spi_nor *nor, u8 cmd, u32 addr)
>>>         }
>>>  }
>>>
>>> +static int aspeed_smc_get_io_mode(struct aspeed_smc_chip *chip)
>>> +{
>>> +       switch (chip->nor.read_proto) {
>>> +       case SNOR_PROTO_1_1_1:
>>> +               return 0;
>>> +       case SNOR_PROTO_1_1_2:
>>> +               return CONTROL_IO_DUAL_DATA;
>>> +       case SNOR_PROTO_1_2_2:
>>> +               return CONTROL_IO_DUAL_ADDR_DATA;
>>> +       default:
>>> +               dev_err(chip->nor.dev, "unsupported SPI read mode\n");
>>> +               return -EINVAL;
>>> +       }
>>> +}
>>> +
>>> +static void aspeed_smc_set_io_mode(struct aspeed_smc_chip *chip)
>>> +{
>>> +       u32 io_mode = aspeed_smc_get_io_mode(chip);
>>> +       u32 ctl;
>>> +
>>> +       if (io_mode > 0) {
>>> +               ctl = readl(chip->ctl) & ~CONTROL_IO_MODE_MASK;
>>> +               ctl |= io_mode;
>>> +               writel(ctl, chip->ctl);
>>> +       }
>>> +}
>>> +
>>>  static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from,
>>>                                     size_t len, u_char *read_buf)
>>>  {
>>> @@ -385,6 +412,7 @@ static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from,
>>>         for (i = 0; i < chip->nor.read_dummy / 8; i++)
>>>                 aspeed_smc_write_to_ahb(chip->ahb_base, &dummy, sizeof(dummy));
>>>
>>> +       aspeed_smc_set_io_mode(chip);
>>
>> Does this actually work for 1_2_2 mode?  I figured you would need to
>> set it to dual mode before sending the address (but after the command)
>> a few lines up from here.
> 
> yes. This needs a fix for CONTROL_IO_DUAL_ADDR_DATA. 

I was wondering why I didn't see the BB command being used.
The spi-nor layer only provides the SNOR_PROTO_1_2_2 definition
but only the SNOR_PROTO_1_1_2 is handled for the moment. 
Any how, this patch needs some more work.  

Thanks,

C. 

Re: [PATCH v2 1/2] mtd: spi-nor: aspeed: add support for SPI dual IO read mode

[Cyrille Pitchen]

Hi Cédric,

Le 23/06/2017 à 09:02, Cédric Le Goater a écrit :
> On 06/23/2017 08:35 AM, Cédric Le Goater wrote:
>> On 06/22/2017 07:17 PM, Rob Lippert wrote:
>>> On Thu, Jun 22, 2017 at 12:18 AM, Cédric Le Goater <clg@kaod.org> wrote:
>>>> 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.
>>>>
>>>> Based on work from Robert Lippert <roblip@gmail.com>
>>>>
>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>>> Cc: Robert Lippert <roblip@gmail.com>
>>>> ---
>>>>
>>>>  Changes since v1:
>>>>
>>>>  - reworked the patch to fit the new spi-nor hwcaps
>>>>  - added dual address and data IO
>>>>  - took ownership due to the amount of rewritten code.
>>>>
>>>>  drivers/mtd/spi-nor/aspeed-smc.c | 52 +++++++++++++++++++++++++++++++---------
>>>>  1 file changed, 41 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c
>>>> index 0106357421bd..93ca2ee65f51 100644
>>>> --- a/drivers/mtd/spi-nor/aspeed-smc.c
>>>> +++ b/drivers/mtd/spi-nor/aspeed-smc.c
>>>> @@ -373,6 +373,33 @@ static void aspeed_smc_send_cmd_addr(struct spi_nor *nor, u8 cmd, u32 addr)
>>>>         }
>>>>  }
>>>>
>>>> +static int aspeed_smc_get_io_mode(struct aspeed_smc_chip *chip)
>>>> +{
>>>> +       switch (chip->nor.read_proto) {
>>>> +       case SNOR_PROTO_1_1_1:
>>>> +               return 0;
>>>> +       case SNOR_PROTO_1_1_2:
>>>> +               return CONTROL_IO_DUAL_DATA;
>>>> +       case SNOR_PROTO_1_2_2:
>>>> +               return CONTROL_IO_DUAL_ADDR_DATA;
>>>> +       default:
>>>> +               dev_err(chip->nor.dev, "unsupported SPI read mode\n");
>>>> +               return -EINVAL;
>>>> +       }
>>>> +}
>>>> +
>>>> +static void aspeed_smc_set_io_mode(struct aspeed_smc_chip *chip)
>>>> +{
>>>> +       u32 io_mode = aspeed_smc_get_io_mode(chip);
>>>> +       u32 ctl;
>>>> +
>>>> +       if (io_mode > 0) {
>>>> +               ctl = readl(chip->ctl) & ~CONTROL_IO_MODE_MASK;
>>>> +               ctl |= io_mode;
>>>> +               writel(ctl, chip->ctl);
>>>> +       }
>>>> +}
>>>> +
>>>>  static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from,
>>>>                                     size_t len, u_char *read_buf)
>>>>  {
>>>> @@ -385,6 +412,7 @@ static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from,
>>>>         for (i = 0; i < chip->nor.read_dummy / 8; i++)
>>>>                 aspeed_smc_write_to_ahb(chip->ahb_base, &dummy, sizeof(dummy));
>>>>
>>>> +       aspeed_smc_set_io_mode(chip);
>>>
>>> Does this actually work for 1_2_2 mode?  I figured you would need to
>>> set it to dual mode before sending the address (but after the command)
>>> a few lines up from here.
>>
>> yes. This needs a fix for CONTROL_IO_DUAL_ADDR_DATA. 
> 
> I was wondering why I didn't see the BB command being used.
> The spi-nor layer only provides the SNOR_PROTO_1_2_2 definition
> but only the SNOR_PROTO_1_1_2 is handled for the moment. 
> Any how, this patch needs some more work.

If you want to test your controller with SPI 1-2-2, you will need this
patch:
http://patchwork.ozlabs.org/patch/778995/

As you noticed, currently SPI 1-2-2 and 1-4-4 protocols has been
introduced in the spi-nor subsystem so the SPI controller drivers can
declare their hardware capabilities but you will need the SFDP patch to
take advantage of these capabilities.

If you succeed in using SPI 1-2-2 with the Aspeed controller, don't
hesitate to add your Tested-by tag on the SFDP patch ;)

Best regards,

Cyrille


> 
> Thanks,
> 
> C. 
> 
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 

Re: [PATCH v2 1/2] mtd: spi-nor: aspeed: add support for SPI dual IO read mode

[Cédric Le Goater]

On 06/23/2017 11:08 AM, Cyrille Pitchen wrote:
> Hi Cédric,
> 
> Le 23/06/2017 à 09:02, Cédric Le Goater a écrit :
>> On 06/23/2017 08:35 AM, Cédric Le Goater wrote:
>>> On 06/22/2017 07:17 PM, Rob Lippert wrote:
>>>> On Thu, Jun 22, 2017 at 12:18 AM, Cédric Le Goater <clg@kaod.org> wrote:
>>>>> 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.
>>>>>
>>>>> Based on work from Robert Lippert <roblip@gmail.com>
>>>>>
>>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>>>> Cc: Robert Lippert <roblip@gmail.com>
>>>>> ---
>>>>>
>>>>>  Changes since v1:
>>>>>
>>>>>  - reworked the patch to fit the new spi-nor hwcaps
>>>>>  - added dual address and data IO
>>>>>  - took ownership due to the amount of rewritten code.
>>>>>
>>>>>  drivers/mtd/spi-nor/aspeed-smc.c | 52 +++++++++++++++++++++++++++++++---------
>>>>>  1 file changed, 41 insertions(+), 11 deletions(-)
>>>>>
>>>>> diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c
>>>>> index 0106357421bd..93ca2ee65f51 100644
>>>>> --- a/drivers/mtd/spi-nor/aspeed-smc.c
>>>>> +++ b/drivers/mtd/spi-nor/aspeed-smc.c
>>>>> @@ -373,6 +373,33 @@ static void aspeed_smc_send_cmd_addr(struct spi_nor *nor, u8 cmd, u32 addr)
>>>>>         }
>>>>>  }
>>>>>
>>>>> +static int aspeed_smc_get_io_mode(struct aspeed_smc_chip *chip)
>>>>> +{
>>>>> +       switch (chip->nor.read_proto) {
>>>>> +       case SNOR_PROTO_1_1_1:
>>>>> +               return 0;
>>>>> +       case SNOR_PROTO_1_1_2:
>>>>> +               return CONTROL_IO_DUAL_DATA;
>>>>> +       case SNOR_PROTO_1_2_2:
>>>>> +               return CONTROL_IO_DUAL_ADDR_DATA;
>>>>> +       default:
>>>>> +               dev_err(chip->nor.dev, "unsupported SPI read mode\n");
>>>>> +               return -EINVAL;
>>>>> +       }
>>>>> +}
>>>>> +
>>>>> +static void aspeed_smc_set_io_mode(struct aspeed_smc_chip *chip)
>>>>> +{
>>>>> +       u32 io_mode = aspeed_smc_get_io_mode(chip);
>>>>> +       u32 ctl;
>>>>> +
>>>>> +       if (io_mode > 0) {
>>>>> +               ctl = readl(chip->ctl) & ~CONTROL_IO_MODE_MASK;
>>>>> +               ctl |= io_mode;
>>>>> +               writel(ctl, chip->ctl);
>>>>> +       }
>>>>> +}
>>>>> +
>>>>>  static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from,
>>>>>                                     size_t len, u_char *read_buf)
>>>>>  {
>>>>> @@ -385,6 +412,7 @@ static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from,
>>>>>         for (i = 0; i < chip->nor.read_dummy / 8; i++)
>>>>>                 aspeed_smc_write_to_ahb(chip->ahb_base, &dummy, sizeof(dummy));
>>>>>
>>>>> +       aspeed_smc_set_io_mode(chip);
>>>>
>>>> Does this actually work for 1_2_2 mode?  I figured you would need to
>>>> set it to dual mode before sending the address (but after the command)
>>>> a few lines up from here.
>>>
>>> yes. This needs a fix for CONTROL_IO_DUAL_ADDR_DATA. 
>>
>> I was wondering why I didn't see the BB command being used.
>> The spi-nor layer only provides the SNOR_PROTO_1_2_2 definition
>> but only the SNOR_PROTO_1_1_2 is handled for the moment. 
>> Any how, this patch needs some more work.
> 
> If you want to test your controller with SPI 1-2-2, you will need this
> patch:
> http://patchwork.ozlabs.org/patch/778995/
> 
> As you noticed, currently SPI 1-2-2 and 1-4-4 protocols has been
> introduced in the spi-nor subsystem so the SPI controller drivers can
> declare their hardware capabilities but you will need the SFDP patch to
> take advantage of these capabilities.
> 
> If you succeed in using SPI 1-2-2 with the Aspeed controller, don't
> hesitate to add your Tested-by tag on the SFDP patch ;)

So, I gave it a quick try on a AST2400 booting from a n25q256a chip
and I had to double the read_dummies to make it work. I need to study 
a little more the question before adding a Tested-by :)

Cheers,

C.

> Best regards,
> 
> Cyrille
> 
> 
>>
>> Thanks,
>>
>> C. 
>>
>>
>> ______________________________________________________
>> Linux MTD discussion mailing list
>> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>>
> 

Re: [PATCH v2 1/2] mtd: spi-nor: aspeed: add support for SPI dual IO read mode

[Cyrille Pitchen]

Le 23/06/2017 à 13:47, Cédric Le Goater a écrit :
> On 06/23/2017 11:08 AM, Cyrille Pitchen wrote:
>> Hi Cédric,
>>
>> Le 23/06/2017 à 09:02, Cédric Le Goater a écrit :
>>> On 06/23/2017 08:35 AM, Cédric Le Goater wrote:
>>>> On 06/22/2017 07:17 PM, Rob Lippert wrote:
>>>>> On Thu, Jun 22, 2017 at 12:18 AM, Cédric Le Goater <clg@kaod.org> wrote:
>>>>>> 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.
>>>>>>
>>>>>> Based on work from Robert Lippert <roblip@gmail.com>
>>>>>>
>>>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>>>>> Cc: Robert Lippert <roblip@gmail.com>
>>>>>> ---
>>>>>>
>>>>>>  Changes since v1:
>>>>>>
>>>>>>  - reworked the patch to fit the new spi-nor hwcaps
>>>>>>  - added dual address and data IO
>>>>>>  - took ownership due to the amount of rewritten code.
>>>>>>
>>>>>>  drivers/mtd/spi-nor/aspeed-smc.c | 52 +++++++++++++++++++++++++++++++---------
>>>>>>  1 file changed, 41 insertions(+), 11 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c
>>>>>> index 0106357421bd..93ca2ee65f51 100644
>>>>>> --- a/drivers/mtd/spi-nor/aspeed-smc.c
>>>>>> +++ b/drivers/mtd/spi-nor/aspeed-smc.c
>>>>>> @@ -373,6 +373,33 @@ static void aspeed_smc_send_cmd_addr(struct spi_nor *nor, u8 cmd, u32 addr)
>>>>>>         }
>>>>>>  }
>>>>>>
>>>>>> +static int aspeed_smc_get_io_mode(struct aspeed_smc_chip *chip)
>>>>>> +{
>>>>>> +       switch (chip->nor.read_proto) {
>>>>>> +       case SNOR_PROTO_1_1_1:
>>>>>> +               return 0;
>>>>>> +       case SNOR_PROTO_1_1_2:
>>>>>> +               return CONTROL_IO_DUAL_DATA;
>>>>>> +       case SNOR_PROTO_1_2_2:
>>>>>> +               return CONTROL_IO_DUAL_ADDR_DATA;
>>>>>> +       default:
>>>>>> +               dev_err(chip->nor.dev, "unsupported SPI read mode\n");
>>>>>> +               return -EINVAL;
>>>>>> +       }
>>>>>> +}
>>>>>> +
>>>>>> +static void aspeed_smc_set_io_mode(struct aspeed_smc_chip *chip)
>>>>>> +{
>>>>>> +       u32 io_mode = aspeed_smc_get_io_mode(chip);
>>>>>> +       u32 ctl;
>>>>>> +
>>>>>> +       if (io_mode > 0) {
>>>>>> +               ctl = readl(chip->ctl) & ~CONTROL_IO_MODE_MASK;
>>>>>> +               ctl |= io_mode;
>>>>>> +               writel(ctl, chip->ctl);
>>>>>> +       }
>>>>>> +}
>>>>>> +
>>>>>>  static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from,
>>>>>>                                     size_t len, u_char *read_buf)
>>>>>>  {
>>>>>> @@ -385,6 +412,7 @@ static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from,
>>>>>>         for (i = 0; i < chip->nor.read_dummy / 8; i++)
>>>>>>                 aspeed_smc_write_to_ahb(chip->ahb_base, &dummy, sizeof(dummy));
>>>>>>
>>>>>> +       aspeed_smc_set_io_mode(chip);
>>>>>
>>>>> Does this actually work for 1_2_2 mode?  I figured you would need to
>>>>> set it to dual mode before sending the address (but after the command)
>>>>> a few lines up from here.
>>>>
>>>> yes. This needs a fix for CONTROL_IO_DUAL_ADDR_DATA. 
>>>
>>> I was wondering why I didn't see the BB command being used.
>>> The spi-nor layer only provides the SNOR_PROTO_1_2_2 definition
>>> but only the SNOR_PROTO_1_1_2 is handled for the moment. 
>>> Any how, this patch needs some more work.
>>
>> If you want to test your controller with SPI 1-2-2, you will need this
>> patch:
>> http://patchwork.ozlabs.org/patch/778995/
>>
>> As you noticed, currently SPI 1-2-2 and 1-4-4 protocols has been
>> introduced in the spi-nor subsystem so the SPI controller drivers can
>> declare their hardware capabilities but you will need the SFDP patch to
>> take advantage of these capabilities.
>>
>> If you succeed in using SPI 1-2-2 with the Aspeed controller, don't
>> hesitate to add your Tested-by tag on the SFDP patch ;)
> 
> So, I gave it a quick try on a AST2400 booting from a n25q256a chip
> and I had to double the read_dummies to make it work. I need to study 
> a little more the question before adding a Tested-by :)

Please have a look at the m25p80_read() function in
drivers/mtd/devices/m25p80.c if you need to convert the number of dummy
clock cycles into the number of dummy bytes:

	unsigned int dummy = nor->read_dummy;

[...]
	/* get transfer protocols. */
	inst_nbits = spi_nor_get_protocol_inst_nbits(nor->read_proto);
	addr_nbits = spi_nor_get_protocol_addr_nbits(nor->read_proto);
	data_nbits = spi_nor_get_protocol_data_nbits(nor->read_proto);

	/* convert the dummy cycles to the number of bytes */
	dummy = (dummy * addr_nbits) / 8;
[...]

nor->read_dummy is the number of dummy clock cycles, hence with the SPI
1-2-2 protocol, your controller sends 2 bits per clock cycles.

> 
> Cheers,
> 
> C.
> 
>> Best regards,
>>
>> Cyrille
>>
>>
>>>
>>> Thanks,
>>>
>>> C. 
>>>
>>>
>>> ______________________________________________________
>>> Linux MTD discussion mailing list
>>> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>>>
>>
> 
> 

[PATCH v2 2/2] mtd: spi-nor: aspeed: use command mode for reads

[Cédric Le Goater]

When reading flash contents, try to use the "command mode" if the AHB
window configured for the flash module is big enough. Else, just fall
back to the "user mode" to perform the read.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---

 Changes since v1:
 
 - reintroduced the setting of the SPI NOR command in the control
   register, which was forgotten in the previous rebase of the patch.

 Changes since initial version :

 - rebased on current patchset which removed DMA support

 drivers/mtd/spi-nor/aspeed-smc.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c
index 93ca2ee65f51..f504fe3de911 100644
--- a/drivers/mtd/spi-nor/aspeed-smc.c
+++ b/drivers/mtd/spi-nor/aspeed-smc.c
@@ -415,6 +415,31 @@ static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from,
 	aspeed_smc_set_io_mode(chip);
 	aspeed_smc_read_from_ahb(read_buf, chip->ahb_base, len);
 	aspeed_smc_stop_user(nor);
+	return 0;
+}
+
+static ssize_t aspeed_smc_read(struct spi_nor *nor, loff_t from, size_t len,
+			       u_char *read_buf)
+{
+	struct aspeed_smc_chip *chip = nor->priv;
+
+	/*
+	 * The AHB window configured for the chip is too small for the
+	 * read offset. Use the "User mode" of the controller to
+	 * perform the read.
+	 */
+	if (from >= chip->ahb_window_size) {
+		aspeed_smc_read_user(nor, from, len, read_buf);
+		goto out;
+	}
+
+	/*
+	 * Use the "Command mode" to do a direct read from the AHB
+	 * window configured for the chip. This should be the default.
+	 */
+	memcpy_fromio(read_buf, chip->ahb_base + from, len);
+
+out:
 	return len;
 }
 
@@ -772,6 +797,7 @@ static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip)
 		cmd = CONTROL_COMMAND_MODE_FREAD;
 
 	chip->ctl_val[smc_read] |= cmd | io_mode |
+		chip->nor.read_opcode << CONTROL_COMMAND_SHIFT |
 		CONTROL_IO_DUMMY_SET(chip->nor.read_dummy / 8);
 
 	dev_dbg(controller->dev, "read control register: %08x\n",
@@ -840,7 +866,7 @@ static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller,
 		nor->dev = dev;
 		nor->priv = chip;
 		spi_nor_set_flash_node(nor, child);
-		nor->read = aspeed_smc_read_user;
+		nor->read = aspeed_smc_read;
 		nor->write = aspeed_smc_write_user;
 		nor->read_reg = aspeed_smc_read_reg;
 		nor->write_reg = aspeed_smc_write_reg;
-- 
2.7.5