| Message ID | 20230914102038.2944844-2-xu.yang_2@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] perf: imx9_ddr_perf: resolve resource map conflict | expand |
On 14/09/2023 12:20, Xu Yang wrote: > Add DDR controller node which will be used by EDAC driver later, also > move the DDR PMU node as the subnode of the DDR controller. > > Signed-off-by: Ye Li <ye.li@nxp.com> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com> > --- > arch/arm64/boot/dts/freescale/imx93.dtsi | 18 ++++++++++++++---- > 1 file changed, 14 insertions(+), 4 deletions(-) > > diff --git a/arch/arm64/boot/dts/freescale/imx93.dtsi b/arch/arm64/boot/dts/freescale/imx93.dtsi > index 6f85a05ee7e1..992bdeef70cd 100644 > --- a/arch/arm64/boot/dts/freescale/imx93.dtsi > +++ b/arch/arm64/boot/dts/freescale/imx93.dtsi > @@ -917,10 +917,20 @@ media_blk_ctrl: system-controller@4ac10000 { > status = "disabled"; > }; > > - ddr-pmu@4e300dc0 { > - compatible = "fsl,imx93-ddr-pmu"; > - reg = <0x4e300dc0 0x200>; > - interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>; > + ddr: memory-controller@4e300000 { > + compatible = "simple-mfd"; No, that's not allowed alone. It does not look like you tested the DTS against bindings. Please run `make dtbs_check W=1` (see Documentation/devicetree/bindings/writing-schema.rst or https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/ for instructions). Best regards, Krzysztof
> -----Original Message----- > From: Xu Yang <xu.yang_2@nxp.com> > Sent: Thursday, September 14, 2023 5:21 AM > To: will@kernel.org; mark.rutland@arm.com; robh+dt@kernel.org; > shawnguo@kernel.org > Cc: krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org; dl-linux-imx > <linux-imx@nxp.com>; devicetree@vger.kernel.org; linux-arm- > kernel@lists.infradead.org; Ye Li <ye.li@nxp.com> > Subject: [EXT] [PATCH 2/2] arm64: dts: imx93: add DDR controller node > > Caution: This is an external email. Please take care when clicking links or > opening attachments. When in doubt, report the message using the 'Report > this email' button > > > Add DDR controller node which will be used by EDAC driver later, also > move the DDR PMU node as the subnode of the DDR controller. > > Signed-off-by: Ye Li <ye.li@nxp.com> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com> > --- > arch/arm64/boot/dts/freescale/imx93.dtsi | 18 ++++++++++++++---- > 1 file changed, 14 insertions(+), 4 deletions(-) > > diff --git a/arch/arm64/boot/dts/freescale/imx93.dtsi > b/arch/arm64/boot/dts/freescale/imx93.dtsi > index 6f85a05ee7e1..992bdeef70cd 100644 > --- a/arch/arm64/boot/dts/freescale/imx93.dtsi > +++ b/arch/arm64/boot/dts/freescale/imx93.dtsi > @@ -917,10 +917,20 @@ media_blk_ctrl: system-controller@4ac10000 { > status = "disabled"; > }; > > - ddr-pmu@4e300dc0 { > - compatible = "fsl,imx93-ddr-pmu"; > - reg = <0x4e300dc0 0x200>; > - interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>; > + ddr: memory-controller@4e300000 { > + compatible = "simple-mfd"; > + reg = <0x4e300000 0x2000>; [Frank Li] Can you just use EDAC register space size? I suppose EDAC and PMU's register space is not over lapped. > + interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>; > + little-endian; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + > + ddr-pmu@4e300dc0 { > + compatible = "fsl,imx93-ddr-pmu"; > + reg = <0x4e300dc0 0x200>; > + interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>; > + }; > }; > }; > }; > -- > 2.34.1 >
Hi Frank, > -----Original Message----- > From: Frank Li <frank.li@nxp.com> > Sent: Thursday, September 14, 2023 10:21 PM > To: Xu Yang <xu.yang_2@nxp.com>; will@kernel.org; mark.rutland@arm.com; > robh+dt@kernel.org; shawnguo@kernel.org > Cc: krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org; dl-linux-imx > <linux-imx@nxp.com>; devicetree@vger.kernel.org; > linux-arm-kernel@lists.infradead.org; Ye Li <ye.li@nxp.com> > Subject: RE: [EXT] [PATCH 2/2] arm64: dts: imx93: add DDR controller node > > > > > -----Original Message----- > > From: Xu Yang <xu.yang_2@nxp.com> > > Sent: Thursday, September 14, 2023 5:21 AM > > To: will@kernel.org; mark.rutland@arm.com; robh+dt@kernel.org; > > shawnguo@kernel.org > > Cc: krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org; > > dl-linux-imx <linux-imx@nxp.com>; devicetree@vger.kernel.org; > > linux-arm- kernel@lists.infradead.org; Ye Li <ye.li@nxp.com> > > Subject: [EXT] [PATCH 2/2] arm64: dts: imx93: add DDR controller node > > > > Caution: This is an external email. Please take care when clicking > > links or opening attachments. When in doubt, report the message using > > the 'Report this email' button > > > > > > Add DDR controller node which will be used by EDAC driver later, also > > move the DDR PMU node as the subnode of the DDR controller. > > > > Signed-off-by: Ye Li <ye.li@nxp.com> > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com> > > --- > > arch/arm64/boot/dts/freescale/imx93.dtsi | 18 ++++++++++++++---- > > 1 file changed, 14 insertions(+), 4 deletions(-) > > > > diff --git a/arch/arm64/boot/dts/freescale/imx93.dtsi > > b/arch/arm64/boot/dts/freescale/imx93.dtsi > > index 6f85a05ee7e1..992bdeef70cd 100644 > > --- a/arch/arm64/boot/dts/freescale/imx93.dtsi > > +++ b/arch/arm64/boot/dts/freescale/imx93.dtsi > > @@ -917,10 +917,20 @@ media_blk_ctrl: system-controller@4ac10000 { > > status = "disabled"; > > }; > > > > - ddr-pmu@4e300dc0 { > > - compatible = "fsl,imx93-ddr-pmu"; > > - reg = <0x4e300dc0 0x200>; > > - interrupts = <GIC_SPI 90 > IRQ_TYPE_LEVEL_HIGH>; > > + ddr: memory-controller@4e300000 { > > + compatible = "simple-mfd"; > > + reg = <0x4e300000 0x2000>; > > [Frank Li] Can you just use EDAC register space size? > I suppose EDAC and PMU's register space is not over lapped. > We will re-use layerscape EDAC driver which needs DDR controller's base address. Best regards, Ye Li > > + interrupts = <GIC_SPI 91 > IRQ_TYPE_LEVEL_HIGH>; > > + little-endian; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + ranges; > > + > > + ddr-pmu@4e300dc0 { > > + compatible = "fsl,imx93-ddr-pmu"; > > + reg = <0x4e300dc0 0x200>; > > + interrupts = <GIC_SPI 90 > IRQ_TYPE_LEVEL_HIGH>; > > + }; > > }; > > }; > > }; > > -- > > 2.34.1 > >
Hi Krzysztof, > On 14/09/2023 12:20, Xu Yang wrote: > > Add DDR controller node which will be used by EDAC driver later, also > > move the DDR PMU node as the subnode of the DDR controller. > > > > Signed-off-by: Ye Li <ye.li@nxp.com> > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com> > > --- > > arch/arm64/boot/dts/freescale/imx93.dtsi | 18 ++++++++++++++---- > > 1 file changed, 14 insertions(+), 4 deletions(-) > > > > diff --git a/arch/arm64/boot/dts/freescale/imx93.dtsi b/arch/arm64/boot/dts/freescale/imx93.dtsi > > index 6f85a05ee7e1..992bdeef70cd 100644 > > --- a/arch/arm64/boot/dts/freescale/imx93.dtsi > > +++ b/arch/arm64/boot/dts/freescale/imx93.dtsi > > @@ -917,10 +917,20 @@ media_blk_ctrl: system-controller@4ac10000 { > > status = "disabled"; > > }; > > > > - ddr-pmu@4e300dc0 { > > - compatible = "fsl,imx93-ddr-pmu"; > > - reg = <0x4e300dc0 0x200>; > > - interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>; > > + ddr: memory-controller@4e300000 { > > + compatible = "simple-mfd"; > > No, that's not allowed alone. Well, then how should I modify this compatible? > > It does not look like you tested the DTS against bindings. Please run > `make dtbs_check W=1` > I just run the check script, it seems no warnings for this node. $ dt-validate -s Documentation/devicetree/bindings/processed-schema.json arch/arm64/boot/dts/freescale/imx93-11x11-evk.dtb /home/nxf75279/work/linux-next/arch/arm64/boot/dts/freescale/imx93-11x11-evk.dtb: tmu@44482000: fsl,tmu-range: 'oneOf' conditional failed, one must be fixed: [2147483866, 2147483881, 2147483906, 2147483946, 2147484006, 2147484071, 2147484086] is too long From schema: /home/nxf75279/work/linux-next/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml /home/nxf75279/work/linux-next/arch/arm64/boot/dts/freescale/imx93-11x11-evk.dtb: tmu@44482000: 'oneOf' conditional failed, one must be fixed: 'interrupts' is a required property 'interrupts-extended' is a required property From schema: /home/nxf75279/work/linux-next/Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml /home/nxf75279/work/linux-next/arch/arm64/boot/dts/freescale/imx93-11x11-evk.dtb: gpio@43820080: gpio-ranges: [[30, 0, 84, 8], [30, 8, 66, 18], [30, 26, 34, 2], [30, 28, 0, 4]] is too long From schema: /home/nxf75279/work/linux-next/Documentation/devicetree/bindings/gpio/gpio-vf610.yaml /home/nxf75279/work/linux-next/arch/arm64/boot/dts/freescale/imx93-11x11-evk.dtb: gpio@43830080: gpio-ranges: [[30, 0, 38, 28], [30, 28, 36, 2]] is too long From schema: /home/nxf75279/work/linux-next/Documentation/devicetree/bindings/gpio/gpio-vf610.yaml Thanks, Xu Yang
On 15/09/2023 04:39, Xu Yang wrote: > Hi Krzysztof, > >> On 14/09/2023 12:20, Xu Yang wrote: >>> Add DDR controller node which will be used by EDAC driver later, also >>> move the DDR PMU node as the subnode of the DDR controller. >>> >>> Signed-off-by: Ye Li <ye.li@nxp.com> >>> Signed-off-by: Xu Yang <xu.yang_2@nxp.com> >>> --- >>> arch/arm64/boot/dts/freescale/imx93.dtsi | 18 ++++++++++++++---- >>> 1 file changed, 14 insertions(+), 4 deletions(-) >>> >>> diff --git a/arch/arm64/boot/dts/freescale/imx93.dtsi b/arch/arm64/boot/dts/freescale/imx93.dtsi >>> index 6f85a05ee7e1..992bdeef70cd 100644 >>> --- a/arch/arm64/boot/dts/freescale/imx93.dtsi >>> +++ b/arch/arm64/boot/dts/freescale/imx93.dtsi >>> @@ -917,10 +917,20 @@ media_blk_ctrl: system-controller@4ac10000 { >>> status = "disabled"; >>> }; >>> >>> - ddr-pmu@4e300dc0 { >>> - compatible = "fsl,imx93-ddr-pmu"; >>> - reg = <0x4e300dc0 0x200>; >>> - interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>; >>> + ddr: memory-controller@4e300000 { >>> + compatible = "simple-mfd"; >> >> No, that's not allowed alone. > > Well, then how should I modify this compatible? You need specific compatible, just like everywhere else. You can use "git grep simple-mfd" or "git grep simple-mfd -- arch/arm*" or on bindings to figure this out. Just remember that simple-mfd means children do not depend on anything provided by the parent. You cannot later remove it "just because" or "oh, now I want driver". That would affect users of DTS. > >> >> It does not look like you tested the DTS against bindings. Please run >> `make dtbs_check W=1` >> > > I just run the check script, it seems no warnings for this node. Right, I always forget we did not convert simple-mfd to DT schema, so the warnings are only for syscon. Best regards, Krzysztof
> On 15/09/2023 04:39, Xu Yang wrote: > > Hi Krzysztof, > > > >> On 14/09/2023 12:20, Xu Yang wrote: > >>> Add DDR controller node which will be used by EDAC driver later, also > >>> move the DDR PMU node as the subnode of the DDR controller. > >>> > >>> Signed-off-by: Ye Li <ye.li@nxp.com> > >>> Signed-off-by: Xu Yang <xu.yang_2@nxp.com> > >>> --- > >>> arch/arm64/boot/dts/freescale/imx93.dtsi | 18 ++++++++++++++---- > >>> 1 file changed, 14 insertions(+), 4 deletions(-) > >>> > >>> diff --git a/arch/arm64/boot/dts/freescale/imx93.dtsi b/arch/arm64/boot/dts/freescale/imx93.dtsi > >>> index 6f85a05ee7e1..992bdeef70cd 100644 > >>> --- a/arch/arm64/boot/dts/freescale/imx93.dtsi > >>> +++ b/arch/arm64/boot/dts/freescale/imx93.dtsi > >>> @@ -917,10 +917,20 @@ media_blk_ctrl: system-controller@4ac10000 { > >>> status = "disabled"; > >>> }; > >>> > >>> - ddr-pmu@4e300dc0 { > >>> - compatible = "fsl,imx93-ddr-pmu"; > >>> - reg = <0x4e300dc0 0x200>; > >>> - interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>; > >>> + ddr: memory-controller@4e300000 { > >>> + compatible = "simple-mfd"; > >> > >> No, that's not allowed alone. > > > > Well, then how should I modify this compatible? > > You need specific compatible, just like everywhere else. You can use > "git grep simple-mfd" or "git grep simple-mfd -- arch/arm*" or on > bindings to figure this out. > > Just remember that simple-mfd means children do not depend on anything > provided by the parent. You cannot later remove it "just because" or > "oh, now I want driver". That would affect users of DTS. I see. Thanks for your explanation. Best Regards, Xu Yang > > > > >> > >> It does not look like you tested the DTS against bindings. Please run > >> `make dtbs_check W=1` > >> > > > > I just run the check script, it seems no warnings for this node. > > Right, I always forget we did not convert simple-mfd to DT schema, so > the warnings are only for syscon. > > Best regards, > Krzysztof
diff --git a/arch/arm64/boot/dts/freescale/imx93.dtsi b/arch/arm64/boot/dts/freescale/imx93.dtsi index 6f85a05ee7e1..992bdeef70cd 100644 --- a/arch/arm64/boot/dts/freescale/imx93.dtsi +++ b/arch/arm64/boot/dts/freescale/imx93.dtsi @@ -917,10 +917,20 @@ media_blk_ctrl: system-controller@4ac10000 { status = "disabled"; }; - ddr-pmu@4e300dc0 { - compatible = "fsl,imx93-ddr-pmu"; - reg = <0x4e300dc0 0x200>; - interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>; + ddr: memory-controller@4e300000 { + compatible = "simple-mfd"; + reg = <0x4e300000 0x2000>; + interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>; + little-endian; + #address-cells = <1>; + #size-cells = <1>; + ranges; + + ddr-pmu@4e300dc0 { + compatible = "fsl,imx93-ddr-pmu"; + reg = <0x4e300dc0 0x200>; + interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>; + }; }; }; };