linux-kernel - Re: [PATCH v8 05/10] dt-bindings: sc16is7xx: Add property to change GPIO function

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20230607105249.6dc95ed226e3578a3c9116aa@hugovil.com>
Date:   Wed, 7 Jun 2023 10:52:49 -0400
From:   Hugo Villeneuve <hugo@...ovil.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc:     gregkh@...uxfoundation.org, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
        jirislaby@...nel.org, jringle@...dpoint.com,
        jesse.sung@...onical.com, isaac.true@...onical.com,
        l.perczak@...lintechnologies.com, tomasz.mon@...lingroup.com,
        linux-serial@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-gpio@...r.kernel.org,
        Hugo Villeneuve <hvilleneuve@...onoff.com>,
        stable@...r.kernel.org, Conor Dooley <conor.dooley@...rochip.com>,
        Lech Perczak <lech.perczak@...lingroup.com>
Subject: Re: [PATCH v8 05/10] dt-bindings: sc16is7xx: Add property to change
 GPIO function

On Wed, 7 Jun 2023 16:46:56 +0200
Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org> wrote:

> On 07/06/2023 16:41, Hugo Villeneuve wrote:
> > On Wed, 7 Jun 2023 16:30:26 +0200
> > Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org> wrote:
> > 
> >> On 07/06/2023 16:05, Hugo Villeneuve wrote:
> >>> From: Hugo Villeneuve <hvilleneuve@...onoff.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@...r.kernel.org> # 6.1.x
> >>> Signed-off-by: Hugo Villeneuve <hvilleneuve@...onoff.com>
> >>> Acked-by: Conor Dooley <conor.dooley@...rochip.com>
> >>> Reviewed-by: Lech Perczak <lech.perczak@...lingroup.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
> 
>