[PATCH 0/5] AMD SPI controller driver bug fix and cleanups

[PATCH 0/5] AMD SPI controller driver bug fix and cleanups

[Cristian Ciocaltea]

This patch series addresses an issue in the spi-amd driver and, while
there, performs some additional cleanups, like simplifying the error
handling in the probe function and removing an unused struct member.

For improving code readability, it also adds some kernel-doc comments.

Cristian Ciocaltea (5):
  spi: amd: Limit max transfer and message size
  spi: amd: Make use of devm_spi_alloc_master()
  spi: amd: Make use of dev_err_probe()
  spi: amd: Drop io_base_addr member from struct amd_spi
  spi: amd: Add struct and enum kernel-doc comments

 drivers/spi/spi-amd.c | 53 ++++++++++++++++++++++++-------------------
 1 file changed, 30 insertions(+), 23 deletions(-)

--
2.36.1

[PATCH 1/5] spi: amd: Limit max transfer and message size

[Cristian Ciocaltea]

Enabling the SPI CS35L41 audio codec driver for Steam Deck [1]
revealed a problem with the current AMD SPI controller driver
implementation, consisting of an unrecoverable system hang.

The issue can be prevented if we ensure the max transfer size
and the max message size do not exceed the FIFO buffer size.

According to the implementation of the downstream driver, the
AMD SPI controller is not able to handle more than 70 bytes per
transfer, which corresponds to the size of the FIFO buffer.

Hence, let's fix this by setting the SPI limits mentioned above.

[1] https://lore.kernel.org/r/20220621213819.262537-1-cristian.ciocaltea@collabora.com

Reported-by: Anastasios Vacharakis <vacharakis@o2mail.de>
Fixes: bbb336f39efc ("spi: spi-amd: Add AMD SPI controller driver support")
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/spi/spi-amd.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
index cba6a4486c24..efdcbe6c4c26 100644
--- a/drivers/spi/spi-amd.c
+++ b/drivers/spi/spi-amd.c
@@ -33,6 +33,7 @@
 #define AMD_SPI_RX_COUNT_REG	0x4B
 #define AMD_SPI_STATUS_REG	0x4C
 
+#define AMD_SPI_FIFO_SIZE	70
 #define AMD_SPI_MEM_SIZE	200
 
 /* M_CMD OP codes for SPI */
@@ -270,6 +271,11 @@ static int amd_spi_master_transfer(struct spi_master *master,
 	return 0;
 }
 
+static size_t amd_spi_max_transfer_size(struct spi_device *spi)
+{
+	return AMD_SPI_FIFO_SIZE;
+}
+
 static int amd_spi_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -302,6 +308,8 @@ static int amd_spi_probe(struct platform_device *pdev)
 	master->flags = SPI_MASTER_HALF_DUPLEX;
 	master->setup = amd_spi_master_setup;
 	master->transfer_one_message = amd_spi_master_transfer;
+	master->max_transfer_size = amd_spi_max_transfer_size;
+	master->max_message_size = amd_spi_max_transfer_size;
 
 	/* Register the controller with SPI framework */
 	err = devm_spi_register_master(dev, master);
-- 
2.36.1

[PATCH 2/5] spi: amd: Make use of devm_spi_alloc_master()

[Cristian Ciocaltea]

Make use of the devm variant of spi_alloc_master() in order to cleanup
and simplify the error handling in the probe function by getting rid
of the goto statements.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/spi/spi-amd.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
index efdcbe6c4c26..3dc17a80c55c 100644
--- a/drivers/spi/spi-amd.c
+++ b/drivers/spi/spi-amd.c
@@ -281,10 +281,10 @@ static int amd_spi_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct spi_master *master;
 	struct amd_spi *amd_spi;
-	int err = 0;
+	int err;
 
 	/* Allocate storage for spi_master and driver private data */
-	master = spi_alloc_master(dev, sizeof(struct amd_spi));
+	master = devm_spi_alloc_master(dev, sizeof(struct amd_spi));
 	if (!master) {
 		dev_err(dev, "Error allocating SPI master\n");
 		return -ENOMEM;
@@ -295,7 +295,7 @@ static int amd_spi_probe(struct platform_device *pdev)
 	if (IS_ERR(amd_spi->io_remap_addr)) {
 		err = PTR_ERR(amd_spi->io_remap_addr);
 		dev_err(dev, "error %d ioremap of SPI registers failed\n", err);
-		goto err_free_master;
+		return err;
 	}
 	dev_dbg(dev, "io_remap_address: %p\n", amd_spi->io_remap_addr);
 
@@ -313,15 +313,8 @@ static int amd_spi_probe(struct platform_device *pdev)
 
 	/* Register the controller with SPI framework */
 	err = devm_spi_register_master(dev, master);
-	if (err) {
+	if (err)
 		dev_err(dev, "error %d registering SPI controller\n", err);
-		goto err_free_master;
-	}
-
-	return 0;
-
-err_free_master:
-	spi_master_put(master);
 
 	return err;
 }
-- 
2.36.1

[PATCH 3/5] spi: amd: Make use of dev_err_probe()

[Cristian Ciocaltea]

Simplify the error handling in probe function by switching from
dev_err() to dev_err_probe().

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/spi/spi-amd.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
index 3dc17a80c55c..1aa19a02a7b6 100644
--- a/drivers/spi/spi-amd.c
+++ b/drivers/spi/spi-amd.c
@@ -285,18 +285,15 @@ static int amd_spi_probe(struct platform_device *pdev)
 
 	/* Allocate storage for spi_master and driver private data */
 	master = devm_spi_alloc_master(dev, sizeof(struct amd_spi));
-	if (!master) {
-		dev_err(dev, "Error allocating SPI master\n");
-		return -ENOMEM;
-	}
+	if (!master)
+		return dev_err_probe(dev, -ENOMEM, "Error allocating SPI master\n");
 
 	amd_spi = spi_master_get_devdata(master);
 	amd_spi->io_remap_addr = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(amd_spi->io_remap_addr)) {
-		err = PTR_ERR(amd_spi->io_remap_addr);
-		dev_err(dev, "error %d ioremap of SPI registers failed\n", err);
-		return err;
-	}
+	if (IS_ERR(amd_spi->io_remap_addr))
+		return dev_err_probe(dev, PTR_ERR(amd_spi->io_remap_addr),
+				     "ioremap of SPI registers failed\n");
+
 	dev_dbg(dev, "io_remap_address: %p\n", amd_spi->io_remap_addr);
 
 	amd_spi->version = (enum amd_spi_versions) device_get_match_data(dev);
@@ -314,9 +311,9 @@ static int amd_spi_probe(struct platform_device *pdev)
 	/* Register the controller with SPI framework */
 	err = devm_spi_register_master(dev, master);
 	if (err)
-		dev_err(dev, "error %d registering SPI controller\n", err);
+		return dev_err_probe(dev, err, "error registering SPI controller\n");
 
-	return err;
+	return 0;
 }
 
 #ifdef CONFIG_ACPI
-- 
2.36.1

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

[Cristian Ciocaltea]

The io_base_addr member of struct amd_spi is not referenced anywhere
in the driver implementation and there is no indication that it could
be used in the future, hence drop it.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/spi/spi-amd.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
index 1aa19a02a7b6..6eddb950e1ad 100644
--- a/drivers/spi/spi-amd.c
+++ b/drivers/spi/spi-amd.c
@@ -47,7 +47,6 @@ enum amd_spi_versions {
 
 struct amd_spi {
 	void __iomem *io_remap_addr;
-	unsigned long io_base_addr;
 	enum amd_spi_versions version;
 };
 
-- 
2.36.1

[PATCH 5/5] spi: amd: Add struct and enum kernel-doc comments

[Cristian Ciocaltea]

Provide documentation comments in the kernel-doc format
for enum amd_spi_versions and struct amd_spi.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 drivers/spi/spi-amd.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
index 6eddb950e1ad..08df4f8d0531 100644
--- a/drivers/spi/spi-amd.c
+++ b/drivers/spi/spi-amd.c
@@ -40,11 +40,21 @@
 #define AMD_SPI_XFER_TX		1
 #define AMD_SPI_XFER_RX		2
 
+/**
+ * enum amd_spi_versions - SPI controller versions
+ * @AMD_SPI_V1:		AMDI0061 hardware version
+ * @AMD_SPI_V2:		AMDI0062 hardware version
+ */
 enum amd_spi_versions {
-	AMD_SPI_V1 = 1,	/* AMDI0061 */
-	AMD_SPI_V2,	/* AMDI0062 */
+	AMD_SPI_V1 = 1,
+	AMD_SPI_V2,
 };
 
+/**
+ * struct amd_spi - SPI driver instance
+ * @io_remap_addr:	Start address of the SPI controller registers
+ * @version:		SPI controller hardware version
+ */
 struct amd_spi {
 	void __iomem *io_remap_addr;
 	enum amd_spi_versions version;
-- 
2.36.1

Re: [PATCH 0/5] AMD SPI controller driver bug fix and cleanups

[Mark Brown]

On Wed, 6 Jul 2022 13:06:21 +0300, Cristian Ciocaltea wrote:
> This patch series addresses an issue in the spi-amd driver and, while
> there, performs some additional cleanups, like simplifying the error
> handling in the probe function and removing an unused struct member.
> 
> For improving code readability, it also adds some kernel-doc comments.
> 
> Cristian Ciocaltea (5):
>   spi: amd: Limit max transfer and message size
>   spi: amd: Make use of devm_spi_alloc_master()
>   spi: amd: Make use of dev_err_probe()
>   spi: amd: Drop io_base_addr member from struct amd_spi
>   spi: amd: Add struct and enum kernel-doc comments
> 
> [...]

Applied to

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

Thanks!

[1/5] spi: amd: Limit max transfer and message size
      commit: 6ece49c56965544262523dae4a071ace3db63507
[2/5] spi: amd: Make use of devm_spi_alloc_master()
      commit: 2e063bb1d4272e7b64ef813566691ea8ea192f9c
[3/5] spi: amd: Make use of dev_err_probe()
      commit: deef4da8be2f7e94a0807e56f856d3e20addce4d
[4/5] spi: amd: Drop io_base_addr member from struct amd_spi
      commit: 1e71ffee97ac02b83b6ff75b52fa7b21b9149f7d
[5/5] spi: amd: Add struct and enum kernel-doc comments
      commit: 55861e36b663f6e584d1b0659c1c5cec0ce26a5d

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