[v4,6/9] dt-bindings: sc16is7xx: Add property to change GPIO function

Message ID 20230529140711.896830-7-hugo@hugovil.com
State New
Headers
Series serial: sc16is7xx: fix GPIO regression and rs485 improvements |

Commit Message

Hugo Villeneuve May 29, 2023, 2:07 p.m. UTC
  From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

Some variants in this series of UART controllers have GPIO pins that
are shared between GPIO and modem control lines.

The pin mux mode (GPIO or modem control lines) can be set for each
ports (channels) supported by the variant.

This adds a property to the device tree to set the GPIO pin mux to
modem control lines on selected ports if needed.

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
 .../bindings/serial/nxp,sc16is7xx.txt         | 46 +++++++++++++++++++
 1 file changed, 46 insertions(+)
  

Comments

Conor Dooley May 29, 2023, 2:16 p.m. UTC | #1
Hey Hugo,

On Mon, May 29, 2023 at 10:07:08AM -0400, Hugo Villeneuve wrote:
> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> 
> Some variants in this series of UART controllers have GPIO pins that
> are shared between GPIO and modem control lines.
> 
> The pin mux mode (GPIO or modem control lines) can be set for each
> ports (channels) supported by the variant.
> 
> This adds a property to the device tree to set the GPIO pin mux to
> modem control lines on selected ports if needed.
> 
> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>

Did I not ack this in v2? I didn't notice a reason for dropping it
in the cover etc. Was it intentionally dropped, or missed?

Cheers,
Conor.
  
Hugo Villeneuve May 29, 2023, 2:26 p.m. UTC | #2
On Mon, 29 May 2023 15:16:47 +0100
Conor Dooley <conor.dooley@microchip.com> wrote:

> Hey Hugo,
> 
> On Mon, May 29, 2023 at 10:07:08AM -0400, Hugo Villeneuve wrote:
> > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > 
> > Some variants in this series of UART controllers have GPIO pins that
> > are shared between GPIO and modem control lines.
> > 
> > The pin mux mode (GPIO or modem control lines) can be set for each
> > ports (channels) supported by the variant.
> > 
> > This adds a property to the device tree to set the GPIO pin mux to
> > modem control lines on selected ports if needed.
> > 
> > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> 
> Did I not ack this in v2? I didn't notice a reason for dropping it
> in the cover etc. Was it intentionally dropped, or missed?
> 
> Cheers,
> Conor.

Hi Conor,
In v3, I slighly modified the example, and that is why I didn't copy your ack.

Hugo.
  
Conor Dooley May 29, 2023, 6:09 p.m. UTC | #3
On Mon, May 29, 2023 at 10:26:01AM -0400, Hugo Villeneuve wrote:
> On Mon, 29 May 2023 15:16:47 +0100
> Conor Dooley <conor.dooley@microchip.com> wrote:
> 
> > Hey Hugo,
> > 
> > On Mon, May 29, 2023 at 10:07:08AM -0400, Hugo Villeneuve wrote:
> > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > 
> > > Some variants in this series of UART controllers have GPIO pins that
> > > are shared between GPIO and modem control lines.
> > > 
> > > The pin mux mode (GPIO or modem control lines) can be set for each
> > > ports (channels) supported by the variant.
> > > 
> > > This adds a property to the device tree to set the GPIO pin mux to
> > > modem control lines on selected ports if needed.
> > > 
> > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > 
> > Did I not ack this in v2? I didn't notice a reason for dropping it
> > in the cover etc. Was it intentionally dropped, or missed?

> In v3, I slighly modified the example, and that is why I didn't copy your ack.

Ah, I would say that for "slight modifications" when you have an ack,
you could keep it - but everyone is different and dropping tags is
likely to piss people off less than keeping them, so I understand
sticking on the safe side.
  
Conor Dooley May 29, 2023, 6:19 p.m. UTC | #4
On Mon, May 29, 2023 at 10:07:08AM -0400, Hugo Villeneuve wrote:
> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> 
> Some variants in this series of UART controllers have GPIO pins that
> are shared between GPIO and modem control lines.
> 
> The pin mux mode (GPIO or modem control lines) can be set for each
> ports (channels) supported by the variant.
> 
> This adds a property to the device tree to set the GPIO pin mux to
> modem control lines on selected ports if needed.
> 
> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> ---
>  .../bindings/serial/nxp,sc16is7xx.txt         | 46 +++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
> index 0fa8e3e43bf8..74dfbbf7b2cb 100644
> --- a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
> +++ b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
> @@ -23,6 +23,9 @@ Optional properties:
>      1 = active low.
>  - irda-mode-ports: An array that lists the indices of the port that
>  		   should operate in IrDA mode.
> +- nxp,modem-control-line-ports: An array that lists the indices of the port that
> +				should have shared GPIO lines configured as
> +				modem control lines.
>  
>  Example:
>          sc16is750: sc16is750@51 {
> @@ -35,6 +38,26 @@ Example:
>                  #gpio-cells = <2>;
>          };
>  
> +	sc16is752: sc16is752@54 {
> +		compatible = "nxp,sc16is752";
> +		reg = <0x54>;
> +		clocks = <&clk20m>;
> +		interrupt-parent = <&gpio3>;
> +		interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
> +		nxp,modem-control-line-ports = <1>; /* Port 1 as modem control lines */
> +		gpio-controller; /* Port 0 as GPIOs */
> +		#gpio-cells = <2>;
> +	};
> +
> +	sc16is752: sc16is752@54 {
> +		compatible = "nxp,sc16is752";
> +		reg = <0x54>;

If this were not a txt binding, dt_binding_check would likely complain
that you have two nodes with the same node address & 3 below.
If you end up re-submitting, could you change that please?
Otherwise,
Acked-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.
  
Hugo Villeneuve May 29, 2023, 6:33 p.m. UTC | #5
On Mon, 29 May 2023 19:19:27 +0100
Conor Dooley <conor@kernel.org> wrote:

> On Mon, May 29, 2023 at 10:07:08AM -0400, Hugo Villeneuve wrote:
> > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > 
> > Some variants in this series of UART controllers have GPIO pins that
> > are shared between GPIO and modem control lines.
> > 
> > The pin mux mode (GPIO or modem control lines) can be set for each
> > ports (channels) supported by the variant.
> > 
> > This adds a property to the device tree to set the GPIO pin mux to
> > modem control lines on selected ports if needed.
> > 
> > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > ---
> >  .../bindings/serial/nxp,sc16is7xx.txt         | 46 +++++++++++++++++++
> >  1 file changed, 46 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
> > index 0fa8e3e43bf8..74dfbbf7b2cb 100644
> > --- a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
> > +++ b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
> > @@ -23,6 +23,9 @@ Optional properties:
> >      1 = active low.
> >  - irda-mode-ports: An array that lists the indices of the port that
> >  		   should operate in IrDA mode.
> > +- nxp,modem-control-line-ports: An array that lists the indices of the port that
> > +				should have shared GPIO lines configured as
> > +				modem control lines.
> >  
> >  Example:
> >          sc16is750: sc16is750@51 {
> > @@ -35,6 +38,26 @@ Example:
> >                  #gpio-cells = <2>;
> >          };
> >  
> > +	sc16is752: sc16is752@54 {
> > +		compatible = "nxp,sc16is752";
> > +		reg = <0x54>;
> > +		clocks = <&clk20m>;
> > +		interrupt-parent = <&gpio3>;
> > +		interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
> > +		nxp,modem-control-line-ports = <1>; /* Port 1 as modem control lines */
> > +		gpio-controller; /* Port 0 as GPIOs */
> > +		#gpio-cells = <2>;
> > +	};
> > +
> > +	sc16is752: sc16is752@54 {
> > +		compatible = "nxp,sc16is752";
> > +		reg = <0x54>;
> 
> If this were not a txt binding, dt_binding_check would likely complain
> that you have two nodes with the same node address & 3 below.
> If you end up re-submitting, could you change that please?
> Otherwise,
> Acked-by: Conor Dooley <conor.dooley@microchip.com>

Hi Conor,
for the I2C section, I will use addresse 51, 52 and 54.

For the SPI section, I will use addresse 0, 1 and 2.

I will resubmit if there is a v5.

Thank you, Hugo.
  

Patch

diff --git a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
index 0fa8e3e43bf8..74dfbbf7b2cb 100644
--- a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
+++ b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt
@@ -23,6 +23,9 @@  Optional properties:
     1 = active low.
 - irda-mode-ports: An array that lists the indices of the port that
 		   should operate in IrDA mode.
+- nxp,modem-control-line-ports: An array that lists the indices of the port that
+				should have shared GPIO lines configured as
+				modem control lines.
 
 Example:
         sc16is750: sc16is750@51 {
@@ -35,6 +38,26 @@  Example:
                 #gpio-cells = <2>;
         };
 
+	sc16is752: sc16is752@54 {
+		compatible = "nxp,sc16is752";
+		reg = <0x54>;
+		clocks = <&clk20m>;
+		interrupt-parent = <&gpio3>;
+		interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
+		nxp,modem-control-line-ports = <1>; /* Port 1 as modem control lines */
+		gpio-controller; /* Port 0 as GPIOs */
+		#gpio-cells = <2>;
+	};
+
+	sc16is752: sc16is752@54 {
+		compatible = "nxp,sc16is752";
+		reg = <0x54>;
+		clocks = <&clk20m>;
+		interrupt-parent = <&gpio3>;
+		interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
+		nxp,modem-control-line-ports = <0 1>; /* Ports 0 and 1 as modem control lines */
+	};
+
 * spi as bus
 
 Required properties:
@@ -59,6 +82,9 @@  Optional properties:
     1 = active low.
 - irda-mode-ports: An array that lists the indices of the port that
 		   should operate in IrDA mode.
+- nxp,modem-control-line-ports: An array that lists the indices of the port that
+				should have shared GPIO lines configured as
+				modem control lines.
 
 Example:
 	sc16is750: sc16is750@0 {
@@ -70,3 +96,23 @@  Example:
 		gpio-controller;
 		#gpio-cells = <2>;
 	};
+
+	sc16is752: sc16is752@0 {
+		compatible = "nxp,sc16is752";
+		reg = <0>;
+		clocks = <&clk20m>;
+		interrupt-parent = <&gpio3>;
+		interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
+		nxp,modem-control-line-ports = <1>; /* Port 1 as modem control lines */
+		gpio-controller; /* Port 0 as GPIOs */
+		#gpio-cells = <2>;
+	};
+
+	sc16is752: sc16is752@0 {
+		compatible = "nxp,sc16is752";
+		reg = <0>;
+		clocks = <&clk20m>;
+		interrupt-parent = <&gpio3>;
+		interrupts = <7 IRQ_TYPE_EDGE_FALLING>;
+		nxp,modem-control-line-ports = <0 1>; /* Ports 0 and 1 as modem control lines */
+	};