2023-11-22 15:47:25

by Vladimir Oltean

[permalink] [raw]
Subject: [PATCH pci] PCI: remove the PCI_VENDOR_ID_NXP alias

What is today NXP is the result of some mergers (with Freescale) and
spin-offs (from Philips).

New NXP hardware (for example NETC version 4.1 of the NXP i.MX95
SoC) uses PCI_VENDOR_ID_PHILIPS. And some older hardware uses
PCI_VENDOR_ID_FREESCALE.

If we have PCI_VENDOR_ID_NXP as an alias for PCI_VENDOR_ID_FREESCALE,
we end up needing something like a PCI_VENDOR_ID_NXP2 alias for
PCI_VENDOR_ID_PHILIPS. I think this is more confusing than just spelling
out the vendor ID of the original company that claimed it.

FWIW, the pci.ids repository as of today has:
1131 Philips Semiconductors
1957 Freescale Semiconductor Inc

so this makes the kernel code consistent with that, and with what
"lspci" prints.

Signed-off-by: Vladimir Oltean <[email protected]>
---
drivers/pci/quirks.c | 50 ++++++++++++++++++++---------------------
include/linux/pci_ids.h | 1 -
2 files changed, 25 insertions(+), 26 deletions(-)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index d208047d1b8f..c95701e36d58 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -5092,39 +5092,39 @@ static const struct pci_dev_acs_enabled {
{ PCI_VENDOR_ID_ZHAOXIN, 0x3038, pci_quirk_mf_endpoint_acs },
{ PCI_VENDOR_ID_ZHAOXIN, 0x3104, pci_quirk_mf_endpoint_acs },
{ PCI_VENDOR_ID_ZHAOXIN, 0x9083, pci_quirk_mf_endpoint_acs },
- /* NXP root ports, xx=16, 12, or 08 cores */
+ /* Freescale/NXP root ports, xx=16, 12, or 08 cores */
/* LX2xx0A : without security features + CAN-FD */
- { PCI_VENDOR_ID_NXP, 0x8d81, pci_quirk_nxp_rp_acs },
- { PCI_VENDOR_ID_NXP, 0x8da1, pci_quirk_nxp_rp_acs },
- { PCI_VENDOR_ID_NXP, 0x8d83, pci_quirk_nxp_rp_acs },
+ { PCI_VENDOR_ID_FREESCALE, 0x8d81, pci_quirk_nxp_rp_acs },
+ { PCI_VENDOR_ID_FREESCALE, 0x8da1, pci_quirk_nxp_rp_acs },
+ { PCI_VENDOR_ID_FREESCALE, 0x8d83, pci_quirk_nxp_rp_acs },
/* LX2xx0C : security features + CAN-FD */
- { PCI_VENDOR_ID_NXP, 0x8d80, pci_quirk_nxp_rp_acs },
- { PCI_VENDOR_ID_NXP, 0x8da0, pci_quirk_nxp_rp_acs },
- { PCI_VENDOR_ID_NXP, 0x8d82, pci_quirk_nxp_rp_acs },
+ { PCI_VENDOR_ID_FREESCALE, 0x8d80, pci_quirk_nxp_rp_acs },
+ { PCI_VENDOR_ID_FREESCALE, 0x8da0, pci_quirk_nxp_rp_acs },
+ { PCI_VENDOR_ID_FREESCALE, 0x8d82, pci_quirk_nxp_rp_acs },
/* LX2xx0E : security features + CAN */
- { PCI_VENDOR_ID_NXP, 0x8d90, pci_quirk_nxp_rp_acs },
- { PCI_VENDOR_ID_NXP, 0x8db0, pci_quirk_nxp_rp_acs },
- { PCI_VENDOR_ID_NXP, 0x8d92, pci_quirk_nxp_rp_acs },
+ { PCI_VENDOR_ID_FREESCALE, 0x8d90, pci_quirk_nxp_rp_acs },
+ { PCI_VENDOR_ID_FREESCALE, 0x8db0, pci_quirk_nxp_rp_acs },
+ { PCI_VENDOR_ID_FREESCALE, 0x8d92, pci_quirk_nxp_rp_acs },
/* LX2xx0N : without security features + CAN */
- { PCI_VENDOR_ID_NXP, 0x8d91, pci_quirk_nxp_rp_acs },
- { PCI_VENDOR_ID_NXP, 0x8db1, pci_quirk_nxp_rp_acs },
- { PCI_VENDOR_ID_NXP, 0x8d93, pci_quirk_nxp_rp_acs },
+ { PCI_VENDOR_ID_FREESCALE, 0x8d91, pci_quirk_nxp_rp_acs },
+ { PCI_VENDOR_ID_FREESCALE, 0x8db1, pci_quirk_nxp_rp_acs },
+ { PCI_VENDOR_ID_FREESCALE, 0x8d93, pci_quirk_nxp_rp_acs },
/* LX2xx2A : without security features + CAN-FD */
- { PCI_VENDOR_ID_NXP, 0x8d89, pci_quirk_nxp_rp_acs },
- { PCI_VENDOR_ID_NXP, 0x8da9, pci_quirk_nxp_rp_acs },
- { PCI_VENDOR_ID_NXP, 0x8d8b, pci_quirk_nxp_rp_acs },
+ { PCI_VENDOR_ID_FREESCALE, 0x8d89, pci_quirk_nxp_rp_acs },
+ { PCI_VENDOR_ID_FREESCALE, 0x8da9, pci_quirk_nxp_rp_acs },
+ { PCI_VENDOR_ID_FREESCALE, 0x8d8b, pci_quirk_nxp_rp_acs },
/* LX2xx2C : security features + CAN-FD */
- { PCI_VENDOR_ID_NXP, 0x8d88, pci_quirk_nxp_rp_acs },
- { PCI_VENDOR_ID_NXP, 0x8da8, pci_quirk_nxp_rp_acs },
- { PCI_VENDOR_ID_NXP, 0x8d8a, pci_quirk_nxp_rp_acs },
+ { PCI_VENDOR_ID_FREESCALE, 0x8d88, pci_quirk_nxp_rp_acs },
+ { PCI_VENDOR_ID_FREESCALE, 0x8da8, pci_quirk_nxp_rp_acs },
+ { PCI_VENDOR_ID_FREESCALE, 0x8d8a, pci_quirk_nxp_rp_acs },
/* LX2xx2E : security features + CAN */
- { PCI_VENDOR_ID_NXP, 0x8d98, pci_quirk_nxp_rp_acs },
- { PCI_VENDOR_ID_NXP, 0x8db8, pci_quirk_nxp_rp_acs },
- { PCI_VENDOR_ID_NXP, 0x8d9a, pci_quirk_nxp_rp_acs },
+ { PCI_VENDOR_ID_FREESCALE, 0x8d98, pci_quirk_nxp_rp_acs },
+ { PCI_VENDOR_ID_FREESCALE, 0x8db8, pci_quirk_nxp_rp_acs },
+ { PCI_VENDOR_ID_FREESCALE, 0x8d9a, pci_quirk_nxp_rp_acs },
/* LX2xx2N : without security features + CAN */
- { PCI_VENDOR_ID_NXP, 0x8d99, pci_quirk_nxp_rp_acs },
- { PCI_VENDOR_ID_NXP, 0x8db9, pci_quirk_nxp_rp_acs },
- { PCI_VENDOR_ID_NXP, 0x8d9b, pci_quirk_nxp_rp_acs },
+ { PCI_VENDOR_ID_FREESCALE, 0x8d99, pci_quirk_nxp_rp_acs },
+ { PCI_VENDOR_ID_FREESCALE, 0x8db9, pci_quirk_nxp_rp_acs },
+ { PCI_VENDOR_ID_FREESCALE, 0x8d9b, pci_quirk_nxp_rp_acs },
/* Zhaoxin Root/Downstream Ports */
{ PCI_VENDOR_ID_ZHAOXIN, PCI_ANY_ID, pci_quirk_zhaoxin_pcie_ports_acs },
/* Wangxun nics */
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 275799b5f535..f837ff427b85 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2469,7 +2469,6 @@
#define PCI_DEVICE_ID_TDI_EHCI 0x0101

#define PCI_VENDOR_ID_FREESCALE 0x1957 /* duplicate: NXP */
-#define PCI_VENDOR_ID_NXP 0x1957 /* duplicate: FREESCALE */
#define PCI_DEVICE_ID_MPC8308 0xc006
#define PCI_DEVICE_ID_MPC8315E 0x00b4
#define PCI_DEVICE_ID_MPC8315 0x00b5
--
2.34.1


2023-11-29 23:42:03

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH pci] PCI: remove the PCI_VENDOR_ID_NXP alias

[+cc Greg because these mergers & spinoffs happen all the time, and
pci_ids.h doesn't necessarily need to keep up, so maybe there's
precedent for what to do here]

On Wed, Nov 22, 2023 at 05:42:41PM +0200, Vladimir Oltean wrote:
> What is today NXP is the result of some mergers (with Freescale) and
> spin-offs (from Philips).
>
> New NXP hardware (for example NETC version 4.1 of the NXP i.MX95
> SoC) uses PCI_VENDOR_ID_PHILIPS. And some older hardware uses
> PCI_VENDOR_ID_FREESCALE.
>
> If we have PCI_VENDOR_ID_NXP as an alias for PCI_VENDOR_ID_FREESCALE,
> we end up needing something like a PCI_VENDOR_ID_NXP2 alias for
> PCI_VENDOR_ID_PHILIPS. I think this is more confusing than just spelling
> out the vendor ID of the original company that claimed it.
>
> FWIW, the pci.ids repository as of today has:
> 1131 Philips Semiconductors
> 1957 Freescale Semiconductor Inc
>
> so this makes the kernel code consistent with that, and with what
> "lspci" prints.

Hmm. I can't find the 0x1957 Vendor ID here:
https://pcisig.com/membership/member-companies, which is supposed to
be the authoritative source AFAICS.

Also, that page lists 0x1131 as "NXP Semiconductors".

There's a contact email on that page if it needs updates.

I don't quite understand the goal here. The company is now called
"NXP", and this patch removes PCI_VENDOR_ID_NXP (the only instance of
"NXP" in pci_ids.h) and uses PCI_VENDOR_ID_FREESCALE (which apparently
does not exist any more)?

Why would we remove name of the current company and use the name of a
company that doesn't exist any more?

> Signed-off-by: Vladimir Oltean <[email protected]>
> ---
> drivers/pci/quirks.c | 50 ++++++++++++++++++++---------------------
> include/linux/pci_ids.h | 1 -
> 2 files changed, 25 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index d208047d1b8f..c95701e36d58 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -5092,39 +5092,39 @@ static const struct pci_dev_acs_enabled {
> { PCI_VENDOR_ID_ZHAOXIN, 0x3038, pci_quirk_mf_endpoint_acs },
> { PCI_VENDOR_ID_ZHAOXIN, 0x3104, pci_quirk_mf_endpoint_acs },
> { PCI_VENDOR_ID_ZHAOXIN, 0x9083, pci_quirk_mf_endpoint_acs },
> - /* NXP root ports, xx=16, 12, or 08 cores */
> + /* Freescale/NXP root ports, xx=16, 12, or 08 cores */
> /* LX2xx0A : without security features + CAN-FD */
> - { PCI_VENDOR_ID_NXP, 0x8d81, pci_quirk_nxp_rp_acs },
> - { PCI_VENDOR_ID_NXP, 0x8da1, pci_quirk_nxp_rp_acs },
> - { PCI_VENDOR_ID_NXP, 0x8d83, pci_quirk_nxp_rp_acs },
> + { PCI_VENDOR_ID_FREESCALE, 0x8d81, pci_quirk_nxp_rp_acs },
> + { PCI_VENDOR_ID_FREESCALE, 0x8da1, pci_quirk_nxp_rp_acs },
> + { PCI_VENDOR_ID_FREESCALE, 0x8d83, pci_quirk_nxp_rp_acs },
> /* LX2xx0C : security features + CAN-FD */
> - { PCI_VENDOR_ID_NXP, 0x8d80, pci_quirk_nxp_rp_acs },
> - { PCI_VENDOR_ID_NXP, 0x8da0, pci_quirk_nxp_rp_acs },
> - { PCI_VENDOR_ID_NXP, 0x8d82, pci_quirk_nxp_rp_acs },
> + { PCI_VENDOR_ID_FREESCALE, 0x8d80, pci_quirk_nxp_rp_acs },
> + { PCI_VENDOR_ID_FREESCALE, 0x8da0, pci_quirk_nxp_rp_acs },
> + { PCI_VENDOR_ID_FREESCALE, 0x8d82, pci_quirk_nxp_rp_acs },
> /* LX2xx0E : security features + CAN */
> - { PCI_VENDOR_ID_NXP, 0x8d90, pci_quirk_nxp_rp_acs },
> - { PCI_VENDOR_ID_NXP, 0x8db0, pci_quirk_nxp_rp_acs },
> - { PCI_VENDOR_ID_NXP, 0x8d92, pci_quirk_nxp_rp_acs },
> + { PCI_VENDOR_ID_FREESCALE, 0x8d90, pci_quirk_nxp_rp_acs },
> + { PCI_VENDOR_ID_FREESCALE, 0x8db0, pci_quirk_nxp_rp_acs },
> + { PCI_VENDOR_ID_FREESCALE, 0x8d92, pci_quirk_nxp_rp_acs },
> /* LX2xx0N : without security features + CAN */
> - { PCI_VENDOR_ID_NXP, 0x8d91, pci_quirk_nxp_rp_acs },
> - { PCI_VENDOR_ID_NXP, 0x8db1, pci_quirk_nxp_rp_acs },
> - { PCI_VENDOR_ID_NXP, 0x8d93, pci_quirk_nxp_rp_acs },
> + { PCI_VENDOR_ID_FREESCALE, 0x8d91, pci_quirk_nxp_rp_acs },
> + { PCI_VENDOR_ID_FREESCALE, 0x8db1, pci_quirk_nxp_rp_acs },
> + { PCI_VENDOR_ID_FREESCALE, 0x8d93, pci_quirk_nxp_rp_acs },
> /* LX2xx2A : without security features + CAN-FD */
> - { PCI_VENDOR_ID_NXP, 0x8d89, pci_quirk_nxp_rp_acs },
> - { PCI_VENDOR_ID_NXP, 0x8da9, pci_quirk_nxp_rp_acs },
> - { PCI_VENDOR_ID_NXP, 0x8d8b, pci_quirk_nxp_rp_acs },
> + { PCI_VENDOR_ID_FREESCALE, 0x8d89, pci_quirk_nxp_rp_acs },
> + { PCI_VENDOR_ID_FREESCALE, 0x8da9, pci_quirk_nxp_rp_acs },
> + { PCI_VENDOR_ID_FREESCALE, 0x8d8b, pci_quirk_nxp_rp_acs },
> /* LX2xx2C : security features + CAN-FD */
> - { PCI_VENDOR_ID_NXP, 0x8d88, pci_quirk_nxp_rp_acs },
> - { PCI_VENDOR_ID_NXP, 0x8da8, pci_quirk_nxp_rp_acs },
> - { PCI_VENDOR_ID_NXP, 0x8d8a, pci_quirk_nxp_rp_acs },
> + { PCI_VENDOR_ID_FREESCALE, 0x8d88, pci_quirk_nxp_rp_acs },
> + { PCI_VENDOR_ID_FREESCALE, 0x8da8, pci_quirk_nxp_rp_acs },
> + { PCI_VENDOR_ID_FREESCALE, 0x8d8a, pci_quirk_nxp_rp_acs },
> /* LX2xx2E : security features + CAN */
> - { PCI_VENDOR_ID_NXP, 0x8d98, pci_quirk_nxp_rp_acs },
> - { PCI_VENDOR_ID_NXP, 0x8db8, pci_quirk_nxp_rp_acs },
> - { PCI_VENDOR_ID_NXP, 0x8d9a, pci_quirk_nxp_rp_acs },
> + { PCI_VENDOR_ID_FREESCALE, 0x8d98, pci_quirk_nxp_rp_acs },
> + { PCI_VENDOR_ID_FREESCALE, 0x8db8, pci_quirk_nxp_rp_acs },
> + { PCI_VENDOR_ID_FREESCALE, 0x8d9a, pci_quirk_nxp_rp_acs },
> /* LX2xx2N : without security features + CAN */
> - { PCI_VENDOR_ID_NXP, 0x8d99, pci_quirk_nxp_rp_acs },
> - { PCI_VENDOR_ID_NXP, 0x8db9, pci_quirk_nxp_rp_acs },
> - { PCI_VENDOR_ID_NXP, 0x8d9b, pci_quirk_nxp_rp_acs },
> + { PCI_VENDOR_ID_FREESCALE, 0x8d99, pci_quirk_nxp_rp_acs },
> + { PCI_VENDOR_ID_FREESCALE, 0x8db9, pci_quirk_nxp_rp_acs },
> + { PCI_VENDOR_ID_FREESCALE, 0x8d9b, pci_quirk_nxp_rp_acs },
> /* Zhaoxin Root/Downstream Ports */
> { PCI_VENDOR_ID_ZHAOXIN, PCI_ANY_ID, pci_quirk_zhaoxin_pcie_ports_acs },
> /* Wangxun nics */
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 275799b5f535..f837ff427b85 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -2469,7 +2469,6 @@
> #define PCI_DEVICE_ID_TDI_EHCI 0x0101
>
> #define PCI_VENDOR_ID_FREESCALE 0x1957 /* duplicate: NXP */
> -#define PCI_VENDOR_ID_NXP 0x1957 /* duplicate: FREESCALE */
> #define PCI_DEVICE_ID_MPC8308 0xc006
> #define PCI_DEVICE_ID_MPC8315E 0x00b4
> #define PCI_DEVICE_ID_MPC8315 0x00b5
> --
> 2.34.1
>

2023-11-30 11:10:29

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH pci] PCI: remove the PCI_VENDOR_ID_NXP alias

On Wed, Nov 29, 2023 at 05:38:27PM -0600, Bjorn Helgaas wrote:
> [+cc Greg because these mergers & spinoffs happen all the time, and
> pci_ids.h doesn't necessarily need to keep up, so maybe there's
> precedent for what to do here]

Yes, the precedent is to leave it alone.

> On Wed, Nov 22, 2023 at 05:42:41PM +0200, Vladimir Oltean wrote:
> > What is today NXP is the result of some mergers (with Freescale) and
> > spin-offs (from Philips).
> >
> > New NXP hardware (for example NETC version 4.1 of the NXP i.MX95
> > SoC) uses PCI_VENDOR_ID_PHILIPS. And some older hardware uses
> > PCI_VENDOR_ID_FREESCALE.
> >
> > If we have PCI_VENDOR_ID_NXP as an alias for PCI_VENDOR_ID_FREESCALE,
> > we end up needing something like a PCI_VENDOR_ID_NXP2 alias for
> > PCI_VENDOR_ID_PHILIPS. I think this is more confusing than just spelling
> > out the vendor ID of the original company that claimed it.
> >
> > FWIW, the pci.ids repository as of today has:
> > 1131 Philips Semiconductors
> > 1957 Freescale Semiconductor Inc
> >
> > so this makes the kernel code consistent with that, and with what
> > "lspci" prints.
>
> Hmm. I can't find the 0x1957 Vendor ID here:
> https://pcisig.com/membership/member-companies, which is supposed to
> be the authoritative source AFAICS.
>
> Also, that page lists 0x1131 as "NXP Semiconductors".
>
> There's a contact email on that page if it needs updates.
>
> I don't quite understand the goal here. The company is now called
> "NXP", and this patch removes PCI_VENDOR_ID_NXP (the only instance of
> "NXP" in pci_ids.h) and uses PCI_VENDOR_ID_FREESCALE (which apparently
> does not exist any more)?
>
> Why would we remove name of the current company and use the name of a
> company that doesn't exist any more?

Yes, this seems very odd. What is the reason for any of this other than
marketing? Kernel code doesn't do marketing :)

thanks,

greg k-h

2023-12-03 15:17:27

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH pci] PCI: remove the PCI_VENDOR_ID_NXP alias

On Wed, Nov 29, 2023 at 05:38:27PM -0600, Bjorn Helgaas wrote:
> Hmm. I can't find the 0x1957 Vendor ID here:
> https://pcisig.com/membership/member-companies which is supposed to
> be the authoritative source AFAICS.
>
> Also, that page lists 0x1131 as "NXP Semiconductors".
>
> There's a contact email on that page if it needs updates.

Sorry, I've no clue what's with that list of PCI SIG member companies.
Of course a company which no longer has a legal status cannot be a
member of the PCI SIG.

FWIW, the lspci tool uses this database: https://pci-ids.ucw.cz/

> I don't quite understand the goal here. The company is now called
> "NXP", and this patch removes PCI_VENDOR_ID_NXP (the only instance of
> "NXP" in pci_ids.h) and uses PCI_VENDOR_ID_FREESCALE (which apparently
> does not exist any more)?
>
> Why would we remove name of the current company and use the name of a
> company that doesn't exist any more?

Because that company doesn't produce new equipment with the old vendor id.

Aliasing the old vendor id to the current company name is polluting the
Linux definitions.

The goal, quite simply, is to call the Freescale PCI ID "Freescale".

2023-12-03 15:17:41

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH pci] PCI: remove the PCI_VENDOR_ID_NXP alias

On Thu, Nov 30, 2023 at 11:10:19AM +0000, Greg Kroah-Hartman wrote:
> > Why would we remove name of the current company and use the name of a
> > company that doesn't exist any more?
>
> Yes, this seems very odd. What is the reason for any of this other than
> marketing? Kernel code doesn't do marketing :)

I'm not sure who is doing the marketing; not me, that's for sure.
The patch that I'm proposing undoes these strange aliases.

2023-12-03 17:31:07

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH pci] PCI: remove the PCI_VENDOR_ID_NXP alias

On Sun, Dec 03, 2023 at 05:16:54PM +0200, Vladimir Oltean wrote:
> On Thu, Nov 30, 2023 at 11:10:19AM +0000, Greg Kroah-Hartman wrote:
> > > Why would we remove name of the current company and use the name of a
> > > company that doesn't exist any more?
> >
> > Yes, this seems very odd. What is the reason for any of this other than
> > marketing? Kernel code doesn't do marketing :)
>
> I'm not sure who is doing the marketing; not me, that's for sure.
> The patch that I'm proposing undoes these strange aliases.

Why? Who did it originally in what commit id and what was wrong with
them then?

thanks,

greg k-h

2023-12-03 17:49:08

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH pci] PCI: remove the PCI_VENDOR_ID_NXP alias

On Sun, Dec 03, 2023 at 06:30:13PM +0100, Greg Kroah-Hartman wrote:
> On Sun, Dec 03, 2023 at 05:16:54PM +0200, Vladimir Oltean wrote:
> > On Thu, Nov 30, 2023 at 11:10:19AM +0000, Greg Kroah-Hartman wrote:
> > > > Why would we remove name of the current company and use the name of a
> > > > company that doesn't exist any more?
> > >
> > > Yes, this seems very odd. What is the reason for any of this other than
> > > marketing? Kernel code doesn't do marketing :)
> >
> > I'm not sure who is doing the marketing; not me, that's for sure.
> > The patch that I'm proposing undoes these strange aliases.
>
> Why?

Why am I undoing the aliases? It's in my commit message. NXP now
produces PCI devices with a different vendor ID. If aliasing is the way
to go, then are we supposed to add a new PCI_VENDOR_ID_NXP2,
PCI_VENDOR_ID_NXP3 etc?

Mellanox was bought by Nvidia and I don't see its PCI ID aliased to
Nvidia. There are probably countless of other examples.

> Who did it originally in what commit id and what was wrong with them
> then?

Does it really matter? "Git blame" on the line with #define PCI_VENDOR_ID_NXP
will point to a random commit by Wasim Khan (also CCed). The usage of
PCI_VENDOR_ID_NXP is not widespread, it's only that commit. Everywhere
else in the kernel, 0x1957 is referred to as PCI_VENDOR_ID_FREESCALE.
I can't comment on what was wrong with Wasim.

2023-12-03 17:59:19

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH pci] PCI: remove the PCI_VENDOR_ID_NXP alias

On Sun, Dec 03, 2023 at 07:48:41PM +0200, Vladimir Oltean wrote:
> On Sun, Dec 03, 2023 at 06:30:13PM +0100, Greg Kroah-Hartman wrote:
> > On Sun, Dec 03, 2023 at 05:16:54PM +0200, Vladimir Oltean wrote:
> > > On Thu, Nov 30, 2023 at 11:10:19AM +0000, Greg Kroah-Hartman wrote:
> > > > > Why would we remove name of the current company and use the name of a
> > > > > company that doesn't exist any more?
> > > >
> > > > Yes, this seems very odd. What is the reason for any of this other than
> > > > marketing? Kernel code doesn't do marketing :)
> > >
> > > I'm not sure who is doing the marketing; not me, that's for sure.
> > > The patch that I'm proposing undoes these strange aliases.
> >
> > Why?
>
> Why am I undoing the aliases? It's in my commit message.

Which is long gone from this email thread, sorry.

> NXP now
> produces PCI devices with a different vendor ID.

"Different" from what, the old one?

> If aliasing is the way
> to go, then are we supposed to add a new PCI_VENDOR_ID_NXP2,
> PCI_VENDOR_ID_NXP3 etc?
>
> Mellanox was bought by Nvidia and I don't see its PCI ID aliased to
> Nvidia. There are probably countless of other examples.

I'm not asking why anything is being aliased, I'm asking why change the
existing names.

> > Who did it originally in what commit id and what was wrong with them
> > then?
>
> Does it really matter? "Git blame" on the line with #define PCI_VENDOR_ID_NXP
> will point to a random commit by Wasim Khan (also CCed). The usage of
> PCI_VENDOR_ID_NXP is not widespread, it's only that commit.

So does your change here just revert the change in that commit, or does
it do it in other places?

thanks,

greg k-h

2023-12-03 18:15:55

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH pci] PCI: remove the PCI_VENDOR_ID_NXP alias

On Sun, Dec 03, 2023 at 06:59:04PM +0100, Greg Kroah-Hartman wrote:
> > > > > > Why would we remove name of the current company and use the name of a
> > > > > > company that doesn't exist any more?
> > > > >
> > > > > Yes, this seems very odd. What is the reason for any of this other than
> > > > > marketing? Kernel code doesn't do marketing :)
> > > >
> > > > I'm not sure who is doing the marketing; not me, that's for sure.
> > > > The patch that I'm proposing undoes these strange aliases.
> > >
> > > Why?
> >
> > Why am I undoing the aliases? It's in my commit message.
>
> Which is long gone from this email thread, sorry.
>
> > NXP now produces PCI devices with a different vendor ID.
>
> "Different" from what, the old one?
>
> > If aliasing is the way
> > to go, then are we supposed to add a new PCI_VENDOR_ID_NXP2,
> > PCI_VENDOR_ID_NXP3 etc?
> >
> > Mellanox was bought by Nvidia and I don't see its PCI ID aliased to
> > Nvidia. There are probably countless of other examples.
>
> I'm not asking why anything is being aliased, I'm asking why change the
> existing names.

With all due respect, I can continue responding and this discussion
will go nowhere, or you can go and actually read the change.
https://lore.kernel.org/linux-pci/[email protected]/

There is nothing more to it than "I think it's confusing and inconsistent
to have multiple PCI vendor IDs for NXP, especially when they even weren't
NXP's to begin with". I've said that once in the original commit and
once to you directly. I don't think we even disagree about this basic fact.

> > > Who did it originally in what commit id and what was wrong with them
> > > then?
> >
> > Does it really matter? "Git blame" on the line with #define PCI_VENDOR_ID_NXP
> > will point to a random commit by Wasim Khan (also CCed). The usage of
> > PCI_VENDOR_ID_NXP is not widespread, it's only that commit.
>
> So does your change here just revert the change in that commit, or does
> it do it in other places?

A tree-wide grep for PCI_VENDOR_ID_NXP will show that the naming
introduced in 2021 commit d08c8b855140 ("PCI: Add ACS quirks for NXP
LX2xx0 and LX2xx2 platforms") is also the only place where 0x1957 is
referred to as NXP rather than Freescale. My change eliminates that
localized NXP alias, and makes the kernel consistently refer to 0x1957
as Freescale everywhere, as a result.