[PATCH 0/4] spi: Adding support for software nodes

[PATCH 0/4] spi: Adding support for software nodes

[Heikki Krogerus]

Hi,

The older API used to supply additional device properties for the
devices - so mainly the function device_add_properties() - is going to
be removed. The reason why the API will be removed is because it gives
false impression that the properties are assigned directly to the
devices, which has actually never been the case - the properties have
always been assigned to a software fwnode which was then just directly
linked with the device when the old API was used. By only accepting
device properties instead of complete software nodes, the subsystems
remove any change of taking advantage of the other features the
software nodes have.

The change that is required from the spi subsystem and the drivers is
trivial. Basically only the "properties" member in struct
spi_board_info, which was a pointer to struct property_entry, is
replaced with a pointer to a complete software node.

thanks,

Heikki Krogerus (4):
  spi: Add support for software nodes
  ARM: pxa: icontrol: Constify the software node
  ARM: pxa: zeus: Constify the software node
  spi: Remove support for dangling device properties

 arch/arm/mach-pxa/icontrol.c | 12 ++++++++----
 arch/arm/mach-pxa/zeus.c     |  6 +++++-
 drivers/spi/spi.c            | 21 ++++++---------------
 include/linux/spi/spi.h      |  7 +++----
 4 files changed, 22 insertions(+), 24 deletions(-)

-- 
2.30.1

[PATCH 1/4] spi: Add support for software nodes

[Heikki Krogerus]

Making it possible for the drivers to assign complete
software fwnodes to the devices instead of only the device
properties in those nodes.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/spi/spi.c       | 13 +++++++++++--
 include/linux/spi/spi.h |  5 ++++-
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index b08efe88ccd6c..9b46998ae2a44 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -686,6 +686,15 @@ struct spi_device *spi_new_device(struct spi_controller *ctlr,
 		}
 	}
 
+	if (chip->swnode) {
+		status = device_add_software_node(&proxy->dev, chip->swnode);
+		if (status) {
+			dev_err(&ctlr->dev, "failed to add softwade node to '%s': %d\n",
+				chip->modalias, status);
+			goto err_remove_props;
+		}
+	}
+
 	status = spi_add_device(proxy);
 	if (status < 0)
 		goto err_remove_props;
@@ -693,8 +702,7 @@ struct spi_device *spi_new_device(struct spi_controller *ctlr,
 	return proxy;
 
 err_remove_props:
-	if (chip->properties)
-		device_remove_properties(&proxy->dev);
+	device_remove_software_node(&proxy->dev);
 err_dev_put:
 	spi_dev_put(proxy);
 	return NULL;
@@ -719,6 +727,7 @@ void spi_unregister_device(struct spi_device *spi)
 	}
 	if (ACPI_COMPANION(&spi->dev))
 		acpi_device_clear_enumerated(ACPI_COMPANION(&spi->dev));
+	device_remove_software_node(&spi->dev);
 	device_unregister(&spi->dev);
 }
 EXPORT_SYMBOL_GPL(spi_unregister_device);
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 592897fa4f030..f47f94ea6fa99 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -20,6 +20,7 @@
 
 struct dma_chan;
 struct property_entry;
+struct software_node;
 struct spi_controller;
 struct spi_transfer;
 struct spi_controller_mem_ops;
@@ -1409,7 +1410,8 @@ static inline ssize_t spi_w8r16be(struct spi_device *spi, u8 cmd)
  * @modalias: Initializes spi_device.modalias; identifies the driver.
  * @platform_data: Initializes spi_device.platform_data; the particular
  *	data stored there is driver-specific.
- * @properties: Additional device properties for the device.
+ * @properties: Deprecated - use software node instead.
+ * @swnode: Software node for the device.
  * @controller_data: Initializes spi_device.controller_data; some
  *	controllers need hints about hardware setup, e.g. for DMA.
  * @irq: Initializes spi_device.irq; depends on how the board is wired.
@@ -1448,6 +1450,7 @@ struct spi_board_info {
 	char		modalias[SPI_NAME_SIZE];
 	const void	*platform_data;
 	const struct property_entry *properties;
+	const struct software_node *swnode;
 	void		*controller_data;
 	int		irq;
 
-- 
2.30.1

[PATCH 2/4] ARM: pxa: icontrol: Constify the software node

[Heikki Krogerus]

When device properties are supplied to the devices, in
reality a software fwnode that holds those properties is
created which is then assigned to the device. If the device
properties are constant the software node can also be
constant.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Daniel Mack <daniel@zonque.org>
Cc: Haojian Zhuang <haojian.zhuang@gmail.com>
Cc: Robert Jarzmik <robert.jarzmik@free.fr>
---
 arch/arm/mach-pxa/icontrol.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-pxa/icontrol.c b/arch/arm/mach-pxa/icontrol.c
index 151e26ec06968..04a12523cdee0 100644
--- a/arch/arm/mach-pxa/icontrol.c
+++ b/arch/arm/mach-pxa/icontrol.c
@@ -74,13 +74,17 @@ static const struct property_entry mcp251x_properties[] = {
 	{}
 };
 
+static const struct software_node mcp251x_node = {
+	.properties = mcp251x_properties,
+};
+
 static struct spi_board_info mcp251x_board_info[] = {
 	{
 		.modalias        = "mcp2515",
 		.max_speed_hz    = 6500000,
 		.bus_num         = 3,
 		.chip_select     = 0,
-		.properties      = mcp251x_properties,
+		.swnode		 = &mcp251x_node,
 		.controller_data = &mcp251x_chip_info1,
 		.irq             = PXA_GPIO_TO_IRQ(ICONTROL_MCP251x_nIRQ1)
 	},
@@ -89,7 +93,7 @@ static struct spi_board_info mcp251x_board_info[] = {
 		.max_speed_hz    = 6500000,
 		.bus_num         = 3,
 		.chip_select     = 1,
-		.properties      = mcp251x_properties,
+		.swnode		 = &mcp251x_node,
 		.controller_data = &mcp251x_chip_info2,
 		.irq             = PXA_GPIO_TO_IRQ(ICONTROL_MCP251x_nIRQ2)
 	},
@@ -98,7 +102,7 @@ static struct spi_board_info mcp251x_board_info[] = {
 		.max_speed_hz    = 6500000,
 		.bus_num         = 4,
 		.chip_select     = 0,
-		.properties      = mcp251x_properties,
+		.swnode		 = &mcp251x_node,
 		.controller_data = &mcp251x_chip_info3,
 		.irq             = PXA_GPIO_TO_IRQ(ICONTROL_MCP251x_nIRQ3)
 	},
@@ -107,7 +111,7 @@ static struct spi_board_info mcp251x_board_info[] = {
 		.max_speed_hz    = 6500000,
 		.bus_num         = 4,
 		.chip_select     = 1,
-		.properties      = mcp251x_properties,
+		.swnode		 = &mcp251x_node,
 		.controller_data = &mcp251x_chip_info4,
 		.irq             = PXA_GPIO_TO_IRQ(ICONTROL_MCP251x_nIRQ4)
 	}
-- 
2.30.1

[PATCH 3/4] ARM: pxa: zeus: Constify the software node

[Heikki Krogerus]

When device properties are supplied to the devices, in
reality a software fwnode that holds those properties is
created which is then assigned to the device. If the device
properties are constant the software node can also be
constant.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Daniel Mack <daniel@zonque.org>
Cc: Haojian Zhuang <haojian.zhuang@gmail.com>
Cc: Robert Jarzmik <robert.jarzmik@free.fr>
---
 arch/arm/mach-pxa/zeus.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-pxa/zeus.c b/arch/arm/mach-pxa/zeus.c
index b27fc7ac9ceab..97700429633ee 100644
--- a/arch/arm/mach-pxa/zeus.c
+++ b/arch/arm/mach-pxa/zeus.c
@@ -433,10 +433,14 @@ static const struct property_entry mcp251x_properties[] = {
 	{}
 };
 
+static const struct software_node mcp251x_node = {
+	.properties = mcp251x_properties,
+};
+
 static struct spi_board_info zeus_spi_board_info[] = {
 	[0] = {
 		.modalias	= "mcp2515",
-		.properties	= mcp251x_properties,
+		.swnode		= &mcp251x_node,
 		.irq		= PXA_GPIO_TO_IRQ(ZEUS_CAN_GPIO),
 		.max_speed_hz	= 1*1000*1000,
 		.bus_num	= 3,
-- 
2.30.1

[PATCH 4/4] spi: Remove support for dangling device properties

[Heikki Krogerus]

From now on only accepting complete software nodes.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/spi/spi.c       | 24 +++---------------------
 include/linux/spi/spi.h |  4 ----
 2 files changed, 3 insertions(+), 25 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 9b46998ae2a44..016bbfbce90b7 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -676,34 +676,23 @@ struct spi_device *spi_new_device(struct spi_controller *ctlr,
 	proxy->controller_data = chip->controller_data;
 	proxy->controller_state = NULL;
 
-	if (chip->properties) {
-		status = device_add_properties(&proxy->dev, chip->properties);
-		if (status) {
-			dev_err(&ctlr->dev,
-				"failed to add properties to '%s': %d\n",
-				chip->modalias, status);
-			goto err_dev_put;
-		}
-	}
-
 	if (chip->swnode) {
 		status = device_add_software_node(&proxy->dev, chip->swnode);
 		if (status) {
 			dev_err(&ctlr->dev, "failed to add softwade node to '%s': %d\n",
 				chip->modalias, status);
-			goto err_remove_props;
+			goto err_dev_put;
 		}
 	}
 
 	status = spi_add_device(proxy);
 	if (status < 0)
-		goto err_remove_props;
+		goto err_dev_put;
 
 	return proxy;
 
-err_remove_props:
-	device_remove_software_node(&proxy->dev);
 err_dev_put:
+	device_remove_software_node(&proxy->dev);
 	spi_dev_put(proxy);
 	return NULL;
 }
@@ -764,7 +753,6 @@ static void spi_match_controller_to_boardinfo(struct spi_controller *ctlr,
  *
  * The board info passed can safely be __initdata ... but be careful of
  * any embedded pointers (platform_data, etc), they're copied as-is.
- * Device properties are deep-copied though.
  *
  * Return: zero on success, else a negative error code.
  */
@@ -784,12 +772,6 @@ int spi_register_board_info(struct spi_board_info const *info, unsigned n)
 		struct spi_controller *ctlr;
 
 		memcpy(&bi->board_info, info, sizeof(*info));
-		if (info->properties) {
-			bi->board_info.properties =
-					property_entries_dup(info->properties);
-			if (IS_ERR(bi->board_info.properties))
-				return PTR_ERR(bi->board_info.properties);
-		}
 
 		mutex_lock(&board_lock);
 		list_add_tail(&bi->list, &board_list);
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index f47f94ea6fa99..7cb3194d5ba59 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -19,7 +19,6 @@
 #include <uapi/linux/spi/spi.h>
 
 struct dma_chan;
-struct property_entry;
 struct software_node;
 struct spi_controller;
 struct spi_transfer;
@@ -1410,7 +1409,6 @@ static inline ssize_t spi_w8r16be(struct spi_device *spi, u8 cmd)
  * @modalias: Initializes spi_device.modalias; identifies the driver.
  * @platform_data: Initializes spi_device.platform_data; the particular
  *	data stored there is driver-specific.
- * @properties: Deprecated - use software node instead.
  * @swnode: Software node for the device.
  * @controller_data: Initializes spi_device.controller_data; some
  *	controllers need hints about hardware setup, e.g. for DMA.
@@ -1444,12 +1442,10 @@ struct spi_board_info {
 	 *
 	 * platform_data goes to spi_device.dev.platform_data,
 	 * controller_data goes to spi_device.controller_data,
-	 * device properties are copied and attached to spi_device,
 	 * irq is copied too
 	 */
 	char		modalias[SPI_NAME_SIZE];
 	const void	*platform_data;
-	const struct property_entry *properties;
 	const struct software_node *swnode;
 	void		*controller_data;
 	int		irq;
-- 
2.30.1

Re: [PATCH 0/4] spi: Adding support for software nodes

[Mark Brown]

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

On Wed, Mar 03, 2021 at 06:28:10PM +0300, Heikki Krogerus wrote:
> Hi,
> 
> The older API used to supply additional device properties for the
> devices - so mainly the function device_add_properties() - is going to
> be removed. The reason why the API will be removed is because it gives

This makes sense to me, OK to apply the PXA patches via the SPI tree?

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

Re: [PATCH 0/4] spi: Adding support for software nodes

[Mark Brown]

On Wed, 3 Mar 2021 18:28:10 +0300, Heikki Krogerus wrote:
> The older API used to supply additional device properties for the
> devices - so mainly the function device_add_properties() - is going to
> be removed. The reason why the API will be removed is because it gives
> false impression that the properties are assigned directly to the
> devices, which has actually never been the case - the properties have
> always been assigned to a software fwnode which was then just directly
> linked with the device when the old API was used. By only accepting
> device properties instead of complete software nodes, the subsystems
> remove any change of taking advantage of the other features the
> software nodes have.
> 
> [...]

Applied to

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

Thanks!

[1/4] spi: Add support for software nodes
      commit: 47afc77bbfeac163d81c7a675d608c18561aa680
[2/4] ARM: pxa: icontrol: Constify the software node
      commit: 2df0c4a640c55c0eff7f97907b98ad6fdfedd226
[3/4] ARM: pxa: zeus: Constify the software node
      commit: d4272a7adf26c62c5afe86b6829712de519b4a26
[4/4] spi: Remove support for dangling device properties
      commit: df41a5dad586c8ead1bb7082b4b6fcb563e02199

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