* [v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
@ 2018-03-26 9:00 ` Tomeu Vizoso
0 siblings, 0 replies; 49+ messages in thread
From: Tomeu Vizoso @ 2018-03-26 9:00 UTC (permalink / raw)
To: linux-kernel
Cc: Felipe Balbi, Heiko Stuebner, Tomeu Vizoso, Amelie Delaunay,
Minas Harutyunyan, Greg Kroah-Hartman, linux-usb
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);
}
/*
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
@ 2018-03-26 10:56 ` Amelie Delaunay
0 siblings, 0 replies; 49+ messages in thread
From: Amelie DELAUNAY @ 2018-03-26 10:56 UTC (permalink / raw)
To: Tomeu Vizoso, linux-kernel
Cc: Felipe Balbi, Heiko Stuebner, Minas Harutyunyan,
Greg Kroah-Hartman, linux-usb
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
^ permalink raw reply [flat|nested] 49+ messages in thread
* [v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
@ 2018-03-26 10:56 ` Amelie Delaunay
0 siblings, 0 replies; 49+ messages in thread
From: Amelie Delaunay @ 2018-03-26 10:56 UTC (permalink / raw)
To: Tomeu Vizoso, linux-kernel
Cc: Felipe Balbi, Heiko Stuebner, Minas Harutyunyan,
Greg Kroah-Hartman, linux-usb
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
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
@ 2018-03-26 11:51 ` Heiko Stuebner
0 siblings, 0 replies; 49+ messages in thread
From: Heiko Stübner @ 2018-03-26 11:51 UTC (permalink / raw)
To: Tomeu Vizoso
Cc: linux-kernel, Felipe Balbi, Amelie Delaunay, Minas Harutyunyan,
Greg Kroah-Hartman, linux-usb
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>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
@ 2018-03-26 11:51 ` Heiko Stuebner
0 siblings, 0 replies; 49+ messages in thread
From: Heiko Stuebner @ 2018-03-26 11:51 UTC (permalink / raw)
To: Tomeu Vizoso
Cc: linux-kernel, Felipe Balbi, Amelie Delaunay, Minas Harutyunyan,
Greg Kroah-Hartman, linux-usb
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
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
@ 2018-04-04 13:33 ` Tomeu Vizoso
0 siblings, 0 replies; 49+ messages in thread
From: Tomeu Vizoso @ 2018-04-04 13:33 UTC (permalink / raw)
To: Heiko Stübner
Cc: open list, Felipe Balbi, Amelie Delaunay, Minas Harutyunyan,
Greg Kroah-Hartman, linux-usb
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>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
@ 2018-04-04 13:33 ` Tomeu Vizoso
0 siblings, 0 replies; 49+ messages in thread
From: Tomeu Vizoso @ 2018-04-04 13:33 UTC (permalink / raw)
To: Heiko Stübner
Cc: open list, Felipe Balbi, Amelie Delaunay, Minas Harutyunyan,
Greg Kroah-Hartman, linux-usb
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
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
@ 2018-04-05 7:54 ` Minas Harutyunyan
0 siblings, 0 replies; 49+ messages in thread
From: Minas Harutyunyan @ 2018-04-05 7:54 UTC (permalink / raw)
To: Tomeu Vizoso, linux-kernel
Cc: Felipe Balbi, Heiko Stuebner, Amelie Delaunay, Minas Harutyunyan,
Greg Kroah-Hartman, linux-usb
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
^ permalink raw reply [flat|nested] 49+ messages in thread
* [v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
@ 2018-04-05 7:54 ` Minas Harutyunyan
0 siblings, 0 replies; 49+ messages in thread
From: Minas Harutyunyan @ 2018-04-05 7:54 UTC (permalink / raw)
To: Tomeu Vizoso, linux-kernel
Cc: Felipe Balbi, Heiko Stuebner, Amelie Delaunay, Minas Harutyunyan,
Greg Kroah-Hartman, linux-usb
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
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
@ 2018-04-05 8:10 ` Tomeu Vizoso
0 siblings, 0 replies; 49+ messages in thread
From: Tomeu Vizoso @ 2018-04-05 8:10 UTC (permalink / raw)
To: Minas Harutyunyan, linux-kernel
Cc: Felipe Balbi, Heiko Stuebner, Amelie Delaunay,
Greg Kroah-Hartman, linux-usb
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
^ permalink raw reply [flat|nested] 49+ messages in thread
* [v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
@ 2018-04-05 8:10 ` Tomeu Vizoso
0 siblings, 0 replies; 49+ messages in thread
From: Tomeu Vizoso @ 2018-04-05 8:10 UTC (permalink / raw)
To: Minas Harutyunyan, linux-kernel
Cc: Felipe Balbi, Heiko Stuebner, Amelie Delaunay,
Greg Kroah-Hartman, linux-usb
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
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
@ 2018-04-05 9:02 ` Grigor Tovmasyan
0 siblings, 0 replies; 49+ messages in thread
From: Grigor Tovmasyan @ 2018-04-05 9:02 UTC (permalink / raw)
To: Tomeu Vizoso, Minas Harutyunyan, linux-kernel
Cc: Felipe Balbi, Heiko Stuebner, Amelie Delaunay,
Greg Kroah-Hartman, linux-usb
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=
>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
@ 2018-04-05 9:02 ` Grigor Tovmasyan
0 siblings, 0 replies; 49+ messages in thread
From: Grigor Tovmasyan @ 2018-04-05 9:02 UTC (permalink / raw)
To: Tomeu Vizoso, Minas Harutyunyan, linux-kernel
Cc: Felipe Balbi, Heiko Stuebner, Amelie Delaunay,
Greg Kroah-Hartman, linux-usb
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
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
@ 2018-04-05 8:58 ` Grigor Tovmasyan
0 siblings, 0 replies; 49+ messages in thread
From: Grigor Tovmasyan @ 2018-04-05 8:58 UTC (permalink / raw)
To: Tomeu Vizoso, linux-kernel
Cc: Felipe Balbi, Heiko Stuebner, Amelie Delaunay, Minas Harutyunyan,
Greg Kroah-Hartman, linux-usb
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>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
@ 2018-04-05 8:58 ` Grigor Tovmasyan
0 siblings, 0 replies; 49+ messages in thread
From: Grigor Tovmasyan @ 2018-04-05 8:58 UTC (permalink / raw)
To: Tomeu Vizoso, linux-kernel
Cc: Felipe Balbi, Heiko Stuebner, Amelie Delaunay, Minas Harutyunyan,
Greg Kroah-Hartman, linux-usb
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
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
@ 2018-04-05 9:51 ` Minas Harutyunyan
0 siblings, 0 replies; 49+ messages in thread
From: Minas Harutyunyan @ 2018-04-05 9:51 UTC (permalink / raw)
To: Grigor Tovmasyan, Tomeu Vizoso, linux-kernel
Cc: Felipe Balbi, Heiko Stuebner, Amelie Delaunay, Minas Harutyunyan,
Greg Kroah-Hartman, linux-usb
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>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
@ 2018-04-05 9:51 ` Minas Harutyunyan
0 siblings, 0 replies; 49+ messages in thread
From: Minas Harutyunyan @ 2018-04-05 9:51 UTC (permalink / raw)
To: Grigor Tovmasyan, Tomeu Vizoso, linux-kernel
Cc: Felipe Balbi, Heiko Stuebner, Amelie Delaunay, Minas Harutyunyan,
Greg Kroah-Hartman, linux-usb
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
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
@ 2018-04-10 12:28 ` Heiko Stuebner
0 siblings, 0 replies; 49+ messages in thread
From: Heiko Stuebner @ 2018-04-10 12:28 UTC (permalink / raw)
To: Tomeu Vizoso
Cc: linux-kernel, Felipe Balbi, Amelie Delaunay, Minas Harutyunyan,
Greg Kroah-Hartman, linux-usb
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);
> }
>
> /*
>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
@ 2018-04-10 12:28 ` Heiko Stuebner
0 siblings, 0 replies; 49+ messages in thread
From: Heiko Stuebner @ 2018-04-10 12:28 UTC (permalink / raw)
To: Tomeu Vizoso
Cc: linux-kernel, Felipe Balbi, Amelie Delaunay, Minas Harutyunyan,
Greg Kroah-Hartman, linux-usb
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
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
@ 2018-04-10 13:52 ` Minas Harutyunyan
0 siblings, 0 replies; 49+ messages in thread
From: Minas Harutyunyan @ 2018-04-10 13:52 UTC (permalink / raw)
To: Heiko Stuebner, Tomeu Vizoso
Cc: linux-kernel, Felipe Balbi, Amelie Delaunay, Minas Harutyunyan,
Greg Kroah-Hartman, linux-usb
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);
>> }
>>
>> /*
>>
>
>
>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
@ 2018-04-10 13:52 ` Minas Harutyunyan
0 siblings, 0 replies; 49+ messages in thread
From: Minas Harutyunyan @ 2018-04-10 13:52 UTC (permalink / raw)
To: Heiko Stuebner, Tomeu Vizoso
Cc: linux-kernel, Felipe Balbi, Amelie Delaunay, Minas Harutyunyan,
Greg Kroah-Hartman, linux-usb
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
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
@ 2018-04-10 15:37 ` Heiko Stuebner
0 siblings, 0 replies; 49+ messages in thread
From: Heiko Stübner @ 2018-04-10 15:37 UTC (permalink / raw)
To: Minas Harutyunyan
Cc: Tomeu Vizoso, linux-kernel, Felipe Balbi, Amelie Delaunay,
Greg Kroah-Hartman, linux-usb
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);
> >>
> >> }
> >>
> >> /*
^ permalink raw reply [flat|nested] 49+ messages in thread
* [v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
@ 2018-04-10 15:37 ` Heiko Stuebner
0 siblings, 0 replies; 49+ messages in thread
From: Heiko Stuebner @ 2018-04-10 15:37 UTC (permalink / raw)
To: Minas Harutyunyan
Cc: Tomeu Vizoso, linux-kernel, Felipe Balbi, Amelie Delaunay,
Greg Kroah-Hartman, linux-usb
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
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
@ 2018-04-11 6:50 ` Minas Harutyunyan
0 siblings, 0 replies; 49+ messages in thread
From: Minas Harutyunyan @ 2018-04-11 6:50 UTC (permalink / raw)
To: Heiko Stübner, Minas Harutyunyan
Cc: Tomeu Vizoso, linux-kernel, Felipe Balbi, Amelie Delaunay,
Greg Kroah-Hartman, linux-usb
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);
>>>>
>>>> }
>>>>
>>>> /*
>
>
>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
@ 2018-04-11 6:50 ` Minas Harutyunyan
0 siblings, 0 replies; 49+ messages in thread
From: Minas Harutyunyan @ 2018-04-11 6:50 UTC (permalink / raw)
To: Heiko Stübner, Minas Harutyunyan
Cc: Tomeu Vizoso, linux-kernel, Felipe Balbi, Amelie Delaunay,
Greg Kroah-Hartman, linux-usb
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
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
@ 2018-04-23 13:24 ` Tomeu Vizoso
0 siblings, 0 replies; 49+ messages in thread
From: Tomeu Vizoso @ 2018-04-23 13:24 UTC (permalink / raw)
To: Minas Harutyunyan
Cc: Heiko Stübner, linux-kernel, Felipe Balbi, Amelie Delaunay,
Greg Kroah-Hartman, linux-usb
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);
>>>>>
>>>>> }
>>>>>
>>>>> /*
>>
>>
>>
>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
@ 2018-04-23 13:24 ` Tomeu Vizoso
0 siblings, 0 replies; 49+ messages in thread
From: Tomeu Vizoso @ 2018-04-23 13:24 UTC (permalink / raw)
To: Minas Harutyunyan
Cc: Heiko Stübner, linux-kernel, Felipe Balbi, Amelie Delaunay,
Greg Kroah-Hartman, linux-usb
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
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
@ 2018-04-24 8:58 ` Heiko Stuebner
0 siblings, 0 replies; 49+ messages in thread
From: Heiko Stübner @ 2018-04-24 8:58 UTC (permalink / raw)
To: Tomeu Vizoso
Cc: Minas Harutyunyan, linux-kernel, Felipe Balbi, Amelie Delaunay,
Greg Kroah-Hartman, linux-usb
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
^ permalink raw reply [flat|nested] 49+ messages in thread
* [v4] usb: dwc2: dwc2_vbus_supply_init: fix error check
@ 2018-04-24 8:58 ` Heiko Stuebner
0 siblings, 0 replies; 49+ messages in thread
From: Heiko Stuebner @ 2018-04-24 8:58 UTC (permalink / raw)
To: Tomeu Vizoso
Cc: Minas Harutyunyan, linux-kernel, Felipe Balbi, Amelie Delaunay,
Greg Kroah-Hartman, linux-usb
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
^ permalink raw reply [flat|nested] 49+ messages in thread