Re: [PATCH v3 10/12] PCI: rockchip-ep: Improve link training

[Damien Le Moal <dlemoal@kernel.org>]

On 10/12/24 21:16, Manivannan Sadhasivam wrote:
> On Fri, Oct 11, 2024 at 05:55:25PM +0900, Damien Le Moal wrote:
>> On 10/10/24 19:35, Manivannan Sadhasivam wrote:
>>>> +static void rockchip_pcie_ep_link_training(struct work_struct *work)
>>>> +{
>>>> +	struct rockchip_pcie_ep *ep =
>>>> +		container_of(work, struct rockchip_pcie_ep, link_training.work);
>>>> +	struct rockchip_pcie *rockchip = &ep->rockchip;
>>>> +	struct device *dev = rockchip->dev;
>>>> +	u32 val;
>>>> +	int ret;
>>>> +
>>>> +	/* Enable Gen1 training and wait for its completion */
>>>> +	ret = readl_poll_timeout(rockchip->apb_base + PCIE_CORE_CTRL,
>>>> +				 val, PCIE_LINK_TRAINING_DONE(val), 50,
>>>> +				 LINK_TRAIN_TIMEOUT);
>>>> +	if (ret)
>>>> +		goto again;
>>>> +
>>>> +	/* Make sure that the link is up */
>>>> +	ret = readl_poll_timeout(rockchip->apb_base + PCIE_CLIENT_BASIC_STATUS1,
>>>> +				 val, PCIE_LINK_UP(val), 50,
>>>> +				 LINK_TRAIN_TIMEOUT);
>>>> +	if (ret)
>>>> +		goto again;
>>>> +
>>>> +	/* Check the current speed */
>>>> +	val = rockchip_pcie_read(rockchip, PCIE_CORE_CTRL);
>>>> +	if (!PCIE_LINK_IS_GEN2(val) && rockchip->link_gen == 2) {
>>>
>>> PCIE_LINK_IS_GEN2()?
>>
>> This is defined in drivers/pci/controller/pcie-rockchip.h. What is it exactly
>> you would like to know about this ?
>>
> 
> !PCIE_LINK_IS_GEN2 means check is for non-Gen2 mode, isn't it? I guess the check
> should be 'if (PCIE_LINK_IS_GEN2...)

Nope, the negative test is correct. The condition means: if we are not at GEN2
speed yet AND gen2 was requested, then initiate training again to get gen2.
So !PCIE_LINK_IS_GEN2() is correct.

-- 
Damien Le Moal
Western Digital Research