Hi Jingoo,
On Thu, Apr 24, 2014 at 6:56 AM, Jingoo Han <jg1.han@xxxxxxxxxxx> 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@xxxxxxxxxxx>
>> > > Cc: Jingoo Han <jg1.han@xxxxxxxxxxx>
>> > > ---
>> > >
>> > > 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 ?
>
> 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.
>
> exynos_ohci_probe()
> exynos_ohci->vdd33 = devm_regulator_get(&pdev->dev, "vdd33");
> if (IS_ERR(exynos_ohci->vdd33)) {
> dev_err(&pdev->dev, "Failed to get VDD33 supply\n");
> } else {
> 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)) {
> dev_err(&pdev->dev, "Failed to get VDD10 supply\n");
> } else {
> err = regulator_enable(exynos_ohci->vdd10);
> if (err) {
> dev_err(&pdev->dev, "Failed to enable VDD10 supply\n");
> goto fail_regulator2;
> }
> }
>
> In this case, suspend/resume can be fixed as below.
>
> exynos_ohci_suspend()
> if (exynos_ohci->vdd10)
> regulator_disable(exynos_ohci->vdd10);
> if (exynos_ohci->vdd33)
> regulator_disable(exynos_ohci->vdd33);
>
> exynos_ohci_resume()
>
> if (exynos_ohci->vdd33) {
> ret = regulator_enable(exynos_ohci->vdd33);
> if (ret) {
> dev_err(dev, "Failed to enable VDD33 supply\n");
> return ret;
> }
> }
> if (exynos_ohci->vdd10) {
> ret = regulator_enable(exynos_ohci->vdd10);
> if (ret) {
> dev_err(dev, "Failed to enable VDD10 supply\n");
> return ret;
> }
> }
Thanks for the suggestion.
I will make the required changes, and post the patchset again.
[snip]
--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
