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

[Cyrille Pitchen <cyrille.pitchen@microchip.com>]

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