Re: [PATCH v4 1/1] ns16550: add support for polling mode when device tree is used

[Julien Grall <julien@xxxxxxx>]

Hi,

On 01/08/2023 10:59, Jan Beulich wrote:
> On 01.08.2023 11:46, Oleksii wrote:
> > On Mon, 2023-07-31 at 15:24 +0200, Jan Beulich wrote:
> > > On 27.07.2023 18:45, Oleksii Kurochko wrote:
> > > > @@ -654,6 +674,9 @@ static void ns16550_init_common(struct ns16550
> > > > *uart)
> > > >   
> > > >       /* Default lsr_mask = UART_LSR_THRE */
> > > >       uart->lsr_mask  = UART_LSR_THRE;
> > > > +
> > > > +    if ( strstr(opt_com1, "poll") || strstr(opt_com2, "poll") )
> > > > +        uart->intr_works = polling;
> > > >   }
> > > A non-__init function may not reference __initdata objects. But
> > > strstr()
> > > is too lax anyway, and you also shouldn't check the wrong port's
> > > options.
> > > You want to recognize "poll" _only_ where all other command line
> > > options
> > > are processed.
> > Just to confirm, do you mean that I should use parse_positional() ( or
> > something similar ) for the device-tree method of initializing ns16550?
> >
> > I checked the polling behavior as described above because
> > parse_positional() is utilized solely for X86.
> >
> > It appears that command line options are parsed only in the case of
> > x86.
> Hmm, looks like you're right. Arm folks will want to clarify how they
> got away without any command line overrides so far,
So far, everything we needed was described in the firmware table (e.g. 
ACPI/Device-Tree).
> and how they think
> now necessary ones should be suitably added.
I think "poll" is the only parameter that we should parse on Arm.

> I recall I had reservations
> when the file was massaged to compile out supposedly x86-only code.
I had a look at the code protected by CONFIG_X86. Most of it still want 
to be configured behind CONFIG_X86 at least until we have PCI support. 
But even with it, some work would be necessary to be able to support PCI 
UART.
Cheers,

--
Julien Grall