2022-03-25 19:03:35

by Clément Léger

[permalink] [raw]
Subject: [PATCH v3 0/9] introduce fwnode in the I2C subsystem

In order to allow the I2C subsystem to be usable with fwnode, add
some functions to retrieve an i2c_adapter from a fwnode and use
these functions in both i2c mux and sfp. ACPI and device-tree are
handled to allow these modifications to work with both descriptions.
I2C mux support has also been modified to support fwnode based
descriptions.

This series is a subset of the one that was first submitted as a larger
series to add swnode support [1]. In this one, it will be focused on
fwnode support only since it seems to have reach a consensus that
adding fwnode to subsystems makes sense.

[1] https://lore.kernel.org/netdev/[email protected]/T/

---------------

Changes in V3:
- Add index parameter to property_read_string_array()
- Implement support for devbice-tree and software nodes
- Restrict index support for ACPI to 0
- Add unittests for of_property_read_string_array_index()
- Add unittests for fwnode_property_read_string_index()

Changes in V2:
- Remove sfp modifications
- Add property_read_string_index fwnode_operation callback
- Implement .property_read_string_index for of and swnode
- Renamed np variable to fwnode

Clément Léger (9):
of: add of_property_read_string_array_index()
of: unittests: add tests for of_property_read_string_array_index()
device property: add index argument to property_read_string_array()
callback
device property: add fwnode_property_read_string_index()
device property: add tests for fwnode_property_read_string_index()
i2c: fwnode: add fwnode_find_i2c_adapter_by_node()
i2c: of: use fwnode_get_i2c_adapter_by_node()
i2c: mux: pinctrl: remove CONFIG_OF dependency and use fwnode API
i2c: mux: add support for fwnode

drivers/acpi/property.c | 5 ++-
drivers/base/property.c | 37 ++++++++++++++++++--
drivers/base/swnode.c | 21 ++++++++----
drivers/base/test/property-entry-test.c | 18 ++++++++++
drivers/i2c/Makefile | 1 +
drivers/i2c/i2c-core-fwnode.c | 45 +++++++++++++++++++++++++
drivers/i2c/i2c-core-of.c | 30 -----------------
drivers/i2c/i2c-mux.c | 39 ++++++++++-----------
drivers/i2c/muxes/Kconfig | 1 -
drivers/i2c/muxes/i2c-mux-pinctrl.c | 23 +++++++------
drivers/of/property.c | 5 +--
drivers/of/unittest.c | 20 +++++++++++
include/linux/fwnode.h | 7 ++--
include/linux/i2c.h | 8 ++++-
include/linux/of.h | 22 ++++++++++++
include/linux/property.h | 3 ++
16 files changed, 207 insertions(+), 78 deletions(-)
create mode 100644 drivers/i2c/i2c-core-fwnode.c

--
2.34.1


2022-03-25 19:14:11

by Clément Léger

[permalink] [raw]
Subject: [PATCH v3 6/9] i2c: fwnode: add fwnode_find_i2c_adapter_by_node()

Add fwnode_find_i2c_adapter_by_node() which allows to retrieve a i2c
adapter using a fwnode. Since dev_fwnode() uses the fwnode provided by
the of_node member of the device, this will also work for devices were
the of_node has been set and not the fwnode field.
For acpi nodes, the check for parent node is skipped since
i2c_acpi_find_adapter_by_handle() does not check it and we don't want
to change this behavior.

Signed-off-by: Clément Léger <[email protected]>
---
drivers/i2c/Makefile | 1 +
drivers/i2c/i2c-core-fwnode.c | 45 +++++++++++++++++++++++++++++++++++
include/linux/i2c.h | 3 +++
3 files changed, 49 insertions(+)
create mode 100644 drivers/i2c/i2c-core-fwnode.c

diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
index c1d493dc9bac..c9c97675163e 100644
--- a/drivers/i2c/Makefile
+++ b/drivers/i2c/Makefile
@@ -6,6 +6,7 @@
obj-$(CONFIG_I2C_BOARDINFO) += i2c-boardinfo.o
obj-$(CONFIG_I2C) += i2c-core.o
i2c-core-objs := i2c-core-base.o i2c-core-smbus.o
+i2c-core-objs += i2c-core-fwnode.o
i2c-core-$(CONFIG_ACPI) += i2c-core-acpi.o
i2c-core-$(CONFIG_I2C_SLAVE) += i2c-core-slave.o
i2c-core-$(CONFIG_OF) += i2c-core-of.o
diff --git a/drivers/i2c/i2c-core-fwnode.c b/drivers/i2c/i2c-core-fwnode.c
new file mode 100644
index 000000000000..fb1c820b686e
--- /dev/null
+++ b/drivers/i2c/i2c-core-fwnode.c
@@ -0,0 +1,45 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Linux I2C core fwnode support code
+ *
+ * Copyright (C) 2022 Microchip
+ */
+
+#include <linux/device.h>
+#include <linux/i2c.h>
+
+#include "i2c-core.h"
+
+static int fwnode_dev_or_parent_node_match(struct device *dev, const void *data)
+{
+ if (device_match_fwnode(dev, data))
+ return 1;
+
+ /*
+ * For ACPI device node, the behavior is to not match the parent (see
+ * i2c_acpi_find_adapter_by_handle())
+ */
+ if (!is_acpi_device_node(dev_fwnode(dev)) && dev->parent)
+ return device_match_fwnode(dev->parent, data);
+
+ return 0;
+}
+
+struct i2c_adapter *
+fwnode_find_i2c_adapter_by_node(struct fwnode_handle *fwnode)
+{
+ struct device *dev;
+ struct i2c_adapter *adapter;
+
+ dev = bus_find_device(&i2c_bus_type, NULL, fwnode,
+ fwnode_dev_or_parent_node_match);
+ if (!dev)
+ return NULL;
+
+ adapter = i2c_verify_adapter(dev);
+ if (!adapter)
+ put_device(dev);
+
+ return adapter;
+}
+EXPORT_SYMBOL(fwnode_find_i2c_adapter_by_node);
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 7d4f52ceb7b5..8dadf8c89fd9 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -967,6 +967,9 @@ int i2c_handle_smbus_host_notify(struct i2c_adapter *adap, unsigned short addr);

#endif /* I2C */

+struct i2c_adapter *
+fwnode_find_i2c_adapter_by_node(struct fwnode_handle *fwnode);
+
#if IS_ENABLED(CONFIG_OF)
/* must call put_device() when done with returned i2c_client device */
struct i2c_client *of_find_i2c_device_by_node(struct device_node *node);
--
2.34.1

2022-03-25 19:17:26

by Clément Léger

[permalink] [raw]
Subject: [PATCH v3 7/9] i2c: of: use fwnode_get_i2c_adapter_by_node()

Since the new fwnode based fwnode_find_i2c_adapter_by_node() function
has the same behavior than of_get_i2c_adapter_by_node(), call it to
avoid code duplication.

Signed-off-by: Clément Léger <[email protected]>
---
drivers/i2c/i2c-core-of.c | 30 ------------------------------
include/linux/i2c.h | 5 ++++-
2 files changed, 4 insertions(+), 31 deletions(-)

diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c
index 3ed74aa4b44b..be7d66aa0f49 100644
--- a/drivers/i2c/i2c-core-of.c
+++ b/drivers/i2c/i2c-core-of.c
@@ -113,17 +113,6 @@ void of_i2c_register_devices(struct i2c_adapter *adap)
of_node_put(bus);
}

-static int of_dev_or_parent_node_match(struct device *dev, const void *data)
-{
- if (dev->of_node == data)
- return 1;
-
- if (dev->parent)
- return dev->parent->of_node == data;
-
- return 0;
-}
-
/* must call put_device() when done with returned i2c_client device */
struct i2c_client *of_find_i2c_device_by_node(struct device_node *node)
{
@@ -142,25 +131,6 @@ struct i2c_client *of_find_i2c_device_by_node(struct device_node *node)
}
EXPORT_SYMBOL(of_find_i2c_device_by_node);

-/* must call put_device() when done with returned i2c_adapter device */
-struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node)
-{
- struct device *dev;
- struct i2c_adapter *adapter;
-
- dev = bus_find_device(&i2c_bus_type, NULL, node,
- of_dev_or_parent_node_match);
- if (!dev)
- return NULL;
-
- adapter = i2c_verify_adapter(dev);
- if (!adapter)
- put_device(dev);
-
- return adapter;
-}
-EXPORT_SYMBOL(of_find_i2c_adapter_by_node);
-
/* must call i2c_put_adapter() when done with returned i2c_adapter device */
struct i2c_adapter *of_get_i2c_adapter_by_node(struct device_node *node)
{
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 8dadf8c89fd9..982918fd0093 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -975,7 +975,10 @@ fwnode_find_i2c_adapter_by_node(struct fwnode_handle *fwnode);
struct i2c_client *of_find_i2c_device_by_node(struct device_node *node);

/* must call put_device() when done with returned i2c_adapter device */
-struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node);
+static inline struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node)
+{
+ return fwnode_find_i2c_adapter_by_node(of_fwnode_handle(node));
+}

/* must call i2c_put_adapter() when done with returned i2c_adapter device */
struct i2c_adapter *of_get_i2c_adapter_by_node(struct device_node *node);
--
2.34.1

2022-03-25 19:24:05

by Clément Léger

[permalink] [raw]
Subject: [PATCH v3 5/9] device property: add tests for fwnode_property_read_string_index()

Add somes tests to validate fwnode_property_read_string_index().

Signed-off-by: Clément Léger <[email protected]>
---
drivers/base/test/property-entry-test.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/drivers/base/test/property-entry-test.c b/drivers/base/test/property-entry-test.c
index 6071d5bc128c..9edbaa53a950 100644
--- a/drivers/base/test/property-entry-test.c
+++ b/drivers/base/test/property-entry-test.c
@@ -318,6 +318,24 @@ static void pe_test_strings(struct kunit *test)
KUNIT_EXPECT_EQ(test, error, 0);
KUNIT_EXPECT_STREQ(test, str, "string-a");

+ error = fwnode_property_read_string_index(node, "str", 0, strs);
+ KUNIT_EXPECT_EQ(test, error, 0);
+ KUNIT_EXPECT_STREQ(test, strs[0], "single");
+
+ error = fwnode_property_read_string_index(node, "strs", 0, strs);
+ KUNIT_EXPECT_EQ(test, error, 0);
+ KUNIT_EXPECT_STREQ(test, strs[0], "string-a");
+
+ error = fwnode_property_read_string_index(node, "strs", 1, strs);
+ KUNIT_EXPECT_EQ(test, error, 0);
+ KUNIT_EXPECT_STREQ(test, strs[0], "string-b");
+
+ error = fwnode_property_read_string_index(node, "str", 1, strs);
+ KUNIT_EXPECT_EQ(test, error, -ENODATA);
+
+ error = fwnode_property_read_string_index(node, "strs", 3, strs);
+ KUNIT_EXPECT_EQ(test, error, -ENODATA);
+
fwnode_remove_software_node(node);
}

--
2.34.1

2022-03-25 19:32:53

by Clément Léger

[permalink] [raw]
Subject: [PATCH v3 1/9] of: add of_property_read_string_array_index()

Add of_property_read_string_array_index() which allows to read a string
array starting at a specific index.

Signed-off-by: Clément Léger <[email protected]>
---
include/linux/of.h | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)

diff --git a/include/linux/of.h b/include/linux/of.h
index 2dc77430a91a..93f04c530bd1 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -1115,6 +1115,28 @@ static inline int of_property_read_string_array(const struct device_node *np,
return of_property_read_string_helper(np, propname, out_strs, sz, 0);
}

+/**
+ * of_property_read_string_array_index() - Read an array of strings from a
+ * multiple strings property starting at a specified index
+ * @np: device node from which the property value is to be read.
+ * @propname: name of the property to be searched.
+ * @out_strs: output array of string pointers.
+ * @sz: number of array elements to read.
+ * @index: index to start reading from
+ *
+ * Search for a property in a device tree node and retrieve a list of
+ * terminated string values (pointer to data, not a copy) in that property
+ * starting at specified index.
+ *
+ * Return: If @out_strs is NULL, the number of strings in the property is returned.
+ */
+static inline int of_property_read_string_array_index(const struct device_node *np,
+ const char *propname,
+ const char **out_strs,
+ size_t sz, int index)
+{
+ return of_property_read_string_helper(np, propname, out_strs, sz, index);
+}
/**
* of_property_count_strings() - Find and return the number of strings from a
* multiple strings property.
--
2.34.1

2022-03-25 19:54:04

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v3 6/9] i2c: fwnode: add fwnode_find_i2c_adapter_by_node()

On Fri, Mar 25, 2022 at 12:31:45PM +0100, Cl?ment L?ger wrote:
> Add fwnode_find_i2c_adapter_by_node() which allows to retrieve a i2c
> adapter using a fwnode. Since dev_fwnode() uses the fwnode provided by
> the of_node member of the device, this will also work for devices were
> the of_node has been set and not the fwnode field.
> For acpi nodes, the check for parent node is skipped since
> i2c_acpi_find_adapter_by_handle() does not check it and we don't want
> to change this behavior.

...

> +#include <linux/device.h>
> +#include <linux/i2c.h>

Missed headers so far:
acpi.h

...

> +static int fwnode_dev_or_parent_node_match(struct device *dev, const void *data)
> +{
> + if (device_match_fwnode(dev, data))
> + return 1;
> +
> + /*
> + * For ACPI device node, the behavior is to not match the parent (see
> + * i2c_acpi_find_adapter_by_handle())
> + */

Would it be harmful to drop this check?

> + if (!is_acpi_device_node(dev_fwnode(dev)) && dev->parent)
> + return device_match_fwnode(dev->parent, data);
> +
> + return 0;
> +}

--
With Best Regards,
Andy Shevchenko


2022-03-30 09:46:30

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v3 1/9] of: add of_property_read_string_array_index()

On Fri, 25 Mar 2022 12:31:40 +0100, Cl?ment L?ger wrote:
> Add of_property_read_string_array_index() which allows to read a string
> array starting at a specific index.
>
> Signed-off-by: Cl?ment L?ger <[email protected]>
> ---
> include/linux/of.h | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>

Reviewed-by: Rob Herring <[email protected]>

2022-05-16 16:07:57

by Peter Rosin

[permalink] [raw]
Subject: Re: [PATCH v3 0/9] introduce fwnode in the I2C subsystem

2022-05-14 at 16:47, Wolfram Sang wrote:
> O
>> This series is a subset of the one that was first submitted as a larger
>> series to add swnode support [1]. In this one, it will be focused on
>> fwnode support only since it seems to have reach a consensus that
>> adding fwnode to subsystems makes sense.
>
> From a high level view, I like this series. Though, it will need Peter's
> ack on the I2C mux patches as he is the I2C mux maintainer. Still, I
> wonder about the way to upstream the series. Feels like the first 5
> patches should not go via I2C but seperately?

Hi Wolfram,

I also think it looks basically sane. However, there are a couple of
comments plus promises to adjust accordingly. I guess I filed it under
"wait for the next iteration"...

Cheers,
Peter

2022-05-16 18:23:47

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH v3 0/9] introduce fwnode in the I2C subsystem

O
> This series is a subset of the one that was first submitted as a larger
> series to add swnode support [1]. In this one, it will be focused on
> fwnode support only since it seems to have reach a consensus that
> adding fwnode to subsystems makes sense.

From a high level view, I like this series. Though, it will need Peter's
ack on the I2C mux patches as he is the I2C mux maintainer. Still, I
wonder about the way to upstream the series. Feels like the first 5
patches should not go via I2C but seperately?


Attachments:
(No filename) (527.00 B)
signature.asc (849.00 B)
Download all attachments

2022-05-16 22:58:40

by Clément Léger

[permalink] [raw]
Subject: Re: [PATCH v3 0/9] introduce fwnode in the I2C subsystem

Le Sun, 15 May 2022 23:34:16 +0200,
Peter Rosin <[email protected]> a écrit :

> 2022-05-14 at 16:47, Wolfram Sang wrote:
> > O
> >> This series is a subset of the one that was first submitted as a larger
> >> series to add swnode support [1]. In this one, it will be focused on
> >> fwnode support only since it seems to have reach a consensus that
> >> adding fwnode to subsystems makes sense.
> >
> > From a high level view, I like this series. Though, it will need Peter's
> > ack on the I2C mux patches as he is the I2C mux maintainer. Still, I
> > wonder about the way to upstream the series. Feels like the first 5
> > patches should not go via I2C but seperately?
>
> Hi Wolfram,
>
> I also think it looks basically sane. However, there are a couple of
> comments plus promises to adjust accordingly. I guess I filed it under
> "wait for the next iteration"...
>
> Cheers,
> Peter

Hi Wolfram & Peter,

While doing the same conversion on the reset subsystem, Rob Herring
stepped in and mention the fact that this could be done using
device-tree overlay (even on system with ACPI) .

The result was that a new serie [1] which add support to create the PCI
devices of_node if not existing, and then allow drivers to applies an
overlay which describe the tree of devices as a child of a specific PCI
device of_node. There are a lot of advantages to this approach (small
patchset working for all susbystems, easier to use, description is
using already existing device-tree). There are still some concerns
about the viability of dynamic overlay but this might be settled soon.

Regards,

[1] https://lore.kernel.org/all/20220509141634.16158c38@xps-bootlin/T/

--
Clément Léger,
Embedded Linux and Kernel engineer at Bootlin
https://bootlin.com

2022-05-17 03:29:16

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH v3 0/9] introduce fwnode in the I2C subsystem


> While doing the same conversion on the reset subsystem, Rob Herring
> stepped in and mention the fact that this could be done using
> device-tree overlay (even on system with ACPI) .

Thanks for the heads up, I'll drop this series then.


Attachments:
(No filename) (248.00 B)
signature.asc (849.00 B)
Download all attachments