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:
signature.asc
Description: PGP signature
