Re: [PATCH v3 17/21] ARM: dts: imx6ull: improve can templates

From: Marcel Ziswiler <marcel.ziswiler@toradex.com>
To: Max Krummenacher <max.krummenacher@toradex.com>,
	"stefan@agner.ch" <stefan@agner.ch>,
	Philippe Schenker <philippe.schenker@toradex.com>,
	"mark.rutland@arm.com" <mark.rutland@arm.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"michal.vokac@ysoft.com" <michal.vokac@ysoft.com>,
	"shawnguo@kernel.org" <shawnguo@kernel.org>,
	"festevam@gmail.com" <festevam@gmail.com>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>
Cc: "linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"s.hauer@pengutronix.de" <s.hauer@pengutronix.de>,
	"kernel@pengutronix.de" <kernel@pengutronix.de>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-imx@nxp.com" <linux-imx@nxp.com>
Subject: Re: [PATCH v3 17/21] ARM: dts: imx6ull: improve can templates
Date: Fri, 9 Aug 2019 15:47:51 +0000	[thread overview]
Message-ID: <8ae5742f29f17e61fd9fc39a8dbd1b7c3a2f45b0.camel@toradex.com> (raw)
In-Reply-To: <20190807082556.5013-18-philippe.schenker@toradex.com>

Hi Philippe

On Wed, 2019-08-07 at 08:26 +0000, Philippe Schenker wrote:
> From: Max Krummenacher <max.krummenacher@toradex.com>
> 
> Add the pinmuxing and a inactive node for flexcan1 on SODIMM 55/63
> and move the inactive flexcan nodes to imx6ull-colibri-eval-v3.dtsi
> where they belong.
> 
> Note that this commit does not enable flexcan functionality, but
> rather
> eases the effort needed to do so.
> 
> Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
> Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
> ---
> 
> Changes in v3: None
> Changes in v2: None
> 
>  arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi | 12 ++++++++++++
>  arch/arm/boot/dts/imx6ull-colibri-nonwifi.dtsi |  2 +-
>  arch/arm/boot/dts/imx6ull-colibri-wifi.dtsi    |  2 +-
>  arch/arm/boot/dts/imx6ull-colibri.dtsi         | 16 ++++++++++++++--
>  4 files changed, 28 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi
> b/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi
> index b6147c76d159..3bee37c75aa6 100644
> --- a/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi
> +++ b/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi
> @@ -83,6 +83,18 @@
>  	};
>  };
>  
> +&can1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_flexcan1>;
> +	status = "disabled";
> +};
> +
> +&can2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_flexcan2>;
> +	status = "disabled";
> +};

As those don't really have anything to do with the eval board directly,
wouldn't it make more sense to rather move them into the module's dtsi
just like the pin muxing further below?

>  &i2c1 {
>  	status = "okay";
>  
> diff --git a/arch/arm/boot/dts/imx6ull-colibri-nonwifi.dtsi
> b/arch/arm/boot/dts/imx6ull-colibri-nonwifi.dtsi
> index fb213bec4654..95a11b8bcbdb 100644
> --- a/arch/arm/boot/dts/imx6ull-colibri-nonwifi.dtsi
> +++ b/arch/arm/boot/dts/imx6ull-colibri-nonwifi.dtsi
> @@ -15,7 +15,7 @@
>  &iomuxc {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_gpio1 &pinctrl_gpio2 &pinctrl_gpio3
> -		&pinctrl_gpio4 &pinctrl_gpio5 &pinctrl_gpio6>;
> +		&pinctrl_gpio4 &pinctrl_gpio5 &pinctrl_gpio6
> &pinctrl_gpio7>;
>  };
>  
>  &iomuxc_snvs {
> diff --git a/arch/arm/boot/dts/imx6ull-colibri-wifi.dtsi
> b/arch/arm/boot/dts/imx6ull-colibri-wifi.dtsi
> index 038d8c90f6df..a0545431b3dc 100644
> --- a/arch/arm/boot/dts/imx6ull-colibri-wifi.dtsi
> +++ b/arch/arm/boot/dts/imx6ull-colibri-wifi.dtsi
> @@ -26,7 +26,7 @@
>  &iomuxc {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_gpio1 &pinctrl_gpio2 &pinctrl_gpio3
> -		&pinctrl_gpio4 &pinctrl_gpio5>;
> +		&pinctrl_gpio4 &pinctrl_gpio5 &pinctrl_gpio7>;
>  
>  };
>  
> diff --git a/arch/arm/boot/dts/imx6ull-colibri.dtsi
> b/arch/arm/boot/dts/imx6ull-colibri.dtsi
> index e3220298dd6f..553d4c1f80e9 100644
> --- a/arch/arm/boot/dts/imx6ull-colibri.dtsi
> +++ b/arch/arm/boot/dts/imx6ull-colibri.dtsi
> @@ -256,6 +256,13 @@
>  		>;
>  	};
>  
> +	pinctrl_flexcan1: flexcan1-grp {
> +		fsl,pins = <
> +			MX6UL_PAD_ENET1_RX_DATA0__FLEXCAN1_TX	0x1b0
> 20
> +			MX6UL_PAD_ENET1_RX_DATA1__FLEXCAN1_RX	0x1b0
> 20
> +		>;
> +	};
> +
>  	pinctrl_flexcan2: flexcan2-grp {
>  		fsl,pins = <
>  			MX6UL_PAD_ENET1_TX_DATA0__FLEXCAN2_RX	0x1b0
> 20
> @@ -271,8 +278,6 @@
>  
>  	pinctrl_gpio1: gpio1-grp {
>  		fsl,pins = <
> -			MX6UL_PAD_ENET1_RX_DATA0__GPIO2_IO00	0x74
> /* SODIMM 55 */
> -			MX6UL_PAD_ENET1_RX_DATA1__GPIO2_IO01	0x74
> /* SODIMM 63 */
>  			MX6UL_PAD_UART3_RX_DATA__GPIO1_IO25	0X14
> /* SODIMM 77 */
>  			MX6UL_PAD_JTAG_TCK__GPIO1_IO14		0x14
> /* SODIMM 99 */
>  			MX6UL_PAD_NAND_CE1_B__GPIO4_IO14	0x14 /*
> SODIMM 133 */
> @@ -325,6 +330,13 @@
>  		>;
>  	};
>  
> +	pinctrl_gpio7: gpio7-grp { /* CAN1 */
> +		fsl,pins = <
> +			MX6UL_PAD_ENET1_RX_DATA0__GPIO2_IO00	0x74
> /* SODIMM 55 */
> +			MX6UL_PAD_ENET1_RX_DATA1__GPIO2_IO01	0x74
> /* SODIMM 63 */
> +		>;
> +	};
> +
>  	pinctrl_gpmi_nand: gpmi-nand-grp {
>  		fsl,pins = <
>  			MX6UL_PAD_NAND_DATA00__RAWNAND_DATA00	0x100
> a9

Cheers

Marcel