Dne sobota, 14. oktober 2023 ob 19:02:36 CEST je Andre Przywara napisal(a): > The original H616 DDR3 ODT configuration code wrote board specific values > into a sequence of paired registers. > For LPDDR3 support we needed to special-case one group of registers, > because for that DRAM type we need to write 0 into the lower register of > each pair. That already made the code less readable. > > LPDDR4 support will make things even messier, so let's refactor that > code now: We allow to write different values into the lower and upper > half of each pair. The masking is moved into a macro, and used in each > write statement. > > The effect is not as obvious yet, as we don't need the full flexibility at > the moment, but the motivation will become clearer with LPDDR4 support. > > The generated binary is identical with and without the patch. > > Signed-off-by: Andre Przywara <[email protected]>
This looks much nicer. Reviewed-by: Jernej Skrabec <[email protected]> Best regards, Jernej > --- > arch/arm/mach-sunxi/dram_sun50i_h616.c | 84 ++++++++++---------------- > 1 file changed, 31 insertions(+), 53 deletions(-) > > diff --git a/arch/arm/mach-sunxi/dram_sun50i_h616.c > b/arch/arm/mach-sunxi/dram_sun50i_h616.c > index 7e580b62dca..ba5659d4094 100644 > --- a/arch/arm/mach-sunxi/dram_sun50i_h616.c > +++ b/arch/arm/mach-sunxi/dram_sun50i_h616.c > @@ -241,61 +241,39 @@ static const u8 phy_init[] = { > #endif > }; > > +#define MASK_BYTE(reg, nr) (((reg) >> ((nr) * 8)) & 0x1f) > static void mctl_phy_configure_odt(const struct dram_para *para) > { > - unsigned int val; > - > - val = para->dx_dri & 0x1f; > - writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x388); > - writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x38c); > - > - val = (para->dx_dri >> 8) & 0x1f; > - writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x3c8); > - writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x3cc); > - > - val = (para->dx_dri >> 16) & 0x1f; > - writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x408); > - writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x40c); > - > - val = (para->dx_dri >> 24) & 0x1f; > - writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x448); > - writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x44c); > - > - val = para->ca_dri & 0x1f; > - writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x340); > - writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x344); > - > - val = (para->ca_dri >> 8) & 0x1f; > - writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x348); > - writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x34c); > - > - val = para->dx_odt & 0x1f; > - if (para->type == SUNXI_DRAM_TYPE_LPDDR3) > - writel_relaxed(0, SUNXI_DRAM_PHY0_BASE + 0x380); > - else > - writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x380); > - writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x384); > - > - val = (para->dx_odt >> 8) & 0x1f; > - if (para->type == SUNXI_DRAM_TYPE_LPDDR3) > - writel_relaxed(0, SUNXI_DRAM_PHY0_BASE + 0x3c0); > - else > - writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x3c0); > - writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x3c4); > - > - val = (para->dx_odt >> 16) & 0x1f; > - if (para->type == SUNXI_DRAM_TYPE_LPDDR3) > - writel_relaxed(0, SUNXI_DRAM_PHY0_BASE + 0x400); > - else > - writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x400); > - writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x404); > - > - val = (para->dx_odt >> 24) & 0x1f; > - if (para->type == SUNXI_DRAM_TYPE_LPDDR3) > - writel_relaxed(0, SUNXI_DRAM_PHY0_BASE + 0x440); > - else > - writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x440); > - writel_relaxed(val, SUNXI_DRAM_PHY0_BASE + 0x444); > + uint32_t val_lo, val_hi; > + > + val_lo = para->dx_dri; > + val_hi = para->dx_dri; > + writel_relaxed(MASK_BYTE(val_lo, 0), SUNXI_DRAM_PHY0_BASE + 0x388); > + writel_relaxed(MASK_BYTE(val_hi, 0), SUNXI_DRAM_PHY0_BASE + 0x38c); > + writel_relaxed(MASK_BYTE(val_lo, 1), SUNXI_DRAM_PHY0_BASE + 0x3c8); > + writel_relaxed(MASK_BYTE(val_hi, 1), SUNXI_DRAM_PHY0_BASE + 0x3cc); > + writel_relaxed(MASK_BYTE(val_lo, 2), SUNXI_DRAM_PHY0_BASE + 0x408); > + writel_relaxed(MASK_BYTE(val_hi, 2), SUNXI_DRAM_PHY0_BASE + 0x40c); > + writel_relaxed(MASK_BYTE(val_lo, 3), SUNXI_DRAM_PHY0_BASE + 0x448); > + writel_relaxed(MASK_BYTE(val_hi, 3), SUNXI_DRAM_PHY0_BASE + 0x44c); > + > + val_lo = para->ca_dri; > + val_hi = para->ca_dri; > + writel_relaxed(MASK_BYTE(val_lo, 0), SUNXI_DRAM_PHY0_BASE + 0x340); > + writel_relaxed(MASK_BYTE(val_hi, 0), SUNXI_DRAM_PHY0_BASE + 0x344); > + writel_relaxed(MASK_BYTE(val_lo, 1), SUNXI_DRAM_PHY0_BASE + 0x348); > + writel_relaxed(MASK_BYTE(val_hi, 1), SUNXI_DRAM_PHY0_BASE + 0x34c); > + > + val_lo = (para->type == SUNXI_DRAM_TYPE_LPDDR3) ? 0 : para->dx_odt; > + val_hi = para->dx_odt; > + writel_relaxed(MASK_BYTE(val_lo, 0), SUNXI_DRAM_PHY0_BASE + 0x380); > + writel_relaxed(MASK_BYTE(val_hi, 0), SUNXI_DRAM_PHY0_BASE + 0x384); > + writel_relaxed(MASK_BYTE(val_lo, 1), SUNXI_DRAM_PHY0_BASE + 0x3c0); > + writel_relaxed(MASK_BYTE(val_hi, 1), SUNXI_DRAM_PHY0_BASE + 0x3c4); > + writel_relaxed(MASK_BYTE(val_lo, 2), SUNXI_DRAM_PHY0_BASE + 0x400); > + writel_relaxed(MASK_BYTE(val_hi, 2), SUNXI_DRAM_PHY0_BASE + 0x404); > + writel_relaxed(MASK_BYTE(val_lo, 3), SUNXI_DRAM_PHY0_BASE + 0x440); > + writel_relaxed(MASK_BYTE(val_hi, 3), SUNXI_DRAM_PHY0_BASE + 0x444); > > dmb(); > } >
