On 9 July 2011 13:05, Hauke Mehrtens <hauke@xxxxxxxxxx> wrote: > This adds support for serial console to bcma, when operating on an SoC. > > Signed-off-by: Hauke Mehrtens <hauke@xxxxxxxxxx> > --- > drivers/bcma/bcma_private.h | 6 +++ > drivers/bcma/driver_chipcommon.c | 64 +++++++++++++++++++++++++++++++++ > drivers/bcma/driver_mips.c | 9 +++++ > include/linux/bcma/bcma_driver_mips.h | 11 ++++++ > 4 files changed, 90 insertions(+), 0 deletions(-) > > diff --git a/drivers/bcma/bcma_private.h b/drivers/bcma/bcma_private.h > index 830386c..92ec671 100644 > --- a/drivers/bcma/bcma_private.h > +++ b/drivers/bcma/bcma_private.h > @@ -26,6 +26,12 @@ int __init bcma_bus_scan_early(struct bcma_bus *bus, > struct bcma_device *core); > void bcma_init_bus(struct bcma_bus *bus); > > +/* driver_chipcommon.c */ > +#ifdef CONFIG_BCMA_DRIVER_MIPS > +extern int bcma_chipco_serial_init(struct bcma_drv_cc *cc, > + struct bcma_drv_mips_serial_port *ports); > +#endif /* CONFIG_BCMA_DRIVER_MIPS */ > + > #ifdef CONFIG_BCMA_HOST_PCI > /* host_pci.c */ > extern int __init bcma_host_pci_init(void); > diff --git a/drivers/bcma/driver_chipcommon.c b/drivers/bcma/driver_chipcommon.c > index 70321c6..88533ca 100644 > --- a/drivers/bcma/driver_chipcommon.c > +++ b/drivers/bcma/driver_chipcommon.c > @@ -92,3 +92,67 @@ u32 bcma_chipco_gpio_polarity(struct bcma_drv_cc *cc, u32 mask, u32 value) > { > return bcma_cc_write32_masked(cc, BCMA_CC_GPIOPOL, mask, value); > } > + > +#ifdef CONFIG_BCMA_DRIVER_MIPS > +int bcma_chipco_serial_init(struct bcma_drv_cc *cc, > + struct bcma_drv_mips_serial_port *ports) > +{ > + int nr_ports = 0; > + u32 plltype; > + unsigned int irq; > + u32 baud_base, div; > + u32 i, n; > + unsigned int ccrev = cc->core->id.rev; > + > + plltype = (cc->capabilities & BCMA_CC_CAP_PLLT); > + irq = bcma_core_mips_irq(cc->core); > + > + if ((ccrev >= 11) && (ccrev != 15) && (ccrev != 20)) { > + /* Fixed ALP clock */ > + baud_base = 20000000; > + if (cc->capabilities & BCMA_CC_CAP_PMU) { > + /* FIXME: baud_base is different for devices with a PMU */ > + WARN_ON(1); > + } > + div = 1; > + if (ccrev >= 21) { > + /* Turn off UART clock before switching clocksource. */ > + bcma_cc_write32(cc, BCMA_CC_CORECTL, > + bcma_cc_read32(cc, BCMA_CC_CORECTL) > + & ~BCMA_CC_CORECTL_UARTCLKEN); > + } > + /* Set the override bit so we don't divide it */ > + bcma_cc_write32(cc, BCMA_CC_CORECTL, > + bcma_cc_read32(cc, BCMA_CC_CORECTL) > + | BCMA_CC_CORECTL_UARTCLK0); > + if (ccrev >= 21) { > + /* Re-enable the UART clock. */ > + bcma_cc_write32(cc, BCMA_CC_CORECTL, > + bcma_cc_read32(cc, BCMA_CC_CORECTL) > + | BCMA_CC_CORECTL_UARTCLKEN); > + } > + } else > + pr_err("serial not supported on this device ccrev: 0x%x\n", > + ccrev); Documentation/CodingStyle and again ;-) > + > + /* Determine the registers of the UARTs */ > + n = (cc->capabilities & BCMA_CC_CAP_NRUART); > + for (i = 0; i < n; i++) { > + void __iomem *cc_mmio; > + void __iomem *uart_regs; > + > + cc_mmio = cc->core->bus->mmio + > + (cc->core->core_index * BCMA_CORE_SIZE); cc_mmio is constant for all uarts, so you should move it out of the loop and calculate it once. > + uart_regs = cc_mmio + BCMA_CC_UART0_DATA; > + uart_regs += (i * 256); Same for the uart_regs base. If you don't modify it at all you could drop the cc_mmio variable (since you only need it to calculate uart_regs) and > + > + nr_ports++; > + ports[i].regs = uart_regs; use here ports[i].regs = uart_regs + (i * 256); This would make the code a bit cleaner IMHO. > + ports[i].irq = irq; > + ports[i].baud_base = baud_base; > + ports[i].reg_shift = 0; > + } > + > + return nr_ports; Isn't n always the same as nr_ports? At least I don't see any case where it could differ, so you should be safe when directly using nr_ports instead of n. ��.n��������+%������w��{.n�����{���zW����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f