[U-Boot] [PATCH RESEND v2 3/3] usb:gadget:s5p Enable the USB Gadget framework at Exynos4210 (C210 Universal)

[Minkyu Kang]

Dear Lukasz Majewski,

On 15 December 2011 01:41, Lukasz Majewski <l.majewski at samsung.com> wrote:
> This commit enables support for USB Gadgets on the Exynos4210 (C210 Universal)
> reference target.
>
> Signed-off-by: Lukasz Majewski <l.majewski at samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park at samsung.com>
> Cc: Minkyu Kang <mk7.kang at samsung.com>
> CC: Remy Bohmer <linux at bohmer.net>
> ---
> Changes for v2:
>        - Rename the S5PC210_Universal to Exynos4210
> ---
> /checkpatch.pl -
> total: 0 errors, 0 warnings, 72 lines checked
>
> ---
>  arch/arm/include/asm/arch-exynos/cpu.h   |    1 +
>  board/samsung/universal_c210/universal.c |   48 ++++++++++++++++++++++++++++++
>  include/configs/s5pc210_universal.h      |    4 ++
>  3 files changed, 53 insertions(+), 0 deletions(-)
>
> diff --git a/board/samsung/universal_c210/universal.c b/board/samsung/universal_c210/universal.c
> index f9b8a76..1f5b454 100644
> --- a/board/samsung/universal_c210/universal.c
> +++ b/board/samsung/universal_c210/universal.c
> @@ -28,6 +28,9 @@
>  #include <asm/arch/gpio.h>
>  #include <asm/arch/mmc.h>
>  #include <pmic.h>
> +#include <usb/s3c_udc.h>
> +#include <asm/arch/cpu.h>
> +#include <max8998_pmic.h>
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> @@ -253,3 +256,48 @@ int board_mmc_init(bd_t *bis)
>
>  }
>  #endif
> +
> +#ifdef CONFIG_USB_GADGET
> +static int s5pc210_phy_control(int on)
> +{
> +       int ret;
> +       static int status;
> +       struct pmic *p = get_pmic();
> +
> +       if (pmic_probe(p))
> +               return -1;
> +
> +       if (on && !status) {

No. Don't need to check status.
There are possibility that LDO values are changed,
so please go on without checking status.


> +               ret = pmic_set_output(p, MAX8998_REG_ONOFF1,
> +                                     MAX8998_LDO3, LDO_ON);
> +               ret = pmic_set_output(p, MAX8998_REG_ONOFF2,
> +                                     MAX8998_LDO8, LDO_ON);
> +               if (ret) {

Then, how to handle the result of LDO3?
And we should on or off the SAFEOUT1 also.

> +                       puts("MAX8998 LDO setting error!\n");
> +                       return -1;
> +               }
> +               status = 1;
> +       } else if (!on && status) {
> +               ret = pmic_set_output(p, MAX8998_REG_ONOFF1,
> +                                     MAX8998_LDO3, LDO_OFF);
> +               ret = pmic_set_output(p, MAX8998_REG_ONOFF2,
> +                                     MAX8998_LDO8, LDO_OFF);
> +               if (ret) {
> +                       puts("MAX8998 LDO setting error!\n");
> +                       return -1;
> +               }
> +               status = 0;
> +       }
> +       udelay(10000);

Why need the delay here?

> +
> +       return 0;
> +}
> +
> +struct s3c_plat_otg_data s5pc210_otg_data = {
> +       .phy_control = s5pc210_phy_control,
> +       .regs_phy = EXYNOS4_USBPHY_BASE,
> +       .regs_otg = EXYNOS4_USBOTG_BASE,
> +       .usb_phy_ctrl = EXYNOS4_USBPHY_CONTROL,
> +       .usb_flags = PHY0_SLEEP,
> +};
> +#endif

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