[PATCH v2 0/4] Add static channel mapping between soundwire master and slave

[PATCH v2 0/4] Add static channel mapping between soundwire master and slave

[Mohammad Rafi Shaik]

Add static channel map support between soundwire master and slave.

Currently, the channel mask for each soundwire port is hardcoded in the wcd937x-sdw driver,
and the same channel mask value is configured in the soundwire master.
The Qualcomm boards like the QCM6490-IDP require different channel mask settings
for the soundwire master and slave ports.

With the introduction of the following channel mapping properties, it is now possible to
configure the master channel mask directly from the device tree.

Added qcom_swrm_set_channel_map api to set the master channel mask which allows more
flexible to configure channel mask in runtime for specific active soundwire ports.

Add get and set channel maps support from codec to cpu dais in common
Qualcomm sdw driver.

Mohammad Rafi Shaik (4):
  ASoC: dt-bindings: wcd937x-sdw: Add static channel mapping support
  ASoC: codecs: wcd937x: Add static channel mapping support in
    wcd937x-sdw
  soundwire: qcom: Add set_channel_map api support
  ASoC: qcom: sdw: Add get and set channel maps support from codec to
    cpu dais

 .../bindings/sound/qcom,wcd937x-sdw.yaml      | 36 +++++++++++++
 drivers/soundwire/qcom.c                      | 26 +++++++++
 sound/soc/codecs/wcd937x-sdw.c                | 38 +++++++++++--
 sound/soc/codecs/wcd937x.c                    | 53 ++++++++++++++++++-
 sound/soc/codecs/wcd937x.h                    |  6 ++-
 sound/soc/qcom/sdw.c                          | 34 ++++++++++--
 6 files changed, 183 insertions(+), 10 deletions(-)


base-commit: 7436324ebd147598f940dde1335b7979dbccc339
-- 
2.25.1

[PATCH v2 1/4] ASoC: dt-bindings: wcd937x-sdw: Add static channel mapping support

[Mohammad Rafi Shaik]

Add static channel mapping between master and slave rx/tx ports for
Qualcomm wcd937x soundwire codec.

Currently, the channel mask for each soundwire port is hardcoded in the
wcd937x-sdw driver, and the same channel mask value is configured in the
soundwire master.

The Qualcomm boards like the QCM6490-IDP require different channel mask settings
for the soundwire master and slave ports.

With the introduction of the following channel mapping properties, it is now possible
to configure the master channel mask directly from the device tree.

The qcom,tx-channel-mapping property specifies the static channel mapping between the slave
and master tx ports in the order of slave port channels which is adc1, adc2, adc3, adc4,
dmic0, dmic1, mbhc, dmic2, dmic3, dmci4, dmic5, dmic6, dmic7.

The qcom,rx-channel-mapping property specifies static channel mapping between the slave
and master rx ports in the order of slave port channels which is hph_l, hph_r, clsh,
comp_l, comp_r, lo, dsd_r, dsd_l.

Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com>
---
 .../bindings/sound/qcom,wcd937x-sdw.yaml      | 36 +++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml
index d3cf8f59cb23..a6bc9b391db0 100644
--- a/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml
+++ b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml
@@ -58,6 +58,38 @@ properties:
     items:
       enum: [1, 2, 3, 4, 5]
 
+  qcom,tx-channel-mapping:
+    description: |
+      Specifies static channel mapping between slave and master tx port
+      channels.
+      In the order of slave port channels which is adc1, adc2, adc3, adc4,
+      dmic0, dmic1, mbhc, dmic2, dmic3, dmci4, dmic5, dmic6, dmic7.
+      ch_mask1 ==> bit mask value 1
+      ch_mask2 ==> bit mask value 2
+      ch_mask3 ==> bit mask value 4
+      ch_mask4 ==> bit mask value 8
+    $ref: /schemas/types.yaml#/definitions/uint8-array
+    minItems: 8
+    maxItems: 13
+    items:
+      enum: [1, 2, 4, 8]
+
+  qcom,rx-channel-mapping:
+    description: |
+      Specifies static channels mapping between slave and master rx port
+      channels.
+      In the order of slave port channels, which is
+      hph_l, hph_r, clsh, comp_l, comp_r, lo, dsd_r, dsd_l.
+      ch_mask1 ==> bit mask value 1
+      ch_mask2 ==> bit mask value 2
+      ch_mask3 ==> bit mask value 4
+      ch_mask4 ==> bit mask value 8
+    $ref: /schemas/types.yaml#/definitions/uint8-array
+    minItems: 8
+    maxItems: 8
+    items:
+      enum: [1, 2, 4, 8]
+
 required:
   - compatible
   - reg
@@ -74,6 +106,8 @@ examples:
             compatible = "sdw20217010a00";
             reg = <0 4>;
             qcom,rx-port-mapping = <1 2 3 4 5>;
+            qcom,rx-channel-mapping =  /bits/ 8 <0x01 0x02 0x01 0x01 0x02
+                                                 0x01 0x01 0x02>;
         };
     };
 
@@ -85,6 +119,8 @@ examples:
             compatible = "sdw20217010a00";
             reg = <0 3>;
             qcom,tx-port-mapping = <2 2 3 4>;
+            qcom,tx-channel-mapping = /bits/ 8 <0x01 0x02 0x01 0x01 0x02 0x04
+                                                0x04 0x08 0x01 0x02 0x04 0x8>;
         };
     };
 
-- 
2.25.1

[PATCH v2 2/4] ASoC: codecs: wcd937x: Add static channel mapping support in wcd937x-sdw

[Mohammad Rafi Shaik]

Add static channel mapping between master and slave ports in wcd937x-sdw driver.

Currently, the channel mask for each soundwire port is hardcoded in the
wcd937x-sdw driver, and the same channel mask value is configured in the
soundwire master.

The Qualcomm boards like the QCM6490-IDP require different channel mask settings
for the soundwire master and slave ports.

Implemented logic to read TX/RX channel mappings from device tree properties
(qcom,tx-channel-mapping and qcom,rx-channel-mapping).

Modified the wcd937x_connect_port to handle master channel masks during port
enable/disable operations.

Added wcd937x_get_channel_map api to retrieve the current master channel map
for TX and RX paths.

Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com>
---
 sound/soc/codecs/wcd937x-sdw.c | 38 +++++++++++++++++++++---
 sound/soc/codecs/wcd937x.c     | 53 ++++++++++++++++++++++++++++++++--
 sound/soc/codecs/wcd937x.h     |  6 +++-
 3 files changed, 90 insertions(+), 7 deletions(-)

diff --git a/sound/soc/codecs/wcd937x-sdw.c b/sound/soc/codecs/wcd937x-sdw.c
index 0c33f7f3dc25..3263bdf25d86 100644
--- a/sound/soc/codecs/wcd937x-sdw.c
+++ b/sound/soc/codecs/wcd937x-sdw.c
@@ -19,7 +19,7 @@
 #include <sound/soc.h>
 #include "wcd937x.h"
 
-static const struct wcd937x_sdw_ch_info wcd937x_sdw_rx_ch_info[] = {
+static struct wcd937x_sdw_ch_info wcd937x_sdw_rx_ch_info[] = {
 	WCD_SDW_CH(WCD937X_HPH_L, WCD937X_HPH_PORT, BIT(0)),
 	WCD_SDW_CH(WCD937X_HPH_R, WCD937X_HPH_PORT, BIT(1)),
 	WCD_SDW_CH(WCD937X_CLSH, WCD937X_CLSH_PORT, BIT(0)),
@@ -30,7 +30,7 @@ static const struct wcd937x_sdw_ch_info wcd937x_sdw_rx_ch_info[] = {
 	WCD_SDW_CH(WCD937X_DSD_R, WCD937X_DSD_PORT, BIT(1)),
 };
 
-static const struct wcd937x_sdw_ch_info wcd937x_sdw_tx_ch_info[] = {
+static struct wcd937x_sdw_ch_info wcd937x_sdw_tx_ch_info[] = {
 	WCD_SDW_CH(WCD937X_ADC1, WCD937X_ADC_1_PORT, BIT(0)),
 	WCD_SDW_CH(WCD937X_ADC2, WCD937X_ADC_2_3_PORT, BIT(0)),
 	WCD_SDW_CH(WCD937X_ADC3, WCD937X_ADC_2_3_PORT, BIT(0)),
@@ -1019,7 +1019,9 @@ static int wcd9370_probe(struct sdw_slave *pdev,
 {
 	struct device *dev = &pdev->dev;
 	struct wcd937x_sdw_priv *wcd;
-	int ret;
+	u8 master_ch_mask[WCD937X_MAX_SWR_CH_IDS];
+	int master_ch_mask_size = 0;
+	int ret, i;
 
 	wcd = devm_kzalloc(dev, sizeof(*wcd), GFP_KERNEL);
 	if (!wcd)
@@ -1048,10 +1050,36 @@ static int wcd9370_probe(struct sdw_slave *pdev,
 				   SDW_SCP_INT1_PARITY;
 	pdev->prop.lane_control_support = true;
 	pdev->prop.simple_clk_stop_capable = true;
+
+	memset(master_ch_mask, 0, WCD937X_MAX_SWR_CH_IDS);
+
+	if (wcd->is_tx) {
+		master_ch_mask_size = of_property_count_u8_elems(dev->of_node,
+								 "qcom,tx-channel-mapping");
+
+		if (master_ch_mask_size)
+			ret = of_property_read_u8_array(dev->of_node, "qcom,tx-channel-mapping",
+							master_ch_mask, master_ch_mask_size);
+	} else {
+		master_ch_mask_size = of_property_count_u8_elems(dev->of_node,
+								 "qcom,rx-channel-mapping");
+
+		if (master_ch_mask_size)
+			ret = of_property_read_u8_array(dev->of_node, "qcom,rx-channel-mapping",
+							master_ch_mask, master_ch_mask_size);
+	}
+
+	if (ret < 0)
+		dev_info(dev, "Static channel mapping not specified using device channel maps\n");
+
 	if (wcd->is_tx) {
-		pdev->prop.source_ports = GENMASK(WCD937X_MAX_TX_SWR_PORTS - 1, 0);
+		pdev->prop.source_ports = GENMASK(WCD937X_MAX_TX_SWR_PORTS, 0);
 		pdev->prop.src_dpn_prop = wcd937x_dpn_prop;
 		wcd->ch_info = &wcd937x_sdw_tx_ch_info[0];
+
+		for (i = 0; i < master_ch_mask_size; i++)
+			wcd->ch_info[i].master_ch_mask = master_ch_mask[i];
+
 		pdev->prop.wake_capable = true;
 
 		wcd->regmap = devm_regmap_init_sdw(pdev, &wcd937x_regmap_config);
@@ -1065,6 +1093,8 @@ static int wcd9370_probe(struct sdw_slave *pdev,
 		pdev->prop.sink_ports = GENMASK(WCD937X_MAX_SWR_PORTS - 1, 0);
 		pdev->prop.sink_dpn_prop = wcd937x_dpn_prop;
 		wcd->ch_info = &wcd937x_sdw_rx_ch_info[0];
+		for (i = 0; i < master_ch_mask_size; i++)
+			wcd->ch_info[i].master_ch_mask = master_ch_mask[i];
 	}
 
 
diff --git a/sound/soc/codecs/wcd937x.c b/sound/soc/codecs/wcd937x.c
index 45f32d281908..c0b18a3da701 100644
--- a/sound/soc/codecs/wcd937x.c
+++ b/sound/soc/codecs/wcd937x.c
@@ -1192,13 +1192,21 @@ static int wcd937x_connect_port(struct wcd937x_sdw_priv *wcd, u8 port_idx, u8 ch
 	const struct wcd937x_sdw_ch_info *ch_info = &wcd->ch_info[ch_id];
 	u8 port_num = ch_info->port_num;
 	u8 ch_mask = ch_info->ch_mask;
+	u8 mstr_port_num, mstr_ch_mask;
+	struct sdw_slave *sdev = wcd->sdev;
 
 	port_config->num = port_num;
 
-	if (enable)
+	mstr_port_num = sdev->m_port_map[port_num];
+	mstr_ch_mask = ch_info->master_ch_mask;
+
+	if (enable) {
 		port_config->ch_mask |= ch_mask;
-	else
+		wcd->master_channel_map[mstr_port_num] |= mstr_ch_mask;
+	} else {
 		port_config->ch_mask &= ~ch_mask;
+		wcd->master_channel_map[mstr_port_num] &= ~mstr_ch_mask;
+	}
 
 	return 0;
 }
@@ -2682,10 +2690,51 @@ static int wcd937x_codec_set_sdw_stream(struct snd_soc_dai *dai,
 	return 0;
 }
 
+static int wcd937x_get_channel_map(const struct snd_soc_dai *dai,
+				   unsigned int *tx_num, unsigned int *tx_slot,
+				   unsigned int *rx_num, unsigned int *rx_slot)
+{
+	struct wcd937x_priv *wcd937x = dev_get_drvdata(dai->dev);
+	struct wcd937x_sdw_priv *wcd = wcd937x->sdw_priv[dai->id];
+	int i;
+
+	switch (dai->id) {
+	case AIF1_PB:
+		if (!rx_slot || !rx_num) {
+			dev_err(dai->dev, "Invalid rx_slot %p or rx_num %p\n",
+				rx_slot, rx_num);
+			return -EINVAL;
+		}
+
+		for (i = 0; i < SDW_MAX_PORTS; i++)
+			rx_slot[i] = wcd->master_channel_map[i];
+
+		*rx_num = i;
+		break;
+	case AIF1_CAP:
+		if (!tx_slot || !tx_num) {
+			dev_err(dai->dev, "Invalid tx_slot %p or tx_num %p\n",
+				tx_slot, tx_num);
+			return -EINVAL;
+		}
+
+		for (i = 0; i < SDW_MAX_PORTS; i++)
+			tx_slot[i] = wcd->master_channel_map[i];
+
+		*tx_num = i;
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
 static const struct snd_soc_dai_ops wcd937x_sdw_dai_ops = {
 	.hw_params = wcd937x_codec_hw_params,
 	.hw_free = wcd937x_codec_free,
 	.set_stream = wcd937x_codec_set_sdw_stream,
+	.get_channel_map = wcd937x_get_channel_map,
 };
 
 static struct snd_soc_dai_driver wcd937x_dais[] = {
diff --git a/sound/soc/codecs/wcd937x.h b/sound/soc/codecs/wcd937x.h
index 35f3d48bd7dd..850aa943b7ab 100644
--- a/sound/soc/codecs/wcd937x.h
+++ b/sound/soc/codecs/wcd937x.h
@@ -506,12 +506,14 @@ enum wcd937x_rx_sdw_ports {
 struct wcd937x_sdw_ch_info {
 	int port_num;
 	unsigned int ch_mask;
+	unsigned int master_ch_mask;
 };
 
 #define WCD_SDW_CH(id, pn, cmask)	\
 	[id] = {			\
 		.port_num = pn,		\
 		.ch_mask = cmask,	\
+		.master_ch_mask = cmask,	\
 	}
 
 struct wcd937x_priv;
@@ -520,9 +522,11 @@ struct wcd937x_sdw_priv {
 	struct sdw_stream_config sconfig;
 	struct sdw_stream_runtime *sruntime;
 	struct sdw_port_config port_config[WCD937X_MAX_SWR_PORTS];
-	const struct wcd937x_sdw_ch_info *ch_info;
+	struct wcd937x_sdw_ch_info *ch_info;
 	bool port_enable[WCD937X_MAX_SWR_CH_IDS];
+	unsigned int master_channel_map[SDW_MAX_PORTS];
 	int active_ports;
+	int num_ports;
 	bool is_tx;
 	struct wcd937x_priv *wcd937x;
 	struct irq_domain *slave_irq;
-- 
2.25.1

[PATCH v2 3/4] soundwire: qcom: Add set_channel_map api support

[Mohammad Rafi Shaik]

Added qcom_swrm_set_channel_map api to set the master channel mask for
TX and RX paths based on the provided slots.

Added a new field ch_mask to the qcom_swrm_port_config structure.
This field is used to store the master channel mask, which allows more
flexible to configure channel mask in runtime for specific active soundwire ports.

Modified the qcom_swrm_port_enable function to configure master channel mask.
If the ch_mask is set to SWR_INVALID_PARAM or is zero, the function will
use the default channel mask.

Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com>
---
 drivers/soundwire/qcom.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 2b403b14066c..007183c6c047 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -156,6 +156,7 @@ struct qcom_swrm_port_config {
 	u8 word_length;
 	u8 blk_group_count;
 	u8 lane_control;
+	u8 ch_mask;
 };
 
 /*
@@ -1048,9 +1049,13 @@ static int qcom_swrm_port_enable(struct sdw_bus *bus,
 {
 	u32 reg = SWRM_DP_PORT_CTRL_BANK(enable_ch->port_num, bank);
 	struct qcom_swrm_ctrl *ctrl = to_qcom_sdw(bus);
+	struct qcom_swrm_port_config *pcfg;
 	u32 val;
 
+	pcfg = &ctrl->pconfig[enable_ch->port_num];
 	ctrl->reg_read(ctrl, reg, &val);
+	if (pcfg->ch_mask != SWR_INVALID_PARAM && pcfg->ch_mask != 0)
+		enable_ch->ch_mask = pcfg->ch_mask;
 
 	if (enable_ch->enable)
 		val |= (enable_ch->ch_mask << SWRM_DP_PORT_CTRL_EN_CHAN_SHFT);
@@ -1270,6 +1275,27 @@ static void *qcom_swrm_get_sdw_stream(struct snd_soc_dai *dai, int direction)
 	return ctrl->sruntime[dai->id];
 }
 
+static int qcom_swrm_set_channel_map(struct snd_soc_dai *dai,
+				     unsigned int tx_num, unsigned int *tx_slot,
+				     unsigned int rx_num, unsigned int *rx_slot)
+{
+	struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dai->dev);
+	struct sdw_stream_runtime *sruntime = ctrl->sruntime[dai->id];
+	int i;
+
+	if (tx_slot) {
+		for (i = 0; i < tx_num; i++)
+			ctrl->pconfig[i].ch_mask = tx_slot[i];
+	}
+
+	if (rx_slot) {
+		for (i = 0; i < rx_num; i++)
+			ctrl->pconfig[i].ch_mask = rx_slot[i];
+	}
+
+	return 0;
+}
+
 static int qcom_swrm_startup(struct snd_pcm_substream *substream,
 			     struct snd_soc_dai *dai)
 {
@@ -1306,6 +1332,7 @@ static const struct snd_soc_dai_ops qcom_swrm_pdm_dai_ops = {
 	.shutdown = qcom_swrm_shutdown,
 	.set_stream = qcom_swrm_set_sdw_stream,
 	.get_stream = qcom_swrm_get_sdw_stream,
+	.set_channel_map = qcom_swrm_set_channel_map,
 };
 
 static const struct snd_soc_component_driver qcom_swrm_dai_component = {
-- 
2.25.1

[PATCH v2 4/4] ASoC: qcom: sdw: Add get and set channel maps support from codec to cpu dais

[Mohammad Rafi Shaik]

Add get and set channel maps support from codec to cpu dais.

Implemented logic to get the channel map in case of only sdw stream and
set channel map only for specific cpu dais.

Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com>
---
 drivers/soundwire/qcom.c |  5 ++---
 sound/soc/qcom/sdw.c     | 34 +++++++++++++++++++++++++++++++---
 2 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 007183c6c047..6c3cff1194aa 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -1276,11 +1276,10 @@ static void *qcom_swrm_get_sdw_stream(struct snd_soc_dai *dai, int direction)
 }
 
 static int qcom_swrm_set_channel_map(struct snd_soc_dai *dai,
-				     unsigned int tx_num, unsigned int *tx_slot,
-				     unsigned int rx_num, unsigned int *rx_slot)
+				     unsigned int tx_num, const unsigned int *tx_slot,
+				     unsigned int rx_num, const unsigned int *rx_slot)
 {
 	struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dai->dev);
-	struct sdw_stream_runtime *sruntime = ctrl->sruntime[dai->id];
 	int i;
 
 	if (tx_slot) {
diff --git a/sound/soc/qcom/sdw.c b/sound/soc/qcom/sdw.c
index f2eda2ff46c0..d4d8ed46e6ff 100644
--- a/sound/soc/qcom/sdw.c
+++ b/sound/soc/qcom/sdw.c
@@ -25,7 +25,9 @@ int qcom_snd_sdw_startup(struct snd_pcm_substream *substream)
 	struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0);
 	struct sdw_stream_runtime *sruntime;
 	struct snd_soc_dai *codec_dai;
-	int ret, i;
+	int ret, i, j;
+	u32 rx_ch[SDW_MAX_PORTS], tx_ch[SDW_MAX_PORTS];
+	u32 rx_ch_cnt = 0, tx_ch_cnt = 0;
 
 	sruntime = sdw_alloc_stream(cpu_dai->name);
 	if (!sruntime)
@@ -35,9 +37,35 @@ int qcom_snd_sdw_startup(struct snd_pcm_substream *substream)
 		ret = snd_soc_dai_set_stream(codec_dai, sruntime,
 					     substream->stream);
 		if (ret < 0 && ret != -ENOTSUPP) {
-			dev_err(rtd->dev, "Failed to set sdw stream on %s\n",
-				codec_dai->name);
+			dev_err(rtd->dev, "Failed to set sdw stream on %s\n", codec_dai->name);
 			goto err_set_stream;
+		} else if (ret == -ENOTSUPP) {
+			/* Ignore unsupported */
+			continue;
+		}
+
+		ret = snd_soc_dai_get_channel_map(codec_dai, &tx_ch_cnt, tx_ch,
+						  &rx_ch_cnt, rx_ch);
+		if (ret != 0 && ret != -ENOTSUPP) {
+			dev_err(rtd->dev, "Failed to get codec chan map %s\n", codec_dai->name);
+			goto err_set_stream;
+		} else if (ret == -ENOTSUPP) {
+			/* Ignore unsupported */
+			continue;
+		}
+	}
+
+	switch (cpu_dai->id) {
+	case RX_CODEC_DMA_RX_0:
+	case TX_CODEC_DMA_TX_3:
+		if (tx_ch_cnt || rx_ch_cnt) {
+			for_each_rtd_codec_dais(rtd, j, codec_dai) {
+				ret = snd_soc_dai_set_channel_map(codec_dai,
+								  tx_ch_cnt, tx_ch,
+								  rx_ch_cnt, rx_ch);
+				if (ret != 0 && ret != -ENOTSUPP)
+					goto err_set_stream;
+			}
 		}
 	}
 
-- 
2.25.1

Re: [PATCH v2 1/4] ASoC: dt-bindings: wcd937x-sdw: Add static channel mapping support

[Krzysztof Kozlowski]

On Wed, Oct 23, 2024 at 11:43:23AM +0530, Mohammad Rafi Shaik wrote:
> Add static channel mapping between master and slave rx/tx ports for
> Qualcomm wcd937x soundwire codec.
> 
> Currently, the channel mask for each soundwire port is hardcoded in the
> wcd937x-sdw driver, and the same channel mask value is configured in the
> soundwire master.
> 
> The Qualcomm boards like the QCM6490-IDP require different channel mask settings
> for the soundwire master and slave ports.

Different than what? Other wcd937x? Which are these?

> 
> With the introduction of the following channel mapping properties, it is now possible
> to configure the master channel mask directly from the device tree.
> 
> The qcom,tx-channel-mapping property specifies the static channel mapping between the slave
> and master tx ports in the order of slave port channels which is adc1, adc2, adc3, adc4,
> dmic0, dmic1, mbhc, dmic2, dmic3, dmci4, dmic5, dmic6, dmic7.

I still don't get what is the channel here.

> 
> The qcom,rx-channel-mapping property specifies static channel mapping between the slave
> and master rx ports in the order of slave port channels which is hph_l, hph_r, clsh,
> comp_l, comp_r, lo, dsd_r, dsd_l.

And this description copies binding :/.

Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597

> 
> Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com>
> ---
>  .../bindings/sound/qcom,wcd937x-sdw.yaml      | 36 +++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml
> index d3cf8f59cb23..a6bc9b391db0 100644
> --- a/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml
> +++ b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml
> @@ -58,6 +58,38 @@ properties:
>      items:
>        enum: [1, 2, 3, 4, 5]
>  
> +  qcom,tx-channel-mapping:
> +    description: |
> +      Specifies static channel mapping between slave and master tx port
> +      channels.
> +      In the order of slave port channels which is adc1, adc2, adc3, adc4,
> +      dmic0, dmic1, mbhc, dmic2, dmic3, dmci4, dmic5, dmic6, dmic7.
> +      ch_mask1 ==> bit mask value 1
> +      ch_mask2 ==> bit mask value 2
> +      ch_mask3 ==> bit mask value 4
> +      ch_mask4 ==> bit mask value 8
> +    $ref: /schemas/types.yaml#/definitions/uint8-array
> +    minItems: 8
> +    maxItems: 13

Why size is variable? This device has fixed amount of slave ports, I
think.

> +    items:
> +      enum: [1, 2, 4, 8]

What is the point of using bits if you cannot actually create a bit mask
out of it? Why this cannot be 7?

> +
> +  qcom,rx-channel-mapping:
> +    description: |
> +      Specifies static channels mapping between slave and master rx port
> +      channels.
> +      In the order of slave port channels, which is
> +      hph_l, hph_r, clsh, comp_l, comp_r, lo, dsd_r, dsd_l.
> +      ch_mask1 ==> bit mask value 1
> +      ch_mask2 ==> bit mask value 2
> +      ch_mask3 ==> bit mask value 4
> +      ch_mask4 ==> bit mask value 8

and the value is what exactly? Index is channel, but what does "ch_mask4 ==> bit
mask value 8" mean? I don't understand this at all.

> +    $ref: /schemas/types.yaml#/definitions/uint8-array
> +    minItems: 8
> +    maxItems: 8
> +    items:
> +      enum: [1, 2, 4, 8]
> +
>  required:
>    - compatible
>    - reg
> @@ -74,6 +106,8 @@ examples:
>              compatible = "sdw20217010a00";
>              reg = <0 4>;
>              qcom,rx-port-mapping = <1 2 3 4 5>;
> +            qcom,rx-channel-mapping =  /bits/ 8 <0x01 0x02 0x01 0x01 0x02
> +                                                 0x01 0x01 0x02>;
>          };
>      };
>  
> @@ -85,6 +119,8 @@ examples:
>              compatible = "sdw20217010a00";
>              reg = <0 3>;
>              qcom,tx-port-mapping = <2 2 3 4>;
> +            qcom,tx-channel-mapping = /bits/ 8 <0x01 0x02 0x01 0x01 0x02 0x04
> +                                                0x04 0x08 0x01 0x02 0x04 0x8>;

Keep it consistent, e.g. everywhere without leading 0... actually not
sure why this is hex, either.

>          };
>      };
>  
> -- 
> 2.25.1
> 

Re: [PATCH v2 1/4] ASoC: dt-bindings: wcd937x-sdw: Add static channel mapping support

[Krzysztof Kozlowski]

On 23/10/2024 09:52, Krzysztof Kozlowski wrote:
> On Wed, Oct 23, 2024 at 11:43:23AM +0530, Mohammad Rafi Shaik wrote:
>> Add static channel mapping between master and slave rx/tx ports for
>> Qualcomm wcd937x soundwire codec.
>>
>> Currently, the channel mask for each soundwire port is hardcoded in the
>> wcd937x-sdw driver, and the same channel mask value is configured in the
>> soundwire master.
>>
>> The Qualcomm boards like the QCM6490-IDP require different channel mask settings
>> for the soundwire master and slave ports.
> 
> Different than what? Other wcd937x? Which are these?
> 
>>
>> With the introduction of the following channel mapping properties, it is now possible
>> to configure the master channel mask directly from the device tree.
>>
>> The qcom,tx-channel-mapping property specifies the static channel mapping between the slave
>> and master tx ports in the order of slave port channels which is adc1, adc2, adc3, adc4,
>> dmic0, dmic1, mbhc, dmic2, dmic3, dmci4, dmic5, dmic6, dmic7.
> 
> I still don't get what is the channel here.
> 
>>
>> The qcom,rx-channel-mapping property specifies static channel mapping between the slave
>> and master rx ports in the order of slave port channels which is hph_l, hph_r, clsh,
>> comp_l, comp_r, lo, dsd_r, dsd_l.
> 
> And this description copies binding :/.
> 
> Please wrap commit message according to Linux coding style / submission
> process (neither too early nor over the limit):
> https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597
> 
>>
>> Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com>
>> ---
>>  .../bindings/sound/qcom,wcd937x-sdw.yaml      | 36 +++++++++++++++++++
>>  1 file changed, 36 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml
>> index d3cf8f59cb23..a6bc9b391db0 100644
>> --- a/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml
>> +++ b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml
>> @@ -58,6 +58,38 @@ properties:
>>      items:
>>        enum: [1, 2, 3, 4, 5]
>>  
>> +  qcom,tx-channel-mapping:
>> +    description: |
>> +      Specifies static channel mapping between slave and master tx port
>> +      channels.
>> +      In the order of slave port channels which is adc1, adc2, adc3, adc4,
>> +      dmic0, dmic1, mbhc, dmic2, dmic3, dmci4, dmic5, dmic6, dmic7.
>> +      ch_mask1 ==> bit mask value 1
>> +      ch_mask2 ==> bit mask value 2
>> +      ch_mask3 ==> bit mask value 4
>> +      ch_mask4 ==> bit mask value 8
>> +    $ref: /schemas/types.yaml#/definitions/uint8-array
>> +    minItems: 8
>> +    maxItems: 13
> 
> Why size is variable? This device has fixed amount of slave ports, I
> think.
> 
>> +    items:
>> +      enum: [1, 2, 4, 8]
> 
> What is the point of using bits if you cannot actually create a bit mask
> out of it? Why this cannot be 7?
> 
>> +
>> +  qcom,rx-channel-mapping:
>> +    description: |
>> +      Specifies static channels mapping between slave and master rx port
>> +      channels.
>> +      In the order of slave port channels, which is
>> +      hph_l, hph_r, clsh, comp_l, comp_r, lo, dsd_r, dsd_l.
>> +      ch_mask1 ==> bit mask value 1
>> +      ch_mask2 ==> bit mask value 2
>> +      ch_mask3 ==> bit mask value 4
>> +      ch_mask4 ==> bit mask value 8
> 
> and the value is what exactly? Index is channel, but what does "ch_mask4 ==> bit
> mask value 8" mean? I don't understand this at all.

Ah, and previous feedback was to use strings, no?

Best regards,
Krzysztof

Re: [PATCH v2 1/4] ASoC: dt-bindings: wcd937x-sdw: Add static channel mapping support

[Mohammad Rafi Shaik]

On 10/23/2024 1:22 PM, Krzysztof Kozlowski wrote:
> On Wed, Oct 23, 2024 at 11:43:23AM +0530, Mohammad Rafi Shaik wrote:
>> Add static channel mapping between master and slave rx/tx ports for
>> Qualcomm wcd937x soundwire codec.
>>
>> Currently, the channel mask for each soundwire port is hardcoded in the
>> wcd937x-sdw driver, and the same channel mask value is configured in the
>> soundwire master.
>>
>> The Qualcomm boards like the QCM6490-IDP require different channel mask settings
>> for the soundwire master and slave ports.
> 
> Different than what? Other wcd937x? Which are these?
> 
For Qualcomm QCM6490-IDP board soundwire master needs a different 
channel mask setting.

The wcd937x channel mask values are hardcoded in wcd driver.
https://elixir.bootlin.com/linux/v6.12-rc5/source/sound/soc/codecs/wcd937x-sdw.c#L35
https://elixir.bootlin.com/linux/v6.12-rc5/source/sound/soc/codecs/wcd938x-sdw.c#L37

In case of QCM6490-IDP the soundwire master and wcd937x require 
different channel mask settings, not the same.
For Example, wcd937x ADC2 connection

                           Master                 Slave (wcd937x)
                      +--------------+           +--------------+
                      |  +--------+  |           |  +--------+  |
          ADC1  ----->|  | PORT1  |  |           |  |   TX1  | 
|<-----------ADC1
          ADC2  ----->|  |        |  |           |  |        |  |
                      |  +--------+  |           |  +--------+  |
                      |              |           |              |
          ADC3  ----->|  +--------+  |           |  +--------+  |
                      |  |  PORT2 |  |           |  |   TX2  | 
|<-----------ADC2
                      |  |        |  |           |  |        | 
|<-----------ADC3
                      |  +--------+  |           |  +--------+  |
                      |              |           |              |
                      |  +--------+  |           |  +--------+  |
  DMIC0...DMIC3------>|  |  PORT3 |  |           |  |   TX3  | 
|<-----------DMIC0...DMIC3
                      |  |        |  |           |  |        | 
|<-----------MBHC
                      |  +--------+  |           |  +--------+  |
                      |              |           |              |
                      |  +--------+  |           |  +--------+  |
  DMIC4...DMIC3 ----->|  |  PORT4 |  |           |  |   TX4  | 
|<-----------DMIC4...DMIC7
                      |  |        |  |           |  |        |  |
                      |  +--------+  |           |  +--------+  |
                      |              |           |              |
                      +------------- +           +--------------+

					
For ADC2, The Slave needs to configure TX2 Port with channel mask value 
1 and
For Master, it required PORT1 with channel mask  value 2.


In existing design master and slave configured with same channel mask, 
it will fail ADC2.
The new design will help to configure channel mapping between master and 
slave from DT.

>>
>> With the introduction of the following channel mapping properties, it is now possible
>> to configure the master channel mask directly from the device tree.
>>
>> The qcom,tx-channel-mapping property specifies the static channel mapping between the slave
>> and master tx ports in the order of slave port channels which is adc1, adc2, adc3, adc4,
>> dmic0, dmic1, mbhc, dmic2, dmic3, dmci4, dmic5, dmic6, dmic7.
> 
> I still don't get what is the channel here.
> 
Typo error,

The qcom,tx-channel-mapping property specifies the static channel 
mapping between the slave

and master tx ports in the order of slave port channel index which are 
adc1, adc2, adc3, adc4,

dmic0, dmic1, mbhc, dmic2, dmic3, dmci4, dmic5, dmic6, dmic7.



https://elixir.bootlin.com/linux/v6.12-rc5/source/sound/soc/codecs/wcd937x.h#L599



Will be fixed in the next version

>>
>> The qcom,rx-channel-mapping property specifies static channel mapping between the slave
>> and master rx ports in the order of slave port channels which is hph_l, hph_r, clsh,
>> comp_l, comp_r, lo, dsd_r, dsd_l.
> 
> And this description copies binding :/.
> 
> Please wrap commit message according to Linux coding style / submission
> process (neither too early nor over the limit):
> https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597
>

Ack

>>
>> Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com>
>> ---
>>   .../bindings/sound/qcom,wcd937x-sdw.yaml      | 36 +++++++++++++++++++
>>   1 file changed, 36 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml
>> index d3cf8f59cb23..a6bc9b391db0 100644
>> --- a/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml
>> +++ b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml
>> @@ -58,6 +58,38 @@ properties:
>>       items:
>>         enum: [1, 2, 3, 4, 5]
>>   
>> +  qcom,tx-channel-mapping:
>> +    description: |
>> +      Specifies static channel mapping between slave and master tx port
>> +      channels.
>> +      In the order of slave port channels which is adc1, adc2, adc3, adc4,
>> +      dmic0, dmic1, mbhc, dmic2, dmic3, dmci4, dmic5, dmic6, dmic7.
>> +      ch_mask1 ==> bit mask value 1
>> +      ch_mask2 ==> bit mask value 2
>> +      ch_mask3 ==> bit mask value 4
>> +      ch_mask4 ==> bit mask value 8
>> +    $ref: /schemas/types.yaml#/definitions/uint8-array
>> +    minItems: 8
>> +    maxItems: 13
> 
> Why size is variable? This device has fixed amount of slave ports, I
> think.
>

yes will check modify

>> +    items:
>> +      enum: [1, 2, 4, 8]
> 
> What is the point of using bits if you cannot actually create a bit mask
> out of it? Why this cannot be 7?
> 
Actually, these values should be fixed: 1 (0001), 2 (0010), 4(0100), 
8(1000).


If required to set 7, it is handled in wcd driver based on mixer commands.
https://elixir.bootlin.com/linux/v6.12-rc5/source/sound/soc/codecs/wcd937x.c#L1199


Example:
WCD937X_HPH_L -> channel mask value is 1
WCD937X_HPH_R -> channel mask value is 2


The final channel mask for that specific port is 3
>> +
>> +  qcom,rx-channel-mapping:
>> +    description: |
>> +      Specifies static channels mapping between slave and master rx port
>> +      channels.
>> +      In the order of slave port channels, which is
>> +      hph_l, hph_r, clsh, comp_l, comp_r, lo, dsd_r, dsd_l.
>> +      ch_mask1 ==> bit mask value 1
>> +      ch_mask2 ==> bit mask value 2
>> +      ch_mask3 ==> bit mask value 4
>> +      ch_mask4 ==> bit mask value 8
> 
> and the value is what exactly? Index is channel, but what does "ch_mask4 ==> bit
> mask value 8" mean? I don't understand this at all.
> 

                           Master
                      +--------------+
                      |  +--------+  |
           ADC1 ----->|  | PORT1  |  |
           ADC2 ----->|  |        |  |
                      |  +--------+  |
                      |              |
           ADC3 ----->|  +--------+  |
                      |  |  PORT2 |  |
                      |  |        |  |
                      |  +--------+  |
                      |              |
                      |  +--------+  |
   DMIC0...DMIC3 ---->|  |  PORT3 |  |
                      |  |        |  |
                      |  +--------+  |
                      |              |
                      |  +--------+  |
   DMIC4...DMIC7----->|  |  PORT4 |  |
                      |  |        |  |
                      |  +--------+  |
                      |              |
                      +------------- +


The PORT1 has 2 ADC connections,

ADC1 -> PORT1 ch_mask index 1 -> channel mask value 1 (0001)
ADC2 -> PORT1 ch_mask index 2 -> channel mask value 2 (0010)


DMIC0 -> PORT3 ch_mask index 1 -> channel mask value 1 (0001)
DMIC1 -> PORT3 ch_mask index 2 -> channel mask value 2 (0010)
DMIC2 -> PORT3 ch_mask index 3 -> channel mask value 4 (0100)
DMIC3 -> PORT3 ch_mask index 4 -> channel mask value 8 (1000)


Will check and add a proper description.


>> +    $ref: /schemas/types.yaml#/definitions/uint8-array
>> +    minItems: 8
>> +    maxItems: 8
>> +    items:
>> +      enum: [1, 2, 4, 8]
>> +
>>   required:
>>     - compatible
>>     - reg
>> @@ -74,6 +106,8 @@ examples:
>>               compatible = "sdw20217010a00";
>>               reg = <0 4>;
>>               qcom,rx-port-mapping = <1 2 3 4 5>;
>> +            qcom,rx-channel-mapping =  /bits/ 8 <0x01 0x02 0x01 0x01 0x02
>> +                                                 0x01 0x01 0x02>;
>>           };
>>       };
>>   
>> @@ -85,6 +119,8 @@ examples:
>>               compatible = "sdw20217010a00";
>>               reg = <0 3>;
>>               qcom,tx-port-mapping = <2 2 3 4>;
>> +            qcom,tx-channel-mapping = /bits/ 8 <0x01 0x02 0x01 0x01 0x02 0x04
>> +                                                0x04 0x08 0x01 0x02 0x04 0x8>;
> 
> Keep it consistent, e.g. everywhere without leading 0... actually not
> sure why this is hex, either.
> 

Ack, okay will check and modify.


For hex value, Actually took the reference from port mask values in swr 
DT entry.
https://elixir.bootlin.com/linux/v6.12-rc5/source/arch/arm64/boot/dts/qcom/sc7280.dtsi#L2528
>>           };
>>       };
>>   
>> -- 
>> 2.25.1

Thanks & Regards,
Rafi.
>>

Re: [PATCH v2 1/4] ASoC: dt-bindings: wcd937x-sdw: Add static channel mapping support

[Krzysztof Kozlowski]

On 30/10/2024 06:07, Mohammad Rafi Shaik wrote:
> On 10/23/2024 1:22 PM, Krzysztof Kozlowski wrote:
>> On Wed, Oct 23, 2024 at 11:43:23AM +0530, Mohammad Rafi Shaik wrote:
>>> Add static channel mapping between master and slave rx/tx ports for
>>> Qualcomm wcd937x soundwire codec.
>>>
>>> Currently, the channel mask for each soundwire port is hardcoded in the
>>> wcd937x-sdw driver, and the same channel mask value is configured in the
>>> soundwire master.
>>>
>>> The Qualcomm boards like the QCM6490-IDP require different channel mask settings
>>> for the soundwire master and slave ports.
>>
>> Different than what? Other wcd937x? Which are these?
>>
> For Qualcomm QCM6490-IDP board soundwire master needs a different 
> channel mask setting.

I understand, but I asked different than which board? Maybe all boards
needs this different channel setting, so basically it is "not different".

> 
> The wcd937x channel mask values are hardcoded in wcd driver.
> https://elixir.bootlin.com/linux/v6.12-rc5/source/sound/soc/codecs/wcd937x-sdw.c#L35
> https://elixir.bootlin.com/linux/v6.12-rc5/source/sound/soc/codecs/wcd938x-sdw.c#L37
> 
> In case of QCM6490-IDP the soundwire master and wcd937x require 
> different channel mask settings, not the same.
> For Example, wcd937x ADC2 connection
> 
>                            Master                 Slave (wcd937x)
>                       +--------------+           +--------------+
>                       |  +--------+  |           |  +--------+  |
>           ADC1  ----->|  | PORT1  |  |           |  |   TX1  | 
> |<-----------ADC1
>           ADC2  ----->|  |        |  |           |  |        |  |
>                       |  +--------+  |           |  +--------+  |
>                       |              |           |              |
>           ADC3  ----->|  +--------+  |           |  +--------+  |
>                       |  |  PORT2 |  |           |  |   TX2  | 
> |<-----------ADC2
>                       |  |        |  |           |  |        | 
> |<-----------ADC3
>                       |  +--------+  |           |  +--------+  |
>                       |              |           |              |
>                       |  +--------+  |           |  +--------+  |
>   DMIC0...DMIC3------>|  |  PORT3 |  |           |  |   TX3  | 
> |<-----------DMIC0...DMIC3
>                       |  |        |  |           |  |        | 
> |<-----------MBHC
>                       |  +--------+  |           |  +--------+  |
>                       |              |           |              |
>                       |  +--------+  |           |  +--------+  |
>   DMIC4...DMIC3 ----->|  |  PORT4 |  |           |  |   TX4  | 
> |<-----------DMIC4...DMIC7
>                       |  |        |  |           |  |        |  |
>                       |  +--------+  |           |  +--------+  |
>                       |              |           |              |
>                       +------------- +           +--------------+
> 
> 					
> For ADC2, The Slave needs to configure TX2 Port with channel mask value 
> 1 and
> For Master, it required PORT1 with channel mask  value 2.
> 
> 
> In existing design master and slave configured with same channel mask, 
> it will fail ADC2.
> The new design will help to configure channel mapping between master and 
> slave from DT.
> 
>>>
>>> With the introduction of the following channel mapping properties, it is now possible
>>> to configure the master channel mask directly from the device tree.
>>>
>>> The qcom,tx-channel-mapping property specifies the static channel mapping between the slave
>>> and master tx ports in the order of slave port channels which is adc1, adc2, adc3, adc4,
>>> dmic0, dmic1, mbhc, dmic2, dmic3, dmci4, dmic5, dmic6, dmic7.
>>
>> I still don't get what is the channel here.
>>
> Typo error,
> 
> The qcom,tx-channel-mapping property specifies the static channel 
> mapping between the slave
> 
> and master tx ports in the order of slave port channel index which are 
> adc1, adc2, adc3, adc4,
> 
> dmic0, dmic1, mbhc, dmic2, dmic3, dmci4, dmic5, dmic6, dmic7.
> 
> 
> 
> https://elixir.bootlin.com/linux/v6.12-rc5/source/sound/soc/codecs/wcd937x.h#L599
> 
> 
> 
> Will be fixed in the next version
> 
>>>
>>> The qcom,rx-channel-mapping property specifies static channel mapping between the slave
>>> and master rx ports in the order of slave port channels which is hph_l, hph_r, clsh,
>>> comp_l, comp_r, lo, dsd_r, dsd_l.
>>
>> And this description copies binding :/.
>>
>> Please wrap commit message according to Linux coding style / submission
>> process (neither too early nor over the limit):
>> https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597
>>
> 
> Ack
> 
>>>
>>> Signed-off-by: Mohammad Rafi Shaik <quic_mohs@quicinc.com>
>>> ---
>>>   .../bindings/sound/qcom,wcd937x-sdw.yaml      | 36 +++++++++++++++++++
>>>   1 file changed, 36 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml
>>> index d3cf8f59cb23..a6bc9b391db0 100644
>>> --- a/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml
>>> +++ b/Documentation/devicetree/bindings/sound/qcom,wcd937x-sdw.yaml
>>> @@ -58,6 +58,38 @@ properties:
>>>       items:
>>>         enum: [1, 2, 3, 4, 5]
>>>   
>>> +  qcom,tx-channel-mapping:
>>> +    description: |
>>> +      Specifies static channel mapping between slave and master tx port
>>> +      channels.
>>> +      In the order of slave port channels which is adc1, adc2, adc3, adc4,
>>> +      dmic0, dmic1, mbhc, dmic2, dmic3, dmci4, dmic5, dmic6, dmic7.
>>> +      ch_mask1 ==> bit mask value 1
>>> +      ch_mask2 ==> bit mask value 2
>>> +      ch_mask3 ==> bit mask value 4
>>> +      ch_mask4 ==> bit mask value 8
>>> +    $ref: /schemas/types.yaml#/definitions/uint8-array
>>> +    minItems: 8
>>> +    maxItems: 13
>>
>> Why size is variable? This device has fixed amount of slave ports, I
>> think.
>>
> 
> yes will check modify
> 
>>> +    items:
>>> +      enum: [1, 2, 4, 8]
>>
>> What is the point of using bits if you cannot actually create a bit mask
>> out of it? Why this cannot be 7?
>>
> Actually, these values should be fixed: 1 (0001), 2 (0010), 4(0100), 
> 8(1000).

What is fixed here?

I asked why these look like bitmasks but they cannot be used as bitmask.
This is a mapping, so index is channel slave port channel number and the
value is master port channel number, no?

> 
> 
> If required to set 7, it is handled in wcd driver based on mixer commands.
> https://elixir.bootlin.com/linux/v6.12-rc5/source/sound/soc/codecs/wcd937x.c#L1199

I talk about binding. Why you are not allowing value of 7 if this is a
mask? If this is not a mask - property says it is channel mapping - then
these should be [1-4].

> 
> 
> Example:
> WCD937X_HPH_L -> channel mask value is 1
> WCD937X_HPH_R -> channel mask value is 2



> 
> 
> The final channel mask for that specific port is 3
>>> +
>>> +  qcom,rx-channel-mapping:
>>> +    description: |
>>> +      Specifies static channels mapping between slave and master rx port
>>> +      channels.
>>> +      In the order of slave port channels, which is
>>> +      hph_l, hph_r, clsh, comp_l, comp_r, lo, dsd_r, dsd_l.
>>> +      ch_mask1 ==> bit mask value 1
>>> +      ch_mask2 ==> bit mask value 2
>>> +      ch_mask3 ==> bit mask value 4
>>> +      ch_mask4 ==> bit mask value 8
>>
>> and the value is what exactly? Index is channel, but what does "ch_mask4 ==> bit
>> mask value 8" mean? I don't understand this at all.
>>
> 
>                            Master
>                       +--------------+
>                       |  +--------+  |
>            ADC1 ----->|  | PORT1  |  |
>            ADC2 ----->|  |        |  |
>                       |  +--------+  |
>                       |              |
>            ADC3 ----->|  +--------+  |
>                       |  |  PORT2 |  |
>                       |  |        |  |
>                       |  +--------+  |
>                       |              |
>                       |  +--------+  |
>    DMIC0...DMIC3 ---->|  |  PORT3 |  |
>                       |  |        |  |
>                       |  +--------+  |
>                       |              |
>                       |  +--------+  |
>    DMIC4...DMIC7----->|  |  PORT4 |  |
>                       |  |        |  |
>                       |  +--------+  |
>                       |              |
>                       +------------- +
> 
> 
> The PORT1 has 2 ADC connections,
> 
> ADC1 -> PORT1 ch_mask index 1 -> channel mask value 1 (0001)
> ADC2 -> PORT1 ch_mask index 2 -> channel mask value 2 (0010)
> 
> 
> DMIC0 -> PORT3 ch_mask index 1 -> channel mask value 1 (0001)
> DMIC1 -> PORT3 ch_mask index 2 -> channel mask value 2 (0010)
> DMIC2 -> PORT3 ch_mask index 3 -> channel mask value 4 (0100)
> DMIC3 -> PORT3 ch_mask index 4 -> channel mask value 8 (1000)
> 
> 
> Will check and add a proper description.


Best regards,
Krzysztof