Re: [PATCH v2 5/7] soundwire: qcom: prepare for handling different register layouts

From: Konrad Dybcio
Date: Thu Apr 06 2023 - 09:24:57 EST

Next message: Thierry Reding: "Re: [PATCH RESEND] dt-bindings: pwm: mediatek: add mediatek,mt7986 compatible"
Previous message: Arnd Bergmann: "Re: LTP: list of failures on 32bit and compat mode"
In reply to: Krzysztof Kozlowski: "[PATCH v2 5/7] soundwire: qcom: prepare for handling different register layouts"
Next in thread: Krzysztof Kozlowski: "[PATCH v2 4/7] soundwire: qcom: use consistently 'ctrl' as state variable name"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 3.04.2023 15:25, Krzysztof Kozlowski wrote:
> Currently the driver supports Qualcomm Soundwire controller versions
> from v1.3 till v1.7 which mostly have same register layout. With
> coming Qualcomm Soundwire v2.0, several registers were moved and
> changed, thus a different register layout will have to be supported.
>
> Prepare for this by:
> 1. Renaming few register defines to indicate v1.3 (earliest supported)
> version,
> 2. Add a simple table for mapping register to its offset,
> 3. Change the code to use the mapping table.
>
> Since only few registers differ, this solution seems easier then
> switching to regmap fields.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>

Konrad
>
> Changes since v1:
> 1. Fix lang typo in subject.
> ---
> drivers/soundwire/qcom.c | 130 +++++++++++++++++++++++++++++----------
> 1 file changed, 97 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
> index 00522de47b6f..b6666ffe37ae 100644
> --- a/drivers/soundwire/qcom.c
> +++ b/drivers/soundwire/qcom.c
> @@ -41,7 +41,7 @@
> #define SWRM_COMP_PARAMS_DOUT_PORTS_MASK GENMASK(4, 0)
> #define SWRM_COMP_PARAMS_DIN_PORTS_MASK GENMASK(9, 5)
> #define SWRM_COMP_MASTER_ID 0x104
> -#define SWRM_INTERRUPT_STATUS 0x200
> +#define SWRM_V1_3_INTERRUPT_STATUS 0x200
> #define SWRM_INTERRUPT_STATUS_RMSK GENMASK(16, 0)
> #define SWRM_INTERRUPT_STATUS_SLAVE_PEND_IRQ BIT(0)
> #define SWRM_INTERRUPT_STATUS_NEW_SLAVE_ATTACHED BIT(1)
> @@ -58,20 +58,20 @@
> #define SWRM_INTERRUPT_STATUS_CLK_STOP_FINISHED_V2 BIT(14)
> #define SWRM_INTERRUPT_STATUS_EXT_CLK_STOP_WAKEUP BIT(16)
> #define SWRM_INTERRUPT_MAX 17
> -#define SWRM_INTERRUPT_MASK_ADDR 0x204
> -#define SWRM_INTERRUPT_CLEAR 0x208
> -#define SWRM_INTERRUPT_CPU_EN 0x210
> -#define SWRM_CMD_FIFO_WR_CMD 0x300
> -#define SWRM_CMD_FIFO_RD_CMD 0x304
> +#define SWRM_V1_3_INTERRUPT_MASK_ADDR 0x204
> +#define SWRM_V1_3_INTERRUPT_CLEAR 0x208
> +#define SWRM_V1_3_INTERRUPT_CPU_EN 0x210
> +#define SWRM_V1_3_CMD_FIFO_WR_CMD 0x300
> +#define SWRM_V1_3_CMD_FIFO_RD_CMD 0x304
> #define SWRM_CMD_FIFO_CMD 0x308
> #define SWRM_CMD_FIFO_FLUSH 0x1
> -#define SWRM_CMD_FIFO_STATUS 0x30C
> +#define SWRM_V1_3_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
> -#define SWRM_CMD_FIFO_RD_FIFO_ADDR 0x318
> +#define SWRM_V1_3_CMD_FIFO_RD_FIFO_ADDR 0x318
> #define SWRM_RD_FIFO_CMD_ID_MASK GENMASK(11, 8)
> #define SWRM_ENUMERATOR_CFG_ADDR 0x500
> #define SWRM_ENUMERATOR_SLAVE_DEV_ID_1(m) (0x530 + 0x8 * (m))
> @@ -97,7 +97,7 @@
> #define SWRM_DP_BLOCK_CTRL3_BANK(n, m) (0x1138 + 0x100 * (n - 1) + 0x40 * m)
> #define SWRM_DP_SAMPLECTRL2_BANK(n, m) (0x113C + 0x100 * (n - 1) + 0x40 * m)
> #define SWRM_DIN_DPn_PCM_PORT_CTRL(n) (0x1054 + 0x100 * (n - 1))
> -#define SWR_MSTR_MAX_REG_ADDR (0x1740)
> +#define SWR_V1_3_MSTR_MAX_REG_ADDR 0x1740
>
> #define SWRM_DP_PORT_CTRL_EN_CHAN_SHFT 0x18
> #define SWRM_DP_PORT_CTRL_OFFSET2_SHFT 0x10
> @@ -143,10 +143,28 @@ struct qcom_swrm_port_config {
> u8 lane_control;
> };
>
> +/*
> + * Internal IDs for different register layouts. Only few registers differ per
> + * each variant, so the list of IDs below does not include all of registers.
> + */
> +enum {
> + SWRM_REG_FRAME_GEN_ENABLED,
> + SWRM_REG_INTERRUPT_STATUS,
> + SWRM_REG_INTERRUPT_MASK_ADDR,
> + SWRM_REG_INTERRUPT_CLEAR,
> + SWRM_REG_INTERRUPT_CPU_EN,
> + SWRM_REG_CMD_FIFO_WR_CMD,
> + SWRM_REG_CMD_FIFO_RD_CMD,
> + SWRM_REG_CMD_FIFO_STATUS,
> + SWRM_REG_CMD_FIFO_RD_FIFO_ADDR,
> +};
> +
> struct qcom_swrm_ctrl {
> struct sdw_bus bus;
> struct device *dev;
> struct regmap *regmap;
> + u32 max_reg;
> + const unsigned int *reg_layout;
> void __iomem *mmio;
> struct reset_control *audio_cgcr;
> #ifdef CONFIG_DEBUG_FS
> @@ -187,22 +205,42 @@ struct qcom_swrm_data {
> u32 default_cols;
> u32 default_rows;
> bool sw_clk_gate_required;
> + u32 max_reg;
> + const unsigned int *reg_layout;
> +};
> +
> +static const unsigned int swrm_v1_3_reg_layout[] = {
> + [SWRM_REG_FRAME_GEN_ENABLED] = SWRM_COMP_STATUS,
> + [SWRM_REG_INTERRUPT_STATUS] = SWRM_V1_3_INTERRUPT_STATUS,
> + [SWRM_REG_INTERRUPT_MASK_ADDR] = SWRM_V1_3_INTERRUPT_MASK_ADDR,
> + [SWRM_REG_INTERRUPT_CLEAR] = SWRM_V1_3_INTERRUPT_CLEAR,
> + [SWRM_REG_INTERRUPT_CPU_EN] = SWRM_V1_3_INTERRUPT_CPU_EN,
> + [SWRM_REG_CMD_FIFO_WR_CMD] = SWRM_V1_3_CMD_FIFO_WR_CMD,
> + [SWRM_REG_CMD_FIFO_RD_CMD] = SWRM_V1_3_CMD_FIFO_RD_CMD,
> + [SWRM_REG_CMD_FIFO_STATUS] = SWRM_V1_3_CMD_FIFO_STATUS,
> + [SWRM_REG_CMD_FIFO_RD_FIFO_ADDR] = SWRM_V1_3_CMD_FIFO_RD_FIFO_ADDR,
> };
>
> static const struct qcom_swrm_data swrm_v1_3_data = {
> .default_rows = 48,
> .default_cols = 16,
> + .max_reg = SWR_V1_3_MSTR_MAX_REG_ADDR,
> + .reg_layout = swrm_v1_3_reg_layout,
> };
>
> static const struct qcom_swrm_data swrm_v1_5_data = {
> .default_rows = 50,
> .default_cols = 16,
> + .max_reg = SWR_V1_3_MSTR_MAX_REG_ADDR,
> + .reg_layout = swrm_v1_3_reg_layout,
> };
>
> static const struct qcom_swrm_data swrm_v1_6_data = {
> .default_rows = 50,
> .default_cols = 16,
> .sw_clk_gate_required = true,
> + .max_reg = SWR_V1_3_MSTR_MAX_REG_ADDR,
> + .reg_layout = swrm_v1_3_reg_layout,
> };
>
> #define to_qcom_sdw(b) container_of(b, struct qcom_swrm_ctrl, bus)
> @@ -286,7 +324,8 @@ static int swrm_wait_for_rd_fifo_avail(struct qcom_swrm_ctrl *ctrl)
>
> do {
> /* Check for fifo underflow during read */
> - ctrl->reg_read(ctrl, SWRM_CMD_FIFO_STATUS, &value);
> + ctrl->reg_read(ctrl, ctrl->reg_layout[SWRM_REG_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 */
> @@ -311,7 +350,8 @@ static int swrm_wait_for_wr_fifo_avail(struct qcom_swrm_ctrl *ctrl)
>
> do {
> /* Check for fifo overflow during write */
> - ctrl->reg_read(ctrl, SWRM_CMD_FIFO_STATUS, &value);
> + ctrl->reg_read(ctrl, ctrl->reg_layout[SWRM_REG_CMD_FIFO_STATUS],
> + &value);
> fifo_outstanding_cmds = FIELD_GET(SWRM_WR_CMD_FIFO_CNT_MASK, value);
>
> /* Check for space in write fifo before writing */
> @@ -353,7 +393,7 @@ static int qcom_swrm_cmd_fifo_wr_cmd(struct qcom_swrm_ctrl *ctrl, u8 cmd_data,
> reinit_completion(&ctrl->broadcast);
>
> /* Its assumed that write is okay as we do not get any status back */
> - ctrl->reg_write(ctrl, SWRM_CMD_FIFO_WR_CMD, val);
> + ctrl->reg_write(ctrl, ctrl->reg_layout[SWRM_REG_CMD_FIFO_WR_CMD], val);
>
> if (ctrl->version <= SWRM_VERSION_1_3_0)
> usleep_range(150, 155);
> @@ -392,7 +432,7 @@ static int qcom_swrm_cmd_fifo_rd_cmd(struct qcom_swrm_ctrl *ctrl,
>
> /* wait for FIFO RD to complete to avoid overflow */
> usleep_range(100, 105);
> - ctrl->reg_write(ctrl, SWRM_CMD_FIFO_RD_CMD, val);
> + ctrl->reg_write(ctrl, ctrl->reg_layout[SWRM_REG_CMD_FIFO_RD_CMD], val);
> /* wait for FIFO RD CMD complete to avoid overflow */
> usleep_range(250, 255);
>
> @@ -400,7 +440,8 @@ static int qcom_swrm_cmd_fifo_rd_cmd(struct qcom_swrm_ctrl *ctrl,
> return SDW_CMD_FAIL_OTHER;
>
> do {
> - ctrl->reg_read(ctrl, SWRM_CMD_FIFO_RD_FIFO_ADDR, &cmd_data);
> + ctrl->reg_read(ctrl, ctrl->reg_layout[SWRM_REG_CMD_FIFO_RD_FIFO_ADDR],
> + &cmd_data);
> rval[0] = cmd_data & 0xFF;
> cmd_id = FIELD_GET(SWRM_RD_FIFO_CMD_ID_MASK, cmd_data);
>
> @@ -410,7 +451,9 @@ static int qcom_swrm_cmd_fifo_rd_cmd(struct qcom_swrm_ctrl *ctrl,
> usleep_range(500, 505);
> ctrl->reg_write(ctrl, SWRM_CMD_FIFO_CMD,
> SWRM_CMD_FIFO_FLUSH);
> - ctrl->reg_write(ctrl, SWRM_CMD_FIFO_RD_CMD, val);
> + ctrl->reg_write(ctrl,
> + ctrl->reg_layout[SWRM_REG_CMD_FIFO_RD_CMD],
> + val);
> }
> retry_attempt++;
> } else {
> @@ -564,7 +607,8 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
> int ret = IRQ_HANDLED;
> clk_prepare_enable(ctrl->hclk);
>
> - ctrl->reg_read(ctrl, SWRM_INTERRUPT_STATUS, &intr_sts);
> + ctrl->reg_read(ctrl, ctrl->reg_layout[SWRM_REG_INTERRUPT_STATUS],
> + &intr_sts);
> intr_sts_masked = intr_sts & ctrl->intr_mask;
>
> do {
> @@ -602,29 +646,39 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
> "%s: SWR bus clsh detected\n",
> __func__);
> ctrl->intr_mask &= ~SWRM_INTERRUPT_STATUS_MASTER_CLASH_DET;
> - ctrl->reg_write(ctrl, SWRM_INTERRUPT_CPU_EN, ctrl->intr_mask);
> + ctrl->reg_write(ctrl,
> + ctrl->reg_layout[SWRM_REG_INTERRUPT_CPU_EN],
> + ctrl->intr_mask);
> break;
> case SWRM_INTERRUPT_STATUS_RD_FIFO_OVERFLOW:
> - ctrl->reg_read(ctrl, SWRM_CMD_FIFO_STATUS, &value);
> + ctrl->reg_read(ctrl,
> + ctrl->reg_layout[SWRM_REG_CMD_FIFO_STATUS],
> + &value);
> dev_err_ratelimited(ctrl->dev,
> "%s: SWR read FIFO overflow fifo status 0x%x\n",
> __func__, value);
> break;
> case SWRM_INTERRUPT_STATUS_RD_FIFO_UNDERFLOW:
> - ctrl->reg_read(ctrl, SWRM_CMD_FIFO_STATUS, &value);
> + ctrl->reg_read(ctrl,
> + ctrl->reg_layout[SWRM_REG_CMD_FIFO_STATUS],
> + &value);
> dev_err_ratelimited(ctrl->dev,
> "%s: SWR read FIFO underflow fifo status 0x%x\n",
> __func__, value);
> break;
> case SWRM_INTERRUPT_STATUS_WR_CMD_FIFO_OVERFLOW:
> - ctrl->reg_read(ctrl, SWRM_CMD_FIFO_STATUS, &value);
> + ctrl->reg_read(ctrl,
> + ctrl->reg_layout[SWRM_REG_CMD_FIFO_STATUS],
> + &value);
> dev_err(ctrl->dev,
> "%s: SWR write FIFO overflow fifo status %x\n",
> __func__, value);
> ctrl->reg_write(ctrl, SWRM_CMD_FIFO_CMD, 0x1);
> break;
> case SWRM_INTERRUPT_STATUS_CMD_ERROR:
> - ctrl->reg_read(ctrl, SWRM_CMD_FIFO_STATUS, &value);
> + ctrl->reg_read(ctrl,
> + ctrl->reg_layout[SWRM_REG_CMD_FIFO_STATUS],
> + &value);
> dev_err_ratelimited(ctrl->dev,
> "%s: SWR CMD error, fifo status 0x%x, flushing fifo\n",
> __func__, value);
> @@ -636,7 +690,8 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
> __func__);
> ctrl->intr_mask &= ~SWRM_INTERRUPT_STATUS_DOUT_PORT_COLLISION;
> ctrl->reg_write(ctrl,
> - SWRM_INTERRUPT_CPU_EN, ctrl->intr_mask);
> + ctrl->reg_layout[SWRM_REG_INTERRUPT_CPU_EN],
> + ctrl->intr_mask);
> break;
> case SWRM_INTERRUPT_STATUS_READ_EN_RD_VALID_MISMATCH:
> dev_err_ratelimited(ctrl->dev,
> @@ -645,7 +700,8 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
> ctrl->intr_mask &=
> ~SWRM_INTERRUPT_STATUS_READ_EN_RD_VALID_MISMATCH;
> ctrl->reg_write(ctrl,
> - SWRM_INTERRUPT_CPU_EN, ctrl->intr_mask);
> + ctrl->reg_layout[SWRM_REG_INTERRUPT_CPU_EN],
> + ctrl->intr_mask);
> break;
> case SWRM_INTERRUPT_STATUS_SPECIAL_CMD_ID_FINISHED:
> complete(&ctrl->broadcast);
> @@ -664,8 +720,10 @@ static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id)
> break;
> }
> }
> - ctrl->reg_write(ctrl, SWRM_INTERRUPT_CLEAR, intr_sts);
> - ctrl->reg_read(ctrl, SWRM_INTERRUPT_STATUS, &intr_sts);
> + ctrl->reg_write(ctrl, ctrl->reg_layout[SWRM_REG_INTERRUPT_CLEAR],
> + intr_sts);
> + ctrl->reg_read(ctrl, ctrl->reg_layout[SWRM_REG_INTERRUPT_STATUS],
> + &intr_sts);
> intr_sts_masked = intr_sts & ctrl->intr_mask;
> } while (intr_sts_masked);
>
> @@ -690,7 +748,7 @@ static int qcom_swrm_init(struct qcom_swrm_ctrl *ctrl)
>
> ctrl->intr_mask = SWRM_INTERRUPT_STATUS_RMSK;
> /* Mask soundwire interrupts */
> - ctrl->reg_write(ctrl, SWRM_INTERRUPT_MASK_ADDR,
> + ctrl->reg_write(ctrl, ctrl->reg_layout[SWRM_REG_INTERRUPT_MASK_ADDR],
> SWRM_INTERRUPT_STATUS_RMSK);
>
> /* Configure No pings */
> @@ -723,7 +781,7 @@ static int qcom_swrm_init(struct qcom_swrm_ctrl *ctrl)
>
> /* enable CPU IRQs */
> if (ctrl->mmio) {
> - ctrl->reg_write(ctrl, SWRM_INTERRUPT_CPU_EN,
> + ctrl->reg_write(ctrl, ctrl->reg_layout[SWRM_REG_INTERRUPT_CPU_EN],
> SWRM_INTERRUPT_STATUS_RMSK);
> }
> ctrl->slave_status = 0;
> @@ -1312,7 +1370,7 @@ static int swrm_reg_show(struct seq_file *s_file, void *data)
> return ret;
> }
>
> - for (reg = 0; reg <= SWR_MSTR_MAX_REG_ADDR; reg += 4) {
> + for (reg = 0; reg <= ctrl->max_reg; reg += 4) {
> ctrl->reg_read(ctrl, reg, &reg_val);
> seq_printf(s_file, "0x%.3x: 0x%.2x\n", reg, reg_val);
> }
> @@ -1340,6 +1398,8 @@ static int qcom_swrm_probe(struct platform_device *pdev)
> return -ENOMEM;
>
> data = of_device_get_match_data(dev);
> + ctrl->max_reg = data->max_reg;
> + ctrl->reg_layout = data->reg_layout;
> ctrl->rows_index = sdw_find_row_index(data->default_rows);
> ctrl->cols_index = sdw_find_col_index(data->default_cols);
> #if IS_REACHABLE(CONFIG_SLIMBUS)
> @@ -1556,12 +1616,14 @@ static int __maybe_unused swrm_runtime_resume(struct device *dev)
> } else {
> ctrl->reg_write(ctrl, SWRM_MCP_BUS_CTRL, SWRM_MCP_BUS_CLK_START);
> }
> - ctrl->reg_write(ctrl, SWRM_INTERRUPT_CLEAR,
> + ctrl->reg_write(ctrl, ctrl->reg_layout[SWRM_REG_INTERRUPT_CLEAR],
> SWRM_INTERRUPT_STATUS_MASTER_CLASH_DET);
>
> ctrl->intr_mask |= SWRM_INTERRUPT_STATUS_MASTER_CLASH_DET;
> - ctrl->reg_write(ctrl, SWRM_INTERRUPT_MASK_ADDR, ctrl->intr_mask);
> - ctrl->reg_write(ctrl, SWRM_INTERRUPT_CPU_EN, ctrl->intr_mask);
> + ctrl->reg_write(ctrl, ctrl->reg_layout[SWRM_REG_INTERRUPT_MASK_ADDR],
> + ctrl->intr_mask);
> + ctrl->reg_write(ctrl, ctrl->reg_layout[SWRM_REG_INTERRUPT_CPU_EN],
> + ctrl->intr_mask);
>
> usleep_range(100, 105);
> if (!swrm_wait_for_frame_gen_enabled(ctrl))
> @@ -1583,8 +1645,10 @@ static int __maybe_unused swrm_runtime_suspend(struct device *dev)
> if (!ctrl->clock_stop_not_supported) {
> /* Mask bus clash interrupt */
> ctrl->intr_mask &= ~SWRM_INTERRUPT_STATUS_MASTER_CLASH_DET;
> - ctrl->reg_write(ctrl, SWRM_INTERRUPT_MASK_ADDR, ctrl->intr_mask);
> - ctrl->reg_write(ctrl, SWRM_INTERRUPT_CPU_EN, ctrl->intr_mask);
> + ctrl->reg_write(ctrl, ctrl->reg_layout[SWRM_REG_INTERRUPT_MASK_ADDR],
> + ctrl->intr_mask);
> + ctrl->reg_write(ctrl, ctrl->reg_layout[SWRM_REG_INTERRUPT_CPU_EN],
> + ctrl->intr_mask);
> /* Prepare slaves for clock stop */
> ret = sdw_bus_prep_clk_stop(&ctrl->bus);
> if (ret < 0 && ret != -ENODATA) {

Next message: Thierry Reding: "Re: [PATCH RESEND] dt-bindings: pwm: mediatek: add mediatek,mt7986 compatible"
Previous message: Arnd Bergmann: "Re: LTP: list of failures on 32bit and compat mode"
In reply to: Krzysztof Kozlowski: "[PATCH v2 5/7] soundwire: qcom: prepare for handling different register layouts"
Next in thread: Krzysztof Kozlowski: "[PATCH v2 4/7] soundwire: qcom: use consistently 'ctrl' as state variable name"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]