Re: [PATCH] soc/tegra: refactor soc_is_tegra()

From: Jon Hunter
Date: Wed Nov 21 2018 - 09:51:01 EST



On 21/11/2018 14:47, Frank Lee wrote:
> On Wed, Nov 21, 2018 at 10:43 PM Thierry Reding
> <thierry.reding@xxxxxxxxx> wrote:
>>
>> On Wed, Nov 21, 2018 at 02:34:57PM +0000, Jon Hunter wrote:
>>>
>>> On 21/11/2018 14:12, Yangtao Li wrote:
>>>> of_find_node_by_path() acquires a reference to the node returned by
>>>> it and that reference needs to be dropped by its caller.soc_is_tegra()
>>>> doesn't do that, so fix it.Call of_machine_is_compatible() to refactor
>>>> soc_is_tegra() whcih automatically manages the reference count.
>>>>
>>>> Signed-off-by: Yangtao Li <tiny.windzz@xxxxxxxxx>
>>>> ---
>>>> drivers/soc/tegra/common.c | 12 +++++++-----
>>>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/soc/tegra/common.c b/drivers/soc/tegra/common.c
>>>> index cd8f41351add..0b40700b672a 100644
>>>> --- a/drivers/soc/tegra/common.c
>>>> +++ b/drivers/soc/tegra/common.c
>>>> @@ -22,11 +22,13 @@ static const struct of_device_id tegra_machine_match[] = {
>>>>
>>>> bool soc_is_tegra(void)
>>>> {
>>>> - struct device_node *root;
>>>> + struct of_device_id *match = tegra_machine_match;
>>>>
>>>> - root = of_find_node_by_path("/");
>>>> - if (!root)
>>>> - return false;
>>>> + while(match->compatible){
>>>> + if(of_machine_is_compatible(match->compatible))
>>>> + return true;
>>>> + match++;
>>>> + }
>>>>
>>>> - return of_match_node(tegra_machine_match, root) != NULL;
>>>> + return false;
>>>> }
>>>
>>> Ugh ... sorry, I thought that of_machine_is_compatible() looped through
>>> the matches. OK, let's stick with your initial fix.
>>
>> Actually I prefer this one. Even if it is slightly more verbose, I think
>> it's much clearer what's actually going on. Also this hides all of the
>> OF node reference counting in a core function, so it's worth the extra
>> line, in my opinion.
>>
>> Thierry
> Hi Jonï
>
> I like both, how aboout you?

Yes fine with me too. However, looks like there is some formatting that
needs to be fixed up above. Please make sure you run checkpatch.pl on
your patches. Otherwise ...

Acked-by: Jon Hunter <jonathanh@xxxxxxxxxx>

Thierry, pick up this version if you prefer.

Cheers
Jon

--
nvpublic