| Message ID | 20201107081420.60325-7-damien.lemoal@wdc.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
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
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}, { /* end of table */} }; MODULE_DEVICE_TABLE(of, dw_spi_mmio_of_match);
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(+)