Hi Peter,
On 29.04.24 17:46, Peter Rosin wrote:
Hi!
2024-04-26 at 18:49, Farouk Bouabid wrote:
Allow the mux to have the same address as a child device. This is useful
when the mux can only use an i2c-address that is used by a child device
because no other addresses are free to use. eg. the mux can only use
address 0x18 which is used by amc6821 connected to the mux.
Signed-off-by: Farouk Bouabid <farouk.bouabid@xxxxxxxxxxxxxxxxxxxxx>
---
drivers/i2c/i2c-mux.c | 10 +++++++++-
include/linux/i2c-mux.h | 1 +
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 57ff09f18c37..f5357dff8cc5 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -331,7 +331,6 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
priv->adap.owner = THIS_MODULE;
priv->adap.algo = &priv->algo;
priv->adap.algo_data = priv;
- priv->adap.dev.parent = &parent->dev;
priv->adap.retries = parent->retries;
priv->adap.timeout = parent->timeout;
priv->adap.quirks = parent->quirks;
@@ -348,6 +347,15 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
else
priv->adap.class = class;
+ /*
+ * When creating the adapter, the node devices are checked for i2c address
+ * match with other devices on the parent adapter, among which is the mux itself.
+ * If a match is found the node device is not probed successfully.
+ * Allow the mux to have the same address as a child device by skipping this check.
+ */
+ if (!(muxc->share_addr_with_children))
+ priv->adap.dev.parent = &parent->dev;
This is a dirty hack that will not generally do the right thing.
The adapter device parent is not there solely for the purpose of
detecting address clashes, so the above has other implications
that are not desirable.
Therefore, NACK on this approach. It simply needs to be more involved.
Sorry.
Cheers,
Peter
Another way to approach this is by implementing this flag as a quirk for
the added adapter:
(tested but not cleaned up)
"""
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index ff5c486a1dbb..6a0237f750db 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -821,9 +821,21 @@ static int i2c_check_mux_children(struct device
*dev, void *addrp)
static int i2c_check_addr_busy(struct i2c_adapter *adapter, int addr)
{
struct i2c_adapter *parent = i2c_parent_is_i2c_adapter(adapter);
+ bool skip_check = false;
int result = 0;
- if (parent)
+ if (adapter->quirks) {
+ if (adapter->quirks->flags & I2C_AQ_SHARE_ADDR) {
+ struct i2c_client *client =
of_find_i2c_device_by_node(adapter->dev.of_node->parent);
+
+ if (client) {
+ skip_check = client->addr == addr;
+ put_device(&client->dev);
+ }
+ }
+ }
+
+ if (parent && !skip_check)
result = i2c_check_mux_parents(parent, addr);
if (!result)
diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 57ff09f18c37..e87cb0e43725 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -334,7 +334,26 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
priv->adap.dev.parent = &parent->dev;
priv->adap.retries = parent->retries;
priv->adap.timeout = parent->timeout;
- priv->adap.quirks = parent->quirks;
+ /*
+ * When creating the adapter, the node devices are checked for
i2c address
+ * match with other devices on the parent adapter, among which
is the mux itself.
+ * If a match is found the node device is not probed successfully.
+ * Allow the mux to have the same address as a child device by
skipping this check.
+ */
+ if (!muxc->share_addr_with_children)
+ priv->adap.quirks = parent->quirks;
+ else {
+ struct i2c_adapter_quirks *quirks =
kzalloc(sizeof(*quirks), GFP_KERNEL);
+ if (!quirks)
+ return -ENOMEM;
+
+ if (parent->quirks)
+ *quirks = *(parent->quirks); // @fixme memcpy
+
+ quirks->flags |= I2C_AQ_SHARE_ADDR;
+ priv->adap.quirks = quirks;
+ }
+
if (muxc->mux_locked)
priv->adap.lock_ops = &i2c_mux_lock_ops;
else
diff --git a/include/linux/i2c-mux.h b/include/linux/i2c-mux.h
index 98ef73b7c8fd..17ac68bf1703 100644
--- a/include/linux/i2c-mux.h
+++ b/include/linux/i2c-mux.h
@@ -21,6 +21,7 @@ struct i2c_mux_core {
unsigned int mux_locked:1;
unsigned int arbitrator:1;
unsigned int gate:1;
+ unsigned int share_addr_with_children:1;
void *priv;
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 5e6cd43a6dbd..2ebac9e672ef 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -711,6 +711,8 @@ struct i2c_adapter_quirks {
#define I2C_AQ_NO_ZERO_LEN (I2C_AQ_NO_ZERO_LEN_READ |
I2C_AQ_NO_ZERO_LEN_WRITE)
/* adapter cannot do repeated START */
#define I2C_AQ_NO_REP_START BIT(7)
+/* @fixme document and find proper name */
+#define I2C_AQ_SHARE_ADDR BIT(8)
/*
* i2c_adapter is the structure used to identify a physical i2c bus along
"""
This works, however this only supports device-tree because of
of_find_i2c_device_by_node. If we want to support acpi then we can either:
1. Get the Mule i2c device address from fwnode_get_next_parent_dev but
this is static since v6.9-rcx.
2. Pass the Mule i2c device address as a new member of struct
i2c_adapter_quirks.
I would go for 2. Do you suggest something else?
Best regards
Farouk
