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
