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