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

[Dinh Nguyen <dinguyen-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx@public.gmane.org>]

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.

Dinh

--
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