[06/32] spi: dw: Add support for the Kendryte K210 SoC

The DW SPI master of the Kendryte K210 RISC-V SoC uses the 32-bits
ctrlr0 register format. This SoC is also quite slow and gets significant
SD card performance improvements from using no-delay polled transfers.
Add the dw_spi_k210_init() function tied to the
"canaan,kendryte-k210-spi" compatible string to set the
DW_SPI_CAP_DFS_32 and DW_SPI_CAP_POLL_NODELAY DW SPI capability fields
for this SoC.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/spi/spi-dw-mmio.c | 9 +++++++++
 1 file changed, 9 insertions(+)

On Sat, Nov 07, 2020 at 05:13:54PM +0900, Damien Le Moal wrote:
> The DW SPI master of the Kendryte K210 RISC-V SoC uses the 32-bits
> ctrlr0 register format. This SoC is also quite slow and gets significant
> SD card performance improvements from using no-delay polled transfers.
> Add the dw_spi_k210_init() function tied to the
> "canaan,kendryte-k210-spi" compatible string to set the
> DW_SPI_CAP_DFS_32 and DW_SPI_CAP_POLL_NODELAY DW SPI capability fields
> for this SoC.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
>  drivers/spi/spi-dw-mmio.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
> index 3f1bc384cb45..a00def6c5b39 100644
> --- a/drivers/spi/spi-dw-mmio.c
> +++ b/drivers/spi/spi-dw-mmio.c
> @@ -223,6 +223,14 @@ static int dw_spi_keembay_init(struct platform_device *pdev,
>  	return 0;
>  }
>  
> +static int dw_spi_k210_init(struct platform_device *pdev,
> +			    struct dw_spi_mmio *dwsmmio)
> +{
> +	dwsmmio->dws.caps = DW_SPI_CAP_DFS_32 | DW_SPI_CAP_POLL_NODELAY;
> +
> +	return 0;
> +}
> +
>  static int dw_spi_mmio_probe(struct platform_device *pdev)
>  {
>  	int (*init_func)(struct platform_device *pdev,
> @@ -340,6 +348,7 @@ static const struct of_device_id dw_spi_mmio_of_match[] = {
>  	{ .compatible = "snps,dwc-ssi-1.01a", .data = dw_spi_dwc_ssi_init},
>  	{ .compatible = "intel,keembay-ssi", .data = dw_spi_keembay_init},
>  	{ .compatible = "microchip,sparx5-spi", dw_spi_mscc_sparx5_init},

> +	{ .compatible = "canaan,kendryte-k210-spi", .data = dw_spi_k210_init},

Other than the comments from Sean and Mark regarding the DFS_32
feature runtime detectability, I couldn't find a patch with adding the
new new compatible string into the DW APB SSI DT schema. Have I missed
it? If I haven't could you add one to the next version of the series?

-Sergey

>  	{ /* end of table */}
>  };
>  MODULE_DEVICE_TABLE(of, dw_spi_mmio_of_match);
> -- 
> 2.28.0
>

On Mon, Nov 09, 2020 at 09:39:19PM +0000, Damien Le Moal wrote:
> On 2020/11/10 6:22, Serge Semin wrote:
> > On Sat, Nov 07, 2020 at 05:13:54PM +0900, Damien Le Moal wrote:
> >> The DW SPI master of the Kendryte K210 RISC-V SoC uses the 32-bits
> >> ctrlr0 register format. This SoC is also quite slow and gets significant
> >> SD card performance improvements from using no-delay polled transfers.
> >> Add the dw_spi_k210_init() function tied to the
> >> "canaan,kendryte-k210-spi" compatible string to set the
> >> DW_SPI_CAP_DFS_32 and DW_SPI_CAP_POLL_NODELAY DW SPI capability fields
> >> for this SoC.
> >>
> >> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> >> ---
> >>  drivers/spi/spi-dw-mmio.c | 9 +++++++++
> >>  1 file changed, 9 insertions(+)
> >>
> >> diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
> >> index 3f1bc384cb45..a00def6c5b39 100644
> >> --- a/drivers/spi/spi-dw-mmio.c
> >> +++ b/drivers/spi/spi-dw-mmio.c
> >> @@ -223,6 +223,14 @@ static int dw_spi_keembay_init(struct platform_device *pdev,
> >>  	return 0;
> >>  }
> >>  
> >> +static int dw_spi_k210_init(struct platform_device *pdev,
> >> +			    struct dw_spi_mmio *dwsmmio)
> >> +{
> >> +	dwsmmio->dws.caps = DW_SPI_CAP_DFS_32 | DW_SPI_CAP_POLL_NODELAY;
> >> +
> >> +	return 0;
> >> +}
> >> +
> >>  static int dw_spi_mmio_probe(struct platform_device *pdev)
> >>  {
> >>  	int (*init_func)(struct platform_device *pdev,
> >> @@ -340,6 +348,7 @@ static const struct of_device_id dw_spi_mmio_of_match[] = {
> >>  	{ .compatible = "snps,dwc-ssi-1.01a", .data = dw_spi_dwc_ssi_init},
> >>  	{ .compatible = "intel,keembay-ssi", .data = dw_spi_keembay_init},
> >>  	{ .compatible = "microchip,sparx5-spi", dw_spi_mscc_sparx5_init},
> > 
> >> +	{ .compatible = "canaan,kendryte-k210-spi", .data = dw_spi_k210_init},
> > 
> > Other than the comments from Sean and Mark regarding the DFS_32
> > feature runtime detectability, I couldn't find a patch with adding the
> > new new compatible string into the DW APB SSI DT schema. Have I missed
> > it? If I haven't could you add one to the next version of the series?
> 
> Yes, I will. I forgot to change the DW DT binding doc for this. I did add a
> patch for the "polling" property but forgot the compatible string.
> 
> In any case, I think that this new compatible string change can be dropped by
> switching to automatically detecting the DFS32 and using a different solution
> than the polling property change I sent for the RX fifo overflow problem.

No, new SoC needs new compatible string. Especially if a new vendor. 

> 
> I am still going through all the emails trying to understand what to try next to
> avoid the polling "hack".

Use compatible.

Rob

On Mon, Nov 9, 2020 at 4:00 PM Damien Le Moal <Damien.LeMoal@wdc.com> wrote:
>
> On 2020/11/10 6:55, Rob Herring wrote:
> > On Mon, Nov 09, 2020 at 09:39:19PM +0000, Damien Le Moal wrote:
> >> On 2020/11/10 6:22, Serge Semin wrote:
> >>> On Sat, Nov 07, 2020 at 05:13:54PM +0900, Damien Le Moal wrote:
> >>>> The DW SPI master of the Kendryte K210 RISC-V SoC uses the 32-bits
> >>>> ctrlr0 register format. This SoC is also quite slow and gets significant
> >>>> SD card performance improvements from using no-delay polled transfers.
> >>>> Add the dw_spi_k210_init() function tied to the
> >>>> "canaan,kendryte-k210-spi" compatible string to set the
> >>>> DW_SPI_CAP_DFS_32 and DW_SPI_CAP_POLL_NODELAY DW SPI capability fields
> >>>> for this SoC.
> >>>>
> >>>> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> >>>> ---
> >>>>  drivers/spi/spi-dw-mmio.c | 9 +++++++++
> >>>>  1 file changed, 9 insertions(+)
> >>>>
> >>>> diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
> >>>> index 3f1bc384cb45..a00def6c5b39 100644
> >>>> --- a/drivers/spi/spi-dw-mmio.c
> >>>> +++ b/drivers/spi/spi-dw-mmio.c
> >>>> @@ -223,6 +223,14 @@ static int dw_spi_keembay_init(struct platform_device *pdev,
> >>>>    return 0;
> >>>>  }
> >>>>
> >>>> +static int dw_spi_k210_init(struct platform_device *pdev,
> >>>> +                      struct dw_spi_mmio *dwsmmio)
> >>>> +{
> >>>> +  dwsmmio->dws.caps = DW_SPI_CAP_DFS_32 | DW_SPI_CAP_POLL_NODELAY;
> >>>> +
> >>>> +  return 0;
> >>>> +}
> >>>> +
> >>>>  static int dw_spi_mmio_probe(struct platform_device *pdev)
> >>>>  {
> >>>>    int (*init_func)(struct platform_device *pdev,
> >>>> @@ -340,6 +348,7 @@ static const struct of_device_id dw_spi_mmio_of_match[] = {
> >>>>    { .compatible = "snps,dwc-ssi-1.01a", .data = dw_spi_dwc_ssi_init},
> >>>>    { .compatible = "intel,keembay-ssi", .data = dw_spi_keembay_init},
> >>>>    { .compatible = "microchip,sparx5-spi", dw_spi_mscc_sparx5_init},
> >>>
> >>>> +  { .compatible = "canaan,kendryte-k210-spi", .data = dw_spi_k210_init},
> >>>
> >>> Other than the comments from Sean and Mark regarding the DFS_32
> >>> feature runtime detectability, I couldn't find a patch with adding the
> >>> new new compatible string into the DW APB SSI DT schema. Have I missed
> >>> it? If I haven't could you add one to the next version of the series?
> >>
> >> Yes, I will. I forgot to change the DW DT binding doc for this. I did add a
> >> patch for the "polling" property but forgot the compatible string.
> >>
> >> In any case, I think that this new compatible string change can be dropped by
> >> switching to automatically detecting the DFS32 and using a different solution
> >> than the polling property change I sent for the RX fifo overflow problem.
> >
> > No, new SoC needs new compatible string. Especially if a new vendor.
>
> My apologies for the bad wording: I meant to say the change to the list of
> compatible strings that the DW SPI support would not be needed. So from the DW
> SPI point of view, there would be no new compatible string to add/document.

No, there is a need for a new compatible string to add/document. You
might not need it in the driver if you have a fallback.

Compatible strings should be SoC specific so you can handle quirks
without a DT change. Otherwise, it's a never ending stream of new
properties and DT updates.

> >> I am still going through all the emails trying to understand what to try next to
> >> avoid the polling "hack".
> >
> > Use compatible.
>
> Yes, that is what this patch used. Again, I think there is a chance this change
> can be dropped.

Looks to me like it used a 'polling' property...

Rob

On Sat, 2020-11-07 at 08:31 -0500, Sean Anderson wrote:
> On 11/7/20 3:13 AM, Damien Le Moal wrote:

> > The DW SPI master of the Kendryte K210 RISC-V SoC uses the 32-bits

> > ctrlr0 register format. This SoC is also quite slow and gets significant

> > SD card performance improvements from using no-delay polled transfers.

> > Add the dw_spi_k210_init() function tied to the

> > "canaan,kendryte-k210-spi" compatible string to set the

> > DW_SPI_CAP_DFS_32 and DW_SPI_CAP_POLL_NODELAY DW SPI capability fields

> > for this SoC.

> > 

> > Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>

> > ---

> >  drivers/spi/spi-dw-mmio.c | 9 +++++++++

> >  1 file changed, 9 insertions(+)

> > 

> > diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c

> > index 3f1bc384cb45..a00def6c5b39 100644

> > --- a/drivers/spi/spi-dw-mmio.c

> > +++ b/drivers/spi/spi-dw-mmio.c

> > @@ -223,6 +223,14 @@ static int dw_spi_keembay_init(struct platform_device *pdev,

> >  	return 0;

> >  }

> >  

> > 

> > 

> > 

> > +static int dw_spi_k210_init(struct platform_device *pdev,

> > +			    struct dw_spi_mmio *dwsmmio)

> > +{

> > +	dwsmmio->dws.caps = DW_SPI_CAP_DFS_32 | DW_SPI_CAP_POLL_NODELAY;

> 

> Can't you do runtime detection of DFS_32 in probe?


Done ! Sending that in V2 (I will split the spi dw from the rest of the
series).



-- 
Damien Le Moal
Western Digital