| Message ID | 20220426123329.775-3-du@axentia.se (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | usb: usb251xb: configurable reset delay | expand |
On Tue, Apr 26, 2022 at 12:34:13PM +0000, Daniels Umanovskis wrote: > According to the datasheet, the hub should be operational 500us after > the reset has been deasserted. Some individual circuits have been > observed not to reset within the specified 500us and require a longer > wait for subsequent configuration to succeed. > > Signed-off-by: Daniels Umanovskis <du@axentia.se> > --- > drivers/usb/misc/usb251xb.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/misc/usb251xb.c b/drivers/usb/misc/usb251xb.c > index 04c4e3fed094..e287e241ef96 100644 > --- a/drivers/usb/misc/usb251xb.c > +++ b/drivers/usb/misc/usb251xb.c > @@ -115,6 +115,7 @@ struct usb251xb { > struct regulator *vdd; > u8 skip_config; > struct gpio_desc *gpio_reset; > + u32 reset_delay_us; > u16 vendor_id; > u16 product_id; > u16 device_id; > @@ -278,7 +279,7 @@ static void usb251xb_reset(struct usb251xb *hub) > gpiod_set_value_cansleep(hub->gpio_reset, 0); > > /* wait for hub recovery/stabilization */ > - usleep_range(500, 750); /* >=500us after RESET_N deasserted */ > + fsleep(hub->reset_delay_us); > > i2c_unlock_bus(hub->i2c->adapter, I2C_LOCK_SEGMENT); > } > @@ -424,6 +425,9 @@ static int usb251xb_get_ofdata(struct usb251xb *hub, > return err; > } > > + if (of_property_read_u32(np, "reset-delay-us", &hub->reset_delay_us)) > + hub->reset_delay_us = 500; So if this fails the delay is 0? And what commit id does this fix? And any reason why you didn't cc: the people you get when running scripts/get_maintainer.pl on the patch? thanks, greg k-h
On 4/26/22 2:46 PM, Greg KH wrote: > On Tue, Apr 26, 2022 at 12:34:13PM +0000, Daniels Umanovskis wrote: >> According to the datasheet, the hub should be operational 500us after >> the reset has been deasserted. Some individual circuits have been >> observed not to reset within the specified 500us and require a longer >> wait for subsequent configuration to succeed. >> >> Signed-off-by: Daniels Umanovskis <du@axentia.se> >> --- >> drivers/usb/misc/usb251xb.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/usb/misc/usb251xb.c b/drivers/usb/misc/usb251xb.c >> index 04c4e3fed094..e287e241ef96 100644 >> --- a/drivers/usb/misc/usb251xb.c >> +++ b/drivers/usb/misc/usb251xb.c >> @@ -115,6 +115,7 @@ struct usb251xb { >> struct regulator *vdd; >> u8 skip_config; >> struct gpio_desc *gpio_reset; >> + u32 reset_delay_us; >> u16 vendor_id; >> u16 product_id; >> u16 device_id; >> @@ -278,7 +279,7 @@ static void usb251xb_reset(struct usb251xb *hub) >> gpiod_set_value_cansleep(hub->gpio_reset, 0); >> >> /* wait for hub recovery/stabilization */ >> - usleep_range(500, 750); /* >=500us after RESET_N deasserted */ >> + fsleep(hub->reset_delay_us); >> >> i2c_unlock_bus(hub->i2c->adapter, I2C_LOCK_SEGMENT); >> } >> @@ -424,6 +425,9 @@ static int usb251xb_get_ofdata(struct usb251xb *hub, >> return err; >> } >> >> + if (of_property_read_u32(np, "reset-delay-us", &hub->reset_delay_us)) >> + hub->reset_delay_us = 500; > So if this fails the delay is 0? of_property_read_u32 can fail with -EINVAL or -ENODATA, then delay is 500, unless I'm missing something more obvious. > And what commit id does this fix? No Fixes: tag because this isn't a bug or regression, just a new workaround for a hardware issue nobody's (presumably) encountered/bothered with. > And any reason why you didn't cc: the people you get when running > scripts/get_maintainer.pl on the patch? get_maintainer showed Richard as the maintainer. You showed as a supporter, if supporters should also be CCed by default then it's my bad - perhaps that should be clarified in submitting-patches.rst. > thanks, > > greg k-h /Daniels
On Tue, Apr 26, 2022 at 03:06:34PM +0200, Daniels Umanovskis wrote: > On 4/26/22 2:46 PM, Greg KH wrote: > > On Tue, Apr 26, 2022 at 12:34:13PM +0000, Daniels Umanovskis wrote: > > > According to the datasheet, the hub should be operational 500us after > > > the reset has been deasserted. Some individual circuits have been > > > observed not to reset within the specified 500us and require a longer > > > wait for subsequent configuration to succeed. > > > > > > Signed-off-by: Daniels Umanovskis <du@axentia.se> > > > --- > > > drivers/usb/misc/usb251xb.c | 6 +++++- > > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/usb/misc/usb251xb.c b/drivers/usb/misc/usb251xb.c > > > index 04c4e3fed094..e287e241ef96 100644 > > > --- a/drivers/usb/misc/usb251xb.c > > > +++ b/drivers/usb/misc/usb251xb.c > > > @@ -115,6 +115,7 @@ struct usb251xb { > > > struct regulator *vdd; > > > u8 skip_config; > > > struct gpio_desc *gpio_reset; > > > + u32 reset_delay_us; > > > u16 vendor_id; > > > u16 product_id; > > > u16 device_id; > > > @@ -278,7 +279,7 @@ static void usb251xb_reset(struct usb251xb *hub) > > > gpiod_set_value_cansleep(hub->gpio_reset, 0); > > > /* wait for hub recovery/stabilization */ > > > - usleep_range(500, 750); /* >=500us after RESET_N deasserted */ > > > + fsleep(hub->reset_delay_us); > > > i2c_unlock_bus(hub->i2c->adapter, I2C_LOCK_SEGMENT); > > > } > > > @@ -424,6 +425,9 @@ static int usb251xb_get_ofdata(struct usb251xb *hub, > > > return err; > > > } > > > + if (of_property_read_u32(np, "reset-delay-us", &hub->reset_delay_us)) > > > + hub->reset_delay_us = 500; > > So if this fails the delay is 0? > of_property_read_u32 can fail with -EINVAL or -ENODATA, then delay is 500, > unless I'm missing something more obvious. Ah, no, you are right, sorry, I read this wrong. greg k-h
On Tue, Apr 26, 2022 at 12:34:13PM +0000, Daniels Umanovskis wrote: > According to the datasheet, the hub should be operational 500us after > the reset has been deasserted. Some individual circuits have been > observed not to reset within the specified 500us and require a longer > wait for subsequent configuration to succeed. > > Signed-off-by: Daniels Umanovskis <du@axentia.se> LGTM. Reviewed-by: Richard Leitner <richard.leitner@skidata.com> > --- > drivers/usb/misc/usb251xb.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/misc/usb251xb.c b/drivers/usb/misc/usb251xb.c > index 04c4e3fed094..e287e241ef96 100644 > --- a/drivers/usb/misc/usb251xb.c > +++ b/drivers/usb/misc/usb251xb.c > @@ -115,6 +115,7 @@ struct usb251xb { > struct regulator *vdd; > u8 skip_config; > struct gpio_desc *gpio_reset; > + u32 reset_delay_us; > u16 vendor_id; > u16 product_id; > u16 device_id; > @@ -278,7 +279,7 @@ static void usb251xb_reset(struct usb251xb *hub) > gpiod_set_value_cansleep(hub->gpio_reset, 0); > > /* wait for hub recovery/stabilization */ > - usleep_range(500, 750); /* >=500us after RESET_N deasserted */ > + fsleep(hub->reset_delay_us); > > i2c_unlock_bus(hub->i2c->adapter, I2C_LOCK_SEGMENT); > } > @@ -424,6 +425,9 @@ static int usb251xb_get_ofdata(struct usb251xb *hub, > return err; > } > > + if (of_property_read_u32(np, "reset-delay-us", &hub->reset_delay_us)) > + hub->reset_delay_us = 500; > + > if (of_property_read_u16_array(np, "vendor-id", &hub->vendor_id, 1)) > hub->vendor_id = USB251XB_DEF_VENDOR_ID; > > -- > 2.30.2 >
diff --git a/drivers/usb/misc/usb251xb.c b/drivers/usb/misc/usb251xb.c index 04c4e3fed094..e287e241ef96 100644 --- a/drivers/usb/misc/usb251xb.c +++ b/drivers/usb/misc/usb251xb.c @@ -115,6 +115,7 @@ struct usb251xb { struct regulator *vdd; u8 skip_config; struct gpio_desc *gpio_reset; + u32 reset_delay_us; u16 vendor_id; u16 product_id; u16 device_id; @@ -278,7 +279,7 @@ static void usb251xb_reset(struct usb251xb *hub) gpiod_set_value_cansleep(hub->gpio_reset, 0); /* wait for hub recovery/stabilization */ - usleep_range(500, 750); /* >=500us after RESET_N deasserted */ + fsleep(hub->reset_delay_us); i2c_unlock_bus(hub->i2c->adapter, I2C_LOCK_SEGMENT); } @@ -424,6 +425,9 @@ static int usb251xb_get_ofdata(struct usb251xb *hub, return err; } + if (of_property_read_u32(np, "reset-delay-us", &hub->reset_delay_us)) + hub->reset_delay_us = 500; + if (of_property_read_u16_array(np, "vendor-id", &hub->vendor_id, 1)) hub->vendor_id = USB251XB_DEF_VENDOR_ID;
According to the datasheet, the hub should be operational 500us after the reset has been deasserted. Some individual circuits have been observed not to reset within the specified 500us and require a longer wait for subsequent configuration to succeed. Signed-off-by: Daniels Umanovskis <du@axentia.se> --- drivers/usb/misc/usb251xb.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)