Re: Re: [PATCH 1/3] arm: socfpga: Enable ECC of L2 and OCRAM on startup.

[Masami Hiramatsu <masami.hiramatsu.pt-FCd8Q96Dh0JBDgjK7y7TUQ@public.gmane.org>]

(2014/10/04 6:42), Dinh Nguyen wrote:
> On 10/03/2014 04:51 AM, Masami Hiramatsu wrote:
>> Hi Dinh,
>>
>> (2014/10/02 20:38), Dinh Nguyen wrote:
>>> On 10/1/14, 5:18 PM, Thor Thayer wrote:
>>>>
>>>> On 10/01/2014 04:10 PM, Dinh Nguyen wrote:
>>>>>
>>>>> On 10/1/14, 4:07 PM, Thor Thayer wrote:
>>>>>> On 10/01/2014 12:13 PM, Dinh Nguyen wrote:
>>>>>>> On 10/1/14, 11:31 AM, tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org wrote:
>>>>>>>> From: Thor Thayer <tthayer-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>
>>>>>>>>
>>>>> [...]
>>>>>>>>      static void socfpga_cyclone5_restart(enum reboot_mode mode, const
>>>>>>>> char *cmd)
>>>>>>>> @@ -98,6 +101,13 @@ static void socfpga_cyclone5_restart(enum
>>>>>>>> reboot_mode mode, const char *cmd)
>>>>>>>>        writel(temp, rst_manager_base_addr + SOCFPGA_RSTMGR_CTRL);
>>>>>>>>    }
>>>>>>>>    +static void __init socfpga_cyclone5_init(void)
>>>>>>>> +{
>>>>>>>> +    of_platform_populate(NULL, of_default_bus_match_table,
>>>>>>>> +                 NULL, NULL);
>>>>>>> Why is this needed?
>>>>>>>
>>>>>>> Dinh
>>>>>> Hi Dinh.
>>>>>>
>>>>>> Are you asking why the of_platform_populate() is needed? If so, it is
>>>>>> used to kick off discovery of devices at the root of the tree.
>>>>> I was asking about of_platform populate(). This was removed in this
>>>>> commit:
>>>>>
>>>>> 8b5c18f05 ARM: l2c: socfpga: convert to generic l2c OF initialisation
>>>>>
>>>>> Just trying to understand what's the need to add it back?
>>>>>
>>>>> Dinh
>>>> It is used to kick off discovery of devices at the root of the tree. 
>>>> This is needed when .init_machine was defined because we're not using
>>>> the default implementation (which called this function).
>>>>
>>>
>>> Then, can you please add a separate patch to fix up the removal of the call?
>>
>> It seems that this patch does enough reasonable thing.
>>
>> To initialize ECC on OCRAM, socfpga_init_ocram_ecc() must be invoked
>> while booting. In that case, he might need this function.
>>
>> 8b5c18f05 is to replace l2x0_of_init() with .l2c_aux_* fields, and
>> this patch has a different reason to call another init function in
>> .init_machine. In this case, it is natural to define .init_machine
>> handler in this patch.
> 
> 8b5c18f05 also removes the need for .init_machine because
> of_platform_populate() is now called by common arm code.
> 
>>
>> IOW, both use same socfpga_cyclone5_init() for the function name,
>> but those have different purpose. I think it is not reviving the
>> old function, but just adding a new function which has same name.
>>
> 
> So I'm just wondering if the call to socfpga_init_ocram_ecc() can be
> done in socfpga_init_irq() along with the call to socfpga_init_l2_ecc(),
> so that you don't have to bring back the custom init_machine callback.

Indeed, it will be better and simpler.

Thank you,

-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt-FCd8Q96Dh0JBDgjK7y7TUQ@public.gmane.org


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html