Hi Kukjin,
> Lukasz Majewski wrote:
> >
> > This patch supports to control usb otg phy of EXYNOS4210.
> >
> > Signed-off-by: Joonyoung Shim <jy0922.shim@xxxxxxxxxxx>
> > Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
> > [Rebased on the newest git/kgene/linux-samsung #for-next]
> > Signed-off-by: Lukasz Majewski <l.majewski@xxxxxxxxxxx>
> > ---
> > arch/arm/mach-exynos/include/mach/irqs.h | 1 +
> > arch/arm/mach-exynos/include/mach/map.h | 4 +
> > arch/arm/mach-exynos/include/mach/regs-pmu.h | 3 +
> > arch/arm/mach-exynos/setup-usb-phy.c | 95
> +++++++++++++++++++-----
> > --
> > 4 files changed, 78 insertions(+), 25 deletions(-)
> >
>
> [...]
>
> Hi Lukasz,
>
> I have small comments on this.
>
> > +static int exynos4_usb_phy0_init(struct platform_device *pdev)
> > +{
> > + u32 rstcon;
> > +
> > + writel(readl(S5P_USBDEVICE_PHY_CONTROL) |
> > S5P_USBDEVICE_PHY_ENABLE,
> > + S5P_USBDEVICE_PHY_CONTROL);
> > +
> > + exynos4_usb_phy_clkset(pdev);
> > +
> > + /* set to normal PHY0 */
> > + writel((readl(EXYNOS4_PHYPWR) & ~PHY0_NORMAL_MASK),
> > EXYNOS4_PHYPWR); +
> > + /* reset PHY0 and Link */
> > + rstcon = readl(EXYNOS4_RSTCON) | PHY0_SWRST_MASK;
> > + writel(rstcon, EXYNOS4_RSTCON);
> > + udelay(10);
> > +
> > + rstcon &= ~PHY0_SWRST_MASK;
> > + writel(rstcon, EXYNOS4_RSTCON);
> > + udelay(80);
>
> Do we _really_ need above udelay(80)?
>
> As I know, we only need it in the phy1_init() for EHCI and we don't
> need it here.
I will test if this removal not break anything.
>
> [...]
>
> > int s5p_usb_phy_init(struct platform_device *pdev, int type)
> > {
> > - if (type == S5P_USB_PHY_HOST)
> > + if (type == S5P_USB_PHY_DEVICE)
> > + return exynos4_usb_phy0_init(pdev);
>
> I think, this should be exynos4210_usb_phy0_init(pdev), because this
> is available on only exynos4210 not
> exynos4x12 and exynos5.
I will change the name and submit a patch.
>
> > + else if (type == S5P_USB_PHY_HOST)
> > return exynos4_usb_phy1_init(pdev);
>
> Same as above.
>
> >
> > return -EINVAL;
> > @@ -144,7 +187,9 @@ int s5p_usb_phy_init(struct platform_device
> > *pdev, int type)
> >
> > int s5p_usb_phy_exit(struct platform_device *pdev, int type)
> > {
> > - if (type == S5P_USB_PHY_HOST)
> > + if (type == S5P_USB_PHY_DEVICE)
> > + return exynos4_usb_phy0_exit(pdev);
>
> Same as above.
>
> > + else if (type == S5P_USB_PHY_HOST)
> > return exynos4_usb_phy1_exit(pdev);
>
> Same as above.
>
> If you're ok on my comments, could you please update it as soon as
> possible for v3.5?
I will prepare a patch.
>
> Thanks.
>
> Best regards,
> Kgene.
> --
> Kukjin Kim <kgene.kim@xxxxxxxxxxx>, Senior Engineer,
> SW Solution Development Team, Samsung Electronics Co., Ltd.
>
--
Best regards,
Lukasz Majewski
Samsung Poland R&D Center | Linux Platform Group
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
