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

[Rob Herring <robh@kernel.org>]

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