On Wed, Aug 02, 2023 at 11:07:36AM -0500, Pierre-Louis Bossart wrote:
> On 8/2/23 10:57, Takashi Iwai wrote:
> > On Wed, 02 Aug 2023 17:52:26 +0200,
> > Bjorn Helgaas wrote:
> >> On Wed, Aug 02, 2023 at 10:01:01AM -0500, Pierre-Louis Bossart wrote:
> >>> Add part ID to common include file
> >>
> >> Please drop period at end of subject and add one at the end of the
> >> commit log.
> >>
> >> Also mention the drivers that will use this new #define; looks like
> >> hda_intel.c and ...
> >>
> >> Well, actually, I only see that one use, which means we probably
> >> shouldn't add this #define to pci_ids.h, per the comment at the top of
> >> the file. If there's only one use, use the hex ID in the driver (or
> >> add a #define in the driver itself).
> >
> > Judging from the previous patterns, the same ID could be required for
> > ASoC SOF driver, too, which isn't included in this patch set. In
> > that case, it's worth to put to pci_ids.h.
> > (OTOH, it can be done at a later stage, too.)
When it becomes shared is the standard point at which we add to
pci_ids.h.
> I am not following. we just agreed a couple of weeks ago to record ALL
> Intel/HDaudio PCI IDs in the same pci_ids.h include file.
I'm not sure who "we" is here. If it included me and I signed up to
it, I apologize for forgetting, and go ahead and add my:
Acked-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
I'm just pointing out the usual practice for pci_ids.h, as mentioned
in the file itself.
> ArrowLake-S is the first addition to first file after the work done by
> Cezary/Amadeusz. Yes it's required to be added since it'll be used in
> other parts later on. But even if there was ONE use of this PCI ID, why
> would we not add it for consistency to the global pci_ids.h file?
> Takashi's hda_intel.c file would look really bad if we have a mix of
> single-use PCIs and shared ones...
We already have a mix:
static const struct pci_device_id azx_ids[] = {
/* CPT */
{ PCI_DEVICE(0x8086, 0x1c20),
.driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_PCH_NOPM },
/* PBG */
{ PCI_DEVICE(0x8086, 0x1d20),
.driver_data = AZX_DRIVER_PCH | AZX_DCAPS_INTEL_PCH_NOPM },
/* Panther Point */
I think the reason we don't add names used only once is because it
makes backporting things harder because it leads to more merge
conflicts in pci_ids.h.
Bjorn
