Re: [PATCH v2 3/5] soundwire: stream: Remove unneeded checks for NULL bus

Re: [PATCH v2 3/5] soundwire: stream: Remove unneeded checks for NULL bus

[kernel test robot]

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20230602101140.2040141-3-ckeepax@opensource.cirrus.com>
References: <20230602101140.2040141-3-ckeepax@opensource.cirrus.com>
TO: Charles Keepax <ckeepax@opensource.cirrus.com>

Hi Charles,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on v6.4-rc4 next-20230602]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Charles-Keepax/soundwire-bandwidth-allocation-Remove-pointless-variable/20230602-181544
base:   linus/master
patch link:    https://lore.kernel.org/r/20230602101140.2040141-3-ckeepax%40opensource.cirrus.com
patch subject: [PATCH v2 3/5] soundwire: stream: Remove unneeded checks for NULL bus
:::::: branch date: 25 hours ago
:::::: commit date: 25 hours ago
config: x86_64-randconfig-m001-20230531 (https://download.01.org/0day-ci/archive/20230603/202306031901.9adyY1Co-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202306031901.9adyY1Co-lkp@intel.com/

smatch warnings:
drivers/soundwire/stream.c:1408 _sdw_prepare_stream() error: uninitialized symbol 'bus'.

vim +/bus +1408 drivers/soundwire/stream.c

0c4a1049cf2987 Sanyog Kale          2018-07-27  1336  
c7a8f049b828dc Pierre-Louis Bossart 2020-01-14  1337  static int _sdw_prepare_stream(struct sdw_stream_runtime *stream,
c7a8f049b828dc Pierre-Louis Bossart 2020-01-14  1338  			       bool update_params)
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1339  {
3a0be1a65b5012 Pierre-Louis Bossart 2019-08-05  1340  	struct sdw_master_runtime *m_rt;
62d9d8d6796c7b Charles Keepax       2023-06-02  1341  	struct sdw_bus *bus;
3a0be1a65b5012 Pierre-Louis Bossart 2019-08-05  1342  	struct sdw_master_prop *prop;
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1343  	struct sdw_bus_params params;
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1344  	int ret;
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1345  
48949722ced4da Vinod Koul           2018-07-27  1346  	/* Prepare  Master(s) and Slave(s) port(s) associated with stream */
48949722ced4da Vinod Koul           2018-07-27  1347  	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
48949722ced4da Vinod Koul           2018-07-27  1348  		bus = m_rt->bus;
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1349  		prop = &bus->prop;
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1350  		memcpy(&params, &bus->params, sizeof(params));
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1351  
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1352  		/* TODO: Support Asynchronous mode */
3424305b8be456 Pierre-Louis Bossart 2019-05-22  1353  		if ((prop->max_clk_freq % stream->params.rate) != 0) {
17ed5bef49f42e Pierre-Louis Bossart 2019-05-01  1354  			dev_err(bus->dev, "Async mode not supported\n");
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1355  			return -EINVAL;
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1356  		}
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1357  
c7a8f049b828dc Pierre-Louis Bossart 2020-01-14  1358  		if (!update_params)
c7a8f049b828dc Pierre-Louis Bossart 2020-01-14  1359  			goto program_params;
c7a8f049b828dc Pierre-Louis Bossart 2020-01-14  1360  
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1361  		/* Increment cumulative bus bandwidth */
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1362  		/* TODO: Update this during Device-Device support */
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1363  		bus->params.bandwidth += m_rt->stream->params.rate *
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1364  			m_rt->ch_count * m_rt->stream->params.bps;
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1365  
c7578c1d628525 Vinod Koul           2019-08-13  1366  		/* Compute params */
c7578c1d628525 Vinod Koul           2019-08-13  1367  		if (bus->compute_params) {
c7578c1d628525 Vinod Koul           2019-08-13  1368  			ret = bus->compute_params(bus);
c7578c1d628525 Vinod Koul           2019-08-13  1369  			if (ret < 0) {
6122d3be2e9aa4 Pierre-Louis Bossart 2021-03-23  1370  				dev_err(bus->dev, "Compute params failed: %d\n",
c7578c1d628525 Vinod Koul           2019-08-13  1371  					ret);
acdae4632e40f0 Shuming Fan          2023-03-16  1372  				goto restore_params;
c7578c1d628525 Vinod Koul           2019-08-13  1373  			}
c7578c1d628525 Vinod Koul           2019-08-13  1374  		}
c7578c1d628525 Vinod Koul           2019-08-13  1375  
c7a8f049b828dc Pierre-Louis Bossart 2020-01-14  1376  program_params:
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1377  		/* Program params */
bfaa3549541cf8 Rander Wang          2020-01-14  1378  		ret = sdw_program_params(bus, true);
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1379  		if (ret < 0) {
17ed5bef49f42e Pierre-Louis Bossart 2019-05-01  1380  			dev_err(bus->dev, "Program params failed: %d\n", ret);
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1381  			goto restore_params;
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1382  		}
48949722ced4da Vinod Koul           2018-07-27  1383  	}
48949722ced4da Vinod Koul           2018-07-27  1384  
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1385  	ret = do_bank_switch(stream);
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1386  	if (ret < 0) {
68d9bfb668d6ee Pierre-Louis Bossart 2023-03-22  1387  		pr_err("%s: do_bank_switch failed: %d\n", __func__, ret);
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1388  		goto restore_params;
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1389  	}
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1390  
48949722ced4da Vinod Koul           2018-07-27  1391  	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
48949722ced4da Vinod Koul           2018-07-27  1392  		bus = m_rt->bus;
48949722ced4da Vinod Koul           2018-07-27  1393  
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1394  		/* Prepare port(s) on the new clock configuration */
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1395  		ret = sdw_prep_deprep_ports(m_rt, true);
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1396  		if (ret < 0) {
17ed5bef49f42e Pierre-Louis Bossart 2019-05-01  1397  			dev_err(bus->dev, "Prepare port(s) failed ret = %d\n",
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1398  				ret);
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1399  			return ret;
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1400  		}
48949722ced4da Vinod Koul           2018-07-27  1401  	}
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1402  
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1403  	stream->state = SDW_STREAM_PREPARED;
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1404  
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1405  	return ret;
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1406  
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1407  restore_params:
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26 @1408  	memcpy(&bus->params, &params, sizeof(params));
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1409  	return ret;
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1410  }
5c3eb9f7bdf1f1 Sanyog Kale          2018-04-26  1411  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v2 1/5] soundwire: stream: Add missing clear of alloc_slave_rt

[Charles Keepax]

The current path that skips allocating the slave runtime does not clear
the alloc_slave_rt flag, this is clearly incorrect. Add the missing
clear, so the runtime won't be erroneously cleaned up.

Fixes: f3016b891c8c ("soundwire: stream: sdw_stream_add_ functions can be called multiple times")
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---

Changes since v1:
 - Added missing fixes tag

Thanks,
Charles

 drivers/soundwire/stream.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index c2191c07442b0..379228f221869 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -2021,8 +2021,10 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 
 skip_alloc_master_rt:
 	s_rt = sdw_slave_rt_find(slave, stream);
-	if (s_rt)
+	if (s_rt) {
+		alloc_slave_rt = false;
 		goto skip_alloc_slave_rt;
+	}
 
 	s_rt = sdw_slave_rt_alloc(slave, m_rt);
 	if (!s_rt) {
-- 
2.30.2

[PATCH v2 3/5] soundwire: stream: Remove unneeded checks for NULL bus

[Charles Keepax]

Version of the code prior to commit d014688eb373 ("soundwire: stream:
remove bus->dev from logs on multiple buses"), used bus->dev in the
error message after do_bank_switch, this necessitated some checking to
ensure the bus pointer was valid. As the code no longer uses bus->dev
said checking is now redundant, so remove it.

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

Changes since v1:
 - Left the duplicate error prints in.

Thanks,
Charles

 drivers/soundwire/stream.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 379228f221869..6595f47b403b5 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1338,7 +1338,7 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream,
 			       bool update_params)
 {
 	struct sdw_master_runtime *m_rt;
-	struct sdw_bus *bus = NULL;
+	struct sdw_bus *bus;
 	struct sdw_master_prop *prop;
 	struct sdw_bus_params params;
 	int ret;
@@ -1382,11 +1382,6 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream,
 		}
 	}
 
-	if (!bus) {
-		pr_err("Configuration error in %s\n", __func__);
-		return -EINVAL;
-	}
-
 	ret = do_bank_switch(stream);
 	if (ret < 0) {
 		pr_err("%s: do_bank_switch failed: %d\n", __func__, ret);
@@ -1467,7 +1462,7 @@ EXPORT_SYMBOL(sdw_prepare_stream);
 static int _sdw_enable_stream(struct sdw_stream_runtime *stream)
 {
 	struct sdw_master_runtime *m_rt;
-	struct sdw_bus *bus = NULL;
+	struct sdw_bus *bus;
 	int ret;
 
 	/* Enable Master(s) and Slave(s) port(s) associated with stream */
@@ -1490,11 +1485,6 @@ static int _sdw_enable_stream(struct sdw_stream_runtime *stream)
 		}
 	}
 
-	if (!bus) {
-		pr_err("Configuration error in %s\n", __func__);
-		return -EINVAL;
-	}
-
 	ret = do_bank_switch(stream);
 	if (ret < 0) {
 		pr_err("%s: do_bank_switch failed: %d\n", __func__, ret);
-- 
2.30.2

Re: [PATCH v2 3/5] soundwire: stream: Remove unneeded checks for NULL bus

[Pierre-Louis Bossart]

On 6/2/23 05:11, Charles Keepax wrote:
> Version of the code prior to commit d014688eb373 ("soundwire: stream:
> remove bus->dev from logs on multiple buses"), used bus->dev in the
> error message after do_bank_switch, this necessitated some checking to
> ensure the bus pointer was valid. As the code no longer uses bus->dev
> said checking is now redundant, so remove it.
> 
> Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Good cleanup, thanks

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

> ---
> 
> Changes since v1:
>  - Left the duplicate error prints in.
> 
> Thanks,
> Charles
> 
>  drivers/soundwire/stream.c | 14 ++------------
>  1 file changed, 2 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
> index 379228f221869..6595f47b403b5 100644
> --- a/drivers/soundwire/stream.c
> +++ b/drivers/soundwire/stream.c
> @@ -1338,7 +1338,7 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream,
>  			       bool update_params)
>  {
>  	struct sdw_master_runtime *m_rt;
> -	struct sdw_bus *bus = NULL;
> +	struct sdw_bus *bus;
>  	struct sdw_master_prop *prop;
>  	struct sdw_bus_params params;
>  	int ret;
> @@ -1382,11 +1382,6 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream,
>  		}
>  	}
>  
> -	if (!bus) {
> -		pr_err("Configuration error in %s\n", __func__);
> -		return -EINVAL;
> -	}
> -
>  	ret = do_bank_switch(stream);
>  	if (ret < 0) {
>  		pr_err("%s: do_bank_switch failed: %d\n", __func__, ret);
> @@ -1467,7 +1462,7 @@ EXPORT_SYMBOL(sdw_prepare_stream);
>  static int _sdw_enable_stream(struct sdw_stream_runtime *stream)
>  {
>  	struct sdw_master_runtime *m_rt;
> -	struct sdw_bus *bus = NULL;
> +	struct sdw_bus *bus;
>  	int ret;
>  
>  	/* Enable Master(s) and Slave(s) port(s) associated with stream */
> @@ -1490,11 +1485,6 @@ static int _sdw_enable_stream(struct sdw_stream_runtime *stream)
>  		}
>  	}
>  
> -	if (!bus) {
> -		pr_err("Configuration error in %s\n", __func__);
> -		return -EINVAL;
> -	}
> -
>  	ret = do_bank_switch(stream);
>  	if (ret < 0) {
>  		pr_err("%s: do_bank_switch failed: %d\n", __func__, ret);