Lukasz Majewski wrote:
> 
> This patch supports to control usb otg phy of EXYNOS4210.
> 
> Signed-off-by: Joonyoung Shim <[email protected]>
> Signed-off-by: Kyungmin Park <[email protected]>
> [Rebased on the newest git/kgene/linux-samsung #for-next]
> Signed-off-by: Lukasz Majewski <[email protected]>
> ---
>  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.

[...]

>  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.

> +     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?

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to