[PATCH] mfd: palmas: initialise client->of_node for dummy created client

[PATCH] mfd: palmas: initialise client->of_node for dummy created client

[Laxman Dewangan]

Palmas device have three different i2c addresses. The device creates
the two new dummy i2c clients for accessing the register by using
primary client adapter. This new dummy i2c client have their of_node
as NULL.

The dummy i2c client is used for registering interrupt and on this,
it creates irq domain handle. This created irq domain handle has
their of_node as NULL.

Now when any child of this device is registered through the DT as
follows:
         palmas: tps65913@58 {
         	::::::::::::::::::

         	#interrupt-cells = <2>;
                interrupt-controller;

                palmas_rtc {
                       compatible = "ti,palmas-rtc";
                       interrupt-parent = <&palmas>;
                       interrupts = <8 0>;
                };
                ::::::::::;;;
         };

And child driver (palam-rtc in this case) get their irq number as
	irq = platform_get_irq(pdev, 0);

The returned irq number is error in this case. The reason is that
the created irq_domain handle for the palmas interrupt does not have
valid node and so matching of node fails with palmas node.

Hence initialising the newly dummy created client->of_node with the
primary clients of_node so that irq_domain handle have proper of_node
for matching.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/mfd/palmas.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/mfd/palmas.c b/drivers/mfd/palmas.c
index 73bf76d..a3f2836 100644
--- a/drivers/mfd/palmas.c
+++ b/drivers/mfd/palmas.c
@@ -349,6 +349,7 @@ static int palmas_i2c_probe(struct i2c_client *i2c,
 				ret = -ENOMEM;
 				goto err;
 			}
+			palmas->i2c_clients[i]->dev.of_node = of_node_get(node);
 		}
 		palmas->regmap[i] = devm_regmap_init_i2c(palmas->i2c_clients[i],
 				&palmas_regmap_config[i]);
-- 
1.7.1.1

Re: [PATCH] mfd: palmas: initialise client->of_node for dummy created client

[Graeme Gregory]

I discussed this with Grant Likely and we came to the same conclusion
that this was the only way to "fix" the issue. But his concern was
because dummys have a probe/remove of their own this might cause issues
in some cases. His opinion was to do this the probe/remove of the dummys
should also be removed to make them not real devices.

I have added him to CC in case my memory is faulty!

This is a generic issue which will affect all multi address i2c mfds.

Graeme

On 19/03/13 08:58, Laxman Dewangan wrote:
> Palmas device have three different i2c addresses. The device creates
> the two new dummy i2c clients for accessing the register by using
> primary client adapter. This new dummy i2c client have their of_node
> as NULL.
>
> The dummy i2c client is used for registering interrupt and on this,
> it creates irq domain handle. This created irq domain handle has
> their of_node as NULL.
>
> Now when any child of this device is registered through the DT as
> follows:
>          palmas: tps65913@58 {
>          	::::::::::::::::::
>
>          	#interrupt-cells = <2>;
>                 interrupt-controller;
>
>                 palmas_rtc {
>                        compatible = "ti,palmas-rtc";
>                        interrupt-parent = <&palmas>;
>                        interrupts = <8 0>;
>                 };
>                 ::::::::::;;;
>          };
>
> And child driver (palam-rtc in this case) get their irq number as
> 	irq = platform_get_irq(pdev, 0);
>
> The returned irq number is error in this case. The reason is that
> the created irq_domain handle for the palmas interrupt does not have
> valid node and so matching of node fails with palmas node.
>
> Hence initialising the newly dummy created client->of_node with the
> primary clients of_node so that irq_domain handle have proper of_node
> for matching.
>
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> ---
>  drivers/mfd/palmas.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mfd/palmas.c b/drivers/mfd/palmas.c
> index 73bf76d..a3f2836 100644
> --- a/drivers/mfd/palmas.c
> +++ b/drivers/mfd/palmas.c
> @@ -349,6 +349,7 @@ static int palmas_i2c_probe(struct i2c_client *i2c,
>  				ret = -ENOMEM;
>  				goto err;
>  			}
> +			palmas->i2c_clients[i]->dev.of_node = of_node_get(node);
>  		}
>  		palmas->regmap[i] = devm_regmap_init_i2c(palmas->i2c_clients[i],
>  				&palmas_regmap_config[i]);

Re: [PATCH] mfd: palmas: initialise client->of_node for dummy created client

[Stephen Warren]

On 03/19/2013 02:58 AM, Laxman Dewangan wrote:
> Palmas device have three different i2c addresses. The device creates
> the two new dummy i2c clients for accessing the register by using
> primary client adapter. This new dummy i2c client have their of_node
> as NULL.
> 
> The dummy i2c client is used for registering interrupt and on this,
> it creates irq domain handle. This created irq domain handle has
> their of_node as NULL.

It seems like part of the solution here is to modify the i2c_client
object itself so that it can directly support devices that have multiple
I2C addresses; instead of 1 i2c_client representing 1 address, 1
i2c_client could represent a list of addresses, that list being
populated directly from the list contained in the top-level node's reg
property. That way, you wouldn't need any dummy i2c_clients, which would
avoid this issue.

If the I2C device itself has multiple I2C addresses, they really should
all be explicitly listed in the device tree reg property.

Re: [PATCH] mfd: palmas: initialise client->of_node for dummy created client

[Stephen Warren]

On 03/19/2013 10:52 AM, Mark Brown wrote:
> On Tue, Mar 19, 2013 at 09:44:24AM -0600, Stephen Warren wrote:
> 
>> It seems like part of the solution here is to modify the
>> i2c_client object itself so that it can directly support devices
>> that have multiple I2C addresses; instead of 1 i2c_client
>> representing 1 address, 1 i2c_client could represent a list of
>> addresses, that list being populated directly from the list
>> contained in the top-level node's reg property. That way, you
>> wouldn't need any dummy i2c_clients, which would avoid this
>> issue.
> 
> This does then make it more complicated for all users of I2C as
> they need to become aware of such devices.  Not sure that's a
> win...

Presumably the existing APIs would work identically, and additional
APIs would be added for the complex case?

Re: [PATCH] mfd: palmas: initialise client->of_node for dummy created client

[Laxman Dewangan]

On Tuesday 19 March 2013 10:56 PM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Tue, Mar 19, 2013 at 11:10:34AM -0600, Stephen Warren wrote:
>> On 03/19/2013 10:52 AM, Mark Brown wrote:
>>> This does then make it more complicated for all users of I2C as
>>> they need to become aware of such devices.  Not sure that's a
>>> win...
>> Presumably the existing APIs would work identically, and additional
>> APIs would be added for the complex case?
> Right, but we'd have to add the additional APIs to things like regmap
> and ASoC which seems like a kerfuffle.
>

Yes, to add the multiple address list in i2c_client, then adding address 
index in i2c apis, adding address index in regmap inits etc. can require 
lots of RFE patches in different subsystem. We can keep discussing this 
and once conclude, we can go for implementation.

But for now, can I assume that this patch is fine for resolving my 
interrupt issue?

Thanks,
Laxman

Re: [PATCH] mfd: palmas: initialise client->of_node for dummy created client

[Laxman Dewangan]

On Wednesday 20 March 2013 08:09 PM, Laxman Dewangan wrote:
> On Tuesday 19 March 2013 10:56 PM, Mark Brown wrote:
>> * PGP Signed by an unknown key
>>
>> On Tue, Mar 19, 2013 at 11:10:34AM -0600, Stephen Warren wrote:
>>> On 03/19/2013 10:52 AM, Mark Brown wrote:
>>>> This does then make it more complicated for all users of I2C as
>>>> they need to become aware of such devices.  Not sure that's a
>>>> win...
>>> Presumably the existing APIs would work identically, and additional
>>> APIs would be added for the complex case?
>> Right, but we'd have to add the additional APIs to things like regmap
>> and ASoC which seems like a kerfuffle.
>>
> Yes, to add the multiple address list in i2c_client, then adding address
> index in i2c apis, adding address index in regmap inits etc. can require
> lots of RFE patches in different subsystem. We can keep discussing this
> and once conclude, we can go for implementation.
>
> But for now, can I assume that this patch is fine for resolving my
> interrupt issue?
>
> Thanks,
> Laxman
>

I did not heard any objection on this patch so requesting Samuel to 
considering this patch as of now?

Re: [PATCH] mfd: palmas: initialise client->of_node for dummy created client

[Samuel Ortiz]

Hi Laxman,

On Tue, Mar 19, 2013 at 02:28:20PM +0530, Laxman Dewangan wrote:
> Palmas device have three different i2c addresses. The device creates
> the two new dummy i2c clients for accessing the register by using
> primary client adapter. This new dummy i2c client have their of_node
> as NULL.
> 
> The dummy i2c client is used for registering interrupt and on this,
> it creates irq domain handle. This created irq domain handle has
> their of_node as NULL.
> 
> Now when any child of this device is registered through the DT as
> follows:
>          palmas: tps65913@58 {
>          	::::::::::::::::::
> 
>          	#interrupt-cells = <2>;
>                 interrupt-controller;
> 
>                 palmas_rtc {
>                        compatible = "ti,palmas-rtc";
>                        interrupt-parent = <&palmas>;
>                        interrupts = <8 0>;
>                 };
>                 ::::::::::;;;
>          };
> 
> And child driver (palam-rtc in this case) get their irq number as
> 	irq = platform_get_irq(pdev, 0);
> 
> The returned irq number is error in this case. The reason is that
> the created irq_domain handle for the palmas interrupt does not have
> valid node and so matching of node fails with palmas node.
> 
> Hence initialising the newly dummy created client->of_node with the
> primary clients of_node so that irq_domain handle have proper of_node
> for matching.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> ---
>  drivers/mfd/palmas.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
Applied to mfd-next, thanks.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/