Re: [PATCH] PCI: exynos: remove redundant code in exynos_pcie_establish_link

From: "Jingoo Han" <jingoohan1@gmail.com>
To: "'Pankaj Dubey'" <pankaj.dubey@samsung.com>,
	"'Krzysztof Kozlowski'" <krzk@kernel.org>
Cc: 'Bjorn Helgaas' <bhelgaas@google.com>,
	'linux-pci' <linux-pci@vger.kernel.org>,
	linux-samsung-soc@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	'Anvesh Salveru' <anvesh.s@samsung.com>
Subject: Re: [PATCH] PCI: exynos: remove redundant code in exynos_pcie_establish_link
Date: Thu, 21 Dec 2017 11:31:36 -0500	[thread overview]
Message-ID: <000101d37a79$2bdb4c20$8391e460$@gmail.com> (raw)
In-Reply-To: <b81a09be-1873-ab81-dcb2-c3b29ff7e430@samsung.com>

On Tuesday, October 10, 2017 9:46 AM, Pankaj Dubey wrote:
> 
> Hi Jingoo,
> 
> 
> On 10/09/2017 09:50 PM, Jingoo Han wrote:
> > On Monday, October 9, 2017 10:44 AM, Krzysztof Kozlowski wrote:
> >> On Mon, Oct 9, 2017 at 4:14 PM, Pankaj Dubey <pankaj.dubey@samsung.com>
> >> wrote:
> >>> From: Anvesh Salveru <anvesh.s@samsung.com>
> >>>
> >>> In exynos_pcie_establish_link if driver is not using generic phy,
> >>> we are resetting PHY twice, which is redundant, so this patch removes
> >> Hi Pankaj,
> >>
> >> This lacks the information why it is redundant.
> > (I resend this mail, because email address of pci list was corrupted.)
> Thanks, somehow I typed wrong email id.
> > I think so, too.
> >
> > Did you test this code on some boards with Exynos PCIe?
> > Or did hardware engineers confirm this?
> > Please add more information on this patch.
> I have replied reason behind this patch in reply to Krzysztof, hope I am
> able to
> explain logic behind this change.
> 
> I do not have access to Exynos5440 PCIe, and this PHY_COMMON_RESET is not
> applicable to other Exynos SoC which I have with me, so I can't test
> this change,
> but if you see the change it is an obvious change, before introducing
> generic phy
> support to this driver PHY_COMMON_RESET was programmed only once, then
> in case platform is not using PHY it suppose to be done only once during
> linkup.
> I am not sure when Jaehoon introduced this patch, he verified this on
> Exynos5440 or
> not. We are just trying to make the logic as it was before without
> affecting anything.
> 
> Thanks,
> Pankaj Dubey
> > Best regards,
> > Jingoo Han
> >
> >>> repeated lines of code for PHY reset.
> >>>
> >>> Signed-off-by: Anvesh Salveru <anvesh.s@samsung.com>
> >> Your Signed-off-by is needed here.

Sorry for being late.
I checked that this patch is right.

Can you send this patch again with your Signed-off-by?
Also, you can add my Acked-by to your new patch.

Acked-by: Jingoo Han <jingoohan1@gmail.com>

Best regards,
Jingoo Han

> >>
> >> Best regards,
> >> Krzysztof
> >>
> >>> ---
> >>>   drivers/pci/dwc/pci-exynos.c | 7 -------
> >>>   1 file changed, 7 deletions(-)
> >>>
> >>> diff --git a/drivers/pci/dwc/pci-exynos.c b/drivers/pci/dwc/pci-
> exynos.c
> >>> index 5596fde..85d2f4b 100644
> >>> --- a/drivers/pci/dwc/pci-exynos.c
> >>> +++ b/drivers/pci/dwc/pci-exynos.c
> >>> @@ -423,13 +423,6 @@ static int exynos_pcie_establish_link(struct
> >> exynos_pcie *ep)
> >>>                  exynos_pcie_deassert_phy_reset(ep);
> >>>                  exynos_pcie_power_on_phy(ep);
> >>>                  exynos_pcie_init_phy(ep);
> >>> -
> >>> -               /* pulse for common reset */
> >>> -               exynos_pcie_writel(ep->mem_res->block_base, 1,
> >>> -                                       PCIE_PHY_COMMON_RESET);
> >>> -               udelay(500);
> >>> -               exynos_pcie_writel(ep->mem_res->block_base, 0,
> >>> -                                       PCIE_PHY_COMMON_RESET);
> >>>          }
> >>>
> >>>          /* pulse for common reset */
> >>> --
> >>> 2.7.4
> >>>
> >
> >
> >
> >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel