[PATCH 2.6.12-rc3] serial: update NEC VR4100 series serial support
From: Yoichi Yuasa
Date: Sat Apr 23 2005 - 08:52:11 EST
Hi,
I had fixed Russell's comment.
On Wed, 30 Mar 2005 10:23:08 +0100
Russell King <rmk@xxxxxxxxxxxxxxxx> wrote:
> On Mon, Mar 07, 2005 at 09:20:22PM -0800, akpm@xxxxxxxx wrote:
> > +static inline const char *siu_type_name(struct uart_port *port)
> > +{
> > + switch (port->type) {
> > + case PORT_VR41XX_SIU:
> > + return "SIU";
> > + case PORT_VR41XX_DSIU:
> > + return "DSIU";
> > + }
> > +
> > + return "unknown";
>
> You can return NULL here.
Changed.
> > +static irqreturn_t siu_interrupt(int irq, void *dev_id, struct pt_regs *regs)
> > +{
> > + struct uart_port *port;
> > + uint8_t iir, lsr;
> > +
> > + if (dev_id == NULL)
> > + return IRQ_NONE;
>
> This driver always registers interrupts with a dev_id. Therefore, this can
> never be NULL.
Removed.
>
> > +static void siu_shutdown(struct uart_port *port)
> > +{
> > + unsigned long flags;
> > + uint8_t lcr;
> > +
> > + if (port->membase == NULL)
> > + return;
>
> How can this happen? We always shut down a port before we change the
> address. Maybe this check should be in siu_startup instead?
This check moved to siu_startup.
> > +#ifdef CONFIG_SERIAL_VR41XX_CONSOLE
> > +
> > +static void early_set_termios(struct uart_port *port, struct termios *new,
> > + struct termios *old)
> > +{
<snip>
> > +}
> > +
> > +static struct uart_ops early_uart_ops = {
> > + .set_termios = early_set_termios,
> > +};
>
> Eww. Is there a reason why your standard termios setting function can't
> be used?
Now using my standerd termios. It changed.
> > +static int siu_probe(struct device *dev)
> > +{
> > + struct uart_port *port;
> > + int num, i, retval;
> > +
> > + num = siu_init_ports();
> > + if (num <= 0)
> > + return -ENODEV;
> > +
> > + siu_uart_driver.nr = num;
> > + retval = uart_register_driver(&siu_uart_driver);
> > + if (retval)
> > + return retval;
> > +
> > + for (i = 0; i < num; i++) {
> > + port = &siu_uart_ports[i];
> > + port->ops = &siu_uart_ops;
> > + port->dev = dev;
> > +
> > + retval = uart_add_one_port(&siu_uart_driver, port);
> > + if (retval)
> > + break;
> > + }
>
> If port 1 fails to be registered...
>
> > +
> > + if (i == 0 && retval < 0) {
> > + uart_unregister_driver(&siu_uart_driver);
> > + return retval;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int siu_remove(struct device *dev)
> > +{
> > + struct uart_port *port;
> > + int i;
> > +
> > + for (i = 0; i < siu_uart_driver.nr; i++) {
> > + port = &siu_uart_ports[i];
> > + if (port->dev == dev) {
>
> ... we try to unregister it here, because port->dev was initialised.
> Maybe you want to NULL out port->dev in the above function when that
> happens?
Fixed.
> On Wed, Apr 13, 2005 at 11:18:27PM +0900, Yoichi Yuasa wrote:
> > static struct uart_ops early_uart_ops = {
> > - .set_termios = early_set_termios,
> > + .set_termios = siu_set_termios,
> > };
>
> In this case, you don't need the early_uart_ops here - the standard
> ones will do just as well. (.set_termios is the only method which
> the console init code will use.)
Use standard one.
Yoichi
Signed-off-by: Yoichi Yuasa <yuasa@xxxxxxxxxxxxxx>
Index: drivers/serial/vr41xx_siu.c
===================================================================
--- 43765d4b40a4a3c64f2a32b684f4041e2f01644d/drivers/serial/vr41xx_siu.c (mode:100644 sha1:307886199f2f6f1301d6aa240b1f3263e9ec8a90)
+++ uncommitted/drivers/serial/vr41xx_siu.c (mode:100644)
@@ -234,7 +234,7 @@
return "DSIU";
}
- return "unknown";
+ return NULL;
}
static unsigned int siu_tx_empty(struct uart_port *port)
@@ -484,9 +484,6 @@
struct uart_port *port;
uint8_t iir, lsr;
- if (dev_id == NULL)
- return IRQ_NONE;
-
port = (struct uart_port *)dev_id;
iir = siu_read(port, UART_IIR);
@@ -509,6 +506,9 @@
{
int retval;
+ if (port->membase == NULL)
+ return -ENODEV;
+
siu_clear_fifo(port);
(void)siu_read(port, UART_LSR);
@@ -547,9 +547,6 @@
unsigned long flags;
uint8_t lcr;
- if (port->membase == NULL)
- return;
-
siu_write(port, UART_IER, 0);
spin_lock_irqsave(&port->lock, flags);
@@ -804,53 +801,6 @@
#ifdef CONFIG_SERIAL_VR41XX_CONSOLE
-static void early_set_termios(struct uart_port *port, struct termios *new,
- struct termios *old)
-{
- tcflag_t c_cflag;
- uint8_t lcr;
- unsigned int baud, quot;
-
- c_cflag = new->c_cflag;
- switch (c_cflag & CSIZE) {
- case CS5:
- lcr = UART_LCR_WLEN5;
- break;
- case CS6:
- lcr = UART_LCR_WLEN6;
- break;
- case CS7:
- lcr = UART_LCR_WLEN7;
- break;
- default:
- lcr = UART_LCR_WLEN8;
- break;
- }
-
- if (c_cflag & CSTOPB)
- lcr |= UART_LCR_STOP;
- if (c_cflag & PARENB)
- lcr |= UART_LCR_PARITY;
- if ((c_cflag & PARODD) != PARODD)
- lcr |= UART_LCR_EPAR;
- if (c_cflag & CMSPAR)
- lcr |= UART_LCR_SPAR;
-
- baud = uart_get_baud_rate(port, new, old, 0, port->uartclk/16);
- quot = uart_get_divisor(port, baud);
-
- siu_write(port, UART_LCR, lcr | UART_LCR_DLAB);
-
- siu_write(port, UART_DLL, (uint8_t)quot);
- siu_write(port, UART_DLM, (uint8_t)(quot >> 8));
-
- siu_write(port, UART_LCR, lcr);
-}
-
-static struct uart_ops early_uart_ops = {
- .set_termios = early_set_termios,
-};
-
#define BOTH_EMPTY (UART_LSR_TEMT | UART_LSR_THRE)
static void wait_for_xmitr(struct uart_port *port)
@@ -917,7 +867,7 @@
if (port->membase == NULL) {
if (port->mapbase == 0)
return -ENODEV;
- port->membase = (unsigned char __iomem *)KSEG1ADDR(port->mapbase);
+ port->membase = ioremap(port->mapbase, siu_port_size(port));
}
vr41xx_select_siu_interface(SIU_INTERFACE_RS232C);
@@ -951,7 +901,7 @@
for (i = 0; i < num; i++) {
port = &siu_uart_ports[i];
- port->ops = &early_uart_ops;
+ port->ops = &siu_uart_ops;
}
register_console(&siu_console);
@@ -996,8 +946,10 @@
port->dev = dev;
retval = uart_add_one_port(&siu_uart_driver, port);
- if (retval)
+ if (retval < 0) {
+ port->dev = NULL;
break;
+ }
}
if (i == 0 && retval < 0) {
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/