Message ID | ZGefR4mTHHo1iQ7H@francesco-nb.int.toradex.com |
---|---|
State | Changes Requested, archived |
Headers | show |
Series | DT checker RS485 unevaluated property, 8250 OMAP UART | expand |
Context | Check | Description |
---|---|---|
robh/checkpatch | warning | total: 1 errors, 2 warnings, 7 lines checked |
robh/patch-applied | success | |
robh/dtbs-check | warning | build log |
robh/dt-meta-schema | success |
On 19/05/23 9:39 pm, Francesco Dolcini wrote: > Hello, > while writing a new DT file I stumbled across this warning > > .../arch/arm64/boot/dts/ti/k3-am625-verdin-wifi-dev.dtb: serial@2810000: Unevaluated properties are not allowed ('rs485-rts-active-high' was unexpected) > From schema: .../Documentation/devicetree/bindings/serial/8250_omap.yaml > > The property is currently used in the OMAP serial driver > > drivers/tty/serial/omap-serial.c > 1511: if (of_property_read_bool(np, "rs485-rts-active-high")) { > Would be it possible to update driver to imply rs485-rts-active-high" this by lack of rs485-rts-active-low property in DT instead? > and a few DT files. > > I do require it, despite being wrong, because of some legacy reasons [1]. > > Before commit 767d3467eb60 ("dt-bindings: serial: 8250_omap: drop rs485 > properties") this property was allowed. > > What should I do? > - ignore the warning > - send a patch to reintroduce `rs485-rts-active-high: true` in 8250_omap.yaml > - something else? > > I would be inclined to send the following patch, do you agree? > > diff --git a/Documentation/devicetree/bindings/serial/8250_omap.yaml b/Documentation/devicetree/bindings/serial/8250_omap.yaml > index eb3488d8f9ee..e634e98aa994 100644 > --- a/Documentation/devicetree/bindings/serial/8250_omap.yaml > +++ b/Documentation/devicetree/bindings/serial/8250_omap.yaml > @@ -70,6 +70,7 @@ properties: > dsr-gpios: true > rng-gpios: true > dcd-gpios: true > + rs485-rts-active-low: true I believe you mean rs485-rts-active-high here > rts-gpio: true > power-domains: true > clock-frequency: true > > [1] https://lore.kernel.org/all/ZBItlBhzo+YETcJO@francesco-nb.int.toradex.com/ > Also, I hope you are using 8250_ompa.c and not omap-serial.c for newer designs. omap-serial.c is mostly there to support legacy SoCs and not to be used with K3 SoCs. Regards Vignesh
On Sat, May 27, 2023 at 11:49:17AM +0530, Vignesh Raghavendra wrote: > On 19/05/23 9:39 pm, Francesco Dolcini wrote: > > Hello, > > while writing a new DT file I stumbled across this warning > > > > .../arch/arm64/boot/dts/ti/k3-am625-verdin-wifi-dev.dtb: serial@2810000: Unevaluated properties are not allowed ('rs485-rts-active-high' was unexpected) > > From schema: .../Documentation/devicetree/bindings/serial/8250_omap.yaml > > > > The property is currently used in the OMAP serial driver > > > > drivers/tty/serial/omap-serial.c > > 1511: if (of_property_read_bool(np, "rs485-rts-active-high")) { > > > > Would be it possible to update driver to imply rs485-rts-active-high" > this by lack of rs485-rts-active-low property in DT instead? What about backward compatibility? This is what is done in all drivers apart omap-serial if I'm not wrong. > > I would be inclined to send the following patch, do you agree? > > > > diff --git a/Documentation/devicetree/bindings/serial/8250_omap.yaml b/Documentation/devicetree/bindings/serial/8250_omap.yaml > > index eb3488d8f9ee..e634e98aa994 100644 > > --- a/Documentation/devicetree/bindings/serial/8250_omap.yaml > > +++ b/Documentation/devicetree/bindings/serial/8250_omap.yaml > > @@ -70,6 +70,7 @@ properties: > > dsr-gpios: true > > rng-gpios: true > > dcd-gpios: true > > + rs485-rts-active-low: true > > I believe you mean rs485-rts-active-high here whoops, yes of course. > > > rts-gpio: true > > power-domains: true > > clock-frequency: true > > > > [1] https://lore.kernel.org/all/ZBItlBhzo+YETcJO@francesco-nb.int.toradex.com/ > > > > Also, I hope you are using 8250_ompa.c and not omap-serial.c for newer > designs. omap-serial.c is mostly there to support legacy SoCs and not to > be used with K3 SoCs. Thanks for this head-up. This confused myself, while the issue I reported here is real, it does affect only omap-serial.c. In my case I am using 8250_omap.c and I can just omit the property from the DTS as generally expected! Francesco
On 30/05/2023 12:13, Francesco Dolcini wrote: > On Sat, May 27, 2023 at 11:49:17AM +0530, Vignesh Raghavendra wrote: >> On 19/05/23 9:39 pm, Francesco Dolcini wrote: >>> Hello, >>> while writing a new DT file I stumbled across this warning >>> >>> .../arch/arm64/boot/dts/ti/k3-am625-verdin-wifi-dev.dtb: serial@2810000: Unevaluated properties are not allowed ('rs485-rts-active-high' was unexpected) >>> From schema: .../Documentation/devicetree/bindings/serial/8250_omap.yaml >>> >>> The property is currently used in the OMAP serial driver >>> >>> drivers/tty/serial/omap-serial.c >>> 1511: if (of_property_read_bool(np, "rs485-rts-active-high")) { >>> >> >> Would be it possible to update driver to imply rs485-rts-active-high" >> this by lack of rs485-rts-active-low property in DT instead? > > What about backward compatibility? This is what is done in all drivers > apart omap-serial if I'm not wrong. Uh, I removed too much in that commit. We need to keep backwards compatibility because driver used default (missing property) as active low. > >>> I would be inclined to send the following patch, do you agree? >>> >>> diff --git a/Documentation/devicetree/bindings/serial/8250_omap.yaml b/Documentation/devicetree/bindings/serial/8250_omap.yaml >>> index eb3488d8f9ee..e634e98aa994 100644 >>> --- a/Documentation/devicetree/bindings/serial/8250_omap.yaml >>> +++ b/Documentation/devicetree/bindings/serial/8250_omap.yaml >>> @@ -70,6 +70,7 @@ properties: >>> dsr-gpios: true >>> rng-gpios: true >>> dcd-gpios: true >>> + rs485-rts-active-low: true >> >> I believe you mean rs485-rts-active-high here > whoops, yes of course. Yes, with a Fixes tag. > >> >>> rts-gpio: true >>> power-domains: true >>> clock-frequency: true >>> >>> [1] https://lore.kernel.org/all/ZBItlBhzo+YETcJO@francesco-nb.int.toradex.com/ >>> >> >> Also, I hope you are using 8250_ompa.c and not omap-serial.c for newer >> designs. omap-serial.c is mostly there to support legacy SoCs and not to >> be used with K3 SoCs. > > Thanks for this head-up. This confused myself, while the issue I > reported here is real, it does affect only omap-serial.c. In my case I > am using 8250_omap.c and I can just omit the property from the DTS as > generally expected! OK. Are you going send a fix for original issue? Best regards, Krzysztof
On Tue, May 30, 2023 at 01:38:42PM +0200, Krzysztof Kozlowski wrote: > On 30/05/2023 12:13, Francesco Dolcini wrote: > > On Sat, May 27, 2023 at 11:49:17AM +0530, Vignesh Raghavendra wrote: > >> On 19/05/23 9:39 pm, Francesco Dolcini wrote: > >>> I would be inclined to send the following patch, do you agree? > >>> > >>> diff --git a/Documentation/devicetree/bindings/serial/8250_omap.yaml b/Documentation/devicetree/bindings/serial/8250_omap.yaml > >>> index eb3488d8f9ee..e634e98aa994 100644 > >>> --- a/Documentation/devicetree/bindings/serial/8250_omap.yaml > >>> +++ b/Documentation/devicetree/bindings/serial/8250_omap.yaml > >>> @@ -70,6 +70,7 @@ properties: > >>> dsr-gpios: true > >>> rng-gpios: true > >>> dcd-gpios: true > >>> + rs485-rts-active-low: true > >> > >> I believe you mean rs485-rts-active-high here > > whoops, yes of course. > > Yes, with a Fixes tag. Ack > >>> rts-gpio: true > >>> power-domains: true > >>> clock-frequency: true > >>> > >>> [1] https://lore.kernel.org/all/ZBItlBhzo+YETcJO@francesco-nb.int.toradex.com/ > >>> > >> > >> Also, I hope you are using 8250_ompa.c and not omap-serial.c for newer > >> designs. omap-serial.c is mostly there to support legacy SoCs and not to > >> be used with K3 SoCs. > > > > Thanks for this head-up. This confused myself, while the issue I > > reported here is real, it does affect only omap-serial.c. In my case I > > am using 8250_omap.c and I can just omit the property from the DTS as > > generally expected! > > OK. Are you going send a fix for original issue? I'll do it, thanks!
diff --git a/Documentation/devicetree/bindings/serial/8250_omap.yaml b/Documentation/devicetree/bindings/serial/8250_omap.yaml index eb3488d8f9ee..e634e98aa994 100644 --- a/Documentation/devicetree/bindings/serial/8250_omap.yaml +++ b/Documentation/devicetree/bindings/serial/8250_omap.yaml @@ -70,6 +70,7 @@ properties: dsr-gpios: true rng-gpios: true dcd-gpios: true + rs485-rts-active-low: true rts-gpio: true power-domains: true clock-frequency: true