Re: [PATCH 6/6] soundwire: qcom: add support to new interrupts

On 01/02/2021 14:31, Vinod Koul wrote:

On 29-01-21, 17:32, Srinivas Kandagatla wrote:

...

Add support to new interrupts and update irq routine in a way to deal with multiple pending interrupts with in a single interrupt!

Signed-off-by: Srinivas Kandagatla srinivas.kandagatla@linaro.org

drivers/soundwire/qcom.c | 191 ++++++++++++++++++++++++++++++--------- 1 file changed, 146 insertions(+), 45 deletions(-)

diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c index d61b204dc284..c699bd51d29a 100644 --- a/drivers/soundwire/qcom.c +++ b/drivers/soundwire/qcom.c @@ -28,10 +28,21 @@ #define SWRM_COMP_PARAMS_DIN_PORTS_MASK GENMASK(9, 5) #define SWRM_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) #define SWRM_INTERRUPT_STATUS_CHANGE_ENUM_SLAVE_STATUS BIT(2) +#define SWRM_INTERRUPT_STATUS_MASTER_CLASH_DET BIT(3) +#define SWRM_INTERRUPT_STATUS_RD_FIFO_OVERFLOW BIT(4) +#define SWRM_INTERRUPT_STATUS_RD_FIFO_UNDERFLOW BIT(5) +#define SWRM_INTERRUPT_STATUS_WR_CMD_FIFO_OVERFLOW BIT(6) #define SWRM_INTERRUPT_STATUS_CMD_ERROR BIT(7) +#define SWRM_INTERRUPT_STATUS_DOUT_PORT_COLLISION BIT(8) +#define SWRM_INTERRUPT_STATUS_READ_EN_RD_VALID_MISMATCH BIT(9) #define SWRM_INTERRUPT_STATUS_SPECIAL_CMD_ID_FINISHED BIT(10) +#define SWRM_INTERRUPT_STATUS_BUS_RESET_FINISHED_V2 BIT(13) +#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 @@ -105,11 +116,8 @@ struct qcom_swrm_ctrl { struct device *dev; struct regmap *regmap; void __iomem *mmio;

• struct completion *comp; struct completion broadcast; struct work_struct slave_work;
• /* read/write lock */
• spinlock_t comp_lock; /* Port alloc/free lock */ struct mutex port_lock; struct mutex io_lock;

@@ -126,6 +134,7 @@ struct qcom_swrm_ctrl { int rows_index; unsigned long dout_port_mask; unsigned long din_port_mask;

• u32 intr_mask; u8 rcmd_id; u8 wcmd_id; struct qcom_swrm_port_config pconfig[QCOM_SDW_MAX_PORTS];

@@ -315,6 +324,27 @@ static int qcom_swrm_cmd_fifo_rd_cmd(struct qcom_swrm_ctrl *swrm, return ret; }

+static int qcom_swrm_get_alert_slave(struct qcom_swrm_ctrl *ctrl) +{

• u32 val;
• int i;
• ctrl->reg_read(ctrl, SWRM_MCP_SLV_STATUS, &val);
• for (i = 0; i < SDW_MAX_DEVICES; i++) {

• u32 s;
  

define at top of the function pls, also maybe better name status?

Sure,

...

• s = (val >> (i * 2));
  

why * 2 ? Maybe add a comment for this logic

This is status mask of 2 bits! Will clarify this in next version!

...

• if ((s & SWRM_MCP_SLV_STATUS_MASK) == SDW_SLAVE_ALERT) {
  

• 	ctrl->status[i] = s;
  

• 	return i;
  

• }
  

• }
• return -EINVAL;

+}

• static void qcom_swrm_get_device_status(struct qcom_swrm_ctrl *ctrl) { u32 val;

@@ -333,40 +363,122 @@ static void qcom_swrm_get_device_status(struct qcom_swrm_ctrl *ctrl)

static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id) {

• struct qcom_swrm_ctrl *ctrl = dev_id;
• u32 sts, value;
• unsigned long flags;
• ctrl->reg_read(ctrl, SWRM_INTERRUPT_STATUS, &sts);
• if (sts & SWRM_INTERRUPT_STATUS_CMD_ERROR) {

• ctrl->reg_read(ctrl, SWRM_CMD_FIFO_STATUS, &value);
  

• dev_err_ratelimited(ctrl->dev,
  

• 		    "CMD error, fifo status 0x%x\n",
  

• 		     value);
  

• ctrl->reg_write(ctrl, SWRM_CMD_FIFO_CMD, 0x1);
  

• }

• struct qcom_swrm_ctrl *swrm = dev_id;
• u32 value, intr_sts, intr_sts_masked;
• u32 i;
• u8 devnum = 0;
• int ret = IRQ_HANDLED;
• swrm->reg_read(swrm, SWRM_INTERRUPT_STATUS, &intr_sts);
• intr_sts_masked = intr_sts & swrm->intr_mask;

+handle_irq:

maybe move this into a fn and avoid a goto for non err path?

Makes sense!

--srini