[PATCH v3 0/5] soundwire: code cleanup

[PATCH v3 0/5] soundwire: code cleanup

[Pierre-Louis Bossart]

SoundWire support will be provided in Linux with the Sound Open
Firmware (SOF) on Intel platforms. Before we start adding the missing
pieces, there are a number of warnings and style issues reported by
checkpatch, cppcheck and Coccinelle that need to be cleaned-up.

Changes since v2:
fixed inversion of devm_kcalloc parameters, detected while rebasing
additional patches.

Changes since v1:
added missing newlines in new patch (suggested by Joe Perches)

Pierre-Louis Bossart (5):
  soundwire: intel: fix inversion in devm_kcalloc parameters
  soundwire: fix style issues
  soundwire: bus: remove useless initializations
  soundwire: stream: remove useless initialization of local variable
  soundwire: add missing newlines in dynamic debug logs

 drivers/soundwire/Kconfig          |   2 +-
 drivers/soundwire/bus.c            | 137 ++++++++-------
 drivers/soundwire/bus.h            |  16 +-
 drivers/soundwire/bus_type.c       |   4 +-
 drivers/soundwire/cadence_master.c |  99 +++++------
 drivers/soundwire/cadence_master.h |  22 +--
 drivers/soundwire/intel.c          | 103 ++++++-----
 drivers/soundwire/intel.h          |   4 +-
 drivers/soundwire/intel_init.c     |  12 +-
 drivers/soundwire/mipi_disco.c     | 116 +++++++------
 drivers/soundwire/slave.c          |  10 +-
 drivers/soundwire/stream.c         | 267 +++++++++++++++--------------
 12 files changed, 404 insertions(+), 388 deletions(-)

-- 
2.17.1

[PATCH v3 1/5] soundwire: intel: fix inversion in devm_kcalloc parameters

[Pierre-Louis Bossart]

the number of elements and size are inverted, fix.

This probably only worked because the number of properties is
hard-coded to 1.

Fixes: 71bb8a1b059e ('soundwire: intel: Add Intel Master driver')
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 drivers/soundwire/intel.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index fd8d034cfec1..8669b314c476 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -796,8 +796,8 @@ static int intel_prop_read(struct sdw_bus *bus)
 
 	/* BIOS is not giving some values correctly. So, lets override them */
 	bus->prop.num_freq = 1;
-	bus->prop.freq = devm_kcalloc(bus->dev, sizeof(*bus->prop.freq),
-					bus->prop.num_freq, GFP_KERNEL);
+	bus->prop.freq = devm_kcalloc(bus->dev, bus->prop.num_freq,
+				      sizeof(*bus->prop.freq), GFP_KERNEL);
 	if (!bus->prop.freq)
 		return -ENOMEM;
 
-- 
2.17.1

Re: [PATCH v3 1/5] soundwire: intel: fix inversion in devm_kcalloc parameters

[Takashi Iwai]

On Thu, 11 Apr 2019 05:16:57 +0200,
Pierre-Louis Bossart wrote:
> 
> the number of elements and size are inverted, fix.
> 
> This probably only worked because the number of properties is
> hard-coded to 1.

Well, both are mathematically equivalent :)


Takashi

> 
> Fixes: 71bb8a1b059e ('soundwire: intel: Add Intel Master driver')
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
>  drivers/soundwire/intel.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
> index fd8d034cfec1..8669b314c476 100644
> --- a/drivers/soundwire/intel.c
> +++ b/drivers/soundwire/intel.c
> @@ -796,8 +796,8 @@ static int intel_prop_read(struct sdw_bus *bus)
>  
>  	/* BIOS is not giving some values correctly. So, lets override them */
>  	bus->prop.num_freq = 1;
> -	bus->prop.freq = devm_kcalloc(bus->dev, sizeof(*bus->prop.freq),
> -					bus->prop.num_freq, GFP_KERNEL);
> +	bus->prop.freq = devm_kcalloc(bus->dev, bus->prop.num_freq,
> +				      sizeof(*bus->prop.freq), GFP_KERNEL);
>  	if (!bus->prop.freq)
>  		return -ENOMEM;
>  
> -- 
> 2.17.1
> 

[PATCH v3 2/5] soundwire: fix style issues

[Pierre-Louis Bossart]

Visual inspections confirmed by checkpatch.pl --strict expose a number
of style issues, specifically parameter alignment is inconsistent as
if different contributors used different styles. Before we restart
support for SoundWire with Sound Open Firmware on Intel platforms,
let's clean all this.

Fix Kconfig help, spelling, SPDX format, alignment, spurious
parentheses, bool comparisons to true/false, macro argument
protection.

No new functionality added.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 drivers/soundwire/Kconfig          |   2 +-
 drivers/soundwire/bus.c            |  87 ++++++++--------
 drivers/soundwire/bus.h            |  16 +--
 drivers/soundwire/bus_type.c       |   4 +-
 drivers/soundwire/cadence_master.c |  87 ++++++++--------
 drivers/soundwire/cadence_master.h |  22 ++--
 drivers/soundwire/intel.c          |  87 ++++++++--------
 drivers/soundwire/intel.h          |   4 +-
 drivers/soundwire/intel_init.c     |  12 +--
 drivers/soundwire/mipi_disco.c     | 116 +++++++++++----------
 drivers/soundwire/slave.c          |  10 +-
 drivers/soundwire/stream.c         | 161 +++++++++++++++--------------
 12 files changed, 313 insertions(+), 295 deletions(-)

diff --git a/drivers/soundwire/Kconfig b/drivers/soundwire/Kconfig
index 19c8efb9a5ee..84876a74874f 100644
--- a/drivers/soundwire/Kconfig
+++ b/drivers/soundwire/Kconfig
@@ -4,7 +4,7 @@
 
 menuconfig SOUNDWIRE
 	bool "SoundWire support"
-	---help---
+	help
 	  SoundWire is a 2-Pin interface with data and clock line ratified
 	  by the MIPI Alliance. SoundWire is used for transporting data
 	  typically related to audio functions. SoundWire interface is
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 1cbfedfc20ef..691a31df9732 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -49,7 +49,7 @@ int sdw_add_bus_master(struct sdw_bus *bus)
 	}
 
 	/*
-	 * Device numbers in SoundWire are 0 thru 15. Enumeration device
+	 * Device numbers in SoundWire are 0 through 15. Enumeration device
 	 * number (0), Broadcast device number (15), Group numbers (12 and
 	 * 13) and Master device number (14) are not used for assignment so
 	 * mask these and other higher bits.
@@ -172,7 +172,8 @@ static inline int do_transfer(struct sdw_bus *bus, struct sdw_msg *msg)
 }
 
 static inline int do_transfer_defer(struct sdw_bus *bus,
-			struct sdw_msg *msg, struct sdw_defer *defer)
+				    struct sdw_msg *msg,
+				    struct sdw_defer *defer)
 {
 	int retry = bus->prop.err_threshold;
 	enum sdw_command_response resp;
@@ -224,7 +225,7 @@ int sdw_transfer(struct sdw_bus *bus, struct sdw_msg *msg)
 	ret = do_transfer(bus, msg);
 	if (ret != 0 && ret != -ENODATA)
 		dev_err(bus->dev, "trf on Slave %d failed:%d\n",
-				msg->dev_num, ret);
+			msg->dev_num, ret);
 
 	if (msg->page)
 		sdw_reset_page(bus, msg->dev_num);
@@ -243,7 +244,7 @@ int sdw_transfer(struct sdw_bus *bus, struct sdw_msg *msg)
  * Caller needs to hold the msg_lock lock while calling this
  */
 int sdw_transfer_defer(struct sdw_bus *bus, struct sdw_msg *msg,
-				struct sdw_defer *defer)
+		       struct sdw_defer *defer)
 {
 	int ret;
 
@@ -253,7 +254,7 @@ int sdw_transfer_defer(struct sdw_bus *bus, struct sdw_msg *msg,
 	ret = do_transfer_defer(bus, msg, defer);
 	if (ret != 0 && ret != -ENODATA)
 		dev_err(bus->dev, "Defer trf on Slave %d failed:%d\n",
-				msg->dev_num, ret);
+			msg->dev_num, ret);
 
 	if (msg->page)
 		sdw_reset_page(bus, msg->dev_num);
@@ -261,9 +262,8 @@ int sdw_transfer_defer(struct sdw_bus *bus, struct sdw_msg *msg,
 	return ret;
 }
 
-
 int sdw_fill_msg(struct sdw_msg *msg, struct sdw_slave *slave,
-		u32 addr, size_t count, u16 dev_num, u8 flags, u8 *buf)
+		 u32 addr, size_t count, u16 dev_num, u8 flags, u8 *buf)
 {
 	memset(msg, 0, sizeof(*msg));
 	msg->addr = addr; /* addr is 16 bit and truncated here */
@@ -284,7 +284,7 @@ int sdw_fill_msg(struct sdw_msg *msg, struct sdw_slave *slave,
 	if (addr < SDW_REG_OPTIONAL_PAGE) { /* 32k but no page */
 		if (slave && !slave->prop.paging_support)
 			return 0;
-		/* no need for else as that will fall thru to paging */
+		/* no need for else as that will fall-through to paging */
 	}
 
 	/* paging mandatory */
@@ -323,7 +323,7 @@ int sdw_nread(struct sdw_slave *slave, u32 addr, size_t count, u8 *val)
 	int ret;
 
 	ret = sdw_fill_msg(&msg, slave, addr, count,
-			slave->dev_num, SDW_MSG_FLAG_READ, val);
+			   slave->dev_num, SDW_MSG_FLAG_READ, val);
 	if (ret < 0)
 		return ret;
 
@@ -351,7 +351,7 @@ int sdw_nwrite(struct sdw_slave *slave, u32 addr, size_t count, u8 *val)
 	int ret;
 
 	ret = sdw_fill_msg(&msg, slave, addr, count,
-			slave->dev_num, SDW_MSG_FLAG_WRITE, val);
+			   slave->dev_num, SDW_MSG_FLAG_WRITE, val);
 	if (ret < 0)
 		return ret;
 
@@ -417,10 +417,10 @@ static struct sdw_slave *sdw_get_slave(struct sdw_bus *bus, int i)
 static int sdw_compare_devid(struct sdw_slave *slave, struct sdw_slave_id id)
 {
 
-	if ((slave->id.unique_id != id.unique_id) ||
-	    (slave->id.mfg_id != id.mfg_id) ||
-	    (slave->id.part_id != id.part_id) ||
-	    (slave->id.class_id != id.class_id))
+	if (slave->id.unique_id != id.unique_id ||
+	    slave->id.mfg_id != id.mfg_id ||
+	    slave->id.part_id != id.part_id ||
+	    slave->id.class_id != id.class_id)
 		return -ENODEV;
 
 	return 0;
@@ -458,13 +458,13 @@ static int sdw_assign_device_num(struct sdw_slave *slave)
 		mutex_unlock(&slave->bus->bus_lock);
 		if (dev_num < 0) {
 			dev_err(slave->bus->dev, "Get dev_num failed: %d",
-								dev_num);
+				dev_num);
 			return dev_num;
 		}
 	} else {
 		dev_info(slave->bus->dev,
-				"Slave already registered dev_num:%d",
-				slave->dev_num);
+			 "Slave already registered dev_num:%d",
+			 slave->dev_num);
 
 		/* Clear the slave->dev_num to transfer message on device 0 */
 		dev_num = slave->dev_num;
@@ -485,7 +485,7 @@ static int sdw_assign_device_num(struct sdw_slave *slave)
 }
 
 void sdw_extract_slave_id(struct sdw_bus *bus,
-			u64 addr, struct sdw_slave_id *id)
+			  u64 addr, struct sdw_slave_id *id)
 {
 	dev_dbg(bus->dev, "SDW Slave Addr: %llx", addr);
 
@@ -525,7 +525,7 @@ static int sdw_program_device_num(struct sdw_bus *bus)
 
 	/* No Slave, so use raw xfer api */
 	ret = sdw_fill_msg(&msg, NULL, SDW_SCP_DEVID_0,
-			SDW_NUM_DEV_ID_REGISTERS, 0, SDW_MSG_FLAG_READ, buf);
+			   SDW_NUM_DEV_ID_REGISTERS, 0, SDW_MSG_FLAG_READ, buf);
 	if (ret < 0)
 		return ret;
 
@@ -573,7 +573,7 @@ static int sdw_program_device_num(struct sdw_bus *bus)
 			}
 		}
 
-		if (found == false) {
+		if (!found) {
 			/* TODO: Park this device in Group 13 */
 			dev_err(bus->dev, "Slave Entry not found");
 		}
@@ -592,7 +592,7 @@ static int sdw_program_device_num(struct sdw_bus *bus)
 }
 
 static void sdw_modify_slave_status(struct sdw_slave *slave,
-				enum sdw_slave_status status)
+				    enum sdw_slave_status status)
 {
 	mutex_lock(&slave->bus->bus_lock);
 	slave->status = status;
@@ -600,7 +600,7 @@ static void sdw_modify_slave_status(struct sdw_slave *slave,
 }
 
 int sdw_configure_dpn_intr(struct sdw_slave *slave,
-			int port, bool enable, int mask)
+			   int port, bool enable, int mask)
 {
 	u32 addr;
 	int ret;
@@ -620,7 +620,7 @@ int sdw_configure_dpn_intr(struct sdw_slave *slave,
 	ret = sdw_update(slave, addr, (mask | SDW_DPN_INT_PORT_READY), val);
 	if (ret < 0)
 		dev_err(slave->bus->dev,
-				"SDW_DPN_INTMASK write failed:%d", val);
+			"SDW_DPN_INTMASK write failed:%d", val);
 
 	return ret;
 }
@@ -644,7 +644,7 @@ static int sdw_initialize_slave(struct sdw_slave *slave)
 	ret = sdw_update(slave, SDW_SCP_INTMASK1, val, val);
 	if (ret < 0) {
 		dev_err(slave->bus->dev,
-				"SDW_SCP_INTMASK1 write failed:%d", ret);
+			"SDW_SCP_INTMASK1 write failed:%d", ret);
 		return ret;
 	}
 
@@ -659,7 +659,7 @@ static int sdw_initialize_slave(struct sdw_slave *slave)
 	ret = sdw_update(slave, SDW_DP0_INTMASK, val, val);
 	if (ret < 0) {
 		dev_err(slave->bus->dev,
-				"SDW_DP0_INTMASK read failed:%d", ret);
+			"SDW_DP0_INTMASK read failed:%d", ret);
 		return val;
 	}
 
@@ -674,7 +674,7 @@ static int sdw_handle_dp0_interrupt(struct sdw_slave *slave, u8 *slave_status)
 	status = sdw_read(slave, SDW_DP0_INT);
 	if (status < 0) {
 		dev_err(slave->bus->dev,
-				"SDW_DP0_INT read failed:%d", status);
+			"SDW_DP0_INT read failed:%d", status);
 		return status;
 	}
 
@@ -737,7 +737,7 @@ static int sdw_handle_dp0_interrupt(struct sdw_slave *slave, u8 *slave_status)
 }
 
 static int sdw_handle_port_interrupt(struct sdw_slave *slave,
-		int port, u8 *slave_status)
+				     int port, u8 *slave_status)
 {
 	u8 clear = 0, impl_int_mask;
 	int status, status2, ret, count = 0;
@@ -750,7 +750,7 @@ static int sdw_handle_port_interrupt(struct sdw_slave *slave,
 	status = sdw_read(slave, addr);
 	if (status < 0) {
 		dev_err(slave->bus->dev,
-				"SDW_DPN_INT read failed:%d", status);
+			"SDW_DPN_INT read failed:%d", status);
 
 		return status;
 	}
@@ -774,7 +774,6 @@ static int sdw_handle_port_interrupt(struct sdw_slave *slave,
 		impl_int_mask = SDW_DPN_INT_IMPDEF1 |
 			SDW_DPN_INT_IMPDEF2 | SDW_DPN_INT_IMPDEF3;
 
-
 		if (status & impl_int_mask) {
 			clear |= impl_int_mask;
 			*slave_status = clear;
@@ -784,7 +783,7 @@ static int sdw_handle_port_interrupt(struct sdw_slave *slave,
 		ret = sdw_write(slave, addr, clear);
 		if (ret < 0) {
 			dev_err(slave->bus->dev,
-					"SDW_DPN_INT write failed:%d", ret);
+				"SDW_DPN_INT write failed:%d", ret);
 			return ret;
 		}
 
@@ -792,7 +791,7 @@ static int sdw_handle_port_interrupt(struct sdw_slave *slave,
 		status2 = sdw_read(slave, addr);
 		if (status2 < 0) {
 			dev_err(slave->bus->dev,
-					"SDW_DPN_INT read failed:%d", status2);
+				"SDW_DPN_INT read failed:%d", status2);
 			return status2;
 		}
 		status &= status2;
@@ -823,14 +822,14 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave)
 	buf = ret = sdw_read(slave, SDW_SCP_INT1);
 	if (ret < 0) {
 		dev_err(slave->bus->dev,
-					"SDW_SCP_INT1 read failed:%d", ret);
+			"SDW_SCP_INT1 read failed:%d", ret);
 		return ret;
 	}
 
 	ret = sdw_nread(slave, SDW_SCP_INTSTAT2, 2, buf2);
 	if (ret < 0) {
 		dev_err(slave->bus->dev,
-					"SDW_SCP_INT2/3 read failed:%d", ret);
+			"SDW_SCP_INT2/3 read failed:%d", ret);
 		return ret;
 	}
 
@@ -869,7 +868,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave)
 		port = port >> SDW_REG_SHIFT(SDW_SCP_INT1_PORT0_3);
 		for_each_set_bit(bit, &port, 8) {
 			sdw_handle_port_interrupt(slave, bit,
-						&port_status[bit]);
+						  &port_status[bit]);
 
 		}
 
@@ -898,11 +897,11 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave)
 		}
 
 		/* Update the Slave driver */
-		if (slave_notify && (slave->ops) &&
-					(slave->ops->interrupt_callback)) {
+		if (slave_notify && slave->ops &&
+		    slave->ops->interrupt_callback) {
 			slave_intr.control_port = clear;
 			memcpy(slave_intr.port, &port_status,
-						sizeof(slave_intr.port));
+			       sizeof(slave_intr.port));
 
 			slave->ops->interrupt_callback(slave, &slave_intr);
 		}
@@ -911,7 +910,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave)
 		ret = sdw_write(slave, SDW_SCP_INT1, clear);
 		if (ret < 0) {
 			dev_err(slave->bus->dev,
-					"SDW_SCP_INT1 write failed:%d", ret);
+				"SDW_SCP_INT1 write failed:%d", ret);
 			return ret;
 		}
 
@@ -922,14 +921,14 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave)
 		_buf = ret = sdw_read(slave, SDW_SCP_INT1);
 		if (ret < 0) {
 			dev_err(slave->bus->dev,
-					"SDW_SCP_INT1 read failed:%d", ret);
+				"SDW_SCP_INT1 read failed:%d", ret);
 			return ret;
 		}
 
 		ret = sdw_nread(slave, SDW_SCP_INTSTAT2, 2, _buf2);
 		if (ret < 0) {
 			dev_err(slave->bus->dev,
-					"SDW_SCP_INT2/3 read failed:%d", ret);
+				"SDW_SCP_INT2/3 read failed:%d", ret);
 			return ret;
 		}
 
@@ -955,9 +954,9 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave)
 }
 
 static int sdw_update_slave_status(struct sdw_slave *slave,
-				enum sdw_slave_status status)
+				   enum sdw_slave_status status)
 {
-	if ((slave->ops) && (slave->ops->update_status))
+	if (slave->ops && slave->ops->update_status)
 		return slave->ops->update_status(slave, status);
 
 	return 0;
@@ -969,7 +968,7 @@ static int sdw_update_slave_status(struct sdw_slave *slave,
  * @status: Status for all Slave(s)
  */
 int sdw_handle_slave_status(struct sdw_bus *bus,
-			enum sdw_slave_status status[])
+			    enum sdw_slave_status status[])
 {
 	enum sdw_slave_status prev_status;
 	struct sdw_slave *slave;
@@ -1030,7 +1029,7 @@ int sdw_handle_slave_status(struct sdw_bus *bus,
 
 		default:
 			dev_err(bus->dev, "Invalid slave %d status:%d",
-							i, status[i]);
+				i, status[i]);
 			break;
 		}
 
diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h
index c77de05b8100..3048ca153f22 100644
--- a/drivers/soundwire/bus.h
+++ b/drivers/soundwire/bus.h
@@ -1,5 +1,5 @@
-// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
-// Copyright(c) 2015-17 Intel Corporation.
+/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */
+/* Copyright(c) 2015-17 Intel Corporation. */
 
 #ifndef __SDW_BUS_H
 #define __SDW_BUS_H
@@ -16,7 +16,7 @@ static inline int sdw_acpi_find_slaves(struct sdw_bus *bus)
 #endif
 
 void sdw_extract_slave_id(struct sdw_bus *bus,
-			u64 addr, struct sdw_slave_id *id);
+			  u64 addr, struct sdw_slave_id *id);
 
 enum {
 	SDW_MSG_FLAG_READ = 0,
@@ -116,19 +116,19 @@ struct sdw_master_runtime {
 };
 
 struct sdw_dpn_prop *sdw_get_slave_dpn_prop(struct sdw_slave *slave,
-				enum sdw_data_direction direction,
-				unsigned int port_num);
+					    enum sdw_data_direction direction,
+					    unsigned int port_num);
 int sdw_configure_dpn_intr(struct sdw_slave *slave, int port,
-					bool enable, int mask);
+			   bool enable, int mask);
 
 int sdw_transfer(struct sdw_bus *bus, struct sdw_msg *msg);
 int sdw_transfer_defer(struct sdw_bus *bus, struct sdw_msg *msg,
-				struct sdw_defer *defer);
+		       struct sdw_defer *defer);
 
 #define SDW_READ_INTR_CLEAR_RETRY	10
 
 int sdw_fill_msg(struct sdw_msg *msg, struct sdw_slave *slave,
-		u32 addr, size_t count, u16 dev_num, u8 flags, u8 *buf);
+		 u32 addr, size_t count, u16 dev_num, u8 flags, u8 *buf);
 
 /* Read-Modify-Write Slave register */
 static inline int
diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c
index 283b2832728e..2655602f0cfb 100644
--- a/drivers/soundwire/bus_type.c
+++ b/drivers/soundwire/bus_type.c
@@ -107,7 +107,7 @@ static int sdw_drv_probe(struct device *dev)
 		slave->prop.clk_stop_timeout = 300;
 
 	slave->bus->clk_stop_timeout = max_t(u32, slave->bus->clk_stop_timeout,
-					slave->prop.clk_stop_timeout);
+					     slave->prop.clk_stop_timeout);
 
 	return 0;
 }
@@ -148,7 +148,7 @@ int __sdw_register_driver(struct sdw_driver *drv, struct module *owner)
 
 	if (!drv->probe) {
 		pr_err("driver %s didn't provide SDW probe routine\n",
-							drv->name);
+		       drv->name);
 		return -EINVAL;
 	}
 
diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c
index cb6a331f448a..5786a2e0be84 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -42,7 +42,6 @@
 #define CDNS_MCP_CONTROL_CMD_ACCEPT		BIT(1)
 #define CDNS_MCP_CONTROL_BLOCK_WAKEUP		BIT(0)
 
-
 #define CDNS_MCP_CMDCTRL			0x8
 #define CDNS_MCP_SSPSTAT			0xC
 #define CDNS_MCP_FRAME_SHAPE			0x10
@@ -226,9 +225,9 @@ static int cdns_clear_bit(struct sdw_cdns *cdns, int offset, u32 value)
 /*
  * IO Calls
  */
-static enum sdw_command_response cdns_fill_msg_resp(
-			struct sdw_cdns *cdns,
-			struct sdw_msg *msg, int count, int offset)
+static enum sdw_command_response
+cdns_fill_msg_resp(struct sdw_cdns *cdns,
+		   struct sdw_msg *msg, int count, int offset)
 {
 	int nack = 0, no_ack = 0;
 	int i;
@@ -263,7 +262,7 @@ static enum sdw_command_response cdns_fill_msg_resp(
 
 static enum sdw_command_response
 _cdns_xfer_msg(struct sdw_cdns *cdns, struct sdw_msg *msg, int cmd,
-				int offset, int count, bool defer)
+	       int offset, int count, bool defer)
 {
 	unsigned long time;
 	u32 base, i, data;
@@ -296,7 +295,7 @@ _cdns_xfer_msg(struct sdw_cdns *cdns, struct sdw_msg *msg, int cmd,
 
 	/* wait for timeout or response */
 	time = wait_for_completion_timeout(&cdns->tx_complete,
-				msecs_to_jiffies(CDNS_TX_TIMEOUT));
+					   msecs_to_jiffies(CDNS_TX_TIMEOUT));
 	if (!time) {
 		dev_err(cdns->dev, "IO transfer timed out\n");
 		msg->len = 0;
@@ -306,8 +305,8 @@ _cdns_xfer_msg(struct sdw_cdns *cdns, struct sdw_msg *msg, int cmd,
 	return cdns_fill_msg_resp(cdns, msg, count, offset);
 }
 
-static enum sdw_command_response cdns_program_scp_addr(
-			struct sdw_cdns *cdns, struct sdw_msg *msg)
+static enum sdw_command_response
+cdns_program_scp_addr(struct sdw_cdns *cdns, struct sdw_msg *msg)
 {
 	int nack = 0, no_ack = 0;
 	unsigned long time;
@@ -336,7 +335,7 @@ static enum sdw_command_response cdns_program_scp_addr(
 	cdns_writel(cdns, base, data[1]);
 
 	time = wait_for_completion_timeout(&cdns->tx_complete,
-				msecs_to_jiffies(CDNS_TX_TIMEOUT));
+					   msecs_to_jiffies(CDNS_TX_TIMEOUT));
 	if (!time) {
 		dev_err(cdns->dev, "SCP Msg trf timed out\n");
 		msg->len = 0;
@@ -410,7 +409,7 @@ cdns_xfer_msg(struct sdw_bus *bus, struct sdw_msg *msg)
 
 	for (i = 0; i < msg->len / CDNS_MCP_CMD_LEN; i++) {
 		ret = _cdns_xfer_msg(cdns, msg, cmd, i * CDNS_MCP_CMD_LEN,
-				CDNS_MCP_CMD_LEN, false);
+				     CDNS_MCP_CMD_LEN, false);
 		if (ret < 0)
 			goto exit;
 	}
@@ -419,7 +418,7 @@ cdns_xfer_msg(struct sdw_bus *bus, struct sdw_msg *msg)
 		goto exit;
 
 	ret = _cdns_xfer_msg(cdns, msg, cmd, i * CDNS_MCP_CMD_LEN,
-			msg->len % CDNS_MCP_CMD_LEN, false);
+			     msg->len % CDNS_MCP_CMD_LEN, false);
 
 exit:
 	return ret;
@@ -428,7 +427,7 @@ EXPORT_SYMBOL(cdns_xfer_msg);
 
 enum sdw_command_response
 cdns_xfer_msg_defer(struct sdw_bus *bus,
-		struct sdw_msg *msg, struct sdw_defer *defer)
+		    struct sdw_msg *msg, struct sdw_defer *defer)
 {
 	struct sdw_cdns *cdns = bus_to_cdns(bus);
 	int cmd = 0, ret;
@@ -483,7 +482,7 @@ static void cdns_read_response(struct sdw_cdns *cdns)
 }
 
 static int cdns_update_slave_status(struct sdw_cdns *cdns,
-					u32 slave0, u32 slave1)
+				    u32 slave0, u32 slave1)
 {
 	enum sdw_slave_status status[SDW_MAX_DEVICES + 1];
 	bool is_slave = false;
@@ -526,8 +525,8 @@ static int cdns_update_slave_status(struct sdw_cdns *cdns,
 		/* first check if Slave reported multiple status */
 		if (set_status > 1) {
 			dev_warn(cdns->dev,
-					"Slave reported multiple Status: %d\n",
-					status[i]);
+				 "Slave reported multiple Status: %d\n",
+				 status[i]);
 			/*
 			 * TODO: we need to reread the status here by
 			 * issuing a PING cmd
@@ -566,11 +565,12 @@ irqreturn_t sdw_cdns_irq(int irq, void *dev_id)
 
 		if (cdns->defer) {
 			cdns_fill_msg_resp(cdns, cdns->defer->msg,
-					cdns->defer->length, 0);
+					   cdns->defer->length, 0);
 			complete(&cdns->defer->complete);
 			cdns->defer = NULL;
-		} else
+		} else {
 			complete(&cdns->tx_complete);
+		}
 	}
 
 	if (int_status & CDNS_MCP_INT_CTRL_CLASH) {
@@ -592,7 +592,7 @@ irqreturn_t sdw_cdns_irq(int irq, void *dev_id)
 	if (int_status & CDNS_MCP_INT_SLAVE_MASK) {
 		/* Mask the Slave interrupt and wake thread */
 		cdns_updatel(cdns, CDNS_MCP_INTMASK,
-				CDNS_MCP_INT_SLAVE_MASK, 0);
+			     CDNS_MCP_INT_SLAVE_MASK, 0);
 
 		int_status &= ~CDNS_MCP_INT_SLAVE_MASK;
 		ret = IRQ_WAKE_THREAD;
@@ -625,7 +625,7 @@ irqreturn_t sdw_cdns_thread(int irq, void *dev_id)
 	/* clear and unmask Slave interrupt now */
 	cdns_writel(cdns, CDNS_MCP_INTSTAT, CDNS_MCP_INT_SLAVE_MASK);
 	cdns_updatel(cdns, CDNS_MCP_INTMASK,
-			CDNS_MCP_INT_SLAVE_MASK, CDNS_MCP_INT_SLAVE_MASK);
+		     CDNS_MCP_INT_SLAVE_MASK, CDNS_MCP_INT_SLAVE_MASK);
 
 	return IRQ_HANDLED;
 }
@@ -639,9 +639,9 @@ static int _cdns_enable_interrupt(struct sdw_cdns *cdns)
 	u32 mask;
 
 	cdns_writel(cdns, CDNS_MCP_SLAVE_INTMASK0,
-				CDNS_MCP_SLAVE_INTMASK0_MASK);
+		    CDNS_MCP_SLAVE_INTMASK0_MASK);
 	cdns_writel(cdns, CDNS_MCP_SLAVE_INTMASK1,
-				CDNS_MCP_SLAVE_INTMASK1_MASK);
+		    CDNS_MCP_SLAVE_INTMASK1_MASK);
 
 	mask = CDNS_MCP_INT_SLAVE_RSVD | CDNS_MCP_INT_SLAVE_ALERT |
 		CDNS_MCP_INT_SLAVE_ATTACH | CDNS_MCP_INT_SLAVE_NATTACH |
@@ -663,7 +663,7 @@ int sdw_cdns_enable_interrupt(struct sdw_cdns *cdns)
 
 	_cdns_enable_interrupt(cdns);
 	ret = cdns_clear_bit(cdns, CDNS_MCP_CONFIG_UPDATE,
-			CDNS_MCP_CONFIG_UPDATE_BIT);
+			     CDNS_MCP_CONFIG_UPDATE_BIT);
 	if (ret < 0)
 		dev_err(cdns->dev, "Config update timedout");
 
@@ -672,8 +672,8 @@ int sdw_cdns_enable_interrupt(struct sdw_cdns *cdns)
 EXPORT_SYMBOL(sdw_cdns_enable_interrupt);
 
 static int cdns_allocate_pdi(struct sdw_cdns *cdns,
-			struct sdw_cdns_pdi **stream,
-			u32 num, u32 pdi_offset)
+			     struct sdw_cdns_pdi **stream,
+			     u32 num, u32 pdi_offset)
 {
 	struct sdw_cdns_pdi *pdi;
 	int i;
@@ -701,7 +701,7 @@ static int cdns_allocate_pdi(struct sdw_cdns *cdns,
  * @config: Stream configurations
  */
 int sdw_cdns_pdi_init(struct sdw_cdns *cdns,
-			struct sdw_cdns_stream_config config)
+		      struct sdw_cdns_stream_config config)
 {
 	struct sdw_cdns_streams *stream;
 	int offset, i, ret;
@@ -770,7 +770,7 @@ int sdw_cdns_pdi_init(struct sdw_cdns *cdns,
 	cdns->num_ports += stream->num_pdi;
 
 	cdns->ports = devm_kcalloc(cdns->dev, cdns->num_ports,
-				sizeof(*cdns->ports), GFP_KERNEL);
+				   sizeof(*cdns->ports), GFP_KERNEL);
 	if (!cdns->ports) {
 		ret = -ENOMEM;
 		return ret;
@@ -796,7 +796,7 @@ int sdw_cdns_init(struct sdw_cdns *cdns)
 
 	/* Exit clock stop */
 	ret = cdns_clear_bit(cdns, CDNS_MCP_CONTROL,
-			CDNS_MCP_CONTROL_CLK_STOP_CLR);
+			     CDNS_MCP_CONTROL_CLK_STOP_CLR);
 	if (ret < 0) {
 		dev_err(cdns->dev, "Couldn't exit from clock stop\n");
 		return ret;
@@ -816,7 +816,7 @@ int sdw_cdns_init(struct sdw_cdns *cdns)
 
 	/* Set cmd accept mode */
 	cdns_updatel(cdns, CDNS_MCP_CONTROL, CDNS_MCP_CONTROL_CMD_ACCEPT,
-					CDNS_MCP_CONTROL_CMD_ACCEPT);
+		     CDNS_MCP_CONTROL_CMD_ACCEPT);
 
 	/* Configure mcp config */
 	val = cdns_readl(cdns, CDNS_MCP_CONFIG);
@@ -873,7 +873,7 @@ int cdns_bus_conf(struct sdw_bus *bus, struct sdw_bus_params *params)
 EXPORT_SYMBOL(cdns_bus_conf);
 
 static int cdns_port_params(struct sdw_bus *bus,
-		struct sdw_port_params *p_params, unsigned int bank)
+			    struct sdw_port_params *p_params, unsigned int bank)
 {
 	struct sdw_cdns *cdns = bus_to_cdns(bus);
 	int dpn_config = 0, dpn_config_off;
@@ -898,8 +898,8 @@ static int cdns_port_params(struct sdw_bus *bus,
 }
 
 static int cdns_transport_params(struct sdw_bus *bus,
-			struct sdw_transport_params *t_params,
-			enum sdw_reg_bank bank)
+				 struct sdw_transport_params *t_params,
+				 enum sdw_reg_bank bank)
 {
 	struct sdw_cdns *cdns = bus_to_cdns(bus);
 	int dpn_offsetctrl = 0, dpn_offsetctrl_off;
@@ -952,7 +952,7 @@ static int cdns_transport_params(struct sdw_bus *bus,
 }
 
 static int cdns_port_enable(struct sdw_bus *bus,
-		struct sdw_enable_ch *enable_ch, unsigned int bank)
+			    struct sdw_enable_ch *enable_ch, unsigned int bank)
 {
 	struct sdw_cdns *cdns = bus_to_cdns(bus);
 	int dpn_chnen_off, ch_mask;
@@ -988,7 +988,7 @@ int sdw_cdns_probe(struct sdw_cdns *cdns)
 EXPORT_SYMBOL(sdw_cdns_probe);
 
 int cdns_set_sdw_stream(struct snd_soc_dai *dai,
-		void *stream, bool pcm, int direction)
+			void *stream, bool pcm, int direction)
 {
 	struct sdw_cdns *cdns = snd_soc_dai_get_drvdata(dai);
 	struct sdw_cdns_dma_data *dma;
@@ -1026,12 +1026,13 @@ EXPORT_SYMBOL(cdns_set_sdw_stream);
  * Find and return a free PDI for a given PDI array
  */
 static struct sdw_cdns_pdi *cdns_find_pdi(struct sdw_cdns *cdns,
-		unsigned int num, struct sdw_cdns_pdi *pdi)
+					  unsigned int num,
+					  struct sdw_cdns_pdi *pdi)
 {
 	int i;
 
 	for (i = 0; i < num; i++) {
-		if (pdi[i].assigned == true)
+		if (pdi[i].assigned)
 			continue;
 		pdi[i].assigned = true;
 		return &pdi[i];
@@ -1050,8 +1051,8 @@ static struct sdw_cdns_pdi *cdns_find_pdi(struct sdw_cdns *cdns,
  * @pdi: PDI to be used
  */
 void sdw_cdns_config_stream(struct sdw_cdns *cdns,
-				struct sdw_cdns_port *port,
-				u32 ch, u32 dir, struct sdw_cdns_pdi *pdi)
+			    struct sdw_cdns_port *port,
+			    u32 ch, u32 dir, struct sdw_cdns_pdi *pdi)
 {
 	u32 offset, val = 0;
 
@@ -1076,13 +1077,13 @@ EXPORT_SYMBOL(sdw_cdns_config_stream);
  * @ch_count: Channel count
  */
 static int cdns_get_num_pdi(struct sdw_cdns *cdns,
-		struct sdw_cdns_pdi *pdi,
-		unsigned int num, u32 ch_count)
+			    struct sdw_cdns_pdi *pdi,
+			    unsigned int num, u32 ch_count)
 {
 	int i, pdis = 0;
 
 	for (i = 0; i < num; i++) {
-		if (pdi[i].assigned == true)
+		if (pdi[i].assigned)
 			continue;
 
 		if (pdi[i].ch_count < ch_count)
@@ -1139,8 +1140,8 @@ EXPORT_SYMBOL(sdw_cdns_get_stream);
  * @dir: Data direction
  */
 int sdw_cdns_alloc_stream(struct sdw_cdns *cdns,
-			struct sdw_cdns_streams *stream,
-			struct sdw_cdns_port *port, u32 ch, u32 dir)
+			  struct sdw_cdns_streams *stream,
+			  struct sdw_cdns_port *port, u32 ch, u32 dir)
 {
 	struct sdw_cdns_pdi *pdi = NULL;
 
@@ -1167,7 +1168,7 @@ int sdw_cdns_alloc_stream(struct sdw_cdns *cdns,
 EXPORT_SYMBOL(sdw_cdns_alloc_stream);
 
 void sdw_cdns_shutdown(struct snd_pcm_substream *substream,
-					struct snd_soc_dai *dai)
+		       struct snd_soc_dai *dai)
 {
 	struct sdw_cdns_dma_data *dma;
 
diff --git a/drivers/soundwire/cadence_master.h b/drivers/soundwire/cadence_master.h
index eb902b19c5a4..fe2af62958b1 100644
--- a/drivers/soundwire/cadence_master.h
+++ b/drivers/soundwire/cadence_master.h
@@ -1,5 +1,5 @@
-// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
-// Copyright(c) 2015-17 Intel Corporation.
+/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */
+/* Copyright(c) 2015-17 Intel Corporation. */
 #include <sound/soc.h>
 
 #ifndef __SDW_CADENCE_H
@@ -160,24 +160,24 @@ irqreturn_t sdw_cdns_thread(int irq, void *dev_id);
 
 int sdw_cdns_init(struct sdw_cdns *cdns);
 int sdw_cdns_pdi_init(struct sdw_cdns *cdns,
-			struct sdw_cdns_stream_config config);
+		      struct sdw_cdns_stream_config config);
 int sdw_cdns_enable_interrupt(struct sdw_cdns *cdns);
 
 int sdw_cdns_get_stream(struct sdw_cdns *cdns,
 			struct sdw_cdns_streams *stream,
 			u32 ch, u32 dir);
 int sdw_cdns_alloc_stream(struct sdw_cdns *cdns,
-			struct sdw_cdns_streams *stream,
-			struct sdw_cdns_port *port, u32 ch, u32 dir);
+			  struct sdw_cdns_streams *stream,
+			  struct sdw_cdns_port *port, u32 ch, u32 dir);
 void sdw_cdns_config_stream(struct sdw_cdns *cdns, struct sdw_cdns_port *port,
-			u32 ch, u32 dir, struct sdw_cdns_pdi *pdi);
+			    u32 ch, u32 dir, struct sdw_cdns_pdi *pdi);
 
 void sdw_cdns_shutdown(struct snd_pcm_substream *substream,
-				struct snd_soc_dai *dai);
+		       struct snd_soc_dai *dai);
 int sdw_cdns_pcm_set_stream(struct snd_soc_dai *dai,
-				void *stream, int direction);
+			    void *stream, int direction);
 int sdw_cdns_pdm_set_stream(struct snd_soc_dai *dai,
-				void *stream, int direction);
+			    void *stream, int direction);
 
 enum sdw_command_response
 cdns_reset_page_addr(struct sdw_bus *bus, unsigned int dev_num);
@@ -187,7 +187,7 @@ cdns_xfer_msg(struct sdw_bus *bus, struct sdw_msg *msg);
 
 enum sdw_command_response
 cdns_xfer_msg_defer(struct sdw_bus *bus,
-		struct sdw_msg *msg, struct sdw_defer *defer);
+		    struct sdw_msg *msg, struct sdw_defer *defer);
 
 enum sdw_command_response
 cdns_reset_page_addr(struct sdw_bus *bus, unsigned int dev_num);
@@ -195,5 +195,5 @@ cdns_reset_page_addr(struct sdw_bus *bus, unsigned int dev_num);
 int cdns_bus_conf(struct sdw_bus *bus, struct sdw_bus_params *params);
 
 int cdns_set_sdw_stream(struct snd_soc_dai *dai,
-		void *stream, bool pcm, int direction);
+			void *stream, bool pcm, int direction);
 #endif /* __SDW_CADENCE_H */
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 8669b314c476..619a57d2af38 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -23,18 +23,18 @@
 #define SDW_SHIM_IPPTR			0x8
 #define SDW_SHIM_SYNC			0xC
 
-#define SDW_SHIM_CTLSCAP(x)		(0x010 + 0x60 * x)
-#define SDW_SHIM_CTLS0CM(x)		(0x012 + 0x60 * x)
-#define SDW_SHIM_CTLS1CM(x)		(0x014 + 0x60 * x)
-#define SDW_SHIM_CTLS2CM(x)		(0x016 + 0x60 * x)
-#define SDW_SHIM_CTLS3CM(x)		(0x018 + 0x60 * x)
-#define SDW_SHIM_PCMSCAP(x)		(0x020 + 0x60 * x)
-
-#define SDW_SHIM_PCMSYCHM(x, y)		(0x022 + (0x60 * x) + (0x2 * y))
-#define SDW_SHIM_PCMSYCHC(x, y)		(0x042 + (0x60 * x) + (0x2 * y))
-#define SDW_SHIM_PDMSCAP(x)		(0x062 + 0x60 * x)
-#define SDW_SHIM_IOCTL(x)		(0x06C + 0x60 * x)
-#define SDW_SHIM_CTMCTL(x)		(0x06E + 0x60 * x)
+#define SDW_SHIM_CTLSCAP(x)		(0x010 + 0x60 * (x))
+#define SDW_SHIM_CTLS0CM(x)		(0x012 + 0x60 * (x))
+#define SDW_SHIM_CTLS1CM(x)		(0x014 + 0x60 * (x))
+#define SDW_SHIM_CTLS2CM(x)		(0x016 + 0x60 * (x))
+#define SDW_SHIM_CTLS3CM(x)		(0x018 + 0x60 * (x))
+#define SDW_SHIM_PCMSCAP(x)		(0x020 + 0x60 * (x))
+
+#define SDW_SHIM_PCMSYCHM(x, y)		(0x022 + (0x60 * (x)) + (0x2 * (y)))
+#define SDW_SHIM_PCMSYCHC(x, y)		(0x042 + (0x60 * (x)) + (0x2 * (y)))
+#define SDW_SHIM_PDMSCAP(x)		(0x062 + 0x60 * (x))
+#define SDW_SHIM_IOCTL(x)		(0x06C + 0x60 * (x))
+#define SDW_SHIM_CTMCTL(x)		(0x06E + 0x60 * (x))
 
 #define SDW_SHIM_WAKEEN			0x190
 #define SDW_SHIM_WAKESTS		0x192
@@ -81,7 +81,7 @@
 #define SDW_SHIM_WAKESTS_STATUS		BIT(0)
 
 /* Intel ALH Register definitions */
-#define SDW_ALH_STRMZCFG(x)		(0x000 + (0x4 * x))
+#define SDW_ALH_STRMZCFG(x)		(0x000 + (0x4 * (x)))
 
 #define SDW_ALH_STRMZCFG_DMAT_VAL	0x3
 #define SDW_ALH_STRMZCFG_DMAT		GENMASK(7, 0)
@@ -235,7 +235,7 @@ static int intel_shim_init(struct sdw_intel *sdw)
 	/* Set SyncCPU bit */
 	sync_reg |= SDW_SHIM_SYNC_SYNCCPU;
 	ret = intel_clear_bit(shim, SDW_SHIM_SYNC, sync_reg,
-				SDW_SHIM_SYNC_SYNCCPU);
+			      SDW_SHIM_SYNC_SYNCCPU);
 	if (ret < 0)
 		dev_err(sdw->cdns.dev, "Failed to set sync period: %d", ret);
 
@@ -246,7 +246,7 @@ static int intel_shim_init(struct sdw_intel *sdw)
  * PDI routines
  */
 static void intel_pdi_init(struct sdw_intel *sdw,
-			struct sdw_cdns_stream_config *config)
+			   struct sdw_cdns_stream_config *config)
 {
 	void __iomem *shim = sdw->res->shim;
 	unsigned int link_id = sdw->instance;
@@ -295,9 +295,9 @@ intel_pdi_get_ch_cap(struct sdw_intel *sdw, unsigned int pdi_num, bool pcm)
 }
 
 static int intel_pdi_get_ch_update(struct sdw_intel *sdw,
-				struct sdw_cdns_pdi *pdi,
-				unsigned int num_pdi,
-				unsigned int *num_ch, bool pcm)
+				   struct sdw_cdns_pdi *pdi,
+				   unsigned int num_pdi,
+				   unsigned int *num_ch, bool pcm)
 {
 	int i, ch_count = 0;
 
@@ -312,16 +312,16 @@ static int intel_pdi_get_ch_update(struct sdw_intel *sdw,
 }
 
 static int intel_pdi_stream_ch_update(struct sdw_intel *sdw,
-				struct sdw_cdns_streams *stream, bool pcm)
+				      struct sdw_cdns_streams *stream, bool pcm)
 {
 	intel_pdi_get_ch_update(sdw, stream->bd, stream->num_bd,
-			&stream->num_ch_bd, pcm);
+				&stream->num_ch_bd, pcm);
 
 	intel_pdi_get_ch_update(sdw, stream->in, stream->num_in,
-			&stream->num_ch_in, pcm);
+				&stream->num_ch_in, pcm);
 
 	intel_pdi_get_ch_update(sdw, stream->out, stream->num_out,
-			&stream->num_ch_out, pcm);
+				&stream->num_ch_out, pcm);
 
 	return 0;
 }
@@ -386,9 +386,9 @@ intel_pdi_alh_configure(struct sdw_intel *sdw, struct sdw_cdns_pdi *pdi)
 }
 
 static int intel_config_stream(struct sdw_intel *sdw,
-			struct snd_pcm_substream *substream,
-			struct snd_soc_dai *dai,
-			struct snd_pcm_hw_params *hw_params, int link_id)
+			       struct snd_pcm_substream *substream,
+			       struct snd_soc_dai *dai,
+			       struct snd_pcm_hw_params *hw_params, int link_id)
 {
 	if (sdw->res->ops && sdw->res->ops->config_stream)
 		return sdw->res->ops->config_stream(sdw->res->arg,
@@ -453,7 +453,7 @@ static int intel_post_bank_switch(struct sdw_bus *bus)
 	sync_reg |= SDW_SHIM_SYNC_SYNCGO;
 
 	ret = intel_clear_bit(shim, SDW_SHIM_SYNC, sync_reg,
-					SDW_SHIM_SYNC_SYNCGO);
+			      SDW_SHIM_SYNC_SYNCGO);
 	if (ret < 0)
 		dev_err(sdw->cdns.dev, "Post bank switch failed: %d", ret);
 
@@ -465,14 +465,14 @@ static int intel_post_bank_switch(struct sdw_bus *bus)
  */
 
 static struct sdw_cdns_port *intel_alloc_port(struct sdw_intel *sdw,
-				u32 ch, u32 dir, bool pcm)
+					      u32 ch, u32 dir, bool pcm)
 {
 	struct sdw_cdns *cdns = &sdw->cdns;
 	struct sdw_cdns_port *port = NULL;
 	int i, ret = 0;
 
 	for (i = 0; i < cdns->num_ports; i++) {
-		if (cdns->ports[i].assigned == true)
+		if (cdns->ports[i].assigned)
 			continue;
 
 		port = &cdns->ports[i];
@@ -525,8 +525,8 @@ static void intel_port_cleanup(struct sdw_cdns_dma_data *dma)
 }
 
 static int intel_hw_params(struct snd_pcm_substream *substream,
-				struct snd_pcm_hw_params *params,
-				struct snd_soc_dai *dai)
+			   struct snd_pcm_hw_params *params,
+			   struct snd_soc_dai *dai)
 {
 	struct sdw_cdns *cdns = snd_soc_dai_get_drvdata(dai);
 	struct sdw_intel *sdw = cdns_to_intel(cdns);
@@ -574,7 +574,7 @@ static int intel_hw_params(struct snd_pcm_substream *substream,
 	/* Inform DSP about PDI stream number */
 	for (i = 0; i < dma->nr_ports; i++) {
 		ret = intel_config_stream(sdw, substream, dai, params,
-				dma->port[i]->pdi->intel_alh_id);
+					  dma->port[i]->pdi->intel_alh_id);
 		if (ret)
 			goto port_error;
 	}
@@ -604,7 +604,7 @@ static int intel_hw_params(struct snd_pcm_substream *substream,
 	}
 
 	ret = sdw_stream_add_master(&cdns->bus, &sconfig,
-				pconfig, dma->nr_ports, dma->stream);
+				    pconfig, dma->nr_ports, dma->stream);
 	if (ret) {
 		dev_err(cdns->dev, "add master to stream failed:%d", ret);
 		goto stream_error;
@@ -635,7 +635,7 @@ intel_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
 	ret = sdw_stream_remove_master(&cdns->bus, dma->stream);
 	if (ret < 0)
 		dev_err(dai->dev, "remove master from stream %s failed: %d",
-							dma->stream->name, ret);
+			dma->stream->name, ret);
 
 	intel_port_cleanup(dma);
 	kfree(dma->port);
@@ -643,13 +643,13 @@ intel_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
 }
 
 static int intel_pcm_set_sdw_stream(struct snd_soc_dai *dai,
-					void *stream, int direction)
+				    void *stream, int direction)
 {
 	return cdns_set_sdw_stream(dai, stream, true, direction);
 }
 
 static int intel_pdm_set_sdw_stream(struct snd_soc_dai *dai,
-					void *stream, int direction)
+				    void *stream, int direction)
 {
 	return cdns_set_sdw_stream(dai, stream, false, direction);
 }
@@ -673,9 +673,9 @@ static const struct snd_soc_component_driver dai_component = {
 };
 
 static int intel_create_dai(struct sdw_cdns *cdns,
-			struct snd_soc_dai_driver *dais,
-			enum intel_pdi_type type,
-			u32 num, u32 off, u32 max_ch, bool pcm)
+			    struct snd_soc_dai_driver *dais,
+			    enum intel_pdi_type type,
+			    u32 num, u32 off, u32 max_ch, bool pcm)
 {
 	int i;
 
@@ -685,7 +685,7 @@ static int intel_create_dai(struct sdw_cdns *cdns,
 	 /* TODO: Read supported rates/formats from hardware */
 	for (i = off; i < (off + num); i++) {
 		dais[i].name = kasprintf(GFP_KERNEL, "SDW%d Pin%d",
-					cdns->instance, i);
+					 cdns->instance, i);
 		if (!dais[i].name)
 			return -ENOMEM;
 
@@ -786,7 +786,7 @@ static int intel_register_dai(struct sdw_intel *sdw)
 		return ret;
 
 	return snd_soc_register_component(cdns->dev, &dai_component,
-				dais, num_dai);
+					  dais, num_dai);
 }
 
 static int intel_prop_read(struct sdw_bus *bus)
@@ -872,12 +872,11 @@ static int intel_probe(struct platform_device *pdev)
 	intel_pdi_ch_update(sdw);
 
 	/* Acquire IRQ */
-	ret = request_threaded_irq(sdw->res->irq, sdw_cdns_irq,
-			sdw_cdns_thread, IRQF_SHARED, KBUILD_MODNAME,
-			&sdw->cdns);
+	ret = request_threaded_irq(sdw->res->irq, sdw_cdns_irq, sdw_cdns_thread,
+				   IRQF_SHARED, KBUILD_MODNAME, &sdw->cdns);
 	if (ret < 0) {
 		dev_err(sdw->cdns.dev, "unable to grab IRQ %d, disabling device\n",
-				sdw->res->irq);
+			sdw->res->irq);
 		goto err_init;
 	}
 
diff --git a/drivers/soundwire/intel.h b/drivers/soundwire/intel.h
index c1a5bac6212e..71050e5f643d 100644
--- a/drivers/soundwire/intel.h
+++ b/drivers/soundwire/intel.h
@@ -1,5 +1,5 @@
-// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
-// Copyright(c) 2015-17 Intel Corporation.
+/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */
+/* Copyright(c) 2015-17 Intel Corporation. */
 
 #ifndef __SDW_INTEL_LOCAL_H
 #define __SDW_INTEL_LOCAL_H
diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c
index 5c8a20d99878..5123c8f79591 100644
--- a/drivers/soundwire/intel_init.c
+++ b/drivers/soundwire/intel_init.c
@@ -67,7 +67,7 @@ static struct sdw_intel_ctx
 	/* Found controller, find links supported */
 	count = 0;
 	ret = fwnode_property_read_u8_array(acpi_fwnode_handle(adev),
-				  "mipi-sdw-master-count", &count, 1);
+					    "mipi-sdw-master-count", &count, 1);
 
 	/* Don't fail on error, continue and use hw value */
 	if (ret) {
@@ -85,7 +85,7 @@ static struct sdw_intel_ctx
 	/* Check count is within bounds */
 	if (count > SDW_MAX_LINKS) {
 		dev_err(&adev->dev, "Link count %d exceeds max %d\n",
-						count, SDW_MAX_LINKS);
+			count, SDW_MAX_LINKS);
 		return NULL;
 	}
 
@@ -145,7 +145,7 @@ static struct sdw_intel_ctx
 }
 
 static acpi_status sdw_intel_acpi_cb(acpi_handle handle, u32 level,
-					void *cdata, void **return_value)
+				     void *cdata, void **return_value)
 {
 	struct sdw_intel_res *res = cdata;
 	struct acpi_device *adev;
@@ -172,9 +172,9 @@ void *sdw_intel_init(acpi_handle *parent_handle, struct sdw_intel_res *res)
 	acpi_status status;
 
 	status = acpi_walk_namespace(ACPI_TYPE_DEVICE,
-					parent_handle, 1,
-					sdw_intel_acpi_cb,
-					NULL, res, NULL);
+				     parent_handle, 1,
+				     sdw_intel_acpi_cb,
+				     NULL, res, NULL);
 	if (ACPI_FAILURE(status))
 		return NULL;
 
diff --git a/drivers/soundwire/mipi_disco.c b/drivers/soundwire/mipi_disco.c
index fdeba0c3b589..2bf4046e68b6 100644
--- a/drivers/soundwire/mipi_disco.c
+++ b/drivers/soundwire/mipi_disco.c
@@ -35,11 +35,12 @@ int sdw_master_read_prop(struct sdw_bus *bus)
 	int nval, i;
 
 	device_property_read_u32(bus->dev,
-			"mipi-sdw-sw-interface-revision", &prop->revision);
+				 "mipi-sdw-sw-interface-revision",
+				 &prop->revision);
 
 	/* Find master handle */
 	snprintf(name, sizeof(name),
-			"mipi-sdw-master-%d-subproperties", bus->link_id);
+		 "mipi-sdw-master-%d-subproperties", bus->link_id);
 
 	link = device_get_named_child_node(bus->dev, name);
 	if (!link) {
@@ -48,15 +49,16 @@ int sdw_master_read_prop(struct sdw_bus *bus)
 	}
 
 	if (fwnode_property_read_bool(link,
-			"mipi-sdw-clock-stop-mode0-supported") == true)
+				      "mipi-sdw-clock-stop-mode0-supported"))
 		prop->clk_stop_mode = SDW_CLK_STOP_MODE0;
 
 	if (fwnode_property_read_bool(link,
-			"mipi-sdw-clock-stop-mode1-supported") == true)
+				      "mipi-sdw-clock-stop-mode1-supported"))
 		prop->clk_stop_mode |= SDW_CLK_STOP_MODE1;
 
 	fwnode_property_read_u32(link,
-			"mipi-sdw-max-clock-frequency", &prop->max_freq);
+				 "mipi-sdw-max-clock-frequency",
+				 &prop->max_freq);
 
 	nval = fwnode_property_read_u32_array(link,
 			"mipi-sdw-clock-frequencies-supported", NULL, 0);
@@ -64,7 +66,7 @@ int sdw_master_read_prop(struct sdw_bus *bus)
 
 		prop->num_freq = nval;
 		prop->freq = devm_kcalloc(bus->dev, prop->num_freq,
-				sizeof(*prop->freq), GFP_KERNEL);
+					  sizeof(*prop->freq), GFP_KERNEL);
 		if (!prop->freq)
 			return -ENOMEM;
 
@@ -91,44 +93,47 @@ int sdw_master_read_prop(struct sdw_bus *bus)
 
 		prop->num_clk_gears = nval;
 		prop->clk_gears = devm_kcalloc(bus->dev, prop->num_clk_gears,
-				sizeof(*prop->clk_gears), GFP_KERNEL);
+					       sizeof(*prop->clk_gears),
+					       GFP_KERNEL);
 		if (!prop->clk_gears)
 			return -ENOMEM;
 
 		fwnode_property_read_u32_array(link,
-				"mipi-sdw-supported-clock-gears",
-				prop->clk_gears, prop->num_clk_gears);
+					       "mipi-sdw-supported-clock-gears",
+					       prop->clk_gears,
+					       prop->num_clk_gears);
 	}
 
 	fwnode_property_read_u32(link, "mipi-sdw-default-frame-rate",
-			&prop->default_frame_rate);
+				 &prop->default_frame_rate);
 
 	fwnode_property_read_u32(link, "mipi-sdw-default-frame-row-size",
-			&prop->default_row);
+				 &prop->default_row);
 
 	fwnode_property_read_u32(link, "mipi-sdw-default-frame-col-size",
-			&prop->default_col);
+				 &prop->default_col);
 
 	prop->dynamic_frame =  fwnode_property_read_bool(link,
 			"mipi-sdw-dynamic-frame-shape");
 
 	fwnode_property_read_u32(link, "mipi-sdw-command-error-threshold",
-			&prop->err_threshold);
+				 &prop->err_threshold);
 
 	return 0;
 }
 EXPORT_SYMBOL(sdw_master_read_prop);
 
 static int sdw_slave_read_dp0(struct sdw_slave *slave,
-		struct fwnode_handle *port, struct sdw_dp0_prop *dp0)
+			      struct fwnode_handle *port,
+			      struct sdw_dp0_prop *dp0)
 {
 	int nval;
 
 	fwnode_property_read_u32(port, "mipi-sdw-port-max-wordlength",
-			&dp0->max_word);
+				 &dp0->max_word);
 
 	fwnode_property_read_u32(port, "mipi-sdw-port-min-wordlength",
-			&dp0->min_word);
+				 &dp0->min_word);
 
 	nval = fwnode_property_read_u32_array(port,
 			"mipi-sdw-port-wordlength-configs", NULL, 0);
@@ -136,8 +141,8 @@ static int sdw_slave_read_dp0(struct sdw_slave *slave,
 
 		dp0->num_words = nval;
 		dp0->words = devm_kcalloc(&slave->dev,
-				dp0->num_words, sizeof(*dp0->words),
-				GFP_KERNEL);
+					  dp0->num_words, sizeof(*dp0->words),
+					  GFP_KERNEL);
 		if (!dp0->words)
 			return -ENOMEM;
 
@@ -146,20 +151,21 @@ static int sdw_slave_read_dp0(struct sdw_slave *slave,
 				dp0->words, dp0->num_words);
 	}
 
-	dp0->flow_controlled = fwnode_property_read_bool(
-			port, "mipi-sdw-bra-flow-controlled");
+	dp0->flow_controlled = fwnode_property_read_bool(port,
+				"mipi-sdw-bra-flow-controlled");
 
-	dp0->simple_ch_prep_sm = fwnode_property_read_bool(
-			port, "mipi-sdw-simplified-channel-prepare-sm");
+	dp0->simple_ch_prep_sm = fwnode_property_read_bool(port,
+				"mipi-sdw-simplified-channel-prepare-sm");
 
-	dp0->device_interrupts = fwnode_property_read_bool(
-			port, "mipi-sdw-imp-def-dp0-interrupts-supported");
+	dp0->device_interrupts = fwnode_property_read_bool(port,
+				"mipi-sdw-imp-def-dp0-interrupts-supported");
 
 	return 0;
 }
 
 static int sdw_slave_read_dpn(struct sdw_slave *slave,
-		struct sdw_dpn_prop *dpn, int count, int ports, char *type)
+			      struct sdw_dpn_prop *dpn, int count, int ports,
+			      char *type)
 {
 	struct fwnode_handle *node;
 	u32 bit, i = 0;
@@ -173,7 +179,7 @@ static int sdw_slave_read_dpn(struct sdw_slave *slave,
 
 	for_each_set_bit(bit, &addr, 32) {
 		snprintf(name, sizeof(name),
-			"mipi-sdw-dp-%d-%s-subproperties", bit, type);
+			 "mipi-sdw-dp-%d-%s-subproperties", bit, type);
 
 		dpn[i].num = bit;
 
@@ -184,9 +190,9 @@ static int sdw_slave_read_dpn(struct sdw_slave *slave,
 		}
 
 		fwnode_property_read_u32(node, "mipi-sdw-port-max-wordlength",
-					&dpn[i].max_word);
+					 &dpn[i].max_word);
 		fwnode_property_read_u32(node, "mipi-sdw-port-min-wordlength",
-					&dpn[i].min_word);
+					 &dpn[i].min_word);
 
 		nval = fwnode_property_read_u32_array(node,
 				"mipi-sdw-port-wordlength-configs", NULL, 0);
@@ -194,8 +200,9 @@ static int sdw_slave_read_dpn(struct sdw_slave *slave,
 
 			dpn[i].num_words = nval;
 			dpn[i].words = devm_kcalloc(&slave->dev,
-					dpn[i].num_words,
-					sizeof(*dpn[i].words), GFP_KERNEL);
+						    dpn[i].num_words,
+						    sizeof(*dpn[i].words),
+						    GFP_KERNEL);
 			if (!dpn[i].words)
 				return -ENOMEM;
 
@@ -205,28 +212,28 @@ static int sdw_slave_read_dpn(struct sdw_slave *slave,
 		}
 
 		fwnode_property_read_u32(node, "mipi-sdw-data-port-type",
-				&dpn[i].type);
+					 &dpn[i].type);
 
 		fwnode_property_read_u32(node,
-				"mipi-sdw-max-grouping-supported",
-				&dpn[i].max_grouping);
+					 "mipi-sdw-max-grouping-supported",
+					 &dpn[i].max_grouping);
 
 		dpn[i].simple_ch_prep_sm = fwnode_property_read_bool(node,
 				"mipi-sdw-simplified-channelprepare-sm");
 
 		fwnode_property_read_u32(node,
-				"mipi-sdw-port-channelprepare-timeout",
-				&dpn[i].ch_prep_timeout);
+					 "mipi-sdw-port-channelprepare-timeout",
+					 &dpn[i].ch_prep_timeout);
 
 		fwnode_property_read_u32(node,
 				"mipi-sdw-imp-def-dpn-interrupts-supported",
 				&dpn[i].device_interrupts);
 
 		fwnode_property_read_u32(node, "mipi-sdw-min-channel-number",
-				&dpn[i].min_ch);
+					 &dpn[i].min_ch);
 
 		fwnode_property_read_u32(node, "mipi-sdw-max-channel-number",
-				&dpn[i].max_ch);
+					 &dpn[i].max_ch);
 
 		nval = fwnode_property_read_u32_array(node,
 				"mipi-sdw-channel-number-list", NULL, 0);
@@ -234,7 +241,8 @@ static int sdw_slave_read_dpn(struct sdw_slave *slave,
 
 			dpn[i].num_ch = nval;
 			dpn[i].ch = devm_kcalloc(&slave->dev, dpn[i].num_ch,
-					sizeof(*dpn[i].ch), GFP_KERNEL);
+						 sizeof(*dpn[i].ch),
+						 GFP_KERNEL);
 			if (!dpn[i].ch)
 				return -ENOMEM;
 
@@ -265,13 +273,13 @@ static int sdw_slave_read_dpn(struct sdw_slave *slave,
 				"mipi-sdw-modes-supported", &dpn[i].modes);
 
 		fwnode_property_read_u32(node, "mipi-sdw-max-async-buffer",
-				&dpn[i].max_async_buffer);
+					 &dpn[i].max_async_buffer);
 
 		dpn[i].block_pack_mode = fwnode_property_read_bool(node,
 				"mipi-sdw-block-packing-mode");
 
 		fwnode_property_read_u32(node, "mipi-sdw-port-encoding-type",
-				&dpn[i].port_encoding);
+					 &dpn[i].port_encoding);
 
 		/* TODO: Read audio mode */
 
@@ -293,7 +301,7 @@ int sdw_slave_read_prop(struct sdw_slave *slave)
 	int num_of_ports, nval, i, dp0 = 0;
 
 	device_property_read_u32(dev, "mipi-sdw-sw-interface-revision",
-				&prop->mipi_revision);
+				 &prop->mipi_revision);
 
 	prop->wake_capable = device_property_read_bool(dev,
 				"mipi-sdw-wake-up-unavailable");
@@ -311,10 +319,10 @@ int sdw_slave_read_prop(struct sdw_slave *slave)
 			"mipi-sdw-simplified-clockstopprepare-sm-supported");
 
 	device_property_read_u32(dev, "mipi-sdw-clockstopprepare-timeout",
-			&prop->clk_stop_timeout);
+				 &prop->clk_stop_timeout);
 
 	device_property_read_u32(dev, "mipi-sdw-slave-channelprepare-timeout",
-			&prop->ch_prep_timeout);
+				 &prop->ch_prep_timeout);
 
 	device_property_read_u32(dev,
 			"mipi-sdw-clockstopprepare-hard-reset-behavior",
@@ -333,13 +341,13 @@ int sdw_slave_read_prop(struct sdw_slave *slave)
 			"mipi-sdw-port15-read-behavior", &prop->p15_behave);
 
 	device_property_read_u32(dev, "mipi-sdw-master-count",
-				&prop->master_count);
+				 &prop->master_count);
 
 	device_property_read_u32(dev, "mipi-sdw-source-port-list",
-				&prop->source_ports);
+				 &prop->source_ports);
 
 	device_property_read_u32(dev, "mipi-sdw-sink-port-list",
-				&prop->sink_ports);
+				 &prop->sink_ports);
 
 	/* Read dp0 properties */
 	port = device_get_named_child_node(dev, "mipi-sdw-dp-0-subproperties");
@@ -348,7 +356,8 @@ int sdw_slave_read_prop(struct sdw_slave *slave)
 	} else {
 
 		prop->dp0_prop = devm_kzalloc(&slave->dev,
-				sizeof(*prop->dp0_prop), GFP_KERNEL);
+					      sizeof(*prop->dp0_prop),
+					      GFP_KERNEL);
 		if (!prop->dp0_prop)
 			return -ENOMEM;
 
@@ -364,23 +373,25 @@ int sdw_slave_read_prop(struct sdw_slave *slave)
 	/* Allocate memory for set bits in port lists */
 	nval = hweight32(prop->source_ports);
 	prop->src_dpn_prop = devm_kcalloc(&slave->dev, nval,
-				sizeof(*prop->src_dpn_prop), GFP_KERNEL);
+					  sizeof(*prop->src_dpn_prop),
+					  GFP_KERNEL);
 	if (!prop->src_dpn_prop)
 		return -ENOMEM;
 
 	/* Read dpn properties for source port(s) */
 	sdw_slave_read_dpn(slave, prop->src_dpn_prop, nval,
-			prop->source_ports, "source");
+			   prop->source_ports, "source");
 
 	nval = hweight32(prop->sink_ports);
 	prop->sink_dpn_prop = devm_kcalloc(&slave->dev, nval,
-				sizeof(*prop->sink_dpn_prop), GFP_KERNEL);
+					   sizeof(*prop->sink_dpn_prop),
+					   GFP_KERNEL);
 	if (!prop->sink_dpn_prop)
 		return -ENOMEM;
 
 	/* Read dpn properties for sink port(s) */
 	sdw_slave_read_dpn(slave, prop->sink_dpn_prop, nval,
-			prop->sink_ports, "sink");
+			   prop->sink_ports, "sink");
 
 	/* some ports are bidirectional so check total ports by ORing */
 	nval = prop->source_ports | prop->sink_ports;
@@ -388,7 +399,8 @@ int sdw_slave_read_prop(struct sdw_slave *slave)
 
 	/* Allocate port_ready based on num_of_ports */
 	slave->port_ready = devm_kcalloc(&slave->dev, num_of_ports,
-				sizeof(*slave->port_ready), GFP_KERNEL);
+					 sizeof(*slave->port_ready),
+					 GFP_KERNEL);
 	if (!slave->port_ready)
 		return -ENOMEM;
 
diff --git a/drivers/soundwire/slave.c b/drivers/soundwire/slave.c
index ac103bd0c176..f39a5815e25d 100644
--- a/drivers/soundwire/slave.c
+++ b/drivers/soundwire/slave.c
@@ -14,7 +14,7 @@ static void sdw_slave_release(struct device *dev)
 }
 
 static int sdw_slave_add(struct sdw_bus *bus,
-		struct sdw_slave_id *id, struct fwnode_handle *fwnode)
+			 struct sdw_slave_id *id, struct fwnode_handle *fwnode)
 {
 	struct sdw_slave *slave;
 	int ret;
@@ -30,8 +30,8 @@ static int sdw_slave_add(struct sdw_bus *bus,
 
 	/* name shall be sdw:link:mfg:part:class:unique */
 	dev_set_name(&slave->dev, "sdw:%x:%x:%x:%x:%x",
-			bus->link_id, id->mfg_id, id->part_id,
-			id->class_id, id->unique_id);
+		     bus->link_id, id->mfg_id, id->part_id,
+		     id->class_id, id->unique_id);
 
 	slave->dev.release = sdw_slave_release;
 	slave->dev.bus = &sdw_bus_type;
@@ -84,11 +84,11 @@ int sdw_acpi_find_slaves(struct sdw_bus *bus)
 		acpi_status status;
 
 		status = acpi_evaluate_integer(adev->handle,
-					METHOD_NAME__ADR, NULL, &addr);
+					       METHOD_NAME__ADR, NULL, &addr);
 
 		if (ACPI_FAILURE(status)) {
 			dev_err(bus->dev, "_ADR resolution failed: %x\n",
-							status);
+				status);
 			return status;
 		}
 
diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index bd879b1a76c8..e3d2bc5cba80 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -53,9 +53,9 @@ static int sdw_find_row_index(int row)
 	return 0;
 }
 static int _sdw_program_slave_port_params(struct sdw_bus *bus,
-				struct sdw_slave *slave,
-				struct sdw_transport_params *t_params,
-				enum sdw_dpn_type type)
+					  struct sdw_slave *slave,
+					  struct sdw_transport_params *t_params,
+					  enum sdw_dpn_type type)
 {
 	u32 addr1, addr2, addr3, addr4;
 	int ret;
@@ -119,8 +119,8 @@ static int _sdw_program_slave_port_params(struct sdw_bus *bus,
 }
 
 static int sdw_program_slave_port_params(struct sdw_bus *bus,
-			struct sdw_slave_runtime *s_rt,
-			struct sdw_port_runtime *p_rt)
+					 struct sdw_slave_runtime *s_rt,
+					 struct sdw_port_runtime *p_rt)
 {
 	struct sdw_transport_params *t_params = &p_rt->transport_params;
 	struct sdw_port_params *p_params = &p_rt->port_params;
@@ -131,8 +131,8 @@ static int sdw_program_slave_port_params(struct sdw_bus *bus,
 	u8 wbuf;
 
 	dpn_prop = sdw_get_slave_dpn_prop(s_rt->slave,
-					s_rt->direction,
-					t_params->port_num);
+					  s_rt->direction,
+					  t_params->port_num);
 	if (!dpn_prop)
 		return -EINVAL;
 
@@ -216,7 +216,7 @@ static int sdw_program_slave_port_params(struct sdw_bus *bus,
 
 	if (dpn_prop->type != SDW_DPN_SIMPLE) {
 		ret = _sdw_program_slave_port_params(bus, s_rt->slave,
-						t_params, dpn_prop->type);
+						     t_params, dpn_prop->type);
 		if (ret < 0)
 			dev_err(&s_rt->slave->dev,
 				"Transport reg write failed for port: %d",
@@ -227,7 +227,7 @@ static int sdw_program_slave_port_params(struct sdw_bus *bus,
 }
 
 static int sdw_program_master_port_params(struct sdw_bus *bus,
-		struct sdw_port_runtime *p_rt)
+					  struct sdw_port_runtime *p_rt)
 {
 	int ret;
 
@@ -244,8 +244,8 @@ static int sdw_program_master_port_params(struct sdw_bus *bus,
 		return ret;
 
 	return bus->port_ops->dpn_set_port_params(bus,
-				&p_rt->port_params,
-				bus->params.next_bank);
+						  &p_rt->port_params,
+						  bus->params.next_bank);
 }
 
 /**
@@ -292,8 +292,9 @@ static int sdw_program_port_params(struct sdw_master_runtime *m_rt)
  * actual enable/disable is done with a bank switch
  */
 static int sdw_enable_disable_slave_ports(struct sdw_bus *bus,
-				struct sdw_slave_runtime *s_rt,
-				struct sdw_port_runtime *p_rt, bool en)
+					  struct sdw_slave_runtime *s_rt,
+					  struct sdw_port_runtime *p_rt,
+					  bool en)
 {
 	struct sdw_transport_params *t_params = &p_rt->transport_params;
 	u32 addr;
@@ -322,7 +323,8 @@ static int sdw_enable_disable_slave_ports(struct sdw_bus *bus,
 }
 
 static int sdw_enable_disable_master_ports(struct sdw_master_runtime *m_rt,
-			struct sdw_port_runtime *p_rt, bool en)
+					   struct sdw_port_runtime *p_rt,
+					   bool en)
 {
 	struct sdw_transport_params *t_params = &p_rt->transport_params;
 	struct sdw_bus *bus = m_rt->bus;
@@ -336,7 +338,8 @@ static int sdw_enable_disable_master_ports(struct sdw_master_runtime *m_rt,
 	/* Perform Master port channel(s) enable/disable */
 	if (bus->port_ops->dpn_port_enable_ch) {
 		ret = bus->port_ops->dpn_port_enable_ch(bus,
-				&enable_ch, bus->params.next_bank);
+							&enable_ch,
+							bus->params.next_bank);
 		if (ret < 0) {
 			dev_err(bus->dev,
 				"Master chn_en write failed:%d port:%d",
@@ -370,7 +373,7 @@ static int sdw_enable_disable_ports(struct sdw_master_runtime *m_rt, bool en)
 	list_for_each_entry(s_rt, &m_rt->slave_rt_list, m_rt_node) {
 		list_for_each_entry(s_port, &s_rt->port_list, port_node) {
 			ret = sdw_enable_disable_slave_ports(m_rt->bus, s_rt,
-							s_port, en);
+							     s_port, en);
 			if (ret < 0)
 				return ret;
 		}
@@ -387,7 +390,8 @@ static int sdw_enable_disable_ports(struct sdw_master_runtime *m_rt, bool en)
 }
 
 static int sdw_do_port_prep(struct sdw_slave_runtime *s_rt,
-		struct sdw_prepare_ch prep_ch, enum sdw_port_prep_ops cmd)
+			    struct sdw_prepare_ch prep_ch,
+			    enum sdw_port_prep_ops cmd)
 {
 	const struct sdw_slave_ops *ops = s_rt->slave->ops;
 	int ret;
@@ -405,8 +409,9 @@ static int sdw_do_port_prep(struct sdw_slave_runtime *s_rt,
 }
 
 static int sdw_prep_deprep_slave_ports(struct sdw_bus *bus,
-			struct sdw_slave_runtime *s_rt,
-			struct sdw_port_runtime *p_rt, bool prep)
+				       struct sdw_slave_runtime *s_rt,
+				       struct sdw_port_runtime *p_rt,
+				       bool prep)
 {
 	struct completion *port_ready = NULL;
 	struct sdw_dpn_prop *dpn_prop;
@@ -420,8 +425,8 @@ static int sdw_prep_deprep_slave_ports(struct sdw_bus *bus,
 	prep_ch.ch_mask = p_rt->ch_mask;
 
 	dpn_prop = sdw_get_slave_dpn_prop(s_rt->slave,
-					s_rt->direction,
-					prep_ch.num);
+					  s_rt->direction,
+					  prep_ch.num);
 	if (!dpn_prop) {
 		dev_err(bus->dev,
 			"Slave Port:%d properties not found", prep_ch.num);
@@ -442,7 +447,7 @@ static int sdw_prep_deprep_slave_ports(struct sdw_bus *bus,
 	 */
 	if (prep && intr) {
 		ret = sdw_configure_dpn_intr(s_rt->slave, p_rt->num, prep,
-						dpn_prop->device_interrupts);
+					     dpn_prop->device_interrupts);
 		if (ret < 0)
 			return ret;
 	}
@@ -456,7 +461,7 @@ static int sdw_prep_deprep_slave_ports(struct sdw_bus *bus,
 
 		if (prep)
 			ret = sdw_update(s_rt->slave, addr,
-					0xFF, p_rt->ch_mask);
+					 0xFF, p_rt->ch_mask);
 		else
 			ret = sdw_update(s_rt->slave, addr, 0xFF, 0x0);
 
@@ -486,13 +491,14 @@ static int sdw_prep_deprep_slave_ports(struct sdw_bus *bus,
 	/* Disable interrupt after Port de-prepare */
 	if (!prep && intr)
 		ret = sdw_configure_dpn_intr(s_rt->slave, p_rt->num, prep,
-						dpn_prop->device_interrupts);
+					     dpn_prop->device_interrupts);
 
 	return ret;
 }
 
 static int sdw_prep_deprep_master_ports(struct sdw_master_runtime *m_rt,
-				struct sdw_port_runtime *p_rt, bool prep)
+					struct sdw_port_runtime *p_rt,
+					bool prep)
 {
 	struct sdw_transport_params *t_params = &p_rt->transport_params;
 	struct sdw_bus *bus = m_rt->bus;
@@ -510,7 +516,7 @@ static int sdw_prep_deprep_master_ports(struct sdw_master_runtime *m_rt,
 		ret = ops->dpn_port_prep(bus, &prep_ch);
 		if (ret < 0) {
 			dev_err(bus->dev, "Port prepare failed for port:%d",
-					t_params->port_num);
+				t_params->port_num);
 			return ret;
 		}
 	}
@@ -535,7 +541,7 @@ static int sdw_prep_deprep_ports(struct sdw_master_runtime *m_rt, bool prep)
 	list_for_each_entry(s_rt, &m_rt->slave_rt_list, m_rt_node) {
 		list_for_each_entry(p_rt, &s_rt->port_list, port_node) {
 			ret = sdw_prep_deprep_slave_ports(m_rt->bus, s_rt,
-							p_rt, prep);
+							  p_rt, prep);
 			if (ret < 0)
 				return ret;
 		}
@@ -579,7 +585,7 @@ static int sdw_notify_config(struct sdw_master_runtime *m_rt)
 			ret = slave->ops->bus_config(slave, &bus->params);
 			if (ret < 0)
 				dev_err(bus->dev, "Notify Slave: %d failed",
-								slave->dev_num);
+					slave->dev_num);
 			return ret;
 		}
 	}
@@ -658,7 +664,7 @@ static int sdw_bank_switch(struct sdw_bus *bus, int m_rt_count)
 		addr = SDW_SCP_FRAMECTRL_B0;
 
 	sdw_fill_msg(wr_msg, NULL, addr, 1, SDW_BROADCAST_DEV_NUM,
-					SDW_MSG_FLAG_WRITE, wbuf);
+		     SDW_MSG_FLAG_WRITE, wbuf);
 	wr_msg->ssp_sync = true;
 
 	/*
@@ -873,7 +879,7 @@ EXPORT_SYMBOL(sdw_alloc_stream);
 
 static struct sdw_master_runtime
 *sdw_find_master_rt(struct sdw_bus *bus,
-			struct sdw_stream_runtime *stream)
+		    struct sdw_stream_runtime *stream)
 {
 	struct sdw_master_runtime *m_rt = NULL;
 
@@ -897,8 +903,8 @@ static struct sdw_master_runtime
  */
 static struct sdw_master_runtime
 *sdw_alloc_master_rt(struct sdw_bus *bus,
-			struct sdw_stream_config *stream_config,
-			struct sdw_stream_runtime *stream)
+		     struct sdw_stream_config *stream_config,
+		     struct sdw_stream_runtime *stream)
 {
 	struct sdw_master_runtime *m_rt;
 
@@ -941,8 +947,8 @@ static struct sdw_master_runtime
  */
 static struct sdw_slave_runtime
 *sdw_alloc_slave_rt(struct sdw_slave *slave,
-			struct sdw_stream_config *stream_config,
-			struct sdw_stream_runtime *stream)
+		    struct sdw_stream_config *stream_config,
+		    struct sdw_stream_runtime *stream)
 {
 	struct sdw_slave_runtime *s_rt = NULL;
 
@@ -959,20 +965,19 @@ static struct sdw_slave_runtime
 }
 
 static void sdw_master_port_release(struct sdw_bus *bus,
-			struct sdw_master_runtime *m_rt)
+				    struct sdw_master_runtime *m_rt)
 {
 	struct sdw_port_runtime *p_rt, *_p_rt;
 
-	list_for_each_entry_safe(p_rt, _p_rt,
-			&m_rt->port_list, port_node) {
+	list_for_each_entry_safe(p_rt, _p_rt, &m_rt->port_list, port_node) {
 		list_del(&p_rt->port_node);
 		kfree(p_rt);
 	}
 }
 
 static void sdw_slave_port_release(struct sdw_bus *bus,
-			struct sdw_slave *slave,
-			struct sdw_stream_runtime *stream)
+				   struct sdw_slave *slave,
+				   struct sdw_stream_runtime *stream)
 {
 	struct sdw_port_runtime *p_rt, *_p_rt;
 	struct sdw_master_runtime *m_rt;
@@ -985,7 +990,7 @@ static void sdw_slave_port_release(struct sdw_bus *bus,
 				continue;
 
 			list_for_each_entry_safe(p_rt, _p_rt,
-					&s_rt->port_list, port_node) {
+						 &s_rt->port_list, port_node) {
 
 				list_del(&p_rt->port_node);
 				kfree(p_rt);
@@ -1003,7 +1008,7 @@ static void sdw_slave_port_release(struct sdw_bus *bus,
  * This function is to be called with bus_lock held.
  */
 static void sdw_release_slave_stream(struct sdw_slave *slave,
-			struct sdw_stream_runtime *stream)
+				     struct sdw_stream_runtime *stream)
 {
 	struct sdw_slave_runtime *s_rt, *_s_rt;
 	struct sdw_master_runtime *m_rt;
@@ -1011,7 +1016,7 @@ static void sdw_release_slave_stream(struct sdw_slave *slave,
 	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
 		/* Retrieve Slave runtime handle */
 		list_for_each_entry_safe(s_rt, _s_rt,
-					&m_rt->slave_rt_list, m_rt_node) {
+					 &m_rt->slave_rt_list, m_rt_node) {
 
 			if (s_rt->slave == slave) {
 				list_del(&s_rt->m_rt_node);
@@ -1034,7 +1039,7 @@ static void sdw_release_slave_stream(struct sdw_slave *slave,
  * no effect as Slave(s) runtime handle would already be freed up.
  */
 static void sdw_release_master_stream(struct sdw_master_runtime *m_rt,
-			struct sdw_stream_runtime *stream)
+				      struct sdw_stream_runtime *stream)
 {
 	struct sdw_slave_runtime *s_rt, *_s_rt;
 
@@ -1057,14 +1062,14 @@ static void sdw_release_master_stream(struct sdw_master_runtime *m_rt,
  * This removes and frees port_rt and master_rt from a stream
  */
 int sdw_stream_remove_master(struct sdw_bus *bus,
-		struct sdw_stream_runtime *stream)
+			     struct sdw_stream_runtime *stream)
 {
 	struct sdw_master_runtime *m_rt, *_m_rt;
 
 	mutex_lock(&bus->bus_lock);
 
 	list_for_each_entry_safe(m_rt, _m_rt,
-			&stream->master_list, stream_node) {
+				 &stream->master_list, stream_node) {
 
 		if (m_rt->bus != bus)
 			continue;
@@ -1092,7 +1097,7 @@ EXPORT_SYMBOL(sdw_stream_remove_master);
  * This removes and frees port_rt and slave_rt from a stream
  */
 int sdw_stream_remove_slave(struct sdw_slave *slave,
-		struct sdw_stream_runtime *stream)
+			    struct sdw_stream_runtime *stream)
 {
 	mutex_lock(&slave->bus->bus_lock);
 
@@ -1116,8 +1121,9 @@ EXPORT_SYMBOL(sdw_stream_remove_slave);
  * This function is to be called with bus_lock held.
  */
 static int sdw_config_stream(struct device *dev,
-		struct sdw_stream_runtime *stream,
-		struct sdw_stream_config *stream_config, bool is_slave)
+			     struct sdw_stream_runtime *stream,
+			     struct sdw_stream_config *stream_config,
+			     bool is_slave)
 {
 	/*
 	 * Update the stream rate, channel and bps based on data
@@ -1128,13 +1134,13 @@ static int sdw_config_stream(struct device *dev,
 	 * comparison and allow the value to be set and stored in stream
 	 */
 	if (stream->params.rate &&
-			stream->params.rate != stream_config->frame_rate) {
+	    stream->params.rate != stream_config->frame_rate) {
 		dev_err(dev, "rate not matching, stream:%s", stream->name);
 		return -EINVAL;
 	}
 
 	if (stream->params.bps &&
-			stream->params.bps != stream_config->bps) {
+	    stream->params.bps != stream_config->bps) {
 		dev_err(dev, "bps not matching, stream:%s", stream->name);
 		return -EINVAL;
 	}
@@ -1151,7 +1157,7 @@ static int sdw_config_stream(struct device *dev,
 }
 
 static int sdw_is_valid_port_range(struct device *dev,
-				struct sdw_port_runtime *p_rt)
+				   struct sdw_port_runtime *p_rt)
 {
 	if (!SDW_VALID_PORT_RANGE(p_rt->num)) {
 		dev_err(dev,
@@ -1162,9 +1168,10 @@ static int sdw_is_valid_port_range(struct device *dev,
 	return 0;
 }
 
-static struct sdw_port_runtime *sdw_port_alloc(struct device *dev,
-				struct sdw_port_config *port_config,
-				int port_index)
+static struct sdw_port_runtime
+*sdw_port_alloc(struct device *dev,
+		struct sdw_port_config *port_config,
+		int port_index)
 {
 	struct sdw_port_runtime *p_rt;
 
@@ -1179,9 +1186,9 @@ static struct sdw_port_runtime *sdw_port_alloc(struct device *dev,
 }
 
 static int sdw_master_port_config(struct sdw_bus *bus,
-			struct sdw_master_runtime *m_rt,
-			struct sdw_port_config *port_config,
-			unsigned int num_ports)
+				  struct sdw_master_runtime *m_rt,
+				  struct sdw_port_config *port_config,
+				  unsigned int num_ports)
 {
 	struct sdw_port_runtime *p_rt;
 	int i;
@@ -1204,9 +1211,9 @@ static int sdw_master_port_config(struct sdw_bus *bus,
 }
 
 static int sdw_slave_port_config(struct sdw_slave *slave,
-			struct sdw_slave_runtime *s_rt,
-			struct sdw_port_config *port_config,
-			unsigned int num_config)
+				 struct sdw_slave_runtime *s_rt,
+				 struct sdw_port_config *port_config,
+				 unsigned int num_config)
 {
 	struct sdw_port_runtime *p_rt;
 	int i, ret;
@@ -1248,10 +1255,10 @@ static int sdw_slave_port_config(struct sdw_slave *slave,
  * @stream: SoundWire stream
  */
 int sdw_stream_add_master(struct sdw_bus *bus,
-		struct sdw_stream_config *stream_config,
-		struct sdw_port_config *port_config,
-		unsigned int num_ports,
-		struct sdw_stream_runtime *stream)
+			  struct sdw_stream_config *stream_config,
+			  struct sdw_port_config *port_config,
+			  unsigned int num_ports,
+			  struct sdw_stream_runtime *stream)
 {
 	struct sdw_master_runtime *m_rt = NULL;
 	int ret;
@@ -1273,8 +1280,8 @@ int sdw_stream_add_master(struct sdw_bus *bus,
 	m_rt = sdw_alloc_master_rt(bus, stream_config, stream);
 	if (!m_rt) {
 		dev_err(bus->dev,
-				"Master runtime config failed for stream:%s",
-				stream->name);
+			"Master runtime config failed for stream:%s",
+			stream->name);
 		ret = -ENOMEM;
 		goto unlock;
 	}
@@ -1313,10 +1320,10 @@ EXPORT_SYMBOL(sdw_stream_add_master);
  *
  */
 int sdw_stream_add_slave(struct sdw_slave *slave,
-		struct sdw_stream_config *stream_config,
-		struct sdw_port_config *port_config,
-		unsigned int num_ports,
-		struct sdw_stream_runtime *stream)
+			 struct sdw_stream_config *stream_config,
+			 struct sdw_port_config *port_config,
+			 unsigned int num_ports,
+			 struct sdw_stream_runtime *stream)
 {
 	struct sdw_slave_runtime *s_rt;
 	struct sdw_master_runtime *m_rt;
@@ -1331,8 +1338,8 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 	m_rt = sdw_alloc_master_rt(slave->bus, stream_config, stream);
 	if (!m_rt) {
 		dev_err(&slave->dev,
-				"alloc master runtime failed for stream:%s",
-				stream->name);
+			"alloc master runtime failed for stream:%s",
+			stream->name);
 		ret = -ENOMEM;
 		goto error;
 	}
@@ -1340,8 +1347,8 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 	s_rt = sdw_alloc_slave_rt(slave, stream_config, stream);
 	if (!s_rt) {
 		dev_err(&slave->dev,
-				"Slave runtime config failed for stream:%s",
-				stream->name);
+			"Slave runtime config failed for stream:%s",
+			stream->name);
 		ret = -ENOMEM;
 		goto stream_error;
 	}
@@ -1385,8 +1392,8 @@ EXPORT_SYMBOL(sdw_stream_add_slave);
  * @port_num: Port number
  */
 struct sdw_dpn_prop *sdw_get_slave_dpn_prop(struct sdw_slave *slave,
-				enum sdw_data_direction direction,
-				unsigned int port_num)
+					    enum sdw_data_direction direction,
+					    unsigned int port_num)
 {
 	struct sdw_dpn_prop *dpn_prop;
 	u8 num_ports;
@@ -1501,7 +1508,7 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream)
 		ret = sdw_prep_deprep_ports(m_rt, true);
 		if (ret < 0) {
 			dev_err(bus->dev, "Prepare port(s) failed ret = %d",
-					ret);
+				ret);
 			return ret;
 		}
 	}
-- 
2.17.1

Re: [PATCH v3 2/5] soundwire: fix style issues

[Vinod Koul]

On 10-04-19, 22:16, Pierre-Louis Bossart wrote:
> Visual inspections confirmed by checkpatch.pl --strict expose a number
> of style issues, specifically parameter alignment is inconsistent as
> if different contributors used different styles. Before we restart
> support for SoundWire with Sound Open Firmware on Intel platforms,
> let's clean all this.
> 
> Fix Kconfig help, spelling, SPDX format, alignment, spurious
> parentheses, bool comparisons to true/false, macro argument
> protection.

Thanks for the cleanup Pierre :)

> 
> No new functionality added.
> 
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
>  drivers/soundwire/Kconfig          |   2 +-
>  drivers/soundwire/bus.c            |  87 ++++++++--------
>  drivers/soundwire/bus.h            |  16 +--
>  drivers/soundwire/bus_type.c       |   4 +-
>  drivers/soundwire/cadence_master.c |  87 ++++++++--------
>  drivers/soundwire/cadence_master.h |  22 ++--
>  drivers/soundwire/intel.c          |  87 ++++++++--------
>  drivers/soundwire/intel.h          |   4 +-
>  drivers/soundwire/intel_init.c     |  12 +--
>  drivers/soundwire/mipi_disco.c     | 116 +++++++++++----------
>  drivers/soundwire/slave.c          |  10 +-
>  drivers/soundwire/stream.c         | 161 +++++++++++++++--------------

I would prefer this to be a patch per module. It doesnt help to have a
single patch for all the files!

It would be great to have cleanup done per logical group, for example
typos in a patch, aligns in another etc...

>  12 files changed, 313 insertions(+), 295 deletions(-)
> 
> diff --git a/drivers/soundwire/Kconfig b/drivers/soundwire/Kconfig
> index 19c8efb9a5ee..84876a74874f 100644
> --- a/drivers/soundwire/Kconfig
> +++ b/drivers/soundwire/Kconfig
> @@ -4,7 +4,7 @@
>  
>  menuconfig SOUNDWIRE
>  	bool "SoundWire support"
> -	---help---
> +	help

Not sure if this is a style issue, kernel seems to have 2990 instances
of this!

>  	if (msg->page)
>  		sdw_reset_page(bus, msg->dev_num);
> @@ -243,7 +244,7 @@ int sdw_transfer(struct sdw_bus *bus, struct sdw_msg *msg)
>   * Caller needs to hold the msg_lock lock while calling this
>   */
>  int sdw_transfer_defer(struct sdw_bus *bus, struct sdw_msg *msg,
> -				struct sdw_defer *defer)
> +		       struct sdw_defer *defer)

this does not seem aligned to me!

>  int sdw_fill_msg(struct sdw_msg *msg, struct sdw_slave *slave,
> -		u32 addr, size_t count, u16 dev_num, u8 flags, u8 *buf)
> +		 u32 addr, size_t count, u16 dev_num, u8 flags, u8 *buf)

this one too

> @@ -458,13 +458,13 @@ static int sdw_assign_device_num(struct sdw_slave *slave)
>  		mutex_unlock(&slave->bus->bus_lock);
>  		if (dev_num < 0) {
>  			dev_err(slave->bus->dev, "Get dev_num failed: %d",
> -								dev_num);
> +				dev_num);

It might read better if we move the log to second line along with
dev_num...

>  int sdw_configure_dpn_intr(struct sdw_slave *slave,
> -			int port, bool enable, int mask)
> +			   int port, bool enable, int mask)

not aligned

>  void sdw_extract_slave_id(struct sdw_bus *bus,
> -			u64 addr, struct sdw_slave_id *id);
> +			  u64 addr, struct sdw_slave_id *id);
>  

Not aligned
 
>  enum sdw_command_response
>  cdns_xfer_msg_defer(struct sdw_bus *bus,
> -		struct sdw_msg *msg, struct sdw_defer *defer)
> +		    struct sdw_msg *msg, struct sdw_defer *defer)

this one too..

>  static int cdns_port_params(struct sdw_bus *bus,
> -		struct sdw_port_params *p_params, unsigned int bank)
> +			    struct sdw_port_params *p_params, unsigned int bank)

here as well.. (and giving up on rest)

-- 
~Vinod

Re: [alsa-devel] [PATCH v3 2/5] soundwire: fix style issues

[Pierre-Louis Bossart]

>>
>> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>> ---
>>   drivers/soundwire/Kconfig          |   2 +-
>>   drivers/soundwire/bus.c            |  87 ++++++++--------
>>   drivers/soundwire/bus.h            |  16 +--
>>   drivers/soundwire/bus_type.c       |   4 +-
>>   drivers/soundwire/cadence_master.c |  87 ++++++++--------
>>   drivers/soundwire/cadence_master.h |  22 ++--
>>   drivers/soundwire/intel.c          |  87 ++++++++--------
>>   drivers/soundwire/intel.h          |   4 +-
>>   drivers/soundwire/intel_init.c     |  12 +--
>>   drivers/soundwire/mipi_disco.c     | 116 +++++++++++----------
>>   drivers/soundwire/slave.c          |  10 +-
>>   drivers/soundwire/stream.c         | 161 +++++++++++++++--------------
> 
> I would prefer this to be a patch per module. It doesnt help to have a
> single patch for all the files!
> 
> It would be great to have cleanup done per logical group, for example
> typos in a patch, aligns in another etc...

You've got to be kidding. I've never seen people ask for this sort of 
detail.

> 
>>   12 files changed, 313 insertions(+), 295 deletions(-)
>>
>> diff --git a/drivers/soundwire/Kconfig b/drivers/soundwire/Kconfig
>> index 19c8efb9a5ee..84876a74874f 100644
>> --- a/drivers/soundwire/Kconfig
>> +++ b/drivers/soundwire/Kconfig
>> @@ -4,7 +4,7 @@
>>   
>>   menuconfig SOUNDWIRE
>>   	bool "SoundWire support"
>> -	---help---
>> +	help
> 
> Not sure if this is a style issue, kernel seems to have 2990 instances
> of this!

this is reported by checkpatch.pl --strict.

> 
>>   	if (msg->page)
>>   		sdw_reset_page(bus, msg->dev_num);
>> @@ -243,7 +244,7 @@ int sdw_transfer(struct sdw_bus *bus, struct sdw_msg *msg)
>>    * Caller needs to hold the msg_lock lock while calling this
>>    */
>>   int sdw_transfer_defer(struct sdw_bus *bus, struct sdw_msg *msg,
>> -				struct sdw_defer *defer)
>> +		       struct sdw_defer *defer)
> 
> this does not seem aligned to me!

It is, I checked. 2 tabs and 7 spaces.

> 
>>   int sdw_fill_msg(struct sdw_msg *msg, struct sdw_slave *slave,
>> -		u32 addr, size_t count, u16 dev_num, u8 flags, u8 *buf)
>> +		 u32 addr, size_t count, u16 dev_num, u8 flags, u8 *buf)
> 
> this one too

2 tabs and one space.

> 
>> @@ -458,13 +458,13 @@ static int sdw_assign_device_num(struct sdw_slave *slave)
>>   		mutex_unlock(&slave->bus->bus_lock);
>>   		if (dev_num < 0) {
>>   			dev_err(slave->bus->dev, "Get dev_num failed: %d",
>> -								dev_num);
>> +				dev_num);
> 
> It might read better if we move the log to second line along with
> dev_num...
> 
>>   int sdw_configure_dpn_intr(struct sdw_slave *slave,
>> -			int port, bool enable, int mask)
>> +			   int port, bool enable, int mask)
> 
> not aligned

it is in the code. It's a diff illusion.

> 
>>   void sdw_extract_slave_id(struct sdw_bus *bus,
>> -			u64 addr, struct sdw_slave_id *id);
>> +			  u64 addr, struct sdw_slave_id *id);
>>   
> 
> Not aligned

it is in the code. It's a diff illusion.

>   
>>   enum sdw_command_response
>>   cdns_xfer_msg_defer(struct sdw_bus *bus,
>> -		struct sdw_msg *msg, struct sdw_defer *defer)
>> +		    struct sdw_msg *msg, struct sdw_defer *defer)
> 
> this one too..
> 
>>   static int cdns_port_params(struct sdw_bus *bus,
>> -		struct sdw_port_params *p_params, unsigned int bank)
>> +			    struct sdw_port_params *p_params, unsigned int bank)
> 
> here as well.. (and giving up on rest)

Please check for yourself that this is a diff illusion w/ tab space.

Re: [alsa-devel] [PATCH v3 2/5] soundwire: fix style issues

[Johan Hovold]

On Mon, Apr 15, 2019 at 08:09:59AM -0500, Pierre-Louis Bossart wrote:
> 
> >>
> >> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> >> ---
> >>   drivers/soundwire/Kconfig          |   2 +-
> >>   drivers/soundwire/bus.c            |  87 ++++++++--------
> >>   drivers/soundwire/bus.h            |  16 +--
> >>   drivers/soundwire/bus_type.c       |   4 +-
> >>   drivers/soundwire/cadence_master.c |  87 ++++++++--------
> >>   drivers/soundwire/cadence_master.h |  22 ++--
> >>   drivers/soundwire/intel.c          |  87 ++++++++--------
> >>   drivers/soundwire/intel.h          |   4 +-
> >>   drivers/soundwire/intel_init.c     |  12 +--
> >>   drivers/soundwire/mipi_disco.c     | 116 +++++++++++----------
> >>   drivers/soundwire/slave.c          |  10 +-
> >>   drivers/soundwire/stream.c         | 161 +++++++++++++++--------------
> > 
> > I would prefer this to be a patch per module. It doesnt help to have a
> > single patch for all the files!
> > 
> > It would be great to have cleanup done per logical group, for example
> > typos in a patch, aligns in another etc...
> 
> You've got to be kidding. I've never seen people ask for this sort of 
> detail.
> 
> > 
> >>   12 files changed, 313 insertions(+), 295 deletions(-)
> >>
> >> diff --git a/drivers/soundwire/Kconfig b/drivers/soundwire/Kconfig
> >> index 19c8efb9a5ee..84876a74874f 100644
> >> --- a/drivers/soundwire/Kconfig
> >> +++ b/drivers/soundwire/Kconfig
> >> @@ -4,7 +4,7 @@
> >>   
> >>   menuconfig SOUNDWIRE
> >>   	bool "SoundWire support"
> >> -	---help---
> >> +	help
> > 
> > Not sure if this is a style issue, kernel seems to have 2990 instances
> > of this!
> 
> this is reported by checkpatch.pl --strict.

Please don't run checkpatch on code that's already in the kernel, and
especially not with the --strict (a.k.a. --subjective) option enabled.

Don't try to fix what isn't broken.

Thanks,
Johan

Re: [alsa-devel] [PATCH v3 2/5] soundwire: fix style issues

[Pierre-Louis Bossart]

>>>> diff --git a/drivers/soundwire/Kconfig b/drivers/soundwire/Kconfig
>>>> index 19c8efb9a5ee..84876a74874f 100644
>>>> --- a/drivers/soundwire/Kconfig
>>>> +++ b/drivers/soundwire/Kconfig
>>>> @@ -4,7 +4,7 @@
>>>>    
>>>>    menuconfig SOUNDWIRE
>>>>    	bool "SoundWire support"
>>>> -	---help---
>>>> +	help
>>>
>>> Not sure if this is a style issue, kernel seems to have 2990 instances
>>> of this!
>>
>> this is reported by checkpatch.pl --strict.
> 
> Please don't run checkpatch on code that's already in the kernel, and
> especially not with the --strict (a.k.a. --subjective) option enabled.
> 
> Don't try to fix what isn't broken.

I would agree in general, but this case is different: the SoundWire code 
in the upstream kernel is missing parts left and right and isn't fully 
functional as is. I will soon be posting what's missing, so this cleanup 
is an opportunity to bring SoundWire to the latest coding standards 
before adding the missing pieces which will be compliant with --strict. 
For the record using --strict already exposed 3 major issues in the 
yet-to-be-released code, so it's not as subjective as you describe it.

Re: [alsa-devel] [PATCH v3 2/5] soundwire: fix style issues

[Johan Hovold]

On Wed, Apr 17, 2019 at 12:18:22PM -0500, Pierre-Louis Bossart wrote:
> 
> >>>> diff --git a/drivers/soundwire/Kconfig b/drivers/soundwire/Kconfig
> >>>> index 19c8efb9a5ee..84876a74874f 100644
> >>>> --- a/drivers/soundwire/Kconfig
> >>>> +++ b/drivers/soundwire/Kconfig
> >>>> @@ -4,7 +4,7 @@
> >>>>    
> >>>>    menuconfig SOUNDWIRE
> >>>>    	bool "SoundWire support"
> >>>> -	---help---
> >>>> +	help
> >>>
> >>> Not sure if this is a style issue, kernel seems to have 2990 instances
> >>> of this!
> >>
> >> this is reported by checkpatch.pl --strict.
> > 
> > Please don't run checkpatch on code that's already in the kernel, and
> > especially not with the --strict (a.k.a. --subjective) option enabled.
> > 
> > Don't try to fix what isn't broken.
> 
> I would agree in general, but this case is different: the SoundWire code 
> in the upstream kernel is missing parts left and right and isn't fully 
> functional as is. I will soon be posting what's missing, so this cleanup 
> is an opportunity to bring SoundWire to the latest coding standards 
> before adding the missing pieces which will be compliant with --strict. 
> For the record using --strict already exposed 3 major issues in the 
> yet-to-be-released code, so it's not as subjective as you describe it.

It's not just me calling it subjective; --subjective is literally
another name for the same switch which enables checks that are
specifically *not* part of the coding standard.

By all my means use it on your own patches before you submit them if
you agree with all or some of those checks, but I doubt all that
open-parenthesis re-alignment is going to expose any major issues. ;)

It does add noise, and makes code forensic and backports harder though.

Johan 

Re: [alsa-devel] [PATCH v3 2/5] soundwire: fix style issues

[Pierre-Louis Bossart]

>>>   enum sdw_command_response
>>>   cdns_xfer_msg_defer(struct sdw_bus *bus,
>>> -        struct sdw_msg *msg, struct sdw_defer *defer)
>>> +            struct sdw_msg *msg, struct sdw_defer *defer)
>>
>> this one too..
>>
>>>   static int cdns_port_params(struct sdw_bus *bus,
>>> -        struct sdw_port_params *p_params, unsigned int bank)
>>> +                struct sdw_port_params *p_params, unsigned int bank)
>>
>> here as well.. (and giving up on rest)
> 
> Please check for yourself that this is a diff illusion w/ tab space.

Vinod, can you please double-check, the alignment issues you reported 
don't exist, see e.g. below what the code looks like after merge.


int sdw_transfer_defer(struct sdw_bus *bus, struct sdw_msg *msg,
                        struct sdw_defer *defer)

int sdw_fill_msg(struct sdw_msg *msg, struct sdw_slave *slave,
                  u32 addr, size_t count, u16 dev_num, u8 flags, u8 *buf)

Re: [PATCH v3 2/5] soundwire: fix style issues

[Pierre-Louis Bossart]

>>>   enum sdw_command_response
>>>   cdns_xfer_msg_defer(struct sdw_bus *bus,
>>> -        struct sdw_msg *msg, struct sdw_defer *defer)
>>> +            struct sdw_msg *msg, struct sdw_defer *defer)
>>
>> this one too..
>>
>>>   static int cdns_port_params(struct sdw_bus *bus,
>>> -        struct sdw_port_params *p_params, unsigned int bank)
>>> +                struct sdw_port_params *p_params, unsigned int bank)
>>
>> here as well.. (and giving up on rest)
> 
> Please check for yourself that this is a diff illusion w/ tab space.

Vinod, can you please double-check, the alignment issues you reported 
don't exist, see e.g. below what the code looks like after merge.


int sdw_transfer_defer(struct sdw_bus *bus, struct sdw_msg *msg,
                        struct sdw_defer *defer)

int sdw_fill_msg(struct sdw_msg *msg, struct sdw_slave *slave,
                  u32 addr, size_t count, u16 dev_num, u8 flags, u8 *buf)

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [PATCH v3 2/5] soundwire: fix style issues

[Vinod Koul]

On 19-04-19, 12:14, Pierre-Louis Bossart wrote:
> 
> > > >   enum sdw_command_response
> > > >   cdns_xfer_msg_defer(struct sdw_bus *bus,
> > > > -        struct sdw_msg *msg, struct sdw_defer *defer)
> > > > +            struct sdw_msg *msg, struct sdw_defer *defer)
> > > 
> > > this one too..
> > > 
> > > >   static int cdns_port_params(struct sdw_bus *bus,
> > > > -        struct sdw_port_params *p_params, unsigned int bank)
> > > > +                struct sdw_port_params *p_params, unsigned int bank)
> > > 
> > > here as well.. (and giving up on rest)
> > 
> > Please check for yourself that this is a diff illusion w/ tab space.
> 
> Vinod, can you please double-check, the alignment issues you reported don't
> exist, see e.g. below what the code looks like after merge.
> 
> 
> int sdw_transfer_defer(struct sdw_bus *bus, struct sdw_msg *msg,
>                        struct sdw_defer *defer)
> 
> int sdw_fill_msg(struct sdw_msg *msg, struct sdw_slave *slave,
>                  u32 addr, size_t count, u16 dev_num, u8 flags, u8 *buf)

Sure, please split up as requested and I shall test apply and check
alignment before reporting...

-- 
~Vinod

Re: [alsa-devel] [PATCH v3 2/5] soundwire: fix style issues

[Vinod Koul]

On 15-04-19, 08:09, Pierre-Louis Bossart wrote:
> 
> > > 
> > > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > > ---
> > >   drivers/soundwire/Kconfig          |   2 +-
> > >   drivers/soundwire/bus.c            |  87 ++++++++--------
> > >   drivers/soundwire/bus.h            |  16 +--
> > >   drivers/soundwire/bus_type.c       |   4 +-
> > >   drivers/soundwire/cadence_master.c |  87 ++++++++--------
> > >   drivers/soundwire/cadence_master.h |  22 ++--
> > >   drivers/soundwire/intel.c          |  87 ++++++++--------
> > >   drivers/soundwire/intel.h          |   4 +-
> > >   drivers/soundwire/intel_init.c     |  12 +--
> > >   drivers/soundwire/mipi_disco.c     | 116 +++++++++++----------
> > >   drivers/soundwire/slave.c          |  10 +-
> > >   drivers/soundwire/stream.c         | 161 +++++++++++++++--------------
> > 
> > I would prefer this to be a patch per module. It doesnt help to have a
> > single patch for all the files!
> > 
> > It would be great to have cleanup done per logical group, for example
> > typos in a patch, aligns in another etc...
> 
> You've got to be kidding. I've never seen people ask for this sort of
> detail.

Nope this is the way it should be. A patch is patch and which
should do one thing! Even if it is a cleanup one.

I dislike a patch which touches everything, core, modules, so please
split up. As a said in review it takes guesswork to find why a change
was done, was it whitespace fix, indentation or not, so please split up
based on type of fixes.

> 
> > 
> > >   12 files changed, 313 insertions(+), 295 deletions(-)
> > > 
> > > diff --git a/drivers/soundwire/Kconfig b/drivers/soundwire/Kconfig
> > > index 19c8efb9a5ee..84876a74874f 100644
> > > --- a/drivers/soundwire/Kconfig
> > > +++ b/drivers/soundwire/Kconfig
> > > @@ -4,7 +4,7 @@
> > >   menuconfig SOUNDWIRE
> > >   	bool "SoundWire support"
> > > -	---help---
> > > +	help
> > 
> > Not sure if this is a style issue, kernel seems to have 2990 instances
> > of this!
> 
> this is reported by checkpatch.pl --strict.
> 
> > 
> > >   	if (msg->page)
> > >   		sdw_reset_page(bus, msg->dev_num);
> > > @@ -243,7 +244,7 @@ int sdw_transfer(struct sdw_bus *bus, struct sdw_msg *msg)
> > >    * Caller needs to hold the msg_lock lock while calling this
> > >    */
> > >   int sdw_transfer_defer(struct sdw_bus *bus, struct sdw_msg *msg,
> > > -				struct sdw_defer *defer)
> > > +		       struct sdw_defer *defer)
> > 
> > this does not seem aligned to me!
> 
> It is, I checked. 2 tabs and 7 spaces.
> 
> > 
> > >   int sdw_fill_msg(struct sdw_msg *msg, struct sdw_slave *slave,
> > > -		u32 addr, size_t count, u16 dev_num, u8 flags, u8 *buf)
> > > +		 u32 addr, size_t count, u16 dev_num, u8 flags, u8 *buf)
> > 
> > this one too
> 
> 2 tabs and one space.
> 
> > 
> > > @@ -458,13 +458,13 @@ static int sdw_assign_device_num(struct sdw_slave *slave)
> > >   		mutex_unlock(&slave->bus->bus_lock);
> > >   		if (dev_num < 0) {
> > >   			dev_err(slave->bus->dev, "Get dev_num failed: %d",
> > > -								dev_num);
> > > +				dev_num);
> > 
> > It might read better if we move the log to second line along with
> > dev_num...

?

> > 
> > >   int sdw_configure_dpn_intr(struct sdw_slave *slave,
> > > -			int port, bool enable, int mask)
> > > +			   int port, bool enable, int mask)
> > 
> > not aligned
> 
> it is in the code. It's a diff illusion.
> 
> > 
> > >   void sdw_extract_slave_id(struct sdw_bus *bus,
> > > -			u64 addr, struct sdw_slave_id *id);
> > > +			  u64 addr, struct sdw_slave_id *id);
> > 
> > Not aligned
> 
> it is in the code. It's a diff illusion.
> 
> > >   enum sdw_command_response
> > >   cdns_xfer_msg_defer(struct sdw_bus *bus,
> > > -		struct sdw_msg *msg, struct sdw_defer *defer)
> > > +		    struct sdw_msg *msg, struct sdw_defer *defer)
> > 
> > this one too..
> > 
> > >   static int cdns_port_params(struct sdw_bus *bus,
> > > -		struct sdw_port_params *p_params, unsigned int bank)
> > > +			    struct sdw_port_params *p_params, unsigned int bank)
> > 
> > here as well.. (and giving up on rest)
> 
> Please check for yourself that this is a diff illusion w/ tab space.

-- 
~Vinod

Re: [alsa-devel] [PATCH v3 2/5] soundwire: fix style issues

[Pierre-Louis Bossart]

On 4/30/19 3:51 AM, Vinod Koul wrote:
> On 15-04-19, 08:09, Pierre-Louis Bossart wrote:
>>
>>>>
>>>> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>>>> ---
>>>>    drivers/soundwire/Kconfig          |   2 +-
>>>>    drivers/soundwire/bus.c            |  87 ++++++++--------
>>>>    drivers/soundwire/bus.h            |  16 +--
>>>>    drivers/soundwire/bus_type.c       |   4 +-
>>>>    drivers/soundwire/cadence_master.c |  87 ++++++++--------
>>>>    drivers/soundwire/cadence_master.h |  22 ++--
>>>>    drivers/soundwire/intel.c          |  87 ++++++++--------
>>>>    drivers/soundwire/intel.h          |   4 +-
>>>>    drivers/soundwire/intel_init.c     |  12 +--
>>>>    drivers/soundwire/mipi_disco.c     | 116 +++++++++++----------
>>>>    drivers/soundwire/slave.c          |  10 +-
>>>>    drivers/soundwire/stream.c         | 161 +++++++++++++++--------------
>>>
>>> I would prefer this to be a patch per module. It doesnt help to have a
>>> single patch for all the files!
>>>
>>> It would be great to have cleanup done per logical group, for example
>>> typos in a patch, aligns in another etc...
>>
>> You've got to be kidding. I've never seen people ask for this sort of
>> detail.
> 
> Nope this is the way it should be. A patch is patch and which
> should do one thing! Even if it is a cleanup one.
> 
> I dislike a patch which touches everything, core, modules, so please
> split up. As a said in review it takes guesswork to find why a change
> was done, was it whitespace fix, indentation or not, so please split up
> based on type of fixes.

With all due respect, you are not helping here but rather slowing things 
down. I've done dozens of cleanups in the ALSA tree and I didn't go in 
this sort of details. The fact that the series was tagged as Reviewed by 
Takashi on April 11 and we are still discussing trivial changes tells me 
the integration model is broken. It's not just me, the patches related 
to runtime-pm from your own Linaro colleagues posted on March 28 went 
nowhere either.

Moving forward, I suggest we merge SoundWire-related patches through the 
sound tree. There will be dependencies in the coming weeks between SOF 
and SoundWire and it makes no sense to have separate maintainers and 
make the life of early adopters more complicated than it needs to be. If 
we have 3-week delays for trivial stuff, I can't imagine what the pace 
will be when I publish the next 20-odd patches I am still working on, 
and the code needed for the SoundWire audio device class being 
standardized as we speak. Things were fine up to now since no one was 
actually using the code, we are in a different model now.

Re: [alsa-devel] [PATCH v3 2/5] soundwire: fix style issues

[Greg KH]

On Tue, Apr 30, 2019 at 08:38:01AM -0500, Pierre-Louis Bossart wrote:
> On 4/30/19 3:51 AM, Vinod Koul wrote:
> > On 15-04-19, 08:09, Pierre-Louis Bossart wrote:
> > > 
> > > > > 
> > > > > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > > > > ---
> > > > >    drivers/soundwire/Kconfig          |   2 +-
> > > > >    drivers/soundwire/bus.c            |  87 ++++++++--------
> > > > >    drivers/soundwire/bus.h            |  16 +--
> > > > >    drivers/soundwire/bus_type.c       |   4 +-
> > > > >    drivers/soundwire/cadence_master.c |  87 ++++++++--------
> > > > >    drivers/soundwire/cadence_master.h |  22 ++--
> > > > >    drivers/soundwire/intel.c          |  87 ++++++++--------
> > > > >    drivers/soundwire/intel.h          |   4 +-
> > > > >    drivers/soundwire/intel_init.c     |  12 +--
> > > > >    drivers/soundwire/mipi_disco.c     | 116 +++++++++++----------
> > > > >    drivers/soundwire/slave.c          |  10 +-
> > > > >    drivers/soundwire/stream.c         | 161 +++++++++++++++--------------
> > > > 
> > > > I would prefer this to be a patch per module. It doesnt help to have a
> > > > single patch for all the files!
> > > > 
> > > > It would be great to have cleanup done per logical group, for example
> > > > typos in a patch, aligns in another etc...
> > > 
> > > You've got to be kidding. I've never seen people ask for this sort of
> > > detail.
> > 
> > Nope this is the way it should be. A patch is patch and which
> > should do one thing! Even if it is a cleanup one.
> > 
> > I dislike a patch which touches everything, core, modules, so please
> > split up. As a said in review it takes guesswork to find why a change
> > was done, was it whitespace fix, indentation or not, so please split up
> > based on type of fixes.
> 
> With all due respect, you are not helping here but rather slowing things
> down. I've done dozens of cleanups in the ALSA tree and I didn't go in this
> sort of details. The fact that the series was tagged as Reviewed by Takashi
> on April 11 and we are still discussing trivial changes tells me the
> integration model is broken. It's not just me, the patches related to
> runtime-pm from your own Linaro colleagues posted on March 28 went nowhere
> either.

My patch-bot would reject a patch that tried to do multiple types of
different cleanups on the same file(s).  Has done so for _years_, this
is not a new thing.

Remember, maintainer/reviewer time is scarce, engineer time is prolific,
we optimize for reviewers, not the people writing the patches.

thanks,

greg k-h

Re: [alsa-devel] [PATCH v3 2/5] soundwire: fix style issues

[Pierre-Louis Bossart]

> My patch-bot would reject a patch that tried to do multiple types of
> different cleanups on the same file(s).  Has done so for _years_, this
> is not a new thing.

If there are tools let's use them (all the fixes in this series were 
reported by tools). Can you share pointers and location of this patch-bot?

Re: [alsa-devel] [PATCH v3 2/5] soundwire: fix style issues

[Greg KH]

On Tue, Apr 30, 2019 at 09:13:55AM -0500, Pierre-Louis Bossart wrote:
> 
> > My patch-bot would reject a patch that tried to do multiple types of
> > different cleanups on the same file(s).  Has done so for _years_, this
> > is not a new thing.
> 
> If there are tools let's use them (all the fixes in this series were
> reported by tools). Can you share pointers and location of this patch-bot?

I talked about my bot a long time ago in one of my presentations, the
source isn't around anywhere public, sorry.

But here's the template for what it can spit out, depending on the patch
input, feel free to cut/paste from it for your use when reviewing
patches.

thanks,

greg k-h
-----------------


Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- Your patch breaks the build.

- Your patch contains warnings and/or errors noticed by the
  scripts/checkpatch.pl tool.

- Your patch is malformed (tabs converted to spaces, linewrapped, etc.)
  and can not be applied.  Please read the file,
  Documentation/email-clients.txt in order to fix this.

- Your patch was attached, please place it inline so that it can be
  applied directly from the email message itself.

- Your patch does not have a Signed-off-by: line.  Please read the
  kernel file, Documentation/SubmittingPatches and resend it after
  adding that line.  Note, the line needs to be in the body of the
  email, before the patch, not at the bottom of the patch or in the
  email signature.

- Your patch was sent privately to Greg.  Kernel development is done in
  public, please always cc: a public mailing list with a patch
  submission.  Using the tool, scripts/get_maintainer.pl on the patch
  will tell you what mailing list to cc.

- Your patch did many different things all at once, making it difficult
  to review.  All Linux kernel patches need to only do one thing at a
  time.  If you need to do multiple things (such as clean up all coding
  style issues in a file/driver), do it in a sequence of patches, each
  one doing only one thing.  This will make it easier to review the
  patches to ensure that they are correct, and to help alleviate any
  merge issues that larger patches can cause.

- Your patch did not apply to any known trees that Greg is in control
  of.  Possibly this is because you made it against Linus's tree, not
  the linux-next tree, which is where all of the development for the
  next version of the kernel is at.  Please refresh your patch against
  the linux-next tree, or even better yet, the development tree
  specified in the MAINTAINERS file for the subsystem you are submitting
  a patch for, and resend it.

- You sent multiple patches, yet no indication of which ones should be
  applied in which order.  Greg could just guess, but if you are
  receiving this email, he guessed wrong and the patches didn't apply.
  Please read the section entitled "The canonical patch format" in the
  kernel file, Documentation/SubmittingPatches for a description of how
  to do this so that Greg has a chance to apply these correctly.

- You did not specify a description of why the patch is needed, or
  possibly, any description at all, in the email body.  Please read the
  section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what is needed in order to
  properly describe the change.

- You did not write a descriptive Subject: for the patch, allowing Greg,
  and everyone else, to know what this patch is all about.  Please read
  the section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what a proper Subject: line should
  look like.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

Re: [alsa-devel] [PATCH v3 2/5] soundwire: fix style issues

[Vfi]

On 30-04-19, 08:38, Pierre-Louis Bossart wrote:
> On 4/30/19 3:51 AM, Vinod Koul wrote:
> > On 15-04-19, 08:09, Pierre-Louis Bossart wrote:
> > > 
> > > > > 
> > > > > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> > > > > ---
> > > > >    drivers/soundwire/Kconfig          |   2 +-
> > > > >    drivers/soundwire/bus.c            |  87 ++++++++--------
> > > > >    drivers/soundwire/bus.h            |  16 +--
> > > > >    drivers/soundwire/bus_type.c       |   4 +-
> > > > >    drivers/soundwire/cadence_master.c |  87 ++++++++--------
> > > > >    drivers/soundwire/cadence_master.h |  22 ++--
> > > > >    drivers/soundwire/intel.c          |  87 ++++++++--------
> > > > >    drivers/soundwire/intel.h          |   4 +-
> > > > >    drivers/soundwire/intel_init.c     |  12 +--
> > > > >    drivers/soundwire/mipi_disco.c     | 116 +++++++++++----------
> > > > >    drivers/soundwire/slave.c          |  10 +-
> > > > >    drivers/soundwire/stream.c         | 161 +++++++++++++++--------------
> > > > 
> > > > I would prefer this to be a patch per module. It doesnt help to have a
> > > > single patch for all the files!
> > > > 
> > > > It would be great to have cleanup done per logical group, for example
> > > > typos in a patch, aligns in another etc...
> > > 
> > > You've got to be kidding. I've never seen people ask for this sort of
> > > detail.
> > 
> > Nope this is the way it should be. A patch is patch and which
> > should do one thing! Even if it is a cleanup one.
> > 
> > I dislike a patch which touches everything, core, modules, so please
> > split up. As a said in review it takes guesswork to find why a change
> > was done, was it whitespace fix, indentation or not, so please split up
> > based on type of fixes.
> 
> With all due respect, you are not helping here but rather slowing things
> down. I've done dozens of cleanups in the ALSA tree and I didn't go in this
> sort of details. 

Thats fine, it is upto people, everyone has different views, mine is
different from Takashi's. We all know for example networking has
different stable and code style rule. That is how it is and I dont think
we would have one rule for all kernel.

All I ask is to be able to review and split up accordingly, I guess that
is a fair request

> The fact that the series was tagged as Reviewed by Takashi
> on April 11 and we are still discussing trivial changes tells me the
> integration model is broken. 

Is it? you got feedback on 15th (that too after my 2 week conf/vacation
break) and I got called crazy for that, not helping!!


> It's not just me the patches related to
> runtime-pm from your own Linaro colleagues posted on March 28 went nowhere
> either.

Does it matter it was a Linaro colleague or not, a patch was posted,
feedback given (similar to cadence one) we agreed that the fix
is not correct and so patch was not applied. I don't think Srini cried
over it!

> Moving forward, I suggest we merge SoundWire-related patches through the
> sound tree. There will be dependencies in the coming weeks between SOF and
> SoundWire and it makes no sense to have separate maintainers and make the
> life of early adopters more complicated than it needs to be. If we have
> 3-week delays for trivial stuff, I can't imagine what the pace will be when
> I publish the next 20-odd patches I am still working on, and the code needed
> for the SoundWire audio device class being standardized as we speak. Things
> were fine up to now since no one was actually using the code, we are in a
> different model now.

I disagree and wont accept it. I dont think you understand that you are
not the most important person in the whole world, the 20 patches series
you are cooking would sure be greatest ever, but that is not the point.
The kernel has a process, you got a feedback, please fix that and post
v2 rather than cribbing, complaining and calling crazy. The energy would
have been better spent on fixing the feedback provided.

Dependencies are _always_ there in kernel development and we know how to
deal with it. Am sure Takashi, Mark and me can come to reasonable
agreement, I wouldn't worry about that!

What we dont do is create new model for your 20 patches.

And I guess I dont have anything more to say on this thread, so I wont
bother replying, please feel free to post v2 and I shall review.

-- 
~Vinod
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [PATCH v3 2/5] soundwire: fix style issues

[Pierre-Louis Bossart]

On 4/30/19 9:54 AM, Vfi\x06inod Koul wrote:
> On 30-04-19, 08:38, Pierre-Louis Bossart wrote:
>> On 4/30/19 3:51 AM, Vinod Koul wrote:
>>> On 15-04-19, 08:09, Pierre-Louis Bossart wrote:
>>>>
>>>>>>
>>>>>> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>>>>>> ---
>>>>>>     drivers/soundwire/Kconfig          |   2 +-
>>>>>>     drivers/soundwire/bus.c            |  87 ++++++++--------
>>>>>>     drivers/soundwire/bus.h            |  16 +--
>>>>>>     drivers/soundwire/bus_type.c       |   4 +-
>>>>>>     drivers/soundwire/cadence_master.c |  87 ++++++++--------
>>>>>>     drivers/soundwire/cadence_master.h |  22 ++--
>>>>>>     drivers/soundwire/intel.c          |  87 ++++++++--------
>>>>>>     drivers/soundwire/intel.h          |   4 +-
>>>>>>     drivers/soundwire/intel_init.c     |  12 +--
>>>>>>     drivers/soundwire/mipi_disco.c     | 116 +++++++++++----------
>>>>>>     drivers/soundwire/slave.c          |  10 +-
>>>>>>     drivers/soundwire/stream.c         | 161 +++++++++++++++--------------
>>>>>
>>>>> I would prefer this to be a patch per module. It doesnt help to have a
>>>>> single patch for all the files!
>>>>>
>>>>> It would be great to have cleanup done per logical group, for example
>>>>> typos in a patch, aligns in another etc...
>>>>
>>>> You've got to be kidding. I've never seen people ask for this sort of
>>>> detail.
>>>
>>> Nope this is the way it should be. A patch is patch and which
>>> should do one thing! Even if it is a cleanup one.
>>>
>>> I dislike a patch which touches everything, core, modules, so please
>>> split up. As a said in review it takes guesswork to find why a change
>>> was done, was it whitespace fix, indentation or not, so please split up
>>> based on type of fixes.
>>
>> With all due respect, you are not helping here but rather slowing things
>> down. I've done dozens of cleanups in the ALSA tree and I didn't go in this
>> sort of details.
> 
> Thats fine, it is upto people, everyone has different views, mine is
> different from Takashi's. We all know for example networking has
> different stable and code style rule. That is how it is and I dont think
> we would have one rule for all kernel.
> 
> All I ask is to be able to review and split up accordingly, I guess that
> is a fair request
> 
>> The fact that the series was tagged as Reviewed by Takashi
>> on April 11 and we are still discussing trivial changes tells me the
>> integration model is broken.
> 
> Is it? you got feedback on 15th (that too after my 2 week conf/vacation
> break) and I got called crazy for that, not helping!!
> 
> 
>> It's not just me the patches related to
>> runtime-pm from your own Linaro colleagues posted on March 28 went nowhere
>> either.
> 
> Does it matter it was a Linaro colleague or not, a patch was posted,
> feedback given (similar to cadence one) we agreed that the fix
> is not correct and so patch was not applied. I don't think Srini cried
> over it!
> 
>> Moving forward, I suggest we merge SoundWire-related patches through the
>> sound tree. There will be dependencies in the coming weeks between SOF and
>> SoundWire and it makes no sense to have separate maintainers and make the
>> life of early adopters more complicated than it needs to be. If we have
>> 3-week delays for trivial stuff, I can't imagine what the pace will be when
>> I publish the next 20-odd patches I am still working on, and the code needed
>> for the SoundWire audio device class being standardized as we speak. Things
>> were fine up to now since no one was actually using the code, we are in a
>> different model now.
> 
> I disagree and wont accept it. I dont think you understand that you are
> not the most important person in the whole world, the 20 patches series
> you are cooking would sure be greatest ever, but that is not the point.
> The kernel has a process, you got a feedback, please fix that and post
> v2 rather than cribbing, complaining and calling crazy. The energy would
> have been better spent on fixing the feedback provided.
> 
> Dependencies are _always_ there in kernel development and we know how to
> deal with it. Am sure Takashi, Mark and me can come to reasonable
> agreement, I wouldn't worry about that!
> 
> What we dont do is create new model for your 20 patches.
> 
> And I guess I dont have anything more to say on this thread, so I wont
> bother replying, please feel free to post v2 and I shall review.

Friends have disagreements. We remain friends and I will provide a v2.

I still believe it makes no sense to split the integration of 
SoundWire-related patches in two different trees. The only rationale for 
it might be that SoundWire is a 'bus' than could be used in other areas. 
Except that for now and the foreseeable future (2022+) it's only for 
audio as a replacement of HDaudio, so the pragmatic way of dealing with 
SoundWire is to merge the code through the audio tree. And given that 
the code is not in a usable state at the moment, dealing with the audio 
tree would not have any negative impact on anyone.

[PATCH v3 3/5] soundwire: bus: remove useless initializations

[Pierre-Louis Bossart]

No need for explicit initialization of page and ssp fields, they are
already zeroed with a memset.

Detected with cppcheck:

[drivers/soundwire/bus.c:309]: (style) Variable 'msg->page' is
reassigned a value before the old one has been used.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 drivers/soundwire/bus.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 691a31df9732..bb697fd68580 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -271,8 +271,6 @@ int sdw_fill_msg(struct sdw_msg *msg, struct sdw_slave *slave,
 	msg->dev_num = dev_num;
 	msg->flags = flags;
 	msg->buf = buf;
-	msg->ssp_sync = false;
-	msg->page = false;
 
 	if (addr < SDW_REG_NO_PAGE) { /* no paging area */
 		return 0;
-- 
2.17.1

[PATCH v3 4/5] soundwire: stream: remove useless initialization of local variable

[Pierre-Louis Bossart]

no need to reset return value.

Detected with cppcheck:
[drivers/soundwire/stream.c:332]: (style) Variable 'ret' is assigned a
value that is never used.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 drivers/soundwire/stream.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index e3d2bc5cba80..ab64c2c4c33f 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -329,7 +329,7 @@ static int sdw_enable_disable_master_ports(struct sdw_master_runtime *m_rt,
 	struct sdw_transport_params *t_params = &p_rt->transport_params;
 	struct sdw_bus *bus = m_rt->bus;
 	struct sdw_enable_ch enable_ch;
-	int ret = 0;
+	int ret;
 
 	enable_ch.port_num = p_rt->num;
 	enable_ch.ch_mask = p_rt->ch_mask;
-- 
2.17.1

[PATCH v3 5/5] soundwire: add missing newlines in dynamic debug logs

[Pierre-Louis Bossart]

For some reason the newlines are not used everywhere. Fix as needed.

Reported-by: Joe Perches <joe@perches.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 drivers/soundwire/bus.c            |  74 +++++++++----------
 drivers/soundwire/cadence_master.c |  12 ++--
 drivers/soundwire/intel.c          |  12 ++--
 drivers/soundwire/stream.c         | 110 ++++++++++++++---------------
 4 files changed, 104 insertions(+), 104 deletions(-)

diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index bb697fd68580..fa86957cb615 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -21,12 +21,12 @@ int sdw_add_bus_master(struct sdw_bus *bus)
 	int ret;
 
 	if (!bus->dev) {
-		pr_err("SoundWire bus has no device");
+		pr_err("SoundWire bus has no device\n");
 		return -ENODEV;
 	}
 
 	if (!bus->ops) {
-		dev_err(bus->dev, "SoundWire Bus ops are not set");
+		dev_err(bus->dev, "SoundWire Bus ops are not set\n");
 		return -EINVAL;
 	}
 
@@ -43,7 +43,7 @@ int sdw_add_bus_master(struct sdw_bus *bus)
 	if (bus->ops->read_prop) {
 		ret = bus->ops->read_prop(bus);
 		if (ret < 0) {
-			dev_err(bus->dev, "Bus read properties failed:%d", ret);
+			dev_err(bus->dev, "Bus read properties failed:%d\n", ret);
 			return ret;
 		}
 	}
@@ -296,7 +296,7 @@ int sdw_fill_msg(struct sdw_msg *msg, struct sdw_slave *slave,
 		return -EINVAL;
 	} else if (!slave->prop.paging_support) {
 		dev_err(&slave->dev,
-			"address %x needs paging but no support", addr);
+			"address %x needs paging but no support\n", addr);
 		return -EINVAL;
 	}
 
@@ -455,13 +455,13 @@ static int sdw_assign_device_num(struct sdw_slave *slave)
 		dev_num = sdw_get_device_num(slave);
 		mutex_unlock(&slave->bus->bus_lock);
 		if (dev_num < 0) {
-			dev_err(slave->bus->dev, "Get dev_num failed: %d",
+			dev_err(slave->bus->dev, "Get dev_num failed: %d\n",
 				dev_num);
 			return dev_num;
 		}
 	} else {
 		dev_info(slave->bus->dev,
-			 "Slave already registered dev_num:%d",
+			 "Slave already registered dev_num:%d\n",
 			 slave->dev_num);
 
 		/* Clear the slave->dev_num to transfer message on device 0 */
@@ -472,7 +472,7 @@ static int sdw_assign_device_num(struct sdw_slave *slave)
 
 	ret = sdw_write(slave, SDW_SCP_DEVNUMBER, dev_num);
 	if (ret < 0) {
-		dev_err(&slave->dev, "Program device_num failed: %d", ret);
+		dev_err(&slave->dev, "Program device_num failed: %d\n", ret);
 		return ret;
 	}
 
@@ -485,7 +485,7 @@ static int sdw_assign_device_num(struct sdw_slave *slave)
 void sdw_extract_slave_id(struct sdw_bus *bus,
 			  u64 addr, struct sdw_slave_id *id)
 {
-	dev_dbg(bus->dev, "SDW Slave Addr: %llx", addr);
+	dev_dbg(bus->dev, "SDW Slave Addr: %llx\n", addr);
 
 	/*
 	 * Spec definition
@@ -505,7 +505,7 @@ void sdw_extract_slave_id(struct sdw_bus *bus,
 	id->class_id = addr & GENMASK(7, 0);
 
 	dev_dbg(bus->dev,
-		"SDW Slave class_id %x, part_id %x, mfg_id %x, unique_id %x, version %x",
+		"SDW Slave class_id %x, part_id %x, mfg_id %x, unique_id %x, version %x\n",
 				id->class_id, id->part_id, id->mfg_id,
 				id->unique_id, id->sdw_version);
 
@@ -562,7 +562,7 @@ static int sdw_program_device_num(struct sdw_bus *bus)
 				ret = sdw_assign_device_num(slave);
 				if (ret) {
 					dev_err(slave->bus->dev,
-						"Assign dev_num failed:%d",
+						"Assign dev_num failed:%d\n",
 						ret);
 					return ret;
 				}
@@ -573,7 +573,7 @@ static int sdw_program_device_num(struct sdw_bus *bus)
 
 		if (!found) {
 			/* TODO: Park this device in Group 13 */
-			dev_err(bus->dev, "Slave Entry not found");
+			dev_err(bus->dev, "Slave Entry not found\n");
 		}
 
 		count++;
@@ -618,7 +618,7 @@ int sdw_configure_dpn_intr(struct sdw_slave *slave,
 	ret = sdw_update(slave, addr, (mask | SDW_DPN_INT_PORT_READY), val);
 	if (ret < 0)
 		dev_err(slave->bus->dev,
-			"SDW_DPN_INTMASK write failed:%d", val);
+			"SDW_DPN_INTMASK write failed:%d\n", val);
 
 	return ret;
 }
@@ -642,7 +642,7 @@ static int sdw_initialize_slave(struct sdw_slave *slave)
 	ret = sdw_update(slave, SDW_SCP_INTMASK1, val, val);
 	if (ret < 0) {
 		dev_err(slave->bus->dev,
-			"SDW_SCP_INTMASK1 write failed:%d", ret);
+			"SDW_SCP_INTMASK1 write failed:%d\n", ret);
 		return ret;
 	}
 
@@ -657,7 +657,7 @@ static int sdw_initialize_slave(struct sdw_slave *slave)
 	ret = sdw_update(slave, SDW_DP0_INTMASK, val, val);
 	if (ret < 0) {
 		dev_err(slave->bus->dev,
-			"SDW_DP0_INTMASK read failed:%d", ret);
+			"SDW_DP0_INTMASK read failed:%d\n", ret);
 		return val;
 	}
 
@@ -672,14 +672,14 @@ static int sdw_handle_dp0_interrupt(struct sdw_slave *slave, u8 *slave_status)
 	status = sdw_read(slave, SDW_DP0_INT);
 	if (status < 0) {
 		dev_err(slave->bus->dev,
-			"SDW_DP0_INT read failed:%d", status);
+			"SDW_DP0_INT read failed:%d\n", status);
 		return status;
 	}
 
 	do {
 
 		if (status & SDW_DP0_INT_TEST_FAIL) {
-			dev_err(&slave->dev, "Test fail for port 0");
+			dev_err(&slave->dev, "Test fail for port 0\n");
 			clear |= SDW_DP0_INT_TEST_FAIL;
 		}
 
@@ -694,7 +694,7 @@ static int sdw_handle_dp0_interrupt(struct sdw_slave *slave, u8 *slave_status)
 		}
 
 		if (status & SDW_DP0_INT_BRA_FAILURE) {
-			dev_err(&slave->dev, "BRA failed");
+			dev_err(&slave->dev, "BRA failed\n");
 			clear |= SDW_DP0_INT_BRA_FAILURE;
 		}
 
@@ -710,7 +710,7 @@ static int sdw_handle_dp0_interrupt(struct sdw_slave *slave, u8 *slave_status)
 		ret = sdw_write(slave, SDW_DP0_INT, clear);
 		if (ret < 0) {
 			dev_err(slave->bus->dev,
-				"SDW_DP0_INT write failed:%d", ret);
+				"SDW_DP0_INT write failed:%d\n", ret);
 			return ret;
 		}
 
@@ -718,7 +718,7 @@ static int sdw_handle_dp0_interrupt(struct sdw_slave *slave, u8 *slave_status)
 		status2 = sdw_read(slave, SDW_DP0_INT);
 		if (status2 < 0) {
 			dev_err(slave->bus->dev,
-				"SDW_DP0_INT read failed:%d", status2);
+				"SDW_DP0_INT read failed:%d\n", status2);
 			return status2;
 		}
 		status &= status2;
@@ -729,7 +729,7 @@ static int sdw_handle_dp0_interrupt(struct sdw_slave *slave, u8 *slave_status)
 	} while (status != 0 && count < SDW_READ_INTR_CLEAR_RETRY);
 
 	if (count == SDW_READ_INTR_CLEAR_RETRY)
-		dev_warn(slave->bus->dev, "Reached MAX_RETRY on DP0 read");
+		dev_warn(slave->bus->dev, "Reached MAX_RETRY on DP0 read\n");
 
 	return ret;
 }
@@ -748,7 +748,7 @@ static int sdw_handle_port_interrupt(struct sdw_slave *slave,
 	status = sdw_read(slave, addr);
 	if (status < 0) {
 		dev_err(slave->bus->dev,
-			"SDW_DPN_INT read failed:%d", status);
+			"SDW_DPN_INT read failed:%d\n", status);
 
 		return status;
 	}
@@ -756,7 +756,7 @@ static int sdw_handle_port_interrupt(struct sdw_slave *slave,
 	do {
 
 		if (status & SDW_DPN_INT_TEST_FAIL) {
-			dev_err(&slave->dev, "Test fail for port:%d", port);
+			dev_err(&slave->dev, "Test fail for port:%d\n", port);
 			clear |= SDW_DPN_INT_TEST_FAIL;
 		}
 
@@ -781,7 +781,7 @@ static int sdw_handle_port_interrupt(struct sdw_slave *slave,
 		ret = sdw_write(slave, addr, clear);
 		if (ret < 0) {
 			dev_err(slave->bus->dev,
-				"SDW_DPN_INT write failed:%d", ret);
+				"SDW_DPN_INT write failed:%d\n", ret);
 			return ret;
 		}
 
@@ -789,7 +789,7 @@ static int sdw_handle_port_interrupt(struct sdw_slave *slave,
 		status2 = sdw_read(slave, addr);
 		if (status2 < 0) {
 			dev_err(slave->bus->dev,
-				"SDW_DPN_INT read failed:%d", status2);
+				"SDW_DPN_INT read failed:%d\n", status2);
 			return status2;
 		}
 		status &= status2;
@@ -820,14 +820,14 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave)
 	buf = ret = sdw_read(slave, SDW_SCP_INT1);
 	if (ret < 0) {
 		dev_err(slave->bus->dev,
-			"SDW_SCP_INT1 read failed:%d", ret);
+			"SDW_SCP_INT1 read failed:%d\n", ret);
 		return ret;
 	}
 
 	ret = sdw_nread(slave, SDW_SCP_INTSTAT2, 2, buf2);
 	if (ret < 0) {
 		dev_err(slave->bus->dev,
-			"SDW_SCP_INT2/3 read failed:%d", ret);
+			"SDW_SCP_INT2/3 read failed:%d\n", ret);
 		return ret;
 	}
 
@@ -837,12 +837,12 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave)
 		 * interrupt
 		 */
 		if (buf & SDW_SCP_INT1_PARITY) {
-			dev_err(&slave->dev, "Parity error detected");
+			dev_err(&slave->dev, "Parity error detected\n");
 			clear |= SDW_SCP_INT1_PARITY;
 		}
 
 		if (buf & SDW_SCP_INT1_BUS_CLASH) {
-			dev_err(&slave->dev, "Bus clash error detected");
+			dev_err(&slave->dev, "Bus clash error detected\n");
 			clear |= SDW_SCP_INT1_BUS_CLASH;
 		}
 
@@ -908,7 +908,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave)
 		ret = sdw_write(slave, SDW_SCP_INT1, clear);
 		if (ret < 0) {
 			dev_err(slave->bus->dev,
-				"SDW_SCP_INT1 write failed:%d", ret);
+				"SDW_SCP_INT1 write failed:%d\n", ret);
 			return ret;
 		}
 
@@ -919,14 +919,14 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave)
 		_buf = ret = sdw_read(slave, SDW_SCP_INT1);
 		if (ret < 0) {
 			dev_err(slave->bus->dev,
-				"SDW_SCP_INT1 read failed:%d", ret);
+				"SDW_SCP_INT1 read failed:%d\n", ret);
 			return ret;
 		}
 
 		ret = sdw_nread(slave, SDW_SCP_INTSTAT2, 2, _buf2);
 		if (ret < 0) {
 			dev_err(slave->bus->dev,
-				"SDW_SCP_INT2/3 read failed:%d", ret);
+				"SDW_SCP_INT2/3 read failed:%d\n", ret);
 			return ret;
 		}
 
@@ -946,7 +946,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave)
 	} while (stat != 0 && count < SDW_READ_INTR_CLEAR_RETRY);
 
 	if (count == SDW_READ_INTR_CLEAR_RETRY)
-		dev_warn(slave->bus->dev, "Reached MAX_RETRY on alert read");
+		dev_warn(slave->bus->dev, "Reached MAX_RETRY on alert read\n");
 
 	return ret;
 }
@@ -975,7 +975,7 @@ int sdw_handle_slave_status(struct sdw_bus *bus,
 	if (status[0] == SDW_SLAVE_ATTACHED) {
 		ret = sdw_program_device_num(bus);
 		if (ret)
-			dev_err(bus->dev, "Slave attach failed: %d", ret);
+			dev_err(bus->dev, "Slave attach failed: %d\n", ret);
 	}
 
 	/* Continue to check other slave statuses */
@@ -1003,7 +1003,7 @@ int sdw_handle_slave_status(struct sdw_bus *bus,
 			ret = sdw_handle_slave_alerts(slave);
 			if (ret)
 				dev_err(bus->dev,
-					"Slave %d alert handling failed: %d",
+					"Slave %d alert handling failed: %d\n",
 					i, ret);
 			break;
 
@@ -1020,13 +1020,13 @@ int sdw_handle_slave_status(struct sdw_bus *bus,
 			ret = sdw_initialize_slave(slave);
 			if (ret)
 				dev_err(bus->dev,
-					"Slave %d initialization failed: %d",
+					"Slave %d initialization failed: %d\n",
 					i, ret);
 
 			break;
 
 		default:
-			dev_err(bus->dev, "Invalid slave %d status:%d",
+			dev_err(bus->dev, "Invalid slave %d status:%d\n",
 				i, status[i]);
 			break;
 		}
@@ -1034,7 +1034,7 @@ int sdw_handle_slave_status(struct sdw_bus *bus,
 		ret = sdw_update_slave_status(slave, status[i]);
 		if (ret)
 			dev_err(slave->bus->dev,
-				"Update Slave status failed:%d", ret);
+				"Update Slave status failed:%d\n", ret);
 
 	}
 
diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c
index 5786a2e0be84..0fdc17b995fc 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -346,10 +346,10 @@ cdns_program_scp_addr(struct sdw_cdns *cdns, struct sdw_msg *msg)
 	for (i = 0; i < 2; i++) {
 		if (!(cdns->response_buf[i] & CDNS_MCP_RESP_ACK)) {
 			no_ack = 1;
-			dev_err(cdns->dev, "Program SCP Ack not received");
+			dev_err(cdns->dev, "Program SCP Ack not received\n");
 			if (cdns->response_buf[i] & CDNS_MCP_RESP_NACK) {
 				nack = 1;
-				dev_err(cdns->dev, "Program SCP NACK received");
+				dev_err(cdns->dev, "Program SCP NACK received\n");
 			}
 		}
 	}
@@ -357,11 +357,11 @@ cdns_program_scp_addr(struct sdw_cdns *cdns, struct sdw_msg *msg)
 	/* For NACK, NO ack, don't return err if we are in Broadcast mode */
 	if (nack) {
 		dev_err(cdns->dev,
-			"SCP_addrpage NACKed for Slave %d", msg->dev_num);
+			"SCP_addrpage NACKed for Slave %d\n", msg->dev_num);
 		return SDW_CMD_FAIL;
 	} else if (no_ack) {
 		dev_dbg(cdns->dev,
-			"SCP_addrpage ignored for Slave %d", msg->dev_num);
+			"SCP_addrpage ignored for Slave %d\n", msg->dev_num);
 		return SDW_CMD_IGNORED;
 	}
 
@@ -665,7 +665,7 @@ int sdw_cdns_enable_interrupt(struct sdw_cdns *cdns)
 	ret = cdns_clear_bit(cdns, CDNS_MCP_CONFIG_UPDATE,
 			     CDNS_MCP_CONFIG_UPDATE_BIT);
 	if (ret < 0)
-		dev_err(cdns->dev, "Config update timedout");
+		dev_err(cdns->dev, "Config update timedout\n");
 
 	return ret;
 }
@@ -853,7 +853,7 @@ int cdns_bus_conf(struct sdw_bus *bus, struct sdw_bus_params *params)
 	int divider;
 
 	if (!params->curr_dr_freq) {
-		dev_err(cdns->dev, "NULL curr_dr_freq");
+		dev_err(cdns->dev, "NULL curr_dr_freq\n");
 		return -EINVAL;
 	}
 
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 619a57d2af38..6f59e930ef85 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -237,7 +237,7 @@ static int intel_shim_init(struct sdw_intel *sdw)
 	ret = intel_clear_bit(shim, SDW_SHIM_SYNC, sync_reg,
 			      SDW_SHIM_SYNC_SYNCCPU);
 	if (ret < 0)
-		dev_err(sdw->cdns.dev, "Failed to set sync period: %d", ret);
+		dev_err(sdw->cdns.dev, "Failed to set sync period: %d\n", ret);
 
 	return ret;
 }
@@ -455,7 +455,7 @@ static int intel_post_bank_switch(struct sdw_bus *bus)
 	ret = intel_clear_bit(shim, SDW_SHIM_SYNC, sync_reg,
 			      SDW_SHIM_SYNC_SYNCGO);
 	if (ret < 0)
-		dev_err(sdw->cdns.dev, "Post bank switch failed: %d", ret);
+		dev_err(sdw->cdns.dev, "Post bank switch failed: %d\n", ret);
 
 	return ret;
 }
@@ -555,7 +555,7 @@ static int intel_hw_params(struct snd_pcm_substream *substream,
 	}
 
 	if (!dma->nr_ports) {
-		dev_err(dai->dev, "ports/resources not available");
+		dev_err(dai->dev, "ports/resources not available\n");
 		return -EINVAL;
 	}
 
@@ -606,7 +606,7 @@ static int intel_hw_params(struct snd_pcm_substream *substream,
 	ret = sdw_stream_add_master(&cdns->bus, &sconfig,
 				    pconfig, dma->nr_ports, dma->stream);
 	if (ret) {
-		dev_err(cdns->dev, "add master to stream failed:%d", ret);
+		dev_err(cdns->dev, "add master to stream failed:%d\n", ret);
 		goto stream_error;
 	}
 
@@ -634,7 +634,7 @@ intel_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
 
 	ret = sdw_stream_remove_master(&cdns->bus, dma->stream);
 	if (ret < 0)
-		dev_err(dai->dev, "remove master from stream %s failed: %d",
+		dev_err(dai->dev, "remove master from stream %s failed: %d\n",
 			dma->stream->name, ret);
 
 	intel_port_cleanup(dma);
@@ -883,7 +883,7 @@ static int intel_probe(struct platform_device *pdev)
 	/* Register DAIs */
 	ret = intel_register_dai(sdw);
 	if (ret) {
-		dev_err(sdw->cdns.dev, "DAI registration failed: %d", ret);
+		dev_err(sdw->cdns.dev, "DAI registration failed: %d\n", ret);
 		snd_soc_unregister_component(sdw->cdns.dev);
 		goto err_dai;
 	}
diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index ab64c2c4c33f..166b0c16003f 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -76,14 +76,14 @@ static int _sdw_program_slave_port_params(struct sdw_bus *bus,
 	/* Program DPN_OffsetCtrl2 registers */
 	ret = sdw_write(slave, addr1, t_params->offset2);
 	if (ret < 0) {
-		dev_err(bus->dev, "DPN_OffsetCtrl2 register write failed");
+		dev_err(bus->dev, "DPN_OffsetCtrl2 register write failed\n");
 		return ret;
 	}
 
 	/* Program DPN_BlockCtrl3 register */
 	ret = sdw_write(slave, addr2, t_params->blk_pkg_mode);
 	if (ret < 0) {
-		dev_err(bus->dev, "DPN_BlockCtrl3 register write failed");
+		dev_err(bus->dev, "DPN_BlockCtrl3 register write failed\n");
 		return ret;
 	}
 
@@ -102,7 +102,7 @@ static int _sdw_program_slave_port_params(struct sdw_bus *bus,
 
 	ret = sdw_write(slave, addr3, wbuf);
 	if (ret < 0) {
-		dev_err(bus->dev, "DPN_SampleCtrl2 register write failed");
+		dev_err(bus->dev, "DPN_SampleCtrl2 register write failed\n");
 		return ret;
 	}
 
@@ -113,7 +113,7 @@ static int _sdw_program_slave_port_params(struct sdw_bus *bus,
 
 	ret = sdw_write(slave, addr4, wbuf);
 	if (ret < 0)
-		dev_err(bus->dev, "DPN_HCtrl register write failed");
+		dev_err(bus->dev, "DPN_HCtrl register write failed\n");
 
 	return ret;
 }
@@ -159,7 +159,7 @@ static int sdw_program_slave_port_params(struct sdw_bus *bus,
 	ret = sdw_update(s_rt->slave, addr1, 0xF, wbuf);
 	if (ret < 0) {
 		dev_err(&s_rt->slave->dev,
-			"DPN_PortCtrl register write failed for port %d",
+			"DPN_PortCtrl register write failed for port %d\n",
 			t_params->port_num);
 		return ret;
 	}
@@ -168,7 +168,7 @@ static int sdw_program_slave_port_params(struct sdw_bus *bus,
 	ret = sdw_write(s_rt->slave, addr2, (p_params->bps - 1));
 	if (ret < 0) {
 		dev_err(&s_rt->slave->dev,
-			"DPN_BlockCtrl1 register write failed for port %d",
+			"DPN_BlockCtrl1 register write failed for port %d\n",
 			t_params->port_num);
 		return ret;
 	}
@@ -178,7 +178,7 @@ static int sdw_program_slave_port_params(struct sdw_bus *bus,
 	ret = sdw_write(s_rt->slave, addr3, wbuf);
 	if (ret < 0) {
 		dev_err(&s_rt->slave->dev,
-			"DPN_SampleCtrl1 register write failed for port %d",
+			"DPN_SampleCtrl1 register write failed for port %d\n",
 			t_params->port_num);
 		return ret;
 	}
@@ -187,7 +187,7 @@ static int sdw_program_slave_port_params(struct sdw_bus *bus,
 	ret = sdw_write(s_rt->slave, addr4, t_params->offset1);
 	if (ret < 0) {
 		dev_err(&s_rt->slave->dev,
-			"DPN_OffsetCtrl1 register write failed for port %d",
+			"DPN_OffsetCtrl1 register write failed for port %d\n",
 			t_params->port_num);
 		return ret;
 	}
@@ -197,7 +197,7 @@ static int sdw_program_slave_port_params(struct sdw_bus *bus,
 		ret = sdw_write(s_rt->slave, addr5, t_params->blk_grp_ctrl);
 		if (ret < 0) {
 			dev_err(&s_rt->slave->dev,
-				"DPN_BlockCtrl2 reg write failed for port %d",
+				"DPN_BlockCtrl2 reg write failed for port %d\n",
 				t_params->port_num);
 			return ret;
 		}
@@ -208,7 +208,7 @@ static int sdw_program_slave_port_params(struct sdw_bus *bus,
 		ret = sdw_write(s_rt->slave, addr6, t_params->lane_ctrl);
 		if (ret < 0) {
 			dev_err(&s_rt->slave->dev,
-				"DPN_LaneCtrl register write failed for port %d",
+				"DPN_LaneCtrl register write failed for port %d\n",
 				t_params->port_num);
 			return ret;
 		}
@@ -219,7 +219,7 @@ static int sdw_program_slave_port_params(struct sdw_bus *bus,
 						     t_params, dpn_prop->type);
 		if (ret < 0)
 			dev_err(&s_rt->slave->dev,
-				"Transport reg write failed for port: %d",
+				"Transport reg write failed for port: %d\n",
 				t_params->port_num);
 	}
 
@@ -316,7 +316,7 @@ static int sdw_enable_disable_slave_ports(struct sdw_bus *bus,
 
 	if (ret < 0)
 		dev_err(&s_rt->slave->dev,
-			"Slave chn_en reg write failed:%d port:%d",
+			"Slave chn_en reg write failed:%d port:%d\n",
 			ret, t_params->port_num);
 
 	return ret;
@@ -342,7 +342,7 @@ static int sdw_enable_disable_master_ports(struct sdw_master_runtime *m_rt,
 							bus->params.next_bank);
 		if (ret < 0) {
 			dev_err(bus->dev,
-				"Master chn_en write failed:%d port:%d",
+				"Master chn_en write failed:%d port:%d\n",
 				ret, t_params->port_num);
 			return ret;
 		}
@@ -400,7 +400,7 @@ static int sdw_do_port_prep(struct sdw_slave_runtime *s_rt,
 		ret = ops->port_prep(s_rt->slave, &prep_ch, cmd);
 		if (ret < 0) {
 			dev_err(&s_rt->slave->dev,
-				"Slave Port Prep cmd %d failed: %d", cmd, ret);
+				"Slave Port Prep cmd %d failed: %d\n", cmd, ret);
 			return ret;
 		}
 	}
@@ -429,7 +429,7 @@ static int sdw_prep_deprep_slave_ports(struct sdw_bus *bus,
 					  prep_ch.num);
 	if (!dpn_prop) {
 		dev_err(bus->dev,
-			"Slave Port:%d properties not found", prep_ch.num);
+			"Slave Port:%d properties not found\n", prep_ch.num);
 		return -EINVAL;
 	}
 
@@ -467,7 +467,7 @@ static int sdw_prep_deprep_slave_ports(struct sdw_bus *bus,
 
 		if (ret < 0) {
 			dev_err(&s_rt->slave->dev,
-				"Slave prep_ctrl reg write failed");
+				"Slave prep_ctrl reg write failed\n");
 			return ret;
 		}
 
@@ -480,7 +480,7 @@ static int sdw_prep_deprep_slave_ports(struct sdw_bus *bus,
 		val &= p_rt->ch_mask;
 		if (!time_left || val) {
 			dev_err(&s_rt->slave->dev,
-				"Chn prep failed for port:%d", prep_ch.num);
+				"Chn prep failed for port:%d\n", prep_ch.num);
 			return -ETIMEDOUT;
 		}
 	}
@@ -515,7 +515,7 @@ static int sdw_prep_deprep_master_ports(struct sdw_master_runtime *m_rt,
 	if (ops->dpn_port_prep) {
 		ret = ops->dpn_port_prep(bus, &prep_ch);
 		if (ret < 0) {
-			dev_err(bus->dev, "Port prepare failed for port:%d",
+			dev_err(bus->dev, "Port prepare failed for port:%d\n",
 				t_params->port_num);
 			return ret;
 		}
@@ -584,7 +584,7 @@ static int sdw_notify_config(struct sdw_master_runtime *m_rt)
 		if (slave->ops->bus_config) {
 			ret = slave->ops->bus_config(slave, &bus->params);
 			if (ret < 0)
-				dev_err(bus->dev, "Notify Slave: %d failed",
+				dev_err(bus->dev, "Notify Slave: %d failed\n",
 					slave->dev_num);
 			return ret;
 		}
@@ -608,13 +608,13 @@ static int sdw_program_params(struct sdw_bus *bus)
 		ret = sdw_program_port_params(m_rt);
 		if (ret < 0) {
 			dev_err(bus->dev,
-				"Program transport params failed: %d", ret);
+				"Program transport params failed: %d\n", ret);
 			return ret;
 		}
 
 		ret = sdw_notify_config(m_rt);
 		if (ret < 0) {
-			dev_err(bus->dev, "Notify bus config failed: %d", ret);
+			dev_err(bus->dev, "Notify bus config failed: %d\n", ret);
 			return ret;
 		}
 
@@ -624,7 +624,7 @@ static int sdw_program_params(struct sdw_bus *bus)
 
 		ret = sdw_enable_disable_ports(m_rt, true);
 		if (ret < 0) {
-			dev_err(bus->dev, "Enable channel failed: %d", ret);
+			dev_err(bus->dev, "Enable channel failed: %d\n", ret);
 			return ret;
 		}
 	}
@@ -679,7 +679,7 @@ static int sdw_bank_switch(struct sdw_bus *bus, int m_rt_count)
 		ret = sdw_transfer(bus, wr_msg);
 
 	if (ret < 0) {
-		dev_err(bus->dev, "Slave frame_ctrl reg write failed");
+		dev_err(bus->dev, "Slave frame_ctrl reg write failed\n");
 		goto error;
 	}
 
@@ -719,7 +719,7 @@ static int sdw_ml_sync_bank_switch(struct sdw_bus *bus)
 						bus->bank_switch_timeout);
 
 	if (!time_left) {
-		dev_err(bus->dev, "Controller Timed out on bank switch");
+		dev_err(bus->dev, "Controller Timed out on bank switch\n");
 		return -ETIMEDOUT;
 	}
 
@@ -756,7 +756,7 @@ static int do_bank_switch(struct sdw_stream_runtime *stream)
 			ret = ops->pre_bank_switch(bus);
 			if (ret < 0) {
 				dev_err(bus->dev,
-					"Pre bank switch op failed: %d", ret);
+					"Pre bank switch op failed: %d\n", ret);
 				goto msg_unlock;
 			}
 		}
@@ -769,7 +769,7 @@ static int do_bank_switch(struct sdw_stream_runtime *stream)
 		 */
 		ret = sdw_bank_switch(bus, stream->m_rt_count);
 		if (ret < 0) {
-			dev_err(bus->dev, "Bank switch failed: %d", ret);
+			dev_err(bus->dev, "Bank switch failed: %d\n", ret);
 			goto error;
 
 		}
@@ -790,12 +790,12 @@ static int do_bank_switch(struct sdw_stream_runtime *stream)
 			ret = ops->post_bank_switch(bus);
 			if (ret < 0) {
 				dev_err(bus->dev,
-					"Post bank switch op failed: %d", ret);
+					"Post bank switch op failed: %d\n", ret);
 				goto error;
 			}
 		} else if (bus->multi_link && stream->m_rt_count > 1) {
 			dev_err(bus->dev,
-				"Post bank switch ops not implemented");
+				"Post bank switch ops not implemented\n");
 			goto error;
 		}
 
@@ -807,7 +807,7 @@ static int do_bank_switch(struct sdw_stream_runtime *stream)
 		ret = sdw_ml_sync_bank_switch(bus);
 		if (ret < 0) {
 			dev_err(bus->dev,
-				"multi link bank switch failed: %d", ret);
+				"multi link bank switch failed: %d\n", ret);
 			goto error;
 		}
 
@@ -1135,13 +1135,13 @@ static int sdw_config_stream(struct device *dev,
 	 */
 	if (stream->params.rate &&
 	    stream->params.rate != stream_config->frame_rate) {
-		dev_err(dev, "rate not matching, stream:%s", stream->name);
+		dev_err(dev, "rate not matching, stream:%s\n", stream->name);
 		return -EINVAL;
 	}
 
 	if (stream->params.bps &&
 	    stream->params.bps != stream_config->bps) {
-		dev_err(dev, "bps not matching, stream:%s", stream->name);
+		dev_err(dev, "bps not matching, stream:%s\n", stream->name);
 		return -EINVAL;
 	}
 
@@ -1161,7 +1161,7 @@ static int sdw_is_valid_port_range(struct device *dev,
 {
 	if (!SDW_VALID_PORT_RANGE(p_rt->num)) {
 		dev_err(dev,
-			"SoundWire: Invalid port number :%d", p_rt->num);
+			"SoundWire: Invalid port number :%d\n", p_rt->num);
 		return -EINVAL;
 	}
 
@@ -1272,7 +1272,7 @@ int sdw_stream_add_master(struct sdw_bus *bus,
 	 */
 	if (!bus->multi_link && stream->m_rt_count > 0) {
 		dev_err(bus->dev,
-			"Multilink not supported, link %d", bus->link_id);
+			"Multilink not supported, link %d\n", bus->link_id);
 		ret = -EINVAL;
 		goto unlock;
 	}
@@ -1280,7 +1280,7 @@ int sdw_stream_add_master(struct sdw_bus *bus,
 	m_rt = sdw_alloc_master_rt(bus, stream_config, stream);
 	if (!m_rt) {
 		dev_err(bus->dev,
-			"Master runtime config failed for stream:%s",
+			"Master runtime config failed for stream:%s\n",
 			stream->name);
 		ret = -ENOMEM;
 		goto unlock;
@@ -1338,7 +1338,7 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 	m_rt = sdw_alloc_master_rt(slave->bus, stream_config, stream);
 	if (!m_rt) {
 		dev_err(&slave->dev,
-			"alloc master runtime failed for stream:%s",
+			"alloc master runtime failed for stream:%s\n",
 			stream->name);
 		ret = -ENOMEM;
 		goto error;
@@ -1347,7 +1347,7 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 	s_rt = sdw_alloc_slave_rt(slave, stream_config, stream);
 	if (!s_rt) {
 		dev_err(&slave->dev,
-			"Slave runtime config failed for stream:%s",
+			"Slave runtime config failed for stream:%s\n",
 			stream->name);
 		ret = -ENOMEM;
 		goto stream_error;
@@ -1477,7 +1477,7 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream)
 
 		/* TODO: Support Asynchronous mode */
 		if ((prop->max_freq % stream->params.rate) != 0) {
-			dev_err(bus->dev, "Async mode not supported");
+			dev_err(bus->dev, "Async mode not supported\n");
 			return -EINVAL;
 		}
 
@@ -1489,7 +1489,7 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream)
 		/* Program params */
 		ret = sdw_program_params(bus);
 		if (ret < 0) {
-			dev_err(bus->dev, "Program params failed: %d", ret);
+			dev_err(bus->dev, "Program params failed: %d\n", ret);
 			goto restore_params;
 		}
 
@@ -1497,7 +1497,7 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream)
 
 	ret = do_bank_switch(stream);
 	if (ret < 0) {
-		dev_err(bus->dev, "Bank switch failed: %d", ret);
+		dev_err(bus->dev, "Bank switch failed: %d\n", ret);
 		goto restore_params;
 	}
 
@@ -1507,7 +1507,7 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream)
 		/* Prepare port(s) on the new clock configuration */
 		ret = sdw_prep_deprep_ports(m_rt, true);
 		if (ret < 0) {
-			dev_err(bus->dev, "Prepare port(s) failed ret = %d",
+			dev_err(bus->dev, "Prepare port(s) failed ret = %d\n",
 				ret);
 			return ret;
 		}
@@ -1534,7 +1534,7 @@ int sdw_prepare_stream(struct sdw_stream_runtime *stream)
 	int ret = 0;
 
 	if (!stream) {
-		pr_err("SoundWire: Handle not found for stream");
+		pr_err("SoundWire: Handle not found for stream\n");
 		return -EINVAL;
 	}
 
@@ -1542,7 +1542,7 @@ int sdw_prepare_stream(struct sdw_stream_runtime *stream)
 
 	ret = _sdw_prepare_stream(stream);
 	if (ret < 0)
-		pr_err("Prepare for stream:%s failed: %d", stream->name, ret);
+		pr_err("Prepare for stream:%s failed: %d\n", stream->name, ret);
 
 	sdw_release_bus_lock(stream);
 	return ret;
@@ -1562,21 +1562,21 @@ static int _sdw_enable_stream(struct sdw_stream_runtime *stream)
 		/* Program params */
 		ret = sdw_program_params(bus);
 		if (ret < 0) {
-			dev_err(bus->dev, "Program params failed: %d", ret);
+			dev_err(bus->dev, "Program params failed: %d\n", ret);
 			return ret;
 		}
 
 		/* Enable port(s) */
 		ret = sdw_enable_disable_ports(m_rt, true);
 		if (ret < 0) {
-			dev_err(bus->dev, "Enable port(s) failed ret: %d", ret);
+			dev_err(bus->dev, "Enable port(s) failed ret: %d\n", ret);
 			return ret;
 		}
 	}
 
 	ret = do_bank_switch(stream);
 	if (ret < 0) {
-		dev_err(bus->dev, "Bank switch failed: %d", ret);
+		dev_err(bus->dev, "Bank switch failed: %d\n", ret);
 		return ret;
 	}
 
@@ -1596,7 +1596,7 @@ int sdw_enable_stream(struct sdw_stream_runtime *stream)
 	int ret = 0;
 
 	if (!stream) {
-		pr_err("SoundWire: Handle not found for stream");
+		pr_err("SoundWire: Handle not found for stream\n");
 		return -EINVAL;
 	}
 
@@ -1604,7 +1604,7 @@ int sdw_enable_stream(struct sdw_stream_runtime *stream)
 
 	ret = _sdw_enable_stream(stream);
 	if (ret < 0)
-		pr_err("Enable for stream:%s failed: %d", stream->name, ret);
+		pr_err("Enable for stream:%s failed: %d\n", stream->name, ret);
 
 	sdw_release_bus_lock(stream);
 	return ret;
@@ -1622,7 +1622,7 @@ static int _sdw_disable_stream(struct sdw_stream_runtime *stream)
 		/* Disable port(s) */
 		ret = sdw_enable_disable_ports(m_rt, false);
 		if (ret < 0) {
-			dev_err(bus->dev, "Disable port(s) failed: %d", ret);
+			dev_err(bus->dev, "Disable port(s) failed: %d\n", ret);
 			return ret;
 		}
 	}
@@ -1633,7 +1633,7 @@ static int _sdw_disable_stream(struct sdw_stream_runtime *stream)
 		/* Program params */
 		ret = sdw_program_params(bus);
 		if (ret < 0) {
-			dev_err(bus->dev, "Program params failed: %d", ret);
+			dev_err(bus->dev, "Program params failed: %d\n", ret);
 			return ret;
 		}
 	}
@@ -1653,7 +1653,7 @@ int sdw_disable_stream(struct sdw_stream_runtime *stream)
 	int ret = 0;
 
 	if (!stream) {
-		pr_err("SoundWire: Handle not found for stream");
+		pr_err("SoundWire: Handle not found for stream\n");
 		return -EINVAL;
 	}
 
@@ -1661,7 +1661,7 @@ int sdw_disable_stream(struct sdw_stream_runtime *stream)
 
 	ret = _sdw_disable_stream(stream);
 	if (ret < 0)
-		pr_err("Disable for stream:%s failed: %d", stream->name, ret);
+		pr_err("Disable for stream:%s failed: %d\n", stream->name, ret);
 
 	sdw_release_bus_lock(stream);
 	return ret;
@@ -1679,7 +1679,7 @@ static int _sdw_deprepare_stream(struct sdw_stream_runtime *stream)
 		/* De-prepare port(s) */
 		ret = sdw_prep_deprep_ports(m_rt, false);
 		if (ret < 0) {
-			dev_err(bus->dev, "De-prepare port(s) failed: %d", ret);
+			dev_err(bus->dev, "De-prepare port(s) failed: %d\n", ret);
 			return ret;
 		}
 
@@ -1690,7 +1690,7 @@ static int _sdw_deprepare_stream(struct sdw_stream_runtime *stream)
 		/* Program params */
 		ret = sdw_program_params(bus);
 		if (ret < 0) {
-			dev_err(bus->dev, "Program params failed: %d", ret);
+			dev_err(bus->dev, "Program params failed: %d\n", ret);
 			return ret;
 		}
 
@@ -1712,14 +1712,14 @@ int sdw_deprepare_stream(struct sdw_stream_runtime *stream)
 	int ret = 0;
 
 	if (!stream) {
-		pr_err("SoundWire: Handle not found for stream");
+		pr_err("SoundWire: Handle not found for stream\n");
 		return -EINVAL;
 	}
 
 	sdw_acquire_bus_lock(stream);
 	ret = _sdw_deprepare_stream(stream);
 	if (ret < 0)
-		pr_err("De-prepare for stream:%d failed: %d", ret, ret);
+		pr_err("De-prepare for stream:%d failed: %d\n", ret, ret);
 
 	sdw_release_bus_lock(stream);
 	return ret;
-- 
2.17.1

Re: [PATCH v3 0/5] soundwire: code cleanup

[Takashi Iwai]

On Thu, 11 Apr 2019 05:16:56 +0200,
Pierre-Louis Bossart wrote:
> 
> SoundWire support will be provided in Linux with the Sound Open
> Firmware (SOF) on Intel platforms. Before we start adding the missing
> pieces, there are a number of warnings and style issues reported by
> checkpatch, cppcheck and Coccinelle that need to be cleaned-up.
> 
> Changes since v2:
> fixed inversion of devm_kcalloc parameters, detected while rebasing
> additional patches.
> 
> Changes since v1:
> added missing newlines in new patch (suggested by Joe Perches)
> 
> Pierre-Louis Bossart (5):
>   soundwire: intel: fix inversion in devm_kcalloc parameters
>   soundwire: fix style issues
>   soundwire: bus: remove useless initializations
>   soundwire: stream: remove useless initialization of local variable
>   soundwire: add missing newlines in dynamic debug logs

All looked fine, for the series:
  Reviewed-by: Takashi Iwai <tiwai@suse.de>


thanks,

Takashi

Re: [PATCH v3 0/5] soundwire: code cleanup

[Vinod Koul]

On 10-04-19, 22:16, Pierre-Louis Bossart wrote:
> SoundWire support will be provided in Linux with the Sound Open
> Firmware (SOF) on Intel platforms. Before we start adding the missing
> pieces, there are a number of warnings and style issues reported by
> checkpatch, cppcheck and Coccinelle that need to be cleaned-up.

Applied, 1, 3 and 4 (5 looked good but didnt apply)

Thanks

> 
> Changes since v2:
> fixed inversion of devm_kcalloc parameters, detected while rebasing
> additional patches.
> 
> Changes since v1:
> added missing newlines in new patch (suggested by Joe Perches)
> 
> Pierre-Louis Bossart (5):
>   soundwire: intel: fix inversion in devm_kcalloc parameters
>   soundwire: fix style issues
>   soundwire: bus: remove useless initializations
>   soundwire: stream: remove useless initialization of local variable
>   soundwire: add missing newlines in dynamic debug logs
> 
>  drivers/soundwire/Kconfig          |   2 +-
>  drivers/soundwire/bus.c            | 137 ++++++++-------
>  drivers/soundwire/bus.h            |  16 +-
>  drivers/soundwire/bus_type.c       |   4 +-
>  drivers/soundwire/cadence_master.c |  99 +++++------
>  drivers/soundwire/cadence_master.h |  22 +--
>  drivers/soundwire/intel.c          | 103 ++++++-----
>  drivers/soundwire/intel.h          |   4 +-
>  drivers/soundwire/intel_init.c     |  12 +-
>  drivers/soundwire/mipi_disco.c     | 116 +++++++------
>  drivers/soundwire/slave.c          |  10 +-
>  drivers/soundwire/stream.c         | 267 +++++++++++++++--------------
>  12 files changed, 404 insertions(+), 388 deletions(-)
> 
> -- 
> 2.17.1

-- 
~Vinod

Re: [alsa-devel] [PATCH v3 0/5] soundwire: code cleanup

[Pierre-Louis Bossart]

On 4/14/19 5:04 AM, Vinod Koul wrote:
> On 10-04-19, 22:16, Pierre-Louis Bossart wrote:
>> SoundWire support will be provided in Linux with the Sound Open
>> Firmware (SOF) on Intel platforms. Before we start adding the missing
>> pieces, there are a number of warnings and style issues reported by
>> checkpatch, cppcheck and Coccinelle that need to be cleaned-up.
> 
> Applied, 1, 3 and 4 (5 looked good but didnt apply)

this applies on top of Mark Brown's for-next tree. This should be the 
reference really for SoundWire work, if we start having another tree 
with deltas it's really counter productive. I already have dependencies 
on acpi...

Re: [alsa-devel] [PATCH v3 0/5] soundwire: code cleanup

[Pierre-Louis Bossart]

On 4/15/19 7:57 AM, Pierre-Louis Bossart wrote:
> 
> 
> On 4/14/19 5:04 AM, Vinod Koul wrote:
>> On 10-04-19, 22:16, Pierre-Louis Bossart wrote:
>>> SoundWire support will be provided in Linux with the Sound Open
>>> Firmware (SOF) on Intel platforms. Before we start adding the missing
>>> pieces, there are a number of warnings and style issues reported by
>>> checkpatch, cppcheck and Coccinelle that need to be cleaned-up.
>>
>> Applied, 1, 3 and 4 (5 looked good but didnt apply)
> 
> this applies on top of Mark Brown's for-next tree. This should be the 
> reference really for SoundWire work, if we start having another tree 
> with deltas it's really counter productive. I already have dependencies 
> on acpi...

Vinod, can you please double-check why patch 5 didn't work for you. I 
tried on Mark and Linus' trees and no issues, see below.

$ git reset --hard v5.1-rc5
HEAD is now at dc4060a5dc25 Linux 5.1-rc5

$ git am ~/Downloads/alsa/39/\[PATCH\ v3\ *.eml
Applying: soundwire: intel: fix inversion in devm_kcalloc parameters
Applying: soundwire: fix style issues
Applying: soundwire: bus: remove useless initializations
Applying: soundwire: stream: remove useless initialization of local variable
Applying: soundwire: add missing newlines in dynamic debug logs

$ git reset --hard broonie/for-next
HEAD is now at 84fdefca04a7 Merge branch 'asoc-5.2' into asoc-next

$ git am ~/Downloads/alsa/39/\[PATCH\ v3\ *.eml
Applying: soundwire: intel: fix inversion in devm_kcalloc parameters
Applying: soundwire: fix style issues
Applying: soundwire: bus: remove useless initializations
Applying: soundwire: stream: remove useless initialization of local variable
Applying: soundwire: add missing newlines in dynamic debug logs

Re: [PATCH v3 0/5] soundwire: code cleanup

[Pierre-Louis Bossart]

On 4/15/19 7:57 AM, Pierre-Louis Bossart wrote:
> 
> 
> On 4/14/19 5:04 AM, Vinod Koul wrote:
>> On 10-04-19, 22:16, Pierre-Louis Bossart wrote:
>>> SoundWire support will be provided in Linux with the Sound Open
>>> Firmware (SOF) on Intel platforms. Before we start adding the missing
>>> pieces, there are a number of warnings and style issues reported by
>>> checkpatch, cppcheck and Coccinelle that need to be cleaned-up.
>>
>> Applied, 1, 3 and 4 (5 looked good but didnt apply)
> 
> this applies on top of Mark Brown's for-next tree. This should be the 
> reference really for SoundWire work, if we start having another tree 
> with deltas it's really counter productive. I already have dependencies 
> on acpi...

Vinod, can you please double-check why patch 5 didn't work for you. I 
tried on Mark and Linus' trees and no issues, see below.

$ git reset --hard v5.1-rc5
HEAD is now at dc4060a5dc25 Linux 5.1-rc5

$ git am ~/Downloads/alsa/39/\[PATCH\ v3\ *.eml
Applying: soundwire: intel: fix inversion in devm_kcalloc parameters
Applying: soundwire: fix style issues
Applying: soundwire: bus: remove useless initializations
Applying: soundwire: stream: remove useless initialization of local variable
Applying: soundwire: add missing newlines in dynamic debug logs

$ git reset --hard broonie/for-next
HEAD is now at 84fdefca04a7 Merge branch 'asoc-5.2' into asoc-next

$ git am ~/Downloads/alsa/39/\[PATCH\ v3\ *.eml
Applying: soundwire: intel: fix inversion in devm_kcalloc parameters
Applying: soundwire: fix style issues
Applying: soundwire: bus: remove useless initializations
Applying: soundwire: stream: remove useless initialization of local variable
Applying: soundwire: add missing newlines in dynamic debug logs