[PATCH v2 0/5] soundwire: add static port map support

[PATCH v2 0/5] soundwire: add static port map support

[Srinivas Kandagatla]

In some cases, SoundWire device ports are statically mapped to Controller
ports during design, however there is no way to expose this information
to the controller. Controllers like Qualcomm ones use this info to setup
static bandwith parameters for those ports.

A generic port allocation is not possible in this cases!
This patch adds a new member m_port_map to SoundWire device so that
it can populate the static master port map and share it with controller
to be able to setup correct bandwidth parameters.

As a user of this feature this patchset also adds new bindings for
wsa881x smart speaker which has 4 ports which are statically mapped
to the 3 output and 1 input port of the controller.

Tested it on DB845c and SM8250 MTP.

thanks,
srini

Srinivas Kandagatla (5):
  soundwire: add static port mapping support
  soundwire: qcom: update port map allocation bit mask
  soundwire: qcom: add static port map support
  ASoC: dt-bindings: wsa881x: add bindings for port mapping
  ASoC: codecs: wsa881x: add static port map support

 .../bindings/sound/qcom,wsa881x.yaml          |  9 ++++++
 drivers/soundwire/qcom.c                      | 30 +++++++++++++++----
 include/linux/soundwire/sdw.h                 |  2 ++
 sound/soc/codecs/wsa881x.c                    |  5 ++++
 4 files changed, 40 insertions(+), 6 deletions(-)

-- 
2.21.0

[PATCH v2 1/5] soundwire: add static port mapping support

[Srinivas Kandagatla]

Some of the SoundWire device ports are statically mapped to Controller
ports during design, however there is no way to expose this information
to the controller. Controllers like Qualcomm ones use this info to setup
static bandwith parameters for those ports.

A generic port allocation is not possible in this cases!
So this patch adds a new member m_port_map to struct sdw_slave to expose
this static map.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 include/linux/soundwire/sdw.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index d08039d65825..2f52d6609076 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -614,6 +614,7 @@ struct sdw_slave_ops {
  * @debugfs: Slave debugfs
  * @node: node for bus list
  * @port_ready: Port ready completion flag for each Slave port
+ * @m_port_map: static Master port map for each Slave port
  * @dev_num: Current Device Number, values can be 0 or dev_num_sticky
  * @dev_num_sticky: one-time static Device Number assigned by Bus
  * @probed: boolean tracking driver state
@@ -645,6 +646,7 @@ struct sdw_slave {
 #endif
 	struct list_head node;
 	struct completion port_ready[SDW_MAX_PORTS];
+	unsigned int m_port_map[SDW_MAX_PORTS];
 	enum sdw_clk_stop_mode curr_clk_stop_mode;
 	u16 dev_num;
 	u16 dev_num_sticky;
-- 
2.21.0

[PATCH v2 2/5] soundwire: qcom: update port map allocation bit mask

[Srinivas Kandagatla]

currently the internal bitmask used for allocating ports starts with offset 0.
This is bit confusing as data port numbers on Qualcomm controller are valid
from 1 to 14. So adjust this bit mask accordingly, this will also help while
adding static port map support.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/soundwire/qcom.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 6d22df01f354..f4f1c5f2af0b 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -519,7 +519,7 @@ static void qcom_swrm_stream_free_ports(struct qcom_swrm_ctrl *ctrl,
 			port_mask = &ctrl->din_port_mask;
 
 		list_for_each_entry(p_rt, &m_rt->port_list, port_node)
-			clear_bit(p_rt->num - 1, port_mask);
+			clear_bit(p_rt->num, port_mask);
 	}
 
 	mutex_unlock(&ctrl->port_lock);
@@ -552,13 +552,13 @@ static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl,
 			list_for_each_entry(p_rt, &s_rt->port_list, port_node) {
 				/* Port numbers start from 1 - 14*/
 				pn = find_first_zero_bit(port_mask, maxport);
-				if (pn > (maxport - 1)) {
+				if (pn > (maxport)) {
 					dev_err(ctrl->dev, "All ports busy\n");
 					ret = -EBUSY;
 					goto err;
 				}
 				set_bit(pn, port_mask);
-				pconfig[nports].num = pn + 1;
+				pconfig[nports].num = pn;
 				pconfig[nports].ch_mask = p_rt->ch_mask;
 				nports++;
 			}
@@ -580,7 +580,7 @@ static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl,
 err:
 	if (ret) {
 		for (i = 0; i < nports; i++)
-			clear_bit(pconfig[i].num - 1, port_mask);
+			clear_bit(pconfig[i].num, port_mask);
 	}
 
 	mutex_unlock(&ctrl->port_lock);
@@ -754,6 +754,9 @@ static int qcom_swrm_get_port_config(struct qcom_swrm_ctrl *ctrl)
 	ctrl->num_dout_ports = val;
 
 	nports = ctrl->num_dout_ports + ctrl->num_din_ports;
+	/* port numbers are non zero, so mark port 0 */
+	set_bit(0, &ctrl->dout_port_mask);
+	set_bit(0, &ctrl->din_port_mask);
 
 	ret = of_property_read_u8_array(np, "qcom,ports-offset1",
 					off1, nports);
-- 
2.21.0

[PATCH v2 3/5] soundwire: qcom: add static port map support

[Srinivas Kandagatla]

SoundWire device ports are statically mapped to Controller ports during
design. Add support to read these from SoundWire devices.
This controller uses static port map info to setup bandwidth
parameters for those ports.

A generic port allocation is not possible in this cases!

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/soundwire/qcom.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index f4f1c5f2af0b..b4d1aaf535c5 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -460,6 +460,8 @@ static int qcom_swrm_compute_params(struct sdw_bus *bus)
 	struct sdw_slave_runtime *s_rt;
 	struct sdw_port_runtime *p_rt;
 	struct qcom_swrm_port_config *pcfg;
+	struct sdw_slave *slave;
+	unsigned int m_port;
 	int i = 0;
 
 	list_for_each_entry(m_rt, &bus->m_rt_list, bus_node) {
@@ -473,8 +475,13 @@ static int qcom_swrm_compute_params(struct sdw_bus *bus)
 		}
 
 		list_for_each_entry(s_rt, &m_rt->slave_rt_list, m_rt_node) {
+			slave = s_rt->slave;
 			list_for_each_entry(p_rt, &s_rt->port_list, port_node) {
-				pcfg = &ctrl->pconfig[i];
+				m_port = slave->m_port_map[p_rt->num - 1];
+				if (m_port)
+					pcfg = &ctrl->pconfig[m_port - 1];
+				else
+					pcfg = &ctrl->pconfig[i];
 				p_rt->transport_params.port_num = p_rt->num;
 				p_rt->transport_params.sample_interval =
 					pcfg->si + 1;
@@ -535,8 +542,10 @@ static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl,
 	struct sdw_master_runtime *m_rt;
 	struct sdw_slave_runtime *s_rt;
 	struct sdw_port_runtime *p_rt;
+	struct sdw_slave *slave;
 	unsigned long *port_mask;
 	int i, maxport, pn, nports = 0, ret = 0;
+	unsigned int m_port;
 
 	mutex_lock(&ctrl->port_lock);
 	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
@@ -549,9 +558,15 @@ static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl,
 		}
 
 		list_for_each_entry(s_rt, &m_rt->slave_rt_list, m_rt_node) {
+			slave = s_rt->slave;
 			list_for_each_entry(p_rt, &s_rt->port_list, port_node) {
+				m_port = slave->m_port_map[p_rt->num - 1];
 				/* Port numbers start from 1 - 14*/
-				pn = find_first_zero_bit(port_mask, maxport);
+				if (m_port)
+					pn = m_port;
+				else
+					pn = find_first_zero_bit(port_mask, maxport);
+
 				if (pn > (maxport)) {
 					dev_err(ctrl->dev, "All ports busy\n");
 					ret = -EBUSY;
-- 
2.21.0

[PATCH v2 4/5] ASoC: dt-bindings: wsa881x: add bindings for port mapping

[Srinivas Kandagatla]

WSA881x SoundWire device ports are statically assigned to master ports
at design time. So add bindings required to specify these mappings!

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 .../devicetree/bindings/sound/qcom,wsa881x.yaml          | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/qcom,wsa881x.yaml b/Documentation/devicetree/bindings/sound/qcom,wsa881x.yaml
index ea44d03e58ca..48bc72304b90 100644
--- a/Documentation/devicetree/bindings/sound/qcom,wsa881x.yaml
+++ b/Documentation/devicetree/bindings/sound/qcom,wsa881x.yaml
@@ -26,6 +26,13 @@ properties:
     description: GPIO spec for Powerdown/Shutdown line to use
     maxItems: 1
 
+  qcom,port-mapping:
+    description: |
+      Specifies static port mapping between slave and master ports.
+      In the order of slave port index.
+    maxItems: 4
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
   '#thermal-sensor-cells':
     const: 0
 
@@ -54,6 +61,7 @@ examples:
             powerdown-gpios = <&wcdpinctrl 2 0>;
             #thermal-sensor-cells = <0>;
             #sound-dai-cells = <0>;
+            qcom-port-mapping = <1 2 3 7>;
         };
 
         speaker@0,2 {
@@ -62,6 +70,7 @@ examples:
             powerdown-gpios = <&wcdpinctrl 2 0>;
             #thermal-sensor-cells = <0>;
             #sound-dai-cells = <0>;
+            qcom-port-mapping = <4 5 6 8>;
         };
     };
 
-- 
2.21.0

[PATCH v2 5/5] ASoC: codecs: wsa881x: add static port map support

[Srinivas Kandagatla]

Two instances of WSA881x(Speaker Right, Speaker Left) ports
are statically mapped to master ports. Allow the driver to parse
those mappings from device tree.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 sound/soc/codecs/wsa881x.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/sound/soc/codecs/wsa881x.c b/sound/soc/codecs/wsa881x.c
index db87e07b11c9..f7b6bbd62728 100644
--- a/sound/soc/codecs/wsa881x.c
+++ b/sound/soc/codecs/wsa881x.c
@@ -1105,6 +1105,11 @@ static int wsa881x_probe(struct sdw_slave *pdev,
 		return PTR_ERR(wsa881x->sd_n);
 	}
 
+	if (of_property_read_u32_array(dev->of_node, "qcom,port-mapping",
+				       pdev->m_port_map,
+				       WSA881X_MAX_SWR_PORTS))
+		dev_info(dev, "Static Port mapping not specified\n");
+
 	dev_set_drvdata(&pdev->dev, wsa881x);
 	wsa881x->slave = pdev;
 	wsa881x->dev = &pdev->dev;
-- 
2.21.0

Re: [PATCH v2 2/5] soundwire: qcom: update port map allocation bit mask

[Pierre-Louis Bossart]

On 3/9/21 8:15 AM, Srinivas Kandagatla wrote:
> currently the internal bitmask used for allocating ports starts with offset 0.
> This is bit confusing as data port numbers on Qualcomm controller are valid
> from 1 to 14. So adjust this bit mask accordingly, this will also help while
> adding static port map support.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>   drivers/soundwire/qcom.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
> index 6d22df01f354..f4f1c5f2af0b 100644
> --- a/drivers/soundwire/qcom.c
> +++ b/drivers/soundwire/qcom.c
> @@ -519,7 +519,7 @@ static void qcom_swrm_stream_free_ports(struct qcom_swrm_ctrl *ctrl,
>   			port_mask = &ctrl->din_port_mask;
>   
>   		list_for_each_entry(p_rt, &m_rt->port_list, port_node)
> -			clear_bit(p_rt->num - 1, port_mask);
> +			clear_bit(p_rt->num, port_mask);
>   	}
>   
>   	mutex_unlock(&ctrl->port_lock);
> @@ -552,13 +552,13 @@ static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl,
>   			list_for_each_entry(p_rt, &s_rt->port_list, port_node) {
>   				/* Port numbers start from 1 - 14*/
>   				pn = find_first_zero_bit(port_mask, maxport);
> -				if (pn > (maxport - 1)) {
> +				if (pn > (maxport)) {

nit-pick: useless parentheses

>   					dev_err(ctrl->dev, "All ports busy\n");
>   					ret = -EBUSY;
>   					goto err;
>   				}
>   				set_bit(pn, port_mask);
> -				pconfig[nports].num = pn + 1;
> +				pconfig[nports].num = pn;
>   				pconfig[nports].ch_mask = p_rt->ch_mask;
>   				nports++;
>   			}
> @@ -580,7 +580,7 @@ static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl,
>   err:
>   	if (ret) {
>   		for (i = 0; i < nports; i++)
> -			clear_bit(pconfig[i].num - 1, port_mask);
> +			clear_bit(pconfig[i].num, port_mask);
>   	}
>   
>   	mutex_unlock(&ctrl->port_lock);
> @@ -754,6 +754,9 @@ static int qcom_swrm_get_port_config(struct qcom_swrm_ctrl *ctrl)
>   	ctrl->num_dout_ports = val;
>   
>   	nports = ctrl->num_dout_ports + ctrl->num_din_ports;
> +	/* port numbers are non zero, so mark port 0 */

mask?

> +	set_bit(0, &ctrl->dout_port_mask);
> +	set_bit(0, &ctrl->din_port_mask);
>   
>   	ret = of_property_read_u8_array(np, "qcom,ports-offset1",
>   					off1, nports);
> 

Re: [PATCH v2 5/5] ASoC: codecs: wsa881x: add static port map support

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 3934 bytes --]

Hi Srinivas,

I love your patch! Yet something to improve:

[auto build test ERROR on asoc/for-next]
[also build test ERROR on vkoul-dmaengine/next robh/for-next linus/master v5.12-rc2 next-20210309]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Srinivas-Kandagatla/soundwire-add-static-port-map-support/20210309-221834
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: nds32-randconfig-r003-20210308 (attached as .config)
compiler: nds32le-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/d0e48ba0d85c188c14216aad9397bcd902d8a09a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Srinivas-Kandagatla/soundwire-add-static-port-map-support/20210309-221834
        git checkout d0e48ba0d85c188c14216aad9397bcd902d8a09a
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=nds32 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   sound/soc/codecs/wsa881x.c: In function 'wsa881x_probe':
>> sound/soc/codecs/wsa881x.c:1108:33: error: 'dev' undeclared (first use in this function); did you mean 'pdev'?
    1108 |  if (of_property_read_u32_array(dev->of_node, "qcom,port-mapping",
         |                                 ^~~
         |                                 pdev
   sound/soc/codecs/wsa881x.c:1108:33: note: each undeclared identifier is reported only once for each function it appears in


vim +1108 sound/soc/codecs/wsa881x.c

  1091	
  1092	static int wsa881x_probe(struct sdw_slave *pdev,
  1093				 const struct sdw_device_id *id)
  1094	{
  1095		struct wsa881x_priv *wsa881x;
  1096	
  1097		wsa881x = devm_kzalloc(&pdev->dev, sizeof(*wsa881x), GFP_KERNEL);
  1098		if (!wsa881x)
  1099			return -ENOMEM;
  1100	
  1101		wsa881x->sd_n = devm_gpiod_get_optional(&pdev->dev, "powerdown",
  1102							GPIOD_FLAGS_BIT_NONEXCLUSIVE);
  1103		if (IS_ERR(wsa881x->sd_n)) {
  1104			dev_err(&pdev->dev, "Shutdown Control GPIO not found\n");
  1105			return PTR_ERR(wsa881x->sd_n);
  1106		}
  1107	
> 1108		if (of_property_read_u32_array(dev->of_node, "qcom,port-mapping",
  1109					       pdev->m_port_map,
  1110					       WSA881X_MAX_SWR_PORTS))
  1111			dev_info(dev, "Static Port mapping not specified\n");
  1112	
  1113		dev_set_drvdata(&pdev->dev, wsa881x);
  1114		wsa881x->slave = pdev;
  1115		wsa881x->dev = &pdev->dev;
  1116		wsa881x->sconfig.ch_count = 1;
  1117		wsa881x->sconfig.bps = 1;
  1118		wsa881x->sconfig.frame_rate = 48000;
  1119		wsa881x->sconfig.direction = SDW_DATA_DIR_RX;
  1120		wsa881x->sconfig.type = SDW_STREAM_PDM;
  1121		pdev->prop.sink_ports = GENMASK(WSA881X_MAX_SWR_PORTS, 0);
  1122		pdev->prop.sink_dpn_prop = wsa_sink_dpn_prop;
  1123		pdev->prop.scp_int1_mask = SDW_SCP_INT1_BUS_CLASH | SDW_SCP_INT1_PARITY;
  1124		gpiod_direction_output(wsa881x->sd_n, 1);
  1125	
  1126		wsa881x->regmap = devm_regmap_init_sdw(pdev, &wsa881x_regmap_config);
  1127		if (IS_ERR(wsa881x->regmap)) {
  1128			dev_err(&pdev->dev, "regmap_init failed\n");
  1129			return PTR_ERR(wsa881x->regmap);
  1130		}
  1131	
  1132		return devm_snd_soc_register_component(&pdev->dev,
  1133						       &wsa881x_component_drv,
  1134						       wsa881x_dais,
  1135						       ARRAY_SIZE(wsa881x_dais));
  1136	}
  1137	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33454 bytes --]

Re: [PATCH v2 3/5] soundwire: qcom: add static port map support

[Pierre-Louis Bossart]

>   	list_for_each_entry(m_rt, &bus->m_rt_list, bus_node) {
> @@ -473,8 +475,13 @@ static int qcom_swrm_compute_params(struct sdw_bus *bus)
>   		}
>   
>   		list_for_each_entry(s_rt, &m_rt->slave_rt_list, m_rt_node) {
> +			slave = s_rt->slave;
>   			list_for_each_entry(p_rt, &s_rt->port_list, port_node) {
> -				pcfg = &ctrl->pconfig[i];
> +				m_port = slave->m_port_map[_rtp->num - 1];
> +				if (m_port)
> +					pcfg = &ctrl->pconfig[m_port - 1];
> +				else
> +					pcfg = &ctrl->pconfig[i];

Maybe add a short comment on port allocation, I had to think a bit to 
figure out why the -1 was required on both peripheral and manager but it 
is not below [1]

>   				p_rt->transport_params.port_num = p_rt->num;
>   				p_rt->transport_params.sample_interval =
>   					pcfg->si + 1;
> @@ -535,8 +542,10 @@ static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl,
>   	struct sdw_master_runtime *m_rt;
>   	struct sdw_slave_runtime *s_rt;
>   	struct sdw_port_runtime *p_rt;
> +	struct sdw_slave *slave;
>   	unsigned long *port_mask;
>   	int i, maxport, pn, nports = 0, ret = 0;
> +	unsigned int m_port;
>   
>   	mutex_lock(&ctrl->port_lock);
>   	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
> @@ -549,9 +558,15 @@ static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl,
>   		}
>   
>   		list_for_each_entry(s_rt, &m_rt->slave_rt_list, m_rt_node) {
> +			slave = s_rt->slave;
>   			list_for_each_entry(p_rt, &s_rt->port_list, port_node) {
> +				m_port = slave->m_port_map[p_rt->num - 1];
>   				/* Port numbers start from 1 - 14*/
> -				pn = find_first_zero_bit(port_mask, maxport);
> +				if (m_port)
> +					pn = m_port;
> +				else
> +					pn = find_first_zero_bit(port_mask, maxport);

[1]

> +
>   				if (pn > (maxport)) {
>   					dev_err(ctrl->dev, "All ports busy\n");
>   					ret = -EBUSY;
> 

Re: [PATCH v2 4/5] ASoC: dt-bindings: wsa881x: add bindings for port mapping

[Rob Herring]

On Tue, 09 Mar 2021 14:15:13 +0000, Srinivas Kandagatla wrote:
> WSA881x SoundWire device ports are statically assigned to master ports
> at design time. So add bindings required to specify these mappings!
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>  .../devicetree/bindings/sound/qcom,wsa881x.yaml          | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/sound/qcom,wsa881x.example.dt.yaml: speaker@0,1: 'qcom-port-mapping' does not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/sound/qcom,wsa881x.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/sound/qcom,wsa881x.example.dt.yaml: speaker@0,2: 'qcom-port-mapping' does not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/sound/qcom,wsa881x.yaml

See https://patchwork.ozlabs.org/patch/1449860

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH v2 5/5] ASoC: codecs: wsa881x: add static port map support

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 4433 bytes --]

Hi Srinivas,

I love your patch! Yet something to improve:

[auto build test ERROR on asoc/for-next]
[also build test ERROR on vkoul-dmaengine/next robh/for-next linus/master v5.12-rc2 next-20210309]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Srinivas-Kandagatla/soundwire-add-static-port-map-support/20210309-221834
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: arm64-randconfig-r016-20210308 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 820f508b08d7c94b2dd7847e9710d2bc36d3dd45)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm64 cross compiling tool for clang build
        # apt-get install binutils-aarch64-linux-gnu
        # https://github.com/0day-ci/linux/commit/d0e48ba0d85c188c14216aad9397bcd902d8a09a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Srinivas-Kandagatla/soundwire-add-static-port-map-support/20210309-221834
        git checkout d0e48ba0d85c188c14216aad9397bcd902d8a09a
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> sound/soc/codecs/wsa881x.c:1108:33: error: use of undeclared identifier 'dev'
           if (of_property_read_u32_array(dev->of_node, "qcom,port-mapping",
                                          ^
   sound/soc/codecs/wsa881x.c:1111:12: error: use of undeclared identifier 'dev'; did you mean 'pdev'?
                   dev_info(dev, "Static Port mapping not specified\n");
                            ^~~
                            pdev
   include/linux/dev_printk.h:118:12: note: expanded from macro 'dev_info'
           _dev_info(dev, dev_fmt(fmt), ##__VA_ARGS__)
                     ^
   sound/soc/codecs/wsa881x.c:1092:44: note: 'pdev' declared here
   static int wsa881x_probe(struct sdw_slave *pdev,
                                              ^
   2 errors generated.


vim +/dev +1108 sound/soc/codecs/wsa881x.c

  1091	
  1092	static int wsa881x_probe(struct sdw_slave *pdev,
  1093				 const struct sdw_device_id *id)
  1094	{
  1095		struct wsa881x_priv *wsa881x;
  1096	
  1097		wsa881x = devm_kzalloc(&pdev->dev, sizeof(*wsa881x), GFP_KERNEL);
  1098		if (!wsa881x)
  1099			return -ENOMEM;
  1100	
  1101		wsa881x->sd_n = devm_gpiod_get_optional(&pdev->dev, "powerdown",
  1102							GPIOD_FLAGS_BIT_NONEXCLUSIVE);
  1103		if (IS_ERR(wsa881x->sd_n)) {
  1104			dev_err(&pdev->dev, "Shutdown Control GPIO not found\n");
  1105			return PTR_ERR(wsa881x->sd_n);
  1106		}
  1107	
> 1108		if (of_property_read_u32_array(dev->of_node, "qcom,port-mapping",
  1109					       pdev->m_port_map,
  1110					       WSA881X_MAX_SWR_PORTS))
  1111			dev_info(dev, "Static Port mapping not specified\n");
  1112	
  1113		dev_set_drvdata(&pdev->dev, wsa881x);
  1114		wsa881x->slave = pdev;
  1115		wsa881x->dev = &pdev->dev;
  1116		wsa881x->sconfig.ch_count = 1;
  1117		wsa881x->sconfig.bps = 1;
  1118		wsa881x->sconfig.frame_rate = 48000;
  1119		wsa881x->sconfig.direction = SDW_DATA_DIR_RX;
  1120		wsa881x->sconfig.type = SDW_STREAM_PDM;
  1121		pdev->prop.sink_ports = GENMASK(WSA881X_MAX_SWR_PORTS, 0);
  1122		pdev->prop.sink_dpn_prop = wsa_sink_dpn_prop;
  1123		pdev->prop.scp_int1_mask = SDW_SCP_INT1_BUS_CLASH | SDW_SCP_INT1_PARITY;
  1124		gpiod_direction_output(wsa881x->sd_n, 1);
  1125	
  1126		wsa881x->regmap = devm_regmap_init_sdw(pdev, &wsa881x_regmap_config);
  1127		if (IS_ERR(wsa881x->regmap)) {
  1128			dev_err(&pdev->dev, "regmap_init failed\n");
  1129			return PTR_ERR(wsa881x->regmap);
  1130		}
  1131	
  1132		return devm_snd_soc_register_component(&pdev->dev,
  1133						       &wsa881x_component_drv,
  1134						       wsa881x_dais,
  1135						       ARRAY_SIZE(wsa881x_dais));
  1136	}
  1137	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33552 bytes --]

Re: [PATCH v2 3/5] soundwire: qcom: add static port map support

[Srinivas Kandagatla]

On 09/03/2021 16:10, Pierre-Louis Bossart wrote:
> 
> 
> 
>>       list_for_each_entry(m_rt, &bus->m_rt_list, bus_node) {
>> @@ -473,8 +475,13 @@ static int qcom_swrm_compute_params(struct 
>> sdw_bus *bus)
>>           }
>>           list_for_each_entry(s_rt, &m_rt->slave_rt_list, m_rt_node) {
>> +            slave = s_rt->slave;
>>               list_for_each_entry(p_rt, &s_rt->port_list, port_node) {
>> -                pcfg = &ctrl->pconfig[i];
>> +                m_port = slave->m_port_map[_rtp->num - 1];
>> +                if (m_port)
>> +                    pcfg = &ctrl->pconfig[m_port - 1];
>> +                else
>> +                    pcfg = &ctrl->pconfig[i];
> 
> Maybe add a short comment on port allocation, I had to think a bit to 
> figure out why the -1 was required on both peripheral and manager but it 
> is not below [1]

I agree, will add some comment here to explain the offsets correctly!

--srini

> 
>>                   p_rt->transport_params.port_num = p_rt->num;
>>                   p_rt->transport_params.sample_interval =
>>                       pcfg->si + 1;
>> @@ -535,8 +542,10 @@ static int qcom_swrm_stream_alloc_ports(struct 
>> qcom_swrm_ctrl *ctrl,
>>       struct sdw_master_runtime *m_rt;
>>       struct sdw_slave_runtime *s_rt;
>>       struct sdw_port_runtime *p_rt;
>> +    struct sdw_slave *slave;
>>       unsigned long *port_mask;
>>       int i, maxport, pn, nports = 0, ret = 0;
>> +    unsigned int m_port;
>>       mutex_lock(&ctrl->port_lock);
>>       list_for_each_entry(m_rt, &stream->master_list, stream_node) {
>> @@ -549,9 +558,15 @@ static int qcom_swrm_stream_alloc_ports(struct 
>> qcom_swrm_ctrl *ctrl,
>>           }
>>           list_for_each_entry(s_rt, &m_rt->slave_rt_list, m_rt_node) {
>> +            slave = s_rt->slave;
>>               list_for_each_entry(p_rt, &s_rt->port_list, port_node) {
>> +                m_port = slave->m_port_map[p_rt->num - 1];
>>                   /* Port numbers start from 1 - 14*/
>> -                pn = find_first_zero_bit(port_mask, maxport);
>> +                if (m_port)
>> +                    pn = m_port;
>> +                else
>> +                    pn = find_first_zero_bit(port_mask, maxport);
> 
> [1]
> 
>> +
>>                   if (pn > (maxport)) {
>>                       dev_err(ctrl->dev, "All ports busy\n");
>>                       ret = -EBUSY;
>>

Re: [PATCH v2 2/5] soundwire: qcom: update port map allocation bit mask

[Srinivas Kandagatla]

On 09/03/2021 15:55, Pierre-Louis Bossart wrote:
> 
> 
> On 3/9/21 8:15 AM, Srinivas Kandagatla wrote:
>> currently the internal bitmask used for allocating ports starts with 
>> offset 0.
>> This is bit confusing as data port numbers on Qualcomm controller are 
>> valid
>> from 1 to 14. So adjust this bit mask accordingly, this will also help 
>> while
>> adding static port map support.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> ---
>>   drivers/soundwire/qcom.c | 11 +++++++----
>>   1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
>> index 6d22df01f354..f4f1c5f2af0b 100644
>> --- a/drivers/soundwire/qcom.c
>> +++ b/drivers/soundwire/qcom.c
>> @@ -519,7 +519,7 @@ static void qcom_swrm_stream_free_ports(struct 
>> qcom_swrm_ctrl *ctrl,
>>               port_mask = &ctrl->din_port_mask;
>>           list_for_each_entry(p_rt, &m_rt->port_list, port_node)
>> -            clear_bit(p_rt->num - 1, port_mask);
>> +            clear_bit(p_rt->num, port_mask);
>>       }
>>       mutex_unlock(&ctrl->port_lock);
>> @@ -552,13 +552,13 @@ static int qcom_swrm_stream_alloc_ports(struct 
>> qcom_swrm_ctrl *ctrl,
>>               list_for_each_entry(p_rt, &s_rt->port_list, port_node) {
>>                   /* Port numbers start from 1 - 14*/
>>                   pn = find_first_zero_bit(port_mask, maxport);
>> -                if (pn > (maxport - 1)) {
>> +                if (pn > (maxport)) {
> 
> nit-pick: useless parentheses

I agree, will remove this and address the other comment too in next spin!

--srini
> 
>>                       dev_err(ctrl->dev, "All ports busy\n");
>>                       ret = -EBUSY;
>>                       goto err;
>>                   }
>>                   set_bit(pn, port_mask);
>> -                pconfig[nports].num = pn + 1;
>> +                pconfig[nports].num = pn;
>>                   pconfig[nports].ch_mask = p_rt->ch_mask;
>>                   nports++;
>>               }
>> @@ -580,7 +580,7 @@ static int qcom_swrm_stream_alloc_ports(struct 
>> qcom_swrm_ctrl *ctrl,
>>   err:
>>       if (ret) {
>>           for (i = 0; i < nports; i++)
>> -            clear_bit(pconfig[i].num - 1, port_mask);
>> +            clear_bit(pconfig[i].num, port_mask);
>>       }
>>       mutex_unlock(&ctrl->port_lock);
>> @@ -754,6 +754,9 @@ static int qcom_swrm_get_port_config(struct 
>> qcom_swrm_ctrl *ctrl)
>>       ctrl->num_dout_ports = val;
>>       nports = ctrl->num_dout_ports + ctrl->num_din_ports;
>> +    /* port numbers are non zero, so mark port 0 */
> 
> mask?
> 
>> +    set_bit(0, &ctrl->dout_port_mask);
>> +    set_bit(0, &ctrl->din_port_mask);
>>       ret = of_property_read_u8_array(np, "qcom,ports-offset1",
>>                       off1, nports);
>>