[PATCH 0/5] Grab bag with AMD SPI fixes

[PATCH 0/5] Grab bag with AMD SPI fixes

[Lukas Wunner]

Here's an assortment of drive-by fixes for the new AMD SPI driver.
All of them are compile-tested only.

Lukas Wunner (5):
  spi: amd: Fix duplicate iounmap in error path
  spi: amd: Pass probe errors back to driver core
  spi: amd: Drop duplicate driver data assignments
  spi: amd: Fix refcount underflow on remove
  spi: amd: Drop superfluous member from struct amd_spi

 drivers/spi/spi-amd.c | 27 +++++----------------------
 1 file changed, 5 insertions(+), 22 deletions(-)

-- 
2.26.2

[PATCH 1/5] spi: amd: Fix duplicate iounmap in error path

[Lukas Wunner]

The AMD SPI driver uses devm_ioremap_resource() to map its registers, so
they're automatically unmapped via device_release() when the last ref on
the SPI controller is dropped.  The additional iounmap() in the ->probe()
error path is thus unnecessary.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/spi/spi-amd.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
index c7cfc3dc20b1..65b53eee5be9 100644
--- a/drivers/spi/spi-amd.c
+++ b/drivers/spi/spi-amd.c
@@ -285,14 +285,12 @@ static int amd_spi_probe(struct platform_device *pdev)
 	err = spi_register_master(master);
 	if (err) {
 		dev_err(dev, "error %d registering SPI controller\n", err);
-		goto err_iounmap;
+		goto err_free_master;
 	}
 	platform_set_drvdata(pdev, amd_spi);
 
 	return 0;
 
-err_iounmap:
-	iounmap(amd_spi->io_remap_addr);
 err_free_master:
 	spi_master_put(master);
 
-- 
2.26.2

[PATCH 2/5] spi: amd: Pass probe errors back to driver core

[Lukas Wunner]

If probing fails, the AMD SPI driver pretends success to the driver core
by returning 0.  Return the errno instead.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/spi/spi-amd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
index 65b53eee5be9..a4248b97b67e 100644
--- a/drivers/spi/spi-amd.c
+++ b/drivers/spi/spi-amd.c
@@ -294,7 +294,7 @@ static int amd_spi_probe(struct platform_device *pdev)
 err_free_master:
 	spi_master_put(master);
 
-	return 0;
+	return err;
 }
 
 static int amd_spi_remove(struct platform_device *pdev)
-- 
2.26.2

[PATCH 3/5] spi: amd: Drop duplicate driver data assignments

[Lukas Wunner]

The AMD SPI driver calls platform_set_drvdata() on probe even though
it's already been set by __spi_alloc_controller().  Likewise, it calls
platform_set_drvdata() on remove even though it's going to be set by
__device_release_driver().  Drop the duplicate assignments.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/spi/spi-amd.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
index a4248b97b67e..d3e3516ef957 100644
--- a/drivers/spi/spi-amd.c
+++ b/drivers/spi/spi-amd.c
@@ -287,7 +287,6 @@ static int amd_spi_probe(struct platform_device *pdev)
 		dev_err(dev, "error %d registering SPI controller\n", err);
 		goto err_free_master;
 	}
-	platform_set_drvdata(pdev, amd_spi);
 
 	return 0;
 
@@ -303,7 +302,6 @@ static int amd_spi_remove(struct platform_device *pdev)
 
 	spi_unregister_master(amd_spi->master);
 	spi_master_put(amd_spi->master);
-	platform_set_drvdata(pdev, NULL);
 
 	return 0;
 }
-- 
2.26.2

[PATCH 4/5] spi: amd: Fix refcount underflow on remove

[Lukas Wunner]

The AMD SPI driver calls spi_master_put() in its ->remove() hook even
though the preceding call to spi_unregister_master() already drops a
ref, thus leading to a refcount underflow.  Drop the superfluous call
to spi_master_put().

This only leaves the call to spi_unregister_master() in the ->remove()
hook, so it's safe to change the ->probe() hook to use the devm version
of spi_register_master() and drop the ->remove() hook altogether.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/spi/spi-amd.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
index d3e3516ef957..00f2f3405e08 100644
--- a/drivers/spi/spi-amd.c
+++ b/drivers/spi/spi-amd.c
@@ -282,7 +282,7 @@ static int amd_spi_probe(struct platform_device *pdev)
 	master->transfer_one_message = amd_spi_master_transfer;
 
 	/* Register the controller with SPI framework */
-	err = spi_register_master(master);
+	err = devm_spi_register_master(dev, master);
 	if (err) {
 		dev_err(dev, "error %d registering SPI controller\n", err);
 		goto err_free_master;
@@ -296,16 +296,6 @@ static int amd_spi_probe(struct platform_device *pdev)
 	return err;
 }
 
-static int amd_spi_remove(struct platform_device *pdev)
-{
-	struct amd_spi *amd_spi = platform_get_drvdata(pdev);
-
-	spi_unregister_master(amd_spi->master);
-	spi_master_put(amd_spi->master);
-
-	return 0;
-}
-
 static const struct acpi_device_id spi_acpi_match[] = {
 	{ "AMDI0061", 0 },
 	{},
@@ -318,7 +308,6 @@ static struct platform_driver amd_spi_driver = {
 		.acpi_match_table = ACPI_PTR(spi_acpi_match),
 	},
 	.probe = amd_spi_probe,
-	.remove = amd_spi_remove,
 };
 
 module_platform_driver(amd_spi_driver);
-- 
2.26.2

[PATCH 5/5] spi: amd: Drop superfluous member from struct amd_spi

[Lukas Wunner]

The AMD SPI driver stores a pointer to the spi_master in struct amd_spi
so that it can get from the latter to the former in amd_spi_fifo_xfer().

It's simpler to just pass the pointer from the sole caller
amd_spi_master_transfer() and drop the pointer from struct amd_spi.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/spi/spi-amd.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
index 00f2f3405e08..d0aacd4de1b9 100644
--- a/drivers/spi/spi-amd.c
+++ b/drivers/spi/spi-amd.c
@@ -38,7 +38,6 @@ struct amd_spi {
 	void __iomem *io_remap_addr;
 	unsigned long io_base_addr;
 	u32 rom_addr;
-	struct spi_master *master;
 	u8 chip_select;
 };
 
@@ -164,9 +163,9 @@ static int amd_spi_master_setup(struct spi_device *spi)
 }
 
 static inline int amd_spi_fifo_xfer(struct amd_spi *amd_spi,
+				    struct spi_master *master,
 				    struct spi_message *message)
 {
-	struct spi_master *master = amd_spi->master;
 	struct spi_transfer *xfer = NULL;
 	u8 cmd_opcode;
 	u8 *buf = NULL;
@@ -241,7 +240,7 @@ static int amd_spi_master_transfer(struct spi_master *master,
 	 * Extract spi_transfers from the spi message and
 	 * program the controller.
 	 */
-	amd_spi_fifo_xfer(amd_spi, msg);
+	amd_spi_fifo_xfer(amd_spi, master, msg);
 
 	return 0;
 }
@@ -262,7 +261,6 @@ static int amd_spi_probe(struct platform_device *pdev)
 	}
 
 	amd_spi = spi_master_get_devdata(master);
-	amd_spi->master = master;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	amd_spi->io_remap_addr = devm_ioremap_resource(&pdev->dev, res);
-- 
2.26.2

Re: [PATCH 0/5] Grab bag with AMD SPI fixes

[Mark Brown]

On Mon, 4 May 2020 13:12:00 +0200, Lukas Wunner wrote:
> Here's an assortment of drive-by fixes for the new AMD SPI driver.
> All of them are compile-tested only.
> 
> Lukas Wunner (5):
>   spi: amd: Fix duplicate iounmap in error path
>   spi: amd: Pass probe errors back to driver core
>   spi: amd: Drop duplicate driver data assignments
>   spi: amd: Fix refcount underflow on remove
>   spi: amd: Drop superfluous member from struct amd_spi
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-5.8

Thanks!

[1/5] spi: amd: Fix duplicate iounmap in error path
      commit: 2b60c49f3ca0648c5b02b60dc0f5b9e2c274bfb5
[2/5] spi: amd: Pass probe errors back to driver core
      commit: cc17fbec2e785926dafce65d014f8301847dff40
[3/5] spi: amd: Drop duplicate driver data assignments
      commit: 4332ea8f40c80d51a534f194291bf3b7738a7beb
[4/5] spi: amd: Fix refcount underflow on remove
      commit: 7b9c94bd13cc9dc9c0c4932ebacf756ae612d52a
[5/5] spi: amd: Drop superfluous member from struct amd_spi
      commit: 36c72a58d472b4032e09f165ea064a0251c9df07

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark