lkml.org 
[lkml]   [2021]   [Jan]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 5/6] soundwire: qcom: update register read/write routine
From
Date


On 1/29/21 11:32 AM, Srinivas Kandagatla wrote:
> In the existing code every soundwire register read and register write
> are kinda blocked. Each of these are using a special command id that

what does 'kinda blocked' mean?

> generates interrupt after it successfully finishes. This is really
> overhead, limiting and not really necessary unless we are doing
> something special.
>
> We can simply read/write the fifo that should also give exactly
> what we need! This will also allow to read/write registers in
> interrupt context, which was not possible with the special
> command approach.

This is really unclear, sorry.

> + if (id != SWR_BROADCAST_CMD_ID) {
> + if (id < 14)
> + id += 1;
> + else
> + id = 0;

that is really odd. if id=13 (group2) then id becomes 14 (master
address). A comment is really needed here.

> + if (cmd_id == SWR_BROADCAST_CMD_ID) {
> + /*
> + * sleep for 10ms for MSM soundwire variant to allow broadcast
> + * command to complete.

that's also super-odd. There is nothing in SoundWire that makes any
difference between a regular and a broadcast command. they all complete
in the same time (a frame).
> + */
> + ret = wait_for_completion_timeout(&swrm->broadcast, (2 * HZ/10));

is this 10ms really or dependent on CONFIG_HZ?

> + if (!ret)
> + ret = SDW_CMD_IGNORED;
> + else
> + ret = SDW_CMD_OK;

no CMD_FAILED support?

> +static int qcom_swrm_cmd_fifo_rd_cmd(struct qcom_swrm_ctrl *swrm,
> + u8 dev_addr, u16 reg_addr,
> + u32 len, u8 *rval)
> +{
> + u32 val;
> + u32 retry_attempt = 0;
> + u32 cmd_data;
> + int ret = SDW_CMD_OK;
> +
> + mutex_lock(&swrm->io_lock);
> + val = swrm_get_packed_reg_val(&swrm->rcmd_id, len, dev_addr, reg_addr);
> +
> + /* wait for FIFO RD to complete to avoid overflow */
> + usleep_range(100, 105);
> + swrm->reg_write(swrm, SWRM_CMD_FIFO_RD_CMD, val);
> + /* wait for FIFO RD CMD complete to avoid overflow */
> + usleep_range(250, 255);
> +
> +retry_read:
> +
> + swrm->reg_read(swrm, SWRM_CMD_FIFO_RD_FIFO_ADDR, &cmd_data);
> + rval[0] = cmd_data & 0xFF;
> +
> + if ((((cmd_data) & 0xF00) >> 8) != swrm->rcmd_id) {
> + if (retry_attempt < MAX_FIFO_RD_FAIL_RETRY) {
> + /* wait 500 us before retry on fifo read failure */
> + usleep_range(500, 505);
> + if (retry_attempt == (MAX_FIFO_RD_FAIL_RETRY - 1)) {
> + swrm->reg_write(swrm, SWRM_CMD_FIFO_CMD, 0x1);
> + swrm->reg_write(swrm, SWRM_CMD_FIFO_RD_CMD, val);
> + }
> + retry_attempt++;
> + goto retry_read;
> + } else {
> + dev_err(swrm->dev,
> + "failed to read fifo: reg: 0x%x, \
> + rcmd_id: 0x%x, dev_num: 0x%x, cmd_data: 0x%x\n",
> + reg_addr, swrm->rcmd_id,
> + dev_addr, cmd_data);
> + ret = SDW_CMD_IGNORED;
> + }
> }

the flow seems complicated with multiple tests and goto? Can this be
simplified?

\
 
 \ /
  Last update: 2021-01-29 21:21    [W:0.212 / U:0.040 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site