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

From: Keerthy
Date: Thu Jun 08 2017 - 00:31:30 EST




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@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...

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
>