diff mbox series

[v3,6/6] ARM: configs: at91: sama7_defconfig: add MCHP PDMC and DMIC drivers

Message ID 20220307122202.2251639-7-codrin.ciubotariu@microchip.com (mailing list archive)
State New, archived
Headers show
Series Add driver for SAMA7G5's PDMC | expand

Commit Message

Codrin Ciubotariu March 7, 2022, 12:22 p.m. UTC
Enable drivers needed for Microchip's PDMC and PDM microphones.

Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
---

Changes in v2,v3:
 - none;

 arch/arm/configs/sama7_defconfig | 2 ++
 1 file changed, 2 insertions(+)

Comments

Nicolas Ferre May 5, 2022, 1:58 p.m. UTC | #1
On 07/03/2022 at 13:22, Codrin Ciubotariu wrote:
> Enable drivers needed for Microchip's PDMC and PDM microphones.
> 
> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
> ---
> 
> Changes in v2,v3:
>   - none;
> 
>   arch/arm/configs/sama7_defconfig | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm/configs/sama7_defconfig b/arch/arm/configs/sama7_defconfig
> index 0368068e04d9..bc29badab890 100644
> --- a/arch/arm/configs/sama7_defconfig
> +++ b/arch/arm/configs/sama7_defconfig
> @@ -138,6 +138,8 @@ CONFIG_SND_SOC_MIKROE_PROTO=m
>   CONFIG_SND_MCHP_SOC_I2S_MCC=y
>   CONFIG_SND_MCHP_SOC_SPDIFTX=y
>   CONFIG_SND_MCHP_SOC_SPDIFRX=y
> +CONFIG_SND_MCHP_SOC_PDMC=y
> +CONFIG_SND_SOC_DMIC=y

I'm fine with that, but I see that some Kconfig entries "select" this 
SND_SOC_DMIC directly (amd, intel, mediatek, stm).
If it's absolutely needed for PDMC to work, what about doing the same as 
it would prevent some broken configurations?

Regards,
   Nicolas

>   CONFIG_SND_SOC_PCM5102A=y
>   CONFIG_SND_SOC_SPDIF=y
>   CONFIG_SND_SIMPLE_CARD=y
Codrin Ciubotariu May 5, 2022, 2:47 p.m. UTC | #2
On 05.05.2022 16:58, Nicolas Ferre wrote:
> On 07/03/2022 at 13:22, Codrin Ciubotariu wrote:
>> Enable drivers needed for Microchip's PDMC and PDM microphones.
>>
>> Signed-off-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
>> ---
>>
>> Changes in v2,v3:
>>   - none;
>>
>>   arch/arm/configs/sama7_defconfig | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/arm/configs/sama7_defconfig 
>> b/arch/arm/configs/sama7_defconfig
>> index 0368068e04d9..bc29badab890 100644
>> --- a/arch/arm/configs/sama7_defconfig
>> +++ b/arch/arm/configs/sama7_defconfig
>> @@ -138,6 +138,8 @@ CONFIG_SND_SOC_MIKROE_PROTO=m
>>   CONFIG_SND_MCHP_SOC_I2S_MCC=y
>>   CONFIG_SND_MCHP_SOC_SPDIFTX=y
>>   CONFIG_SND_MCHP_SOC_SPDIFRX=y
>> +CONFIG_SND_MCHP_SOC_PDMC=y
>> +CONFIG_SND_SOC_DMIC=y
> 
> I'm fine with that, but I see that some Kconfig entries "select" this 
> SND_SOC_DMIC directly (amd, intel, mediatek, stm).
> If it's absolutely needed for PDMC to work, what about doing the same as 
> it would prevent some broken configurations?

The only way it makes sense to me to have this driver selected somewhere 
is in a sound card driver, used for a specific board, which we know it 
has PDM microphones. Since, for now, we use the simple sound card for 
our audio interfaces, we have no place to add this select.
The reason I do not like to add this select under the controller driver, 
as some of the vendors did, is because, in the future, we might have 
different PDM microphones that might not work with SND_SOC_DMIC and 
might need a different driver.
I don't have a strong opinion on this. If you think I am overthinking, 
please let me know and I will change this.

Best regards,
Codrin
Mark Brown May 5, 2022, 3:01 p.m. UTC | #3
On Thu, May 05, 2022 at 02:47:04PM +0000, Codrin.Ciubotariu@microchip.com wrote:
> On 05.05.2022 16:58, Nicolas Ferre wrote:

> > I'm fine with that, but I see that some Kconfig entries "select" this 
> > SND_SOC_DMIC directly (amd, intel, mediatek, stm).
> > If it's absolutely needed for PDMC to work, what about doing the same as 
> > it would prevent some broken configurations?

> The only way it makes sense to me to have this driver selected somewhere 
> is in a sound card driver, used for a specific board, which we know it 
> has PDM microphones. Since, for now, we use the simple sound card for 
> our audio interfaces, we have no place to add this select.
> The reason I do not like to add this select under the controller driver, 
> as some of the vendors did, is because, in the future, we might have 
> different PDM microphones that might not work with SND_SOC_DMIC and 
> might need a different driver.
> I don't have a strong opinion on this. If you think I am overthinking, 
> please let me know and I will change this.

It's unlikely but possible that there could be some other device
connected (eg, you could have a DSP or something that generates PDM
output).  If the driver doesn't directly instantiate the DMIC itself
then it's probably reasonable for it to be user controllable if the DMIC
driver is there.
Nicolas Ferre May 5, 2022, 3:07 p.m. UTC | #4
On 05/05/2022 at 17:01, Mark Brown wrote:
> On Thu, May 05, 2022 at 02:47:04PM +0000,Codrin.Ciubotariu@microchip.com  wrote:
>> On 05.05.2022 16:58, Nicolas Ferre wrote:
>>> I'm fine with that, but I see that some Kconfig entries "select" this
>>> SND_SOC_DMIC directly (amd, intel, mediatek, stm).
>>> If it's absolutely needed for PDMC to work, what about doing the same as
>>> it would prevent some broken configurations?
>> The only way it makes sense to me to have this driver selected somewhere
>> is in a sound card driver, used for a specific board, which we know it
>> has PDM microphones. Since, for now, we use the simple sound card for
>> our audio interfaces, we have no place to add this select.
>> The reason I do not like to add this select under the controller driver,
>> as some of the vendors did, is because, in the future, we might have
>> different PDM microphones that might not work with SND_SOC_DMIC and
>> might need a different driver.
>> I don't have a strong opinion on this. If you think I am overthinking,
>> please let me know and I will change this.
> It's unlikely but possible that there could be some other device
> connected (eg, you could have a DSP or something that generates PDM
> output).  If the driver doesn't directly instantiate the DMIC itself
> then it's probably reasonable for it to be user controllable if the DMIC
> driver is there.

Fair enough, It makes perfect sense to me as it is then.
Thanks for the feedback!

Best regards,
   Nicolas
diff mbox series

Patch

diff --git a/arch/arm/configs/sama7_defconfig b/arch/arm/configs/sama7_defconfig
index 0368068e04d9..bc29badab890 100644
--- a/arch/arm/configs/sama7_defconfig
+++ b/arch/arm/configs/sama7_defconfig
@@ -138,6 +138,8 @@  CONFIG_SND_SOC_MIKROE_PROTO=m
 CONFIG_SND_MCHP_SOC_I2S_MCC=y
 CONFIG_SND_MCHP_SOC_SPDIFTX=y
 CONFIG_SND_MCHP_SOC_SPDIFRX=y
+CONFIG_SND_MCHP_SOC_PDMC=y
+CONFIG_SND_SOC_DMIC=y
 CONFIG_SND_SOC_PCM5102A=y
 CONFIG_SND_SOC_SPDIF=y
 CONFIG_SND_SIMPLE_CARD=y