>>> static void acp63_disable_interrupts(void __iomem *acp_base) >>> @@ -102,23 +103,55 @@ static irqreturn_t acp63_irq_handler(int irq, void *dev_id) >>> { >>> struct acp63_dev_data *adata; >>> struct pdm_dev_data *ps_pdm_data; >>> - u32 val; >>> + struct amd_sdw_manager *amd_manager; >>> + u32 ext_intr_stat, ext_intr_stat1; >>> + u16 irq_flag = 0; >>> u16 pdev_index; >>> >>> adata = dev_id; >>> if (!adata) >>> return IRQ_NONE; >>> + ext_intr_stat = readl(adata->acp63_base + ACP_EXTERNAL_INTR_STAT); >>> + if (ext_intr_stat & ACP_SDW0_STAT) { >>> + writel(ACP_SDW0_STAT, adata->acp63_base + ACP_EXTERNAL_INTR_STAT); >> [1] >> >> this is confusing, if this is w1c, should this be: >> >> writel(ext_intr_stat, adata->acp63_base + ACP_EXTERNAL_INTR_STAT); >> >> Otherwise you may be clearing fields that have not been set? > As per our register spec, writing zero to register fields doesn't > have any effect. Only writing 1 to register bit will clear that > interrupt. > We are handling bit by bit irq check and clearing the irq mask > based on irq bit and take an action related to that particular irq > bit. Right, maybe an explanation in the commit message would help.