[PATCH v2 0/4] Add missing dt-binding properties to rt5682(s)
This series adds the missing sound-dai-cells to rt5682s and supplies for both rt5682s and rt5682.
These properties are already used by sc7180-trogdor.dtsi (and derived DTs like sc7180-trogdor-kingoftown.dtsi), so the commits in this series are really just documenting their usage.
v1: https://lore.kernel.org/all/20221021190908.1502026-1-nfraprado@collabora.com...
Changes in v2: - Changed sound-dai-cells to 1 on rt5682s - Added commit fixing sound-dai-cells on rt5682 - Added mention to commit messages that properties are already being used by DTs
Nícolas F. R. A. Prado (4): ASoC: dt-bindings: realtek,rt5682s: Add #sound-dai-cells ASoC: dt-bindings: realtek,rt5682s: Add AVDD and MICVDD supplies ASoC: dt-bindings: rt5682: Set sound-dai-cells to 1 ASoC: dt-bindings: rt5682: Add AVDD, MICVDD and VBAT supplies
.../devicetree/bindings/sound/realtek,rt5682s.yaml | 7 +++++++ Documentation/devicetree/bindings/sound/rt5682.txt | 8 +++++++- 2 files changed, 14 insertions(+), 1 deletion(-)
The rt5682s codec is a DAI provider with two interfaces - AIF1 and AIF2 - and therefore should have a #sound-dai-cells property that is equal to 1. Add it.
Acked-by: Krzysztof Kozlowski krzysztof.kozlowski@linaro.org Signed-off-by: Nícolas F. R. A. Prado nfraprado@collabora.com
---
Changes in v2: - Changed commit message - Changed value to 1
Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml b/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml index ca5b8987b749..1c0b06d82369 100644 --- a/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml +++ b/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml @@ -87,6 +87,9 @@ properties: maxItems: 2 description: Name given for DAI word clock and bit clock outputs.
+ "#sound-dai-cells": + const: 1 + additionalProperties: false
required:
Il 25/10/22 00:00, Nícolas F. R. A. Prado ha scritto:
The rt5682s codec is a DAI provider with two interfaces - AIF1 and AIF2
- and therefore should have a #sound-dai-cells property that is equal to
- Add it.
Acked-by: Krzysztof Kozlowski krzysztof.kozlowski@linaro.org Signed-off-by: Nícolas F. R. A. Prado nfraprado@collabora.com
Reviewed-by: AngeloGioacchino Del Regno angelogioacchino.delregno@collabora.com
On Tue, Oct 25, 2022 at 3:07 AM AngeloGioacchino Del Regno angelogioacchino.delregno@collabora.com wrote:
Il 25/10/22 00:00, Nícolas F. R. A. Prado ha scritto:
The rt5682s codec is a DAI provider with two interfaces - AIF1 and AIF2
- and therefore should have a #sound-dai-cells property that is equal to
- Add it.
Acked-by: Krzysztof Kozlowski krzysztof.kozlowski@linaro.org Signed-off-by: Nícolas F. R. A. Prado nfraprado@collabora.com
Reviewed-by: AngeloGioacchino Del Regno angelogioacchino.delregno@collabora.com
Reviewed-by: Chen-Yu Tsai wenst@chromium.org
The rt5682s codec can have two supplies: AVDD and MICVDD. They are already used by sc7180-trogdor-kingoftown.dtsi, so document them in the binding.
Signed-off-by: Nícolas F. R. A. Prado nfraprado@collabora.com
---
Changes in v2: - Added mention that property is already used in a DT to the commit message
Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml b/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml index 1c0b06d82369..ac1dea5b4450 100644 --- a/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml +++ b/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml @@ -90,6 +90,10 @@ properties: "#sound-dai-cells": const: 1
+ AVDD-supply: true + + MICVDD-supply: true + additionalProperties: false
required:
Il 25/10/22 00:00, Nícolas F. R. A. Prado ha scritto:
The rt5682s codec can have two supplies: AVDD and MICVDD. They are already used by sc7180-trogdor-kingoftown.dtsi, so document them in the binding.
Signed-off-by: Nícolas F. R. A. Prado nfraprado@collabora.com
I also don't like these uppercase supply names... I wonder if it's worth changing the driver to get "avdd" *or* "AVDD" (so, if "avdd" fails -> backwards compat)...
...this way, we can change the devicetree to use the lowercase names without breaking abi.
Of course, this commit would need to be changed to document only the lowercase supply names.
Driver-wise, we have a rt5682s_supply_names array... we could do something like:
static const char *rt5682s_supply_names_legacy[RT5682S_NUM_SUPPLIES] = { [RT5682S_SUPPLY_AVDD] = "AVDD", [RT5682S_SUPPLY_MICVDD] = "MICVDD", };
static const char *rt5682s_supply_names[RT5682S_NUM_SUPPLIES] = { [RT5682S_SUPPLY_AVDD] = "avdd", [RT5682S_SUPPLY_MICVDD] = "micvdd", };
for (...) assign_supply_names; ret = devm_regulator_bulk_get(...);
if (ret) { for (...) assign_legacy_supply_names; ret = devm_regulator_bulk_get(...) if (ret) return ret; }
What do you think?
Cheers, Angelo
On Tue, Oct 25, 2022 at 12:06:23PM +0200, AngeloGioacchino Del Regno wrote:
Il 25/10/22 00:00, Nícolas F. R. A. Prado ha scritto:
The rt5682s codec can have two supplies: AVDD and MICVDD. They are already used by sc7180-trogdor-kingoftown.dtsi, so document them in the binding.
Signed-off-by: Nícolas F. R. A. Prado nfraprado@collabora.com
I also don't like these uppercase supply names... I wonder if it's worth changing the driver to get "avdd" *or* "AVDD" (so, if "avdd" fails -> backwards compat)...
...this way, we can change the devicetree to use the lowercase names without breaking abi.
Of course, this commit would need to be changed to document only the lowercase supply names.
Driver-wise, we have a rt5682s_supply_names array... we could do something like:
static const char *rt5682s_supply_names_legacy[RT5682S_NUM_SUPPLIES] = { [RT5682S_SUPPLY_AVDD] = "AVDD", [RT5682S_SUPPLY_MICVDD] = "MICVDD", };
static const char *rt5682s_supply_names[RT5682S_NUM_SUPPLIES] = { [RT5682S_SUPPLY_AVDD] = "avdd", [RT5682S_SUPPLY_MICVDD] = "micvdd", };
for (...) assign_supply_names; ret = devm_regulator_bulk_get(...);
if (ret) { for (...) assign_legacy_supply_names; ret = devm_regulator_bulk_get(...) if (ret) return ret; }
What do you think?
No one seems opposed to it, so I'll add that to the next version.
Thanks, Nícolas
On Tue, Oct 25, 2022 at 12:06:23PM +0200, AngeloGioacchino Del Regno wrote:
Il 25/10/22 00:00, Nícolas F. R. A. Prado ha scritto:
The rt5682s codec can have two supplies: AVDD and MICVDD. They are already used by sc7180-trogdor-kingoftown.dtsi, so document them in the binding.
Signed-off-by: Nícolas F. R. A. Prado nfraprado@collabora.com
I also don't like these uppercase supply names... I wonder if it's worth changing the driver to get "avdd" *or* "AVDD" (so, if "avdd" fails -> backwards compat)...
...this way, we can change the devicetree to use the lowercase names without breaking abi.
Of course, this commit would need to be changed to document only the lowercase supply names.
Driver-wise, we have a rt5682s_supply_names array... we could do something like:
static const char *rt5682s_supply_names_legacy[RT5682S_NUM_SUPPLIES] = { [RT5682S_SUPPLY_AVDD] = "AVDD", [RT5682S_SUPPLY_MICVDD] = "MICVDD", };
static const char *rt5682s_supply_names[RT5682S_NUM_SUPPLIES] = { [RT5682S_SUPPLY_AVDD] = "avdd", [RT5682S_SUPPLY_MICVDD] = "micvdd", };
for (...) assign_supply_names; ret = devm_regulator_bulk_get(...);
if (ret) { for (...) assign_legacy_supply_names; ret = devm_regulator_bulk_get(...) if (ret) return ret; }
What do you think?
Hi,
I took a stab at this, but the resulting code wasn't as elegant. The default behavior of the regulator core when a regulator is missing is to just print a warning and give a dummy regulator in its place. A way around this is the "optional" variant, but it doesn't have a bulk version (in fact seems like it was added and then removed a few years back, but I haven't dug out the reason).
So the result was a code block that wasn't nearly as neat as your draft above and it didn't seem worth it to add this complexity just to gain the usage of lowercase properties, which is why in the end I decided to not include this in the series I just sent [1].
I've included the patch below. If you do think there's a more reasonable approach or if having the lowercase supplies is worth it, let me know.
Thanks, Nícolas
[1] https://lore.kernel.org/all/20221028205540.3197304-1-nfraprado@collabora.com
From 8de4a86f10ba2e13458afe63fe658df685b21b45 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?N=C3=ADcolas=20F=2E=20R=2E=20A=2E=20Prado?= nfraprado@collabora.com Date: Thu, 27 Oct 2022 16:10:22 -0400 Subject: [PATCH] ASoC: rt5682s: Handle lowercase supply name and fallback if needed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
Supply names provided by devicetree are conventionally lowercase. In order to be able to use lowercase names without breaking existing setups, detect if any of the older names are used and if so fallback to them.
Signed-off-by: Nícolas F. R. A. Prado nfraprado@collabora.com --- sound/soc/codecs/rt5682s.c | 40 ++++++++++++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 8 deletions(-)
diff --git a/sound/soc/codecs/rt5682s.c b/sound/soc/codecs/rt5682s.c index 466a37f3500c..3cefa016be77 100644 --- a/sound/soc/codecs/rt5682s.c +++ b/sound/soc/codecs/rt5682s.c @@ -41,11 +41,16 @@ static const struct rt5682s_platform_data i2s_default_platform_data = { .dai_clk_names[RT5682S_DAI_BCLK_IDX] = "rt5682-dai-bclk", };
-static const char *rt5682s_supply_names[RT5682S_NUM_SUPPLIES] = { +static const char *rt5682s_legacy_supply_names[RT5682S_NUM_SUPPLIES] = { [RT5682S_SUPPLY_AVDD] = "AVDD", [RT5682S_SUPPLY_MICVDD] = "MICVDD", };
+static const char *rt5682s_supply_names[RT5682S_NUM_SUPPLIES] = { + [RT5682S_SUPPLY_AVDD] = "avdd", + [RT5682S_SUPPLY_MICVDD] = "micvdd", +}; + static const struct reg_sequence patch_list[] = { {RT5682S_I2C_CTRL, 0x0007}, {RT5682S_DIG_IN_CTRL_1, 0x0000}, @@ -3090,7 +3095,9 @@ static int rt5682s_i2c_probe(struct i2c_client *i2c) struct rt5682s_platform_data *pdata = dev_get_platdata(&i2c->dev); struct rt5682s_priv *rt5682s; int i, ret; + struct regulator *reg; unsigned int val; + bool using_legacy_supply_names = false;
rt5682s = devm_kzalloc(&i2c->dev, sizeof(struct rt5682s_priv), GFP_KERNEL); if (!rt5682s) @@ -3112,14 +3119,31 @@ static int rt5682s_i2c_probe(struct i2c_client *i2c) return ret; }
- for (i = 0; i < ARRAY_SIZE(rt5682s->supplies); i++) - rt5682s->supplies[i].supply = rt5682s_supply_names[i]; + for (i = 0; i < ARRAY_SIZE(rt5682s_supply_names); i++) { + reg = devm_regulator_get_optional(&i2c->dev, rt5682s_supply_names[i]); + if (IS_ERR(reg)) { + if (PTR_ERR(reg) == -ENODEV) { + using_legacy_supply_names = true; + break; + } else { + dev_err(&i2c->dev, "Failed to request supplies: %d\n", ret); + return PTR_ERR(reg); + } + }
- ret = devm_regulator_bulk_get(&i2c->dev, - ARRAY_SIZE(rt5682s->supplies), rt5682s->supplies); - if (ret) { - dev_err(&i2c->dev, "Failed to request supplies: %d\n", ret); - return ret; + rt5682s->supplies[i].consumer = reg; + } + + if (using_legacy_supply_names) { + for (i = 0; i < ARRAY_SIZE(rt5682s->supplies); i++) + rt5682s->supplies[i].supply = rt5682s_legacy_supply_names[i]; + + ret = devm_regulator_bulk_get(&i2c->dev, ARRAY_SIZE(rt5682s->supplies), + rt5682s->supplies); + if (ret) { + dev_err(&i2c->dev, "Failed to request supplies: %d\n", ret); + return ret; + } }
ret = devm_add_action_or_reset(&i2c->dev, rt5682s_i2c_disable_regulators, rt5682s); -- 2.38.1
On 24/10/2022 18:00, Nícolas F. R. A. Prado wrote:
The rt5682s codec can have two supplies: AVDD and MICVDD. They are already used by sc7180-trogdor-kingoftown.dtsi, so document them in the binding.
Signed-off-by: Nícolas F. R. A. Prado nfraprado@collabora.com
Changes in v2:
- Added mention that property is already used in a DT to the commit message
You already got an ack for it. Don't ignore it.
Acked-by: Krzysztof Kozlowski krzysztof.kozlowski@linaro.org
Best regards, Krzysztof
On Mon, Oct 24, 2022 at 3:01 PM Nícolas F. R. A. Prado nfraprado@collabora.com wrote:
The rt5682s codec can have two supplies: AVDD and MICVDD. They are
The actual chip also has LDO1_IN (for digital core and charge pump) and DBVDD (for I/O). However in the Chromebook designs these two and AVDD are all provided from the same power rail, through separate filter banks.
Neither does the datasheet specify the ordering of AVDD, DBVDD, and LDO1_IN for power sequencing, just that three should be toggled together.
Should we model these? Or wait until some design actually splits these?
ChenYu
already used by sc7180-trogdor-kingoftown.dtsi, so document them in the binding.
Signed-off-by: Nícolas F. R. A. Prado nfraprado@collabora.com
Changes in v2:
- Added mention that property is already used in a DT to the commit message
Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml b/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml index 1c0b06d82369..ac1dea5b4450 100644 --- a/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml +++ b/Documentation/devicetree/bindings/sound/realtek,rt5682s.yaml @@ -90,6 +90,10 @@ properties: "#sound-dai-cells": const: 1
- AVDD-supply: true
- MICVDD-supply: true
additionalProperties: false
required:
2.38.1
On Tue, Oct 25, 2022 at 01:12:49PM -0700, Chen-Yu Tsai wrote:
On Mon, Oct 24, 2022 at 3:01 PM Nícolas F. R. A. Prado
The rt5682s codec can have two supplies: AVDD and MICVDD. They are
Neither does the datasheet specify the ordering of AVDD, DBVDD, and LDO1_IN for power sequencing, just that three should be toggled together.
Should we model these? Or wait until some design actually splits these?
Yes, the driver for a chip should be a driver for the chip not for some specific board.
On Tue, Oct 25, 2022 at 01:12:49PM -0700, Chen-Yu Tsai wrote:
On Mon, Oct 24, 2022 at 3:01 PM Nícolas F. R. A. Prado nfraprado@collabora.com wrote:
The rt5682s codec can have two supplies: AVDD and MICVDD. They are
The actual chip also has LDO1_IN (for digital core and charge pump) and DBVDD (for I/O). However in the Chromebook designs these two and AVDD are all provided from the same power rail, through separate filter banks.
What about rt5682 (no s), does that chip also have these same supplies?
Also, since you already gave the purpose of these other supplies, could you also tell the purpose of AVDD, MICVDD and (for rt5682) VBAT? That way I could add some description for them in the binding.
Thanks, Nícolas
Neither does the datasheet specify the ordering of AVDD, DBVDD, and LDO1_IN for power sequencing, just that three should be toggled together.
Should we model these? Or wait until some design actually splits these?
[..]
On Thu, Oct 27, 2022 at 10:36:27AM -0400, Nícolas F. R. A. Prado wrote:
Also, since you already gave the purpose of these other supplies, could you also tell the purpose of AVDD, MICVDD and (for rt5682) VBAT? That way I could add some description for them in the binding.
Those are all very conventional names - analog, microphone and battery supplies.
On Thu, Oct 27, 2022 at 7:36 AM Nícolas F. R. A. Prado nfraprado@collabora.com wrote:
On Tue, Oct 25, 2022 at 01:12:49PM -0700, Chen-Yu Tsai wrote:
On Mon, Oct 24, 2022 at 3:01 PM Nícolas F. R. A. Prado nfraprado@collabora.com wrote:
The rt5682s codec can have two supplies: AVDD and MICVDD. They are
The actual chip also has LDO1_IN (for digital core and charge pump) and DBVDD (for I/O). However in the Chromebook designs these two and AVDD are all provided from the same power rail, through separate filter banks.
What about rt5682 (no s), does that chip also have these same supplies?
Also, since you already gave the purpose of these other supplies, could you also tell the purpose of AVDD, MICVDD and (for rt5682) VBAT? That way I could add some description for them in the binding.
As Mark mentioned in his reply, these are quite standard names.
AVDD is for the analog bits. MICVDD is for the microphone bias. VBAT is called battery power in the datasheet. The block diagram shows it going through an internal controllable LDO whose output then powers MICVDD. This could be used in designs that don't include a suitable external supply for MICVDD. If MICVDD is provided, then one would turn the internal LDO off.
So either VBAT or MICVDD has to be provided.
ChenYu
Thanks, Nícolas
Neither does the datasheet specify the ordering of AVDD, DBVDD, and LDO1_IN for power sequencing, just that three should be toggled together.
Should we model these? Or wait until some design actually splits these?
[..]
On Thu, Oct 27, 2022 at 10:48 AM Chen-Yu Tsai wenst@chromium.org wrote:
On Thu, Oct 27, 2022 at 7:36 AM Nícolas F. R. A. Prado nfraprado@collabora.com wrote:
On Tue, Oct 25, 2022 at 01:12:49PM -0700, Chen-Yu Tsai wrote:
On Mon, Oct 24, 2022 at 3:01 PM Nícolas F. R. A. Prado nfraprado@collabora.com wrote:
The rt5682s codec can have two supplies: AVDD and MICVDD. They are
The actual chip also has LDO1_IN (for digital core and charge pump) and DBVDD (for I/O). However in the Chromebook designs these two and AVDD are all provided from the same power rail, through separate filter banks.
What about rt5682 (no s), does that chip also have these same supplies?
(Missed this question)
The RT5682 has the same supplies, plus the VBAT one.
ChenYu
Also, since you already gave the purpose of these other supplies, could you also tell the purpose of AVDD, MICVDD and (for rt5682) VBAT? That way I could add some description for them in the binding.
As Mark mentioned in his reply, these are quite standard names.
AVDD is for the analog bits. MICVDD is for the microphone bias. VBAT is called battery power in the datasheet. The block diagram shows it going through an internal controllable LDO whose output then powers MICVDD. This could be used in designs that don't include a suitable external supply for MICVDD. If MICVDD is provided, then one would turn the internal LDO off.
So either VBAT or MICVDD has to be provided.
ChenYu
Thanks, Nícolas
Neither does the datasheet specify the ordering of AVDD, DBVDD, and LDO1_IN for power sequencing, just that three should be toggled together.
Should we model these? Or wait until some design actually splits these?
[..]
On Thu, Oct 27, 2022 at 11:11:08AM -0700, Chen-Yu Tsai wrote:
On Thu, Oct 27, 2022 at 10:48 AM Chen-Yu Tsai wenst@chromium.org wrote:
On Thu, Oct 27, 2022 at 7:36 AM Nícolas F. R. A. Prado nfraprado@collabora.com wrote:
On Tue, Oct 25, 2022 at 01:12:49PM -0700, Chen-Yu Tsai wrote:
On Mon, Oct 24, 2022 at 3:01 PM Nícolas F. R. A. Prado nfraprado@collabora.com wrote:
The rt5682s codec can have two supplies: AVDD and MICVDD. They are
The actual chip also has LDO1_IN (for digital core and charge pump) and DBVDD (for I/O). However in the Chromebook designs these two and AVDD are all provided from the same power rail, through separate filter banks.
What about rt5682 (no s), does that chip also have these same supplies?
(Missed this question)
The RT5682 has the same supplies, plus the VBAT one.
ChenYu
Also, since you already gave the purpose of these other supplies, could you also tell the purpose of AVDD, MICVDD and (for rt5682) VBAT? That way I could add some description for them in the binding.
As Mark mentioned in his reply, these are quite standard names.
AVDD is for the analog bits. MICVDD is for the microphone bias. VBAT is called battery power in the datasheet. The block diagram shows it going through an internal controllable LDO whose output then powers MICVDD. This could be used in designs that don't include a suitable external supply for MICVDD. If MICVDD is provided, then one would turn the internal LDO off.
So either VBAT or MICVDD has to be provided.
So if I get this right we should be making the following supplies required: AVDD VBAT or MICVDD (on rt5682s there's no VBAT so just MICVDD)
And for LDO1_ON and DBVDD what would be the best way to model? Should we make them required in the binding as well and add them pointing to the same regulator from AVDD in the chromebook DT or just leave them as optional?
Thanks, Nícolas
ChenYu
Thanks, Nícolas
Neither does the datasheet specify the ordering of AVDD, DBVDD, and LDO1_IN for power sequencing, just that three should be toggled together.
Should we model these? Or wait until some design actually splits these?
[..]
On Thu, Oct 27, 2022 at 12:20 PM Nícolas F. R. A. Prado nfraprado@collabora.com wrote:
On Thu, Oct 27, 2022 at 11:11:08AM -0700, Chen-Yu Tsai wrote:
On Thu, Oct 27, 2022 at 10:48 AM Chen-Yu Tsai wenst@chromium.org wrote:
On Thu, Oct 27, 2022 at 7:36 AM Nícolas F. R. A. Prado nfraprado@collabora.com wrote:
On Tue, Oct 25, 2022 at 01:12:49PM -0700, Chen-Yu Tsai wrote:
On Mon, Oct 24, 2022 at 3:01 PM Nícolas F. R. A. Prado nfraprado@collabora.com wrote:
The rt5682s codec can have two supplies: AVDD and MICVDD. They are
The actual chip also has LDO1_IN (for digital core and charge pump) and DBVDD (for I/O). However in the Chromebook designs these two and AVDD are all provided from the same power rail, through separate filter banks.
What about rt5682 (no s), does that chip also have these same supplies?
(Missed this question)
The RT5682 has the same supplies, plus the VBAT one.
ChenYu
Also, since you already gave the purpose of these other supplies, could you also tell the purpose of AVDD, MICVDD and (for rt5682) VBAT? That way I could add some description for them in the binding.
As Mark mentioned in his reply, these are quite standard names.
AVDD is for the analog bits. MICVDD is for the microphone bias. VBAT is called battery power in the datasheet. The block diagram shows it going through an internal controllable LDO whose output then powers MICVDD. This could be used in designs that don't include a suitable external supply for MICVDD. If MICVDD is provided, then one would turn the internal LDO off.
So either VBAT or MICVDD has to be provided.
So if I get this right we should be making the following supplies required: AVDD VBAT or MICVDD (on rt5682s there's no VBAT so just MICVDD)
That's right.
And for LDO1_ON and DBVDD what would be the best way to model? Should we make them required in the binding as well and add them pointing to the same regulator from AVDD in the chromebook DT or just leave them as optional?
They should be required. And we then describe them to match the board.
ChenYu
Thanks, Nícolas
ChenYu
Thanks, Nícolas
Neither does the datasheet specify the ordering of AVDD, DBVDD, and LDO1_IN for power sequencing, just that three should be toggled together.
Should we model these? Or wait until some design actually splits these?
[..]
Commit 0adccaf1eac9 ("ASoC: dt-bindings: rt5682: Add #sound-dai-cells") defined the sound-dai-cells property as 0. However, rt5682 has two DAIs, AIF1 and AIF2, and therefore should have sound-dai-cells set to 1. Fix it.
Fixes: 0adccaf1eac9 ("ASoC: dt-bindings: rt5682: Add #sound-dai-cells") Signed-off-by: Nícolas F. R. A. Prado nfraprado@collabora.com
---
Changes in v2: - Added this commit
Documentation/devicetree/bindings/sound/rt5682.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/sound/rt5682.txt b/Documentation/devicetree/bindings/sound/rt5682.txt index c5f2b8febcee..6b87db68337c 100644 --- a/Documentation/devicetree/bindings/sound/rt5682.txt +++ b/Documentation/devicetree/bindings/sound/rt5682.txt @@ -46,7 +46,7 @@ Optional properties:
- realtek,dmic-clk-driving-high : Set the high driving of the DMIC clock out.
-- #sound-dai-cells: Should be set to '<0>'. +- #sound-dai-cells: Should be set to '<1>'.
Pins on the device (for linking into audio routes) for RT5682:
Il 25/10/22 00:00, Nícolas F. R. A. Prado ha scritto:
Commit 0adccaf1eac9 ("ASoC: dt-bindings: rt5682: Add #sound-dai-cells") defined the sound-dai-cells property as 0. However, rt5682 has two DAIs, AIF1 and AIF2, and therefore should have sound-dai-cells set to 1. Fix it.
Fixes: 0adccaf1eac9 ("ASoC: dt-bindings: rt5682: Add #sound-dai-cells") Signed-off-by: Nícolas F. R. A. Prado nfraprado@collabora.com
Reviewed-by: AngeloGioacchino Del Regno angelogioacchino.delregno@collabora.com
On 24/10/2022 18:00, Nícolas F. R. A. Prado wrote:
Commit 0adccaf1eac9 ("ASoC: dt-bindings: rt5682: Add #sound-dai-cells") defined the sound-dai-cells property as 0. However, rt5682 has two DAIs, AIF1 and AIF2, and therefore should have sound-dai-cells set to 1. Fix it.
Fixes: 0adccaf1eac9 ("ASoC: dt-bindings: rt5682: Add #sound-dai-cells") Signed-off-by: Nícolas F. R. A. Prado nfraprado@collabora.com
Acked-by: Krzysztof Kozlowski krzysztof.kozlowski@linaro.org
Best regards, Krzysztof
On Mon, Oct 24, 2022 at 3:02 PM Nícolas F. R. A. Prado nfraprado@collabora.com wrote:
Commit 0adccaf1eac9 ("ASoC: dt-bindings: rt5682: Add #sound-dai-cells") defined the sound-dai-cells property as 0. However, rt5682 has two DAIs, AIF1 and AIF2, and therefore should have sound-dai-cells set to 1. Fix it.
Fixes: 0adccaf1eac9 ("ASoC: dt-bindings: rt5682: Add #sound-dai-cells") Signed-off-by: Nícolas F. R. A. Prado nfraprado@collabora.com
Reviewed-by: Chen-Yu Tsai wenst@chromium.org
The rt5682 codec can have three supplies: AVDD, MICVDD and VBAT. They are already used by sc7180-trogdor.dtsi, so document them in the binding.
Signed-off-by: Nícolas F. R. A. Prado nfraprado@collabora.com
---
Changes in v2: - Added mention that property is already used in a DT to the commit message
Documentation/devicetree/bindings/sound/rt5682.txt | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/rt5682.txt b/Documentation/devicetree/bindings/sound/rt5682.txt index 6b87db68337c..c07a1fc7b122 100644 --- a/Documentation/devicetree/bindings/sound/rt5682.txt +++ b/Documentation/devicetree/bindings/sound/rt5682.txt @@ -48,6 +48,12 @@ Optional properties:
- #sound-dai-cells: Should be set to '<1>'.
+- AVDD-supply: phandle to the regulator supplying AVDD + +- MICVDD-supply: phandle to the regulator supplying MICVDD + +- VBAT-supply: phandle to the regulator supplying VBAT + Pins on the device (for linking into audio routes) for RT5682:
* DMIC L1
On 24/10/2022 18:00, Nícolas F. R. A. Prado wrote:
The rt5682 codec can have three supplies: AVDD, MICVDD and VBAT. They are already used by sc7180-trogdor.dtsi, so document them in the binding.
Signed-off-by: Nícolas F. R. A. Prado nfraprado@collabora.com
Acked-by: Krzysztof Kozlowski krzysztof.kozlowski@linaro.org
Best regards, Krzysztof
On Mon, 24 Oct 2022 18:00:11 -0400, Nícolas F. R. A. Prado wrote:
This series adds the missing sound-dai-cells to rt5682s and supplies for both rt5682s and rt5682.
These properties are already used by sc7180-trogdor.dtsi (and derived DTs like sc7180-trogdor-kingoftown.dtsi), so the commits in this series are really just documenting their usage.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/4] ASoC: dt-bindings: realtek,rt5682s: Add #sound-dai-cells commit: 1c44873993d97f1da264ae9a7adb336d6af8ff9b [3/4] ASoC: dt-bindings: rt5682: Set sound-dai-cells to 1 commit: 07b16192f3f01d002d8ff37dcd4372980330ea93
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
participants (5)
-
AngeloGioacchino Del Regno -
Chen-Yu Tsai -
Krzysztof Kozlowski -
Mark Brown -
Nícolas F. R. A. Prado