Hi Kukjin,

------- Original Message -------

Sender : Kukjin Kim<kgene.kim@samsung.com> E5/Senior Engineer/SW Solution Development Team/Samsung Electronics

Date : Aug 31, 2011 10:42 (GMT+05:00)

Title : RE: [PATCH 1/2] ARM: EXYNOS4: Code cleanup and remove extra functions.

Sachin Kamat wrote:
>
> From: Pankaj Dubey
>
> This patch removes the intermediate exynos4_usb_phy1_init and
> exynos4_usb_phy1_exit function calls.
>
> Signed-off-by: Pankaj Dubey
> Signed-off-by: Sachin Kamat
> ---
>  arch/arm/mach-exynos4/setup-usb-phy.c |  166 +++++++++++++++-------------
----
>  1 files changed, 78 insertions(+), 88 deletions(-)
>
> diff --git a/arch/arm/mach-exynos4/setup-usb-phy.c b/arch/arm/mach-
> exynos4/setup-usb-phy.c
> index 39aca04..f4c944a 100644
> --- a/arch/arm/mach-exynos4/setup-usb-phy.c
> +++ b/arch/arm/mach-exynos4/setup-usb-phy.c
> @@ -19,118 +19,108 @@
>  #include
>  #include
>
> -static int exynos4_usb_phy1_init(struct platform_device *pdev)
> +int s5p_usb_phy_init(struct platform_device *pdev, int type)
>  {
> - struct clk *otg_clk;
> - struct clk *xusbxti_clk;
> - u32 phyclk;
> - u32 rstcon;
> - int err;
> -
> - otg_clk = clk_get(&pdev->dev, "otg");
> - if (IS_ERR(otg_clk)) {
> - dev_err(&pdev->dev, "Failed to get otg clock ");
> - return PTR_ERR(otg_clk);
> - }
> + if (type == S5P_USB_PHY_HOST) {
> + struct clk *otg_clk;
> + struct clk *xusbxti_clk;
> + u32 phyclk;
> + u32 rstcon;
> + int err;
> +
> + otg_clk = clk_get(&pdev->dev, "otg");
> + if (IS_ERR(otg_clk)) {
> + dev_err(&pdev->dev, "Failed to get otg clock ");
> + return PTR_ERR(otg_clk);
> + }
>
> - err = clk_enable(otg_clk);
> - if (err) {
> - clk_put(otg_clk);
> - return err;
> - }
> + err = clk_enable(otg_clk);
> + if (err) {
> + clk_put(otg_clk);
> + return err;
> + }
>
> - writel(readl(S5P_USBHOST_PHY_CONTROL) |
> S5P_USBHOST_PHY_ENABLE,
> + writel(readl(S5P_USBHOST_PHY_CONTROL) |
> S5P_USBHOST_PHY_ENABLE,
>   S5P_USBHOST_PHY_CONTROL);
>
> - /* set clock frequency for PLL */
> - phyclk = readl(EXYNOS4_PHYCLK) & ~CLKSEL_MASK;
> -
> - xusbxti_clk = clk_get(&pdev->dev, "xusbxti");
> - if (xusbxti_clk && !IS_ERR(xusbxti_clk)) {
> - switch (clk_get_rate(xusbxti_clk)) {
> - case 12 * MHZ:
> - phyclk |= CLKSEL_12M;
> - break;
> - case 24 * MHZ:
> - phyclk |= CLKSEL_24M;
> - break;
> - default:
> - case 48 * MHZ:
> - /* default reference clock */
> - break;
> - }
> + /* set clock frequency for PLL */
> + phyclk = readl(EXYNOS4_PHYCLK) & ~CLKSEL_MASK;
> +
> + xusbxti_clk = clk_get(&pdev->dev, "xusbxti");
> + if (xusbxti_clk && !IS_ERR(xusbxti_clk)) {
> + switch (clk_get_rate(xusbxti_clk)) {
> + case 12 * MHZ:
> + phyclk |= CLKSEL_12M;
> + break;
> + case 24 * MHZ:
> + phyclk |= CLKSEL_24M;
> + break;
> + default:
> + case 48 * MHZ:
> + /* default reference clock */
> + break;
> + }
>   clk_put(xusbxti_clk);
> - }
> + }
>
> - writel(phyclk, EXYNOS4_PHYCLK);
> + writel(phyclk, EXYNOS4_PHYCLK);
>
> - /* floating prevention logic: disable */
> - writel((readl(EXYNOS4_PHY1CON) | FPENABLEN),
> EXYNOS4_PHY1CON);
> + /* floating prevention logic: disable */
> + writel((readl(EXYNOS4_PHY1CON) | FPENABLEN),
> EXYNOS4_PHY1CON);
>
> - /* set to normal HSIC 0 and 1 of PHY1 */
> - writel((readl(EXYNOS4_PHYPWR) & ~PHY1_HSIC_NORMAL_MASK),
> + /* set to normal HSIC 0 and 1 of PHY1 */
> + writel((readl(EXYNOS4_PHYPWR) &
> ~PHY1_HSIC_NORMAL_MASK),
>   EXYNOS4_PHYPWR);
>
> - /* set to normal standard USB of PHY1 */
> - writel((readl(EXYNOS4_PHYPWR) & ~PHY1_STD_NORMAL_MASK),
> EXYNOS4_PHYPWR);
> + /* set to normal standard USB of PHY1 */
> + writel((readl(EXYNOS4_PHYPWR) &
> ~PHY1_STD_NORMAL_MASK),
> + EXYNOS4_PHYPWR);
>
> - /* reset all ports of both PHY and Link */
> - rstcon = readl(EXYNOS4_RSTCON) | HOST_LINK_PORT_SWRST_MASK |
> - PHY1_SWRST_MASK;
> - writel(rstcon, EXYNOS4_RSTCON);
> - udelay(10);
> + /* reset all ports of both PHY and Link */
> + rstcon = readl(EXYNOS4_RSTCON) |
> HOST_LINK_PORT_SWRST_MASK |
> + PHY1_SWRST_MASK;
> + writel(rstcon, EXYNOS4_RSTCON);
> + udelay(10);
>
> - rstcon &= ~(HOST_LINK_PORT_SWRST_MASK | PHY1_SWRST_MASK);
> - writel(rstcon, EXYNOS4_RSTCON);
> - udelay(80);
> + rstcon &= ~(HOST_LINK_PORT_SWRST_MASK |
> PHY1_SWRST_MASK);
> + writel(rstcon, EXYNOS4_RSTCON);
> + udelay(80);
>
> - clk_disable(otg_clk);
> - clk_put(otg_clk);
> + clk_disable(otg_clk);
> + clk_put(otg_clk);
>
> - return 0;
> + return 0;
> + }
> + return -EINVAL;
>  }
>
> -static int exynos4_usb_phy1_exit(struct platform_device *pdev)
> +int s5p_usb_phy_exit(struct platform_device *pdev, int type)
>  {
> - struct clk *otg_clk;
> - int err;
> -
> - otg_clk = clk_get(&pdev->dev, "otg");
> - if (IS_ERR(otg_clk)) {
> - dev_err(&pdev->dev, "Failed to get otg clock ");
> - return PTR_ERR(otg_clk);
> - }
> + if (type == S5P_USB_PHY_HOST) {
> + struct clk *otg_clk;
> + int err;
> + otg_clk = clk_get(&pdev->dev, "otg");
> + if (IS_ERR(otg_clk)) {
> + dev_err(&pdev->dev, "Failed to get otg clock ");
> + return PTR_ERR(otg_clk);
> + }
>
> - err = clk_enable(otg_clk);
> - if (err) {
> - clk_put(otg_clk);
> - return err;
> - }
> + err = clk_enable(otg_clk);
> + if (err) {
> + clk_put(otg_clk);
> + return err;
> + }
>
> - writel((readl(EXYNOS4_PHYPWR) |
> PHY1_STD_ANALOG_POWERDOWN),
> + writel((readl(EXYNOS4_PHYPWR) |
> PHY1_STD_ANALOG_POWERDOWN),
>   EXYNOS4_PHYPWR);
>
> - writel(readl(S5P_USBHOST_PHY_CONTROL) &
> ~S5P_USBHOST_PHY_ENABLE,
> + writel(readl(S5P_USBHOST_PHY_CONTROL) &
> ~S5P_USBHOST_PHY_ENABLE,
>   S5P_USBHOST_PHY_CONTROL);
>
> - clk_disable(otg_clk);
> - clk_put(otg_clk);
> -
> - return 0;
> -}
> -
> -int s5p_usb_phy_init(struct platform_device *pdev, int type)
> -{
> - if (type == S5P_USB_PHY_HOST)
> - return exynos4_usb_phy1_init(pdev);
> -
> - return -EINVAL;
> -}
> -
> -int s5p_usb_phy_exit(struct platform_device *pdev, int type)
> -{
> - if (type == S5P_USB_PHY_HOST)
> - return exynos4_usb_phy1_exit(pdev);
> + clk_disable(otg_clk);
> + clk_put(otg_clk);
>
> + return 0;
> + }
>   return -EINVAL;
>  }
> --
> 1.7.4.1

(Cc'ed Yulgon Kim and Kyoungil Kim)

Hi Sachin,

I don't know why this is required on usb-phy control for EXYNOS4 now.

Actually, other upcoming EXYNOS SoCs have more usb phys than EXYNOS4210, so
this can make it more difficult to control.

-----

I think when more then one USB PHY will come we can handle them using same file and functions, only thing device/host has to take care is they need to send proper "type" value to phy_init/phy_exit function. And depending upon value of "type" we can add initilization/exit code in phy_init/phy_exit.

Thanks,

Pankaj Dubey

---

Thanks.

Best regards,
Kgene.
--
Kukjin Kim , Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.