Re: [PATCH 1/3] usb: ohci-exynos: Make provision for vdd regulators

[Jingoo Han <jg1.han@samsung.com>]

On Thursday, April 24, 2014 3:40 PM, Vivek Gautam wrote:
> On Thu, Apr 24, 2014 at 6:56 AM, Jingoo Han <jg1.han@samsung.com> wrote:
> > On Thursday, April 24, 2014 9:33 AM, Jingoo Han wrote:
> >> On Thursday, April 24, 2014 9:18 AM, Anton Tikhomirov wrote:
> >> > On Monday, April 21, 2014 9:17 PM, Vivek Gautam wrote:
> >> > >
> >> > > Facilitate getting required 3.3V and 1.0V VDD supply for
> >> > > OHCI controller on Exynos.
> >> > >
> >> > > With patches for regulators' nodes merged in 3.15:
> >> > > c8c253f ARM: dts: Add regulator entries to smdk5420
> >> > > 275dcd2 ARM: dts: add max77686 pmic node for smdk5250,
> >> > >
> >> > > certain perripherals will now need to ensure that,
> >> > > they request VDD regulators in their drivers, and enable
> >> > > them so as to make them working.
> >> > >
> >> > > Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> >> > > Cc: Jingoo Han <jg1.han@samsung.com>
> >> > > ---
> >> > >
> >> > > Based on 'usb-next' branch of Greg's usb tree.
> >> > >
> >> > >  drivers/usb/host/ohci-exynos.c |   47
> >> > > ++++++++++++++++++++++++++++++++++++++++
> >> > >  1 file changed, 47 insertions(+)
> >> > >
> >> > > diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-
> >> > > exynos.c
> >> > > index 68588d8..e2e72a8 100644
> >> > > --- a/drivers/usb/host/ohci-exynos.c
> >> > > +++ b/drivers/usb/host/ohci-exynos.c
> 
> [snip]
> 
> >> > > @@ -98,6 +101,28 @@ static int exynos_ohci_probe(struct platform_device
> >> > > *pdev)
> >> > >           exynos_ohci->otg = phy->otg;
> >> > >   }
> >> > >
> >> > > + exynos_ohci->vdd33 = devm_regulator_get(&pdev->dev, "vdd33");
> >> > > + if (IS_ERR(exynos_ohci->vdd33)) {
> >> > > +         err = PTR_ERR(exynos_ohci->vdd33);
> >> > > +         goto fail_regulator1;
> >> > > + }
> >> > > + err = regulator_enable(exynos_ohci->vdd33);
> >> > > + if (err) {
> >> > > +         dev_err(&pdev->dev, "Failed to enable VDD33 supply\n");
> >> > > +         goto fail_regulator1;
> >> > > + }
> >> > > +
> >> > > + exynos_ohci->vdd10 = devm_regulator_get(&pdev->dev, "vdd10");
> >> > > + if (IS_ERR(exynos_ohci->vdd10)) {
> >> > > +         err = PTR_ERR(exynos_ohci->vdd10);
> >> > > +         goto fail_regulator2;
> >> > > + }
> >> > > + err = regulator_enable(exynos_ohci->vdd10);
> >> > > + if (err) {
> >> > > +         dev_err(&pdev->dev, "Failed to enable VDD10 supply\n");
> >> > > +         goto fail_regulator2;
> >> > > + }
> >> > > +
> >> >
> >> > Do we need to skip regulator settings together with PHY configuration
> >> > in case of exynos5440?
> >>
> >> Oh, right. In the case of exynos5440, regulator settings is not
> >> necessary. Vivek, would you fix it in order skip regulator settings
> >> in exynos5440? It also applies to ehci-exynos.
> >
> > Sorry, in the case of exynos5440, this patch already skips
> > regulator settings.
> >
> > In the case of exynos5440, there is no need to set PHY setting
> > and regulator setting.
> 
> Right, in case of exynos5440, we are skipping PHY setting and regulator setting.
> Actually i had missed taking into account 5440, so just curious. Do we
> really not need a regulator
> settings for Exynos5440 ?

To be more specific, there is no regulator on ssdk5440 board
which is the Exynos5440-based board.

Best regards,
Jingoo Han

> > How about making regulator setting "optional"?
> > Then, regulator setting can be done, only when regulator
> > is supported.
> 
> True, so with Exynos5440 not needing the regulator, we should make the
> regulator settings optional.

[.....]

> Thanks for the suggestion.
> I will make the required changes, and post the patchset again.

OK, I see.
Thank you for accepting my suggestion.

Best regards,
Jingoo Han