[PATCH 0/5] mfd: Add a dependency on OF

[PATCH 0/5] mfd: Add a dependency on OF

[Keerthy]

Currently few of the drivers boot only via device tree hence add a
dependency on OF.

Signed-off-by: Keerthy <j-keerthy@ti.com> 

Keerthy (5):
  mfd: palmas: Add a dependency on OF
  mfd: tps65218: Add a dependency on OF
  mfd: tps65217: Add a dependency on OF
  mfd: lp873x: Add a dependency on OF
  mfd: lp3943: Add a dependency on OF

 drivers/mfd/Kconfig | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

-- 
1.9.1

[PATCH 1/5] mfd: palmas: Add a dependency on OF

[Keerthy]

Currently the driver boots only via device tree hence add a
dependency on OF.

Signed-off-by: Keerthy <j-keerthy@ti.com>
---
 drivers/mfd/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 3eb5c93..b845d28 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1223,7 +1223,7 @@ config MFD_PALMAS
 	select MFD_CORE
 	select REGMAP_I2C
 	select REGMAP_IRQ
-	depends on I2C=y
+	depends on I2C=y && OF
 	help
 	  If you say yes here you get support for the Palmas
 	  series of PMIC chips from Texas Instruments.
-- 
1.9.1

[PATCH 2/5] mfd: tps65218: Add a dependency on OF

[Keerthy]

Currently the driver boots only via device tree hence add a
dependency on OF.

Signed-off-by: Keerthy <j-keerthy@ti.com>
---
 drivers/mfd/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index b845d28..75b59f1 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1327,7 +1327,7 @@ config MFD_TI_LP873X
 
 config MFD_TPS65218
 	tristate "TI TPS65218 Power Management chips"
-	depends on I2C
+	depends on I2C && OF
 	select MFD_CORE
 	select REGMAP_I2C
 	select REGMAP_IRQ
-- 
1.9.1

[PATCH 3/5] mfd: tps65217: Add a dependency on OF

[Keerthy]

Currently the driver boots only via device tree hence add a
dependency on OF.

Signed-off-by: Keerthy <j-keerthy@ti.com>
---
 drivers/mfd/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 75b59f1..2d1425d 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1297,7 +1297,7 @@ config MFD_TPS65090
 
 config MFD_TPS65217
 	tristate "TI TPS65217 Power Management / White LED chips"
-	depends on I2C
+	depends on I2C && OF
 	select MFD_CORE
 	select REGMAP_I2C
 	select IRQ_DOMAIN
-- 
1.9.1

[PATCH 4/5] mfd: lp873x: Add a dependency on OF

[Keerthy]

Currently the driver boots only via device tree hence add a
dependency on OF.

Signed-off-by: Keerthy <j-keerthy@ti.com>
---
 drivers/mfd/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 2d1425d..83d6b57 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1313,7 +1313,7 @@ config MFD_TPS65217
 
 config MFD_TI_LP873X
 	tristate "TI LP873X Power Management IC"
-	depends on I2C
+	depends on I2C && OF
 	select MFD_CORE
 	select REGMAP_I2C
 	help
-- 
1.9.1

[PATCH 5/5] mfd: lp3943: Add a dependency on OF

[Keerthy]

Currently the driver boots only via device tree hence add a
dependency on OF.

Signed-off-by: Keerthy <j-keerthy@ti.com>
---
 drivers/mfd/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 83d6b57..4decbfb 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1178,7 +1178,7 @@ config MFD_DM355EVM_MSP
 
 config MFD_LP3943
 	tristate "TI/National Semiconductor LP3943 MFD Driver"
-	depends on I2C
+	depends on I2C && OF
 	select MFD_CORE
 	select REGMAP_I2C
 	help
-- 
1.9.1

Re: [PATCH 3/5] mfd: tps65217: Add a dependency on OF

[Enric Balletbo Serra]

Hi Keerthy,

By change I was looking at this. Some comments below that I think can
be applied to all patches in this series

2017-06-06 16:45 GMT+02:00 Keerthy <j-keerthy@ti.com>:
> Currently the driver boots only via device tree hence add a
> dependency on OF.
>
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
>  drivers/mfd/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 75b59f1..2d1425d 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1297,7 +1297,7 @@ config MFD_TPS65090
>
>  config MFD_TPS65217
>         tristate "TI TPS65217 Power Management / White LED chips"
> -       depends on I2C
> +       depends on I2C && OF

Shouldn't you add || COMPILE_TEST here ?

>         select MFD_CORE
>         select REGMAP_I2C
>         select IRQ_DOMAIN
> --
> 1.9.1
>

I think you can remove the of_match_device checks in some drivers too

i.e:

http://elixir.free-electrons.com/linux/latest/source/drivers/mfd/tps65217.c#L330

Regards,
 Enric

Re: [PATCH 3/5] mfd: tps65217: Add a dependency on OF

[Keerthy]

On Tuesday 06 June 2017 08:34 PM, Enric Balletbo Serra wrote:
> Hi Keerthy,
> 
> By change I was looking at this. Some comments below that I think can
> be applied to all patches in this series
> 
> 2017-06-06 16:45 GMT+02:00 Keerthy <j-keerthy@ti.com>:
>> Currently the driver boots only via device tree hence add a
>> dependency on OF.
>>
>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>> ---
>>  drivers/mfd/Kconfig | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>> index 75b59f1..2d1425d 100644
>> --- a/drivers/mfd/Kconfig
>> +++ b/drivers/mfd/Kconfig
>> @@ -1297,7 +1297,7 @@ config MFD_TPS65090
>>
>>  config MFD_TPS65217
>>         tristate "TI TPS65217 Power Management / White LED chips"
>> -       depends on I2C
>> +       depends on I2C && OF
> 
> Shouldn't you add || COMPILE_TEST here ?

Sure.

> 
>>         select MFD_CORE
>>         select REGMAP_I2C
>>         select IRQ_DOMAIN
>> --
>> 1.9.1
>>
> 
> I think you can remove the of_match_device checks in some drivers too
> 
> i.e:
> 
> http://elixir.free-electrons.com/linux/latest/source/drivers/mfd/tps65217.c#L330

Yes that and removal of unused i2c_device_id. I will follow it up once
this OF dependency is in.

> 
> Regards,
>  Enric
> 

Re: [PATCH 3/5] mfd: tps65217: Add a dependency on OF

[Lee Jones]

On Wed, 07 Jun 2017, Keerthy wrote:

> 
> 
> On Tuesday 06 June 2017 08:34 PM, Enric Balletbo Serra wrote:
> > Hi Keerthy,
> > 
> > By change I was looking at this. Some comments below that I think can
> > be applied to all patches in this series
> > 
> > 2017-06-06 16:45 GMT+02:00 Keerthy <j-keerthy@ti.com>:
> >> Currently the driver boots only via device tree hence add a
> >> dependency on OF.
> >>
> >> Signed-off-by: Keerthy <j-keerthy@ti.com>
> >> ---
> >>  drivers/mfd/Kconfig | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> >> index 75b59f1..2d1425d 100644
> >> --- a/drivers/mfd/Kconfig
> >> +++ b/drivers/mfd/Kconfig
> >> @@ -1297,7 +1297,7 @@ config MFD_TPS65090
> >>
> >>  config MFD_TPS65217
> >>         tristate "TI TPS65217 Power Management / White LED chips"
> >> -       depends on I2C
> >> +       depends on I2C && OF
> > 
> > Shouldn't you add || COMPILE_TEST here ?
> 
> Sure.
> 
> > 
> >>         select MFD_CORE
> >>         select REGMAP_I2C
> >>         select IRQ_DOMAIN
> >>
> > 
> > I think you can remove the of_match_device checks in some drivers too
> > 
> > i.e:
> > 
> > http://elixir.free-electrons.com/linux/latest/source/drivers/mfd/tps65217.c#L330
> 
> Yes that and removal of unused i2c_device_id. I will follow it up once
> this OF dependency is in.

The of_match_device() checks should be removed with the OF patch.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 3/5] mfd: tps65217: Add a dependency on OF

[Lee Jones]

On Wed, 07 Jun 2017, Lee Jones wrote:

> On Wed, 07 Jun 2017, Keerthy wrote:
> 
> > 
> > 
> > On Tuesday 06 June 2017 08:34 PM, Enric Balletbo Serra wrote:
> > > Hi Keerthy,
> > > 
> > > By change I was looking at this. Some comments below that I think can
> > > be applied to all patches in this series
> > > 
> > > 2017-06-06 16:45 GMT+02:00 Keerthy <j-keerthy@ti.com>:
> > >> Currently the driver boots only via device tree hence add a
> > >> dependency on OF.
> > >>
> > >> Signed-off-by: Keerthy <j-keerthy@ti.com>
> > >> ---
> > >>  drivers/mfd/Kconfig | 2 +-
> > >>  1 file changed, 1 insertion(+), 1 deletion(-)
> > >>
> > >> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > >> index 75b59f1..2d1425d 100644
> > >> --- a/drivers/mfd/Kconfig
> > >> +++ b/drivers/mfd/Kconfig
> > >> @@ -1297,7 +1297,7 @@ config MFD_TPS65090
> > >>
> > >>  config MFD_TPS65217
> > >>         tristate "TI TPS65217 Power Management / White LED chips"
> > >> -       depends on I2C
> > >> +       depends on I2C && OF
> > > 
> > > Shouldn't you add || COMPILE_TEST here ?
> > 
> > Sure.
> > 
> > > 
> > >>         select MFD_CORE
> > >>         select REGMAP_I2C
> > >>         select IRQ_DOMAIN
> > >>
> > > 
> > > I think you can remove the of_match_device checks in some drivers too
> > > 
> > > i.e:
> > > 
> > > http://elixir.free-electrons.com/linux/latest/source/drivers/mfd/tps65217.c#L330
> > 
> > Yes that and removal of unused i2c_device_id. I will follow it up once
> > this OF dependency is in.
> 
> The of_match_device() checks should be removed with the OF patch.

In fact, just squash these changes into the I2C removal patches.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 3/5] mfd: tps65217: Add a dependency on OF

[Keerthy]

On Wednesday 07 June 2017 04:07 PM, Lee Jones wrote:
> On Wed, 07 Jun 2017, Keerthy wrote:
> 
>>
>>
>> On Tuesday 06 June 2017 08:34 PM, Enric Balletbo Serra wrote:
>>> Hi Keerthy,
>>>
>>> By change I was looking at this. Some comments below that I think can
>>> be applied to all patches in this series
>>>
>>> 2017-06-06 16:45 GMT+02:00 Keerthy <j-keerthy@ti.com>:
>>>> Currently the driver boots only via device tree hence add a
>>>> dependency on OF.
>>>>
>>>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>>>> ---
>>>>  drivers/mfd/Kconfig | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>>>> index 75b59f1..2d1425d 100644
>>>> --- a/drivers/mfd/Kconfig
>>>> +++ b/drivers/mfd/Kconfig
>>>> @@ -1297,7 +1297,7 @@ config MFD_TPS65090
>>>>
>>>>  config MFD_TPS65217
>>>>         tristate "TI TPS65217 Power Management / White LED chips"
>>>> -       depends on I2C
>>>> +       depends on I2C && OF
>>>
>>> Shouldn't you add || COMPILE_TEST here ?
>>
>> Sure.
>>
>>>
>>>>         select MFD_CORE
>>>>         select REGMAP_I2C
>>>>         select IRQ_DOMAIN
>>>>
>>>
>>> I think you can remove the of_match_device checks in some drivers too
>>>
>>> i.e:
>>>
>>> http://elixir.free-electrons.com/linux/latest/source/drivers/mfd/tps65217.c#L330
>>
>> Yes that and removal of unused i2c_device_id. I will follow it up once
>> this OF dependency is in.
> 
> The of_match_device() checks should be removed with the OF patch.

okay

> 

Re: [PATCH 3/5] mfd: tps65217: Add a dependency on OF

[Keerthy]

On Wednesday 07 June 2017 04:08 PM, Lee Jones wrote:
> On Wed, 07 Jun 2017, Lee Jones wrote:
> 
>> On Wed, 07 Jun 2017, Keerthy wrote:
>>
>>>
>>>
>>> On Tuesday 06 June 2017 08:34 PM, Enric Balletbo Serra wrote:
>>>> Hi Keerthy,
>>>>
>>>> By change I was looking at this. Some comments below that I think can
>>>> be applied to all patches in this series
>>>>
>>>> 2017-06-06 16:45 GMT+02:00 Keerthy <j-keerthy@ti.com>:
>>>>> Currently the driver boots only via device tree hence add a
>>>>> dependency on OF.
>>>>>
>>>>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>>>>> ---
>>>>>  drivers/mfd/Kconfig | 2 +-
>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>>>>> index 75b59f1..2d1425d 100644
>>>>> --- a/drivers/mfd/Kconfig
>>>>> +++ b/drivers/mfd/Kconfig
>>>>> @@ -1297,7 +1297,7 @@ config MFD_TPS65090
>>>>>
>>>>>  config MFD_TPS65217
>>>>>         tristate "TI TPS65217 Power Management / White LED chips"
>>>>> -       depends on I2C
>>>>> +       depends on I2C && OF
>>>>
>>>> Shouldn't you add || COMPILE_TEST here ?
>>>
>>> Sure.
>>>
>>>>
>>>>>         select MFD_CORE
>>>>>         select REGMAP_I2C
>>>>>         select IRQ_DOMAIN
>>>>>
>>>>
>>>> I think you can remove the of_match_device checks in some drivers too
>>>>
>>>> i.e:
>>>>
>>>> http://elixir.free-electrons.com/linux/latest/source/drivers/mfd/tps65217.c#L330
>>>
>>> Yes that and removal of unused i2c_device_id. I will follow it up once
>>> this OF dependency is in.
>>
>> The of_match_device() checks should be removed with the OF patch.

Lee Jones/ Enric,

IIUC of_match_device call is still needed to obtain a match and in case
there are multiple compatibles with different match data then this call
is definitely needed.

There is no need to check for return value as we will find one match for
sure and that can be removed.

Even checks like 'if (client->dev.of_node) {' can surely be removed with
depends on OF.

Please correct me if i am wrong.

Regards,
Keerthy
> 
> In fact, just squash these changes into the I2C removal patches.
> 

Re: [PATCH 3/5] mfd: tps65217: Add a dependency on OF

[Enric Balletbo Serra]

2017-06-07 13:24 GMT+02:00 Keerthy <j-keerthy@ti.com>:
>
>
> On Wednesday 07 June 2017 04:08 PM, Lee Jones wrote:
>> On Wed, 07 Jun 2017, Lee Jones wrote:
>>
>>> On Wed, 07 Jun 2017, Keerthy wrote:
>>>
>>>>
>>>>
>>>> On Tuesday 06 June 2017 08:34 PM, Enric Balletbo Serra wrote:
>>>>> Hi Keerthy,
>>>>>
>>>>> By change I was looking at this. Some comments below that I think can
>>>>> be applied to all patches in this series
>>>>>
>>>>> 2017-06-06 16:45 GMT+02:00 Keerthy <j-keerthy@ti.com>:
>>>>>> Currently the driver boots only via device tree hence add a
>>>>>> dependency on OF.
>>>>>>
>>>>>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>>>>>> ---
>>>>>>  drivers/mfd/Kconfig | 2 +-
>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>>>>>> index 75b59f1..2d1425d 100644
>>>>>> --- a/drivers/mfd/Kconfig
>>>>>> +++ b/drivers/mfd/Kconfig
>>>>>> @@ -1297,7 +1297,7 @@ config MFD_TPS65090
>>>>>>
>>>>>>  config MFD_TPS65217
>>>>>>         tristate "TI TPS65217 Power Management / White LED chips"
>>>>>> -       depends on I2C
>>>>>> +       depends on I2C && OF
>>>>>
>>>>> Shouldn't you add || COMPILE_TEST here ?
>>>>
>>>> Sure.
>>>>
>>>>>
>>>>>>         select MFD_CORE
>>>>>>         select REGMAP_I2C
>>>>>>         select IRQ_DOMAIN
>>>>>>
>>>>>
>>>>> I think you can remove the of_match_device checks in some drivers too
>>>>>
>>>>> i.e:
>>>>>
>>>>> http://elixir.free-electrons.com/linux/latest/source/drivers/mfd/tps65217.c#L330
>>>>
>>>> Yes that and removal of unused i2c_device_id. I will follow it up once
>>>> this OF dependency is in.
>>>
>>> The of_match_device() checks should be removed with the OF patch.
>
> Lee Jones/ Enric,
>
> IIUC of_match_device call is still needed to obtain a match and in case
> there are multiple compatibles with different match data then this call
> is definitely needed.
>

Not sure if I follow you. My understanding is that with DT the probe
of this driver is only called if there is a node with the compatible =
"ti,tps65217" string. So if probe is called there is always a match
and the call to of_match_device is redundant.

> There is no need to check for return value as we will find one match for
> sure and that can be removed.
>
> Even checks like 'if (client->dev.of_node) {' can surely be removed with
> depends on OF.
>

Yes I think this should be removed too.

> Please correct me if i am wrong.
>
> Regards,
> Keerthy
>>
>> In fact, just squash these changes into the I2C removal patches.
>>

Regards,
 Enric

Re: [PATCH 3/5] mfd: tps65217: Add a dependency on OF

[Keerthy]

On Wednesday 07 June 2017 07:07 PM, Enric Balletbo Serra wrote:
> 2017-06-07 13:24 GMT+02:00 Keerthy <j-keerthy@ti.com>:
>>
>>
>> On Wednesday 07 June 2017 04:08 PM, Lee Jones wrote:
>>> On Wed, 07 Jun 2017, Lee Jones wrote:
>>>
>>>> On Wed, 07 Jun 2017, Keerthy wrote:
>>>>
>>>>>
>>>>>
>>>>> On Tuesday 06 June 2017 08:34 PM, Enric Balletbo Serra wrote:
>>>>>> Hi Keerthy,
>>>>>>
>>>>>> By change I was looking at this. Some comments below that I think can
>>>>>> be applied to all patches in this series
>>>>>>
>>>>>> 2017-06-06 16:45 GMT+02:00 Keerthy <j-keerthy@ti.com>:
>>>>>>> Currently the driver boots only via device tree hence add a
>>>>>>> dependency on OF.
>>>>>>>
>>>>>>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>>>>>>> ---
>>>>>>>  drivers/mfd/Kconfig | 2 +-
>>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>>>>>>> index 75b59f1..2d1425d 100644
>>>>>>> --- a/drivers/mfd/Kconfig
>>>>>>> +++ b/drivers/mfd/Kconfig
>>>>>>> @@ -1297,7 +1297,7 @@ config MFD_TPS65090
>>>>>>>
>>>>>>>  config MFD_TPS65217
>>>>>>>         tristate "TI TPS65217 Power Management / White LED chips"
>>>>>>> -       depends on I2C
>>>>>>> +       depends on I2C && OF
>>>>>>
>>>>>> Shouldn't you add || COMPILE_TEST here ?
>>>>>
>>>>> Sure.
>>>>>
>>>>>>
>>>>>>>         select MFD_CORE
>>>>>>>         select REGMAP_I2C
>>>>>>>         select IRQ_DOMAIN
>>>>>>>
>>>>>>
>>>>>> I think you can remove the of_match_device checks in some drivers too
>>>>>>
>>>>>> i.e:
>>>>>>
>>>>>> http://elixir.free-electrons.com/linux/latest/source/drivers/mfd/tps65217.c#L330
>>>>>
>>>>> Yes that and removal of unused i2c_device_id. I will follow it up once
>>>>> this OF dependency is in.
>>>>
>>>> The of_match_device() checks should be removed with the OF patch.
>>
>> Lee Jones/ Enric,
>>
>> IIUC of_match_device call is still needed to obtain a match and in case
>> there are multiple compatibles with different match data then this call
>> is definitely needed.
>>
> 
> Not sure if I follow you. My understanding is that with DT the probe
> of this driver is only called if there is a node with the compatible =
> "ti,tps65217" string. So if probe is called there is always a match
> and the call to of_match_device is redundant.

How will you get the matching data?

For the tps65217 case you mentioned we need the match pointer to get the
chip_id right?

chip_id = (unsigned long)match->data;

Also one more case of when we have multiple compatibles with different
matching data. Ex:

http://elixir.free-electrons.com/linux/latest/source/drivers/mfd/palmas.c#L522

You need the match pointer to get the corresponding data.

Hope i am clear.

> 
>> There is no need to check for return value as we will find one match for
>> sure and that can be removed.
>>
>> Even checks like 'if (client->dev.of_node) {' can surely be removed with
>> depends on OF.
>>
> 
> Yes I think this should be removed too.
> 
>> Please correct me if i am wrong.
>>
>> Regards,
>> Keerthy
>>>
>>> In fact, just squash these changes into the I2C removal patches.
>>>
> 
> Regards,
>  Enric
> 

Re: [PATCH 3/5] mfd: tps65217: Add a dependency on OF

[Javier Martinez Canillas]

Hello Keerthy,

On Wed, Jun 7, 2017 at 3:45 PM, Keerthy <j-keerthy@ti.com> wrote:

[snip]

>>>>>>>>
>>>>>>>
>>>>>>> I think you can remove the of_match_device checks in some drivers too
>>>>>>>
>>>>>>> i.e:
>>>>>>>
>>>>>>> http://elixir.free-electrons.com/linux/latest/source/drivers/mfd/tps65217.c#L330
>>>>>>
>>>>>> Yes that and removal of unused i2c_device_id. I will follow it up once
>>>>>> this OF dependency is in.
>>>>>
>>>>> The of_match_device() checks should be removed with the OF patch.
>>>
>>> Lee Jones/ Enric,
>>>
>>> IIUC of_match_device call is still needed to obtain a match and in case
>>> there are multiple compatibles with different match data then this call
>>> is definitely needed.
>>>

That's correct...

>>
>> Not sure if I follow you. My understanding is that with DT the probe
>> of this driver is only called if there is a node with the compatible =
>> "ti,tps65217" string. So if probe is called there is always a match
>> and the call to of_match_device is redundant.
>
> How will you get the matching data?
>
> For the tps65217 case you mentioned we need the match pointer to get the
> chip_id right?
>
> chip_id = (unsigned long)match->data;
>

...but this particular driver only has a single entry in the OF table
and so you can just do:

tps->id = TPS65217;

Later if there's a variant for this chip, then you can add the logic
to query the struct of_device_id .data. But for now I think that's
better to just remove as Enric proposes and also remove the .data
field from the struct of_device_id entry.

Best regards,
Javier

Re: [PATCH 3/5] mfd: tps65217: Add a dependency on OF

[Javier Martinez Canillas]

On Wed, Jun 7, 2017 at 4:10 PM, Javier Martinez Canillas
<javier@dowhile0.org> wrote:

[snip]

>>
>> chip_id = (unsigned long)match->data;
>>
>
> ...but this particular driver only has a single entry in the OF table
> and so you can just do:
>
> tps->id = TPS65217;
>

In fact, it seems that the whole chip id logic can go away since no
one is using it. I see that the regulator driver checks for the chip
id value but I don't think that makes sense since the regulator
driver's probe function won't be called for other chips since they
won't match.

Best regards,
Javier

Re: [PATCH 3/5] mfd: tps65217: Add a dependency on OF

[Keerthy]

On Wednesday 07 June 2017 07:40 PM, Javier Martinez Canillas wrote:
> Hello Keerthy,
> 
> On Wed, Jun 7, 2017 at 3:45 PM, Keerthy <j-keerthy@ti.com> wrote:
> 
> [snip]
> 
>>>>>>>>>
>>>>>>>>
>>>>>>>> I think you can remove the of_match_device checks in some drivers too
>>>>>>>>
>>>>>>>> i.e:
>>>>>>>>
>>>>>>>> http://elixir.free-electrons.com/linux/latest/source/drivers/mfd/tps65217.c#L330
>>>>>>>
>>>>>>> Yes that and removal of unused i2c_device_id. I will follow it up once
>>>>>>> this OF dependency is in.
>>>>>>
>>>>>> The of_match_device() checks should be removed with the OF patch.
>>>>
>>>> Lee Jones/ Enric,
>>>>
>>>> IIUC of_match_device call is still needed to obtain a match and in case
>>>> there are multiple compatibles with different match data then this call
>>>> is definitely needed.
>>>>
> 
> That's correct...

That is what i wanted to know. Thanks.

> 
>>>
>>> Not sure if I follow you. My understanding is that with DT the probe
>>> of this driver is only called if there is a node with the compatible =
>>> "ti,tps65217" string. So if probe is called there is always a match
>>> and the call to of_match_device is redundant.
>>
>> How will you get the matching data?
>>
>> For the tps65217 case you mentioned we need the match pointer to get the
>> chip_id right?
>>
>> chip_id = (unsigned long)match->data;
>>
> 
> ...but this particular driver only has a single entry in the OF table
> and so you can just do:
> 
> tps->id = TPS65217;
> 
> Later if there's a variant for this chip, then you can add the logic
> to query the struct of_device_id .data. But for now I think that's
> better to just remove as Enric proposes and also remove the .data
> field from the struct of_device_id entry.

okay agreed for tps65217.

> 
> Best regards,
> Javier
>