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

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

At 2022-06-30 22:19:45, Claudiu.Beznea@microchip.com wrote:
>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.

OK, soon.
>
>> 
>> 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
>>>
>