Hi,
On 3/22/22 07:11, Dominic Rath wrote:
> The K3 DDRSS used by AM64x and J721e (and probably J7200 in the future)
> used to define only two register ranges: one labeled "cfg" that was
> internally referenced as "DDRSS wrapper", and the LPDDR4 FSP handshake
> registers in the CTRL_MMR register space, labeled "ctrl_mmr_lp4".
> 
> The address specified as the "cfg" register space was actually the
> DDR controller's configuration registers, not the wrapper.
> 
> The "cfg" registers were mostly used correctly as the address passed
> to Cadence code that actually configured the DDR controller and
> subsystems, but the code also attempted to use this register space when
> it actually wanted to write the DDRSS_V2A_CTL_REG and DDRSS_ECC_CTRL_REG
> registers.
> 
> This patch adds a third register property "ddrss", uses that for the
> DDRSS wrapper, and continues to use the "cfg" property for the actual
> controller configuration registers.
> 
> The "cfg" register space was also configured too small at 16 KB.
> The TRMs say the range is 32KB, and the PHY registers actually start
> at the 16KB offset.
> 
> Signed-off-by: Dominic Rath <r...@ibv-augsburg.net>
> ---

I just introduced this same change last week here to add ecc support for
am64x:

https://lore.kernel.org/u-boot/20220317170346.31162-6-d-gerl...@ti.com/
https://lore.kernel.org/u-boot/20220317170346.31162-8-d-gerl...@ti.com/

Major difference in approach is that I made this an optional region, as
J721e does not require it.

Regards,
Dave


>   arch/arm/dts/k3-am64-ddr.dtsi   |  5 +++--
>   arch/arm/dts/k3-j721e-ddr.dtsi  |  5 +++--
>   drivers/ram/k3-ddrss/k3-ddrss.c | 12 ++++++++++--
>   3 files changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/dts/k3-am64-ddr.dtsi b/arch/arm/dts/k3-am64-ddr.dtsi
> index 026a547f0e..967bd55e75 100644
> --- a/arch/arm/dts/k3-am64-ddr.dtsi
> +++ b/arch/arm/dts/k3-am64-ddr.dtsi
> @@ -6,9 +6,10 @@
>   / {
>       memorycontroller: memorycontroller@f300000 {
>               compatible = "ti,am64-ddrss";
> -             reg = <0x00 0x0f308000 0x00 0x4000>,
> +             reg = <0x00 0x0f300000 0x00 0x200>,
> +                   <0x00 0x0f308000 0x00 0x8000>,
>                     <0x00 0x43014000 0x00 0x100>;
> -             reg-names = "cfg", "ctrl_mmr_lp4";
> +             reg-names = "ddrss", "cfg", "ctrl_mmr_lp4";
>               power-domains = <&k3_pds 138 TI_SCI_PD_SHARED>,
>                       <&k3_pds 55 TI_SCI_PD_SHARED>;
>               clocks = <&k3_clks 138 0>, <&k3_clks 16 4>;
> diff --git a/arch/arm/dts/k3-j721e-ddr.dtsi b/arch/arm/dts/k3-j721e-ddr.dtsi
> index 21d63802a5..8f1b792849 100644
> --- a/arch/arm/dts/k3-j721e-ddr.dtsi
> +++ b/arch/arm/dts/k3-j721e-ddr.dtsi
> @@ -6,9 +6,10 @@
>   / {
>       memorycontroller: memorycontroller@0298e000 {
>               compatible = "ti,j721e-ddrss";
> -             reg = <0x0 0x02990000 0x0 0x4000>,
> +             reg = <0x0 0x02980000 0x0 0x200>,
> +                   <0x0 0x02990000 0x0 0x8000>,
>                     <0x0 0x0114000 0x0 0x100>;
> -             reg-names = "cfg", "ctrl_mmr_lp4";
> +             reg-names = "ddrss", "cfg", "ctrl_mmr_lp4";
>               power-domains = <&k3_pds 47 TI_SCI_PD_SHARED>,
>                       <&k3_pds 90 TI_SCI_PD_SHARED>;
>               clocks = <&k3_clks 47 2>, <&k3_clks 30 9>;
> diff --git a/drivers/ram/k3-ddrss/k3-ddrss.c 
> b/drivers/ram/k3-ddrss/k3-ddrss.c
> index 261ba64506..7e2a4c5811 100644
> --- a/drivers/ram/k3-ddrss/k3-ddrss.c
> +++ b/drivers/ram/k3-ddrss/k3-ddrss.c
> @@ -105,6 +105,7 @@ struct k3_msmc {
>   struct k3_ddrss_desc {
>       struct udevice *dev;
>       void __iomem *ddrss_ss_cfg;
> +     void __iomem *ddrss_ctrl_cfg;
>       void __iomem *ddrss_ctrl_mmr;
>       struct power_domain ddrcfg_pwrdmn;
>       struct power_domain ddrdata_pwrdmn;
> @@ -314,13 +315,20 @@ static int k3_ddrss_ofdata_to_priv(struct udevice 
> *dev)
>       debug("%s(dev=%p)\n", __func__, dev);
>   -   reg = dev_read_addr_name(dev, "cfg");
> +     reg = dev_read_addr_name(dev, "ddrss");
>       if (reg == FDT_ADDR_T_NONE) {
>               dev_err(dev, "No reg property for DDRSS wrapper logic\n");
>               return -EINVAL;
>       }
>       ddrss->ddrss_ss_cfg = (void *)reg;
>   +   reg = dev_read_addr_name(dev, "cfg");
> +     if (reg == FDT_ADDR_T_NONE) {
> +             dev_err(dev, "No reg property for controller registers\n");
> +             return -EINVAL;
> +     }
> +     ddrss->ddrss_ctrl_cfg = (void *)reg;
> +
>       reg = dev_read_addr_name(dev, "ctrl_mmr_lp4");
>       if (reg == FDT_ADDR_T_NONE) {
>               dev_err(dev, "No reg property for CTRL MMR\n");
> @@ -403,7 +411,7 @@ void k3_lpddr4_init(struct k3_ddrss_desc *ddrss)
>               hang();
>       }
>   -   config->ctlbase = (struct lpddr4_ctlregs_s *)ddrss->ddrss_ss_cfg;
> +     config->ctlbase = (struct lpddr4_ctlregs_s *)ddrss->ddrss_ctrl_cfg;
>       config->infohandler = (lpddr4_infocallback) k3_lpddr4_info_handler;
>       status = driverdt->init(pd, config);

Reply via email to