On 06/07/2011 02:33 AM, Rafał Miłecki wrote:

W dniu 7 czerwca 2011 00:00 użytkownik Hauke Mehrtens
[off-list ref]  napisał:

quoted

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

quoted

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.

Ah, OK, I can see what does happen here. You are using:
1) bus->mmio for first core
2) bus->host_embedded for first agent/wrapper

I'm not sure if this is a correct approach. Doing "core_index *
BCMA_CORE_SIZE" comes from ssb, where it was the way to calculate
offset. In case of BCMA we are reading all the info from (E)EPROM,
which also includes addresses of the cores.

IMO you should use core->addr and core->wrap for read/write ops. I
believe this is approach Broadcom decided to use for BCMA, when
designing (E)EPROM.

Agree. There is no guarantee for the core index to relate to the 
physical address. Chip designer may be systematic in this and the 
index*size method may work, but not by design.

Gr. AvS

-- 
Almost nobody dances sober, unless they happen to be insane.
-- H.P. Lovecraft --