| Message ID | 20190712034904.5747-9-chris.packham@alliedtelesis.co.nz (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 23d103ae3e061b466dc68dd10a035dcd3b2de99a |
| Headers | show |
| Series | EDAC drivers for Armada XP L2 and DDR | expand |
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 <james.morse@arm.com> Thanks, James
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 <james.morse@arm.com> > > > Thanks, > > James
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);
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 <chris.packham@alliedtelesis.co.nz> --- drivers/edac/armada_xp_edac.c | 5 +++++ 1 file changed, 5 insertions(+)