Re: [PATCH V3] pci: exynos: split into two parts such as Synopsys part and Exynos part

[Kishon Vijay Abraham I <kishon@ti.com>]

Hi,

On Tuesday 23 July 2013 06:44 AM, Jingoo Han wrote:
> On Tuesday, July 23, 2013 12:04 AM, Kishon Vijay Abraham I wrote:
>> On Thursday 18 July 2013 10:51 AM, Jingoo Han wrote:
>>> Exynos PCIe IP consists of Synopsys specific part and Exynos
>>> specific part. Only core block is a Synopsys designware part;
>>> other parts are Exynos specific.
>>> Also, the Synopsys designware part can be shared with other
>>> platforms; thus, it can be split two parts such as Synopsys
>>> designware part and Exynos specific part.
>>
>> some more queries and comments..
> 
.
.
<snip>
.
.
>>> +			of_pci_range_to_resource(&range, np, &pp->cfg);
>>> +			pp->config.cfg0_size = resource_size(&pp->cfg)/2;
>>> +			pp->config.cfg1_size = resource_size(&pp->cfg)/2;
>>> +		}
>>> +	}
>>> +
>>> +	pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
>>> +				resource_size(&pp->cfg));
>>
>> Why is configuraion space divided into two?
> 
> Sorry, I don't know the exact reason. :(
> Pratyush Anand may know about this.
> Pratyush Anand, could you answer the question?
> 
> Also, if you find some problems, please let me know.
> 
> 
>> Why should it be same as dbi_base?
>> AFAIK, jacinto6 has a dedicated configuration/io/memory space that is entirely
>> different from dbi_base.
> 
> According to the Synopsys designware PCIe datasheet,
> in chapter 5.1.1 Register Space Layout,
> 'Port Logic Registers' are placed between (config space 0x0 + 0x700)
> and (config space 0x0 + 0xFFF).
> 'dbi_base' is used for reading/writing 'Port Logic Registers'.
> Exynos are using 'dbi_base' like this. Thus, the base addresses of
> both 'dbi_base' and configuration/io/memory space are same.
> 
> Just now, I looked at Spear PCIe driver.
> However, in the case of Spear, the base address of configuration/io/memory
> space is defined as 0x80000000. The base address of 'Port Logic Registers'
> is defined as 0xb1000000.
> I think that the situation of 'jacinto6' is similar with Spear, right?
> 
> Then, I will move pp->dbi_base mapping code from pcie-designware.c
> to pci-exynos.c.

I think you need not move this to exynos (since registers in dbi_base is common
for all platforms) but modify the pcie-designware.c to use different address
space for dbi_base. In your case, you'll duplicate the address space for
dbi_base and configuration space.

Also I have one more query.
In your dt binding, your pci address and cpu address is the same. But the pci
address should start at 0x00000000 and end at 0xffffffff (for 32bit). Shouldn't
the cpu address map to something within this range of pci address?

Thanks
Kishon