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

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

Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
Signed-off-by: Che-Liang Chiou <clchiou@chromium.org>
Signed-off-by: Rajeshwari Shinde <rajeshwari.s@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(-)

On 2 May 2012 22:52, Rajeshwari Shinde <rajeshwari.s@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@samsung.com>
> Signed-off-by: Che-Liang Chiou <clchiou@chromium.org>
> Signed-off-by: Rajeshwari Shinde <rajeshwari.s@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.

Hi Minkyu,

Thank you for the comments.

On Thu, May 3, 2012 at 1:03 PM, Minkyu Kang <promsoft@gmail.com> wrote:
> On 2 May 2012 22:52, Rajeshwari Shinde <rajeshwari.s@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.
--ok
>
>>
>> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
>> Signed-off-by: Che-Liang Chiou <clchiou@chromium.org>
>> Signed-off-by: Rajeshwari Shinde <rajeshwari.s@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.
--- will do so
>
>> +#include <asm/arch/sysreg.h>
>
> also you don't need this header file.. maybe.
-- will remove this header file.
>
>>
>>  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 :)
-- will remove the same :)

>
>> +       } 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.
-- will correct this
>
>> +       }
>> +
>> +       /* 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.
-- ok
>
>> +
>> +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.
-- ok
>
>>        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
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

Regards,
Rajeshwari Shinde.