Hello Guenter, On Wed, Oct 17, 2018 at 09:55:43AM -0700, Guenter Roeck wrote: > > @@ -676,6 +701,13 @@ static int __maybe_unused ina3221_resume(struct device *dev) > > if (ret) > > return ret; > > > > + /* Make sure data conversion is finished */ > > + ret = ina3221_wait_for_data_if_active(ina); > > This is quite expensive and would delay resume (and enable, for that matter). > A less expensive solution would be to save the enable time here and in > ina3221_write_enable(). ina3221_wait_for_data_if_active() can then be called > from read functions and would wait if not enough time has expired. > > if (time_before(...)) > usleep_range(missing wait time, missing_wait_time * 2); > > or something like that. This could be done per channel or, to keep > things simple, just using a single time for all channels. I was thinking something that'd fit one-shot mode too so decided to add a polling. But you are right. It does make sense to skip polling until an actual read happens, though it also feels a bit reasonable to me that putting a polling to the enabling routine. The wait time would be sightly more complicated than the polling actually, as it needs to involve total conversion time which may vary depending on the number of enabled channels. I will see what would be safer and easier and apply that in v2. Thanks Nicolin
