device property: Consitify a few APIs and

PATCH In review Andy Shevchenko 2 participants 10 messages Oct 4, 2022 – Oct 22, 2022

Subsystems (5)

USB ACPI Driver core USB Type-C class +1 more subsystems

lore

Share & tools

lore

Andy Shevchenko

author reviewer

2022-10-04 09:22 UTC #0 lore raw

v3 cover device property: Consitify a few APIs and

The property.h has inconsistency in how we annotate the parameters which are not modified anyhow by the certain APIs. Also dev_fwnode() needs to be rectified in sense of the handling const qualifier.

This series improves the above with only a couple of APIs left for now untouched (PHY, which I believe doesn't belong to property.h to begin with).

Changelog v3:
- used _Generic() to hide the _const API (Sakari, Greg)

Changelog v2:
- fixed USB Type-C compilation issues (LKP)
- added tags (Sakari, Heikki)

Andy Shevchenko (5):
device property: Allow const parameter to dev_fwnode() device property: Constify fwnode connection match APIs device property: Constify parameter in fwnode_graph_is_endpoint() device property: Constify device child node APIs device property: Constify parameter in device_dma_supported() and
device_get_dma_attr()

 drivers/base/property.c     | 29 ++++++++++++++++++-----------
 drivers/usb/roles/class.c   |  2 +-
 drivers/usb/typec/mux.c     |  8 ++++----
 drivers/usb/typec/retimer.c |  2 +-
 include/linux/property.h    | 34 +++++++++++++++++++---------------
 5 files changed, 43 insertions(+), 32 deletions(-)

Signature · 1 line

2.35.1

Andy Shevchenko

author reviewer

2022-10-04 09:22 UTC #1 lore raw

PATCH v3 1/5 device property: Allow const parameter to dev_fwnode()

It's not fully correct to take a const parameter pointer to a struct and return a non-const pointer to a member of that struct.

Instead, introduce a const version of the dev_fwnode() API which takes and returns const pointers and use it where it's applicable.

With this, convert dev_fwnode() to be a macro wrapper on top of const and non-const APIs that chooses one based on the type.

Fixes aade55c86033 ("device property: Add const qualifier to device_get_match_data() parameter")

Signed-off-by Andy Shevchenko <[email protected]>

Acked-by Heikki Krogerus <[email protected]>

Reviewed-by Sakari Ailus <[email protected]>

---
 drivers/base/property.c  | 11 +++++++++--
 include/linux/property.h |  7 ++++++-
 2 files changed, 15 insertions(+), 3 deletions(-)

drivers/base/property.c +9 -2

index 4d6278a84868..d77302d28566 100644
@@ -17,12 +17,19 @@
 #include <linux/property.h>
 #include <linux/phy.h>
 
-struct fwnode_handle *dev_fwnode(const struct device *dev)
+struct fwnode_handle *__dev_fwnode(struct device *dev)
 {
 	return IS_ENABLED(CONFIG_OF) && dev->of_node ?
 		of_fwnode_handle(dev->of_node) : dev->fwnode;
 }
-EXPORT_SYMBOL_GPL(dev_fwnode);
+EXPORT_SYMBOL_GPL(__dev_fwnode);
+
+const struct fwnode_handle *__dev_fwnode_const(const struct device *dev)
+{
+	return IS_ENABLED(CONFIG_OF) && dev->of_node ?
+		of_fwnode_handle(dev->of_node) : dev->fwnode;
+}
+EXPORT_SYMBOL_GPL(__dev_fwnode_const);
 
 /**
  * device_property_present - check if a property of a device is present

include/linux/property.h +6 -1

index 117cc200c656..587b5b666b5b 100644
@@ -32,7 +32,12 @@ enum dev_dma_attr {
 	DEV_DMA_COHERENT,
 };
 
-struct fwnode_handle *dev_fwnode(const struct device *dev);
+const struct fwnode_handle *__dev_fwnode_const(const struct device *dev);
+struct fwnode_handle *__dev_fwnode(struct device *dev);
+#define dev_fwnode(dev)							\
+	_Generic((dev),							\
+		 const struct device *: __dev_fwnode_const,	\
+		 struct device *: __dev_fwnode)(dev)
 
 bool device_property_present(struct device *dev, const char *propname);
 int device_property_read_u8_array(struct device *dev, const char *propname,

Signature · 1 line

2.35.1

Andy Shevchenko

author reviewer

2022-10-04 09:39 UTC #2 lore raw

Re: [PATCH v3 1/5] device property: Allow const parameter to dev_fwnode()

Quoted 8 lines · Andy Shevchenko

It's not fully correct to take a const parameter pointer to a struct and return a non-const pointer to a member of that struct.
Instead, introduce a const version of the dev_fwnode() API which takes and returns const pointers and use it where it's applicable.
With this, convert dev_fwnode() to be a macro wrapper on top of const and non-const APIs that chooses one based on the type.

Hmm... it missed the device_get_match_data() implementation (reverse) change somehow. And it still compiles to me, probably I rebased wrongly and the hunk went to another patch. I'll investigate and resend as v4 the fixed version.

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Andy Shevchenko

author reviewer

2022-10-04 09:41 UTC #3 lore raw

Re: [PATCH v3 1/5] device property: Allow const parameter to dev_fwnode()

Quoted 7 lines · Andy Shevchenko

Quoted 6 lines · Andy Shevchenko
It's not fully correct to take a const parameter pointer to a struct and return a non-const pointer to a member of that struct.
Instead, introduce a const version of the dev_fwnode() API which takes and returns const pointers and use it where it's applicable.

Quoted 6 lines

Quoted 2 lines
With this, convert dev_fwnode() to be a macro wrapper on top of const and non-const APIs that chooses one based on the type.
Hmm... it missed the device_get_match_data() implementation (reverse) change somehow. And it still compiles to me, probably I rebased wrongly and the hunk went to another patch. I'll investigate and resend as v4 the fixed version.

Ah, sorry, I mixed this with previous version of the patch here, so everything is fine in v3!

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Andy Shevchenko

author reviewer

2022-10-04 09:22 UTC #4 lore raw

PATCH v3 2/5 device property: Constify fwnode connection match APIs

In reply to Andy Shevchenko

The fwnode and device parameters are not altered in the fwnode connection match APIs, constify them.

Signed-off-by Andy Shevchenko <[email protected]>

Acked-by Heikki Krogerus <[email protected]>

Reviewed-by Sakari Ailus <[email protected]>

---
 drivers/base/property.c     | 8 ++++----
 drivers/usb/roles/class.c   | 2 +-
 drivers/usb/typec/mux.c     | 8 ++++----
 drivers/usb/typec/retimer.c | 2 +-
 include/linux/property.h    | 8 ++++----
 5 files changed, 14 insertions(+), 14 deletions(-)

drivers/base/property.c +4 -4

index d77302d28566..58b8158add5c 100644
@@ -1213,7 +1213,7 @@ const void *device_get_match_data(const struct device *dev)
 }
 EXPORT_SYMBOL_GPL(device_get_match_data);
 
-static unsigned int fwnode_graph_devcon_matches(struct fwnode_handle *fwnode,
+static unsigned int fwnode_graph_devcon_matches(const struct fwnode_handle *fwnode,
 						const char *con_id, void *data,
 						devcon_match_fn_t match,
 						void **matches,

@@ -1247,7 +1247,7 @@ static unsigned int fwnode_graph_devcon_matches(struct fwnode_handle *fwnode,
 	return count;
 }
 
-static unsigned int fwnode_devcon_matches(struct fwnode_handle *fwnode,
+static unsigned int fwnode_devcon_matches(const struct fwnode_handle *fwnode,
 					  const char *con_id, void *data,
 					  devcon_match_fn_t match,
 					  void **matches,

@@ -1289,7 +1289,7 @@ static unsigned int fwnode_devcon_matches(struct fwnode_handle *fwnode,
  * device node. @match will be used to convert the connection description to
  * data the caller is expecting to be returned.
  */
-void *fwnode_connection_find_match(struct fwnode_handle *fwnode,
+void *fwnode_connection_find_match(const struct fwnode_handle *fwnode,
 				   const char *con_id, void *data,
 				   devcon_match_fn_t match)
 {

@@ -1326,7 +1326,7 @@ EXPORT_SYMBOL_GPL(fwnode_connection_find_match);
  *
  * Return: Number of matches resolved, or negative errno.
  */
-int fwnode_connection_find_matches(struct fwnode_handle *fwnode,
+int fwnode_connection_find_matches(const struct fwnode_handle *fwnode,
 				   const char *con_id, void *data,
 				   devcon_match_fn_t match,
 				   void **matches, unsigned int matches_len)

drivers/usb/roles/class.c +1 -1

index dfaed7eee94f..a3575a5a18ce 100644
@@ -87,7 +87,7 @@ enum usb_role usb_role_switch_get_role(struct usb_role_switch *sw)
 }
 EXPORT_SYMBOL_GPL(usb_role_switch_get_role);
 
-static void *usb_role_switch_match(struct fwnode_handle *fwnode, const char *id,
+static void *usb_role_switch_match(const struct fwnode_handle *fwnode, const char *id,
 				   void *data)
 {
 	struct device *dev;

drivers/usb/typec/mux.c +4 -4

index 464330776cd6..f81ea26ab389 100644
@@ -32,8 +32,8 @@ static int switch_fwnode_match(struct device *dev, const void *fwnode)
 	return dev_fwnode(dev) == fwnode;
 }
 
-static void *typec_switch_match(struct fwnode_handle *fwnode, const char *id,
-				void *data)
+static void *typec_switch_match(const struct fwnode_handle *fwnode,
+				const char *id, void *data)
 {
 	struct device *dev;

@@ -262,8 +262,8 @@ static int mux_fwnode_match(struct device *dev, const void *fwnode)
 	return dev_fwnode(dev) == fwnode;
 }
 
-static void *typec_mux_match(struct fwnode_handle *fwnode, const char *id,
-			     void *data)
+static void *typec_mux_match(const struct fwnode_handle *fwnode,
+			     const char *id, void *data)
 {
 	const struct typec_altmode_desc *desc = data;
 	struct device *dev;

drivers/usb/typec/retimer.c +1 -1

index 2003731f1bee..8edfdc709a28 100644
@@ -34,7 +34,7 @@ static int retimer_fwnode_match(struct device *dev, const void *fwnode)
 	return dev_fwnode(dev) == fwnode && dev_name_ends_with(dev, "-retimer");
 }
 
-static void *typec_retimer_match(struct fwnode_handle *fwnode, const char *id, void *data)
+static void *typec_retimer_match(const struct fwnode_handle *fwnode, const char *id, void *data)
 {
 	struct device *dev;

include/linux/property.h +4 -4

index 587b5b666b5b..8d82775a901a 100644
@@ -442,21 +442,21 @@ unsigned int fwnode_graph_get_endpoint_count(struct fwnode_handle *fwnode,
 int fwnode_graph_parse_endpoint(const struct fwnode_handle *fwnode,
 				struct fwnode_endpoint *endpoint);
 
-typedef void *(*devcon_match_fn_t)(struct fwnode_handle *fwnode, const char *id,
+typedef void *(*devcon_match_fn_t)(const struct fwnode_handle *fwnode, const char *id,
 				   void *data);
 
-void *fwnode_connection_find_match(struct fwnode_handle *fwnode,
+void *fwnode_connection_find_match(const struct fwnode_handle *fwnode,
 				   const char *con_id, void *data,
 				   devcon_match_fn_t match);
 
-static inline void *device_connection_find_match(struct device *dev,
+static inline void *device_connection_find_match(const struct device *dev,
 						 const char *con_id, void *data,
 						 devcon_match_fn_t match)
 {
 	return fwnode_connection_find_match(dev_fwnode(dev), con_id, data, match);
 }
 
-int fwnode_connection_find_matches(struct fwnode_handle *fwnode,
+int fwnode_connection_find_matches(const struct fwnode_handle *fwnode,
 				   const char *con_id, void *data,
 				   devcon_match_fn_t match,
 				   void **matches, unsigned int matches_len);

Signature · 1 line

2.35.1

Andy Shevchenko

author reviewer

2022-10-04 09:22 UTC #5 lore raw

PATCH v3 3/5 device property: Constify parameter in fwnode_graph_is_endpoint()

In reply to Andy Shevchenko

Constify parameter in fwnode_graph_is_endpoint() since it doesn't alter anything related to it.

Signed-off-by Andy Shevchenko <[email protected]>

Acked-by Heikki Krogerus <[email protected]>

Reviewed-by Sakari Ailus <[email protected]>

---
 include/linux/property.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

include/linux/property.h +1 -1

index 8d82775a901a..7abb1792044f 100644
@@ -410,7 +410,7 @@ struct fwnode_handle *fwnode_graph_get_remote_port(
 struct fwnode_handle *fwnode_graph_get_remote_endpoint(
 	const struct fwnode_handle *fwnode);
 
-static inline bool fwnode_graph_is_endpoint(struct fwnode_handle *fwnode)
+static inline bool fwnode_graph_is_endpoint(const struct fwnode_handle *fwnode)
 {
 	return fwnode_property_present(fwnode, "remote-endpoint");
 }

Signature · 1 line

2.35.1

Andy Shevchenko

author reviewer

2022-10-04 09:22 UTC #6 lore raw

PATCH v3 4/5 device property: Constify device child node APIs

In reply to Andy Shevchenko

The device parameter is not altered in the device child node APIs, constify them.

Signed-off-by Andy Shevchenko <[email protected]>

Acked-by Heikki Krogerus <[email protected]>

Reviewed-by Sakari Ailus <[email protected]>

---
 drivers/base/property.c  |  6 +++---
 include/linux/property.h | 12 ++++++------
 2 files changed, 9 insertions(+), 9 deletions(-)

drivers/base/property.c +3 -3

index 58b8158add5c..d3ea5f82978f 100644
@@ -763,7 +763,7 @@ EXPORT_SYMBOL_GPL(fwnode_get_next_available_child_node);
  * @dev: Device to find the next child node for.
  * @child: Handle to one of the device's child nodes or a null handle.
  */
-struct fwnode_handle *device_get_next_child_node(struct device *dev,
+struct fwnode_handle *device_get_next_child_node(const struct device *dev,
 						 struct fwnode_handle *child)
 {
 	const struct fwnode_handle *fwnode = dev_fwnode(dev);

@@ -800,7 +800,7 @@ EXPORT_SYMBOL_GPL(fwnode_get_named_child_node);
  * @dev: Device to find the named child node for.
  * @childname: String to match child node name against.
  */
-struct fwnode_handle *device_get_named_child_node(struct device *dev,
+struct fwnode_handle *device_get_named_child_node(const struct device *dev,
 						  const char *childname)
 {
 	return fwnode_get_named_child_node(dev_fwnode(dev), childname);

@@ -859,7 +859,7 @@ EXPORT_SYMBOL_GPL(fwnode_device_is_available);
  * device_get_child_node_count - return the number of child nodes for device
  * @dev: Device to cound the child nodes for
  */
-unsigned int device_get_child_node_count(struct device *dev)
+unsigned int device_get_child_node_count(const struct device *dev)
 {
 	struct fwnode_handle *child;
 	unsigned int count = 0;

include/linux/property.h +6 -6

index 7abb1792044f..472689e53ade 100644
@@ -114,16 +114,16 @@ struct fwnode_handle *fwnode_get_next_available_child_node(
 	for (child = fwnode_get_next_available_child_node(fwnode, NULL); child;\
 	     child = fwnode_get_next_available_child_node(fwnode, child))
 
-struct fwnode_handle *device_get_next_child_node(
-	struct device *dev, struct fwnode_handle *child);
+struct fwnode_handle *device_get_next_child_node(const struct device *dev,
+						 struct fwnode_handle *child);
 
 #define device_for_each_child_node(dev, child)				\
 	for (child = device_get_next_child_node(dev, NULL); child;	\
 	     child = device_get_next_child_node(dev, child))
 
-struct fwnode_handle *fwnode_get_named_child_node(
-	const struct fwnode_handle *fwnode, const char *childname);
-struct fwnode_handle *device_get_named_child_node(struct device *dev,
+struct fwnode_handle *fwnode_get_named_child_node(const struct fwnode_handle *fwnode,
+						  const char *childname);
+struct fwnode_handle *device_get_named_child_node(const struct device *dev,
 						  const char *childname);
 
 struct fwnode_handle *fwnode_handle_get(struct fwnode_handle *fwnode);

@@ -132,7 +132,7 @@ void fwnode_handle_put(struct fwnode_handle *fwnode);
 int fwnode_irq_get(const struct fwnode_handle *fwnode, unsigned int index);
 int fwnode_irq_get_byname(const struct fwnode_handle *fwnode, const char *name);
 
-unsigned int device_get_child_node_count(struct device *dev);
+unsigned int device_get_child_node_count(const struct device *dev);
 
 static inline bool device_property_read_bool(struct device *dev,
 					     const char *propname)

Signature · 1 line

2.35.1

Andy Shevchenko

author reviewer

2022-10-04 09:22 UTC #7 lore raw

PATCH v3 5/5 device property: Constify parameter in device_dma_supported() and device_get_dma_attr()

In reply to Andy Shevchenko

Constify parameter in device_dma_supported() and device_get_dma_attr() since they don't alter anything related to it.

Signed-off-by Andy Shevchenko <[email protected]>

Acked-by Heikki Krogerus <[email protected]>

Reviewed-by Sakari Ailus <[email protected]>

---
 drivers/base/property.c  | 4 ++--
 include/linux/property.h | 5 ++---
 2 files changed, 4 insertions(+), 5 deletions(-)

drivers/base/property.c +2 -2

index d3ea5f82978f..68f61d3e3857 100644
@@ -871,13 +871,13 @@ unsigned int device_get_child_node_count(const struct device *dev)
 }
 EXPORT_SYMBOL_GPL(device_get_child_node_count);
 
-bool device_dma_supported(struct device *dev)
+bool device_dma_supported(const struct device *dev)
 {
 	return fwnode_call_bool_op(dev_fwnode(dev), device_dma_supported);
 }
 EXPORT_SYMBOL_GPL(device_dma_supported);
 
-enum dev_dma_attr device_get_dma_attr(struct device *dev)
+enum dev_dma_attr device_get_dma_attr(const struct device *dev)
 {
 	if (!fwnode_has_op(dev_fwnode(dev), device_get_dma_attr))
 		return DEV_DMA_NOT_SUPPORTED;

include/linux/property.h +2 -3

index 472689e53ade..83674f968a8f 100644
@@ -388,9 +388,8 @@ property_entries_dup(const struct property_entry *properties);
 
 void property_entries_free(const struct property_entry *properties);
 
-bool device_dma_supported(struct device *dev);
-
-enum dev_dma_attr device_get_dma_attr(struct device *dev);
+bool device_dma_supported(const struct device *dev);
+enum dev_dma_attr device_get_dma_attr(const struct device *dev);
 
 const void *device_get_match_data(const struct device *dev);

Signature · 1 line

2.35.1

Greg Kroah-Hartman

maintainer

2022-10-04 12:11 UTC #8 lore raw

In reply to Andy Shevchenko

Quoted 7 lines · Andy Shevchenko

The property.h has inconsistency in how we annotate the parameters which are not modified anyhow by the certain APIs. Also dev_fwnode() needs to be rectified in sense of the handling const qualifier.
This series improves the above with only a couple of APIs left for now untouched (PHY, which I believe doesn't belong to property.h to begin with).

Looks sane at first glance. I'll look at it some more once 6.1-rc1 is out, thanks.

greg k-h

Greg Kroah-Hartman

maintainer

2022-10-22 11:52 UTC #9 lore raw

Quoted 11 lines · Greg Kroah-Hartman

Quoted 7 lines · Andy Shevchenko
The property.h has inconsistency in how we annotate the parameters which are not modified anyhow by the certain APIs. Also dev_fwnode() needs to be rectified in sense of the handling const qualifier.
This series improves the above with only a couple of APIs left for now untouched (PHY, which I believe doesn't belong to property.h to begin with).
Looks sane at first glance. I'll look at it some more once 6.1-rc1 is out, thanks.

All now applied, thanks.,

greg k-h