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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 21 Sep 2022 00:35:52 +0100 (BST)
From:   "Maciej W. Rozycki" <macro@...am.me.uk>
To:     Jiri Slaby <jirislaby@...nel.org>
cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Josh Triplett <josh@...htriplett.org>,
        Anders Blomdell <anders.blomdell@...trol.lth.se>,
        linux-serial@...r.kernel.org, linux-kernel@...r.kernel.org,
        stable@...r.kernel.org
Subject: Re: [PATCH 2/2] serial: 8250: Request full 16550A feature probing
 for OxSemi PCIe devices

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