On 04/05/2012 02:22 PM, Jiri Svoboda wrote:
Hi Adrian,
nice! Thanks for sending the patch. I have two nits:
- the patch seems to use CRLF line endings. I know we don't mention
this anywhere, but I am not sure what would happen if somebody missed
this and tried to apply it on a Linux box :-/
Patch has been prepared on Linux box and it has LF line endings for
sure. I've just double-checked it.
- never *EVER* declare a register structure as packed. You were lucky
here because all the members are just 8 bits wide, but if there were
16-bit or 32-bit fields, chaos would ensue.
Why? A packed structure means not just that the fields are packed
together without padding for natural alignment. It also means that the
address of the structure itself in memory need not be aligned to the
size of its largest element. So, GCC will generate code to read/write
any field byte-by-byte instead. Of course this works only for memory,
not for registers!
I've attached modified patch with __attribute__ ((packed)) removed. But
I wonder why is it present in for example i8042_regs_t in i8042 driver?
Regards,
Adrian.
=== modified file 'uspace/drv/char/ns8250/ns8250.c'
--- uspace/drv/char/ns8250/ns8250.c 2011-11-14 20:50:08 +0000
+++ uspace/drv/char/ns8250/ns8250.c 2012-04-05 12:33:01 +0000
@@ -73,6 +73,58 @@
#define MAX_BAUD_RATE 115200
#define DLAB_MASK (1 << 7)
+/** Interrupt Enable Register definition. */
+#define NS8250_IER_RXREADY (1 << 0)
+#define NS8250_IER_THRE (1 << 1)
+#define NS8250_IER_RXSTATUS (1 << 2)
+#define NS8250_IER_MODEM_STATUS (1 << 3)
+
+/** Interrupt ID Register definition. */
+#define NS8250_IID_ACTIVE (1 << 0)
+
+/** FIFO Control Register definition. */
+#define NS8250_FCR_FIFOENABLE (1 << 0)
+#define NS8250_FCR_RXFIFORESET (1 << 1)
+#define NS8250_FCR_TXFIFORESET (1 << 2)
+#define NS8250_FCR_DMAMODE (1 << 3)
+#define NS8250_FCR_RXTRIGGERLOW (1 << 6)
+#define NS8250_FCR_RXTRIGGERHI (1 << 7)
+
+/** Line Control Register definition. */
+#define NS8250_LCR_STOPBITS (1 << 2)
+#define NS8250_LCR_PARITY (1 << 3)
+#define NS8250_LCR_SENDBREAK (1 << 6)
+#define NS8250_LCR_DLAB (1 << 7)
+
+/** Modem Control Register definition. */
+#define NS8250_MCR_DTR (1 << 0)
+#define NS8250_MCR_RTS (1 << 1)
+#define NS8250_MCR_OUT1 (1 << 2)
+#define NS8250_MCR_OUT2 (1 << 3)
+#define NS8250_MCR_LOOPBACK (1 << 4)
+#define NS8250_MCR_ALL (0x1f)
+
+/** Line Status Register definition. */
+#define NS8250_LSR_RXREADY (1 << 0)
+#define NS8250_LSR_OE (1 << 1)
+#define NS8250_LSR_PE (1 << 2)
+#define NS8250_LSR_FE (1 << 3)
+#define NS8250_LSR_BREAK (1 << 4)
+#define NS8250_LSR_THRE (1 << 5)
+#define NS8250_LSR_TSE (1 << 6)
+
+/** Modem Status Register definition. */
+#define NS8250_MSR_DELTACTS (1 << 0)
+#define NS8250_MSR_DELTADSR (1 << 1)
+#define NS8250_MSR_RITRAILING (1 << 2)
+#define NS8250_MSR_DELTADCD (1 << 3)
+#define NS8250_MSR_CTS (1 << 4)
+#define NS8250_MSR_DSR (1 << 5)
+#define NS8250_MSR_RI (1 << 6)
+#define NS8250_MSR_DCD (1 << 7)
+#define NS8250_MSR_SIGNALS (NS8250_MSR_CTS | NS8250_MSR_DSR \
+ | NS8250_MSR_RI | NS8250_MSR_DCD)
+
/** Obtain soft-state structure from function node */
#define NS8250(fnode) ((ns8250_t *) ((fnode)->dev->driver_data))
@@ -95,12 +147,25 @@
TWO_STOP_BITS
} stop_bit_t;
+/** 8250 UART registers layout. */
+typedef struct {
+ ioport8_t data; /**< Data register. */
+ ioport8_t ier; /**< Interrupt Enable Reg. */
+ ioport8_t iid; /**< Interrupt ID Reg. */
+ ioport8_t lcr; /**< Line Control Reg. */
+ ioport8_t mcr; /**< Modem Control Reg. */
+ ioport8_t lsr; /**< Line Status Reg. */
+ ioport8_t msr; /**< Modem Status Reg. */
+} ns8250_regs_t;
+
/** The driver data for the serial port devices. */
typedef struct ns8250 {
/** DDF device node */
ddf_dev_t *dev;
/** DDF function node */
ddf_fun_t *fun;
+ /** I/O registers **/
+ ns8250_regs_t *regs;
/** Is there any client conntected to the device? */
bool client_connected;
/** The irq assigned to this device. */
@@ -123,9 +188,9 @@
* @return True if there are data waiting to be read, false
* otherwise.
*/
-static bool ns8250_received(ioport8_t *port)
+static bool ns8250_received(ns8250_regs_t *regs)
{
- return (pio_read_8(port + 5) & 1) != 0;
+ return (pio_read_8(®s->lsr) & NS8250_LSR_RXREADY) != 0;
}
/** Read one byte from the serial port.
@@ -133,18 +198,18 @@
* @param port The base address of the serial port device's ports.
* @return The data read.
*/
-static uint8_t ns8250_read_8(ioport8_t *port)
+static uint8_t ns8250_read_8(ns8250_regs_t *regs)
{
- return pio_read_8(port);
+ return pio_read_8(®s->data);
}
/** Find out wheter it is possible to send data.
*
* @param port The base address of the serial port device's ports.
*/
-static bool is_transmit_empty(ioport8_t *port)
+static bool is_transmit_empty(ns8250_regs_t *regs)
{
- return (pio_read_8(port + 5) & 0x20) != 0;
+ return (pio_read_8(®s->lsr) & NS8250_LSR_THRE) != 0;
}
/** Write one character on the serial port.
@@ -152,12 +217,12 @@
* @param port The base address of the serial port device's ports.
* @param c The character to be written to the serial port device.
*/
-static void ns8250_write_8(ioport8_t *port, uint8_t c)
+static void ns8250_write_8(ns8250_regs_t *regs, uint8_t c)
{
- while (!is_transmit_empty(port))
+ while (!is_transmit_empty(regs))
;
- pio_write_8(port, c);
+ pio_write_8(®s->data, c);
}
/** Read data from the serial port device.
@@ -192,7 +257,7 @@
static inline void ns8250_putchar(ns8250_t *ns, uint8_t c)
{
fibril_mutex_lock(&ns->mutex);
- ns8250_write_8(ns->port, c);
+ ns8250_write_8(ns->regs, c);
fibril_mutex_unlock(&ns->mutex);
}
@@ -265,6 +330,8 @@
" for device %s.", ns->io_addr, ns->dev->name);
return false;
}
+
+ ns->regs = (ns8250_regs_t *)ns->port;
return true;
}
@@ -278,21 +345,21 @@
{
ddf_msg(LVL_DEBUG, "ns8250_dev_probe %s", ns->dev->name);
- ioport8_t *port_addr = ns->port;
bool res = true;
uint8_t olddata;
- olddata = pio_read_8(port_addr + 4);
-
- pio_write_8(port_addr + 4, 0x10);
- if (pio_read_8(port_addr + 6) & 0xf0)
- res = false;
-
- pio_write_8(port_addr + 4, 0x1f);
- if ((pio_read_8(port_addr + 6) & 0xf0) != 0xf0)
- res = false;
-
- pio_write_8(port_addr + 4, olddata);
+ olddata = pio_read_8(&ns->regs->mcr);
+
+ pio_write_8(&ns->regs->mcr, NS8250_MCR_LOOPBACK);
+ if (pio_read_8(&ns->regs->msr) & NS8250_MSR_SIGNALS)
+ res = false;
+
+ pio_write_8(&ns->regs->mcr, NS8250_MCR_ALL);
+ if ((pio_read_8(&ns->regs->msr) & NS8250_MSR_SIGNALS)
+ != NS8250_MSR_SIGNALS)
+ res = false;
+
+ pio_write_8(&ns->regs->mcr, olddata);
if (!res) {
ddf_msg(LVL_DEBUG, "Device %s is not present.",
@@ -389,19 +456,21 @@
*
* @param port The base address of the serial port device's ports.
*/
-static inline void ns8250_port_interrupts_enable(ioport8_t *port)
+static inline void ns8250_port_interrupts_enable(ns8250_regs_t *regs)
{
- pio_write_8(port + 1, 0x1); /* Interrupt when data received. */
- pio_write_8(port + 4, 0xB);
+ /* Interrupt when data received. */
+ pio_write_8(®s->ier, NS8250_IER_RXREADY);
+ pio_write_8(®s->mcr, NS8250_MCR_DTR | NS8250_MCR_RTS
+ | NS8250_MCR_OUT2);
}
/** Disable interrupts on the serial port device.
*
* @param port The base address of the serial port device's ports
*/
-static inline void ns8250_port_interrupts_disable(ioport8_t *port)
+static inline void ns8250_port_interrupts_disable(ns8250_regs_t *regs)
{
- pio_write_8(port + 1, 0x0); /* Disable all interrupts. */
+ pio_write_8(®s->ier, 0x0); /* Disable all interrupts. */
}
/** Enable interrupts for the serial port device.
@@ -430,7 +499,7 @@
async_exchange_end(exch);
/* Enable interrupt on the serial port. */
- ns8250_port_interrupts_enable(ns->port);
+ ns8250_port_interrupts_enable(ns->regs);
return EOK;
}
@@ -442,20 +511,20 @@
*
* @param port The base address of the serial port device's ports.
*/
-static inline void enable_dlab(ioport8_t *port)
+static inline void enable_dlab(ns8250_regs_t *regs)
{
- uint8_t val = pio_read_8(port + 3);
- pio_write_8(port + 3, val | DLAB_MASK);
+ uint8_t val = pio_read_8(®s->lcr);
+ pio_write_8(®s->lcr, val | NS8250_LCR_DLAB);
}
/** Clear Divisor Latch Access Bit.
*
* @param port The base address of the serial port device's ports.
*/
-static inline void clear_dlab(ioport8_t *port)
+static inline void clear_dlab(ns8250_regs_t *regs)
{
- uint8_t val = pio_read_8(port + 3);
- pio_write_8(port + 3, val & (~DLAB_MASK));
+ uint8_t val = pio_read_8(®s->lcr);
+ pio_write_8(®s->lcr, val & (~NS8250_LCR_DLAB));
}
/** Set baud rate of the serial communication on the serial device.
@@ -465,7 +534,7 @@
* @return Zero on success, negative error number otherwise (EINVAL
* if the specified baud_rate is not valid).
*/
-static int ns8250_port_set_baud_rate(ioport8_t *port, unsigned int baud_rate)
+static int ns8250_port_set_baud_rate(ns8250_regs_t *regs, unsigned int baud_rate)
{
uint16_t divisor;
uint8_t div_low, div_high;
@@ -481,14 +550,14 @@
div_high = (uint8_t)(divisor >> 8);
/* Enable DLAB to be able to access baud rate divisor. */
- enable_dlab(port);
+ enable_dlab(regs);
/* Set divisor low byte. */
- pio_write_8(port + 0, div_low);
+ pio_write_8(®s->data, div_low);
/* Set divisor high byte. */
- pio_write_8(port + 1, div_high);
+ pio_write_8(®s->ier, div_high);
- clear_dlab(port);
+ clear_dlab(regs);
return EOK;
}
@@ -498,20 +567,20 @@
* @param port The base address of the serial port device's ports.
* @param baud_rate The ouput parameter to which the baud rate is stored.
*/
-static unsigned int ns8250_port_get_baud_rate(ioport8_t *port)
+static unsigned int ns8250_port_get_baud_rate(ns8250_regs_t *regs)
{
uint16_t divisor;
uint8_t div_low, div_high;
/* Enable DLAB to be able to access baud rate divisor. */
- enable_dlab(port);
+ enable_dlab(regs);
/* Get divisor low byte. */
- div_low = pio_read_8(port + 0);
+ div_low = pio_read_8(®s->data);
/* Get divisor high byte. */
- div_high = pio_read_8(port + 1);
+ div_high = pio_read_8(®s->ier);
- clear_dlab(port);
+ clear_dlab(regs);
divisor = (div_high << 8) | div_low;
return MAX_BAUD_RATE / divisor;
@@ -524,13 +593,13 @@
* @param word_length The length of one data unit in bits.
* @param stop_bits The number of stop bits used (one or two).
*/
-static void ns8250_port_get_com_props(ioport8_t *port, unsigned int *parity,
+static void ns8250_port_get_com_props(ns8250_regs_t *regs, unsigned int *parity,
unsigned int *word_length, unsigned int *stop_bits)
{
uint8_t val;
- val = pio_read_8(port + 3);
- *parity = ((val >> 3) & 7);
+ val = pio_read_8(®s->lcr);
+ *parity = ((val >> NS8250_LCR_PARITY) & 7);
switch (val & 3) {
case WORD_LENGTH_5:
@@ -547,7 +616,7 @@
break;
}
- if ((val >> 2) & 1)
+ if ((val >> NS8250_LCR_STOPBITS) & 1)
*stop_bits = 2;
else
*stop_bits = 1;
@@ -561,7 +630,7 @@
* @return Zero on success, EINVAL if some of the specified values
* is invalid.
*/
-static int ns8250_port_set_com_props(ioport8_t *port, unsigned int parity,
+static int ns8250_port_set_com_props(ns8250_regs_t *regs, unsigned int parity,
unsigned int word_length, unsigned int stop_bits)
{
uint8_t val;
@@ -585,10 +654,10 @@
switch (stop_bits) {
case 1:
- val |= ONE_STOP_BIT << 2;
+ val |= ONE_STOP_BIT << NS8250_LCR_STOPBITS;
break;
case 2:
- val |= TWO_STOP_BITS << 2;
+ val |= TWO_STOP_BITS << NS8250_LCR_STOPBITS;
break;
default:
return EINVAL;
@@ -600,13 +669,13 @@
case SERIAL_EVEN_PARITY:
case SERIAL_MARK_PARITY:
case SERIAL_SPACE_PARITY:
- val |= parity << 3;
+ val |= parity << NS8250_LCR_PARITY;
break;
default:
return EINVAL;
}
- pio_write_8(port + 3, val);
+ pio_write_8(®s->lcr, val);
return EOK;
}
@@ -619,21 +688,22 @@
*/
static void ns8250_initialize_port(ns8250_t *ns)
{
- ioport8_t *port = ns->port;
-
/* Disable interrupts. */
- ns8250_port_interrupts_disable(port);
+ ns8250_port_interrupts_disable(ns->regs);
/* Set baud rate. */
- ns8250_port_set_baud_rate(port, 38400);
+ ns8250_port_set_baud_rate(ns->regs, 38400);
/* 8 bits, no parity, two stop bits. */
- ns8250_port_set_com_props(port, SERIAL_NO_PARITY, 8, 2);
+ ns8250_port_set_com_props(ns->regs, SERIAL_NO_PARITY, 8, 2);
/* Enable FIFO, clear them, with 14-byte threshold. */
- pio_write_8(port + 2, 0xC7);
+ pio_write_8(&ns->regs->iid, NS8250_FCR_FIFOENABLE
+ | NS8250_FCR_RXFIFORESET | NS8250_FCR_TXFIFORESET
+ | NS8250_FCR_RXTRIGGERLOW | NS8250_FCR_RXTRIGGERHI);
/*
* RTS/DSR set (Request to Send and Data Terminal Ready lines enabled),
* Aux Output2 set - needed for interrupts.
*/
- pio_write_8(port + 4, 0x0B);
+ pio_write_8(&ns->regs->mcr, NS8250_MCR_DTR | NS8250_MCR_RTS
+ | NS8250_MCR_OUT2);
}
/** Deinitialize the serial port device.
@@ -643,11 +713,11 @@
static void ns8250_port_cleanup(ns8250_t *ns)
{
/* Disable FIFO */
- pio_write_8(ns->port + 2, 0x00);
+ pio_write_8(&ns->regs->iid, 0x00);
/* Disable DTR, RTS, OUT1, OUT2 (int. enable) */
- pio_write_8(ns->port + 4, 0x00);
+ pio_write_8(&ns->regs->mcr, 0x00);
/* Disable all interrupts from the port */
- ns8250_port_interrupts_disable(ns->port);
+ ns8250_port_interrupts_disable(ns->regs);
}
/** Read the data from the serial port device and store them to the input
@@ -657,15 +727,15 @@
*/
static void ns8250_read_from_device(ns8250_t *ns)
{
- ioport8_t *port = ns->port;
+ ns8250_regs_t *regs = ns->regs;
bool cont = true;
while (cont) {
fibril_mutex_lock(&ns->mutex);
- cont = ns8250_received(port);
+ cont = ns8250_received(regs);
if (cont) {
- uint8_t val = ns8250_read_8(port);
+ uint8_t val = ns8250_read_8(regs);
if (ns->client_connected) {
if (!buf_push_back(&ns->input_buffer, val)) {
@@ -895,13 +965,13 @@
unsigned int *word_length, unsigned int* stop_bits)
{
ns8250_t *data = (ns8250_t *) dev->driver_data;
- ioport8_t *port = data->port;
+ ns8250_regs_t *regs = data->regs;
fibril_mutex_lock(&data->mutex);
- ns8250_port_interrupts_disable(port);
- *baud_rate = ns8250_port_get_baud_rate(port);
- ns8250_port_get_com_props(port, parity, word_length, stop_bits);
- ns8250_port_interrupts_enable(port);
+ ns8250_port_interrupts_disable(regs);
+ *baud_rate = ns8250_port_get_baud_rate(regs);
+ ns8250_port_get_com_props(regs, parity, word_length, stop_bits);
+ ns8250_port_interrupts_enable(regs);
fibril_mutex_unlock(&data->mutex);
ddf_msg(LVL_DEBUG, "ns8250_get_props: baud rate %d, parity 0x%x, word "
@@ -926,15 +996,15 @@
stop_bits);
ns8250_t *data = (ns8250_t *) dev->driver_data;
- ioport8_t *port = data->port;
+ ns8250_regs_t *regs = data->regs;
int ret;
fibril_mutex_lock(&data->mutex);
- ns8250_port_interrupts_disable(port);
- ret = ns8250_port_set_baud_rate(port, baud_rate);
+ ns8250_port_interrupts_disable(regs);
+ ret = ns8250_port_set_baud_rate(regs, baud_rate);
if (ret == EOK)
- ret = ns8250_port_set_com_props(port, parity, word_length, stop_bits);
- ns8250_port_interrupts_enable(port);
+ ret = ns8250_port_set_com_props(regs, parity, word_length, stop_bits);
+ ns8250_port_interrupts_enable(regs);
fibril_mutex_unlock(&data->mutex);
return ret;
_______________________________________________
HelenOS-devel mailing list
[email protected]
http://lists.modry.cz/cgi-bin/listinfo/helenos-devel
