EDAC on imx8mp and ddr scaling

[Michael Nazzareno Trimarchi]

Hi Sherry

On Tue, Feb 11, 2025 at 7:34 AM Sherry Sun <sherry.sun at nxp.com> wrote:
>
> > -----Original Message-----
> > From: Michael Nazzareno Trimarchi <michael at amarulasolutions.com>
> > Sent: Friday, February 7, 2025 5:37 PM
> > To: Shawn Guo <shawnguo at kernel.org>; Sherry Sun <sherry.sun at nxp.com>
> > Cc: linux-arm-kernel <linux-arm-kernel at lists.infradead.org>
> > Subject: EDAC on imx8mp and ddr scaling
> >
> > Hi Sherry and Shawn
> >
> > I have seen that with this patch "arm64: dts: imx8mp: add ddr controller node
> > to support EDAC on imx8mp" but on the same time the ddr scaling is coming
> > from arm_smccc_smc DVFS frequency scaling that is managed from imx8m-
> > ddrc. We have tested it a bit to understand if the circle to have the code call
> > from linux to ATF was somehow working. We don't check other than small
> > things.
> > If the imx8mp is compatible with imx8m-ddrc and synopsys memory
> > controller, should not needed to merge the support in the synopsys
> > connected to the board platform data?
> >
>
> Hi Michael,
>
> The imx8m-ddrc you reference here is totally different thing with the
> synopsys_edac that my patch adds.
> imx8m-ddrc targets dynamic frequency scaling of DRAM, while synopsys_edac
> targets DDR ECC feature(error detection and correction).
>

Ok, that is clear but both insist on the same object, the memory
controller. Now I need
to check better but the memory controller is de-facto coordinate by the ATF in:
- memory frequency scaling
- suspend/resume

At the moment I don;t know if  I can consider them independent. What
II have done quick is:

diff --git a/drivers/clk/imx/clk-imx8mp.c b/drivers/clk/imx/clk-imx8mp.c
index 5db541f0eaf1..e923d789757c 100644
--- a/drivers/clk/imx/clk-imx8mp.c
+++ b/drivers/clk/imx/clk-imx8mp.c
@@ -556,8 +556,14 @@ static int imx8mp_clocks_probe(struct
platform_device *pdev)

  hws[IMX8MP_CLK_IPG_ROOT] = imx_clk_hw_divider2("ipg_root",
"ahb_root", ccm_base + 0x9080, 0, 1);

- hws[IMX8MP_CLK_DRAM_ALT] = imx8m_clk_hw_composite("dram_alt",
imx8mp_dram_alt_sels, ccm_base + 0xa000);
- hws[IMX8MP_CLK_DRAM_APB] =
imx8m_clk_hw_composite_critical("dram_apb", imx8mp_dram_apb_sels,
ccm_base + 0xa080);
+ /*
+         * DRAM clocks are manipulated from TF-A outside clock framework.
+         * The fw_managed helper sets GET_RATE_NOCACHE and clears
SET_PARENT_GATE
+         * as div value should always be read from hardware
+         */
+ hws[IMX8MP_CLK_DRAM_ALT] =
imx8m_clk_hw_fw_managed_composite("dram_alt", imx8mp_dram_alt_sels,
ccm_base + 0xa000);
+ hws[IMX8MP_CLK_DRAM_APB] =
imx8m_clk_hw_fw_managed_composite_critical("dram_apb",
imx8mp_dram_apb_sels, ccm_base + 0xa080);
+
  hws[IMX8MP_CLK_VPU_G1] = imx8m_clk_hw_composite("vpu_g1",
imx8mp_vpu_g1_sels, ccm_base + 0xa100);
  hws[IMX8MP_CLK_VPU_G2] = imx8m_clk_hw_composite("vpu_g2",
imx8mp_vpu_g2_sels, ccm_base + 0xa180);
  hws[IMX8MP_CLK_CAN1] = imx8m_clk_hw_composite("can1",
imx8mp_can1_sels, ccm_base + 0xa200);

And in the board:

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp.dtsi
b/arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp.dtsi
index ffd3672255c0..f96a9d9f64d1 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp.dtsi
@@ -194,3 +194,23 @@ &edacmc {
  <&clk IMX8MP_CLK_DRAM_ALT>,
  <&clk IMX8MP_CLK_DRAM_APB>;
 };
+
+&edacmc {
+ operating-points-v2 = <&ddrc_opp_table>;
+
+ ddrc_opp_table: opp-table {
+ compatible = "operating-points-v2";
+
+ opp-25000000 {
+ opp-hz = /bits/ 64 <25000000>;
+ };
+
+ opp-100000000 {
+ opp-hz = /bits/ 64 <100000000>;
+ };
+
+ opp-900000000 {
+ opp-hz = /bits/ 64 <900000000>;
+ };
+ };
+};

+
+&edacmc {
+ compatible = "fsl,imx8mm-ddrc", "fsl,imx8m-ddrc";
+ reg = <0x3d400000 0x400000>;
+ clock-names = "core", "pll", "alt", "apb";
+ clocks = <&clk IMX8MP_CLK_DRAM_CORE>,
+ <&clk IMX8MP_DRAM_PLL>,
+ <&clk IMX8MP_CLK_DRAM_ALT>,
+ <&clk IMX8MP_CLK_DRAM_APB>;
+};

I have just a quick copy and paste in the email. This gives me a quick
idea of the status. So from one side
I have on reg = <0x3d400000 0x400000> the imx8m-ddrc driver and on the
other the synopsys. Do you have a plan
on how those two functions can be coordinated with each other?

Michael


> Best Regards
> Sherry
>
> > Best regards
> > Michael



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael at amarulasolutions.com
__________________________________

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info at amarulasolutions.com
www.amarulasolutions.com