Report the ability to support SoundWire clock-stop mode 1 if this is
enabled in ACPI. Mode 1 allows the device to lose state, so it can
reduce power consumption in clock-stop. Also add the necessary
handling to wait for re-enumeration on resume.

This does not use sdw_slave_read_prop(), because that also fills in
other properties from ACPI that were not previously set by the driver
and this has been observed to break some systems. Instead, the
"mipi-sdw-clock-stop-mode1-supported" property is checked directly.

When a SoundWire peripheral has been put into clock-stop mode 1 it
must be re-enumerated after the clock is restarted. A new flag
sdw_in_clock_stop_1 is set to true in cs35l56_sdw_clk_stop() if the
SoundWire core notifies that it is entering clock-stop 1.
cs35l56_sdw_handle_unattach() will wait for re-enumeration if
sdw_in_clock_stop_1 is true.

sdw_in_clock_stop_1 will be reset to false when an ATTACH notification
is received in cs35l56_sdw_update_status().

Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
---
 sound/soc/codecs/cs35l56-sdw.c | 34 +++++++++++++++++++++++++++++-----
 sound/soc/codecs/cs35l56.h     |  1 +
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/sound/soc/codecs/cs35l56-sdw.c b/sound/soc/codecs/cs35l56-sdw.c
index 30b3192d6ce9..9dc47fec1ea0 100644
--- a/sound/soc/codecs/cs35l56-sdw.c
+++ b/sound/soc/codecs/cs35l56-sdw.c
@@ -14,6 +14,7 @@
 #include <linux/soundwire/sdw.h>
 #include <linux/soundwire/sdw_registers.h>
 #include <linux/soundwire/sdw_type.h>
+#include <linux/string_choices.h>
 #include <linux/swab.h>
 #include <linux/types.h>
 #include <linux/workqueue.h>
@@ -340,6 +341,14 @@ static int cs35l56_sdw_read_prop(struct sdw_slave *peripheral)
 	struct cs35l56_private *cs35l56 = dev_get_drvdata(&peripheral->dev);
 	struct sdw_slave_prop *prop = &peripheral->prop;
 	struct sdw_dpn_prop *ports;
+	u8 clock_stop_1 = false;
+	int ret;
+
+	ret = fwnode_property_read_u8(dev_fwnode(cs35l56->base.dev),
+				      "mipi-sdw-clock-stop-mode1-supported",
+				      &clock_stop_1);
+	if (ret == 0)
+		prop->clk_stop_mode1 = !!clock_stop_1;
 
 	ports = devm_kcalloc(cs35l56->base.dev, 2, sizeof(*ports), GFP_KERNEL);
 	if (!ports)
@@ -363,6 +372,9 @@ static int cs35l56_sdw_read_prop(struct sdw_slave *peripheral)
 	ports[1].ch_prep_timeout = 10;
 	prop->src_dpn_prop = &ports[1];
 
+	dev_dbg(&peripheral->dev, "clock stop mode 1 supported: %s\n",
+		str_yes_no(prop->clk_stop_mode1));
+
 	return 0;
 }
 
@@ -374,6 +386,7 @@ static int cs35l56_sdw_update_status(struct sdw_slave *peripheral,
 	switch (status) {
 	case SDW_SLAVE_ATTACHED:
 		dev_dbg(cs35l56->base.dev, "%s: ATTACHED\n", __func__);
+		cs35l56->sdw_in_clock_stop_1 = false;
 		if (cs35l56->sdw_attached)
 			break;
 
@@ -399,25 +412,35 @@ static int __maybe_unused cs35l56_sdw_clk_stop(struct sdw_slave *peripheral,
 {
 	struct cs35l56_private *cs35l56 = dev_get_drvdata(&peripheral->dev);
 
-	dev_dbg(cs35l56->base.dev, "%s: mode:%d type:%d\n", __func__, mode, type);
+	dev_dbg(cs35l56->base.dev, "%s: clock_stop_mode%d stage:%d\n", __func__, mode, type);
 
-	return 0;
+	switch (type) {
+	case SDW_CLK_POST_PREPARE:
+		if (mode == SDW_CLK_STOP_MODE1)
+			cs35l56->sdw_in_clock_stop_1 = true;
+		else
+			cs35l56->sdw_in_clock_stop_1 = false;
+		return 0;
+	default:
+		return 0;
+	}
 }
 
 static const struct sdw_slave_ops cs35l56_sdw_ops = {
 	.read_prop = cs35l56_sdw_read_prop,
 	.interrupt_callback = cs35l56_sdw_interrupt,
 	.update_status = cs35l56_sdw_update_status,
-#ifdef DEBUG
 	.clk_stop = cs35l56_sdw_clk_stop,
-#endif
 };
 
 static int __maybe_unused cs35l56_sdw_handle_unattach(struct cs35l56_private *cs35l56)
 {
 	struct sdw_slave *peripheral = cs35l56->sdw_peripheral;
 
-	if (peripheral->unattach_request) {
+	dev_dbg(cs35l56->base.dev, "attached:%u unattach_request:%u in_clock_stop_1:%u\n",
+		cs35l56->sdw_attached, peripheral->unattach_request, cs35l56->sdw_in_clock_stop_1);
+
+	if (cs35l56->sdw_in_clock_stop_1 || peripheral->unattach_request) {
 		/* Cannot access registers until bus is re-initialized. */
 		dev_dbg(cs35l56->base.dev, "Wait for initialization_complete\n");
 		if (!wait_for_completion_timeout(&peripheral->initialization_complete,
@@ -427,6 +450,7 @@ static int __maybe_unused cs35l56_sdw_handle_unattach(struct cs35l56_private *cs
 		}
 
 		peripheral->unattach_request = 0;
+		cs35l56->sdw_in_clock_stop_1 = false;
 
 		/*
 		 * Don't call regcache_mark_dirty(), we can't be sure that the
diff --git a/sound/soc/codecs/cs35l56.h b/sound/soc/codecs/cs35l56.h
index 747529be3e2e..36d239d571cd 100644
--- a/sound/soc/codecs/cs35l56.h
+++ b/sound/soc/codecs/cs35l56.h
@@ -42,6 +42,7 @@ struct cs35l56_private {
 	bool sdw_irq_no_unmask;
 	bool soft_resetting;
 	bool sdw_attached;
+	bool sdw_in_clock_stop_1;
 	struct completion init_completion;
 
 	int speaker_id;
-- 
2.47.3

On Wed, 11 Mar 2026 14:21:53 +0000, Richard Fitzgerald wrote:
> ASoC: cs35l56: Support clock stop mode 1 if enabled in ACPI

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-7.1

Thanks!

[1/1] ASoC: cs35l56: Support clock stop mode 1 if enabled in ACPI
      https://git.kernel.org/broonie/misc/c/48863104d2e1

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

On Wed, 11 Mar 2026 14:21:53 +0000, Richard Fitzgerald wrote:
> Report the ability to support SoundWire clock-stop mode 1 if this is
> enabled in ACPI. Mode 1 allows the device to lose state, so it can
> reduce power consumption in clock-stop. Also add the necessary
> handling to wait for re-enumeration on resume.
> 
> This does not use sdw_slave_read_prop(), because that also fills in
> other properties from ACPI that were not previously set by the driver
> and this has been observed to break some systems. Instead, the
> "mipi-sdw-clock-stop-mode1-supported" property is checked directly.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: cs35l56: Support clock stop mode 1 if enabled in ACPI
      commit: 48863104d2e1ea70a68cbd8d87c0d75270f3c6e5

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

On Wed, Mar 11, 2026 at 02:21:53PM +0000, Richard Fitzgerald wrote:

> +	ret = fwnode_property_read_u8(dev_fwnode(cs35l56->base.dev),
> +				      "mipi-sdw-clock-stop-mode1-supported",
> +				      &clock_stop_1);
> +	if (ret == 0)
> +		prop->clk_stop_mode1 = !!clock_stop_1;

Should this be using fwnode_property_read_bool()?

On 11/03/2026 3:03 pm, Mark Brown wrote:
> On Wed, Mar 11, 2026 at 02:21:53PM +0000, Richard Fitzgerald wrote:
> 
>> +	ret = fwnode_property_read_u8(dev_fwnode(cs35l56->base.dev),
>> +				      "mipi-sdw-clock-stop-mode1-supported",
>> +				      &clock_stop_1);
>> +	if (ret == 0)
>> +		prop->clk_stop_mode1 = !!clock_stop_1;
> 
> Should this be using fwnode_property_read_bool()?

No, that only reports whether the property exists (it calls
acpi_fwnode_property_present()).

The property can (and usually does) exist with value 0 to disable
clock-stop-mode1.

I copied the use of fwnode_property_read_u8() to read this property from
the implementation of sdw_slave_read_prop().

On Wed, Mar 11, 2026 at 03:17:16PM +0000, Richard Fitzgerald wrote:
> On 11/03/2026 3:03 pm, Mark Brown wrote:
> > On Wed, Mar 11, 2026 at 02:21:53PM +0000, Richard Fitzgerald wrote:

> > > +	ret = fwnode_property_read_u8(dev_fwnode(cs35l56->base.dev),
> > > +				      "mipi-sdw-clock-stop-mode1-supported",
> > > +				      &clock_stop_1);
> > > +	if (ret == 0)
> > > +		prop->clk_stop_mode1 = !!clock_stop_1;

> > Should this be using fwnode_property_read_bool()?

> No, that only reports whether the property exists (it calls
> acpi_fwnode_property_present()).

> The property can (and usually does) exist with value 0 to disable
> clock-stop-mode1.

> I copied the use of fwnode_property_read_u8() to read this property from
> the implementation of sdw_slave_read_prop().

Hrm, OK - the property seems to be confusingly named and defined :(  Can
it have any values other than 0 and 1?

On 11/03/2026 3:30 pm, Mark Brown wrote:
> On Wed, Mar 11, 2026 at 03:17:16PM +0000, Richard Fitzgerald wrote:
>> On 11/03/2026 3:03 pm, Mark Brown wrote:
>>> On Wed, Mar 11, 2026 at 02:21:53PM +0000, Richard Fitzgerald wrote:
> 
>>>> +	ret = fwnode_property_read_u8(dev_fwnode(cs35l56->base.dev),
>>>> +				      "mipi-sdw-clock-stop-mode1-supported",
>>>> +				      &clock_stop_1);
>>>> +	if (ret == 0)
>>>> +		prop->clk_stop_mode1 = !!clock_stop_1;
> 
>>> Should this be using fwnode_property_read_bool()?
> 
>> No, that only reports whether the property exists (it calls
>> acpi_fwnode_property_present()).
> 
>> The property can (and usually does) exist with value 0 to disable
>> clock-stop-mode1.
> 
>> I copied the use of fwnode_property_read_u8() to read this property from
>> the implementation of sdw_slave_read_prop().
> 
> Hrm, OK - the property seems to be confusingly named and defined :(  Can
> it have any values other than 0 and 1?

It's part of the SoundWire spec. We have to follow it.

The current spec says:

mipi-sdw-clock-stop-mode1-supported
This optional Boolean Property shall indicate whether or not Clock-stop
is supported on this Device.
Integer (Boolean)
Values:
False (zero): Clock-stop mode1 is not supported on this Device.
True (nonzero): Clock-stop mode1 is supported on this Device.

On Wed, Mar 11, 2026 at 03:34:54PM +0000, Richard Fitzgerald wrote:
> On 11/03/2026 3:30 pm, Mark Brown wrote:

> > Hrm, OK - the property seems to be confusingly named and defined :(  Can
> > it have any values other than 0 and 1?

> It's part of the SoundWire spec. We have to follow it.

> The current spec says:

> mipi-sdw-clock-stop-mode1-supported
> This optional Boolean Property shall indicate whether or not Clock-stop
> is supported on this Device.
> Integer (Boolean)
> Values:
> False (zero): Clock-stop mode1 is not supported on this Device.
> True (nonzero): Clock-stop mode1 is supported on this Device.

Oh dear, cute enterprise nonsense hacks :(

On 11/03/2026 3:50 pm, Mark Brown wrote:
> On Wed, Mar 11, 2026 at 03:34:54PM +0000, Richard Fitzgerald wrote:
>> On 11/03/2026 3:30 pm, Mark Brown wrote:
> 
>>> Hrm, OK - the property seems to be confusingly named and defined :(  Can
>>> it have any values other than 0 and 1?
> 
>> It's part of the SoundWire spec. We have to follow it.
> 
>> The current spec says:
> 
>> mipi-sdw-clock-stop-mode1-supported
>> This optional Boolean Property shall indicate whether or not Clock-stop
>> is supported on this Device.
>> Integer (Boolean)
>> Values:
>> False (zero): Clock-stop mode1 is not supported on this Device.
>> True (nonzero): Clock-stop mode1 is supported on this Device.
> 
> Oh dear, cute enterprise nonsense hacks :(

The ACPI spec says that each property must be a {key,value} pair.

If you always need a value there's the possibility that someone will
create it with value 0 and expect that to mean false. So relying on its
presence to indicate true is dodgy, safer to define that the value
has meaning.

On Wed, Mar 11, 2026 at 03:58:35PM +0000, Richard Fitzgerald wrote:
> On 11/03/2026 3:50 pm, Mark Brown wrote:
> > On Wed, Mar 11, 2026 at 03:34:54PM +0000, Richard Fitzgerald wrote:

> > > mipi-sdw-clock-stop-mode1-supported
> > > This optional Boolean Property shall indicate whether or not Clock-stop
> > > is supported on this Device.
> > > Integer (Boolean)
> > > Values:
> > > False (zero): Clock-stop mode1 is not supported on this Device.
> > > True (nonzero): Clock-stop mode1 is supported on this Device.

> > Oh dear, cute enterprise nonsense hacks :(

> The ACPI spec says that each property must be a {key,value} pair.

> If you always need a value there's the possibility that someone will
> create it with value 0 and expect that to mean false. So relying on its
> presence to indicate true is dodgy, safer to define that the value
> has meaning.

But of course we have the default bool operation doing the just check
for presence thing, and probably people looking at that for a good way
to do these things.