Re: [PATCH v2 2/2] i2c: mark device nodes only in case of successful instantiation

[Wolfram Sang]

On Mon, Oct 17, 2016 at 03:59:57PM +0200, Ralf Ramsauer wrote:
> Instantiated I2C device nodes are marked with OF_POPULATE. This was
> introduced in 4f001fd. On unloading, loaded device nodes will of course
> be unmarked. The problem are nodes that fail during initialisation: If a
> node fails, it won't be unloaded and hence not be unmarked.
> 
> If a I2C driver module is unloaded and reloaded, it will skip nodes that
> failed before.
> 
> Skip device nodes that are already populated and mark them only in case
> of success.
> 
> Note that the same issue exists for SPI.
> 
> Fixes: 4f001fd ("i2c: Mark instantiated device nodes with OF_POPULATE")
> Signed-off-by: Ralf Ramsauer 

Applied to for-current, thanks!



signature.asc
Description: PGP signature

Re: [PATCH v2 2/2] i2c: mark device nodes only in case of successful instantiation

[Wolfram Sang]

On Mon, Oct 17, 2016 at 03:59:57PM +0200, Ralf Ramsauer wrote:
> Instantiated I2C device nodes are marked with OF_POPULATE. This was
> introduced in 4f001fd. On unloading, loaded device nodes will of course
> be unmarked. The problem are nodes that fail during initialisation: If a
> node fails, it won't be unloaded and hence not be unmarked.
> 
> If a I2C driver module is unloaded and reloaded, it will skip nodes that
> failed before.
> 
> Skip device nodes that are already populated and mark them only in case
> of success.
> 
> Note that the same issue exists for SPI.
> 
> Fixes: 4f001fd ("i2c: Mark instantiated device nodes with OF_POPULATE")
> Signed-off-by: Ralf Ramsauer 

Applied to for-current, thanks!



signature.asc
Description: PGP signature

Re: [PATCH v2 2/2] i2c: mark device nodes only in case of successful instantiation

[Pantelis Antoniou]

Hi Ralf,

> On Oct 17, 2016, at 16:59 , Ralf Ramsauer  wrote:
> 
> Instantiated I2C device nodes are marked with OF_POPULATE. This was
> introduced in 4f001fd. On unloading, loaded device nodes will of course
> be unmarked. The problem are nodes that fail during initialisation: If a
> node fails, it won't be unloaded and hence not be unmarked.
> 
> If a I2C driver module is unloaded and reloaded, it will skip nodes that
> failed before.
> 
> Skip device nodes that are already populated and mark them only in case
> of success.
> 
> Note that the same issue exists for SPI.
> 
> Fixes: 4f001fd ("i2c: Mark instantiated device nodes with OF_POPULATE")
> Signed-off-by: Ralf Ramsauer 
> ---
> drivers/i2c/i2c-core.c | 11 ++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 5ab6721..1704fc8 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -1681,6 +1681,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 *bus, *node;
> + struct i2c_client *client;
> 
>   /* Only register child devices if the adapter has a node pointer set */
>   if (!adap->dev.of_node)
> @@ -1695,7 +1696,14 @@ static void of_i2c_register_devices(struct i2c_adapter 
> *adap)
>   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);
> +
> + client = of_i2c_register_device(adap, node);
> + if (IS_ERR(client)) {
> + dev_warn(>dev,
> +  "Failed to create I2C device for %s\n",
> +  node->full_name);
> + of_node_clear_flag(node, OF_POPULATED);
> + }
>   }
> 
>   of_node_put(bus);
> @@ -2299,6 +2307,7 @@ static int of_i2c_notify(struct notifier_block *nb, 
> unsigned long action,
>   if (IS_ERR(client)) {
>   dev_err(>dev, "failed to create client for 
> '%s'\n",
>rd->dn->full_name);
> + of_node_clear_flag(rd->dn, OF_POPULATED);
>   return notifier_from_errno(PTR_ERR(client));
>   }
>   break;
> -- 
> 2.10.1
> 

Thanks for this

Acked-by: Pantelis Antoniou 

Re: [PATCH v2 2/2] i2c: mark device nodes only in case of successful instantiation

[Pantelis Antoniou]

Hi Ralf,

> On Oct 17, 2016, at 16:59 , Ralf Ramsauer  wrote:
> 
> Instantiated I2C device nodes are marked with OF_POPULATE. This was
> introduced in 4f001fd. On unloading, loaded device nodes will of course
> be unmarked. The problem are nodes that fail during initialisation: If a
> node fails, it won't be unloaded and hence not be unmarked.
> 
> If a I2C driver module is unloaded and reloaded, it will skip nodes that
> failed before.
> 
> Skip device nodes that are already populated and mark them only in case
> of success.
> 
> Note that the same issue exists for SPI.
> 
> Fixes: 4f001fd ("i2c: Mark instantiated device nodes with OF_POPULATE")
> Signed-off-by: Ralf Ramsauer 
> ---
> drivers/i2c/i2c-core.c | 11 ++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 5ab6721..1704fc8 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -1681,6 +1681,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 *bus, *node;
> + struct i2c_client *client;
> 
>   /* Only register child devices if the adapter has a node pointer set */
>   if (!adap->dev.of_node)
> @@ -1695,7 +1696,14 @@ static void of_i2c_register_devices(struct i2c_adapter 
> *adap)
>   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);
> +
> + client = of_i2c_register_device(adap, node);
> + if (IS_ERR(client)) {
> + dev_warn(>dev,
> +  "Failed to create I2C device for %s\n",
> +  node->full_name);
> + of_node_clear_flag(node, OF_POPULATED);
> + }
>   }
> 
>   of_node_put(bus);
> @@ -2299,6 +2307,7 @@ static int of_i2c_notify(struct notifier_block *nb, 
> unsigned long action,
>   if (IS_ERR(client)) {
>   dev_err(>dev, "failed to create client for 
> '%s'\n",
>rd->dn->full_name);
> + of_node_clear_flag(rd->dn, OF_POPULATED);
>   return notifier_from_errno(PTR_ERR(client));
>   }
>   break;
> -- 
> 2.10.1
> 

Thanks for this

Acked-by: Pantelis Antoniou 

Re: [PATCH v2 2/2] i2c: mark device nodes only in case of successful instantiation

[Ralf Ramsauer]

On 10/17/2016 04:16 PM, Geert Uytterhoeven wrote:
> On Mon, Oct 17, 2016 at 3:59 PM, Ralf Ramsauer
>  wrote:
>> Instantiated I2C device nodes are marked with OF_POPULATE. This was
>> introduced in 4f001fd. On unloading, loaded device nodes will of course
>> be unmarked. The problem are nodes that fail during initialisation: If a
>> node fails, it won't be unloaded and hence not be unmarked.
>>
>> If a I2C driver module is unloaded and reloaded, it will skip nodes that
>> failed before.
>>
>> Skip device nodes that are already populated and mark them only in case
>> of success.
>>
>> Note that the same issue exists for SPI.
>>
>> Fixes: 4f001fd ("i2c: Mark instantiated device nodes with OF_POPULATE")
>> Signed-off-by: Ralf Ramsauer 
> 
> Reviewed-by: Geert Uytterhoeven 
> 
>> --- a/drivers/i2c/i2c-core.c
>> +++ b/drivers/i2c/i2c-core.c
> 
>> @@ -1695,7 +1696,14 @@ static void of_i2c_register_devices(struct 
>> i2c_adapter *adap)
>> 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);
>> +
>> +   client = of_i2c_register_device(adap, node);
>> +   if (IS_ERR(client)) {
>> +   dev_warn(>dev,
>> +"Failed to create I2C device for %s\n",
>> +node->full_name);
> 
> I don't think there's a need to add this warning, as of_i2c_register_device()
> already prints messages in all failure paths.
So does of_register_spi_device(). And there's a dev_warn() in
of_spi_register_devices() as well...
But yes, I was also thinking of this.

Nevertheless, I added it as I thought this would keep both drivers
somehow consistent. And it's a very rare error case, though.

  Ralf
> 
>> +   of_node_clear_flag(node, OF_POPULATED);
>> +   }
>> }
> 
> Gr{oetje,eeting}s,
> 
> Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
> ge...@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like 
> that.
> -- Linus Torvalds
> 

-- 
Ralf Ramsauer
PGP: 0x8F10049B

Re: [PATCH v2 2/2] i2c: mark device nodes only in case of successful instantiation

[Ralf Ramsauer]

On 10/17/2016 04:16 PM, Geert Uytterhoeven wrote:
> On Mon, Oct 17, 2016 at 3:59 PM, Ralf Ramsauer
>  wrote:
>> Instantiated I2C device nodes are marked with OF_POPULATE. This was
>> introduced in 4f001fd. On unloading, loaded device nodes will of course
>> be unmarked. The problem are nodes that fail during initialisation: If a
>> node fails, it won't be unloaded and hence not be unmarked.
>>
>> If a I2C driver module is unloaded and reloaded, it will skip nodes that
>> failed before.
>>
>> Skip device nodes that are already populated and mark them only in case
>> of success.
>>
>> Note that the same issue exists for SPI.
>>
>> Fixes: 4f001fd ("i2c: Mark instantiated device nodes with OF_POPULATE")
>> Signed-off-by: Ralf Ramsauer 
> 
> Reviewed-by: Geert Uytterhoeven 
> 
>> --- a/drivers/i2c/i2c-core.c
>> +++ b/drivers/i2c/i2c-core.c
> 
>> @@ -1695,7 +1696,14 @@ static void of_i2c_register_devices(struct 
>> i2c_adapter *adap)
>> 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);
>> +
>> +   client = of_i2c_register_device(adap, node);
>> +   if (IS_ERR(client)) {
>> +   dev_warn(>dev,
>> +"Failed to create I2C device for %s\n",
>> +node->full_name);
> 
> I don't think there's a need to add this warning, as of_i2c_register_device()
> already prints messages in all failure paths.
So does of_register_spi_device(). And there's a dev_warn() in
of_spi_register_devices() as well...
But yes, I was also thinking of this.

Nevertheless, I added it as I thought this would keep both drivers
somehow consistent. And it's a very rare error case, though.

  Ralf
> 
>> +   of_node_clear_flag(node, OF_POPULATED);
>> +   }
>> }
> 
> Gr{oetje,eeting}s,
> 
> Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
> ge...@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like 
> that.
> -- Linus Torvalds
> 

-- 
Ralf Ramsauer
PGP: 0x8F10049B

Re: [PATCH v2 2/2] i2c: mark device nodes only in case of successful instantiation

[Geert Uytterhoeven]

On Mon, Oct 17, 2016 at 3:59 PM, Ralf Ramsauer
 wrote:
> Instantiated I2C device nodes are marked with OF_POPULATE. This was
> introduced in 4f001fd. On unloading, loaded device nodes will of course
> be unmarked. The problem are nodes that fail during initialisation: If a
> node fails, it won't be unloaded and hence not be unmarked.
>
> If a I2C driver module is unloaded and reloaded, it will skip nodes that
> failed before.
>
> Skip device nodes that are already populated and mark them only in case
> of success.
>
> Note that the same issue exists for SPI.
>
> Fixes: 4f001fd ("i2c: Mark instantiated device nodes with OF_POPULATE")
> Signed-off-by: Ralf Ramsauer 

Reviewed-by: Geert Uytterhoeven 

> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c

> @@ -1695,7 +1696,14 @@ static void of_i2c_register_devices(struct i2c_adapter 
> *adap)
> 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);
> +
> +   client = of_i2c_register_device(adap, node);
> +   if (IS_ERR(client)) {
> +   dev_warn(>dev,
> +"Failed to create I2C device for %s\n",
> +node->full_name);

I don't think there's a need to add this warning, as of_i2c_register_device()
already prints messages in all failure paths.

> +   of_node_clear_flag(node, OF_POPULATED);
> +   }
> }

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH v2 2/2] i2c: mark device nodes only in case of successful instantiation

[Geert Uytterhoeven]

On Mon, Oct 17, 2016 at 3:59 PM, Ralf Ramsauer
 wrote:
> Instantiated I2C device nodes are marked with OF_POPULATE. This was
> introduced in 4f001fd. On unloading, loaded device nodes will of course
> be unmarked. The problem are nodes that fail during initialisation: If a
> node fails, it won't be unloaded and hence not be unmarked.
>
> If a I2C driver module is unloaded and reloaded, it will skip nodes that
> failed before.
>
> Skip device nodes that are already populated and mark them only in case
> of success.
>
> Note that the same issue exists for SPI.
>
> Fixes: 4f001fd ("i2c: Mark instantiated device nodes with OF_POPULATE")
> Signed-off-by: Ralf Ramsauer 

Reviewed-by: Geert Uytterhoeven 

> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c

> @@ -1695,7 +1696,14 @@ static void of_i2c_register_devices(struct i2c_adapter 
> *adap)
> 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);
> +
> +   client = of_i2c_register_device(adap, node);
> +   if (IS_ERR(client)) {
> +   dev_warn(>dev,
> +"Failed to create I2C device for %s\n",
> +node->full_name);

I don't think there's a need to add this warning, as of_i2c_register_device()
already prints messages in all failure paths.

> +   of_node_clear_flag(node, OF_POPULATED);
> +   }
> }

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

[PATCH v2 2/2] i2c: mark device nodes only in case of successful instantiation

[Ralf Ramsauer]

Instantiated I2C device nodes are marked with OF_POPULATE. This was
introduced in 4f001fd. On unloading, loaded device nodes will of course
be unmarked. The problem are nodes that fail during initialisation: If a
node fails, it won't be unloaded and hence not be unmarked.

If a I2C driver module is unloaded and reloaded, it will skip nodes that
failed before.

Skip device nodes that are already populated and mark them only in case
of success.

Note that the same issue exists for SPI.

Fixes: 4f001fd ("i2c: Mark instantiated device nodes with OF_POPULATE")
Signed-off-by: Ralf Ramsauer 
---
 drivers/i2c/i2c-core.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 5ab6721..1704fc8 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1681,6 +1681,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 *bus, *node;
+   struct i2c_client *client;
 
/* Only register child devices if the adapter has a node pointer set */
if (!adap->dev.of_node)
@@ -1695,7 +1696,14 @@ static void of_i2c_register_devices(struct i2c_adapter 
*adap)
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);
+
+   client = of_i2c_register_device(adap, node);
+   if (IS_ERR(client)) {
+   dev_warn(>dev,
+"Failed to create I2C device for %s\n",
+node->full_name);
+   of_node_clear_flag(node, OF_POPULATED);
+   }
}
 
of_node_put(bus);
@@ -2299,6 +2307,7 @@ static int of_i2c_notify(struct notifier_block *nb, 
unsigned long action,
if (IS_ERR(client)) {
dev_err(>dev, "failed to create client for 
'%s'\n",
 rd->dn->full_name);
+   of_node_clear_flag(rd->dn, OF_POPULATED);
return notifier_from_errno(PTR_ERR(client));
}
break;
-- 
2.10.1

[PATCH v2 2/2] i2c: mark device nodes only in case of successful instantiation

[Ralf Ramsauer]

Instantiated I2C device nodes are marked with OF_POPULATE. This was
introduced in 4f001fd. On unloading, loaded device nodes will of course
be unmarked. The problem are nodes that fail during initialisation: If a
node fails, it won't be unloaded and hence not be unmarked.

If a I2C driver module is unloaded and reloaded, it will skip nodes that
failed before.

Skip device nodes that are already populated and mark them only in case
of success.

Note that the same issue exists for SPI.

Fixes: 4f001fd ("i2c: Mark instantiated device nodes with OF_POPULATE")
Signed-off-by: Ralf Ramsauer 
---
 drivers/i2c/i2c-core.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 5ab6721..1704fc8 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1681,6 +1681,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 *bus, *node;
+   struct i2c_client *client;
 
/* Only register child devices if the adapter has a node pointer set */
if (!adap->dev.of_node)
@@ -1695,7 +1696,14 @@ static void of_i2c_register_devices(struct i2c_adapter 
*adap)
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);
+
+   client = of_i2c_register_device(adap, node);
+   if (IS_ERR(client)) {
+   dev_warn(>dev,
+"Failed to create I2C device for %s\n",
+node->full_name);
+   of_node_clear_flag(node, OF_POPULATED);
+   }
}
 
of_node_put(bus);
@@ -2299,6 +2307,7 @@ static int of_i2c_notify(struct notifier_block *nb, 
unsigned long action,
if (IS_ERR(client)) {
dev_err(>dev, "failed to create client for 
'%s'\n",
 rd->dn->full_name);
+   of_node_clear_flag(rd->dn, OF_POPULATED);
return notifier_from_errno(PTR_ERR(client));
}
break;
-- 
2.10.1