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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 10/04/2014 11:18 PM, Masami Hiramatsu wrote:
(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@xxxxxxxxxxxxxxxxxxxxx  wrote:
From: Thor Thayer<tthayer@xxxxxxxxxxxxxxxxxxxxx>

[...]
      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,

Hi. I didn't explain this well in my previous comments.

The call to socfpga_init_ocram_ecc() needs to be in .init_machine because socfpga_init_ocram_ecc() needs the device tree structure to be populated to dereference the sram handle. It is in the .init_machine function that the device tree discovery/population process is kicked off so it makes sense to add it here.

I originally tried adding socfpga_init_ocram_ecc() in the socfpga_init_irq() but the sram phandle couldn't be parsed.

I'm moving the OCRAM addition into a separate patch as requested by Dinh.

Thor

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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux