Re: [PATCH 2/2] serial: 8250: Request full 16550A feature probing for OxSemi PCIe devices

From: Maciej W. Rozycki
Date: Tue Sep 20 2022 - 19:41:46 EST


On Mon, 19 Sep 2022, Maciej W. Rozycki wrote:

> > > linux-serial-8250-oxsemi-efr.diff
> > > Index: linux-macro/drivers/tty/serial/8250/8250_pci.c
> > > ===================================================================
> > > --- linux-macro.orig/drivers/tty/serial/8250/8250_pci.c
> > > +++ linux-macro/drivers/tty/serial/8250/8250_pci.c
> > > @@ -1232,6 +1232,10 @@ static void pci_oxsemi_tornado_set_mctrl
> > > serial8250_do_set_mctrl(port, mctrl);
> > > }
> > > +/*
> > > + * We require EFR features for clock programming, so set UPF_FULL_PROBE
> > > + * for full probing regardless of CONFIG_SERIAL_8250_16550A_VARIANTS
> > > setting.
> > > + */
> >
> > It'd make more sense to me to move this comment right before the line you add
> > below.
>
> I favour the style where what a function does is documented above it, but
> I won't insist on it if having a comment within is what we prefer here.

Having looked at it again I changed my mind and decided it'll be more
consistent with the rest of the code if this comment remains above the
function after all.

My rationale is it is the only function for OxSemi Tornado devices still
without an introductory comment, the other functions have their internals
documented solely within their leading comments, and last but not least it
is obvious what the comment refers to, especially as the function is so
small (as to fit even in an 80x24 character glass TTY device).

I have posted v2 with your other suggestions applied. Thank you for your
review.

Maciej