linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RESEND PATCH 00/11] soundwire: some cleanup patches
@ 2021-03-26  9:15 Bard Liao
  2021-03-26  9:15 ` [RESEND PATCH 01/11] soundwire: bus: use correct driver name in error messages Bard Liao
                   ` (10 more replies)
  0 siblings, 11 replies; 12+ messages in thread
From: Bard Liao @ 2021-03-26  9:15 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	rander.wang, hui.wang, pierre-louis.bossart, sanyog.r.kale,
	bard.liao

To make soundwire driver more decent and less Cppcheck complaint.

Pierre-Louis Bossart (11):
  soundwire: bus: use correct driver name in error messages
  soundwire: bus: test read status
  soundwire: bus: use consistent tests for return values
  soundwire: bus: demote clock stop prepare log to dev_dbg()
  soundwire: bus: uniquify dev_err() for SCP_INT access
  soundwire: bus: remove useless initialization
  soundwire: generic_bandwidth_allocation: remove useless init
  soundwire: intel: remove useless readl
  soundwire: qcom: check of_property_read status
  soundwire: stream: remove useless initialization
  soundwire: stream: remove useless bus initializations

 drivers/soundwire/bus.c                       | 54 +++++++++++--------
 drivers/soundwire/bus_type.c                  | 15 ++++--
 .../soundwire/generic_bandwidth_allocation.c  |  4 +-
 drivers/soundwire/intel.c                     |  2 -
 drivers/soundwire/qcom.c                      |  3 ++
 drivers/soundwire/stream.c                    |  8 +--
 6 files changed, 52 insertions(+), 34 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RESEND PATCH 01/11] soundwire: bus: use correct driver name in error messages
  2021-03-26  9:15 [RESEND PATCH 00/11] soundwire: some cleanup patches Bard Liao
@ 2021-03-26  9:15 ` Bard Liao
  2021-03-26  9:15 ` [RESEND PATCH 02/11] soundwire: bus: test read status Bard Liao
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Bard Liao @ 2021-03-26  9:15 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	rander.wang, hui.wang, pierre-louis.bossart, sanyog.r.kale,
	bard.liao

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

None of the existing codec drivers set the sdw_driver.name, but
instead set sdw_driver.driver.name.

This leads to error messages such as
[   23.935355] rt700 sdw:2:25d:700:0: Probe of (null) failed: -19

We could remove this sdw_driver.name if it doesn't have any
purpose. This patch only suggests using the proper indirection.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/bus_type.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c
index 575b9bad99d5..893296f3fe39 100644
--- a/drivers/soundwire/bus_type.c
+++ b/drivers/soundwire/bus_type.c
@@ -82,6 +82,7 @@ static int sdw_drv_probe(struct device *dev)
 	struct sdw_slave *slave = dev_to_sdw_dev(dev);
 	struct sdw_driver *drv = drv_to_sdw_driver(dev->driver);
 	const struct sdw_device_id *id;
+	const char *name;
 	int ret;
 
 	/*
@@ -108,7 +109,10 @@ static int sdw_drv_probe(struct device *dev)
 
 	ret = drv->probe(slave, id);
 	if (ret) {
-		dev_err(dev, "Probe of %s failed: %d\n", drv->name, ret);
+		name = drv->name;
+		if (!name)
+			name = drv->driver.name;
+		dev_err(dev, "Probe of %s failed: %d\n", name, ret);
 		dev_pm_domain_detach(dev, false);
 		return ret;
 	}
@@ -174,11 +178,16 @@ static void sdw_drv_shutdown(struct device *dev)
  */
 int __sdw_register_driver(struct sdw_driver *drv, struct module *owner)
 {
+	const char *name;
+
 	drv->driver.bus = &sdw_bus_type;
 
 	if (!drv->probe) {
-		pr_err("driver %s didn't provide SDW probe routine\n",
-		       drv->name);
+		name = drv->name;
+		if (!name)
+			name = drv->driver.name;
+
+		pr_err("driver %s didn't provide SDW probe routine\n", name);
 		return -EINVAL;
 	}
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RESEND PATCH 02/11] soundwire: bus: test read status
  2021-03-26  9:15 [RESEND PATCH 00/11] soundwire: some cleanup patches Bard Liao
  2021-03-26  9:15 ` [RESEND PATCH 01/11] soundwire: bus: use correct driver name in error messages Bard Liao
@ 2021-03-26  9:15 ` Bard Liao
  2021-03-26  9:15 ` [RESEND PATCH 03/11] soundwire: bus: use consistent tests for return values Bard Liao
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Bard Liao @ 2021-03-26  9:15 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	rander.wang, hui.wang, pierre-louis.bossart, sanyog.r.kale,
	bard.liao

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

In the existing code we may read a negative error value but still mask
it and write it back.

Make sure all reads are tested and errors not propagated further.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/bus.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 04eb879de145..1c01cc192cbd 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -875,8 +875,12 @@ static int sdw_slave_clk_stop_prepare(struct sdw_slave *slave,
 		if (wake_en)
 			val |= SDW_SCP_SYSTEMCTRL_WAKE_UP_EN;
 	} else {
-		val = sdw_read_no_pm(slave, SDW_SCP_SYSTEMCTRL);
-
+		ret = sdw_read_no_pm(slave, SDW_SCP_SYSTEMCTRL);
+		if (ret < 0) {
+			dev_err(&slave->dev, "SDW_SCP_SYSTEMCTRL read failed:%d\n", ret);
+			return ret;
+		}
+		val = ret;
 		val &= ~(SDW_SCP_SYSTEMCTRL_CLK_STP_PREP);
 	}
 
@@ -895,8 +899,12 @@ static int sdw_bus_wait_for_clk_prep_deprep(struct sdw_bus *bus, u16 dev_num)
 	int val;
 
 	do {
-		val = sdw_bread_no_pm(bus, dev_num, SDW_SCP_STAT) &
-			SDW_SCP_STAT_CLK_STP_NF;
+		val = sdw_bread_no_pm(bus, dev_num, SDW_SCP_STAT);
+		if (val < 0) {
+			dev_err(bus->dev, "SDW_SCP_STAT bread failed:%d\n", val);
+			return val;
+		}
+		val &= SDW_SCP_STAT_CLK_STP_NF;
 		if (!val) {
 			dev_info(bus->dev, "clock stop prep/de-prep done slave:%d",
 				 dev_num);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RESEND PATCH 03/11] soundwire: bus: use consistent tests for return values
  2021-03-26  9:15 [RESEND PATCH 00/11] soundwire: some cleanup patches Bard Liao
  2021-03-26  9:15 ` [RESEND PATCH 01/11] soundwire: bus: use correct driver name in error messages Bard Liao
  2021-03-26  9:15 ` [RESEND PATCH 02/11] soundwire: bus: test read status Bard Liao
@ 2021-03-26  9:15 ` Bard Liao
  2021-03-26  9:15 ` [RESEND PATCH 04/11] soundwire: bus: demote clock stop prepare log to dev_dbg() Bard Liao
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Bard Liao @ 2021-03-26  9:15 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	rander.wang, hui.wang, pierre-louis.bossart, sanyog.r.kale,
	bard.liao

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

We use different styles to check the return values of IO related
routines. The majority of the cases use 'if (ret < 0)', align the
remaining cases for consistency.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/bus.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 1c01cc192cbd..d39e5baa3e64 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -44,13 +44,13 @@ int sdw_bus_master_add(struct sdw_bus *bus, struct device *parent,
 	}
 
 	ret = sdw_get_id(bus);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(parent, "Failed to get bus id\n");
 		return ret;
 	}
 
 	ret = sdw_master_device_add(bus, parent, fwnode);
-	if (ret) {
+	if (ret < 0) {
 		dev_err(parent, "Failed to add master device at link %d\n",
 			bus->link_id);
 		return ret;
@@ -121,7 +121,7 @@ int sdw_bus_master_add(struct sdw_bus *bus, struct device *parent,
 	else
 		ret = -ENOTSUPP; /* No ACPI/DT so error out */
 
-	if (ret) {
+	if (ret < 0) {
 		dev_err(bus->dev, "Finding slaves failed:%d\n", ret);
 		return ret;
 	}
@@ -422,7 +422,7 @@ sdw_bread_no_pm(struct sdw_bus *bus, u16 dev_num, u32 addr)
 
 	ret = sdw_fill_msg(&msg, NULL, addr, 1, dev_num,
 			   SDW_MSG_FLAG_READ, &buf);
-	if (ret)
+	if (ret < 0)
 		return ret;
 
 	ret = sdw_transfer(bus, &msg);
@@ -440,7 +440,7 @@ sdw_bwrite_no_pm(struct sdw_bus *bus, u16 dev_num, u32 addr, u8 value)
 
 	ret = sdw_fill_msg(&msg, NULL, addr, 1, dev_num,
 			   SDW_MSG_FLAG_WRITE, &value);
-	if (ret)
+	if (ret < 0)
 		return ret;
 
 	return sdw_transfer(bus, &msg);
@@ -454,7 +454,7 @@ int sdw_bread_no_pm_unlocked(struct sdw_bus *bus, u16 dev_num, u32 addr)
 
 	ret = sdw_fill_msg(&msg, NULL, addr, 1, dev_num,
 			   SDW_MSG_FLAG_READ, &buf);
-	if (ret)
+	if (ret < 0)
 		return ret;
 
 	ret = sdw_transfer_unlocked(bus, &msg);
@@ -472,7 +472,7 @@ int sdw_bwrite_no_pm_unlocked(struct sdw_bus *bus, u16 dev_num, u32 addr, u8 val
 
 	ret = sdw_fill_msg(&msg, NULL, addr, 1, dev_num,
 			   SDW_MSG_FLAG_WRITE, &value);
-	if (ret)
+	if (ret < 0)
 		return ret;
 
 	return sdw_transfer_unlocked(bus, &msg);
@@ -749,7 +749,7 @@ static int sdw_program_device_num(struct sdw_bus *bus)
 				 * dev_num
 				 */
 				ret = sdw_assign_device_num(slave);
-				if (ret) {
+				if (ret < 0) {
 					dev_err(bus->dev,
 						"Assign dev_num failed:%d\n",
 						ret);
@@ -886,7 +886,7 @@ static int sdw_slave_clk_stop_prepare(struct sdw_slave *slave,
 
 	ret = sdw_write_no_pm(slave, SDW_SCP_SYSTEMCTRL, val);
 
-	if (ret != 0)
+	if (ret < 0)
 		dev_err(&slave->dev,
 			"Clock Stop prepare failed for slave: %d", ret);
 
@@ -1748,7 +1748,7 @@ int sdw_handle_slave_status(struct sdw_bus *bus,
 	if (status[0] == SDW_SLAVE_ATTACHED) {
 		dev_dbg(bus->dev, "Slave attached, programming device number\n");
 		ret = sdw_program_device_num(bus);
-		if (ret)
+		if (ret < 0)
 			dev_err(bus->dev, "Slave attach failed: %d\n", ret);
 		/*
 		 * programming a device number will have side effects,
@@ -1782,7 +1782,7 @@ int sdw_handle_slave_status(struct sdw_bus *bus,
 
 		case SDW_SLAVE_ALERT:
 			ret = sdw_handle_slave_alerts(slave);
-			if (ret)
+			if (ret < 0)
 				dev_err(&slave->dev,
 					"Slave %d alert handling failed: %d\n",
 					i, ret);
@@ -1801,7 +1801,7 @@ int sdw_handle_slave_status(struct sdw_bus *bus,
 			attached_initializing = true;
 
 			ret = sdw_initialize_slave(slave);
-			if (ret)
+			if (ret < 0)
 				dev_err(&slave->dev,
 					"Slave %d initialization failed: %d\n",
 					i, ret);
@@ -1815,7 +1815,7 @@ int sdw_handle_slave_status(struct sdw_bus *bus,
 		}
 
 		ret = sdw_update_slave_status(slave, status[i]);
-		if (ret)
+		if (ret < 0)
 			dev_err(&slave->dev,
 				"Update Slave status failed:%d\n", ret);
 		if (attached_initializing) {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RESEND PATCH 04/11] soundwire: bus: demote clock stop prepare log to dev_dbg()
  2021-03-26  9:15 [RESEND PATCH 00/11] soundwire: some cleanup patches Bard Liao
                   ` (2 preceding siblings ...)
  2021-03-26  9:15 ` [RESEND PATCH 03/11] soundwire: bus: use consistent tests for return values Bard Liao
@ 2021-03-26  9:15 ` Bard Liao
  2021-03-26  9:15 ` [RESEND PATCH 05/11] soundwire: bus: uniquify dev_err() for SCP_INT access Bard Liao
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Bard Liao @ 2021-03-26  9:15 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	rander.wang, hui.wang, pierre-louis.bossart, sanyog.r.kale,
	bard.liao

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

There is no real reason to provide this information except for debug
sessions, hence dev_dbg() is a better fit.

Reported-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/bus.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index d39e5baa3e64..8b6d8fe934ae 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -906,8 +906,8 @@ static int sdw_bus_wait_for_clk_prep_deprep(struct sdw_bus *bus, u16 dev_num)
 		}
 		val &= SDW_SCP_STAT_CLK_STP_NF;
 		if (!val) {
-			dev_info(bus->dev, "clock stop prep/de-prep done slave:%d",
-				 dev_num);
+			dev_dbg(bus->dev, "clock stop prep/de-prep done slave:%d",
+				dev_num);
 			return 0;
 		}
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RESEND PATCH 05/11] soundwire: bus: uniquify dev_err() for SCP_INT access
  2021-03-26  9:15 [RESEND PATCH 00/11] soundwire: some cleanup patches Bard Liao
                   ` (3 preceding siblings ...)
  2021-03-26  9:15 ` [RESEND PATCH 04/11] soundwire: bus: demote clock stop prepare log to dev_dbg() Bard Liao
@ 2021-03-26  9:15 ` Bard Liao
  2021-03-26  9:15 ` [RESEND PATCH 06/11] soundwire: bus: remove useless initialization Bard Liao
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Bard Liao @ 2021-03-26  9:15 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	rander.wang, hui.wang, pierre-louis.bossart, sanyog.r.kale,
	bard.liao

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

We have multiple cases where we read/write SCP_INT registers, but the
same error message in all cases. Add a distinct error message for each
case to help debug.

Reported-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/bus.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 8b6d8fe934ae..a38b017f7a54 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -1636,7 +1636,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave)
 		ret = sdw_read_no_pm(slave, SDW_SCP_INT1);
 		if (ret < 0) {
 			dev_err(&slave->dev,
-				"SDW_SCP_INT1 read failed:%d\n", ret);
+				"SDW_SCP_INT1 recheck read failed:%d\n", ret);
 			goto io_err;
 		}
 		_buf = ret;
@@ -1644,7 +1644,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave)
 		ret = sdw_nread_no_pm(slave, SDW_SCP_INTSTAT2, 2, _buf2);
 		if (ret < 0) {
 			dev_err(&slave->dev,
-				"SDW_SCP_INT2/3 read failed:%d\n", ret);
+				"SDW_SCP_INT2/3 recheck read failed:%d\n", ret);
 			goto io_err;
 		}
 
@@ -1652,7 +1652,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave)
 			ret = sdw_read_no_pm(slave, SDW_DP0_INT);
 			if (ret < 0) {
 				dev_err(&slave->dev,
-					"SDW_DP0_INT read failed:%d\n", ret);
+					"SDW_DP0_INT recheck read failed:%d\n", ret);
 				goto io_err;
 			}
 			sdca_cascade = ret & SDW_DP0_SDCA_CASCADE;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RESEND PATCH 06/11] soundwire: bus: remove useless initialization
  2021-03-26  9:15 [RESEND PATCH 00/11] soundwire: some cleanup patches Bard Liao
                   ` (4 preceding siblings ...)
  2021-03-26  9:15 ` [RESEND PATCH 05/11] soundwire: bus: uniquify dev_err() for SCP_INT access Bard Liao
@ 2021-03-26  9:15 ` Bard Liao
  2021-03-26  9:15 ` [RESEND PATCH 07/11] soundwire: generic_bandwidth_allocation: remove useless init Bard Liao
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Bard Liao @ 2021-03-26  9:15 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	rander.wang, hui.wang, pierre-louis.bossart, sanyog.r.kale,
	bard.liao

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

Cppcheck complains about a possible null pointer dereference, but it's
more like there is an unnecessary initialization before walking
through a list.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/bus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index a38b017f7a54..1a9e307e6a4c 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -593,7 +593,7 @@ EXPORT_SYMBOL(sdw_write);
 /* called with bus_lock held */
 static struct sdw_slave *sdw_get_slave(struct sdw_bus *bus, int i)
 {
-	struct sdw_slave *slave = NULL;
+	struct sdw_slave *slave;
 
 	list_for_each_entry(slave, &bus->slaves, node) {
 		if (slave->dev_num == i)
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RESEND PATCH 07/11] soundwire: generic_bandwidth_allocation: remove useless init
  2021-03-26  9:15 [RESEND PATCH 00/11] soundwire: some cleanup patches Bard Liao
                   ` (5 preceding siblings ...)
  2021-03-26  9:15 ` [RESEND PATCH 06/11] soundwire: bus: remove useless initialization Bard Liao
@ 2021-03-26  9:15 ` Bard Liao
  2021-03-26  9:15 ` [RESEND PATCH 08/11] soundwire: intel: remove useless readl Bard Liao
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Bard Liao @ 2021-03-26  9:15 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	rander.wang, hui.wang, pierre-louis.bossart, sanyog.r.kale,
	bard.liao

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

Cppcheck complains about two possible null pointer dereferences, but
it's more like there are unnecessary initializations before walking
through a list.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/generic_bandwidth_allocation.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soundwire/generic_bandwidth_allocation.c b/drivers/soundwire/generic_bandwidth_allocation.c
index a9abb9722fde..805f5b6ebe86 100644
--- a/drivers/soundwire/generic_bandwidth_allocation.c
+++ b/drivers/soundwire/generic_bandwidth_allocation.c
@@ -143,7 +143,7 @@ static void sdw_compute_master_ports(struct sdw_master_runtime *m_rt,
 static void _sdw_compute_port_params(struct sdw_bus *bus,
 				     struct sdw_group_params *params, int count)
 {
-	struct sdw_master_runtime *m_rt = NULL;
+	struct sdw_master_runtime *m_rt;
 	int hstop = bus->params.col - 1;
 	int block_offset, port_bo, i;
 
@@ -169,7 +169,7 @@ static int sdw_compute_group_params(struct sdw_bus *bus,
 				    struct sdw_group_params *params,
 				    int *rates, int count)
 {
-	struct sdw_master_runtime *m_rt = NULL;
+	struct sdw_master_runtime *m_rt;
 	int sel_col = bus->params.col;
 	unsigned int rate, bps, ch;
 	int i, column_needed = 0;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RESEND PATCH 08/11] soundwire: intel: remove useless readl
  2021-03-26  9:15 [RESEND PATCH 00/11] soundwire: some cleanup patches Bard Liao
                   ` (6 preceding siblings ...)
  2021-03-26  9:15 ` [RESEND PATCH 07/11] soundwire: generic_bandwidth_allocation: remove useless init Bard Liao
@ 2021-03-26  9:15 ` Bard Liao
  2021-03-26  9:15 ` [RESEND PATCH 09/11] soundwire: qcom: check of_property_read status Bard Liao
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Bard Liao @ 2021-03-26  9:15 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	rander.wang, hui.wang, pierre-louis.bossart, sanyog.r.kale,
	bard.liao

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

Cppcheck complains:

drivers/soundwire/intel.c:564:15: style: Variable 'link_control' is
assigned a value that is never used. [unreadVariable]
 link_control = intel_readl(shim, SDW_SHIM_LCTL);

This looks like a leftover from a previous version, remove.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/intel.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index e2e95115832a..fd95f94630b1 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -561,8 +561,6 @@ static int intel_link_power_down(struct sdw_intel *sdw)
 		ret = intel_clear_bit(shim, SDW_SHIM_LCTL, link_control, cpa_mask);
 	}
 
-	link_control = intel_readl(shim, SDW_SHIM_LCTL);
-
 	mutex_unlock(sdw->link_res->shim_lock);
 
 	if (ret < 0) {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RESEND PATCH 09/11] soundwire: qcom: check of_property_read status
  2021-03-26  9:15 [RESEND PATCH 00/11] soundwire: some cleanup patches Bard Liao
                   ` (7 preceding siblings ...)
  2021-03-26  9:15 ` [RESEND PATCH 08/11] soundwire: intel: remove useless readl Bard Liao
@ 2021-03-26  9:15 ` Bard Liao
  2021-03-26  9:15 ` [RESEND PATCH 10/11] soundwire: stream: remove useless initialization Bard Liao
  2021-03-26  9:15 ` [RESEND PATCH 11/11] soundwire: stream: remove useless bus initializations Bard Liao
  10 siblings, 0 replies; 12+ messages in thread
From: Bard Liao @ 2021-03-26  9:15 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	rander.wang, hui.wang, pierre-louis.bossart, sanyog.r.kale,
	bard.liao

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

Cppcheck complains:

drivers/soundwire/qcom.c:773:6: style: Variable 'ret' is assigned a
value that is never used. [unreadVariable]
 ret = of_property_read_u8_array(np, "qcom,ports-block-pack-mode",
     ^

The return value is checked for all other cases, not sure why it was
missed here.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/qcom.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 9cce09cba068..277f711e374d 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -772,6 +772,9 @@ static int qcom_swrm_get_port_config(struct qcom_swrm_ctrl *ctrl)
 
 	ret = of_property_read_u8_array(np, "qcom,ports-block-pack-mode",
 					bp_mode, nports);
+	if (ret)
+		return ret;
+
 	for (i = 0; i < nports; i++) {
 		ctrl->pconfig[i].si = si[i];
 		ctrl->pconfig[i].off1 = off1[i];
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RESEND PATCH 10/11] soundwire: stream: remove useless initialization
  2021-03-26  9:15 [RESEND PATCH 00/11] soundwire: some cleanup patches Bard Liao
                   ` (8 preceding siblings ...)
  2021-03-26  9:15 ` [RESEND PATCH 09/11] soundwire: qcom: check of_property_read status Bard Liao
@ 2021-03-26  9:15 ` Bard Liao
  2021-03-26  9:15 ` [RESEND PATCH 11/11] soundwire: stream: remove useless bus initializations Bard Liao
  10 siblings, 0 replies; 12+ messages in thread
From: Bard Liao @ 2021-03-26  9:15 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	rander.wang, hui.wang, pierre-louis.bossart, sanyog.r.kale,
	bard.liao

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

Cppcheck complains about possible null pointer dereferences, but it's
more like there are unnecessary initializations before walking
through a list.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 4915676c4ac2..6a682179cd05 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -261,7 +261,7 @@ static int sdw_program_master_port_params(struct sdw_bus *bus,
  */
 static int sdw_program_port_params(struct sdw_master_runtime *m_rt)
 {
-	struct sdw_slave_runtime *s_rt = NULL;
+	struct sdw_slave_runtime *s_rt;
 	struct sdw_bus *bus = m_rt->bus;
 	struct sdw_port_runtime *p_rt;
 	int ret = 0;
@@ -1470,7 +1470,7 @@ static void sdw_acquire_bus_lock(struct sdw_stream_runtime *stream)
  */
 static void sdw_release_bus_lock(struct sdw_stream_runtime *stream)
 {
-	struct sdw_master_runtime *m_rt = NULL;
+	struct sdw_master_runtime *m_rt;
 	struct sdw_bus *bus = NULL;
 
 	/* Iterate for all Master(s) in Master list */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RESEND PATCH 11/11] soundwire: stream: remove useless bus initializations
  2021-03-26  9:15 [RESEND PATCH 00/11] soundwire: some cleanup patches Bard Liao
                   ` (9 preceding siblings ...)
  2021-03-26  9:15 ` [RESEND PATCH 10/11] soundwire: stream: remove useless initialization Bard Liao
@ 2021-03-26  9:15 ` Bard Liao
  10 siblings, 0 replies; 12+ messages in thread
From: Bard Liao @ 2021-03-26  9:15 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	rander.wang, hui.wang, pierre-louis.bossart, sanyog.r.kale,
	bard.liao

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

There is no need to assign a pointer to NULL if it's only used in a
loop and assigned within that loop.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 6a682179cd05..9c064b672745 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1449,7 +1449,7 @@ struct sdw_dpn_prop *sdw_get_slave_dpn_prop(struct sdw_slave *slave,
 static void sdw_acquire_bus_lock(struct sdw_stream_runtime *stream)
 {
 	struct sdw_master_runtime *m_rt;
-	struct sdw_bus *bus = NULL;
+	struct sdw_bus *bus;
 
 	/* Iterate for all Master(s) in Master list */
 	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
@@ -1471,7 +1471,7 @@ static void sdw_acquire_bus_lock(struct sdw_stream_runtime *stream)
 static void sdw_release_bus_lock(struct sdw_stream_runtime *stream)
 {
 	struct sdw_master_runtime *m_rt;
-	struct sdw_bus *bus = NULL;
+	struct sdw_bus *bus;
 
 	/* Iterate for all Master(s) in Master list */
 	list_for_each_entry_reverse(m_rt, &stream->master_list, stream_node) {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-03-26  9:16 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-26  9:15 [RESEND PATCH 00/11] soundwire: some cleanup patches Bard Liao
2021-03-26  9:15 ` [RESEND PATCH 01/11] soundwire: bus: use correct driver name in error messages Bard Liao
2021-03-26  9:15 ` [RESEND PATCH 02/11] soundwire: bus: test read status Bard Liao
2021-03-26  9:15 ` [RESEND PATCH 03/11] soundwire: bus: use consistent tests for return values Bard Liao
2021-03-26  9:15 ` [RESEND PATCH 04/11] soundwire: bus: demote clock stop prepare log to dev_dbg() Bard Liao
2021-03-26  9:15 ` [RESEND PATCH 05/11] soundwire: bus: uniquify dev_err() for SCP_INT access Bard Liao
2021-03-26  9:15 ` [RESEND PATCH 06/11] soundwire: bus: remove useless initialization Bard Liao
2021-03-26  9:15 ` [RESEND PATCH 07/11] soundwire: generic_bandwidth_allocation: remove useless init Bard Liao
2021-03-26  9:15 ` [RESEND PATCH 08/11] soundwire: intel: remove useless readl Bard Liao
2021-03-26  9:15 ` [RESEND PATCH 09/11] soundwire: qcom: check of_property_read status Bard Liao
2021-03-26  9:15 ` [RESEND PATCH 10/11] soundwire: stream: remove useless initialization Bard Liao
2021-03-26  9:15 ` [RESEND PATCH 11/11] soundwire: stream: remove useless bus initializations Bard Liao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).