On 4 August 2011 23:37, Jean-Christophe PLAGNIOL-VILLARD
<plagnioj@xxxxxxxxxxxx> wrote:
> --- a/drivers/serial/serial_ns16550.c
> +++ b/drivers/serial/serial_ns16550.c
> @@ -48,6 +48,55 @@
>
> /*********** Private Functions **********************************/
>
> +/*
> + * We wrap our port structure around the generic console_device.
> + */
> +struct ns16550_uart_port {
> + void __iomem *base;
> + uint32_t shift;
> + uint32_t clock;
> + uint32_t (*read)(void __iomem *base, uint8_t off);
> + void (*write)(uint32_t val, void __iomem *base, uint8_t off);
> +
> + struct console_device uart;
nice trick :)))
But why the name is 'uart', not 'cdev'?
> +};
As a rule, structure declarations go to header files.
> +
> +static uint32_t ns16550_readb(void __iomem *base, uint8_t off)
> +{
> + return readb(base + off);
warning: pointer of type ‘void *’ used in arithmetic
> -static unsigned int ns16550_calc_divisor(struct console_device *cdev,
> +static unsigned int ns16550_calc_divisor(struct ns16550_uart_port *uart,
> unsigned int baudrate)
> {
> - struct NS16550_plat *plat = (struct NS16550_plat *)
> - cdev->dev->platform_data;
> - unsigned int clk = plat->clock;
> -
> - return (clk / MODE_X_DIV / baudrate);
> -
> + return (uart->clock / MODE_X_DIV / baudrate);
> }
inline?
> + if (!plat->reg_read) {
> + switch (width) {
> + case IORESOURCE_MEM_8BIT:
> + uart->read = ns16550_readb;
> + break;
> + case IORESOURCE_MEM_16BIT:
> + uart->read = ns16550_readw;
> + break;
> + case IORESOURCE_MEM_32BIT:
> + uart->read = ns16550_readl;
> + break;
> + }
> + } else {
> + uart->read = plat->reg_read;
> + }
In this if the 'else' block is very short. 'if' block is much longer.
Swapping of them will improve readability.
> + if (!plat->reg_write) {
> + switch (width) {
> + case IORESOURCE_MEM_8BIT:
> + uart->write = ns16550_writeb;
> + break;
> + case IORESOURCE_MEM_16BIT:
> + uart->write = ns16550_writew;
> + break;
> + case IORESOURCE_MEM_32BIT:
> + uart->write = ns16550_writel;
> + break;
> + }
> + } else {
> + uart->write = plat->reg_write;
> + }
why do it twice?
--
Best regards,
Antony Pavlov
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
