lkml.org 
[lkml]   [2015]   [Jan]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 0/2] w1: slaves: w1_therm: Add sysfs entry for current temperature
    On Tue, Jan 06, 2015 at 03:29:54PM +0100, Mariusz Gorski wrote:
    > DS18B20 and it's brothers are pretty popular in the RaspberryPi world
    > when it comes to temperature measurement. All tutorials on the Internet
    > use the same way of parsing the output of the w1_slave sysfs file.
    > These patches add a dedicated sysfs entry called 'temp' whose only job
    > is to output the current temperature.
    >
    > What could be improved here is the way how dev->bus_mutex is locked/unlocked,
    > which is split right now between two functions - maybe it would be better
    > to move all dev->bus_mutex locking into read_rom() and introduce another
    > mutex for protecting the slave's structures (e.g. family_data).

    As to adding another mutex, there are enough little mutexes around the
    one wire system that it already feels like a mess and I tried to get
    all the race conditions that I could.

    I think you don't understand what the bus_mutex protects. It protects
    other devices from accessing the one wire bus. The way you coded it,
    temp_show will allow only one temperature sensor to be accessed and
    read at a time, considering it takes 750 ms that's holding up the bus
    for a long time, and depending on how the devices are connected, isn't
    needed. If the temperature sensor is parasite powered, nothing else
    can use the bus while it is converting, if it has dedicated power,
    other slaves can be accessed at the same time.

    I second using a standard sensor attribute if there is one appropriate
    rather than here. I didn't in my application, but then I wasn't
    adding a new method to get the same data, I was using the existing
    netlink method. If it does get added to a standard sensor access
    method, I do request that it be optional (like w1_therm which I don't
    use), and not sit there polling the sensor for the current temperature
    every X seconds when no one is using the data (OpenBSD does this, or
    at least last I saw, and you have no idea how old the sensor data
    reading is or have any control over how often it will poll it).

    I have 15 DS18B20 temperature sensors scattered around the house on a
    single one wire network. I don't use the w1_therm system, I talk
    through netlink to the kernel. This has the advantage of being an
    asynchronous socket based interface so I can write a packet to request
    each sensor to do a conversion, get the write command status, and then
    after a timeout, write another to get a return, then read back the
    data.

    Using the sysfs means blocking while the read is in progress. Reading
    15 means either taking at least 11.250 seconds as you read them
    serially, or having 15 threads blocked on one read each (provided they
    don't block the other readers).

    Thanks for Cc'ing me on this.

    > Mariusz Gorski (2):
    > w1: slaves: w1_therm: Extract read_rom function
    > w1: slaves: w1_therm: Add temp attribute
    >
    > drivers/w1/slaves/w1_therm.c | 83 ++++++++++++++++++++++++++++++++++----------
    > 1 file changed, 65 insertions(+), 18 deletions(-)
    >
    > --
    > 2.2.1

    --
    David Fries <david@fries.net> PGP pub CB1EE8F0
    http://fries.net/~david/


    \
     
     \ /
      Last update: 2015-01-07 03:21    [W:3.129 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site