diff mbox

[08/12] i2c: core: Add support for 'i2c-bus' subnode

Message ID 1466697545-11868-9-git-send-email-jonathanh@nvidia.com
State Superseded
Headers show

Commit Message

Jon Hunter June 23, 2016, 3:59 p.m. UTC
If the 'i2c-bus' device-tree node is present for an I2C adapter then
parse this subnode for I2C slaves.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/i2c/i2c-core.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

Thierry Reding June 28, 2016, 9:10 p.m. UTC | #1
On Thu, Jun 23, 2016 at 04:59:01PM +0100, Jon Hunter wrote:
> If the 'i2c-bus' device-tree node is present for an I2C adapter then
> parse this subnode for I2C slaves.
> 
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
>  drivers/i2c/i2c-core.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)

Hi Wolfram,

are you okay with this change? I'd like to take this through the Tegra
tree, with your Acked-by, to resolve the dependency of subsequent
patches.

Would you be okay if I take this patch along with the device tree
binding change into a stable branch and provide a tag for you to pull
into the I2C tree?

Thierry
Wolfram Sang June 28, 2016, 9:33 p.m. UTC | #2
> Would you be okay if I take this patch along with the device tree
> binding change into a stable branch and provide a tag for you to pull
> into the I2C tree?

Yes, once my comments to the previous patch are addressed, we can do
that. For this patch already:

Acked-by: Wolfram Sang <wsa@the-dreams.de>
Tomeu Vizoso Aug. 2, 2016, 6:26 a.m. UTC | #3
On 23 June 2016 at 17:59, Jon Hunter <jonathanh@nvidia.com> wrote:
> If the 'i2c-bus' device-tree node is present for an I2C adapter then
> parse this subnode for I2C slaves.
>
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
>  drivers/i2c/i2c-core.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 952d2f0c02c5..71ad532be1d8 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -1452,7 +1452,7 @@ static struct i2c_client *of_i2c_register_device(struct i2c_adapter *adap,
>
>  static void of_i2c_register_devices(struct i2c_adapter *adap)
>  {
> -       struct device_node *node;
> +       struct device_node *bus, *node;
>
>         /* Only register child devices if the adapter has a node pointer set */
>         if (!adap->dev.of_node)
> @@ -1460,11 +1460,17 @@ static void of_i2c_register_devices(struct i2c_adapter *adap)
>
>         dev_dbg(&adap->dev, "of_i2c: walking child nodes\n");
>
> -       for_each_available_child_of_node(adap->dev.of_node, node) {
> +       bus = of_get_child_by_name(adap->dev.of_node, "i2c-bus");
> +       if (!bus)
> +               bus = of_node_get(adap->dev.of_node);
> +
> +       for_each_available_child_of_node(bus, node) {
>                 if (of_node_test_and_set_flag(node, OF_POPULATED))
>                         continue;
>                 of_i2c_register_device(adap, node);
>         }
> +
> +       of_node_put(bus);
>  }

Sorry for not commenting earlier, but I only found the issue yesterday.

I'm bothered as well by the "modalias failure" error message but in my
case the node doesn't have any i2c devices, so this patch isn't a
complete solution to that problem.

Has this been considered already?

Thanks,

Tomeu

>  static int of_dev_node_match(struct device *dev, void *data)
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jon Hunter Aug. 2, 2016, 6:49 a.m. UTC | #4
On 02/08/16 07:26, Tomeu Vizoso wrote:
> On 23 June 2016 at 17:59, Jon Hunter <jonathanh@nvidia.com> wrote:
>> If the 'i2c-bus' device-tree node is present for an I2C adapter then
>> parse this subnode for I2C slaves.
>>
>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>> ---
>>  drivers/i2c/i2c-core.c | 10 ++++++++--
>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
>> index 952d2f0c02c5..71ad532be1d8 100644
>> --- a/drivers/i2c/i2c-core.c
>> +++ b/drivers/i2c/i2c-core.c
>> @@ -1452,7 +1452,7 @@ static struct i2c_client *of_i2c_register_device(struct i2c_adapter *adap,
>>
>>  static void of_i2c_register_devices(struct i2c_adapter *adap)
>>  {
>> -       struct device_node *node;
>> +       struct device_node *bus, *node;
>>
>>         /* Only register child devices if the adapter has a node pointer set */
>>         if (!adap->dev.of_node)
>> @@ -1460,11 +1460,17 @@ static void of_i2c_register_devices(struct i2c_adapter *adap)
>>
>>         dev_dbg(&adap->dev, "of_i2c: walking child nodes\n");
>>
>> -       for_each_available_child_of_node(adap->dev.of_node, node) {
>> +       bus = of_get_child_by_name(adap->dev.of_node, "i2c-bus");
>> +       if (!bus)
>> +               bus = of_node_get(adap->dev.of_node);
>> +
>> +       for_each_available_child_of_node(bus, node) {
>>                 if (of_node_test_and_set_flag(node, OF_POPULATED))
>>                         continue;
>>                 of_i2c_register_device(adap, node);
>>         }
>> +
>> +       of_node_put(bus);
>>  }
> 
> Sorry for not commenting earlier, but I only found the issue yesterday.
> 
> I'm bothered as well by the "modalias failure" error message but in my
> case the node doesn't have any i2c devices, so this patch isn't a
> complete solution to that problem.

But it is a i2c adapter correct? I guess I don't completely understand
the problem?

> Has this been considered already?

Why can't you add a place-holder 'i2c-bus' node for i2c devices so when
they are adding in the future they are placed under this node? This is
what I did for dpaux. See patch 12/12 of this series.

Jon
Tomeu Vizoso Aug. 4, 2016, 6:25 a.m. UTC | #5
On 2 August 2016 at 08:49, Jon Hunter <jonathanh@nvidia.com> wrote:
>
> On 02/08/16 07:26, Tomeu Vizoso wrote:
>> On 23 June 2016 at 17:59, Jon Hunter <jonathanh@nvidia.com> wrote:
>>> If the 'i2c-bus' device-tree node is present for an I2C adapter then
>>> parse this subnode for I2C slaves.
>>>
>>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>>> ---
>>>  drivers/i2c/i2c-core.c | 10 ++++++++--
>>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
>>> index 952d2f0c02c5..71ad532be1d8 100644
>>> --- a/drivers/i2c/i2c-core.c
>>> +++ b/drivers/i2c/i2c-core.c
>>> @@ -1452,7 +1452,7 @@ static struct i2c_client *of_i2c_register_device(struct i2c_adapter *adap,
>>>
>>>  static void of_i2c_register_devices(struct i2c_adapter *adap)
>>>  {
>>> -       struct device_node *node;
>>> +       struct device_node *bus, *node;
>>>
>>>         /* Only register child devices if the adapter has a node pointer set */
>>>         if (!adap->dev.of_node)
>>> @@ -1460,11 +1460,17 @@ static void of_i2c_register_devices(struct i2c_adapter *adap)
>>>
>>>         dev_dbg(&adap->dev, "of_i2c: walking child nodes\n");
>>>
>>> -       for_each_available_child_of_node(adap->dev.of_node, node) {
>>> +       bus = of_get_child_by_name(adap->dev.of_node, "i2c-bus");
>>> +       if (!bus)
>>> +               bus = of_node_get(adap->dev.of_node);
>>> +
>>> +       for_each_available_child_of_node(bus, node) {
>>>                 if (of_node_test_and_set_flag(node, OF_POPULATED))
>>>                         continue;
>>>                 of_i2c_register_device(adap, node);
>>>         }
>>> +
>>> +       of_node_put(bus);
>>>  }
>>
>> Sorry for not commenting earlier, but I only found the issue yesterday.
>>
>> I'm bothered as well by the "modalias failure" error message but in my
>> case the node doesn't have any i2c devices, so this patch isn't a
>> complete solution to that problem.
>
> But it is a i2c adapter correct? I guess I don't completely understand
> the problem?
>
>> Has this been considered already?
>
> Why can't you add a place-holder 'i2c-bus' node for i2c devices so when
> they are adding in the future they are placed under this node? This is
> what I did for dpaux. See patch 12/12 of this series.

Hadn't realized, and it sounds good enough to me.

Thanks,

Tomeu
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 952d2f0c02c5..71ad532be1d8 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1452,7 +1452,7 @@  static struct i2c_client *of_i2c_register_device(struct i2c_adapter *adap,
 
 static void of_i2c_register_devices(struct i2c_adapter *adap)
 {
-	struct device_node *node;
+	struct device_node *bus, *node;
 
 	/* Only register child devices if the adapter has a node pointer set */
 	if (!adap->dev.of_node)
@@ -1460,11 +1460,17 @@  static void of_i2c_register_devices(struct i2c_adapter *adap)
 
 	dev_dbg(&adap->dev, "of_i2c: walking child nodes\n");
 
-	for_each_available_child_of_node(adap->dev.of_node, node) {
+	bus = of_get_child_by_name(adap->dev.of_node, "i2c-bus");
+	if (!bus)
+		bus = of_node_get(adap->dev.of_node);
+
+	for_each_available_child_of_node(bus, node) {
 		if (of_node_test_and_set_flag(node, OF_POPULATED))
 			continue;
 		of_i2c_register_device(adap, node);
 	}
+
+	of_node_put(bus);
 }
 
 static int of_dev_node_match(struct device *dev, void *data)