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.
