On 06/06/2011 12:22 PM, Rafał Miłecki wrote:

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.

At first I named it bcma_host_bcma_ but then renamed the file name, but
forgot the function names. I will rename it all to bcma_host_soc_*
host_sco.c and so on.

2011/6/6 Hauke Mehrtens [off-list ref]:

quoted

--- /dev/null
+++ b/drivers/bcma/host_embedded.c

@@ -0,0 +1,93 @@
+/*
+ * Broadcom specific AMBA
+ * PCI Host

s/PCI/Embedded/

quoted

+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(...);
? :)

yes makes sens.

quoted

+       /* 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.

yes

quoted

+
+       /* If we are an embedded device we now know the number of avaliable
+        * core and ioremap the correct space.
+        */

Typo: avaliable

my favorite typo ;-)

quoted

+       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]?

These are two different memory regions, it should be possible to
calculate the other address, but I do not like that. As host_embedded is
in a union this does not waste any memory.

quoted

+       }
+       /* 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.

No they are different eromptr is 0x1810e000 and bus->mmio is 0xb8000000
for example on my device. I tried using eromptr = bus->mmio; on embedded
and it did not work.

Hauke