From: Marc St-Jean <Marc_St-Jean@pmc-sierra.com>
To: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Cc: linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-mips@linux-mips.org
Subject: Re: [PATCH] serial driver PMC MSP71xx, kernel linux-mips.git mast er
Date: Fri, 9 Feb 2007 11:39:00 -0800 [thread overview]
Message-ID: <45CCCDD4.7030104@pmc-sierra.com> (raw)
Sergei Shtylyov wrote:
> Marc St-Jean wrote:
> > Fourth attempt at the serial driver patch for the PMC-Sierra MSP71xx
> device.
> >
> > There are three different fixes:
> > 1. Fix for DesignWare THRE errata
> > - Dropped our fix since the "8250-uart-backup-timer.patch" in the "mm"
> > tree also fixes it. This patch needs to be applied on top of "mm" patch.
> >
> > 2. Fix for Busy Detect on LCR write
> > - Changed the ordering of test in serial8250_interrupt().
> > - Combined test for UPIO_DWAPB with UPIO_MEM in
> serial8250_start_console().
> > - Renamed new uart_8250_port member to private_data.
> >
> > 3. Workaround for interrupt/data concurrency issue
> > - No changes since last patch.
>
> > @@ -1383,6 +1399,19 @@ static irqreturn_t serial8250_interrupt(
> > handled = 1;
> >
> > end = NULL;
> > + } else if (up->port.iotype == UPIO_DWAPB &&
> > + (iir & UART_IIR_BUSY) == UART_IIR_BUSY) {
>
> Worth aligning this line with the opening paren of if... but's that's
> nitpicking. :-)
No problem I'll change it. I just usually align to the closest tab stop to
the opening parenthesis.
> > + /* The DesignWare APB UART has an Busy Detect
> (0x07)
> > + * interrupt meaning an LCR write attempt
> occured while the
> > + * UART was busy. The interrupt must be cleared
> by reading
> > + * the UART status register (USR) and the LCR
> re-written. */
> > + unsigned int status;
> > + status = *(volatile u32 *)up->port.private_data;
> > + serial_out(up, UART_LCR, up->lcr);
> > +
> > + handled = 1;
> > +
> > + end = NULL;
> > } else if (end == NULL)
> > end = l;
> >
> > return 0;
>
> Still, shouldn't you be doing this in serial8250_timeout()
No, the serial8250_timeout is for issue 1 at the top, this is for
issue 2.
> also?
> What IRQ numbers this UART is using, BTW?
For the ports on the device they are 27 and 42. Is there any significance
that I'm not aware of?
> > diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> > index cf23813..bd9711a 100644
> > --- a/include/linux/serial_core.h
> > +++ b/include/linux/serial_core.h
> > @@ -276,6 +277,7 @@ #define UPF_USR_MASK ((__force upf_t) (
> > struct device *dev; /* parent
> device */
> > unsigned char hub6; /* this should
> be in the 8250 driver */
> > unsigned char unused[3];
> > + void *private_data; /*
> generic platform data pointer */
>
> One tab to many before *private_data...
In my editor using 4 columns per tab it lines up with everything. Is there
some convention that patches should be made assuming 8 columns?
> > diff --git a/include/linux/serial_reg.h b/include/linux/serial_reg.h
> > index 3c8a6aa..1c5ed7d 100644
> > --- a/include/linux/serial_reg.h
> > +++ b/include/linux/serial_reg.h
> > @@ -38,6 +38,8 @@ #define UART_IIR_THRI 0x02 /* Transmitt
> > #define UART_IIR_RDI 0x04 /* Receiver data interrupt */
> > #define UART_IIR_RLSI 0x06 /* Receiver line status
> interrupt */
> >
> > +#define UART_IIR_BUSY 0x07 /* DesignWare APB Busy
> Detect */
> > +
> > #define UART_FCR 2 /* Out: FIFO Control Register */
> > #define UART_FCR_ENABLE_FIFO 0x01 /* Enable the FIFO */
> > #define UART_FCR_CLEAR_RCVR 0x02 /* Clear the RCVR FIFO */
>
> Oops, your mailer went and did it again. :-)
I'm completely giving up on Thunderbird,unless someone can point out
the specific internal configuration items which needs a kick!
Thanks,
Marc
next reply other threads:[~2007-02-09 19:39 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-02-09 19:39 Marc St-Jean [this message]
2007-02-10 17:30 ` [PATCH] serial driver PMC MSP71xx, kernel linux-mips.git mast er Sergei Shtylyov
-- strict thread matches above, loose matches on Subject: below --
2007-02-16 17:06 Marc St-Jean
2007-02-16 17:21 ` Sergei Shtylyov
2007-02-16 16:30 Marc St-Jean
2007-02-13 22:11 Marc St-Jean
2007-02-12 17:46 Marc St-Jean
2007-02-12 17:56 ` Sergei Shtylyov
2007-02-07 20:50 Marc St-Jean
2007-02-06 16:52 Marc St-Jean
2007-01-27 0:28 Marc St-Jean
2007-01-25 23:10 Marc St-Jean
2007-01-26 13:58 ` Sergei Shtylyov
2007-01-24 21:10 Marc St-Jean
2007-01-25 14:29 ` Sergei Shtylyov
2007-01-24 16:48 Marc St-Jean
2007-01-24 20:38 ` Sergei Shtylyov
2007-01-24 16:40 Marc St-Jean
2007-01-24 19:40 ` Sergei Shtylyov
2007-01-23 22:37 Marc St-Jean
2007-01-23 23:41 ` Alan
2007-01-24 15:33 ` Sergei Shtylyov
2007-01-22 20:35 Marc St-Jean
2007-01-22 20:34 Marc St-Jean
2007-01-22 19:07 Marc St-Jean
2007-01-22 18:11 Marc St-Jean
2007-01-22 18:11 ` Marc St-Jean
2007-01-22 18:11 ` Marc St-Jean
2007-01-22 20:23 ` Sergei Shtylyov
2007-01-24 15:25 ` Sergei Shtylyov
2007-01-19 18:40 Marc St-Jean
2007-01-19 17:33 Marc St-Jean
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=45CCCDD4.7030104@pmc-sierra.com \
--to=marc_st-jean@pmc-sierra.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mips@linux-mips.org \
--cc=linux-serial@vger.kernel.org \
--cc=sshtylyov@ru.mvista.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.