linux-kernel - Re: [PATCH v3 1/7] tty: 8250: Fix IS-200 PCI ID comment

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] [day] [month] [year] [list]

Message-ID: <DU0PR02MB789959879FC6EC46936A157CC4D7A@DU0PR02MB7899.eurprd02.prod.outlook.com>
Date:   Mon, 16 Oct 2023 08:29:48 +0100
From:   Cameron Williams <cang1@...e.co.uk>
To:     Jiri Slaby <jirislaby@...nel.org>
Cc:     gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org,
        linux-serial@...r.kernel.org
Subject: Re: [PATCH v3 1/7] tty: 8250: Fix IS-200 PCI ID comment

On Mon, Oct 16, 2023 at 07:37:43AM +0200, Jiri Slaby wrote:
Hi Jiri
> On 15. 10. 23, 19:10, Cameron Williams wrote:
> > Fix the PCI comment for the IS-200 card. The PCI ID for the IS-200
> > is 0x0d80, and the definition used (PCI_DEVICE_ID_INTASHIELD_IS200)
> > is indeed 0x0d80, clarify that by fixing the comment as its
> > neighbouring cards are all at 0x0020 offsets.
> 
> The real question is why to maintain a comment here at all? I suggest
> dropping them both instead.
>
I try to avoid touching other people's code unless it's wrong as is the
case here. I could remove them both but since they both (IS-200 and IS-400)
refer to the definition in pci_ids.h, and the rest of the cards use the raw
hex IDs instead, its more consistent to have them all showing the card ID in
one way or another, IMHO.
> > Signed-off-by: Cameron Williams <cang1@...e.co.uk>
> > ---
> > v2 - v3:
> > Clarify commit message with better explanation of the change.
> > Re-submit patch series using git send-email to make threading work.
> > 
> > v1 - v2:
> > This is a resubmission series for the patch series below. That series
> > was lots of changes sent to lots of maintainers, this series is just for
> > the tty/serial/8250 subsystem.
> > 
> > [1] https://lore.kernel.org/all/DU0PR02MB789950E64D808DB57E9D7312C4F8A@DU0PR02MB7899.eurprd02.prod.outlook.com/
> > [2] https://lore.kernel.org/all/DU0PR02MB7899DE53DFC900EFB50E53F2C4F8A@DU0PR02MB7899.eurprd02.prod.outlook.com/
> > [3] https://lore.kernel.org/all/DU0PR02MB7899033E7E81EAF3694BC20AC4F8A@DU0PR02MB7899.eurprd02.prod.outlook.com/
> > [4] https://lore.kernel.org/all/DU0PR02MB7899EABA8C3DCAC94DCC79D4C4F8A@DU0PR02MB7899.eurprd02.prod.outlook.com/
> > 
> >   drivers/tty/serial/8250/8250_pci.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
> > index 62a9bd30b4db..ecb4e9acc70d 100644
> > --- a/drivers/tty/serial/8250/8250_pci.c
> > +++ b/drivers/tty/serial/8250/8250_pci.c
> > @@ -4917,7 +4917,7 @@ static const struct pci_device_id serial_pci_tbl[] = {
> >   	 * IntaShield IS-200
> >   	 */
> >   	{	PCI_VENDOR_ID_INTASHIELD, PCI_DEVICE_ID_INTASHIELD_IS200,
> > -		PCI_ANY_ID, PCI_ANY_ID, 0, 0,	/* 135a.0811 */
> > +		PCI_ANY_ID, PCI_ANY_ID, 0, 0,	/* 135a.0d80 */
> >   		pbn_b2_2_115200 },
> >   	/*
> >   	 * IntaShield IS-400
> 
> thanks,
> -- 
> js
> suse labs
>