[PATCH v2] soundwire: qcom: wait for fifo space to be available before read/write
If we write registers very fast we can endup in a situation where some of the writes will be dropped without any notice.
So wait for the fifo space to be available before reading/writing the soundwire registers.
Signed-off-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org ---
Changes since v1: merged some of the loop code to make it simple as suggested by Pierre updated error code and comments as suggested by Vinod
drivers/soundwire/qcom.c | 66 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+)
diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c index 6affa3cd4039..5fd4a99cc8ac 100644 --- a/drivers/soundwire/qcom.c +++ b/drivers/soundwire/qcom.c @@ -24,6 +24,8 @@ #define SWRM_COMP_CFG_IRQ_LEVEL_OR_PULSE_MSK BIT(1) #define SWRM_COMP_CFG_ENABLE_MSK BIT(0) #define SWRM_COMP_PARAMS 0x100 +#define SWRM_COMP_PARAMS_WR_FIFO_DEPTH GENMASK(14, 10) +#define SWRM_COMP_PARAMS_RD_FIFO_DEPTH GENMASK(19, 15) #define SWRM_COMP_PARAMS_DOUT_PORTS_MASK GENMASK(4, 0) #define SWRM_COMP_PARAMS_DIN_PORTS_MASK GENMASK(9, 5) #define SWRM_INTERRUPT_STATUS 0x200 @@ -51,6 +53,8 @@ #define SWRM_CMD_FIFO_CMD 0x308 #define SWRM_CMD_FIFO_FLUSH 0x1 #define SWRM_CMD_FIFO_STATUS 0x30C +#define SWRM_RD_CMD_FIFO_CNT_MASK GENMASK(20, 16) +#define SWRM_WR_CMD_FIFO_CNT_MASK GENMASK(12, 8) #define SWRM_CMD_FIFO_CFG_ADDR 0x314 #define SWRM_CONTINUE_EXEC_ON_CMD_IGNORE BIT(31) #define SWRM_RD_WR_CMD_RETRIES 0x7 @@ -104,6 +108,7 @@ #define SWR_BROADCAST_CMD_ID 0x0F #define SWR_MAX_CMD_ID 14 #define MAX_FIFO_RD_RETRY 3 +#define SWR_OVERFLOW_RETRY_COUNT 30
struct qcom_swrm_port_config { u8 si; @@ -147,6 +152,8 @@ struct qcom_swrm_ctrl { int (*reg_read)(struct qcom_swrm_ctrl *ctrl, int reg, u32 *val); int (*reg_write)(struct qcom_swrm_ctrl *ctrl, int reg, int val); u32 slave_status; + u32 wr_fifo_depth; + u32 rd_fifo_depth; };
struct qcom_swrm_data { @@ -238,6 +245,55 @@ static u32 swrm_get_packed_reg_val(u8 *cmd_id, u8 cmd_data, return val; }
+static int swrm_wait_for_rd_fifo_avail(struct qcom_swrm_ctrl *swrm) +{ + u32 fifo_outstanding_data, value; + int fifo_retry_count = SWR_OVERFLOW_RETRY_COUNT; + + do { + /* Check for fifo underflow during read */ + swrm->reg_read(swrm, SWRM_CMD_FIFO_STATUS, &value); + fifo_outstanding_data = FIELD_GET(SWRM_RD_CMD_FIFO_CNT_MASK, value); + + /* Check if read data is available in read fifo */ + if (fifo_outstanding_data > 0) + return 0; + + usleep_range(500, 510); + } while (fifo_retry_count--); + + if (fifo_outstanding_data == 0) { + dev_err_ratelimited(swrm->dev, "%s err read underflow\n", __func__); + return -EIO; + } + + return 0; +} + +static int swrm_wait_for_wr_fifo_avail(struct qcom_swrm_ctrl *swrm) +{ + u32 fifo_outstanding_cmds, value; + int fifo_retry_count = SWR_OVERFLOW_RETRY_COUNT; + + do { + /* Check for fifo overflow during write */ + swrm->reg_read(swrm, SWRM_CMD_FIFO_STATUS, &value); + fifo_outstanding_cmds = FIELD_GET(SWRM_WR_CMD_FIFO_CNT_MASK, value); + + /* Check for space in write fifo before writing */ + if (fifo_outstanding_cmds < swrm->wr_fifo_depth) + return 0; + + usleep_range(500, 510); + } while (fifo_retry_count--); + + if (fifo_outstanding_cmds == swrm->wr_fifo_depth) { + dev_err_ratelimited(swrm->dev, "%s err write overflow\n", __func__); + return -EIO; + } + + return 0; +}
static int qcom_swrm_cmd_fifo_wr_cmd(struct qcom_swrm_ctrl *swrm, u8 cmd_data, u8 dev_addr, u16 reg_addr) @@ -256,6 +312,9 @@ static int qcom_swrm_cmd_fifo_wr_cmd(struct qcom_swrm_ctrl *swrm, u8 cmd_data, dev_addr, reg_addr); }
+ if (swrm_wait_for_wr_fifo_avail(swrm)) + return SDW_CMD_FAIL_OTHER; + /* Its assumed that write is okay as we do not get any status back */ swrm->reg_write(swrm, SWRM_CMD_FIFO_WR_CMD, val);
@@ -295,6 +354,9 @@ static int qcom_swrm_cmd_fifo_rd_cmd(struct qcom_swrm_ctrl *swrm, /* wait for FIFO RD CMD complete to avoid overflow */ usleep_range(250, 255);
+ if (swrm_wait_for_rd_fifo_avail(swrm)) + return SDW_CMD_FAIL_OTHER; + do { swrm->reg_read(swrm, SWRM_CMD_FIFO_RD_FIFO_ADDR, &cmd_data); rval[0] = cmd_data & 0xFF; @@ -586,6 +648,10 @@ static int qcom_swrm_init(struct qcom_swrm_ctrl *ctrl) SWRM_INTERRUPT_STATUS_RMSK); } ctrl->slave_status = 0; + ctrl->reg_read(ctrl, SWRM_COMP_PARAMS, &val); + ctrl->rd_fifo_depth = FIELD_GET(SWRM_COMP_PARAMS_RD_FIFO_DEPTH, val); + ctrl->wr_fifo_depth = FIELD_GET(SWRM_COMP_PARAMS_WR_FIFO_DEPTH, val); + return 0; }
On 4/1/21 4:00 AM, Srinivas Kandagatla wrote:
If we write registers very fast we can endup in a situation where some of the writes will be dropped without any notice.
So wait for the fifo space to be available before reading/writing the soundwire registers.
Out of curiosity, do you actually need to do a check in the read case as well?
The commit message talks about writes getting dropped, is the opposite also a problem?
On 01/04/2021 15:36, Pierre-Louis Bossart wrote:
On 4/1/21 4:00 AM, Srinivas Kandagatla wrote:
If we write registers very fast we can endup in a situation where some of the writes will be dropped without any notice.
So wait for the fifo space to be available before reading/writing the soundwire registers.
Out of curiosity, do you actually need to do a check in the read case as well?
Yes, This is just to make sure the read command is finished and fifo is ready with data.
If not we will be reading quickly an empty fifo!
The commit message talks about writes getting dropped, is the opposite also a problem?
Its highly likely, for safety I have added support for both write and read waits in this patch.
--srini
On 01-04-21, 10:00, Srinivas Kandagatla wrote:
If we write registers very fast we can endup in a situation where some of the writes will be dropped without any notice.
So wait for the fifo space to be available before reading/writing the soundwire registers.
Applied, thanks
participants (3)
-
Pierre-Louis Bossart
-
Srinivas Kandagatla
-
Vinod Koul