[PATCH] arm64: dts: imx8mp: remove fallback compatible string for FlexCAN

[PATCH] arm64: dts: imx8mp: remove fallback compatible string for FlexCAN

[Joakim Zhang]

FlexCAN on i.MX8MP is not derived from i.MX6Q, instead resues from
i.MX8QM with extra ECC added. With "fsl,imx6q-flexcan" compatible string,
i.MX8MP FlexCAN would not work, so remove this fallback compatible string.

Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mp.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index 9f7c7f587d38..1bfb359dba4a 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -579,7 +579,7 @@
 			};
 
 			flexcan1: can@308c0000 {
-				compatible = "fsl,imx8mp-flexcan", "fsl,imx6q-flexcan";
+				compatible = "fsl,imx8mp-flexcan";
 				reg = <0x308c0000 0x10000>;
 				interrupts = <GIC_SPI 142 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clk IMX8MP_CLK_IPG_ROOT>,
@@ -594,7 +594,7 @@
 			};
 
 			flexcan2: can@308d0000 {
-				compatible = "fsl,imx8mp-flexcan", "fsl,imx6q-flexcan";
+				compatible = "fsl,imx8mp-flexcan";
 				reg = <0x308d0000 0x10000>;
 				interrupts = <GIC_SPI 144 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clk IMX8MP_CLK_IPG_ROOT>,
-- 
2.17.1

Re: [PATCH] arm64: dts: imx8mp: remove fallback compatible string for FlexCAN

[Fabio Estevam]

Hi Joakim,

On Thu, Jul 15, 2021 at 8:49 AM Joakim Zhang <qiangqing.zhang@nxp.com> wrote:
>
> FlexCAN on i.MX8MP is not derived from i.MX6Q, instead resues from
> i.MX8QM with extra ECC added. With "fsl,imx6q-flexcan" compatible string,
> i.MX8MP FlexCAN would not work, so remove this fallback compatible string.

I agree with the removal of "fsl,imx6q-flexcan", but I don't
understand your comment
saying that:

"With "fsl,imx6q-flexcan" compatible string, i.MX8MP FlexCAN would not work"

Why?

"fsl,imx8mp-flexcan" is passed as the more specific compatible string
and it should
match against it first.

Please clarify.

Re: [PATCH] arm64: dts: imx8mp: remove fallback compatible string for FlexCAN

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1224 bytes --]

On 15.07.2021 09:03:55, Fabio Estevam wrote:
> Hi Joakim,
> 
> On Thu, Jul 15, 2021 at 8:49 AM Joakim Zhang <qiangqing.zhang@nxp.com> wrote:
> >
> > FlexCAN on i.MX8MP is not derived from i.MX6Q, instead resues from
> > i.MX8QM with extra ECC added. With "fsl,imx6q-flexcan" compatible string,
> > i.MX8MP FlexCAN would not work, so remove this fallback compatible string.
> 
> I agree with the removal of "fsl,imx6q-flexcan", but I don't
> understand your comment
> saying that:
> 
> "With "fsl,imx6q-flexcan" compatible string, i.MX8MP FlexCAN would not work"
> 
> Why?

Don't remember exactly why It doesn't work. I think it was a missing
quirk that the imx6 doesn't need.

> "fsl,imx8mp-flexcan" is passed as the more specific compatible string
> and it should match against it first.

ACK - but why specify the imx6 in the compatible list if the flexcan IP
core isn't compatible with the one of the imx6?

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] arm64: dts: imx8mp: remove fallback compatible string for FlexCAN

[Fabio Estevam]

Hi Marc,

On Thu, Jul 15, 2021 at 9:33 AM Marc Kleine-Budde <mkl@pengutronix.de> wrote:

> > Why?
>
> Don't remember exactly why It doesn't work. I think it was a missing
> quirk that the imx6 doesn't need.
>
> > "fsl,imx8mp-flexcan" is passed as the more specific compatible string
> > and it should match against it first.
>
> ACK - but why specify the imx6 in the compatible list if the flexcan IP
> core isn't compatible with the one of the imx6?

Correct. The change in this patch looks good.

The commit log needs improvement though.

RE: [PATCH] arm64: dts: imx8mp: remove fallback compatible string for FlexCAN

[Joakim Zhang]

Hi Mac, Fabio,

> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 2021年7月15日 20:15
> To: Fabio Estevam <festevam@gmail.com>
> Cc: Joakim Zhang <qiangqing.zhang@nxp.com>; Rob Herring
> <robh+dt@kernel.org>; Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
> <s.hauer@pengutronix.de>; Sascha Hauer <kernel@pengutronix.de>;
> dl-linux-imx <linux-imx@nxp.com>; open list:OPEN FIRMWARE AND FLATTENED
> DEVICE TREE BINDINGS <devicetree@vger.kernel.org>; Aisheng Dong
> <aisheng.dong@nxp.com>
> Subject: Re: [PATCH] arm64: dts: imx8mp: remove fallback compatible string
> for FlexCAN
> 
> On 15.07.2021 09:03:55, Fabio Estevam wrote:
> > Hi Joakim,
> >
> > On Thu, Jul 15, 2021 at 8:49 AM Joakim Zhang <qiangqing.zhang@nxp.com>
> wrote:
> > >
> > > FlexCAN on i.MX8MP is not derived from i.MX6Q, instead resues from
> > > i.MX8QM with extra ECC added. With "fsl,imx6q-flexcan" compatible
> > > string, i.MX8MP FlexCAN would not work, so remove this fallback
> compatible string.
> >
> > I agree with the removal of "fsl,imx6q-flexcan", but I don't
> > understand your comment saying that:
> >
> > "With "fsl,imx6q-flexcan" compatible string, i.MX8MP FlexCAN would not
> work"
> >
> > Why?
> 
> Don't remember exactly why It doesn't work. I think it was a missing quirk that
> the imx6 doesn't need.

I could explain more if I remember correctly, i.MX8MP with ECC added and default is enabled, without FLEXCAN_QUIRK_DISABLE_MECR quirk,
FlexCAN doesn't work, it will put device in freeze mode. However, as Mac described, i.MX6Q doesn't need it.

> > "fsl,imx8mp-flexcan" is passed as the more specific compatible string
> > and it should match against it first.
> 
> ACK - but why specify the imx6 in the compatible list if the flexcan IP core isn't
> compatible with the one of the imx6?

Best Regards,
Joakim Zhang
> regards,
> Marc
> 
> --
> Pengutronix e.K.                 | Marc Kleine-Budde           |
> Embedded Linux                   | https://www.pengutronix.de  |
> Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
> Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

RE: [PATCH] arm64: dts: imx8mp: remove fallback compatible string for FlexCAN

[Joakim Zhang]

> -----Original Message-----
> From: Fabio Estevam <festevam@gmail.com>
> Sent: 2021年7月15日 20:40
> To: Marc Kleine-Budde <mkl@pengutronix.de>
> Cc: Joakim Zhang <qiangqing.zhang@nxp.com>; Rob Herring
> <robh+dt@kernel.org>; Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
> <s.hauer@pengutronix.de>; Sascha Hauer <kernel@pengutronix.de>;
> dl-linux-imx <linux-imx@nxp.com>; open list:OPEN FIRMWARE AND FLATTENED
> DEVICE TREE BINDINGS <devicetree@vger.kernel.org>; Aisheng Dong
> <aisheng.dong@nxp.com>
> Subject: Re: [PATCH] arm64: dts: imx8mp: remove fallback compatible string
> for FlexCAN
> 
> Hi Marc,
> 
> On Thu, Jul 15, 2021 at 9:33 AM Marc Kleine-Budde <mkl@pengutronix.de>
> wrote:
> 
> > > Why?
> >
> > Don't remember exactly why It doesn't work. I think it was a missing
> > quirk that the imx6 doesn't need.
> >
> > > "fsl,imx8mp-flexcan" is passed as the more specific compatible
> > > string and it should match against it first.
> >
> > ACK - but why specify the imx6 in the compatible list if the flexcan
> > IP core isn't compatible with the one of the imx6?
> 
> Correct. The change in this patch looks good.
> 
> The commit log needs improvement though.

Ok, I will improve the commit message and then resend the patch.

Best Regards,
Joakim Zhang

Re: [PATCH] arm64: dts: imx8mp: remove fallback compatible string for FlexCAN

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 796 bytes --]

On 16.07.2021 02:03:44, Joakim Zhang wrote:
>>> "With "fsl,imx6q-flexcan" compatible string, i.MX8MP FlexCAN would
>>> not work"
>>
>> Why?

> I could explain more if I remember correctly, i.MX8MP with ECC added
> and default is enabled, without FLEXCAN_QUIRK_DISABLE_MECR quirk,
> FlexCAN doesn't work, it will put device in freeze mode. However, as
> Mac described, i.MX6Q doesn't need it.

The bits that are used in the FLEXCAN_QUIRK_DISABLE_MECR are marked as
reserved on the imx6's flexcan IP core.

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]