<div dir="ltr">my 50 cents on this.<div><br></div><div>We could change the driver(s) to periodically poll the sensors like every second, and store the latest value. Then the sysfs read shall always return the last completed measurement data. We should do this for both the tachometers reading and the temperature sensors. There shall be some logic implemented for aging (when sensor read constantly fails)</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, May 5, 2017 at 11:37 AM, Nancy Yuen <span dir="ltr"><<a href="mailto:yuenn@google.com" target="_blank">yuenn@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">1. General design issue if time between reads of a sensor is dependent on the number of sensors in the system.  And drivers can fail, due to miss behaving code or hardware.  In this design, sensor report could be significantly delayed if one sensor/driver were bad or misbehaving.<div><br></div><div>2. So putting the onus on each application to implement timestamps is better design than having timestamps recorded by a central source, the source that actually is reading from the driver?</div></div><div class="gmail_extra"><br clear="all"><div><div class="m_2461350142886178694gmail_signature" data-smartmail="gmail_signature">----------<br>Nancy</div></div><div><div class="h5">
<br><div class="gmail_quote">On Fri, May 5, 2017 at 11:05 AM, Rick Altherr <span dir="ltr"><<a href="mailto:raltherr@google.com" target="_blank">raltherr@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Jagha is currently on leave.</div><div class="m_2461350142886178694HOEnZb"><div class="m_2461350142886178694h5"><div class="gmail_extra"><br><div class="gmail_quote">On Fri, May 5, 2017 at 11:02 AM, Patrick Williams <span dir="ltr"><<a href="mailto:patrick@stwcx.xyz" target="_blank">patrick@stwcx.xyz</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Jaghu,<br>
<br>
Can you comment?  I'm looking at the version that was merged into Linus'<br>
master for 4.12:<br>
<br>
commit 2d7a548a3eff382da5cd743670693b<a href="tel:(765)%20732-7714" value="+17657327714" target="_blank"><wbr>7657327714</a><br>
Author: Jaghathiswari Rankappagounder Natarajan <<a href="mailto:jaghu@google.com" target="_blank">jaghu@google.com</a>><br>
Date:   Tue Apr 4 17:52:41 2017 -0700<br>
<br>
    drivers: hwmon: Support for ASPEED PWM/Fan tach<br>
<div class="m_2461350142886178694m_-3791221182464829462HOEnZb"><div class="m_2461350142886178694m_-3791221182464829462h5"><br>
<br>
On Fri, May 05, 2017 at 10:48:39AM -0700, Rick Altherr wrote:<br>
> I can't comment on the driver implementation choices.  They have been<br>
> reviewed and accepted by upstream.  I'm not sure what version of the driver<br>
> you are looking at.<br>
><br>
> On Fri, May 5, 2017 at 10:43 AM, Patrick Williams <<a href="mailto:patrick@stwcx.xyz" target="_blank">patrick@stwcx.xyz</a>> wrote:<br>
><br>
> > Rick,<br>
> ><br>
> > On Fri, May 05, 2017 at 09:48:01AM -0700, Rick Altherr wrote:<br>
> > > I've chatted with Patrick V. separately about the driver.  AST2400/2500<br>
> > fan<br>
> > > tach hardware measures only one fan at a time.  I think we can adjust the<br>
> > > driver settings to reduce the measurement time but it will scale with #<br>
> > of<br>
> > > tachs being read.<br>
> ><br>
> > I never looked at this driver before but it looks like it is doing an<br>
> > 'msleep' in the hwmon read path after resetting a counter register and<br>
> > then counting rotations?  Two comments:<br>
> ><br>
> > 1. As it stands, it doesn't appear that this driver is actually<br>
> > multi-reader safe (either thread or process).  There is no locking or<br>
> > queueing to prevent one reader from resetting the result register while<br>
> > another is performing the msleep loop.  Multi-threading might "go<br>
> > faster" but it will give entirely wrong results by my reading.<br>
> ><br>
> > 2. It seems bad to do a long-running msleep in the hwmon read path to<br>
> > begin with.  Should this driver be restructured to have a kthread read<br>
> > the channels in the background on a polling interval instead of<br>
> > initiating by userspace action?<br>
> ><br>
> > --<br>
> > Patrick Williams<br>
> ><br>
<br>
</div></div><span class="m_2461350142886178694m_-3791221182464829462HOEnZb"><font color="#888888">--<br>
Patrick Williams<br>
</font></span></blockquote></div><br></div>
</div></div></blockquote></div><br></div></div></div>
</blockquote></div><br></div>