[PATCH 00/16] ARM: omap3: pandora: Modernize audio and backlight to Device Tree to finally remove pdata-quirks

[PATCH 00/16] ARM: omap3: pandora: Modernize audio and backlight to Device Tree to finally remove pdata-quirks

[H. Nikolaus Schaller]

PATCH V1 2026-07-11 08:02:02:
This series modernizes the OpenPandora handheld support by converting the 
sound subsystem and backlight control to proper platform drivers and Device 
Tree definitions.

The primary goal is the complete removal of omap3_pandora_legacy_init() 
and the elimination of the last remaining user of machine_is_omap3_pandora().

History and Context
===================
The necessity of this cleanup was highlighted in several recent discussions:

1. Tony Lindgren noted that machine_is_omap3_pandora() checks never succeed 
   on Device Tree-booted machines, leaving the audio subsystem functionally 
   broken since the DT conversion years ago:
   https://marc.info/?l=alsa-devel&m=165632351319716&w=2

2. Ethan Nelson-Moore submitted a patch to replace the macro with DT 
   compatibility checks (accepted as commit 45efb8fbdae3 in v7.2-rc1):
   https://marc.info/?l=linux-omap&m=177841578110704&w=2

3. Sascha Hauer pointed out that the entire omap3pandora driver fundamentally 
   depended on this broken machine ID check:
   https://marc.info/?l=linux-omap&m=178048982272252&w=2

Design Choices and Implementation
=================================
Based on these discussions and a long-standing and working out-of-tree solution,
we initially attempted a migration to generic frameworks like simple-audio-card 
or audio-graph-card. However, the hardware routing peculiarities of the 
OpenPandora introduced too many functional gaps. Replacing omap3pandora.c 
entirely with pure DT definitions proved unfeasible. 

Therefore, we opted to base a new proposal on the out-of-tree rewrite of
the old-style sound board file kernel driver (sound/soc/ti/omap3pandora.c)
into a proper platform driver that cleanly fetches its resources from the
Device Tree, while adding specific handling for the twl4030-regen output
and adapting the pandora device tree files.

Series Structure
================
* Patches 01–06 (Preparation): Move highly specific legacy code to its proper 
  infrastructure (TWL4030 regulator regeneration and codec handling).
* Patch 07-08 (New Driver): Introduce a dedicated platform driver for the 
  PCM1773 audio DAC.
* Patches 09-14 (Audio Core): Rewrite the omap3pandora ASoC fabric driver 
  and update the respective Device Tree nodes.
* Patches 14–15 (Backlight): Remove the custom, legacy Pandora backlight driver,
  convert it to generic Device Tree nodes
* Patch 16: (Cleanup) safely delete the obsolete omap3_pandora_legacy_init().

Notes & Testing
===============
* Testing: This series has been tested on real OpenPandora hardware.

Credits
=======
This work is a collaborative effort by multiple authors over several years:
* Grond <grond66@riseup.net>
* Stefan Leichter <sle85276@gmx.de>
* H. Nikolaus Schaller <hns@goldelico.com>


Grond (8):
  dt-bindings: twl-regulator: Add bindings for exposing ti,twl4030-regen
  regulator: twl: Expose the TWL4030's REGEN signal as a regulator
  arm: dts: twl4030: add twl4030-regen child
  arm: dts: omap3pandora: Populate DT data for the TWL4030's REGEN
    regulator
  arm: dts: omap3pandora: Don't use DMA channels for unused SPI masters
  ASoC: twl4030-codec: Allow setting APLL rate through the .set_sysclk()
    interface
  ASoC: pcm1773-codec: write a driver for the PCM1773 chip from TI
  arm: dts: omap3pandora: Add device node for PCM1773 codec

H. Nikolaus Schaller (5):
  ASoC: dt-bindings: add TI PCM1773
  ASoC: dt-bindings: add OpenPandora Sound Card
  arm: dts: omap3-pandora-common: backlight: switch to twl4030 pwm and
    pwm_bl
  backlight: remove pandora_bl
  arm: omap2: remove remaining pdata-quirks for pandora legacy devices

Stefan Leichter (3):
  ASoC: omap3pandora: Rewrite sound card driver as a platform driver
    with DT
  ARM: dts: omap3-pandora-common: Enable audio in/out (mcbsp4/2)
  arm: dts: omap3pandora: create new DT node for the sound card

 .../devicetree/bindings/mfd/ti,twl.yaml       |   1 +
 .../sound/openpandora,omap3pandora-sound.yaml |  94 ++++
 .../devicetree/bindings/sound/pcm1773.yaml    |  32 ++
 .../dts/ti/omap/omap3-pandora-common.dtsi     |  84 ++++
 arch/arm/boot/dts/ti/omap/twl4030.dtsi        |  14 +
 arch/arm/mach-omap2/pdata-quirks.c            |  23 -
 drivers/regulator/twl-regulator.c             |  52 ++
 drivers/video/backlight/Kconfig               |   7 -
 drivers/video/backlight/Makefile              |   1 -
 drivers/video/backlight/pandora_bl.c          | 159 ------
 include/linux/mfd/twl.h                       |   3 +
 include/linux/mfd/twl4030-audio.h             |   3 +
 sound/soc/codecs/Kconfig                      |   5 +
 sound/soc/codecs/pcm1773.c                    | 149 ++++++
 sound/soc/codecs/twl4030.c                    | 140 ++++--
 sound/soc/ti/omap3pandora.c                   | 454 ++++++++++++------
 16 files changed, 855 insertions(+), 366 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/openpandora,omap3pandora-sound.yaml
 create mode 100644 Documentation/devicetree/bindings/sound/pcm1773.yaml
 delete mode 100644 drivers/video/backlight/pandora_bl.c
 create mode 100644 sound/soc/codecs/pcm1773.c

-- 
2.50.1 (Apple Git-155)

[PATCH 01/16] dt-bindings: twl-regulator: Add bindings for exposing ti, twl4030-regen

[H. Nikolaus Schaller]

From: Grond <grond66@riseup.net>

FIXME: not reflected in the bindings

+Reguired properties:
+For twl3040 REGEN signal:
+ - regulator-min-microvolt:
+  - Same meaning as in bindings/regulator/regulator.yaml, but must match
+    regulator-max-microvolt.
+ - regulator-max-microvolt:
+  - Same meaning as in bindings/regulator/regulator.yaml, but must match
+    regulator-min-microvolt.
+ - startup-delay-us:
+  - Same meaning as in bindings/regulator/fixed-regulator.yaml.

Signed-off-by: Grond <grond66@riseup.net>
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 Documentation/devicetree/bindings/mfd/ti,twl.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mfd/ti,twl.yaml b/Documentation/devicetree/bindings/mfd/ti,twl.yaml
index 9cc3e4721612e..183fc015e40ad 100644
--- a/Documentation/devicetree/bindings/mfd/ti,twl.yaml
+++ b/Documentation/devicetree/bindings/mfd/ti,twl.yaml
@@ -46,6 +46,7 @@ allOf:
                 - ti,twl4030-vusb1v5
                 - ti,twl4030-vusb1v8
                 - ti,twl4030-vusb3v1
+                - ti,twl4030-regen
             ti,retain-on-reset: false
 
       properties:
-- 
2.50.1 (Apple Git-155)

[PATCH 02/16] regulator: twl: Expose the TWL4030's REGEN signal as a regulator

[H. Nikolaus Schaller]

From: Grond <grond66@riseup.net>

This is needed on some boards (for example, the Pandora), where REGEN is
used to drive external regulators.

Signed-off-by: Grond <grond66@riseup.net>
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/regulator/twl-regulator.c | 52 +++++++++++++++++++++++++++++++
 include/linux/mfd/twl.h           |  3 ++
 2 files changed, 55 insertions(+)

diff --git a/drivers/regulator/twl-regulator.c b/drivers/regulator/twl-regulator.c
index 5bacfcebf59aa..d883ee8abec32 100644
--- a/drivers/regulator/twl-regulator.c
+++ b/drivers/regulator/twl-regulator.c
@@ -451,6 +451,16 @@ static const struct regulator_ops twl4030fixed_ops = {
 	.get_status	= twl4030reg_get_status,
 };
 
+static const struct regulator_ops twl4030signal_ops = {
+	.enable		= twl4030reg_enable,
+	.disable	= twl4030reg_disable,
+	.is_enabled	= twl4030reg_is_enabled,
+
+	.set_mode	= twl4030reg_set_mode,
+
+	.get_status	= twl4030reg_get_status,
+};
+
 /*----------------------------------------------------------------------*/
 
 #define TWL4030_ADJUSTABLE_LDO(label, offset, num, turnon_delay, remap_conf) \
@@ -511,6 +521,24 @@ static const struct twlreg_info TWLFIXED_INFO_##label = { \
 		}, \
 	}
 
+#define TWL4030_SIGNAL(label, offset, num, remap_conf) \
+static const struct twlreg_info TWLSIGNAL_INFO_##label = { \
+	.base = offset, \
+	.id = num, \
+	.remap = remap_conf, \
+	.desc = { \
+		.name = #label, \
+		.id = TWL4030_REG_##label, \
+		.n_voltages = 1, \
+		.fixed_uV = 0, /* filled in from DT later */ \
+		.ops = &twl4030signal_ops, \
+		.type = REGULATOR_VOLTAGE, \
+		.owner = THIS_MODULE, \
+		.enable_time = 0, /* also filled in from DT */ \
+		.of_map_mode = twl4030reg_map_mode, \
+		}, \
+	}
+
 /*
  * We list regulators here if systems need some level of
  * software control over them after boot.
@@ -536,6 +564,7 @@ TWL4030_FIXED_LDO(VINTDIG, 0x47, 1500, 13, 100, 0x08);
 TWL4030_FIXED_LDO(VUSB1V5, 0x71, 1500, 17, 100, 0x08);
 TWL4030_FIXED_LDO(VUSB1V8, 0x74, 1800, 18, 100, 0x08);
 TWL4030_FIXED_LDO(VUSB3V1, 0x77, 3100, 19, 150, 0x08);
+TWL4030_SIGNAL(REGEN, 0x7f, 21, 0x08);
 
 #define TWL_OF_MATCH(comp, family, label) \
 	{ \
@@ -548,6 +577,7 @@ TWL4030_FIXED_LDO(VUSB3V1, 0x77, 3100, 19, 150, 0x08);
 #define TWL6032_OF_MATCH(comp, label) TWL_OF_MATCH(comp, TWL6032, label)
 #define TWLFIXED_OF_MATCH(comp, label) TWL_OF_MATCH(comp, TWLFIXED, label)
 #define TWLSMPS_OF_MATCH(comp, label) TWL_OF_MATCH(comp, TWLSMPS, label)
+#define TWLSIGNAL_OF_MATCH(comp, label) TWL_OF_MATCH(comp, TWLSIGNAL, label)
 
 static const struct of_device_id twl_of_match[] = {
 	TWL4030_OF_MATCH("ti,twl4030-vaux1", VAUX1),
@@ -570,6 +600,7 @@ static const struct of_device_id twl_of_match[] = {
 	TWLFIXED_OF_MATCH("ti,twl4030-vusb1v5", VUSB1V5),
 	TWLFIXED_OF_MATCH("ti,twl4030-vusb1v8", VUSB1V8),
 	TWLFIXED_OF_MATCH("ti,twl4030-vusb3v1", VUSB3V1),
+	TWLSIGNAL_OF_MATCH("ti,twl4030-regen", REGEN),
 	{},
 };
 MODULE_DEVICE_TABLE(of, twl_of_match);
@@ -583,6 +614,7 @@ static int twlreg_probe(struct platform_device *pdev)
 	struct regulation_constraints	*c;
 	struct regulator_dev		*rdev;
 	struct regulator_config		config = { };
+	int ret;
 
 	template = of_device_get_match_data(&pdev->dev);
 	if (!template)
@@ -615,6 +647,26 @@ static int twlreg_probe(struct platform_device *pdev)
 	case TWL4030_REG_VINTANA2:
 	case TWL4030_REG_VINTDIG:
 		c->always_on = true;
+		break;
+	case TWL4030_REG_REGEN:
+		if (c->min_uV == 0 || c->max_uV == 0) {
+			dev_err(&pdev->dev, "minimum or maximum regulator voltage zero in DT\n");
+			return -EINVAL;
+		}
+
+		if (c->min_uV != c->max_uV) {
+			dev_err(&pdev->dev, "minimum and maximum voltage are different in DT\n");
+			return -EINVAL;
+		}
+		info->desc.fixed_uV = c->min_uV;
+
+		ret = of_property_read_u32(pdev->dev.of_node, "startup-delay-us",
+					   &info->desc.enable_time);
+		if (ret) {
+			dev_err(&pdev->dev, "no valid startup delay set in DT: %d\n", ret);
+			return ret;
+		}
+
 		break;
 	default:
 		break;
diff --git a/include/linux/mfd/twl.h b/include/linux/mfd/twl.h
index b31e07fa4d516..d249b4b22c898 100644
--- a/include/linux/mfd/twl.h
+++ b/include/linux/mfd/twl.h
@@ -738,6 +738,9 @@ int twl4030_sih_setup(struct device *dev, int module, int irq_base);
 #define TWL4030_REG_VUSB1V8	18
 #define TWL4030_REG_VUSB3V1	19
 
+/* digital output signal for enabling external regulators */
+#define TWL4030_REG_REGEN	21
+
 /* TWL6030 SMPS/LDO's */
 /* EXTERNAL dc-to-dc buck convertor controllable via SR */
 #define TWL6030_REG_VDD1	30
-- 
2.50.1 (Apple Git-155)

[PATCH 03/16] arm: dts: twl4030: add twl4030-regen child

[H. Nikolaus Schaller]

From: Grond <grond66@riseup.net>

This is needed on some boards (for example, the Pandora), where REGEN is
used to drive external regulators.

Signed-off-by: Grond <grond66@riseup.net>
---
 arch/arm/boot/dts/ti/omap/twl4030.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm/boot/dts/ti/omap/twl4030.dtsi b/arch/arm/boot/dts/ti/omap/twl4030.dtsi
index 07b9ca942e78d..e6f80b8f0a731 100644
--- a/arch/arm/boot/dts/ti/omap/twl4030.dtsi
+++ b/arch/arm/boot/dts/ti/omap/twl4030.dtsi
@@ -112,6 +112,20 @@ vsim: regulator-vsim {
 		regulator-max-microvolt = <3000000>;
 	};
 
+	regen: regulator-regen {
+		compatible = "ti,twl4030-regen";
+		/*
+		 * Since REGEN is technically just a signal that actuates an
+		 * external regulator, we do not know what voltage it will
+		 * eventually have, or what it's on/off timing should be.
+		 * Therefore, disable it here; boards that use it can override
+		 * the status to "okay" once they have set the
+		 * regulator-{min,max}-microvolt and startup-delay-us
+		 * attributes.
+		 */
+		status = "disabled";
+	};
+
 	twl_gpio: gpio {
 		compatible = "ti,twl4030-gpio";
 		gpio-controller;
-- 
2.50.1 (Apple Git-155)

[PATCH 04/16] arm: dts: omap3pandora: Populate DT data for the TWL4030's REGEN regulator

[H. Nikolaus Schaller]

From: Grond <grond66@riseup.net>

On the Pandora, REGEN is used to enable TPS61029DRC external regulator
which runs a 5V power rail. The 5ms startup delay is taken from Pandora's
3.2 kernel, where it is labeled as a "guess". So it may be possible to
activate the regulator faster.

On Pandora CC units, this regulator is used as the input to the LCD power
supply. Therefore, problems are likely to arise if the regulator is ever
disabled, so we add the regulator-always-on property to compensate.

Signed-off-by: Grond <grond66@riseup.net>
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi b/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi
index 06c5b23589991..a0116823ffdef 100644
--- a/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi
+++ b/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi
@@ -486,6 +486,14 @@ &vsim {
 	regulator-always-on;
 };
 
+&regen {
+	regulator-min-microvolt = <5000000>;
+	regulator-max-microvolt = <5000000>;
+	startup-delay-us = <5000>;
+	regulator-always-on;
+	status = "okay";
+};
+
 &i2c2 {
 	clock-frequency = <100000>;
 	/* no clients so we should disable clock */
-- 
2.50.1 (Apple Git-155)

[PATCH 05/16] arm: dts: omap3pandora: Don't use DMA channels for unused SPI masters

[H. Nikolaus Schaller]

From: Grond <grond66@riseup.net>

On the pandora, only McSPI1 (spi0) is actually used. Because the device
tree defaults in arch/arm/boot/dts/omap3.dtsi leave all of the mcspi
interfaces enabled, the DMA channels which the device tree assigns to them
cannot be used for anything else. This is a problem because OMAP3's sDMA
controller can only have 32 DMA channels configured at any one time.

So when we try to use (for example) the McBSP subsystem (which is required
for sound) it doesn't work because we've already exhausted our available
DMA channels.

Fix this by disabling mcspi[2-4] on the pandora device tree.

Signed-off-by: Grond <grond66@riseup.net>
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 .../boot/dts/ti/omap/omap3-pandora-common.dtsi  | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi b/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi
index a0116823ffdef..b4acab1625212 100644
--- a/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi
+++ b/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi
@@ -693,6 +693,23 @@ lcd_in: endpoint {
 
 };
 
+/*
+ * Only mcspi1 is used on the pandora, the others do not surface on the board
+ * due to pinmux configuration. disable the unused ones so that they do not
+ * consume DMA channels
+ */
+&mcspi2 {
+	status = "disabled";
+};
+
+&mcspi3 {
+	status = "disabled";
+};
+
+&mcspi4 {
+	status = "disabled";
+};
+
 /* n/a - used as GPIOs */
 &mcbsp1 {
 };
-- 
2.50.1 (Apple Git-155)

[PATCH 06/16] ASoC: twl4030-codec: Allow setting APLL rate through the .set_sysclk() interface

[H. Nikolaus Schaller]

From: Grond <grond66@riseup.net>

On the Pandora, the APLL is actually what drives all of the audio clocks on
the board. Since the TWL4030 is only used for capture, we need some way of
setting the correct rate for playback streams, since they will not go
through the TWL4030's .hw_params() code path (which is normally how APLL
rate is set).

Signed-off-by: Grond <grond66@riseup.net>
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 include/linux/mfd/twl4030-audio.h |   3 +
 sound/soc/codecs/twl4030.c        | 140 +++++++++++++++++++++---------
 2 files changed, 101 insertions(+), 42 deletions(-)

diff --git a/include/linux/mfd/twl4030-audio.h b/include/linux/mfd/twl4030-audio.h
index 1c28605dfda80..4378eef97b0fc 100644
--- a/include/linux/mfd/twl4030-audio.h
+++ b/include/linux/mfd/twl4030-audio.h
@@ -244,6 +244,9 @@
 #define TWL4030_VIBRA_SEL		0x10
 #define TWL4030_VIBRA_DIR_SEL		0x20
 
+/* clock ID used for setting APLL rate */
+#define TWL4030_CLOCK_APLL		0xffff
+
 /* TWL4030 codec resource IDs */
 enum twl4030_audio_res {
 	TWL4030_AUDIO_RES_POWER = 0,
diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c
index 9476cdfd4dde9..ff36679166e20 100644
--- a/sound/soc/codecs/twl4030.c
+++ b/sound/soc/codecs/twl4030.c
@@ -198,6 +198,38 @@ static void twl4030_codec_enable(struct snd_soc_component *component, int enable
 	udelay(10);
 }
 
+static int twl4030_perform_writes(struct snd_soc_component *component,
+				  const unsigned int *regs,
+				  const unsigned int *vals,
+				  unsigned int n)
+{
+	struct twl4030_priv *twl4030 = snd_soc_component_get_drvdata(component);
+	int reboot_codec = twl4030->codec_powered;
+	unsigned int i;
+	int ret;
+
+	if (reboot_codec)
+		twl4030_codec_enable(component, 0);
+
+	for (i = 0; i < n; ++i) {
+		ret = twl4030_write(component, regs[i], vals[i]);
+		if (ret)
+			return ret;
+	}
+
+	if (reboot_codec)
+		twl4030_codec_enable(component, 1);
+
+	return 0;
+}
+
+static inline int
+twl4030_perform_single_write(struct snd_soc_component *component,
+			     unsigned int reg, unsigned int val)
+{
+	return twl4030_perform_writes(component, &reg, &val, 1);
+}
+
 static void
 twl4030_get_board_param_values(struct twl4030_board_params *board_params,
 			       struct device_node *node)
@@ -1690,38 +1722,16 @@ static void twl4030_shutdown(struct snd_pcm_substream *substream,
 		twl4030_tdm_enable(component, substream->stream, 0);
 }
 
-static int twl4030_hw_params(struct snd_pcm_substream *substream,
-			     struct snd_pcm_hw_params *params,
-			     struct snd_soc_dai *dai)
+static int twl4030_get_codec_mode_for_apll_rate(struct snd_soc_component *component,
+						unsigned int rate, u8 *res)
 {
-	struct snd_soc_component *component = dai->component;
-	struct twl4030_priv *twl4030 = snd_soc_component_get_drvdata(component);
-	u8 mode, old_mode, format, old_format;
-
-	 /* If the substream has 4 channel, do the necessary setup */
-	if (params_channels(params) == 4) {
-		format = twl4030_read(component, TWL4030_REG_AUDIO_IF);
-		mode = twl4030_read(component, TWL4030_REG_CODEC_MODE);
-
-		/* Safety check: are we in the correct operating mode and
-		 * the interface is in TDM mode? */
-		if ((mode & TWL4030_OPTION_1) &&
-		    ((format & TWL4030_AIF_FORMAT) == TWL4030_AIF_FORMAT_TDM))
-			twl4030_tdm_enable(component, substream->stream, 1);
-		else
-			return -EINVAL;
-	}
-
-	if (twl4030->configured)
-		/* Ignoring hw_params for already configured DAI */
-		return 0;
+	u8 old_mode, mode;
 
-	/* bit rate */
 	old_mode = twl4030_read(component,
 				TWL4030_REG_CODEC_MODE) & ~TWL4030_CODECPDZ;
 	mode = old_mode & ~TWL4030_APLL_RATE;
 
-	switch (params_rate(params)) {
+	switch (rate) {
 	case 8000:
 		mode |= TWL4030_APLL_RATE_8000;
 		break;
@@ -1753,11 +1763,50 @@ static int twl4030_hw_params(struct snd_pcm_substream *substream,
 		mode |= TWL4030_APLL_RATE_96000;
 		break;
 	default:
-		dev_err(component->dev, "%s: unknown rate %d\n", __func__,
-			params_rate(params));
+		dev_err(component->dev, "%s: unknown rate %u\n", __func__,
+			rate);
 		return -EINVAL;
 	}
 
+	*res = mode;
+
+	return mode != old_mode ? 1 : 0;
+}
+
+static int twl4030_hw_params(struct snd_pcm_substream *substream,
+			     struct snd_pcm_hw_params *params,
+			     struct snd_soc_dai *dai)
+{
+	struct snd_soc_component *component = dai->component;
+	struct twl4030_priv *twl4030 = snd_soc_component_get_drvdata(component);
+	u8 mode, format, old_format;
+	int s;
+	unsigned int regs[2], vals[2];
+
+	 /* If the substream has 4 channel, do the necessary setup */
+	if (params_channels(params) == 4) {
+		format = twl4030_read(component, TWL4030_REG_AUDIO_IF);
+		mode = twl4030_read(component, TWL4030_REG_CODEC_MODE);
+
+		/* Safety check: are we in the correct operating mode and
+		 * the interface is in TDM mode?
+		 */
+		if ((mode & TWL4030_OPTION_1) &&
+		    ((format & TWL4030_AIF_FORMAT) == TWL4030_AIF_FORMAT_TDM))
+			twl4030_tdm_enable(component, substream->stream, 1);
+		else
+			return -EINVAL;
+	}
+
+	if (twl4030->configured)
+		/* Ignoring hw_params for already configured DAI */
+		return 0;
+
+	s = twl4030_get_codec_mode_for_apll_rate(component,
+						 params_rate(params), &mode);
+	if (s < 0)
+		return s;
+
 	/* sample size */
 	old_format = twl4030_read(component, TWL4030_REG_AUDIO_IF);
 	format = old_format;
@@ -1775,20 +1824,12 @@ static int twl4030_hw_params(struct snd_pcm_substream *substream,
 		return -EINVAL;
 	}
 
-	if (format != old_format || mode != old_mode) {
-		if (twl4030->codec_powered) {
-			/*
-			 * If the codec is powered, than we need to toggle the
-			 * codec power.
-			 */
-			twl4030_codec_enable(component, 0);
-			twl4030_write(component, TWL4030_REG_CODEC_MODE, mode);
-			twl4030_write(component, TWL4030_REG_AUDIO_IF, format);
-			twl4030_codec_enable(component, 1);
-		} else {
-			twl4030_write(component, TWL4030_REG_CODEC_MODE, mode);
-			twl4030_write(component, TWL4030_REG_AUDIO_IF, format);
-		}
+	if (format != old_format || s) {
+		regs[0] = TWL4030_REG_CODEC_MODE;
+		vals[0] = mode;
+		regs[1] = TWL4030_REG_AUDIO_IF;
+		vals[1] = format;
+		twl4030_perform_writes(component, regs, vals, ARRAY_SIZE(regs));
 	}
 
 	/* Store the important parameters for the DAI configuration and set
@@ -1813,6 +1854,21 @@ static int twl4030_set_dai_sysclk(struct snd_soc_dai *codec_dai, int clk_id,
 {
 	struct snd_soc_component *component = codec_dai->component;
 	struct twl4030_priv *twl4030 = snd_soc_component_get_drvdata(component);
+	int s;
+	u8 mode;
+
+	if (clk_id == TWL4030_CLOCK_APLL) {
+		s = twl4030_get_codec_mode_for_apll_rate(component,
+							 freq, &mode);
+		if (s < 0)
+			return s;
+
+		if (s)
+			return twl4030_perform_single_write(component,
+					TWL4030_REG_CODEC_MODE, mode);
+
+		return 0;
+	}
 
 	switch (freq) {
 	case 19200000:
-- 
2.50.1 (Apple Git-155)

[PATCH 07/16] ASoC: dt-bindings: add TI PCM1773

[H. Nikolaus Schaller]

PCM1771/3 is a simple audio codec that can be enabled through an
enable-gpio.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 .../devicetree/bindings/sound/pcm1773.yaml    | 32 +++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/pcm1773.yaml

diff --git a/Documentation/devicetree/bindings/sound/pcm1773.yaml b/Documentation/devicetree/bindings/sound/pcm1773.yaml
new file mode 100644
index 0000000000000..f3e640705bf70
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/pcm1773.yaml
@@ -0,0 +1,32 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bindings/sound/pcm1773.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments pcm1773 audio codec
+
+maintainers:
+  - to-be-defined@openpandora.org
+
+description: |+
+  PCM1771 is a simple audio codec that can be enabled
+  through a gpio.
+
+properties:
+  compatible:
+    const: ti,pcm1773
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    audio-codec {
+      compatible = "ti,pcm1773";
+      enable-gpios = <&gpio4 22 GPIO_ACTIVE_LOW>;
+    };
+
+...
-- 
2.50.1 (Apple Git-155)

[PATCH 08/16] ASoC: pcm1773-codec: write a driver for the PCM1773 chip from TI

[H. Nikolaus Schaller]

From: Grond <grond66@riseup.net>

This chip is used in the OpenPandora.

Signed-off-by: Grond <grond66@riseup.net>
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 sound/soc/codecs/Kconfig   |   5 ++
 sound/soc/codecs/pcm1773.c | 149 +++++++++++++++++++++++++++++++++++++
 2 files changed, 154 insertions(+)
 create mode 100644 sound/soc/codecs/pcm1773.c

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 252f683be3c18..3e186652fa06d 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -182,6 +182,7 @@ config SND_SOC_ALL_CODECS
 	imply SND_SOC_HDMI_CODEC
 	imply SND_SOC_PCM1681
 	imply SND_SOC_PCM1754
+	imply SND_SOC_PCM1773
 	imply SND_SOC_PCM1789_I2C
 	imply SND_SOC_PCM179X_I2C
 	imply SND_SOC_PCM179X_SPI
@@ -1541,6 +1542,10 @@ config SND_SOC_PCM1754
 	tristate "Texas Instruments PCM1754 CODEC"
 	depends on GPIOLIB
 
+config SND_SOC_PCM1773
+	tristate "Texas Instruments PCM1773 CODEC"
+	select GPIOLIB
+
 config SND_SOC_PCM1789
 	tristate
 
diff --git a/sound/soc/codecs/pcm1773.c b/sound/soc/codecs/pcm1773.c
new file mode 100644
index 0000000000000..75f9fe40a89d5
--- /dev/null
+++ b/sound/soc/codecs/pcm1773.c
@@ -0,0 +1,149 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * pcm1773.c -- codec for the simple PCM1773 output codec from TI
+ *
+ * Shamelessly cobbled together from sound/soc/ti/omap3pandora.c and a few
+ * other codec drivers in sound/soc/codecs/
+ *
+ * Author: Grond <grond66@riseup.net>
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include <sound/soc.h>
+
+struct pcm1773 {
+	struct regulator *regulator;
+	struct gpio_desc *enable_gpio;
+};
+
+static int pcm1773_dac_event(struct snd_soc_dapm_widget *w,
+			     struct snd_kcontrol *k, int event)
+{
+	struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
+	struct pcm1773 *ctx = snd_soc_component_get_drvdata(component);
+	struct device *dev = component->dev;
+	int ret;
+
+	/*
+	 * The PCM1773 DAC datasheet requires 1ms delay between switching
+	 * VCC power on/off and /PD pin high/low
+	 */
+	if (SND_SOC_DAPM_EVENT_ON(event)) {
+		if (ctx->regulator) {
+			ret = regulator_enable(ctx->regulator);
+			if (ret) {
+				dev_err(dev, "Failed to power DAC: %d\n", ret);
+				return ret;
+			}
+			mdelay(1);
+		}
+
+		if (ctx->enable_gpio)
+			gpiod_set_value_cansleep(ctx->enable_gpio, 1);
+	} else {
+		if (ctx->enable_gpio)
+			gpiod_set_value_cansleep(ctx->enable_gpio, 0);
+
+		if (ctx->regulator) {
+			mdelay(1);
+			regulator_disable(ctx->regulator);
+		}
+	}
+
+	return 0;
+}
+
+static const struct snd_soc_dapm_widget pcm1773_dapm_widgets[] = {
+	SND_SOC_DAPM_DAC_E("PCM1773 DAC", "HiFi Playback", SND_SOC_NOPM,
+			   0, 0, pcm1773_dac_event,
+			   SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD),
+};
+
+static const struct snd_soc_dapm_route pcm1773_dapm_routes[] = {
+	/* tell DAPM that the main stream flows to the PCM1773 */
+	{"PCM1773 DAC", NULL, "PCM1773 IN"},
+};
+
+static struct snd_soc_dai_driver pcm1773_dai = {
+	.name = "pcm1773-hifi",
+	.playback = {
+		.stream_name = "PCM1773 IN",
+		.channels_min = 2,
+		.channels_max = 2,
+		.rates = SNDRV_PCM_RATE_8000_48000,
+		// [TODO] these really should be BE, per the data sheet but for
+		// some reason the omap-mcbsp driver claims only to support LE.
+		// investigate
+		.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
+	},
+};
+
+static int pcm1773_probe(struct snd_soc_component *component)
+{
+	struct pcm1773 *ctx = NULL;
+	struct device *dev = component->dev;
+	int ret;
+
+	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+	snd_soc_component_set_drvdata(component, ctx);
+
+	ctx->enable_gpio = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_LOW);
+	if (IS_ERR(ctx->enable_gpio)) {
+		return dev_err_probe(dev, PTR_ERR(ctx->enable_gpio),
+				     "invalid GPIO specification for enable");
+	}
+	if (ctx->enable_gpio)
+		dev_dbg(dev, "got enable-gpio\n");
+	else
+		dev_warn(dev, "enable-gpio not specified\n");
+
+	ctx->regulator = devm_regulator_get(dev, "vcc");
+	if (IS_ERR(ctx->regulator)) {
+		dev_warn(dev, "cannot get regulator 'vcc'");
+		ctx->regulator = NULL;
+	}
+
+	return 0;
+}
+
+static const struct snd_soc_component_driver soc_component_dev_pcm1773 = {
+	.probe = pcm1773_probe,
+	.dapm_widgets = pcm1773_dapm_widgets,
+	.num_dapm_widgets = ARRAY_SIZE(pcm1773_dapm_widgets),
+	.dapm_routes = pcm1773_dapm_routes,
+	.num_dapm_routes = ARRAY_SIZE(pcm1773_dapm_routes),
+};
+
+static int pcm1773_codec_probe(struct platform_device *pdev)
+{
+	return devm_snd_soc_register_component(&pdev->dev,
+		&soc_component_dev_pcm1773,
+		&pcm1773_dai, 1);
+}
+
+static const struct of_device_id pcm1773_of_match[] = {
+	{
+		.compatible = "ti,pcm1773",
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(of, pcm1773_of_match);
+
+static struct platform_driver pcm1773_codec_driver = {
+	.probe = pcm1773_codec_probe,
+	.driver = {
+		.name = "pcm1773-codec",
+		.of_match_table = pcm1773_of_match,
+	},
+};
+
+module_platform_driver(pcm1773_codec_driver);
+
+MODULE_DESCRIPTION("ASoC codec driver PCM1773");
+MODULE_AUTHOR("Grond");
+MODULE_LICENSE("GPL");
-- 
2.50.1 (Apple Git-155)

[PATCH 09/16] ASoC: dt-bindings: add OpenPandora Sound Card

[H. Nikolaus Schaller]

The OpenPandora audio subsystem describes the routing links between the
OMAP3 McBSP interface, the external PCM1773 DAC, and the TWL4030 audio
codec, alongside amplifiers and power supplies.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 .../sound/openpandora,omap3pandora-sound.yaml | 94 +++++++++++++++++++
 1 file changed, 94 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/openpandora,omap3pandora-sound.yaml

diff --git a/Documentation/devicetree/bindings/sound/openpandora,omap3pandora-sound.yaml b/Documentation/devicetree/bindings/sound/openpandora,omap3pandora-sound.yaml
new file mode 100644
index 0000000000000..d3c747e5c58d6
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/openpandora,omap3pandora-sound.yaml
@@ -0,0 +1,94 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org
+$schema: http://devicetree.org
+
+title: OpenPandora OMAP3 Audio Complex
+
+maintainers:
+ - H. Nikolaus Schaller <hns@goldelico.com>
+
+description:
+ The OpenPandora audio subsystem describes the routing links between the
+ OMAP3 McBSP interface, the external PCM1773 DAC, and the TWL4030 audio codec,
+ alongside amplifiers and power supplies.
+
+properties:
+ compatible:
+   const: openpandora,omap3pandora-sound
+
+ status: true
+
+ label:
+   $ref: /schemas/types.yaml#/definitions/string
+   description: User-visible name for this sound card.
+
+ amp-gpios:
+   maxItems: 1
+   description: GPIO pin controlling the amplifier power state.
+
+ amp-supply:
+   description: Regulator supplying power to the amplifier.
+
+ sound-dai:
+   $ref: /schemas/types.yaml#/definitions/phandle-array
+   minItems: 2
+   maxItems: 2
+   description:
+     Phandles to the audio CPU DAI (OMAP3 McBSP) and the codec DAI.
+
+ routing:
+   $ref: /schemas/types.yaml#/definitions/non-unique-string-array
+   description:
+     A list of the connections between audio components.
+     Each entry is a pair of strings, the first being the connection's sink,
+     the second being the connection's source.
+
+ widgets:
+   $ref: /schemas/types.yaml#/definitions/non-unique-string-array
+   description:
+     User-specified audio sound widgets.
+     Each entry is a pair of strings, the first being the widget type
+     (e.g., "Speaker", "Headphone", "Microphone", "Line") and the second being its name.
+
+required:
+ - compatible
+ - sound-dai
+
+additionalProperties: false
+
+examples:
+ - |
+   #include <dt-bindings/gpio/gpio.h>
+
+   sound: sound {
+       compatible = "openpandora,omap3pandora-sound";
+       status = "okay";
+
+       label = "OpenPandora";
+
+       amp-gpios = <&gpio1 14 GPIO_ACTIVE_HIGH>;
+       amp-supply = <&regen>;
+
+       widgets =
+           "Line", "Line Out",
+           "Line", "Line In",
+           "Headphone", "Headphone Jack",
+           "Microphone", "Mic (internal)",
+           "Microphone", "Mic (external)";
+
+       routing =
+           "PCM1773 DAC", "APLL Enable",
+           "Headphone Amplifier", "PCM1773 DAC",
+           "Line Out", "PCM1773 DAC",
+           "Headphone Jack", "Headphone Amplifier",
+           "AUXL", "Line In",
+           "AUXR", "Line In",
+           "MAINMIC", "Mic (internal)",
+           "Mic (internal)", "Mic Bias 1",
+           "SUBMIC", "Mic (external)",
+           "Mic (external)", "Mic Bias 2";
+
+       sound-dai = <&mcbsp2>, <&dac>;
+   };
-- 
2.50.1 (Apple Git-155)

[PATCH 10/16] ASoC: omap3pandora: Rewrite sound card driver as a platform driver with DT

[H. Nikolaus Schaller]

From: Stefan Leichter <sle85276@gmx.de>

This change takes advantage of numerous prior changes and moves a
significant amount of code into the new PCM1773 codec driver (and removes
a hardcoded GPIO and regulator in the process). By using this new driver we
can remove the prior fiction that playback audio goes through the TWL4030.

Previously, the sound card driver worked by checking (through various means)
if it was running on the Pandora, and if so, it would create a new platform
device from nothing to function as the sound card, and rely on some weirdness
to get registered as an ASoC card driver. This has not been the way to do ASoC
card drivers for a while now, so instead we register a platform driver which
matches a device in the Pandora's DT and registers the appropriate ASoC card
driver when found. This also means that the driver will only load when the DT
says that we need it, and we can stop manually loading this thing.

Other significant changes:
- as a side-effect this patch retires the last call to machine_is_omap3_pandora()
- conversion to devm
- separate capture and playback ops
- conversion to dev_err
- use new code for twl4030 to set apll clock
- move widgets and routing to device tree definitions

Signed-off-by: Stefan Leichter <sle85276@gmx.de>
Co-developed-by: Grond <grond66@riseup.net>
Signed-off-by: Grond <grond66@riseup.net>
Co-developed-by: H. Nikolaus Schaller <hns@goldelico.com>
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 sound/soc/ti/omap3pandora.c | 454 +++++++++++++++++++++++++-----------
 1 file changed, 320 insertions(+), 134 deletions(-)

diff --git a/sound/soc/ti/omap3pandora.c b/sound/soc/ti/omap3pandora.c
index 6c9c184cd9d6f..d3509473f5df0 100644
--- a/sound/soc/ti/omap3pandora.c
+++ b/sound/soc/ti/omap3pandora.c
@@ -6,138 +6,279 @@
  */
 
 #include <linux/clk.h>
-#include <linux/platform_device.h>
-#include <linux/gpio/consumer.h>
 #include <linux/delay.h>
-#include <linux/regulator/consumer.h>
+#include <linux/gpio/consumer.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
 
 #include <sound/core.h>
 #include <sound/pcm.h>
 #include <sound/soc.h>
 
+#include <asm/mach-types.h>
+#include <linux/mfd/twl4030-audio.h>
 #include <linux/platform_data/asoc-ti-mcbsp.h>
 
 #include "omap-mcbsp.h"
 
-#define PREFIX "ASoC omap3pandora: "
+struct omap3pandora_sound {
+	struct gpio_desc *amp_gpio;
+	struct regulator *amp_regulator;
+
+	struct snd_pcm_substream *playback_stream;
+	struct snd_pcm_substream *capture_stream;
 
-static struct regulator *omap3pandora_dac_reg;
-static struct gpio_desc *dac_power_gpio;
-static struct gpio_desc *amp_power_gpio;
+	struct mutex sample_rate_lock; // protects all fields after
+	unsigned int sample_rate;
+	int sample_rate_users;
+};
 
-static int omap3pandora_hw_params(struct snd_pcm_substream *substream,
-	struct snd_pcm_hw_params *params)
+static struct snd_soc_dai_link omap3pandora_dai[];
+
+static int omap3pandora_common_hw_params(struct snd_pcm_substream *substream,
+					 struct snd_pcm_hw_params *params)
 {
 	struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
 	struct snd_soc_dai *codec_dai = snd_soc_rtd_to_codec(rtd, 0);
 	struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0);
+	struct device *dev = rtd->dev;
 	int ret;
 
-	/* Set the codec system clock for DAC and ADC */
-	ret = snd_soc_dai_set_sysclk(codec_dai, 0, 26000000,
-					    SND_SOC_CLOCK_IN);
-	if (ret < 0) {
-		pr_err(PREFIX "can't set codec system clock\n");
-		return ret;
-	}
-
 	/* Set McBSP clock to external */
 	ret = snd_soc_dai_set_sysclk(cpu_dai, OMAP_MCBSP_SYSCLK_CLKS_EXT,
 				     256 * params_rate(params),
 				     SND_SOC_CLOCK_IN);
 	if (ret < 0) {
-		pr_err(PREFIX "can't set cpu system clock\n");
+		dev_err(dev, "cannot set McBSP clock to external: %d\n", ret);
 		return ret;
 	}
 
 	ret = snd_soc_dai_set_clkdiv(cpu_dai, OMAP_MCBSP_CLKGDV, 8);
 	if (ret < 0) {
-		pr_err(PREFIX "can't set SRG clock divider\n");
+		dev_err(dev, "cannot set McBSP clock divider: %d\n", ret);
 		return ret;
 	}
 
 	return 0;
 }
 
-static int omap3pandora_dac_event(struct snd_soc_dapm_widget *w,
-	struct snd_kcontrol *k, int event)
+static inline int constrain_sample_rate(struct snd_pcm_substream *substream,
+					unsigned int sample_rate)
 {
+	return snd_pcm_hw_constraint_single(substream->runtime,
+					    SNDRV_PCM_HW_PARAM_RATE,
+					    sample_rate);
+}
+
+static inline void relax_sample_rate(struct snd_pcm_substream *substream)
+{
+	const struct snd_interval default_sample_rate_range = {
+		.min = substream->runtime->hw.rate_min,
+		.max = substream->runtime->hw.rate_max,
+		.openmin = 1,
+		.openmax = 1,
+	};
+
+	*constrs_interval(&substream->runtime->hw_constraints,
+			  SNDRV_PCM_HW_PARAM_RATE) =
+		default_sample_rate_range;
+}
+
+static void release_sample_rate(struct omap3pandora_sound *ctx,
+				struct snd_pcm_substream *substream)
+{
+	mutex_lock(&ctx->sample_rate_lock);
+
+	if (ctx->sample_rate_users > 0)
+		--ctx->sample_rate_users;
+	if (ctx->sample_rate_users == 0)
+		ctx->sample_rate = 0;
+
+	relax_sample_rate(substream);
+
+	mutex_unlock(&ctx->sample_rate_lock);
+}
+
+static int grab_sample_rate(struct omap3pandora_sound *ctx,
+			    struct snd_pcm_substream *substream,
+			    struct snd_pcm_hw_params *params)
+{
+	struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
+	struct device *dev = rtd->dev;
+	int ret;
+
+	ret = mutex_lock_interruptible(&ctx->sample_rate_lock);
+	if (ret)
+		return ret;
+
+	if (++ctx->sample_rate_users == 1)
+		ctx->sample_rate = params_rate(params);
+
+	mutex_unlock(&ctx->sample_rate_lock);
+
+	ret = constrain_sample_rate(substream, ctx->sample_rate);
+	if (ret < 0)
+		goto err;
+
+	ret = constrain_sample_rate(substream, params_rate(params));
+	if (ret < 0) {
+		dev_dbg(dev, "sample rate mismatch; playback/capture share same clock");
+		goto err;
+	}
+
+	return 0;
+
+err:
+	release_sample_rate(ctx, substream);
+
+	return ret;
+}
+
+static int omap3pandora_playback_hw_params(struct snd_pcm_substream *substream,
+					   struct snd_pcm_hw_params *params)
+{
+	struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
+	struct snd_soc_card *card = rtd->card;
+	struct omap3pandora_sound *ctx = snd_soc_card_get_drvdata(card);
+	struct device *dev = rtd->dev;
+	struct snd_soc_pcm_runtime *tmp_rtd;
+	struct snd_soc_pcm_runtime *twl4030_rtd = NULL;
 	int ret;
 
 	/*
-	 * The PCM1773 DAC datasheet requires 1ms delay between switching
-	 * VCC power on/off and /PD pin high/low
+	 * We need to set the APLL clock rate on the TWL4030 because it feeds
+	 * both the DAI of the PCM1773. So find the appropriate RTD and call
+	 * the TWL's .set_sysclk callback through there. Ugly, but it must be
+	 * done.
 	 */
-	if (SND_SOC_DAPM_EVENT_ON(event)) {
-		struct device *dev = snd_soc_dapm_to_dev(w->dapm);
 
-		ret = regulator_enable(omap3pandora_dac_reg);
-		if (ret) {
-			dev_err(dev, "Failed to power DAC: %d\n", ret);
-			return ret;
+	for_each_card_rtds(card, tmp_rtd)
+		if (tmp_rtd->dai_link == &omap3pandora_dai[1]) {
+			twl4030_rtd = tmp_rtd;
+			break;
 		}
-		mdelay(1);
-		gpiod_set_value(dac_power_gpio, 1);
-	} else {
-		gpiod_set_value(dac_power_gpio, 0);
-		mdelay(1);
-		regulator_disable(omap3pandora_dac_reg);
+	if (!twl4030_rtd) {
+		dev_err(dev, "cannot find TWL4030 runtime data to set APLL rate\n");
+		return -EINVAL;
 	}
 
-	return 0;
+	ret = snd_soc_dai_set_sysclk(snd_soc_rtd_to_codec(twl4030_rtd, 0),
+				     TWL4030_CLOCK_APLL, params_rate(params),
+				     SND_SOC_CLOCK_OUT);
+	if (ret) {
+		dev_err(dev, "cannot set TWL4030 APLL rate via set_sysclk interface: %d\n",
+			ret);
+		return ret;
+	}
+
+	if (!ctx->playback_stream) {
+		ctx->playback_stream = substream;
+		ret = grab_sample_rate(ctx, substream, params);
+		if (ret)
+			return ret;
+	}
+
+	return omap3pandora_common_hw_params(substream, params);
 }
 
-static int omap3pandora_hp_event(struct snd_soc_dapm_widget *w,
-	struct snd_kcontrol *k, int event)
+static int omap3pandora_capture_hw_params(struct snd_pcm_substream *substream,
+					  struct snd_pcm_hw_params *params)
+{
+	struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
+	struct snd_soc_dai *codec_dai = snd_soc_rtd_to_codec(rtd, 0);
+	struct device *dev = rtd->dev;
+	struct snd_soc_card *card = rtd->card;
+	struct omap3pandora_sound *ctx = snd_soc_card_get_drvdata(card);
+	int ret;
+
+	/* Set the codec system clock for DAC and ADC */
+	ret = snd_soc_dai_set_sysclk(codec_dai, 0, 26000000,
+				     SND_SOC_CLOCK_IN);
+	if (ret < 0) {
+		dev_err(dev, "cannot set TWL4030 system clock: %d\n", ret);
+		return ret;
+	}
+
+	if (!ctx->capture_stream) {
+		ctx->capture_stream = substream;
+		ret = grab_sample_rate(ctx, substream, params);
+		if (ret)
+			return ret;
+	}
+
+	return omap3pandora_common_hw_params(substream, params);
+}
+
+static int omap3pandora_playback_hw_free(struct snd_pcm_substream *substream)
 {
-	if (SND_SOC_DAPM_EVENT_ON(event))
-		gpiod_set_value(amp_power_gpio, 1);
-	else
-		gpiod_set_value(amp_power_gpio, 0);
+	struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
+	struct snd_soc_card *card = rtd->card;
+	struct omap3pandora_sound *ctx = snd_soc_card_get_drvdata(card);
+
+	if (ctx->playback_stream)
+		release_sample_rate(ctx, substream);
+	ctx->playback_stream = NULL;
 
 	return 0;
 }
 
-/*
- * Audio paths on Pandora board:
- *
- *  |O| ---> PCM DAC +-> AMP -> Headphone Jack
- *  |M|         A    +--------> Line Out
- *  |A| <~~clk~~+
- *  |P| <--- TWL4030 <--------- Line In and MICs
- */
-static const struct snd_soc_dapm_widget omap3pandora_dapm_widgets[] = {
-	SND_SOC_DAPM_DAC_E("PCM DAC", "HiFi Playback", SND_SOC_NOPM,
-			   0, 0, omap3pandora_dac_event,
-			   SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD),
-	SND_SOC_DAPM_PGA_E("Headphone Amplifier", SND_SOC_NOPM,
-			   0, 0, NULL, 0, omap3pandora_hp_event,
-			   SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD),
-	SND_SOC_DAPM_HP("Headphone Jack", NULL),
-	SND_SOC_DAPM_LINE("Line Out", NULL),
+static int omap3pandora_common_startup(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
+	struct snd_soc_card *card = rtd->card;
+	struct omap3pandora_sound *ctx = snd_soc_card_get_drvdata(card);
+	unsigned int sample_rate = ctx->sample_rate;
 
-	SND_SOC_DAPM_MIC("Mic (internal)", NULL),
-	SND_SOC_DAPM_MIC("Mic (external)", NULL),
-	SND_SOC_DAPM_LINE("Line In", NULL),
-};
+	if (!sample_rate)
+		return 0;
 
-static const struct snd_soc_dapm_route omap3pandora_map[] = {
-	{"PCM DAC", NULL, "APLL Enable"},
-	{"Headphone Amplifier", NULL, "PCM DAC"},
-	{"Line Out", NULL, "PCM DAC"},
-	{"Headphone Jack", NULL, "Headphone Amplifier"},
+	return constrain_sample_rate(substream, sample_rate);
+}
 
-	{"AUXL", NULL, "Line In"},
-	{"AUXR", NULL, "Line In"},
+static int omap3pandora_capture_hw_free(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
+	struct snd_soc_card *card = rtd->card;
+	struct omap3pandora_sound *ctx = snd_soc_card_get_drvdata(card);
 
-	{"MAINMIC", NULL, "Mic (internal)"},
-	{"Mic (internal)", NULL, "Mic Bias 1"},
+	if (ctx->capture_stream)
+		release_sample_rate(ctx, substream);
+	ctx->capture_stream = NULL;
 
-	{"SUBMIC", NULL, "Mic (external)"},
-	{"Mic (external)", NULL, "Mic Bias 2"},
-};
+	return 0;
+}
+
+static int omap3pandora_hp_event(struct snd_soc_dapm_widget *w,
+				 struct snd_kcontrol *k, int event)
+{
+	struct snd_soc_card *card = snd_soc_dapm_to_card(w->dapm);
+	struct device *dev = card->dev;
+	struct omap3pandora_sound *ctx =
+		snd_soc_card_get_drvdata(card);
+	int ret;
+
+	if (SND_SOC_DAPM_EVENT_ON(event)) {
+		ret = regulator_enable(ctx->amp_regulator);
+		if (ret) {
+			dev_err(dev, "error enabling amplifier regulator: %d\n", ret);
+			return ret;
+		}
+
+		gpiod_set_value(ctx->amp_gpio, 1);
+	} else {
+		gpiod_set_value(ctx->amp_gpio, 0);
+
+		ret = regulator_disable(ctx->amp_regulator);
+		if (ret) {
+			dev_err(dev, "error disabling amplifier regulator: %d\n", ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
 
 static int omap3pandora_out_init(struct snd_soc_pcm_runtime *rtd)
 {
@@ -171,20 +312,40 @@ static int omap3pandora_in_init(struct snd_soc_pcm_runtime *rtd)
 	return 0;
 }
 
-static const struct snd_soc_ops omap3pandora_ops = {
-	.hw_params = omap3pandora_hw_params,
+static const struct snd_soc_ops omap3pandora_playback_ops = {
+	.startup = omap3pandora_common_startup,
+	.hw_params = omap3pandora_playback_hw_params,
+	.hw_free = omap3pandora_playback_hw_free,
+};
+
+static const struct snd_soc_ops omap3pandora_capture_ops = {
+	.startup = omap3pandora_common_startup,
+	.hw_params = omap3pandora_capture_hw_params,
+	.hw_free = omap3pandora_capture_hw_free,
 };
 
 /* Digital audio interface glue - connects codec <--> CPU */
+#if IS_BUILTIN(CONFIG_SND_SOC_OMAP3_PANDORA)
 SND_SOC_DAILINK_DEFS(out,
 	DAILINK_COMP_ARRAY(COMP_CPU("omap-mcbsp.2")),
-	DAILINK_COMP_ARRAY(COMP_CODEC("twl4030-codec", "twl4030-hifi")),
+	DAILINK_COMP_ARRAY(COMP_CODEC("pcm1773-codec", "pcm1773-hifi")),
 	DAILINK_COMP_ARRAY(COMP_PLATFORM("omap-mcbsp.2")));
 
 SND_SOC_DAILINK_DEFS(in,
 	DAILINK_COMP_ARRAY(COMP_CPU("omap-mcbsp.4")),
 	DAILINK_COMP_ARRAY(COMP_CODEC("twl4030-codec", "twl4030-hifi")),
 	DAILINK_COMP_ARRAY(COMP_PLATFORM("omap-mcbsp.4")));
+#else /* IS_BUILTIN(CONFIG_SND_SOC_OMAP3_PANDORA) */
+SND_SOC_DAILINK_DEFS(out,
+	DAILINK_COMP_ARRAY(COMP_CPU("49022000.mcbsp")),
+	DAILINK_COMP_ARRAY(COMP_CODEC("pcm1773-codec", "pcm1773-hifi")),
+	DAILINK_COMP_ARRAY(COMP_PLATFORM("49022000.mcbsp")));
+
+SND_SOC_DAILINK_DEFS(in,
+	DAILINK_COMP_ARRAY(COMP_CPU("49026000.mcbsp")),
+	DAILINK_COMP_ARRAY(COMP_CODEC("twl4030-codec", "twl4030-hifi")),
+	DAILINK_COMP_ARRAY(COMP_PLATFORM("49026000.mcbsp")));
+#endif /* IS_BUILTIN(CONFIG_SND_SOC_OMAP3_PANDORA) */
 
 static struct snd_soc_dai_link omap3pandora_dai[] = {
 	{
@@ -192,100 +353,125 @@ static struct snd_soc_dai_link omap3pandora_dai[] = {
 		.stream_name = "HiFi Out",
 		.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
 			   SND_SOC_DAIFMT_CBC_CFC,
-		.ops = &omap3pandora_ops,
+		.ops = &omap3pandora_playback_ops,
 		.init = omap3pandora_out_init,
+		.playback_only = 1,
 		SND_SOC_DAILINK_REG(out),
 	}, {
 		.name = "TWL4030",
 		.stream_name = "Line/Mic In",
 		.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
 			   SND_SOC_DAIFMT_CBC_CFC,
-		.ops = &omap3pandora_ops,
+		.ops = &omap3pandora_capture_ops,
 		.init = omap3pandora_in_init,
+		.capture_only = 1,
 		SND_SOC_DAILINK_REG(in),
 	}
 };
 
+static const struct snd_soc_dapm_widget omap3pandora_local_widgets[] = {
+	SND_SOC_DAPM_PGA_E("Headphone Amplifier", SND_SOC_NOPM,
+			   0, 0, NULL, 0, omap3pandora_hp_event,
+			   SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD),
+};
+
 /* SoC card */
 static struct snd_soc_card snd_soc_card_omap3pandora = {
-	.name = "omap3pandora",
-	.owner = THIS_MODULE,
-	.dai_link = omap3pandora_dai,
-	.num_links = ARRAY_SIZE(omap3pandora_dai),
-
-	.dapm_widgets = omap3pandora_dapm_widgets,
-	.num_dapm_widgets = ARRAY_SIZE(omap3pandora_dapm_widgets),
-	.dapm_routes = omap3pandora_map,
-	.num_dapm_routes = ARRAY_SIZE(omap3pandora_map),
+	.name        = "omap3pandora",
+	.owner       = THIS_MODULE,
+	.dai_link    = omap3pandora_dai,
+	.num_links   = ARRAY_SIZE(omap3pandora_dai),
+	.dapm_widgets     = omap3pandora_local_widgets,
+	.num_dapm_widgets = ARRAY_SIZE(omap3pandora_local_widgets),
 };
 
-static struct platform_device *omap3pandora_snd_device;
-
-static int __init omap3pandora_soc_init(void)
+static int omap3pandora_probe(struct platform_device *pdev)
 {
 	int ret;
+	struct device *dev = &pdev->dev;
+	struct snd_soc_card *card = &snd_soc_card_omap3pandora;
+	struct omap3pandora_sound *ctx;
 
-	if (!of_machine_is_compatible("openpandora,omap3-pandora-600mhz") &&
-		!of_machine_is_compatible("openpandora,omap3-pandora-1ghz"))
-		return -ENODEV;
-
-	pr_info("OMAP3 Pandora SoC init\n");
-
-	omap3pandora_snd_device = platform_device_alloc("soc-audio", -1);
-	if (omap3pandora_snd_device == NULL) {
-		pr_err(PREFIX "Platform device allocation failed\n");
+	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
 		return -ENOMEM;
-	}
 
-	platform_set_drvdata(omap3pandora_snd_device, &snd_soc_card_omap3pandora);
+	mutex_init(&ctx->sample_rate_lock);
 
-	ret = platform_device_add(omap3pandora_snd_device);
-	if (ret) {
-		pr_err(PREFIX "Unable to add platform device\n");
-		goto fail2;
+	ctx->amp_gpio = devm_gpiod_get(dev, "amp", GPIOD_OUT_LOW);
+	if (IS_ERR(ctx->amp_gpio)) {
+		dev_err(dev, "cannot find amplifier gpio\n");
+		return PTR_ERR(ctx->amp_gpio);
 	}
 
-	dac_power_gpio = devm_gpiod_get(&omap3pandora_snd_device->dev,
-					"dac", GPIOD_OUT_LOW);
-	if (IS_ERR(dac_power_gpio)) {
-		ret = PTR_ERR(dac_power_gpio);
-		goto fail3;
+	ctx->amp_regulator = devm_regulator_get(dev, "amp");
+	if (IS_ERR(ctx->amp_regulator)) {
+		ret = PTR_ERR(ctx->amp_regulator);
+		dev_err(dev, "Failed to request regulator for amplifier power: %d\n", ret);
+		return ret;
 	}
 
-	amp_power_gpio = devm_gpiod_get(&omap3pandora_snd_device->dev,
-					"amp", GPIOD_OUT_LOW);
-	if (IS_ERR(amp_power_gpio)) {
-		ret = PTR_ERR(amp_power_gpio);
-		goto fail3;
+	card->dev = dev;
+
+	if (dev->of_node) {
+		ret = snd_soc_of_parse_card_name(card, "label");
+		if (ret)
+			card->name = "omap3pandora";
+
+		ret = snd_soc_of_parse_audio_simple_widgets(card, "widgets");
+		if (ret) {
+			dev_err(dev, "Failed to parse DAPM widgets: %d\n", ret);
+			goto err_mutex;
+		}
+
+		ret = snd_soc_of_parse_audio_routing(card, "routing");
+		if (ret) {
+			dev_err(dev, "Failed to parse audio routing: %d\n", ret);
+			goto err_mutex;
+		}
 	}
 
-	omap3pandora_dac_reg = regulator_get(&omap3pandora_snd_device->dev, "vcc");
-	if (IS_ERR(omap3pandora_dac_reg)) {
-		pr_err(PREFIX "Failed to get DAC regulator from %s: %ld\n",
-			dev_name(&omap3pandora_snd_device->dev),
-			PTR_ERR(omap3pandora_dac_reg));
-		ret = PTR_ERR(omap3pandora_dac_reg);
-		goto fail3;
+	snd_soc_card_set_drvdata(card, ctx);
+
+	ret = devm_snd_soc_register_card(dev, card);
+	if (ret) {
+		dev_err(dev, "Failed to register sound card: %d\n", ret);
+		goto err_mutex;
 	}
 
 	return 0;
 
-fail3:
-	platform_device_del(omap3pandora_snd_device);
-fail2:
-	platform_device_put(omap3pandora_snd_device);
-
+err_mutex:
+	mutex_destroy(&ctx->sample_rate_lock);
 	return ret;
 }
-module_init(omap3pandora_soc_init);
 
-static void __exit omap3pandora_soc_exit(void)
+static void omap3pandora_remove(struct platform_device *pdev)
 {
-	regulator_put(omap3pandora_dac_reg);
-	platform_device_unregister(omap3pandora_snd_device);
+	struct snd_soc_card *card = platform_get_drvdata(pdev);
+	struct omap3pandora_sound *ctx = snd_soc_card_get_drvdata(card);
+
+	mutex_destroy(&ctx->sample_rate_lock);
 }
-module_exit(omap3pandora_soc_exit);
+
+static const struct of_device_id omap3pandora_of_match[] = {
+	{ .compatible = "openpandora,omap3pandora-sound", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, omap3pandora_of_match);
+
+static struct platform_driver omap3pandora_driver = {
+	.driver = {
+		.name = "omap3pandora-sound",
+		.of_match_table = omap3pandora_of_match,
+	},
+	.probe = omap3pandora_probe,
+	.remove = omap3pandora_remove,
+};
+
+module_platform_driver(omap3pandora_driver);
 
 MODULE_AUTHOR("Grazvydas Ignotas <notasas@gmail.com>");
 MODULE_DESCRIPTION("ALSA SoC OMAP3 Pandora");
 MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:omap3pandora-sound");
-- 
2.50.1 (Apple Git-155)

[PATCH 11/16] ARM: dts: omap3-pandora-common: Enable audio in/out (mcbsp4/2)

[H. Nikolaus Schaller]

From: Stefan Leichter <sle85276@gmx.de>

enable mcbsp2 and mcbsp4.

Signed-off-by: Stefan Leichter <sle85276@gmx.de>
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi b/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi
index b4acab1625212..5907b2455f75b 100644
--- a/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi
+++ b/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi
@@ -716,6 +716,7 @@ &mcbsp1 {
 
 /* audio DAC */
 &mcbsp2 {
+	status = "okay";
 };
 
 /* bluetooth */
@@ -724,6 +725,7 @@ &mcbsp3 {
 
 /* to twl4030*/
 &mcbsp4 {
+	status = "okay";
 };
 
 &venc {
-- 
2.50.1 (Apple Git-155)

[PATCH 12/16] arm: dts: omap3pandora: Add device node for PCM1773 codec

[H. Nikolaus Schaller]

From: Grond <grond66@riseup.net>

This will ensure that the new PCM1773 driver gets loaded automatically.

Signed-off-by: Grond <grond66@riseup.net>
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi b/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi
index 5907b2455f75b..de9860526b323 100644
--- a/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi
+++ b/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi
@@ -42,6 +42,13 @@ tv_connector_in: endpoint {
 		};
 	};
 
+	dac: pcm1773-codec {
+		compatible = "ti,pcm1773";
+		vcc-supply = <&vsim>;
+		enable-gpio = <&gpio4 22 GPIO_ACTIVE_HIGH>; /* GPIO 118 */
+		status = "okay";
+	};
+
 	gpio-leds {
 
 		compatible = "gpio-leds";
-- 
2.50.1 (Apple Git-155)

[PATCH 13/16] arm: dts: omap3pandora: create new DT node for the sound card

[H. Nikolaus Schaller]

From: Stefan Leichter <sle85276@gmx.de>

This means that the sound card driver (snd-soc-omap3pandora) will get
loaded by default.

To make this work, we also add dac pcm1773-codec and widgets and
routing as needed.

Signed-off-by: Stefan Leichter <sle85276@gmx.de>
Signed-off-by: Grond <grond66@riseup.net>
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 .../dts/ti/omap/omap3-pandora-common.dtsi     | 42 +++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi b/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi
index de9860526b323..13821d926afb0 100644
--- a/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi
+++ b/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi
@@ -47,6 +47,45 @@ dac: pcm1773-codec {
 		vcc-supply = <&vsim>;
 		enable-gpio = <&gpio4 22 GPIO_ACTIVE_HIGH>; /* GPIO 118 */
 		status = "okay";
+		#sound-dai-cells = <0>;
+	};
+
+	twl_audio_clk: twl-audio-clock {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <12288000>;
+		clock-output-names = "clk256fs";
+	};
+
+	sound: sound {
+		compatible = "openpandora,omap3pandora-sound";
+		status = "okay";
+
+		label = "OpenPandora";
+
+		amp-gpios = <&gpio1 14 GPIO_ACTIVE_HIGH>;
+		amp-supply = <&regen>;
+
+		widgets =
+			"Line", "Line Out",
+			"Line", "Line In",
+			"Headphone", "Headphone Jack",
+			"Microphone", "Mic (internal)",
+			"Microphone", "Mic (external)";
+
+		routing =
+			"PCM1773 DAC", "APLL Enable",
+			"Headphone Amplifier", "PCM1773 DAC",
+			"Line Out", "PCM1773 DAC",
+			"Headphone Jack", "Headphone Amplifier",
+			"AUXL", "Line In",
+			"AUXR", "Line In",
+			"MAINMIC", "Mic (internal)",
+			"Mic (internal)", "Mic Bias 1",
+			"SUBMIC", "Mic (external)",
+			"Mic (external)", "Mic Bias 2";
+
+		sound-dai = <&mcbsp2>, <&dac>;
 	};
 
 	gpio-leds {
@@ -724,6 +763,9 @@ &mcbsp1 {
 /* audio DAC */
 &mcbsp2 {
 	status = "okay";
+	#sound-dai-cells = <0>;
+	clocks = <&mcbsp2_fck>, <&mcbsp2_ick>;
+	clock-names = "fck", "ick";
 };
 
 /* bluetooth */
-- 
2.50.1 (Apple Git-155)

[PATCH 14/16] arm: dts: omap3-pandora-common: backlight: switch to twl4030 pwm and pwm_bl

[H. Nikolaus Schaller]

This allows to remove the pandora_bl driver and pdata-quirks.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi b/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi
index 13821d926afb0..fea7132ce7c5e 100644
--- a/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi
+++ b/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi
@@ -31,6 +31,14 @@ hfclk_26m: oscillator {
 		clock-frequency = <26000000>;
 	};
 
+	backlight {
+		compatible = "pwm-backlight";
+		pwms = <&twl_pwm 0 1922710>;
+
+		brightness-levels = <0 150 158 166 174 185 205 230 255>;
+		default-brightness-level = <6>;
+	};
+
 	tv: connector {
 		compatible = "connector-analog-tv";
 		label = "tv";
-- 
2.50.1 (Apple Git-155)

[PATCH 15/16] backlight: remove pandora_bl

[H. Nikolaus Schaller]

Retire the platform specific pandora backlight driver since
it can now be replaced by twl_pm and device tree.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/video/backlight/Kconfig      |   7 --
 drivers/video/backlight/Makefile     |   1 -
 drivers/video/backlight/pandora_bl.c | 159 ---------------------------
 3 files changed, 167 deletions(-)
 delete mode 100644 drivers/video/backlight/pandora_bl.c

diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 7aa1c4b21111f..a250b3edc152c 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -451,13 +451,6 @@ config BACKLIGHT_MP3309C
 	  To compile this driver as a module, choose M here: the module will
 	  be called mp3309c.
 
-config BACKLIGHT_PANDORA
-	tristate "Backlight driver for Pandora console"
-	depends on TWL4030_CORE
-	help
-	  If you have a Pandora console, say Y to enable the
-	  backlight driver.
-
 config BACKLIGHT_SKY81452
 	tristate "Backlight driver for SKY81452"
 	depends on MFD_SKY81452
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index 21c8313cfb121..f97ab8ba85807 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -52,7 +52,6 @@ obj-$(CONFIG_BACKLIGHT_MAX8925)		+= max8925_bl.o
 obj-$(CONFIG_BACKLIGHT_MP3309C)		+= mp3309c.o
 obj-$(CONFIG_BACKLIGHT_MT6370)		+= mt6370-backlight.o
 obj-$(CONFIG_BACKLIGHT_OMAP1)		+= omap1_bl.o
-obj-$(CONFIG_BACKLIGHT_PANDORA)		+= pandora_bl.o
 obj-$(CONFIG_BACKLIGHT_PWM)		+= pwm_bl.o
 obj-$(CONFIG_BACKLIGHT_QCOM_WLED)	+= qcom-wled.o
 obj-$(CONFIG_BACKLIGHT_RT4831)		+= rt4831-backlight.o
diff --git a/drivers/video/backlight/pandora_bl.c b/drivers/video/backlight/pandora_bl.c
deleted file mode 100644
index 8a63ded0fa90f..0000000000000
--- a/drivers/video/backlight/pandora_bl.c
+++ /dev/null
@@ -1,159 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * Backlight driver for Pandora handheld.
- * Pandora uses TWL4030 PWM0 -> TPS61161 combo for control backlight.
- * Based on pwm_bl.c
- *
- * Copyright 2009,2012 Gražvydas Ignotas <notasas@gmail.com>
- */
-
-#include <linux/module.h>
-#include <linux/kernel.h>
-#include <linux/platform_device.h>
-#include <linux/delay.h>
-#include <linux/backlight.h>
-#include <linux/mfd/twl.h>
-#include <linux/err.h>
-
-#define TWL_PWM0_ON		0x00
-#define TWL_PWM0_OFF		0x01
-
-#define TWL_INTBR_GPBR1		0x0c
-#define TWL_INTBR_PMBR1		0x0d
-
-#define TWL_PMBR1_PWM0_MUXMASK	0x0c
-#define TWL_PMBR1_PWM0		0x04
-#define PWM0_CLK_ENABLE		BIT(0)
-#define PWM0_ENABLE		BIT(2)
-
-/* range accepted by hardware */
-#define MIN_VALUE 9
-#define MAX_VALUE 63
-#define MAX_USER_VALUE (MAX_VALUE - MIN_VALUE)
-
-struct pandora_private {
-	unsigned old_state;
-#define PANDORABL_WAS_OFF 1
-};
-
-static int pandora_backlight_update_status(struct backlight_device *bl)
-{
-	int brightness = bl->props.brightness;
-	struct pandora_private *priv = bl_get_data(bl);
-	u8 r;
-
-	if (bl->props.power != BACKLIGHT_POWER_ON)
-		brightness = 0;
-	if (bl->props.state & BL_CORE_FBBLANK)
-		brightness = 0;
-	if (bl->props.state & BL_CORE_SUSPENDED)
-		brightness = 0;
-
-	if ((unsigned int)brightness > MAX_USER_VALUE)
-		brightness = MAX_USER_VALUE;
-
-	if (brightness == 0) {
-		if (priv->old_state == PANDORABL_WAS_OFF)
-			goto done;
-
-		/* first disable PWM0 output, then clock */
-		twl_i2c_read_u8(TWL4030_MODULE_INTBR, &r, TWL_INTBR_GPBR1);
-		r &= ~PWM0_ENABLE;
-		twl_i2c_write_u8(TWL4030_MODULE_INTBR, r, TWL_INTBR_GPBR1);
-		r &= ~PWM0_CLK_ENABLE;
-		twl_i2c_write_u8(TWL4030_MODULE_INTBR, r, TWL_INTBR_GPBR1);
-
-		goto done;
-	}
-
-	if (priv->old_state == PANDORABL_WAS_OFF) {
-		/*
-		 * set PWM duty cycle to max. TPS61161 seems to use this
-		 * to calibrate it's PWM sensitivity when it starts.
-		 */
-		twl_i2c_write_u8(TWL_MODULE_PWM, MAX_VALUE, TWL_PWM0_OFF);
-
-		/* first enable clock, then PWM0 out */
-		twl_i2c_read_u8(TWL4030_MODULE_INTBR, &r, TWL_INTBR_GPBR1);
-		r &= ~PWM0_ENABLE;
-		r |= PWM0_CLK_ENABLE;
-		twl_i2c_write_u8(TWL4030_MODULE_INTBR, r, TWL_INTBR_GPBR1);
-		r |= PWM0_ENABLE;
-		twl_i2c_write_u8(TWL4030_MODULE_INTBR, r, TWL_INTBR_GPBR1);
-
-		/*
-		 * TI made it very easy to enable digital control, so easy that
-		 * it often triggers unintentionally and disabes PWM control,
-		 * so wait until 1 wire mode detection window ends.
-		 */
-		usleep_range(2000, 10000);
-	}
-
-	twl_i2c_write_u8(TWL_MODULE_PWM, MIN_VALUE + brightness, TWL_PWM0_OFF);
-
-done:
-	if (brightness != 0)
-		priv->old_state = 0;
-	else
-		priv->old_state = PANDORABL_WAS_OFF;
-
-	return 0;
-}
-
-static const struct backlight_ops pandora_backlight_ops = {
-	.options	= BL_CORE_SUSPENDRESUME,
-	.update_status	= pandora_backlight_update_status,
-};
-
-static int pandora_backlight_probe(struct platform_device *pdev)
-{
-	struct backlight_properties props;
-	struct backlight_device *bl;
-	struct pandora_private *priv;
-	u8 r;
-
-	priv = devm_kmalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
-	if (!priv)
-		return -ENOMEM;
-
-	memset(&props, 0, sizeof(props));
-	props.max_brightness = MAX_USER_VALUE;
-	props.type = BACKLIGHT_RAW;
-	bl = devm_backlight_device_register(&pdev->dev, pdev->name, &pdev->dev,
-					priv, &pandora_backlight_ops, &props);
-	if (IS_ERR(bl)) {
-		dev_err(&pdev->dev, "failed to register backlight\n");
-		return PTR_ERR(bl);
-	}
-
-	platform_set_drvdata(pdev, bl);
-
-	/* 64 cycle period, ON position 0 */
-	twl_i2c_write_u8(TWL_MODULE_PWM, 0x80, TWL_PWM0_ON);
-
-	priv->old_state = PANDORABL_WAS_OFF;
-	bl->props.brightness = MAX_USER_VALUE;
-	backlight_update_status(bl);
-
-	/* enable PWM function in pin mux */
-	twl_i2c_read_u8(TWL4030_MODULE_INTBR, &r, TWL_INTBR_PMBR1);
-	r &= ~TWL_PMBR1_PWM0_MUXMASK;
-	r |= TWL_PMBR1_PWM0;
-	twl_i2c_write_u8(TWL4030_MODULE_INTBR, r, TWL_INTBR_PMBR1);
-
-	return 0;
-}
-
-static struct platform_driver pandora_backlight_driver = {
-	.driver		= {
-		.name	= "pandora-backlight",
-	},
-	.probe		= pandora_backlight_probe,
-};
-
-module_platform_driver(pandora_backlight_driver);
-
-MODULE_AUTHOR("Gražvydas Ignotas <notasas@gmail.com>");
-MODULE_DESCRIPTION("Pandora Backlight Driver");
-MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:pandora-backlight");
-- 
2.50.1 (Apple Git-155)

[PATCH 16/16] arm: omap2: remove remaining pdata-quirks for pandora legacy devices

[H. Nikolaus Schaller]

After updating the pandora-backligt setup and removing the omap3pandora
sound system and defining related gpios by device tree, we can remove
omap3_pandora_legacy_init() and the related legacy devices completely.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 arch/arm/mach-omap2/pdata-quirks.c | 23 -----------------------
 1 file changed, 23 deletions(-)

diff --git a/arch/arm/mach-omap2/pdata-quirks.c b/arch/arm/mach-omap2/pdata-quirks.c
index b947bacf23a37..aca7097a692ed 100644
--- a/arch/arm/mach-omap2/pdata-quirks.c
+++ b/arch/arm/mach-omap2/pdata-quirks.c
@@ -268,27 +268,6 @@ static void __init omap3_logicpd_torpedo_init(void)
 	omap3_gpio126_127_129();
 }
 
-/* omap3pandora legacy devices */
-
-static struct platform_device pandora_backlight = {
-	.name	= "pandora-backlight",
-	.id	= -1,
-};
-
-static struct gpiod_lookup_table pandora_soc_audio_gpios = {
-	.dev_id = "soc-audio",
-	.table = {
-		GPIO_LOOKUP("gpio-112-127", 6, "dac", GPIO_ACTIVE_HIGH),
-		GPIO_LOOKUP("gpio-0-15", 14, "amp", GPIO_ACTIVE_HIGH),
-		{ }
-	},
-};
-
-static void __init omap3_pandora_legacy_init(void)
-{
-	platform_device_register(&pandora_backlight);
-	gpiod_add_lookup_table(&pandora_soc_audio_gpios);
-}
 #endif /* CONFIG_ARCH_OMAP3 */
 
 #ifdef CONFIG_SOC_DRA7XX
@@ -514,8 +493,6 @@ static struct pdata_init pdata_quirks[] __initdata = {
 	{ "ti,omap3-evm-37xx", omap3_evm_legacy_init, },
 	{ "ti,am3517-evm", am3517_evm_legacy_init, },
 	{ "technexion,omap3-tao3530", omap3_tao3530_legacy_init, },
-	{ "openpandora,omap3-pandora-600mhz", omap3_pandora_legacy_init, },
-	{ "openpandora,omap3-pandora-1ghz", omap3_pandora_legacy_init, },
 #endif
 	{ /* sentinel */ },
 };
-- 
2.50.1 (Apple Git-155)

Re: [PATCH 05/16] arm: dts: omap3pandora: Don't use DMA channels for unused SPI masters

[sashiko-bot]

> From: Grond <grond66@riseup.net>
> 
> On the pandora, only McSPI1 (spi0) is actually used. Because the device
> tree defaults in arch/arm/boot/dts/omap3.dtsi leave all of the mcspi
> interfaces enabled, the DMA channels which the device tree assigns to them
> cannot be used for anything else. This is a problem because OMAP3's sDMA
> controller can only have 32 DMA channels configured at any one time.
> 
> So when we try to use (for example) the McBSP subsystem (which is required
> for sound) it doesn't work because we've already exhausted our available
> DMA channels.
> 
> Fix this by disabling mcspi[2-4] on the pandora device tree.
> 
> Signed-off-by: Grond <grond66@riseup.net>
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1783749722.git.hns@goldelico.com?part=5

Re: [PATCH 07/16] ASoC: dt-bindings: add TI PCM1773

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] Incorrect `$id` path with extra `bindings/` directory component.
- [Low] Undocumented property `enable-gpios` is used in the example but missing from the `properties` block.
--

--- Patch [7]: [PATCH 07/16] ASoC: dt-bindings: add TI PCM1773 ---
commit 743fe66d5b39db7483986e7887f8d34c7081d440
Author: H. Nikolaus Schaller <hns@goldelico.com>

ASoC: dt-bindings: add TI PCM1773

This commit adds a new device tree binding for the Texas Instruments PCM1771
and PCM1773 simple audio codecs. It documents the compatible string and shows
basic usage.

> diff --git a/Documentation/devicetree/bindings/sound/pcm1773.yaml b/Documentation/devicetree/bindings/sound/pcm1773.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/pcm1773.yaml
> @@ -0,0 +1,32 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/bindings/sound/pcm1773.yaml#

[Severity: Low]
Does this $id path match the exact file path relative to
Documentation/devicetree/bindings/? It looks like there is an extra
bindings/ directory component here which might cause schema cross-reference
resolution to fail or dt_binding_check to skip validation.

[ ... ]

> +properties:
> +  compatible:
> +    const: ti,pcm1773
> +
> +required:
> +  - compatible
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    audio-codec {
> +      compatible = "ti,pcm1773";
> +      enable-gpios = <&gpio4 22 GPIO_ACTIVE_LOW>;

[Severity: Low]
Will this example fail dt_binding_check validation? The schema uses
additionalProperties: false, but enable-gpios is omitted from the properties
block above, which will cause the strict schema validation to reject this
example and any actual device tree nodes using the property.

> +    };
> +
> +...

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1783749722.git.hns@goldelico.com?part=7

Re: [PATCH 09/16] ASoC: dt-bindings: add OpenPandora Sound Card

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `$id` and `$schema` fields in the YAML schema contain invalid URIs, breaking schema validation.
- [Low] The `status` property is explicitly and unnecessarily defined in the binding schema.
--

--- Patch [9]: [PATCH 09/16] ASoC: dt-bindings: add OpenPandora Sound Card ---
commit 94d05b63d0a0b038d3a5355e6b58cc3b78b60eeb
Author: H. Nikolaus Schaller <hns@goldelico.com>

ASoC: dt-bindings: add OpenPandora Sound Card

The OpenPandora audio subsystem describes the routing links between the
OMAP3 McBSP interface, the external PCM1773 DAC, and the TWL4030 audio
codec, alongside amplifiers and power supplies.

> diff --git a/Documentation/devicetree/bindings/sound/openpandora,omap3pandora-sound.yaml b/Documentation/devicetree/bindings/sound/openpandora,omap3pandora-sound.yaml
> new file mode 100644
> index 0000000000000..d3c747e5c58d6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/openpandora,omap3pandora-sound.yaml
> @@ -0,0 +1,94 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org
> +$schema: http://devicetree.org

[Severity: Medium]
Is the $id field missing the full path? As per the device tree binding
guidelines, an incorrect $id field breaks the schema cross-reference system
and causes dt_binding_check to silently skip validation. It typically needs
to begin with http://devicetree.org/schemas/ and exactly match the file path.

Additionally, shouldn't $schema be set to the standard meta-schema
(http://devicetree.org/meta-schemas/core.yaml#) instead of the base URL?

> +
> +title: OpenPandora OMAP3 Audio Complex
> +
> +maintainers:
> + - H. Nikolaus Schaller <hns@goldelico.com>
> +
> +description:
> + The OpenPandora audio subsystem describes the routing links between the
> + OMAP3 McBSP interface, the external PCM1773 DAC, and the TWL4030 audio codec,
> + alongside amplifiers and power supplies.
> +
> +properties:
> + compatible:
> +   const: openpandora,omap3pandora-sound
> +
> + status: true

[Severity: Low]
This isn't a bug, but is it necessary to explicitly define the status
property here? Since status is a core standard device tree property, it is
implicitly handled by the core dt-schema. Redefining it here without
constraints is redundant.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1783749722.git.hns@goldelico.com?part=9

Re: [PATCH 01/16] dt-bindings: twl-regulator: Add bindings for exposing ti, twl4030-regen

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The binding schema fails to enforce the required properties for the `ti,twl4030-regen` compatible string and rejects the `startup-delay-us` property.
- [Low] The commit message contains misspellings and an incorrect hardware model number.
--

--- Patch [1]: [PATCH 01/16] dt-bindings: twl-regulator: Add bindings for exposing ti,twl4030-regen ---
commit 47a2b827603c26c697c09f74aba57400724a478c
Author: Grond <grond66@riseup.net>

dt-bindings: twl-regulator: Add bindings for exposing ti,twl4030-regen

This commit adds the ti,twl4030-regen compatible string to the bindings. It
also updates the commit message to document its newly required regulator
properties.

>     +Reguired properties:
>     +For twl3040 REGEN signal:

[Severity: Low]
This isn't a bug, but should "Reguired" be "Required" and "twl3040" be
"twl4030"?

> diff --git a/Documentation/devicetree/bindings/mfd/ti,twl.yaml b/Documentation/devicetree/bindings/mfd/ti,twl.yaml
> index 9cc3e4721612e..183fc015e40ad 100644
> --- a/Documentation/devicetree/bindings/mfd/ti,twl.yaml
> +++ b/Documentation/devicetree/bindings/mfd/ti,twl.yaml
> @@ -46,6 +46,7 @@ allOf:
>                  - ti,twl4030-vusb1v5
>                  - ti,twl4030-vusb1v8
>                  - ti,twl4030-vusb3v1
> +                - ti,twl4030-regen
>              ti,retain-on-reset: false
>  
>        properties:

[Severity: Medium]
Will this schema allow validation to pass without the properties described in
the commit message?

Since there is no corresponding if-then block added to enforce
regulator-min-microvolt and regulator-max-microvolt, they won't be required
for ti,twl4030-regen.

Also, does this cause validation to fail when startup-delay-us is used? 

Because startup-delay-us is not defined in the base regulator schema and
unevaluatedProperties is set to false for the regulator pattern block, any
device tree attempting to use startup-delay-us as instructed will likely be
rejected during dt_binding_check and dtbs_check.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1783749722.git.hns@goldelico.com?part=1

Re: [PATCH 06/16] ASoC: twl4030-codec: Allow setting APLL rate through the .set_sysclk() interface

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing cleanup on write failure in `twl4030_perform_writes()` leaves the codec permanently disabled.
- [Medium] `twl4030_hw_params()` completely ignores the return value of batch register writes, masking hardware configuration failures.
--

--- Patch [6]: [PATCH 06/16] ASoC: twl4030-codec: Allow setting APLL rate through the .set_sysclk() interface ---
commit db9973d614eb8b37cb4d8a6cf9cada8e9b6d1cf8
Author: Grond <grond66@riseup.net>
Subject: ASoC: twl4030-codec: Allow setting APLL rate through the .set_sysclk() interface

This patch allows setting the APLL rate through the .set_sysclk() interface
for devices where the APLL drives audio clocks, but playback bypasses the
standard .hw_params() path. It extracts the APLL rate calculation into a
helper and introduces batch register write functions.

> diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c
> index 9476cdfd4dde9..ff36679166e20 100644
> --- a/sound/soc/codecs/twl4030.c
> +++ b/sound/soc/codecs/twl4030.c
> @@ -198,6 +198,38 @@ static void twl4030_codec_enable(struct snd_soc_component *component, int enable
>  	udelay(10);
>  }
>  
> +static int twl4030_perform_writes(struct snd_soc_component *component,
> +				  const unsigned int *regs,
> +				  const unsigned int *vals,
> +				  unsigned int n)
> +{
> +	struct twl4030_priv *twl4030 = snd_soc_component_get_drvdata(component);
> +	int reboot_codec = twl4030->codec_powered;
> +	unsigned int i;
> +	int ret;
> +
> +	if (reboot_codec)
> +		twl4030_codec_enable(component, 0);
> +
> +	for (i = 0; i < n; ++i) {
> +		ret = twl4030_write(component, regs[i], vals[i]);
> +		if (ret)
> +			return ret;

[Severity: High]
If twl4030_write() fails and returns early, will it leave the codec permanently
disabled because we skip the twl4030_codec_enable(component, 1) call? This
seems to cause a hardware state mismatch where the ALSA core considers the
codec powered on, but it remains physically powered off.

> +	}
> +
> +	if (reboot_codec)
> +		twl4030_codec_enable(component, 1);
> +
> +	return 0;
> +}

[ ... ]

> @@ -1775,20 +1824,12 @@ static int twl4030_hw_params(struct snd_pcm_substream *substream,
>  		return -EINVAL;
>  	}
>  
> -	if (format != old_format || mode != old_mode) {
> -		if (twl4030->codec_powered) {
> -			/*
> -			 * If the codec is powered, than we need to toggle the
> -			 * codec power.
> -			 */
> -			twl4030_codec_enable(component, 0);
> -			twl4030_write(component, TWL4030_REG_CODEC_MODE, mode);
> -			twl4030_write(component, TWL4030_REG_AUDIO_IF, format);
> -			twl4030_codec_enable(component, 1);
> -		} else {
> -			twl4030_write(component, TWL4030_REG_CODEC_MODE, mode);
> -			twl4030_write(component, TWL4030_REG_AUDIO_IF, format);
> -		}
> +	if (format != old_format || s) {
> +		regs[0] = TWL4030_REG_CODEC_MODE;
> +		vals[0] = mode;
> +		regs[1] = TWL4030_REG_AUDIO_IF;
> +		vals[1] = format;
> +		twl4030_perform_writes(component, regs, vals, ARRAY_SIZE(regs));

[Severity: Medium]
Does this code ignore the return value of twl4030_perform_writes()?
If the batch register writes fail, it seems the hardware remains unconfigured
but the function will still return 0 (success) and falsely advance the stream
state.

>  	}
>  
>  	/* Store the important parameters for the DAI configuration and set

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1783749722.git.hns@goldelico.com?part=6

Re: [PATCH 16/16] arm: omap2: remove remaining pdata-quirks for pandora legacy devices

[sashiko-bot]

> After updating the pandora-backligt setup and removing the omap3pandora
> sound system and defining related gpios by device tree, we can remove
> omap3_pandora_legacy_init() and the related legacy devices completely.
> 
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1783749722.git.hns@goldelico.com?part=16

Re: [PATCH 08/16] ASoC: pcm1773-codec: write a driver for the PCM1773 chip from TI

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Device-managed allocations (devm_*) are incorrectly placed in the ASoC component probe, causing memory leaks and -EBUSY errors on card unbind/rebind.
- [High] The driver silently ignores errors from devm_regulator_get(), breaking deferred probing.
- [High] The codec driver defines a DAC widget but lacks physical output endpoints (SND_SOC_DAPM_OUTPUT).
- [Medium] The driver uses mdelay() inside a sleepable DAPM event handler, needlessly wasting CPU cycles.
- [Low] SND_SOC_PCM1773 uses `select GPIOLIB` instead of `depends on GPIOLIB`, violating cross-config consistency.
--

--- Patch [8]: [PATCH 08/16] ASoC: pcm1773-codec: write a driver for the PCM1773 chip from TI ---
commit 834434a73863a6e3a112c74fe0cc88b2cf342a3d
Author: Grond <grond66@riseup.net>

ASoC: pcm1773-codec: write a driver for the PCM1773 chip from TI

This commit adds a new ASoC codec driver for the Texas Instruments PCM1773
chip, which is used in the OpenPandora.

> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> index 252f683be3c18..3e186652fa06d 100644
> --- a/sound/soc/codecs/Kconfig
> +++ b/sound/soc/codecs/Kconfig
[ ... ]
> @@ -1541,6 +1542,10 @@ config SND_SOC_PCM1754
>  	tristate "Texas Instruments PCM1754 CODEC"
>  	depends on GPIOLIB
>  
> +config SND_SOC_PCM1773
> +	tristate "Texas Instruments PCM1773 CODEC"
> +	select GPIOLIB

[Severity: Low]
Does this config violate cross-config consistency? 

The adjacent SND_SOC_PCM1754 config uses depends on GPIOLIB, while this 
one uses select GPIOLIB.

> diff --git a/sound/soc/codecs/pcm1773.c b/sound/soc/codecs/pcm1773.c
> new file mode 100644
> index 0000000000000..75f9fe40a89d5
> --- /dev/null
> +++ b/sound/soc/codecs/pcm1773.c
[ ... ]
> +static int pcm1773_dac_event(struct snd_soc_dapm_widget *w,
> +			     struct snd_kcontrol *k, int event)
> +{
[ ... ]
> +	if (SND_SOC_DAPM_EVENT_ON(event)) {
> +		if (ctx->regulator) {
> +			ret = regulator_enable(ctx->regulator);
> +			if (ret) {
> +				dev_err(dev, "Failed to power DAC: %d\n", ret);
> +				return ret;
> +			}
> +			mdelay(1);

[Severity: Medium]
Since this DAPM event handler is called in process context where sleeping is
allowed (as seen by the use of gpiod_set_value_cansleep below), should this
use a sleeping delay instead of a busy-wait loop to avoid wasting CPU cycles?

> +		}
> +
> +		if (ctx->enable_gpio)
> +			gpiod_set_value_cansleep(ctx->enable_gpio, 1);
> +	} else {
> +		if (ctx->enable_gpio)
> +			gpiod_set_value_cansleep(ctx->enable_gpio, 0);
> +
> +		if (ctx->regulator) {
> +			mdelay(1);

[Severity: Medium]
Could this also be a sleeping delay instead of wasting CPU cycles?

> +			regulator_disable(ctx->regulator);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct snd_soc_dapm_widget pcm1773_dapm_widgets[] = {
> +	SND_SOC_DAPM_DAC_E("PCM1773 DAC", "HiFi Playback", SND_SOC_NOPM,
> +			   0, 0, pcm1773_dac_event,
> +			   SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD),
> +};

[Severity: High]
Are physical output endpoints missing here?

The DAPM widgets define the DAC, but there are no corresponding physical
sink endpoints (like SND_SOC_DAPM_OUTPUT) provided to terminate the
playback path. Without a path to a sink endpoint, the DAPM graph remains
incomplete and the DAC will not be powered on during playback.

[ ... ]
> +static int pcm1773_probe(struct snd_soc_component *component)
> +{
> +	struct pcm1773 *ctx = NULL;
> +	struct device *dev = component->dev;
> +	int ret;
> +
> +	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);

[Severity: High]
Will placing device-managed allocations inside the ASoC component probe
cause memory leaks and failures on rebind?

When a sound card unbinds and rebinds (such as during deferred probing of a
machine driver), pcm1773_probe() is called multiple times. Because these
allocations are tied to the underlying platform device dev rather than the
component lifetime, the memory won't be freed until the platform device
itself is removed.

> +	if (!ctx)
> +		return -ENOMEM;
> +	snd_soc_component_set_drvdata(component, ctx);
> +
> +	ctx->enable_gpio = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_LOW);

[Severity: High]
Does requesting the GPIO here also fail on card rebind?

Since devm_gpiod_get_optional is tied to the platform device, an unbind and
rebind of the sound card will attempt to request the same GPIO again before
the platform device is removed, resulting in a permanent -EBUSY error.

> +	if (IS_ERR(ctx->enable_gpio)) {
> +		return dev_err_probe(dev, PTR_ERR(ctx->enable_gpio),
> +				     "invalid GPIO specification for enable");
> +	}
> +	if (ctx->enable_gpio)
> +		dev_dbg(dev, "got enable-gpio\n");
> +	else
> +		dev_warn(dev, "enable-gpio not specified\n");
> +
> +	ctx->regulator = devm_regulator_get(dev, "vcc");
> +	if (IS_ERR(ctx->regulator)) {
> +		dev_warn(dev, "cannot get regulator 'vcc'");
> +		ctx->regulator = NULL;
> +	}

[Severity: High]
Does this silently break deferred probing?

If devm_regulator_get returns -EPROBE_DEFER (because the regulator driver
probes after this codec), the error is swallowed, ctx->regulator is set to
NULL, and the function returns 0. The driver will successfully probe
without its regulator, resulting in silent audio failures since the DAC
won't be powered.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1783749722.git.hns@goldelico.com?part=8

Re: [PATCH 14/16] arm: dts: omap3-pandora-common: backlight: switch to twl4030 pwm and pwm_bl

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing TPS61161 hardware initialization quirk causes backlight to fail.
- [High] Concurrent probing of legacy and generic backlight drivers causes I2C register corruption.
- [Medium] Hardware misconfiguration due to unsupported PWM period, leading to artificially restricted brightness range.
--

--- Patch [14]: [PATCH 14/16] arm: dts: omap3-pandora-common: backlight: switch to twl4030 pwm and pwm_bl ---
commit 062b6b894a7997d211a245d15a6bd9071869adad
Author: H. Nikolaus Schaller <hns@goldelico.com>

arm: dts: omap3-pandora-common: backlight: switch to twl4030 pwm and pwm_bl

This commit adds a generic pwm-backlight device tree node for the Pandora
backlight. The commit message states this allows removing the pandora_bl
driver and pdata-quirks.

> diff --git a/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi b/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi
> index 13821d926afb0..fea7132ce7c5e 100644
> --- a/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi
> +++ b/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi
> @@ -31,6 +31,14 @@ hfclk_26m: oscillator {
>  		clock-frequency = <26000000>;
>  	};
>  
> +	backlight {
> +		compatible = "pwm-backlight";

[Severity: High]
Will this result in a hardware conflict if the legacy driver isn't actually
removed in this patch?

The commit message mentions this allows removing the legacy pandora_bl driver
and pdata-quirks.c registration, but this commit doesn't remove or
conditionally disable the legacy platform device registration. 

If CONFIG_BACKLIGHT_PANDORA is enabled, both the legacy pandora_bl driver
and the new pwm-backlight driver will probe concurrently. Since pandora_bl
writes directly to I2C registers and bypasses the PWM framework, this dual
driver setup can cause race conditions, register corruption, and duplicate
sysfs entries for the backlight.

[Severity: High]
Does switching to the generic driver cause the backlight to fail on startup?

The legacy pandora_bl driver included a TPS61161 hardware quirk that forced
a 100% duty cycle for 2-10ms at startup. Without this continuous >2.5ms high
pulse, the TPS61161 enters 1-wire detection mode and disables PWM control. 

The generic pwm-backlight driver immediately applies the computed duty cycle
on power-up, which lacks this required hardware initialization sequence.

> +		pwms = <&twl_pwm 0 1922710>;
> +
> +		brightness-levels = <0 150 158 166 174 185 205 230 255>;

[Severity: Medium]
Is this artificially restricted minimum brightness of 150/255 (59%) a
workaround for an unsupported PWM period?

The device tree requests a period of 1922710ns (64-tick mode), but the
pwm-twl driver unconditionally sets TWL_PWM0_ON to 1, forcing 128-tick mode
(~3.9ms period). 

To prevent the longer off-times of this slower frequency from exceeding the
2.5ms threshold that triggers the TPS61161 1-wire mode, it appears the
minimum brightness was raised to 59%. This seems to be a significant
functional regression from the original 14% minimum brightness.

> +		default-brightness-level = <6>;
> +	};
> +
>  	tv: connector {
>  		compatible = "connector-analog-tv";
>  		label = "tv";

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1783749722.git.hns@goldelico.com?part=14

Re: [PATCH 10/16] ASoC: omap3pandora: Rewrite sound card driver as a platform driver with DT

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [Critical] Device Tree ABI breakage due to removal of fallback machine-compatible matching and strict requirement for new DT nodes/properties.
- [High] Double-decrement of sample rate user count when `grab_sample_rate()` fails.
- [High] `relax_sample_rate()` incorrectly excludes hardware's minimum and maximum sample rates and permanently erases constraints applied by other components.
- [High] Static global `snd_soc_card` structure reuse causes list corruption on multiple probes and dangling pointer Use-After-Free.
- [High] Calling `mutex_destroy()` synchronously in `.remove` while using devm-based sound card registration creates a Use-After-Free vulnerability.
- [Medium] Missing `mutex_destroy()` on early probe error paths.
- [Low] Unused variable `codec_dai` in `omap3pandora_common_hw_params`.
--

--- Patch [10]: [PATCH 10/16] ASoC: omap3pandora: Rewrite sound card driver as a platform driver with DT ---
commit 8781d4ef4d1eb0b2da4ee910133603e631f2f4a9
Author: Stefan Leichter <sle85276@gmx.de>

ASoC: omap3pandora: Rewrite sound card driver as a platform driver with DT

This change converts the omap3pandora sound card driver to a standard
platform driver matched via Device Tree. It also moves code into the
PCM1773 codec driver and splits capture/playback operations.

> diff --git a/sound/soc/ti/omap3pandora.c b/sound/soc/ti/omap3pandora.c
> index 6c9c184cd9d6f..d3509473f5df0 100644
> --- a/sound/soc/ti/omap3pandora.c
> +++ b/sound/soc/ti/omap3pandora.c

[ ... ]

> -static int omap3pandora_hw_params(struct snd_pcm_substream *substream,
> -	struct snd_pcm_hw_params *params)
> +static int omap3pandora_common_hw_params(struct snd_pcm_substream *substream,
> +					 struct snd_pcm_hw_params *params)
>  {
>  	struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
>  	struct snd_soc_dai *codec_dai = snd_soc_rtd_to_codec(rtd, 0);

[Severity: Low]
Is codec_dai used anywhere in this function now? It looks like the usage
was moved to omap3pandora_capture_hw_params(), leaving this variable unused
and potentially causing build warnings.

[ ... ]

> +static inline void relax_sample_rate(struct snd_pcm_substream *substream)
> +{
> +	const struct snd_interval default_sample_rate_range = {
> +		.min = substream->runtime->hw.rate_min,
> +		.max = substream->runtime->hw.rate_max,
> +		.openmin = 1,
> +		.openmax = 1,
> +	};
> +
> +	*constrs_interval(&substream->runtime->hw_constraints,
> +			  SNDRV_PCM_HW_PARAM_RATE) =
> +		default_sample_rate_range;
> +}

[Severity: High]
Does setting openmin and openmax to 1 permanently exclude the hardware's
exact minimum and maximum supported rates? It seems this would block rates
like exactly 8000 Hz or 96000 Hz from being selected, as these flags make
the interval mathematically open.

Also, does overwriting the hw_constraints interval directly via
pointer assignment permanently erase other constraints that might have
been applied by the CPU DAI or Codec DAI?

[ ... ]

> +static int omap3pandora_playback_hw_params(struct snd_pcm_substream *substream,
> +					   struct snd_pcm_hw_params *params)
> +{

[ ... ]

> +	if (!ctx->playback_stream) {
> +		ctx->playback_stream = substream;
> +		ret = grab_sample_rate(ctx, substream, params);
> +		if (ret)
> +			return ret;
> +	}

[Severity: High]
Could this lead to a double-decrement of the sample rate user count?

If grab_sample_rate() fails due to a sample rate mismatch, it will call
release_sample_rate() and decrement the count. However, since
ctx->playback_stream is assigned before grab_sample_rate(), the ALSA core's
subsequent call to omap3pandora_playback_hw_free() will see
ctx->playback_stream as non-NULL and call release_sample_rate() a second
time.

The same pattern appears in omap3pandora_capture_hw_params(). Should the
stream pointer assignment happen after grab_sample_rate() succeeds, or
should it be cleared in the error path?

[ ... ]

>  /* SoC card */
>  static struct snd_soc_card snd_soc_card_omap3pandora = {
> -	.name = "omap3pandora",
> -	.owner = THIS_MODULE,
> -	.dai_link = omap3pandora_dai,
> -	.num_links = ARRAY_SIZE(omap3pandora_dai),
> -
> -	.dapm_widgets = omap3pandora_dapm_widgets,
> -	.num_dapm_widgets = ARRAY_SIZE(omap3pandora_dapm_widgets),
> -	.dapm_routes = omap3pandora_map,
> -	.num_dapm_routes = ARRAY_SIZE(omap3pandora_map),
> +	.name        = "omap3pandora",
> +	.owner       = THIS_MODULE,
> +	.dai_link    = omap3pandora_dai,
> +	.num_links   = ARRAY_SIZE(omap3pandora_dai),
> +	.dapm_widgets     = omap3pandora_local_widgets,
> +	.num_dapm_widgets = ARRAY_SIZE(omap3pandora_local_widgets),
>  };
>  
> -static struct platform_device *omap3pandora_snd_device;
> -
> -static int __init omap3pandora_soc_init(void)
> +static int omap3pandora_probe(struct platform_device *pdev)
>  {
>  	int ret;
> +	struct device *dev = &pdev->dev;
> +	struct snd_soc_card *card = &snd_soc_card_omap3pandora;
> +	struct omap3pandora_sound *ctx;
>  
> +	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> +	if (!ctx)
>  		return -ENOMEM;
>  
> +	mutex_init(&ctx->sample_rate_lock);
>  
> +	ctx->amp_gpio = devm_gpiod_get(dev, "amp", GPIOD_OUT_LOW);
> +	if (IS_ERR(ctx->amp_gpio)) {
> +		dev_err(dev, "cannot find amplifier gpio\n");
> +		return PTR_ERR(ctx->amp_gpio);
>  	}
>  
> +	ctx->amp_regulator = devm_regulator_get(dev, "amp");
> +	if (IS_ERR(ctx->amp_regulator)) {
> +		ret = PTR_ERR(ctx->amp_regulator);
> +		dev_err(dev, "Failed to request regulator for amplifier power: %d\n", ret);
> +		return ret;
>  	}

[Severity: Medium]
Do these early error returns leak the lockdep tracking for the mutex?

Since mutex_init() is called before these checks, shouldn't they use goto
err_mutex instead of returning directly?

[ ... ]

> +	if (dev->of_node) {
> +		ret = snd_soc_of_parse_card_name(card, "label");
> +		if (ret)
> +			card->name = "omap3pandora";
> +
> +		ret = snd_soc_of_parse_audio_simple_widgets(card, "widgets");

[Severity: High]
Does modifying the static global snd_soc_card_omap3pandora structure
in probe() create issues if the driver is unbound and rebound? 

Functions like snd_soc_of_parse_audio_simple_widgets() allocate memory
with devm_kcalloc() and assign it to the card structure. If unbound, devres
frees the memory, leaving dangling pointers in the static structure.
Furthermore, probing multiple times could corrupt the global linked list
by calling list_add() on the same card->list node twice.

> +		if (ret) {
> +			dev_err(dev, "Failed to parse DAPM widgets: %d\n", ret);
> +			goto err_mutex;
> +		}
> +
> +		ret = snd_soc_of_parse_audio_routing(card, "routing");
> +		if (ret) {
> +			dev_err(dev, "Failed to parse audio routing: %d\n", ret);
> +			goto err_mutex;
> +		}
>  	}

[Severity: Critical]
Does this strict property check break compatibility with older Device Trees?

The previous driver automatically instantiated the sound card based on the
root machine compatible string. By unconditionally requiring the new widgets
and routing properties and failing if they are absent, will users booting a
new kernel with an older DT lose audio functionality completely?

> +	snd_soc_card_set_drvdata(card, ctx);
> +
> +	ret = devm_snd_soc_register_card(dev, card);
> +	if (ret) {
> +		dev_err(dev, "Failed to register sound card: %d\n", ret);
> +		goto err_mutex;
>  	}
>  
>  	return 0;
>  
> +err_mutex:
> +	mutex_destroy(&ctx->sample_rate_lock);
>  	return ret;
>  }
>  
> -module_init(omap3pandora_soc_init);
>  
> -static void __exit omap3pandora_soc_exit(void)
> +static void omap3pandora_remove(struct platform_device *pdev)
>  {
> -	regulator_put(omap3pandora_dac_reg);
> -	platform_device_unregister(omap3pandora_snd_device);
> +	struct snd_soc_card *card = platform_get_drvdata(pdev);
> +	struct omap3pandora_sound *ctx = snd_soc_card_get_drvdata(card);
> +
> +	mutex_destroy(&ctx->sample_rate_lock);
>  }

[Severity: High]
Can destroying the mutex here lead to a use-after-free or lockdep panic?

The sound card is registered using devm_snd_soc_register_card(), which
defers unregistration to the devres cleanup phase. This runs after the
omap3pandora_remove() function returns.

If we call mutex_destroy() synchronously in .remove, does this leave a
window where the sound card is still fully active and exposed to userspace,
but its internal lock has been destroyed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1783749722.git.hns@goldelico.com?part=10

Re: [PATCH 12/16] arm: dts: omap3pandora: Add device node for PCM1773 codec

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Incorrect lifecycle for `devm_*` resource allocations in the ASoC component probe causes memory leaks and rebinding failures.
- [High] The driver swallows `-EPROBE_DEFER` from `devm_regulator_get()`, breaking deferred probing for the VCC supply.
- [Low] The commit uses the deprecated property name `enable-gpio` instead of `enable-gpios`.
- [Low] Missing property definitions in the PCM1773 YAML binding schema will cause `dtbs_check` failures.
- [Low] Busy-waiting with `mdelay()` is used in a sleepable context.
--

--- Patch [12]: [PATCH 12/16] arm: dts: omap3pandora: Add device node for PCM1773 codec ---
commit 4dbdbb04f5f389f900287ce1a74c35a57890d4b3
Author: Grond <grond66@riseup.net>

arm: dts: omap3pandora: Add device node for PCM1773 codec

This commit adds a new device node for the PCM1773 codec to the OpenPandora
device tree. This ensures the new PCM1773 driver gets loaded automatically.

> diff --git a/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi b/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi
> --- a/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi
> +++ b/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi
> @@ -42,6 +42,13 @@ tv_connector_in: endpoint {
>  		};
>  	};
>  
> +	dac: pcm1773-codec {
> +		compatible = "ti,pcm1773";
> +		vcc-supply = <&vsim>;
> +		enable-gpio = <&gpio4 22 GPIO_ACTIVE_HIGH>; /* GPIO 118 */

[Severity: Low]
Is it expected to use the deprecated property name enable-gpio instead of the
mandated enable-gpios suffix?

The standard requires the plural form even for single GPIOs.

> +		status = "okay";
> +	};
> +
>  	gpio-leds {

> diff --git a/Documentation/devicetree/bindings/sound/pcm1773.yaml b/Documentation/devicetree/bindings/sound/pcm1773.yaml
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/pcm1773.yaml
[ ... ]
> +properties:
> +  compatible:
> +    const: ti,pcm1773
> +
> +required:
> +  - compatible
> +
> +additionalProperties: false

[Severity: Low]
Will this schema cause dtbs_check validation errors?

The schema includes additionalProperties: false but fails to list enable-gpios
and vcc-supply in the properties dictionary.

The provided device tree nodes specify these properties, which will lead to
schema validation failures.

> +
> +examples:

> diff --git a/sound/soc/codecs/pcm1773.c b/sound/soc/codecs/pcm1773.c
> --- /dev/null
> +++ b/sound/soc/codecs/pcm1773.c
[ ... ]
> +static int pcm1773_dac_event(struct snd_soc_dapm_widget *w,
> +			     struct snd_kcontrol *k, int event)
> +{
[ ... ]
> +	if (SND_SOC_DAPM_EVENT_ON(event)) {
> +		if (ctx->regulator) {
> +			ret = regulator_enable(ctx->regulator);
> +			if (ret) {
> +				dev_err(dev, "Failed to power DAC: %d\n", ret);
> +				return ret;
> +			}
> +			mdelay(1);

[Severity: Low]
Should this use usleep_range() or msleep() instead of busy-waiting?

pcm1773_dac_event() is executed during DAPM widget power up/down events, which
run in a sleepable context (as demonstrated by calls to
gpiod_set_value_cansleep()). Using mdelay() here wastes CPU cycles.

> +		}
[ ... ]
> +static int pcm1773_probe(struct snd_soc_component *component)
> +{
> +	struct pcm1773 *ctx = NULL;
> +	struct device *dev = component->dev;
> +	int ret;
> +
> +	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +	snd_soc_component_set_drvdata(component, ctx);
> +
> +	ctx->enable_gpio = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_LOW);

[Severity: High]
Can this incorrect lifecycle for devm_ allocations cause memory leaks and
rebinding failures?

pcm1773_probe() executes during sound card binding, using devm_kzalloc() and
devm_gpiod_get_optional() with the platform device (component->dev).

Since the platform device outlives the sound card lifecycle, these resources
are not freed upon unbind, and a subsequent bind will hit -EBUSY when
requesting the GPIO or regulator.

> +	if (IS_ERR(ctx->enable_gpio)) {
[ ... ]
> +	ctx->regulator = devm_regulator_get(dev, "vcc");
> +	if (IS_ERR(ctx->regulator)) {
> +		dev_warn(dev, "cannot get regulator 'vcc'");
> +		ctx->regulator = NULL;
> +	}

[Severity: High]
Does this break deferred probing for the VCC supply by swallowing
-EPROBE_DEFER?

When the VCC regulator driver is loaded after the sound driver,
devm_regulator_get() returns -EPROBE_DEFER.

This code intercepts the error, sets the regulator to NULL, and continues
instead of propagating the error. This will cause the DAC to silently fail to
power up later.

> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1783749722.git.hns@goldelico.com?part=12

Re: [PATCH 15/16] backlight: remove pandora_bl

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Removal of the TPS61161 backlight controller PWM mode initialization sequence, potentially breaking backlight control.
- [Low] The 'pandora-backlight' platform device is still registered in legacy pdata quirks despite the driver being removed, resulting in an unbound dead device.
- [Low] Stale Kconfig reference to `CONFIG_BACKLIGHT_PANDORA` remains in `omap2plus_defconfig`.
--

--- Patch [15]: [PATCH 15/16] backlight: remove pandora_bl ---
commit 6eef80f63fc65c8cd1ed05d5c2e07cc3c726434e
Author: H. Nikolaus Schaller <hns@goldelico.com>

backlight: remove pandora_bl

Retire the platform specific pandora backlight driver since
it can now be replaced by twl_pm and device tree.

[Severity: Low]
Does this commit leave an unbound dead device registered in the system?

The pandora-backlight platform driver is removed here, but it appears
arch/arm/mach-omap2/pdata-quirks.c still statically defines and registers
the device in omap3_pandora_legacy_init():

    platform_device_register(&pandora_backlight);

Should this platform device registration be removed as well?

> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> index 7aa1c4b21111f..a250b3edc152c 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
> @@ -451,13 +451,6 @@ config BACKLIGHT_MP3309C
[ ... ]
> -config BACKLIGHT_PANDORA
> -	tristate "Backlight driver for Pandora console"
> -	depends on TWL4030_CORE
> -	help
> -	  If you have a Pandora console, say Y to enable the
> -	  backlight driver.
> -

[Severity: Low]
Will this removal trigger a Kconfig warning during omap2plus_defconfig
generation?

The defconfig file still explicitly enables CONFIG_BACKLIGHT_PANDORA=m.
Should arch/arm/configs/omap2plus_defconfig be updated to remove this stale
entry?

> diff --git a/drivers/video/backlight/pandora_bl.c b/drivers/video/backlight/pandora_bl.c
> deleted file mode 100644
> index 8a63ded0fa90f..0000000000000
> --- a/drivers/video/backlight/pandora_bl.c
> +++ /dev/null
> @@ -1,159 +0,0 @@
[ ... ]
> -	if (priv->old_state == PANDORABL_WAS_OFF) {
> -		/*
> -		 * set PWM duty cycle to max. TPS61161 seems to use this
> -		 * to calibrate it's PWM sensitivity when it starts.
> -		 */
> -		twl_i2c_write_u8(TWL_MODULE_PWM, MAX_VALUE, TWL_PWM0_OFF);
> -
> -		/* first enable clock, then PWM0 out */
> -		twl_i2c_read_u8(TWL4030_MODULE_INTBR, &r, TWL_INTBR_GPBR1);
> -		r &= ~PWM0_ENABLE;
> -		r |= PWM0_CLK_ENABLE;
> -		twl_i2c_write_u8(TWL4030_MODULE_INTBR, r, TWL_INTBR_GPBR1);
> -		r |= PWM0_ENABLE;
> -		twl_i2c_write_u8(TWL4030_MODULE_INTBR, r, TWL_INTBR_GPBR1);
> -
> -		/*
> -		 * TI made it very easy to enable digital control, so easy that
> -		 * it often triggers unintentionally and disabes PWM control,
> -		 * so wait until 1 wire mode detection window ends.
> -		 */
> -		usleep_range(2000, 10000);
> -	}

[Severity: High]
When switching to the generic pwm-backlight driver, how is the TPS61161
initialized into PWM mode?

Because the TPS61161 shares its EN pin for both 1-wire EasyScale digital
control and PWM control, the removed pandora_bl driver included this
hardware workaround to force a 100% PWM duty cycle and sleep for 2-10ms at
startup.

Without this initialization pulse, could the TPS61161 misinterpret the initial
PWM signal as a 1-wire EasyScale command and disable PWM control, preventing
the backlight from turning on properly?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1783749722.git.hns@goldelico.com?part=15

Re: [PATCH 02/16] regulator: twl: Expose the TWL4030's REGEN signal as a regulator

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Unnecessarily strict DT property enforcement causes probe failure.
--

--- Patch [2]: [PATCH 02/16] regulator: twl: Expose the TWL4030's REGEN signal as a regulator ---
commit d13717da4f5b51bb52a516562b40db8ac7694aad
Author: Grond <grond66@riseup.net>

regulator: twl: Expose the TWL4030's REGEN signal as a regulator

This commit exposes the TWL4030 REGEN signal as a regulator. This functionality
is needed on some boards where REGEN is used to drive external regulators.

> diff --git a/drivers/regulator/twl-regulator.c b/drivers/regulator/twl-regulator.c
> index 5bacfcebf59aa..d883ee8abec32 100644
> --- a/drivers/regulator/twl-regulator.c
> +++ b/drivers/regulator/twl-regulator.c

[ ... ]

> @@ -615,6 +647,26 @@ static int twlreg_probe(struct platform_device *pdev)
>  	case TWL4030_REG_VINTANA2:
>  	case TWL4030_REG_VINTDIG:
>  		c->always_on = true;
> +		break;
> +	case TWL4030_REG_REGEN:

[ ... ]

> +		info->desc.fixed_uV = c->min_uV;
> +
> +		ret = of_property_read_u32(pdev->dev.of_node, "startup-delay-us",
> +					   &info->desc.enable_time);
> +		if (ret) {
> +			dev_err(&pdev->dev, "no valid startup delay set in DT: %d\n", ret);
> +			return ret;
> +		}

[Severity: Low]
This isn't a bug, but does this code need to fail the probe if the
startup-delay-us property is missing?

Since enable_time already defaults to 0 in the template structure, could this
be treated as an optional property similar to standard fixed-regulator
behavior, rather than strictly enforcing its presence and returning an error?

> +
> +		break;
>  	default:
>  		break;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1783749722.git.hns@goldelico.com?part=2

Re: [PATCH 13/16] arm: dts: omap3pandora: create new DT node for the sound card

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Double decrement of sample_rate_users on hw_params error path.
- [High] Using `IS_BUILTIN()` to change CPU and Platform component names breaks DAI linking for built-in driver initialization.
- [Medium] Incorrect `$id` and `$schema` fields in the newly added audio DT bindings.
- [Medium] The `ti,pcm1773` binding schema does not document necessary properties and sets `additionalProperties: false`, causing DT validation failures.
--

--- Patch [13]: [PATCH 13/16] arm: dts: omap3pandora: create new DT node for the sound card ---
commit 9dab89ea3f463775080f6924b99e5cea84e88df3
Author: Stefan Leichter <sle85276@gmx.de>

arm: dts: omap3pandora: create new DT node for the sound card

This means that the sound card driver (snd-soc-omap3pandora) will get
loaded by default. To make this work, we also add dac pcm1773-codec and
widgets and routing as needed.

> diff --git a/Documentation/devicetree/bindings/sound/openpandora,omap3pandora-sound.yaml b/Documentation/devicetree/bindings/sound/openpandora,omap3pandora-sound.yaml
> new file mode 100644
> index 0000000000000..d3c747e5c58d6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/openpandora,omap3pandora-sound.yaml
> @@ -0,0 +1,94 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org
> +$schema: http://devicetree.org

[Severity: Medium]
Are these $id and $schema paths correct? The $id field breaks the schema
cross-reference system if it doesn't include the full path, and could cause
dt_binding_check to report misleading errors or skip validation.

[ ... ]

> diff --git a/Documentation/devicetree/bindings/sound/pcm1773.yaml b/Documentation/devicetree/bindings/sound/pcm1773.yaml
> new file mode 100644
> index 0000000000000..f3e640705bf70
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/pcm1773.yaml
> @@ -0,0 +1,32 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/bindings/sound/pcm1773.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#

[Severity: Medium]
Should this $id field omit the extraneous bindings/ directory to match the
standard path format?

[ ... ]

> +properties:
> +  compatible:
> +    const: ti,pcm1773
> +
> +required:
> +  - compatible
> +
> +additionalProperties: false

[Severity: Medium]
Since additionalProperties is set to false but only the compatible string is
documented, won't valid device tree nodes that use properties like
vcc-supply, enable-gpio, status, and #sound-dai-cells fail validation?

[ ... ]

> diff --git a/sound/soc/ti/omap3pandora.c b/sound/soc/ti/omap3pandora.c
> index 6c9c184cd9d6f..d3509473f5df0 100644
> --- a/sound/soc/ti/omap3pandora.c
> +++ b/sound/soc/ti/omap3pandora.c
[ ... ]
> +static int omap3pandora_playback_hw_params(struct snd_pcm_substream *substream,
> +					   struct snd_pcm_hw_params *params)
> +{
[ ... ]
> +	if (!ctx->playback_stream) {
> +		ctx->playback_stream = substream;
> +		ret = grab_sample_rate(ctx, substream, params);
> +		if (ret)
> +			return ret;
> +	}

[Severity: High]
If grab_sample_rate() returns an error, it drops sample_rate_users back
down, but ctx->playback_stream remains set to this substream.

When this stream is subsequently closed, omap3pandora_playback_hw_free()
will see ctx->playback_stream is set and call release_sample_rate() again.

Will this result in a double decrement of sample_rate_users that could
prematurely unlock constraints for an active stream?

[ ... ]

> +static int omap3pandora_capture_hw_params(struct snd_pcm_substream *substream,
> +					  struct snd_pcm_hw_params *params)
> +{
[ ... ]
> +	if (!ctx->capture_stream) {
> +		ctx->capture_stream = substream;
> +		ret = grab_sample_rate(ctx, substream, params);
> +		if (ret)
> +			return ret;
> +	}

[Severity: High]
Similarly, does this have the same double decrement issue if
grab_sample_rate() fails, leaving ctx->capture_stream set for
omap3pandora_capture_hw_free() to clean up later?

[ ... ]

> +#if IS_BUILTIN(CONFIG_SND_SOC_OMAP3_PANDORA)
>  SND_SOC_DAILINK_DEFS(out,
>  	DAILINK_COMP_ARRAY(COMP_CPU("omap-mcbsp.2")),
> -	DAILINK_COMP_ARRAY(COMP_CODEC("twl4030-codec", "twl4030-hifi")),
> +	DAILINK_COMP_ARRAY(COMP_CODEC("pcm1773-codec", "pcm1773-hifi")),
>  	DAILINK_COMP_ARRAY(COMP_PLATFORM("omap-mcbsp.2")));
>  
>  SND_SOC_DAILINK_DEFS(in,
>  	DAILINK_COMP_ARRAY(COMP_CPU("omap-mcbsp.4")),
>  	DAILINK_COMP_ARRAY(COMP_CODEC("twl4030-codec", "twl4030-hifi")),
>  	DAILINK_COMP_ARRAY(COMP_PLATFORM("omap-mcbsp.4")));
> +#else /* IS_BUILTIN(CONFIG_SND_SOC_OMAP3_PANDORA) */
> +SND_SOC_DAILINK_DEFS(out,
> +	DAILINK_COMP_ARRAY(COMP_CPU("49022000.mcbsp")),
> +	DAILINK_COMP_ARRAY(COMP_CODEC("pcm1773-codec", "pcm1773-hifi")),
> +	DAILINK_COMP_ARRAY(COMP_PLATFORM("49022000.mcbsp")));
> +
> +SND_SOC_DAILINK_DEFS(in,
> +	DAILINK_COMP_ARRAY(COMP_CPU("49026000.mcbsp")),
> +	DAILINK_COMP_ARRAY(COMP_CODEC("twl4030-codec", "twl4030-hifi")),
> +	DAILINK_COMP_ARRAY(COMP_PLATFORM("49026000.mcbsp")));
> +#endif /* IS_BUILTIN(CONFIG_SND_SOC_OMAP3_PANDORA) */

[Severity: High]
Does this IS_BUILTIN() check break DAI linking when the driver is built into
the kernel?

Since the McBSP devices are populated via the Device Tree and use unit
addresses (like 49022000.mcbsp) regardless of the driver's compilation
state, could ALSA fail to find omap-mcbsp.2 when probing the built-in
driver?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1783749722.git.hns@goldelico.com?part=13

Re: [PATCH 11/16] ARM: dts: omap3-pandora-common: Enable audio in/out (mcbsp4/2)

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing Sound Card devicetree node leaves audio broken.
- [Medium] Missing Pin Multiplexing for McBSP nodes.
--

--- Patch [11]: [PATCH 11/16] ARM: dts: omap3-pandora-common: Enable audio in/out (mcbsp4/2) ---
commit e4ecb9f743bc9100e161520f1482dafaecae52b7
Author: Stefan Leichter <sle85276@gmx.de>

ARM: dts: omap3-pandora-common: Enable audio in/out (mcbsp4/2)

This commit enables the mcbsp2 and mcbsp4 nodes in the omap3-pandora-common
device tree to support audio.

> diff --git a/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi b/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi
> index b4acab1625212..5907b2455f75b 100644
> --- a/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi
> +++ b/arch/arm/boot/dts/ti/omap/omap3-pandora-common.dtsi
> @@ -716,6 +716,7 @@ &mcbsp1 {
>  
>  /* audio DAC */
>  &mcbsp2 {
> +	status = "okay";

[Severity: High]
Is the audio hardware left non-functional because the sound card devicetree
node is missing?

While this enables the McBSP nodes, the machine sound driver itself is never
instantiated because its device node (matching openpandora,omap3pandora-sound)
is missing.

[Severity: Medium]
Are the required pinctrl configurations missing for this node?

It is a standard devicetree requirement on OMAP3 to define
pinctrl-names = "default"; and pinctrl-0 = <...>; properties for enabled
peripherals so that the kernel explicitly claims and configures the pins
independent of the bootloader state.

>  };
>  
>  /* bluetooth */
> @@ -724,6 +725,7 @@ &mcbsp3 {
>  
>  /* to twl4030*/
>  &mcbsp4 {
> +	status = "okay";

[Severity: Medium]
Should this node also include pinctrl configurations to ensure the kernel
explicitly configures the pins independent of the bootloader state?

>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1783749722.git.hns@goldelico.com?part=11

Re: [PATCH 03/16] arm: dts: twl4030: add twl4030-regen child

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The YAML binding schema fails to allow 'startup-delay-us', causing a conflict between driver probe requirements and dtbs_check validation.
- [Medium] Architectural flaw in Device Tree design: conflating a PMIC digital control signal with an external voltage regulator.
--

--- Patch [3]: [PATCH 03/16] arm: dts: twl4030: add twl4030-regen child ---
commit a39086d961abe1def74820ce0f67a5656f16f99d
Author: Grond <grond66@riseup.net>

arm: dts: twl4030: add twl4030-regen child
    
This is needed on some boards (for example, the Pandora), where REGEN is
used to drive external regulators.

> diff --git a/arch/arm/boot/dts/ti/omap/twl4030.dtsi b/arch/arm/boot/dts/ti/omap/twl4030.dtsi
> index 07b9ca942e78d..e6f80b8f0a731 100644
> --- a/arch/arm/boot/dts/ti/omap/twl4030.dtsi
> +++ b/arch/arm/boot/dts/ti/omap/twl4030.dtsi
> @@ -112,6 +112,20 @@ vsim: regulator-vsim {
>  		regulator-max-microvolt = <3000000>;
>  	};
>  
> +	regen: regulator-regen {
> +		compatible = "ti,twl4030-regen";
> +		/*
> +		 * Since REGEN is technically just a signal that actuates an
> +		 * external regulator, we do not know what voltage it will

[Severity: Medium]
Is there an architectural mismatch in how this external regulator is modeled?

The comment acknowledges that REGEN is just a control signal, but the driver
forces the PMIC's digital signal node to define the external component's
voltage parameters:

drivers/regulator/twl-regulator.c:twlreg_probe() {
        if (c->min_uV == 0 || c->max_uV == 0) {
                dev_err(&pdev->dev, "minimum or maximum regulator voltage zero in DT\n");
                return -EINVAL;
        }
}

Would it be more aligned with Device Tree principles to model the PMIC's
REGEN pin as an enable-gpio or a voltage-less switch, and define a separate
regulator-fixed node for the external regulator that uses REGEN to enable it?

> +		 * eventually have, or what it's on/off timing should be.
> +		 * Therefore, disable it here; boards that use it can override
> +		 * the status to "okay" once they have set the
> +		 * regulator-{min,max}-microvolt and startup-delay-us
> +		 * attributes.
> +		 */

[Severity: High]
Will this cause a conflict between driver probe requirements and the
dtbs_check validation?

Looking at twlreg_probe() in drivers/regulator/twl-regulator.c, the driver
explicitly requires the startup-delay-us property:

drivers/regulator/twl-regulator.c:twlreg_probe() {
        ret = of_property_read_u32(pdev->dev.of_node, "startup-delay-us",
                                   &info->desc.enable_time);
        if (ret) {
                dev_err(&pdev->dev, "no valid startup delay set in DT: %d\n", ret);
                return ret;
        }
}

However, the ti,twl.yaml binding schema introduces ti,twl4030-regen but fails
to add startup-delay-us to the allowed properties, leading to
unevaluatedProperties: false rejecting it during validation.

If board developers provide the property to satisfy the driver, they will
fail schema validation, but if they omit it to pass validation, the driver
will fail to probe at runtime. Can the schema be updated to allow this, or
is there a different expected behavior?

> +		status = "disabled";
> +	};

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1783749722.git.hns@goldelico.com?part=3

Re: [PATCH 04/16] arm: dts: omap3pandora: Populate DT data for the TWL4030's REGEN regulator

[sashiko-bot]

> From: Grond <grond66@riseup.net>
> 
> On the Pandora, REGEN is used to enable TPS61029DRC external regulator
> which runs a 5V power rail. The 5ms startup delay is taken from Pandora's
> 3.2 kernel, where it is labeled as a "guess". So it may be possible to
> activate the regulator faster.
> 
> On Pandora CC units, this regulator is used as the input to the LCD power
> supply. Therefore, problems are likely to arise if the regulator is ever
> disabled, so we add the regulator-always-on property to compensate.
> 
> Signed-off-by: Grond <grond66@riseup.net>
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1783749722.git.hns@goldelico.com?part=4