[PATCH] ARM: dts: imx: ventana: add RS485 txen gpio support

[PATCH] ARM: dts: imx: ventana: add RS485 txen gpio support

[Tim Harvey]

The GW52xx/GW53xx/GW54xx have an on-board RS485 transceiver for half-duplex
RS485 using uart1. The active-high TXEN is GPIO7__IO1 which we can configure
as the rts-gpio as long as we specify it as active-low to invert the polarity
managed by mctrl_gpio helpers. This allows for RS485 to be used from
userspace by setting flags to SER_RS485_RTS_ON_SEND in the serial_rs485
struct when using the TIOCGRS485 ioctl.

Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
 arch/arm/boot/dts/imx6qdl-gw52xx.dtsi | 3 +++
 arch/arm/boot/dts/imx6qdl-gw53xx.dtsi | 3 +++
 arch/arm/boot/dts/imx6qdl-gw54xx.dtsi | 3 +++
 3 files changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
index 8cccc4a..50d6039 100644
--- a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
@@ -315,6 +315,8 @@
 &uart1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_uart1>;
+	fsl,uart-has-rtscts;
+	rts-gpio = <&gpio7 1 GPIO_ACTIVE_LOW>;
 	status = "okay";
 };
 
@@ -493,6 +495,7 @@
 			fsl,pins = <
 				MX6QDL_PAD_SD3_DAT7__UART1_TX_DATA	0x1b0b1
 				MX6QDL_PAD_SD3_DAT6__UART1_RX_DATA	0x1b0b1
+				MX6QDL_PAD_SD3_DAT4__GPIO7_IO01		0x4001b0b1 /* TEN */
 			>;
 		};
 
diff --git a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
index 5f700cc..60b0ae9 100644
--- a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
@@ -312,6 +312,8 @@
 &uart1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_uart1>;
+	fsl,uart-has-rtscts;
+	rts-gpio = <&gpio7 1 GPIO_ACTIVE_LOW>;
 	status = "okay";
 };
 
@@ -482,6 +484,7 @@
 			fsl,pins = <
 				MX6QDL_PAD_SD3_DAT7__UART1_TX_DATA	0x1b0b1
 				MX6QDL_PAD_SD3_DAT6__UART1_RX_DATA	0x1b0b1
+				MX6QDL_PAD_SD3_DAT4__GPIO7_IO01		0x4001b0b1 /* TEN */
 			>;
 		};
 
diff --git a/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
index d19b4cc..c217ecc 100644
--- a/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
@@ -414,6 +414,8 @@
 &uart1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_uart1>;
+	fsl,uart-has-rtscts;
+	rts-gpio = <&gpio7 1 GPIO_ACTIVE_LOW>;
 	status = "okay";
 };
 
@@ -603,6 +605,7 @@
 			fsl,pins = <
 				MX6QDL_PAD_SD3_DAT7__UART1_TX_DATA	0x1b0b1
 				MX6QDL_PAD_SD3_DAT6__UART1_RX_DATA	0x1b0b1
+				MX6QDL_PAD_SD3_DAT4__GPIO7_IO01		0x4001b0b1 /* TEN */
 			>;
 		};
 
-- 
1.9.1

[PATCH] ARM: dts: imx: ventana: add RS485 txen gpio support

[Baruch Siach]

Hi Tim,

On Thu, Apr 14, 2016 at 06:19:09AM -0700, Tim Harvey wrote:
> The GW52xx/GW53xx/GW54xx have an on-board RS485 transceiver for half-duplex
> RS485 using uart1. The active-high TXEN is GPIO7__IO1 which we can configure
> as the rts-gpio as long as we specify it as active-low to invert the polarity
> managed by mctrl_gpio helpers. This allows for RS485 to be used from
> userspace by setting flags to SER_RS485_RTS_ON_SEND in the serial_rs485
> struct when using the TIOCGRS485 ioctl.

Do you use http://article.gmane.org/gmane.linux.ports.arm.kernel/482641 for 
that?

> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> ---
>  arch/arm/boot/dts/imx6qdl-gw52xx.dtsi | 3 +++
>  arch/arm/boot/dts/imx6qdl-gw53xx.dtsi | 3 +++
>  arch/arm/boot/dts/imx6qdl-gw54xx.dtsi | 3 +++
>  3 files changed, 9 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
> index 8cccc4a..50d6039 100644
> --- a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
> @@ -315,6 +315,8 @@
>  &uart1 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_uart1>;
> +	fsl,uart-has-rtscts;
> +	rts-gpio = <&gpio7 1 GPIO_ACTIVE_LOW>;

See http://marc.info/?l=linux-serial&m=146063601520851&w=2.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

[PATCH] ARM: dts: imx: ventana: add RS485 txen gpio support

[Tim Harvey]

On Thu, Apr 14, 2016 at 6:32 AM, Baruch Siach <baruch@tkos.co.il> wrote:
> Hi Tim,
>
> On Thu, Apr 14, 2016 at 06:19:09AM -0700, Tim Harvey wrote:
>> The GW52xx/GW53xx/GW54xx have an on-board RS485 transceiver for half-duplex
>> RS485 using uart1. The active-high TXEN is GPIO7__IO1 which we can configure
>> as the rts-gpio as long as we specify it as active-low to invert the polarity
>> managed by mctrl_gpio helpers. This allows for RS485 to be used from
>> userspace by setting flags to SER_RS485_RTS_ON_SEND in the serial_rs485
>> struct when using the TIOCGRS485 ioctl.
>
> Do you use http://article.gmane.org/gmane.linux.ports.arm.kernel/482641 for
> that?

Baruch,

No - that wasn't in my tree, but I will test it and respond to that thread.

>
>> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
>> ---
>>  arch/arm/boot/dts/imx6qdl-gw52xx.dtsi | 3 +++
>>  arch/arm/boot/dts/imx6qdl-gw53xx.dtsi | 3 +++
>>  arch/arm/boot/dts/imx6qdl-gw54xx.dtsi | 3 +++
>>  3 files changed, 9 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
>> index 8cccc4a..50d6039 100644
>> --- a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
>> +++ b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
>> @@ -315,6 +315,8 @@
>>  &uart1 {
>>       pinctrl-names = "default";
>>       pinctrl-0 = <&pinctrl_uart1>;
>> +     fsl,uart-has-rtscts;
>> +     rts-gpio = <&gpio7 1 GPIO_ACTIVE_LOW>;
>
> See http://marc.info/?l=linux-serial&m=146063601520851&w=2.

Thanks - I didn't see this one yet. I will change the prop and re-submit.

Does my model of defining rts-gpio active-low in dts because I'm using
this as an active-high TXEN make sense?

For reference, we use the MAX14840 transceiver and its DE (active high
transmit enable) is connected directly to GPIO7_IO1. If I define
rts-gpio as active-high and set SER_RS485_RTS_AFTER_SEND the polarity
is also correct on the scope during transmit but there is a funny high
pulse before transmit.

I honestly don't quite understand where there is a
SER_RS485_RTS_ON_SEND and SER_RS485_RTS_AFTER_SEND in
serial_rs485.flags as I would assume those are mutually exclusive and
there could have been just a single bit. Perhaps I'm missing something
there.

I'm also not sure if there is a desire to define the prop as
'rts-gpios' or 'rts-gpio' - both are picked up and there are many dts
props that use -gpios for a single gpio and I didn't see any
devicetree binding documentation for SERIAL_MCTRL_GPIO.

Regards,

Tim

[PATCH] ARM: dts: imx: ventana: add RS485 txen gpio support

[Baruch Siach]

Hi Tim,

On Thu, Apr 14, 2016 at 07:39:40AM -0700, Tim Harvey wrote:
> On Thu, Apr 14, 2016 at 6:32 AM, Baruch Siach <baruch@tkos.co.il> wrote:
> > On Thu, Apr 14, 2016 at 06:19:09AM -0700, Tim Harvey wrote:
> >> The GW52xx/GW53xx/GW54xx have an on-board RS485 transceiver for half-duplex
> >> RS485 using uart1. The active-high TXEN is GPIO7__IO1 which we can configure
> >> as the rts-gpio as long as we specify it as active-low to invert the polarity
> >> managed by mctrl_gpio helpers. This allows for RS485 to be used from
> >> userspace by setting flags to SER_RS485_RTS_ON_SEND in the serial_rs485
> >> struct when using the TIOCGRS485 ioctl.
> >
> > Do you use http://article.gmane.org/gmane.linux.ports.arm.kernel/482641 for
> > that?
> 
> No - that wasn't in my tree, but I will test it and respond to that thread.

I see that the MAX14840 transceiver has built-in Rx disable on Tx logic, so 
you don't strictly need this patch. But since my patch changes the behaviour 
when SER_RS485_RX_DURING_TX is not set, it would be nice if you give it a spin 
to verify that it doesn't break you use case.

I think though that a SER_RS485_DISABLE_RX_ON_TX flag that inverts the logic 
of SER_RS485_RX_DURING_TX would make much more sense, both on serial drivers 
side, and on userspace. But that's history now since we can't break the ABI.

Thanks,
baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

[PATCH] ARM: dts: imx: ventana: add RS485 txen gpio support

[Geert Uytterhoeven]

On Thu, Apr 14, 2016 at 4:39 PM, Tim Harvey <tharvey@gateworks.com> wrote:
> On Thu, Apr 14, 2016 at 6:32 AM, Baruch Siach <baruch@tkos.co.il> wrote:
>> On Thu, Apr 14, 2016 at 06:19:09AM -0700, Tim Harvey wrote:
>>> diff --git a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
>>> index 8cccc4a..50d6039 100644
>>> --- a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
>>> +++ b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
>>> @@ -315,6 +315,8 @@
>>>  &uart1 {
>>>       pinctrl-names = "default";
>>>       pinctrl-0 = <&pinctrl_uart1>;
>>> +     fsl,uart-has-rtscts;
>>> +     rts-gpio = <&gpio7 1 GPIO_ACTIVE_LOW>;
>>
>> See http://marc.info/?l=linux-serial&m=146063601520851&w=2.
>
> Thanks - I didn't see this one yet. I will change the prop and re-submit.

I think it's a bit early to change the property, before the code handling
the changes has been accepted.

> I'm also not sure if there is a desire to define the prop as
> 'rts-gpios' or 'rts-gpio' - both are picked up and there are many dts
> props that use -gpios for a single gpio and I didn't see any
> devicetree binding documentation for SERIAL_MCTRL_GPIO.

Both are picked up by the core GPIO helpers for backwards compatibility.

Quoting Documentation/devicetree/bindings/gpio/gpio.txt:
| GPIO properties should be named "[<name>-]gpios", [...]
| Also, GPIO properties named "[<name>-]gpio" are valid and old bindings use it,
| but are only supported for compatibility reasons and should not be used for
| newer bindings since it has been deprecated.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH] ARM: dts: imx: ventana: add RS485 txen gpio support

[Tim Harvey]

The GW54xx, GW53xx, and GW52xx all have an RS485 transceiver that uses an
IMX GPIO for the TX enable of the transceiver. Declare this properly.

Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
 arch/arm/boot/dts/imx6qdl-gw52xx.dtsi | 3 +++
 arch/arm/boot/dts/imx6qdl-gw53xx.dtsi | 3 +++
 arch/arm/boot/dts/imx6qdl-gw54xx.dtsi | 3 +++
 3 files changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
index 8bf1020..25ed57d 100644
--- a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
@@ -315,6 +315,8 @@
 &uart1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_uart1>;
+	fsl,uart-has-rtscts;
+	rts-gpio = <&gpio7 1 GPIO_ACTIVE_HIGH>;
 	status = "okay";
 };
 
@@ -493,6 +495,7 @@
 			fsl,pins = <
 				MX6QDL_PAD_SD3_DAT7__UART1_TX_DATA	0x1b0b1
 				MX6QDL_PAD_SD3_DAT6__UART1_RX_DATA	0x1b0b1
+				MX6QDL_PAD_SD3_DAT4__GPIO7_IO01		0x4001b0b1 /* TEN */
 			>;
 		};
 
diff --git a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
index e5f4b6c..69e21b9 100644
--- a/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw53xx.dtsi
@@ -311,6 +311,8 @@
 &uart1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_uart1>;
+	fsl,uart-has-rtscts;
+	rts-gpio = <&gpio7 1 GPIO_ACTIVE_HIGH>;
 	status = "okay";
 };
 
@@ -481,6 +483,7 @@
 			fsl,pins = <
 				MX6QDL_PAD_SD3_DAT7__UART1_TX_DATA	0x1b0b1
 				MX6QDL_PAD_SD3_DAT6__UART1_RX_DATA	0x1b0b1
+				MX6QDL_PAD_SD3_DAT4__GPIO7_IO01		0x4001b0b1 /* TEN */
 			>;
 		};
 
diff --git a/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
index 5a8dbab..b602e2a 100644
--- a/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw54xx.dtsi
@@ -414,6 +414,8 @@
 &uart1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_uart1>;
+	fsl,uart-has-rtscts;
+	rts-gpio = <&gpio7 1 GPIO_ACTIVE_HIGH>;
 	status = "okay";
 };
 
@@ -603,6 +605,7 @@
 			fsl,pins = <
 				MX6QDL_PAD_SD3_DAT7__UART1_TX_DATA	0x1b0b1
 				MX6QDL_PAD_SD3_DAT6__UART1_RX_DATA	0x1b0b1
+				MX6QDL_PAD_SD3_DAT4__GPIO7_IO01		0x4001b0b1 /* TEN */
 			>;
 		};
 
-- 
1.9.1

[PATCH] ARM: dts: imx: ventana: add RS485 txen gpio support

[Shawn Guo]

On Wed, Jun 29, 2016 at 06:38:24AM -0700, Tim Harvey wrote:
> The GW54xx, GW53xx, and GW52xx all have an RS485 transceiver that uses an
> IMX GPIO for the TX enable of the transceiver. Declare this properly.
> 
> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> ---
>  arch/arm/boot/dts/imx6qdl-gw52xx.dtsi | 3 +++
>  arch/arm/boot/dts/imx6qdl-gw53xx.dtsi | 3 +++
>  arch/arm/boot/dts/imx6qdl-gw54xx.dtsi | 3 +++
>  3 files changed, 9 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
> index 8bf1020..25ed57d 100644
> --- a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
> @@ -315,6 +315,8 @@
>  &uart1 {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_uart1>;
> +	fsl,uart-has-rtscts;

This vendor specific property is deprecated.  Please use generic
uart-has-rtscts instead.

> +	rts-gpio = <&gpio7 1 GPIO_ACTIVE_HIGH>;

I cannot find this property in any bindings doc.  What I found is
rts-gpios in bindings/serial/serial.txt.  Also, I'm not sure how this
property is supported by IMX driver.

Shawn

>  	status = "okay";
>  };
>  
> @@ -493,6 +495,7 @@
>  			fsl,pins = <
>  				MX6QDL_PAD_SD3_DAT7__UART1_TX_DATA	0x1b0b1
>  				MX6QDL_PAD_SD3_DAT6__UART1_RX_DATA	0x1b0b1
> +				MX6QDL_PAD_SD3_DAT4__GPIO7_IO01		0x4001b0b1 /* TEN */
>  			>;
>  		};
>  

[PATCH] ARM: dts: imx: ventana: add RS485 txen gpio support

[Tim Harvey]

On Mon, Aug 8, 2016 at 1:40 AM, Shawn Guo <shawnguo@kernel.org> wrote:
> On Wed, Jun 29, 2016 at 06:38:24AM -0700, Tim Harvey wrote:
>> The GW54xx, GW53xx, and GW52xx all have an RS485 transceiver that uses an
>> IMX GPIO for the TX enable of the transceiver. Declare this properly.
>>
>> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
>> ---
>>  arch/arm/boot/dts/imx6qdl-gw52xx.dtsi | 3 +++
>>  arch/arm/boot/dts/imx6qdl-gw53xx.dtsi | 3 +++
>>  arch/arm/boot/dts/imx6qdl-gw54xx.dtsi | 3 +++
>>  3 files changed, 9 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
>> index 8bf1020..25ed57d 100644
>> --- a/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
>> +++ b/arch/arm/boot/dts/imx6qdl-gw52xx.dtsi
>> @@ -315,6 +315,8 @@
>>  &uart1 {
>>       pinctrl-names = "default";
>>       pinctrl-0 = <&pinctrl_uart1>;
>> +     fsl,uart-has-rtscts;
>
> This vendor specific property is deprecated.  Please use generic
> uart-has-rtscts instead.
>

ok - will do for v2

>> +     rts-gpio = <&gpio7 1 GPIO_ACTIVE_HIGH>;
>
> I cannot find this property in any bindings doc.  What I found is
> rts-gpios in bindings/serial/serial.txt.  Also, I'm not sure how this
> property is supported by IMX driver.
>

This is supported by serial_mctrl_gpio
(drivers/tty/serial/serial_mctrl_gpio.c) and the IMX serial driver
does support mctrl-gpio. The serial_mctrl_gpio driver calls
devm_gpiod_get_index_optional for pin names of cts,dsr,dcd,rng,rts,
and dtr. This ends up calling 'of_find_gpio' for these names which
looks for them based on 'gpio_suffixes' which is defined as { "gpios",
"gpio" } so that plural or singular is supported.

Its not documented that gpio specifies can be singular in
Documentation/devicetree/bindings/serial/serial.txt but I always hate
seeing gpio properties named '*-gpios' that only specify a single gpio
so I figured I would use the singular representation. Because this is
not clear in the documentation and there is only one other board using
'rts-gpio' (am335x-nano.dts) vs 'rts-gpios' I will go ahead and change
it for v2 to be more consistent.

Regards,

Tim