diff mbox series

[RFC,09/10] i2c: mux: add support for fwnode

Message ID 20220221162652.103834-10-clement.leger@bootlin.com (mailing list archive)
State RFC, archived
Headers show
Series add support for fwnode in i2c mux system and sfp | expand

Commit Message

Clément Léger Feb. 21, 2022, 4:26 p.m. UTC
Modify i2c_mux_add_adapter() to use with fwnode API to allow creating
mux adapters with fwnode based devices. This allows to have a node
independent support for i2c muxes.

Signed-off-by: Clément Léger <clement.leger@bootlin.com>
---
 drivers/i2c/i2c-mux.c | 39 ++++++++++++++++++---------------------
 1 file changed, 18 insertions(+), 21 deletions(-)

Comments

Andy Shevchenko Feb. 21, 2022, 5:55 p.m. UTC | #1
On Mon, Feb 21, 2022 at 05:26:51PM +0100, Clément Léger wrote:
> Modify i2c_mux_add_adapter() to use with fwnode API to allow creating
> mux adapters with fwnode based devices. This allows to have a node
> independent support for i2c muxes.

I^2C muxes have their own description for DT and ACPI platforms, I'm not sure
swnode should be used here at all. Just upload a corresponding SSDT overlay or
DT overlay depending on the platform. Can it be achieved?
Clément Léger Feb. 22, 2022, 8:53 a.m. UTC | #2
Le Mon, 21 Feb 2022 19:55:25 +0200,
Andy Shevchenko <andriy.shevchenko@linux.intel.com> a écrit :

> On Mon, Feb 21, 2022 at 05:26:51PM +0100, Clément Léger wrote:
> > Modify i2c_mux_add_adapter() to use with fwnode API to allow creating
> > mux adapters with fwnode based devices. This allows to have a node
> > independent support for i2c muxes.  
> 
> I^2C muxes have their own description for DT and ACPI platforms, I'm not sure
> swnode should be used here at all. Just upload a corresponding SSDT overlay or
> DT overlay depending on the platform. Can it be achieved?
> 

Problem is that this PCIe card can be plugged either in a X86 platform
using ACPI or on a ARM one with device-tree. So it means I should have
two "identical" descriptions for each platforms.
Andrew Lunn Feb. 22, 2022, 10:57 a.m. UTC | #3
On Tue, Feb 22, 2022 at 09:53:25AM +0100, Clément Léger wrote:
> Le Mon, 21 Feb 2022 19:55:25 +0200,
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> a écrit :
> 
> > On Mon, Feb 21, 2022 at 05:26:51PM +0100, Clément Léger wrote:
> > > Modify i2c_mux_add_adapter() to use with fwnode API to allow creating
> > > mux adapters with fwnode based devices. This allows to have a node
> > > independent support for i2c muxes.  
> > 
> > I^2C muxes have their own description for DT and ACPI platforms, I'm not sure
> > swnode should be used here at all. Just upload a corresponding SSDT overlay or
> > DT overlay depending on the platform. Can it be achieved?
> > 
> 
> Problem is that this PCIe card can be plugged either in a X86 platform
> using ACPI or on a ARM one with device-tree. So it means I should have
> two "identical" descriptions for each platforms.

ACPI != DT.

I know people like stuffing DT properties into ACPI tables, when ACPI
does not have a binding. But in this case, there is a well defined
ACPI mechanism for I2C muxes. You cannot ignore it because it is
different to DT. So you need to handle the muxes in both the ACPI way
and the DT way.

For other parts of what you are doing, you might be able to get away
by just stuffing DT properties into ACPI tables. But that is not for
me to decide, that is up to the ACPI maintainers.

	Andrew
Alexandre Belloni Feb. 22, 2022, 8:31 p.m. UTC | #4
On 22/02/2022 11:57:39+0100, Andrew Lunn wrote:
> On Tue, Feb 22, 2022 at 09:53:25AM +0100, Clément Léger wrote:
> > Le Mon, 21 Feb 2022 19:55:25 +0200,
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> a écrit :
> > 
> > > On Mon, Feb 21, 2022 at 05:26:51PM +0100, Clément Léger wrote:
> > > > Modify i2c_mux_add_adapter() to use with fwnode API to allow creating
> > > > mux adapters with fwnode based devices. This allows to have a node
> > > > independent support for i2c muxes.  
> > > 
> > > I^2C muxes have their own description for DT and ACPI platforms, I'm not sure
> > > swnode should be used here at all. Just upload a corresponding SSDT overlay or
> > > DT overlay depending on the platform. Can it be achieved?
> > > 
> > 
> > Problem is that this PCIe card can be plugged either in a X86 platform
> > using ACPI or on a ARM one with device-tree. So it means I should have
> > two "identical" descriptions for each platforms.
> 
> ACPI != DT.
> 
> I know people like stuffing DT properties into ACPI tables, when ACPI
> does not have a binding. But in this case, there is a well defined
> ACPI mechanism for I2C muxes. You cannot ignore it because it is
> different to DT. So you need to handle the muxes in both the ACPI way
> and the DT way.
> 
> For other parts of what you are doing, you might be able to get away
> by just stuffing DT properties into ACPI tables. But that is not for
> me to decide, that is up to the ACPI maintainers.
> 

What I'm wondering is why you would have to stuff anything in ACPI when
plugging any PCIe card in a PC. Wouldn't that be a first?

I don't have to do so when plugging an Intel network card, I don't
expect to have to do it when plugging any other network card...
diff mbox series

Patch

diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 774507b54b57..93c916220da5 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -24,7 +24,7 @@ 
 #include <linux/i2c-mux.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/of.h>
+#include <linux/property.h>
 #include <linux/slab.h>
 #include <linux/sysfs.h>
 
@@ -347,38 +347,35 @@  int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
 	else
 		priv->adap.class = class;
 
-	/*
-	 * Try to populate the mux adapter's of_node, expands to
-	 * nothing if !CONFIG_OF.
-	 */
-	if (muxc->dev->of_node) {
-		struct device_node *dev_node = muxc->dev->of_node;
-		struct device_node *mux_node, *child = NULL;
+	/* Try to populate the mux adapter's device node */
+	if (dev_fwnode(muxc->dev) && !has_acpi_companion(muxc->dev)) {
+		struct fwnode_handle *dev_node = dev_fwnode(muxc->dev);
+		struct fwnode_handle *mux_node, *child = NULL;
 		u32 reg;
 
 		if (muxc->arbitrator)
-			mux_node = of_get_child_by_name(dev_node, "i2c-arb");
+			mux_node = fwnode_get_named_child_node(dev_node, "i2c-arb");
 		else if (muxc->gate)
-			mux_node = of_get_child_by_name(dev_node, "i2c-gate");
+			mux_node = fwnode_get_named_child_node(dev_node, "i2c-gate");
 		else
-			mux_node = of_get_child_by_name(dev_node, "i2c-mux");
+			mux_node = fwnode_get_named_child_node(dev_node, "i2c-mux");
 
 		if (mux_node) {
 			/* A "reg" property indicates an old-style DT entry */
-			if (!of_property_read_u32(mux_node, "reg", &reg)) {
-				of_node_put(mux_node);
+			if (!fwnode_property_read_u32(mux_node, "reg", &reg)) {
+				fwnode_handle_put(mux_node);
 				mux_node = NULL;
 			}
 		}
 
 		if (!mux_node)
-			mux_node = of_node_get(dev_node);
+			mux_node = fwnode_handle_get(dev_node);
 		else if (muxc->arbitrator || muxc->gate)
-			child = of_node_get(mux_node);
+			child = fwnode_handle_get(mux_node);
 
 		if (!child) {
-			for_each_child_of_node(mux_node, child) {
-				ret = of_property_read_u32(child, "reg", &reg);
+			fwnode_for_each_child_node(mux_node, child) {
+				ret = fwnode_property_read_u32(child, "reg", &reg);
 				if (ret)
 					continue;
 				if (chan_id == reg)
@@ -386,8 +383,8 @@  int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
 			}
 		}
 
-		priv->adap.dev.of_node = child;
-		of_node_put(mux_node);
+		device_set_node(&priv->adap.dev, child);
+		fwnode_handle_put(mux_node);
 	}
 
 	/*
@@ -444,7 +441,7 @@  void i2c_mux_del_adapters(struct i2c_mux_core *muxc)
 	while (muxc->num_adapters) {
 		struct i2c_adapter *adap = muxc->adapter[--muxc->num_adapters];
 		struct i2c_mux_priv *priv = adap->algo_data;
-		struct device_node *np = adap->dev.of_node;
+		struct fwnode_handle *np = dev_fwnode(&adap->dev);
 
 		muxc->adapter[muxc->num_adapters] = NULL;
 
@@ -454,7 +451,7 @@  void i2c_mux_del_adapters(struct i2c_mux_core *muxc)
 
 		sysfs_remove_link(&priv->adap.dev.kobj, "mux_device");
 		i2c_del_adapter(adap);
-		of_node_put(np);
+		fwnode_handle_put(np);
 		kfree(priv);
 	}
 }