> +/**
> + * acp_pdev_mask corresponds to platform device mask based on audio endpoint combinations.
> + * acp_pdev_mask will be calculated based on ACPI Scan under ACP PCI device and
> + * ACP PIN Configuration.
> + * Based acp_pdev_mask, platform devices will be created.
> + * Below are possible platform device combinations.
> + * 1) ACP PDM Controller, dmic-codec, machine driver platform device node
> + * 2) ACP PDM Controller , dmic-codec, SW0 SoundWire manager instance, platform device for
> + * SoundWire DMA driver
> + * 3) SW0, SW1 SoundWire manager instances, platform device for SoundWire DMA driver
> + * 4) ACP PDM Controller, dmic-codec, SDW0, SDW1 manager instances, platform device for
> + * SoundWire DMA driver
> + * ACP63_PDM_DEV_MASK corresponds to platform device mask for ACP PDM controller.
> + * ACP63_SDW_DEV_MASK corresponds to platform device mask for SDW manager instances.
> + * ACP63_SDW_PDM_DEV_MASK corresponds to platform device mask for ACP PDM + SDW manager combination
> + */
> +enum acp_pdev_mask {
> + ACP63_PDM_DEV_MASK = 1,
> + ACP63_SDW_DEV_MASK,
> + ACP63_SDW_PDM_DEV_MASK,
> +};
This does not look like a mask, the definitions prevent bit-wise
operations from happening.
Either use BIT(0), BIT(1), BIT(2) or GENMASK(1, 0), or demote this to a
regular enum (e.g. pdev_config or something)
> +
> struct pdm_stream_instance {
> u16 num_pages;
> u16 channels;
> @@ -95,14 +144,38 @@ struct pdm_dev_data {
> struct snd_pcm_substream *capture_stream;
> };
>
> +/**
> + * struct acp63_dev_data - acp pci driver context
> + * @acp63_base: acp mmio base
> + * @res: resource
> + * @pdev: array of child platform device node structures
> + * @acp_lock: used to protect acp common registers
> + * @sdw_fw_node: SoundWire controller fw node handle
> + * @pdev_mask: platform device mask
> + * @pdev_count: platform devices count
> + * @pdm_dev_index: pdm platform device index
> + * @sdw_manager_count: SoundWire manager instance count
> + * @sdw0_dev_index: SoundWire Manager-0 platform device index
> + * @sdw1_dev_index: SoundWire Manager-1 platform device index
> + * @sdw_dma_dev_index: SoundWire DMA controller platform device index
> + * @acp_reset: flag set to true when bus reset is applied across all
> + * the active SoundWire manager instances
> + */
> +
> struct acp63_dev_data {
> void __iomem *acp63_base;
> struct resource *res;
> struct platform_device *pdev[ACP63_DEVS];
> struct mutex acp_lock; /* protect shared registers */
> + struct fwnode_handle *sdw_fw_node;
> u16 pdev_mask;
> u16 pdev_count;
> u16 pdm_dev_index;
> + u8 sdw_manager_count;
> + u16 sdw0_dev_index;
> + u16 sdw1_dev_index;
> + u16 sdw_dma_dev_index;
> + bool acp_reset;
> };
>
> int snd_amd_acp_find_config(struct pci_dev *pci);
> diff --git a/sound/soc/amd/ps/pci-ps.c b/sound/soc/amd/ps/pci-ps.c
> index 54752d6040d6..816c22e7f1ab 100644
> --- a/sound/soc/amd/ps/pci-ps.c
> +++ b/sound/soc/amd/ps/pci-ps.c
> @@ -6,6 +6,7 @@
> */
>
> #include <linux/pci.h>
> +#include <linux/bitops.h>
> #include <linux/module.h>
> #include <linux/io.h>
> #include <linux/delay.h>
> @@ -15,6 +16,7 @@
> #include <sound/pcm_params.h>
> #include <linux/pm_runtime.h>
> #include <linux/iopoll.h>
> +#include <linux/soundwire/sdw_amd.h>
>
> #include "acp63.h"
>
> @@ -119,37 +121,162 @@ static irqreturn_t acp63_irq_handler(int irq, void *dev_id)
> return IRQ_NONE;
> }
>
> -static void get_acp63_device_config(u32 config, struct pci_dev *pci,
> - struct acp63_dev_data *acp_data)
> +static int sdw_amd_scan_controller(struct device *dev)
> +{
> + struct acp63_dev_data *acp_data;
> + struct fwnode_handle *link;
> + char name[32];
> + u32 sdw_manager_bitmap;
> + u8 count = 0;
> + u32 acp_sdw_power_mode = 0;
> + int index;
> + int ret;
> +
> + acp_data = dev_get_drvdata(dev);
> + acp_data->acp_reset = true;
> + /* Found controller, find links supported */
> + ret = fwnode_property_read_u32_array((acp_data->sdw_fw_node), "mipi-sdw-manager-list",
> + &sdw_manager_bitmap, 1);
IIRC this is only defined in the DisCo 2.0 spec, previous editions had a
'mipi-master-count'. A comment would not hurt to point to the minimal
DisCo spec version.
> +
> + if (ret) {
> + dev_err(dev, "Failed to read mipi-sdw-manager-list: %d\n", ret);
> + return -EINVAL;
> + }
> + count = hweight32(sdw_manager_bitmap);
> + /* Check count is within bounds */
> + if (count > AMD_SDW_MAX_MANAGERS) {
> + dev_err(dev, "Manager count %d exceeds max %d\n", count, AMD_SDW_MAX_MANAGERS);
> + return -EINVAL;
> + }
> +
> + if (!count) {
> + dev_dbg(dev, "No SoundWire Managers detected\n");
> + return -EINVAL;
> + }
> + dev_dbg(dev, "ACPI reports %d SoundWire Manager devices\n", count);
> + acp_data->sdw_manager_count = count;
> + for (index = 0; index < count; index++) {
> + snprintf(name, sizeof(name), "mipi-sdw-link-%d-subproperties", index);
> + link = fwnode_get_named_child_node(acp_data->sdw_fw_node, name);
> + if (!link) {
> + dev_err(dev, "Manager node %s not found\n", name);
> + return -EIO;
> + }
> +
> + ret = fwnode_property_read_u32(link, "amd-sdw-power-mode", &acp_sdw_power_mode);
> + if (ret)
> + return ret;
> + /*
> + * when SoundWire configuration is selected from acp pin config,
> + * based on manager instances count, acp init/de-init sequence should be
> + * executed as part of PM ops only when Bus reset is applied for the active
> + * SoundWire manager instances.
> + */
> + if (acp_sdw_power_mode != AMD_SDW_POWER_OFF_MODE) {
> + acp_data->acp_reset = false;
> + return 0;
> + }
> + }
> + return 0;
> +}
> +
> +static int get_acp63_device_config(u32 config, struct pci_dev *pci, struct acp63_dev_data *acp_data)
> {
> struct acpi_device *dmic_dev;
> + struct acpi_device *sdw_dev;
> const union acpi_object *obj;
> bool is_dmic_dev = false;
> + bool is_sdw_dev = false;
> + int ret;
>
> dmic_dev = acpi_find_child_device(ACPI_COMPANION(&pci->dev), ACP63_DMIC_ADDR, 0);
> if (dmic_dev) {
> + /* is_dmic_dev flag will be set when ACP PDM controller device exists */
> if (!acpi_dev_get_property(dmic_dev, "acp-audio-device-type",
usually properties start with the 'mipi-' or 'vendor-' prefix. Is there
a missing 'amd-' here or is 'acp-' unique enough?
