diff mbox

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

Message ID 1498115883-31499-2-git-send-email-clg@kaod.org
State Changes Requested
Delegated to: Cyrille Pitchen
Headers show

Commit Message

Cédric Le Goater June 22, 2017, 7:18 a.m. UTC
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(-)

Comments

Robert Lippert June 22, 2017, 5:17 p.m. UTC | #1
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
>
Cédric Le Goater June 23, 2017, 6:35 a.m. UTC | #2
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
>>
Cédric Le Goater June 23, 2017, 7:02 a.m. UTC | #3
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.
Cyrille Pitchen - M19942 June 23, 2017, 9:08 a.m. UTC | #4
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/
>
Cédric Le Goater June 23, 2017, 11:47 a.m. UTC | #5
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/
>>
>
Cyrille Pitchen - M19942 June 23, 2017, 12:04 p.m. UTC | #6
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/
>>>
>>
> 
>
diff mbox

Patch

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;