| Message ID | 20200819123208.12337-3-l.stelmach@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/8] spi: spi-s3c64xx: swap s3c64xx_spi_set_cs() and s3c64xx_enable_datapath() | expand |
On Wed, Aug 19, 2020 at 02:32:02PM +0200, Łukasz Stelmach wrote: > Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> Add a quirk - why? There is here no commit msg, no explanation. Best regards, Krzysztof > --- > drivers/spi/spi-s3c64xx.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c > index fb5e2ba4b6b9..8fe44451d303 100644 > --- a/drivers/spi/spi-s3c64xx.c > +++ b/drivers/spi/spi-s3c64xx.c > @@ -1372,6 +1372,7 @@ static struct s3c64xx_spi_port_config exynos4_spi_port_config = { > .tx_st_done = 25, > .high_speed = true, > .clk_from_cmu = true, > + .quirks = S3C64XX_SPI_QUIRK_CS_AUTO, > }; > > static struct s3c64xx_spi_port_config exynos7_spi_port_config = { > -- > 2.26.2 >
It was <2020-08-19 śro 14:39>, when Krzysztof Kozlowski wrote: > On Wed, Aug 19, 2020 at 02:32:02PM +0200, Łukasz Stelmach wrote: >> Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> > > Add a quirk - why? Because stuff does not work without it and works with it and it is turned on for other SoCs which have simmilar SPI HW. > There is here no commit msg, no explanation. As I wrote in the cover letter, this and previous commits make things work on Exynos3250 (ARTIK5 precisely). I can't explain why. I read everything I could about this HW and there were no details about automatic CS handling other than how to turn it on and off. >> --- >> drivers/spi/spi-s3c64xx.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c >> index fb5e2ba4b6b9..8fe44451d303 100644 >> --- a/drivers/spi/spi-s3c64xx.c >> +++ b/drivers/spi/spi-s3c64xx.c >> @@ -1372,6 +1372,7 @@ static struct s3c64xx_spi_port_config exynos4_spi_port_config = { >> .tx_st_done = 25, >> .high_speed = true, >> .clk_from_cmu = true, >> + .quirks = S3C64XX_SPI_QUIRK_CS_AUTO, >> }; >> >> static struct s3c64xx_spi_port_config exynos7_spi_port_config = { >> -- >> 2.26.2 >> >
On Wed, Aug 19, 2020 at 03:01:21PM +0200, Lukasz Stelmach wrote: > It was <2020-08-19 śro 14:39>, when Krzysztof Kozlowski wrote: > > On Wed, Aug 19, 2020 at 02:32:02PM +0200, Łukasz Stelmach wrote: > >> Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> > > > > Add a quirk - why? > > Because stuff does not work without it and works with it and it is > turned on for other SoCs which have simmilar SPI HW. > > > There is here no commit msg, no explanation. > > As I wrote in the cover letter, this and previous commits make things > work on Exynos3250 (ARTIK5 precisely). I can't explain why. I read > everything I could about this HW and there were no details about > automatic CS handling other than how to turn it on and off. At least this information would be useful. If vendor kernel also does it, that's another argument to use, since there are no better ones... Best regards, Krzysztof
On Wed, Aug 19, 2020 at 03:01:21PM +0200, Lukasz Stelmach wrote: > It was <2020-08-19 śro 14:39>, when Krzysztof Kozlowski wrote: > > There is here no commit msg, no explanation. > As I wrote in the cover letter, this and previous commits make things > work on Exynos3250 (ARTIK5 precisely). I can't explain why. I read > everything I could about this HW and there were no details about > automatic CS handling other than how to turn it on and off. What is similar about those other SoCs - could you be more specific here, or what goes wrong if you don't set this? The auto mode (or at least the auto mode that was on the Exynos7) is not compatible with many SPI devices if the controller chip select is actually in use, the quirk was added for controllers that just don't have the manual mode. See also: https://lore.kernel.org/linux-spi/CAAgF-BfGwcNzMx0meFVkJqNMTbQ4_PP1PZ3i6edOm6U3bc26_Q@mail.gmail.com/ for an explanation of the quirk.
It was <2020-08-19 śro 20:38>, when Mark Brown wrote: > On Wed, Aug 19, 2020 at 03:01:21PM +0200, Lukasz Stelmach wrote: >> It was <2020-08-19 śro 14:39>, when Krzysztof Kozlowski wrote: > >>> There is here no commit msg, no explanation. > >> As I wrote in the cover letter, this and previous commits make things >> work on Exynos3250 (ARTIK5 precisely). I can't explain why. I read >> everything I could about this HW and there were no details about >> automatic CS handling other than how to turn it on and off. > > What is similar about those other SoCs - could you be more specific > here, or what goes wrong if you don't set this? Without the quirk set DMA transfers longer than 512 bytes fail. They simply stop and hit the timeout with a few (<20) bytes pending. As far as I can tell the SPI controller is the same in different Exynos SoCs. > The auto mode (or at least the auto mode that was on the Exynos7) is > not compatible with many SPI devices if the controller chip select is > actually in use, the quirk was added for controllers that just don't > have the manual mode. According to the manual the auto mode makes the controller toggle CS between SPI packets (bytes?). I didn't have any problem transferring data (>512 bytes) from the SPI device in the polling mode. Only the DMA caused problems. > See also: > > https://lore.kernel.org/linux-spi/CAAgF-BfGwcNzMx0meFVkJqNMTbQ4_PP1PZ3i6edOm6U3bc26_Q@mail.gmail.com/ > > for an explanation of the quirk. > >> CS can also be controlled automatically by setting AUTO_N_MANUAL to 1 >> in CS_CFG. When it is auto CS automatically toggles between packet to >> packet. NCS_TIME_COUNT in CS_CFG controls the inactive period. The >> driver by default uses manual mode. But on exynos7 the manual mode is >> removed. I *suspect* that the automatic CS toggling between packets gives better (?) synchronisation between the SPI device and the controller's internals and prevents some kind of a deadlock inside the controller. These are just speculations.
diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c index fb5e2ba4b6b9..8fe44451d303 100644 --- a/drivers/spi/spi-s3c64xx.c +++ b/drivers/spi/spi-s3c64xx.c @@ -1372,6 +1372,7 @@ static struct s3c64xx_spi_port_config exynos4_spi_port_config = { .tx_st_done = 25, .high_speed = true, .clk_from_cmu = true, + .quirks = S3C64XX_SPI_QUIRK_CS_AUTO, }; static struct s3c64xx_spi_port_config exynos7_spi_port_config = {
Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> --- drivers/spi/spi-s3c64xx.c | 1 + 1 file changed, 1 insertion(+)