diff mbox series

[6/9] soundwire: cadence: use FIELD_{GET|PREP}

Message ID 20200828072101.3781956-7-vkoul@kernel.org (mailing list archive)
State New, archived
Headers show
Series soundwire: use FIELD_{GET|PREP} in subsystem | expand

Commit Message

Vinod Koul Aug. 28, 2020, 7:20 a.m. UTC 
use FIELD_{GET|PREP} in cadence driver to get/set field values instead
of open coding masks and shift operations.

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/soundwire/cadence_master.c | 53 +++++++++++++-----------------
 1 file changed, 22 insertions(+), 31 deletions(-)

Comments

Pierre-Louis Bossart Aug. 28, 2020, 6:13 p.m. UTC | #1 
> -	val = (r << CDNS_MCP_FRAME_SHAPE_ROW_OFFSET) | c;
> +	val = FIELD_PREP(CDNS_MCP_FRAME_SHAPE_ROW_OFFSET, r) | c;

Confusion between shift and mask here.

Testing a fix now (attached), but may I suggest you use the SOF 
GitHub/Travis CI directly for any updates? You'll get an answer in 30mn 
for the CML RVP w/ SoundWire.
Vinod Koul Aug. 29, 2020, 10:55 a.m. UTC | #2 
On 28-08-20, 13:13, Pierre-Louis Bossart wrote:
> 
> > -	val = (r << CDNS_MCP_FRAME_SHAPE_ROW_OFFSET) | c;
> > +	val = FIELD_PREP(CDNS_MCP_FRAME_SHAPE_ROW_OFFSET, r) | c;
> 
> Confusion between shift and mask here.

thanks, yes I had doubts on this, folder the fix

> 
> Testing a fix now (attached), but may I suggest you use the SOF
> GitHub/Travis CI directly for any updates? You'll get an answer in 30mn for
> the CML RVP w/ SoundWire.

> >From 5d0ca63bee2c6e2456195499908ecdc8a7709238 Mon Sep 17 00:00:00 2001
> From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Date: Fri, 28 Aug 2020 13:04:01 -0500
> Subject: [PATCH] fixup! soundwire: cadence: use FIELD_{GET|PREP}
> 
> Fix confusion between shift and mask.
> 
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
>  drivers/soundwire/cadence_master.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c
> index b6796aa19aa8..5dd06483c835 100644
> --- a/drivers/soundwire/cadence_master.c
> +++ b/drivers/soundwire/cadence_master.c
> @@ -58,7 +58,7 @@ MODULE_PARM_DESC(cdns_mcp_int_mask, "Cadence MCP IntMask");
>  #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_FRAME_SHAPE_ROW_MASK		GENMASK(7, 3)
>  
>  #define CDNS_MCP_CONFIG_UPDATE			0x18
>  #define CDNS_MCP_CONFIG_UPDATE_BIT		BIT(0)
> @@ -1165,9 +1165,10 @@ static u32 cdns_set_initial_frame_shape(int n_rows, int n_cols)
>  	int r;
>  
>  	r = sdw_find_row_index(n_rows);
> -	c = sdw_find_col_index(n_cols) & CDNS_MCP_FRAME_SHAPE_COL_MASK;
> +	c = sdw_find_col_index(n_cols);
>  
> -	val = FIELD_PREP(CDNS_MCP_FRAME_SHAPE_ROW_OFFSET, r) | c;
> +	val = FIELD_PREP(CDNS_MCP_FRAME_SHAPE_ROW_MASK, r) |
> +		FIELD_PREP(CDNS_MCP_FRAME_SHAPE_COL_MASK, c);
>  
>  	return val;
>  }
> -- 
> 2.25.1
>
diff mbox series

Patch

diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c
index 24eafe0aa1c3..ac9adf38ce94 100644
--- a/drivers/soundwire/cadence_master.c
+++ b/drivers/soundwire/cadence_master.c
@@ -417,8 +417,7 @@  cdns_fill_msg_resp(struct sdw_cdns *cdns,
 
 	/* fill response */
 	for (i = 0; i < count; i++)
-		msg->buf[i + offset] = cdns->response_buf[i] >>
-				SDW_REG_SHIFT(CDNS_MCP_RESP_RDATA);
+		msg->buf[i + offset] = FIELD_GET(CDNS_MCP_RESP_RDATA, cdns->response_buf[i]);
 
 	return SDW_CMD_OK;
 }
@@ -441,14 +440,15 @@  _cdns_xfer_msg(struct sdw_cdns *cdns, struct sdw_msg *msg, int cmd,
 	addr = msg->addr;
 
 	for (i = 0; i < count; i++) {
-		data = msg->dev_num << SDW_REG_SHIFT(CDNS_MCP_CMD_DEV_ADDR);
-		data |= cmd << SDW_REG_SHIFT(CDNS_MCP_CMD_COMMAND);
-		data |= addr++  << SDW_REG_SHIFT(CDNS_MCP_CMD_REG_ADDR_L);
+		data = FIELD_PREP(CDNS_MCP_CMD_DEV_ADDR, msg->dev_num);
+		data |= FIELD_PREP(CDNS_MCP_CMD_COMMAND, cmd);
+		data |= FIELD_PREP(CDNS_MCP_CMD_REG_ADDR_L, addr);
+		addr++;
 
 		if (msg->flags == SDW_MSG_FLAG_WRITE)
 			data |= msg->buf[i + offset];
 
-		data |= msg->ssp_sync << SDW_REG_SHIFT(CDNS_MCP_CMD_SSP_TAG);
+		data |= FIELD_PREP(CDNS_MCP_CMD_SSP_TAG, msg->ssp_sync);
 		cdns_writel(cdns, base, data);
 		base += CDNS_MCP_CMD_WORD_LEN;
 	}
@@ -483,12 +483,12 @@  cdns_program_scp_addr(struct sdw_cdns *cdns, struct sdw_msg *msg)
 		cdns->msg_count = CDNS_SCP_RX_FIFOLEVEL;
 	}
 
-	data[0] = msg->dev_num << SDW_REG_SHIFT(CDNS_MCP_CMD_DEV_ADDR);
-	data[0] |= 0x3 << SDW_REG_SHIFT(CDNS_MCP_CMD_COMMAND);
+	data[0] = FIELD_PREP(CDNS_MCP_CMD_DEV_ADDR, msg->dev_num);
+	data[0] |= FIELD_PREP(CDNS_MCP_CMD_COMMAND, 0x3);
 	data[1] = data[0];
 
-	data[0] |= SDW_SCP_ADDRPAGE1 << SDW_REG_SHIFT(CDNS_MCP_CMD_REG_ADDR_L);
-	data[1] |= SDW_SCP_ADDRPAGE2 << SDW_REG_SHIFT(CDNS_MCP_CMD_REG_ADDR_L);
+	data[0] |= FIELD_PREP(CDNS_MCP_CMD_REG_ADDR_L, SDW_SCP_ADDRPAGE1);
+	data[1] |= FIELD_PREP(CDNS_MCP_CMD_REG_ADDR_L, SDW_SCP_ADDRPAGE2);
 
 	data[0] |= msg->addr_page1;
 	data[1] |= msg->addr_page2;
@@ -1043,7 +1043,7 @@  static u32 cdns_set_initial_frame_shape(int n_rows, int n_cols)
 	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;
+	val = FIELD_PREP(CDNS_MCP_FRAME_SHAPE_ROW_OFFSET, r) | c;
 
 	return val;
 }
@@ -1170,12 +1170,9 @@  static int cdns_port_params(struct sdw_bus *bus,
 
 	dpn_config = cdns_readl(cdns, dpn_config_off);
 
-	dpn_config |= ((p_params->bps - 1) <<
-				SDW_REG_SHIFT(CDNS_DPN_CONFIG_WL));
-	dpn_config |= (p_params->flow_mode <<
-				SDW_REG_SHIFT(CDNS_DPN_CONFIG_PORT_FLOW));
-	dpn_config |= (p_params->data_mode <<
-				SDW_REG_SHIFT(CDNS_DPN_CONFIG_PORT_DAT));
+	dpn_config |= FIELD_PREP(CDNS_DPN_CONFIG_WL, (p_params->bps - 1));
+	dpn_config |= FIELD_PREP(CDNS_DPN_CONFIG_PORT_FLOW, p_params->flow_mode);
+	dpn_config |= FIELD_PREP(CDNS_DPN_CONFIG_PORT_DAT, p_params->data_mode);
 
 	cdns_writel(cdns, dpn_config_off, dpn_config);
 
@@ -1212,23 +1209,17 @@  static int cdns_transport_params(struct sdw_bus *bus,
 
 	dpn_config = cdns_readl(cdns, dpn_config_off);
 
-	dpn_config |= (t_params->blk_grp_ctrl <<
-				SDW_REG_SHIFT(CDNS_DPN_CONFIG_BGC));
-	dpn_config |= (t_params->blk_pkg_mode <<
-				SDW_REG_SHIFT(CDNS_DPN_CONFIG_BPM));
+	dpn_config |= FIELD_PREP(CDNS_DPN_CONFIG_BGC, t_params->blk_grp_ctrl);
+	dpn_config |= FIELD_PREP(CDNS_DPN_CONFIG_BPM, t_params->blk_pkg_mode);
 	cdns_writel(cdns, dpn_config_off, dpn_config);
 
-	dpn_offsetctrl |= (t_params->offset1 <<
-				SDW_REG_SHIFT(CDNS_DPN_OFFSET_CTRL_1));
-	dpn_offsetctrl |= (t_params->offset2 <<
-				SDW_REG_SHIFT(CDNS_DPN_OFFSET_CTRL_2));
+	dpn_offsetctrl |= FIELD_PREP(CDNS_DPN_OFFSET_CTRL_1, t_params->offset1);
+	dpn_offsetctrl |= FIELD_PREP(CDNS_DPN_OFFSET_CTRL_2, t_params->offset2);
 	cdns_writel(cdns, dpn_offsetctrl_off,  dpn_offsetctrl);
 
-	dpn_hctrl |= (t_params->hstart <<
-				SDW_REG_SHIFT(CDNS_DPN_HCTRL_HSTART));
-	dpn_hctrl |= (t_params->hstop << SDW_REG_SHIFT(CDNS_DPN_HCTRL_HSTOP));
-	dpn_hctrl |= (t_params->lane_ctrl <<
-				SDW_REG_SHIFT(CDNS_DPN_HCTRL_LCTRL));
+	dpn_hctrl |= FIELD_PREP(CDNS_DPN_HCTRL_HSTART, t_params->hstart);
+	dpn_hctrl |= FIELD_PREP(CDNS_DPN_HCTRL_HSTOP, t_params->hstop);
+	dpn_hctrl |= FIELD_PREP(CDNS_DPN_HCTRL_LCTRL, t_params->lane_ctrl);
 
 	cdns_writel(cdns, dpn_hctrl_off, dpn_hctrl);
 	cdns_writel(cdns, dpn_samplectrl_off, (t_params->sample_interval - 1));
@@ -1534,7 +1525,7 @@  void sdw_cdns_config_stream(struct sdw_cdns *cdns,
 
 	val = pdi->num;
 	val |= CDNS_PDI_CONFIG_SOFT_RESET;
-	val |= ((1 << ch) - 1) << SDW_REG_SHIFT(CDNS_PDI_CONFIG_CHANNEL);
+	val |= FIELD_PREP(CDNS_PDI_CONFIG_CHANNEL, (1 << ch) - 1);
 	cdns_writel(cdns, CDNS_PDI_CONFIG(pdi->num), val);
 }
 EXPORT_SYMBOL(sdw_cdns_config_stream);