Hauke,
My idea for naming schema was to use:
bcma_host_TYPE_*
Like:
bcma_host_pci_*
bcma_host_sdio_*
You are using:
bcma_host_bcma_*
What do you think about changing this to:
bcma_host_embedded_*
or just some:
bcma_host_emb_*
?
Does it make more sense to you? I was trying to keep names in bcma
really clear, so every first-time-reader can see differences between
hosts, host and driver, etc.
2011/6/6 Hauke Mehrtens <hauke@xxxxxxxxxx>:
> --- /dev/null
> +++ b/drivers/bcma/host_embedded.c
> @@ -0,0 +1,93 @@
> +/*
> + * Broadcom specific AMBA
> + * PCI Host
s/PCI/Embedded/
> +int bcma_host_bcma_register(struct bcma_bus *bus)
> +{
> + Â Â Â u32 __iomem *mmio;
> + Â Â Â /* iomap only first core. We have to read some register on this core
> + Â Â Â Â* to get the number of cores. This is sone in bcma_scan()
> + Â Â Â Â*/
> + Â Â Â mmio = ioremap(BCMA_ADDR_BASE, BCMA_CORE_SIZE * 1);
> + Â Â Â if (!mmio)
> + Â Â Â Â Â Â Â return -ENOMEM;
> + Â Â Â bus->mmio = mmio;
Maybe just:
bus->mmio = ioremap(...);
? :)
> + Â Â Â /* Host specific */
> + Â Â Â bus->hosttype = BCMA_HOSTTYPE_EMBEDDED;
> + Â Â Â bus->ops = &bcma_host_bcma_ops;
> +
> + Â Â Â /* Register */
> + Â Â Â return bcma_bus_register(bus);
> +}
> diff --git a/drivers/bcma/main.c b/drivers/bcma/main.c
> index 1afa107..c5bcb5f 100644
> --- a/drivers/bcma/main.c
> +++ b/drivers/bcma/main.c
> @@ -119,6 +119,7 @@ static int bcma_register_cores(struct bcma_bus *bus)
> Â Â Â Â Â Â Â Â Â Â Â Âbreak;
> Â Â Â Â Â Â Â Âcase BCMA_HOSTTYPE_NONE:
> Â Â Â Â Â Â Â Âcase BCMA_HOSTTYPE_SDIO:
> + Â Â Â Â Â Â Â case BCMA_HOSTTYPE_EMBEDDED:
> Â Â Â Â Â Â Â Â Â Â Â Âbreak;
> Â Â Â Â Â Â Â Â}
>
> diff --git a/drivers/bcma/scan.c b/drivers/bcma/scan.c
> index 70b39f7..9229615 100644
> --- a/drivers/bcma/scan.c
> +++ b/drivers/bcma/scan.c
> @@ -203,7 +203,7 @@ static s32 bcma_erom_get_addr_desc(struct bcma_bus *bus, u32 **eromptr,
> Âint bcma_bus_scan(struct bcma_bus *bus)
> Â{
> Â Â Â Âu32 erombase;
> - Â Â Â u32 __iomem *eromptr, *eromend;
> + Â Â Â u32 __iomem *eromptr, *eromend, *mmio;
>
> Â Â Â Âs32 cia, cib;
> Â Â Â Âu8 ports[2], wrappers[2];
> @@ -219,9 +219,34 @@ int bcma_bus_scan(struct bcma_bus *bus)
> Â Â Â Âbus->chipinfo.id = (tmp & BCMA_CC_ID_ID) >> BCMA_CC_ID_ID_SHIFT;
> Â Â Â Âbus->chipinfo.rev = (tmp & BCMA_CC_ID_REV) >> BCMA_CC_ID_REV_SHIFT;
> Â Â Â Âbus->chipinfo.pkg = (tmp & BCMA_CC_ID_PKG) >> BCMA_CC_ID_PKG_SHIFT;
> + Â Â Â bus->nr_cores = (tmp & BCMA_CC_ID_NRCORES) >> BCMA_CC_ID_NRCORES_SHIFT;
I'd use different variable as Julian suggested.
> +
> + Â Â Â /* If we are an embedded device we now know the number of avaliable
> + Â Â Â Â* core and ioremap the correct space.
> + Â Â Â Â*/
Typo: avaliable
> + Â Â Â if (bus->hosttype == BCMA_HOSTTYPE_EMBEDDED) {
> + Â Â Â Â Â Â Â iounmap(bus->mmio);
> + Â Â Â Â Â Â Â mmio = ioremap(BCMA_ADDR_BASE, BCMA_CORE_SIZE * bus->nr_cores);
> + Â Â Â Â Â Â Â if (!mmio)
> + Â Â Â Â Â Â Â Â Â Â Â return -ENOMEM;
> + Â Â Â Â Â Â Â bus->mmio = mmio;
> +
> + Â Â Â Â Â Â Â mmio = ioremap(BCMA_WRAP_BASE, BCMA_CORE_SIZE * bus->nr_cores);
> + Â Â Â Â Â Â Â if (!mmio)
> + Â Â Â Â Â Â Â Â Â Â Â return -ENOMEM;
> + Â Â Â Â Â Â Â bus->host_embedded = mmio;
Do we really need both? mmio and host_embedded? What about keeping
mmio only and using it in calculation for read/write[8,16,32]?
> + Â Â Â }
> + Â Â Â /* reset it to 0 as we use it for counting */
> + Â Â Â bus->nr_cores = 0;
>
> Â Â Â Âerombase = bcma_scan_read32(bus, 0, BCMA_CC_EROM);
> - Â Â Â eromptr = bus->mmio;
> + Â Â Â if (bus->hosttype == BCMA_HOSTTYPE_EMBEDDED) {
> + Â Â Â Â Â Â Â eromptr = ioremap(erombase, BCMA_CORE_SIZE);
> + Â Â Â Â Â Â Â if (!eromptr)
> + Â Â Â Â Â Â Â Â Â Â Â return -ENOMEM;
> + Â Â Â } else
> + Â Â Â Â Â Â Â eromptr = bus->mmio;
I though eromptr = bus->mmio; will do the trick for embedded as well.
I think I need some time to read about IO mapping and understand that.
--
RafaÅ
