Re: [PATCH v9 8/8] EDAC: armada_xp: Add support for more SoCs
On Fri, 2019-07-26 at 15:51 +0100, James Morse wrote:
> Hi Chris,
>
> On 12/07/2019 04:49, Chris Packham wrote:
> >
> > The Armada 38x and other integrated SoCs use a reduced pin count so
> > the
> > width of the SDRAM interface is smaller than the Armada XP SoCs.
> > This
> > means that the definition of "full" and "half" width is reduced
> > from
> > 64/32 to 32/16.
> >
> > diff --git a/drivers/edac/armada_xp_edac.c
> > b/drivers/edac/armada_xp_edac.c
> > index 3759a4fbbdee..7f227bdcbc84 100644
> > --- a/drivers/edac/armada_xp_edac.c
> > +++ b/drivers/edac/armada_xp_edac.c
> > @@ -332,6 +332,11 @@ static int axp_mc_probe(struct platform_device
> > *pdev)
> >
> > axp_mc_read_config(mci);
> >
> > + /* These SoCs have a reduced width bus */
> > + if (of_machine_is_compatible("marvell,armada380") ||
> > + of_machine_is_compatible("marvell,armadaxp-98dx3236"))
> > + drvdata->width /= 2;
> So the hardware's SDRAM_CONFIG_BUS_WIDTH value is wrong? Yuck.
>
The maximum width differs between Armada-XP (64-bit) and Armada-38x
(32-bit). There is still strapping to control half-width vs full-width.
> Is it too late for the DTs on these two systems to provide a DT
> version of the 'bus_width'
> to override the hardware's mis-advertised value?
In an earlier iteration I did have a DT property as you suggest. The
problem is that something like "bus-width = <32>" is ambiguous. On
Armada-XP this means the strapping is for half-width but on Armada-38x
you'd need to strap to full-width. That's why we settled on the mode
interpreting the strapping against SoC[1].
[1] https://lore.kernel.org/linux-arm-kernel/1502444067.1333.7.camel@pe
ngutronix.de/
>
> This way you don't need to grow this list.
>
> Acked-by: James Morse
>
>
> Thanks,
>
> James
Re: [PATCH v9 8/8] EDAC: armada_xp: Add support for more SoCs
Hi Chris,
On 12/07/2019 04:49, Chris Packham wrote:
> The Armada 38x and other integrated SoCs use a reduced pin count so the
> width of the SDRAM interface is smaller than the Armada XP SoCs. This
> means that the definition of "full" and "half" width is reduced from
> 64/32 to 32/16.
> diff --git a/drivers/edac/armada_xp_edac.c b/drivers/edac/armada_xp_edac.c
> index 3759a4fbbdee..7f227bdcbc84 100644
> --- a/drivers/edac/armada_xp_edac.c
> +++ b/drivers/edac/armada_xp_edac.c
> @@ -332,6 +332,11 @@ static int axp_mc_probe(struct platform_device *pdev)
>
> axp_mc_read_config(mci);
>
> + /* These SoCs have a reduced width bus */
> + if (of_machine_is_compatible("marvell,armada380") ||
> + of_machine_is_compatible("marvell,armadaxp-98dx3236"))
> + drvdata->width /= 2;
So the hardware's SDRAM_CONFIG_BUS_WIDTH value is wrong? Yuck.
Is it too late for the DTs on these two systems to provide a DT version of the
'bus_width'
to override the hardware's mis-advertised value?
This way you don't need to grow this list.
Acked-by: James Morse
Thanks,
James
[PATCH v9 8/8] EDAC: armada_xp: Add support for more SoCs
The Armada 38x and other integrated SoCs use a reduced pin count so the
width of the SDRAM interface is smaller than the Armada XP SoCs. This
means that the definition of "full" and "half" width is reduced from
64/32 to 32/16.
Signed-off-by: Chris Packham
---
drivers/edac/armada_xp_edac.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/edac/armada_xp_edac.c b/drivers/edac/armada_xp_edac.c
index 3759a4fbbdee..7f227bdcbc84 100644
--- a/drivers/edac/armada_xp_edac.c
+++ b/drivers/edac/armada_xp_edac.c
@@ -332,6 +332,11 @@ static int axp_mc_probe(struct platform_device *pdev)
axp_mc_read_config(mci);
+ /* These SoCs have a reduced width bus */
+ if (of_machine_is_compatible("marvell,armada380") ||
+ of_machine_is_compatible("marvell,armadaxp-98dx3236"))
+ drvdata->width /= 2;
+
/* configure SBE threshold */
/* it seems that SBEs are not captured otherwise */
writel(1 << SDRAM_ERR_CTRL_THR_OFFSET, drvdata->base +
SDRAM_ERR_CTRL_REG);
--
2.22.0
