DT checker RS485 unevaluated property, 8250 OMAP UART

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")) {

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?


[1] https://lore.kernel.org/all/ZBItlBhzo+YETcJO@francesco-nb.int.toradex.com/

Francesco

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