Re: [PATCH v3 01/11] i2c: Export of_i2c_get_board_info()

[Boris Brezillon]

On Sat, 24 Mar 2018 23:38:32 +0100
Wolfram Sang  wrote:

> > > - info.archdata = _ad;  
> > 
> > Why did you drop this?  
> 
> If the removal is safe, it should be a seperate patch, I mean.
> 

Sure, I'll move that in a separate patch. Actually, I had a closer look
and it seems the only user of info->archdata is i2c-core-of.c, so we
can even remove the info->archdata field.

-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 01/11] i2c: Export of_i2c_get_board_info()

[Boris Brezillon]

Hi Wolfram,

On Sat, 24 Mar 2018 23:35:18 +0100
Wolfram Sang  wrote:

> Hi Boris,
> 
> > - rebase on v4.15-rc1  
> 
> This code has changed a little meanwhile. Please check my for-next
> branch. Some changes are identical, some similar.

Actually it was rebased on top of v4.16-rc1, but I'll check if this
patch applies correctly on top of your for-next branch.

> 
> > -   info.archdata = _ad;  
> 
> Why did you drop this?

Well, this line could have been moved in of_i2c_register_device(), but
I realized dev_ad was zero initialized without any further
modification, and leaving info->archdata to NULL has the exact same
effect (see [1]).

Regards,

Boris

[1]https://elixir.bootlin.com/linux/v4.16-rc6/source/drivers/i2c/i2c-core-base.c#L711

-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 01/11] i2c: Export of_i2c_get_board_info()

[Wolfram Sang]

> > -   info.archdata = _ad;
> 
> Why did you drop this?

If the removal is safe, it should be a seperate patch, I mean.



signature.asc
Description: PGP signature

Re: [PATCH v3 01/11] i2c: Export of_i2c_get_board_info()

[Wolfram Sang]

Hi Boris,

> - rebase on v4.15-rc1

This code has changed a little meanwhile. Please check my for-next
branch. Some changes are identical, some similar.

> - info.archdata = _ad;

Why did you drop this?

Regards,

   Wolfram



signature.asc
Description: PGP signature

[PATCH v3 01/11] i2c: Export of_i2c_get_board_info()

[Boris Brezillon]

From: Boris Brezillon 

I3C busses have to know about all I2C devices connected on the I3C bus
to properly initialize the I3C master, and I2C frames can't be sent on
the bus until this initialization is done.

We can't let the I2C core parse the DT and instantiate I2C devices as
part of its i2c_add_adapter() procedure because, when done this way,
I2C devices are directly registered to the device-model and might be
attached to drivers which could in turn start sending frames on the bus,
which won't work since, as said above, the bus is not yet initialized.

Export of_i2c_register_device() in order to let the I3C core parse the
I2C device nodes by itself and initialize the bus.

Signed-off-by: Boris Brezillon 
---
Changes in v2:
- fix memset() call
- rebase on v4.15-rc1
---
 drivers/i2c/i2c-core-base.c |  2 +-
 drivers/i2c/i2c-core-of.c   | 66 ++---
 include/linux/i2c.h | 10 +++
 3 files changed, 49 insertions(+), 29 deletions(-)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 5a00bf443d06..e57715f5064c 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -736,7 +736,7 @@ i2c_new_device(struct i2c_adapter *adap, struct 
i2c_board_info const *info)
client->dev.parent = >adapter->dev;
client->dev.bus = _bus_type;
client->dev.type = _client_type;
-   client->dev.of_node = info->of_node;
+   client->dev.of_node = of_node_get(info->of_node);
client->dev.fwnode = info->fwnode;
 
i2c_dev_set_name(adap, client, info);
diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c
index 8d474bb1dc15..7470bc418a3b 100644
--- a/drivers/i2c/i2c-core-of.c
+++ b/drivers/i2c/i2c-core-of.c
@@ -22,56 +22,66 @@
 
 #include "i2c-core.h"
 
-static struct i2c_client *of_i2c_register_device(struct i2c_adapter *adap,
-struct device_node *node)
+int of_i2c_get_board_info(struct device *dev, struct device_node *node,
+ struct i2c_board_info *info)
 {
-   struct i2c_client *result;
-   struct i2c_board_info info = {};
-   struct dev_archdata dev_ad = {};
-   const __be32 *addr_be;
u32 addr;
-   int len;
+   int ret;
 
-   dev_dbg(>dev, "of_i2c: register %pOF\n", node);
+   memset(info, 0, sizeof(*info));
 
-   if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {
-   dev_err(>dev, "of_i2c: modalias failure on %pOF\n",
-   node);
-   return ERR_PTR(-EINVAL);
+   if (of_modalias_node(node, info->type, sizeof(info->type)) < 0) {
+   dev_err(dev, "of_i2c: modalias failure on %pOF\n", node);
+   return -EINVAL;
}
 
-   addr_be = of_get_property(node, "reg", );
-   if (!addr_be || (len < sizeof(*addr_be))) {
-   dev_err(>dev, "of_i2c: invalid reg on %pOF\n", node);
-   return ERR_PTR(-EINVAL);
+   ret = of_property_read_u32(node, "reg", );
+   if (ret) {
+   dev_err(dev, "of_i2c: invalid reg on %pOF\n", node);
+   return ret;
}
 
-   addr = be32_to_cpup(addr_be);
if (addr & I2C_TEN_BIT_ADDRESS) {
addr &= ~I2C_TEN_BIT_ADDRESS;
-   info.flags |= I2C_CLIENT_TEN;
+   info->flags |= I2C_CLIENT_TEN;
}
 
if (addr & I2C_OWN_SLAVE_ADDRESS) {
addr &= ~I2C_OWN_SLAVE_ADDRESS;
-   info.flags |= I2C_CLIENT_SLAVE;
+   info->flags |= I2C_CLIENT_SLAVE;
}
 
-   if (i2c_check_addr_validity(addr, info.flags)) {
-   dev_err(>dev, "of_i2c: invalid addr=%x on %pOF\n",
-   addr, node);
-   return ERR_PTR(-EINVAL);
+   ret = i2c_check_addr_validity(addr, info->flags);
+   if (ret) {
+   dev_err(dev, "of_i2c: invalid addr=%x on %pOF\n", addr, node);
+   return ret;
}
 
-   info.addr = addr;
-   info.of_node = of_node_get(node);
-   info.archdata = _ad;
+   info->addr = addr;
+   info->of_node = node;
 
if (of_property_read_bool(node, "host-notify"))
-   info.flags |= I2C_CLIENT_HOST_NOTIFY;
+   info->flags |= I2C_CLIENT_HOST_NOTIFY;
 
if (of_get_property(node, "wakeup-source", NULL))
-   info.flags |= I2C_CLIENT_WAKE;
+   info->flags |= I2C_CLIENT_WAKE;
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(of_i2c_get_board_info);
+
+static struct i2c_client *of_i2c_register_device(struct i2c_adapter *adap,
+struct device_node *node)
+{
+   struct i2c_client *result;
+   struct i2c_board_info info;
+   int ret;
+
+   dev_dbg(>dev, "of_i2c: register %pOF\n", node);
+
+   ret = of_i2c_get_board_info(>dev, node, );
+   if (ret)