Re: [PATCH v4 7/7] usb: typec: mux: Add On Semi fsa4480 driver

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Mon, Mar 07, 2022 at 06:48:25AM -0800, Bjorn Andersson wrote:
> On Mon 07 Mar 02:16 PST 2022, Andy Shevchenko wrote:
> > On Sun, Mar 06, 2022 at 07:40:40PM -0800, Bjorn Andersson wrote:

...

> > > +		/* 15us to allow the SBU switch to turn off */
> > > +		usleep_range(15, 1000);
> > 
> > This is quite unusual range.
> > 
> > If you are fine with the long delay, why to stress the system on it?
> > Otherwise the use of 1000 is unclear.
> > 
> > That said, I would expect one of the below:
> > 
> > 		usleep_range(15, 30);
> > 		usleep_range(500, 1000);
> 
> Glad you asked about that, as you say the typical form is to keep the
> range within 2x of the lower value, or perhaps lower + 5.
> 
> But if the purpose is to specify a minimum time and then give a max to
> give the system some flexibility in it's decision of when to wake up.
> And in situations such as this, we're talking about someone connecting a
> cable, so we're in "no rush" and I picked the completely arbitrary 1ms
> as the max.
> 
> Do you see any drawback of this much higher number? (Other than it
> looking "wrong")

I see the drawback of low number. The 1000 makes not much sense to me with
the minimum 66x times less. If there is no rush, use some reasonable values,
what about

		usleep_range(100, 1000);

? 10x is way better than 66x.

-- 
With Best Regards,
Andy Shevchenko