[PATCH v1 0/5] spi: mpc5xxx-psc: Clean up even more and fix

[PATCH v1 0/5] spi: mpc5xxx-psc: Clean up even more and fix

[Andy Shevchenko]

The recent cleanup series broke the error path in the drivers.
So fix it and do even more cleanups.

Andy Shevchenko (5):
  spi: mpc5xxx-psc: Correct error check for
    devm_platform_get_and_ioremap_resource()
  spi: mpc5xxx-psc: Return immediately if IRQ resource is unavailable
  spi: mpc5xxx-psc: use devm_clk_get_enabled() for core clock
  spi: mpc5xxx-psc: Propagate firmware node
  spi: mpc5xxx-psc: Consistently use device property APIs

 drivers/spi/spi-mpc512x-psc.c | 50 +++++++++--------------------------
 drivers/spi/spi-mpc52xx-psc.c | 15 +++++++----
 2 files changed, 22 insertions(+), 43 deletions(-)

-- 
2.39.1

[PATCH v1 1/5] spi: mpc5xxx-psc: Correct error check for devm_platform_get_and_ioremap_resource()

[Andy Shevchenko]

devm_platform_get_and_ioremap_resource() may return pointer or error
pointer, never the NULL. Correct error check for it.

Fixes: 60a6c8257f41 ("spi: mpc5xxx-psc: Use platform resources instead of parsing DT properties")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi-mpc512x-psc.c | 4 ++--
 drivers/spi/spi-mpc52xx-psc.c | 5 +++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-mpc512x-psc.c b/drivers/spi/spi-mpc512x-psc.c
index 5bdfe4a740e9..a2ea24a63cb4 100644
--- a/drivers/spi/spi-mpc512x-psc.c
+++ b/drivers/spi/spi-mpc512x-psc.c
@@ -483,8 +483,8 @@ static int mpc512x_psc_spi_of_probe(struct platform_device *pdev)
 	master->dev.of_node = dev->of_node;
 
 	tempp = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
-	if (!tempp)
-		return dev_err_probe(dev, -EFAULT, "could not ioremap I/O port range\n");
+	if (IS_ERR(tempp))
+		return dev_err_probe(dev, PTR_ERR(tempp), "could not ioremap I/O port range\n");
 	mps->psc = tempp;
 	mps->fifo =
 		(struct mpc512x_psc_fifo *)(tempp + sizeof(struct mpc52xx_psc));
diff --git a/drivers/spi/spi-mpc52xx-psc.c b/drivers/spi/spi-mpc52xx-psc.c
index 95a4a511c388..1bf728f4d766 100644
--- a/drivers/spi/spi-mpc52xx-psc.c
+++ b/drivers/spi/spi-mpc52xx-psc.c
@@ -321,8 +321,9 @@ static int mpc52xx_psc_spi_of_probe(struct platform_device *pdev)
 	master->dev.of_node = dev->of_node;
 
 	mps->psc = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
-	if (!mps->psc)
-		return dev_err_probe(dev, -EFAULT, "could not ioremap I/O port range\n");
+	if (IS_ERR(mps->psc))
+		return dev_err_probe(dev, PTR_ERR(mps->psc), "could not ioremap I/O port range\n");
+
 	/* On the 5200, fifo regs are immediately ajacent to the psc regs */
 	mps->fifo = ((void __iomem *)mps->psc) + sizeof(struct mpc52xx_psc);
 
-- 
2.39.1

[PATCH v1 2/5] spi: mpc5xxx-psc: Return immediately if IRQ resource is unavailable

[Andy Shevchenko]

Return immediately if IRQ resource is unavailable. This will also
propagate the correct error code in such cases.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi-mpc512x-psc.c | 3 +++
 drivers/spi/spi-mpc52xx-psc.c | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/spi/spi-mpc512x-psc.c b/drivers/spi/spi-mpc512x-psc.c
index a2ea24a63cb4..8a436d1906eb 100644
--- a/drivers/spi/spi-mpc512x-psc.c
+++ b/drivers/spi/spi-mpc512x-psc.c
@@ -490,6 +490,9 @@ static int mpc512x_psc_spi_of_probe(struct platform_device *pdev)
 		(struct mpc512x_psc_fifo *)(tempp + sizeof(struct mpc52xx_psc));
 
 	mps->irq = platform_get_irq(pdev, 0);
+	if (mps->irq < 0)
+		return mps->irq;
+
 	ret = devm_request_irq(dev, mps->irq, mpc512x_psc_spi_isr, IRQF_SHARED,
 				"mpc512x-psc-spi", mps);
 	if (ret)
diff --git a/drivers/spi/spi-mpc52xx-psc.c b/drivers/spi/spi-mpc52xx-psc.c
index 1bf728f4d766..b75bc2457883 100644
--- a/drivers/spi/spi-mpc52xx-psc.c
+++ b/drivers/spi/spi-mpc52xx-psc.c
@@ -328,6 +328,9 @@ static int mpc52xx_psc_spi_of_probe(struct platform_device *pdev)
 	mps->fifo = ((void __iomem *)mps->psc) + sizeof(struct mpc52xx_psc);
 
 	mps->irq = platform_get_irq(pdev, 0);
+	if (mps->irq < 0)
+		return mps->irq;
+
 	ret = devm_request_irq(dev, mps->irq, mpc52xx_psc_spi_isr, 0,
 			       "mpc52xx-psc-spi", mps);
 	if (ret)
-- 
2.39.1

[PATCH v1 3/5] spi: mpc5xxx-psc: use devm_clk_get_enabled() for core clock

[Andy Shevchenko]

Use devm_clk_get_enabled() to simplify the code.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi-mpc512x-psc.c | 39 ++++-------------------------------
 1 file changed, 4 insertions(+), 35 deletions(-)

diff --git a/drivers/spi/spi-mpc512x-psc.c b/drivers/spi/spi-mpc512x-psc.c
index 8a436d1906eb..739df2b241e9 100644
--- a/drivers/spi/spi-mpc512x-psc.c
+++ b/drivers/spi/spi-mpc512x-psc.c
@@ -54,8 +54,6 @@ struct mpc512x_psc_spi {
 	struct mpc512x_psc_fifo __iomem *fifo;
 	unsigned int irq;
 	u8 bits_per_word;
-	struct clk *clk_mclk;
-	struct clk *clk_ipg;
 	u32 mclk_rate;
 
 	struct completion txisrdone;
@@ -499,25 +497,15 @@ static int mpc512x_psc_spi_of_probe(struct platform_device *pdev)
 		return ret;
 	init_completion(&mps->txisrdone);
 
-	clk = devm_clk_get(dev, "mclk");
+	clk = devm_clk_get_enabled(dev, "mclk");
 	if (IS_ERR(clk))
 		return PTR_ERR(clk);
 
-	ret = clk_prepare_enable(clk);
-	if (ret)
-		return ret;
-	mps->clk_mclk = clk;
 	mps->mclk_rate = clk_get_rate(clk);
 
-	clk = devm_clk_get(dev, "ipg");
-	if (IS_ERR(clk)) {
-		ret = PTR_ERR(clk);
-		goto free_mclk_clock;
-	}
-	ret = clk_prepare_enable(clk);
-	if (ret)
-		goto free_mclk_clock;
-	mps->clk_ipg = clk;
+	clk = devm_clk_get_enabled(dev, "ipg");
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
 
 	ret = mpc512x_psc_spi_port_config(master, mps);
 	if (ret < 0)
@@ -528,24 +516,6 @@ static int mpc512x_psc_spi_of_probe(struct platform_device *pdev)
 		goto free_ipg_clock;
 
 	return ret;
-
-free_ipg_clock:
-	clk_disable_unprepare(mps->clk_ipg);
-free_mclk_clock:
-	clk_disable_unprepare(mps->clk_mclk);
-
-	return ret;
-}
-
-static int mpc512x_psc_spi_of_remove(struct platform_device *pdev)
-{
-	struct spi_master *master = dev_get_drvdata(&pdev->dev);
-	struct mpc512x_psc_spi *mps = spi_master_get_devdata(master);
-
-	clk_disable_unprepare(mps->clk_mclk);
-	clk_disable_unprepare(mps->clk_ipg);
-
-	return 0;
 }
 
 static const struct of_device_id mpc512x_psc_spi_of_match[] = {
@@ -558,7 +528,6 @@ MODULE_DEVICE_TABLE(of, mpc512x_psc_spi_of_match);
 
 static struct platform_driver mpc512x_psc_spi_of_driver = {
 	.probe = mpc512x_psc_spi_of_probe,
-	.remove = mpc512x_psc_spi_of_remove,
 	.driver = {
 		.name = "mpc512x-psc-spi",
 		.of_match_table = mpc512x_psc_spi_of_match,
-- 
2.39.1

[PATCH v1 4/5] spi: mpc5xxx-psc: Propagate firmware node

[Andy Shevchenko]

Propagate firmware node by using a specific API call, i.e. device_set_node().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi-mpc512x-psc.c | 4 +++-
 drivers/spi/spi-mpc52xx-psc.c | 5 +++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-mpc512x-psc.c b/drivers/spi/spi-mpc512x-psc.c
index 739df2b241e9..77a228f633d1 100644
--- a/drivers/spi/spi-mpc512x-psc.c
+++ b/drivers/spi/spi-mpc512x-psc.c
@@ -17,6 +17,7 @@
 #include <linux/completion.h>
 #include <linux/io.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/delay.h>
 #include <linux/clk.h>
 #include <linux/spi/spi.h>
@@ -478,7 +479,8 @@ static int mpc512x_psc_spi_of_probe(struct platform_device *pdev)
 	master->unprepare_transfer_hardware = mpc512x_psc_spi_unprep_xfer_hw;
 	master->use_gpio_descriptors = true;
 	master->cleanup = mpc512x_psc_spi_cleanup;
-	master->dev.of_node = dev->of_node;
+
+	device_set_node(&master->dev, dev_fwnode(dev));
 
 	tempp = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
 	if (IS_ERR(tempp))
diff --git a/drivers/spi/spi-mpc52xx-psc.c b/drivers/spi/spi-mpc52xx-psc.c
index b75bc2457883..335a6b9eb141 100644
--- a/drivers/spi/spi-mpc52xx-psc.c
+++ b/drivers/spi/spi-mpc52xx-psc.c
@@ -11,8 +11,8 @@
 #include <linux/types.h>
 #include <linux/errno.h>
 #include <linux/interrupt.h>
-#include <linux/of.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/workqueue.h>
 #include <linux/completion.h>
 #include <linux/io.h>
@@ -318,7 +318,8 @@ static int mpc52xx_psc_spi_of_probe(struct platform_device *pdev)
 	master->setup = mpc52xx_psc_spi_setup;
 	master->transfer_one_message = mpc52xx_psc_spi_transfer_one_message;
 	master->cleanup = mpc52xx_psc_spi_cleanup;
-	master->dev.of_node = dev->of_node;
+
+	device_set_node(&master->dev, dev_fwnode(dev));
 
 	mps->psc = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
 	if (IS_ERR(mps->psc))
-- 
2.39.1

[PATCH v1 5/5] spi: mpc5xxx-psc: Consistently use device property APIs

[Andy Shevchenko]

Instead of calling the OF APIs mixed with device property APIs,
just switch to use the latter everywhere.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi-mpc52xx-psc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-mpc52xx-psc.c b/drivers/spi/spi-mpc52xx-psc.c
index 335a6b9eb141..9a1a080fb688 100644
--- a/drivers/spi/spi-mpc52xx-psc.c
+++ b/drivers/spi/spi-mpc52xx-psc.c
@@ -309,7 +309,7 @@ static int mpc52xx_psc_spi_of_probe(struct platform_device *pdev)
 	/* the spi->mode bits understood by this driver: */
 	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | SPI_LSB_FIRST;
 
-	ret = of_property_read_u32(dev->of_node, "cell-index", &bus_num);
+	ret = device_property_read_u32(dev, "cell-index", &bus_num);
 	if (ret || bus_num > 5)
 		return dev_err_probe(dev, ret ? : -EINVAL, "Invalid cell-index property\n");
 	master->bus_num = bus_num + 1;
-- 
2.39.1

Re: [PATCH v1 0/5] spi: mpc5xxx-psc: Clean up even more and fix

[Mark Brown]

On Mon, 06 Mar 2023 20:31:10 +0200, Andy Shevchenko wrote:
> The recent cleanup series broke the error path in the drivers.
> So fix it and do even more cleanups.
> 
> Andy Shevchenko (5):
>   spi: mpc5xxx-psc: Correct error check for
>     devm_platform_get_and_ioremap_resource()
>   spi: mpc5xxx-psc: Return immediately if IRQ resource is unavailable
>   spi: mpc5xxx-psc: use devm_clk_get_enabled() for core clock
>   spi: mpc5xxx-psc: Propagate firmware node
>   spi: mpc5xxx-psc: Consistently use device property APIs
> 
> [...]

Applied to

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

Thanks!

[1/5] spi: mpc5xxx-psc: Correct error check for devm_platform_get_and_ioremap_resource()
      commit: ee493fa5d91dec02402239a072820b18beb36265
[2/5] spi: mpc5xxx-psc: Return immediately if IRQ resource is unavailable
      commit: 208ee586f86237969a91ac60ea10f48db9135143
[3/5] spi: mpc5xxx-psc: use devm_clk_get_enabled() for core clock
      commit: 9e21720a49589304aef9e37e8b6c6a4196daf156
[4/5] spi: mpc5xxx-psc: Propagate firmware node
      commit: 289c084ddc1317e6ed911911f95371679c10af1e
[5/5] spi: mpc5xxx-psc: Consistently use device property APIs
      commit: 3169c5816a55ba671e9c8a671c6c75818d30c657

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