The PCI device registers contain a subsystem ID (SSID), that is
separate from the silicon ID. The PCI specification defines it thus:
"They provide a mechanism for board vendors to distiguish their
boards from one another even thought the boards may have the same
PCI controller on them."
This allows the driver for the silicon part to apply board-speficic
settings based on this SSID.
The CS35L56 driver uses this to select the correct firmware file for
the board. The actual ID is part of the PCI register set of the
host audio interface so this set of patches includes extracting the
SSID from the Intel audio controller and passing it to the machine
driver and then to ASoC components. Other PCI audio controllers
will have the same SSID registers, so can use the same mechanism to
pass the SSID.
Richard Fitzgerald (4):
ASoC: soc-card: Add storage for PCI SSID
ASoC: SOF: Pass PCI SSID to machine driver
ASoC: Intel: sof_sdw: Copy PCI SSID to struct snd_soc_card
ASoC: cs35l56: Use PCI SSID as the firmware UID
include/sound/soc-acpi.h | 7 ++++++
include/sound/soc-card.h | 37 ++++++++++++++++++++++++++++++++
include/sound/soc.h | 11 ++++++++++
include/sound/sof.h | 8 +++++++
sound/soc/codecs/cs35l56.c | 11 ++++++++++
sound/soc/intel/boards/sof_sdw.c | 6 ++++++
sound/soc/sof/sof-audio.c | 7 ++++++
sound/soc/sof/sof-pci-dev.c | 8 +++++++
8 files changed, 95 insertions(+)
--
2.30.2
If the PCI SSID has been set in the struct snd_soc_acpi_mach_params,
copy this to struct snd_soc_card so that it can be used by other
ASoC components.
This is important for components that must apply system-specific
configuration.
Signed-off-by: Richard Fitzgerald <[email protected]>
---
sound/soc/intel/boards/sof_sdw.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/sound/soc/intel/boards/sof_sdw.c b/sound/soc/intel/boards/sof_sdw.c
index 5a1c750e6ae6..961241100012 100644
--- a/sound/soc/intel/boards/sof_sdw.c
+++ b/sound/soc/intel/boards/sof_sdw.c
@@ -1924,6 +1924,12 @@ static int mc_probe(struct platform_device *pdev)
for (i = 0; i < ARRAY_SIZE(codec_info_list); i++)
codec_info_list[i].amp_num = 0;
+ if (mach->mach_params.subsystem_id_set) {
+ snd_soc_card_set_pci_ssid(card,
+ mach->mach_params.subsystem_vendor,
+ mach->mach_params.subsystem_device);
+ }
+
ret = sof_card_dai_links_create(card);
if (ret < 0)
return ret;
--
2.30.2
On 9/12/23 12:32, Richard Fitzgerald wrote:
> The PCI device registers contain a subsystem ID (SSID), that is
> separate from the silicon ID. The PCI specification defines it thus:
>
> "They provide a mechanism for board vendors to distiguish their
> boards from one another even thought the boards may have the same
> PCI controller on them."
>
> This allows the driver for the silicon part to apply board-speficic
> settings based on this SSID.
>
> The CS35L56 driver uses this to select the correct firmware file for
> the board. The actual ID is part of the PCI register set of the
> host audio interface so this set of patches includes extracting the
> SSID from the Intel audio controller and passing it to the machine
> driver and then to ASoC components. Other PCI audio controllers
> will have the same SSID registers, so can use the same mechanism to
> pass the SSID.
>
> Richard Fitzgerald (4):
> ASoC: soc-card: Add storage for PCI SSID
> ASoC: SOF: Pass PCI SSID to machine driver
> ASoC: Intel: sof_sdw: Copy PCI SSID to struct snd_soc_card
> ASoC: cs35l56: Use PCI SSID as the firmware UID
for the series
Reviewed-by: Pierre-Louis Bossart <[email protected]>
>
> include/sound/soc-acpi.h | 7 ++++++
> include/sound/soc-card.h | 37 ++++++++++++++++++++++++++++++++
> include/sound/soc.h | 11 ++++++++++
> include/sound/sof.h | 8 +++++++
> sound/soc/codecs/cs35l56.c | 11 ++++++++++
> sound/soc/intel/boards/sof_sdw.c | 6 ++++++
> sound/soc/sof/sof-audio.c | 7 ++++++
> sound/soc/sof/sof-pci-dev.c | 8 +++++++
> 8 files changed, 95 insertions(+)
>
Pass the PCI SSID of the audio interface through to the machine driver.
This allows the machine driver to use the SSID to uniquely identify the
specific hardware configuration and apply any platform-specific
configuration.
struct snd_sof_pdata is passed around inside the SOF code, but it then
passes configuration information to the machine driver through
struct snd_soc_acpi_mach and struct snd_soc_acpi_mach_params. So SSID
information has been added to both snd_sof_pdata and
snd_soc_acpi_mach_params.
PCI does not define 0x0000 as an invalid value so we can't use zero to
indicate that the struct member was not written. Instead a flag is
included to indicate that a value has been written to the
subsystem_vendor and subsystem_device members.
sof_pci_probe() creates the struct snd_sof_pdata. It is passed a struct
pci_dev so it can fill in the SSID value.
sof_machine_check() finds the appropriate struct snd_soc_acpi_mach. It
copies the SSID information across to the struct snd_soc_acpi_mach_params.
This done before calling any custom set_mach_params() so that it could be
used by the set_mach_params() callback to apply variant params.
The machine driver receives the struct snd_soc_acpi_mach as its
platform_data.
Signed-off-by: Richard Fitzgerald <[email protected]>
---
include/sound/soc-acpi.h | 7 +++++++
include/sound/sof.h | 8 ++++++++
sound/soc/sof/sof-audio.c | 7 +++++++
sound/soc/sof/sof-pci-dev.c | 8 ++++++++
4 files changed, 30 insertions(+)
diff --git a/include/sound/soc-acpi.h b/include/sound/soc-acpi.h
index 6d31d535e8f6..23d6d6bfb073 100644
--- a/include/sound/soc-acpi.h
+++ b/include/sound/soc-acpi.h
@@ -68,6 +68,10 @@ static inline struct snd_soc_acpi_mach *snd_soc_acpi_codec_list(void *arg)
* @i2s_link_mask: I2S/TDM links enabled on the board
* @num_dai_drivers: number of elements in @dai_drivers
* @dai_drivers: pointer to dai_drivers, used e.g. in nocodec mode
+ * @subsystem_vendor: optional PCI SSID vendor value
+ * @subsystem_device: optional PCI SSID device value
+ * @subsystem_id_set: true if a value has been written to
+ * subsystem_vendor and subsystem_device.
*/
struct snd_soc_acpi_mach_params {
u32 acpi_ipc_irq_index;
@@ -80,6 +84,9 @@ struct snd_soc_acpi_mach_params {
u32 i2s_link_mask;
u32 num_dai_drivers;
struct snd_soc_dai_driver *dai_drivers;
+ unsigned short subsystem_vendor;
+ unsigned short subsystem_device;
+ bool subsystem_id_set;
};
/**
diff --git a/include/sound/sof.h b/include/sound/sof.h
index d3c41f87ac31..51294f2ba302 100644
--- a/include/sound/sof.h
+++ b/include/sound/sof.h
@@ -64,6 +64,14 @@ struct snd_sof_pdata {
const char *name;
const char *platform;
+ /*
+ * PCI SSID. As PCI does not define 0 as invalid, the subsystem_id_set
+ * flag indicates that a value has been written to these members.
+ */
+ unsigned short subsystem_vendor;
+ unsigned short subsystem_device;
+ bool subsystem_id_set;
+
struct device *dev;
/*
diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c
index e7ef77012c35..9c2359d10ecf 100644
--- a/sound/soc/sof/sof-audio.c
+++ b/sound/soc/sof/sof-audio.c
@@ -1031,6 +1031,13 @@ int sof_machine_check(struct snd_sof_dev *sdev)
mach = snd_sof_machine_select(sdev);
if (mach) {
sof_pdata->machine = mach;
+
+ if (sof_pdata->subsystem_id_set) {
+ mach->mach_params.subsystem_vendor = sof_pdata->subsystem_vendor;
+ mach->mach_params.subsystem_device = sof_pdata->subsystem_device;
+ mach->mach_params.subsystem_id_set = true;
+ }
+
snd_sof_set_mach_params(mach, sdev);
return 0;
}
diff --git a/sound/soc/sof/sof-pci-dev.c b/sound/soc/sof/sof-pci-dev.c
index f5ece43d0ec2..146d25983b08 100644
--- a/sound/soc/sof/sof-pci-dev.c
+++ b/sound/soc/sof/sof-pci-dev.c
@@ -214,6 +214,14 @@ int sof_pci_probe(struct pci_dev *pci, const struct pci_device_id *pci_id)
return ret;
sof_pdata->name = pci_name(pci);
+
+ /* PCI defines a vendor ID of 0xFFFF as invalid. */
+ if (pci->subsystem_vendor != 0xFFFF) {
+ sof_pdata->subsystem_vendor = pci->subsystem_vendor;
+ sof_pdata->subsystem_device = pci->subsystem_device;
+ sof_pdata->subsystem_id_set = true;
+ }
+
sof_pdata->desc = desc;
sof_pdata->dev = dev;
--
2.30.2
On Tue, 12 Sep 2023 17:32:03 +0100, Richard Fitzgerald wrote:
> The PCI device registers contain a subsystem ID (SSID), that is
> separate from the silicon ID. The PCI specification defines it thus:
>
> "They provide a mechanism for board vendors to distiguish their
> boards from one another even thought the boards may have the same
> PCI controller on them."
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/4] ASoC: soc-card: Add storage for PCI SSID
commit: 47f56e38a199bd45514b8e0142399cba4feeaf1a
[2/4] ASoC: SOF: Pass PCI SSID to machine driver
commit: ba2de401d32625fe538d3f2c00ca73740dd2d516
[3/4] ASoC: Intel: sof_sdw: Copy PCI SSID to struct snd_soc_card
commit: d8b387544ff4d02eda1d1839a0c601de4b037c33
[4/4] ASoC: cs35l56: Use PCI SSID as the firmware UID
commit: 1a1c3d794ef65ef2978c5e65e1aed3fe6f014e90
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
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 <[email protected]>
---
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
@@ -59,6 +59,43 @@ int snd_soc_card_add_dai_link(struct snd_soc_card *card,
void snd_soc_card_remove_dai_link(struct snd_soc_card *card,
struct snd_soc_dai_link *dai_link);
+#ifdef CONFIG_PCI
+static inline void snd_soc_card_set_pci_ssid(struct snd_soc_card *card,
+ unsigned short vendor,
+ unsigned short device)
+{
+ card->pci_subsystem_vendor = vendor;
+ card->pci_subsystem_device = device;
+ card->pci_subsystem_set = true;
+}
+
+static inline int snd_soc_card_get_pci_ssid(struct snd_soc_card *card,
+ unsigned short *vendor,
+ unsigned short *device)
+{
+ if (!card->pci_subsystem_set)
+ return -ENOENT;
+
+ *vendor = card->pci_subsystem_vendor;
+ *device = card->pci_subsystem_device;
+
+ return 0;
+}
+#else /* !CONFIG_PCI */
+static inline void snd_soc_card_set_pci_ssid(struct snd_soc_card *card,
+ unsigned short vendor,
+ unsigned short device)
+{
+}
+
+static inline int snd_soc_card_get_pci_ssid(struct snd_soc_card *card,
+ unsigned short *vendor,
+ unsigned short *device)
+{
+ return -ENOENT;
+}
+#endif /* CONFIG_PCI */
+
/* device driver data */
static inline void snd_soc_card_set_drvdata(struct snd_soc_card *card,
void *data)
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;
--
2.30.2