| Message ID | 1507558478-3218-1-git-send-email-pankaj.dubey@samsung.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
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. > repeated lines of code for PHY reset. > > Signed-off-by: Anvesh Salveru <anvesh.s@samsung.com> Your Signed-off-by is needed here. 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 > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 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. 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. > > 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 > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.) 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. 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. > > 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 > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/09/2017 08:13 PM, 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. Sure. Ok probably commit message should have been covered more information. I can see this patch tries to fix redundancy added by patch [1]: [1] e7cd7ef58e1f "PCI: exynos: Support the PHY generic framework" Before adding support for the generic PHY, exynos5440 was doing PHY reset via /* 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); But when Jaehoon introduced generic PHY support, above two lines have been copied in else part (else case covers: if platform does not uses generic PHY), so eventually we will end-up doing PHY reset twice. For more clarity copy pasting part of above patch: ------------------------------------ exynos_pcie_assert_core_reset(ep); - exynos_pcie_assert_phy_reset(ep); - exynos_pcie_deassert_phy_reset(ep); - exynos_pcie_power_on_phy(ep); - exynos_pcie_init_phy(ep); + + if (ep->using_phy) { + phy_reset(ep->phy); + + exynos_pcie_writel(ep->mem_res->elbi_base, 1, + PCIE_PWR_RESET); + + phy_power_on(ep->phy); + phy_init(ep->phy); + } else { + exynos_pcie_assert_phy_reset(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 */ 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); ----------------------------------------------- So if you see, eventually now we will end up doing common reset twice in case platform not using generic PHY (once in else part and immediately after else). We have submitted this patch without verifying on actual Exynos5440 board, as currently we do not have access to this board. But I feel in the original patch this flow has been changed, that too probably without verifying on Exynos5440 board (Probably as I came across Exynos5433 PCIe patch of Jaehoon where he mentioned he also do not have access to Exynos5440 board). So this patch removes that double reset of PHY and makes the code flow as it was originally, before introduction of generic PHY. Thanks, Pankaj Dubey >> repeated lines of code for PHY reset. >> >> Signed-off-by: Anvesh Salveru <anvesh.s@samsung.com> > Your Signed-off-by is needed here. I missed it, will add in v2. > > 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 >> > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. >> >> 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 >>> > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 > >>> > > > > > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Jingoo, On 12/21/2017 10:01 PM, Jingoo Han wrote: > 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> Thanks for review and ack. Will resubmit the patch along with my Signed-off-by and your Acked-by. Thanks, Pankaj Dubey > 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 >>>>> >>> >>> >>> > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Pankaj, On 12/28/2017 06:50 PM, Pankaj Dubey wrote: > Hi Jingoo, > > > On 12/21/2017 10:01 PM, Jingoo Han wrote: >> 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> > > Thanks for review and ack. > Will resubmit the patch along with my Signed-off-by and your Acked-by. Maybe it will be removed with my patch. https://patchwork.ozlab.org/patch/853119/ Because of my personal reason, i can't send the email to Bjorn's account..so i removed his account from cc. Sorry. Best Regards, Jaehoon Chung > > Thanks, > Pankaj Dubey >> 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 >>>>>> >>>> >>>> >>>> >> >> >> >> > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Jaehoon, On 12/28/2017 03:28 PM, Jaehoon Chung wrote: > Hi Pankaj, > > On 12/28/2017 06:50 PM, Pankaj Dubey wrote: >> Hi Jingoo, >> >> >> On 12/21/2017 10:01 PM, Jingoo Han wrote: >>> 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> >> Thanks for review and ack. >> Will resubmit the patch along with my Signed-off-by and your Acked-by. > Maybe it will be removed with my patch. > > https://patchwork.ozlab.org/patch/853119/ > > Because of my personal reason, i can't send the email to Bjorn's account..so i removed his account from cc. > Sorry. As per submission date, we have submitted it first, so it should be taken in that order, but I I am fine with any of the patch, so I will leave this on Bjorn. Thanks, Pankaj Dubey > Best Regards, > Jaehoon Chung > >> Thanks, >> Pankaj Dubey >>> 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 >>>>>>> >>>>> >>>>> >>> >>> >>> >> >> >> > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 */