Re: [PATCH v2 5/5] board: add support for Schneider HMIBSC board

[Stephan Gerhold <stephan@gerhold.net>]

On Wed, Mar 13, 2024 at 12:08:58PM +0530, Sumit Garg wrote:
> On Mon, 11 Mar 2024 at 20:07, Stephan Gerhold <stephan@gerhold.net> wrote:
> > On Mon, Mar 11, 2024 at 04:40:26PM +0530, Sumit Garg wrote:
> > > Support for Schneider Electric HMIBSC. Features:
> > > - Qualcomm Snapdragon 410C SoC - APQ8016 (4xCortex A53, Adreno 306)
> > > - 2GiB RAM
> > > - 64GiB eMMC, SD slot
> > > - WiFi and Bluetooth
> > > - 2x Host, 1x Device USB port
> > > - HDMI
> > > - Discrete TPM2 chip over SPI
> > >
> > > Features enabled in U-Boot:
> > > - RAUC updates
> > > - Environment protection
> > > - USB based ethernet adaptors
> > >
> > > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> >
> [...]
> > > +     gpio_rs232_high: gpio_rs232_high {
> >
> > Pretty sure DT schema checks would complain about this node name (need
> > -state suffix, no underscores).
> 
> We have the dtbs_check in U-Boot too. I will use that before posting
> the next version.
> 
> >
> > > +             bootph-all;
> > > +             pins = "gpio99";
> > > +             function = "gpio";
> > > +
> > > +             drive-strength = <16>;
> > > +             bias-disable;
> > > +             output-high;
> > > +     };
> > > +
> > > +     gpio_rs232_low: gpio_rs232_low {
> >
> > Same here.
> >
> > Also, since I'm looking at this a bit more closely now, are there maybe
> > more clear label/node names you could use here, or a comment you could
> > add what exactly these pins do? I guess this enables something about
> > RS232 but it's not clear what exactly.
> 
> Actually these GPIOs are a mux to select among different UART modes
> (RS232/422/485). This configuration allows you to select RS232 mode.
> How about following label/node names?
> 
> uart1_mux0_rs232_high: uart1-mux0-rs232-state
> 
> uart1_mux1_rs232_low: uart1-mux1-rs232-state
> 

Hm, is it a 2 bit mux selector like

	gpio99	gpio100	UART mode
	0	0	?
	0	1	?
	1	0	RS232
	1	1	?

and the others are RS422 and RS485? If yes, a comment with the table of
the function assignments would help a lot for clarity. With that,
precise naming would not be that important anymore. :-)

Thanks,
Stephan