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

[Hugo Villeneuve]

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

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