[PATCH] usb: dwc2: dwc2_vbus_supply_init: fix error check

[PATCH] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Tomeu Vizoso]

devm_regulator_get_optional returns -ENODEV if the regulator isn't
there, so if that's the case we have to make sure not to leave -ENODEV
in the regulator pointer.

Also, make sure we return 0 in that case, but correctly propagate any
other errors. Also propagate the error from _dwc2_hcd_start.

Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus supply")
Cc: Amelie Delaunay <amelie.delaunay@st.com>
Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 drivers/usb/dwc2/hcd.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index dcfda5eb4cac..4ae211f65e85 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -359,8 +359,13 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
 static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
 {
 	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
-	if (IS_ERR(hsotg->vbus_supply))
-		return 0;
+	if (IS_ERR(hsotg->vbus_supply)) {
+		hsotg->vbus_supply = NULL;
+		if (PTR_ERR(hsotg->vbus_supply) == -ENODEV)
+			return 0;
+		else
+			return PTR_ERR(hsotg->vbus_supply);
+	}
 
 	return regulator_enable(hsotg->vbus_supply);
 }
@@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
 
 	spin_unlock_irqrestore(&hsotg->lock, flags);
 
-	dwc2_vbus_supply_init(hsotg);
-
-	return 0;
+	return dwc2_vbus_supply_init(hsotg);
 }
 
 /*
-- 
2.14.3

usb: dwc2: dwc2_vbus_supply_init: fix error check

[Tomeu Vizoso]

devm_regulator_get_optional returns -ENODEV if the regulator isn't
there, so if that's the case we have to make sure not to leave -ENODEV
in the regulator pointer.

Also, make sure we return 0 in that case, but correctly propagate any
other errors. Also propagate the error from _dwc2_hcd_start.

Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus supply")
Cc: Amelie Delaunay <amelie.delaunay@st.com>
Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 drivers/usb/dwc2/hcd.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index dcfda5eb4cac..4ae211f65e85 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -359,8 +359,13 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
 static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
 {
 	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
-	if (IS_ERR(hsotg->vbus_supply))
-		return 0;
+	if (IS_ERR(hsotg->vbus_supply)) {
+		hsotg->vbus_supply = NULL;
+		if (PTR_ERR(hsotg->vbus_supply) == -ENODEV)
+			return 0;
+		else
+			return PTR_ERR(hsotg->vbus_supply);
+	}
 
 	return regulator_enable(hsotg->vbus_supply);
 }
@@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
 
 	spin_unlock_irqrestore(&hsotg->lock, flags);
 
-	dwc2_vbus_supply_init(hsotg);
-
-	return 0;
+	return dwc2_vbus_supply_init(hsotg);
 }
 
 /*

Re: [PATCH] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Heiko Stübner]

Am Donnerstag, 22. März 2018, 10:39:43 CET schrieb Tomeu Vizoso:
> devm_regulator_get_optional returns -ENODEV if the regulator isn't
> there, so if that's the case we have to make sure not to leave -ENODEV
> in the regulator pointer.
> 
> Also, make sure we return 0 in that case, but correctly propagate any
> other errors. Also propagate the error from _dwc2_hcd_start.
> 
> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus
> supply") Cc: Amelie Delaunay <amelie.delaunay@st.com>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>

Thanks for catching that oops in your tests.
Reviewed-by: Heiko Stuebner <heiko@sntech.de>


> ---
>  drivers/usb/dwc2/hcd.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index dcfda5eb4cac..4ae211f65e85 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -359,8 +359,13 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
> static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
>  {
>  	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
> -	if (IS_ERR(hsotg->vbus_supply))
> -		return 0;
> +	if (IS_ERR(hsotg->vbus_supply)) {
> +		hsotg->vbus_supply = NULL;
> +		if (PTR_ERR(hsotg->vbus_supply) == -ENODEV)
> +			return 0;
> +		else
> +			return PTR_ERR(hsotg->vbus_supply);
> +	}
> 
>  	return regulator_enable(hsotg->vbus_supply);
>  }
> @@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
> 
>  	spin_unlock_irqrestore(&hsotg->lock, flags);
> 
> -	dwc2_vbus_supply_init(hsotg);
> -
> -	return 0;
> +	return dwc2_vbus_supply_init(hsotg);
>  }
> 
>  /*

usb: dwc2: dwc2_vbus_supply_init: fix error check

[Heiko Stuebner]

Am Donnerstag, 22. März 2018, 10:39:43 CET schrieb Tomeu Vizoso:
> devm_regulator_get_optional returns -ENODEV if the regulator isn't
> there, so if that's the case we have to make sure not to leave -ENODEV
> in the regulator pointer.
> 
> Also, make sure we return 0 in that case, but correctly propagate any
> other errors. Also propagate the error from _dwc2_hcd_start.
> 
> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus
> supply") Cc: Amelie Delaunay <amelie.delaunay@st.com>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>

Thanks for catching that oops in your tests.
Reviewed-by: Heiko Stuebner <heiko@sntech.de>


> ---
>  drivers/usb/dwc2/hcd.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index dcfda5eb4cac..4ae211f65e85 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -359,8 +359,13 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
> static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
>  {
>  	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
> -	if (IS_ERR(hsotg->vbus_supply))
> -		return 0;
> +	if (IS_ERR(hsotg->vbus_supply)) {
> +		hsotg->vbus_supply = NULL;
> +		if (PTR_ERR(hsotg->vbus_supply) == -ENODEV)
> +			return 0;
> +		else
> +			return PTR_ERR(hsotg->vbus_supply);
> +	}
> 
>  	return regulator_enable(hsotg->vbus_supply);
>  }
> @@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
> 
>  	spin_unlock_irqrestore(&hsotg->lock, flags);
> 
> -	dwc2_vbus_supply_init(hsotg);
> -
> -	return 0;
> +	return dwc2_vbus_supply_init(hsotg);
>  }
> 
>  /*
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Heiko Stübner]

Hi Tomeu,

Am Donnerstag, 22. März 2018, 12:39:13 CET schrieb Heiko Stübner:
> Am Donnerstag, 22. März 2018, 10:39:43 CET schrieb Tomeu Vizoso:
> > devm_regulator_get_optional returns -ENODEV if the regulator isn't
> > there, so if that's the case we have to make sure not to leave -ENODEV
> > in the regulator pointer.
> > 
> > Also, make sure we return 0 in that case, but correctly propagate any
> > other errors. Also propagate the error from _dwc2_hcd_start.
> > 
> > Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus
> > supply") Cc: Amelie Delaunay <amelie.delaunay@st.com>
> > Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> 
> Thanks for catching that oops in your tests.
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>

I take that back :-)
see below

> > ---
> > 
> >  drivers/usb/dwc2/hcd.c | 13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> > index dcfda5eb4cac..4ae211f65e85 100644
> > --- a/drivers/usb/dwc2/hcd.c
> > +++ b/drivers/usb/dwc2/hcd.c
> > @@ -359,8 +359,13 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg
> > *hsotg) static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
> > 
> >  {
> >  
> >  	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
> > 
> > -	if (IS_ERR(hsotg->vbus_supply))
> > -		return 0;
> > +	if (IS_ERR(hsotg->vbus_supply)) {
> > +		hsotg->vbus_supply = NULL;
> > +		if (PTR_ERR(hsotg->vbus_supply) == -ENODEV)

hsotg->vbus_supply is already NULL here

usb: dwc2: dwc2_vbus_supply_init: fix error check

[Heiko Stuebner]

Hi Tomeu,

Am Donnerstag, 22. März 2018, 12:39:13 CET schrieb Heiko Stübner:
> Am Donnerstag, 22. März 2018, 10:39:43 CET schrieb Tomeu Vizoso:
> > devm_regulator_get_optional returns -ENODEV if the regulator isn't
> > there, so if that's the case we have to make sure not to leave -ENODEV
> > in the regulator pointer.
> > 
> > Also, make sure we return 0 in that case, but correctly propagate any
> > other errors. Also propagate the error from _dwc2_hcd_start.
> > 
> > Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus
> > supply") Cc: Amelie Delaunay <amelie.delaunay@st.com>
> > Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> 
> Thanks for catching that oops in your tests.
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>

I take that back :-)
see below

> > ---
> > 
> >  drivers/usb/dwc2/hcd.c | 13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> > index dcfda5eb4cac..4ae211f65e85 100644
> > --- a/drivers/usb/dwc2/hcd.c
> > +++ b/drivers/usb/dwc2/hcd.c
> > @@ -359,8 +359,13 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg
> > *hsotg) static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
> > 
> >  {
> >  
> >  	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
> > 
> > -	if (IS_ERR(hsotg->vbus_supply))
> > -		return 0;
> > +	if (IS_ERR(hsotg->vbus_supply)) {
> > +		hsotg->vbus_supply = NULL;
> > +		if (PTR_ERR(hsotg->vbus_supply) == -ENODEV)

hsotg->vbus_supply is already NULL here
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Amelie DELAUNAY]

Hi Heiko,
Hi Tomeu,

On 03/22/2018 12:41 PM, Heiko Stübner wrote:
> Hi Tomeu,
> 
> Am Donnerstag, 22. März 2018, 12:39:13 CET schrieb Heiko Stübner:
>> Am Donnerstag, 22. März 2018, 10:39:43 CET schrieb Tomeu Vizoso:
>>> devm_regulator_get_optional returns -ENODEV if the regulator isn't
>>> there, so if that's the case we have to make sure not to leave -ENODEV
>>> in the regulator pointer.
>>>
>>> Also, make sure we return 0 in that case, but correctly propagate any
>>> other errors. Also propagate the error from _dwc2_hcd_start.
>>>
>>> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus
>>> supply") Cc: Amelie Delaunay <amelie.delaunay@st.com>
>>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>>
>> Thanks for catching that oops in your tests.
>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> 
> I take that back :-)
> see below
> 
>>> ---
>>>
>>>   drivers/usb/dwc2/hcd.c | 13 ++++++++-----
>>>   1 file changed, 8 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
>>> index dcfda5eb4cac..4ae211f65e85 100644
>>> --- a/drivers/usb/dwc2/hcd.c
>>> +++ b/drivers/usb/dwc2/hcd.c
>>> @@ -359,8 +359,13 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg
>>> *hsotg) static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
>>>
>>>   {
>>>   
>>>   	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
>>>
>>> -	if (IS_ERR(hsotg->vbus_supply))
>>> -		return 0;
>>> +	if (IS_ERR(hsotg->vbus_supply)) {
>>> +		hsotg->vbus_supply = NULL;
>>> +		if (PTR_ERR(hsotg->vbus_supply) == -ENODEV)
> 
> hsotg->vbus_supply is already NULL here
> 

Tomeu is right, here, hsotg->vbus_supply = ERR_PTR(-ENODEV).
/**
  * regulator_get_optional - obtain optional access to a regulator.
  * ...
  * Returns a struct regulator corresponding to the regulator producer,
  * or IS_ERR() condition containing errno.
  * ...
  */

You can add my Reviewed-by: Amelie Delaunay <amelie.delaunay@st.com>

Thanks for the fix,
Amelie

usb: dwc2: dwc2_vbus_supply_init: fix error check

[Amelie Delaunay]

SGkgSGVpa28sDQpIaSBUb21ldSwNCg0KT24gMDMvMjIvMjAxOCAxMjo0MSBQTSwgSGVpa28gU3TD
vGJuZXIgd3JvdGU6DQo+IEhpIFRvbWV1LA0KPiANCj4gQW0gRG9ubmVyc3RhZywgMjIuIE3DpHJ6
IDIwMTgsIDEyOjM5OjEzIENFVCBzY2hyaWViIEhlaWtvIFN0w7xibmVyOg0KPj4gQW0gRG9ubmVy
c3RhZywgMjIuIE3DpHJ6IDIwMTgsIDEwOjM5OjQzIENFVCBzY2hyaWViIFRvbWV1IFZpem9zbzoN
Cj4+PiBkZXZtX3JlZ3VsYXRvcl9nZXRfb3B0aW9uYWwgcmV0dXJucyAtRU5PREVWIGlmIHRoZSBy
ZWd1bGF0b3IgaXNuJ3QNCj4+PiB0aGVyZSwgc28gaWYgdGhhdCdzIHRoZSBjYXNlIHdlIGhhdmUg
dG8gbWFrZSBzdXJlIG5vdCB0byBsZWF2ZSAtRU5PREVWDQo+Pj4gaW4gdGhlIHJlZ3VsYXRvciBw
b2ludGVyLg0KPj4+DQo+Pj4gQWxzbywgbWFrZSBzdXJlIHdlIHJldHVybiAwIGluIHRoYXQgY2Fz
ZSwgYnV0IGNvcnJlY3RseSBwcm9wYWdhdGUgYW55DQo+Pj4gb3RoZXIgZXJyb3JzLiBBbHNvIHBy
b3BhZ2F0ZSB0aGUgZXJyb3IgZnJvbSBfZHdjMl9oY2Rfc3RhcnQuDQo+Pj4NCj4+PiBGaXhlczog
NTMxZWY1ZWJlYTk2ICgidXNiOiBkd2MyOiBhZGQgc3VwcG9ydCBmb3IgaG9zdCBtb2RlIGV4dGVy
bmFsIHZidXMNCj4+PiBzdXBwbHkiKSBDYzogQW1lbGllIERlbGF1bmF5IDxhbWVsaWUuZGVsYXVu
YXlAc3QuY29tPg0KPj4+IFNpZ25lZC1vZmYtYnk6IFRvbWV1IFZpem9zbyA8dG9tZXUudml6b3Nv
QGNvbGxhYm9yYS5jb20+DQo+Pg0KPj4gVGhhbmtzIGZvciBjYXRjaGluZyB0aGF0IG9vcHMgaW4g
eW91ciB0ZXN0cy4NCj4+IFJldmlld2VkLWJ5OiBIZWlrbyBTdHVlYm5lciA8aGVpa29Ac250ZWNo
LmRlPg0KPiANCj4gSSB0YWtlIHRoYXQgYmFjayA6LSkNCj4gc2VlIGJlbG93DQo+IA0KPj4+IC0t
LQ0KPj4+DQo+Pj4gICBkcml2ZXJzL3VzYi9kd2MyL2hjZC5jIHwgMTMgKysrKysrKystLS0tLQ0K
Pj4+ICAgMSBmaWxlIGNoYW5nZWQsIDggaW5zZXJ0aW9ucygrKSwgNSBkZWxldGlvbnMoLSkNCj4+
Pg0KPj4+IGRpZmYgLS1naXQgYS9kcml2ZXJzL3VzYi9kd2MyL2hjZC5jIGIvZHJpdmVycy91c2Iv
ZHdjMi9oY2QuYw0KPj4+IGluZGV4IGRjZmRhNWViNGNhYy4uNGFlMjExZjY1ZTg1IDEwMDY0NA0K
Pj4+IC0tLSBhL2RyaXZlcnMvdXNiL2R3YzIvaGNkLmMNCj4+PiArKysgYi9kcml2ZXJzL3VzYi9k
d2MyL2hjZC5jDQo+Pj4gQEAgLTM1OSw4ICszNTksMTMgQEAgc3RhdGljIHZvaWQgZHdjMl9ndXNi
Y2ZnX2luaXQoc3RydWN0IGR3YzJfaHNvdGcNCj4+PiAqaHNvdGcpIHN0YXRpYyBpbnQgZHdjMl92
YnVzX3N1cHBseV9pbml0KHN0cnVjdCBkd2MyX2hzb3RnICpoc290ZykNCj4+Pg0KPj4+ICAgew0K
Pj4+ICAgDQo+Pj4gICAJaHNvdGctPnZidXNfc3VwcGx5ID0gZGV2bV9yZWd1bGF0b3JfZ2V0X29w
dGlvbmFsKGhzb3RnLT5kZXYsICJ2YnVzIik7DQo+Pj4NCj4+PiAtCWlmIChJU19FUlIoaHNvdGct
PnZidXNfc3VwcGx5KSkNCj4+PiAtCQlyZXR1cm4gMDsNCj4+PiArCWlmIChJU19FUlIoaHNvdGct
PnZidXNfc3VwcGx5KSkgew0KPj4+ICsJCWhzb3RnLT52YnVzX3N1cHBseSA9IE5VTEw7DQo+Pj4g
KwkJaWYgKFBUUl9FUlIoaHNvdGctPnZidXNfc3VwcGx5KSA9PSAtRU5PREVWKQ0KPiANCj4gaHNv
dGctPnZidXNfc3VwcGx5IGlzIGFscmVhZHkgTlVMTCBoZXJlDQo+IA0KDQpUb21ldSBpcyByaWdo
dCwgaGVyZSwgaHNvdGctPnZidXNfc3VwcGx5ID0gRVJSX1BUUigtRU5PREVWKS4NCi8qKg0KICAq
IHJlZ3VsYXRvcl9nZXRfb3B0aW9uYWwgLSBvYnRhaW4gb3B0aW9uYWwgYWNjZXNzIHRvIGEgcmVn
dWxhdG9yLg0KICAqIC4uLg0KICAqIFJldHVybnMgYSBzdHJ1Y3QgcmVndWxhdG9yIGNvcnJlc3Bv
bmRpbmcgdG8gdGhlIHJlZ3VsYXRvciBwcm9kdWNlciwNCiAgKiBvciBJU19FUlIoKSBjb25kaXRp
b24gY29udGFpbmluZyBlcnJuby4NCiAgKiAuLi4NCiAgKi8NCg0KWW91IGNhbiBhZGQgbXkgUmV2
aWV3ZWQtYnk6IEFtZWxpZSBEZWxhdW5heSA8YW1lbGllLmRlbGF1bmF5QHN0LmNvbT4NCg0KVGhh
bmtzIGZvciB0aGUgZml4LA0KQW1lbGll
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Tomeu Vizoso]

devm_regulator_get_optional returns -ENODEV if the regulator isn't
there, so if that's the case we have to make sure not to leave -ENODEV
in the regulator pointer.

Also, make sure we return 0 in that case, but correctly propagate any
other errors. Also propagate the error from _dwc2_hcd_start.

Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus supply")
Cc: Amelie Delaunay <amelie.delaunay@st.com>
Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>

---

v2: Only overwrite the error in the pointer after checking it (Heiko
    Stübner <heiko@sntech.de>)
---
 arch/arm/configs/multi_v7_defconfig |  3 +++
 drivers/usb/dwc2/hcd.c              | 11 +++++++----
 scripts/setlocalversion             |  9 ---------
 3 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 846ce7bb24bc..33148fcabd17 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -1029,3 +1029,6 @@ CONFIG_VIRTIO=y
 CONFIG_VIRTIO_PCI=y
 CONFIG_VIRTIO_PCI_LEGACY=y
 CONFIG_VIRTIO_MMIO=y
+CONFIG_LOCALVERSION_AUTO=n
+CONFIG_LOCALVERSION=""
+
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index dcfda5eb4cac..863aed20517f 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -359,8 +359,13 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
 static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
 {
 	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
-	if (IS_ERR(hsotg->vbus_supply))
+	if (PTR_ERR(hsotg->vbus_supply) == -ENODEV) {
+		hsotg->vbus_supply = NULL;
 		return 0;
+	} else if (IS_ERR(hsotg->vbus_supply)) {
+		hsotg->vbus_supply = NULL;
+		return PTR_ERR(hsotg->vbus_supply);
+	}
 
 	return regulator_enable(hsotg->vbus_supply);
 }
@@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
 
 	spin_unlock_irqrestore(&hsotg->lock, flags);
 
-	dwc2_vbus_supply_init(hsotg);
-
-	return 0;
+	return dwc2_vbus_supply_init(hsotg);
 }
 
 /*
diff --git a/scripts/setlocalversion b/scripts/setlocalversion
index 71f39410691b..cbc36d3b4d0f 100755
--- a/scripts/setlocalversion
+++ b/scripts/setlocalversion
@@ -161,15 +161,6 @@ res="${res}${CONFIG_LOCALVERSION}${LOCALVERSION}"
 if test "$CONFIG_LOCALVERSION_AUTO" = "y"; then
 	# full scm version string
 	res="$res$(scm_version)"
-else
-	# append a plus sign if the repository is not in a clean
-	# annotated or signed tagged state (as git describe only
-	# looks at signed or annotated tags - git tag -a/-s) and
-	# LOCALVERSION= is not specified
-	if test "${LOCALVERSION+set}" != "set"; then
-		scm=$(scm_version --short)
-		res="$res${scm:++}"
-	fi
 fi
 
 echo "$res"
-- 
2.14.3

[PATCH v2] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Tomeu Vizoso]

devm_regulator_get_optional returns -ENODEV if the regulator isn't
there, so if that's the case we have to make sure not to leave -ENODEV
in the regulator pointer.

Also, make sure we return 0 in that case, but correctly propagate any
other errors. Also propagate the error from _dwc2_hcd_start.

Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus supply")
Cc: Amelie Delaunay <amelie.delaunay@st.com>
Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>

---

v2: Only overwrite the error in the pointer after checking it (Heiko
    St?bner <heiko@sntech.de>)
---
 arch/arm/configs/multi_v7_defconfig |  3 +++
 drivers/usb/dwc2/hcd.c              | 11 +++++++----
 scripts/setlocalversion             |  9 ---------
 3 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 846ce7bb24bc..33148fcabd17 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -1029,3 +1029,6 @@ CONFIG_VIRTIO=y
 CONFIG_VIRTIO_PCI=y
 CONFIG_VIRTIO_PCI_LEGACY=y
 CONFIG_VIRTIO_MMIO=y
+CONFIG_LOCALVERSION_AUTO=n
+CONFIG_LOCALVERSION=""
+
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index dcfda5eb4cac..863aed20517f 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -359,8 +359,13 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
 static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
 {
 	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
-	if (IS_ERR(hsotg->vbus_supply))
+	if (PTR_ERR(hsotg->vbus_supply) == -ENODEV) {
+		hsotg->vbus_supply = NULL;
 		return 0;
+	} else if (IS_ERR(hsotg->vbus_supply)) {
+		hsotg->vbus_supply = NULL;
+		return PTR_ERR(hsotg->vbus_supply);
+	}
 
 	return regulator_enable(hsotg->vbus_supply);
 }
@@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
 
 	spin_unlock_irqrestore(&hsotg->lock, flags);
 
-	dwc2_vbus_supply_init(hsotg);
-
-	return 0;
+	return dwc2_vbus_supply_init(hsotg);
 }
 
 /*
diff --git a/scripts/setlocalversion b/scripts/setlocalversion
index 71f39410691b..cbc36d3b4d0f 100755
--- a/scripts/setlocalversion
+++ b/scripts/setlocalversion
@@ -161,15 +161,6 @@ res="${res}${CONFIG_LOCALVERSION}${LOCALVERSION}"
 if test "$CONFIG_LOCALVERSION_AUTO" = "y"; then
 	# full scm version string
 	res="$res$(scm_version)"
-else
-	# append a plus sign if the repository is not in a clean
-	# annotated or signed tagged state (as git describe only
-	# looks at signed or annotated tags - git tag -a/-s) and
-	# LOCALVERSION= is not specified
-	if test "${LOCALVERSION+set}" != "set"; then
-		scm=$(scm_version --short)
-		res="$res${scm:++}"
-	fi
 fi
 
 echo "$res"
-- 
2.14.3

[v2] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Tomeu Vizoso]

devm_regulator_get_optional returns -ENODEV if the regulator isn't
there, so if that's the case we have to make sure not to leave -ENODEV
in the regulator pointer.

Also, make sure we return 0 in that case, but correctly propagate any
other errors. Also propagate the error from _dwc2_hcd_start.

Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus supply")
Cc: Amelie Delaunay <amelie.delaunay@st.com>
Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---

v2: Only overwrite the error in the pointer after checking it (Heiko
    Stübner <heiko@sntech.de>)
---
 arch/arm/configs/multi_v7_defconfig |  3 +++
 drivers/usb/dwc2/hcd.c              | 11 +++++++----
 scripts/setlocalversion             |  9 ---------
 3 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 846ce7bb24bc..33148fcabd17 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -1029,3 +1029,6 @@ CONFIG_VIRTIO=y
 CONFIG_VIRTIO_PCI=y
 CONFIG_VIRTIO_PCI_LEGACY=y
 CONFIG_VIRTIO_MMIO=y
+CONFIG_LOCALVERSION_AUTO=n
+CONFIG_LOCALVERSION=""
+
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index dcfda5eb4cac..863aed20517f 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -359,8 +359,13 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
 static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
 {
 	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
-	if (IS_ERR(hsotg->vbus_supply))
+	if (PTR_ERR(hsotg->vbus_supply) == -ENODEV) {
+		hsotg->vbus_supply = NULL;
 		return 0;
+	} else if (IS_ERR(hsotg->vbus_supply)) {
+		hsotg->vbus_supply = NULL;
+		return PTR_ERR(hsotg->vbus_supply);
+	}
 
 	return regulator_enable(hsotg->vbus_supply);
 }
@@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
 
 	spin_unlock_irqrestore(&hsotg->lock, flags);
 
-	dwc2_vbus_supply_init(hsotg);
-
-	return 0;
+	return dwc2_vbus_supply_init(hsotg);
 }
 
 /*
diff --git a/scripts/setlocalversion b/scripts/setlocalversion
index 71f39410691b..cbc36d3b4d0f 100755
--- a/scripts/setlocalversion
+++ b/scripts/setlocalversion
@@ -161,15 +161,6 @@ res="${res}${CONFIG_LOCALVERSION}${LOCALVERSION}"
 if test "$CONFIG_LOCALVERSION_AUTO" = "y"; then
 	# full scm version string
 	res="$res$(scm_version)"
-else
-	# append a plus sign if the repository is not in a clean
-	# annotated or signed tagged state (as git describe only
-	# looks at signed or annotated tags - git tag -a/-s) and
-	# LOCALVERSION= is not specified
-	if test "${LOCALVERSION+set}" != "set"; then
-		scm=$(scm_version --short)
-		res="$res${scm:++}"
-	fi
 fi
 
 echo "$res"

[PATCH v3] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Tomeu Vizoso]

devm_regulator_get_optional returns -ENODEV if the regulator isn't
there, so if that's the case we have to make sure not to leave -ENODEV
in the regulator pointer.

Also, make sure we return 0 in that case, but correctly propagate any
other errors. Also propagate the error from _dwc2_hcd_start.

Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus supply")
Cc: Amelie Delaunay <amelie.delaunay@st.com>
Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>

---

v2: Only overwrite the error in the pointer after checking it (Heiko
    Stübner <heiko@sntech.de>)
v3: Remove hunks that shouldn't be in this patch
---
 drivers/usb/dwc2/hcd.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index dcfda5eb4cac..863aed20517f 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -359,8 +359,13 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
 static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
 {
 	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
-	if (IS_ERR(hsotg->vbus_supply))
+	if (PTR_ERR(hsotg->vbus_supply) == -ENODEV) {
+		hsotg->vbus_supply = NULL;
 		return 0;
+	} else if (IS_ERR(hsotg->vbus_supply)) {
+		hsotg->vbus_supply = NULL;
+		return PTR_ERR(hsotg->vbus_supply);
+	}
 
 	return regulator_enable(hsotg->vbus_supply);
 }
@@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
 
 	spin_unlock_irqrestore(&hsotg->lock, flags);
 
-	dwc2_vbus_supply_init(hsotg);
-
-	return 0;
+	return dwc2_vbus_supply_init(hsotg);
 }
 
 /*
-- 
2.14.3

[v3] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Tomeu Vizoso]

devm_regulator_get_optional returns -ENODEV if the regulator isn't
there, so if that's the case we have to make sure not to leave -ENODEV
in the regulator pointer.

Also, make sure we return 0 in that case, but correctly propagate any
other errors. Also propagate the error from _dwc2_hcd_start.

Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus supply")
Cc: Amelie Delaunay <amelie.delaunay@st.com>
Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---

v2: Only overwrite the error in the pointer after checking it (Heiko
    Stübner <heiko@sntech.de>)
v3: Remove hunks that shouldn't be in this patch
---
 drivers/usb/dwc2/hcd.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index dcfda5eb4cac..863aed20517f 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -359,8 +359,13 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
 static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
 {
 	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
-	if (IS_ERR(hsotg->vbus_supply))
+	if (PTR_ERR(hsotg->vbus_supply) == -ENODEV) {
+		hsotg->vbus_supply = NULL;
 		return 0;
+	} else if (IS_ERR(hsotg->vbus_supply)) {
+		hsotg->vbus_supply = NULL;
+		return PTR_ERR(hsotg->vbus_supply);
+	}
 
 	return regulator_enable(hsotg->vbus_supply);
 }
@@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
 
 	spin_unlock_irqrestore(&hsotg->lock, flags);
 
-	dwc2_vbus_supply_init(hsotg);
-
-	return 0;
+	return dwc2_vbus_supply_init(hsotg);
 }
 
 /*

Re: [PATCH v3] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Heiko Stübner]

Am Donnerstag, 22. März 2018, 14:14:51 CET schrieb Tomeu Vizoso:
> devm_regulator_get_optional returns -ENODEV if the regulator isn't
> there, so if that's the case we have to make sure not to leave -ENODEV
> in the regulator pointer.
> 
> Also, make sure we return 0 in that case, but correctly propagate any
> other errors. Also propagate the error from _dwc2_hcd_start.
> 
> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus
> supply") Cc: Amelie Delaunay <amelie.delaunay@st.com>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> 
> ---
> 
> v2: Only overwrite the error in the pointer after checking it (Heiko
>     Stübner <heiko@sntech.de>)
> v3: Remove hunks that shouldn't be in this patch
> ---
>  drivers/usb/dwc2/hcd.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index dcfda5eb4cac..863aed20517f 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -359,8 +359,13 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
> static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
>  {
>  	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
> -	if (IS_ERR(hsotg->vbus_supply))
> +	if (PTR_ERR(hsotg->vbus_supply) == -ENODEV) {
> +		hsotg->vbus_supply = NULL;
>  		return 0;
> +	} else if (IS_ERR(hsotg->vbus_supply)) {
> +		hsotg->vbus_supply = NULL;
> +		return PTR_ERR(hsotg->vbus_supply);
> +	}

my personal cluelessnes, but can you use PTR_ERR without checking for
IS_ERR first?

I would've expected something along the line of

if (IS_ERR(hsotg->vbus_supply)) {
	if (PTR_ERR(hsotg->vbus_supply) == -ENODEV) {
		hsotg->vbus_supply = NULL;
		return 0;
	} else {
		return PTR_ERR(hsotg->vbus_supply);
	}
}

[v3] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Heiko Stuebner]

Am Donnerstag, 22. März 2018, 14:14:51 CET schrieb Tomeu Vizoso:
> devm_regulator_get_optional returns -ENODEV if the regulator isn't
> there, so if that's the case we have to make sure not to leave -ENODEV
> in the regulator pointer.
> 
> Also, make sure we return 0 in that case, but correctly propagate any
> other errors. Also propagate the error from _dwc2_hcd_start.
> 
> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus
> supply") Cc: Amelie Delaunay <amelie.delaunay@st.com>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> 
> ---
> 
> v2: Only overwrite the error in the pointer after checking it (Heiko
>     Stübner <heiko@sntech.de>)
> v3: Remove hunks that shouldn't be in this patch
> ---
>  drivers/usb/dwc2/hcd.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index dcfda5eb4cac..863aed20517f 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -359,8 +359,13 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
> static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
>  {
>  	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
> -	if (IS_ERR(hsotg->vbus_supply))
> +	if (PTR_ERR(hsotg->vbus_supply) == -ENODEV) {
> +		hsotg->vbus_supply = NULL;
>  		return 0;
> +	} else if (IS_ERR(hsotg->vbus_supply)) {
> +		hsotg->vbus_supply = NULL;
> +		return PTR_ERR(hsotg->vbus_supply);
> +	}

my personal cluelessnes, but can you use PTR_ERR without checking for
IS_ERR first?

I would've expected something along the line of

if (IS_ERR(hsotg->vbus_supply)) {
	if (PTR_ERR(hsotg->vbus_supply) == -ENODEV) {
		hsotg->vbus_supply = NULL;
		return 0;
	} else {
		return PTR_ERR(hsotg->vbus_supply);
	}
}
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Tomeu Vizoso]

On 03/22/2018 02:26 PM, Heiko Stübner wrote:
> Am Donnerstag, 22. März 2018, 14:14:51 CET schrieb Tomeu Vizoso:
>> devm_regulator_get_optional returns -ENODEV if the regulator isn't
>> there, so if that's the case we have to make sure not to leave -ENODEV
>> in the regulator pointer.
>>
>> Also, make sure we return 0 in that case, but correctly propagate any
>> other errors. Also propagate the error from _dwc2_hcd_start.
>>
>> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus
>> supply") Cc: Amelie Delaunay <amelie.delaunay@st.com>
>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>>
>> ---
>>
>> v2: Only overwrite the error in the pointer after checking it (Heiko
>>      Stübner <heiko@sntech.de>)
>> v3: Remove hunks that shouldn't be in this patch
>> ---
>>   drivers/usb/dwc2/hcd.c | 11 +++++++----
>>   1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
>> index dcfda5eb4cac..863aed20517f 100644
>> --- a/drivers/usb/dwc2/hcd.c
>> +++ b/drivers/usb/dwc2/hcd.c
>> @@ -359,8 +359,13 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
>> static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
>>   {
>>   	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
>> -	if (IS_ERR(hsotg->vbus_supply))
>> +	if (PTR_ERR(hsotg->vbus_supply) == -ENODEV) {
>> +		hsotg->vbus_supply = NULL;
>>   		return 0;
>> +	} else if (IS_ERR(hsotg->vbus_supply)) {
>> +		hsotg->vbus_supply = NULL;
>> +		return PTR_ERR(hsotg->vbus_supply);
>> +	}
> 
> my personal cluelessnes, but can you use PTR_ERR without checking for
> IS_ERR first?

It's just a cast, so it should be fine.

> I would've expected something along the line of
> 
> if (IS_ERR(hsotg->vbus_supply)) {
> 	if (PTR_ERR(hsotg->vbus_supply) == -ENODEV) {
> 		hsotg->vbus_supply = NULL;
> 		return 0;
> 	} else {
> 		return PTR_ERR(hsotg->vbus_supply);
> 	}
> }

I kind of liked to avoid one indentation level. Also wanted to play safe 
and NULLify the pointer in both code paths.

Thanks,

Tomeu

[v3] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Tomeu Vizoso]

On 03/22/2018 02:26 PM, Heiko Stübner wrote:
> Am Donnerstag, 22. März 2018, 14:14:51 CET schrieb Tomeu Vizoso:
>> devm_regulator_get_optional returns -ENODEV if the regulator isn't
>> there, so if that's the case we have to make sure not to leave -ENODEV
>> in the regulator pointer.
>>
>> Also, make sure we return 0 in that case, but correctly propagate any
>> other errors. Also propagate the error from _dwc2_hcd_start.
>>
>> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus
>> supply") Cc: Amelie Delaunay <amelie.delaunay@st.com>
>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>>
>> ---
>>
>> v2: Only overwrite the error in the pointer after checking it (Heiko
>>      Stübner <heiko@sntech.de>)
>> v3: Remove hunks that shouldn't be in this patch
>> ---
>>   drivers/usb/dwc2/hcd.c | 11 +++++++----
>>   1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
>> index dcfda5eb4cac..863aed20517f 100644
>> --- a/drivers/usb/dwc2/hcd.c
>> +++ b/drivers/usb/dwc2/hcd.c
>> @@ -359,8 +359,13 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
>> static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
>>   {
>>   	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
>> -	if (IS_ERR(hsotg->vbus_supply))
>> +	if (PTR_ERR(hsotg->vbus_supply) == -ENODEV) {
>> +		hsotg->vbus_supply = NULL;
>>   		return 0;
>> +	} else if (IS_ERR(hsotg->vbus_supply)) {
>> +		hsotg->vbus_supply = NULL;
>> +		return PTR_ERR(hsotg->vbus_supply);
>> +	}
> 
> my personal cluelessnes, but can you use PTR_ERR without checking for
> IS_ERR first?

It's just a cast, so it should be fine.

> I would've expected something along the line of
> 
> if (IS_ERR(hsotg->vbus_supply)) {
> 	if (PTR_ERR(hsotg->vbus_supply) == -ENODEV) {
> 		hsotg->vbus_supply = NULL;
> 		return 0;
> 	} else {
> 		return PTR_ERR(hsotg->vbus_supply);
> 	}
> }

I kind of liked to avoid one indentation level. Also wanted to play safe 
and NULLify the pointer in both code paths.

Thanks,

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

Re: [PATCH v3] usb: dwc2: dwc2_vbus_supply_init: fix error check

[kbuild test robot]

Hi Tomeu,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on balbi-usb/next]
[also build test WARNING on next-20180323]
[cannot apply to v4.16-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Tomeu-Vizoso/usb-dwc2-dwc2_vbus_supply_init-fix-error-check/20180324-063752
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next


coccinelle warnings: (new ones prefixed by >>)

>> drivers/usb/dwc2/hcd.c:367:9-16: ERROR: PTR_ERR applied after initialization to constant on line 366

vim +367 drivers/usb/dwc2/hcd.c

   358	
   359	static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
   360	{
   361		hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
   362		if (PTR_ERR(hsotg->vbus_supply) == -ENODEV) {
   363			hsotg->vbus_supply = NULL;
   364			return 0;
   365		} else if (IS_ERR(hsotg->vbus_supply)) {
 > 366			hsotg->vbus_supply = NULL;
 > 367			return PTR_ERR(hsotg->vbus_supply);
   368		}
   369	
   370		return regulator_enable(hsotg->vbus_supply);
   371	}
   372	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[v3] usb: dwc2: dwc2_vbus_supply_init: fix error check

[kbuild test robot]

Hi Tomeu,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on balbi-usb/next]
[also build test WARNING on next-20180323]
[cannot apply to v4.16-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Tomeu-Vizoso/usb-dwc2-dwc2_vbus_supply_init-fix-error-check/20180324-063752
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next


coccinelle warnings: (new ones prefixed by >>)

>> drivers/usb/dwc2/hcd.c:367:9-16: ERROR: PTR_ERR applied after initialization to constant on line 366

vim +367 drivers/usb/dwc2/hcd.c

   358	
   359	static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
   360	{
   361		hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
   362		if (PTR_ERR(hsotg->vbus_supply) == -ENODEV) {
   363			hsotg->vbus_supply = NULL;
   364			return 0;
   365		} else if (IS_ERR(hsotg->vbus_supply)) {
 > 366			hsotg->vbus_supply = NULL;
 > 367			return PTR_ERR(hsotg->vbus_supply);
   368		}
   369	
   370		return regulator_enable(hsotg->vbus_supply);
   371	}
   372
---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Tomeu Vizoso]

devm_regulator_get_optional returns -ENODEV if the regulator isn't
there, so if that's the case we have to make sure not to leave -ENODEV
in the regulator pointer.

Also, make sure we return 0 in that case, but correctly propagate any
other errors. Also propagate the error from _dwc2_hcd_start.

Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus supply")
Cc: Amelie Delaunay <amelie.delaunay@st.com>
Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>

---

v2: Only overwrite the error in the pointer after checking it (Heiko
    Stübner <heiko@sntech.de>)
v3: Remove hunks that shouldn't be in this patch
v4: Don't overwrite the error code before returning it (kbuild test
    robot <lkp@intel.com>)
---
 drivers/usb/dwc2/hcd.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 190f95964000..c51b73b3e048 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -358,9 +358,14 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
 
 static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
 {
+	int ret;
+
 	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
-	if (IS_ERR(hsotg->vbus_supply))
-		return 0;
+	if (IS_ERR(hsotg->vbus_supply)) {
+		ret = PTR_ERR(hsotg->vbus_supply);
+		hsotg->vbus_supply = NULL;
+		return ret == -ENODEV ? 0 : ret;
+	}
 
 	return regulator_enable(hsotg->vbus_supply);
 }
@@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
 
 	spin_unlock_irqrestore(&hsotg->lock, flags);
 
-	dwc2_vbus_supply_init(hsotg);
-
-	return 0;
+	return dwc2_vbus_supply_init(hsotg);
 }
 
 /*
-- 
2.14.3

[v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Tomeu Vizoso]

devm_regulator_get_optional returns -ENODEV if the regulator isn't
there, so if that's the case we have to make sure not to leave -ENODEV
in the regulator pointer.

Also, make sure we return 0 in that case, but correctly propagate any
other errors. Also propagate the error from _dwc2_hcd_start.

Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus supply")
Cc: Amelie Delaunay <amelie.delaunay@st.com>
Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---

v2: Only overwrite the error in the pointer after checking it (Heiko
    Stübner <heiko@sntech.de>)
v3: Remove hunks that shouldn't be in this patch
v4: Don't overwrite the error code before returning it (kbuild test
    robot <lkp@intel.com>)
---
 drivers/usb/dwc2/hcd.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index 190f95964000..c51b73b3e048 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -358,9 +358,14 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
 
 static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
 {
+	int ret;
+
 	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
-	if (IS_ERR(hsotg->vbus_supply))
-		return 0;
+	if (IS_ERR(hsotg->vbus_supply)) {
+		ret = PTR_ERR(hsotg->vbus_supply);
+		hsotg->vbus_supply = NULL;
+		return ret == -ENODEV ? 0 : ret;
+	}
 
 	return regulator_enable(hsotg->vbus_supply);
 }
@@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
 
 	spin_unlock_irqrestore(&hsotg->lock, flags);
 
-	dwc2_vbus_supply_init(hsotg);
-
-	return 0;
+	return dwc2_vbus_supply_init(hsotg);
 }
 
 /*

Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Amelie DELAUNAY]

Hi,

On 03/26/2018 11:00 AM, Tomeu Vizoso wrote:
> devm_regulator_get_optional returns -ENODEV if the regulator isn't
> there, so if that's the case we have to make sure not to leave -ENODEV
> in the regulator pointer.
> 
> Also, make sure we return 0 in that case, but correctly propagate any
> other errors. Also propagate the error from _dwc2_hcd_start.
> 
> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus supply")
> Cc: Amelie Delaunay <amelie.delaunay@st.com>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> 
> ---
> 
> v2: Only overwrite the error in the pointer after checking it (Heiko
>      Stübner <heiko@sntech.de>)
> v3: Remove hunks that shouldn't be in this patch
> v4: Don't overwrite the error code before returning it (kbuild test
>      robot <lkp@intel.com>)
> ---
>   drivers/usb/dwc2/hcd.c | 13 ++++++++-----
>   1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index 190f95964000..c51b73b3e048 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -358,9 +358,14 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
>   
>   static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
>   {
> +	int ret;
> +
>   	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
> -	if (IS_ERR(hsotg->vbus_supply))
> -		return 0;
> +	if (IS_ERR(hsotg->vbus_supply)) {
> +		ret = PTR_ERR(hsotg->vbus_supply);
> +		hsotg->vbus_supply = NULL;
> +		return ret == -ENODEV ? 0 : ret;
> +	}
>   
>   	return regulator_enable(hsotg->vbus_supply);
>   }
> @@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
>   
>   	spin_unlock_irqrestore(&hsotg->lock, flags);
>   
> -	dwc2_vbus_supply_init(hsotg);
> -
> -	return 0;
> +	return dwc2_vbus_supply_init(hsotg);
>   }
>   
>   /*
> 

Reviewed-by: Amelie Delaunay <amelie.delaunay@st.com>

Thanks,
Amelie

[v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Amelie Delaunay]

Hi,

On 03/26/2018 11:00 AM, Tomeu Vizoso wrote:
> devm_regulator_get_optional returns -ENODEV if the regulator isn't
> there, so if that's the case we have to make sure not to leave -ENODEV
> in the regulator pointer.
> 
> Also, make sure we return 0 in that case, but correctly propagate any
> other errors. Also propagate the error from _dwc2_hcd_start.
> 
> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus supply")
> Cc: Amelie Delaunay <amelie.delaunay@st.com>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> 
> ---
> 
> v2: Only overwrite the error in the pointer after checking it (Heiko
>      Stübner <heiko@sntech.de>)
> v3: Remove hunks that shouldn't be in this patch
> v4: Don't overwrite the error code before returning it (kbuild test
>      robot <lkp@intel.com>)
> ---
>   drivers/usb/dwc2/hcd.c | 13 ++++++++-----
>   1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index 190f95964000..c51b73b3e048 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -358,9 +358,14 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
>   
>   static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
>   {
> +	int ret;
> +
>   	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
> -	if (IS_ERR(hsotg->vbus_supply))
> -		return 0;
> +	if (IS_ERR(hsotg->vbus_supply)) {
> +		ret = PTR_ERR(hsotg->vbus_supply);
> +		hsotg->vbus_supply = NULL;
> +		return ret == -ENODEV ? 0 : ret;
> +	}
>   
>   	return regulator_enable(hsotg->vbus_supply);
>   }
> @@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
>   
>   	spin_unlock_irqrestore(&hsotg->lock, flags);
>   
> -	dwc2_vbus_supply_init(hsotg);
> -
> -	return 0;
> +	return dwc2_vbus_supply_init(hsotg);
>   }
>   
>   /*
> 

Reviewed-by: Amelie Delaunay <amelie.delaunay@st.com>

Thanks,
Amelie

Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Heiko Stübner]

Am Montag, 26. März 2018, 11:00:01 CEST schrieb Tomeu Vizoso:
> devm_regulator_get_optional returns -ENODEV if the regulator isn't
> there, so if that's the case we have to make sure not to leave -ENODEV
> in the regulator pointer.
> 
> Also, make sure we return 0 in that case, but correctly propagate any
> other errors. Also propagate the error from _dwc2_hcd_start.
> 
> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus
> supply") Cc: Amelie Delaunay <amelie.delaunay@st.com>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>

Reviewed-by: Heiko Stuebner <heiko@sntech.de>

[v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Heiko Stuebner]

Am Montag, 26. März 2018, 11:00:01 CEST schrieb Tomeu Vizoso:
> devm_regulator_get_optional returns -ENODEV if the regulator isn't
> there, so if that's the case we have to make sure not to leave -ENODEV
> in the regulator pointer.
> 
> Also, make sure we return 0 in that case, but correctly propagate any
> other errors. Also propagate the error from _dwc2_hcd_start.
> 
> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus
> supply") Cc: Amelie Delaunay <amelie.delaunay@st.com>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>

Reviewed-by: Heiko Stuebner <heiko@sntech.de>
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Tomeu Vizoso]

Could this patch be picked up, please?

Thanks,

Tomeu

On 26 March 2018 at 13:51, Heiko Stübner <heiko@sntech.de> wrote:
> Am Montag, 26. März 2018, 11:00:01 CEST schrieb Tomeu Vizoso:
>> devm_regulator_get_optional returns -ENODEV if the regulator isn't
>> there, so if that's the case we have to make sure not to leave -ENODEV
>> in the regulator pointer.
>>
>> Also, make sure we return 0 in that case, but correctly propagate any
>> other errors. Also propagate the error from _dwc2_hcd_start.
>>
>> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus
>> supply") Cc: Amelie Delaunay <amelie.delaunay@st.com>
>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>

[v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Tomeu Vizoso]

Could this patch be picked up, please?

Thanks,

Tomeu

On 26 March 2018 at 13:51, Heiko Stübner <heiko@sntech.de> wrote:
> Am Montag, 26. März 2018, 11:00:01 CEST schrieb Tomeu Vizoso:
>> devm_regulator_get_optional returns -ENODEV if the regulator isn't
>> there, so if that's the case we have to make sure not to leave -ENODEV
>> in the regulator pointer.
>>
>> Also, make sure we return 0 in that case, but correctly propagate any
>> other errors. Also propagate the error from _dwc2_hcd_start.
>>
>> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus
>> supply") Cc: Amelie Delaunay <amelie.delaunay@st.com>
>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Minas Harutyunyan]

Hi Tomeu,

On 3/26/2018 1:01 PM, Tomeu Vizoso wrote:
> devm_regulator_get_optional returns -ENODEV if the regulator isn't
> there, so if that's the case we have to make sure not to leave -ENODEV
> in the regulator pointer.
> 
> Also, make sure we return 0 in that case, but correctly propagate any
> other errors. Also propagate the error from _dwc2_hcd_start.
> 
> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus supply")
> Cc: Amelie Delaunay <amelie.delaunay@st.com>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> 
> ---
> 
> v2: Only overwrite the error in the pointer after checking it (Heiko
>      Stübner <heiko@sntech.de>)
> v3: Remove hunks that shouldn't be in this patch
> v4: Don't overwrite the error code before returning it (kbuild test
>      robot <lkp@intel.com>)
> ---
>   drivers/usb/dwc2/hcd.c | 13 ++++++++-----
>   1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index 190f95964000..c51b73b3e048 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -358,9 +358,14 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
>   
>   static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
>   {
> +	int ret;
> +
>   	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
> -	if (IS_ERR(hsotg->vbus_supply))
> -		return 0;
> +	if (IS_ERR(hsotg->vbus_supply)) {
> +		ret = PTR_ERR(hsotg->vbus_supply);
> +		hsotg->vbus_supply = NULL;
> +		return ret == -ENODEV ? 0 : ret;
> +	}
>   
>   	return regulator_enable(hsotg->vbus_supply);
>   }
> @@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
>   
>   	spin_unlock_irqrestore(&hsotg->lock, flags);
>   
> -	dwc2_vbus_supply_init(hsotg);
> -
> -	return 0;
> +	return dwc2_vbus_supply_init(hsotg);
>   }
>   
>   /*
> 

Not able to apply patch. Please rebase to balbi/next

Thanks,
Minas

[v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Minas Harutyunyan]

Hi Tomeu,

On 3/26/2018 1:01 PM, Tomeu Vizoso wrote:
> devm_regulator_get_optional returns -ENODEV if the regulator isn't
> there, so if that's the case we have to make sure not to leave -ENODEV
> in the regulator pointer.
> 
> Also, make sure we return 0 in that case, but correctly propagate any
> other errors. Also propagate the error from _dwc2_hcd_start.
> 
> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus supply")
> Cc: Amelie Delaunay <amelie.delaunay@st.com>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> 
> ---
> 
> v2: Only overwrite the error in the pointer after checking it (Heiko
>      Stübner <heiko@sntech.de>)
> v3: Remove hunks that shouldn't be in this patch
> v4: Don't overwrite the error code before returning it (kbuild test
>      robot <lkp@intel.com>)
> ---
>   drivers/usb/dwc2/hcd.c | 13 ++++++++-----
>   1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index 190f95964000..c51b73b3e048 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -358,9 +358,14 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
>   
>   static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
>   {
> +	int ret;
> +
>   	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
> -	if (IS_ERR(hsotg->vbus_supply))
> -		return 0;
> +	if (IS_ERR(hsotg->vbus_supply)) {
> +		ret = PTR_ERR(hsotg->vbus_supply);
> +		hsotg->vbus_supply = NULL;
> +		return ret == -ENODEV ? 0 : ret;
> +	}
>   
>   	return regulator_enable(hsotg->vbus_supply);
>   }
> @@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
>   
>   	spin_unlock_irqrestore(&hsotg->lock, flags);
>   
> -	dwc2_vbus_supply_init(hsotg);
> -
> -	return 0;
> +	return dwc2_vbus_supply_init(hsotg);
>   }
>   
>   /*
> 

Not able to apply patch. Please rebase to balbi/next

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

Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Tomeu Vizoso]

Hi Minas,

On 04/05/2018 09:54 AM, Minas Harutyunyan wrote:
> Hi Tomeu,
> 
> On 3/26/2018 1:01 PM, Tomeu Vizoso wrote:
>> devm_regulator_get_optional returns -ENODEV if the regulator isn't
>> there, so if that's the case we have to make sure not to leave -ENODEV
>> in the regulator pointer.
>>
>> Also, make sure we return 0 in that case, but correctly propagate any
>> other errors. Also propagate the error from _dwc2_hcd_start.
>>
>> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus supply")
>> Cc: Amelie Delaunay <amelie.delaunay@st.com>
>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>>
>> ---
>>
>> v2: Only overwrite the error in the pointer after checking it (Heiko
>>       Stübner <heiko@sntech.de>)
>> v3: Remove hunks that shouldn't be in this patch
>> v4: Don't overwrite the error code before returning it (kbuild test
>>       robot <lkp@intel.com>)
>> ---
>>    drivers/usb/dwc2/hcd.c | 13 ++++++++-----
>>    1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
>> index 190f95964000..c51b73b3e048 100644
>> --- a/drivers/usb/dwc2/hcd.c
>> +++ b/drivers/usb/dwc2/hcd.c
>> @@ -358,9 +358,14 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
>>    
>>    static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
>>    {
>> +	int ret;
>> +
>>    	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
>> -	if (IS_ERR(hsotg->vbus_supply))
>> -		return 0;
>> +	if (IS_ERR(hsotg->vbus_supply)) {
>> +		ret = PTR_ERR(hsotg->vbus_supply);
>> +		hsotg->vbus_supply = NULL;
>> +		return ret == -ENODEV ? 0 : ret;
>> +	}
>>    
>>    	return regulator_enable(hsotg->vbus_supply);
>>    }
>> @@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
>>    
>>    	spin_unlock_irqrestore(&hsotg->lock, flags);
>>    
>> -	dwc2_vbus_supply_init(hsotg);
>> -
>> -	return 0;
>> +	return dwc2_vbus_supply_init(hsotg);
>>    }
>>    
>>    /*
>>
> 
> Not able to apply patch. Please rebase to balbi/next

Are you sure? Just rebased and the resulting patch is identical to what 
was sent.

Thanks,

Tomeu

[v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Tomeu Vizoso]

Hi Minas,

On 04/05/2018 09:54 AM, Minas Harutyunyan wrote:
> Hi Tomeu,
> 
> On 3/26/2018 1:01 PM, Tomeu Vizoso wrote:
>> devm_regulator_get_optional returns -ENODEV if the regulator isn't
>> there, so if that's the case we have to make sure not to leave -ENODEV
>> in the regulator pointer.
>>
>> Also, make sure we return 0 in that case, but correctly propagate any
>> other errors. Also propagate the error from _dwc2_hcd_start.
>>
>> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus supply")
>> Cc: Amelie Delaunay <amelie.delaunay@st.com>
>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>>
>> ---
>>
>> v2: Only overwrite the error in the pointer after checking it (Heiko
>>       Stübner <heiko@sntech.de>)
>> v3: Remove hunks that shouldn't be in this patch
>> v4: Don't overwrite the error code before returning it (kbuild test
>>       robot <lkp@intel.com>)
>> ---
>>    drivers/usb/dwc2/hcd.c | 13 ++++++++-----
>>    1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
>> index 190f95964000..c51b73b3e048 100644
>> --- a/drivers/usb/dwc2/hcd.c
>> +++ b/drivers/usb/dwc2/hcd.c
>> @@ -358,9 +358,14 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
>>    
>>    static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
>>    {
>> +	int ret;
>> +
>>    	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
>> -	if (IS_ERR(hsotg->vbus_supply))
>> -		return 0;
>> +	if (IS_ERR(hsotg->vbus_supply)) {
>> +		ret = PTR_ERR(hsotg->vbus_supply);
>> +		hsotg->vbus_supply = NULL;
>> +		return ret == -ENODEV ? 0 : ret;
>> +	}
>>    
>>    	return regulator_enable(hsotg->vbus_supply);
>>    }
>> @@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
>>    
>>    	spin_unlock_irqrestore(&hsotg->lock, flags);
>>    
>> -	dwc2_vbus_supply_init(hsotg);
>> -
>> -	return 0;
>> +	return dwc2_vbus_supply_init(hsotg);
>>    }
>>    
>>    /*
>>
> 
> Not able to apply patch. Please rebase to balbi/next

Are you sure? Just rebased and the resulting patch is identical to what 
was sent.

Thanks,

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

Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Grigor Tovmasyan]

On 4/5/2018 12:11 PM, Tomeu Vizoso wrote:
> Hi Minas,
> 
> On 04/05/2018 09:54 AM, Minas Harutyunyan wrote:
>> Hi Tomeu,
>>
>> On 3/26/2018 1:01 PM, Tomeu Vizoso wrote:
>>> devm_regulator_get_optional returns -ENODEV if the regulator isn't
>>> there, so if that's the case we have to make sure not to leave -ENODEV
>>> in the regulator pointer.
>>>
>>> Also, make sure we return 0 in that case, but correctly propagate any
>>> other errors. Also propagate the error from _dwc2_hcd_start.
>>>
>>> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus supply")
>>> Cc: Amelie Delaunay <amelie.delaunay@st.com>
>>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>>>
>>> ---
>>>
>>> v2: Only overwrite the error in the pointer after checking it (Heiko
>>>        Stübner <heiko@sntech.de>)
>>> v3: Remove hunks that shouldn't be in this patch
>>> v4: Don't overwrite the error code before returning it (kbuild test
>>>        robot <lkp@intel.com>)
>>> ---
>>>     drivers/usb/dwc2/hcd.c | 13 ++++++++-----
>>>     1 file changed, 8 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
>>> index 190f95964000..c51b73b3e048 100644
>>> --- a/drivers/usb/dwc2/hcd.c
>>> +++ b/drivers/usb/dwc2/hcd.c
>>> @@ -358,9 +358,14 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
>>>     
>>>     static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
>>>     {
>>> +	int ret;
>>> +
>>>     	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
>>> -	if (IS_ERR(hsotg->vbus_supply))
>>> -		return 0;
>>> +	if (IS_ERR(hsotg->vbus_supply)) {
>>> +		ret = PTR_ERR(hsotg->vbus_supply);
>>> +		hsotg->vbus_supply = NULL;
>>> +		return ret == -ENODEV ? 0 : ret;
>>> +	}
>>>     
>>>     	return regulator_enable(hsotg->vbus_supply);
>>>     }
>>> @@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
>>>     
>>>     	spin_unlock_irqrestore(&hsotg->lock, flags);
>>>     
>>> -	dwc2_vbus_supply_init(hsotg);
>>> -
>>> -	return 0;
>>> +	return dwc2_vbus_supply_init(hsotg);
>>>     }
>>>     
>>>     /*
>>>
>>
>> Not able to apply patch. Please rebase to balbi/next
> 
> Are you sure? Just rebased and the resulting patch is identical to what
> was sent.

Sorry for inconvenience. We fixed the problem.
No need to rebase.

Thanks,
Grigor.

> 
> Thanks,
> 
> Tomeu
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  https://urldefense.proofpoint.com/v2/url?u=http-3A__vger.kernel.org_majordomo-2Dinfo.html&d=DwIDaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=K1ULVL1slpLXpMJJlAXSOxws4tRq0IkTBqxDkyW2hUQ&m=q-x0UcPJc6m4O6WQ6ZklCRyxp1ZT8QOQDRuaA_B6m6c&s=F7VAPO-hcO1TMfvPHhEkJYgX-8exTHxrioO3C6oNubs&e=
> 

[v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Grigor Tovmasyan]

On 4/5/2018 12:11 PM, Tomeu Vizoso wrote:
> Hi Minas,
> 
> On 04/05/2018 09:54 AM, Minas Harutyunyan wrote:
>> Hi Tomeu,
>>
>> On 3/26/2018 1:01 PM, Tomeu Vizoso wrote:
>>> devm_regulator_get_optional returns -ENODEV if the regulator isn't
>>> there, so if that's the case we have to make sure not to leave -ENODEV
>>> in the regulator pointer.
>>>
>>> Also, make sure we return 0 in that case, but correctly propagate any
>>> other errors. Also propagate the error from _dwc2_hcd_start.
>>>
>>> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus supply")
>>> Cc: Amelie Delaunay <amelie.delaunay@st.com>
>>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>>>
>>> ---
>>>
>>> v2: Only overwrite the error in the pointer after checking it (Heiko
>>>        Stübner <heiko@sntech.de>)
>>> v3: Remove hunks that shouldn't be in this patch
>>> v4: Don't overwrite the error code before returning it (kbuild test
>>>        robot <lkp@intel.com>)
>>> ---
>>>     drivers/usb/dwc2/hcd.c | 13 ++++++++-----
>>>     1 file changed, 8 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
>>> index 190f95964000..c51b73b3e048 100644
>>> --- a/drivers/usb/dwc2/hcd.c
>>> +++ b/drivers/usb/dwc2/hcd.c
>>> @@ -358,9 +358,14 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
>>>     
>>>     static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
>>>     {
>>> +	int ret;
>>> +
>>>     	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
>>> -	if (IS_ERR(hsotg->vbus_supply))
>>> -		return 0;
>>> +	if (IS_ERR(hsotg->vbus_supply)) {
>>> +		ret = PTR_ERR(hsotg->vbus_supply);
>>> +		hsotg->vbus_supply = NULL;
>>> +		return ret == -ENODEV ? 0 : ret;
>>> +	}
>>>     
>>>     	return regulator_enable(hsotg->vbus_supply);
>>>     }
>>> @@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
>>>     
>>>     	spin_unlock_irqrestore(&hsotg->lock, flags);
>>>     
>>> -	dwc2_vbus_supply_init(hsotg);
>>> -
>>> -	return 0;
>>> +	return dwc2_vbus_supply_init(hsotg);
>>>     }
>>>     
>>>     /*
>>>
>>
>> Not able to apply patch. Please rebase to balbi/next
> 
> Are you sure? Just rebased and the resulting patch is identical to what
> was sent.

Sorry for inconvenience. We fixed the problem.
No need to rebase.

Thanks,
Grigor.

> 
> Thanks,
> 
> Tomeu
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  https://urldefense.proofpoint.com/v2/url?u=http-3A__vger.kernel.org_majordomo-2Dinfo.html&d=DwIDaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=K1ULVL1slpLXpMJJlAXSOxws4tRq0IkTBqxDkyW2hUQ&m=q-x0UcPJc6m4O6WQ6ZklCRyxp1ZT8QOQDRuaA_B6m6c&s=F7VAPO-hcO1TMfvPHhEkJYgX-8exTHxrioO3C6oNubs&e=
>
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Grigor Tovmasyan]

On 3/26/2018 1:01 PM, Tomeu Vizoso wrote:
> devm_regulator_get_optional returns -ENODEV if the regulator isn't
> there, so if that's the case we have to make sure not to leave -ENODEV
> in the regulator pointer.
> 
> Also, make sure we return 0 in that case, but correctly propagate any
> other errors. Also propagate the error from _dwc2_hcd_start.
> 
> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus supply")
> Cc: Amelie Delaunay <amelie.delaunay@st.com>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> 
> ---
> 
> v2: Only overwrite the error in the pointer after checking it (Heiko
>      Stübner <heiko@sntech.de>)
> v3: Remove hunks that shouldn't be in this patch
> v4: Don't overwrite the error code before returning it (kbuild test
>      robot <lkp@intel.com>)
> ---
>   drivers/usb/dwc2/hcd.c | 13 ++++++++-----
>   1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index 190f95964000..c51b73b3e048 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -358,9 +358,14 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
>   
>   static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
>   {
> +	int ret;
> +
>   	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
> -	if (IS_ERR(hsotg->vbus_supply))
> -		return 0;
> +	if (IS_ERR(hsotg->vbus_supply)) {
> +		ret = PTR_ERR(hsotg->vbus_supply);
> +		hsotg->vbus_supply = NULL;
> +		return ret == -ENODEV ? 0 : ret;
> +	}
>   
>   	return regulator_enable(hsotg->vbus_supply);
>   }
> @@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
>   
>   	spin_unlock_irqrestore(&hsotg->lock, flags);
>   
> -	dwc2_vbus_supply_init(hsotg);
> -
> -	return 0;
> +	return dwc2_vbus_supply_init(hsotg);
>   }
>   
>   /*
> 

Reviewed-by: Grigor Tovmasyan <tovmasya@synopsys.com>

[v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Grigor Tovmasyan]

On 3/26/2018 1:01 PM, Tomeu Vizoso wrote:
> devm_regulator_get_optional returns -ENODEV if the regulator isn't
> there, so if that's the case we have to make sure not to leave -ENODEV
> in the regulator pointer.
> 
> Also, make sure we return 0 in that case, but correctly propagate any
> other errors. Also propagate the error from _dwc2_hcd_start.
> 
> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus supply")
> Cc: Amelie Delaunay <amelie.delaunay@st.com>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> 
> ---
> 
> v2: Only overwrite the error in the pointer after checking it (Heiko
>      Stübner <heiko@sntech.de>)
> v3: Remove hunks that shouldn't be in this patch
> v4: Don't overwrite the error code before returning it (kbuild test
>      robot <lkp@intel.com>)
> ---
>   drivers/usb/dwc2/hcd.c | 13 ++++++++-----
>   1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index 190f95964000..c51b73b3e048 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -358,9 +358,14 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
>   
>   static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
>   {
> +	int ret;
> +
>   	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
> -	if (IS_ERR(hsotg->vbus_supply))
> -		return 0;
> +	if (IS_ERR(hsotg->vbus_supply)) {
> +		ret = PTR_ERR(hsotg->vbus_supply);
> +		hsotg->vbus_supply = NULL;
> +		return ret == -ENODEV ? 0 : ret;
> +	}
>   
>   	return regulator_enable(hsotg->vbus_supply);
>   }
> @@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
>   
>   	spin_unlock_irqrestore(&hsotg->lock, flags);
>   
> -	dwc2_vbus_supply_init(hsotg);
> -
> -	return 0;
> +	return dwc2_vbus_supply_init(hsotg);
>   }
>   
>   /*
> 

Reviewed-by: Grigor Tovmasyan <tovmasya@synopsys.com>
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Minas Harutyunyan]

On 4/5/2018 12:59 PM, Grigor Tovmasyan wrote:
> On 3/26/2018 1:01 PM, Tomeu Vizoso wrote:
>> devm_regulator_get_optional returns -ENODEV if the regulator isn't
>> there, so if that's the case we have to make sure not to leave -ENODEV
>> in the regulator pointer.
>>
>> Also, make sure we return 0 in that case, but correctly propagate any
>> other errors. Also propagate the error from _dwc2_hcd_start.
>>
>> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus supply")
>> Cc: Amelie Delaunay <amelie.delaunay@st.com>
>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>>
>> ---
>>
>> v2: Only overwrite the error in the pointer after checking it (Heiko
>>       Stübner <heiko@sntech.de>)
>> v3: Remove hunks that shouldn't be in this patch
>> v4: Don't overwrite the error code before returning it (kbuild test
>>       robot <lkp@intel.com>)
>> ---
>>    drivers/usb/dwc2/hcd.c | 13 ++++++++-----
>>    1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
>> index 190f95964000..c51b73b3e048 100644
>> --- a/drivers/usb/dwc2/hcd.c
>> +++ b/drivers/usb/dwc2/hcd.c
>> @@ -358,9 +358,14 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
>>    
>>    static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
>>    {
>> +	int ret;
>> +
>>    	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
>> -	if (IS_ERR(hsotg->vbus_supply))
>> -		return 0;
>> +	if (IS_ERR(hsotg->vbus_supply)) {
>> +		ret = PTR_ERR(hsotg->vbus_supply);
>> +		hsotg->vbus_supply = NULL;
>> +		return ret == -ENODEV ? 0 : ret;
>> +	}
>>    
>>    	return regulator_enable(hsotg->vbus_supply);
>>    }
>> @@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
>>    
>>    	spin_unlock_irqrestore(&hsotg->lock, flags);
>>    
>> -	dwc2_vbus_supply_init(hsotg);
>> -
>> -	return 0;
>> +	return dwc2_vbus_supply_init(hsotg);
>>    }
>>    
>>    /*
>>
> 
> Reviewed-by: Grigor Tovmasyan <tovmasya@synopsys.com>
> 

Acked-by: Minas Harutyunyan <hminas@synopsys.com>

[v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Minas Harutyunyan]

On 4/5/2018 12:59 PM, Grigor Tovmasyan wrote:
> On 3/26/2018 1:01 PM, Tomeu Vizoso wrote:
>> devm_regulator_get_optional returns -ENODEV if the regulator isn't
>> there, so if that's the case we have to make sure not to leave -ENODEV
>> in the regulator pointer.
>>
>> Also, make sure we return 0 in that case, but correctly propagate any
>> other errors. Also propagate the error from _dwc2_hcd_start.
>>
>> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus supply")
>> Cc: Amelie Delaunay <amelie.delaunay@st.com>
>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>>
>> ---
>>
>> v2: Only overwrite the error in the pointer after checking it (Heiko
>>       Stübner <heiko@sntech.de>)
>> v3: Remove hunks that shouldn't be in this patch
>> v4: Don't overwrite the error code before returning it (kbuild test
>>       robot <lkp@intel.com>)
>> ---
>>    drivers/usb/dwc2/hcd.c | 13 ++++++++-----
>>    1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
>> index 190f95964000..c51b73b3e048 100644
>> --- a/drivers/usb/dwc2/hcd.c
>> +++ b/drivers/usb/dwc2/hcd.c
>> @@ -358,9 +358,14 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
>>    
>>    static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
>>    {
>> +	int ret;
>> +
>>    	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
>> -	if (IS_ERR(hsotg->vbus_supply))
>> -		return 0;
>> +	if (IS_ERR(hsotg->vbus_supply)) {
>> +		ret = PTR_ERR(hsotg->vbus_supply);
>> +		hsotg->vbus_supply = NULL;
>> +		return ret == -ENODEV ? 0 : ret;
>> +	}
>>    
>>    	return regulator_enable(hsotg->vbus_supply);
>>    }
>> @@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
>>    
>>    	spin_unlock_irqrestore(&hsotg->lock, flags);
>>    
>> -	dwc2_vbus_supply_init(hsotg);
>> -
>> -	return 0;
>> +	return dwc2_vbus_supply_init(hsotg);
>>    }
>>    
>>    /*
>>
> 
> Reviewed-by: Grigor Tovmasyan <tovmasya@synopsys.com>
> 

Acked-by: Minas Harutyunyan <hminas@synopsys.com>
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Heiko Stuebner]

Am Montag, 26. März 2018, 11:00:01 CEST schrieb Tomeu Vizoso:
> devm_regulator_get_optional returns -ENODEV if the regulator isn't
> there, so if that's the case we have to make sure not to leave -ENODEV
> in the regulator pointer.
> 
> Also, make sure we return 0 in that case, but correctly propagate any
> other errors. Also propagate the error from _dwc2_hcd_start.
> 
> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus supply")
> Cc: Amelie Delaunay <amelie.delaunay@st.com>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>

The patch that gets fixed here also breaks display-output on dwc2-based
Rockchip devices (likely even more), probably due to making the regulator
framework hickup.

With this patch applied, apart from not seeing the NULL-ptr, I also get
display output on my rk3288-veycron Chromebook again, so

Tested-by: Heiko Stuebner <heiko@sntech.de>


> v2: Only overwrite the error in the pointer after checking it (Heiko
>     Stübner <heiko@sntech.de>)
> v3: Remove hunks that shouldn't be in this patch
> v4: Don't overwrite the error code before returning it (kbuild test
>     robot <lkp@intel.com>)
> ---
>  drivers/usb/dwc2/hcd.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index 190f95964000..c51b73b3e048 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -358,9 +358,14 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
>  
>  static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
>  {
> +	int ret;
> +
>  	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
> -	if (IS_ERR(hsotg->vbus_supply))
> -		return 0;
> +	if (IS_ERR(hsotg->vbus_supply)) {
> +		ret = PTR_ERR(hsotg->vbus_supply);
> +		hsotg->vbus_supply = NULL;
> +		return ret == -ENODEV ? 0 : ret;
> +	}
>  
>  	return regulator_enable(hsotg->vbus_supply);
>  }
> @@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
>  
>  	spin_unlock_irqrestore(&hsotg->lock, flags);
>  
> -	dwc2_vbus_supply_init(hsotg);
> -
> -	return 0;
> +	return dwc2_vbus_supply_init(hsotg);
>  }
>  
>  /*
> 

[v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Heiko Stuebner]

Am Montag, 26. März 2018, 11:00:01 CEST schrieb Tomeu Vizoso:
> devm_regulator_get_optional returns -ENODEV if the regulator isn't
> there, so if that's the case we have to make sure not to leave -ENODEV
> in the regulator pointer.
> 
> Also, make sure we return 0 in that case, but correctly propagate any
> other errors. Also propagate the error from _dwc2_hcd_start.
> 
> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus supply")
> Cc: Amelie Delaunay <amelie.delaunay@st.com>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>

The patch that gets fixed here also breaks display-output on dwc2-based
Rockchip devices (likely even more), probably due to making the regulator
framework hickup.

With this patch applied, apart from not seeing the NULL-ptr, I also get
display output on my rk3288-veycron Chromebook again, so

Tested-by: Heiko Stuebner <heiko@sntech.de>


> v2: Only overwrite the error in the pointer after checking it (Heiko
>     Stübner <heiko@sntech.de>)
> v3: Remove hunks that shouldn't be in this patch
> v4: Don't overwrite the error code before returning it (kbuild test
>     robot <lkp@intel.com>)
> ---
>  drivers/usb/dwc2/hcd.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index 190f95964000..c51b73b3e048 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -358,9 +358,14 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
>  
>  static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
>  {
> +	int ret;
> +
>  	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
> -	if (IS_ERR(hsotg->vbus_supply))
> -		return 0;
> +	if (IS_ERR(hsotg->vbus_supply)) {
> +		ret = PTR_ERR(hsotg->vbus_supply);
> +		hsotg->vbus_supply = NULL;
> +		return ret == -ENODEV ? 0 : ret;
> +	}
>  
>  	return regulator_enable(hsotg->vbus_supply);
>  }
> @@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
>  
>  	spin_unlock_irqrestore(&hsotg->lock, flags);
>  
> -	dwc2_vbus_supply_init(hsotg);
> -
> -	return 0;
> +	return dwc2_vbus_supply_init(hsotg);
>  }
>  
>  /*
>
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Minas Harutyunyan]

Hi Heiko,

On 4/10/2018 4:28 PM, Heiko Stuebner wrote:
> Am Montag, 26. März 2018, 11:00:01 CEST schrieb Tomeu Vizoso:
>> devm_regulator_get_optional returns -ENODEV if the regulator isn't
>> there, so if that's the case we have to make sure not to leave -ENODEV
>> in the regulator pointer.
>>
>> Also, make sure we return 0 in that case, but correctly propagate any
>> other errors. Also propagate the error from _dwc2_hcd_start.
>>
>> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus supply")
>> Cc: Amelie Delaunay <amelie.delaunay@st.com>
>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> 
> The patch that gets fixed here also breaks display-output on dwc2-based
> Rockchip devices (likely even more), probably due to making the regulator
> framework hickup.
> 
Could you please elaborate what mean "breaks display-output".
On which Kernel version you apply this patch?

Thanks,
Minas


> With this patch applied, apart from not seeing the NULL-ptr, I also get
> display output on my rk3288-veycron Chromebook again, so
> 
> Tested-by: Heiko Stuebner <heiko@sntech.de>
> 
> 
>> v2: Only overwrite the error in the pointer after checking it (Heiko
>>      Stübner <heiko@sntech.de>)
>> v3: Remove hunks that shouldn't be in this patch
>> v4: Don't overwrite the error code before returning it (kbuild test
>>      robot <lkp@intel.com>)
>> ---
>>   drivers/usb/dwc2/hcd.c | 13 ++++++++-----
>>   1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
>> index 190f95964000..c51b73b3e048 100644
>> --- a/drivers/usb/dwc2/hcd.c
>> +++ b/drivers/usb/dwc2/hcd.c
>> @@ -358,9 +358,14 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
>>   
>>   static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
>>   {
>> +	int ret;
>> +
>>   	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
>> -	if (IS_ERR(hsotg->vbus_supply))
>> -		return 0;
>> +	if (IS_ERR(hsotg->vbus_supply)) {
>> +		ret = PTR_ERR(hsotg->vbus_supply);
>> +		hsotg->vbus_supply = NULL;
>> +		return ret == -ENODEV ? 0 : ret;
>> +	}
>>   
>>   	return regulator_enable(hsotg->vbus_supply);
>>   }
>> @@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
>>   
>>   	spin_unlock_irqrestore(&hsotg->lock, flags);
>>   
>> -	dwc2_vbus_supply_init(hsotg);
>> -
>> -	return 0;
>> +	return dwc2_vbus_supply_init(hsotg);
>>   }
>>   
>>   /*
>>
> 
> 
> 

[v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Minas Harutyunyan]

Hi Heiko,

On 4/10/2018 4:28 PM, Heiko Stuebner wrote:
> Am Montag, 26. März 2018, 11:00:01 CEST schrieb Tomeu Vizoso:
>> devm_regulator_get_optional returns -ENODEV if the regulator isn't
>> there, so if that's the case we have to make sure not to leave -ENODEV
>> in the regulator pointer.
>>
>> Also, make sure we return 0 in that case, but correctly propagate any
>> other errors. Also propagate the error from _dwc2_hcd_start.
>>
>> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus supply")
>> Cc: Amelie Delaunay <amelie.delaunay@st.com>
>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> 
> The patch that gets fixed here also breaks display-output on dwc2-based
> Rockchip devices (likely even more), probably due to making the regulator
> framework hickup.
> 
Could you please elaborate what mean "breaks display-output".
On which Kernel version you apply this patch?

Thanks,
Minas


> With this patch applied, apart from not seeing the NULL-ptr, I also get
> display output on my rk3288-veycron Chromebook again, so
> 
> Tested-by: Heiko Stuebner <heiko@sntech.de>
> 
> 
>> v2: Only overwrite the error in the pointer after checking it (Heiko
>>      Stübner <heiko@sntech.de>)
>> v3: Remove hunks that shouldn't be in this patch
>> v4: Don't overwrite the error code before returning it (kbuild test
>>      robot <lkp@intel.com>)
>> ---
>>   drivers/usb/dwc2/hcd.c | 13 ++++++++-----
>>   1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
>> index 190f95964000..c51b73b3e048 100644
>> --- a/drivers/usb/dwc2/hcd.c
>> +++ b/drivers/usb/dwc2/hcd.c
>> @@ -358,9 +358,14 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg *hsotg)
>>   
>>   static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
>>   {
>> +	int ret;
>> +
>>   	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
>> -	if (IS_ERR(hsotg->vbus_supply))
>> -		return 0;
>> +	if (IS_ERR(hsotg->vbus_supply)) {
>> +		ret = PTR_ERR(hsotg->vbus_supply);
>> +		hsotg->vbus_supply = NULL;
>> +		return ret == -ENODEV ? 0 : ret;
>> +	}
>>   
>>   	return regulator_enable(hsotg->vbus_supply);
>>   }
>> @@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
>>   
>>   	spin_unlock_irqrestore(&hsotg->lock, flags);
>>   
>> -	dwc2_vbus_supply_init(hsotg);
>> -
>> -	return 0;
>> +	return dwc2_vbus_supply_init(hsotg);
>>   }
>>   
>>   /*
>>
> 
> 
>
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Heiko Stübner]

Am Dienstag, 10. April 2018, 15:52:25 CEST schrieb Minas Harutyunyan:
> Hi Heiko,
> 
> On 4/10/2018 4:28 PM, Heiko Stuebner wrote:
> > Am Montag, 26. März 2018, 11:00:01 CEST schrieb Tomeu Vizoso:
> >> devm_regulator_get_optional returns -ENODEV if the regulator isn't
> >> there, so if that's the case we have to make sure not to leave -ENODEV
> >> in the regulator pointer.
> >> 
> >> Also, make sure we return 0 in that case, but correctly propagate any
> >> other errors. Also propagate the error from _dwc2_hcd_start.
> >> 
> >> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus
> >> supply") Cc: Amelie Delaunay <amelie.delaunay@st.com>
> >> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> > 
> > The patch that gets fixed here also breaks display-output on dwc2-based
> > Rockchip devices (likely even more), probably due to making the regulator
> > framework hickup.
> 
> Could you please elaborate what mean "breaks display-output".
> On which Kernel version you apply this patch?

I think I may have written that poorly. _Without_ this patch I get
display breakage on the most recent torvalds/master (merge-window)
where "usb: dwc2: add support for host mode external vbus supply" is
applied and this patch fixes the issue.

"breaks display output" means both hdmi + edp output are missing
also including the backlight staying off.

The patch we're fixing here, causes a null-pointer dereference in the
regulator framework, which seems to also cause issues when other
regulators are enabled, which I think is what I'm seeing here.


Heiko

> 
> Thanks,
> Minas
> 
> > With this patch applied, apart from not seeing the NULL-ptr, I also get
> > display output on my rk3288-veycron Chromebook again, so
> > 
> > Tested-by: Heiko Stuebner <heiko@sntech.de>
> > 
> >> v2: Only overwrite the error in the pointer after checking it (Heiko
> >> 
> >>      Stübner <heiko@sntech.de>)
> >> 
> >> v3: Remove hunks that shouldn't be in this patch
> >> v4: Don't overwrite the error code before returning it (kbuild test
> >> 
> >>      robot <lkp@intel.com>)
> >> 
> >> ---
> >> 
> >>   drivers/usb/dwc2/hcd.c | 13 ++++++++-----
> >>   1 file changed, 8 insertions(+), 5 deletions(-)
> >> 
> >> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> >> index 190f95964000..c51b73b3e048 100644
> >> --- a/drivers/usb/dwc2/hcd.c
> >> +++ b/drivers/usb/dwc2/hcd.c
> >> @@ -358,9 +358,14 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg
> >> *hsotg)>> 
> >>   static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
> >>   {
> >> 
> >> +	int ret;
> >> +
> >> 
> >>   	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
> >> 
> >> -	if (IS_ERR(hsotg->vbus_supply))
> >> -		return 0;
> >> +	if (IS_ERR(hsotg->vbus_supply)) {
> >> +		ret = PTR_ERR(hsotg->vbus_supply);
> >> +		hsotg->vbus_supply = NULL;
> >> +		return ret == -ENODEV ? 0 : ret;
> >> +	}
> >> 
> >>   	return regulator_enable(hsotg->vbus_supply);
> >>   
> >>   }
> >> 
> >> @@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
> >> 
> >>   	spin_unlock_irqrestore(&hsotg->lock, flags);
> >> 
> >> -	dwc2_vbus_supply_init(hsotg);
> >> -
> >> -	return 0;
> >> +	return dwc2_vbus_supply_init(hsotg);
> >> 
> >>   }
> >>   
> >>   /*

[v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Heiko Stuebner]

Am Dienstag, 10. April 2018, 15:52:25 CEST schrieb Minas Harutyunyan:
> Hi Heiko,
> 
> On 4/10/2018 4:28 PM, Heiko Stuebner wrote:
> > Am Montag, 26. März 2018, 11:00:01 CEST schrieb Tomeu Vizoso:
> >> devm_regulator_get_optional returns -ENODEV if the regulator isn't
> >> there, so if that's the case we have to make sure not to leave -ENODEV
> >> in the regulator pointer.
> >> 
> >> Also, make sure we return 0 in that case, but correctly propagate any
> >> other errors. Also propagate the error from _dwc2_hcd_start.
> >> 
> >> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus
> >> supply") Cc: Amelie Delaunay <amelie.delaunay@st.com>
> >> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> > 
> > The patch that gets fixed here also breaks display-output on dwc2-based
> > Rockchip devices (likely even more), probably due to making the regulator
> > framework hickup.
> 
> Could you please elaborate what mean "breaks display-output".
> On which Kernel version you apply this patch?

I think I may have written that poorly. _Without_ this patch I get
display breakage on the most recent torvalds/master (merge-window)
where "usb: dwc2: add support for host mode external vbus supply" is
applied and this patch fixes the issue.

"breaks display output" means both hdmi + edp output are missing
also including the backlight staying off.

The patch we're fixing here, causes a null-pointer dereference in the
regulator framework, which seems to also cause issues when other
regulators are enabled, which I think is what I'm seeing here.


Heiko

> 
> Thanks,
> Minas
> 
> > With this patch applied, apart from not seeing the NULL-ptr, I also get
> > display output on my rk3288-veycron Chromebook again, so
> > 
> > Tested-by: Heiko Stuebner <heiko@sntech.de>
> > 
> >> v2: Only overwrite the error in the pointer after checking it (Heiko
> >> 
> >>      Stübner <heiko@sntech.de>)
> >> 
> >> v3: Remove hunks that shouldn't be in this patch
> >> v4: Don't overwrite the error code before returning it (kbuild test
> >> 
> >>      robot <lkp@intel.com>)
> >> 
> >> ---
> >> 
> >>   drivers/usb/dwc2/hcd.c | 13 ++++++++-----
> >>   1 file changed, 8 insertions(+), 5 deletions(-)
> >> 
> >> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> >> index 190f95964000..c51b73b3e048 100644
> >> --- a/drivers/usb/dwc2/hcd.c
> >> +++ b/drivers/usb/dwc2/hcd.c
> >> @@ -358,9 +358,14 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg
> >> *hsotg)>> 
> >>   static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
> >>   {
> >> 
> >> +	int ret;
> >> +
> >> 
> >>   	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
> >> 
> >> -	if (IS_ERR(hsotg->vbus_supply))
> >> -		return 0;
> >> +	if (IS_ERR(hsotg->vbus_supply)) {
> >> +		ret = PTR_ERR(hsotg->vbus_supply);
> >> +		hsotg->vbus_supply = NULL;
> >> +		return ret == -ENODEV ? 0 : ret;
> >> +	}
> >> 
> >>   	return regulator_enable(hsotg->vbus_supply);
> >>   
> >>   }
> >> 
> >> @@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
> >> 
> >>   	spin_unlock_irqrestore(&hsotg->lock, flags);
> >> 
> >> -	dwc2_vbus_supply_init(hsotg);
> >> -
> >> -	return 0;
> >> +	return dwc2_vbus_supply_init(hsotg);
> >> 
> >>   }
> >>   
> >>   /*
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Minas Harutyunyan]

Hi Heiko,

On 4/10/2018 7:37 PM, Heiko Stübner wrote:
> Am Dienstag, 10. April 2018, 15:52:25 CEST schrieb Minas Harutyunyan:
>> Hi Heiko,
>>
>> On 4/10/2018 4:28 PM, Heiko Stuebner wrote:
>>> Am Montag, 26. März 2018, 11:00:01 CEST schrieb Tomeu Vizoso:
>>>> devm_regulator_get_optional returns -ENODEV if the regulator isn't
>>>> there, so if that's the case we have to make sure not to leave -ENODEV
>>>> in the regulator pointer.
>>>>
>>>> Also, make sure we return 0 in that case, but correctly propagate any
>>>> other errors. Also propagate the error from _dwc2_hcd_start.
>>>>
>>>> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus
>>>> supply") Cc: Amelie Delaunay <amelie.delaunay@st.com>
>>>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>>>
>>> The patch that gets fixed here also breaks display-output on dwc2-based
>>> Rockchip devices (likely even more), probably due to making the regulator
>>> framework hickup.
>>
>> Could you please elaborate what mean "breaks display-output".
>> On which Kernel version you apply this patch?
> 
> I think I may have written that poorly. _Without_ this patch I get
> display breakage on the most recent torvalds/master (merge-window)
> where "usb: dwc2: add support for host mode external vbus supply" is
> applied and this patch fixes the issue.
> 
> "breaks display output" means both hdmi + edp output are missing
> also including the backlight staying off.
> 
> The patch we're fixing here, causes a null-pointer dereference in the
> regulator framework, which seems to also cause issues when other
> regulators are enabled, which I think is what I'm seeing here.
> 
> 
Thank you for clarification. Earlier you added "reviewed" tag, this 
feedback can assumed as "tested" tag.

Thanks,
Minas

> Heiko
> 
>>
>> Thanks,
>> Minas
>>
>>> With this patch applied, apart from not seeing the NULL-ptr, I also get
>>> display output on my rk3288-veycron Chromebook again, so
>>>
>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>>>
>>>> v2: Only overwrite the error in the pointer after checking it (Heiko
>>>>
>>>>       Stübner <heiko@sntech.de>)
>>>>
>>>> v3: Remove hunks that shouldn't be in this patch
>>>> v4: Don't overwrite the error code before returning it (kbuild test
>>>>
>>>>       robot <lkp@intel.com>)
>>>>
>>>> ---
>>>>
>>>>    drivers/usb/dwc2/hcd.c | 13 ++++++++-----
>>>>    1 file changed, 8 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
>>>> index 190f95964000..c51b73b3e048 100644
>>>> --- a/drivers/usb/dwc2/hcd.c
>>>> +++ b/drivers/usb/dwc2/hcd.c
>>>> @@ -358,9 +358,14 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg
>>>> *hsotg)>>
>>>>    static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
>>>>    {
>>>>
>>>> +	int ret;
>>>> +
>>>>
>>>>    	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
>>>>
>>>> -	if (IS_ERR(hsotg->vbus_supply))
>>>> -		return 0;
>>>> +	if (IS_ERR(hsotg->vbus_supply)) {
>>>> +		ret = PTR_ERR(hsotg->vbus_supply);
>>>> +		hsotg->vbus_supply = NULL;
>>>> +		return ret == -ENODEV ? 0 : ret;
>>>> +	}
>>>>
>>>>    	return regulator_enable(hsotg->vbus_supply);
>>>>    
>>>>    }
>>>>
>>>> @@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
>>>>
>>>>    	spin_unlock_irqrestore(&hsotg->lock, flags);
>>>>
>>>> -	dwc2_vbus_supply_init(hsotg);
>>>> -
>>>> -	return 0;
>>>> +	return dwc2_vbus_supply_init(hsotg);
>>>>
>>>>    }
>>>>    
>>>>    /*
> 
> 
> 

[v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Minas Harutyunyan]

Hi Heiko,

On 4/10/2018 7:37 PM, Heiko Stübner wrote:
> Am Dienstag, 10. April 2018, 15:52:25 CEST schrieb Minas Harutyunyan:
>> Hi Heiko,
>>
>> On 4/10/2018 4:28 PM, Heiko Stuebner wrote:
>>> Am Montag, 26. März 2018, 11:00:01 CEST schrieb Tomeu Vizoso:
>>>> devm_regulator_get_optional returns -ENODEV if the regulator isn't
>>>> there, so if that's the case we have to make sure not to leave -ENODEV
>>>> in the regulator pointer.
>>>>
>>>> Also, make sure we return 0 in that case, but correctly propagate any
>>>> other errors. Also propagate the error from _dwc2_hcd_start.
>>>>
>>>> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus
>>>> supply") Cc: Amelie Delaunay <amelie.delaunay@st.com>
>>>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>>>
>>> The patch that gets fixed here also breaks display-output on dwc2-based
>>> Rockchip devices (likely even more), probably due to making the regulator
>>> framework hickup.
>>
>> Could you please elaborate what mean "breaks display-output".
>> On which Kernel version you apply this patch?
> 
> I think I may have written that poorly. _Without_ this patch I get
> display breakage on the most recent torvalds/master (merge-window)
> where "usb: dwc2: add support for host mode external vbus supply" is
> applied and this patch fixes the issue.
> 
> "breaks display output" means both hdmi + edp output are missing
> also including the backlight staying off.
> 
> The patch we're fixing here, causes a null-pointer dereference in the
> regulator framework, which seems to also cause issues when other
> regulators are enabled, which I think is what I'm seeing here.
> 
> 
Thank you for clarification. Earlier you added "reviewed" tag, this 
feedback can assumed as "tested" tag.

Thanks,
Minas

> Heiko
> 
>>
>> Thanks,
>> Minas
>>
>>> With this patch applied, apart from not seeing the NULL-ptr, I also get
>>> display output on my rk3288-veycron Chromebook again, so
>>>
>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>>>
>>>> v2: Only overwrite the error in the pointer after checking it (Heiko
>>>>
>>>>       Stübner <heiko@sntech.de>)
>>>>
>>>> v3: Remove hunks that shouldn't be in this patch
>>>> v4: Don't overwrite the error code before returning it (kbuild test
>>>>
>>>>       robot <lkp@intel.com>)
>>>>
>>>> ---
>>>>
>>>>    drivers/usb/dwc2/hcd.c | 13 ++++++++-----
>>>>    1 file changed, 8 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
>>>> index 190f95964000..c51b73b3e048 100644
>>>> --- a/drivers/usb/dwc2/hcd.c
>>>> +++ b/drivers/usb/dwc2/hcd.c
>>>> @@ -358,9 +358,14 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg
>>>> *hsotg)>>
>>>>    static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
>>>>    {
>>>>
>>>> +	int ret;
>>>> +
>>>>
>>>>    	hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
>>>>
>>>> -	if (IS_ERR(hsotg->vbus_supply))
>>>> -		return 0;
>>>> +	if (IS_ERR(hsotg->vbus_supply)) {
>>>> +		ret = PTR_ERR(hsotg->vbus_supply);
>>>> +		hsotg->vbus_supply = NULL;
>>>> +		return ret == -ENODEV ? 0 : ret;
>>>> +	}
>>>>
>>>>    	return regulator_enable(hsotg->vbus_supply);
>>>>    
>>>>    }
>>>>
>>>> @@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
>>>>
>>>>    	spin_unlock_irqrestore(&hsotg->lock, flags);
>>>>
>>>> -	dwc2_vbus_supply_init(hsotg);
>>>> -
>>>> -	return 0;
>>>> +	return dwc2_vbus_supply_init(hsotg);
>>>>
>>>>    }
>>>>    
>>>>    /*
> 
> 
>
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Tomeu Vizoso]

Hi,

could this patch be picked up, please? Or if for some reason it cannot
be, could the commit that introduced the regression be reverted?

It's causing some tests in KernelCI to fail:

https://storage.kernelci.org/next/master/next-20180423/arm/multi_v7_defconfig/lab-collabora/sleep-rk3288-veyron-jaq.html

Thanks,

Tomeu


On 11 April 2018 at 08:50, Minas Harutyunyan
<Minas.Harutyunyan@synopsys.com> wrote:
> Hi Heiko,
>
> On 4/10/2018 7:37 PM, Heiko Stübner wrote:
>> Am Dienstag, 10. April 2018, 15:52:25 CEST schrieb Minas Harutyunyan:
>>> Hi Heiko,
>>>
>>> On 4/10/2018 4:28 PM, Heiko Stuebner wrote:
>>>> Am Montag, 26. März 2018, 11:00:01 CEST schrieb Tomeu Vizoso:
>>>>> devm_regulator_get_optional returns -ENODEV if the regulator isn't
>>>>> there, so if that's the case we have to make sure not to leave -ENODEV
>>>>> in the regulator pointer.
>>>>>
>>>>> Also, make sure we return 0 in that case, but correctly propagate any
>>>>> other errors. Also propagate the error from _dwc2_hcd_start.
>>>>>
>>>>> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus
>>>>> supply") Cc: Amelie Delaunay <amelie.delaunay@st.com>
>>>>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>>>>
>>>> The patch that gets fixed here also breaks display-output on dwc2-based
>>>> Rockchip devices (likely even more), probably due to making the regulator
>>>> framework hickup.
>>>
>>> Could you please elaborate what mean "breaks display-output".
>>> On which Kernel version you apply this patch?
>>
>> I think I may have written that poorly. _Without_ this patch I get
>> display breakage on the most recent torvalds/master (merge-window)
>> where "usb: dwc2: add support for host mode external vbus supply" is
>> applied and this patch fixes the issue.
>>
>> "breaks display output" means both hdmi + edp output are missing
>> also including the backlight staying off.
>>
>> The patch we're fixing here, causes a null-pointer dereference in the
>> regulator framework, which seems to also cause issues when other
>> regulators are enabled, which I think is what I'm seeing here.
>>
>>
> Thank you for clarification. Earlier you added "reviewed" tag, this
> feedback can assumed as "tested" tag.
>
> Thanks,
> Minas
>
>> Heiko
>>
>>>
>>> Thanks,
>>> Minas
>>>
>>>> With this patch applied, apart from not seeing the NULL-ptr, I also get
>>>> display output on my rk3288-veycron Chromebook again, so
>>>>
>>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>>>>
>>>>> v2: Only overwrite the error in the pointer after checking it (Heiko
>>>>>
>>>>>       Stübner <heiko@sntech.de>)
>>>>>
>>>>> v3: Remove hunks that shouldn't be in this patch
>>>>> v4: Don't overwrite the error code before returning it (kbuild test
>>>>>
>>>>>       robot <lkp@intel.com>)
>>>>>
>>>>> ---
>>>>>
>>>>>    drivers/usb/dwc2/hcd.c | 13 ++++++++-----
>>>>>    1 file changed, 8 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
>>>>> index 190f95964000..c51b73b3e048 100644
>>>>> --- a/drivers/usb/dwc2/hcd.c
>>>>> +++ b/drivers/usb/dwc2/hcd.c
>>>>> @@ -358,9 +358,14 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg
>>>>> *hsotg)>>
>>>>>    static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
>>>>>    {
>>>>>
>>>>> +  int ret;
>>>>> +
>>>>>
>>>>>            hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
>>>>>
>>>>> -  if (IS_ERR(hsotg->vbus_supply))
>>>>> -          return 0;
>>>>> +  if (IS_ERR(hsotg->vbus_supply)) {
>>>>> +          ret = PTR_ERR(hsotg->vbus_supply);
>>>>> +          hsotg->vbus_supply = NULL;
>>>>> +          return ret == -ENODEV ? 0 : ret;
>>>>> +  }
>>>>>
>>>>>            return regulator_enable(hsotg->vbus_supply);
>>>>>
>>>>>    }
>>>>>
>>>>> @@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
>>>>>
>>>>>            spin_unlock_irqrestore(&hsotg->lock, flags);
>>>>>
>>>>> -  dwc2_vbus_supply_init(hsotg);
>>>>> -
>>>>> -  return 0;
>>>>> +  return dwc2_vbus_supply_init(hsotg);
>>>>>
>>>>>    }
>>>>>
>>>>>    /*
>>
>>
>>
>

[v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Tomeu Vizoso]

Hi,

could this patch be picked up, please? Or if for some reason it cannot
be, could the commit that introduced the regression be reverted?

It's causing some tests in KernelCI to fail:

https://storage.kernelci.org/next/master/next-20180423/arm/multi_v7_defconfig/lab-collabora/sleep-rk3288-veyron-jaq.html

Thanks,

Tomeu


On 11 April 2018 at 08:50, Minas Harutyunyan
<Minas.Harutyunyan@synopsys.com> wrote:
> Hi Heiko,
>
> On 4/10/2018 7:37 PM, Heiko Stübner wrote:
>> Am Dienstag, 10. April 2018, 15:52:25 CEST schrieb Minas Harutyunyan:
>>> Hi Heiko,
>>>
>>> On 4/10/2018 4:28 PM, Heiko Stuebner wrote:
>>>> Am Montag, 26. März 2018, 11:00:01 CEST schrieb Tomeu Vizoso:
>>>>> devm_regulator_get_optional returns -ENODEV if the regulator isn't
>>>>> there, so if that's the case we have to make sure not to leave -ENODEV
>>>>> in the regulator pointer.
>>>>>
>>>>> Also, make sure we return 0 in that case, but correctly propagate any
>>>>> other errors. Also propagate the error from _dwc2_hcd_start.
>>>>>
>>>>> Fixes: 531ef5ebea96 ("usb: dwc2: add support for host mode external vbus
>>>>> supply") Cc: Amelie Delaunay <amelie.delaunay@st.com>
>>>>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>>>>
>>>> The patch that gets fixed here also breaks display-output on dwc2-based
>>>> Rockchip devices (likely even more), probably due to making the regulator
>>>> framework hickup.
>>>
>>> Could you please elaborate what mean "breaks display-output".
>>> On which Kernel version you apply this patch?
>>
>> I think I may have written that poorly. _Without_ this patch I get
>> display breakage on the most recent torvalds/master (merge-window)
>> where "usb: dwc2: add support for host mode external vbus supply" is
>> applied and this patch fixes the issue.
>>
>> "breaks display output" means both hdmi + edp output are missing
>> also including the backlight staying off.
>>
>> The patch we're fixing here, causes a null-pointer dereference in the
>> regulator framework, which seems to also cause issues when other
>> regulators are enabled, which I think is what I'm seeing here.
>>
>>
> Thank you for clarification. Earlier you added "reviewed" tag, this
> feedback can assumed as "tested" tag.
>
> Thanks,
> Minas
>
>> Heiko
>>
>>>
>>> Thanks,
>>> Minas
>>>
>>>> With this patch applied, apart from not seeing the NULL-ptr, I also get
>>>> display output on my rk3288-veycron Chromebook again, so
>>>>
>>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>>>>
>>>>> v2: Only overwrite the error in the pointer after checking it (Heiko
>>>>>
>>>>>       Stübner <heiko@sntech.de>)
>>>>>
>>>>> v3: Remove hunks that shouldn't be in this patch
>>>>> v4: Don't overwrite the error code before returning it (kbuild test
>>>>>
>>>>>       robot <lkp@intel.com>)
>>>>>
>>>>> ---
>>>>>
>>>>>    drivers/usb/dwc2/hcd.c | 13 ++++++++-----
>>>>>    1 file changed, 8 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
>>>>> index 190f95964000..c51b73b3e048 100644
>>>>> --- a/drivers/usb/dwc2/hcd.c
>>>>> +++ b/drivers/usb/dwc2/hcd.c
>>>>> @@ -358,9 +358,14 @@ static void dwc2_gusbcfg_init(struct dwc2_hsotg
>>>>> *hsotg)>>
>>>>>    static int dwc2_vbus_supply_init(struct dwc2_hsotg *hsotg)
>>>>>    {
>>>>>
>>>>> +  int ret;
>>>>> +
>>>>>
>>>>>            hsotg->vbus_supply = devm_regulator_get_optional(hsotg->dev, "vbus");
>>>>>
>>>>> -  if (IS_ERR(hsotg->vbus_supply))
>>>>> -          return 0;
>>>>> +  if (IS_ERR(hsotg->vbus_supply)) {
>>>>> +          ret = PTR_ERR(hsotg->vbus_supply);
>>>>> +          hsotg->vbus_supply = NULL;
>>>>> +          return ret == -ENODEV ? 0 : ret;
>>>>> +  }
>>>>>
>>>>>            return regulator_enable(hsotg->vbus_supply);
>>>>>
>>>>>    }
>>>>>
>>>>> @@ -4342,9 +4347,7 @@ static int _dwc2_hcd_start(struct usb_hcd *hcd)
>>>>>
>>>>>            spin_unlock_irqrestore(&hsotg->lock, flags);
>>>>>
>>>>> -  dwc2_vbus_supply_init(hsotg);
>>>>> -
>>>>> -  return 0;
>>>>> +  return dwc2_vbus_supply_init(hsotg);
>>>>>
>>>>>    }
>>>>>
>>>>>    /*
>>
>>
>>
>
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Heiko Stübner]

Hi Tomeu,

Am Montag, 23. April 2018, 15:24:04 CEST schrieb Tomeu Vizoso:
> Hi,
> 
> could this patch be picked up, please? Or if for some reason it cannot
> be, could the commit that introduced the regression be reverted?
> 
> It's causing some tests in KernelCI to fail:
> 
> https://storage.kernelci.org/next/master/next-20180423/arm/multi_v7_defconfi
> g/lab-collabora/sleep-rk3288-veyron-jaq.html

I think at this point, it might be good to do a "v4 RESEND" in a _new_ mail 
thread, because from the lack of communication it really looks like this
fell through completely.

Ideally also include in the commit message that his breaks kernelCI tests
and real board users (and of course recent tags).


Heiko

[v4] usb: dwc2: dwc2_vbus_supply_init: fix error check

[Heiko Stuebner]

Hi Tomeu,

Am Montag, 23. April 2018, 15:24:04 CEST schrieb Tomeu Vizoso:
> Hi,
> 
> could this patch be picked up, please? Or if for some reason it cannot
> be, could the commit that introduced the regression be reverted?
> 
> It's causing some tests in KernelCI to fail:
> 
> https://storage.kernelci.org/next/master/next-20180423/arm/multi_v7_defconfi
> g/lab-collabora/sleep-rk3288-veyron-jaq.html

I think at this point, it might be good to do a "v4 RESEND" in a _new_ mail 
thread, because from the lack of communication it really looks like this
fell through completely.

Ideally also include in the commit message that his breaks kernelCI tests
and real board users (and of course recent tags).


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