On Thu, 2 Jun 2005 17:45:34 -0400 <BGardner at Wabtec.com> wrote: > Evgeniy, > > Thank you for your response. > > I will split this patch into a few separate patches: > > 1) Touch/read/write changes only > > 2) Triplet/w1_search() changes only > > 3) Search timing change with your suggested sysfs modification > Should it default to continual or on-demand scan? > > 4) Default family change > > How does that sound? It sounds very good. I think defult should be continual scan for systems without "extended" userspace. Please post them so I will test it tomorrow and push upstrem. And could you please create some documentation about new features in Documentation/w1. Thank you. > Thanks, > Ben > > -----Original Message----- > From: Evgeniy Polyakov [mailto:johnpol at 2ka.mipt.ru] > Sent: Thursday, June 02, 2005 3:03 PM > To: Gardner, Ben > Cc: lm-sensors at lm-sensors.org > Subject: Re: [PATCH 2.6.12-rc5-mm2] w1 search cleanup > > On Thu, 2 Jun 2005 13:09:52 -0400 > <BGardner at Wabtec.com> wrote: > > Hello, Ben. > > I very like your changes, although I did not tested it yet, will > do it later this week. > But I have some comments about. > > > While writing a driver for the DS2482 (an i2c to w1 bridge), > > I made a few enhancements to the w1 subsystem. > > > > 1. > > I added a new I/O function: triplet > > A triplet reads two bits and writes a direction bit. The DS2482 > > implements this in hardware. > > I modified w1_search() to use w1_triplet() at its core. > > > > 2. > > I cleaned up the I/O functions to separate emulated vs native w1 > > support. > > A w1 bus master must be able to do one of: > > a. Set and sample the line via write_bit() and read_bit() > > b. Support reset_bus() and touch_bit() > > Function set (a) is only needed for emulated devices (ie, a parallel > > port). > > I hid w1_read_bit() and w1_write_bit() behind w1_touch_bit(), and > > changed functions to call touch_bit() instead or read/write_bit(). > > This changes are quite good. > > > 3. > > Searching is fairly slow - it requires about 200 w1 bit cycles per > > device, multiplied by the number of devices on the bus. > > I modified the w1_process() to NOT periodically search the bus. > > A sysfs entry was added to request a search. (w1_master_search). > > To request a search, echo anything into w1_master_search and it'll run > > another search. > > This one is very usefull, but we do want a mode, when w1 core > repeatedly scans for devices - concider ibuttons which can only > "exists" and be found for a couple of seconds, and it is > not always possible and sensible to have some userspace writing to the > sysfs > file. > I would like to have 2 search modes - writing "1" to sysfs turns default > always-scan mode into write-on-request mode, and writing "2" turns it > back. > Or something similar... > > > 4. > > I added a default family so that a slave device will get reported even > > if there isn't a driver for that family. > > > > Signed-off-by: Ben Gardner <bgardner at wabtec.com> > > Besides some whitespace and some brackets places [ugh, Greg will kill me > > for such patches] and my above comment I do not have objections, > but I would like to test your changes first and see them presented > in a splitted series of patches. I see almost 10 major changes > in your patch :) > > Thank you. > > Evgeniy Polyakov > > Only failure makes us experts. -- Theo de Raadt > > Evgeniy Polyakov Only failure makes us experts. -- Theo de Raadt