[U-Boot] [PATCH 3/5] exynos5: Add power Enable/Disable for USB-EHCI

[Minkyu Kang]

On 2 May 2012 22:52, Rajeshwari Shinde <rajeshwari.s at samsung.com> wrote:
> This patch adds functions to enable/disable the power of USB
> host controller for exynos5.
>
> This patch depends on the patch:
> USB: S5P: Add ehci support.patch

This is not a commit message.
Please move it under the "---" line.

>
> Signed-off-by: Vivek Gautam <gautam.vivek at samsung.com>
> Signed-off-by: Che-Liang Chiou <clchiou at chromium.org>
> Signed-off-by: Rajeshwari Shinde <rajeshwari.s at samsung.com>
> ---
>  arch/arm/cpu/armv7/exynos/power.c         |   59 +++++++++++++++++++++++++++++
>  arch/arm/include/asm/arch-exynos/power.h  |    5 ++
>  arch/arm/include/asm/arch-exynos/sysreg.h |    1 +
>  drivers/usb/host/ehci-s5p.c               |    3 +
>  4 files changed, 68 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/cpu/armv7/exynos/power.c b/arch/arm/cpu/armv7/exynos/power.c
> index c765304..a943219 100644
> --- a/arch/arm/cpu/armv7/exynos/power.c
> +++ b/arch/arm/cpu/armv7/exynos/power.c
> @@ -24,6 +24,8 @@
>  #include <common.h>
>  #include <asm/io.h>
>  #include <asm/arch/power.h>
> +#include <asm/arch/cpu.h>

cpu.h is already included.
please remove it.

> +#include <asm/arch/sysreg.h>

also you don't need this header file.. maybe.

>
>  static void exynos4_mipi_phy_control(unsigned int dev_index,
>                                        unsigned int enable)
> @@ -52,3 +54,60 @@ void set_mipi_phy_ctrl(unsigned int dev_index, unsigned int enable)
>        if (cpu_is_exynos4())
>                exynos4_mipi_phy_control(dev_index, enable);
>  }
> +
> +void exynos5_ps_hold_setup(void)
> +{
> +       struct exynos5_power *power =
> +               (struct exynos5_power *)samsung_get_base_power();
> +
> +       /* Set PS-Hold high */
> +       setbits_le32(&power->ps_hold_control, POWER_PS_HOLD_CONTROL_DATA_HIGH);
> +}
> +
> +void exynos5_enable_usb_phy(void)
> +{
> +       struct exynos5_sysreg *sysreg =
> +               (struct exynos5_sysreg *)samsung_get_base_sysreg();
> +       struct exynos5_power *power =
> +               (struct exynos5_power *)samsung_get_base_power();
> +       unsigned int phy_cfg;
> +
> +       /* Setting USB20PHY_CONFIG register to USB 2.0 HOST link */
> +       phy_cfg = readl(&sysreg->usb20_phy_cfg);
> +       if (phy_cfg & USB20_PHY_CFG_EN) {
> +               debug("USB 2.0 HOST link already selected\n");

Is it useful information for your debugging? just question :)

> +       } else {
> +               phy_cfg |= USB20_PHY_CFG_EN;
> +               writel(phy_cfg, &sysreg->usb20_phy_cfg);

NAK.
The codes that are related with sysreg should be moved to system.c.

> +       }
> +
> +       /* Enabling USBHOST_PHY */
> +       setbits_le32(&power->usbhost_phy_control, POWER_USB_HOST_PHY_CTRL_EN);
> +}
> +
> +void exynos5_disable_usb_phy(void)
> +{
> +       struct exynos5_power *power =
> +               (struct exynos5_power *)samsung_get_base_power();
> +
> +       /* Disabling USBHost_PHY */
> +       clrbits_le32(&power->usbhost_phy_control, POWER_USB_HOST_PHY_CTRL_EN);
> +}

I think, you can combine enable/disable function to one.

> +
> +void ps_hold_setup(void)
> +{
> +       if (cpu_is_exynos5())
> +               exynos5_ps_hold_setup();
> +}
> +
> +void power_enable_usb_phy(void)
> +{
> +       if (cpu_is_exynos5())
> +               exynos5_enable_usb_phy();
> +}
> +
> +void power_disable_usb_phy(void)
> +{
> +       if (cpu_is_exynos5())
> +               exynos5_disable_usb_phy();
> +}
> diff --git a/arch/arm/include/asm/arch-exynos/power.h b/arch/arm/include/asm/arch-exynos/power.h
> index 4236beb..4e2448b 100644
> --- a/arch/arm/include/asm/arch-exynos/power.h
> +++ b/arch/arm/include/asm/arch-exynos/power.h
> @@ -855,4 +855,9 @@ void set_mipi_phy_ctrl(unsigned int dev_index, unsigned int enable);
>  #define EXYNOS_MIPI_PHY_SRESETN                (1 << 1)
>  #define EXYNOS_MIPI_PHY_MRESETN                (1 << 2)
>
> +#define POWER_USB_HOST_PHY_CTRL_EN             (1 << 0)
> +#define POWER_PS_HOLD_CONTROL_DATA_HIGH                (1 << 8)
> +void power_enable_usb_phy(void);
> +void power_disable_usb_phy(void);
> +
>  #endif
> diff --git a/arch/arm/include/asm/arch-exynos/sysreg.h b/arch/arm/include/asm/arch-exynos/sysreg.h
> index aca4b2b..2d8d35a 100644
> --- a/arch/arm/include/asm/arch-exynos/sysreg.h
> +++ b/arch/arm/include/asm/arch-exynos/sysreg.h
> @@ -40,4 +40,5 @@ struct exynos5_sysreg {
>  };
>  #endif
>
> +#define USB20_PHY_CFG_EN       (1 << 0)
>  #endif
> diff --git a/drivers/usb/host/ehci-s5p.c b/drivers/usb/host/ehci-s5p.c
> index 4dd4ec1..e575c48 100644
> --- a/drivers/usb/host/ehci-s5p.c
> +++ b/drivers/usb/host/ehci-s5p.c
> @@ -30,6 +30,7 @@
>  /* Setup the EHCI host controller. */
>  static void setup_usb_phy(struct s5p_usb_phy *usb)
>  {
> +       power_enable_usb_phy();

need space here.
and you should separate the function for power and system.

>        clrbits_le32(&usb->usbphyctrl0,
>                        HOST_CTRL0_FSEL_MASK |
>                        HOST_CTRL0_COMMONON_N |
> @@ -70,6 +71,8 @@ static void reset_usb_phy(struct s5p_usb_phy *usb)
>                        HOST_CTRL0_SIDDQ |
>                        HOST_CTRL0_FORCESUSPEND |
>                        HOST_CTRL0_FORCESLEEP);
> +
> +       power_disable_usb_phy();
>  }
>
>  /*
> --

Thanks.
Minkyu Kang.
-- 
from. prom.
www.promsoft.net