Re: [PATCH v2] clk: at91: dt-compat: Hold reference returned by of_get_parent()

[<Claudiu.Beznea@microchip.com>]

On 30.06.2022 15:58, Liang He wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> At 2022-06-30 18:20:36, Claudiu.Beznea@microchip.com wrote:
>> On 30.06.2022 11:59, Liang He wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> Hi,  Claudiu.Beznea,
>>>
>>> At 2022-06-30 16:39:26, Claudiu.Beznea@microchip.com wrote:
>>>> On 28.06.2022 16:22, Liang He wrote:
>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>> [...]
>>>>> @@ -694,8 +732,8 @@ of_at91_clk_prog_setup(struct device_node *np,
>>>>>         unsigned int num_parents;
>>>>>         const char *parent_names[PROG_SOURCE_MAX];
>>>>>         const char *name;
>>>>> -       struct device_node *progclknp;
>>>>>         struct regmap *regmap;
>>>>> +       struct device_node *progclknp, *parent_np;
>>>>
>>>> Is there a reason you chosed to move this on a new line?
>>>
>>>>
>>>
>>> In fact, I just want to keep long declaration on bottom.
>>>
>>> However, this modification maybe useless.
>>>
>>>
>>>>> [...]
>>>>>         struct clk_hw *hw;
>>>>>         const char *name;
>>>>> -       struct device_node *sysclknp;
>>>>>         const char *parent_name;
>>>>>         struct regmap *regmap;
>>>>> +       struct device_node *sysclknp, *parent_np;
>>>>
>>>> Same here.
>>>
>>>>
>>>
>>>
>>> Same reason as above.
>>>
>>>>>
>>>>>         num = of_get_child_count(np);
>>>>>         if (num > (SYSTEM_MAX_ID + 1))
>>>
>>>>> @@ -885,9 +936,10 @@ CLK_OF_DECLARE(at91sam9x5_clk_usb, "atmel,at91sam9x5-clk-usb",
>>>>>  static void __init of_at91sam9n12_clk_usb_setup(struct device_node *np)
>>>>>  {
>>>>>         struct clk_hw *hw;
>>>>> +       struct regmap *regmap;
>>>>>         const char *parent_name;
>>>>>         const char *name = np->name;
>>>>> -       struct regmap *regmap;
>>>>> +       struct device_node *parent_np;
>>>>
>>>> You moved around the declarations.
>>>
>>>>
>>>
>>>
>>> Sorry, I have been told to keep reverse christmas tree.
>>
>> I thought this rule is only for networking subsystem.
>>
> 
> OK, thanks, I will keep the rule only when I fix bugs in networking subsystem.
> 
> So  it needs to send a v3 to restore the order of the declarations?

Yes, please.

> 
> If yes, I will resend it soon.
> 
> Thanks.
> 
> Liang
> 
> 
>>>
>>>
>>> So this look like a normal christmas tree?
>>>
>>>
>>>>>>>         parent_name = of_clk_get_parent_name(np, 0);
>>> [...]
>>>>> +       struct regmap *regmap;
>>>>>         const char *parent_name;
>>>>>         const char *name = np->name;
>>>>> +       struct device_node *parent_np;
>>>>>         u32 divisors[4] = {0, 0, 0, 0};
>>>>> -       struct regmap *regmap;
>>>>
>>>> Same here.
>>>
>>>>
>>>
>>>
>>> Same reason as above.
>>>
>>>>>
>>>>>         parent_name = of_clk_get_parent_name(np, 0);
>>>>>         if (!parent_name)
>>>>> @@ -926,7 +981,9 @@ static void __init of_at91rm9200_clk_usb_setup(struct device_node *np)
>>>
>>>>>
>>>
>>>
>>> If these declaration chages are not needed, I can resend a new patch keeping the original order of declarations.
>>>
>>>
>>> Thanks.
>>>
>>>
>>> Liang
>>