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

["Cédric Le Goater" <clg@kaod.org>]

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/
>>
>