- 07 Jul, 2020 1 commit
-
-
Peng Fan authored
There is no need to call spi_master_put() if spi_alloc_master() failed, it should return -ENOMEM directly. Signed-off-by: Peng Fan <fanpeng@loongson.cn> Link: https://lore.kernel.org/r/1594111842-9468-1-git-send-email-fanpeng@loongson.cnSigned-off-by: Mark Brown <broonie@kernel.org>
-
- 01 Jul, 2020 12 commits
-
-
Mark Brown authored
Merge series "Add Renesas RPC-IF support" from Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>: Hello! Here's a set of 2 patches against Linus' repo. Renesas Reduced Pin Count Interface (RPC-IF) allows a SPI flash or HyperFlash connected to the SoC to be accessed via the external address space read mode or the manual mode. The memory controller driver for RPC-IF registers either SPI or HyperFLash subdevice, depending on the contents of the device tree subnode; it also provides the abstract "back end" API that can be used by the "front end" SPI/MTD drivers to talk to the real hardware... Based on the original patch by Mason Yang <masonccyang@mxic.com.tw>. [1/2] dt-bindings: memory: document Renesas RPC-IF bindings [2/2] memory: add Renesas RPC-IF driver MBR, Sergei
-
Mark Brown authored
Merge series "spi: bcm2835: Interrupt-handling optimisations" from Robin Murphy <robin.murphy@arm.com>: Hi all, Although Florian was concerned about a trivial inline check to deal with shared IRQs adding overhead, the reality is that it would be so small as to not be worth even thinking about unless the driver was already tuned to squeeze out every last cycle. And a brief look over the code shows that that clearly isn't the case. This is an example of some of the easy low-hanging fruit that jumps out just from code inspection. Based on disassembly and ARM1176 cycle timings, patch #2 should save the equivalent of 2-3 shared interrupt checks off the critical path in all cases, and patch #3 possibly up to about 100x more. I don't have any means to test these patches, let alone measure performance, so they're only backed by the principle that less code - and in particular fewer memory accesses - is almost always better. There is almost certainly a *lot* more to be had from careful use of relaxed I/O accessors, not doing a read-modify-write of CS at every reset, tweaking the loops further to avoid unnecessary writebacks to variables, and so on. However since I'm not invested in this personally I'm not going to pursue it any further; I'm throwing these patches out as more of a demonstration to back up my original drive-by review comments, so if anyone want to pick them up and run with them then please do so. Robin. Robin Murphy (3): spi: bcm3835: Tidy up bcm2835_spi_reset_hw() spi: bcm2835: Micro-optimise IRQ handler spi: bcm2835: Micro-optimise FIFO loops drivers/spi/spi-bcm2835.c | 45 +++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 23 deletions(-) -- 2.23.0.dirty _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
-
Linus Walleij authored
This switches the Lantiq SSC driver over to use GPIO descriptor handling in the core. The driver was already utilizing the core to look up and request GPIOs from the device tree so this is a pretty small change just switching it over to use descriptors directly instead. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> Cc: Hauke Mehrtens <hauke@hauke-m.de> Link: https://lore.kernel.org/r/20200625202149.209276-1-linus.walleij@linaro.orgSigned-off-by: Mark Brown <broonie@kernel.org>
-
Linus Walleij authored
This converts the IMG SPFI SPI driver to use GPIO descriptors as obtained from the core instead of GPIO numbers. The driver was already relying on the core code to look up the GPIO numbers from the device tree and allocate memory for storing state etc. By moving to use descriptors handled by the core we can delete the setup/cleanup functions and the device state handler that were only dealing with this. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> Cc: Ionela Voinescu <ionela.voinescu@imgtec.com> Cc: Sifan Naeem <sifan.naeem@imgtec.com> Link: https://lore.kernel.org/r/20200625201422.208640-1-linus.walleij@linaro.orgSigned-off-by: Mark Brown <broonie@kernel.org>
-
Linus Walleij authored
The Nuvoton PSPI driver already uses the core to handle GPIO chip selects but is using the old GPIO number method and retrieveing the GPIOs in the probe() call. Switch it over to using GPIO descriptors saving a bunch of code and modernizing it. Compile tested med ARMv7 multiplatform config augmented with the Nuvoton arch and this driver. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> Cc: Tomer Maimon <tmaimon77@gmail.com> Link: https://lore.kernel.org/r/20200625225759.273911-1-linus.walleij@linaro.orgSigned-off-by: Mark Brown <broonie@kernel.org>
-
Douglas Anderson authored
On some SPI controllers (like spi-geni-qcom) setting the chip select is a heavy operation. For instance on spi-geni-qcom, with the current code, is was measured as taking upwards of 20 us. Even on SPI controllers that aren't as heavy, setting the chip select is at least something like a MMIO operation over some peripheral bus which isn't as fast as a RAM access. While it would be good to find ways to mitigate problems like this in the drivers for those SPI controllers, it can also be noted that the SPI framework could also help out. Specifically, in some situations, we can see the SPI framework calling the driver's set_cs() with the same parameter several times in a row. This is specifically observed when looking at the way the Chrome OS EC SPI driver (cros_ec_spi) works but other drivers likely trip it to some extent. Let's solve this by caching the chip select state in the core and only calling into the controller if there was a change. We check not only the "enable" state but also the chip select mode (active high or active low) since controllers may care about both the mode and the enable flag in their callback. Signed-off-by: Douglas Anderson <dianders@chromium.org> Link: https://lore.kernel.org/r/20200629164103.1.Ied8e8ad8bbb2df7f947e3bc5ea1c315e041785a2@changeidSigned-off-by: Mark Brown <broonie@kernel.org>
-
Luc Van Oostenryck authored
The field mspi->reg_base is annotated as an __iomem pointer. Good. However, this field is often assigned to a temporary variable: before being used. For example: struct fsl_spi_reg *reg_base = mspi->reg_base; But this variable is missing the __iomem annotation. So, add the missing __iomem and make sparse & the bot happier. Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> Link: https://lore.kernel.org/r/20200622162611.83694-1-luc.vanoostenryck@gmail.comSigned-off-by: Mark Brown <broonie@kernel.org>
-
Sergei Shtylyov authored
Add the memory driver for Renesas RPC-IF which registers either SPI or HyperFLash device depending on the contents of the device tree subnode. It also provides the absract "back end" device APIs that can be used by the "front end" SPI/MTD drivers to talk to the real hardware. Based on the original patch by Mason Yang <masonccyang@mxic.com.tw>. Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> Link: https://lore.kernel.org/r/9a3606ec-d4d0-c63a-4fb6-631ab38e621c@cogentembedded.comSigned-off-by: Mark Brown <broonie@kernel.org>
-
Sergei Shtylyov authored
Renesas Reduced Pin Count Interface (RPC-IF) allows a SPI flash or HyperFlash connected to the SoC to be accessed via the external address space read mode or the manual mode. Document the device tree bindings for the Renesas RPC-IF found in the R-Car gen3 SoCs. Based on the original patch by Mason Yang <masonccyang@mxic.com.tw>. Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> Link: https://lore.kernel.org/r/54a84c75-fa17-9976-d9a6-a69ef67c418b@cogentembedded.comSigned-off-by: Mark Brown <broonie@kernel.org>
-
Robin Murphy authored
The blind and counted loops are always called with nonzero count, so convert them to do-while loops that lead to slightly more efficient code generation. With GCC 8.3 this shaves off 1-2 instructions per iteration in each case. Signed-off-by: Robin Murphy <robin.murphy@arm.com> Link: https://lore.kernel.org/r/9242863077acf9a64e4b3720e479855b88d19e82.1592261248.git.robin.murphy@arm.comSigned-off-by: Mark Brown <broonie@kernel.org>
-
Robin Murphy authored
The IRQ handler only needs the struct spi_controller for the sake of the completion at the end of a transfer. Passing the struct bcm2835_spi directly as the IRQ data allows that level of indirection to be pushed into the completion path for the reverse lookup, and avoided entirely in all other cases. This saves one explicit load in the critical path, plus (for a GCC 8.3 build) two registers worth of stack frame overhead. Signed-off-by: Robin Murphy <robin.murphy@arm.com> Link: https://lore.kernel.org/r/6b401cb521539caffab21f05b4c8cba6c9d27c6e.1592261248.git.robin.murphy@arm.comSigned-off-by: Mark Brown <broonie@kernel.org>
-
Robin Murphy authored
It doesn't need a struct spi_controller, and every callsite has already retrieved the appropriate struct bcm2835_spi, so just pass that directly. Signed-off-by: Robin Murphy <robin.murphy@arm.com> Link: https://lore.kernel.org/r/eca458ae1a0d3934d0627f90e25d294fefd4b13d.1592261248.git.robin.murphy@arm.comSigned-off-by: Mark Brown <broonie@kernel.org>
-
- 29 Jun, 2020 2 commits
-
-
Linus Walleij authored
The OMAP2 MCSPI has some kind of half-baked GPIO CS support: it includes code like this: if (gpio_is_valid(spi->cs_gpio)) { ret = gpio_request(spi->cs_gpio, dev_name(&spi->dev)); (...) But it doesn't parse the "cs-gpios" attribute in the device tree to count the number of GPIOs or pick out the GPIO numbers and put these in the SPI master's .cs_gpios property. We complete the implementation of supporting CS GPIOs from the device tree and switch it over to use the SPI core for this. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> Acked-by: Tony Lindgren <tony@atomide.com> Link: https://lore.kernel.org/r/20200625231257.280615-1-linus.walleij@linaro.orgSigned-off-by: Mark Brown <broonie@kernel.org>
-
Douglas Anderson authored
Setting the chip select on the Qualcomm geni SPI controller isn't exactly cheap. Let's cache the current setting and avoid setting the chip select if it's already right. Using "flashrom" to read or write the EC firmware on a Chromebook shows roughly a 25% reduction in interrupts and a 15% speedup. Signed-off-by: Douglas Anderson <dianders@chromium.org> Link: https://lore.kernel.org/r/20200626151946.1.I06134fd669bf91fd387dc6ecfe21d44c202bd412@changeidSigned-off-by: Mark Brown <broonie@kernel.org>
-
- 24 Jun, 2020 2 commits
-
-
Xu Yilun authored
Add the MODULE_DEVICE_TABLE macro for the platform_device_id table to allow proper creation of modalias strings and fix autoloading module for this driver. Signed-off-by: Xu Yilun <yilun.xu@intel.com> Signed-off-by: Russ Weight <russell.h.weight@intel.com> Link: https://lore.kernel.org/r/1592962286-25752-3-git-send-email-yilun.xu@intel.comSigned-off-by: Mark Brown <broonie@kernel.org>
-
Xu Yilun authored
The driver is expected to support device ID "spi_altera" for MMIO accessed devices, device ID "subdev_spi_altera" for indirect accessed devices. But the platform bus will not try driver name match anymore if the platform driver has an id_table. So the "spi_altera" should also be added to id_table. Signed-off-by: Xu Yilun <yilun.xu@intel.com> Signed-off-by: Russ Weight <russell.h.weight@intel.com> Link: https://lore.kernel.org/r/1592962286-25752-2-git-send-email-yilun.xu@intel.comSigned-off-by: Mark Brown <broonie@kernel.org>
-
- 23 Jun, 2020 2 commits
-
-
Robin Gong authored
Add fallback pio feature in case dma transfer failed before start. Besides, another whole pio transfer including setup_transfer will be issued by spi core, no need to restore jobs like commit bcd8e776 ("spi: imx: fallback to PIO if dma setup failure"). Signed-off-by: Robin Gong <yibin.gong@nxp.com> Link: https://lore.kernel.org/r/1592347329-28363-3-git-send-email-yibin.gong@nxp.comSigned-off-by: Mark Brown <broonie@kernel.org>
-
Robin Gong authored
Add fallback to pio mode in case dma transfer failed with error status SPI_TRANS_FAIL_NO_START. If spi client driver want to enable this feature please set xfer->error in the proper place such as dmaengine_prep_slave_sg() failure detect(but no any data put into spi bus yet). Besides, add master->fallback checking in its can_dma() so that spi core could switch to pio next time. Please refer to spi-imx.c. Signed-off-by: Robin Gong <yibin.gong@nxp.com> Link: https://lore.kernel.org/r/1592347329-28363-2-git-send-email-yibin.gong@nxp.comSigned-off-by: Mark Brown <broonie@kernel.org>
-
- 22 Jun, 2020 3 commits
-
-
Mark Brown authored
To follow onto Doug's latest spi geni series[1] this simplifies and reduces the code a little more. [1] https://lore.kernel.org/r/20200618150626.237027-1-dianders@chromium.org Stephen Boyd (2): spi: spi-geni-qcom: Simplify setup_fifo_xfer() spi: spi-geni-qcom: Don't set {tx,rx}_rem_bytes unnecessarily drivers/spi/spi-geni-qcom.c | 55 +++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 30 deletions(-) base-commit: 7ba9bdcb -- Sent by a computer, using git, on the internet
-
Stephen Boyd authored
We only need to test for these counters being non-zero when we see the end of a transfer. If we're doing a CS change then they will already be zero. This implies that we don't need to set these to 0 if we're cancelling an in flight transfer too, because we only care to test these counters when the 'DONE' bit is set in the hardware and we've set them to non-zero for a transfer. This is a non-functional change, just cleanup to consolidate code. Signed-off-by: Stephen Boyd <swboyd@chromium.org> Reviewed-by: Douglas Anderson <dianders@chromium.org> Link: https://lore.kernel.org/r/20200620022233.64716-3-swboyd@chromium.orgSigned-off-by: Mark Brown <broonie@kernel.org>
-
Stephen Boyd authored
The definition of SPI_FULL_DUPLEX (3) is really SPI_TX_ONLY (1) ORed with SPI_RX_ONLY (2). Let's drop the define and simplify the code here a bit by collapsing the setting of 'm_cmd' into conditions that are the same. This is a non-functional change, just cleanup to consolidate code. Signed-off-by: Stephen Boyd <swboyd@chromium.org> Reviewed-by: Douglas Anderson <dianders@chromium.org> Link: https://lore.kernel.org/r/20200620022233.64716-2-swboyd@chromium.orgSigned-off-by: Mark Brown <broonie@kernel.org>
-
- 19 Jun, 2020 18 commits
-
-
Stephen Boyd authored
The definition of SPI_FULL_DUPLEX (3) is really SPI_TX_ONLY (1) ORed with SPI_RX_ONLY (2). Let's drop the define and simplify the code here a bit by collapsing the setting of 'm_cmd' into conditions that are the same. This is a non-functional change, just cleanup to consolidate code. Signed-off-by: Stephen Boyd <swboyd@chromium.org> Reviewed-by: Douglas Anderson <dianders@chromium.org> Cc: Douglas Anderson <dianders@chromium.org> Link: https://lore.kernel.org/r/20200618233959.160032-1-swboyd@chromium.orgSigned-off-by: Mark Brown <broonie@kernel.org>
-
Mark Brown authored
Merge series "mtd: spi-nor: Move cadence-qaudspi to spi-mem framework" from Vignesh Raghavendra <vigneshr@ti.com>: This series is a subset of "[PATCH v12 0/4] spi: cadence-quadspi: Add support for the Cadence QSPI controller" by Ramuthevar,Vadivel MuruganX <vadivel.muruganx.ramuthevar@linux.intel.com> that intended to move cadence-quadspi driver to spi-mem framework Those patches were trying to accomplish too many things in a single set of patches and need to split into smaller patches. This is reduced version of above series. Changes that are intended to make migration easy are split into separate patches. Patches 1 to 3 drop features that cannot be supported under spi-mem at the moment (backward compatibility is maintained). Patch 4-5 are trivial cleanups. Patch 6 does the actual conversion to spi-mem and patch 7 moves the driver to drivers/spi folder. I have tested both INDAC mode (used by non TI platforms like Altera SoCFPGA) and DAC mode (used by TI platforms) on TI EVMs. Patches to move move bindings over to "Documentation/devicetree/bindings/spi/" directory and also conversion of bindig doc to YAML will be posted separately. Support for Intel platform would follow that. Resend v3: Rebased onto v5.7-c1 v3: Split handling of probe deferral into separate patch (out of 5/6) Split dropping of redundant WREN to separate patch (out of 5/6) Fix a possible memleak due to lack of spi_master_put() Parse all SPI slave nodes in cqspi_setup_flash() Address misc comments from Tudor on v2 Rebase onto latest spi-nor/next v2: Rework patch 1/6 to keep "cdns,is-decoded-cs" property supported. Ramuthevar Vadivel Murugan (2): mtd: spi-nor: Convert cadence-quadspi to use spi-mem framework spi: Move cadence-quadspi driver to drivers/spi/ Vignesh Raghavendra (6): mtd: spi-nor: cadence-quadspi: Make driver independent of flash geometry mtd: spi-nor: cadence-quadspi: Provide a way to disable DAC mode mtd: spi-nor: cadence-quadspi: Don't initialize rx_dma_complete on failure mtd: spi-nor: cadence-quadspi: Fix error path on failure to acquire reset lines mtd: spi-nor: cadence-quadspi: Handle probe deferral while requesting DMA channel mtd: spi-nor: cadence-quadspi: Drop redundant WREN in erase path drivers/mtd/spi-nor/controllers/Kconfig | 11 - drivers/mtd/spi-nor/controllers/Makefile | 1 - drivers/spi/Kconfig | 11 + drivers/spi/Makefile | 1 + .../spi-cadence-quadspi.c} | 541 +++++++----------- 5 files changed, 222 insertions(+), 343 deletions(-) rename drivers/{mtd/spi-nor/controllers/cadence-quadspi.c => spi/spi-cadence-quadspi.c} (74%) base-commit: b3a9e3b9 -- 2.26.2 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
-
Ramuthevar Vadivel Murugan authored
Now that cadence-quadspi has been converted to use spi-mem framework, move it under drivers/spi/ Update license header to match SPI subsystem style Signed-off-by: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com> Acked-by: Tudor Ambarus <tudor.ambarus@microchip.com> Link: https://lore.kernel.org/r/20200601070444.16923-9-vigneshr@ti.comSigned-off-by: Mark Brown <broonie@kernel.org>
-
Ramuthevar Vadivel Murugan authored
Move cadence-quadspi driver to use spi-mem framework. This is required to make the driver support for SPI NAND flashes in future. Driver is feature compliant with existing SPI NOR version. Signed-off-by: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com> Acked-by: Tudor Ambarus <tudor.ambarus@microchip.com> Link: https://lore.kernel.org/r/20200601070444.16923-8-vigneshr@ti.comSigned-off-by: Mark Brown <broonie@kernel.org>
-
Vignesh Raghavendra authored
Drop redundant WREN command in cqspi_erase() as SPI NOR core takes care of sending WREN command before sending erase command. Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com> Acked-by: Tudor Ambarus <tudor.ambarus@microchip.com> Link: https://lore.kernel.org/r/20200601070444.16923-7-vigneshr@ti.comSigned-off-by: Mark Brown <broonie@kernel.org>
-
Vignesh Raghavendra authored
dma_request_chan_by_mask() can throw EPROBE_DEFER if DMA provider is not yet probed. Currently driver just falls back to using PIO mode (which is less efficient) in this case. Instead return probe deferral error as is so that driver will be re probed once DMA provider is available. Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com> Acked-by: Tudor Ambarus <tudor.ambarus@microchip.com> Link: https://lore.kernel.org/r/20200601070444.16923-6-vigneshr@ti.comSigned-off-by: Mark Brown <broonie@kernel.org>
-
Vignesh Raghavendra authored
Make sure to undo the prior changes done by the driver when exiting due to failure to acquire reset lines. Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com> Acked-by: Tudor Ambarus <tudor.ambarus@microchip.com> Link: https://lore.kernel.org/r/20200601070444.16923-5-vigneshr@ti.comSigned-off-by: Mark Brown <broonie@kernel.org>
-
Vignesh Raghavendra authored
If driver fails to acquire DMA channel then don't initialize rx_dma_complete struct as it won't be used. Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com> Acked-by: Tudor Ambarus <tudor.ambarus@microchip.com> Link: https://lore.kernel.org/r/20200601070444.16923-4-vigneshr@ti.comSigned-off-by: Mark Brown <broonie@kernel.org>
-
Vignesh Raghavendra authored
Currently direct access mode is used on platforms that have AHB window (memory mapped window) larger than flash size. This feature is limited to TI platforms as non TI platforms have < 1MB of AHB window. Therefore introduce a driver quirk to disable DAC mode and set it for non TI compatibles. This is in preparation to move to spi-mem framework where flash geometry cannot be known. Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com> Acked-by: Tudor Ambarus <tudor.ambarus@microchip.com> Link: https://lore.kernel.org/r/20200601070444.16923-3-vigneshr@ti.comSigned-off-by: Mark Brown <broonie@kernel.org>
-
Vignesh Raghavendra authored
Drop configuration of Flash size, erase size and page size configuration. Flash size is needed only if using AHB decoder (BIT 23 of CONFIG_REG) which is not used by the driver. Erase size and page size are needed if IP is configured to send WREN automatically. But since SPI NOR layer takes care of sending WREN, there is no need to configure these fields either. Therefore drop these in preparation to move the driver to spi-mem framework where flash geometry is not visible to controller driver. Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com> Acked-by: Tudor Ambarus <tudor.ambarus@microchip.com> Link: https://lore.kernel.org/r/20200601070444.16923-2-vigneshr@ti.comSigned-off-by: Mark Brown <broonie@kernel.org>
-
Mark Brown authored
Updated the regmap & indirect access support for spi-altera. Patch #1 is an 1:1 replacement of of readl/writel with regmap_read/write Patch #2 introduced a new platform_device_id to support indirect access as a sub device. Patch #3 is a minor fix. Main changes from v1: - Split the regmap supporting patch to 2 patches. - Add a new platform_device_id to support indirect access. - Removed the v1 patch "move driver name string to header file". Now we use driver name string directly. - Add Yilun's Signed-off-by for Patch #3. - Add Tom's Reviewed-by. Matthew Gerlach (1): spi: altera: fix size mismatch on 64 bit processors Xu Yilun (2): spi: altera: use regmap-mmio instead of direct mmio register access spi: altera: support indirect access to the registers drivers/spi/Kconfig | 1 + drivers/spi/spi-altera.c | 127 +++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 107 insertions(+), 21 deletions(-) -- 2.7.4
-
Mark Brown authored
Merge series "spi: spi-geni-qcom: Fixes / perf improvements" from Douglas Anderson <dianders@chromium.org>: This patch series is a new version of the previous patch posted: [PATCH v2] spi: spi-geni-qcom: Speculative fix of "nobody cared" about interrupt https://lore.kernel.org/r/20200317133653.v2.1.I752ebdcfd5e8bf0de06d66e767b8974932b3620e@changeid At this point I've done enough tracing to know that there was a real race in the old code (not just weakly ordered memory problems) and that should be fixed with the locking patches. While looking at this driver, I also noticed we weren't properly noting error interrupts and also weren't actually using our FIFO effectively, so I fixed those. The last patch in the series addresses review feedback about dislike for the "cur_mcmd" state variable. It also could possibly make "abort" work ever-so-slightly more reliably. Changes in v4: - Drop 'controller' in comment. - Use Stephen's diagram to explain the race better. Changes in v3: - ("spi: spi-geni-qcom: No need for irqsave variant...") new for v3 - Split out some lock cleanup to previous patch. - Don't need to read IRQ status register inside spinlock. - Don't check for state CMD_NONE; later patch is removing state var. - Don't hold the lock for all of setup_fifo_xfer(). - Comment about why it's safe to Ack interrupts at the end. - Subject/desc changed since race is definitely there. - ("spi: spi-geni-qcom: Check for error IRQs") new in v3. - ("spi: spi-geni-qcom: Actually use our FIFO") new in v3. - ("spi: spi-geni-qcom: Don't keep a local state variable") new in v3. Changes in v2: - Detect true spurious interrupt. - Still return IRQ_NONE for state machine mismatch, but print warn. Douglas Anderson (5): spi: spi-geni-qcom: No need for irqsave variant of spinlock calls spi: spi-geni-qcom: Mo' betta locking spi: spi-geni-qcom: Check for error IRQs spi: spi-geni-qcom: Actually use our FIFO spi: spi-geni-qcom: Don't keep a local state variable drivers/spi/spi-geni-qcom.c | 120 ++++++++++++++++++++++++------------ 1 file changed, 81 insertions(+), 39 deletions(-) -- 2.27.0.290.gba653c62da-goog
-
Andy Shevchenko authored
No need to redefine already existing definition. So, replace custom by generic one. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Link: https://lore.kernel.org/r/20200618170144.57433-1-andriy.shevchenko@linux.intel.comSigned-off-by: Mark Brown <broonie@kernel.org>
-
Douglas Anderson authored
The variable "cur_mcmd" kept track of our current state (idle, xfer, cs, cancel). We don't really need it, so get rid of it. Instead: * Use separate condition variables for "chip select done", "cancel done", and "abort done". This is important so that if a "done" comes through (perhaps some previous interrupt finally came through) it can't confuse the cancel/abort function. * Use the "done" interrupt only for when a chip select or transfer is done and we can tell the difference by looking at whether "cur_xfer" is NULL. This is mostly a no-op change. However, it is possible it could fix an issue where a super delayed interrupt for a cancel command could have confused our waiting for an abort command. Signed-off-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Stephen Boyd <swboyd@chromium.org> Link: https://lore.kernel.org/r/20200618080459.v4.5.Ib1e6855405fc9c99916ab7c7dee84d73a8bf3d68@changeidSigned-off-by: Mark Brown <broonie@kernel.org>
-
Douglas Anderson authored
The geni hardware has a FIFO that can hold up to 64 bytes (it has 16 entries that can hold 4 bytes each), at least on the two SoCs I tested (sdm845 and sc7180). We configured our RX Watermark to 0, which basically meant we got an interrupt as soon as the first 4 bytes showed up in the FIFO. Tracing the IRQ handler showed that we often only read 4 or 8 bytes per IRQ handler. I tried setting the RX Watermark to "fifo size - 2" but that just got me a bunch of overrun errors reported. Setting it to "fifo size - 3" seemed to work great, though. This made me worried that we'd start getting overruns if we had long interrupt latency, but that doesn't appear to be the case and delays inserted in the IRQ handler while using "fifo size - 3" didn't cause any errors. Presumably there is some interaction with the poorly-documented RFR (ready for receive) level means that "fifo size - 3" is the max. We are the SPI master, so it makes sense that there would be no problems with overruns, the master should just stop clocking. Despite "fifo size - 3" working, I chose "fifo size / 2" (8 entries = 32 bytes) which gives us a little extra time to get to the interrupt handler and should reduce dead time on the SPI wires. With this setting, I often saw the IRQ handler handle 40 bytes but sometimes up to 56 if we had bad interrupt latency. Testing by running "flashrom -p ec -r" on a Chromebook saw interrupts from the SPI driver cut roughly in half. Time was roughly the same. Fixes: 561de45f ("spi: spi-geni-qcom: Add SPI driver support for GENI based QUP") Signed-off-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Stephen Boyd <swboyd@chromium.org> Link: https://lore.kernel.org/r/20200618080459.v4.4.I988281f7c6ee0ed00325559bfce7539f403da69e@changeidSigned-off-by: Mark Brown <broonie@kernel.org>
-
Douglas Anderson authored
>From reading the #defines it seems like we should shout if we ever see one of these error bits. Let's do so. This doesn't do anything functional except print a yell in the log if the error bits are seen. Signed-off-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Stephen Boyd <swboyd@chromium.org> Link: https://lore.kernel.org/r/20200618080459.v4.3.Id8bebdbdb4d2ed9468634343a7e6207d6cffff8a@changeidSigned-off-by: Mark Brown <broonie@kernel.org>
-
Douglas Anderson authored
If you added a bit of a delay (like a trace_printk) into the ISR for the spi-geni-qcom driver, you would suddenly start seeing some errors spit out. The problem was that, though the ISR itself held a lock, other parts of the driver didn't always grab the lock. One example race was this: CPU0 CPU1 ---- ---- spi_geni_set_cs() mas->cur_mcmd = CMD_CS; geni_se_setup_m_cmd(...) wait_for_completion_timeout(&xfer_done); <INTERRUPT> geni_spi_isr() complete(&xfer_done); <wakeup> pm_runtime_put(mas->dev); ... // back to SPI core spi_geni_transfer_one() setup_fifo_xfer() mas->cur_mcmd = CMD_XFER; mas->cur_cmd = CMD_NONE; // bad! return IRQ_HANDLED; Let's fix this. Before we start messing with hardware, we'll grab the lock to make sure that the IRQ handler from some previous command has really finished. We don't need to hold the lock unless we're in a state where more interrupts can come in, but we at least need to make sure the previous IRQ is done. This lock is used exclusively to prevent the IRQ handler and non-IRQ from stomping on each other. The SPI core handles all other mutual exclusion. As part of this, we change the way that the IRQ handler detects spurious interrupts. Previously we checked for our state variable being set to IRQ_NONE, but that was done outside the spinlock. We could move it into the spinlock, but instead let's just change it to look for the lack of any IRQ status bits being set. This can be done outside the lock--the hardware certainly isn't grabbing or looking at the spinlock when it updates its status register. It's possible that this will fix real (but very rare) errors seen in the field that look like: irq ...: nobody cared (try booting with the "irqpoll" option) NOTE: an alternate strategy considered here was to always make the complete() / spi_finalize_current_transfer() the very last thing in our IRQ handler. With such a change you could consider that we could be "lockless". In that case, though, we'd have to be very careful w/ memory barriers so we made sure we didn't have any bugs with weakly ordered memory. Using spinlocks makes the driver much easier to understand. Fixes: 561de45f ("spi: spi-geni-qcom: Add SPI driver support for GENI based QUP") Signed-off-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Stephen Boyd <swboyd@chromium.org> Link: https://lore.kernel.org/r/20200618080459.v4.2.I752ebdcfd5e8bf0de06d66e767b8974932b3620e@changeidSigned-off-by: Mark Brown <broonie@kernel.org>
-
Matthew Gerlach authored
The spi-altera driver was originally written with a 32 bit processor, where sizeof(unsigned long) is 4. On a 64 bit processor sizeof(unsigned long) is 8. Change the structure member to u32 to match the actual size of the control register. Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com> Signed-off-by: Xu Yilun <yilun.xu@intel.com> Reviewed-by: Tom Rix <trix@redhat.com> Link: https://lore.kernel.org/r/1592531021-11412-4-git-send-email-yilun.xu@intel.comSigned-off-by: Mark Brown <broonie@kernel.org>
-