[PATCH 1/2] soundwire: qcom: set the bus mclk_freq property

[PATCH 1/2] soundwire: qcom: set the bus mclk_freq property

[Jorijn van der Graaf]

sdw_slave_get_scale_index() needs bus->prop.mclk_freq to compute the
SCP bus-clock base and scale register values, and fails with "no bus
MCLK" when it is unset. The qcom controller never set it, so slave
initialization cannot program the clock registers on this bus: it
would fail outright for an SDCA-class slave, and a slave driver
declaring clock_reg_supported hits the same error. The pending WCD9378
codec driver hand-rolls these writes as a workaround, following its
downstream counterpart.

Report the controller clock, letting the core derive the same values
the hand-rolled writes program: 19.2 MHz base and the scale matching
the bus clock (half the double rate).

Assisted-by: Claude:claude-fable-5
Signed-off-by: Jorijn van der Graaf <jorijnvdgraaf@catcrafts.net>
---
This is the soundwire side of the plan from the WCD9378 v1 review
discussion [1]; the codec's v2 will set prop.clock_reg_supported and
delete its hand-rolled base/scale writes, relying on this property.
Patch 2 is the helper extension requested in that thread.

Validated on the Fairphone 6 (SM7635, WCD9378): with the hand-rolled
writes deleted, the core reports "Configured bus base 1, scale 2,
mclk 19200000, curr_freq 9600000" for both slaves at enumeration, the
codec's SDCA sequencer powers up on those values, and capture works,
across reboots.

No behavior change for existing devices: without a class_id or
clock_reg_supported, sdw_slave_set_frequency() returns before reading
mclk_freq.

[1] https://lore.kernel.org/all/20260707151329.67858-1-jorijnvdgraaf@catcrafts.net/

 drivers/soundwire/qcom.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 3562802f4204..55678a30cd4a 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -1633,6 +1633,7 @@ static int qcom_swrm_probe(struct platform_device *pdev)
 
 	prop = &ctrl->bus.prop;
 	prop->max_clk_freq = DEFAULT_CLK_FREQ;
+	prop->mclk_freq = DEFAULT_CLK_FREQ;
 	prop->num_clk_gears = 0;
 	prop->num_clk_freq = MAX_FREQ_NUM;
 	prop->clk_freq = &qcom_swrm_freq_tbl[0];

base-commit: 6409292ab5f7d1a60a6f8948bd6dcd8633c36dfe
-- 
2.55.0

[PATCH 2/2] soundwire: honor clock_reg_supported in the clock scaling check

[Jorijn van der Graaf]

sdw_slave_set_frequency() treats class_id and prop.clock_reg_supported
as equivalent evidence that a slave implements the bus-clock base and
scale registers, but the bank-switch reprogramming path checks class_id
alone, so a class-0 slave that declared the registers never gets the
next-bank scale written there. The registers are SoundWire 1.2, not
SDCA, so a device may well implement them without setting the class
field.

Extend the helper to honor clock_reg_supported, as discussed with
Pierre-Louis in the WCD9378 review. This also makes a link whose
peripherals all declare clock_reg_supported eligible for dynamic clock
scaling in the generic bandwidth allocation, which is what declaring
the registers means.

Link: https://lore.kernel.org/all/5717102b-f7ab-42b2-8065-064d94dd2bee@linux.dev/
Assisted-by: Claude:claude-fable-5
Signed-off-by: Jorijn van der Graaf <jorijnvdgraaf@catcrafts.net>
---
Pierre-Louis agreed the helper should be extended in the WCD9378 v1
thread [1]. On the qcom bus this only adds next-bank scale writes of
the same value on bank switches (the clock is fixed); verified on the
Fairphone 6 that capture still works with them.

One behavior change I cannot test: the helper also feeds
is_clock_scaling_supported() in the generic bandwidth allocation, so an
Intel link whose peripherals all pass the check - max98363 is the
in-tree clock_reg_supported case - becomes eligible for dynamic clock
scaling where it previously ran at a fixed clock. Only configurations
that fail the bandwidth check today can select a different frequency; I
could not test that combination, flagging it for the Intel side.

[1] https://lore.kernel.org/all/5717102b-f7ab-42b2-8065-064d94dd2bee@linux.dev/

 drivers/soundwire/bus.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 0490777fa406..d94e44b59050 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -817,8 +817,11 @@ bool is_clock_scaling_supported_by_slave(struct sdw_slave *slave)
 	/*
 	 * Dynamic scaling is a defined by SDCA. However, some devices expose the class ID but
 	 * can't support dynamic scaling. We might need a quirk to handle such devices.
+	 * The clock base and scale registers themselves are SoundWire 1.2, so a device
+	 * may implement them without setting the class field; the driver says so with
+	 * clock_reg_supported.
 	 */
-	return slave->id.class_id;
+	return slave->id.class_id || slave->prop.clock_reg_supported;
 }
 EXPORT_SYMBOL(is_clock_scaling_supported_by_slave);
 
-- 
2.55.0

Re: [PATCH 2/2] soundwire: honor clock_reg_supported in the clock scaling check

[Pierre-Louis Bossart]

On 7/27/26 18:27, Jorijn van der Graaf wrote:
> sdw_slave_set_frequency() treats class_id and prop.clock_reg_supported
> as equivalent evidence that a slave implements the bus-clock base and
> scale registers, but the bank-switch reprogramming path checks class_id
> alone, so a class-0 slave that declared the registers never gets the
> next-bank scale written there. The registers are SoundWire 1.2, not
> SDCA, so a device may well implement them without setting the class
> field.
> 
> Extend the helper to honor clock_reg_supported, as discussed with
> Pierre-Louis in the WCD9378 review. This also makes a link whose
> peripherals all declare clock_reg_supported eligible for dynamic clock
> scaling in the generic bandwidth allocation, which is what declaring
> the registers means.
> 
> Link: https://lore.kernel.org/all/5717102b-f7ab-42b2-8065-064d94dd2bee@linux.dev/
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Jorijn van der Graaf <jorijnvdgraaf@catcrafts.net>
> ---
> Pierre-Louis agreed the helper should be extended in the WCD9378 v1
> thread [1]. On the qcom bus this only adds next-bank scale writes of
> the same value on bank switches (the clock is fixed); verified on the
> Fairphone 6 that capture still works with them.
> 
> One behavior change I cannot test: the helper also feeds
> is_clock_scaling_supported() in the generic bandwidth allocation, so an
> Intel link whose peripherals all pass the check - max98363 is the
> in-tree clock_reg_supported case - becomes eligible for dynamic clock
> scaling where it previously ran at a fixed clock. Only configurations
> that fail the bandwidth check today can select a different frequency; I
> could not test that combination, flagging it for the Intel side.
> 
> [1] https://lore.kernel.org/all/5717102b-f7ab-42b2-8065-064d94dd2bee@linux.dev/
> 
>  drivers/soundwire/bus.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
> index 0490777fa406..d94e44b59050 100644
> --- a/drivers/soundwire/bus.c
> +++ b/drivers/soundwire/bus.c
> @@ -817,8 +817,11 @@ bool is_clock_scaling_supported_by_slave(struct sdw_slave *slave)
>  	/*
>  	 * Dynamic scaling is a defined by SDCA. However, some devices expose the class ID but
>  	 * can't support dynamic scaling. We might need a quirk to handle such devices.
> +	 * The clock base and scale registers themselves are SoundWire 1.2, so a device
> +	 * may implement them without setting the class field; the driver says so with
> +	 * clock_reg_supported.
>  	 */
> -	return slave->id.class_id;
> +	return slave->id.class_id || slave->prop.clock_reg_supported;
>  }
>  EXPORT_SYMBOL(is_clock_scaling_supported_by_slave);

This looks fine, but can I suggest an improvement? It'd be good if that
helper was used below instead of having the same test twice. If we ever
need quirks or updates it'll be done in a single location.

static int sdw_slave_set_frequency(struct sdw_slave *slave)
{
	int scale_index;
	u8 base;
	int ret;

	/*
	 * frequency base and scale registers are required for SDCA
	 * devices. They may also be used for 1.2+/non-SDCA devices.
	 * Driver can set the property directly, for now there's no
	 * DisCo property to discover support for the scaling registers
	 * from platform firmware.
	 */
	if (!slave->id.class_id && !slave->prop.clock_reg_supported)
		return 0;



>  

Re: [PATCH 1/2] soundwire: qcom: set the bus mclk_freq property

[Srinivas Kandagatla]

On 7/27/26 5:27 PM, Jorijn van der Graaf wrote:
> sdw_slave_get_scale_index() needs bus->prop.mclk_freq to compute the
> SCP bus-clock base and scale register values, and fails with "no bus
> MCLK" when it is unset. The qcom controller never set it, so slave
> initialization cannot program the clock registers on this bus: it
> would fail outright for an SDCA-class slave, and a slave driver
> declaring clock_reg_supported hits the same error. The pending WCD9378
> codec driver hand-rolls these writes as a workaround, following its
> downstream counterpart.
> 
> Report the controller clock, letting the core derive the same values
> the hand-rolled writes program: 19.2 MHz base and the scale matching
> the bus clock (half the double rate).
> 
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Jorijn van der Graaf <jorijnvdgraaf@catcrafts.net>
> ---
Tested-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>

--srini

> This is the soundwire side of the plan from the WCD9378 v1 review
> discussion [1]; the codec's v2 will set prop.clock_reg_supported and
> delete its hand-rolled base/scale writes, relying on this property.
> Patch 2 is the helper extension requested in that thread.
> 
> Validated on the Fairphone 6 (SM7635, WCD9378): with the hand-rolled
> writes deleted, the core reports "Configured bus base 1, scale 2,
> mclk 19200000, curr_freq 9600000" for both slaves at enumeration, the
> codec's SDCA sequencer powers up on those values, and capture works,
> across reboots.
> 
> No behavior change for existing devices: without a class_id or
> clock_reg_supported, sdw_slave_set_frequency() returns before reading
> mclk_freq.
> 
> [1] https://lore.kernel.org/all/20260707151329.67858-1-jorijnvdgraaf@catcrafts.net/
> 
>  drivers/soundwire/qcom.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
> index 3562802f4204..55678a30cd4a 100644
> --- a/drivers/soundwire/qcom.c
> +++ b/drivers/soundwire/qcom.c
> @@ -1633,6 +1633,7 @@ static int qcom_swrm_probe(struct platform_device *pdev)
>  
>  	prop = &ctrl->bus.prop;
>  	prop->max_clk_freq = DEFAULT_CLK_FREQ;
> +	prop->mclk_freq = DEFAULT_CLK_FREQ;
>  	prop->num_clk_gears = 0;
>  	prop->num_clk_freq = MAX_FREQ_NUM;
>  	prop->clk_freq = &qcom_swrm_freq_tbl[0];
> 
> base-commit: 6409292ab5f7d1a60a6f8948bd6dcd8633c36dfe