On 10/7/19 10:19 AM, Chen-Yu Tsai wrote:
> On Sun, Aug 25, 2019 at 1:50 AM Samuel Holland <sam...@sholland.org> wrote:
>>
>> The RSB controller has two registers for controlling interrupt inputs:
>> RSB_INTE, which has bits for each possible interrupt, and the global
>> interrupt enable bit in RSB_CTRL.
>>
>> Currently, we enable the bits in RSB_INTE before each transfer, but this
>> is unnecessary because we never disable them. Move the initialization of
>> RSB_INTE so it is done only once.
>>
>> We also set the global interrupt enable bit before each transfer. Unlike
>> other bits in RSB_CTRL, this bit is cleared by writing a zero. Thus, we
>> clear the bit in the post-timeout cleanup code, so note that in the
>> comment.
>>
>> However, if we do receive an interrupt, we do not clear the bit. Nor do
>> we clear interrupt statuses before starting a transfer. Thus, if some
>> other driver uses the RSB bus while Linux is suspended (as both Trusted
>> Firmware and SCP firmware do to control the PMIC), we receive spurious
>> interrupts upon resume. This causes false completion of a transfer, and
>> the next transfer starts prematurely, causing a LOAD_BSY condition. The
>> end result is that some transfers at resume fail with -EBUSY.
>
> If we are expecting the hardware to not be in the state we assume to be
> or left it in, then maybe we should also keep setting the interrupt enable
> bits on each transfer?
>
> Surely we expect to have exclusive use of the controller most of the time.
> If it's to handle suspend/resume, shouldn't we be adding power management
> callbacks instead? That would reset the controller to a known state when
> the system comes out of suspend, including clearing any pending interrupts.
Yes, this change is only to handle suspend/resume. You're right, that's a better
way to do it. I'll develop a patch using device power management callbacks.
Samuel
> Maxime, anything you want to add? (BTW, Maxime switched email addresses.)
>
> ChenYu
>
>> With this patch, all transfers reliably succeed during/after resume.
>>
>> Signed-off-by: Samuel Holland <sam...@sholland.org>
>> ---
>> drivers/bus/sunxi-rsb.c | 10 ++++++++--
>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c
>> index be79d6c6a4e4..b8043b58568a 100644
>> --- a/drivers/bus/sunxi-rsb.c
>> +++ b/drivers/bus/sunxi-rsb.c
>> @@ -274,7 +274,7 @@ static int _sunxi_rsb_run_xfer(struct sunxi_rsb *rsb)
>> reinit_completion(&rsb->complete);
>>
>> writel(RSB_INTS_LOAD_BSY | RSB_INTS_TRANS_ERR | RSB_INTS_TRANS_OVER,
>> - rsb->regs + RSB_INTE);
>> + rsb->regs + RSB_INTS);
>> writel(RSB_CTRL_START_TRANS | RSB_CTRL_GLOBAL_INT_ENB,
>> rsb->regs + RSB_CTRL);
>>
>> @@ -282,7 +282,7 @@ static int _sunxi_rsb_run_xfer(struct sunxi_rsb *rsb)
>> msecs_to_jiffies(100))) {
>> 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 */
>> @@ -480,6 +480,9 @@ static irqreturn_t sunxi_rsb_irq(int irq, void *dev_id)
>> status = readl(rsb->regs + RSB_INTS);
>> rsb->status = status;
>>
>> + /* Disable any further interrupts */
>> + writel(0, rsb->regs + RSB_CTRL);
>> +
>> /* Clear interrupts */
>> status &= (RSB_INTS_LOAD_BSY | RSB_INTS_TRANS_ERR |
>> RSB_INTS_TRANS_OVER);
>> @@ -718,6 +721,9 @@ static int sunxi_rsb_probe(struct platform_device *pdev)
>> goto err_reset_assert;
>> }
>>
>> + writel(RSB_INTS_LOAD_BSY | RSB_INTS_TRANS_ERR | RSB_INTS_TRANS_OVER,
>> + rsb->regs + RSB_INTE);
>> +
>> /* initialize all devices on the bus into RSB mode */
>> ret = sunxi_rsb_init_device_mode(rsb);
>> if (ret)
>> --
>> 2.21.0
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "linux-sunxi" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to linux-sunxi+unsubscr...@googlegroups.com.
>> To view this discussion on the web, visit
>> https://groups.google.com/d/msgid/linux-sunxi/20190824175013.28840-1-samuel%40sholland.org.
