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... > 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. > Signed-off-by: Damien Le Moal <dlemoal@xxxxxxxxxx> > --- > drivers/pci/controller/pcie-rockchip-ep.c | 11 ++++++----- > drivers/pci/controller/pcie-rockchip.c | 5 +++-- > 2 files changed, 9 insertions(+), 7 deletions(-) > > diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c > index 7a1798fcc2ad..99f26f4a485b 100644 > --- a/drivers/pci/controller/pcie-rockchip-ep.c > +++ b/drivers/pci/controller/pcie-rockchip-ep.c > @@ -459,6 +459,12 @@ static int rockchip_pcie_ep_start(struct pci_epc *epc) > > rockchip_pcie_write(rockchip, cfg, PCIE_CORE_PHY_FUNC_CFG); > > + /* Enable configuration and start link training */ > + rockchip_pcie_write(rockchip, > + PCIE_CLIENT_LINK_TRAIN_ENABLE | > + PCIE_CLIENT_CONF_ENABLE, > + PCIE_CLIENT_CONFIG); > + > return 0; > } > > @@ -537,7 +543,6 @@ static int rockchip_pcie_ep_get_resources(struct rockchip_pcie_ep *ep) > > ep->ob_addr = devm_kcalloc(dev, ep->max_regions, sizeof(*ep->ob_addr), > GFP_KERNEL); > - Spurious change. - Mani > if (!ep->ob_addr) > return -ENOMEM; > > @@ -648,10 +653,6 @@ static int rockchip_pcie_ep_probe(struct platform_device *pdev) > > rockchip_pcie_ep_hide_msix_cap(rockchip); > > - /* Establish the link automatically */ > - rockchip_pcie_write(rockchip, PCIE_CLIENT_LINK_TRAIN_ENABLE, > - PCIE_CLIENT_CONFIG); > - > /* Only enable function 0 by default */ > rockchip_pcie_write(rockchip, BIT(0), PCIE_CORE_PHY_FUNC_CFG); > > diff --git a/drivers/pci/controller/pcie-rockchip.c b/drivers/pci/controller/pcie-rockchip.c > index c07d7129f1c7..154e78819e6e 100644 > --- a/drivers/pci/controller/pcie-rockchip.c > +++ b/drivers/pci/controller/pcie-rockchip.c > @@ -244,11 +244,12 @@ int rockchip_pcie_init_port(struct rockchip_pcie *rockchip) > rockchip_pcie_write(rockchip, PCIE_CLIENT_GEN_SEL_1, > PCIE_CLIENT_CONFIG); > > - regs = PCIE_CLIENT_LINK_TRAIN_ENABLE | PCIE_CLIENT_ARI_ENABLE | > + regs = PCIE_CLIENT_ARI_ENABLE | > PCIE_CLIENT_CONF_LANE_NUM(rockchip->lanes); > > if (rockchip->is_rc) > - regs |= PCIE_CLIENT_CONF_ENABLE | PCIE_CLIENT_MODE_RC; > + regs |= PCIE_CLIENT_LINK_TRAIN_ENABLE | > + PCIE_CLIENT_CONF_ENABLE | PCIE_CLIENT_MODE_RC; > else > regs |= PCIE_CLIENT_CONF_DISABLE | PCIE_CLIENT_MODE_EP; > > -- > 2.46.2 > -- மணிவண்ணன் சதாசிவம்
