[PATCH 1/2] dt-bindings: power: define ID for Marvell PXA1908 audio domain

[PATCH 1/2] dt-bindings: power: define ID for Marvell PXA1908 audio domain

[Karel Balej]

Define an identifier for the SoC's audio power island so that it can be
referenced through device tree.

Signed-off-by: Karel Balej <balejk@matfyz.cz>
---
 include/dt-bindings/power/marvell,pxa1908-power.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/dt-bindings/power/marvell,pxa1908-power.h b/include/dt-bindings/power/marvell,pxa1908-power.h
index 19b088351af1..173f47e0e69d 100644
--- a/include/dt-bindings/power/marvell,pxa1908-power.h
+++ b/include/dt-bindings/power/marvell,pxa1908-power.h
@@ -13,5 +13,6 @@
 #define PXA1908_POWER_DOMAIN_GPU2D	2
 #define PXA1908_POWER_DOMAIN_DSI	3
 #define PXA1908_POWER_DOMAIN_ISP	4
+#define PXA1908_POWER_DOMAIN_AUDIO	5
 
 #endif
-- 
2.51.2

[PATCH 2/2] pmdomain: add audio power island for Marvell PXA1908 SoC

[Karel Balej]

Define power domain which needs to be enabled in order for audio to work
on the PXA1908-based samsung,coreprimevelte smartphone. In the
downstream code, this power-on method is marked as specific to the ulcx
series which is likely some variant of the board or some part of it. No
other audio components needed for sound to work on this phone are
currently available mainline but some successful testing was performed
with the vendor variants of the respective drivers and with the domain
forced always-on.

Signed-off-by: Karel Balej <balejk@matfyz.cz>
---
 .../marvell/pxa1908-power-controller.c        | 38 +++++++++++++++++--
 1 file changed, 34 insertions(+), 4 deletions(-)

diff --git a/drivers/pmdomain/marvell/pxa1908-power-controller.c b/drivers/pmdomain/marvell/pxa1908-power-controller.c
index ff5e6e82d3f8..e32eb227f235 100644
--- a/drivers/pmdomain/marvell/pxa1908-power-controller.c
+++ b/drivers/pmdomain/marvell/pxa1908-power-controller.c
@@ -29,7 +29,10 @@
 #define POWER_POLL_TIMEOUT_US	(25 * USEC_PER_MSEC)
 #define POWER_POLL_SLEEP_US	6
 
-#define NR_DOMAINS	5
+#define APMU_AUD_CLK		0x80
+#define AUDIO_ULCX_ENABLE	0x0d
+
+#define NR_DOMAINS	6
 
 #define to_pxa1908_pd(_genpd) container_of(_genpd, struct pxa1908_pd, genpd)
 
@@ -59,9 +62,13 @@ static inline bool pxa1908_pd_is_on(struct pxa1908_pd *pd)
 {
 	struct pxa1908_pd_ctrl *ctrl = pd->ctrl;
 
-	return pd->data.id != PXA1908_POWER_DOMAIN_DSI
-		? regmap_test_bits(ctrl->base, APMU_PWR_STATUS_REG, pd->data.pwr_state)
-		: regmap_test_bits(ctrl->base, APMU_DEBUG, DSI_PHY_DVM_MASK);
+	switch (pd->data.id) {
+	case PXA1908_POWER_DOMAIN_AUDIO:
+		return regmap_test_bits(ctrl->base, APMU_AUD_CLK, AUDIO_ULCX_ENABLE);
+	case PXA1908_POWER_DOMAIN_DSI:
+		return regmap_test_bits(ctrl->base, APMU_DEBUG, DSI_PHY_DVM_MASK);
+	}
+	return regmap_test_bits(ctrl->base, APMU_PWR_STATUS_REG, pd->data.pwr_state);
 }
 
 static int pxa1908_pd_power_on(struct generic_pm_domain *genpd)
@@ -123,6 +130,22 @@ static inline int pxa1908_dsi_power_off(struct generic_pm_domain *genpd)
 	return regmap_clear_bits(ctrl->base, APMU_DEBUG, DSI_PHY_DVM_MASK);
 }
 
+static inline int pxa1908_audio_power_on(struct generic_pm_domain *genpd)
+{
+	struct pxa1908_pd *pd = to_pxa1908_pd(genpd);
+	struct pxa1908_pd_ctrl *ctrl = pd->ctrl;
+
+	return regmap_set_bits(ctrl->base, APMU_AUD_CLK, AUDIO_ULCX_ENABLE);
+}
+
+static inline int pxa1908_audio_power_off(struct generic_pm_domain *genpd)
+{
+	struct pxa1908_pd *pd = to_pxa1908_pd(genpd);
+	struct pxa1908_pd_ctrl *ctrl = pd->ctrl;
+
+	return regmap_clear_bits(ctrl->base, APMU_AUD_CLK, AUDIO_ULCX_ENABLE);
+}
+
 #define DOMAIN(_id, _name, ctrl, mode, state) \
 	[_id] = { \
 		.data = { \
@@ -159,6 +182,13 @@ static struct pxa1908_pd domains[NR_DOMAINS] = {
 			.keep_on = true,
 		},
 	},
+	[PXA1908_POWER_DOMAIN_AUDIO] = {
+		.genpd = {
+			.name = "audio",
+			.power_on = pxa1908_audio_power_on,
+			.power_off = pxa1908_audio_power_off,
+		},
+	},
 };
 
 static void pxa1908_pd_remove(struct auxiliary_device *auxdev)
-- 
2.51.2

Re: [PATCH 2/2] pmdomain: add audio power island for Marvell PXA1908 SoC

[Duje Mihanović]

On 11/27/2025 8:02 PM, Karel Balej wrote:
> Define power domain which needs to be enabled in order for audio to work
> on the PXA1908-based samsung,coreprimevelte smartphone. In the
> downstream code, this power-on method is marked as specific to the ulcx
> series which is likely some variant of the board or some part of it. No
> other audio components needed for sound to work on this phone are
> currently available mainline but some successful testing was performed
> with the vendor variants of the respective drivers and with the domain
> forced always-on.

From what I know, ULCx is actually meant to be some SoC codename. ULC1
happens to be PXA1908, but I couldn't find any such codename for the rest.

Other than that, just a few nits:

> diff --git a/drivers/pmdomain/marvell/pxa1908-power-controller.c b/drivers/pmdomain/marvell/pxa1908-power-controller.c
> index ff5e6e82d3f8..e32eb227f235 100644
> --- a/drivers/pmdomain/marvell/pxa1908-power-controller.c
> +++ b/drivers/pmdomain/marvell/pxa1908-power-controller.c
> @@ -29,7 +29,10 @@
>  #define POWER_POLL_TIMEOUT_US	(25 * USEC_PER_MSEC)
>  #define POWER_POLL_SLEEP_US	6
>  
> -#define NR_DOMAINS	5
> +#define APMU_AUD_CLK		0x80
> +#define AUDIO_ULCX_ENABLE	0x0d

I would group these with the other register definitions.

Also, it's probably better to be more consistent with the naming, so I'd
prefer APMU_AUDIO_CLK.

> @@ -59,9 +62,13 @@ static inline bool pxa1908_pd_is_on(struct pxa1908_pd *pd)
>  {
>  	struct pxa1908_pd_ctrl *ctrl = pd->ctrl;
>  
> -	return pd->data.id != PXA1908_POWER_DOMAIN_DSI
> -		? regmap_test_bits(ctrl->base, APMU_PWR_STATUS_REG, pd->data.pwr_state)
> -		: regmap_test_bits(ctrl->base, APMU_DEBUG, DSI_PHY_DVM_MASK);
> +	switch (pd->data.id) {
> +	case PXA1908_POWER_DOMAIN_AUDIO:
> +		return regmap_test_bits(ctrl->base, APMU_AUD_CLK, AUDIO_ULCX_ENABLE);
> +	case PXA1908_POWER_DOMAIN_DSI:
> +		return regmap_test_bits(ctrl->base, APMU_DEBUG, DSI_PHY_DVM_MASK);
> +	}
> +	return regmap_test_bits(ctrl->base, APMU_PWR_STATUS_REG, pd->data.pwr_state);

Perhaps this should go into the switch?

Regards,
--
Duje

Re: [PATCH 2/2] pmdomain: add audio power island for Marvell PXA1908 SoC

[Karel Balej]

Duje Mihanović, 2025-11-28T22:30:55+01:00:
> On 11/27/2025 8:02 PM, Karel Balej wrote:
>> diff --git a/drivers/pmdomain/marvell/pxa1908-power-controller.c b/drivers/pmdomain/marvell/pxa1908-power-controller.c
>> index ff5e6e82d3f8..e32eb227f235 100644
>> --- a/drivers/pmdomain/marvell/pxa1908-power-controller.c
>> +++ b/drivers/pmdomain/marvell/pxa1908-power-controller.c
>> @@ -29,7 +29,10 @@
>>  #define POWER_POLL_TIMEOUT_US	(25 * USEC_PER_MSEC)
>>  #define POWER_POLL_SLEEP_US	6
>>  
>> -#define NR_DOMAINS	5
>> +#define APMU_AUD_CLK		0x80
>> +#define AUDIO_ULCX_ENABLE	0x0d
>
> I would group these with the other register definitions.
>
> Also, it's probably better to be more consistent with the naming, so I'd
> prefer APMU_AUDIO_CLK.

So would I, but this is how the downstream code calls it so my idea was
that it possibly matches the datasheet and it would seem preferable to
me to match that even though we don't have it available.

I could then do the reverse of what you say and call the other
definition AUD_ULCX_ENABLE but AUDIO seems nicer to me too and this one
is not defined in the downstream code.

What do you think?

Re: [PATCH 2/2] pmdomain: add audio power island for Marvell PXA1908 SoC

[Duje Mihanović]

On 11/29/25 20:53, Karel Balej wrote:
> Duje Mihanović, 2025-11-28T22:30:55+01:00:
>>> -#define NR_DOMAINS	5
>>> +#define APMU_AUD_CLK		0x80
>>> +#define AUDIO_ULCX_ENABLE	0x0d
>>
>> I would group these with the other register definitions.
>>
>> Also, it's probably better to be more consistent with the naming, so I'd
>> prefer APMU_AUDIO_CLK.
> 
> So would I, but this is how the downstream code calls it so my idea was
> that it possibly matches the datasheet and it would seem preferable to
> me to match that even though we don't have it available.

AUDIO is indeed nicer, and IMO it doesn't matter if the names are 
perfectly matched with downstream. Matching with the datasheet would be 
a stronger argument, but it indeed isn't available so the whole point is 
moot.

> I could then do the reverse of what you say and call the other
> definition AUD_ULCX_ENABLE but AUDIO seems nicer to me too and this one
> is not defined in the downstream code.
> 
> What do you think?

I'd still prefer AUDIO, but don't care about it that much, so up to you.

Regards,
--
Duje

Re: [PATCH 1/2] dt-bindings: power: define ID for Marvell PXA1908 audio domain

[Krzysztof Kozlowski]

On 27/11/2025 20:02, Karel Balej wrote:
> Define an identifier for the SoC's audio power island so that it can be
> referenced through device tree.
> 
> Signed-off-by: Karel Balej <balejk@matfyz.cz>
> ---
>  include/dt-bindings/power/marvell,pxa1908-power.h | 1 +
>  1 file changed, 1 insertion(+)


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: power: define ID for Marvell PXA1908 audio domain

[Duje Mihanović]

On 11/27/2025 8:02 PM, Karel Balej wrote:
> Define an identifier for the SoC's audio power island so that it can be
> referenced through device tree.
> 
> Signed-off-by: Karel Balej <balejk@matfyz.cz>

Reviewed-by: Duje Mihanović <duje@dujemihanovic.xyz>

Regards,
--
Duje