[PATCH v1 0/7] spi: microchip-core: Code improvements

[PATCH v1 0/7] spi: microchip-core: Code improvements

[Andy Shevchenko]

While reading some other stuff, I noticed that this driver may
be improved. Here is the set of refactoring and cleaning it up.

Andy Shevchenko (7):
  spi: microchip-core: use min() instead of min_t()
  spi: microchip-core: Make use of device properties
  spi: microchip-core: Refactor FIFO read and write handlers
  spi: microchip-core: Replace dead code (-ENOMEM error message)
  spi: microchip-core: Utilise temporary variable for struct device
  spi: microchip-core: Use SPI_MODE_X_MASK
  spi: microchip-core: Remove unneeded PM related macro

 drivers/spi/spi-microchip-core-spi.c | 113 +++++++++++----------------
 1 file changed, 46 insertions(+), 67 deletions(-)

-- 
2.50.1

[PATCH v1 1/7] spi: microchip-core: use min() instead of min_t()

[Andy Shevchenko]

min_t(int, a, b) casts an 'unsigned int' to 'int'. This might lead
to the cases when big number is wrongly chosen. On the other hand,
the SPI transfer length is unsigned and driver uses signed type for
an unknown reason. Change the type of the transfer length to be
unsigned and convert use min() instead of min_t().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi-microchip-core-spi.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi-microchip-core-spi.c b/drivers/spi/spi-microchip-core-spi.c
index 16e0885474a0..08ccdc5f0cc9 100644
--- a/drivers/spi/spi-microchip-core-spi.c
+++ b/drivers/spi/spi-microchip-core-spi.c
@@ -74,8 +74,8 @@ struct mchp_corespi {
 	u8 *rx_buf;
 	u32 clk_gen;
 	int irq;
-	int tx_len;
-	int rx_len;
+	unsigned int tx_len;
+	unsigned int rx_len;
 	u32 fifo_depth;
 };
 
@@ -214,7 +214,7 @@ static irqreturn_t mchp_corespi_interrupt(int irq, void *dev_id)
 		       spi->regs + MCHP_CORESPI_REG_INTCLEAR);
 		finalise = true;
 		dev_err(&host->dev,
-			"RX OVERFLOW: rxlen: %d, txlen: %d\n",
+			"RX OVERFLOW: rxlen: %u, txlen: %u\n",
 			spi->rx_len, spi->tx_len);
 	}
 
@@ -223,7 +223,7 @@ static irqreturn_t mchp_corespi_interrupt(int irq, void *dev_id)
 		       spi->regs + MCHP_CORESPI_REG_INTCLEAR);
 		finalise = true;
 		dev_err(&host->dev,
-			"TX UNDERFLOW: rxlen: %d, txlen: %d\n",
+			"TX UNDERFLOW: rxlen: %u, txlen: %u\n",
 			spi->rx_len, spi->tx_len);
 	}
 
@@ -283,7 +283,7 @@ static int mchp_corespi_transfer_one(struct spi_controller *host,
 	spi->rx_len = xfer->len;
 
 	while (spi->tx_len) {
-		int fifo_max = min_t(int, spi->tx_len, spi->fifo_depth);
+		unsigned int fifo_max = min(spi->tx_len, spi->fifo_depth);
 
 		mchp_corespi_write_fifo(spi, fifo_max);
 		mchp_corespi_read_fifo(spi, fifo_max);
-- 
2.50.1

[PATCH v1 2/7] spi: microchip-core: Make use of device properties

[Andy Shevchenko]

Convert the module to be property provider agnostic and allow
it to be used on non-OF platforms.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi-microchip-core-spi.c | 36 ++++++++++++++--------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/spi/spi-microchip-core-spi.c b/drivers/spi/spi-microchip-core-spi.c
index 08ccdc5f0cc9..d2d1e86568a3 100644
--- a/drivers/spi/spi-microchip-core-spi.c
+++ b/drivers/spi/spi-microchip-core-spi.c
@@ -12,9 +12,10 @@
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
+#include <linux/mod_devicetable.h>
 #include <linux/module.h>
-#include <linux/of.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/spi/spi.h>
 
 #define MCHP_CORESPI_MAX_CS				(8)
@@ -296,6 +297,7 @@ static int mchp_corespi_transfer_one(struct spi_controller *host,
 static int mchp_corespi_probe(struct platform_device *pdev)
 {
 	const char *protocol = "motorola";
+	struct device *dev = &pdev->dev;
 	struct spi_controller *host;
 	struct mchp_corespi *spi;
 	struct resource *res;
@@ -310,7 +312,7 @@ static int mchp_corespi_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, host);
 
-	if (of_property_read_u32(pdev->dev.of_node, "num-cs", &num_cs))
+	if (device_property_read_u32(dev, "num-cs", &num_cs))
 		num_cs = MCHP_CORESPI_MAX_CS;
 
 	/*
@@ -318,20 +320,18 @@ static int mchp_corespi_probe(struct platform_device *pdev)
 	 * CoreSPI can be configured for Motorola, TI or NSC.
 	 * The current driver supports only Motorola mode.
 	 */
-	ret = of_property_read_string(pdev->dev.of_node, "microchip,protocol-configuration",
-				      &protocol);
-	if (ret && ret != -EINVAL)
-		return dev_err_probe(&pdev->dev, ret, "Error reading protocol-configuration\n");
-	if (strcmp(protocol, "motorola") != 0)
-		return dev_err_probe(&pdev->dev, -EINVAL,
-				     "CoreSPI: protocol '%s' not supported by this driver\n",
-				      protocol);
+	ret = device_property_match_property_string(dev, "microchip,protocol-configuration",
+						    &protocol, 1);
+	if (ret == -ENOENT)
+		return dev_err_probe(dev, ret, "CoreSPI: protocol is not supported by this driver\n");
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Error reading protocol-configuration\n");
 
 	/*
 	 * Motorola mode (0-3): CFG_MOT_MODE
 	 * Mode is fixed in the IP configurator.
 	 */
-	ret = of_property_read_u32(pdev->dev.of_node, "microchip,motorola-mode", &mode);
+	ret = device_property_read_u32(dev, "microchip,motorola-mode", &mode);
 	if (ret)
 		mode = MCHP_CORESPI_DEFAULT_MOTOROLA_MODE;
 	else if (mode > 3)
@@ -343,7 +343,7 @@ static int mchp_corespi_probe(struct platform_device *pdev)
 	 * The hardware allows frame sizes <= APB data width.
 	 * However, this driver currently only supports 8-bit frames.
 	 */
-	ret = of_property_read_u32(pdev->dev.of_node, "microchip,frame-size", &frame_size);
+	ret = device_property_read_u32(dev, "microchip,frame-size", &frame_size);
 	if (!ret && frame_size != 8)
 		return dev_err_probe(&pdev->dev, -EINVAL,
 				     "CoreSPI: frame size %u not supported by this driver\n",
@@ -355,7 +355,7 @@ static int mchp_corespi_probe(struct platform_device *pdev)
 	 * To prevent CS deassertion when TX FIFO drains, the ssel-active property
 	 * keeps CS asserted for the full SPI transfer.
 	 */
-	assert_ssel = of_property_read_bool(pdev->dev.of_node, "microchip,ssel-active");
+	assert_ssel = device_property_read_bool(dev, "microchip,ssel-active");
 	if (!assert_ssel)
 		return dev_err_probe(&pdev->dev, -EINVAL,
 				     "hardware must enable 'microchip,ssel-active' to keep CS asserted for the SPI transfer\n");
@@ -369,9 +369,10 @@ static int mchp_corespi_probe(struct platform_device *pdev)
 	host->bits_per_word_mask = SPI_BPW_RANGE_MASK(4, 32);
 	host->transfer_one = mchp_corespi_transfer_one;
 	host->set_cs = mchp_corespi_set_cs;
-	host->dev.of_node = pdev->dev.of_node;
 
-	ret = of_property_read_u32(pdev->dev.of_node, "fifo-depth", &spi->fifo_depth);
+	device_set_node(&host->dev, dev_fwnode(dev));
+
+	ret = device_property_read_u32(dev, "fifo-depth", &spi->fifo_depth);
 	if (ret)
 		spi->fifo_depth = MCHP_CORESPI_DEFAULT_FIFO_DEPTH;
 
@@ -421,24 +422,23 @@ static void mchp_corespi_remove(struct platform_device *pdev)
  * Platform driver data structure
  */
 
-#if defined(CONFIG_OF)
 static const struct of_device_id mchp_corespi_dt_ids[] = {
 	{ .compatible = "microchip,corespi-rtl-v5" },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, mchp_corespi_dt_ids);
-#endif
 
 static struct platform_driver mchp_corespi_driver = {
 	.probe = mchp_corespi_probe,
 	.driver = {
 		.name = "microchip-corespi",
 		.pm = MICROCHIP_SPI_PM_OPS,
-		.of_match_table = of_match_ptr(mchp_corespi_dt_ids),
+		.of_match_table = mchp_corespi_dt_ids,
 	},
 	.remove = mchp_corespi_remove,
 };
 module_platform_driver(mchp_corespi_driver);
+
 MODULE_DESCRIPTION("Microchip CoreSPI controller driver");
 MODULE_AUTHOR("Prajna Rajendra Kumar <prajna.rajendrakumar@microchip.com>");
 MODULE_LICENSE("GPL");
-- 
2.50.1

[PATCH v1 3/7] spi: microchip-core: Refactor FIFO read and write handlers

[Andy Shevchenko]

Make both handlers to be shorter and easier to understand.
While at it, unify their style.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi-microchip-core-spi.c | 32 +++++++++-------------------
 1 file changed, 10 insertions(+), 22 deletions(-)

diff --git a/drivers/spi/spi-microchip-core-spi.c b/drivers/spi/spi-microchip-core-spi.c
index d2d1e86568a3..9620aa886912 100644
--- a/drivers/spi/spi-microchip-core-spi.c
+++ b/drivers/spi/spi-microchip-core-spi.c
@@ -92,21 +92,14 @@ static inline void mchp_corespi_disable(struct mchp_corespi *spi)
 static inline void mchp_corespi_read_fifo(struct mchp_corespi *spi, u32 fifo_max)
 {
 	for (int i = 0; i < fifo_max; i++) {
-		u32 data;
-
 		while (readb(spi->regs + MCHP_CORESPI_REG_STAT) &
 		       MCHP_CORESPI_STATUS_RXFIFO_EMPTY)
 			;
 
-		data = readb(spi->regs + MCHP_CORESPI_REG_RXDATA);
+		if (spi->rx_buf)
+			*spi->rx_buf++ = readb(spi->regs + MCHP_CORESPI_REG_RXDATA);
 
 		spi->rx_len--;
-		if (!spi->rx_buf)
-			continue;
-
-		*spi->rx_buf = data;
-
-		spi->rx_buf++;
 	}
 }
 
@@ -128,23 +121,18 @@ static void mchp_corespi_disable_ints(struct mchp_corespi *spi)
 
 static inline void mchp_corespi_write_fifo(struct mchp_corespi *spi, u32 fifo_max)
 {
-	int i = 0;
-
-	while ((i < fifo_max) &&
-	       !(readb(spi->regs + MCHP_CORESPI_REG_STAT) &
-		 MCHP_CORESPI_STATUS_TXFIFO_FULL)) {
-		u32 word;
-
-		word = spi->tx_buf ? *spi->tx_buf : 0xaa;
-		writeb(word, spi->regs + MCHP_CORESPI_REG_TXDATA);
+	for (int i = 0; i < fifo_max; i++) {
+		if (readb(spi->regs + MCHP_CORESPI_REG_STAT) &
+		    MCHP_CORESPI_STATUS_TXFIFO_FULL)
+			break;
 
 		if (spi->tx_buf)
-			spi->tx_buf++;
+			writeb(*spi->tx_buf++, spi->regs + MCHP_CORESPI_REG_TXDATA);
+		else
+			writeb(0xaa, spi->regs + MCHP_CORESPI_REG_TXDATA);
 
-		i++;
+		spi->tx_len--;
 	}
-
-	spi->tx_len -= i;
 }
 
 static void mchp_corespi_set_cs(struct spi_device *spi, bool disable)
-- 
2.50.1

[PATCH v1 4/7] spi: microchip-core: Replace dead code (-ENOMEM error message)

[Andy Shevchenko]

First of all, the convention in the kernel that we do not issue
error messages for -ENOMEM. Second, it's ignored by dev_err_probe().
Replace dead code by a simple return statement.

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

diff --git a/drivers/spi/spi-microchip-core-spi.c b/drivers/spi/spi-microchip-core-spi.c
index 9620aa886912..dbbfb395c272 100644
--- a/drivers/spi/spi-microchip-core-spi.c
+++ b/drivers/spi/spi-microchip-core-spi.c
@@ -295,8 +295,7 @@ static int mchp_corespi_probe(struct platform_device *pdev)
 
 	host = devm_spi_alloc_host(&pdev->dev, sizeof(*spi));
 	if (!host)
-		return dev_err_probe(&pdev->dev, -ENOMEM,
-				     "unable to allocate host for SPI controller\n");
+		return -ENOMEM;
 
 	platform_set_drvdata(pdev, host);
 
-- 
2.50.1

[PATCH v1 5/7] spi: microchip-core: Utilise temporary variable for struct device

[Andy Shevchenko]

We have a temporary variable to keep a pointer to struct device.
Utilise it where it makes sense.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi-microchip-core-spi.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/spi/spi-microchip-core-spi.c b/drivers/spi/spi-microchip-core-spi.c
index dbbfb395c272..68b136157fac 100644
--- a/drivers/spi/spi-microchip-core-spi.c
+++ b/drivers/spi/spi-microchip-core-spi.c
@@ -293,7 +293,7 @@ static int mchp_corespi_probe(struct platform_device *pdev)
 	bool assert_ssel;
 	int ret = 0;
 
-	host = devm_spi_alloc_host(&pdev->dev, sizeof(*spi));
+	host = devm_spi_alloc_host(dev, sizeof(*spi));
 	if (!host)
 		return -ENOMEM;
 
@@ -322,7 +322,7 @@ static int mchp_corespi_probe(struct platform_device *pdev)
 	if (ret)
 		mode = MCHP_CORESPI_DEFAULT_MOTOROLA_MODE;
 	else if (mode > 3)
-		return dev_err_probe(&pdev->dev, -EINVAL,
+		return dev_err_probe(dev, -EINVAL,
 				     "invalid 'microchip,motorola-mode' value %u\n", mode);
 
 	/*
@@ -332,7 +332,7 @@ static int mchp_corespi_probe(struct platform_device *pdev)
 	 */
 	ret = device_property_read_u32(dev, "microchip,frame-size", &frame_size);
 	if (!ret && frame_size != 8)
-		return dev_err_probe(&pdev->dev, -EINVAL,
+		return dev_err_probe(dev, -EINVAL,
 				     "CoreSPI: frame size %u not supported by this driver\n",
 				     frame_size);
 
@@ -344,7 +344,7 @@ static int mchp_corespi_probe(struct platform_device *pdev)
 	 */
 	assert_ssel = device_property_read_bool(dev, "microchip,ssel-active");
 	if (!assert_ssel)
-		return dev_err_probe(&pdev->dev, -EINVAL,
+		return dev_err_probe(dev, -EINVAL,
 				     "hardware must enable 'microchip,ssel-active' to keep CS asserted for the SPI transfer\n");
 
 	spi = spi_controller_get_devdata(host);
@@ -371,24 +371,21 @@ static int mchp_corespi_probe(struct platform_device *pdev)
 	if (spi->irq < 0)
 		return spi->irq;
 
-	ret = devm_request_irq(&pdev->dev, spi->irq, mchp_corespi_interrupt,
-			       IRQF_SHARED, dev_name(&pdev->dev), host);
+	ret = devm_request_irq(dev, spi->irq, mchp_corespi_interrupt, IRQF_SHARED,
+			       dev_name(dev), host);
 	if (ret)
-		return dev_err_probe(&pdev->dev, ret,
-				     "could not request irq\n");
+		return dev_err_probe(dev, ret, "could not request irq\n");
 
-	spi->clk = devm_clk_get_enabled(&pdev->dev, NULL);
+	spi->clk = devm_clk_get_enabled(dev, NULL);
 	if (IS_ERR(spi->clk))
-		return dev_err_probe(&pdev->dev, PTR_ERR(spi->clk),
-				     "could not get clk\n");
+		return dev_err_probe(dev, PTR_ERR(spi->clk), "could not get clk\n");
 
 	mchp_corespi_init(host, spi);
 
-	ret = devm_spi_register_controller(&pdev->dev, host);
+	ret = devm_spi_register_controller(dev, host);
 	if (ret) {
 		mchp_corespi_disable(spi);
-		return dev_err_probe(&pdev->dev, ret,
-				     "unable to register host for CoreSPI controller\n");
+		return dev_err_probe(dev, ret, "unable to register host for CoreSPI controller\n");
 	}
 
 	return 0;
-- 
2.50.1

[PATCH v1 6/7] spi: microchip-core: Use SPI_MODE_X_MASK

[Andy Shevchenko]

Use SPI_MODE_X_MASK instead of open coded variant.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi-microchip-core-spi.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/spi/spi-microchip-core-spi.c b/drivers/spi/spi-microchip-core-spi.c
index 68b136157fac..2d7ee556eafd 100644
--- a/drivers/spi/spi-microchip-core-spi.c
+++ b/drivers/spi/spi-microchip-core-spi.c
@@ -149,8 +149,6 @@ static void mchp_corespi_set_cs(struct spi_device *spi, bool disable)
 
 static int mchp_corespi_setup(struct spi_device *spi)
 {
-	u32 dev_mode = spi->mode & (SPI_CPOL | SPI_CPHA);
-
 	if (spi_get_csgpiod(spi, 0))
 		return 0;
 
@@ -159,7 +157,7 @@ static int mchp_corespi_setup(struct spi_device *spi)
 		return -EOPNOTSUPP;
 	}
 
-	if (dev_mode & ~spi->controller->mode_bits) {
+	if (spi->mode & SPI_MODE_X_MASK & ~spi->controller->mode_bits) {
 		dev_err(&spi->dev, "incompatible CPOL/CPHA, must match controller's Motorola mode\n");
 		return -EINVAL;
 	}
-- 
2.50.1

[PATCH v1 7/7] spi: microchip-core: Remove unneeded PM related macro

[Andy Shevchenko]

Static declaration by default are 0 or NULL, no need to initialise
them explicitly. Remove unneeded PM related macro.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/spi/spi-microchip-core-spi.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/spi/spi-microchip-core-spi.c b/drivers/spi/spi-microchip-core-spi.c
index 2d7ee556eafd..d09daa25984c 100644
--- a/drivers/spi/spi-microchip-core-spi.c
+++ b/drivers/spi/spi-microchip-core-spi.c
@@ -398,8 +398,6 @@ static void mchp_corespi_remove(struct platform_device *pdev)
 	mchp_corespi_disable(spi);
 }
 
-#define MICROCHIP_SPI_PM_OPS (NULL)
-
 /*
  * Platform driver data structure
  */
@@ -414,7 +412,6 @@ static struct platform_driver mchp_corespi_driver = {
 	.probe = mchp_corespi_probe,
 	.driver = {
 		.name = "microchip-corespi",
-		.pm = MICROCHIP_SPI_PM_OPS,
 		.of_match_table = mchp_corespi_dt_ids,
 	},
 	.remove = mchp_corespi_remove,
-- 
2.50.1

Re: [PATCH v1 2/7] spi: microchip-core: Make use of device properties

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 246 bytes --]

On Tue, Nov 25, 2025 at 09:15:32PM +0100, Andy Shevchenko wrote:
> Convert the module to be property provider agnostic and allow
> it to be used on non-OF platforms.

Are we sure that these properties are tasteful and sensible on an ACPI
system?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 2/7] spi: microchip-core: Make use of device properties

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 769 bytes --]

On Tue, Nov 25, 2025 at 10:42:12PM +0000, Mark Brown wrote:
> On Tue, Nov 25, 2025 at 09:15:32PM +0100, Andy Shevchenko wrote:
> > Convert the module to be property provider agnostic and allow
> > it to be used on non-OF platforms.
> 
> Are we sure that these properties are tasteful and sensible on an ACPI
> system?

If you're on an ACPI platform, you're probably not using this IP. It's
really old and in need of an update (like what the "hard" version of
it got), we just happened to have some customers that wanted more SPI
controllers on the FPGA. I dunno if using these provider agnostic APIs
is something we should be doing where possible as good practise, but if
it's to make the IP more broadly usable I'd say that that's mostly
wasted effort.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1 2/7] spi: microchip-core: Make use of device properties

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1041 bytes --]

On Tue, Nov 25, 2025 at 11:00:48PM +0000, Conor Dooley wrote:
> On Tue, Nov 25, 2025 at 10:42:12PM +0000, Mark Brown wrote:
> > On Tue, Nov 25, 2025 at 09:15:32PM +0100, Andy Shevchenko wrote:
> > > Convert the module to be property provider agnostic and allow
> > > it to be used on non-OF platforms.

> > Are we sure that these properties are tasteful and sensible on an ACPI
> > system?

> If you're on an ACPI platform, you're probably not using this IP. It's
> really old and in need of an update (like what the "hard" version of
> it got), we just happened to have some customers that wanted more SPI
> controllers on the FPGA. I dunno if using these provider agnostic APIs
> is something we should be doing where possible as good practise, but if
> it's to make the IP more broadly usable I'd say that that's mostly
> wasted effort.

I really don't think it's a good idea to just do it as a thoughtless
default given that there are cases where we actively want a different
interface on ACPI or don't want to see a device used at all.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 2/7] spi: microchip-core: Make use of device properties

[Andy Shevchenko]

On Tue, Nov 25, 2025 at 11:19:22PM +0000, Mark Brown wrote:
> On Tue, Nov 25, 2025 at 11:00:48PM +0000, Conor Dooley wrote:
> > On Tue, Nov 25, 2025 at 10:42:12PM +0000, Mark Brown wrote:
> > > On Tue, Nov 25, 2025 at 09:15:32PM +0100, Andy Shevchenko wrote:
> > > > Convert the module to be property provider agnostic and allow
> > > > it to be used on non-OF platforms.
> 
> > > Are we sure that these properties are tasteful and sensible on an ACPI
> > > system?
> 
> > If you're on an ACPI platform, you're probably not using this IP.

OK.

> > It's
> > really old and in need of an update (like what the "hard" version of
> > it got), we just happened to have some customers that wanted more SPI
> > controllers on the FPGA. I dunno if using these provider agnostic APIs
> > is something we should be doing where possible as good practise, but if
> > it's to make the IP more broadly usable I'd say that that's mostly
> > wasted effort.

See below.

> I really don't think it's a good idea to just do it as a thoughtless
> default given that there are cases where we actively want a different
> interface on ACPI or don't want to see a device used at all.

Okay,I think the commit message is a bit misleading. There are two ideas behind
this change, one is to have agnostic APIs in use, second one to make code
shorter and cleaner. Assuming we are targeting the second one as a main point,
does this make sense?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 2/7] spi: microchip-core: Make use of device properties

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 845 bytes --]

On Wed, Nov 26, 2025 at 08:35:10AM +0200, Andy Shevchenko wrote:
> On Tue, Nov 25, 2025 at 11:19:22PM +0000, Mark Brown wrote:

> > I really don't think it's a good idea to just do it as a thoughtless
> > default given that there are cases where we actively want a different
> > interface on ACPI or don't want to see a device used at all.

> Okay,I think the commit message is a bit misleading. There are two ideas behind
> this change, one is to have agnostic APIs in use, second one to make code
> shorter and cleaner. Assuming we are targeting the second one as a main point,
> does this make sense?

I'm still a bit dubious TBH, I didn't spot anything super obvious in the
patch and there's the whole should you actually use anything other than
DT question still.  It'd seem better to make the OF APIs better if
there's some big win there.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 2/7] spi: microchip-core: Make use of device properties

[Andy Shevchenko]

On Wed, Nov 26, 2025 at 12:03:40PM +0000, Mark Brown wrote:
> On Wed, Nov 26, 2025 at 08:35:10AM +0200, Andy Shevchenko wrote:
> > On Tue, Nov 25, 2025 at 11:19:22PM +0000, Mark Brown wrote:
> 
> > > I really don't think it's a good idea to just do it as a thoughtless
> > > default given that there are cases where we actively want a different
> > > interface on ACPI or don't want to see a device used at all.
> 
> > Okay,I think the commit message is a bit misleading. There are two ideas behind
> > this change, one is to have agnostic APIs in use, second one to make code
> > shorter and cleaner. Assuming we are targeting the second one as a main point,
> > does this make sense?

> I'm still a bit dubious TBH, I didn't spot anything super obvious in the
> patch and there's the whole should you actually use anything other than
> DT question still.

The only helpful API which has no par on DT side is matching strings.
Otherwise it's only about shorten lines and less LoCs.

> It'd seem better to make the OF APIs better if
> there's some big win there.

I dropped it in v2. Can that be applied instead?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 2/7] spi: microchip-core: Make use of device properties

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 310 bytes --]

On Wed, Nov 26, 2025 at 06:27:15PM +0200, Andy Shevchenko wrote:
> On Wed, Nov 26, 2025 at 12:03:40PM +0000, Mark Brown wrote:

> > It'd seem better to make the OF APIs better if
> > there's some big win there.

> I dropped it in v2. Can that be applied instead?

The microchip people do usually review stuff.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v1 2/7] spi: microchip-core: Make use of device properties

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 547 bytes --]

On Wed, Nov 26, 2025 at 05:24:49PM +0000, Mark Brown wrote:
> On Wed, Nov 26, 2025 at 06:27:15PM +0200, Andy Shevchenko wrote:
> > On Wed, Nov 26, 2025 at 12:03:40PM +0000, Mark Brown wrote:
> 
> > > It'd seem better to make the OF APIs better if
> > > there's some big win there.
> 
> > I dropped it in v2. Can that be applied instead?
> 
> The microchip people do usually review stuff.

I mentioned v1 to Prajna this morning, she was definitely intending
providing review, so I'd expect that you hear from her on v2 soon
enough.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1 2/7] spi: microchip-core: Make use of device properties

[Andy Shevchenko]

On Wed, Nov 26, 2025 at 05:29:20PM +0000, Conor Dooley wrote:
> On Wed, Nov 26, 2025 at 05:24:49PM +0000, Mark Brown wrote:
> > On Wed, Nov 26, 2025 at 06:27:15PM +0200, Andy Shevchenko wrote:
> > > On Wed, Nov 26, 2025 at 12:03:40PM +0000, Mark Brown wrote:
> > 
> > > > It'd seem better to make the OF APIs better if
> > > > there's some big win there.
> > 
> > > I dropped it in v2. Can that be applied instead?
> > 
> > The microchip people do usually review stuff.
> 
> I mentioned v1 to Prajna this morning, she was definitely intending
> providing review, so I'd expect that you hear from her on v2 soon
> enough.

Thank you!

-- 
With Best Regards,
Andy Shevchenko