| Message ID | 20230607140525.833982-6-hugo@hugovil.com |
|---|---|
| State | New |
| Headers | show |
| Series | serial: sc16is7xx: fix GPIO regression and rs485 improvements | expand |
On 07/06/2023 16:05, 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. > > Cc: <stable@vger.kernel.org> # 6.1.x > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> > Acked-by: Conor Dooley <conor.dooley@microchip.com> > Reviewed-by: Lech Perczak <lech.perczak@camlingroup.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..1a7e4bff0456 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@53 { Since you keep sending new versions, fix the names. nNode names should be generic. See also explanation and list of examples in DT specification: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation Best regards, Krzysztof
On Wed, 7 Jun 2023 16:30:26 +0200 Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > On 07/06/2023 16:05, 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. > > > > Cc: <stable@vger.kernel.org> # 6.1.x > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> > > Acked-by: Conor Dooley <conor.dooley@microchip.com> > > Reviewed-by: Lech Perczak <lech.perczak@camlingroup.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..1a7e4bff0456 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@53 { > > Since you keep sending new versions, fix the names. nNode names should > be generic. See also explanation and list of examples in DT specification: > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation Hi, by the way, I do not "keep sending new versions" just for the fun of it... Even after reading your link, I cannot see what is wrong with that name. You are the first one since V1 to think it is inappropriate. Please suggest what the name should be. Hugo.
On 07/06/2023 16:41, Hugo Villeneuve wrote: > On Wed, 7 Jun 2023 16:30:26 +0200 > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > >> On 07/06/2023 16:05, 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. >>> >>> Cc: <stable@vger.kernel.org> # 6.1.x >>> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> >>> Acked-by: Conor Dooley <conor.dooley@microchip.com> >>> Reviewed-by: Lech Perczak <lech.perczak@camlingroup.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..1a7e4bff0456 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@53 { >> >> Since you keep sending new versions, fix the names. nNode names should >> be generic. See also explanation and list of examples in DT specification: >> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > > Hi, > by the way, I do not "keep sending new versions" just for the fun of > it... Sure, I know. So when the next version is necessary to send, fix also this. > > Even after reading your link, I cannot see what is wrong with that > name. They are not generic. They are specific. > > You are the first one since V1 to think it is inappropriate. > > Please suggest what the name should be. Depends what this is. I know nothing about this device, so I cannot suggest you. Therefore I gave you link with ideas to choose from. Best regards, Krzysztof
On Wed, 7 Jun 2023 16:46:56 +0200 Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > On 07/06/2023 16:41, Hugo Villeneuve wrote: > > On Wed, 7 Jun 2023 16:30:26 +0200 > > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > > >> On 07/06/2023 16:05, 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. > >>> > >>> Cc: <stable@vger.kernel.org> # 6.1.x > >>> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> > >>> Acked-by: Conor Dooley <conor.dooley@microchip.com> > >>> Reviewed-by: Lech Perczak <lech.perczak@camlingroup.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..1a7e4bff0456 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@53 { > >> > >> Since you keep sending new versions, fix the names. nNode names should > >> be generic. See also explanation and list of examples in DT specification: > >> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > > > > Hi, > > by the way, I do not "keep sending new versions" just for the fun of > > it... > > Sure, I know. So when the next version is necessary to send, fix also this. > > > > > Even after reading your link, I cannot see what is wrong with that > > name. > > They are not generic. They are specific. What do you mean by "They"? My patch adds only a new property... This patch will not fix old names, but a future patch could do it. And what do you mean by "They are not generic. They are specific". Of course the property is specific, because it relates to something very specific to this chip? > > You are the first one since V1 to think it is inappropriate. > > > > Please suggest what the name should be. > > Depends what this is. I know nothing about this device, so I cannot > suggest you. Therefore I gave you link with ideas to choose from. > > Best regards, > Krzysztof > >
On 07/06/2023 16:52, Hugo Villeneuve wrote: > On Wed, 7 Jun 2023 16:46:56 +0200 > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > >> On 07/06/2023 16:41, Hugo Villeneuve wrote: >>> On Wed, 7 Jun 2023 16:30:26 +0200 >>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: >>> >>>> On 07/06/2023 16:05, 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. >>>>> >>>>> Cc: <stable@vger.kernel.org> # 6.1.x >>>>> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> >>>>> Acked-by: Conor Dooley <conor.dooley@microchip.com> >>>>> Reviewed-by: Lech Perczak <lech.perczak@camlingroup.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..1a7e4bff0456 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@53 { >>>> >>>> Since you keep sending new versions, fix the names. nNode names should >>>> be generic. See also explanation and list of examples in DT specification: >>>> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation >>> >>> Hi, >>> by the way, I do not "keep sending new versions" just for the fun of >>> it... >> >> Sure, I know. So when the next version is necessary to send, fix also this. >> >>> >>> Even after reading your link, I cannot see what is wrong with that >>> name. >> >> They are not generic. They are specific. > > What do you mean by "They"? My patch adds only a new property... Your patch adds multiple new examples. My comment was at appropriate place to fix, although only the first place. I did not refer to unrelated pieces or other parts of the code. Specifically - I wrote nothing about property. > > This patch will not fix old names, but a future patch could do it. No worries, I asked for new code. > > And what do you mean by "They are not generic. They are specific". Of > course the property is specific, because it relates to something very > specific to this chip? Again, I did not comment under a property. I did not refer to any property. Best regards, Krzysztof
On Wed, 7 Jun 2023 16:57:48 +0200 Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > On 07/06/2023 16:52, Hugo Villeneuve wrote: > > On Wed, 7 Jun 2023 16:46:56 +0200 > > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > > >> On 07/06/2023 16:41, Hugo Villeneuve wrote: > >>> On Wed, 7 Jun 2023 16:30:26 +0200 > >>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > >>> > >>>> On 07/06/2023 16:05, 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. > >>>>> > >>>>> Cc: <stable@vger.kernel.org> # 6.1.x > >>>>> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com> > >>>>> Acked-by: Conor Dooley <conor.dooley@microchip.com> > >>>>> Reviewed-by: Lech Perczak <lech.perczak@camlingroup.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..1a7e4bff0456 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@53 { > >>>> > >>>> Since you keep sending new versions, fix the names. nNode names should > >>>> be generic. See also explanation and list of examples in DT specification: > >>>> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > >>> > >>> Hi, > >>> by the way, I do not "keep sending new versions" just for the fun of > >>> it... > >> > >> Sure, I know. So when the next version is necessary to send, fix also this. > >> > >>> > >>> Even after reading your link, I cannot see what is wrong with that > >>> name. > >> > >> They are not generic. They are specific. > > > > What do you mean by "They"? My patch adds only a new property... > > Your patch adds multiple new examples. My comment was at appropriate > place to fix, although only the first place. I did not refer to > unrelated pieces or other parts of the code. Specifically - I wrote > nothing about property. Ahhh!!!!!!!!!!!! Ok, now I see. But I simply added new examples based on the previous examples, which already used "inappropriate" names. If I modify only the node names in my new examples, this will clash with the existing examples. I have already prepared a new patch, that I will send one day if we can ever finish this never ending series, to convert the binding to YAML, and to fix a few things with old properties missing vendor prefix, etc. I will modify all the examples in this new series to follow what you suggest and incorporate it into this new serie. Hugo. > > This patch will not fix old names, but a future patch could do it. > > No worries, I asked for new code. > > > > > And what do you mean by "They are not generic. They are specific". Of > > course the property is specific, because it relates to something very > > specific to this chip? > > Again, I did not comment under a property. I did not refer to any property. > > > Best regards, > Krzysztof
diff --git a/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt b/Documentation/devicetree/bindings/serial/nxp,sc16is7xx.txt index 0fa8e3e43bf8..1a7e4bff0456 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@53 { + compatible = "nxp,sc16is752"; + reg = <0x53>; + 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@1 { + compatible = "nxp,sc16is752"; + reg = <1>; + 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@2 { + compatible = "nxp,sc16is752"; + reg = <2>; + 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 */ + };