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 ?
>
>> + /* Enable retrain for gen2 */
>> + rockchip_pcie_ep_retrain_link(rockchip);
>> + readl_poll_timeout(rockchip->apb_base + PCIE_CORE_CTRL,
>> + val, PCIE_LINK_IS_GEN2(val), 50,
>> + LINK_TRAIN_TIMEOUT);
>> + }
>> +
>> + /* Check again that the link is up */
>> + if (!rockchip_pcie_ep_link_up(rockchip))
>> + goto again;
>
> TRM doesn't mention this check. Is this really necessary?
I think so, to check the result of the second training for gen2.
Even though the TRM does not say so, I prefer checking that the result is what
we expect: the link is up.
--
Damien Le Moal
Western Digital Research
