Re: [PATCH 1/4] ASoC: soc-card: Add storage for PCI SSID

[Amadeusz Sławiński]

On 9/12/2023 6:32 PM, Richard Fitzgerald wrote:
> Add members to struct snd_soc_card to store the PCI subsystem ID (SSID)
> of the soundcard.
>
> The PCI specification provides two registers to store a vendor-specific
> SSID that can be read by drivers to uniquely identify a particular
> "soundcard". This is defined in the PCI specification to distinguish
> products that use the same silicon (and therefore have the same silicon
> ID) so that product-specific differences can be applied.
>
> PCI only defines 0xFFFF as an invalid value. 0x0000 is not defined as
> invalid. So the usual pattern of zero-filling the struct and then
> assuming a zero value unset will not work. A flag is included to
> indicate when the SSID information has been filled in.
>
> Unlike DMI information, which has a free-format entirely up to the vendor,
> the PCI SSID has a strictly defined format and a registry of vendor IDs.
>
> It is usual in Windows drivers that the SSID is used as the sole identifier
> of the specific end-product and the Windows driver contains tables mapping
> that to information about the hardware setup, rather than using ACPI
> properties.
>
> This SSID is important information for ASoC components that need to apply
> hardware-specific configuration on PCI-based systems.
>
> As the SSID is a generic part of the PCI specification and is treated as
> identifying the "soundcard", it is reasonable to include this information
> in struct snd_soc_card, instead of components inventing their own custom
> ways to pass this information around.
>
> Signed-off-by: Richard Fitzgerald <rf@xxxxxxxxxxxxxxxxxxxxx>
> ---
>   include/sound/soc-card.h | 37 +++++++++++++++++++++++++++++++++++++
>   include/sound/soc.h      | 11 +++++++++++
>   2 files changed, 48 insertions(+)
>
> diff --git a/include/sound/soc-card.h b/include/sound/soc-card.h
> index fc94dfb0021f..e8ff2e089cd0 100644
> --- a/include/sound/soc-card.h
> +++ b/include/sound/soc-card.h

...

> diff --git a/include/sound/soc.h b/include/sound/soc.h
> index 509386ff5212..81ed08c5c67d 100644
> --- a/include/sound/soc.h
> +++ b/include/sound/soc.h
> @@ -929,6 +929,17 @@ struct snd_soc_card {
>   #ifdef CONFIG_DMI
>   	char dmi_longname[80];
>   #endif /* CONFIG_DMI */
> +
> +#ifdef CONFIG_PCI
> +	/*
> +	 * PCI does not define 0 as invalid, so pci_subsystem_set indicates
> +	 * whether a value has been written to these fields.
> +	 */
> +	unsigned short pci_subsystem_vendor;
> +	unsigned short pci_subsystem_device;
> +	bool pci_subsystem_set;
> +#endif /* CONFIG_PCI */
> +
>   	char topology_shortname[32];
>   
>   	struct device *dev;

This looks bit weird to me, snd_soc_card is _generic_ struct which is 
not device specific in any way, and now you add fields for PCI, can't 
this somehow be done using drvdata or something?