It appears that the users of usb3503_reset expect state to be the original active-low value.
Signed-off-by: David Shah dave@ds0.me --- drivers/usb/misc/usb3503.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/misc/usb3503.c b/drivers/usb/misc/usb3503.c index e0c11331632a..9a228bc0566c 100644 --- a/drivers/usb/misc/usb3503.c +++ b/drivers/usb/misc/usb3503.c @@ -62,7 +62,7 @@ static int usb3503_reset(struct usb3503 *hub, int state) gpiod_set_value_cansleep(hub->connect, 0);
if (hub->reset) - gpiod_set_value_cansleep(hub->reset, !state); + gpiod_set_value_cansleep(hub->reset, state);
/* Wait T_HUBINIT == 4ms for hub logic to stabilize */ if (state)
Hi Dave,
Am 23.07.2020 um 16:31 schrieb David Shah dave@ds0.me:
It appears that the users of usb3503_reset expect state to be the original active-low value.
Signed-off-by: David Shah dave@ds0.me
drivers/usb/misc/usb3503.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/misc/usb3503.c b/drivers/usb/misc/usb3503.c index e0c11331632a..9a228bc0566c 100644 --- a/drivers/usb/misc/usb3503.c +++ b/drivers/usb/misc/usb3503.c @@ -62,7 +62,7 @@ static int usb3503_reset(struct usb3503 *hub, int state) gpiod_set_value_cansleep(hub->connect, 0);
if (hub->reset)
gpiod_set_value_cansleep(hub->reset, !state);
gpiod_set_value_cansleep(hub->reset, state);
good finding!
Seems that it should have a
Fixes: 51d22e855ea3 ("usb: usb3503: Convert to use GPIO descriptors")
because it looks as if it was probably introduced there as a typo...
/* Wait T_HUBINIT == 4ms for hub logic to stabilize */ if (state) -- 2.27.0
I'll pick it to all branches that contain 51d22e855ea3
BR and thanks, Nikolaus
Hi Nikolaus,
Sorry, please ignore this patch for now, if not already merged.
The proper solution is to set GPIO_ACTIVE_LOW in the device tree, but there are some other reliability issues with hub startup (the same reason the modem sometimes took 255 seconds to appear on older kernels).
Hopefully I will have an improved v2 patch soon :)
Best
David
On Thu, 2020-07-23 at 17:29 +0200, H. Nikolaus Schaller wrote:
Hi Dave,
Am 23.07.2020 um 16:31 schrieb David Shah dave@ds0.me:
It appears that the users of usb3503_reset expect state to be the original active-low value.
Signed-off-by: David Shah dave@ds0.me
drivers/usb/misc/usb3503.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/misc/usb3503.c b/drivers/usb/misc/usb3503.c index e0c11331632a..9a228bc0566c 100644 --- a/drivers/usb/misc/usb3503.c +++ b/drivers/usb/misc/usb3503.c @@ -62,7 +62,7 @@ static int usb3503_reset(struct usb3503 *hub, int state) gpiod_set_value_cansleep(hub->connect, 0);
if (hub->reset)
gpiod_set_value_cansleep(hub->reset, !state);
gpiod_set_value_cansleep(hub->reset, state);good finding!
Seems that it should have a
Fixes: 51d22e855ea3 ("usb: usb3503: Convert to use GPIO descriptors")
because it looks as if it was probably introduced there as a typo...
/* Wait T_HUBINIT == 4ms for hub logic to stabilize */ if (state) -- 2.27.0
I'll pick it to all branches that contain 51d22e855ea3
BR and thanks, Nikolaus
Hi David, no worry...
I have found a 5 years old note that it seems as if the usb3503 responds on the i2c bus only after it has seen the reset.
But if we control the reset after probing the driver it will never probe because i2c detection occurs before probing the driver...
I remember there were some reliability issues in the first prototypes.
So we may have a bug that made it work :)
BR and thanks, Nikolaus
Am 25.07.2020 um 16:11 schrieb David Shah dave@ds0.me:
Hi Nikolaus,
Sorry, please ignore this patch for now, if not already merged.
The proper solution is to set GPIO_ACTIVE_LOW in the device tree, but there are some other reliability issues with hub startup (the same reason the modem sometimes took 255 seconds to appear on older kernels).
Hopefully I will have an improved v2 patch soon :)
Best
David
On Thu, 2020-07-23 at 17:29 +0200, H. Nikolaus Schaller wrote:
Hi Dave,
Am 23.07.2020 um 16:31 schrieb David Shah dave@ds0.me:
It appears that the users of usb3503_reset expect state to be the original active-low value.
Signed-off-by: David Shah dave@ds0.me
drivers/usb/misc/usb3503.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/misc/usb3503.c b/drivers/usb/misc/usb3503.c index e0c11331632a..9a228bc0566c 100644 --- a/drivers/usb/misc/usb3503.c +++ b/drivers/usb/misc/usb3503.c @@ -62,7 +62,7 @@ static int usb3503_reset(struct usb3503 *hub, int state) gpiod_set_value_cansleep(hub->connect, 0);
if (hub->reset)
gpiod_set_value_cansleep(hub->reset, !state);
gpiod_set_value_cansleep(hub->reset, state);good finding!
Seems that it should have a
Fixes: 51d22e855ea3 ("usb: usb3503: Convert to use GPIO descriptors")
because it looks as if it was probably introduced there as a typo...
/* Wait T_HUBINIT == 4ms for hub logic to stabilize */ if (state) -- 2.27.0
I'll pick it to all branches that contain 51d22e855ea3
BR and thanks, Nikolaus
-
David Shah -
H. Nikolaus Schaller