On 2017/8/23 9:03, Shawn Lin wrote: > Hi Bjorn, > > On 2017/8/23 4:47, Bjorn Helgaas wrote: >> On Wed, Aug 09, 2017 at 07:14:23PM +0800, Shawn Lin wrote: >>> We observed that the clk_pciephy_ref is still enabled when we actually >>> fail to probe the driver. >>> >>> root at linaro-alip:~# cat /sys/kernel/debug/clk/clk_summary | grep pcie >>> clk_pciephy_ref????????????? 1??? 1???? 24000000??? 0 0 >>> clk_pcie_pm????????????? 0??? 0???? 24000000??? 0 0 >>> ?????? clk_pcie_core_cru?????????? 0??? 0??? 125000000??? 0 0 >>> ?????? clk_pciephy_ref100m???????? 0??? 0??? 100000000??? 0 0 >>> ??????? aclk_pcie???????? 0??? 0??? 148500000??? 0 0 >>> ??????? aclk_perf_pcie??? 0??? 0??? 148500000??? 0 0 >>> ??????????? pclk_pcie 0??? 0???? 37125000??? 0 0 >>> clk_pcie_core???????????????????? 0??? 0??????? 0??? 0 0 >>> >>> clk_pciephy_ref is used by phy driver and we need to properly disable >>> it for this case. So this patch add error handle for the function of >>> rockchip_pcie_init_port to fix this issue. >>> >>> Signed-off-by: Shawn Lin <shawn.lin at rock-chips.com> >> >> Applied to pci/host-rockchip for v4.14, thanks! > > I actually posted a v2 sometime ago, so could you please drop this one? > > And there are still some other error handling path should be fixed. > So I think that is what Jeffy's patch did: > > http://patchwork.ozlabs.org/patch/804239/ > Just talk to Jeffy off-list and we agree that there are many patches for pcie-rockchip in flight that conflict each other. So I would like to pick up all these patches, including the bug-fixing for shared irq and reconstruction, and send out the a bigger series which would make you easy to review and avoid rebasing pathes for others. > >> >>> --- >>> >>> ? drivers/pci/host/pcie-rockchip.c | 38 >>> ++++++++++++++++++++++++-------------- >>> ? 1 file changed, 24 insertions(+), 14 deletions(-) >>> >>> diff --git a/drivers/pci/host/pcie-rockchip.c >>> b/drivers/pci/host/pcie-rockchip.c >>> index 2eccd53..ce6371b 100644 >>> --- a/drivers/pci/host/pcie-rockchip.c >>> +++ b/drivers/pci/host/pcie-rockchip.c >>> @@ -561,32 +561,32 @@ static int rockchip_pcie_init_port(struct >>> rockchip_pcie *rockchip) >>> ????????? err = phy_init(rockchip->phys[i]); >>> ????????? if (err) { >>> ????????????? dev_err(dev, "init phy%d err %d\n", i, err); >>> -??????????? return err; >>> +??????????? goto err_phy_exit; >>> ????????? } >>> ????? } >>> ????? err = reset_control_assert(rockchip->core_rst); >>> ????? if (err) { >>> ????????? dev_err(dev, "assert core_rst err %d\n", err); >>> -??????? return err; >>> +??????? goto err_phy_exit; >>> ????? } >>> ????? err = reset_control_assert(rockchip->mgmt_rst); >>> ????? if (err) { >>> ????????? dev_err(dev, "assert mgmt_rst err %d\n", err); >>> -??????? return err; >>> +??????? goto err_phy_exit; >>> ????? } >>> ????? err = reset_control_assert(rockchip->mgmt_sticky_rst); >>> ????? if (err) { >>> ????????? dev_err(dev, "assert mgmt_sticky_rst err %d\n", err); >>> -??????? return err; >>> +??????? goto err_phy_exit; >>> ????? } >>> ????? err = reset_control_assert(rockchip->pipe_rst); >>> ????? if (err) { >>> ????????? dev_err(dev, "assert pipe_rst err %d\n", err); >>> -??????? return err; >>> +??????? goto err_phy_exit; >>> ????? } >>> ????? udelay(10); >>> @@ -594,19 +594,19 @@ static int rockchip_pcie_init_port(struct >>> rockchip_pcie *rockchip) >>> ????? err = reset_control_deassert(rockchip->pm_rst); >>> ????? if (err) { >>> ????????? dev_err(dev, "deassert pm_rst err %d\n", err); >>> -??????? return err; >>> +??????? goto err_phy_exit; >>> ????? } >>> ????? err = reset_control_deassert(rockchip->aclk_rst); >>> ????? if (err) { >>> ????????? dev_err(dev, "deassert aclk_rst err %d\n", err); >>> -??????? return err; >>> +??????? goto err_phy_exit; >>> ????? } >>> ????? err = reset_control_deassert(rockchip->pclk_rst); >>> ????? if (err) { >>> ????????? dev_err(dev, "deassert pclk_rst err %d\n", err); >>> -??????? return err; >>> +??????? goto err_phy_exit; >>> ????? } >>> ????? if (rockchip->link_gen == 2) >>> @@ -628,7 +628,7 @@ static int rockchip_pcie_init_port(struct >>> rockchip_pcie *rockchip) >>> ????????? err = phy_power_on(rockchip->phys[i]); >>> ????????? if (err) { >>> ????????????? dev_err(dev, "power on phy%d err %d\n", i, err); >>> -??????????? return err; >>> +??????????? goto err_phy_power_off; >>> ????????? } >>> ????? } >>> @@ -639,25 +639,25 @@ static int rockchip_pcie_init_port(struct >>> rockchip_pcie *rockchip) >>> ????? err = reset_control_deassert(rockchip->mgmt_sticky_rst); >>> ????? if (err) { >>> ????????? dev_err(dev, "deassert mgmt_sticky_rst err %d\n", err); >>> -??????? return err; >>> +??????? goto err_phy_power_off; >>> ????? } >>> ????? err = reset_control_deassert(rockchip->core_rst); >>> ????? if (err) { >>> ????????? dev_err(dev, "deassert core_rst err %d\n", err); >>> -??????? return err; >>> +??????? goto err_phy_power_off; >>> ????? } >>> ????? err = reset_control_deassert(rockchip->mgmt_rst); >>> ????? if (err) { >>> ????????? dev_err(dev, "deassert mgmt_rst err %d\n", err); >>> -??????? return err; >>> +??????? goto err_phy_power_off; >>> ????? } >>> ????? err = reset_control_deassert(rockchip->pipe_rst); >>> ????? if (err) { >>> ????????? dev_err(dev, "deassert pipe_rst err %d\n", err); >>> -??????? return err; >>> +??????? goto err_phy_power_off; >>> ????? } >>> ????? /* Fix the transmitted FTS count desired to exit from L0s. */ >>> @@ -690,7 +690,8 @@ static int rockchip_pcie_init_port(struct >>> rockchip_pcie *rockchip) >>> ?????????????????? 500 * USEC_PER_MSEC); >>> ????? if (err) { >>> ????????? dev_err(dev, "PCIe link training gen1 timeout!\n"); >>> -??????? return -ETIMEDOUT; >>> +??????? err = -ETIMEDOUT; >>> +??????? goto err_phy_power_off; >>> ????? } >>> ????? if (rockchip->link_gen == 2) { >>> @@ -751,6 +752,15 @@ static int rockchip_pcie_init_port(struct >>> rockchip_pcie *rockchip) >>> ????? rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_DCSR); >>> ????? return 0; >>> + >>> +err_phy_exit: >>> +??? for (; i >= 0; --i) >>> +??????? phy_exit(rockchip->phys[i]); >>> +err_phy_power_off: >>> +??? for (; i >= 0; --i) >>> +??????? phy_power_off(rockchip->phys[i]); >>> + >>> +??? return err; >>> ? } >>> ? static irqreturn_t rockchip_pcie_subsys_irq_handler(int irq, void >>> *arg) >>> -- >>> 1.9.1 >>> >>> >> >> >> > > >
