Re: [PATCH 3/3] edac: altera: Add SDRAM EDAC support for CycloneV/ArriaV

[Mark Rutland]

On Mon, Apr 07, 2014 at 10:54:09PM +0100, tthayer@xxxxxxxxxx wrote:
> From: Thor Thayer <tthayer@xxxxxxxxxx>
> 
> Added EDAC support for reporting ECC errors of CycloneV
> and ArriaV SDRAM controller.
> - The SDRAM Controller registers are used by the FPGA bridge so
>   these are accessed through the syscon interface.
> - The configuration of the SDRAM memory size for the EDAC framework
>   is discovered from the memory node of the device tree.
> - Documentation of the bindings in devicetree/bindings/arm/altera/
>   socfpga-sdram-edac.txt
> - Correction of single bit errors, detection of double bit errors.
> 
> Signed-off-by: Thor Thayer <tthayer@xxxxxxxxxx>
> To: Rob Herring <robherring2@xxxxxxxxx>
> To: Doug Thompson <dougthompson@xxxxxxxxxxxx>
> To: Grant Likely <grant.likely@xxxxxxxxxx>
> Cc: Dinh Nguyen <dinguyen@xxxxxxxxxx>
> Cc: devicetree@xxxxxxxxxxxxxxx
> Cc: linux-edac@xxxxxxxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> ---
>  drivers/edac/Kconfig          |    9 ++
>  drivers/edac/Makefile         |    2 +
>  drivers/edac/altera_mc_edac.c |  360 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 371 insertions(+)
>  create mode 100644 drivers/edac/altera_mc_edac.c

[...]

> +/* Get total memory size from Open Firmware DTB */
> +static u32 altr_sdram_get_total_mem_size(void)
> +{
> +       struct device_node *np;
> +       u32 retcode, reg_array[2];
> +
> +       np = of_find_node_by_type(NULL, "memory");
> +       if (!np)
> +               return 0;
> +
> +       retcode = of_property_read_u32_array(np, "reg",
> +                                            reg_array, ARRAY_SIZE(reg_array));

There's no requirement that #address-cells = <1> or #size-cells = <1>,
even if any values in either would fit into 32 bits. If we must read
this from the DTB rather than elsewhere, please check
of_n_{addr,size}_cells.

Additionally, it's possible that the physical memory might be described
over multiple reg entries, or multiple memory nodes for some arbitrary
reason.

Can we not get this info from elsewhere rather than having to parse the
memory node within a driver?

Cheers,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/