From 5e31ba0c0543a04483b53151eb5b7413efece94c Mon Sep 17 00:00:00 2001 From: Martin Hundebøll Date: Wed, 14 Oct 2020 11:02:30 +0200 Subject: spi: bcm2835: fix gpio cs level inversion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The work on improving gpio chip-select in spi core, and the following fixes, has caused the bcm2835 spi driver to use wrong levels. Fix this by simply removing level handling in the bcm2835 driver, and let the core do its work. Fixes: 3e5ec1db8bfe ("spi: Fix SPI_CS_HIGH setting when using native and GPIO CS") Cc: Signed-off-by: Martin Hundebøll Link: https://lore.kernel.org/r/20201014090230.2706810-1-martin@geanix.com Signed-off-by: Mark Brown --- drivers/spi/spi-bcm2835.c | 12 ------------ 1 file changed, 12 deletions(-) (limited to 'drivers/spi') diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c index b87116e9b413..9b6ba94fe878 100644 --- a/drivers/spi/spi-bcm2835.c +++ b/drivers/spi/spi-bcm2835.c @@ -1259,18 +1259,6 @@ static int bcm2835_spi_setup(struct spi_device *spi) if (!chip) return 0; - /* - * Retrieve the corresponding GPIO line used for CS. - * The inversion semantics will be handled by the GPIO core - * code, so we pass GPIOD_OUT_LOW for "unasserted" and - * the correct flag for inversion semantics. The SPI_CS_HIGH - * on spi->mode cannot be checked for polarity in this case - * as the flag use_gpio_descriptors enforces SPI_CS_HIGH. - */ - if (of_property_read_bool(spi->dev.of_node, "spi-cs-high")) - lflags = GPIO_ACTIVE_HIGH; - else - lflags = GPIO_ACTIVE_LOW; spi->cs_gpiod = gpiochip_request_own_desc(chip, 8 - spi->chip_select, DRV_NAME, lflags, -- cgit v1.2.3-59-g8ed1b From 9bd77a9ce31dd242fece27219d14fbee5068dd85 Mon Sep 17 00:00:00 2001 From: Zhao Qiang Date: Tue, 3 Nov 2020 10:05:46 +0800 Subject: spi: fsl-dspi: fix wrong pointer in suspend/resume Since commit 530b5affc675 ("spi: fsl-dspi: fix use-after-free in remove path"), this driver causes a "NULL pointer dereference" in dspi_suspend/resume. This is because since this commit, the drivers private data point to "dspi" instead of "ctlr", the codes in suspend and resume func were not modified correspondly. Fixes: 530b5affc675 ("spi: fsl-dspi: fix use-after-free in remove path") Signed-off-by: Zhao Qiang Reviewed-by: Vladimir Oltean Link: https://lore.kernel.org/r/20201103020546.1822-1-qiang.zhao@nxp.com Signed-off-by: Mark Brown --- drivers/spi/spi-fsl-dspi.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'drivers/spi') diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c index 3967afa465f0..1a08c1d584ab 100644 --- a/drivers/spi/spi-fsl-dspi.c +++ b/drivers/spi/spi-fsl-dspi.c @@ -1080,12 +1080,11 @@ MODULE_DEVICE_TABLE(of, fsl_dspi_dt_ids); #ifdef CONFIG_PM_SLEEP static int dspi_suspend(struct device *dev) { - struct spi_controller *ctlr = dev_get_drvdata(dev); - struct fsl_dspi *dspi = spi_controller_get_devdata(ctlr); + struct fsl_dspi *dspi = dev_get_drvdata(dev); if (dspi->irq) disable_irq(dspi->irq); - spi_controller_suspend(ctlr); + spi_controller_suspend(dspi->ctlr); clk_disable_unprepare(dspi->clk); pinctrl_pm_select_sleep_state(dev); @@ -1095,8 +1094,7 @@ static int dspi_suspend(struct device *dev) static int dspi_resume(struct device *dev) { - struct spi_controller *ctlr = dev_get_drvdata(dev); - struct fsl_dspi *dspi = spi_controller_get_devdata(ctlr); + struct fsl_dspi *dspi = dev_get_drvdata(dev); int ret; pinctrl_pm_select_default_state(dev); @@ -1104,7 +1102,7 @@ static int dspi_resume(struct device *dev) ret = clk_prepare_enable(dspi->clk); if (ret) return ret; - spi_controller_resume(ctlr); + spi_controller_resume(dspi->ctlr); if (dspi->irq) enable_irq(dspi->irq); -- cgit v1.2.3-59-g8ed1b From bc7f2cd7559c5595dc38b909ae9a8d43e0215994 Mon Sep 17 00:00:00 2001 From: Martin Hundebøll Date: Thu, 5 Nov 2020 10:06:15 +0100 Subject: spi: bcm2835: remove use of uninitialized gpio flags variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removing the duplicate gpio chip select level handling in bcm2835_spi_setup() left the lflags variable uninitialized. Avoid trhe use of such variable by passing default flags to gpiochip_request_own_desc(). Fixes: 5e31ba0c0543 ("spi: bcm2835: fix gpio cs level inversion") Signed-off-by: Martin Hundebøll Link: https://lore.kernel.org/r/20201105090615.620315-1-martin@geanix.com Signed-off-by: Mark Brown --- drivers/spi/spi-bcm2835.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/spi') diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c index 9b6ba94fe878..7104cf17b848 100644 --- a/drivers/spi/spi-bcm2835.c +++ b/drivers/spi/spi-bcm2835.c @@ -1193,7 +1193,6 @@ static int bcm2835_spi_setup(struct spi_device *spi) struct spi_controller *ctlr = spi->controller; struct bcm2835_spi *bs = spi_controller_get_devdata(ctlr); struct gpio_chip *chip; - enum gpio_lookup_flags lflags; u32 cs; /* @@ -1261,7 +1260,7 @@ static int bcm2835_spi_setup(struct spi_device *spi) spi->cs_gpiod = gpiochip_request_own_desc(chip, 8 - spi->chip_select, DRV_NAME, - lflags, + GPIO_LOOKUP_FLAGS_DEFAULT, GPIOD_OUT_LOW); if (IS_ERR(spi->cs_gpiod)) return PTR_ERR(spi->cs_gpiod); -- cgit v1.2.3-59-g8ed1b From 766c6b63aa044e84b045803b40b14754d69a2a1d Mon Sep 17 00:00:00 2001 From: Sven Van Asbroeck Date: Fri, 6 Nov 2020 10:07:06 -0500 Subject: spi: fix client driver breakages when using GPIO descriptors Commit f3186dd87669 ("spi: Optionally use GPIO descriptors for CS GPIOs") introduced the optional use of GPIO descriptors for chip selects. A side-effect of this change: when a SPI bus uses GPIO descriptors, all its client devices have SPI_CS_HIGH set in spi->mode. This flag is required for the SPI bus to operate correctly. This unfortunately breaks many client drivers, which use the following pattern to configure their underlying SPI bus: static int client_device_probe(struct spi_device *spi) { ... spi->mode = SPI_MODE_0; spi->bits_per_word = 8; err = spi_setup(spi); .. } In short, many client drivers overwrite the SPI_CS_HIGH bit in spi->mode, and break the underlying SPI bus driver. This is especially true for Freescale/NXP imx ecspi, where large numbers of spi client drivers now no longer work. Proposed fix: ------------- When using gpio descriptors, depend on gpiolib to handle CS polarity. Existing quirks in gpiolib ensure that this is handled correctly. Existing gpiolib behaviour will force the polarity of any chip-select gpiod to active-high (if 'spi-active-high' devicetree prop present) or active-low (if 'spi-active-high' absent). Irrespective of whether the gpio is marked GPIO_ACTIVE_[HIGH|LOW] in the devicetree. Loose ends: ----------- If this fix is applied: - is commit 138c9c32f090 ("spi: spidev: Fix CS polarity if GPIO descriptors are used") still necessary / correct ? Fixes: f3186dd87669 ("spi: Optionally use GPIO descriptors for CS GPIOs") Signed-off-by: Sven Van Asbroeck Acked-by: Linus Walleij Link: https://lore.kernel.org/r/20201106150706.29089-1-TheSven73@gmail.com Signed-off-by: Mark Brown --- drivers/spi/spi.c | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) (limited to 'drivers/spi') diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 0cab239d8e7f..7566482c052c 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -812,18 +812,16 @@ static void spi_set_cs(struct spi_device *spi, bool enable) enable = !enable; if (spi->cs_gpiod || gpio_is_valid(spi->cs_gpio)) { - /* - * Honour the SPI_NO_CS flag and invert the enable line, as - * active low is default for SPI. Execution paths that handle - * polarity inversion in gpiolib (such as device tree) will - * enforce active high using the SPI_CS_HIGH resulting in a - * double inversion through the code above. - */ if (!(spi->mode & SPI_NO_CS)) { if (spi->cs_gpiod) + /* polarity handled by gpiolib */ gpiod_set_value_cansleep(spi->cs_gpiod, - !enable); + enable1); else + /* + * invert the enable line, as active low is + * default for SPI. + */ gpio_set_value_cansleep(spi->cs_gpio, !enable); } /* Some SPI masters need both GPIO CS & slave_select */ @@ -1992,15 +1990,6 @@ static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi, } spi->chip_select = value; - /* - * For descriptors associated with the device, polarity inversion is - * handled in the gpiolib, so all gpio chip selects are "active high" - * in the logical sense, the gpiolib will invert the line if need be. - */ - if ((ctlr->use_gpio_descriptors) && ctlr->cs_gpiods && - ctlr->cs_gpiods[spi->chip_select]) - spi->mode |= SPI_CS_HIGH; - /* Device speed */ if (!of_property_read_u32(nc, "spi-max-frequency", &value)) spi->max_speed_hz = value; -- cgit v1.2.3-59-g8ed1b From ee4ad5d06509b3aea79b6a77bebd09ef891bed8d Mon Sep 17 00:00:00 2001 From: Eddie James Date: Tue, 10 Nov 2020 15:47:36 -0600 Subject: spi: fsi: Fix transfer returning without finalizing message In the case that the SPI mux isn't set, the transfer_one_message function returns without finalizing the message. This means that the transfer never completes, resulting in hung tasks and an eventual kernel panic. Fix it by finalizing the transfer in this case. Fixes: 9211a441e606 ("spi: fsi: Check mux status before transfers") Signed-off-by: Eddie James Reviewed-by: Joel Stanley Link: https://lore.kernel.org/r/20201110214736.25718-1-eajames@linux.ibm.com Signed-off-by: Mark Brown --- drivers/spi/spi-fsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/spi') diff --git a/drivers/spi/spi-fsi.c b/drivers/spi/spi-fsi.c index 8a440c7078ef..3920cd3286d8 100644 --- a/drivers/spi/spi-fsi.c +++ b/drivers/spi/spi-fsi.c @@ -477,7 +477,7 @@ static int fsi_spi_transfer_one_message(struct spi_controller *ctlr, rc = fsi_spi_check_mux(ctx->fsi, ctx->dev); if (rc) - return rc; + goto error; list_for_each_entry(transfer, &mesg->transfers, transfer_list) { struct fsi_spi_sequence seq; -- cgit v1.2.3-59-g8ed1b From 5e844cc37a5cbaa460e68f9a989d321d63088a89 Mon Sep 17 00:00:00 2001 From: Lukas Wunner Date: Wed, 11 Nov 2020 20:07:10 +0100 Subject: spi: Introduce device-managed SPI controller allocation SPI driver probing currently comprises two steps, whereas removal comprises only one step: spi_alloc_master() spi_register_controller() spi_unregister_controller() That's because spi_unregister_controller() calls device_unregister() instead of device_del(), thereby releasing the reference on the spi_controller which was obtained by spi_alloc_master(). An SPI driver's private data is contained in the same memory allocation as the spi_controller struct. Thus, once spi_unregister_controller() has been called, the private data is inaccessible. But some drivers need to access it after spi_unregister_controller() to perform further teardown steps. Introduce devm_spi_alloc_master() and devm_spi_alloc_slave(), which release a reference on the spi_controller struct only after the driver has unbound, thereby keeping the memory allocation accessible. Change spi_unregister_controller() to not release a reference if the spi_controller was allocated by one of these new devm functions. The present commit is small enough to be backportable to stable. It allows fixing drivers which use the private data in their ->remove() hook after it's been freed. It also allows fixing drivers which neglect to release a reference on the spi_controller in the probe error path. Long-term, most SPI drivers shall be moved over to the devm functions introduced herein. The few that can't shall be changed in a treewide commit to explicitly release the last reference on the controller. That commit shall amend spi_unregister_controller() to no longer release a reference, thereby completing the migration. As a result, the behaviour will be less surprising and more consistent with subsystems such as IIO, which also includes the private data in the allocation of the generic iio_dev struct, but calls device_del() in iio_device_unregister(). Signed-off-by: Lukas Wunner Link: https://lore.kernel.org/r/272bae2ef08abd21388c98e23729886663d19192.1605121038.git.lukas@wunner.de Signed-off-by: Mark Brown --- drivers/spi/spi.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++- include/linux/spi/spi.h | 19 ++++++++++++++++ 2 files changed, 76 insertions(+), 1 deletion(-) (limited to 'drivers/spi') diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 7566482c052c..05c75f890ace 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -2442,6 +2442,49 @@ struct spi_controller *__spi_alloc_controller(struct device *dev, } EXPORT_SYMBOL_GPL(__spi_alloc_controller); +static void devm_spi_release_controller(struct device *dev, void *ctlr) +{ + spi_controller_put(*(struct spi_controller **)ctlr); +} + +/** + * __devm_spi_alloc_controller - resource-managed __spi_alloc_controller() + * @dev: physical device of SPI controller + * @size: how much zeroed driver-private data to allocate + * @slave: whether to allocate an SPI master (false) or SPI slave (true) + * Context: can sleep + * + * Allocate an SPI controller and automatically release a reference on it + * when @dev is unbound from its driver. Drivers are thus relieved from + * having to call spi_controller_put(). + * + * The arguments to this function are identical to __spi_alloc_controller(). + * + * Return: the SPI controller structure on success, else NULL. + */ +struct spi_controller *__devm_spi_alloc_controller(struct device *dev, + unsigned int size, + bool slave) +{ + struct spi_controller **ptr, *ctlr; + + ptr = devres_alloc(devm_spi_release_controller, sizeof(*ptr), + GFP_KERNEL); + if (!ptr) + return NULL; + + ctlr = __spi_alloc_controller(dev, size, slave); + if (ctlr) { + *ptr = ctlr; + devres_add(dev, ptr); + } else { + devres_free(ptr); + } + + return ctlr; +} +EXPORT_SYMBOL_GPL(__devm_spi_alloc_controller); + #ifdef CONFIG_OF static int of_spi_get_gpio_numbers(struct spi_controller *ctlr) { @@ -2778,6 +2821,11 @@ int devm_spi_register_controller(struct device *dev, } EXPORT_SYMBOL_GPL(devm_spi_register_controller); +static int devm_spi_match_controller(struct device *dev, void *res, void *ctlr) +{ + return *(struct spi_controller **)res == ctlr; +} + static int __unregister(struct device *dev, void *null) { spi_unregister_device(to_spi_device(dev)); @@ -2819,7 +2867,15 @@ void spi_unregister_controller(struct spi_controller *ctlr) list_del(&ctlr->list); mutex_unlock(&board_lock); - device_unregister(&ctlr->dev); + device_del(&ctlr->dev); + + /* Release the last reference on the controller if its driver + * has not yet been converted to devm_spi_alloc_master/slave(). + */ + if (!devres_find(ctlr->dev.parent, devm_spi_release_controller, + devm_spi_match_controller, ctlr)) + put_device(&ctlr->dev); + /* free bus id */ mutex_lock(&board_lock); if (found == ctlr) diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index 99380c0825db..b390fdac1587 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -734,6 +734,25 @@ static inline struct spi_controller *spi_alloc_slave(struct device *host, return __spi_alloc_controller(host, size, true); } +struct spi_controller *__devm_spi_alloc_controller(struct device *dev, + unsigned int size, + bool slave); + +static inline struct spi_controller *devm_spi_alloc_master(struct device *dev, + unsigned int size) +{ + return __devm_spi_alloc_controller(dev, size, false); +} + +static inline struct spi_controller *devm_spi_alloc_slave(struct device *dev, + unsigned int size) +{ + if (!IS_ENABLED(CONFIG_SPI_SLAVE)) + return NULL; + + return __devm_spi_alloc_controller(dev, size, true); +} + extern int spi_register_controller(struct spi_controller *ctlr); extern int devm_spi_register_controller(struct device *dev, struct spi_controller *ctlr); -- cgit v1.2.3-59-g8ed1b From e1483ac030fb4c57734289742f1c1d38dca61e22 Mon Sep 17 00:00:00 2001 From: Lukas Wunner Date: Wed, 11 Nov 2020 20:07:20 +0100 Subject: spi: bcm2835: Fix use-after-free on unbind bcm2835_spi_remove() accesses the driver's private data after calling spi_unregister_controller() even though that function releases the last reference on the spi_controller and thereby frees the private data. Fix by switching over to the new devm_spi_alloc_master() helper which keeps the private data accessible until the driver has unbound. Fixes: f8043872e796 ("spi: add driver for BCM2835") Reported-by: Sascha Hauer Reported-by: Florian Fainelli Signed-off-by: Lukas Wunner Cc: # v3.10+: 123456789abc: spi: Introduce device-managed SPI controller allocation Cc: # v3.10+ Cc: Vladimir Oltean Tested-by: Florian Fainelli Acked-by: Florian Fainelli Link: https://lore.kernel.org/r/ad66e0a0ad96feb848814842ecf5b6a4539ef35c.1605121038.git.lukas@wunner.de Signed-off-by: Mark Brown --- drivers/spi/spi-bcm2835.c | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) (limited to 'drivers/spi') diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c index 7104cf17b848..197485f2c2b2 100644 --- a/drivers/spi/spi-bcm2835.c +++ b/drivers/spi/spi-bcm2835.c @@ -1278,7 +1278,7 @@ static int bcm2835_spi_probe(struct platform_device *pdev) struct bcm2835_spi *bs; int err; - ctlr = spi_alloc_master(&pdev->dev, ALIGN(sizeof(*bs), + ctlr = devm_spi_alloc_master(&pdev->dev, ALIGN(sizeof(*bs), dma_get_cache_alignment())); if (!ctlr) return -ENOMEM; @@ -1299,23 +1299,17 @@ static int bcm2835_spi_probe(struct platform_device *pdev) bs->ctlr = ctlr; bs->regs = devm_platform_ioremap_resource(pdev, 0); - if (IS_ERR(bs->regs)) { - err = PTR_ERR(bs->regs); - goto out_controller_put; - } + if (IS_ERR(bs->regs)) + return PTR_ERR(bs->regs); bs->clk = devm_clk_get(&pdev->dev, NULL); - if (IS_ERR(bs->clk)) { - err = dev_err_probe(&pdev->dev, PTR_ERR(bs->clk), - "could not get clk\n"); - goto out_controller_put; - } + if (IS_ERR(bs->clk)) + return dev_err_probe(&pdev->dev, PTR_ERR(bs->clk), + "could not get clk\n"); bs->irq = platform_get_irq(pdev, 0); - if (bs->irq <= 0) { - err = bs->irq ? bs->irq : -ENODEV; - goto out_controller_put; - } + if (bs->irq <= 0) + return bs->irq ? bs->irq : -ENODEV; clk_prepare_enable(bs->clk); @@ -1349,8 +1343,6 @@ out_dma_release: bcm2835_dma_release(ctlr, bs); out_clk_disable: clk_disable_unprepare(bs->clk); -out_controller_put: - spi_controller_put(ctlr); return err; } -- cgit v1.2.3-59-g8ed1b From e13ee6cc4781edaf8c7321bee19217e3702ed481 Mon Sep 17 00:00:00 2001 From: Lukas Wunner Date: Wed, 11 Nov 2020 20:07:30 +0100 Subject: spi: bcm2835aux: Fix use-after-free on unbind bcm2835aux_spi_remove() accesses the driver's private data after calling spi_unregister_master() even though that function releases the last reference on the spi_master and thereby frees the private data. Fix by switching over to the new devm_spi_alloc_master() helper which keeps the private data accessible until the driver has unbound. Fixes: b9dd3f6d4172 ("spi: bcm2835aux: Fix controller unregister order") Signed-off-by: Lukas Wunner Cc: # v4.4+: 123456789abc: spi: Introduce device-managed SPI controller allocation Cc: # v4.4+: b9dd3f6d4172: spi: bcm2835aux: Fix controller unregister order Cc: # v4.4+ Link: https://lore.kernel.org/r/b290b06357d0c0bdee9cecc539b840a90630f101.1605121038.git.lukas@wunner.de Signed-off-by: Mark Brown --- drivers/spi/spi-bcm2835aux.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) (limited to 'drivers/spi') diff --git a/drivers/spi/spi-bcm2835aux.c b/drivers/spi/spi-bcm2835aux.c index 03b034c15d2b..fd58547110e6 100644 --- a/drivers/spi/spi-bcm2835aux.c +++ b/drivers/spi/spi-bcm2835aux.c @@ -494,7 +494,7 @@ static int bcm2835aux_spi_probe(struct platform_device *pdev) unsigned long clk_hz; int err; - master = spi_alloc_master(&pdev->dev, sizeof(*bs)); + master = devm_spi_alloc_master(&pdev->dev, sizeof(*bs)); if (!master) return -ENOMEM; @@ -524,29 +524,24 @@ static int bcm2835aux_spi_probe(struct platform_device *pdev) /* the main area */ bs->regs = devm_platform_ioremap_resource(pdev, 0); - if (IS_ERR(bs->regs)) { - err = PTR_ERR(bs->regs); - goto out_master_put; - } + if (IS_ERR(bs->regs)) + return PTR_ERR(bs->regs); bs->clk = devm_clk_get(&pdev->dev, NULL); if (IS_ERR(bs->clk)) { - err = PTR_ERR(bs->clk); dev_err(&pdev->dev, "could not get clk: %d\n", err); - goto out_master_put; + return PTR_ERR(bs->clk); } bs->irq = platform_get_irq(pdev, 0); - if (bs->irq <= 0) { - err = bs->irq ? bs->irq : -ENODEV; - goto out_master_put; - } + if (bs->irq <= 0) + return bs->irq ? bs->irq : -ENODEV; /* this also enables the HW block */ err = clk_prepare_enable(bs->clk); if (err) { dev_err(&pdev->dev, "could not prepare clock: %d\n", err); - goto out_master_put; + return err; } /* just checking if the clock returns a sane value */ @@ -581,8 +576,6 @@ static int bcm2835aux_spi_probe(struct platform_device *pdev) out_clk_disable: clk_disable_unprepare(bs->clk); -out_master_put: - spi_master_put(master); return err; } -- cgit v1.2.3-59-g8ed1b From 63c5395bb7a9777a33f0e7b5906f2c0170a23692 Mon Sep 17 00:00:00 2001 From: Lukas Wunner Date: Wed, 11 Nov 2020 20:07:40 +0100 Subject: spi: bcm-qspi: Fix use-after-free on unbind bcm_qspi_remove() calls spi_unregister_master() even though bcm_qspi_probe() calls devm_spi_register_master(). The spi_master is therefore unregistered and freed twice on unbind. Moreover, since commit 0392727c261b ("spi: bcm-qspi: Handle clock probe deferral"), bcm_qspi_probe() leaks the spi_master allocation if the call to devm_clk_get_optional() fails. Fix by switching over to the new devm_spi_alloc_master() helper which keeps the private data accessible until the driver has unbound and also avoids the spi_master leak on probe. While at it, fix an ordering issue in bcm_qspi_remove() wherein spi_unregister_master() is called after uninitializing the hardware, disabling the clock and freeing an IRQ data structure. The correct order is to call spi_unregister_master() *before* those teardown steps because bus accesses may still be ongoing until that function returns. Fixes: fa236a7ef240 ("spi: bcm-qspi: Add Broadcom MSPI driver") Signed-off-by: Lukas Wunner Cc: # v4.9+: 123456789abc: spi: Introduce device-managed SPI controller allocation Cc: # v4.9+ Cc: Kamal Dasu Acked-by: Florian Fainelli Tested-by: Florian Fainelli Link: https://lore.kernel.org/r/5e31a9a59fd1c0d0b795b2fe219f25e5ee855f9d.1605121038.git.lukas@wunner.de Signed-off-by: Mark Brown --- drivers/spi/spi-bcm-qspi.c | 34 ++++++++++++---------------------- 1 file changed, 12 insertions(+), 22 deletions(-) (limited to 'drivers/spi') diff --git a/drivers/spi/spi-bcm-qspi.c b/drivers/spi/spi-bcm-qspi.c index 14c9d0133bce..c028446c7460 100644 --- a/drivers/spi/spi-bcm-qspi.c +++ b/drivers/spi/spi-bcm-qspi.c @@ -1327,7 +1327,7 @@ int bcm_qspi_probe(struct platform_device *pdev, data = of_id->data; - master = spi_alloc_master(dev, sizeof(struct bcm_qspi)); + master = devm_spi_alloc_master(dev, sizeof(struct bcm_qspi)); if (!master) { dev_err(dev, "error allocating spi_master\n"); return -ENOMEM; @@ -1367,21 +1367,17 @@ int bcm_qspi_probe(struct platform_device *pdev, if (res) { qspi->base[MSPI] = devm_ioremap_resource(dev, res); - if (IS_ERR(qspi->base[MSPI])) { - ret = PTR_ERR(qspi->base[MSPI]); - goto qspi_resource_err; - } + if (IS_ERR(qspi->base[MSPI])) + return PTR_ERR(qspi->base[MSPI]); } else { - goto qspi_resource_err; + return 0; } res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "bspi"); if (res) { qspi->base[BSPI] = devm_ioremap_resource(dev, res); - if (IS_ERR(qspi->base[BSPI])) { - ret = PTR_ERR(qspi->base[BSPI]); - goto qspi_resource_err; - } + if (IS_ERR(qspi->base[BSPI])) + return PTR_ERR(qspi->base[BSPI]); qspi->bspi_mode = true; } else { qspi->bspi_mode = false; @@ -1392,18 +1388,14 @@ int bcm_qspi_probe(struct platform_device *pdev, res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cs_reg"); if (res) { qspi->base[CHIP_SELECT] = devm_ioremap_resource(dev, res); - if (IS_ERR(qspi->base[CHIP_SELECT])) { - ret = PTR_ERR(qspi->base[CHIP_SELECT]); - goto qspi_resource_err; - } + if (IS_ERR(qspi->base[CHIP_SELECT])) + return PTR_ERR(qspi->base[CHIP_SELECT]); } qspi->dev_ids = kcalloc(num_irqs, sizeof(struct bcm_qspi_dev_id), GFP_KERNEL); - if (!qspi->dev_ids) { - ret = -ENOMEM; - goto qspi_resource_err; - } + if (!qspi->dev_ids) + return -ENOMEM; for (val = 0; val < num_irqs; val++) { irq = -1; @@ -1484,7 +1476,7 @@ int bcm_qspi_probe(struct platform_device *pdev, qspi->xfer_mode.addrlen = -1; qspi->xfer_mode.hp = -1; - ret = devm_spi_register_master(&pdev->dev, master); + ret = spi_register_master(master); if (ret < 0) { dev_err(dev, "can't register master\n"); goto qspi_reg_err; @@ -1497,8 +1489,6 @@ qspi_reg_err: clk_disable_unprepare(qspi->clk); qspi_probe_err: kfree(qspi->dev_ids); -qspi_resource_err: - spi_master_put(master); return ret; } /* probe function to be called by SoC specific platform driver probe */ @@ -1508,10 +1498,10 @@ int bcm_qspi_remove(struct platform_device *pdev) { struct bcm_qspi *qspi = platform_get_drvdata(pdev); + spi_unregister_master(qspi->master); bcm_qspi_hw_uninit(qspi); clk_disable_unprepare(qspi->clk); kfree(qspi->dev_ids); - spi_unregister_master(qspi->master); return 0; } -- cgit v1.2.3-59-g8ed1b From 4def49da620c84a682d9361d6bef0a97eed46fe0 Mon Sep 17 00:00:00 2001 From: Lukas Wunner Date: Sun, 8 Nov 2020 23:41:00 +0100 Subject: spi: lpspi: Fix use-after-free on unbind Normally the last reference on an spi_controller is released by spi_unregister_controller(). In the case of the i.MX lpspi driver, the spi_controller is registered with devm_spi_register_controller(), so spi_unregister_controller() is invoked automatically after the driver has unbound. However the driver already releases the last reference in fsl_lpspi_remove() through a gratuitous call to spi_master_put(), causing a use-after-free when spi_unregister_controller() is subsequently invoked by the devres framework. Fix by dropping the superfluous spi_master_put(). Fixes: 944c01a889d9 ("spi: lpspi: enable runtime pm for lpspi") Signed-off-by: Lukas Wunner Cc: # v5.2+ Cc: Han Xu Link: https://lore.kernel.org/r/ab3c0b18bd820501a12c85e440006e09ec0e275f.1604874488.git.lukas@wunner.de Signed-off-by: Mark Brown --- drivers/spi/spi-fsl-lpspi.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'drivers/spi') diff --git a/drivers/spi/spi-fsl-lpspi.c b/drivers/spi/spi-fsl-lpspi.c index 986b9793fd3c..a2886ee44e4c 100644 --- a/drivers/spi/spi-fsl-lpspi.c +++ b/drivers/spi/spi-fsl-lpspi.c @@ -938,9 +938,6 @@ static int fsl_lpspi_remove(struct platform_device *pdev) spi_controller_get_devdata(controller); pm_runtime_disable(fsl_lpspi->dev); - - spi_master_put(controller); - return 0; } -- cgit v1.2.3-59-g8ed1b From d853b3406903a7dc5b14eb5bada3e8cd677f66a2 Mon Sep 17 00:00:00 2001 From: Nathan Chancellor Date: Fri, 13 Nov 2020 11:07:02 -0700 Subject: spi: bcm2835aux: Restore err assignment in bcm2835aux_spi_probe Clang warns: drivers/spi/spi-bcm2835aux.c:532:50: warning: variable 'err' is uninitialized when used here [-Wuninitialized] dev_err(&pdev->dev, "could not get clk: %d\n", err); ^~~ ./include/linux/dev_printk.h:112:32: note: expanded from macro 'dev_err' _dev_err(dev, dev_fmt(fmt), ##__VA_ARGS__) ^~~~~~~~~~~ drivers/spi/spi-bcm2835aux.c:495:9: note: initialize the variable 'err' to silence this warning int err; ^ = 0 1 warning generated. Restore the assignment so that the error value can be used in the dev_err statement and there is no uninitialized memory being leaked. Fixes: e13ee6cc4781 ("spi: bcm2835aux: Fix use-after-free on unbind") Link: https://github.com/ClangBuiltLinux/linux/issues/1199 Signed-off-by: Nathan Chancellor Link: https://lore.kernel.org/r/20201113180701.455541-1-natechancellor@gmail.com Signed-off-by: Mark Brown --- drivers/spi/spi-bcm2835aux.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/spi') diff --git a/drivers/spi/spi-bcm2835aux.c b/drivers/spi/spi-bcm2835aux.c index fd58547110e6..1a26865c42f8 100644 --- a/drivers/spi/spi-bcm2835aux.c +++ b/drivers/spi/spi-bcm2835aux.c @@ -529,8 +529,9 @@ static int bcm2835aux_spi_probe(struct platform_device *pdev) bs->clk = devm_clk_get(&pdev->dev, NULL); if (IS_ERR(bs->clk)) { + err = PTR_ERR(bs->clk); dev_err(&pdev->dev, "could not get clk: %d\n", err); - return PTR_ERR(bs->clk); + return err; } bs->irq = platform_get_irq(pdev, 0); -- cgit v1.2.3-59-g8ed1b From ac9978fcad3c5abc43cdd225441ce9459c36e16b Mon Sep 17 00:00:00 2001 From: Zhihao Cheng Date: Mon, 16 Nov 2020 22:18:36 +0800 Subject: spi: cadence-quadspi: Fix error return code in cqspi_probe Fix to return the error code from devm_reset_control_get_optional_exclusive() instaed of 0 in cqspi_probe(). Fixes: 31fb632b5d43ca ("spi: Move cadence-quadspi driver to drivers/spi/") Reported-by: Hulk Robot Signed-off-by: Zhihao Cheng Reviewed-by: Philipp Zabel Link: https://lore.kernel.org/r/20201116141836.2970579-1-chengzhihao1@huawei.com Signed-off-by: Mark Brown --- drivers/spi/spi-cadence-quadspi.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/spi') diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c index 40938cf3806d..ba7d40c2922f 100644 --- a/drivers/spi/spi-cadence-quadspi.c +++ b/drivers/spi/spi-cadence-quadspi.c @@ -1260,12 +1260,14 @@ static int cqspi_probe(struct platform_device *pdev) /* Obtain QSPI reset control */ rstc = devm_reset_control_get_optional_exclusive(dev, "qspi"); if (IS_ERR(rstc)) { + ret = PTR_ERR(rstc); dev_err(dev, "Cannot get QSPI reset.\n"); goto probe_reset_failed; } rstc_ocp = devm_reset_control_get_optional_exclusive(dev, "qspi-ocp"); if (IS_ERR(rstc_ocp)) { + ret = PTR_ERR(rstc_ocp); dev_err(dev, "Cannot get QSPI OCP reset.\n"); goto probe_reset_failed; } -- cgit v1.2.3-59-g8ed1b From a41b0ad07bfa081584218431cb0cd7e7ecc71210 Mon Sep 17 00:00:00 2001 From: Serge Semin Date: Tue, 17 Nov 2020 12:40:54 +0300 Subject: spi: dw: Set transfer handler before unmasking the IRQs It turns out the IRQs most like can be unmasked before the controller is enabled with no problematic consequences. The manual doesn't explicitly state that, but the examples perform the controller initialization procedure in that order. So the commit da8f58909e7e ("spi: dw: Unmask IRQs after enabling the chip") hasn't been that required as I thought. But anyway setting the IRQs up after the chip enabling still worth adding since it has simplified the code a bit. The problem is that it has introduced a potential bug. The transfer handler pointer is now initialized after the IRQs are enabled. That may and eventually will cause an invalid or uninitialized callback invocation. Fix that just by performing the callback initialization before the IRQ unmask procedure. Fixes: da8f58909e7e ("spi: dw: Unmask IRQs after enabling the chip") Signed-off-by: Serge Semin Link: https://lore.kernel.org/r/20201117094054.4696-1-Sergey.Semin@baikalelectronics.ru Signed-off-by: Mark Brown --- drivers/spi/spi-dw-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/spi') diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c index 2e50cc0a9291..0b2236ade412 100644 --- a/drivers/spi/spi-dw-core.c +++ b/drivers/spi/spi-dw-core.c @@ -357,11 +357,11 @@ static void dw_spi_irq_setup(struct dw_spi *dws) dw_writel(dws, DW_SPI_TXFTLR, level); dw_writel(dws, DW_SPI_RXFTLR, level - 1); + dws->transfer_handler = dw_spi_transfer_handler; + imask = SPI_INT_TXEI | SPI_INT_TXOI | SPI_INT_RXUI | SPI_INT_RXOI | SPI_INT_RXFI; spi_umask_intr(dws, imask); - - dws->transfer_handler = dw_spi_transfer_handler; } /* -- cgit v1.2.3-59-g8ed1b From 04a9cd51d3f3308a98cbc6adc07acb12fbade011 Mon Sep 17 00:00:00 2001 From: Lukas Wunner Date: Mon, 16 Nov 2020 09:23:10 +0100 Subject: spi: npcm-fiu: Don't leak SPI master in probe error path If the calls to of_match_device(), of_alias_get_id(), devm_ioremap_resource(), devm_regmap_init_mmio() or devm_clk_get() fail on probe of the NPCM FIU SPI driver, the spi_controller struct is erroneously not freed. Fix by switching over to the new devm_spi_alloc_master() helper. Fixes: ace55c411b11 ("spi: npcm-fiu: add NPCM FIU controller driver") Signed-off-by: Lukas Wunner Cc: # v5.4+: 5e844cc37a5c: spi: Introduce device-managed SPI controller allocation Cc: # v5.4+ Cc: Tomer Maimon Link: https://lore.kernel.org/r/a420c23a363a3bc9aa684c6e790c32a8af106d17.1605512876.git.lukas@wunner.de Signed-off-by: Mark Brown --- drivers/spi/spi-npcm-fiu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/spi') diff --git a/drivers/spi/spi-npcm-fiu.c b/drivers/spi/spi-npcm-fiu.c index 341f7cffeaac..1cb9329de945 100644 --- a/drivers/spi/spi-npcm-fiu.c +++ b/drivers/spi/spi-npcm-fiu.c @@ -679,7 +679,7 @@ static int npcm_fiu_probe(struct platform_device *pdev) struct resource *res; int id; - ctrl = spi_alloc_master(dev, sizeof(*fiu)); + ctrl = devm_spi_alloc_master(dev, sizeof(*fiu)); if (!ctrl) return -ENOMEM; -- cgit v1.2.3-59-g8ed1b From 4fae3a58ab59d8a286864d61fe1846283a0316f2 Mon Sep 17 00:00:00 2001 From: Serge Semin Date: Tue, 17 Nov 2020 12:45:17 +0300 Subject: spi: Take the SPI IO-mutex in the spi_setup() method I've discovered that due to the recent commit 49d7d695ca4b ("spi: dw: Explicitly de-assert CS on SPI transfer completion") a concurrent usage of the spidev devices with different chip-selects causes the "SPI transfer timed out" error. The root cause of the problem has turned to be in a race condition of the SPI-transfer execution procedure and the spi_setup() method being called at the same time. In particular in calling the spi_set_cs(false) while there is an SPI-transfer being executed. In my case due to the commit cited above all CSs get to be switched off by calling the spi_setup() for /dev/spidev0.1 while there is an concurrent SPI-transfer execution performed on /dev/spidev0.0. Of course a situation of the spi_setup() being called while there is an SPI-transfer being executed for two different SPI peripheral devices of the same controller may happen not only for the spidev driver, but for instance for MMC SPI + some another device, or spi_setup() being called from an SPI-peripheral probe method while some other device has already been probed and is being used by a corresponding driver... Of course I could have provided a fix affecting the DW APB SSI driver only, for instance, by creating a mutual exclusive access to the set_cs callback and setting/clearing only the bit responsible for the corresponding chip-select. But after a short research I've discovered that the problem most likely affects a lot of the other drivers: - drivers/spi/spi-sun4i.c - RMW the chip-select register; - drivers/spi/spi-rockchip.c - RMW the chip-select register; - drivers/spi/spi-qup.c - RMW a generic force-CS flag in a CSR. - drivers/spi/spi-sifive.c - set a generic CS-mode flag in a CSR. - drivers/spi/spi-bcm63xx-hsspi.c - uses an internal mutex to serialize the bus config changes, but still isn't protected from the race condition described above; - drivers/spi/spi-geni-qcom.c - RMW a chip-select internal flag and set the CS state in HW; - drivers/spi/spi-orion.c - RMW a chip-select register; - drivers/spi/spi-cadence.c - RMW a chip-select register; - drivers/spi/spi-armada-3700.c - RMW a chip-select register; - drivers/spi/spi-lantiq-ssc.c - overwrites the chip-select register; - drivers/spi/spi-sun6i.c - RMW a chip-select register; - drivers/spi/spi-synquacer.c - RMW a chip-select register; - drivers/spi/spi-altera.c - directly sets the chip-select state; - drivers/spi/spi-omap2-mcspi.c - RMW an internally cached CS state and writes it to HW; - drivers/spi/spi-mt65xx.c - RMW some CSR; - drivers/spi/spi-jcore.c - directly sets the chip-selects state; - drivers/spi/spi-mt7621.c - RMW a chip-select register; I could have missed some drivers, but a scale of the problem is obvious. As you can see most of the drivers perform an unprotected Read-modify-write chip-select register modification in the set_cs callback. Seeing the spi_setup() function is calling the spi_set_cs() and it can be executed concurrently with SPI-transfers exec procedure, which also calls spi_set_cs() in the SPI core spi_transfer_one_message() method, the race condition of the register modification turns to be obvious. To sum up the problem denoted above affects each driver for a controller having more than one chip-select lane and which: 1) performs the RMW to some CS-related register with no serialization; 2) directly disables any CS on spi_set_cs(dev, false). * the later is the case of the DW APB SSI driver. The controllers which equipped with a single CS theoretically can also experience the problem, but in practice will not since normally the spi_setup() isn't called concurrently with the SPI-transfers executed on the same SPI peripheral device. In order to generically fix the denoted bug I'd suggest to serialize an access to the controller IO by taking the IO mutex in the spi_setup() callback. The mutex is held while there is an SPI communication going on on the SPI-bus of the corresponding SPI-controller. So calling the spi_setup() method and disabling/updating the CS state within it would be safe while there is no any SPI-transfers being executed. Also note I suppose it would be safer to protect the spi_controller->setup() callback invocation too, seeing some of the SPI-controller drivers update a HW state in there. Fixes: 49d7d695ca4b ("spi: dw: Explicitly de-assert CS on SPI transfer completion") Signed-off-by: Serge Semin Link: https://lore.kernel.org/r/20201117094517.5654-1-Sergey.Semin@baikalelectronics.ru Signed-off-by: Mark Brown --- drivers/spi/spi.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'drivers/spi') diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 05c75f890ace..fc9a59788d2e 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -3372,12 +3372,15 @@ int spi_setup(struct spi_device *spi) if (!spi->max_speed_hz) spi->max_speed_hz = spi->controller->max_speed_hz; + mutex_lock(&spi->controller->io_mutex); + if (spi->controller->setup) status = spi->controller->setup(spi); if (spi->controller->auto_runtime_pm && spi->controller->set_cs) { status = pm_runtime_get_sync(spi->controller->dev.parent); if (status < 0) { + mutex_unlock(&spi->controller->io_mutex); pm_runtime_put_noidle(spi->controller->dev.parent); dev_err(&spi->controller->dev, "Failed to power device: %d\n", status); @@ -3399,6 +3402,8 @@ int spi_setup(struct spi_device *spi) spi_set_cs(spi, false); } + mutex_unlock(&spi->controller->io_mutex); + if (spi->rt && !spi->controller->rt) { spi->controller->rt = true; spi_set_thread_rt(spi->controller); -- cgit v1.2.3-59-g8ed1b From 71d80563b0760a411cd90a3680536f5d887fff6b Mon Sep 17 00:00:00 2001 From: Ran Wang Date: Mon, 23 Nov 2020 10:57:15 +0800 Subject: spi: spi-nxp-fspi: fix fspi panic by unexpected interrupts Given the case that bootloader(such as UEFI)'s FSPI driver might not handle all interrupts before loading kernel, those legacy interrupts would assert immidiately once kernel's FSPI driver enable them. Further, if it was FSPI_INTR_IPCMDDONE, the irq handler nxp_fspi_irq_handler() would call complete(&f->c) to notify others. However, f->c might not be initialized yet at that time, then cause kernel panic. Of cause, we should fix this issue within bootloader. But it would be better to have this pacth to make dirver more robust (by clearing all interrupt status bits before enabling interrupts). Suggested-by: Han Xu Signed-off-by: Ran Wang Link: https://lore.kernel.org/r/20201123025715.14635-1-ran.wang_1@nxp.com Signed-off-by: Mark Brown --- drivers/spi/spi-nxp-fspi.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'drivers/spi') diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c index 0d41406c036d..ab9035662717 100644 --- a/drivers/spi/spi-nxp-fspi.c +++ b/drivers/spi/spi-nxp-fspi.c @@ -1001,6 +1001,7 @@ static int nxp_fspi_probe(struct platform_device *pdev) struct resource *res; struct nxp_fspi *f; int ret; + u32 reg; ctlr = spi_alloc_master(&pdev->dev, sizeof(*f)); if (!ctlr) @@ -1032,6 +1033,12 @@ static int nxp_fspi_probe(struct platform_device *pdev) goto err_put_ctrl; } + /* Clear potential interrupts */ + reg = fspi_readl(f, f->iobase + FSPI_INTR); + if (reg) + fspi_writel(f, reg, f->iobase + FSPI_INTR); + + /* find the resources - controller memory mapped space */ if (is_acpi_node(f->dev->fwnode)) res = platform_get_resource(pdev, IORESOURCE_MEM, 1); -- cgit v1.2.3-59-g8ed1b From 7cd71202961090d8f2d2b863ec66b25ae43e1d39 Mon Sep 17 00:00:00 2001 From: Clark Wang Date: Tue, 24 Nov 2020 16:52:47 +0800 Subject: spi: imx: fix the unbalanced spi runtime pm management If set active without increase the usage count of pm, the dont use autosuspend function will call the suspend callback to close the two clocks of spi because the usage count is reduced to -1. This will cause the warning dump below when the defer-probe occurs. [ 129.379701] ecspi2_root_clk already disabled [ 129.384005] WARNING: CPU: 1 PID: 33 at drivers/clk/clk.c:952 clk_core_disable+0xa4/0xb0 So add the get noresume function before set active. Fixes: 43b6bf406cd0 spi: imx: fix runtime pm support for !CONFIG_PM Signed-off-by: Clark Wang Link: https://lore.kernel.org/r/20201124085247.18025-1-xiaoning.wang@nxp.com Signed-off-by: Mark Brown --- drivers/spi/spi-imx.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/spi') diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c index 4b80e27ecdbf..0b597905ee72 100644 --- a/drivers/spi/spi-imx.c +++ b/drivers/spi/spi-imx.c @@ -1686,6 +1686,7 @@ static int spi_imx_probe(struct platform_device *pdev) pm_runtime_set_autosuspend_delay(spi_imx->dev, MXC_RPM_TIMEOUT); pm_runtime_use_autosuspend(spi_imx->dev); + pm_runtime_get_noresume(spi_imx->dev); pm_runtime_set_active(spi_imx->dev); pm_runtime_enable(spi_imx->dev); -- cgit v1.2.3-59-g8ed1b From 0abdb0fba07322ce960d32a92a64847b3009b2e2 Mon Sep 17 00:00:00 2001 From: Lars Povlsen Date: Fri, 20 Nov 2020 22:34:14 +0100 Subject: spi: dw: Fix spi registration for controllers overriding CS When SPI DW memory ops support was introduced, there was a check for excluding controllers which supplied their own CS function. Even so, the mem_ops pointer is *always* presented to the SPI core. This causes the SPI core sanity check in spi_controller_check_ops() to refuse registration, since a mem_ops pointer is being supplied without an exec_op member function. The end result is failure of the SPI DW driver on sparx5 and similar platforms. The fix in the core SPI DW driver is to avoid presenting the mem_ops pointer if the exec_op function is not set. Fixes: 6423207e57ea (spi: dw: Add memory operations support) Signed-off-by: Lars Povlsen Acked-by: Serge Semin Link: https://lore.kernel.org/r/20201120213414.339701-1-lars.povlsen@microchip.com Signed-off-by: Mark Brown --- drivers/spi/spi-dw-core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/spi') diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c index 0b2236ade412..c33866f747db 100644 --- a/drivers/spi/spi-dw-core.c +++ b/drivers/spi/spi-dw-core.c @@ -875,7 +875,8 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws) master->set_cs = dw_spi_set_cs; master->transfer_one = dw_spi_transfer_one; master->handle_err = dw_spi_handle_err; - master->mem_ops = &dws->mem_ops; + if (dws->mem_ops.exec_op) + master->mem_ops = &dws->mem_ops; master->max_speed_hz = dws->max_freq; master->dev.of_node = dev->of_node; master->dev.fwnode = dev->fwnode; -- cgit v1.2.3-59-g8ed1b From 7218838109fef61cdec988ff728e902d434c9cc5 Mon Sep 17 00:00:00 2001 From: Serge Semin Date: Fri, 27 Nov 2020 17:46:11 +0300 Subject: spi: dw-bt1: Fix undefined devm_mux_control_get symbol I mistakenly added the select attributes to the SPI_DW_BT1_DIRMAP config instead of having them defined in SPI_DW_BT1. If the kernel doesn't have the MULTIPLEXER and MUX_MMIO configs manually enabled and the SPI_DW_BT1_DIRMAP config hasn't been selected, Baikal-T1 SPI device will always fail to be probed by the driver. Fix that and the error reported by the test robot: >> ld.lld: error: undefined symbol: devm_mux_control_get >>> referenced by spi-dw-bt1.c >>> spi/spi-dw-bt1.o:(dw_spi_bt1_sys_init) in archive drivers/built-in.a by moving the MULTIPLEXER/MUX_MMIO configs selection to the SPI_DW_BT1 config. Link: https://lore.kernel.org/lkml/202011161745.uYRlekse-lkp@intel.com/ Link: https://lore.kernel.org/linux-spi/20201116040721.8001-1-rdunlap@infradead.org/ Fixes: abf00907538e ("spi: dw: Add Baikal-T1 SPI Controller glue driver") Reported-by: kernel test robot Signed-off-by: Serge Semin Cc: Randy Dunlap Cc: Ramil Zaripov Link: https://lore.kernel.org/r/20201127144612.4204-1-Sergey.Semin@baikalelectronics.ru Signed-off-by: Mark Brown --- drivers/spi/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/spi') diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index 5cff60de8e83..aadaea052f51 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -255,6 +255,8 @@ config SPI_DW_MMIO config SPI_DW_BT1 tristate "Baikal-T1 SPI driver for DW SPI core" depends on MIPS_BAIKAL_T1 || COMPILE_TEST + select MULTIPLEXER + select MUX_MMIO help Baikal-T1 SoC is equipped with three DW APB SSI-based MMIO SPI controllers. Two of them are pretty much normal: with IRQ, DMA, @@ -268,8 +270,6 @@ config SPI_DW_BT1 config SPI_DW_BT1_DIRMAP bool "Directly mapped Baikal-T1 Boot SPI flash support" depends on SPI_DW_BT1 - select MULTIPLEXER - select MUX_MMIO help Directly mapped SPI flash memory is an interface specific to the Baikal-T1 System Boot Controller. It is a 16MB MMIO region, which -- cgit v1.2.3-59-g8ed1b