Re: [PATCH v6 4/5] PCI: add PCI controller for keystone PCIe h/w

[Murali Karicheri <m-karicheri2@ti.com>]

On 07/20/2014 09:44 PM, Jingoo Han wrote:
> On Saturday, July 19, 2014 5:29 AM, Murali Karicheri wrote:
>> On 07/18/2014 03:31 PM, Rob Herring wrote:
>>> On Fri, Jul 18, 2014 at 10:14 AM, Murali Karicheri<m-karicheri2@ti.com>   wrote:
>> --- Cut ---
>>>> +
>>>> +Optional properties:-
>>>> +       phys: phandle to Generic Keystone SerDes phy for PCI
>>>> +       phy-names: name of the Generic Keystine SerDes phy for PCI
>>>> +         - If boot loader already does PCI link establishment, then phys and
>>>> +           phy-names shouldn't be present.
>>>> +       ti,enable-linktrain - Enable Link training.
>>>> +         - If boot loader already does PCI link establishment, then this
>>>> +           shouldn't be present.
>>>
>>> Can't you read from the h/w if the link is trained?
>
> I agree with Rob Herring's suggestion.
>
>>
>> Yes.
>>
>> There are customers who use this driver with PCI Link setup done in the
>> boot loader. This property tells the driver to bypass Link setup
>> procedure in that case. Is this undesirable and if so. how other
>> platforms handle it? Check first if link is trained or start the link
>> setup procedure? Let me know. If this is fine, please provide your Ack.
>
> Please, check the following code of Exynos PCIe diver.
>
> ./drivers/pci/host/pci-exynos.c
>
> static int exynos_pcie_establish_link(struct pcie_port *pp)
> {
> 	struct exynos_pcie *exynos_pcie = to_exynos_pcie(pp);
> 	void __iomem *elbi_base = exynos_pcie->elbi_base;
> 	void __iomem *pmu_base = exynos_pcie->pmu_base;
>
> 	if (dw_pcie_link_up(pp)) {
> 		dev_err(pp->dev, "Link already up\n");
> 		return 0;
> 	}
> 	.....
>
> In the case of Exynos PCIe, the Exynos PCIe driver checks the
> h/w bit such as PCIE_ELBI_LTSSM_ENABLE bit of PCIE_ELBI_RDLH_LINKUP
> offset register.
>
> If the link is already set up by the boot loader or other reasons,
> the driver will skip some initialization codes.
>
> The first step is that you find such h/w bit for checking link up.
> If so, please add the code for skipping, when the link is already
> set up.
>
Rob, Jingoo,

We have similar bit to check for Link status and I have removed the DT 
property and skip Link retrain if PCIe Link is already Up. I will be 
resending the series with Patch 4/5 updated.

Thanks.

Murali

> Best regards,
> Jingoo Han
>