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

[Damien Le Moal]

On 2020/11/10 8:08, Rob Herring wrote:
> On Mon, Nov 9, 2020 at 4:00 PM Damien Le Moal <Damien.LeMoal at 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 at 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.

Ah. OK. I get it now. Thanks for clarifying. So I will keep the new compatible
string (renamed with proper vendor name instead of brand) and document it.

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

I hope to be able to get rid of this change if a proper solution can be found to
the transfer speed problem I am seeing.

> 
> Rob
> 


-- 
Damien Le Moal
Western Digital Research