On 5/17/23 03:38, Mukunda,Vijendar wrote:
On 16/05/23 20:02, Pierre-Louis Bossart wrote:
On 5/16/23 05:35, Vijendar Mukunda wrote:
Create platform devices for Soundwire Manager instances and PDM controller based on ACP pin config selection and ACPI fw handle for pink sardine platform.
Signed-off-by: Vijendar Mukunda Vijendar.Mukunda@amd.com
sound/soc/amd/ps/acp63.h | 43 ++++++- sound/soc/amd/ps/pci-ps.c | 250 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 280 insertions(+), 13 deletions(-)
diff --git a/sound/soc/amd/ps/acp63.h b/sound/soc/amd/ps/acp63.h index 2f94448102d0..f27f71116598 100644 --- a/sound/soc/amd/ps/acp63.h +++ b/sound/soc/amd/ps/acp63.h @@ -10,7 +10,7 @@ #define ACP_DEVICE_ID 0x15E2 #define ACP63_REG_START 0x1240000 #define ACP63_REG_END 0x1250200 -#define ACP63_DEVS 3 +#define ACP63_DEVS 5
#define ACP_SOFT_RESET_SOFTRESET_AUDDONE_MASK 0x00010001 #define ACP_PGFSM_CNTL_POWER_ON_MASK 1 @@ -55,8 +55,14 @@
#define ACP63_DMIC_ADDR 2 #define ACP63_PDM_MODE_DEVS 3 -#define ACP63_PDM_DEV_MASK 1 #define ACP_DMIC_DEV 2 +#define ACP63_SDW0_MODE_DEVS 2 +#define ACP63_SDW0_SDW1_MODE_DEVS 3 +#define ACP63_SDW0_PDM_MODE_DEVS 4 +#define ACP63_SDW0_SDW1_PDM_MODE_DEVS 5 +#define ACP63_DMIC_ADDR 2 +#define ACP63_SDW_ADDR 5 +#define AMD_SDW_MAX_MANAGERS 2
/* time in ms for acp timeout */ #define ACP_TIMEOUT 500 @@ -80,6 +86,12 @@ enum acp_config { ACP_CONFIG_15, };
+enum acp_pdev_mask {
- ACP63_PDM_DEV_MASK = 1,
- ACP63_SDW_DEV_MASK,
- ACP63_SDW_PDM_DEV_MASK,
+};
a comment or kernel-doc wouldn't hurt to explain the difference between ACP63_PDM_DEV_MASK and ACP63_SDW_PDM_DEV_MASK, the meaning of the 'SDW" prefix is far from obvious.
Above enum's are listed to know the platform device masks. For example - if ACP63_PDM_DEV_MASK is set, then ACP PCI driver will create platform device for PDM controller.
If ACP63_SDW_DEV_MASK is set, ACP PCI driver will create platform device nodes for soundwire manager instances based on instance count retrieved by scanning the SoundWire Controller.
If ACP63_SDW_PDM_DEV_MASK is set, ACP PCI driver will create platform device nodes for PDM controller and SoundWire manager instances.
We will add comment for the same.
Ah ok, I completely missed that you could have PDM, SoundWire or PDM+SoundWire configurations. I was reading this with SoundWire blinders and thought you wanted to have PDM over SoundWire or something.
dev_dbg(&pci->dev, "No PDM devices found\n");
dev_dbg(&pci->dev, "No PDM or Soundwire manager devices found\n");what does this mean? I find this debug adds more confusion.
Currently, we are trying to create platform devices for PDM controller and SoundWire Manager instances based on ACP pin config and ACPI _ADDR fields scan under ACP PCI device scope. Earlier We have added support for ACP PDM controller. ACP PIN config supports different audio configurations other than PDM and SoundWire based audio endpoints.
If there is no pdev_mask set, it refers to default switch case. This dev_dbg statement to notify that no PDM and Soundwire manager devices found from ACPI scan.
This patch adds support for platform device creation logic for Soundwire manager instances & PDM controller combinations based on ACP PIN Config and ACPI _ADDR field scan.
Possible combination of platform device nodes:
- ACP PDM Controller, dmic-codec, machine driver platform device node
- ACP PDM Controller , dmic-codec, SW0 manager instance, platform device for SoundWire DMA driver
- SW0, SW1 SoundWire manager instances, platform device for SoundWire DMA driver
- ACP PDM Controller, dmic-codec, SDW0, SDW1 manager instances, platform device for SoundWire DMA driver
right, you really want this in the commit message so that reviewers understand the various configurations upfront. Trying to reverse-engineer the code induces migraines ;-)