* [PATCH 0/8] Some fixes for spi-s3c64xx [not found] <CGME20200819123225eucas1p28be1b1920ade0ba8997bc17da97599b6@eucas1p2.samsung.com> @ 2020-08-19 12:32 ` Łukasz Stelmach [not found] ` <CGME20200819123226eucas1p16c9b90330d957344f99f6ee461190085@eucas1p1.samsung.com> ` (8 more replies) 0 siblings, 9 replies; 34+ messages in thread From: Łukasz Stelmach @ 2020-08-19 12:32 UTC (permalink / raw) To: Kukjin Kim, Krzysztof Kozlowski, Andi Shyti, Mark Brown, linux-spi, linux-samsung-soc, linux-arm-kernel, linux-kernel Cc: b.zolnierkie, Łukasz Stelmach, m.szyprowski This is a series of fixes created during porting a device driver (these patches will be released soon too) for an SPI device to the current kernel. The two most important are spi: spi-s3c64xx: swap s3c64xx_spi_set_cs() and s3c64xx_enable_datapath() spi: spi-s3s64xx: Add S3C64XX_SPI_QUIRK_CS_AUTO for Exynos3250 Without them DMA transfers larger than 512 bytes from the SPI controller would fail. Łukasz Stelmach (8): spi: spi-s3c64xx: swap s3c64xx_spi_set_cs() and s3c64xx_enable_datapath() spi: spi-s3s64xx: Add S3C64XX_SPI_QUIRK_CS_AUTO for Exynos3250 spi: spi-s3c64xx: Report more information when errors occur spi: spi-s3c64xx: Rename S3C64XX_SPI_SLAVE_* to S3C64XX_SPI_CS_* spi: spi-s3c64xx: Fix doc comment for struct s3c64xx_spi_driver_data spi: spi-s3c64xx: Check return values spi: spi-s3c64xx: Increase tranfer timeout spi: spi-s3c64xx: Turn on interrupts upon resume drivers/spi/spi-s3c64xx.c | 81 +++++++++++++++++++++++++++------------ 1 file changed, 56 insertions(+), 25 deletions(-) -- 2.26.2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <CGME20200819123226eucas1p16c9b90330d957344f99f6ee461190085@eucas1p1.samsung.com>]
* [PATCH 1/8] spi: spi-s3c64xx: swap s3c64xx_spi_set_cs() and s3c64xx_enable_datapath() [not found] ` <CGME20200819123226eucas1p16c9b90330d957344f99f6ee461190085@eucas1p1.samsung.com> @ 2020-08-19 12:32 ` Łukasz Stelmach 2020-08-19 12:38 ` Krzysztof Kozlowski 0 siblings, 1 reply; 34+ messages in thread From: Łukasz Stelmach @ 2020-08-19 12:32 UTC (permalink / raw) To: Kukjin Kim, Krzysztof Kozlowski, Andi Shyti, Mark Brown, linux-spi, linux-samsung-soc, linux-arm-kernel, linux-kernel Cc: b.zolnierkie, Łukasz Stelmach, m.szyprowski Fix issues with DMA transfers bigger than 512 on Exynos3250. Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> --- drivers/spi/spi-s3c64xx.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c index cf67ea60dc0e..fb5e2ba4b6b9 100644 --- a/drivers/spi/spi-s3c64xx.c +++ b/drivers/spi/spi-s3c64xx.c @@ -678,11 +678,11 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master, sdd->state &= ~RXBUSY; sdd->state &= ~TXBUSY; - s3c64xx_enable_datapath(sdd, xfer, use_dma); - /* Start the signals */ s3c64xx_spi_set_cs(spi, true); + s3c64xx_enable_datapath(sdd, xfer, use_dma); + spin_unlock_irqrestore(&sdd->lock, flags); if (use_dma) -- 2.26.2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH 1/8] spi: spi-s3c64xx: swap s3c64xx_spi_set_cs() and s3c64xx_enable_datapath() 2020-08-19 12:32 ` [PATCH 1/8] spi: spi-s3c64xx: swap s3c64xx_spi_set_cs() and s3c64xx_enable_datapath() Łukasz Stelmach @ 2020-08-19 12:38 ` Krzysztof Kozlowski [not found] ` <CGME20200819125150eucas1p1965fab59b6e75cf54cac262161c5695b@eucas1p1.samsung.com> 0 siblings, 1 reply; 34+ messages in thread From: Krzysztof Kozlowski @ 2020-08-19 12:38 UTC (permalink / raw) To: Łukasz Stelmach Cc: linux-samsung-soc, b.zolnierkie, linux-kernel, linux-spi, Mark Brown, Kukjin Kim, Andi Shyti, linux-arm-kernel, m.szyprowski On Wed, Aug 19, 2020 at 02:32:01PM +0200, Łukasz Stelmach wrote: > Fix issues with DMA transfers bigger than 512 on Exynos3250. Fix, but how? With proper explanation this should go to CC-stable. Best regards, Krzysztof > > Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> > --- > drivers/spi/spi-s3c64xx.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c > index cf67ea60dc0e..fb5e2ba4b6b9 100644 > --- a/drivers/spi/spi-s3c64xx.c > +++ b/drivers/spi/spi-s3c64xx.c > @@ -678,11 +678,11 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master, > sdd->state &= ~RXBUSY; > sdd->state &= ~TXBUSY; > > - s3c64xx_enable_datapath(sdd, xfer, use_dma); > - > /* Start the signals */ > s3c64xx_spi_set_cs(spi, true); > > + s3c64xx_enable_datapath(sdd, xfer, use_dma); > + > spin_unlock_irqrestore(&sdd->lock, flags); > > if (use_dma) > -- > 2.26.2 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <CGME20200819125150eucas1p1965fab59b6e75cf54cac262161c5695b@eucas1p1.samsung.com>]
* Re: [PATCH 1/8] spi: spi-s3c64xx: swap s3c64xx_spi_set_cs() and s3c64xx_enable_datapath() [not found] ` <CGME20200819125150eucas1p1965fab59b6e75cf54cac262161c5695b@eucas1p1.samsung.com> @ 2020-08-19 12:51 ` Lukasz Stelmach 2020-08-19 12:58 ` Krzysztof Kozlowski 0 siblings, 1 reply; 34+ messages in thread From: Lukasz Stelmach @ 2020-08-19 12:51 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: linux-samsung-soc, b.zolnierkie, linux-kernel, linux-spi, Mark Brown, Kukjin Kim, Andi Shyti, linux-arm-kernel, m.szyprowski [-- Attachment #1.1: Type: text/plain, Size: 1438 bytes --] It was <2020-08-19 śro 14:38>, when Krzysztof Kozlowski wrote: > On Wed, Aug 19, 2020 at 02:32:01PM +0200, Łukasz Stelmach wrote: >> Fix issues with DMA transfers bigger than 512 on Exynos3250. > > Fix, but how? With proper explanation this should go to CC-stable. Honestly, I don't know and I couldn't find out why. It makes stuff work. There has been a commit like this before 0f5a751ace25 spi/s3c64xx: Enable GPIO /CS prior to starting hardware Apparently, it was lost in 0732a9d2a155 spi/s3c64xx: Use core message handling >> >> Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> >> --- >> drivers/spi/spi-s3c64xx.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c >> index cf67ea60dc0e..fb5e2ba4b6b9 100644 >> --- a/drivers/spi/spi-s3c64xx.c >> +++ b/drivers/spi/spi-s3c64xx.c >> @@ -678,11 +678,11 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master, >> sdd->state &= ~RXBUSY; >> sdd->state &= ~TXBUSY; >> >> - s3c64xx_enable_datapath(sdd, xfer, use_dma); >> - >> /* Start the signals */ >> s3c64xx_spi_set_cs(spi, true); >> >> + s3c64xx_enable_datapath(sdd, xfer, use_dma); >> + >> spin_unlock_irqrestore(&sdd->lock, flags); >> >> if (use_dma) >> -- >> 2.26.2 >> -- Łukasz Stelmach Samsung R&D Institute Poland Samsung Electronics [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 487 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 1/8] spi: spi-s3c64xx: swap s3c64xx_spi_set_cs() and s3c64xx_enable_datapath() 2020-08-19 12:51 ` Lukasz Stelmach @ 2020-08-19 12:58 ` Krzysztof Kozlowski 2020-08-19 13:16 ` Mark Brown 0 siblings, 1 reply; 34+ messages in thread From: Krzysztof Kozlowski @ 2020-08-19 12:58 UTC (permalink / raw) To: Lukasz Stelmach Cc: linux-samsung-soc, b.zolnierkie, linux-kernel, linux-spi, Mark Brown, Kukjin Kim, Andi Shyti, linux-arm-kernel, m.szyprowski On Wed, Aug 19, 2020 at 02:51:27PM +0200, Lukasz Stelmach wrote: > It was <2020-08-19 śro 14:38>, when Krzysztof Kozlowski wrote: > > On Wed, Aug 19, 2020 at 02:32:01PM +0200, Łukasz Stelmach wrote: > >> Fix issues with DMA transfers bigger than 512 on Exynos3250. > > > > Fix, but how? With proper explanation this should go to CC-stable. > > Honestly, I don't know and I couldn't find out why. It makes stuff > work. There has been a commit like this before > > 0f5a751ace25 spi/s3c64xx: Enable GPIO /CS prior to starting hardware > > Apparently, it was lost in > > 0732a9d2a155 spi/s3c64xx: Use core message handling Then describe at least this... maybe Mark knows why he brough back old code after refactoring? Best regards, Krzysztof _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 1/8] spi: spi-s3c64xx: swap s3c64xx_spi_set_cs() and s3c64xx_enable_datapath() 2020-08-19 12:58 ` Krzysztof Kozlowski @ 2020-08-19 13:16 ` Mark Brown [not found] ` <CGME20200819140203eucas1p2818858289f2394b32f3c647e47705cd2@eucas1p2.samsung.com> 0 siblings, 1 reply; 34+ messages in thread From: Mark Brown @ 2020-08-19 13:16 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: linux-samsung-soc, b.zolnierkie, Lukasz Stelmach, linux-kernel, linux-spi, Kukjin Kim, Andi Shyti, linux-arm-kernel, m.szyprowski [-- Attachment #1.1: Type: text/plain, Size: 1203 bytes --] On Wed, Aug 19, 2020 at 02:58:22PM +0200, Krzysztof Kozlowski wrote: > On Wed, Aug 19, 2020 at 02:51:27PM +0200, Lukasz Stelmach wrote: > > Honestly, I don't know and I couldn't find out why. It makes stuff > > work. There has been a commit like this before > > 0f5a751ace25 spi/s3c64xx: Enable GPIO /CS prior to starting hardware > > Apparently, it was lost in > > 0732a9d2a155 spi/s3c64xx: Use core message handling > Then describe at least this... maybe Mark knows why he brough back old > code after refactoring? I'm not sure what's being referred to as being lost in the second commit TBH. The first commit is simple code motion rather than a correctness thing, and more related to the handling of GPIO controlled chip selects according to the description (which people should be using with that controller anyway where possible IIRC, the native chip select has too many assumptions about what it's doing). I don't know that I ever actually used a system that used the native chip select as the actual chip select. Perhaps some quirk was introduced where the chip select signal does something? The commit is also lacking a description of what the issues that are being fixed are. [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <CGME20200819140203eucas1p2818858289f2394b32f3c647e47705cd2@eucas1p2.samsung.com>]
* Re: [PATCH 1/8] spi: spi-s3c64xx: swap s3c64xx_spi_set_cs() and s3c64xx_enable_datapath() [not found] ` <CGME20200819140203eucas1p2818858289f2394b32f3c647e47705cd2@eucas1p2.samsung.com> @ 2020-08-19 14:01 ` Lukasz Stelmach 2020-08-19 19:12 ` Mark Brown 0 siblings, 1 reply; 34+ messages in thread From: Lukasz Stelmach @ 2020-08-19 14:01 UTC (permalink / raw) To: Mark Brown Cc: linux-samsung-soc, b.zolnierkie, linux-kernel, Krzysztof Kozlowski, linux-spi, Kukjin Kim, Andi Shyti, linux-arm-kernel, m.szyprowski [-- Attachment #1.1: Type: text/plain, Size: 1842 bytes --] It was <2020-08-19 śro 14:16>, when Mark Brown wrote: > On Wed, Aug 19, 2020 at 02:58:22PM +0200, Krzysztof Kozlowski wrote: >> On Wed, Aug 19, 2020 at 02:51:27PM +0200, Lukasz Stelmach wrote: > >> > Honestly, I don't know and I couldn't find out why. It makes stuff >> > work. There has been a commit like this before > >> > 0f5a751ace25 spi/s3c64xx: Enable GPIO /CS prior to starting hardware > >> > Apparently, it was lost in > >> > 0732a9d2a155 spi/s3c64xx: Use core message handling > >> Then describe at least this... maybe Mark knows why he brough back old >> code after refactoring? > > I'm not sure what's being referred to as being lost in the second commit > TBH. Order of enable_cs() and enable_datapath(). The order 0f5a sets makes (for a reaseon I don't know) my devices work. In the latter commit, which rewrites "everything", enable_datapath() is called before what later (in aa4964c4eb3e) became s3c64xx_spi_set_cs(). > The first commit is simple code motion rather than a correctness > thing, and more related to the handling of GPIO controlled chip > selects according to the description (which people should be using > with that controller anyway where possible IIRC, the native chip > select has too many assumptions about what it's doing). Funny, but without the automatic CS control (see the next patch in this series) my stuff does not work. > I don't know that I ever actually used a system that used the native > chip select as the actual chip select. Perhaps some quirk was > introduced where the chip select signal does something? > > The commit is also lacking a description of what the issues that are > being fixed are. On Exynos3250 DMA transfers from SPI longer than 512 fail. -- Łukasz Stelmach Samsung R&D Institute Poland Samsung Electronics [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 487 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 1/8] spi: spi-s3c64xx: swap s3c64xx_spi_set_cs() and s3c64xx_enable_datapath() 2020-08-19 14:01 ` Lukasz Stelmach @ 2020-08-19 19:12 ` Mark Brown [not found] ` <CGME20200820101251eucas1p237a794cc11f44c709c0ccdfef766702c@eucas1p2.samsung.com> 0 siblings, 1 reply; 34+ messages in thread From: Mark Brown @ 2020-08-19 19:12 UTC (permalink / raw) To: Lukasz Stelmach Cc: linux-samsung-soc, b.zolnierkie, linux-kernel, Krzysztof Kozlowski, linux-spi, Kukjin Kim, Andi Shyti, linux-arm-kernel, m.szyprowski [-- Attachment #1.1: Type: text/plain, Size: 3507 bytes --] On Wed, Aug 19, 2020 at 04:01:52PM +0200, Lukasz Stelmach wrote: > It was <2020-08-19 śro 14:16>, when Mark Brown wrote: > > On Wed, Aug 19, 2020 at 02:58:22PM +0200, Krzysztof Kozlowski wrote: > >> On Wed, Aug 19, 2020 at 02:51:27PM +0200, Lukasz Stelmach wrote: > >> > 0732a9d2a155 spi/s3c64xx: Use core message handling > >> Then describe at least this... maybe Mark knows why he brough back old > >> code after refactoring? > > I'm not sure what's being referred to as being lost in the second commit > > TBH. > Order of enable_cs() and enable_datapath(). The order 0f5a sets makes > (for a reaseon I don't know) my devices work. In the latter commit, > which rewrites "everything", enable_datapath() is called before what > later (in aa4964c4eb3e) became s3c64xx_spi_set_cs(). That's doesn't look like what the changes do. Note that the enable_cs() function that got moved in 0f5a751ace250097 (spi/s3c64xx: Enable GPIO /CS prior to starting hardware) does not touch the chip registers at all, it only manipulates GPIOs, code that was subsequently factored out into the core. The write to the _SLAVE_SEL register has so far as I can see always been after enable_datapath() right back to the initial commit, it just got made more complex for the Exynos7 controller (I'm guessing your new one might be an ancestor of that?) in bf77cba95f8c06 (spi: s3c64xx: add support for exynos7 SPI controller) and then factored out in the commit you mention above. Are you sure your new ordering works for all controller revisions? According to the comment the _set_cs() is what's actually kicking off the transfer which suggests that the data/DMA needed to be ready beforehand to avoid an underflow or something (and nobody reported issues before, I know people have done things like downloaded firmwares using this controller...), this could be something that got changed between revisions. Please include human readable descriptions of things like commits and issues being discussed in e-mail in your mails, this makes them much easier for humans to read especially when they have no internet access. > > The first commit is simple code motion rather than a correctness > > thing, and more related to the handling of GPIO controlled chip > > selects according to the description (which people should be using > > with that controller anyway where possible IIRC, the native chip > > select has too many assumptions about what it's doing). > Funny, but without the automatic CS control (see the next patch in this > series) my stuff does not work. There's two things, there's changing the controller registers and there is the use of the signal coming out of the controller as the chip select that the device on the SPI bus sees. Most systems have pinmuxing which allows the internal chip select to just not be connected to anything which is what I'm talking about in the above text, IIRC all versions of the controller have unfortunate assumptions about how chip selects should work which make GPIO controlled chip selects much better. > > I don't know that I ever actually used a system that used the native > > chip select as the actual chip select. Perhaps some quirk was > > introduced where the chip select signal does something? > > The commit is also lacking a description of what the issues that are > > being fixed are. > On Exynos3250 DMA transfers from SPI longer than 512 fail. Could you expand on "fail" please? [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <CGME20200820101251eucas1p237a794cc11f44c709c0ccdfef766702c@eucas1p2.samsung.com>]
* Re: [PATCH 1/8] spi: spi-s3c64xx: swap s3c64xx_spi_set_cs() and s3c64xx_enable_datapath() [not found] ` <CGME20200820101251eucas1p237a794cc11f44c709c0ccdfef766702c@eucas1p2.samsung.com> @ 2020-08-20 10:12 ` Lukasz Stelmach 0 siblings, 0 replies; 34+ messages in thread From: Lukasz Stelmach @ 2020-08-20 10:12 UTC (permalink / raw) To: Mark Brown Cc: linux-samsung-soc, b.zolnierkie, linux-kernel, Krzysztof Kozlowski, linux-spi, Kukjin Kim, Andi Shyti, linux-arm-kernel, m.szyprowski [-- Attachment #1.1: Type: text/plain, Size: 3067 bytes --] It was <2020-08-19 śro 20:12>, when Mark Brown wrote: > On Wed, Aug 19, 2020 at 04:01:52PM +0200, Lukasz Stelmach wrote: >> It was <2020-08-19 śro 14:16>, when Mark Brown wrote: >>> On Wed, Aug 19, 2020 at 02:58:22PM +0200, Krzysztof Kozlowski wrote: >>>> On Wed, Aug 19, 2020 at 02:51:27PM +0200, Lukasz Stelmach wrote: >>>> >>>>> 0732a9d2a155 spi/s3c64xx: Use core message handling >>>> >>>> Then describe at least this... maybe Mark knows why he brough back old >>>> code after refactoring? >>>> >>> I'm not sure what's being referred to as being lost in the second commit >>> TBH. > >> Order of enable_cs() and enable_datapath(). The order 0f5a sets makes >> (for a reaseon I don't know) my devices work. In the latter commit, >> which rewrites "everything", enable_datapath() is called before what >> later (in aa4964c4eb3e) became s3c64xx_spi_set_cs(). > > That's doesn't look like what the changes do. Note that the enable_cs() > function that got moved in 0f5a751ace250097 (spi/s3c64xx: Enable GPIO > /CS prior to starting hardware) does not touch the chip registers at > all, it only manipulates GPIOs, code that was subsequently factored out > into the core. Indeed, you are 100% right. Anyway that commit has inspired me after days of trying different stuff to switch enable_datapath() and set_cs() and it worked. Even if without any technical connection with your commit. > The write to the _SLAVE_SEL register has so far as I can see always > been after enable_datapath() right back to the initial commit, it just > got made more complex for the Exynos7 controller (I'm guessing your > new one might be an ancestor of that?) in bf77cba95f8c06 (spi: > s3c64xx: add support for exynos7 SPI controller) and then factored out > in the commit you mention above. > > Are you sure your new ordering works for all controller revisions? Not 100%, but we've tested it on several differnt SoCs, and haven't seen any problems. > According to the comment the _set_cs() is what's actually kicking off > the transfer I don't think so. Indeed, without the CS_AUTO quirk CS is pulled down (the SPI device is selected) but for the transfer to start the SPI clock needs to start ticking. > Please include human readable descriptions of things like commits and > issues being discussed in e-mail in your mails, this makes them much > easier for humans to read especially when they have no internet access. I will. >>> I don't know that I ever actually used a system that used the native >>> chip select as the actual chip select. Perhaps some quirk was >>> introduced where the chip select signal does something? >>> >>> The commit is also lacking a description of what the issues that are >>> being fixed are. >> >> On Exynos3250 DMA transfers from SPI longer than 512 fail. > > Could you expand on "fail" please? Stopping a transfer and hitting timeout with a few bytes (<20) left pending in the SPI controller. -- Łukasz Stelmach Samsung R&D Institute Poland Samsung Electronics [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 487 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <CGME20200819123226eucas1p2f4be625abd7ddaac2f09bdf94395346b@eucas1p2.samsung.com>]
* [PATCH 2/8] spi: spi-s3s64xx: Add S3C64XX_SPI_QUIRK_CS_AUTO for Exynos3250 [not found] ` <CGME20200819123226eucas1p2f4be625abd7ddaac2f09bdf94395346b@eucas1p2.samsung.com> @ 2020-08-19 12:32 ` Łukasz Stelmach 2020-08-19 12:39 ` Krzysztof Kozlowski 0 siblings, 1 reply; 34+ messages in thread From: Łukasz Stelmach @ 2020-08-19 12:32 UTC (permalink / raw) To: Kukjin Kim, Krzysztof Kozlowski, Andi Shyti, Mark Brown, linux-spi, linux-samsung-soc, linux-arm-kernel, linux-kernel Cc: b.zolnierkie, Łukasz Stelmach, m.szyprowski Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> --- 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH 2/8] spi: spi-s3s64xx: Add S3C64XX_SPI_QUIRK_CS_AUTO for Exynos3250 2020-08-19 12:32 ` [PATCH 2/8] spi: spi-s3s64xx: Add S3C64XX_SPI_QUIRK_CS_AUTO for Exynos3250 Łukasz Stelmach @ 2020-08-19 12:39 ` Krzysztof Kozlowski [not found] ` <CGME20200819130122eucas1p27e9e84c4399d01409858de6d01e11b52@eucas1p2.samsung.com> 0 siblings, 1 reply; 34+ messages in thread From: Krzysztof Kozlowski @ 2020-08-19 12:39 UTC (permalink / raw) To: Łukasz Stelmach Cc: linux-samsung-soc, b.zolnierkie, linux-kernel, linux-spi, Mark Brown, Kukjin Kim, Andi Shyti, linux-arm-kernel, m.szyprowski 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 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <CGME20200819130122eucas1p27e9e84c4399d01409858de6d01e11b52@eucas1p2.samsung.com>]
* Re: [PATCH 2/8] spi: spi-s3s64xx: Add S3C64XX_SPI_QUIRK_CS_AUTO for Exynos3250 [not found] ` <CGME20200819130122eucas1p27e9e84c4399d01409858de6d01e11b52@eucas1p2.samsung.com> @ 2020-08-19 13:01 ` Lukasz Stelmach 2020-08-19 13:06 ` Krzysztof Kozlowski 2020-08-19 19:38 ` Mark Brown 0 siblings, 2 replies; 34+ messages in thread From: Lukasz Stelmach @ 2020-08-19 13:01 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: linux-samsung-soc, b.zolnierkie, linux-kernel, linux-spi, Mark Brown, Kukjin Kim, Andi Shyti, linux-arm-kernel, m.szyprowski [-- Attachment #1.1: Type: text/plain, Size: 1340 bytes --] 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 >> > -- Łukasz Stelmach Samsung R&D Institute Poland Samsung Electronics [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 487 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 2/8] spi: spi-s3s64xx: Add S3C64XX_SPI_QUIRK_CS_AUTO for Exynos3250 2020-08-19 13:01 ` Lukasz Stelmach @ 2020-08-19 13:06 ` Krzysztof Kozlowski 2020-08-19 19:38 ` Mark Brown 1 sibling, 0 replies; 34+ messages in thread From: Krzysztof Kozlowski @ 2020-08-19 13:06 UTC (permalink / raw) To: Lukasz Stelmach Cc: linux-samsung-soc, b.zolnierkie, linux-kernel, linux-spi, Mark Brown, Kukjin Kim, Andi Shyti, linux-arm-kernel, m.szyprowski 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 2/8] spi: spi-s3s64xx: Add S3C64XX_SPI_QUIRK_CS_AUTO for Exynos3250 2020-08-19 13:01 ` Lukasz Stelmach 2020-08-19 13:06 ` Krzysztof Kozlowski @ 2020-08-19 19:38 ` Mark Brown [not found] ` <CGME20200820104737eucas1p140e3c575eb692a1de54c5a702951ebfe@eucas1p1.samsung.com> 1 sibling, 1 reply; 34+ messages in thread From: Mark Brown @ 2020-08-19 19:38 UTC (permalink / raw) To: Lukasz Stelmach Cc: linux-samsung-soc, b.zolnierkie, linux-kernel, Krzysztof Kozlowski, linux-spi, Kukjin Kim, Andi Shyti, linux-arm-kernel, m.szyprowski [-- Attachment #1.1: Type: text/plain, Size: 971 bytes --] 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. [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <CGME20200820104737eucas1p140e3c575eb692a1de54c5a702951ebfe@eucas1p1.samsung.com>]
* Re: [PATCH 2/8] spi: spi-s3s64xx: Add S3C64XX_SPI_QUIRK_CS_AUTO for Exynos3250 [not found] ` <CGME20200820104737eucas1p140e3c575eb692a1de54c5a702951ebfe@eucas1p1.samsung.com> @ 2020-08-20 10:47 ` Lukasz Stelmach 0 siblings, 0 replies; 34+ messages in thread From: Lukasz Stelmach @ 2020-08-20 10:47 UTC (permalink / raw) To: Mark Brown Cc: linux-samsung-soc, b.zolnierkie, linux-kernel, Krzysztof Kozlowski, linux-spi, Kukjin Kim, Andi Shyti, linux-arm-kernel, m.szyprowski [-- Attachment #1.1: Type: text/plain, Size: 2140 bytes --] 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. -- Łukasz Stelmach Samsung R&D Institute Poland Samsung Electronics [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 487 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <CGME20200819123226eucas1p2dc50cd60f71f2155524ec21bf4dcdd74@eucas1p2.samsung.com>]
* [PATCH 3/8] spi: spi-s3c64xx: Report more information when errors occur [not found] ` <CGME20200819123226eucas1p2dc50cd60f71f2155524ec21bf4dcdd74@eucas1p2.samsung.com> @ 2020-08-19 12:32 ` Łukasz Stelmach 2020-08-19 12:43 ` Krzysztof Kozlowski 0 siblings, 1 reply; 34+ messages in thread From: Łukasz Stelmach @ 2020-08-19 12:32 UTC (permalink / raw) To: Kukjin Kim, Krzysztof Kozlowski, Andi Shyti, Mark Brown, linux-spi, linux-samsung-soc, linux-arm-kernel, linux-kernel Cc: b.zolnierkie, Łukasz Stelmach, m.szyprowski Report amount of pending data when a transfer stops due to errors. Report if DMA was used to transfer data and print the status code. Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> --- drivers/spi/spi-s3c64xx.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c index 8fe44451d303..4ab68cd1b821 100644 --- a/drivers/spi/spi-s3c64xx.c +++ b/drivers/spi/spi-s3c64xx.c @@ -122,6 +122,7 @@ struct s3c64xx_spi_dma_data { struct dma_chan *ch; + dma_cookie_t cookie; enum dma_transfer_direction direction; }; @@ -297,7 +298,7 @@ static void prepare_dma(struct s3c64xx_spi_dma_data *dma, desc->callback = s3c64xx_spi_dmacb; desc->callback_param = dma; - dmaengine_submit(desc); + dma->cookie = dmaengine_submit(desc); dma_async_issue_pending(dma->ch); } @@ -692,17 +693,28 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master, if (status) { dev_err(&spi->dev, - "I/O Error: rx-%d tx-%d res:rx-%c tx-%c len-%d\n", + "I/O Error: rx-%d tx-%d rx-%c tx-%c len-%d dma-%d res-(%d)\n", xfer->rx_buf ? 1 : 0, xfer->tx_buf ? 1 : 0, (sdd->state & RXBUSY) ? 'f' : 'p', (sdd->state & TXBUSY) ? 'f' : 'p', - xfer->len); + xfer->len, use_dma ? 1 : 0, status); if (use_dma) { - if (xfer->tx_buf && (sdd->state & TXBUSY)) + struct dma_tx_state s; + + if (xfer->tx_buf && (sdd->state & TXBUSY)) { + dmaengine_pause(sdd->tx_dma.ch); + dmaengine_tx_status(sdd->tx_dma.ch, sdd->tx_dma.cookie, &s); dmaengine_terminate_all(sdd->tx_dma.ch); - if (xfer->rx_buf && (sdd->state & RXBUSY)) + dev_err(&spi->dev, "TX residue: %d\n", s.residue); + + } + if (xfer->rx_buf && (sdd->state & RXBUSY)) { + dmaengine_pause(sdd->rx_dma.ch); + dmaengine_tx_status(sdd->rx_dma.ch, sdd->rx_dma.cookie, &s); dmaengine_terminate_all(sdd->rx_dma.ch); + dev_err(&spi->dev, "RX residue: %d\n", s.residue); + } } } else { s3c64xx_flush_fifo(sdd); -- 2.26.2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH 3/8] spi: spi-s3c64xx: Report more information when errors occur 2020-08-19 12:32 ` [PATCH 3/8] spi: spi-s3c64xx: Report more information when errors occur Łukasz Stelmach @ 2020-08-19 12:43 ` Krzysztof Kozlowski 0 siblings, 0 replies; 34+ messages in thread From: Krzysztof Kozlowski @ 2020-08-19 12:43 UTC (permalink / raw) To: Łukasz Stelmach Cc: linux-samsung-soc, b.zolnierkie, linux-kernel, linux-spi, Mark Brown, Kukjin Kim, Andi Shyti, linux-arm-kernel, m.szyprowski On Wed, Aug 19, 2020 at 02:32:03PM +0200, Łukasz Stelmach wrote: > Report amount of pending data when a transfer stops due to errors. > > Report if DMA was used to transfer data and print the status code. > > Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> > --- > drivers/spi/spi-s3c64xx.c | 22 +++++++++++++++++----- > 1 file changed, 17 insertions(+), 5 deletions(-) Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org> Best regards, Krzysztof _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <CGME20200819123227eucas1p1f56cc06dc6c368abf2d3952ba04f37e2@eucas1p1.samsung.com>]
* [PATCH 4/8] spi: spi-s3c64xx: Rename S3C64XX_SPI_SLAVE_* to S3C64XX_SPI_CS_* [not found] ` <CGME20200819123227eucas1p1f56cc06dc6c368abf2d3952ba04f37e2@eucas1p1.samsung.com> @ 2020-08-19 12:32 ` Łukasz Stelmach 2020-08-19 12:44 ` Krzysztof Kozlowski 0 siblings, 1 reply; 34+ messages in thread From: Łukasz Stelmach @ 2020-08-19 12:32 UTC (permalink / raw) To: Kukjin Kim, Krzysztof Kozlowski, Andi Shyti, Mark Brown, linux-spi, linux-samsung-soc, linux-arm-kernel, linux-kernel Cc: b.zolnierkie, Łukasz Stelmach, m.szyprowski Rename S3C64XX_SPI_SLAVE_* to S3C64XX_SPI_CS_* to match documentation. Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> --- drivers/spi/spi-s3c64xx.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c index 4ab68cd1b821..6fe896f2be18 100644 --- a/drivers/spi/spi-s3c64xx.c +++ b/drivers/spi/spi-s3c64xx.c @@ -29,7 +29,7 @@ #define S3C64XX_SPI_CH_CFG 0x00 #define S3C64XX_SPI_CLK_CFG 0x04 #define S3C64XX_SPI_MODE_CFG 0x08 -#define S3C64XX_SPI_SLAVE_SEL 0x0C +#define S3C64XX_SPI_CS_REG 0x0C #define S3C64XX_SPI_INT_EN 0x10 #define S3C64XX_SPI_STATUS 0x14 #define S3C64XX_SPI_TX_DATA 0x18 @@ -64,9 +64,9 @@ #define S3C64XX_SPI_MODE_TXDMA_ON (1<<1) #define S3C64XX_SPI_MODE_4BURST (1<<0) -#define S3C64XX_SPI_SLAVE_AUTO (1<<1) -#define S3C64XX_SPI_SLAVE_SIG_INACT (1<<0) -#define S3C64XX_SPI_SLAVE_NSC_CNT_2 (2<<4) +#define S3C64XX_SPI_CS_NSC_CNT_2 (2<<4) +#define S3C64XX_SPI_CS_AUTO (1<<1) +#define S3C64XX_SPI_CS_SIG_INACT (1<<0) #define S3C64XX_SPI_INT_TRAILING_EN (1<<6) #define S3C64XX_SPI_INT_RX_OVERRUN_EN (1<<5) @@ -312,18 +312,18 @@ static void s3c64xx_spi_set_cs(struct spi_device *spi, bool enable) if (enable) { if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO)) { - writel(0, sdd->regs + S3C64XX_SPI_SLAVE_SEL); + writel(0, sdd->regs + S3C64XX_SPI_CS_REG); } else { - u32 ssel = readl(sdd->regs + S3C64XX_SPI_SLAVE_SEL); + u32 ssel = readl(sdd->regs + S3C64XX_SPI_CS_REG); - ssel |= (S3C64XX_SPI_SLAVE_AUTO | - S3C64XX_SPI_SLAVE_NSC_CNT_2); - writel(ssel, sdd->regs + S3C64XX_SPI_SLAVE_SEL); + ssel |= (S3C64XX_SPI_CS_AUTO | + S3C64XX_SPI_CS_NSC_CNT_2); + writel(ssel, sdd->regs + S3C64XX_SPI_CS_REG); } } else { if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO)) - writel(S3C64XX_SPI_SLAVE_SIG_INACT, - sdd->regs + S3C64XX_SPI_SLAVE_SEL); + writel(S3C64XX_SPI_CS_SIG_INACT, + sdd->regs + S3C64XX_SPI_CS_REG); } } @@ -944,9 +944,9 @@ static void s3c64xx_spi_hwinit(struct s3c64xx_spi_driver_data *sdd) sdd->cur_speed = 0; if (sci->no_cs) - writel(0, sdd->regs + S3C64XX_SPI_SLAVE_SEL); + writel(0, sdd->regs + S3C64XX_SPI_CS_REG); else if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO)) - writel(S3C64XX_SPI_SLAVE_SIG_INACT, sdd->regs + S3C64XX_SPI_SLAVE_SEL); + writel(S3C64XX_SPI_CS_SIG_INACT, sdd->regs + S3C64XX_SPI_CS_REG); /* Disable Interrupts - we use Polling if not DMA mode */ writel(0, regs + S3C64XX_SPI_INT_EN); -- 2.26.2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH 4/8] spi: spi-s3c64xx: Rename S3C64XX_SPI_SLAVE_* to S3C64XX_SPI_CS_* 2020-08-19 12:32 ` [PATCH 4/8] spi: spi-s3c64xx: Rename S3C64XX_SPI_SLAVE_* to S3C64XX_SPI_CS_* Łukasz Stelmach @ 2020-08-19 12:44 ` Krzysztof Kozlowski 0 siblings, 0 replies; 34+ messages in thread From: Krzysztof Kozlowski @ 2020-08-19 12:44 UTC (permalink / raw) To: Łukasz Stelmach Cc: linux-samsung-soc, b.zolnierkie, linux-kernel, linux-spi, Mark Brown, Kukjin Kim, Andi Shyti, linux-arm-kernel, m.szyprowski On Wed, Aug 19, 2020 at 02:32:04PM +0200, Łukasz Stelmach wrote: > Rename S3C64XX_SPI_SLAVE_* to S3C64XX_SPI_CS_* to match documentation. > > Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> > --- > drivers/spi/spi-s3c64xx.c | 26 +++++++++++++------------- > 1 file changed, 13 insertions(+), 13 deletions(-) Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org> Best regards, Krzysztof _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <CGME20200819123227eucas1p11cd47cf281a035ed02d5fc819a0370c1@eucas1p1.samsung.com>]
* [PATCH 5/8] spi: spi-s3c64xx: Fix doc comment for struct s3c64xx_spi_driver_data [not found] ` <CGME20200819123227eucas1p11cd47cf281a035ed02d5fc819a0370c1@eucas1p1.samsung.com> @ 2020-08-19 12:32 ` Łukasz Stelmach 2020-08-19 12:37 ` Krzysztof Kozlowski 0 siblings, 1 reply; 34+ messages in thread From: Łukasz Stelmach @ 2020-08-19 12:32 UTC (permalink / raw) To: Kukjin Kim, Krzysztof Kozlowski, Andi Shyti, Mark Brown, linux-spi, linux-samsung-soc, linux-arm-kernel, linux-kernel Cc: b.zolnierkie, Łukasz Stelmach, m.szyprowski Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> --- drivers/spi/spi-s3c64xx.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c index 6fe896f2be18..505789f91fdf 100644 --- a/drivers/spi/spi-s3c64xx.c +++ b/drivers/spi/spi-s3c64xx.c @@ -155,19 +155,21 @@ struct s3c64xx_spi_port_config { * @clk: Pointer to the spi clock. * @src_clk: Pointer to the clock used to generate SPI signals. * @ioclk: Pointer to the i/o clock between master and slave + * @pdev: Pointer to the SPI controller platform device * @master: Pointer to the SPI Protocol master. * @cntrlr_info: Platform specific data for the controller this driver manages. * @lock: Controller specific lock. * @state: Set of FLAGS to indicate status. - * @rx_dmach: Controller's DMA channel for Rx. - * @tx_dmach: Controller's DMA channel for Tx. + * @rx_dma: Controller's DMA channel for Rx. + * @tx_dma: Controller's DMA channel for Tx. * @sfr_start: BUS address of SPI controller regs. * @regs: Pointer to ioremap'ed controller registers. - * @irq: interrupt * @xfer_completion: To indicate completion of xfer task. * @cur_mode: Stores the active configuration of the controller. * @cur_bpw: Stores the active bits per word settings. * @cur_speed: Stores the active xfer clock speed. + * @port_conf: Pointer to the SPI port configuration + * @port_id: SPI port ID number */ struct s3c64xx_spi_driver_data { void __iomem *regs; @@ -176,7 +178,7 @@ struct s3c64xx_spi_driver_data { struct clk *ioclk; struct platform_device *pdev; struct spi_master *master; - struct s3c64xx_spi_info *cntrlr_info; + struct s3c64xx_spi_info *cntrlr_info; spinlock_t lock; unsigned long sfr_start; struct completion xfer_completion; -- 2.26.2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH 5/8] spi: spi-s3c64xx: Fix doc comment for struct s3c64xx_spi_driver_data 2020-08-19 12:32 ` [PATCH 5/8] spi: spi-s3c64xx: Fix doc comment for struct s3c64xx_spi_driver_data Łukasz Stelmach @ 2020-08-19 12:37 ` Krzysztof Kozlowski [not found] ` <CGME20200819132540eucas1p1897948a4f4008ab4946120dd4fa5c192@eucas1p1.samsung.com> 0 siblings, 1 reply; 34+ messages in thread From: Krzysztof Kozlowski @ 2020-08-19 12:37 UTC (permalink / raw) To: Łukasz Stelmach Cc: linux-samsung-soc, b.zolnierkie, linux-kernel, linux-spi, Mark Brown, Kukjin Kim, Andi Shyti, linux-arm-kernel, m.szyprowski On Wed, Aug 19, 2020 at 02:32:05PM +0200, Łukasz Stelmach wrote: > Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> Hi, Missing commit msg - fix what exactly? You need to rebase your patch as most of these were already fixed by Lee Jones. However he did not remove the @rx_dmach and tx entries... Best regards, Krzysztof > --- > drivers/spi/spi-s3c64xx.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c > index 6fe896f2be18..505789f91fdf 100644 > --- a/drivers/spi/spi-s3c64xx.c > +++ b/drivers/spi/spi-s3c64xx.c > @@ -155,19 +155,21 @@ struct s3c64xx_spi_port_config { > * @clk: Pointer to the spi clock. > * @src_clk: Pointer to the clock used to generate SPI signals. > * @ioclk: Pointer to the i/o clock between master and slave > + * @pdev: Pointer to the SPI controller platform device > * @master: Pointer to the SPI Protocol master. > * @cntrlr_info: Platform specific data for the controller this driver manages. > * @lock: Controller specific lock. > * @state: Set of FLAGS to indicate status. > - * @rx_dmach: Controller's DMA channel for Rx. > - * @tx_dmach: Controller's DMA channel for Tx. > + * @rx_dma: Controller's DMA channel for Rx. > + * @tx_dma: Controller's DMA channel for Tx. > * @sfr_start: BUS address of SPI controller regs. > * @regs: Pointer to ioremap'ed controller registers. > - * @irq: interrupt > * @xfer_completion: To indicate completion of xfer task. > * @cur_mode: Stores the active configuration of the controller. > * @cur_bpw: Stores the active bits per word settings. > * @cur_speed: Stores the active xfer clock speed. > + * @port_conf: Pointer to the SPI port configuration > + * @port_id: SPI port ID number > */ _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <CGME20200819132540eucas1p1897948a4f4008ab4946120dd4fa5c192@eucas1p1.samsung.com>]
* Re: [PATCH 5/8] spi: spi-s3c64xx: Fix doc comment for struct s3c64xx_spi_driver_data [not found] ` <CGME20200819132540eucas1p1897948a4f4008ab4946120dd4fa5c192@eucas1p1.samsung.com> @ 2020-08-19 13:25 ` Lukasz Stelmach 0 siblings, 0 replies; 34+ messages in thread From: Lukasz Stelmach @ 2020-08-19 13:25 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: linux-samsung-soc, b.zolnierkie, linux-kernel, linux-spi, Mark Brown, Kukjin Kim, Andi Shyti, linux-arm-kernel, m.szyprowski [-- Attachment #1.1: Type: text/plain, Size: 2017 bytes --] It was <2020-08-19 śro 14:37>, when Krzysztof Kozlowski wrote: > On Wed, Aug 19, 2020 at 02:32:05PM +0200, Łukasz Stelmach wrote: >> Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> > > Hi, > > Missing commit msg - fix what exactly? > > You need to rebase your patch as most of these were already fixed by Lee > Jones. > > However he did not remove the @rx_dmach and tx entries... I will. >> --- >> drivers/spi/spi-s3c64xx.c | 10 ++++++---- >> 1 file changed, 6 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c >> index 6fe896f2be18..505789f91fdf 100644 >> --- a/drivers/spi/spi-s3c64xx.c >> +++ b/drivers/spi/spi-s3c64xx.c >> @@ -155,19 +155,21 @@ struct s3c64xx_spi_port_config { >> * @clk: Pointer to the spi clock. >> * @src_clk: Pointer to the clock used to generate SPI signals. >> * @ioclk: Pointer to the i/o clock between master and slave >> + * @pdev: Pointer to the SPI controller platform device >> * @master: Pointer to the SPI Protocol master. >> * @cntrlr_info: Platform specific data for the controller this driver manages. >> * @lock: Controller specific lock. >> * @state: Set of FLAGS to indicate status. >> - * @rx_dmach: Controller's DMA channel for Rx. >> - * @tx_dmach: Controller's DMA channel for Tx. >> + * @rx_dma: Controller's DMA channel for Rx. >> + * @tx_dma: Controller's DMA channel for Tx. >> * @sfr_start: BUS address of SPI controller regs. >> * @regs: Pointer to ioremap'ed controller registers. >> - * @irq: interrupt >> * @xfer_completion: To indicate completion of xfer task. >> * @cur_mode: Stores the active configuration of the controller. >> * @cur_bpw: Stores the active bits per word settings. >> * @cur_speed: Stores the active xfer clock speed. >> + * @port_conf: Pointer to the SPI port configuration >> + * @port_id: SPI port ID number >> */ > > -- Łukasz Stelmach Samsung R&D Institute Poland Samsung Electronics [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 487 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <CGME20200819123227eucas1p11ec027714c16d5a66c89c6ef1f7b3604@eucas1p1.samsung.com>]
* [PATCH 6/8] spi: spi-s3c64xx: Check return values [not found] ` <CGME20200819123227eucas1p11ec027714c16d5a66c89c6ef1f7b3604@eucas1p1.samsung.com> @ 2020-08-19 12:32 ` Łukasz Stelmach 2020-08-19 12:48 ` Krzysztof Kozlowski 0 siblings, 1 reply; 34+ messages in thread From: Łukasz Stelmach @ 2020-08-19 12:32 UTC (permalink / raw) To: Kukjin Kim, Krzysztof Kozlowski, Andi Shyti, Mark Brown, linux-spi, linux-samsung-soc, linux-arm-kernel, linux-kernel Cc: b.zolnierkie, Łukasz Stelmach, m.szyprowski Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> --- drivers/spi/spi-s3c64xx.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c index 505789f91fdf..27d77600a820 100644 --- a/drivers/spi/spi-s3c64xx.c +++ b/drivers/spi/spi-s3c64xx.c @@ -273,6 +273,7 @@ static void prepare_dma(struct s3c64xx_spi_dma_data *dma, struct s3c64xx_spi_driver_data *sdd; struct dma_slave_config config; struct dma_async_tx_descriptor *desc; + int ret; memset(&config, 0, sizeof(config)); @@ -296,11 +297,22 @@ static void prepare_dma(struct s3c64xx_spi_dma_data *dma, desc = dmaengine_prep_slave_sg(dma->ch, sgt->sgl, sgt->nents, dma->direction, DMA_PREP_INTERRUPT); + if (!desc) { + dev_err(&sdd->pdev->dev, "unable to prepare %s scatterlist", + dma->direction == DMA_DEV_TO_MEM ? "rx" : "tx"); + return; + } desc->callback = s3c64xx_spi_dmacb; desc->callback_param = dma; dma->cookie = dmaengine_submit(desc); + ret = dma_submit_error(dma->cookie); + if (ret) { + dev_err(&sdd->pdev->dev, "dmaengine_submit() failed %d", ret); + return; + } + dma_async_issue_pending(dma->ch); } -- 2.26.2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH 6/8] spi: spi-s3c64xx: Check return values 2020-08-19 12:32 ` [PATCH 6/8] spi: spi-s3c64xx: Check return values Łukasz Stelmach @ 2020-08-19 12:48 ` Krzysztof Kozlowski [not found] ` <CGME20200819154154eucas1p1e88747d2495e6d1074991419504066df@eucas1p1.samsung.com> 0 siblings, 1 reply; 34+ messages in thread From: Krzysztof Kozlowski @ 2020-08-19 12:48 UTC (permalink / raw) To: Łukasz Stelmach Cc: linux-samsung-soc, b.zolnierkie, linux-kernel, linux-spi, Mark Brown, Kukjin Kim, Andi Shyti, linux-arm-kernel, m.szyprowski On Wed, Aug 19, 2020 at 02:32:06PM +0200, Łukasz Stelmach wrote: > Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> > --- > drivers/spi/spi-s3c64xx.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) Oh, come on, stop fixing the same local issue without fixing bigger picture... or at least documenting why bigger picture does not have to be fixed and simple 'return' is enough. That's the third, same fix for the same problem. https://lore.kernel.org/lkml/20190314064202.14864-1-kjlu@umn.edu/ https://lore.kernel.org/lkml/20170207204520.h2eo3yn5kge56lk7@kozik-lap/ Best regards, Krzysztof > > diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c > index 505789f91fdf..27d77600a820 100644 > --- a/drivers/spi/spi-s3c64xx.c > +++ b/drivers/spi/spi-s3c64xx.c > @@ -273,6 +273,7 @@ static void prepare_dma(struct s3c64xx_spi_dma_data *dma, > struct s3c64xx_spi_driver_data *sdd; > struct dma_slave_config config; > struct dma_async_tx_descriptor *desc; > + int ret; > > memset(&config, 0, sizeof(config)); > > @@ -296,11 +297,22 @@ static void prepare_dma(struct s3c64xx_spi_dma_data *dma, > > desc = dmaengine_prep_slave_sg(dma->ch, sgt->sgl, sgt->nents, > dma->direction, DMA_PREP_INTERRUPT); > + if (!desc) { > + dev_err(&sdd->pdev->dev, "unable to prepare %s scatterlist", > + dma->direction == DMA_DEV_TO_MEM ? "rx" : "tx"); > + return; > + } > > desc->callback = s3c64xx_spi_dmacb; > desc->callback_param = dma; > > dma->cookie = dmaengine_submit(desc); > + ret = dma_submit_error(dma->cookie); > + if (ret) { > + dev_err(&sdd->pdev->dev, "dmaengine_submit() failed %d", ret); > + return; > + } > + > dma_async_issue_pending(dma->ch); > } > > -- > 2.26.2 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <CGME20200819154154eucas1p1e88747d2495e6d1074991419504066df@eucas1p1.samsung.com>]
* Re: [PATCH 6/8] spi: spi-s3c64xx: Check return values [not found] ` <CGME20200819154154eucas1p1e88747d2495e6d1074991419504066df@eucas1p1.samsung.com> @ 2020-08-19 15:41 ` Lukasz Stelmach 2020-08-19 16:13 ` Krzysztof Kozlowski 0 siblings, 1 reply; 34+ messages in thread From: Lukasz Stelmach @ 2020-08-19 15:41 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: linux-samsung-soc, b.zolnierkie, linux-kernel, linux-spi, Mark Brown, Kukjin Kim, Andi Shyti, linux-arm-kernel, m.szyprowski [-- Attachment #1.1: Type: text/plain, Size: 2182 bytes --] It was <2020-08-19 śro 14:48>, when Krzysztof Kozlowski wrote: > On Wed, Aug 19, 2020 at 02:32:06PM +0200, Łukasz Stelmach wrote: >> Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> >> --- >> drivers/spi/spi-s3c64xx.c | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) > > Oh, come on, stop fixing the same local issue without fixing bigger > picture... or at least documenting why bigger picture does not have to be > fixed and simple 'return' is enough. > > That's the third, same fix for the same problem. > > https://lore.kernel.org/lkml/20190314064202.14864-1-kjlu@umn.edu/ > https://lore.kernel.org/lkml/20170207204520.h2eo3yn5kge56lk7@kozik-lap/ No wonder. There is a possible NULL dereference below. Now at least we know something about conditions that led to this. Should I drop the entire patch, or just the dmaengine_prep_slave_sg() part? >> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c >> index 505789f91fdf..27d77600a820 100644 >> --- a/drivers/spi/spi-s3c64xx.c >> +++ b/drivers/spi/spi-s3c64xx.c >> @@ -273,6 +273,7 @@ static void prepare_dma(struct s3c64xx_spi_dma_data *dma, >> struct s3c64xx_spi_driver_data *sdd; >> struct dma_slave_config config; >> struct dma_async_tx_descriptor *desc; >> + int ret; >> >> memset(&config, 0, sizeof(config)); >> >> @@ -296,11 +297,22 @@ static void prepare_dma(struct s3c64xx_spi_dma_data *dma, >> >> desc = dmaengine_prep_slave_sg(dma->ch, sgt->sgl, sgt->nents, >> dma->direction, DMA_PREP_INTERRUPT); >> + if (!desc) { >> + dev_err(&sdd->pdev->dev, "unable to prepare %s scatterlist", >> + dma->direction == DMA_DEV_TO_MEM ? "rx" : "tx"); >> + return; >> + } >> >> desc->callback = s3c64xx_spi_dmacb; >> desc->callback_param = dma; >> >> dma->cookie = dmaengine_submit(desc); >> + ret = dma_submit_error(dma->cookie); >> + if (ret) { >> + dev_err(&sdd->pdev->dev, "dmaengine_submit() failed %d", ret); >> + return; >> + } >> + >> dma_async_issue_pending(dma->ch); >> } >> >> -- >> 2.26.2 >> > > -- Łukasz Stelmach Samsung R&D Institute Poland Samsung Electronics [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 487 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 6/8] spi: spi-s3c64xx: Check return values 2020-08-19 15:41 ` Lukasz Stelmach @ 2020-08-19 16:13 ` Krzysztof Kozlowski 0 siblings, 0 replies; 34+ messages in thread From: Krzysztof Kozlowski @ 2020-08-19 16:13 UTC (permalink / raw) To: Lukasz Stelmach Cc: linux-samsung-soc, b.zolnierkie, linux-kernel, linux-spi, Mark Brown, Kukjin Kim, Andi Shyti, linux-arm-kernel, m.szyprowski On Wed, Aug 19, 2020 at 05:41:43PM +0200, Lukasz Stelmach wrote: > It was <2020-08-19 śro 14:48>, when Krzysztof Kozlowski wrote: > > On Wed, Aug 19, 2020 at 02:32:06PM +0200, Łukasz Stelmach wrote: > >> Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> > >> --- > >> drivers/spi/spi-s3c64xx.c | 12 ++++++++++++ > >> 1 file changed, 12 insertions(+) > > > > Oh, come on, stop fixing the same local issue without fixing bigger > > picture... or at least documenting why bigger picture does not have to be > > fixed and simple 'return' is enough. > > > > That's the third, same fix for the same problem. > > > > https://lore.kernel.org/lkml/20190314064202.14864-1-kjlu@umn.edu/ > > https://lore.kernel.org/lkml/20170207204520.h2eo3yn5kge56lk7@kozik-lap/ > > No wonder. There is a possible NULL dereference below. Now at least we > know something about conditions that led to this. > > Should I drop the entire patch, or just the dmaengine_prep_slave_sg() part? The best would be to really go through the call stack and handle the error properly. This means returning an error code and propagating it further. It is not a trivial change... Best regards, Krzysztof _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <CGME20200819123228eucas1p19ac7fc04dec52c733ab9c770e91f6ace@eucas1p1.samsung.com>]
* [PATCH 7/8] spi: spi-s3c64xx: Increase transfer timeout [not found] ` <CGME20200819123228eucas1p19ac7fc04dec52c733ab9c770e91f6ace@eucas1p1.samsung.com> @ 2020-08-19 12:32 ` Łukasz Stelmach 2020-08-19 12:49 ` Krzysztof Kozlowski 0 siblings, 1 reply; 34+ messages in thread From: Łukasz Stelmach @ 2020-08-19 12:32 UTC (permalink / raw) To: Kukjin Kim, Krzysztof Kozlowski, Andi Shyti, Mark Brown, linux-spi, linux-samsung-soc, linux-arm-kernel, linux-kernel Cc: b.zolnierkie, Łukasz Stelmach, m.szyprowski Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> --- drivers/spi/spi-s3c64xx.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c index 27d77600a820..27db1e0f6f32 100644 --- a/drivers/spi/spi-s3c64xx.c +++ b/drivers/spi/spi-s3c64xx.c @@ -464,7 +464,8 @@ static int s3c64xx_wait_for_dma(struct s3c64xx_spi_driver_data *sdd, /* millisecs to xfer 'len' bytes @ 'cur_speed' */ ms = xfer->len * 8 * 1000 / sdd->cur_speed; - ms += 10; /* some tolerance */ + ms = (ms * 10) + 30; /* some tolerance */ + ms = max(ms, 100); /* minimum timeout */ val = msecs_to_jiffies(ms) + 10; val = wait_for_completion_timeout(&sdd->xfer_completion, val); -- 2.26.2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH 7/8] spi: spi-s3c64xx: Increase transfer timeout 2020-08-19 12:32 ` [PATCH 7/8] spi: spi-s3c64xx: Increase transfer timeout Łukasz Stelmach @ 2020-08-19 12:49 ` Krzysztof Kozlowski [not found] ` <CGME20200819133957eucas1p293192baeabb9788ac9148068c1627a57@eucas1p2.samsung.com> 0 siblings, 1 reply; 34+ messages in thread From: Krzysztof Kozlowski @ 2020-08-19 12:49 UTC (permalink / raw) To: Łukasz Stelmach Cc: linux-samsung-soc, b.zolnierkie, linux-kernel, linux-spi, Mark Brown, Kukjin Kim, Andi Shyti, linux-arm-kernel, m.szyprowski On Wed, Aug 19, 2020 at 02:32:07PM +0200, Łukasz Stelmach wrote: > Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> Why? Everything works fine and suddenly minimum timeout is 100 ms? Best regards, Krzysztof > --- > drivers/spi/spi-s3c64xx.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c > index 27d77600a820..27db1e0f6f32 100644 > --- a/drivers/spi/spi-s3c64xx.c > +++ b/drivers/spi/spi-s3c64xx.c > @@ -464,7 +464,8 @@ static int s3c64xx_wait_for_dma(struct s3c64xx_spi_driver_data *sdd, > > /* millisecs to xfer 'len' bytes @ 'cur_speed' */ > ms = xfer->len * 8 * 1000 / sdd->cur_speed; > - ms += 10; /* some tolerance */ > + ms = (ms * 10) + 30; /* some tolerance */ > + ms = max(ms, 100); /* minimum timeout */ > > val = msecs_to_jiffies(ms) + 10; > val = wait_for_completion_timeout(&sdd->xfer_completion, val); > -- > 2.26.2 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <CGME20200819133957eucas1p293192baeabb9788ac9148068c1627a57@eucas1p2.samsung.com>]
* Re: [PATCH 7/8] spi: spi-s3c64xx: Increase transfer timeout [not found] ` <CGME20200819133957eucas1p293192baeabb9788ac9148068c1627a57@eucas1p2.samsung.com> @ 2020-08-19 13:39 ` Lukasz Stelmach 2020-08-21 7:10 ` Krzysztof Kozlowski 0 siblings, 1 reply; 34+ messages in thread From: Lukasz Stelmach @ 2020-08-19 13:39 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: linux-samsung-soc, b.zolnierkie, linux-kernel, linux-spi, Mark Brown, Kukjin Kim, Andi Shyti, linux-arm-kernel, m.szyprowski [-- Attachment #1.1: Type: text/plain, Size: 1329 bytes --] It was <2020-08-19 śro 14:49>, when Krzysztof Kozlowski wrote: > On Wed, Aug 19, 2020 at 02:32:07PM +0200, Łukasz Stelmach wrote: >> Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> > > Why? Everything works fine and suddenly minimum timeout is 100 ms? Actually I am not 100% sure the max() call is required, maybe +30 is enough. Definitely some minimum value is required because for small tranfers (100s of bytes) ms is 0 after the first assignment. >> --- >> drivers/spi/spi-s3c64xx.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c >> index 27d77600a820..27db1e0f6f32 100644 >> --- a/drivers/spi/spi-s3c64xx.c >> +++ b/drivers/spi/spi-s3c64xx.c >> @@ -464,7 +464,8 @@ static int s3c64xx_wait_for_dma(struct s3c64xx_spi_driver_data *sdd, >> >> /* millisecs to xfer 'len' bytes @ 'cur_speed' */ >> ms = xfer->len * 8 * 1000 / sdd->cur_speed; >> - ms += 10; /* some tolerance */ >> + ms = (ms * 10) + 30; /* some tolerance */ >> + ms = max(ms, 100); /* minimum timeout */ >> >> val = msecs_to_jiffies(ms) + 10; >> val = wait_for_completion_timeout(&sdd->xfer_completion, val); >> -- >> 2.26.2 >> > > -- Łukasz Stelmach Samsung R&D Institute Poland Samsung Electronics [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 487 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 7/8] spi: spi-s3c64xx: Increase transfer timeout 2020-08-19 13:39 ` Lukasz Stelmach @ 2020-08-21 7:10 ` Krzysztof Kozlowski 0 siblings, 0 replies; 34+ messages in thread From: Krzysztof Kozlowski @ 2020-08-21 7:10 UTC (permalink / raw) To: Lukasz Stelmach Cc: linux-samsung-soc, b.zolnierkie, linux-kernel, linux-spi, Mark Brown, Kukjin Kim, Andi Shyti, linux-arm-kernel, m.szyprowski On Wed, Aug 19, 2020 at 03:39:56PM +0200, Lukasz Stelmach wrote: > It was <2020-08-19 śro 14:49>, when Krzysztof Kozlowski wrote: > > On Wed, Aug 19, 2020 at 02:32:07PM +0200, Łukasz Stelmach wrote: > >> Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> > > > > Why? Everything works fine and suddenly minimum timeout is 100 ms? > > Actually I am not 100% sure the max() call is required, maybe +30 is > enough. Definitely some minimum value is required because for small > tranfers (100s of bytes) ms is 0 after the first assignment. Sure, just please describe it all in commit msg. All these questions "why?" came because of lack of explanation in commit msg. I guess minimum timeout 100 ms for each transfer is quite high, so maybe just bump the tolerance to 30 which also would be a minimum timeout. Best regards, Krzysztof > > >> --- > >> drivers/spi/spi-s3c64xx.c | 3 ++- > >> 1 file changed, 2 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c > >> index 27d77600a820..27db1e0f6f32 100644 > >> --- a/drivers/spi/spi-s3c64xx.c > >> +++ b/drivers/spi/spi-s3c64xx.c > >> @@ -464,7 +464,8 @@ static int s3c64xx_wait_for_dma(struct s3c64xx_spi_driver_data *sdd, > >> > >> /* millisecs to xfer 'len' bytes @ 'cur_speed' */ > >> ms = xfer->len * 8 * 1000 / sdd->cur_speed; > >> - ms += 10; /* some tolerance */ > >> + ms = (ms * 10) + 30; /* some tolerance */ > >> + ms = max(ms, 100); /* minimum timeout */ > >> > >> val = msecs_to_jiffies(ms) + 10; > >> val = wait_for_completion_timeout(&sdd->xfer_completion, val); > >> -- > >> 2.26.2 > >> > > > > > > -- > Łukasz Stelmach > Samsung R&D Institute Poland > Samsung Electronics _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <CGME20200819123228eucas1p132d530d17256f887d93ed7792f5b9587@eucas1p1.samsung.com>]
* [PATCH 8/8] spi: spi-s3c64xx: Turn on interrupts upon resume [not found] ` <CGME20200819123228eucas1p132d530d17256f887d93ed7792f5b9587@eucas1p1.samsung.com> @ 2020-08-19 12:32 ` Łukasz Stelmach 2020-08-19 12:53 ` Krzysztof Kozlowski 0 siblings, 1 reply; 34+ messages in thread From: Łukasz Stelmach @ 2020-08-19 12:32 UTC (permalink / raw) To: Kukjin Kim, Krzysztof Kozlowski, Andi Shyti, Mark Brown, linux-spi, linux-samsung-soc, linux-arm-kernel, linux-kernel Cc: b.zolnierkie, Łukasz Stelmach, m.szyprowski Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> --- drivers/spi/spi-s3c64xx.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c index 27db1e0f6f32..5dcad43fb847 100644 --- a/drivers/spi/spi-s3c64xx.c +++ b/drivers/spi/spi-s3c64xx.c @@ -1356,6 +1356,10 @@ static int s3c64xx_spi_runtime_resume(struct device *dev) s3c64xx_spi_hwinit(sdd); + writel(S3C64XX_SPI_INT_RX_OVERRUN_EN | S3C64XX_SPI_INT_RX_UNDERRUN_EN | + S3C64XX_SPI_INT_TX_OVERRUN_EN | S3C64XX_SPI_INT_TX_UNDERRUN_EN, + sdd->regs + S3C64XX_SPI_INT_EN); + return 0; err_disable_src_clk: -- 2.26.2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH 8/8] spi: spi-s3c64xx: Turn on interrupts upon resume 2020-08-19 12:32 ` [PATCH 8/8] spi: spi-s3c64xx: Turn on interrupts upon resume Łukasz Stelmach @ 2020-08-19 12:53 ` Krzysztof Kozlowski [not found] ` <CGME20200819133334eucas1p2080182850c7ba84829e3304a1172afff@eucas1p2.samsung.com> 0 siblings, 1 reply; 34+ messages in thread From: Krzysztof Kozlowski @ 2020-08-19 12:53 UTC (permalink / raw) To: Łukasz Stelmach Cc: linux-samsung-soc, b.zolnierkie, linux-kernel, linux-spi, Mark Brown, Kukjin Kim, Andi Shyti, linux-arm-kernel, m.szyprowski On Wed, Aug 19, 2020 at 02:32:08PM +0200, Łukasz Stelmach wrote: > Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> > --- > drivers/spi/spi-s3c64xx.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c > index 27db1e0f6f32..5dcad43fb847 100644 > --- a/drivers/spi/spi-s3c64xx.c > +++ b/drivers/spi/spi-s3c64xx.c > @@ -1356,6 +1356,10 @@ static int s3c64xx_spi_runtime_resume(struct device *dev) > > s3c64xx_spi_hwinit(sdd); > > + writel(S3C64XX_SPI_INT_RX_OVERRUN_EN | S3C64XX_SPI_INT_RX_UNDERRUN_EN | > + S3C64XX_SPI_INT_TX_OVERRUN_EN | S3C64XX_SPI_INT_TX_UNDERRUN_EN, > + sdd->regs + S3C64XX_SPI_INT_EN); > + Makes sense but you should explain in the commit msg what happens without this (or why they are not enabled). Best regards, Krzysztof _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <CGME20200819133334eucas1p2080182850c7ba84829e3304a1172afff@eucas1p2.samsung.com>]
* Re: [PATCH 8/8] spi: spi-s3c64xx: Turn on interrupts upon resume [not found] ` <CGME20200819133334eucas1p2080182850c7ba84829e3304a1172afff@eucas1p2.samsung.com> @ 2020-08-19 13:33 ` Lukasz Stelmach 0 siblings, 0 replies; 34+ messages in thread From: Lukasz Stelmach @ 2020-08-19 13:33 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: linux-samsung-soc, b.zolnierkie, linux-kernel, linux-spi, Mark Brown, Kukjin Kim, Andi Shyti, linux-arm-kernel, m.szyprowski [-- Attachment #1.1: Type: text/plain, Size: 1019 bytes --] It was <2020-08-19 śro 14:53>, when Krzysztof Kozlowski wrote: > On Wed, Aug 19, 2020 at 02:32:08PM +0200, Łukasz Stelmach wrote: >> Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> >> --- >> drivers/spi/spi-s3c64xx.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c >> index 27db1e0f6f32..5dcad43fb847 100644 >> --- a/drivers/spi/spi-s3c64xx.c >> +++ b/drivers/spi/spi-s3c64xx.c >> @@ -1356,6 +1356,10 @@ static int s3c64xx_spi_runtime_resume(struct device *dev) >> >> s3c64xx_spi_hwinit(sdd); >> >> + writel(S3C64XX_SPI_INT_RX_OVERRUN_EN | S3C64XX_SPI_INT_RX_UNDERRUN_EN | >> + S3C64XX_SPI_INT_TX_OVERRUN_EN | S3C64XX_SPI_INT_TX_UNDERRUN_EN, >> + sdd->regs + S3C64XX_SPI_INT_EN); >> + > > Makes sense but you should explain in the commit msg what happens > without this (or why they are not enabled). Done, thanks. -- Łukasz Stelmach Samsung R&D Institute Poland Samsung Electronics [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 487 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 0/8] Some fixes for spi-s3c64xx 2020-08-19 12:32 ` [PATCH 0/8] Some fixes for spi-s3c64xx Łukasz Stelmach ` (7 preceding siblings ...) [not found] ` <CGME20200819123228eucas1p132d530d17256f887d93ed7792f5b9587@eucas1p1.samsung.com> @ 2020-08-19 12:50 ` Krzysztof Kozlowski 8 siblings, 0 replies; 34+ messages in thread From: Krzysztof Kozlowski @ 2020-08-19 12:50 UTC (permalink / raw) To: Łukasz Stelmach Cc: linux-samsung-soc, b.zolnierkie, linux-kernel, linux-spi, Mark Brown, Kukjin Kim, Andi Shyti, linux-arm-kernel, m.szyprowski On Wed, Aug 19, 2020 at 02:32:00PM +0200, Łukasz Stelmach wrote: > This is a series of fixes created during porting a device driver (these > patches will be released soon too) for an SPI device to the current kernel. > > The two most important are > > spi: spi-s3c64xx: swap s3c64xx_spi_set_cs() and s3c64xx_enable_datapath() > spi: spi-s3s64xx: Add S3C64XX_SPI_QUIRK_CS_AUTO for Exynos3250 > > Without them DMA transfers larger than 512 bytes from the SPI controller > would fail. If these are two most important patches here, there should have a commit message explaining their importance. Best regards, Krzysztof _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2020-08-21 7:12 UTC | newest] Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CGME20200819123225eucas1p28be1b1920ade0ba8997bc17da97599b6@eucas1p2.samsung.com> 2020-08-19 12:32 ` [PATCH 0/8] Some fixes for spi-s3c64xx Łukasz Stelmach [not found] ` <CGME20200819123226eucas1p16c9b90330d957344f99f6ee461190085@eucas1p1.samsung.com> 2020-08-19 12:32 ` [PATCH 1/8] spi: spi-s3c64xx: swap s3c64xx_spi_set_cs() and s3c64xx_enable_datapath() Łukasz Stelmach 2020-08-19 12:38 ` Krzysztof Kozlowski [not found] ` <CGME20200819125150eucas1p1965fab59b6e75cf54cac262161c5695b@eucas1p1.samsung.com> 2020-08-19 12:51 ` Lukasz Stelmach 2020-08-19 12:58 ` Krzysztof Kozlowski 2020-08-19 13:16 ` Mark Brown [not found] ` <CGME20200819140203eucas1p2818858289f2394b32f3c647e47705cd2@eucas1p2.samsung.com> 2020-08-19 14:01 ` Lukasz Stelmach 2020-08-19 19:12 ` Mark Brown [not found] ` <CGME20200820101251eucas1p237a794cc11f44c709c0ccdfef766702c@eucas1p2.samsung.com> 2020-08-20 10:12 ` Lukasz Stelmach [not found] ` <CGME20200819123226eucas1p2f4be625abd7ddaac2f09bdf94395346b@eucas1p2.samsung.com> 2020-08-19 12:32 ` [PATCH 2/8] spi: spi-s3s64xx: Add S3C64XX_SPI_QUIRK_CS_AUTO for Exynos3250 Łukasz Stelmach 2020-08-19 12:39 ` Krzysztof Kozlowski [not found] ` <CGME20200819130122eucas1p27e9e84c4399d01409858de6d01e11b52@eucas1p2.samsung.com> 2020-08-19 13:01 ` Lukasz Stelmach 2020-08-19 13:06 ` Krzysztof Kozlowski 2020-08-19 19:38 ` Mark Brown [not found] ` <CGME20200820104737eucas1p140e3c575eb692a1de54c5a702951ebfe@eucas1p1.samsung.com> 2020-08-20 10:47 ` Lukasz Stelmach [not found] ` <CGME20200819123226eucas1p2dc50cd60f71f2155524ec21bf4dcdd74@eucas1p2.samsung.com> 2020-08-19 12:32 ` [PATCH 3/8] spi: spi-s3c64xx: Report more information when errors occur Łukasz Stelmach 2020-08-19 12:43 ` Krzysztof Kozlowski [not found] ` <CGME20200819123227eucas1p1f56cc06dc6c368abf2d3952ba04f37e2@eucas1p1.samsung.com> 2020-08-19 12:32 ` [PATCH 4/8] spi: spi-s3c64xx: Rename S3C64XX_SPI_SLAVE_* to S3C64XX_SPI_CS_* Łukasz Stelmach 2020-08-19 12:44 ` Krzysztof Kozlowski [not found] ` <CGME20200819123227eucas1p11cd47cf281a035ed02d5fc819a0370c1@eucas1p1.samsung.com> 2020-08-19 12:32 ` [PATCH 5/8] spi: spi-s3c64xx: Fix doc comment for struct s3c64xx_spi_driver_data Łukasz Stelmach 2020-08-19 12:37 ` Krzysztof Kozlowski [not found] ` <CGME20200819132540eucas1p1897948a4f4008ab4946120dd4fa5c192@eucas1p1.samsung.com> 2020-08-19 13:25 ` Lukasz Stelmach [not found] ` <CGME20200819123227eucas1p11ec027714c16d5a66c89c6ef1f7b3604@eucas1p1.samsung.com> 2020-08-19 12:32 ` [PATCH 6/8] spi: spi-s3c64xx: Check return values Łukasz Stelmach 2020-08-19 12:48 ` Krzysztof Kozlowski [not found] ` <CGME20200819154154eucas1p1e88747d2495e6d1074991419504066df@eucas1p1.samsung.com> 2020-08-19 15:41 ` Lukasz Stelmach 2020-08-19 16:13 ` Krzysztof Kozlowski [not found] ` <CGME20200819123228eucas1p19ac7fc04dec52c733ab9c770e91f6ace@eucas1p1.samsung.com> 2020-08-19 12:32 ` [PATCH 7/8] spi: spi-s3c64xx: Increase transfer timeout Łukasz Stelmach 2020-08-19 12:49 ` Krzysztof Kozlowski [not found] ` <CGME20200819133957eucas1p293192baeabb9788ac9148068c1627a57@eucas1p2.samsung.com> 2020-08-19 13:39 ` Lukasz Stelmach 2020-08-21 7:10 ` Krzysztof Kozlowski [not found] ` <CGME20200819123228eucas1p132d530d17256f887d93ed7792f5b9587@eucas1p1.samsung.com> 2020-08-19 12:32 ` [PATCH 8/8] spi: spi-s3c64xx: Turn on interrupts upon resume Łukasz Stelmach 2020-08-19 12:53 ` Krzysztof Kozlowski [not found] ` <CGME20200819133334eucas1p2080182850c7ba84829e3304a1172afff@eucas1p2.samsung.com> 2020-08-19 13:33 ` Lukasz Stelmach 2020-08-19 12:50 ` [PATCH 0/8] Some fixes for spi-s3c64xx Krzysztof Kozlowski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).