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

From: Javier Martinez Canillas
Date: Wed Jun 07 2017 - 10:10:52 EST


Hello Keerthy,

On Wed, Jun 7, 2017 at 3:45 PM, Keerthy <j-keerthy@xxxxxx> 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