Re: [PATCH] usb: dwc2: introduce config parameter to ignore supplies vusb_a and vusb_d

2017-01-31 Thread Heiner Kallweit 
Am 31.01.2017 um 20:23 schrieb Heiner Kallweit:
> Am 31.01.2017 um 19:48 schrieb John Youn:
>> On 1/30/2017 11:13 PM, Heiner Kallweit wrote:
>>> Am 31.01.2017 um 03:32 schrieb John Youn:
 On 1/28/2017 2:06 PM, Heiner Kallweit wrote:
> Supplies for vusb_a and vusb_d are needed only on a minority of systems
> supported by the dwc2 driver (AFAIK systems with Samsung SoCs).
>
> On all other systems this results in these harmless but annoying
> warnings:
>
> c900.usb supply vusb_d not found, using dummy regulator
> c900.usb supply vusb_a not found, using dummy regulator
>
> Introduce a configuration parameter to ignore the supplies on
> systems not needing it.

 You can probably just check for the existence of those bindings. Or
 possibly use devm_regulator_get_optional().

>>> Right, that we could do to silence the warning. But then we would
>>> loose the warning on systems where these supplies are mandatory
>>> (in case somebody adds a system with such a SoC and forgets to define
>>> the supplies in the DT).
>>
>> I don't think the dwc2 driver should be doing that though. It should
>> treat these as optional properties and use them if available, just
>> like it does now for the reset control, phy, clocks, etc. Otherwise
>> you could make the case for adding the same flag for these other
>> components as well.
>>
>> But really, I just don't want to add new parameters, if anything I
>> want to remove more of them.
>>
> I see. Then most likely it's best to switch to
> devm_regulator_get_optional. I'll send a patch for it.
> 
Hmm, I just see that devm_regulator_bulk_get is used. And there's
no bulk function for optional regulators (yet).

> Heiner
> 
>> Regards,
>> John
>>
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: dwc2: introduce config parameter to ignore supplies vusb_a and vusb_d

2017-01-31 Thread Heiner Kallweit 
Am 31.01.2017 um 19:48 schrieb John Youn:
> On 1/30/2017 11:13 PM, Heiner Kallweit wrote:
>> Am 31.01.2017 um 03:32 schrieb John Youn:
>>> On 1/28/2017 2:06 PM, Heiner Kallweit wrote:
 Supplies for vusb_a and vusb_d are needed only on a minority of systems
 supported by the dwc2 driver (AFAIK systems with Samsung SoCs).

 On all other systems this results in these harmless but annoying
 warnings:

 c900.usb supply vusb_d not found, using dummy regulator
 c900.usb supply vusb_a not found, using dummy regulator

 Introduce a configuration parameter to ignore the supplies on
 systems not needing it.
>>>
>>> You can probably just check for the existence of those bindings. Or
>>> possibly use devm_regulator_get_optional().
>>>
>> Right, that we could do to silence the warning. But then we would
>> loose the warning on systems where these supplies are mandatory
>> (in case somebody adds a system with such a SoC and forgets to define
>> the supplies in the DT).
> 
> I don't think the dwc2 driver should be doing that though. It should
> treat these as optional properties and use them if available, just
> like it does now for the reset control, phy, clocks, etc. Otherwise
> you could make the case for adding the same flag for these other
> components as well.
> 
> But really, I just don't want to add new parameters, if anything I
> want to remove more of them.
> 
I see. Then most likely it's best to switch to
devm_regulator_get_optional. I'll send a patch for it.

Heiner

> Regards,
> John
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: dwc2: introduce config parameter to ignore supplies vusb_a and vusb_d

2017-01-31 Thread John Youn 
On 1/30/2017 11:13 PM, Heiner Kallweit wrote:
> Am 31.01.2017 um 03:32 schrieb John Youn:
>> On 1/28/2017 2:06 PM, Heiner Kallweit wrote:
>>> Supplies for vusb_a and vusb_d are needed only on a minority of systems
>>> supported by the dwc2 driver (AFAIK systems with Samsung SoCs).
>>>
>>> On all other systems this results in these harmless but annoying
>>> warnings:
>>>
>>> c900.usb supply vusb_d not found, using dummy regulator
>>> c900.usb supply vusb_a not found, using dummy regulator
>>>
>>> Introduce a configuration parameter to ignore the supplies on
>>> systems not needing it.
>>
>> You can probably just check for the existence of those bindings. Or
>> possibly use devm_regulator_get_optional().
>>
> Right, that we could do to silence the warning. But then we would
> loose the warning on systems where these supplies are mandatory
> (in case somebody adds a system with such a SoC and forgets to define
> the supplies in the DT).

I don't think the dwc2 driver should be doing that though. It should
treat these as optional properties and use them if available, just
like it does now for the reset control, phy, clocks, etc. Otherwise
you could make the case for adding the same flag for these other
components as well.

But really, I just don't want to add new parameters, if anything I
want to remove more of them.

Regards,
John
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: dwc2: introduce config parameter to ignore supplies vusb_a and vusb_d

2017-01-30 Thread Heiner Kallweit 
Am 31.01.2017 um 03:32 schrieb John Youn:
> On 1/28/2017 2:06 PM, Heiner Kallweit wrote:
>> Supplies for vusb_a and vusb_d are needed only on a minority of systems
>> supported by the dwc2 driver (AFAIK systems with Samsung SoCs).
>>
>> On all other systems this results in these harmless but annoying
>> warnings:
>>
>> c900.usb supply vusb_d not found, using dummy regulator
>> c900.usb supply vusb_a not found, using dummy regulator
>>
>> Introduce a configuration parameter to ignore the supplies on
>> systems not needing it.
> 
> You can probably just check for the existence of those bindings. Or
> possibly use devm_regulator_get_optional().
> 
Right, that we could do to silence the warning. But then we would
loose the warning on systems where these supplies are mandatory
(in case somebody adds a system with such a SoC and forgets to define
the supplies in the DT).

> Also, reviewing this code, it looks like vusb_d and vusb_a should have
> been documented as dwc2 bindings but never were...
> 
> John
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: dwc2: introduce config parameter to ignore supplies vusb_a and vusb_d

2017-01-30 Thread John Youn 
On 1/28/2017 2:06 PM, Heiner Kallweit wrote:
> Supplies for vusb_a and vusb_d are needed only on a minority of systems
> supported by the dwc2 driver (AFAIK systems with Samsung SoCs).
>
> On all other systems this results in these harmless but annoying
> warnings:
>
> c900.usb supply vusb_d not found, using dummy regulator
> c900.usb supply vusb_a not found, using dummy regulator
>
> Introduce a configuration parameter to ignore the supplies on
> systems not needing it.

You can probably just check for the existence of those bindings. Or
possibly use devm_regulator_get_optional().

Also, reviewing this code, it looks like vusb_d and vusb_a should have
been documented as dwc2 bindings but never were...

John
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb: dwc2: introduce config parameter to ignore supplies vusb_a and vusb_d

2017-01-28 Thread Heiner Kallweit 
Supplies for vusb_a and vusb_d are needed only on a minority of systems
supported by the dwc2 driver (AFAIK systems with Samsung SoCs).

On all other systems this results in these harmless but annoying
warnings:

c900.usb supply vusb_d not found, using dummy regulator
c900.usb supply vusb_a not found, using dummy regulator

Introduce a configuration parameter to ignore the supplies on
systems not needing it.

Signed-off-by: Heiner Kallweit 
---
 drivers/usb/dwc2/core.h |  2 ++
 drivers/usb/dwc2/params.c   |  6 ++
 drivers/usb/dwc2/platform.c | 30 +++---
 3 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index a708e4fa..42deeeb4 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -433,6 +433,7 @@ enum dwc2_ep0_state {
  * needed.
  * 0 - No (default)
  * 1 - Yes
+ * @needs_supplies:Chip needs supplies (for vusb_a and vusb_d)
  * @ahb_burst:  Specifies the AHB burst.
  *   0 - Single
  *   1 - INCR
@@ -506,6 +507,7 @@ struct dwc2_core_params {
int uframe_sched;
int external_id_pin_ctl;
int hibernation;
+   bool needs_supplies;
 
/*
 * The following parameters are *only* set via device
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 014e269f..baf8b821 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -67,6 +67,7 @@ static const struct dwc2_core_params params_hi6220 = {
.uframe_sched   = 0,
.external_id_pin_ctl= -1,
.hibernation= -1,
+   .needs_supplies = false,
 };
 
 static const struct dwc2_core_params params_bcm2835 = {
@@ -104,6 +105,7 @@ static const struct dwc2_core_params params_bcm2835 = {
.uframe_sched   = 0,
.external_id_pin_ctl= -1,
.hibernation= -1,
+   .needs_supplies = false,
 };
 
 static const struct dwc2_core_params params_rk3066 = {
@@ -135,6 +137,7 @@ static const struct dwc2_core_params params_rk3066 = {
.uframe_sched   = -1,
.external_id_pin_ctl= -1,
.hibernation= -1,
+   .needs_supplies = false,
 };
 
 static const struct dwc2_core_params params_ltq = {
@@ -166,6 +169,7 @@ static const struct dwc2_core_params params_ltq = {
.uframe_sched   = -1,
.external_id_pin_ctl= -1,
.hibernation= -1,
+   .needs_supplies = false,
 };
 
 static const struct dwc2_core_params params_amlogic = {
@@ -197,6 +201,7 @@ static const struct dwc2_core_params params_amlogic = {
.uframe_sched   = 0,
.external_id_pin_ctl= -1,
.hibernation= -1,
+   .needs_supplies = false,
 };
 
 static const struct dwc2_core_params params_default = {
@@ -234,6 +239,7 @@ static const struct dwc2_core_params params_default = {
.uframe_sched   = -1,
.external_id_pin_ctl= -1,
.hibernation= -1,
+   .needs_supplies = true,
 };
 
 const struct of_device_id dwc2_of_match_table[] = {
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index 3f59a73d..b218a72e 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -125,10 +125,12 @@ static int __dwc2_lowlevel_hw_enable(struct dwc2_hsotg 
*hsotg)
struct platform_device *pdev = to_platform_device(hsotg->dev);
int ret;
 
-   ret = regulator_bulk_enable(ARRAY_SIZE(hsotg->supplies),
-   hsotg->supplies);
-   if (ret)
-   return ret;
+   if (hsotg->params.needs_supplies) {
+   ret = regulator_bulk_enable(ARRAY_SIZE(hsotg->supplies),
+   hsotg->supplies);
+   if (ret)
+   return ret;
+   }
 
if (hsotg->clk) {
ret = clk_prepare_enable(hsotg->clk);
@@ -185,8 +187,9 @@ static int __dwc2_lowlevel_hw_disable(struct dwc2_hsotg 
*hsotg)
if (hsotg->clk)
clk_disable_unprepare(hsotg->clk);
 
-   ret = regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies),
-hsotg->supplies);
+   if (hsotg->params.needs_supplies)
+   ret = regulator_bulk_disable(ARRAY_SIZE(hsotg->supplies),
+hsotg->supplies);
 
return ret;
 }
@@ -293,12 +296,17 @@ static int dwc2_lowlevel_hw_init(struct dwc2_hsotg *hsotg)
for (i = 0; i < ARRAY_SIZE(hsotg->supplies); i++)