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

["Liang He" <windhl@126.com>]

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?

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
>