| Message ID | 20221211163213.98540-9-jernej.skrabec@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Andre Przywara |
| Headers | show |
| Series | sunxi: Update H616 DRAM driver | expand |
Dne nedelja, 11. december 2022 ob 17:32:13 CET je Jernej Skrabec napisal(a): > Part of the code, previously known as "unknown feature" also doesn't > have constant values. They are derived from TPR0 parameter in vendor > DRAM code. Introduce that parameter here too, to ease adding new boards. > > Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com> > --- > .../include/asm/arch-sunxi/dram_sun50i_h616.h | 1 + > arch/arm/mach-sunxi/Kconfig | 6 ++++ > arch/arm/mach-sunxi/dram_sun50i_h616.c | 35 +++++++++++++++---- > 3 files changed, 35 insertions(+), 7 deletions(-) <snip> > diff --git a/arch/arm/mach-sunxi/dram_sun50i_h616.c > b/arch/arm/mach-sunxi/dram_sun50i_h616.c index df06cea42464..6d8f8d371bfe > 100644 > --- a/arch/arm/mach-sunxi/dram_sun50i_h616.c > +++ b/arch/arm/mach-sunxi/dram_sun50i_h616.c > @@ -808,15 +808,35 @@ static bool mctl_phy_init(struct dram_para *para) > writel(phy_init[i], &ptr[i]); > > if (para->tpr10 & TPR10_UNKNOWN_FEAT0) { > + if (para->tpr0 & BIT(30)) > + val = (para->tpr0 >> 7) & 0x3e; > + else > + val = (para->tpr10 >> 3) & 0x1e; > + > ptr = (u32 *)(SUNXI_DRAM_PHY0_BASE + 0x780); > for (i = 0; i < 32; i++) > - writel(0x16, &ptr[i]); > - writel(0xe, SUNXI_DRAM_PHY0_BASE + 0x78c); > - writel(0xe, SUNXI_DRAM_PHY0_BASE + 0x7a4); > - writel(0xe, SUNXI_DRAM_PHY0_BASE + 0x7b8); > - writel(0x8, SUNXI_DRAM_PHY0_BASE + 0x7d4); > - writel(0xe, SUNXI_DRAM_PHY0_BASE + 0x7dc); > - writel(0xe, SUNXI_DRAM_PHY0_BASE + 0x7e0); > + writel(val, &ptr[i]); > + > + val = (para->tpr10 << 1) & 0x1e; > + writel(val, SUNXI_DRAM_PHY0_BASE + 0x7dc); > + writel(val, SUNXI_DRAM_PHY0_BASE + 0x7e0); > + > + /* following configuration is DDR3 specific */ > + val = (para->tpr10 >> 7) & 0x1e; > + writel(val, SUNXI_DRAM_PHY0_BASE + 0x7d4); > + /* > + * TODO: Offsets 0x79c, 0x794 and 0x7e4 may need > + * to be set here. However, this doesn't seem to > + * be needed by any board seen in the wild for now. > + * It's not implemented because it would unnecessarily > + * introduce PARA2 and TPR2 options. > + */ I just noticed that PARA2 check actually checks rank. I think it's important to implement it (register 0x79c) and uses only TPR10 value, which is already present. Best regards, Jernej > + if (para->tpr0 & BIT(31)) { > + val = (para->tpr0 << 1) & 0x3e; > + writel(val, SUNXI_DRAM_PHY0_BASE + 0x78c); > + writel(val, SUNXI_DRAM_PHY0_BASE + 0x7a4); > + writel(val, SUNXI_DRAM_PHY0_BASE + 0x7b8); > + } > } > > writel(0x80, SUNXI_DRAM_PHY0_BASE + 0x3dc);
On Sun, 11 Dec 2022 17:32:13 +0100 Jernej Skrabec <jernej.skrabec@gmail.com> wrote: > Part of the code, previously known as "unknown feature" also doesn't > have constant values. They are derived from TPR0 parameter in vendor > DRAM code. Introduce that parameter here too, to ease adding new boards. That seems to also miss the value for the OPi Zero2 defconfig. > > Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com> > --- > .../include/asm/arch-sunxi/dram_sun50i_h616.h | 1 + > arch/arm/mach-sunxi/Kconfig | 6 ++++ > arch/arm/mach-sunxi/dram_sun50i_h616.c | 35 +++++++++++++++---- > 3 files changed, 35 insertions(+), 7 deletions(-) > > diff --git a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h616.h b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h616.h > index c7890c83391f..ff736bd88d10 100644 > --- a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h616.h > +++ b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h616.h > @@ -158,6 +158,7 @@ struct dram_para { > u32 dx_dri; > u32 ca_dri; > u32 odt_en; > + u32 tpr0; > u32 tpr10; > u32 tpr11; > u32 tpr12; > diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig > index b050f0a56971..7858a7045f7e 100644 > --- a/arch/arm/mach-sunxi/Kconfig > +++ b/arch/arm/mach-sunxi/Kconfig > @@ -73,6 +73,12 @@ config DRAM_SUN50I_H616_ODT_EN > help > ODT EN value from vendor DRAM settings. > > +config DRAM_SUN50I_H616_TPR0 > + hex "H616 DRAM TPR0 parameter" > + default 0x0 Is 0x0 really a feasible default value? I'd suggest we don't provide a default, so force people to be prompted for a value, if nothing is in (a new board's) defconfig. The rest looks OK, though I can't really comment on the actual bits - but who can anyway ;-) Cheers, Andre > + help > + TPR0 value from vendor DRAM settings. > + > config DRAM_SUN50I_H616_TPR10 > hex "H616 DRAM TPR10 parameter" > help > diff --git a/arch/arm/mach-sunxi/dram_sun50i_h616.c b/arch/arm/mach-sunxi/dram_sun50i_h616.c > index df06cea42464..6d8f8d371bfe 100644 > --- a/arch/arm/mach-sunxi/dram_sun50i_h616.c > +++ b/arch/arm/mach-sunxi/dram_sun50i_h616.c > @@ -808,15 +808,35 @@ static bool mctl_phy_init(struct dram_para *para) > writel(phy_init[i], &ptr[i]); > > if (para->tpr10 & TPR10_UNKNOWN_FEAT0) { > + if (para->tpr0 & BIT(30)) > + val = (para->tpr0 >> 7) & 0x3e; > + else > + val = (para->tpr10 >> 3) & 0x1e; > + > ptr = (u32 *)(SUNXI_DRAM_PHY0_BASE + 0x780); > for (i = 0; i < 32; i++) > - writel(0x16, &ptr[i]); > - writel(0xe, SUNXI_DRAM_PHY0_BASE + 0x78c); > - writel(0xe, SUNXI_DRAM_PHY0_BASE + 0x7a4); > - writel(0xe, SUNXI_DRAM_PHY0_BASE + 0x7b8); > - writel(0x8, SUNXI_DRAM_PHY0_BASE + 0x7d4); > - writel(0xe, SUNXI_DRAM_PHY0_BASE + 0x7dc); > - writel(0xe, SUNXI_DRAM_PHY0_BASE + 0x7e0); > + writel(val, &ptr[i]); > + > + val = (para->tpr10 << 1) & 0x1e; > + writel(val, SUNXI_DRAM_PHY0_BASE + 0x7dc); > + writel(val, SUNXI_DRAM_PHY0_BASE + 0x7e0); > + > + /* following configuration is DDR3 specific */ > + val = (para->tpr10 >> 7) & 0x1e; > + writel(val, SUNXI_DRAM_PHY0_BASE + 0x7d4); > + /* > + * TODO: Offsets 0x79c, 0x794 and 0x7e4 may need > + * to be set here. However, this doesn't seem to > + * be needed by any board seen in the wild for now. > + * It's not implemented because it would unnecessarily > + * introduce PARA2 and TPR2 options. > + */ > + if (para->tpr0 & BIT(31)) { > + val = (para->tpr0 << 1) & 0x3e; > + writel(val, SUNXI_DRAM_PHY0_BASE + 0x78c); > + writel(val, SUNXI_DRAM_PHY0_BASE + 0x7a4); > + writel(val, SUNXI_DRAM_PHY0_BASE + 0x7b8); > + } > } > > writel(0x80, SUNXI_DRAM_PHY0_BASE + 0x3dc); > @@ -1110,6 +1130,7 @@ unsigned long sunxi_dram_init(void) > .dx_dri = CONFIG_DRAM_SUN50I_H616_DX_DRI, > .ca_dri = CONFIG_DRAM_SUN50I_H616_CA_DRI, > .odt_en = CONFIG_DRAM_SUN50I_H616_ODT_EN, > + .tpr0 = CONFIG_DRAM_SUN50I_H616_TPR0, > .tpr10 = CONFIG_DRAM_SUN50I_H616_TPR10, > .tpr11 = CONFIG_DRAM_SUN50I_H616_TPR11, > .tpr12 = CONFIG_DRAM_SUN50I_H616_TPR12,
Dne sreda, 04. januar 2023 ob 01:38:12 CET je Andre Przywara napisal(a): > On Sun, 11 Dec 2022 17:32:13 +0100 > > Jernej Skrabec <jernej.skrabec@gmail.com> wrote: > > Part of the code, previously known as "unknown feature" also doesn't > > have constant values. They are derived from TPR0 parameter in vendor > > DRAM code. Introduce that parameter here too, to ease adding new boards. > > That seems to also miss the value for the OPi Zero2 defconfig. Again, Zero2 doesn't use all features. > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com> > > --- > > > > .../include/asm/arch-sunxi/dram_sun50i_h616.h | 1 + > > arch/arm/mach-sunxi/Kconfig | 6 ++++ > > arch/arm/mach-sunxi/dram_sun50i_h616.c | 35 +++++++++++++++---- > > 3 files changed, 35 insertions(+), 7 deletions(-) > > > > diff --git a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h616.h > > b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h616.h index > > c7890c83391f..ff736bd88d10 100644 > > --- a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h616.h > > +++ b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h616.h > > @@ -158,6 +158,7 @@ struct dram_para { > > > > u32 dx_dri; > > u32 ca_dri; > > u32 odt_en; > > > > + u32 tpr0; > > > > u32 tpr10; > > u32 tpr11; > > u32 tpr12; > > > > diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig > > index b050f0a56971..7858a7045f7e 100644 > > --- a/arch/arm/mach-sunxi/Kconfig > > +++ b/arch/arm/mach-sunxi/Kconfig > > @@ -73,6 +73,12 @@ config DRAM_SUN50I_H616_ODT_EN > > > > help > > > > ODT EN value from vendor DRAM settings. > > > > +config DRAM_SUN50I_H616_TPR0 > > + hex "H616 DRAM TPR0 parameter" > > + default 0x0 > > Is 0x0 really a feasible default value? I'd suggest we don't provide a > default, so force people to be prompted for a value, if nothing is in > (a new board's) defconfig. This is not used on all boards, so imo is ok to have it as default. Best regards, Jernej > > The rest looks OK, though I can't really comment on the actual bits - > but who can anyway ;-) > > Cheers, > Andre > > > + help > > + TPR0 value from vendor DRAM settings. > > + > > > > config DRAM_SUN50I_H616_TPR10 > > > > hex "H616 DRAM TPR10 parameter" > > help > > > > diff --git a/arch/arm/mach-sunxi/dram_sun50i_h616.c > > b/arch/arm/mach-sunxi/dram_sun50i_h616.c index df06cea42464..6d8f8d371bfe > > 100644 > > --- a/arch/arm/mach-sunxi/dram_sun50i_h616.c > > +++ b/arch/arm/mach-sunxi/dram_sun50i_h616.c > > @@ -808,15 +808,35 @@ static bool mctl_phy_init(struct dram_para *para) > > > > writel(phy_init[i], &ptr[i]); > > > > if (para->tpr10 & TPR10_UNKNOWN_FEAT0) { > > > > + if (para->tpr0 & BIT(30)) > > + val = (para->tpr0 >> 7) & 0x3e; > > + else > > + val = (para->tpr10 >> 3) & 0x1e; > > + > > > > ptr = (u32 *)(SUNXI_DRAM_PHY0_BASE + 0x780); > > for (i = 0; i < 32; i++) > > > > - writel(0x16, &ptr[i]); > > - writel(0xe, SUNXI_DRAM_PHY0_BASE + 0x78c); > > - writel(0xe, SUNXI_DRAM_PHY0_BASE + 0x7a4); > > - writel(0xe, SUNXI_DRAM_PHY0_BASE + 0x7b8); > > - writel(0x8, SUNXI_DRAM_PHY0_BASE + 0x7d4); > > - writel(0xe, SUNXI_DRAM_PHY0_BASE + 0x7dc); > > - writel(0xe, SUNXI_DRAM_PHY0_BASE + 0x7e0); > > + writel(val, &ptr[i]); > > + > > + val = (para->tpr10 << 1) & 0x1e; > > + writel(val, SUNXI_DRAM_PHY0_BASE + 0x7dc); > > + writel(val, SUNXI_DRAM_PHY0_BASE + 0x7e0); > > + > > + /* following configuration is DDR3 specific */ > > + val = (para->tpr10 >> 7) & 0x1e; > > + writel(val, SUNXI_DRAM_PHY0_BASE + 0x7d4); > > + /* > > + * TODO: Offsets 0x79c, 0x794 and 0x7e4 may need > > + * to be set here. However, this doesn't seem to > > + * be needed by any board seen in the wild for now. > > + * It's not implemented because it would unnecessarily > > + * introduce PARA2 and TPR2 options. > > + */ > > + if (para->tpr0 & BIT(31)) { > > + val = (para->tpr0 << 1) & 0x3e; > > + writel(val, SUNXI_DRAM_PHY0_BASE + 0x78c); > > + writel(val, SUNXI_DRAM_PHY0_BASE + 0x7a4); > > + writel(val, SUNXI_DRAM_PHY0_BASE + 0x7b8); > > + } > > > > } > > > > writel(0x80, SUNXI_DRAM_PHY0_BASE + 0x3dc); > > > > @@ -1110,6 +1130,7 @@ unsigned long sunxi_dram_init(void) > > > > .dx_dri = CONFIG_DRAM_SUN50I_H616_DX_DRI, > > .ca_dri = CONFIG_DRAM_SUN50I_H616_CA_DRI, > > .odt_en = CONFIG_DRAM_SUN50I_H616_ODT_EN, > > > > + .tpr0 = CONFIG_DRAM_SUN50I_H616_TPR0, > > > > .tpr10 = CONFIG_DRAM_SUN50I_H616_TPR10, > > .tpr11 = CONFIG_DRAM_SUN50I_H616_TPR11, > > .tpr12 = CONFIG_DRAM_SUN50I_H616_TPR12,
diff --git a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h616.h b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h616.h index c7890c83391f..ff736bd88d10 100644 --- a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h616.h +++ b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h616.h @@ -158,6 +158,7 @@ struct dram_para { u32 dx_dri; u32 ca_dri; u32 odt_en; + u32 tpr0; u32 tpr10; u32 tpr11; u32 tpr12; diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig index b050f0a56971..7858a7045f7e 100644 --- a/arch/arm/mach-sunxi/Kconfig +++ b/arch/arm/mach-sunxi/Kconfig @@ -73,6 +73,12 @@ config DRAM_SUN50I_H616_ODT_EN help ODT EN value from vendor DRAM settings. +config DRAM_SUN50I_H616_TPR0 + hex "H616 DRAM TPR0 parameter" + default 0x0 + help + TPR0 value from vendor DRAM settings. + config DRAM_SUN50I_H616_TPR10 hex "H616 DRAM TPR10 parameter" help diff --git a/arch/arm/mach-sunxi/dram_sun50i_h616.c b/arch/arm/mach-sunxi/dram_sun50i_h616.c index df06cea42464..6d8f8d371bfe 100644 --- a/arch/arm/mach-sunxi/dram_sun50i_h616.c +++ b/arch/arm/mach-sunxi/dram_sun50i_h616.c @@ -808,15 +808,35 @@ static bool mctl_phy_init(struct dram_para *para) writel(phy_init[i], &ptr[i]); if (para->tpr10 & TPR10_UNKNOWN_FEAT0) { + if (para->tpr0 & BIT(30)) + val = (para->tpr0 >> 7) & 0x3e; + else + val = (para->tpr10 >> 3) & 0x1e; + ptr = (u32 *)(SUNXI_DRAM_PHY0_BASE + 0x780); for (i = 0; i < 32; i++) - writel(0x16, &ptr[i]); - writel(0xe, SUNXI_DRAM_PHY0_BASE + 0x78c); - writel(0xe, SUNXI_DRAM_PHY0_BASE + 0x7a4); - writel(0xe, SUNXI_DRAM_PHY0_BASE + 0x7b8); - writel(0x8, SUNXI_DRAM_PHY0_BASE + 0x7d4); - writel(0xe, SUNXI_DRAM_PHY0_BASE + 0x7dc); - writel(0xe, SUNXI_DRAM_PHY0_BASE + 0x7e0); + writel(val, &ptr[i]); + + val = (para->tpr10 << 1) & 0x1e; + writel(val, SUNXI_DRAM_PHY0_BASE + 0x7dc); + writel(val, SUNXI_DRAM_PHY0_BASE + 0x7e0); + + /* following configuration is DDR3 specific */ + val = (para->tpr10 >> 7) & 0x1e; + writel(val, SUNXI_DRAM_PHY0_BASE + 0x7d4); + /* + * TODO: Offsets 0x79c, 0x794 and 0x7e4 may need + * to be set here. However, this doesn't seem to + * be needed by any board seen in the wild for now. + * It's not implemented because it would unnecessarily + * introduce PARA2 and TPR2 options. + */ + if (para->tpr0 & BIT(31)) { + val = (para->tpr0 << 1) & 0x3e; + writel(val, SUNXI_DRAM_PHY0_BASE + 0x78c); + writel(val, SUNXI_DRAM_PHY0_BASE + 0x7a4); + writel(val, SUNXI_DRAM_PHY0_BASE + 0x7b8); + } } writel(0x80, SUNXI_DRAM_PHY0_BASE + 0x3dc); @@ -1110,6 +1130,7 @@ unsigned long sunxi_dram_init(void) .dx_dri = CONFIG_DRAM_SUN50I_H616_DX_DRI, .ca_dri = CONFIG_DRAM_SUN50I_H616_CA_DRI, .odt_en = CONFIG_DRAM_SUN50I_H616_ODT_EN, + .tpr0 = CONFIG_DRAM_SUN50I_H616_TPR0, .tpr10 = CONFIG_DRAM_SUN50I_H616_TPR10, .tpr11 = CONFIG_DRAM_SUN50I_H616_TPR11, .tpr12 = CONFIG_DRAM_SUN50I_H616_TPR12,
Part of the code, previously known as "unknown feature" also doesn't have constant values. They are derived from TPR0 parameter in vendor DRAM code. Introduce that parameter here too, to ease adding new boards. Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com> --- .../include/asm/arch-sunxi/dram_sun50i_h616.h | 1 + arch/arm/mach-sunxi/Kconfig | 6 ++++ arch/arm/mach-sunxi/dram_sun50i_h616.c | 35 +++++++++++++++---- 3 files changed, 35 insertions(+), 7 deletions(-)