2007-06-22 02:34:32

by Mark A. Greer

[permalink] [raw]
Subject: [PATCH] serial: Clear proper MPSC interrupt cause bits

From: Jay Lubomirski <[email protected]>

Don't clobber the interrupt cause bits for both MPSC controllers when
clearing the interrupt for one of them. Just clear the one that is
supposed to be cleared.

Signed-off-by: Jay Lubomirski <[email protected]>
Acked-by: Mark A. Greer <[email protected]>
---
drivers/serial/mpsc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/serial/mpsc.c b/drivers/serial/mpsc.c
index d09f209..00924fe 100644
--- a/drivers/serial/mpsc.c
+++ b/drivers/serial/mpsc.c
@@ -503,7 +503,8 @@ mpsc_sdma_intr_ack(struct mpsc_port_info *pi)

if (pi->mirror_regs)
pi->shared_regs->SDMA_INTR_CAUSE_m = 0;
- writel(0, pi->shared_regs->sdma_intr_base + SDMA_INTR_CAUSE);
+ writeb(0x00, pi->shared_regs->sdma_intr_base + SDMA_INTR_CAUSE +
+ pi->port.line);
return;
}


2007-06-23 16:52:19

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] serial: Clear proper MPSC interrupt cause bits

> On Thu, 21 Jun 2007 19:32:08 -0700 "Mark A. Greer" <[email protected]> wrote:
> From: Jay Lubomirski <[email protected]>
>
> Don't clobber the interrupt cause bits for both MPSC controllers when
> clearing the interrupt for one of them. Just clear the one that is
> supposed to be cleared.
>
> Signed-off-by: Jay Lubomirski <[email protected]>
> Acked-by: Mark A. Greer <[email protected]>
> ---
> drivers/serial/mpsc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/serial/mpsc.c b/drivers/serial/mpsc.c
> index d09f209..00924fe 100644
> --- a/drivers/serial/mpsc.c
> +++ b/drivers/serial/mpsc.c
> @@ -503,7 +503,8 @@ mpsc_sdma_intr_ack(struct mpsc_port_info *pi)
>
> if (pi->mirror_regs)
> pi->shared_regs->SDMA_INTR_CAUSE_m = 0;
> - writel(0, pi->shared_regs->sdma_intr_base + SDMA_INTR_CAUSE);
> + writeb(0x00, pi->shared_regs->sdma_intr_base + SDMA_INTR_CAUSE +
> + pi->port.line);
> return;
> }

In my naive little mpscless bubbleworld, I am believing that this is a
pretty important fix, and that people in an mpscful world might want it in
2.6.22. And even in 2.6.21.x.

But alas, that's just a guess, which was forced upon me by the lack of
suitable information in your changelog.

So please, tell us what are the real-world consequences of your patch (or
the lack of it), thanks.

2007-06-25 21:01:10

by Mark A. Greer

[permalink] [raw]
Subject: Re: [PATCH] serial: Clear proper MPSC interrupt cause bits

On Sat, Jun 23, 2007 at 09:51:44AM -0700, Andrew Morton wrote:
> > On Thu, 21 Jun 2007 19:32:08 -0700 "Mark A. Greer" <[email protected]> wrote:
> > From: Jay Lubomirski <[email protected]>
> >
> > Don't clobber the interrupt cause bits for both MPSC controllers when
> > clearing the interrupt for one of them. Just clear the one that is
> > supposed to be cleared.
> >
> > Signed-off-by: Jay Lubomirski <[email protected]>
> > Acked-by: Mark A. Greer <[email protected]>
> > ---
> > drivers/serial/mpsc.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/serial/mpsc.c b/drivers/serial/mpsc.c
> > index d09f209..00924fe 100644
> > --- a/drivers/serial/mpsc.c
> > +++ b/drivers/serial/mpsc.c
> > @@ -503,7 +503,8 @@ mpsc_sdma_intr_ack(struct mpsc_port_info *pi)
> >
> > if (pi->mirror_regs)
> > pi->shared_regs->SDMA_INTR_CAUSE_m = 0;
> > - writel(0, pi->shared_regs->sdma_intr_base + SDMA_INTR_CAUSE);
> > + writeb(0x00, pi->shared_regs->sdma_intr_base + SDMA_INTR_CAUSE +
> > + pi->port.line);
> > return;
> > }
>
> In my naive little mpscless bubbleworld, I am believing that this is a
> pretty important fix, and that people in an mpscful world might want it in
> 2.6.22. And even in 2.6.21.x.

Yes, that's probably true although the timing has to be just right for
it to occurs so its not that frequent.
I don't know what I have to do to get it into 2.6.22 at this late moment
(or 2.6.21.x for that matter).

> But alas, that's just a guess, which was forced upon me by the lack of
> suitable information in your changelog.

My aplogies.

> So please, tell us what are the real-world consequences of your patch (or
> the lack of it), thanks.

I'll resubmit the patch with a better description momentarily.
Sorry for the hassle.

Mark

2007-06-25 21:04:24

by Mark A. Greer

[permalink] [raw]
Subject: [PATCH] serial: Clear proper MPSC interrupt cause bits

From: Jay Lubomirski <[email protected]>

The interrupt clearing code in mpsc_sdma_intr_ack() mistakenly clears the
interrupt for both controllers instead of just the one its supposed to.
This can result in the other controller appearing to hang because its
interrupt was effectively lost.

So, don't clear the interrupt cause bits for both MPSC controllers when
clearing the interrupt for one of them. Just clear the one that is
supposed to be cleared.

Signed-off-by: Jay Lubomirski <[email protected]>
Acked-by: Mark A. Greer <[email protected]>
---
Please add this to 2.6.21.x and 2.6.22-rc6, if its not too late.

drivers/serial/mpsc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/serial/mpsc.c b/drivers/serial/mpsc.c
index d09f209..00924fe 100644
--- a/drivers/serial/mpsc.c
+++ b/drivers/serial/mpsc.c
@@ -503,7 +503,8 @@ mpsc_sdma_intr_ack(struct mpsc_port_info *pi)

if (pi->mirror_regs)
pi->shared_regs->SDMA_INTR_CAUSE_m = 0;
- writel(0, pi->shared_regs->sdma_intr_base + SDMA_INTR_CAUSE);
+ writeb(0x00, pi->shared_regs->sdma_intr_base + SDMA_INTR_CAUSE +
+ pi->port.line);
return;
}