diff mbox series

[1/7] dt-bindings: can: flexcan: fix imx8mp compatbile

Message ID 20210715082536.1882077-2-aisheng.dong@nxp.com (mailing list archive)
State New, archived
Headers show
Series dt-bindings: imx8mp: fix dt schema check errors | expand

Commit Message

Aisheng Dong July 15, 2021, 8:25 a.m. UTC 
This patch fixes the following errors during make dtbs_check:
arch/arm64/boot/dts/freescale/imx8mp-evk.dt.yaml: can@308c0000: compatible: 'oneOf' conditional failed, one must be fixed:
	['fsl,imx8mp-flexcan', 'fsl,imx6q-flexcan'] is too long
	Additional items are not allowed ('fsl,imx6q-flexcan' was unexpected)
	'fsl,imx8mp-flexcan' is not one of ['fsl,imx53-flexcan', 'fsl,imx35-flexcan']
	'fsl,imx8mp-flexcan' is not one of ['fsl,imx7d-flexcan', 'fsl,imx6ul-flexcan', 'fsl,imx6sx-flexcan']
	'fsl,imx8mp-flexcan' is not one of ['fsl,ls1028ar1-flexcan']
	'fsl,imx25-flexcan' was expected
	'fsl,lx2160ar1-flexcan' was expected

Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: Joakim Zhang <qiangqing.zhang@nxp.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: linux-can@vger.kernel.org
Cc: netdev@vger.kernel.org
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Marc Kleine-Budde July 15, 2021, 9:12 a.m. UTC | #1 
On 15.07.2021 16:25:30, Dong Aisheng wrote:
> This patch fixes the following errors during make dtbs_check:
> arch/arm64/boot/dts/freescale/imx8mp-evk.dt.yaml: can@308c0000: compatible: 'oneOf' conditional failed, one must be fixed:
> 	['fsl,imx8mp-flexcan', 'fsl,imx6q-flexcan'] is too long

IIRC the fsl,imx6q-flexcan binding doesn't work on the imx8mp. Maybe
better change the dtsi?

regards,
Marc
Dong Aisheng July 15, 2021, 10:45 a.m. UTC | #2 
Hi Marc,

On Thu, Jul 15, 2021 at 5:12 PM Marc Kleine-Budde <mkl@pengutronix.de> wrote:
>
> On 15.07.2021 16:25:30, Dong Aisheng wrote:
> > This patch fixes the following errors during make dtbs_check:
> > arch/arm64/boot/dts/freescale/imx8mp-evk.dt.yaml: can@308c0000: compatible: 'oneOf' conditional failed, one must be fixed:
> >       ['fsl,imx8mp-flexcan', 'fsl,imx6q-flexcan'] is too long
>
> IIRC the fsl,imx6q-flexcan binding doesn't work on the imx8mp. Maybe
> better change the dtsi?

I checked with Joakim that the flexcan on MX8MP is derived from MX6Q with extra
ECC added. Maybe we should still keep it from HW point of view?

Regards
Aisheng

>
> 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 |
Joakim Zhang July 15, 2021, 11 a.m. UTC | #3 
Hi Aisheng, Marc,

> -----Original Message-----
> From: Dong Aisheng <dongas86@gmail.com>
> Sent: 2021年7月15日 18:46
> To: Marc Kleine-Budde <mkl@pengutronix.de>
> Cc: Aisheng Dong <aisheng.dong@nxp.com>; devicetree
> <devicetree@vger.kernel.org>; moderated list:ARM/FREESCALE IMX / MXC
> ARM ARCHITECTURE <linux-arm-kernel@lists.infradead.org>; dl-linux-imx
> <linux-imx@nxp.com>; Sascha Hauer <kernel@pengutronix.de>; Rob Herring
> <robh+dt@kernel.org>; Shawn Guo <shawnguo@kernel.org>; Joakim Zhang
> <qiangqing.zhang@nxp.com>; linux-can@vger.kernel.org;
> netdev@vger.kernel.org
> Subject: Re: [PATCH 1/7] dt-bindings: can: flexcan: fix imx8mp compatbile
> 
> Hi Marc,
> 
> On Thu, Jul 15, 2021 at 5:12 PM Marc Kleine-Budde <mkl@pengutronix.de>
> wrote:
> >
> > On 15.07.2021 16:25:30, Dong Aisheng wrote:
> > > This patch fixes the following errors during make dtbs_check:
> > > arch/arm64/boot/dts/freescale/imx8mp-evk.dt.yaml: can@308c0000:
> compatible: 'oneOf' conditional failed, one must be fixed:
> > >       ['fsl,imx8mp-flexcan', 'fsl,imx6q-flexcan'] is too long
> >
> > IIRC the fsl,imx6q-flexcan binding doesn't work on the imx8mp. Maybe
> > better change the dtsi?
> 
> I checked with Joakim that the flexcan on MX8MP is derived from MX6Q with
> extra ECC added. Maybe we should still keep it from HW point of view?

Sorry, Aisheng, I double check the history, and get the below results:

8MP reuses 8QXP(8QM), except ECC_EN (ipv_flexcan3_syn_006/D_IP_FlexCAN3_SYN_057 which corresponds to version d_ip_flexcan3_syn.03.00.17.01)

I prefer to change the dtsi as Mac suggested if possible, shall I send a fix patch?
 
Best Regards,
Joakim Zhang
> Regards
> Aisheng
> 
> >
> > regards,
> > Marc
> >
> > --
> > Pengutronix e.K.                 | Marc Kleine-Budde           |
> > Embedded Linux                   |
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.p
> engutronix.de%2F&amp;data=04%7C01%7Cqiangqing.zhang%40nxp.com%7Ce
> df7b681c04c48c0695e08d9477e03b0%7C686ea1d3bc2b4c6fa92cd99c5c30163
> 5%7C0%7C0%7C637619428815826860%7CUnknown%7CTWFpbGZsb3d8eyJWI
> joiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1
> 000&amp;sdata=Sd01Qk9H%2F8pBD0FAFQdQnQU9qp%2Br2ItGKdljK%2BWTiG
> Q%3D&amp;reserved=0  |
> > Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
> > Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |
Marc Kleine-Budde July 15, 2021, 11:07 a.m. UTC | #4 
On 15.07.2021 11:00:07, Joakim Zhang wrote:
> > I checked with Joakim that the flexcan on MX8MP is derived from MX6Q with
> > extra ECC added. Maybe we should still keep it from HW point of view?
> 
> Sorry, Aisheng, I double check the history, and get the below results:
> 
> 8MP reuses 8QXP(8QM), except ECC_EN
> (ipv_flexcan3_syn_006/D_IP_FlexCAN3_SYN_057 which corresponds to
> version d_ip_flexcan3_syn.03.00.17.01)

Also see commit message of:

https://lore.kernel.org/linux-can/20200929211557.14153-2-qiangqing.zhang@nxp.com/

> I prefer to change the dtsi as Mac suggested if possible, shall I send
> a fix patch?

Make it so!

regards,
Marc
Dong Aisheng July 15, 2021, 11:36 a.m. UTC | #5 
On Thu, Jul 15, 2021 at 7:07 PM Marc Kleine-Budde <mkl@pengutronix.de> wrote:
>
> On 15.07.2021 11:00:07, Joakim Zhang wrote:
> > > I checked with Joakim that the flexcan on MX8MP is derived from MX6Q with
> > > extra ECC added. Maybe we should still keep it from HW point of view?
> >
> > Sorry, Aisheng, I double check the history, and get the below results:
> >
> > 8MP reuses 8QXP(8QM), except ECC_EN
> > (ipv_flexcan3_syn_006/D_IP_FlexCAN3_SYN_057 which corresponds to
> > version d_ip_flexcan3_syn.03.00.17.01)
>
> Also see commit message of:
>
> https://lore.kernel.org/linux-can/20200929211557.14153-2-qiangqing.zhang@nxp.com/
>
> > I prefer to change the dtsi as Mac suggested if possible, shall I send
> > a fix patch?
>
> Make it so!

Then should it be "fsl,imx8mp-flexcan", "fsl,imx8qxp-flexcan" rather than only
drop "fsl,imx6q-flexcan"?

Regards
Aisheng

>
> 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 |
Joakim Zhang July 15, 2021, 11:44 a.m. UTC | #6 
> -----Original Message-----
> From: Dong Aisheng <dongas86@gmail.com>
> Sent: 2021年7月15日 19:36
> To: Marc Kleine-Budde <mkl@pengutronix.de>
> Cc: Joakim Zhang <qiangqing.zhang@nxp.com>; Aisheng Dong
> <aisheng.dong@nxp.com>; devicetree <devicetree@vger.kernel.org>;
> moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
> <linux-arm-kernel@lists.infradead.org>; dl-linux-imx <linux-imx@nxp.com>;
> Sascha Hauer <kernel@pengutronix.de>; Rob Herring <robh+dt@kernel.org>;
> Shawn Guo <shawnguo@kernel.org>; linux-can@vger.kernel.org;
> netdev@vger.kernel.org
> Subject: Re: [PATCH 1/7] dt-bindings: can: flexcan: fix imx8mp compatbile
> 
> On Thu, Jul 15, 2021 at 7:07 PM Marc Kleine-Budde <mkl@pengutronix.de>
> wrote:
> >
> > On 15.07.2021 11:00:07, Joakim Zhang wrote:
> > > > I checked with Joakim that the flexcan on MX8MP is derived from
> > > > MX6Q with extra ECC added. Maybe we should still keep it from HW point
> of view?
> > >
> > > Sorry, Aisheng, I double check the history, and get the below results:
> > >
> > > 8MP reuses 8QXP(8QM), except ECC_EN
> > > (ipv_flexcan3_syn_006/D_IP_FlexCAN3_SYN_057 which corresponds to
> > > version d_ip_flexcan3_syn.03.00.17.01)
> >
> > Also see commit message of:
> >
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore
> > .kernel.org%2Flinux-can%2F20200929211557.14153-2-qiangqing.zhang%40n
> xp
> > .com%2F&amp;data=04%7C01%7Cqiangqing.zhang%40nxp.com%7Cf5cd871
> e13b34e9
> >
> 5817b08d9478504af%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C
> 6376194
> >
> 58893680146%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIj
> oiV2luMz
> >
> IiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=YwH3vD%2FtIol5
> OXPHPM
> > VbiVCLTC7gowOdIP3Ih1lBHh0%3D&amp;reserved=0
> >
> > > I prefer to change the dtsi as Mac suggested if possible, shall I
> > > send a fix patch?
> >
> > Make it so!
> 
> Then should it be "fsl,imx8mp-flexcan", "fsl,imx8qxp-flexcan" rather than only
> drop "fsl,imx6q-flexcan"?

No, I will only use " fsl,imx8mp-flexcan" to avoid ECC impact.

Best Regards,
Joakim Zhang
> Regards
> Aisheng
> 
> >
> > regards,
> > Marc
> >
> > --
> > Pengutronix e.K.                 | Marc Kleine-Budde           |
> > Embedded Linux                   |
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.p
> engutronix.de%2F&amp;data=04%7C01%7Cqiangqing.zhang%40nxp.com%7Cf
> 5cd871e13b34e95817b08d9478504af%7C686ea1d3bc2b4c6fa92cd99c5c30163
> 5%7C0%7C1%7C637619458893680146%7CUnknown%7CTWFpbGZsb3d8eyJWI
> joiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3
> 000&amp;sdata=soLd53hGDcxtF42AjJ7u5k9TT%2FsZt6TG%2Bljw4rvtdy4%3D&
> amp;reserved=0  |
> > Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
> > Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |
Dong Aisheng July 15, 2021, 11:49 a.m. UTC | #7 
On Thu, Jul 15, 2021 at 7:44 PM Joakim Zhang <qiangqing.zhang@nxp.com> wrote:
>
>
> > -----Original Message-----
> > From: Dong Aisheng <dongas86@gmail.com>
> > Sent: 2021年7月15日 19:36
> > To: Marc Kleine-Budde <mkl@pengutronix.de>
> > Cc: Joakim Zhang <qiangqing.zhang@nxp.com>; Aisheng Dong
> > <aisheng.dong@nxp.com>; devicetree <devicetree@vger.kernel.org>;
> > moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
> > <linux-arm-kernel@lists.infradead.org>; dl-linux-imx <linux-imx@nxp.com>;
> > Sascha Hauer <kernel@pengutronix.de>; Rob Herring <robh+dt@kernel.org>;
> > Shawn Guo <shawnguo@kernel.org>; linux-can@vger.kernel.org;
> > netdev@vger.kernel.org
> > Subject: Re: [PATCH 1/7] dt-bindings: can: flexcan: fix imx8mp compatbile
> >
> > On Thu, Jul 15, 2021 at 7:07 PM Marc Kleine-Budde <mkl@pengutronix.de>
> > wrote:
> > >
> > > On 15.07.2021 11:00:07, Joakim Zhang wrote:
> > > > > I checked with Joakim that the flexcan on MX8MP is derived from
> > > > > MX6Q with extra ECC added. Maybe we should still keep it from HW point
> > of view?
> > > >
> > > > Sorry, Aisheng, I double check the history, and get the below results:
> > > >
> > > > 8MP reuses 8QXP(8QM), except ECC_EN
> > > > (ipv_flexcan3_syn_006/D_IP_FlexCAN3_SYN_057 which corresponds to
> > > > version d_ip_flexcan3_syn.03.00.17.01)
> > >
> > > Also see commit message of:
> > >
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore
> > > .kernel.org%2Flinux-can%2F20200929211557.14153-2-qiangqing.zhang%40n
> > xp
> > > .com%2F&amp;data=04%7C01%7Cqiangqing.zhang%40nxp.com%7Cf5cd871
> > e13b34e9
> > >
> > 5817b08d9478504af%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C
> > 6376194
> > >
> > 58893680146%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIj
> > oiV2luMz
> > >
> > IiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=YwH3vD%2FtIol5
> > OXPHPM
> > > VbiVCLTC7gowOdIP3Ih1lBHh0%3D&amp;reserved=0
> > >
> > > > I prefer to change the dtsi as Mac suggested if possible, shall I
> > > > send a fix patch?
> > >
> > > Make it so!
> >
> > Then should it be "fsl,imx8mp-flexcan", "fsl,imx8qxp-flexcan" rather than only
> > drop "fsl,imx6q-flexcan"?
>
> No, I will only use " fsl,imx8mp-flexcan" to avoid ECC impact.
>

Is ECC issue SW or HW compatibility issue?
If SW, then we should keep the backward compatible string as DT is
describing HW.

Regards
Aisheng

> Best Regards,
> Joakim Zhang
> > Regards
> > Aisheng
> >
> > >
> > > regards,
> > > Marc
> > >
> > > --
> > > Pengutronix e.K.                 | Marc Kleine-Budde           |
> > > Embedded Linux                   |
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.p
> > engutronix.de%2F&amp;data=04%7C01%7Cqiangqing.zhang%40nxp.com%7Cf
> > 5cd871e13b34e95817b08d9478504af%7C686ea1d3bc2b4c6fa92cd99c5c30163
> > 5%7C0%7C1%7C637619458893680146%7CUnknown%7CTWFpbGZsb3d8eyJWI
> > joiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3
> > 000&amp;sdata=soLd53hGDcxtF42AjJ7u5k9TT%2FsZt6TG%2Bljw4rvtdy4%3D&
> > amp;reserved=0  |
> > > Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
> > > Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |
Marc Kleine-Budde July 15, 2021, 12:07 p.m. UTC | #8 
On 15.07.2021 19:36:06, Dong Aisheng wrote:
> Then should it be "fsl,imx8mp-flexcan", "fsl,imx8qxp-flexcan" rather than only
> drop "fsl,imx6q-flexcan"?

The driver has compatibles for the 8qm, not for the 8qxp:

|	{ .compatible = "fsl,imx8qm-flexcan", .data = &fsl_imx8qm_devtype_data, },
|	{ .compatible = "fsl,imx8mp-flexcan", .data = &fsl_imx8mp_devtype_data, },

Marc
Marc Kleine-Budde July 15, 2021, 12:10 p.m. UTC | #9 
On 15.07.2021 19:49:42, Dong Aisheng wrote:
> > > Then should it be "fsl,imx8mp-flexcan", "fsl,imx8qxp-flexcan" rather than only
> > > drop "fsl,imx6q-flexcan"?
> >
> > No, I will only use " fsl,imx8mp-flexcan" to avoid ECC impact.
> >
> 
> Is ECC issue SW or HW compatibility issue?
> If SW, then we should keep the backward compatible string as DT is
> describing HW.

The commit messages describes the needed initialization for devices
supporting ECC:

https://lore.kernel.org/linux-can/20200929211557.14153-2-qiangqing.zhang@nxp.com/

Marc
Joakim Zhang July 16, 2021, 2:04 a.m. UTC | #10 
Hi Mac, Aisheng,

> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 2021年7月15日 20:07
> To: Dong Aisheng <dongas86@gmail.com>
> Cc: Joakim Zhang <qiangqing.zhang@nxp.com>; Aisheng Dong
> <aisheng.dong@nxp.com>; devicetree <devicetree@vger.kernel.org>;
> moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
> <linux-arm-kernel@lists.infradead.org>; dl-linux-imx <linux-imx@nxp.com>;
> Sascha Hauer <kernel@pengutronix.de>; Rob Herring <robh+dt@kernel.org>;
> Shawn Guo <shawnguo@kernel.org>; linux-can@vger.kernel.org;
> netdev@vger.kernel.org
> Subject: Re: [PATCH 1/7] dt-bindings: can: flexcan: fix imx8mp compatbile
> 
> On 15.07.2021 19:36:06, Dong Aisheng wrote:
> > Then should it be "fsl,imx8mp-flexcan", "fsl,imx8qxp-flexcan" rather
> > than only drop "fsl,imx6q-flexcan"?
> 
> The driver has compatibles for the 8qm, not for the 8qxp:
> 
> |	{ .compatible = "fsl,imx8qm-flexcan", .data =
> &fsl_imx8qm_devtype_data, },
> |	{ .compatible = "fsl,imx8mp-flexcan", .data =
> |&fsl_imx8mp_devtype_data, },

AFAIK, we first design the i.MX8QM FlexCAN and later i.MX8QXP reuses IP from i.MX8QM, so there is no difference for them.

IMHO, IP design is always backwards compatible, then we need list each as fallback compatible string? I think it's unnecessary.

Best Regards,
Joakim Zhang
> 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 |
Marc Kleine-Budde July 16, 2021, 9:06 a.m. UTC | #11 
On 16.07.2021 02:04:56, Joakim Zhang wrote:
> > On 15.07.2021 19:36:06, Dong Aisheng wrote:
> > > Then should it be "fsl,imx8mp-flexcan", "fsl,imx8qxp-flexcan" rather
> > > than only drop "fsl,imx6q-flexcan"?
> > 
> > The driver has compatibles for the 8qm, not for the 8qxp:
> > 
> > |	{ .compatible = "fsl,imx8qm-flexcan", .data =
> > &fsl_imx8qm_devtype_data, },
> > |	{ .compatible = "fsl,imx8mp-flexcan", .data =
> > |&fsl_imx8mp_devtype_data, },
> 
> AFAIK, we first design the i.MX8QM FlexCAN and later i.MX8QXP reuses
> IP from i.MX8QM, so there is no difference for them.
> 
> IMHO, IP design is always backwards compatible,

Hopefully the IP blocks of the i.MX8Q* are compatible, but the other
flexcan IP core are not.

> then we need list each as fallback compatible string? I think it's
> unnecessary.

In the DTs we usually use the name of the SoC we're just describing as
the first compatible, and add a second compatible with the oldest SoC
having this IP core or an IP core that is compatible (so that the driver
works).

As the imx8mp needs the DISABLE_MECR quirk it's not compatible with the
imx6.

regards,
Marc
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml b/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml
index 55bff1586b6f..ca9caac68777 100644
--- a/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml
+++ b/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml
@@ -18,7 +18,6 @@  properties:
     oneOf:
       - enum:
           - fsl,imx8qm-flexcan
-          - fsl,imx8mp-flexcan
           - fsl,imx6q-flexcan
           - fsl,imx28-flexcan
           - fsl,imx25-flexcan
@@ -33,6 +32,7 @@  properties:
           - const: fsl,imx25-flexcan
       - items:
           - enum:
+              - fsl,imx8mp-flexcan
               - fsl,imx7d-flexcan
               - fsl,imx6ul-flexcan
               - fsl,imx6sx-flexcan