[PATCH 0/2] ASoC: es8316: Add regulator support

[PATCH 0/2] ASoC: es8316: Add regulator support

[Hongyang Zhao]

Add regulator support for the four ES8316 power domains so
board descriptions can model and control the codec supplies.

The binding patch documents AVDD, CPVDD, DVDD and PVDD as optional
supplies to preserve compatibility with existing device-tree descriptions.
The driver patch requests and enables them before clocking or accessing the
codec, and unwinds them on failure and removal.

The supplies remain enabled over system suspend because the existing
suspend and resume callbacks do not handle a complete power loss.
Powering the codec down would require separate reset and
reinitialization support.

The missing supply model was identified while reviewing the RubikPi 3
audio support:
https://lore.kernel.org/linux-arm-msm/c293d9c7-bdb7-4303-80c8-404228c434d7@oss.qualcomm.com/

Signed-off-by: Hongyang Zhao <hongyang.zhao@thundersoft.com>
---
Hongyang Zhao (2):
      ASoC: dt-bindings: es8316: Add regulator supplies
      ASoC: codecs: es8316: Add regulator support

 .../devicetree/bindings/sound/everest,es8316.yaml  | 16 ++++++++++
 sound/soc/codecs/es8316.c                          | 35 ++++++++++++++++++++--
 2 files changed, 49 insertions(+), 2 deletions(-)
---
base-commit: b4515cf4156356e8f4fe6e0fdc17f59adab9772f
change-id: 20260723-es8316-regulator-next-20260722-7d96badfd4da

Best regards,
-- 
Hongyang Zhao <hongyang.zhao@thundersoft.com>

[PATCH 1/2] ASoC: dt-bindings: es8316: Add regulator supplies

[Hongyang Zhao]

The ES8316 has separate AVDD, CPVDD, DVDD and PVDD supply inputs
for its analog, charge pump, digital core and digital I/O domains.
Describe all four inputs so boards can model the codec power topology.

Keep the properties optional for compatibility with existing
descriptions.

Signed-off-by: Hongyang Zhao <hongyang.zhao@thundersoft.com>
---
 .../devicetree/bindings/sound/everest,es8316.yaml        | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/everest,es8316.yaml b/Documentation/devicetree/bindings/sound/everest,es8316.yaml
index fe5d938ca310..f45f59b7b070 100644
--- a/Documentation/devicetree/bindings/sound/everest,es8316.yaml
+++ b/Documentation/devicetree/bindings/sound/everest,es8316.yaml
@@ -49,6 +49,18 @@ properties:
     items:
       - const: mclk
 
+  avdd-supply:
+    description: Regulator providing the analog supply, from 2.0 V to 3.6 V
+
+  cpvdd-supply:
+    description: Regulator providing the charge pump supply, from 1.6 V to 2.0 V
+
+  dvdd-supply:
+    description: Regulator providing the digital core supply, from 1.6 V to 3.6 V
+
+  pvdd-supply:
+    description: Regulator providing the digital I/O supply, from 1.6 V to 3.6 V
+
   interrupts:
     maxItems: 1
     description: Headphone detect interrupt
@@ -77,6 +89,10 @@ examples:
         reg = <0x11>;
         clocks = <&clks 10>;
         clock-names = "mclk";
+        avdd-supply = <&reg_3p3v>;
+        cpvdd-supply = <&reg_1p8v>;
+        dvdd-supply = <&reg_1p8v>;
+        pvdd-supply = <&reg_1p8v>;
         #sound-dai-cells = <0>;
       };
     };

-- 
2.43.0

Re: [PATCH 1/2] ASoC: dt-bindings: es8316: Add regulator supplies

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Variant-specific supply properties are defined globally, improperly allowing them for other incompatible hardware variants covered by the same binding.
--

--- Patch [1]: [PATCH 1/2] ASoC: dt-bindings: es8316: Add regulator supplies ---
commit d435f3d7346be3dbbefc6d1734447b71df8be5a3
Author: Hongyang Zhao <hongyang.zhao@thundersoft.com>

ASoC: dt-bindings: es8316: Add regulator supplies

The ES8316 has separate AVDD, CPVDD, DVDD and PVDD supply inputs
for its analog, charge pump, digital core and digital I/O domains.
Describe all four inputs so boards can model the codec power topology.

> diff --git a/Documentation/devicetree/bindings/sound/everest,es8316.yaml b/Documentation/devicetree/bindings/sound/everest,es8316.yaml
> index fe5d938ca3102..f45f59b7b0709 100644
> --- a/Documentation/devicetree/bindings/sound/everest,es8316.yaml
> +++ b/Documentation/devicetree/bindings/sound/everest,es8316.yaml
> @@ -49,6 +49,18 @@ properties:
>      items:
>        - const: mclk
>  
> +  avdd-supply:
> +    description: Regulator providing the analog supply, from 2.0 V to 3.6 V
> +
> +  cpvdd-supply:
> +    description: Regulator providing the charge pump supply, from 1.6 V to 2.0 V

[Severity: Low]
Does defining this property globally inadvertently allow it for other
incompatible hardware variants covered by the same binding?

Since cpvdd-supply is defined at the global properties level, the schema
evaluates it as a valid optional property for all compatibles listed in the
file. This means if hardware variants like everest,es8311 and everest,es8323
lack a charge pump, the schema would silently accept cpvdd-supply in their
device tree nodes instead of properly rejecting it.

Should variant-specific supplies be guarded by an if:properties:compatible
conditional block to restrict them only to the hardware that actually supports
them?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723-es8316-regulator-next-20260722-v1-0-721f757879a0@thundersoft.com?part=1

[PATCH 2/2] ASoC: codecs: es8316: Add regulator support

[Hongyang Zhao]

ES8316 has separate AVDD, CPVDD, DVDD and PVDD supply inputs.

Request the supplies during I2C probe. Enable them before enabling MCLK
and accessing the device registers, and disable them on probe failure
and component removal.

Leave the supplies enabled during system suspend because the existing
suspend and resume callbacks do not handle a complete power loss.
Powering the codec down would require separate reset and
reinitialization support.

Signed-off-by: Hongyang Zhao <hongyang.zhao@thundersoft.com>
---
 sound/soc/codecs/es8316.c | 35 +++++++++++++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/es8316.c b/sound/soc/codecs/es8316.c
index 3abe77423f29..904056b2569a 100644
--- a/sound/soc/codecs/es8316.c
+++ b/sound/soc/codecs/es8316.c
@@ -14,6 +14,7 @@
 #include <linux/i2c.h>
 #include <linux/mutex.h>
 #include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
@@ -30,10 +31,20 @@ static const unsigned int supported_mclk_lrck_ratios[] = {
 	256, 384, 400, 500, 512, 768, 1024
 };
 
+static const char * const es8316_supply_names[] = {
+	"avdd",
+	"cpvdd",
+	"dvdd",
+	"pvdd",
+};
+
+#define ES8316_NUM_SUPPLIES ARRAY_SIZE(es8316_supply_names)
+
 struct es8316_priv {
 	struct mutex lock;
 	struct clk *mclk;
 	struct regmap *regmap;
+	struct regulator_bulk_data supplies[ES8316_NUM_SUPPLIES];
 	struct snd_soc_component *component;
 	struct snd_soc_jack *jack;
 	int irq;
@@ -767,10 +778,16 @@ static int es8316_probe(struct snd_soc_component *component)
 	if (!es8316->mclk)
 		dev_warn(component->dev, "assuming static mclk\n");
 
+	ret = regulator_bulk_enable(ES8316_NUM_SUPPLIES, es8316->supplies);
+	if (ret) {
+		dev_err(component->dev, "unable to enable supplies\n");
+		return ret;
+	}
+
 	ret = clk_prepare_enable(es8316->mclk);
 	if (ret) {
 		dev_err(component->dev, "unable to enable mclk\n");
-		return ret;
+		goto err_disable_supplies;
 	}
 
 	/* Reset codec and enable current state machine */
@@ -793,6 +810,11 @@ static int es8316_probe(struct snd_soc_component *component)
 	snd_soc_component_write(component, ES8316_CLKMGR_ADCOSR, 0x32);
 
 	return 0;
+
+err_disable_supplies:
+	regulator_bulk_disable(ES8316_NUM_SUPPLIES, es8316->supplies);
+
+	return ret;
 }
 
 static void es8316_remove(struct snd_soc_component *component)
@@ -800,6 +822,7 @@ static void es8316_remove(struct snd_soc_component *component)
 	struct es8316_priv *es8316 = snd_soc_component_get_drvdata(component);
 
 	clk_disable_unprepare(es8316->mclk);
+	regulator_bulk_disable(ES8316_NUM_SUPPLIES, es8316->supplies);
 }
 
 static int es8316_resume(struct snd_soc_component *component)
@@ -862,7 +885,7 @@ static int es8316_i2c_probe(struct i2c_client *i2c_client)
 {
 	struct device *dev = &i2c_client->dev;
 	struct es8316_priv *es8316;
-	int ret;
+	int i, ret;
 
 	es8316 = devm_kzalloc(&i2c_client->dev, sizeof(struct es8316_priv),
 			      GFP_KERNEL);
@@ -871,6 +894,14 @@ static int es8316_i2c_probe(struct i2c_client *i2c_client)
 
 	i2c_set_clientdata(i2c_client, es8316);
 
+	for (i = 0; i < ES8316_NUM_SUPPLIES; i++)
+		es8316->supplies[i].supply = es8316_supply_names[i];
+
+	ret = devm_regulator_bulk_get(dev, ES8316_NUM_SUPPLIES,
+				      es8316->supplies);
+	if (ret)
+		return dev_err_probe(dev, ret, "unable to get supplies\n");
+
 	es8316->regmap = devm_regmap_init_i2c(i2c_client, &es8316_regmap);
 	if (IS_ERR(es8316->regmap))
 		return PTR_ERR(es8316->regmap);

-- 
2.43.0

Re: [PATCH 2/2] ASoC: codecs: es8316: Add regulator support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] The regmap lacks cache-only protection while regulators are disabled, allowing physical I2C access to an unpowered device.
- [High] Regmap cache becomes permanently out-of-sync with hardware after component unbind/rebind due to newly added unhandled power loss.

Pre-existing issues:
- [High] Unchecked return value of regmap_read() leads to usage of an uninitialized stack variable in the IRQ handler.
--

--- Patch [2]: [PATCH 2/2] ASoC: codecs: es8316: Add regulator support ---
commit 8810711bae01652cebd6d01e2ef62b85b877ad30
Author: Hongyang Zhao <hongyang.zhao@thundersoft.com>

ASoC: codecs: es8316: Add regulator support

ES8316 has separate AVDD, CPVDD, DVDD and PVDD supply inputs. Request the
supplies during I2C probe. Enable them before enabling MCLK and accessing
the device registers, and disable them on probe failure and component
removal.

> diff --git a/sound/soc/codecs/es8316.c b/sound/soc/codecs/es8316.c
> index 3abe77423f29b..904056b2569a5 100644
> --- a/sound/soc/codecs/es8316.c
> +++ b/sound/soc/codecs/es8316.c
[ ... ]
> @@ -767,10 +778,16 @@ static int es8316_probe(struct snd_soc_component *component)
>  	if (!es8316->mclk)
>  		dev_warn(component->dev, "assuming static mclk\n");
>  
> +	ret = regulator_bulk_enable(ES8316_NUM_SUPPLIES, es8316->supplies);
> +	if (ret) {
> +		dev_err(component->dev, "unable to enable supplies\n");
> +		return ret;
> +	}
> +

[Severity: High]
Does this leave the regmap cache permanently out-of-sync with the hardware
after a component unbind and rebind?

When the component is unbound in es8316_remove(), regulator_bulk_disable()
cuts power to the hardware, wiping the hardware register state. However, the
regmap cache survives because it is tied to the persistent i2c device. When
power is restored here during a rebind, the hardware resets to defaults, but
because regcache_mark_dirty() is not called, subsequent regmap updates that
match the stale cache will be skipped.

[ ... ]
> @@ -800,6 +822,7 @@ static void es8316_remove(struct snd_soc_component *component)
>  	struct es8316_priv *es8316 = snd_soc_component_get_drvdata(component);
>  
>  	clk_disable_unprepare(es8316->mclk);
> +	regulator_bulk_disable(ES8316_NUM_SUPPLIES, es8316->supplies);
>  }

[Severity: High]
Is the regmap missing cache-only protection while the regulators are
disabled?

The regmap is initialized in es8316_i2c_probe() and persists until the i2c
device is removed. When the hardware is powered down here, can a read via
the regmap debugfs interface attempt physical i2c accesses to the unpowered
device?

[Severity: High]
This isn't a bug introduced by this patch, but does es8316_irq() use an
uninitialized stack variable if the i2c read fails?

sound/soc/codecs/es8316.c:es8316_irq() {
	...
	unsigned int flags;

	mutex_lock(&es8316->lock);

	regmap_read(es8316->regmap, ES8316_GPIO_FLAG, &flags);
	if (flags == 0x00)
		goto out;
	...
}

If a transient i2c bus error causes regmap_read() to fail and return an
error code, flags remains uninitialized. Will the function then evaluate
garbage data from the stack to determine the jack status?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723-es8316-regulator-next-20260722-v1-0-721f757879a0@thundersoft.com?part=2