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

[PATCH v3 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.

Changes since v2:
 - Moved the parsing of min/max channels to only be processed if
   num_channels is missing

Changes since v1:
 - Update wording on min_ch/max_ch comments

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 v3 1/2] soundwire: Improve channel min/max comments

[Charles Keepax]

Presently, the comments for the minimum and maximum channels are
misleading. In the SoundWire/Disco 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>
---

No changes since v2.

 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..b8f5ad39e9b37 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: Maximum Channel Number of this Audio Port
+ * @min_ch: 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
-- 
2.47.3

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

[Charles Keepax]

The SoundWire/DisCo 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.
Also as the specification says only parse the channel number properties
if there is no channel number list.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---

Changes since v2:
 - Moved the parsing of min/max channels to only be processed if
   num_channels is missing

 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..2dad95f8e1ab2 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;
@@ -307,6 +301,14 @@ static int sdw_slave_read_dpn(struct sdw_slave *slave,
 					dpn[i].channels, dpn[i].num_channels);
 			if (ret < 0)
 				return ret;
+		} else {
+			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].max_ch = 7;
 		}
 
 		nval = fwnode_property_count_u32(node, "mipi-sdw-channel-combination-list");
-- 
2.47.3