[PATCH 00/17] soundwire: fixes for 5.4

[PATCH 00/17] soundwire: fixes for 5.4

[Pierre-Louis Bossart]

This series provides an update on the initial RFC. Debugfs and Intel
updates will be provided in follow-up patches. The order of patches
was changed since the RFC so detailed change logs are provided below.

Changes since RFC:

patch1: feedback from Vinod and Cezary
use local variable to reduce number of de-references
document that first argument is mandatory

patch2: feedback from Vinod
document that the work-around is required for all existing controllers

patch 3: feedback from Vinod
clarify comment that MCP_INT_IRQ ungates all other settings.
use MCP_INT_SLAVE_MASK instead of all individual settings ORed.

Patch 4: feedback from Vinod:
demote dynamic debug log to dev_dbg (was dev_err)

patch5:
clarify commit message that the helpers will be used in the Cadence
parts as well.

patch 6: feedback from Vinod
s/BIOS/firmware
remove magic numbers, introduce macros

patch 7:
add kbuild warning message in commit message

patch 8:
no change

patch 9: feedback from Guennadi
remove unnecessary initializations

patch 10/11/12: feedback from Vinod
split initial patch in 3 (prototype, Intel, Cadence)
add explanations on what mclk_freq is in commit messages
remove pr_err logs missed in RFC

patch 13: feedback from Bard
remove unecessary reads before update

patch 14:
no change

patch 15:
update commit message

patch 16:
update commit message
remove unnecessary dynamic debug log

patch 17:
no change

Bard liao (1):
  soundwire: include mod_devicetable.h to avoid compiling warnings

Pierre-Louis Bossart (15):
  soundwire: intel: prevent possible dereference in hw_params
  soundwire: intel: fix channel number reported by hardware
  soundwire: cadence_master: revisit interrupt settings
  soundwire: bus: improve dynamic debug comments for enumeration
  soundwire: export helpers to find row and column values
  soundwire: cadence_master: use firmware defaults for frame shape
  soundwire: stream: fix disable sequence
  soundwire: stream: remove unnecessary variable initializations
  soundwire: add new mclk_freq field for properties
  soundwire: intel: read mclk_freq property from firmware
  soundwire: cadence_master: make use of mclk_freq property
  soundwire: intel: handle disabled links
  soundwire: intel_init: add kernel module parameter to filter out links
  soundwire: cadence_master: add kernel parameter to override interrupt
    mask
  soundwire: intel: move shutdown() callback and don't export symbol

Rander Wang (1):
  soundwire: cadence_master: fix divider setting in clock register

 drivers/soundwire/bus.c             |   5 +-
 drivers/soundwire/bus.h             |   7 +-
 drivers/soundwire/cadence_master.c  | 102 +++++++++++++++++++---------
 drivers/soundwire/cadence_master.h  |   2 -
 drivers/soundwire/intel.c           |  83 ++++++++++++++++++++--
 drivers/soundwire/intel_init.c      |  11 +++
 drivers/soundwire/stream.c          |  93 ++++++++++++++++---------
 include/linux/soundwire/sdw.h       |   6 ++
 include/linux/soundwire/sdw_intel.h |   1 +
 9 files changed, 236 insertions(+), 74 deletions(-)

-- 
2.20.1

[PATCH 01/17] soundwire: intel: prevent possible dereference in hw_params

[Pierre-Louis Bossart]

This should not happen in production systems but we should test for
all callback arguments before invoking the config_stream callback.

Update the prototype to clarify that the first argument is mandatory.

Also use local variable instead of multiple dereferences to improve
readability.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 drivers/soundwire/intel.c           | 6 ++++--
 include/linux/soundwire/sdw_intel.h | 1 +
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 3d9ca6479e94..a906789c7f78 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -498,8 +498,10 @@ static int intel_config_stream(struct sdw_intel *sdw,
 			       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,
+	struct sdw_intel_link_res *res = sdw->res;
+
+	if (res->ops && res->ops->config_stream && res->arg)
+		return res->ops->config_stream(res->arg,
 				substream, dai, hw_params, link_id);
 
 	return -EIO;
diff --git a/include/linux/soundwire/sdw_intel.h b/include/linux/soundwire/sdw_intel.h
index 4d70da45363d..c9427cb6020b 100644
--- a/include/linux/soundwire/sdw_intel.h
+++ b/include/linux/soundwire/sdw_intel.h
@@ -8,6 +8,7 @@
  * struct sdw_intel_ops: Intel audio driver callback ops
  *
  * @config_stream: configure the stream with the hw_params
+ * the first argument containing the context is mandatory
  */
 struct sdw_intel_ops {
 	int (*config_stream)(void *arg, void *substream,
-- 
2.20.1

[PATCH 02/17] soundwire: intel: fix channel number reported by hardware

[Pierre-Louis Bossart]

On all released Intel controllers (CNL/CML/ICL), PDI2 reports an
invalid count, force the correct hardware-supported value

This may have to be revisited with platform-specific values if the
hardware changes, but for now this is good enough.

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

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index a906789c7f78..fe3266bc1d12 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -390,6 +390,16 @@ intel_pdi_get_ch_cap(struct sdw_intel *sdw, unsigned int pdi_num, bool pcm)
 
 	if (pcm) {
 		count = intel_readw(shim, SDW_SHIM_PCMSYCHC(link_id, pdi_num));
+
+		/*
+		 * WORKAROUND: on all existing Intel controllers, pdi
+		 * number 2 reports channel count as 1 even though it
+		 * supports 8 channels. Performing hardcoding for pdi
+		 * number 2.
+		 */
+		if (pdi_num == 2)
+			count = 7;
+
 	} else {
 		count = intel_readw(shim, SDW_SHIM_PDMSCAP(link_id));
 		count = ((count & SDW_SHIM_PDMSCAP_CPSS) >>
-- 
2.20.1

[PATCH 03/17] soundwire: cadence_master: revisit interrupt settings

[Pierre-Louis Bossart]

Adding missing interrupt masks (parity, etc) and missing checks.
Clarify which masks are for which usage.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 drivers/soundwire/cadence_master.c | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c
index a1c5f05ef27b..5d9729b4d634 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -76,9 +76,12 @@
 #define CDNS_MCP_INT_DPINT			BIT(11)
 #define CDNS_MCP_INT_CTRL_CLASH			BIT(10)
 #define CDNS_MCP_INT_DATA_CLASH			BIT(9)
+#define CDNS_MCP_INT_PARITY			BIT(8)
 #define CDNS_MCP_INT_CMD_ERR			BIT(7)
+#define CDNS_MCP_INT_RX_NE			BIT(3)
 #define CDNS_MCP_INT_RX_WL			BIT(2)
 #define CDNS_MCP_INT_TXE			BIT(1)
+#define CDNS_MCP_INT_TXF			BIT(0)
 
 #define CDNS_MCP_INTSET				0x4C
 
@@ -669,6 +672,11 @@ irqreturn_t sdw_cdns_irq(int irq, void *dev_id)
 		}
 	}
 
+	if (int_status & CDNS_MCP_INT_PARITY) {
+		/* Parity error detected by Master */
+		dev_err_ratelimited(cdns->dev, "Parity error\n");
+	}
+
 	if (int_status & CDNS_MCP_INT_CTRL_CLASH) {
 		/* Slave is driving bit slot during control word */
 		dev_err_ratelimited(cdns->dev, "Bus clash for control word\n");
@@ -736,10 +744,23 @@ static int _cdns_enable_interrupt(struct sdw_cdns *cdns)
 	cdns_writel(cdns, CDNS_MCP_SLAVE_INTMASK1,
 		    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 |
-		CDNS_MCP_INT_CTRL_CLASH | CDNS_MCP_INT_DATA_CLASH |
-		CDNS_MCP_INT_RX_WL | CDNS_MCP_INT_IRQ | CDNS_MCP_INT_DPINT;
+	/* enable detection of all slave state changes */
+	mask = CDNS_MCP_INT_SLAVE_MASK;
+
+	/* enable detection of bus issues */
+	mask |= CDNS_MCP_INT_CTRL_CLASH | CDNS_MCP_INT_DATA_CLASH |
+		CDNS_MCP_INT_PARITY;
+
+	/* no detection of port interrupts for now */
+
+	/* enable detection of RX fifo level */
+	mask |= CDNS_MCP_INT_RX_WL;
+
+	/*
+	 * CDNS_MCP_INT_IRQ needs to be set otherwise all previous
+	 * settings are irrelevant
+	 */
+	mask |= CDNS_MCP_INT_IRQ;
 
 	cdns_writel(cdns, CDNS_MCP_INTMASK, mask);
 
-- 
2.20.1

[PATCH 04/17] soundwire: bus: improve dynamic debug comments for enumeration

[Pierre-Louis Bossart]

update comments to provide better understanding of enumeration flows.

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

diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 89d5f1537d9b..40f7d0dc59a6 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -476,7 +476,8 @@ 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\n", ret);
+		dev_err(&slave->dev, "Program device_num %d failed: %d\n",
+			dev_num, ret);
 		return ret;
 	}
 
@@ -533,6 +534,7 @@ static int sdw_program_device_num(struct sdw_bus *bus)
 	do {
 		ret = sdw_transfer(bus, &msg);
 		if (ret == -ENODATA) { /* end of device id reads */
+			dev_dbg(bus->dev, "No more devices to enumerate\n");
 			ret = 0;
 			break;
 		}
@@ -975,6 +977,7 @@ int sdw_handle_slave_status(struct sdw_bus *bus,
 	int i, ret = 0;
 
 	if (status[0] == SDW_SLAVE_ATTACHED) {
+		dev_dbg(bus->dev, "Slave attached, programming device number\n");
 		ret = sdw_program_device_num(bus);
 		if (ret)
 			dev_err(bus->dev, "Slave attach failed: %d\n", ret);
-- 
2.20.1

[PATCH 05/17] soundwire: export helpers to find row and column values

[Pierre-Louis Bossart]

Add a prefix for common tables and export 2 helpers to set the frame
shapes based on row/col values.

These changes simplify bandwidth allocation algorithms as well as the
Cadence parts which all need to convert from frame shape to indices
used by the standard. These helpers are used in the following patch.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 drivers/soundwire/bus.h    |  7 +++++--
 drivers/soundwire/stream.c | 14 ++++++++------
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h
index 648436a995a3..89e74adf96fa 100644
--- a/drivers/soundwire/bus.h
+++ b/drivers/soundwire/bus.h
@@ -73,8 +73,11 @@ struct sdw_msg {
 
 #define SDW_DOUBLE_RATE_FACTOR		2
 
-extern int rows[SDW_FRAME_ROWS];
-extern int cols[SDW_FRAME_COLS];
+extern int sdw_rows[SDW_FRAME_ROWS];
+extern int sdw_cols[SDW_FRAME_COLS];
+
+int sdw_find_row_index(int row);
+int sdw_find_col_index(int col);
 
 /**
  * sdw_port_runtime: Runtime port parameters for Master or Slave
diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index a0476755a459..53f5e790fcd7 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -21,37 +21,39 @@
  * The rows are arranged as per the array index value programmed
  * in register. The index 15 has dummy value 0 in order to fill hole.
  */
-int rows[SDW_FRAME_ROWS] = {48, 50, 60, 64, 75, 80, 125, 147,
+int sdw_rows[SDW_FRAME_ROWS] = {48, 50, 60, 64, 75, 80, 125, 147,
 			96, 100, 120, 128, 150, 160, 250, 0,
 			192, 200, 240, 256, 72, 144, 90, 180};
 
-int cols[SDW_FRAME_COLS] = {2, 4, 6, 8, 10, 12, 14, 16};
+int sdw_cols[SDW_FRAME_COLS] = {2, 4, 6, 8, 10, 12, 14, 16};
 
-static int sdw_find_col_index(int col)
+int sdw_find_col_index(int col)
 {
 	int i;
 
 	for (i = 0; i < SDW_FRAME_COLS; i++) {
-		if (cols[i] == col)
+		if (sdw_cols[i] == col)
 			return i;
 	}
 
 	pr_warn("Requested column not found, selecting lowest column no: 2\n");
 	return 0;
 }
+EXPORT_SYMBOL(sdw_find_col_index);
 
-static int sdw_find_row_index(int row)
+int sdw_find_row_index(int row)
 {
 	int i;
 
 	for (i = 0; i < SDW_FRAME_ROWS; i++) {
-		if (rows[i] == row)
+		if (sdw_rows[i] == row)
 			return i;
 	}
 
 	pr_warn("Requested row not found, selecting lowest row no: 48\n");
 	return 0;
 }
+EXPORT_SYMBOL(sdw_find_row_index);
 
 static int _sdw_program_slave_port_params(struct sdw_bus *bus,
 					  struct sdw_slave *slave,
-- 
2.20.1

[PATCH 06/17] soundwire: cadence_master: use firmware defaults for frame shape

[Pierre-Louis Bossart]

Remove hard-coding and use firmware (BIOS/DT) values. If they are
wrong use default 48x2 frame shape.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 drivers/soundwire/cadence_master.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c
index 5d9729b4d634..89c55e4bb72c 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -48,6 +48,8 @@
 #define CDNS_MCP_SSPSTAT			0xC
 #define CDNS_MCP_FRAME_SHAPE			0x10
 #define CDNS_MCP_FRAME_SHAPE_INIT		0x14
+#define CDNS_MCP_FRAME_SHAPE_COL_MASK		GENMASK(2, 0)
+#define CDNS_MCP_FRAME_SHAPE_ROW_OFFSET		3
 
 #define CDNS_MCP_CONFIG_UPDATE			0x18
 #define CDNS_MCP_CONFIG_UPDATE_BIT		BIT(0)
@@ -175,7 +177,6 @@
 /* Driver defaults */
 
 #define CDNS_DEFAULT_CLK_DIVIDER		0
-#define CDNS_DEFAULT_FRAME_SHAPE		0x30
 #define CDNS_DEFAULT_SSP_INTERVAL		0x18
 #define CDNS_TX_TIMEOUT				2000
 
@@ -901,6 +902,20 @@ int sdw_cdns_pdi_init(struct sdw_cdns *cdns,
 }
 EXPORT_SYMBOL(sdw_cdns_pdi_init);
 
+static u32 cdns_set_initial_frame_shape(int n_rows, int n_cols)
+{
+	u32 val;
+	int c;
+	int r;
+
+	r = sdw_find_row_index(n_rows);
+	c = sdw_find_col_index(n_cols) & CDNS_MCP_FRAME_SHAPE_COL_MASK;
+
+	val = (r << CDNS_MCP_FRAME_SHAPE_ROW_OFFSET) | c;
+
+	return val;
+}
+
 /**
  * sdw_cdns_init() - Cadence initialization
  * @cdns: Cadence instance
@@ -923,8 +938,13 @@ int sdw_cdns_init(struct sdw_cdns *cdns)
 	val |= CDNS_DEFAULT_CLK_DIVIDER;
 	cdns_writel(cdns, CDNS_MCP_CLK_CTRL0, val);
 
-	/* Set the default frame shape */
-	cdns_writel(cdns, CDNS_MCP_FRAME_SHAPE_INIT, CDNS_DEFAULT_FRAME_SHAPE);
+	/*
+	 * Frame shape changes after initialization have to be done
+	 * with the bank switch mechanism
+	 */
+	val = cdns_set_initial_frame_shape(prop->default_row,
+					   prop->default_col);
+	cdns_writel(cdns, CDNS_MCP_FRAME_SHAPE_INIT, val);
 
 	/* Set SSP interval to default value */
 	cdns_writel(cdns, CDNS_MCP_SSP_CTRL0, CDNS_DEFAULT_SSP_INTERVAL);
-- 
2.20.1

Re: [PATCH 06/17] soundwire: cadence_master: use firmware defaults for frame shape

[Cezary Rojewski]

On 2019-08-06 02:55, Pierre-Louis Bossart wrote:
> diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c
> index 5d9729b4d634..89c55e4bb72c 100644
> --- a/drivers/soundwire/cadence_master.c
> +++ b/drivers/soundwire/cadence_master.c
> @@ -48,6 +48,8 @@
>   #define CDNS_MCP_SSPSTAT			0xC
>   #define CDNS_MCP_FRAME_SHAPE			0x10
>   #define CDNS_MCP_FRAME_SHAPE_INIT		0x14
> +#define CDNS_MCP_FRAME_SHAPE_COL_MASK		GENMASK(2, 0)
> +#define CDNS_MCP_FRAME_SHAPE_ROW_OFFSET		3
>   
>   #define CDNS_MCP_CONFIG_UPDATE			0x18
>   #define CDNS_MCP_CONFIG_UPDATE_BIT		BIT(0)
> @@ -175,7 +177,6 @@
>   /* Driver defaults */
>   
>   #define CDNS_DEFAULT_CLK_DIVIDER		0
> -#define CDNS_DEFAULT_FRAME_SHAPE		0x30
>   #define CDNS_DEFAULT_SSP_INTERVAL		0x18
>   #define CDNS_TX_TIMEOUT				2000
>   
> @@ -901,6 +902,20 @@ int sdw_cdns_pdi_init(struct sdw_cdns *cdns,
>   }
>   EXPORT_SYMBOL(sdw_cdns_pdi_init);
>   
> +static u32 cdns_set_initial_frame_shape(int n_rows, int n_cols)
> +{
> +	u32 val;
> +	int c;
> +	int r;
> +
> +	r = sdw_find_row_index(n_rows);
> +	c = sdw_find_col_index(n_cols) & CDNS_MCP_FRAME_SHAPE_COL_MASK;
> +
> +	val = (r << CDNS_MCP_FRAME_SHAPE_ROW_OFFSET) | c;
> +
> +	return val;
> +}
> +

Guess this have been said already, but this function could be simplified 
- unless you really want to keep explicit variable declaration. Both "c" 
and "r" declarations could be merged into single line while "val" is not 
needed at all.

One more thing - is AND bitwise op really needed for cols explicitly? We 
know all col values upfront - these are static and declared in global 
table nearby. Static declaration takes care of "initial range-check". Is 
another one necessary?

Moreover, this is a _get_ and certainly not a _set_ type of function. 
I'd even consider renaming it to: "cdns_get_frame_shape" as this is 
neither a _set_ nor an explicit initial frame shape setter.

It might be even helpful to split two usages:

#define sdw_frame_shape(col_idx, row_idx) \
	((row_idx << CDNS_MCP_FRAME_SHAPE_ROW_OFFSET) | col_idx)

u32 cdns_get_frame_shape(u16 rows, u16 cols)
{
	u16 c, r;

	r = sdw_find_row_index(rows);
	c = sdw_find_col_index(cols);

	return sdw_frame_shape(c, r);
}

The above may even be simplified into one-liner.

Re: [alsa-devel] [PATCH 06/17] soundwire: cadence_master: use firmware defaults for frame shape

[Pierre-Louis Bossart]

On 8/6/19 10:27 AM, Cezary Rojewski wrote:
> On 2019-08-06 02:55, Pierre-Louis Bossart wrote:
>> diff --git a/drivers/soundwire/cadence_master.c 
>> b/drivers/soundwire/cadence_master.c
>> index 5d9729b4d634..89c55e4bb72c 100644
>> --- a/drivers/soundwire/cadence_master.c
>> +++ b/drivers/soundwire/cadence_master.c
>> @@ -48,6 +48,8 @@
>>   #define CDNS_MCP_SSPSTAT            0xC
>>   #define CDNS_MCP_FRAME_SHAPE            0x10
>>   #define CDNS_MCP_FRAME_SHAPE_INIT        0x14
>> +#define CDNS_MCP_FRAME_SHAPE_COL_MASK        GENMASK(2, 0)
>> +#define CDNS_MCP_FRAME_SHAPE_ROW_OFFSET        3
>>   #define CDNS_MCP_CONFIG_UPDATE            0x18
>>   #define CDNS_MCP_CONFIG_UPDATE_BIT        BIT(0)
>> @@ -175,7 +177,6 @@
>>   /* Driver defaults */
>>   #define CDNS_DEFAULT_CLK_DIVIDER        0
>> -#define CDNS_DEFAULT_FRAME_SHAPE        0x30
>>   #define CDNS_DEFAULT_SSP_INTERVAL        0x18
>>   #define CDNS_TX_TIMEOUT                2000
>> @@ -901,6 +902,20 @@ int sdw_cdns_pdi_init(struct sdw_cdns *cdns,
>>   }
>>   EXPORT_SYMBOL(sdw_cdns_pdi_init);
>> +static u32 cdns_set_initial_frame_shape(int n_rows, int n_cols)
>> +{
>> +    u32 val;
>> +    int c;
>> +    int r;
>> +
>> +    r = sdw_find_row_index(n_rows);
>> +    c = sdw_find_col_index(n_cols) & CDNS_MCP_FRAME_SHAPE_COL_MASK;
>> +
>> +    val = (r << CDNS_MCP_FRAME_SHAPE_ROW_OFFSET) | c;
>> +
>> +    return val;
>> +}
>> +
> 
> Guess this have been said already, but this function could be simplified 
> - unless you really want to keep explicit variable declaration. Both "c" 
> and "r" declarations could be merged into single line while "val" is not 
> needed at all.
> 
> One more thing - is AND bitwise op really needed for cols explicitly? We 
> know all col values upfront - these are static and declared in global 
> table nearby. Static declaration takes care of "initial range-check". Is 
> another one necessary?
> 
> Moreover, this is a _get_ and certainly not a _set_ type of function. 
> I'd even consider renaming it to: "cdns_get_frame_shape" as this is 
> neither a _set_ nor an explicit initial frame shape setter.
> 
> It might be even helpful to split two usages:
> 
> #define sdw_frame_shape(col_idx, row_idx) \
>      ((row_idx << CDNS_MCP_FRAME_SHAPE_ROW_OFFSET) | col_idx)
> 
> u32 cdns_get_frame_shape(u16 rows, u16 cols)
> {
>      u16 c, r;
> 
>      r = sdw_find_row_index(rows);
>      c = sdw_find_col_index(cols);
> 
>      return sdw_frame_shape(c, r);
> }
> 
> The above may even be simplified into one-liner.

This is a function used once on startup, there is no real need to 
simplify further. The separate variables help add debug traces as needed 
and keep the code readable while showing how the values are encoded into 
a register.

Re: [alsa-devel] [PATCH 06/17] soundwire: cadence_master: use firmware defaults for frame shape

[Cezary Rojewski]

On 2019-08-06 17:36, Pierre-Louis Bossart wrote:
> 
> 
> On 8/6/19 10:27 AM, Cezary Rojewski wrote:
>> On 2019-08-06 02:55, Pierre-Louis Bossart wrote:
>>> diff --git a/drivers/soundwire/cadence_master.c 
>>> b/drivers/soundwire/cadence_master.c
>>> index 5d9729b4d634..89c55e4bb72c 100644
>>> --- a/drivers/soundwire/cadence_master.c
>>> +++ b/drivers/soundwire/cadence_master.c
>>> @@ -48,6 +48,8 @@
>>>   #define CDNS_MCP_SSPSTAT            0xC
>>>   #define CDNS_MCP_FRAME_SHAPE            0x10
>>>   #define CDNS_MCP_FRAME_SHAPE_INIT        0x14
>>> +#define CDNS_MCP_FRAME_SHAPE_COL_MASK        GENMASK(2, 0)
>>> +#define CDNS_MCP_FRAME_SHAPE_ROW_OFFSET        3
>>>   #define CDNS_MCP_CONFIG_UPDATE            0x18
>>>   #define CDNS_MCP_CONFIG_UPDATE_BIT        BIT(0)
>>> @@ -175,7 +177,6 @@
>>>   /* Driver defaults */
>>>   #define CDNS_DEFAULT_CLK_DIVIDER        0
>>> -#define CDNS_DEFAULT_FRAME_SHAPE        0x30
>>>   #define CDNS_DEFAULT_SSP_INTERVAL        0x18
>>>   #define CDNS_TX_TIMEOUT                2000
>>> @@ -901,6 +902,20 @@ int sdw_cdns_pdi_init(struct sdw_cdns *cdns,
>>>   }
>>>   EXPORT_SYMBOL(sdw_cdns_pdi_init);
>>> +static u32 cdns_set_initial_frame_shape(int n_rows, int n_cols)
>>> +{
>>> +    u32 val;
>>> +    int c;
>>> +    int r;
>>> +
>>> +    r = sdw_find_row_index(n_rows);
>>> +    c = sdw_find_col_index(n_cols) & CDNS_MCP_FRAME_SHAPE_COL_MASK;
>>> +
>>> +    val = (r << CDNS_MCP_FRAME_SHAPE_ROW_OFFSET) | c;
>>> +
>>> +    return val;
>>> +}
>>> +
>>
>> Guess this have been said already, but this function could be 
>> simplified - unless you really want to keep explicit variable 
>> declaration. Both "c" and "r" declarations could be merged into single 
>> line while "val" is not needed at all.
>>
>> One more thing - is AND bitwise op really needed for cols explicitly? 
>> We know all col values upfront - these are static and declared in 
>> global table nearby. Static declaration takes care of "initial 
>> range-check". Is another one necessary?
>>
>> Moreover, this is a _get_ and certainly not a _set_ type of function. 
>> I'd even consider renaming it to: "cdns_get_frame_shape" as this is 
>> neither a _set_ nor an explicit initial frame shape setter.
>>
>> It might be even helpful to split two usages:
>>
>> #define sdw_frame_shape(col_idx, row_idx) \
>>      ((row_idx << CDNS_MCP_FRAME_SHAPE_ROW_OFFSET) | col_idx)
>>
>> u32 cdns_get_frame_shape(u16 rows, u16 cols)
>> {
>>      u16 c, r;
>>
>>      r = sdw_find_row_index(rows);
>>      c = sdw_find_col_index(cols);
>>
>>      return sdw_frame_shape(c, r);
>> }
>>
>> The above may even be simplified into one-liner.
> 
> This is a function used once on startup, there is no real need to 
> simplify further. The separate variables help add debug traces as needed 
> and keep the code readable while showing how the values are encoded into 
> a register.

Eh, I've thought it's gonna be exposed to userspace (via uapi) so it can 
be fetched by tests or tools.

In such case - if there is a single usage only - guess function is fine 
as is.

Re: [alsa-devel] [PATCH 06/17] soundwire: cadence_master: use firmware defaults for frame shape

[Vinod Koul]

On 06-08-19, 18:06, Cezary Rojewski wrote:
> On 2019-08-06 17:36, Pierre-Louis Bossart wrote:
> > 
> > 
> > On 8/6/19 10:27 AM, Cezary Rojewski wrote:
> > > On 2019-08-06 02:55, Pierre-Louis Bossart wrote:
> > > > diff --git a/drivers/soundwire/cadence_master.c
> > > > b/drivers/soundwire/cadence_master.c
> > > > index 5d9729b4d634..89c55e4bb72c 100644
> > > > --- a/drivers/soundwire/cadence_master.c
> > > > +++ b/drivers/soundwire/cadence_master.c
> > > > @@ -48,6 +48,8 @@
> > > >   #define CDNS_MCP_SSPSTAT            0xC
> > > >   #define CDNS_MCP_FRAME_SHAPE            0x10
> > > >   #define CDNS_MCP_FRAME_SHAPE_INIT        0x14
> > > > +#define CDNS_MCP_FRAME_SHAPE_COL_MASK        GENMASK(2, 0)
> > > > +#define CDNS_MCP_FRAME_SHAPE_ROW_OFFSET        3
> > > >   #define CDNS_MCP_CONFIG_UPDATE            0x18
> > > >   #define CDNS_MCP_CONFIG_UPDATE_BIT        BIT(0)
> > > > @@ -175,7 +177,6 @@
> > > >   /* Driver defaults */
> > > >   #define CDNS_DEFAULT_CLK_DIVIDER        0
> > > > -#define CDNS_DEFAULT_FRAME_SHAPE        0x30
> > > >   #define CDNS_DEFAULT_SSP_INTERVAL        0x18
> > > >   #define CDNS_TX_TIMEOUT                2000
> > > > @@ -901,6 +902,20 @@ int sdw_cdns_pdi_init(struct sdw_cdns *cdns,
> > > >   }
> > > >   EXPORT_SYMBOL(sdw_cdns_pdi_init);
> > > > +static u32 cdns_set_initial_frame_shape(int n_rows, int n_cols)
> > > > +{
> > > > +    u32 val;
> > > > +    int c;
> > > > +    int r;
> > > > +
> > > > +    r = sdw_find_row_index(n_rows);
> > > > +    c = sdw_find_col_index(n_cols) & CDNS_MCP_FRAME_SHAPE_COL_MASK;
> > > > +
> > > > +    val = (r << CDNS_MCP_FRAME_SHAPE_ROW_OFFSET) | c;
> > > > +
> > > > +    return val;
> > > > +}
> > > > +
> > > 
> > > Guess this have been said already, but this function could be
> > > simplified - unless you really want to keep explicit variable
> > > declaration. Both "c" and "r" declarations could be merged into
> > > single line while "val" is not needed at all.
> > > 
> > > One more thing - is AND bitwise op really needed for cols
> > > explicitly? We know all col values upfront - these are static and
> > > declared in global table nearby. Static declaration takes care of
> > > "initial range-check". Is another one necessary?
> > > 
> > > Moreover, this is a _get_ and certainly not a _set_ type of
> > > function. I'd even consider renaming it to: "cdns_get_frame_shape"
> > > as this is neither a _set_ nor an explicit initial frame shape
> > > setter.
> > > 
> > > It might be even helpful to split two usages:
> > > 
> > > #define sdw_frame_shape(col_idx, row_idx) \
> > >      ((row_idx << CDNS_MCP_FRAME_SHAPE_ROW_OFFSET) | col_idx)
> > > 
> > > u32 cdns_get_frame_shape(u16 rows, u16 cols)
> > > {
> > >      u16 c, r;
> > > 
> > >      r = sdw_find_row_index(rows);
> > >      c = sdw_find_col_index(cols);
> > > 
> > >      return sdw_frame_shape(c, r);
> > > }
> > > 
> > > The above may even be simplified into one-liner.
> > 
> > This is a function used once on startup, there is no real need to
> > simplify further. The separate variables help add debug traces as needed
> > and keep the code readable while showing how the values are encoded into
> > a register.
> 
> Eh, I've thought it's gonna be exposed to userspace (via uapi) so it can be
> fetched by tests or tools.

Uapi? I dont see anything in this or other series posted, did I miss
something? Also I am not sure I like the idea of exposing these to
userland!

> 
> In such case - if there is a single usage only - guess function is fine as
> is.

-- 
~Vinod

Re: [alsa-devel] [PATCH 06/17] soundwire: cadence_master: use firmware defaults for frame shape

[Pierre-Louis Bossart]

>>>>> +static u32 cdns_set_initial_frame_shape(int n_rows, int n_cols)
>>>>> +{
>>>>> +    u32 val;
>>>>> +    int c;
>>>>> +    int r;
>>>>> +
>>>>> +    r = sdw_find_row_index(n_rows);
>>>>> +    c = sdw_find_col_index(n_cols) & CDNS_MCP_FRAME_SHAPE_COL_MASK;
>>>>> +
>>>>> +    val = (r << CDNS_MCP_FRAME_SHAPE_ROW_OFFSET) | c;
>>>>> +
>>>>> +    return val;
>>>>> +}
>>>>> +
>>>>
>>>> Guess this have been said already, but this function could be
>>>> simplified - unless you really want to keep explicit variable
>>>> declaration. Both "c" and "r" declarations could be merged into
>>>> single line while "val" is not needed at all.
>>>>
>>>> One more thing - is AND bitwise op really needed for cols
>>>> explicitly? We know all col values upfront - these are static and
>>>> declared in global table nearby. Static declaration takes care of
>>>> "initial range-check". Is another one necessary?
>>>>
>>>> Moreover, this is a _get_ and certainly not a _set_ type of
>>>> function. I'd even consider renaming it to: "cdns_get_frame_shape"
>>>> as this is neither a _set_ nor an explicit initial frame shape
>>>> setter.
>>>>
>>>> It might be even helpful to split two usages:
>>>>
>>>> #define sdw_frame_shape(col_idx, row_idx) \
>>>>       ((row_idx << CDNS_MCP_FRAME_SHAPE_ROW_OFFSET) | col_idx)
>>>>
>>>> u32 cdns_get_frame_shape(u16 rows, u16 cols)
>>>> {
>>>>       u16 c, r;
>>>>
>>>>       r = sdw_find_row_index(rows);
>>>>       c = sdw_find_col_index(cols);
>>>>
>>>>       return sdw_frame_shape(c, r);
>>>> }
>>>>
>>>> The above may even be simplified into one-liner.
>>>
>>> This is a function used once on startup, there is no real need to
>>> simplify further. The separate variables help add debug traces as needed
>>> and keep the code readable while showing how the values are encoded into
>>> a register.
>>
>> Eh, I've thought it's gonna be exposed to userspace (via uapi) so it can be
>> fetched by tests or tools.
> 
> Uapi? I dont see anything in this or other series posted, did I miss
> something? Also I am not sure I like the idea of exposing these to
> userland!

Vinod, that was never the intent, and Cezary agreed, see following line

> 
>>
>> In such case - if there is a single usage only - guess function is fine as
>> is.
> 

[PATCH 07/17] soundwire: include mod_devicetable.h to avoid compiling warnings

[Pierre-Louis Bossart]

From: Bard liao <yung-chuan.liao@linux.intel.com>

When integrating SoundWire, kbuild throws this warning with randconfig:

>> include/linux/soundwire/sdw.h:571:17: warning: 'struct
   sdw_device_id' declared inside parameter list will not be visible
   outside of this definition or declaration

       const struct sdw_device_id *id);
                    ^~~~~~~~~~~~~

Fix by adding the relevant include

Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Bard liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 include/linux/soundwire/sdw.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index a49028e9d666..31d1e8acf25b 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -4,6 +4,8 @@
 #ifndef __SOUNDWIRE_H
 #define __SOUNDWIRE_H
 
+#include <linux/mod_devicetable.h>
+
 struct sdw_bus;
 struct sdw_slave;
 
-- 
2.20.1

[PATCH 08/17] soundwire: stream: fix disable sequence

[Pierre-Louis Bossart]

When we disable the stream and then call hw_free, two bank switches
will be handled and as a result we re-enable the stream on hw_free.

Make sure the stream is disabled on both banks.

TODO: we need to completely revisit all this and make sure we have a
mirroring mechanism between current and alternate banks.

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

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 53f5e790fcd7..75b9ad1fb1a6 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1637,7 +1637,24 @@ static int _sdw_disable_stream(struct sdw_stream_runtime *stream)
 		}
 	}
 
-	return do_bank_switch(stream);
+	ret = do_bank_switch(stream);
+	if (ret < 0) {
+		dev_err(bus->dev, "Bank switch failed: %d\n", ret);
+		return ret;
+	}
+
+	/* make sure alternate bank (previous current) is also disabled */
+	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
+		bus = m_rt->bus;
+		/* Disable port(s) */
+		ret = sdw_enable_disable_ports(m_rt, false);
+		if (ret < 0) {
+			dev_err(bus->dev, "Disable port(s) failed: %d\n", ret);
+			return ret;
+		}
+	}
+
+	return 0;
 }
 
 /**
-- 
2.20.1

[PATCH 09/17] soundwire: stream: remove unnecessary variable initializations

[Pierre-Louis Bossart]

A number of variables don't need to be initialized.

In a couple of cases where we loop on a list of runtimes, the code
handling of the 'bus' variable leads to warnings that it may not be
initialized. Add a specific error case to trap such cases.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 drivers/soundwire/stream.c | 64 ++++++++++++++++++++++----------------
 1 file changed, 38 insertions(+), 26 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 75b9ad1fb1a6..8d6c13528b68 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -369,7 +369,7 @@ static int sdw_enable_disable_master_ports(struct sdw_master_runtime *m_rt,
 static int sdw_enable_disable_ports(struct sdw_master_runtime *m_rt, bool en)
 {
 	struct sdw_port_runtime *s_port, *m_port;
-	struct sdw_slave_runtime *s_rt = NULL;
+	struct sdw_slave_runtime *s_rt;
 	int ret = 0;
 
 	/* Enable/Disable Slave port(s) */
@@ -417,7 +417,7 @@ static int sdw_prep_deprep_slave_ports(struct sdw_bus *bus,
 				       struct sdw_port_runtime *p_rt,
 				       bool prep)
 {
-	struct completion *port_ready = NULL;
+	struct completion *port_ready;
 	struct sdw_dpn_prop *dpn_prop;
 	struct sdw_prepare_ch prep_ch;
 	unsigned int time_left;
@@ -537,7 +537,7 @@ static int sdw_prep_deprep_master_ports(struct sdw_master_runtime *m_rt,
  */
 static int sdw_prep_deprep_ports(struct sdw_master_runtime *m_rt, bool prep)
 {
-	struct sdw_slave_runtime *s_rt = NULL;
+	struct sdw_slave_runtime *s_rt;
 	struct sdw_port_runtime *p_rt;
 	int ret = 0;
 
@@ -605,7 +605,7 @@ static int sdw_notify_config(struct sdw_master_runtime *m_rt)
  */
 static int sdw_program_params(struct sdw_bus *bus)
 {
-	struct sdw_master_runtime *m_rt = NULL;
+	struct sdw_master_runtime *m_rt;
 	int ret = 0;
 
 	list_for_each_entry(m_rt, &bus->m_rt_list, bus_node) {
@@ -642,8 +642,8 @@ static int sdw_bank_switch(struct sdw_bus *bus, int m_rt_count)
 	int col_index, row_index;
 	bool multi_link;
 	struct sdw_msg *wr_msg;
-	u8 *wbuf = NULL;
-	int ret = 0;
+	u8 *wbuf;
+	int ret;
 	u16 addr;
 
 	wr_msg = kzalloc(sizeof(*wr_msg), GFP_KERNEL);
@@ -741,9 +741,9 @@ static int sdw_ml_sync_bank_switch(struct sdw_bus *bus)
 
 static int do_bank_switch(struct sdw_stream_runtime *stream)
 {
-	struct sdw_master_runtime *m_rt = NULL;
+	struct sdw_master_runtime *m_rt;
 	const struct sdw_master_ops *ops;
-	struct sdw_bus *bus = NULL;
+	struct sdw_bus *bus;
 	bool multi_link = false;
 	int ret = 0;
 
@@ -886,7 +886,7 @@ static struct sdw_master_runtime
 *sdw_find_master_rt(struct sdw_bus *bus,
 		    struct sdw_stream_runtime *stream)
 {
-	struct sdw_master_runtime *m_rt = NULL;
+	struct sdw_master_runtime *m_rt;
 
 	/* Retrieve Bus handle if already available */
 	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
@@ -955,7 +955,7 @@ static struct sdw_slave_runtime
 		    struct sdw_stream_config *stream_config,
 		    struct sdw_stream_runtime *stream)
 {
-	struct sdw_slave_runtime *s_rt = NULL;
+	struct sdw_slave_runtime *s_rt;
 
 	s_rt = kzalloc(sizeof(*s_rt), GFP_KERNEL);
 	if (!s_rt)
@@ -1261,7 +1261,7 @@ int sdw_stream_add_master(struct sdw_bus *bus,
 			  unsigned int num_ports,
 			  struct sdw_stream_runtime *stream)
 {
-	struct sdw_master_runtime *m_rt = NULL;
+	struct sdw_master_runtime *m_rt;
 	int ret;
 
 	mutex_lock(&bus->bus_lock);
@@ -1428,7 +1428,7 @@ struct sdw_dpn_prop *sdw_get_slave_dpn_prop(struct sdw_slave *slave,
  */
 static void sdw_acquire_bus_lock(struct sdw_stream_runtime *stream)
 {
-	struct sdw_master_runtime *m_rt = NULL;
+	struct sdw_master_runtime *m_rt;
 	struct sdw_bus *bus = NULL;
 
 	/* Iterate for all Master(s) in Master list */
@@ -1462,9 +1462,9 @@ static void sdw_release_bus_lock(struct sdw_stream_runtime *stream)
 
 static int _sdw_prepare_stream(struct sdw_stream_runtime *stream)
 {
-	struct sdw_master_runtime *m_rt = NULL;
+	struct sdw_master_runtime *m_rt;
 	struct sdw_bus *bus = NULL;
-	struct sdw_master_prop *prop = NULL;
+	struct sdw_master_prop *prop;
 	struct sdw_bus_params params;
 	int ret;
 
@@ -1493,6 +1493,11 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream)
 		}
 	}
 
+	if (!bus) {
+		pr_err("Configuration error in %s\n", __func__);
+		return -EINVAL;
+	}
+
 	ret = do_bank_switch(stream);
 	if (ret < 0) {
 		dev_err(bus->dev, "Bank switch failed: %d\n", ret);
@@ -1549,7 +1554,7 @@ EXPORT_SYMBOL(sdw_prepare_stream);
 
 static int _sdw_enable_stream(struct sdw_stream_runtime *stream)
 {
-	struct sdw_master_runtime *m_rt = NULL;
+	struct sdw_master_runtime *m_rt;
 	struct sdw_bus *bus = NULL;
 	int ret;
 
@@ -1573,6 +1578,11 @@ static int _sdw_enable_stream(struct sdw_stream_runtime *stream)
 		}
 	}
 
+	if (!bus) {
+		pr_err("Configuration error in %s\n", __func__);
+		return -EINVAL;
+	}
+
 	ret = do_bank_switch(stream);
 	if (ret < 0) {
 		dev_err(bus->dev, "Bank switch failed: %d\n", ret);
@@ -1592,7 +1602,7 @@ static int _sdw_enable_stream(struct sdw_stream_runtime *stream)
  */
 int sdw_enable_stream(struct sdw_stream_runtime *stream)
 {
-	int ret = 0;
+	int ret;
 
 	if (!stream) {
 		pr_err("SoundWire: Handle not found for stream\n");
@@ -1612,12 +1622,12 @@ EXPORT_SYMBOL(sdw_enable_stream);
 
 static int _sdw_disable_stream(struct sdw_stream_runtime *stream)
 {
-	struct sdw_master_runtime *m_rt = NULL;
-	struct sdw_bus *bus = NULL;
+	struct sdw_master_runtime *m_rt;
 	int ret;
 
 	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
-		bus = m_rt->bus;
+		struct sdw_bus *bus = m_rt->bus;
+
 		/* Disable port(s) */
 		ret = sdw_enable_disable_ports(m_rt, false);
 		if (ret < 0) {
@@ -1628,7 +1638,8 @@ static int _sdw_disable_stream(struct sdw_stream_runtime *stream)
 	stream->state = SDW_STREAM_DISABLED;
 
 	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
-		bus = m_rt->bus;
+		struct sdw_bus *bus = m_rt->bus;
+
 		/* Program params */
 		ret = sdw_program_params(bus);
 		if (ret < 0) {
@@ -1639,13 +1650,14 @@ static int _sdw_disable_stream(struct sdw_stream_runtime *stream)
 
 	ret = do_bank_switch(stream);
 	if (ret < 0) {
-		dev_err(bus->dev, "Bank switch failed: %d\n", ret);
+		pr_err("Bank switch failed: %d\n", ret);
 		return ret;
 	}
 
 	/* make sure alternate bank (previous current) is also disabled */
 	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
-		bus = m_rt->bus;
+		struct sdw_bus *bus = m_rt->bus;
+
 		/* Disable port(s) */
 		ret = sdw_enable_disable_ports(m_rt, false);
 		if (ret < 0) {
@@ -1666,7 +1678,7 @@ static int _sdw_disable_stream(struct sdw_stream_runtime *stream)
  */
 int sdw_disable_stream(struct sdw_stream_runtime *stream)
 {
-	int ret = 0;
+	int ret;
 
 	if (!stream) {
 		pr_err("SoundWire: Handle not found for stream\n");
@@ -1686,8 +1698,8 @@ EXPORT_SYMBOL(sdw_disable_stream);
 
 static int _sdw_deprepare_stream(struct sdw_stream_runtime *stream)
 {
-	struct sdw_master_runtime *m_rt = NULL;
-	struct sdw_bus *bus = NULL;
+	struct sdw_master_runtime *m_rt;
+	struct sdw_bus *bus;
 	int ret = 0;
 
 	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
@@ -1725,7 +1737,7 @@ static int _sdw_deprepare_stream(struct sdw_stream_runtime *stream)
  */
 int sdw_deprepare_stream(struct sdw_stream_runtime *stream)
 {
-	int ret = 0;
+	int ret;
 
 	if (!stream) {
 		pr_err("SoundWire: Handle not found for stream\n");
-- 
2.20.1

Re: [PATCH 09/17] soundwire: stream: remove unnecessary variable initializations

[Cezary Rojewski]

On 2019-08-06 02:55, Pierre-Louis Bossart wrote:
> @@ -1493,6 +1493,11 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream)
>   		}
>   	}
>   
> +	if (!bus) {
> +		pr_err("Configuration error in %s\n", __func__);
> +		return -EINVAL;
> +	}
> +

This should probably be located in separate path - not at all an 
initialization removal.

> @@ -1573,6 +1578,11 @@ static int _sdw_enable_stream(struct sdw_stream_runtime *stream)
>   		}
>   	}
>   
> +	if (!bus) {
> +		pr_err("Configuration error in %s\n", __func__);
> +		return -EINVAL;
> +	}
> +

Same here.

> @@ -1639,13 +1650,14 @@ static int _sdw_disable_stream(struct sdw_stream_runtime *stream)
>   
>   	ret = do_bank_switch(stream);
>   	if (ret < 0) {
> -		dev_err(bus->dev, "Bank switch failed: %d\n", ret);
> +		pr_err("Bank switch failed: %d\n", ret);
>   		return ret;
>   	}

Here too.

I might have missed something though I bet you got my point.

Re: [PATCH 09/17] soundwire: stream: remove unnecessary variable initializations

[Pierre-Louis Bossart]

On 8/6/19 10:31 AM, Cezary Rojewski wrote:
> On 2019-08-06 02:55, Pierre-Louis Bossart wrote:
>> @@ -1493,6 +1493,11 @@ static int _sdw_prepare_stream(struct 
>> sdw_stream_runtime *stream)
>>           }
>>       }
>> +    if (!bus) {
>> +        pr_err("Configuration error in %s\n", __func__);
>> +        return -EINVAL;
>> +    }
>> +
> 
> This should probably be located in separate path - not at all an 
> initialization removal.

It's a consequence of the initialization removal: because we are 
removing the default init, there is a risk that the loop just before do 
not set it, so it's required to trap the case where the variable in not 
initialized.

> 
>> @@ -1573,6 +1578,11 @@ static int _sdw_enable_stream(struct 
>> sdw_stream_runtime *stream)
>>           }
>>       }
>> +    if (!bus) {
>> +        pr_err("Configuration error in %s\n", __func__);
>> +        return -EINVAL;
>> +    }
>> +
> 
> Same here.

same reply

> 
>> @@ -1639,13 +1650,14 @@ static int _sdw_disable_stream(struct 
>> sdw_stream_runtime *stream)
>>       ret = do_bank_switch(stream);
>>       if (ret < 0) {
>> -        dev_err(bus->dev, "Bank switch failed: %d\n", ret);
>> +        pr_err("Bank switch failed: %d\n", ret);
>>           return ret;
>>       }
> 
> Here too.

no, same thing, the bus variable is initialized in loops so tools will 
report a possible path where bus->dev is an invalid dereference.

> I might have missed something though I bet you got my point.

[PATCH 10/17] soundwire: add new mclk_freq field for properties

[Pierre-Louis Bossart]

To help pass platform-specific values, add a new field that can either
be set by the Master driver or read from firmware (BIOS/DT).

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 include/linux/soundwire/sdw.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index 31d1e8acf25b..b6acc436ac80 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -379,6 +379,7 @@ struct sdw_slave_prop {
  * @dynamic_frame: Dynamic frame shape supported
  * @err_threshold: Number of times that software may retry sending a single
  * command
+ * @mclk_freq: clock reference passed to SoundWire Master, in Hz.
  */
 struct sdw_master_prop {
 	u32 revision;
@@ -393,6 +394,7 @@ struct sdw_master_prop {
 	u32 default_col;
 	bool dynamic_frame;
 	u32 err_threshold;
+	u32 mclk_freq;
 };
 
 int sdw_master_read_prop(struct sdw_bus *bus);
-- 
2.20.1

[PATCH 11/17] soundwire: intel: read mclk_freq property from firmware

[Pierre-Louis Bossart]

The BIOS provides an Intel-specific property, let's use it to avoid
hard-coded clock dividers.

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

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index fe3266bc1d12..3f876642cb7f 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -909,11 +909,37 @@ static int intel_register_dai(struct sdw_intel *sdw)
 					  dais, num_dai);
 }
 
+static int sdw_master_read_intel_prop(struct sdw_bus *bus)
+{
+	struct sdw_master_prop *prop = &bus->prop;
+	struct fwnode_handle *link;
+	char name[32];
+	int nval, i;
+
+	/* Find master handle */
+	snprintf(name, sizeof(name),
+		 "mipi-sdw-link-%d-subproperties", bus->link_id);
+
+	link = device_get_named_child_node(bus->dev, name);
+	if (!link) {
+		dev_err(bus->dev, "Master node %s not found\n", name);
+		return -EIO;
+	}
+
+	fwnode_property_read_u32(link,
+				 "intel-sdw-ip-clock",
+				 &prop->mclk_freq);
+	return 0;
+}
+
 static int intel_prop_read(struct sdw_bus *bus)
 {
 	/* Initialize with default handler to read all DisCo properties */
 	sdw_master_read_prop(bus);
 
+	/* read Intel-specific properties */
+	sdw_master_read_intel_prop(bus);
+
 	return 0;
 }
 
-- 
2.20.1

[PATCH 12/17] soundwire: cadence_master: make use of mclk_freq property

[Pierre-Louis Bossart]

Now that the prototype and Intel implementation are enabled, use this
property to avoid hard-coded values.

For example for ICL the mclk_freq value is 38.4 MHz while on CNL/CML
it's 24 MHz. The mclk_freq should not be confused with the
max_clk_freq, which si the maximum bus clock. The mclk_freq is
typically tied to the oscillator frequency and does not change between
platforms. The max_clk_freq value is linked to the maximum bandwidth
needed and topology/trace length.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 drivers/soundwire/cadence_master.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c
index 89c55e4bb72c..049ecfad2c00 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -175,8 +175,6 @@
 #define CDNS_PDI_CONFIG_PORT			GENMASK(4, 0)
 
 /* Driver defaults */
-
-#define CDNS_DEFAULT_CLK_DIVIDER		0
 #define CDNS_DEFAULT_SSP_INTERVAL		0x18
 #define CDNS_TX_TIMEOUT				2000
 
@@ -922,7 +920,10 @@ static u32 cdns_set_initial_frame_shape(int n_rows, int n_cols)
  */
 int sdw_cdns_init(struct sdw_cdns *cdns)
 {
+	struct sdw_bus *bus = &cdns->bus;
+	struct sdw_master_prop *prop = &bus->prop;
 	u32 val;
+	int divider;
 	int ret;
 
 	/* Exit clock stop */
@@ -934,9 +935,11 @@ int sdw_cdns_init(struct sdw_cdns *cdns)
 	}
 
 	/* Set clock divider */
+	divider	= (prop->mclk_freq / prop->max_clk_freq) - 1;
 	val = cdns_readl(cdns, CDNS_MCP_CLK_CTRL0);
-	val |= CDNS_DEFAULT_CLK_DIVIDER;
+	val |= divider;
 	cdns_writel(cdns, CDNS_MCP_CLK_CTRL0, val);
+	cdns_writel(cdns, CDNS_MCP_CLK_CTRL1, val);
 
 	/*
 	 * Frame shape changes after initialization have to be done
@@ -984,6 +987,7 @@ EXPORT_SYMBOL(sdw_cdns_init);
 
 int cdns_bus_conf(struct sdw_bus *bus, struct sdw_bus_params *params)
 {
+	struct sdw_master_prop *prop = &bus->prop;
 	struct sdw_cdns *cdns = bus_to_cdns(bus);
 	int mcp_clkctrl_off, mcp_clkctrl;
 	int divider;
@@ -993,7 +997,9 @@ int cdns_bus_conf(struct sdw_bus *bus, struct sdw_bus_params *params)
 		return -EINVAL;
 	}
 
-	divider	= (params->max_dr_freq / params->curr_dr_freq) - 1;
+	divider	= prop->mclk_freq * SDW_DOUBLE_RATE_FACTOR /
+		params->curr_dr_freq;
+	divider--; /* divider is 1/(N+1) */
 
 	if (params->next_bank)
 		mcp_clkctrl_off = CDNS_MCP_CLK_CTRL1;
-- 
2.20.1

[PATCH 13/17] soundwire: cadence_master: fix divider setting in clock register

[Pierre-Louis Bossart]

From: Rander Wang <rander.wang@linux.intel.com>

The existing code uses an OR operation which would mix the original
divider setting with the new one, resulting in an invalid
configuration that can make codecs hang.

Add the mask definition and use cdns_updatel to update divider

Signed-off-by: Rander Wang <rander.wang@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 drivers/soundwire/cadence_master.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c
index 049ecfad2c00..fb198a806efd 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -59,6 +59,7 @@
 #define CDNS_MCP_SSP_CTRL1			0x28
 #define CDNS_MCP_CLK_CTRL0			0x30
 #define CDNS_MCP_CLK_CTRL1			0x38
+#define CDNS_MCP_CLK_MCLKD_MASK		GENMASK(7, 0)
 
 #define CDNS_MCP_STAT				0x40
 
@@ -936,10 +937,11 @@ int sdw_cdns_init(struct sdw_cdns *cdns)
 
 	/* Set clock divider */
 	divider	= (prop->mclk_freq / prop->max_clk_freq) - 1;
-	val = cdns_readl(cdns, CDNS_MCP_CLK_CTRL0);
-	val |= divider;
-	cdns_writel(cdns, CDNS_MCP_CLK_CTRL0, val);
-	cdns_writel(cdns, CDNS_MCP_CLK_CTRL1, val);
+
+	cdns_updatel(cdns, CDNS_MCP_CLK_CTRL0,
+		     CDNS_MCP_CLK_MCLKD_MASK, divider);
+	cdns_updatel(cdns, CDNS_MCP_CLK_CTRL1,
+		     CDNS_MCP_CLK_MCLKD_MASK, divider);
 
 	/*
 	 * Frame shape changes after initialization have to be done
@@ -989,7 +991,7 @@ int cdns_bus_conf(struct sdw_bus *bus, struct sdw_bus_params *params)
 {
 	struct sdw_master_prop *prop = &bus->prop;
 	struct sdw_cdns *cdns = bus_to_cdns(bus);
-	int mcp_clkctrl_off, mcp_clkctrl;
+	int mcp_clkctrl_off;
 	int divider;
 
 	if (!params->curr_dr_freq) {
@@ -1006,9 +1008,7 @@ int cdns_bus_conf(struct sdw_bus *bus, struct sdw_bus_params *params)
 	else
 		mcp_clkctrl_off = CDNS_MCP_CLK_CTRL0;
 
-	mcp_clkctrl = cdns_readl(cdns, mcp_clkctrl_off);
-	mcp_clkctrl |= divider;
-	cdns_writel(cdns, mcp_clkctrl_off, mcp_clkctrl);
+	cdns_updatel(cdns, mcp_clkctrl_off, CDNS_MCP_CLK_MCLKD_MASK, divider);
 
 	return 0;
 }
-- 
2.20.1

[PATCH 14/17] soundwire: intel: handle disabled links

[Pierre-Louis Bossart]

On most hardware platforms, SoundWire interfaces are pin-muxed with
other interfaces (typically DMIC or I2S) and the status of each link
needs to be checked at boot time.

For Intel platforms, the BIOS provides a menu to enable/disable the
links separately, and the information is provided to the OS with an
Intel-specific _DSD property. The same capability will be added to
revisions of the MIPI DisCo specification.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 drivers/soundwire/intel.c     | 26 ++++++++++++++++++++++----
 include/linux/soundwire/sdw.h |  2 ++
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 3f876642cb7f..e702187c0609 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -90,6 +90,8 @@
 #define SDW_ALH_STRMZCFG_DMAT		GENMASK(7, 0)
 #define SDW_ALH_STRMZCFG_CHN		GENMASK(19, 16)
 
+#define SDW_INTEL_QUIRK_MASK_BUS_DISABLE	BIT(1)
+
 enum intel_pdi_type {
 	INTEL_PDI_IN = 0,
 	INTEL_PDI_OUT = 1,
@@ -914,7 +916,7 @@ static int sdw_master_read_intel_prop(struct sdw_bus *bus)
 	struct sdw_master_prop *prop = &bus->prop;
 	struct fwnode_handle *link;
 	char name[32];
-	int nval, i;
+	u32 quirk_mask;
 
 	/* Find master handle */
 	snprintf(name, sizeof(name),
@@ -929,6 +931,14 @@ static int sdw_master_read_intel_prop(struct sdw_bus *bus)
 	fwnode_property_read_u32(link,
 				 "intel-sdw-ip-clock",
 				 &prop->mclk_freq);
+
+	fwnode_property_read_u32(link,
+				 "intel-quirk-mask",
+				 &quirk_mask);
+
+	if (quirk_mask & SDW_INTEL_QUIRK_MASK_BUS_DISABLE)
+		prop->hw_disabled = true;
+
 	return 0;
 }
 
@@ -989,6 +999,12 @@ static int intel_probe(struct platform_device *pdev)
 		goto err_master_reg;
 	}
 
+	if (sdw->cdns.bus.prop.hw_disabled) {
+		dev_info(&pdev->dev, "SoundWire master %d is disabled, ignoring\n",
+			 sdw->cdns.bus.link_id);
+		return 0;
+	}
+
 	/* Initialize shim and controller */
 	intel_link_power_up(sdw);
 	intel_shim_init(sdw);
@@ -1042,9 +1058,11 @@ static int intel_remove(struct platform_device *pdev)
 
 	sdw = platform_get_drvdata(pdev);
 
-	intel_debugfs_exit(sdw);
-	free_irq(sdw->res->irq, sdw);
-	snd_soc_unregister_component(sdw->cdns.dev);
+	if (!sdw->cdns.bus.prop.hw_disabled) {
+		intel_debugfs_exit(sdw);
+		free_irq(sdw->res->irq, sdw);
+		snd_soc_unregister_component(sdw->cdns.dev);
+	}
 	sdw_delete_bus_master(&sdw->cdns.bus);
 
 	return 0;
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index b6acc436ac80..a3c7448d5817 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -380,6 +380,7 @@ struct sdw_slave_prop {
  * @err_threshold: Number of times that software may retry sending a single
  * command
  * @mclk_freq: clock reference passed to SoundWire Master, in Hz.
+ * @hw_disabled: if true, the Master is not functional, typically due to pin-mux
  */
 struct sdw_master_prop {
 	u32 revision;
@@ -395,6 +396,7 @@ struct sdw_master_prop {
 	bool dynamic_frame;
 	u32 err_threshold;
 	u32 mclk_freq;
+	bool hw_disabled;
 };
 
 int sdw_master_read_prop(struct sdw_bus *bus);
-- 
2.20.1

[PATCH 15/17] soundwire: intel_init: add kernel module parameter to filter out links

[Pierre-Louis Bossart]

The hardware and ACPI info may report the presence of links that are
not physically enabled (e.g. due to pin-muxing or hardware reworks),
which in turn can result in errors being thrown. This shouldn't be the
case for production devices but will happen a lot on development
devices - even more so when they expose a connector.

Even when the ACPI information is correct, it's useful to be able to
only enable the links that need attention - mostly to filter out
dynamic debug messages.

Add a module parameter to filter out such links, e.g. adding the
following config to a file in /etc/modprobe.d will select the second
and third links only.

options soundwire_intel_init sdw_link_mask=0x6

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

diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c
index 70637a0383d2..b74c2f144962 100644
--- a/drivers/soundwire/intel_init.c
+++ b/drivers/soundwire/intel_init.c
@@ -22,6 +22,10 @@
 #define SDW_LINK_BASE		0x30000
 #define SDW_LINK_SIZE		0x10000
 
+static int link_mask;
+module_param_named(sdw_link_mask, link_mask, int, 0444);
+MODULE_PARM_DESC(sdw_link_mask, "Intel link mask (one bit per link)");
+
 struct sdw_link_data {
 	struct sdw_intel_link_res res;
 	struct platform_device *pdev;
@@ -111,6 +115,13 @@ static struct sdw_intel_ctx
 
 	/* Create SDW Master devices */
 	for (i = 0; i < count; i++) {
+		if (link_mask && !(link_mask & BIT(i))) {
+			dev_dbg(&adev->dev,
+				"Link %d masked, will not be enabled\n", i);
+			link++;
+			continue;
+		}
+
 		link->res.irq = res->irq;
 		link->res.registers = res->mmio_base + SDW_LINK_BASE
 					+ (SDW_LINK_SIZE * i);
-- 
2.20.1

[PATCH 16/17] soundwire: cadence_master: add kernel parameter to override interrupt mask

[Pierre-Louis Bossart]

The code has a set of defaults which may not be relevant in all cases,
add kernel parameter as a helper - mostly for early board bring-up.

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

diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c
index fb198a806efd..4ab3174cbb04 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -20,6 +20,10 @@
 #include "bus.h"
 #include "cadence_master.h"
 
+static int interrupt_mask;
+module_param_named(cnds_mcp_int_mask, interrupt_mask, int, 0444);
+MODULE_PARM_DESC(cdns_mcp_int_mask, "Cadence MCP IntMask");
+
 #define CDNS_MCP_CONFIG				0x0
 
 #define CDNS_MCP_CONFIG_MCMD_RETRY		GENMASK(27, 24)
@@ -762,6 +766,9 @@ static int _cdns_enable_interrupt(struct sdw_cdns *cdns)
 	 */
 	mask |= CDNS_MCP_INT_IRQ;
 
+	if (interrupt_mask) /* parameter override */
+		mask = interrupt_mask;
+
 	cdns_writel(cdns, CDNS_MCP_INTMASK, mask);
 
 	return 0;
-- 
2.20.1

[PATCH 17/17] soundwire: intel: move shutdown() callback and don't export symbol

[Pierre-Louis Bossart]

All DAI callbacks are in intel.c except for shutdown. Move and remove
export symbol

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 drivers/soundwire/cadence_master.c | 14 --------------
 drivers/soundwire/cadence_master.h |  2 --
 drivers/soundwire/intel.c          | 17 +++++++++++++++--
 3 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c
index 4ab3174cbb04..c5891c8a824e 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -1316,19 +1316,5 @@ 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 sdw_cdns_dma_data *dma;
-
-	dma = snd_soc_dai_get_dma_data(dai, substream);
-	if (!dma)
-		return;
-
-	snd_soc_dai_set_dma_data(dai, substream, NULL);
-	kfree(dma);
-}
-EXPORT_SYMBOL(sdw_cdns_shutdown);
-
 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_DESCRIPTION("Cadence Soundwire Library");
diff --git a/drivers/soundwire/cadence_master.h b/drivers/soundwire/cadence_master.h
index c0bf6ff00a44..d1022125f182 100644
--- a/drivers/soundwire/cadence_master.h
+++ b/drivers/soundwire/cadence_master.h
@@ -174,8 +174,6 @@ int sdw_cdns_alloc_stream(struct sdw_cdns *cdns,
 void sdw_cdns_config_stream(struct sdw_cdns *cdns, struct sdw_cdns_port *port,
 			    u32 ch, u32 dir, struct sdw_cdns_pdi *pdi);
 
-void sdw_cdns_shutdown(struct snd_pcm_substream *substream,
-		       struct snd_soc_dai *dai);
 int sdw_cdns_pcm_set_stream(struct snd_soc_dai *dai,
 			    void *stream, int direction);
 int sdw_cdns_pdm_set_stream(struct snd_soc_dai *dai,
diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index e702187c0609..fe5f21edccfc 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -764,6 +764,19 @@ intel_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
 	return ret;
 }
 
+static void intel_shutdown(struct snd_pcm_substream *substream,
+			   struct snd_soc_dai *dai)
+{
+	struct sdw_cdns_dma_data *dma;
+
+	dma = snd_soc_dai_get_dma_data(dai, substream);
+	if (!dma)
+		return;
+
+	snd_soc_dai_set_dma_data(dai, substream, NULL);
+	kfree(dma);
+}
+
 static int intel_pcm_set_sdw_stream(struct snd_soc_dai *dai,
 				    void *stream, int direction)
 {
@@ -779,14 +792,14 @@ static int intel_pdm_set_sdw_stream(struct snd_soc_dai *dai,
 static const struct snd_soc_dai_ops intel_pcm_dai_ops = {
 	.hw_params = intel_hw_params,
 	.hw_free = intel_hw_free,
-	.shutdown = sdw_cdns_shutdown,
+	.shutdown = intel_shutdown,
 	.set_sdw_stream = intel_pcm_set_sdw_stream,
 };
 
 static const struct snd_soc_dai_ops intel_pdm_dai_ops = {
 	.hw_params = intel_hw_params,
 	.hw_free = intel_hw_free,
-	.shutdown = sdw_cdns_shutdown,
+	.shutdown = intel_shutdown,
 	.set_sdw_stream = intel_pdm_set_sdw_stream,
 };
 
-- 
2.20.1

Re: [PATCH 00/17] soundwire: fixes for 5.4

[Vinod Koul]

On 05-08-19, 19:55, Pierre-Louis Bossart wrote:
> This series provides an update on the initial RFC. Debugfs and Intel
> updates will be provided in follow-up patches. The order of patches
> was changed since the RFC so detailed change logs are provided below.

Applied all except 14, which didnt apply, thanks

-- 
~Vinod

Re: [alsa-devel] [PATCH 00/17] soundwire: fixes for 5.4

[Pierre-Louis Bossart]

On 8/21/19 4:07 AM, Vinod Koul wrote:
> On 05-08-19, 19:55, Pierre-Louis Bossart wrote:
>> This series provides an update on the initial RFC. Debugfs and Intel
>> updates will be provided in follow-up patches. The order of patches
>> was changed since the RFC so detailed change logs are provided below.
> 
> Applied all except 14, which didnt apply, thanks

yes, that's a miss, there was a debugfs line that created a dependency 
with the other patches that were not applied because of the lack of 
alignment between soundwire/fixes and soundwire/next.
Will resubmit.