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

[Pratyush Anand <pratyush.anand@st.com>]

Hi Jingoo,


On Thu, Sep 12, 2013 at 03:15:04PM +0800, Jingoo Han wrote:
> On Tuesday 23 July 2013 12:30 PM, Kishon Vijay Abraham I wrote:
> > >> .
> > >> .
> > >>>>> +			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.
> > 
> > One more query..
> > 
> > Where is inbound translation configuration done in your driver? how should it
> > be done?
> 

Yes, Kishon is right. Inbound translation configuration is missing in
your code and I think it should be implemented.

> Hi Kishon,
> 
> Sorry, I cannot understand your question exactly.
> However, the following thread would be helpful.
> 
> http://www.spinics.net/lists/arm-kernel/msg252078.html
> https://lkml.org/lkml/2013/6/17/890

>From this conversation, It seems that you
have tested this driver and it works fine without inbound translation
function. I am sure that you would have tested a PCIe card with DMA
capability such as PCIe2USB or PCIe2Ethernet. Since it worked, it
means that by default your controller is supporting one to one mapping
in case of inbound transaction even when address translation is enabled.

In my opinion you should call a function like as follows from
dw_pcie_host_init in pcie-designware.c.  It will insure one to one
mapping for any inbound request in memory range 0 to (in_mem_size -
1) for all dw implementation.

static void dw_pcie_prog_viewport_mem_inbound(struct pcie_port *pp)
{        
	u32 val;
	void __iomem *dbi_base = pp->dbi_base;

	/* Program viewport 0 : INBOUND : MEMORY*/
	val = PCIE_ATU_REGION_INBOUND | (0 & 0xF);
	dw_pcie_writel_rc(pp, val, dbi_base + PCIE_ATU_VIEWPORT));
	dw_pcie_writel_rc(pp, PCIE_ATU_TYPE_MEM, dbi_base + PCIE_ATU_CR1));
	val = PCIE_ATU_ENABLE | PCIE_ATU_BAR_MODE_ENABLE;
	dw_pcie_writel_rc(pp, val, dbi_base + PCIE_ATU_CR2));
	dw_pcie_writel_rc(pp, 0, dbi_base + PCIE_ATU_LOWER_BASE));
	dw_pcie_writel_rc(pp, 0, dbi_base + PCIE_ATU_UPPER_BASE));
	/* in_mem_size must be in power of 2 */
	dw_pcie_writel_rc(pp, pp->config.in_mem_size - 1, dbi_base + PCIE_ATU_LIMIT));
	dw_pcie_writel_rc(pp, 0, dbi_base + PCIE_ATU_LOWER_TARGET));
	dw_pcie_writel_rc(pp, 0, dbi_base + PCIE_ATU_UPPER_TARGET));
}

Regards
Pratyush

> 
> Best regards.
> Jingoo Han