devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] ARM: dts: nitrogen6x: add CAN support
@ 2015-05-21 17:45 Peter Seiderer
  2015-05-22 11:05 ` Philipp Zabel
  0 siblings, 1 reply; 4+ messages in thread
From: Peter Seiderer @ 2015-05-21 17:45 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Russell King,
	Kumar Gala, Ian Campbell, Mark Rutland, Pawel Moll, Rob Herring,
	Sascha Hauer, Shawn Guo

Regulator stuff copied from imx6qdl-tx6.dtsi, pin configuration
taken from Boundary Devices linux kernel tree ([1] and [2]).

[1] https://github.com/boundarydevices/linux-imx6/blob/boundary-imx_3.10.17_1.0.2_ga/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
[2] https://github.com/boundarydevices/linux-imx6/blob/boundary-imx_3.10.17_1.0.2_ga/arch/arm/boot/dts/imx6qdl.dtsi

Signed-off-by: Peter Seiderer <ps.report-hi6Y0CQ0nG0@public.gmane.org>
---
v2:
 - fix imx6qdl-nitrogen6x.dtsi url
 - use real PAD settings (suggested by Fabio Estevam)
 - remove _1 suffix (suggested by Shawn Guo)
---
 arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi | 32 +++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi b/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
index fd096dc..d40092e 100644
--- a/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
@@ -54,6 +54,17 @@
 			gpio = <&gpio3 22 0>;
 			enable-active-high;
 		};
+
+		reg_can_xcvr: regulator@3 {
+			compatible = "regulator-fixed";
+			reg = <3>;
+			regulator-name = "CAN XCVR";
+			regulator-min-microvolt = <3300000>;
+			regulator-max-microvolt = <3300000>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_can_xcvr>;
+			gpio = <&gpio1 2 GPIO_ACTIVE_HIGH>;
+		};
 	};
 
 	gpio-keys {
@@ -138,6 +149,13 @@
 	status = "okay";
 };
 
+&can1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_can1>;
+	xceiver-supply = <&reg_can_xcvr>;
+	status = "okay";
+};
+
 &ecspi1 {
 	fsl,spi-num-chipselects = <1>;
 	cs-gpios = <&gpio3 19 0>;
@@ -234,6 +252,20 @@
 			>;
 		};
 
+		pinctrl_can1: can1grp {
+			fsl,pins = <
+				MX6QDL_PAD_KEY_COL2__FLEXCAN1_TX	0x1b0b0
+				MX6QDL_PAD_KEY_ROW2__FLEXCAN1_RX	0x1b0b0
+			>;
+		};
+
+		pinctrl_can_xcvr: can-xcvrgrp {
+			fsl,pins = <
+				/* Flexcan XCVR enable */
+				MX6QDL_PAD_GPIO_2__GPIO1_IO02		0x1b0b0
+			>;
+		};
+
 		pinctrl_ecspi1: ecspi1grp {
 			fsl,pins = <
 				MX6QDL_PAD_EIM_D17__ECSPI1_MISO		0x100b1
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH v2] ARM: dts: nitrogen6x: add CAN support
  2015-05-21 17:45 [PATCH v2] ARM: dts: nitrogen6x: add CAN support Peter Seiderer
@ 2015-05-22 11:05 ` Philipp Zabel
  2015-05-22 19:30   ` Aw: " Peter Seiderer
  0 siblings, 1 reply; 4+ messages in thread
From: Philipp Zabel @ 2015-05-22 11:05 UTC (permalink / raw)
  To: Peter Seiderer
  Cc: linux-kernel, Mark Rutland, devicetree, Russell King, Pawel Moll,
	Ian Campbell, Rob Herring, Sascha Hauer, Kumar Gala, Shawn Guo,
	linux-arm-kernel

Hi Peter,

Am Donnerstag, den 21.05.2015, 19:45 +0200 schrieb Peter Seiderer:
> Regulator stuff copied from imx6qdl-tx6.dtsi, pin configuration
> taken from Boundary Devices linux kernel tree ([1] and [2]).
> 
> [1] https://github.com/boundarydevices/linux-imx6/blob/boundary-imx_3.10.17_1.0.2_ga/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
> [2] https://github.com/boundarydevices/linux-imx6/blob/boundary-imx_3.10.17_1.0.2_ga/arch/arm/boot/dts/imx6qdl.dtsi
> 
> Signed-off-by: Peter Seiderer <ps.report@gmx.net>
> ---
> v2:
>  - fix imx6qdl-nitrogen6x.dtsi url
>  - use real PAD settings (suggested by Fabio Estevam)
>  - remove _1 suffix (suggested by Shawn Guo)
> ---
>  arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi | 32 +++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi b/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
> index fd096dc..d40092e 100644
> --- a/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
> @@ -54,6 +54,17 @@
>  			gpio = <&gpio3 22 0>;
>  			enable-active-high;
>  		};
> +
> +		reg_can_xcvr: regulator@3 {
> +			compatible = "regulator-fixed";
> +			reg = <3>;
> +			regulator-name = "CAN XCVR";
> +			regulator-min-microvolt = <3300000>;
> +			regulator-max-microvolt = <3300000>;
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&pinctrl_can_xcvr>;
> +			gpio = <&gpio1 2 GPIO_ACTIVE_HIGH>;

According to
Documentation/devicetree/bindings/regulator/fixed-regulator.txt
this should have:
			enable-active-high;

instead of the gpio phandle flag (which is ignored). Otherwise an active
low GPIO is assumed.

regards
Philipp

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Aw: Re: [PATCH v2] ARM: dts: nitrogen6x: add CAN support
  2015-05-22 11:05 ` Philipp Zabel
@ 2015-05-22 19:30   ` Peter Seiderer
  2015-05-23  0:44     ` Aw: " Eric Nelson
  0 siblings, 1 reply; 4+ messages in thread
From: Peter Seiderer @ 2015-05-22 19:30 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: linux-kernel, Mark Rutland, devicetree, Russell King, Pawel Moll,
	Ian Campbell, Rob Herring, Sascha Hauer, Kumar Gala, Shawn Guo,
	linux-arm-kernel

Hello Philipp,

> Gesendet: Freitag, 22. Mai 2015 um 13:05 Uhr
> Von: "Philipp Zabel" <p.zabel@pengutronix.de>
> An: "Peter Seiderer" <ps.report@gmx.net>
> Cc: linux-kernel@vger.kernel.org, "Mark Rutland" <mark.rutland@arm.com>, devicetree@vger.kernel.org, "Russell King" <linux@arm.linux.org.uk>, "Pawel Moll" <pawel.moll@arm.com>, "Ian Campbell" <ijc+devicetree@hellion.org.uk>, "Rob Herring" <robh+dt@kernel.org>, "Sascha Hauer" <kernel@pengutronix.de>, "Kumar Gala" <galak@codeaurora.org>, "Shawn Guo" <shawn.guo@linaro.org>, linux-arm-kernel@lists.infradead.org
> Betreff: Re: [PATCH v2] ARM: dts: nitrogen6x: add CAN support
>
> Hi Peter,
> 
> Am Donnerstag, den 21.05.2015, 19:45 +0200 schrieb Peter Seiderer:
> > Regulator stuff copied from imx6qdl-tx6.dtsi, pin configuration
> > taken from Boundary Devices linux kernel tree ([1] and [2]).
> > 
> > [1] https://github.com/boundarydevices/linux-imx6/blob/boundary-imx_3.10.17_1.0.2_ga/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
> > [2] https://github.com/boundarydevices/linux-imx6/blob/boundary-imx_3.10.17_1.0.2_ga/arch/arm/boot/dts/imx6qdl.dtsi
> > 
> > Signed-off-by: Peter Seiderer <ps.report@gmx.net>
> > ---
> > v2:
> >  - fix imx6qdl-nitrogen6x.dtsi url
> >  - use real PAD settings (suggested by Fabio Estevam)
> >  - remove _1 suffix (suggested by Shawn Guo)
> > ---
> >  arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi | 32 +++++++++++++++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi b/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
> > index fd096dc..d40092e 100644
> > --- a/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
> > +++ b/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
> > @@ -54,6 +54,17 @@
> >  			gpio = <&gpio3 22 0>;
> >  			enable-active-high;
> >  		};
> > +
> > +		reg_can_xcvr: regulator@3 {
> > +			compatible = "regulator-fixed";
> > +			reg = <3>;
> > +			regulator-name = "CAN XCVR";
> > +			regulator-min-microvolt = <3300000>;
> > +			regulator-max-microvolt = <3300000>;
> > +			pinctrl-names = "default";
> > +			pinctrl-0 = <&pinctrl_can_xcvr>;
> > +			gpio = <&gpio1 2 GPIO_ACTIVE_HIGH>;
> 
> According to
> Documentation/devicetree/bindings/regulator/fixed-regulator.txt
> this should have:
> 			enable-active-high;
> 
> instead of the gpio phandle flag (which is ignored). Otherwise an active
> low GPIO is assumed.
> 

Thanks for review...

I was a bit confused from the original:

imx6qdl-tx6.dtsi:103:           reg_can_xcvr: regulator@3 {
imx6qdl-tx6.dtsi-104-                   compatible = "regulator-fixed";
imx6qdl-tx6.dtsi-105-                   reg = <3>;
imx6qdl-tx6.dtsi-106-                   regulator-name = "CAN XCVR";
imx6qdl-tx6.dtsi-107-                   regulator-min-microvolt = <3300000>;
imx6qdl-tx6.dtsi-108-                   regulator-max-microvolt = <3300000>;
imx6qdl-tx6.dtsi-109-                   pinctrl-names = "default";
imx6qdl-tx6.dtsi:110:                   pinctrl-0 = <&pinctrl_flexcan_xcvr>;
imx6qdl-tx6.dtsi-111-                   gpio = <&gpio4 21 GPIO_ACTIVE_HIGH>;
imx6qdl-tx6.dtsi-112-                   enable-active-low;
imx6qdl-tx6.dtsi-113-           };

...and removed the default 'enable-active-low'...

Maybe GPIO_ACTIVE_LOW is the right thing?

>From the other files:

imx28-tx28.dts:90:              reg_can_xcvr: regulator@4 {
imx28-tx28.dts-91-                      compatible = "regulator-fixed";
imx28-tx28.dts-92-                      reg = <4>;
imx28-tx28.dts-93-                      regulator-name = "CAN XCVR";
imx28-tx28.dts-94-                      regulator-min-microvolt = <3300000>;
imx28-tx28.dts-95-                      regulator-max-microvolt = <3300000>;
imx28-tx28.dts-96-                      gpio = <&gpio1 0 GPIO_ACTIVE_HIGH>;
imx28-tx28.dts-97-                      pinctrl-names = "default";
imx28-tx28.dts:98:                      pinctrl-0 = <&tx28_flexcan_xcvr_pins>;
imx28-tx28.dts-99-              };


imx53-tx53.dtsi:90:             reg_can_xcvr: regulator@2 {
imx53-tx53.dtsi-91-                     compatible = "regulator-fixed";
imx53-tx53.dtsi-92-                     reg = <2>;
imx53-tx53.dtsi-93-                     regulator-name = "CAN XCVR";
imx53-tx53.dtsi-94-                     regulator-min-microvolt = <3300000>;
imx53-tx53.dtsi-95-                     regulator-max-microvolt = <3300000>;
imx53-tx53.dtsi-96-                     pinctrl-names = "default";
imx53-tx53.dtsi:97:                     pinctrl-0 = <&pinctrl_can_xcvr>;
imx53-tx53.dtsi-98-                     gpio = <&gpio4 21 GPIO_ACTIVE_HIGH>;
imx53-tx53.dtsi-99-             };

But there are also imx regulator-fixed definitions with GPIO_ACTIVE_HIGH/enable-active-high:

imx28-tx28.dts-101-             reg_lcd: regulator@5 {
imx28-tx28.dts-102-                     compatible = "regulator-fixed";
imx28-tx28.dts-103-                     reg = <5>;
imx28-tx28.dts-104-                     regulator-name = "LCD POWER";
imx28-tx28.dts-105-                     regulator-min-microvolt = <3300000>;
imx28-tx28.dts-106-                     regulator-max-microvolt = <3300000>;
imx28-tx28.dts-107-                     gpio = <&gpio1 31 GPIO_ACTIVE_HIGH>;
imx28-tx28.dts-108-                     enable-active-high;
imx28-tx28.dts-109-             };


imx53-tx53.dtsi-101-            reg_usbh1_vbus: regulator@3 {
imx53-tx53.dtsi-102-                    compatible = "regulator-fixed";
imx53-tx53.dtsi-103-                    reg = <3>;
imx53-tx53.dtsi-104-                    regulator-name = "usbh1_vbus";
imx53-tx53.dtsi-105-                    regulator-min-microvolt = <5000000>;
imx53-tx53.dtsi-106-                    regulator-max-microvolt = <5000000>;
imx53-tx53.dtsi-107-                    pinctrl-names = "default";
imx53-tx53.dtsi-108-                    pinctrl-0 = <&pinctrl_usbh1_vbus>;
imx53-tx53.dtsi-109-                    gpio = <&gpio3 31 GPIO_ACTIVE_HIGH>;
imx53-tx53.dtsi-110-                    enable-active-high;
imx53-tx53.dtsi-111-            };


imx6qdl-tx6.dtsi-115-           reg_lcd0_pwr: regulator@4 {
imx6qdl-tx6.dtsi-116-                   compatible = "regulator-fixed";
imx6qdl-tx6.dtsi-117-                   reg = <4>;
imx6qdl-tx6.dtsi-118-                   regulator-name = "LCD0 POWER";
imx6qdl-tx6.dtsi-119-                   regulator-min-microvolt = <3300000>;
imx6qdl-tx6.dtsi-120-                   regulator-max-microvolt = <3300000>;
imx6qdl-tx6.dtsi-121-                   pinctrl-names = "default";
imx6qdl-tx6.dtsi-122-                   pinctrl-0 = <&pinctrl_lcd0_pwr>;
imx6qdl-tx6.dtsi-123-                   gpio = <&gpio3 29 GPIO_ACTIVE_HIGH>;
imx6qdl-tx6.dtsi-124-                   enable-active-high;
imx6qdl-tx6.dtsi-125-                   regulator-boot-on;
imx6qdl-tx6.dtsi-126-                   regulator-always-on;
imx6qdl-tx6.dtsi-127-           };

Any further advice from your side which solution is the right one?

 - GPIO_ACTIVE_HIGH/enable-active-high
 - GPIO_ACTIVE_LOW

Regards,
Peter

> regards
> Philipp
> 
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Aw: [PATCH v2] ARM: dts: nitrogen6x: add CAN support
  2015-05-22 19:30   ` Aw: " Peter Seiderer
@ 2015-05-23  0:44     ` Eric Nelson
  0 siblings, 0 replies; 4+ messages in thread
From: Eric Nelson @ 2015-05-23  0:44 UTC (permalink / raw)
  To: Peter Seiderer, Philipp Zabel
  Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA, Russell King,
	Pawel Moll, Ian Campbell, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	Rob Herring, Sascha Hauer, Kumar Gala, Shawn Guo,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Hello Peter,

On 05/22/2015 12:30 PM, Peter Seiderer wrote:
> Hello Philipp,
> 
>> Gesendet: Freitag, 22. Mai 2015 um 13:05 Uhr
>> Von: "Philipp Zabel" <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>> Am Donnerstag, den 21.05.2015, 19:45 +0200 schrieb Peter Seiderer:
>>>
>>> <snip>
>>>
>>> +
>>> +		reg_can_xcvr: regulator@3 {
>>> +			compatible = "regulator-fixed";
>>> +			reg = <3>;
>>> +			regulator-name = "CAN XCVR";
>>> +			regulator-min-microvolt = <3300000>;
>>> +			regulator-max-microvolt = <3300000>;
>>> +			pinctrl-names = "default";
>>> +			pinctrl-0 = <&pinctrl_can_xcvr>;
>>> +			gpio = <&gpio1 2 GPIO_ACTIVE_HIGH>;
>>
>> According to
>> Documentation/devicetree/bindings/regulator/fixed-regulator.txt
>> this should have:
>> 			enable-active-high;
>>
>> instead of the gpio phandle flag (which is ignored). Otherwise an active
>> low GPIO is assumed.
>>
> 
> Thanks for review...
> 
> I was a bit confused from the original:
> 
> imx6qdl-tx6.dtsi:103:           reg_can_xcvr: regulator@3 {
> imx6qdl-tx6.dtsi-104-                   compatible = "regulator-fixed";
> imx6qdl-tx6.dtsi-105-                   reg = <3>;
> imx6qdl-tx6.dtsi-106-                   regulator-name = "CAN XCVR";
> imx6qdl-tx6.dtsi-107-                   regulator-min-microvolt = <3300000>;
> imx6qdl-tx6.dtsi-108-                   regulator-max-microvolt = <3300000>;
> imx6qdl-tx6.dtsi-109-                   pinctrl-names = "default";
> imx6qdl-tx6.dtsi:110:                   pinctrl-0 = <&pinctrl_flexcan_xcvr>;
> imx6qdl-tx6.dtsi-111-                   gpio = <&gpio4 21 GPIO_ACTIVE_HIGH>;
> imx6qdl-tx6.dtsi-112-                   enable-active-low;
> imx6qdl-tx6.dtsi-113-           };
> 
> ...and removed the default 'enable-active-low'...
> 
> Maybe GPIO_ACTIVE_LOW is the right thing?
> 

No. The flags aren't read from the device tree and enable-active-low
is the default.
	https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/drivers/regulator/fixed.c#n81

> From the other files:
> 

So if this board really wants an "active high" enable pin, it's likely
not operating properly:

> imx28-tx28.dts:90:              reg_can_xcvr: regulator@4 {
> imx28-tx28.dts-91-                      compatible = "regulator-fixed";
> imx28-tx28.dts-92-                      reg = <4>;
> imx28-tx28.dts-93-                      regulator-name = "CAN XCVR";
> imx28-tx28.dts-94-                      regulator-min-microvolt = <3300000>;
> imx28-tx28.dts-95-                      regulator-max-microvolt = <3300000>;
> imx28-tx28.dts-96-                      gpio = <&gpio1 0 GPIO_ACTIVE_HIGH>;
> imx28-tx28.dts-97-                      pinctrl-names = "default";
> imx28-tx28.dts:98:                      pinctrl-0 = <&tx28_flexcan_xcvr_pins>;
> imx28-tx28.dts-99-              };
> 
> <snip>
> 
> Any further advice from your side which solution is the right one?
> 
>  - GPIO_ACTIVE_HIGH/enable-active-high
>  - GPIO_ACTIVE_LOW
> 

The pad is active low on the TJA1040 transceiver on the Nitrogen6x,
so you don't want "enable-active-high" and could be more explicit with
GPIO_ACTIVE_LOW in the gpio reference, but it won't be parsed or
acted upon.

i.e.
	reg_can_xcvr: regulator@3 {
		compatible = "regulator-fixed";
		reg = <3>;
		regulator-name = "CAN XCVR";
		regulator-min-microvolt = <3300000>;
		regulator-max-microvolt = <3300000>;
		pinctrl-names = "default";
		pinctrl-0 = <&pinctrl_can_xcvr>;
		gpio = <&gpio1 2 0>;
	}

Regards,


Eric
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-05-23  0:44 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-21 17:45 [PATCH v2] ARM: dts: nitrogen6x: add CAN support Peter Seiderer
2015-05-22 11:05 ` Philipp Zabel
2015-05-22 19:30   ` Aw: " Peter Seiderer
2015-05-23  0:44     ` Aw: " Eric Nelson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).