Re: [RFC][PATCH 03/10] bcma: add embedded bus

["Arend van Spriel" <arend@broadcom.com>]

On 06/07/2011 02:33 AM, Rafał Miłecki wrote:
> W dniu 7 czerwca 2011 00:00 użytkownik Hauke Mehrtens
> <hauke@hauke-m.de>  napisał:
>> On 06/06/2011 12:22 PM, Rafał Miłecki wrote:
>>>> +       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 --