On 10/10/24 17:22, Manivannan Sadhasivam wrote: > On Mon, Oct 07, 2024 at 01:12:14PM +0900, Damien Le Moal wrote: >> The function rockchip_pcie_init_port() enables link training for a >> controller configured in EP mode. Enabling link training is again done >> in rockchip_pcie_ep_probe() after that function executed >> rockchip_pcie_init_port(). Enabling link training only needs to be done >> once, and doing so at the probe stage before the controller is actually >> started by the user serves no purpose. >> > > I hope that the dual enablement is done as a mistake and not on purpose... Yes, I think that was a mistake, likely when the EP mode support was added. >> Refactor this by removing the link training enablement from both >> rockchip_pcie_init_port() and rockchip_pcie_ep_probe() and moving it to >> the endpoint start operation defined with rockchip_pcie_ep_start(). >> Enabling the controller configuration using the PCIE_CLIENT_CONF_ENABLE >> bit in the same PCIE_CLIENT_CONFIG register is also move to >> rockchip_pcie_ep_start() and both the controller configuration and link >> training enable bits are set with a single call to >> rockchip_pcie_write(). >> > > But you didn't remove the existing code in probe() that sets > PCIE_CLIENT_CONF_ENABLE. Indeed. Removed. It does not seem to hurt though. -- Damien Le Moal Western Digital Research
