diff mbox series

ARM: dts: imx7s: change i2c compatible string for applying errata ERR007805

Message ID 20241021031148.2682015-1-carlos.song@nxp.com (mailing list archive)
State New
Headers show
Series ARM: dts: imx7s: change i2c compatible string for applying errata ERR007805 | expand

Commit Message

Carlos Song Oct. 21, 2024, 3:11 a.m. UTC 
From: Carlos Song <carlos.song@nxp.com>

Compatible string "fsl,imx7d-i2c" is not exited at i2c-imx driver
compatible string table, at the result, "fsl,imx21-i2c" will be
matched, but it will cause errata ERR007805 not be applied in fact.

Replace "fsl,imx7d-i2c" by "fsl,imx7s-i2c" compatible string in dts
file for appling the errata ERR007805.

Signed-off-by: Carlos Song <carlos.song@nxp.com>
Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 arch/arm/boot/dts/nxp/imx/imx7s.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Francesco Dolcini Oct. 21, 2024, 9:27 a.m. UTC | #1 
Hello Carlos,

On Mon, Oct 21, 2024 at 11:11:48AM +0800, carlos.song@nxp.com wrote:
> From: Carlos Song <carlos.song@nxp.com>
> 
> Compatible string "fsl,imx7d-i2c" is not exited at i2c-imx driver
> compatible string table, at the result, "fsl,imx21-i2c" will be
> matched, but it will cause errata ERR007805 not be applied in fact.
> 
> Replace "fsl,imx7d-i2c" by "fsl,imx7s-i2c" compatible string in dts
> file for appling the errata ERR007805.

Did you considered changing the driver instead? If I understand this
correctly it would be a better solution.

Francesco
Carlos Song Oct. 21, 2024, 10:07 a.m. UTC | #2 
> -----Original Message-----
> From: Francesco Dolcini <francesco@dolcini.it>
> Sent: Monday, October 21, 2024 5:28 PM
> To: Carlos Song <carlos.song@nxp.com>
> Cc: robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org;
> shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> festevam@gmail.com; Frank Li <frank.li@nxp.com>; Bough Chen
> <haibo.chen@nxp.com>; devicetree@vger.kernel.org; imx@lists.linux.dev;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: [EXT] Re: [PATCH] ARM: dts: imx7s: change i2c compatible string for
> applying errata ERR007805
> 
> 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
> 
> 
> Hello Carlos,
> 
> On Mon, Oct 21, 2024 at 11:11:48AM +0800, carlos.song@nxp.com wrote:
> > From: Carlos Song <carlos.song@nxp.com>
> >
> > Compatible string "fsl,imx7d-i2c" is not exited at i2c-imx driver
> > compatible string table, at the result, "fsl,imx21-i2c" will be
> > matched, but it will cause errata ERR007805 not be applied in fact.
> >
> > Replace "fsl,imx7d-i2c" by "fsl,imx7s-i2c" compatible string in dts
> > file for appling the errata ERR007805.
> 
> Did you considered changing the driver instead? If I understand this correctly it
> would be a better solution.
> 
> Francesco

Hi, Francesco

Thank you for your comment.

This errata fix patch has been applied on i2c-imx.c driver:
39c025721d70 i2c: imx: Implement errata ERR007805 or e7805 bus frequency limit

The patch commit log says: this errata is found on all MX7{S,D}, MX6{UL{,L,Z},S{,LL,X},S,D,DL,Q,DP,QP} and MX8M{M,N,P,Q}.
So imx7d and imx7s both needs this errata fix. "fsl,imx7s-i2c" was added into compatible string table in the i2c-imx patch, this is the patch diff snap.
So when "fsl,imx7s-i2c" is matched, the arrata fix will be applied.

@@ -266,6 +282,16 @@ MODULE_DEVICE_TABLE(platform, imx_i2c_devtype);
 static const struct of_device_id i2c_imx_dt_ids[] = {
        { .compatible = "fsl,imx1-i2c", .data = &imx1_i2c_hwdata, },
        { .compatible = "fsl,imx21-i2c", .data = &imx21_i2c_hwdata, },
+       { .compatible = "fsl,imx6q-i2c", .data = &imx6_i2c_hwdata, },
+       { .compatible = "fsl,imx6sl-i2c", .data = &imx6_i2c_hwdata, },
+       { .compatible = "fsl,imx6sll-i2c", .data = &imx6_i2c_hwdata, },
+       { .compatible = "fsl,imx6sx-i2c", .data = &imx6_i2c_hwdata, },
+       { .compatible = "fsl,imx6ul-i2c", .data = &imx6_i2c_hwdata, },
+       { .compatible = "fsl,imx7s-i2c", .data = &imx6_i2c_hwdata, },
+       { .compatible = "fsl,imx8mm-i2c", .data = &imx6_i2c_hwdata, },
+       { .compatible = "fsl,imx8mn-i2c", .data = &imx6_i2c_hwdata, },
+       { .compatible = "fsl,imx8mp-i2c", .data = &imx6_i2c_hwdata, },
+       { .compatible = "fsl,imx8mq-i2c", .data = &imx6_i2c_hwdata, },
        { .compatible = "fsl,vf610-i2c", .data = &vf610_i2c_hwdata, },
        { /* sentinel */ }

However, there is no "fsl,imx7d-i2c" in i2c-imx driver compatible string table, but imx7d and imx7s both needs this errata fix.
Also I find imx7d.dtsi is including the imx7s.dtsi, so I think compatible "fsl,imx7s-i2c" is more reasonable in imx7s.dtsi.

So I add the patch to change i2c node compatible in imx7s.dtsi.

BR
Carlos
Francesco Dolcini Oct. 22, 2024, 7:39 a.m. UTC | #3 
On Mon, Oct 21, 2024 at 10:07:23AM +0000, Carlos Song wrote:
> > From: Francesco Dolcini <francesco@dolcini.it>
> > 
> > On Mon, Oct 21, 2024 at 11:11:48AM +0800, carlos.song@nxp.com wrote:
> > > From: Carlos Song <carlos.song@nxp.com>
> > >
> > > Compatible string "fsl,imx7d-i2c" is not exited at i2c-imx driver
> > > compatible string table, at the result, "fsl,imx21-i2c" will be
> > > matched, but it will cause errata ERR007805 not be applied in fact.
> > >
> > > Replace "fsl,imx7d-i2c" by "fsl,imx7s-i2c" compatible string in dts
> > > file for appling the errata ERR007805.
> > 
> > Did you considered changing the driver instead? If I understand this correctly it
> > would be a better solution.
> 
> This errata fix patch has been applied on i2c-imx.c driver: 39c025721d70 i2c:
> imx: Implement errata ERR007805 or e7805 bus frequency limit
> 
> The patch commit log says: this errata is found on all MX7{S,D},
> MX6{UL{,L,Z},S{,LL,X},S,D,DL,Q,DP,QP} and MX8M{M,N,P,Q}.  So imx7d and imx7s
> both needs this errata fix. "fsl,imx7s-i2c" was added into compatible string
> table in the i2c-imx patch, this is the patch diff snap.  So when
> "fsl,imx7s-i2c" is matched, the arrata fix will be applied.

It is clear what you did. What I wrote is that IMO it's not the correct
solution.

Kernel and the FDT can be updated indipendently, with the FDT being part of
the firmware, and you should not expect people to update the FDT to fix
an errata that is implemented in the driver.

My suggestion is that the errata should be applied in the driver to both
compatibles, and such a fix should be backported to stable kernel.

Francesco
Carlos Song Oct. 22, 2024, 8:45 a.m. UTC | #4 
> -----Original Message-----
> From: Francesco Dolcini <francesco@dolcini.it>
> Sent: Tuesday, October 22, 2024 3:40 PM
> To: Carlos Song <carlos.song@nxp.com>
> Cc: Francesco Dolcini <francesco@dolcini.it>; robh@kernel.org;
> krzk+dt@kernel.org; conor+dt@kernel.org; shawnguo@kernel.org;
> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> Frank Li <frank.li@nxp.com>; Bough Chen <haibo.chen@nxp.com>;
> devicetree@vger.kernel.org; imx@lists.linux.dev;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: [EXT] Re: [PATCH] ARM: dts: imx7s: change i2c compatible string for
> applying errata ERR007805
> 
> 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
> 
> 
> On Mon, Oct 21, 2024 at 10:07:23AM +0000, Carlos Song wrote:
> > > From: Francesco Dolcini <francesco@dolcini.it>
> > >
> > > On Mon, Oct 21, 2024 at 11:11:48AM +0800, carlos.song@nxp.com wrote:
> > > > From: Carlos Song <carlos.song@nxp.com>
> > > >
> > > > Compatible string "fsl,imx7d-i2c" is not exited at i2c-imx driver
> > > > compatible string table, at the result, "fsl,imx21-i2c" will be
> > > > matched, but it will cause errata ERR007805 not be applied in fact.
> > > >
> > > > Replace "fsl,imx7d-i2c" by "fsl,imx7s-i2c" compatible string in
> > > > dts file for appling the errata ERR007805.
> > >
> > > Did you considered changing the driver instead? If I understand this
> > > correctly it would be a better solution.
> >
> > This errata fix patch has been applied on i2c-imx.c driver: 39c025721d70 i2c:
> > imx: Implement errata ERR007805 or e7805 bus frequency limit
> >
> > The patch commit log says: this errata is found on all MX7{S,D},
> > MX6{UL{,L,Z},S{,LL,X},S,D,DL,Q,DP,QP} and MX8M{M,N,P,Q}.  So imx7d and
> > imx7s both needs this errata fix. "fsl,imx7s-i2c" was added into
> > compatible string table in the i2c-imx patch, this is the patch diff
> > snap.  So when "fsl,imx7s-i2c" is matched, the arrata fix will be applied.
> 
> It is clear what you did. What I wrote is that IMO it's not the correct solution.
> 
> Kernel and the FDT can be updated indipendently, with the FDT being part of the
> firmware, and you should not expect people to update the FDT to fix an errata
> that is implemented in the driver.
> 
> My suggestion is that the errata should be applied in the driver to both
> compatibles, and such a fix should be backported to stable kernel.
> 
> Francesco

Hi, Francesco

Thanks! I get it. I will send V2 patch in i2c-imx.c driver to add one compatible string for imx7d
to fix this errata.

Carlos
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/nxp/imx/imx7s.dtsi b/arch/arm/boot/dts/nxp/imx/imx7s.dtsi
index 22dd72499ef2..6a1d8350192f 100644
--- a/arch/arm/boot/dts/nxp/imx/imx7s.dtsi
+++ b/arch/arm/boot/dts/nxp/imx/imx7s.dtsi
@@ -1087,7 +1087,7 @@  flexcan2: can@30a10000 {
 			i2c1: i2c@30a20000 {
 				#address-cells = <1>;
 				#size-cells = <0>;
-				compatible = "fsl,imx7d-i2c", "fsl,imx21-i2c";
+				compatible = "fsl,imx7s-i2c", "fsl,imx21-i2c";
 				reg = <0x30a20000 0x10000>;
 				interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX7D_I2C1_ROOT_CLK>;
@@ -1097,7 +1097,7 @@  i2c1: i2c@30a20000 {
 			i2c2: i2c@30a30000 {
 				#address-cells = <1>;
 				#size-cells = <0>;
-				compatible = "fsl,imx7d-i2c", "fsl,imx21-i2c";
+				compatible = "fsl,imx7s-i2c", "fsl,imx21-i2c";
 				reg = <0x30a30000 0x10000>;
 				interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX7D_I2C2_ROOT_CLK>;
@@ -1107,7 +1107,7 @@  i2c2: i2c@30a30000 {
 			i2c3: i2c@30a40000 {
 				#address-cells = <1>;
 				#size-cells = <0>;
-				compatible = "fsl,imx7d-i2c", "fsl,imx21-i2c";
+				compatible = "fsl,imx7s-i2c", "fsl,imx21-i2c";
 				reg = <0x30a40000 0x10000>;
 				interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX7D_I2C3_ROOT_CLK>;
@@ -1117,7 +1117,7 @@  i2c3: i2c@30a40000 {
 			i2c4: i2c@30a50000 {
 				#address-cells = <1>;
 				#size-cells = <0>;
-				compatible = "fsl,imx7d-i2c", "fsl,imx21-i2c";
+				compatible = "fsl,imx7s-i2c", "fsl,imx21-i2c";
 				reg = <0x30a50000 0x10000>;
 				interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX7D_I2C4_ROOT_CLK>;