[PATCH 0/2] Update handling of min and max channel in DisCo

[PATCH 0/2] Update handling of min and max channel in DisCo

[Charles Keepax]

The DisCo properties "mipi-sdw-min-channel-number" and
"mipi-sdw-max-channel-number" are both channel indexes rather than
counts. Update the kernel code to more accurately reflect this.

There are a few Qualcomm drivers that likely specify these incorrect,
min_ch=1, max_ch=X, rather than min_ch=0, max_ch=X-1, however these
should work fine as the number of supported channels is equivalent.
If people feel strongly I can send another series to fix those up.

Charles Keepax (2):
  soundwire: Improve channel min/max comments
  soundwire: Assume default maximum valid channel if not specified

 drivers/soundwire/mipi_disco.c | 14 ++++++++------
 include/linux/soundwire/sdw.h  |  4 ++--
 2 files changed, 10 insertions(+), 8 deletions(-)

-- 
2.47.3

[PATCH 1/2] soundwire: Improve channel min/max comments

[Charles Keepax]

Presently, the comments for the minimum and maximum channels are
misleading. In the SDCA specification these are actually channel
indexes not a count of the number of channels. So for example
min_ch=4 and max_ch=5 means that 1 or 2 channels are supported.
Update the comments associated with the structure to make this
more clear.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 include/linux/soundwire/sdw.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index f710e5932b4b2..22fb7158bb709 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -289,8 +289,8 @@ struct sdw_dp0_prop {
  * @ch_prep_timeout: Port-specific timeout value, in milliseconds
  * @imp_def_interrupts: If set, each bit corresponds to support for
  * implementation-defined interrupts
- * @max_ch: Maximum channels supported
- * @min_ch: Minimum channels supported
+ * @max_ch: Index of the maximum channel supported
+ * @min_ch: Index of the minimum channel supported
  * @num_channels: Number of discrete channels supported
  * @num_ch_combinations: Number of channel combinations supported
  * @channels: Discrete channels supported
-- 
2.47.3

Re: [PATCH 1/2] soundwire: Improve channel min/max comments

[Pierre-Louis Bossart]

On 11/9/26 18:38, Charles Keepax wrote:
> Presently, the comments for the minimum and maximum channels are
> misleading. In the SDCA specification these are actually channel
> indexes not a count of the number of channels. So for example
> min_ch=4 and max_ch=5 means that 1 or 2 channels are supported.
> Update the comments associated with the structure to make this
> more clear.

It's the DisCo spec, not SDCA.

The DisCo spec reads:

"
mipi-sdw-min-channel-number : This optional integer Property shall
indicate the minimum Channel Number for this Audio Port, if the
Audio Port uses consecutive inclusive Channel Numbers.
"

> Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> ---
>  include/linux/soundwire/sdw.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
> index f710e5932b4b2..22fb7158bb709 100644
> --- a/include/linux/soundwire/sdw.h
> +++ b/include/linux/soundwire/sdw.h
> @@ -289,8 +289,8 @@ struct sdw_dp0_prop {
>   * @ch_prep_timeout: Port-specific timeout value, in milliseconds
>   * @imp_def_interrupts: If set, each bit corresponds to support for
>   * implementation-defined interrupts
> - * @max_ch: Maximum channels supported
> - * @min_ch: Minimum channels supported
> + * @max_ch: Index of the maximum channel supported
> + * @min_ch: Index of the minimum channel supported

that's also ambiguous.

Maybe just use the spec definition "minimum Channel Number of this Audio
Port"

>   * @num_channels: Number of discrete channels supported
>   * @num_ch_combinations: Number of channel combinations supported
>   * @channels: Discrete channels supported

Re: [PATCH 1/2] soundwire: Improve channel min/max comments

[Charles Keepax]

On Tue, Nov 10, 2026 at 09:29:00AM +0100, Pierre-Louis Bossart wrote:
> On 11/9/26 18:38, Charles Keepax wrote:
> > Presently, the comments for the minimum and maximum channels are
> > misleading. In the SDCA specification these are actually channel
> > indexes not a count of the number of channels. So for example
> > min_ch=4 and max_ch=5 means that 1 or 2 channels are supported.
> > Update the comments associated with the structure to make this
> > more clear.
> 
> It's the DisCo spec, not SDCA.

Ooops yeah thank you, I will fix.

> > + * @max_ch: Index of the maximum channel supported
> > + * @min_ch: Index of the minimum channel supported
> 
> that's also ambiguous.
> 
> Maybe just use the spec definition "minimum Channel Number of this Audio
> Port"

Yeah I am happy with that wording will update for the spin.

Thanks,
Charles

[PATCH 2/2] soundwire: Assume default maximum valid channel if not specified

[Charles Keepax]

The SDCA specification defines all the supported channel properties
as optional and states that in the case none are specified defaults
of min_ch=0 and max_ch=7 should be used. If both properties are
not present in the DisCo the current code results in min_ch=0 and
max_ch=0. As min_ch and max_ch are both channel indexes, users can't
discern if they should interpret this as the default case (1-8
channels supported) or the DisCo specified a single valid channel of
0 (1 channel supported).

In the case there are no valid supported channel properties populate
max_ch with 7 as the specification suggests, to avoid the ambiguity.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 drivers/soundwire/mipi_disco.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/soundwire/mipi_disco.c b/drivers/soundwire/mipi_disco.c
index 259ad98b5b7a9..eadbda55f377f 100644
--- a/drivers/soundwire/mipi_disco.c
+++ b/drivers/soundwire/mipi_disco.c
@@ -284,12 +284,6 @@ static int sdw_slave_read_dpn(struct sdw_slave *slave,
 				"mipi-sdw-imp-def-dpn-interrupts-supported",
 				&dpn[i].imp_def_interrupts);
 
-		fwnode_property_read_u32(node, "mipi-sdw-min-channel-number",
-					 &dpn[i].min_ch);
-
-		fwnode_property_read_u32(node, "mipi-sdw-max-channel-number",
-					 &dpn[i].max_ch);
-
 		nval = fwnode_property_count_u32(node, "mipi-sdw-channel-number-list");
 		if (nval > 0) {
 			dpn[i].num_channels = nval;
@@ -309,6 +303,14 @@ static int sdw_slave_read_dpn(struct sdw_slave *slave,
 				return ret;
 		}
 
+		fwnode_property_read_u32(node, "mipi-sdw-min-channel-number",
+					 &dpn[i].min_ch);
+
+		ret = fwnode_property_read_u32(node, "mipi-sdw-max-channel-number",
+					       &dpn[i].max_ch);
+		if (ret && !dpn[i].num_channels)
+			dpn[i].max_ch = 7;
+
 		nval = fwnode_property_count_u32(node, "mipi-sdw-channel-combination-list");
 		if (nval > 0) {
 			dpn[i].num_ch_combinations = nval;
-- 
2.47.3