Skip to contentSkip to conversation

ASoC: rt5640: Use the platform data for DMIC settings

PATCH Applied Oder Chiou 3 participants 8 messages Mar 28, 2014 – Mar 31, 2014

Subsystems (3)
Sound ASoC Realtek audio codecs
lore
Share & tools
lore

Oder Chiou

author maintainer
#0 lore raw

PATCH 1/1 ASoC: rt5640: Use the platform data for DMIC settings

The patch uses the platform data for DMIC settings.

Signed-off-by Oder Chiou <[email protected]>
---
 include/sound/rt5640.h    |  4 +++
 sound/soc/codecs/rt5640.c | 77 ++++++++++++++---------------------------------
 2 files changed, 27 insertions(+), 54 deletions(-)
include/sound/rt5640.h +4 -0
index 27cc75e..59d26dd 100644
@@ -16,6 +16,10 @@ struct rt5640_platform_data {
 	bool in1_diff;
 	bool in2_diff;
 
+	bool dmic_en;
+	bool dmic1_data_pin; /* 0 = IN1P; 1 = GPIO3 */
+	bool dmic2_data_pin; /* 0 = IN1N; 1 = GPIO4 */
+
 	int ldo1_en; /* GPIO for LDO1_EN */
 };
sound/soc/codecs/rt5640.c +23 -54

95 lines not on this page · raw message

Signature · 1 line
1.8.1.1.439.g50a6b54
maintainer Applied
#1 lore raw
Quoted 1 line · Oder Chiou

The patch uses the platform data for DMIC settings.

Applied, thanks.

#2 lore raw
In reply to Oder Chiou
Quoted 1 line · Oder Chiou

The patch uses the platform data for DMIC settings.

Shouldn't this also support parsing the same information from device tree?

maintainer
#3 lore raw
Quoted 5 lines · Stephen Warren
Quoted 1 line · Oder Chiou

The patch uses the platform data for DMIC settings.

Shouldn't this also support parsing the same information from device tree?

The driver has no DT support at all at the minute but if it's being used on platforms using DT (which of course it is now I think about it - I just looked for the DT support when reviewing) then yes it should.

#4 lore raw
Quoted 10 lines · Mark Brown
Quoted 5 lines · Stephen Warren
Quoted 1 line · Oder Chiou

The patch uses the platform data for DMIC settings.

Shouldn't this also support parsing the same information from device tree?

The driver has no DT support at all at the minute but if it's being used on platforms using DT (which of course it is now I think about it - I just looked for the DT support when reviewing) then yes it should.

The driver doesn't have an OF match table (I'll send a patch to fix that soon), but certainly does support DT; see rt5640_parse_dt().

maintainer
#5 lore raw
Quoted 8 lines · Stephen Warren
Quoted 3 lines · Mark Brown

The driver has no DT support at all at the minute but if it's being used on platforms using DT (which of course it is now I think about it - I just looked for the DT support when reviewing) then yes it should.

The driver doesn't have an OF match table (I'll send a patch to fix that soon), but certainly does support DT; see rt5640_parse_dt().

Oh, dear. That's not clever and we do need the IDs adding, that's the baseline thing needed for DT support.

#6 lore raw
Quoted 7 lines · Mark Brown
Quoted 5 lines · Stephen Warren
Quoted 3 lines · Mark Brown

The driver has no DT support at all at the minute but if it's being used on platforms using DT (which of course it is now I think about it - I just looked for the DT support when reviewing) then yes it should.

Quoted 5 lines
Quoted 2 lines

The driver doesn't have an OF match table (I'll send a patch to fix that soon), but certainly does support DT; see rt5640_parse_dt().

Oh, dear. That's not clever and we do need the IDs adding, that's the baseline thing needed for DT support.

I really wish we would make up our minds about this.

For I2C (and SPI and perhaps others) the I2C match table works fine as a replacement for the of_match table. The only issue might be different manufacturers with the same chip names. If this is a problem, why is fallback to the I2C match table even allowed any more; we should mandate that OF matching only works via the OF match table.

When DT was young, Grant tried to require of_match for everything for completeness, and then I tried enforcing that for reviews, and then Grant said not to bother with that, so I stopped, and now you're saying it's required again. I really wish I could get consistency in how this kind of thing is supposed to work. It's difficult for contributors to know what to do if reviewers keep flip-flopping over time.

maintainer
#7 lore raw
Quoted 8 lines · Stephen Warren

I really wish we would make up our minds about this.

For I2C (and SPI and perhaps others) the I2C match table works fine as a replacement for the of_match table. The only issue might be different manufacturers with the same chip names. If this is a problem, why is fallback to the I2C match table even allowed any more; we should mandate that OF matching only works via the OF match table.

Quoted 6 lines

When DT was young, Grant tried to require of_match for everything for completeness, and then I tried enforcing that for reviews, and then Grant said not to bother with that, so I stopped, and now you're saying it's required again. I really wish I could get consistency in how this kind of thing is supposed to work. It's difficult for contributors to know what to do if reviewers keep flip-flopping over time.

Well, *I've* not been flip flopping on this, frankly I was unaware that anyone thought it was a particularly good idea to actively not include the match table. It's true that as a matter of practicality you don't need to bother at the minute but I think especially once you're adding any explicit code at all to the driver the explicit match strings ought to be there too.

I suspect this may have been a pragmatic suggestion due to all the complaints about churn generated by DT.