Re: [PATCH v3 08/12] PCI: rockchip-ep: Refactor endpoint link training enable

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

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