Messages in this thread Patch in this message | | | From | Jernej Škrabec <> | Subject | Re: [PATCH v3 3/3] bus: sunxi-rsb: Clear interrupt status before each transfer | Date | Mon, 14 Nov 2022 22:00:06 +0100 |
| |
Hi Samuel,
Dne ponedeljek, 14. november 2022 ob 02:57:49 CET je Samuel Holland napisal(a): > Currently, the driver clears the interrupt status bits after anything > could have set them. However, this requires duplicating the same logic > in several places. > > Instead of clearing the status flags in the interrupt handler, disable > all further interrupts by clearing the RSB_CTRL_GLOBAL_INT_ENB bit.
where is this bit cleared?
> Then we can delay the status register write until the start of the next > transfer, so it only has to be done in one place. > > Signed-off-by: Samuel Holland <samuel@sholland.org> > --- > > Changes in v3: > - Add a patch refactoring how the status bits are cleared > > drivers/bus/sunxi-rsb.c | 20 +++++--------------- > 1 file changed, 5 insertions(+), 15 deletions(-) > > diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c > index 3aa91aed3bf7..cb622e60897b 100644 > --- a/drivers/bus/sunxi-rsb.c > +++ b/drivers/bus/sunxi-rsb.c > @@ -279,6 +279,7 @@ static int _sunxi_rsb_run_xfer(struct sunxi_rsb *rsb) > > int_mask = RSB_INTS_LOAD_BSY | RSB_INTS_TRANS_ERR | RSB_INTS_TRANS_OVER; > writel(int_mask, rsb->regs + RSB_INTE); > + writel(int_mask, rsb->regs + RSB_INTS);
Wouldn't be better to clear status before enabling interrupts? Unless global interrupt flag is cleared beforehand, but I don't see that anywhere.
Best regards, Jernej
> writel(RSB_CTRL_START_TRANS | RSB_CTRL_GLOBAL_INT_ENB, > rsb->regs + RSB_CTRL); > > @@ -286,7 +287,6 @@ static int _sunxi_rsb_run_xfer(struct sunxi_rsb *rsb) > timeout = readl_poll_timeout_atomic(rsb->regs + RSB_INTS, > status, (status & int_mask), > 10, 100000); > - writel(status, rsb->regs + RSB_INTS); > } else { > timeout = !wait_for_completion_io_timeout(&rsb- >complete, > msecs_to_jiffies(100)); > @@ -296,12 +296,9 @@ static int _sunxi_rsb_run_xfer(struct sunxi_rsb *rsb) > if (timeout) { > dev_dbg(rsb->dev, "RSB timeout\n"); > > - /* abort the transfer */ > + /* abort the transfer and disable interrupts */ > writel(RSB_CTRL_ABORT_TRANS, rsb->regs + RSB_CTRL); > > - /* clear any interrupt flags */ > - writel(readl(rsb->regs + RSB_INTS), rsb->regs + RSB_INTS); > - > return -ETIMEDOUT; > } > > @@ -503,15 +500,11 @@ EXPORT_SYMBOL_GPL(__devm_regmap_init_sunxi_rsb); > static irqreturn_t sunxi_rsb_irq(int irq, void *dev_id) > { > struct sunxi_rsb *rsb = dev_id; > - u32 status; > > - status = readl(rsb->regs + RSB_INTS); > - rsb->status = status; > + /* disable interrupts */ > + writel(0, rsb->regs + RSB_CTRL); > > - /* Clear interrupts */ > - status &= (RSB_INTS_LOAD_BSY | RSB_INTS_TRANS_ERR | > - RSB_INTS_TRANS_OVER); > - writel(status, rsb->regs + RSB_INTS); > + rsb->status = readl(rsb->regs + RSB_INTS); > > complete(&rsb->complete); > > @@ -532,9 +525,6 @@ static int sunxi_rsb_init_device_mode(struct sunxi_rsb > *rsb) if (reg & RSB_DMCR_DEVICE_START) > ret = -ETIMEDOUT; > > - /* clear interrupt status bits */ > - writel(readl(rsb->regs + RSB_INTS), rsb->regs + RSB_INTS); > - > return ret; > }
| |