Re: [PATCH 4/5] hwmon: (ina3221) Make sure data is ready after channel enabling

[Nicolin Chen <nicoleotsuka@gmail.com>]

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