[PATCH 0/35] W1: w1 core deadlock and oops fixes, ds2490 updates

[PATCH 0/35] W1: w1 core deadlock and oops fixes, ds2490 updates

[David Fries]

[-- Attachment #1: Type: text/plain, Size: 2453 bytes --]

The Linux one wire system allows talking to little devices, mostly
sensors.  Sensors such as temperature sensors that I want to put
around the house, but first the one wire system and ds2490 USB to one
wire controller driver needs to be stable.  After a kernel panic, I
did most all of the development inside of qemu.  I had an issue with
bulk transfers failing unless I told qemu to disconnect and reconnect
the USB device every time I reloaded, the module, but it worked really
nicely.  What follows is a long list of fixes and enhancements.  I
would like some tester feedback.  I only have the ds2490 USB to one
wire controller and ds18b20 thermometer.

This set of patches fixes bugs in the one wire driver and the ds2490
one wire USB controller.  Some of the bugs will deadlock the one wire
system and print out a message every second reminding you of this.
Others will panic the system.  One will spam with printk in an
infinite loop.  The w1_therm slave device driver would overflow the
userspace buffer, and didn't even work properly with `cat`.

In the name of being nice to the rest of the system, I've eliminated a
thread that was waking up and polling every second, for pretty much
only startup and shutdown events.  The other thread, w1_process, will
now block when there isn't anything to do, and if sleeping it will be
immediately woken for a new search or for preparing to exit.

ds2490 USB to one wire master driver has been improved.  The original
code was observed to take 3.91 seconds for a temperature conversion
and reading with 0.002s user and 3.001s system times.  The system was
very unresponsive, mostly due to some mdelay(750) calls.  Now it is
taking 0.860s elapsed with 0.004s user and 0.004s system time.  That
is pretty good considering that the temperature conversion takes 750ms
(rounded up to 752ms).  Some of the 108ms overload could be reduced by
a shorter reset period, overdrive data transfer speed, and combining
USB operations.  The driver now supports the strong pullup, which is
useful for parasite powered devices.

I was keeping track of my changes in cvs.  I've included the file, cvs
version number and log for that commit that make up the given patch.
The cvs number is just to help me keep everything organized to make
the patches.  The patches are against 2.6.25-rc4.

Signed-off-by: David Fries <david@fries.net>

--
David Fries <david@fries.net>
http://fries.net/~david/ (PGP encryption key available)

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 0/35] W1: w1 core deadlock and oops fixes, ds2490 updates

[Evgeniy Polyakov]

Hi David.

On Fri, Mar 28, 2008 at 07:23:11AM -0500, David Fries (david@fries.net) wrote:
> The Linux one wire system allows talking to little devices, mostly
> sensors.  Sensors such as temperature sensors that I want to put
> around the house, but first the one wire system and ds2490 USB to one
> wire controller driver needs to be stable.  After a kernel panic, I
> did most all of the development inside of qemu.  I had an issue with
> bulk transfers failing unless I told qemu to disconnect and reconnect
> the USB device every time I reloaded, the module, but it worked really
> nicely.  What follows is a long list of fixes and enhancements.  I
> would like some tester feedback.  I only have the ds2490 USB to one
> wire controller and ds18b20 thermometer.

Well, in our private converstion you never showed kernel panic dump or
at least talks about where it was located, but nevertheless I'm overall
ok with your changes. I ack all ds2490 changes and generally do not
object against others, although description of them some times are
really wrong, like what you call a deadlock was intended - driver
specially can not be unloaded while there are users of given bus.

There is another problem with your submission - please split it into 3-5
patches which are logically compound, so that there would not be things
like: patch1: replace A with B, patch2: remove B and the like.
You also have lots of codying style issues in the patchset.

> This set of patches fixes bugs in the one wire driver and the ds2490
> one wire USB controller.  Some of the bugs will deadlock the one wire
> system and print out a message every second reminding you of this.
> Others will panic the system.  One will spam with printk in an
> infinite loop.  The w1_therm slave device driver would overflow the
> userspace buffer, and didn't even work properly with `cat`.

The latter is wrong, and you never showed signs of previous errors,
but looking at the end results I agree that it is better solution
(especially eliminating of the additional thread and related changes).

> In the name of being nice to the rest of the system, I've eliminated a
> thread that was waking up and polling every second, for pretty much
> only startup and shutdown events.  The other thread, w1_process, will
> now block when there isn't anything to do, and if sleeping it will be
> immediately woken for a new search or for preparing to exit.
> 
> ds2490 USB to one wire master driver has been improved.  The original
> code was observed to take 3.91 seconds for a temperature conversion
> and reading with 0.002s user and 3.001s system times.  The system was
> very unresponsive, mostly due to some mdelay(750) calls.  Now it is
> taking 0.860s elapsed with 0.004s user and 0.004s system time.  That
> is pretty good considering that the temperature conversion takes 750ms
> (rounded up to 752ms).  Some of the 108ms overload could be reduced by
> a shorter reset period, overdrive data transfer speed, and combining
> USB operations.  The driver now supports the strong pullup, which is
> useful for parasite powered devices.

Yeah, mdelay was not a good solution.

> I was keeping track of my changes in cvs.  I've included the file, cvs
> version number and log for that commit that make up the given patch.
> The cvs number is just to help me keep everything organized to make
> the patches.  The patches are against 2.6.25-rc4.

Please reorganize your patches into something like this:
1. threads changes
2. master device removal changes (remove slaves which are on the bus
master being removed) and related things
3. ds2490 changes
4. cleanups

or something like that, reviewing 35 small patches which are removing
things past another is not a really good time...

Thank you. I will comment on separate patches.

-- 
	Evgeniy Polyakov

Re: [PATCH 0/35] W1: w1 core deadlock and oops fixes, ds2490 updates

[David Fries]

[-- Attachment #1: Type: text/plain, Size: 22195 bytes --]

Andrew Morton, I quoted you near the end.

On Sun, Mar 30, 2008 at 03:27:16PM +0400, Evgeniy Polyakov wrote:
> Hi David.
> 
> On Fri, Mar 28, 2008 at 07:23:11AM -0500, David Fries (david@fries.net) wrote:
> > The Linux one wire system allows talking to little devices, mostly
> > sensors.  Sensors such as temperature sensors that I want to put
> > around the house, but first the one wire system and ds2490 USB to one
> > wire controller driver needs to be stable.  After a kernel panic, I
> > did most all of the development inside of qemu.  I had an issue with
> > bulk transfers failing unless I told qemu to disconnect and reconnect
> > the USB device every time I reloaded, the module, but it worked really
> > nicely.  What follows is a long list of fixes and enhancements.  I
> > would like some tester feedback.  I only have the ds2490 USB to one
> > wire controller and ds18b20 thermometer.
> 
> Well, in our private converstion you never showed kernel panic dump or
> at least talks about where it was located, but nevertheless I'm overall
> ok with your changes. I ack all ds2490 changes and generally do not
> object against others, although description of them some times are
> really wrong, like what you call a deadlock was intended - driver
> specially can not be unloaded while there are users of given bus.

I dug out a panic before I started modifying the driver, see the end
of the message.  There is still a remaining oops.  Register the same
sensor with two masters.  The serial id of the sensor is assumed to be
unique and the sysfs code will cause an oops when the second sensor is
registered.  If you have two master devices that are both doing bus
searching, let one detect the sensor, the move it to the other master.
The second master will detect the device before the first one times it
out.  None of my patches address this.

Could you be more specific on "really wrong, like what you call a
deadlock was intended"?  I only use 'intended' once, and that is is
patch 1 in "The slave reconnect was incorrectly coded."  That
paragraph is describing a infinite loop inserting a slave driver, not
unloading a device driver.

If something is really wrong with the descriptions I'll fix it.

> There is another problem with your submission - please split it into 3-5
> patches which are logically compound, so that there would not be things
> like: patch1: replace A with B, patch2: remove B and the like.

You must be talking about patch 14 and patch 15 w1_slave_read_id sort
read bug.  That's the only place it does that.

I debated on how to submit
patch 14 "w1_slave_read_id multiple short read bug"
and
patch 15 "w1_slave_read_id from bin_attribute to device_attribute"
and I did it that way to first show what he bug was and fix it, then
show how simple the code was using a different method.  I'll submit it
in one patch.

> You also have lots of codying style issues in the patchset.
Such as?

> > This set of patches fixes bugs in the one wire driver and the ds2490
> > one wire USB controller.  Some of the bugs will deadlock the one wire
> > system and print out a message every second reminding you of this.
> > Others will panic the system.  One will spam with printk in an
> > infinite loop.  The w1_therm slave device driver would overflow the
> > userspace buffer, and didn't even work properly with `cat`.
> 
> The latter is wrong, and you never showed signs of previous errors,

This was the overview e-mail, and it was long enough without going
into details on each of the issues.  But here they are.

"will deadlock the one wire system and print out a message every second
reminding you of this."

This example requires a temperature sensor that uses w1_therm (it
isn't specific to w1_therm though).

insmod wire.ko
insmod slaves/w1_therm.ko
insmod masters/ds2490.ko
(wait for devices to be detected)
rmmod slaves/w1_therm

Waiting for family 40 to become free: refcnt=1.
Waiting for family 40 to become free: refcnt=1.
Waiting for family 40 to become free: refcnt=1.
...

40 = 0x28
#define W1_THERM_DS18B20        0x28
It is w1_therm that is trying to be removed, but it can't because the
slave is using w1_therm.  The error is nothing told the slave to stop
using w1_therm, so unless you do something it never breaks out of the
loop.  In this case,
rmmod ds2490
will allow you to break out of it, but it is in a deadlock until then.

[PATCH 1/35] W1: fix deadlocks and remove w1_control_thread
w1.c 1.2
w1_family.c 1.3
Fixed the infinite loop that happens if a slave device module is
removed from the kernel and there is a slave using that driver family.
That involves unattaching all slaves that are using that family after
the family has been removed from the list of available family drivers.
This is not a race condition, it will happen everytime.



"One will spam with printk in an infinite loop."

This requires two devices that aren't used by w1_smem, the slave
module doesn't matter, it just requires two devices that will not be
claimed by the driver loaded.

insmod wire.ko
insmod masters/ds2490.ko
(wait for devices to be detected)
insmod slaves/w1_smem.ko

w1_slave_release: Releasing 21-34c0000039c5.
w1_master_driver w1_bus_master1: Family 21 for 21.34c0000039c5.ba is not registered.
w1_slave_release: Releasing 23-00000002937d.
w1_master_driver w1_bus_master1: Family 23 for 23.00000002937d.84 is not registered.
w1_slave_release: Releasing 28-0000000e84a2.
w1_master_driver w1_bus_master1: Family 28 for 28.0000000e84a2.ed is not registered.
w1_slave_release: Releasing 21-34c0000039c5.
w1_master_driver w1_bus_master1: Family 21 for 21.34c0000039c5.ba is not registered.
w1_slave_release: Releasing 23-00000002937d.
w1_master_driver w1_bus_master1: Family 23 for 23.00000002937d.84 is not registered.
w1_slave_release: Releasing 28-0000000e84a2.
w1_master_driver w1_bus_master1: Family 28 for 28.0000000e84a2.ed is not registered.

21, 23, 28, LOOP

And yes, I had to hang the machine to collect this output.

[PATCH 1/35] W1: fix deadlocks and remove w1_control_thread
w1.c 1.3
w1.h 1.6
The slave reconnect was incorrectly coded.  When a new slave device
driver (and family code) was available it was intended to give slaves
using the default device a chance to use the specific slave device
driver.  It was using the list_for_each_entry_safe and took the brute
force method of any device using the default device was detached and
reconnected.  If there was a slave device driver that matched the
family code it would be added back to the list with that driver,
otherwise it would be added back with the default driver.  It only
takes two devices that don't match the current set of family codes to
be added to the end of the list to get into an infinite loop of
detach, add to the end of the list, get next entry.

(copy)
> The latter is wrong, and you never showed signs of previous errors,
Which latter are you referring to?

"The w1_therm slave device driver would overflow the userspace buffer"

Strace make it obvious, request one byte, '..., 1)', get 72, that's a
buffer overflow.

cait:/sys/devices/w1_bus_master1/28-0000000e84a2$ strace -e read,open dd if=w1_slave bs=1 count=1
open("w1_slave", O_RDONLY)              = 0
read(0, "00 01 4b 46 7f ff 10 10 54 : crc"..., 1) = 72
00 01 4b 46 7f ff 10 10 54 : crc=54 YES
00 01 4b 46 7f ff 10 10 54 t=16
1+0 records in
0+1 records out
72 bytes (72 B) copied, 32.8605 seconds, 0.0 kB/s
Process 2204 detached

[PATCH 16/35] [W1] w1_therm fix user buffer overflow and cat
slaves/w1_therm.c 1.8
buffer overflow:
	Execute a short read on w1_slave and w1_therm_read_bin would still
	return the full string size worth of data clobbering the user space
	buffer when it returned.  Switching to device_attribute avoids the
	buffer overflow problems.  With the snprintf formatted output dealing
	with short reads without doing a conversion per read would have
	been difficult.

'and didn't even work properly with `cat`."

cait:/sys/devices/w1_bus_master1/28-0000000e84a2$ cat w1_slave
05 01 4b 46 7f ff 0b 10 cd : crc=cd YES
05 01 4b 46 7f ff 0b 10 cd t=16
05 01 4b 46 7f ff 0b 10 cd : crc=cd YES
05 01 4b 46 7f ff 0b 10 cd t=16

That's two reads, two temperature conversions when there should have
only been one.

cait:/sys/devices/w1_bus_master1/28-0000000e84a2$ strace -e read,open
cat w1_slave 
read(3, "ff 00 4b 46 7f ff 01 10 9c : crc"..., 4096) = 72
read(3, "ff 00 4b 46 7f ff 01 10 9c : crc"..., 4096) = 72
read(3, "", 4096)                       = 0

[PATCH 16/35] [W1] w1_therm fix user buffer overflow and cat
slaves/w1_therm.c 1.8
bad behavior:
	`cat w1_slave` would cause two temperature conversions to take place.
	Previously the code assumed W1_SLAVE_DATA_SIZE would be returned with
	each read.  It would not return 0 unless the offset was less
	than W1_SLAVE_DATA_SIZE.  The result was the first read did a
	temperature conversion, filled the buffer and returned, the
	offset in the second read would be less than
	W1_SLAVE_DATA_SIZE and also fill the buffer and return, the
	third read would finnally have a big enough offset to return 0
	and cause cat to stop.  Now w1_therm_read will be called at
	most once per open.


Interesting, now that I have more sensors connected to the network and
I loaded the old in tree w1 driver (for the above tests), I see it
fails to detect one sensor.  The old driver consistently gets 4, while
with my changes it is consistently getting 5 and there should be 5
devices out there.  I must have fixed some bug without knowning it.

2.6.24 w1 system
28-0000000e84a2
81-00000020bbd0
21-34c0000039c5
23-00000002937d

submitted patch,
21-34c0000039c5
23-00000002937d
28-0000000e84a2
28-0000000e9812
81-00000020bbd0

> but looking at the end results I agree that it is better solution
> (especially eliminating of the additional thread and related changes).

Thanks, it pleased me greatly to remove the w1_thread that only polled
when it wasn't required.

> > In the name of being nice to the rest of the system, I've eliminated a
> > thread that was waking up and polling every second, for pretty much
> > only startup and shutdown events.  The other thread, w1_process, will
> > now block when there isn't anything to do, and if sleeping it will be
> > immediately woken for a new search or for preparing to exit.
> > 
> > ds2490 USB to one wire master driver has been improved.  The original
> > code was observed to take 3.91 seconds for a temperature conversion
> > and reading with 0.002s user and 3.001s system times.  The system was
> > very unresponsive, mostly due to some mdelay(750) calls.  Now it is
> > taking 0.860s elapsed with 0.004s user and 0.004s system time.  That
> > is pretty good considering that the temperature conversion takes 750ms
> > (rounded up to 752ms).  Some of the 108ms overload could be reduced by
> > a shorter reset period, overdrive data transfer speed, and combining
> > USB operations.  The driver now supports the strong pullup, which is
> > useful for parasite powered devices.
> 
> Yeah, mdelay was not a good solution.

How's the status of your one wire network?  What kind of sensors do
you have available?

> > I was keeping track of my changes in cvs.  I've included the file, cvs
> > version number and log for that commit that make up the given patch.
> > The cvs number is just to help me keep everything organized to make
> > the patches.  The patches are against 2.6.25-rc4.
> 
> Please reorganize your patches into something like this:
> 1. threads changes
> 2. master device removal changes (remove slaves which are on the bus
> master being removed) and related things
> 3. ds2490 changes
> 4. cleanups
> 
> or something like that, reviewing 35 small patches which are removing
> things past another is not a really good time...

You want me to take 35 patches and merge them down to 4?  There were a
lot of issues in here, and I don't have an automagic patch reorder
tool.  If you want to review fewer patches apply 1-6, do a diff and
look at that.

Documentation/SubmittingPatches '3) Separate your changes.' It goes on
to specifically separate out bug fixes and performance enhancements.

You requested in another e-mail to merge patch 1-6.  The deadlocks in
1 is clearly a bug.  6 block (instead of sleeping with a timeout) is a
performance enhancement.  5 might be a deadlock, but it's not at all
related to 1.  2 and 3 have nothing to do with any of the above.
Logically this come out to be a bunch of patches.

On Mon, Jan 21, 2008 at 06:56:35PM -0800, Andrew Morton wrote:
> On Mon, 21 Jan 2008 20:47:19 -0600 David Fries <david@fries.net> wrote:
> > Thanks.  Question, at what point should I just submit these one liner,
> > but independent bugs in one patch?
> 
> You shouldn't, unless they are dependent in both directions.

If Andrew Morton is going to say no for independent one line bugs, I'm
going to say No (for a bunch of complicated sometimes overlapping
bugs, features, and performance enhancements) as well.  Besides I have
something like 27KB in 664 lines of descriptions, if I tried to merge
the changes down there's no way I could figure out which patch lines
went with what descriptions, and no one else could either.

> Thank you. I will comment on separate patches.
> 
> -- 
> 	Evgeniy Polyakov

-- 
David Fries <david@fries.net>
http://fries.net/~david/ (PGP encryption key available)


ohci_hcd: 2006 August 04 USB 1.1 'Open' Host Controller (OHCI) Driver
ACPI: PCI Interrupt 0000:00:02.2[D] -> Link [LNKD] -> GSI 11 (level, low) -> IRQ 11
ohci_hcd 0000:00:02.2: OHCI Host Controller
ohci_hcd 0000:00:02.2: new USB bus registered, assigned bus number 1
ohci_hcd 0000:00:02.2: irq 11, io mem 0xdfffe000
usb usb1: configuration #1 chosen from 1 choice
hub 1-0:1.0: USB hub found
hub 1-0:1.0: 3 ports detected
ACPI: PCI Interrupt 0000:00:02.3[D] -> Link [LNKD] -> GSI 11 (level, low) -> IRQ 11
ohci_hcd 0000:00:02.3: OHCI Host Controller
ohci_hcd 0000:00:02.3: new USB bus registered, assigned bus number 2
ohci_hcd 0000:00:02.3: irq 11, io mem 0xdffff000
usb usb2: configuration #1 chosen from 1 choice
usb 1-2: new low speed USB device using ohci_hcd and address 2
hub 2-0:1.0: USB hub found
hub 2-0:1.0: 3 ports detected
usb 1-2: configuration #1 chosen from 1 choice
hiddev96: USB HID v1.00 Device [American Power Conversion BackUPS Pro 500 FW:16.3.D USB FW:4] on usb-0000:00:02.2-2
usb 2-1: new full speed USB device using ohci_hcd and address 2
usb 2-1: configuration #1 chosen from 1 choice
drivers/usb/class/usblp.c: usblp0: USB Bidirectional printer dev 2 if 0 alt 0 proto 2 vid 0x04A9 pid 0x1056
usb 2-1: USB disconnect, address 2
drivers/usb/class/usblp.c: usblp0: removed
usb 2-1: new full speed USB device using ohci_hcd and address 3
usb 2-1: configuration #1 chosen from 1 choice
drivers/usb/class/usblp.c: usblp0: USB Bidirectional printer dev 3 if 0 alt 0 proto 2 vid 0x04A9 pid 0x1056
usb 2-1: USB disconnect, address 3
drivers/usb/class/usblp.c: usblp0: removed
usb 2-1: new full speed USB device using ohci_hcd and address 4
usb 2-1: configuration #1 chosen from 1 choice
drivers/usb/class/usblp.c: usblp0: USB Bidirectional printer dev 4 if 0 alt 0 proto 2 vid 0x04A9 pid 0x1056
usb 2-3: new full speed USB device using ohci_hcd and address 5
usb 2-3: configuration #1 chosen from 1 choice
Driver for 1-wire Dallas network protocol.
usbcore: registered new interface driver DS9490R
w1_master_driver w1_bus_master1: Family 81 for 81.00000020bbd0.21 is not registered.
w1_slave_release: Releasing 81-00000020bbd0.
w1_master_driver w1_bus_master1: Family 81 for 81.00000020bbd0.21 is not registered.
w1_slave_release: Releasing 81-00000020bbd0.
0x81: count=16, status: 00 00 2f 40 05 04 04 00 20 29 00 00 00 01 00 00 
                                  enable flag:        0
                                 1-wire speed:        0
                       strong pullup duration:       2f
                   programming pulse duration:       40
                   pulldown slew rate control:        5
                             write-1 low time:        4
     data sample offset/write-0 recovery time:        4
                     reserved (test register):        0
                          device status flags:       20
                 communication command byte 1:       29
                 communication command byte 2:        0
          communication command buffer status:        0
             1-wire data output buffer status:        0
              1-wire data input buffer status:        1
                                     reserved:        0
                                     reserved:        0
Clearing ep0x83.
0x81: count=16, status: 00 00 2f 40 05 04 04 00 20 29 00 00 00 00 00 00 
                                  enable flag:        0
                                 1-wire speed:        0
                       strong pullup duration:       2f
                   programming pulse duration:       40
                   pulldown slew rate control:        5
                             write-1 low time:        4
     data sample offset/write-0 recovery time:        4
                     reserved (test register):        0
                          device status flags:       20
                 communication command byte 1:       29
                 communication command byte 2:        0
          communication command buffer status:        0
             1-wire data output buffer status:        0
              1-wire data input buffer status:        0
                                     reserved:        0
                                     reserved:        0
w1_master_driver w1_bus_master1: Family 21 for 21.34c0000039c5.ba is not registered.
Failed to read 1-wire data from 0x02: err=-110.
Failed to read 1-wire data from 0x02: err=-110.
usb 2-3: USB disconnect, address 5
w1_master_driver w1_bus_master1: Waiting for w1_bus_master1 to become free: refcnt=1.
w1_slave_release: Releasing 81-00000020bbd0.
w1_slave_release: Releasing 21-34c0000039c5.
usb 2-3: new full speed USB device using ohci_hcd and address 6
usb 2-3: configuration #1 chosen from 1 choice
usb 2-3: USB disconnect, address 6
usb 2-3: new full speed USB device using ohci_hcd and address 7
usb 2-3: configuration #1 chosen from 1 choice
usb 2-3: USB disconnect, address 7
w1_slave_release: Releasing 28-0000000e84a2.
w1_slave_release: Releasing 81-00000020bbd0.
usb 2-3: new full speed USB device using ohci_hcd and address 8
usb 2-3: device descriptor read/64, error -62
hub 2-0:1.0: Cannot enable port 3.  Maybe the USB cable is bad?
hub 2-0:1.0: Cannot enable port 3.  Maybe the USB cable is bad?
hub 2-0:1.0: Cannot enable port 3.  Maybe the USB cable is bad?
hub 2-0:1.0: Cannot enable port 3.  Maybe the USB cable is bad?
usb 2-3: new full speed USB device using ohci_hcd and address 12
usb 2-3: configuration #1 chosen from 1 choice
usb 2-3: USB disconnect, address 12
BUG: unable to handle kernel NULL pointer dereference at virtual address 0000000c
 printing eip:
c02fa9a6
*pde = 00000000
Oops: 0000 [#1]
PREEMPT 
Modules linked in: w1_smem w1_therm ds2490 wire cn ohci_hcd it87 hwmon_vid hwmon i2c_isa eeprom i2c_sis96x cx8800 cx88_dvb cx8802 cx88xx videodev tuner v4l1_compat v4l2_common nf_conntrack_ipv4 nfsd exportfs xt_state nf_conntrack nfnetlink ipt_REJECT iptable_filter ip_tables parport_pc parport floppy snd_intel8x0 snd_intel8x0m snd_ac97_codec ac97_bus snd_pcm_oss snd_mixer_oss snd_pcm snd_timer snd soundcore snd_page_alloc xt_tcpudp xt_multiport x_tables compat_ioctl32 cx88_vp3054_i2c mt352 ir_common i2c_algo_bit tveeprom btcx_risc dvb_pll or51132 video_buf_dvb video_buf nxt200x firmware_class isl6421 zl10353 cx24123 lgdt330x dvb_core cx22702 i2c_core nfs lockd sunrpc udf isofs nls_base usblp usbhid usbcore ff_memless sis900
CPU:    0
EIP:    0060:[<c02fa9a6>]    Not tainted VLI
EFLAGS: 00010282   (2.6.22 #10)
EIP is at klist_del+0x6/0x50
eax: 00000000   ebx: da0012a0   ecx: dba80000   edx: d8f05780
esi: da0012a0   edi: dba80ebc   ebp: 00000000   esp: dba80e4c
ds: 007b   es: 007b   fs: 0000  gs: 0000  ss: 0068
Process khubd (pid: 1079, ti=dba80000 task=c148ea50 task.ti=dba80000)
Stack: da0012a0 e0bb6100 c02faaa8 da001274 c0218dd1 da001274 da001200 c02191cd 
       da001274 c02187eb da001274 c02171aa da001274 da001200 dba80ebc 00000000 
       c0217228 ca46fcc0 e0bb323d c0110af8 00000000 00000000 df25e250 c02fa970 
Call Trace:
 [<c02faaa8>] klist_remove+0x8/0x20
 [<c0218dd1>] __device_release_driver+0x31/0x90
 [<c02191cd>] device_release_driver+0x1d/0x40
 [<c02187eb>] bus_remove_device+0x5b/0x70
 [<c02171aa>] device_del+0x19a/0x210
 [<c0217228>] device_unregister+0x8/0x10
 [<e0bb323d>] __w1_remove_master_device+0x8d/0xa0 [wire]
 [<c0110af8>] complete+0x48/0x70
 [<c02fa970>] klist_release+0x0/0x30
 [<e0b7b051>] ds_disconnect+0x41/0x60 [ds2490]
 [<e0a4470a>] usb_unbind_interface+0x2a/0x60 [usbcore]
 [<c0218e0e>] __device_release_driver+0x6e/0x90
 [<c02191cd>] device_release_driver+0x1d/0x40
 [<c02187eb>] bus_remove_device+0x5b/0x70
 [<c02171aa>] device_del+0x19a/0x210
 [<e0a4238c>] usb_disable_device+0x5c/0xc0 [usbcore]
 [<e0a3ebe2>] usb_disconnect+0x82/0x100 [usbcore]
 [<e0a3f15d>] hub_thread+0x31d/0xa10 [usbcore]
 [<c010f82c>] __activate_task+0x1c/0x30
 [<c02fb59f>] schedule+0x48f/0x550
 [<c02fb638>] schedule+0x528/0x550
 [<c0123990>] autoremove_wake_function+0x0/0x40
 [<e0a3ee40>] hub_thread+0x0/0xa10 [usbcore]
 [<c01238b8>] kthread+0x38/0x60
 [<c0123880>] kthread+0x0/0x60
 [<c0104187>] kernel_thread_helper+0x7/0x10
 =======================
Code: 04 89 42 04 89 10 c7 43 f8 00 01 10 00 c7 41 04 00 02 20 00 8d 43 04 e8 19 61 e1 ff c7 43 f4 00 00 00 00 5b c3 56 53 89 c6 8b 00 <8b> 58 0c b8 01 00 00 00 e8 5d 51 e1 ff 89 f0 e8 a6 ff ff ff 85 
EIP: [<c02fa9a6>] klist_del+0x6/0x50 SS:ESP 0068:dba80e4c

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

[PATCH 7/33 replaces 7/35] W1: feature, enable hardware strong pullup

[David Fries]

[-- Attachment #1: Type: text/plain, Size: 10544 bytes --]

===================================================================
This is an update to the previous set of patches, don't merge them
yet.  Some of the context was messed up and I don't expect patch to
apply the updates with the previous set cleanly.  I'll resubmit the
entire set if these updates look good.
===================================================================

Add a strong pullup option to the w1 system.  This supplies extra
power for parasite powered devices.

The one wire bus requires at a minimum one wire and ground.  The
common wire is used for sending and receiving data as well as
supplying power to devices that are parasite powered.  Temperature
sensors are one device that can be parasite powered and require no bus
transactions while the temperature conversion is progress.  A bus
transaction would pull the common wire down.  In addition to the
normal pullup resistor, which allows a device to pull the bus line
down to send data, some devices support a strong pullup mechanism to
deliver more power.  The documentation lists this as useful for larger
networks and when temperature sensors are exposed to higher
temperatures they can exceed the current supplied by a simple pullup
resistor.  This patch adds a mechanism to support a strong pullup, and
fall back to sleeping with the line high otherwise.  This patch
doesn't enable the strong pullup for any master or slave device, those
will follow as they require this patch.

The hardware USB 2490 one wire bus master has a bit on a few commands
which will enable the strong pullup as soon as the command finishes
executing.  That requires the ds2490 driver to be notified if it
should be set before the command is executed.  That's why I added the
w1_next_pullup.  That will call the master set_pullup just before the
next write, and reset the value to zero after it has completed.  If
the master driver doesn't support the set_pullup, it will delay the
appropriate amount of time after the write returns.

w1.h 1.2 1.3 1.4 1.10
Add set_pullup to w1_bus_master, and add pullup_duration to w1_master
to hold the strong pullup duration for the next write operation.  Add
enable_pullup to w1_master for a per master enable/disable option.

w1.c 1.20
Add w1_master_pullup sysfs entry.

w1_int.c 1.3 1.9
Add a check to disable set_pullup and print a warning message when the
operation isn't supported.  Add enable_pullup module parameter.

w1_io.c 1.2 1.3 1.4 1.6
Add w1_next_pullup, call w1_pre_write and w1_post_write to call
set_pullup before and after, or just sleep after if set_pullup isn't
supported.

Signed-off-by: David Fries <david@fries.net>
---
 drivers/w1/w1.c     |   30 ++++++++++++++++++++++
 drivers/w1/w1.h     |   12 +++++++++
 drivers/w1/w1_int.c |   16 ++++++++++++
 drivers/w1/w1_io.c  |   68 ++++++++++++++++++++++++++++++++++++++++++++++++---
 4 files changed, 122 insertions(+), 4 deletions(-)

diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 5d8c2c7..401c56a 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -270,6 +270,34 @@ static ssize_t w1_master_attribute_show_search(struct device *dev,
 	return count;
 }
 
+static ssize_t w1_master_attribute_store_pullup(struct device * dev,
+						struct device_attribute *attr,
+						const char * buf, size_t count)
+{
+	struct w1_master *md = dev_to_w1_master(dev);
+
+	mutex_lock(&md->mutex);
+	md->enable_pullup = simple_strtol(buf, NULL, 0);
+	mutex_unlock(&md->mutex);
+	wake_up_process(md->thread);
+
+	return count;
+}
+
+static ssize_t w1_master_attribute_show_pullup(struct device *dev,
+					       struct device_attribute *attr,
+					       char *buf)
+{
+	struct w1_master *md = dev_to_w1_master(dev);
+	ssize_t count;
+
+	mutex_lock(&md->mutex);
+	count = sprintf(buf, "%d\n", md->enable_pullup);
+	mutex_unlock(&md->mutex);
+
+	return count;
+}
+
 static ssize_t w1_master_attribute_show_pointer(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct w1_master *md = dev_to_w1_master(dev);
@@ -365,6 +393,7 @@ static W1_MASTER_ATTR_RO(attempts, S_IRUGO);
 static W1_MASTER_ATTR_RO(timeout, S_IRUGO);
 static W1_MASTER_ATTR_RO(pointer, S_IRUGO);
 static W1_MASTER_ATTR_RW(search, S_IRUGO | S_IWUGO);
+static W1_MASTER_ATTR_RW(pullup, S_IRUGO | S_IWUGO);
 
 static struct attribute *w1_master_default_attrs[] = {
 	&w1_master_attribute_name.attr,
@@ -375,6 +404,7 @@ static struct attribute *w1_master_default_attrs[] = {
 	&w1_master_attribute_timeout.attr,
 	&w1_master_attribute_pointer.attr,
 	&w1_master_attribute_search.attr,
+	&w1_master_attribute_pullup.attr,
 	NULL
 };
 
diff --git a/drivers/w1/w1.h b/drivers/w1/w1.h
index c47ca1e..83d8208 100644
--- a/drivers/w1/w1.h
+++ b/drivers/w1/w1.h
@@ -142,6 +142,12 @@ struct w1_bus_master
 	 */
 	u8		(*reset_bus)(void *);
 
+	/**
+	 * Put out a strong pull-up pulse of the specified duration.
+	 * @return -1=Error, 0=completed
+	 */
+	u8		(*set_pullup)(void *, int);
+
 	/** Really nice hardware can handles the different types of ROM search
 	 *  w1_master* is passed to the slave found callback.
 	 */
@@ -167,6 +173,11 @@ struct w1_master
 	void			*priv;
 	int			priv_size;
 
+	/** 5V strong pullup enabled flag, 1 enabled, zero disabled. */
+	int			enable_pullup;
+	/** 5V strong pullup duration in milliseconds, zero disabled. */
+	int			pullup_duration;
+
 	struct task_struct	*thread;
 	struct mutex		mutex;
 
@@ -201,6 +212,7 @@ u8 w1_calc_crc8(u8 *, int);
 void w1_write_block(struct w1_master *, const u8 *, int);
 u8 w1_read_block(struct w1_master *, u8 *, int);
 int w1_reset_select_slave(struct w1_slave *sl);
+void w1_next_pullup(struct w1_master *, int);
 
 static inline struct w1_slave* dev_to_w1_slave(struct device *dev)
 {
diff --git a/drivers/w1/w1_int.c b/drivers/w1/w1_int.c
index 3388d8f..8deb76c 100644
--- a/drivers/w1/w1_int.c
+++ b/drivers/w1/w1_int.c
@@ -31,6 +31,9 @@
 
 static u32 w1_ids = 1;
 
+static int w1_enable_pullup = 1;
+module_param_named(enable_pullup, w1_enable_pullup, int, 0);
+
 static struct w1_master * w1_alloc_dev(u32 id, int slave_count, int slave_ttl,
 				       struct device_driver *driver,
 				       struct device *device)
@@ -59,6 +62,7 @@ static struct w1_master * w1_alloc_dev(u32 id, int slave_count, int slave_ttl,
 	dev->initialized	= 0;
 	dev->id			= id;
 	dev->slave_ttl		= slave_ttl;
+	dev->enable_pullup	= w1_enable_pullup;
         dev->search_count	= -1; /* continual scan */
 
 	/* 1 for w1_process to decrement
@@ -107,6 +111,18 @@ int w1_add_master_device(struct w1_bus_master *master)
 		printk(KERN_ERR "w1_add_master_device: invalid function set\n");
 		return(-EINVAL);
         }
+	/* While it would be electrically possible to make a device that
+	 * generated a strong pullup in bit bang mode, only hardare that
+	 * controls 1-wire time frames are even expected to support a strong
+	 * pullup.  w1_io.c would need to support calling set_pullup before
+	 * the last write_bit operation of a w1_write_8 which it currently
+	 * doesn't.
+	 */
+	if(!master->write_byte && !master->touch_bit && master->set_pullup) {
+		printk(KERN_ERR "w1_add_master_device: set_pullup requires "
+			"write_byte or touch_bit, disabling\n");
+		master->set_pullup=NULL;
+	}
 
 	dev = w1_alloc_dev(w1_ids++, w1_max_slave_count, w1_max_slave_ttl, &w1_master_driver, &w1_master_device);
 	if (!dev)
diff --git a/drivers/w1/w1_io.c b/drivers/w1/w1_io.c
index 0056ef6..3cdac8b 100644
--- a/drivers/w1/w1_io.c
+++ b/drivers/w1/w1_io.c
@@ -93,6 +93,40 @@ static void w1_write_bit(struct w1_master *dev, int bit)
 }
 
 /**
+ * Pre-write operation, currently only supporting strong pullups.
+ * Program the hardware for a strong pullup, if one has been requested and
+ * the hardware supports it.
+ *
+ * @param dev     the master device
+ */
+static void w1_pre_write(struct w1_master *dev)
+{
+	if(dev->pullup_duration &&
+		dev->enable_pullup && dev->bus_master->set_pullup) {
+		dev->bus_master->set_pullup(dev->bus_master->data,
+			dev->pullup_duration);
+	}
+}
+
+/**
+ * Post-write operation, currently only supporting strong pullups.
+ * If a strong pullup was requested, clear it if the hardware supports
+ * them, or execute the delay otherwise, in either case clear the request.
+ *
+ * @param dev     the master device
+ */
+static void w1_post_write(struct w1_master *dev)
+{
+	if(dev->pullup_duration) {
+		if(dev->enable_pullup && dev->bus_master->set_pullup)
+			dev->bus_master->set_pullup(dev->bus_master->data, 0);
+		else
+			msleep(dev->pullup_duration);
+		dev->pullup_duration=0;
+	}
+}
+
+/**
  * Writes 8 bits.
  *
  * @param dev     the master device
@@ -102,11 +136,17 @@ void w1_write_8(struct w1_master *dev, u8 byte)
 {
 	int i;
 
-	if (dev->bus_master->write_byte)
+	if (dev->bus_master->write_byte) {
+		w1_pre_write(dev);
 		dev->bus_master->write_byte(dev->bus_master->data, byte);
+	}
 	else
-		for (i = 0; i < 8; ++i)
+		for (i = 0; i < 8; ++i) {
+			if(i==7)
+				w1_pre_write(dev);
 			w1_touch_bit(dev, (byte >> i) & 0x1);
+		}
+	w1_post_write(dev);
 }
 EXPORT_SYMBOL_GPL(w1_write_8);
 
@@ -203,11 +243,14 @@ void w1_write_block(struct w1_master *dev, const u8 *buf, int len)
 {
 	int i;
 
-	if (dev->bus_master->write_block)
+	if (dev->bus_master->write_block) {
+		w1_pre_write(dev);
 		dev->bus_master->write_block(dev->bus_master->data, buf, len);
+	}
 	else
 		for (i = 0; i < len; ++i)
-			w1_write_8(dev, buf[i]);
+			w1_write_8(dev, buf[i]); // calls w1_pre_write
+	w1_post_write(dev);
 }
 EXPORT_SYMBOL_GPL(w1_write_block);
 
@@ -306,3 +349,20 @@ int w1_reset_select_slave(struct w1_slave *sl)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(w1_reset_select_slave);
+
+/**
+ * Put out a strong pull-up of the specified duration after the next write
+ * operation.  Not all hardware supports strong pullups.  Hardware that
+ * doesn't support strong pullups will sleep for the given time after the
+ * write operation without a strong pullup.  This is a one shot request for
+ * the next write, specifying zero will clear a previous request.
+ * The w1 master lock must be held.
+ *
+ * @param delay	time in milliseconds
+ * @return	0=success, anything else=error
+ */
+void w1_next_pullup(struct w1_master *dev, int delay)
+{
+	dev->pullup_duration=delay;
+}
+EXPORT_SYMBOL_GPL(w1_next_pullup);
-- 
1.4.4.4

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

[PATCH 8/33 replaces 8/35 and 9/35] W1: feature, w1_therm.c use strong pullup and documentation

[David Fries]

[-- Attachment #1: Type: text/plain, Size: 5173 bytes --]

===================================================================
This is an update to the previous set of patches, don't merge them
yet.  Some of the context was messed up and I don't expect patch to
apply the updates with the previous set cleanly.  I'll resubmit the
entire set if these updates look good.
===================================================================

w1_therm.c 1.3 1.4 1.10 1.11
Use the strong pullup not the sleep.  Note that the pullup goes before
W1_CONVERT_TEMP not after.
+w1_next_pullup(dev, tm);
 w1_write_8(dev, W1_CONVERT_TEMP);
-msleep(tm);

Also added a module parameter strong_pullup to disable the strong
pullup and sleep if requested.

New directory for temperature sensor documentation.

Documentation/w1/00-INDEX 1.2
Added new subdirectory slaves

Documentation/w1/slaves/00-INDEX 1.1 1.2
Added index and w1_therm entry.

Documentation/w1/slaves/w1_therm 1.1 1.2 1.3
Initial entry for the temperature sensor.

Signed-off-by: David Fries <david@fries.net>
---
 Documentation/w1/00-INDEX        |    2 +
 Documentation/w1/slaves/00-INDEX |    4 +++
 Documentation/w1/slaves/w1_therm |   41 ++++++++++++++++++++++++++++++++++++++
 drivers/w1/slaves/w1_therm.c     |   15 ++++++++++++-
 4 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/Documentation/w1/00-INDEX b/Documentation/w1/00-INDEX
index 5270cf4..cb49802 100644
--- a/Documentation/w1/00-INDEX
+++ b/Documentation/w1/00-INDEX
@@ -1,5 +1,7 @@
 00-INDEX
 	- This file
+slaves/
+	- Drivers that provide support for specific family codes.
 masters/
 	- Individual chips providing 1-wire busses.
 w1.generic
diff --git a/Documentation/w1/slaves/00-INDEX b/Documentation/w1/slaves/00-INDEX
new file mode 100644
index 0000000..f8101d6
--- /dev/null
+++ b/Documentation/w1/slaves/00-INDEX
@@ -0,0 +1,4 @@
+00-INDEX
+	- This file
+w1_therm
+	- The Maxim/Dallas Semiconductor ds18*20 temperature sensor.
diff --git a/Documentation/w1/slaves/w1_therm b/Documentation/w1/slaves/w1_therm
new file mode 100644
index 0000000..0403aaa
--- /dev/null
+++ b/Documentation/w1/slaves/w1_therm
@@ -0,0 +1,41 @@
+Kernel driver w1_therm
+====================
+
+Supported chips:
+  * Maxim ds18*20 based temperature sensors.
+
+Author: Evgeniy Polyakov <johnpol@2ka.mipt.ru>
+
+
+Description
+-----------
+
+w1_therm provides basic temperature conversion for ds18*20 devices.
+supported family codes:
+W1_THERM_DS18S20	0x10
+W1_THERM_DS1822		0x22
+W1_THERM_DS18B20	0x28
+
+Support is provided through the sysfs w1_slave file.  Each open and
+read sequence will initiate a temperature conversion then provide two
+lines of ASCII output.  The first line contains the nine hex bytes
+read along with a calculated crc value and YES or NO if it matched.
+If the crc matched the returned values are retained.  The second line
+displays the retained values along with a temperature in millidegrees
+Centigrade after t=.
+
+Parasite powered devices are limited to one slave performing a
+temperature conversion at a time.  If none of the devices are parasite
+powered it would be possible to convert all the devices at the same
+time and then go back to read individual sensors.  That isn't
+currently supported.  The driver also doesn't support reduced
+precision (which would also reduce the conversion time).
+
+The module parameter strong_pullup can be set to 0 to disable the
+strong pullup or 1 to enable.  If enabled the 5V strong pullup will be
+enabled when the conversion is taking place provided the master driver
+must support the strong pullup (or it falls back to a pullup
+resistor).  The DS18b20 temperature sensor specification lists a
+maximum current draw of 1.5mA and that a 5k pullup resistor is not
+sufficient.  The strong pullup is designed to provide the additional
+current required.
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index fb28aca..f659380 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -37,6 +37,14 @@ MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Evgeniy Polyakov <johnpol@2ka.mipt.ru>");
 MODULE_DESCRIPTION("Driver for 1-wire Dallas network protocol, temperature family.");
 
+/* Allow the strong pullup to be disabled, but default to enabled.
+ * If it was disabled a parasite powered device might not get the require
+ * current to do a temperature conversion.  If it is enabled parasite powered
+ * devices have a better chance of getting the current required.
+ */
+static int w1_strong_pullup=1;
+module_param_named(strong_pullup, w1_strong_pullup, int, 0);
+
 static u8 bad_roms[][9] = {
 				{0xaa, 0x00, 0x4b, 0x46, 0xff, 0xff, 0x0c, 0x10, 0x87},
 				{}
@@ -192,9 +200,12 @@ static ssize_t w1_therm_read_bin(struct kobject *kobj,
 			int count = 0;
 			unsigned int tm = 750;
 
+			/* 750ms strong pullup (or delay) after the convert */
+			if(w1_strong_pullup)
+				w1_next_pullup(dev, tm);
 			w1_write_8(dev, W1_CONVERT_TEMP);
-
-			msleep(tm);
+			if(!w1_strong_pullup)
+				msleep(tm);
 
 			if (!w1_reset_select_slave(sl)) {
 
-- 
1.4.4.4

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

[PATCH 9/33 replaces 10/35] W1: be able to manually add and remove slaves

[David Fries]

[-- Attachment #1: Type: text/plain, Size: 6540 bytes --]

===================================================================
This is an update to the previous set of patches, don't merge them
yet.  Some of the context was messed up and I don't expect patch to
apply the updates with the previous set cleanly.  I'll resubmit the
entire set if these updates look good.
===================================================================

w1.c 1.11 1.12 1.14
I'm intending to be working with a static network and run without the
bus searching, but bus searching is currently the only way to add
slave devices.  This allows any slave to be added or removed.  There
is one file for adding and another for removing.  If the files are
read a short comment including the expected serial number format is
returned.

Signed-off-by: David Fries <david@fries.net>
---
 drivers/w1/w1.c |  140 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 137 insertions(+), 3 deletions(-)

diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 401c56a..4ed661c 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -56,6 +56,9 @@ module_param_named(slave_ttl, w1_max_slave_ttl, int, 0);
 DEFINE_MUTEX(w1_mlock);
 LIST_HEAD(w1_masters);
 
+static int w1_attach_slave_device(struct w1_master *dev, struct w1_reg_num *rn);
+void w1_slave_detach(struct w1_slave *sl);
+
 static int w1_master_match(struct device *dev, struct device_driver *drv)
 {
 	return 1;
@@ -349,7 +352,8 @@ static ssize_t w1_master_attribute_show_slave_count(struct device *dev, struct d
 	return count;
 }
 
-static ssize_t w1_master_attribute_show_slaves(struct device *dev, struct device_attribute *attr, char *buf)
+static ssize_t w1_master_attribute_show_slaves(struct device *dev,
+	struct device_attribute *attr, char *buf)
 {
 	struct w1_master *md = dev_to_w1_master(dev);
 	int c = PAGE_SIZE;
@@ -374,6 +378,134 @@ static ssize_t w1_master_attribute_show_slaves(struct device *dev, struct device
 	return PAGE_SIZE - c;
 }
 
+static ssize_t w1_master_attribute_show_add(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	int c = PAGE_SIZE;
+	c -= snprintf(buf+PAGE_SIZE - c, c,
+		"write device id xx-xxxxxxxxxxxx to add slave\n");
+	return PAGE_SIZE - c;
+}
+
+static int w1_atoreg_num(struct device * dev, const char *buf, size_t count,
+	struct w1_reg_num *rn)
+{
+	unsigned int family;
+	u64 id;
+	int i;
+	u64 rn64_le;
+	/* The CRC value isn't read from the user because the sysfs directory
+	 * doesn't include it and most messages from the bus search don't
+	 * print it either.  It would be unreasonable for the user to then
+	 * provide it.
+	 */
+	const char *error_msg="bad slave string format, expecting "
+		"ff-dddddddddddd\n";
+
+	if(buf[2]!='-') {
+		dev_err(dev, "%s", error_msg);
+		return -EINVAL;
+	}
+	i=sscanf(buf, "%02x-%012llx", &family, &id);
+	if(i!=2) {
+		dev_err(dev, "%s", error_msg);
+		return -EINVAL;
+	}
+	rn->family=family;
+	rn->id=id;
+
+	rn64_le=cpu_to_le64(*(u64*)rn);
+	rn->crc=w1_calc_crc8((u8 *)&rn64_le, 7);
+
+	#if 0
+	dev_info(dev, "With CRC device is %02x.%012llx.%02x.\n",
+		  rn->family, (unsigned long long)rn->id, rn->crc);
+	#endif
+
+	return 0;
+}
+
+/* Searches the slaves in the w1_master and returns a pointer or NULL.
+ * Note: must hold the mutex
+ */
+static struct w1_slave *w1_slave_search_device(struct w1_master *dev,
+	struct w1_reg_num *rn)
+{
+	struct w1_slave *sl;
+	list_for_each_entry(sl, &dev->slist, w1_slave_entry) {
+		if (sl->reg_num.family == rn->family &&
+				sl->reg_num.id == rn->id &&
+				sl->reg_num.crc == rn->crc) {
+			return sl;
+		}
+	}
+	return NULL;
+}
+
+static ssize_t w1_master_attribute_store_add(struct device * dev,
+						struct device_attribute *attr,
+						const char * buf, size_t count)
+{
+	struct w1_master *md = dev_to_w1_master(dev);
+	struct w1_reg_num rn;
+	struct w1_slave *sl;
+	ssize_t result=count;
+
+	if(w1_atoreg_num(dev, buf, count, &rn))
+		return -EINVAL;
+
+	mutex_lock(&md->mutex);
+	sl=w1_slave_search_device(md, &rn);
+	/* It would be nice to do a targeted search one the one-wire bus
+	 * for the new device to see if it is out there or not.  But the
+	 * current search doesn't support that.
+	 */
+	if(sl) {
+		dev_info(dev, "Device %s already exists\n", sl->name);
+		result=-EINVAL;
+	} else {
+		w1_attach_slave_device(md, &rn);
+	}
+	mutex_unlock(&md->mutex);
+
+	return result;
+}
+
+static ssize_t w1_master_attribute_show_remove(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	int c = PAGE_SIZE;
+	c -= snprintf(buf+PAGE_SIZE - c, c,
+		"write device id xx-xxxxxxxxxxxx to remove slave\n");
+	return PAGE_SIZE - c;
+}
+
+static ssize_t w1_master_attribute_store_remove(struct device * dev,
+						struct device_attribute *attr,
+						const char * buf, size_t count)
+{
+	struct w1_master *md = dev_to_w1_master(dev);
+	struct w1_reg_num rn;
+	struct w1_slave *sl;
+	ssize_t result=count;
+
+	if(w1_atoreg_num(dev, buf, count, &rn))
+		return -EINVAL;
+
+	mutex_lock(&md->mutex);
+	sl=w1_slave_search_device(md, &rn);
+	if(sl) {
+		w1_slave_detach(sl);
+	} else {
+		dev_info(dev, "Device %02x-%012llx doesn't exists\n", rn.family,
+			(unsigned long long)rn.id);
+		result=-EINVAL;
+	}
+	mutex_unlock(&md->mutex);
+
+	return result;
+}
+
 #define W1_MASTER_ATTR_RO(_name, _mode)				\
 	struct device_attribute w1_master_attribute_##_name =	\
 		__ATTR(w1_master_##_name, _mode,		\
@@ -394,6 +526,8 @@ static W1_MASTER_ATTR_RO(timeout, S_IRUGO);
 static W1_MASTER_ATTR_RO(pointer, S_IRUGO);
 static W1_MASTER_ATTR_RW(search, S_IRUGO | S_IWUGO);
 static W1_MASTER_ATTR_RW(pullup, S_IRUGO | S_IWUGO);
+static W1_MASTER_ATTR_RW(add, S_IRUGO | S_IWUGO);
+static W1_MASTER_ATTR_RW(remove, S_IRUGO | S_IWUGO);
 
 static struct attribute *w1_master_default_attrs[] = {
 	&w1_master_attribute_name.attr,
@@ -405,6 +539,8 @@ static struct attribute *w1_master_default_attrs[] = {
 	&w1_master_attribute_pointer.attr,
 	&w1_master_attribute_search.attr,
 	&w1_master_attribute_pullup.attr,
+	&w1_master_attribute_add.attr,
+	&w1_master_attribute_remove.attr,
 	NULL
 };
 
@@ -700,9 +836,7 @@ void w1_reconnect_slaves(struct w1_family *f, int attach)
 
 static void w1_slave_found(struct w1_master *dev, u64 rn)
 {
-	int slave_count;
 	struct w1_slave *sl;
-	struct list_head *ent;
 	struct w1_reg_num *tmp;
 	u64 rn_le = cpu_to_le64(rn);
 
-- 
1.4.4.4

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

[PATCH 11/33 replaces 12/35] W1: new module parameter search_count

[David Fries]

[-- Attachment #1: Type: text/plain, Size: 1539 bytes --]

===================================================================
This is an update to the previous set of patches, don't merge them
yet.  Some of the context was messed up and I don't expect patch to
apply the updates with the previous set cleanly.  I'll resubmit the
entire set if these updates look good.
===================================================================

w1_int.c 1.6 1.10
Added a new module parameter search_count which allows overriding the
default search count.  -1 continual, 0 disabled, N that many times.

Signed-off-by: David Fries <david@fries.net>
---
 drivers/w1/w1_int.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/w1/w1_int.c b/drivers/w1/w1_int.c
index 8deb76c..47e27f5 100644
--- a/drivers/w1/w1_int.c
+++ b/drivers/w1/w1_int.c
@@ -30,6 +30,8 @@
 #include "w1_int.h"
 
 static u32 w1_ids = 1;
+static int w1_search_count = -1; /* Default is continual scan */
+module_param_named(search_count, w1_search_count, int, 0);
 
 static int w1_enable_pullup = 1;
 module_param_named(enable_pullup, w1_enable_pullup, int, 0);
@@ -62,8 +64,8 @@ static struct w1_master * w1_alloc_dev(u32 id, int slave_count, int slave_ttl,
 	dev->initialized	= 0;
 	dev->id			= id;
 	dev->slave_ttl		= slave_ttl;
+	dev->search_count	= w1_search_count;
 	dev->enable_pullup	= w1_enable_pullup;
-        dev->search_count	= -1; /* continual scan */
 
 	/* 1 for w1_process to decrement
 	 * 1 for __w1_remove_master_device to decrement
-- 
1.4.4.4

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

[PATCH 12/33 replaces 13/35] W1: Document add, remove, search_count, and pullup.

[David Fries]

[-- Attachment #1: Type: text/plain, Size: 2445 bytes --]

===================================================================
This is an update to the previous set of patches, don't merge them
yet.  Some of the context was messed up and I don't expect patch to
apply the updates with the previous set cleanly.  I'll resubmit the
entire set if these updates look good.
===================================================================

Documentation/w1/w1.generic 1.2 1.3
Document w1_master_add, w1_master_remove, search_count, and pullup.

Signed-off-by: David Fries <david@fries.net>
---
 Documentation/w1/w1.generic |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/Documentation/w1/w1.generic b/Documentation/w1/w1.generic
index 4c6509d..e3333ee 100644
--- a/Documentation/w1/w1.generic
+++ b/Documentation/w1/w1.generic
@@ -79,10 +79,13 @@ w1 master sysfs interface
 <xx-xxxxxxxxxxxxx> - a directory for a found device. The format is family-serial
 bus                - (standard) symlink to the w1 bus
 driver             - (standard) symlink to the w1 driver
+w1_master_add      - Manually register a slave device
 w1_master_attempts - the number of times a search was attempted
 w1_master_max_slave_count
                    - the maximum slaves that may be attached to a master
 w1_master_name     - the name of the device (w1_bus_masterX)
+w1_master_pullup   - 5V strong pullup 0 enabled, 1 disabled
+w1_master_remove   - Manually remove a slave device
 w1_master_search   - the number of searches left to do, -1=continual (default)
 w1_master_slave_count
                    - the number of slaves found
@@ -90,7 +93,13 @@ w1_master_slaves   - the names of the slaves, one per line
 w1_master_timeout  - the delay in seconds between searches
 
 If you have a w1 bus that never changes (you don't add or remove devices),
-you can set w1_master_search to a positive value to disable searches.
+you can set the module parameter search_count to a small positive number
+for an initially small number of bus searches.  Alternatively it could be
+set to zero, then manually add the slave device serial numbers by
+w1_master_add device file.  The w1_master_add and w1_master_remove files
+generally only make sense when searching is disabled, as a search will
+redetect manually removed devices that are present and timeout manually
+added devices that aren't on the bus.
 
 
 w1 slave sysfs interface
-- 
1.4.4.4

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

[PATCH 13/33 replaces 14/35 and 15/35] W1: w1_slave_read_id multiple short read bug

[David Fries]

[-- Attachment #1: Type: text/plain, Size: 3501 bytes --]

===================================================================
This is an update to the previous set of patches, don't merge them
yet.  Some of the context was messed up and I don't expect patch to
apply the updates with the previous set cleanly.  I'll resubmit the
entire set if these updates look good.
===================================================================

This is more to point out the bug in w1_slave_read_id, the next patch
rewrites the routine.

w1.c 1.15
Reading at an offset other than zero, ie reading less than 8 bytes at
a time would result in reading the first bytes over and over until 8
bytes were returned.  Added the offset to the buffer.
- memcpy(buf, (u8 *)&sl->reg_num, count);
+ memcpy(buf, (u8 *)&sl->reg_num+off, count);
But there is a better way...

w1.c 1.16
Switching w1_slave_read_id from being a bin_attribute to a
device_attribute.  It only has to return 8 bytes per read and lets
kobject handle the buffering.  That simplifies the logic in
w1_slave_read_id.

Signed-off-by: David Fries <david@fries.net>
---
 drivers/w1/w1.c |   35 ++++++++++-------------------------
 1 files changed, 10 insertions(+), 25 deletions(-)

diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 5053bc8..c008493 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -104,35 +104,20 @@ static ssize_t w1_slave_read_name(struct device *dev, struct device_attribute *a
 	return sprintf(buf, "%s\n", sl->name);
 }
 
-static ssize_t w1_slave_read_id(struct kobject *kobj,
-				struct bin_attribute *bin_attr,
-				char *buf, loff_t off, size_t count)
+static ssize_t w1_slave_read_id(struct device *dev,
+	struct device_attribute *attr, char *buf)
 {
-	struct w1_slave *sl = kobj_to_w1_slave(kobj);
-
-	if (off > 8) {
-		count = 0;
-	} else {
-		if (off + count > 8)
-			count = 8 - off;
-
-		memcpy(buf, (u8 *)&sl->reg_num, count);
-	}
+	struct w1_slave *sl = dev_to_w1_slave(dev);
+	ssize_t count=sizeof(sl->reg_num);
 
+	memcpy(buf, (u8 *)&sl->reg_num, count);
 	return count;
 }
 
 static struct device_attribute w1_slave_attr_name =
 	__ATTR(name, S_IRUGO, w1_slave_read_name, NULL);
-
-static struct bin_attribute w1_slave_attr_bin_id = {
-      .attr = {
-              .name = "id",
-              .mode = S_IRUGO,
-      },
-      .size = 8,
-      .read = w1_slave_read_id,
-};
+static struct device_attribute w1_slave_attr_id =
+	__ATTR(id, S_IRUGO, w1_slave_read_id, NULL);
 
 /* Default family */
 
@@ -642,7 +627,7 @@ static int __w1_attach_slave_device(struct w1_slave *sl)
 	}
 
 	/* Create "id" entry */
-	err = sysfs_create_bin_file(&sl->dev.kobj, &w1_slave_attr_bin_id);
+	err = device_create_file(&sl->dev, &w1_slave_attr_id);
 	if (err < 0) {
 		dev_err(&sl->dev,
 			"sysfs file creation for [%s] failed. err=%d\n",
@@ -664,7 +649,7 @@ static int __w1_attach_slave_device(struct w1_slave *sl)
 	return 0;
 
 out_rem2:
-	sysfs_remove_bin_file(&sl->dev.kobj, &w1_slave_attr_bin_id);
+	device_remove_file(&sl->dev, &w1_slave_attr_id);
 out_rem1:
 	device_remove_file(&sl->dev, &w1_slave_attr_name);
 out_unreg:
@@ -746,7 +731,7 @@ void w1_slave_detach(struct w1_slave *sl)
 	msg.type = W1_SLAVE_REMOVE;
 	w1_netlink_send(sl->master, &msg);
 
-	sysfs_remove_bin_file(&sl->dev.kobj, &w1_slave_attr_bin_id);
+	device_remove_file(&sl->dev, &w1_slave_attr_id);
 	device_remove_file(&sl->dev, &w1_slave_attr_name);
 	device_unregister(&sl->dev);
 
-- 
1.4.4.4

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

[PATCH 32/33 replaces 34/35] Documentation/w1/masters/ds2490 update

[David Fries]

[-- Attachment #1: Type: text/plain, Size: 4959 bytes --]

===================================================================
This is an update to the previous set of patches, don't merge them
yet.  Some of the context was messed up and I don't expect patch to
apply the updates with the previous set cleanly.  I'll resubmit the
entire set if these updates look good.
===================================================================

Documentation/w1/masters/ds2490 1.1 through 1.4
Provide some additional details about the status of the driver.

Signed-off-by: David Fries <david@fries.net>
---
 Documentation/w1/masters/ds2490 |   62 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 62 insertions(+), 0 deletions(-)

diff --git a/Documentation/w1/masters/ds2490 b/Documentation/w1/masters/ds2490
index 239f9ae..f8648d2 100644
--- a/Documentation/w1/masters/ds2490
+++ b/Documentation/w1/masters/ds2490
@@ -16,3 +16,65 @@ which allows to build USB <-> W1 bridges.
 DS9490(R) is a USB <-> W1 bus master device
 which has 0x81 family ID integrated chip and DS2490
 low-level operational chip.
+
+Notes and limitations.
+- The weak pullup current is a minimum of 0.9mA and maximum of 6.0mA.
+- The 5V strong pullup is supported with a minimum of 5.9mA and a
+  maximum of 30.4 mA.  (From DS2490.pdf)
+- While the ds2490 supports a hardware search the code doesn't take
+  advantage of it.
+- The hardware will detect when devices are attached to the bus on the
+  next bus (reset?) operation, however only a message is printed as
+  the core w1 code doesn't make use of the information.  Connecting
+  one device tends to give multiple new device notifications.
+- The number of USB bus transactions could be reduced if w1_reset_send
+  was added to the API.  The name is just a suggestion.  It would take
+  a write buffer and a read buffer (along with sizes) as arguments.
+  It would add match rom and rom to the send buffer, reset the bus,
+  and read the result.  The ds2490 block I/O command supports reset,
+  write, read, and strong pullup all in one command.  That would
+  reduce the time and overhead for a set of commands.
+
+  The core w1 functions required to do a temperature conversion are,
+  w1_reset_select_slave, w1_next_pullup, and w1_write_8, which turn
+  around and execute the master functions, reset_bus, write_block,
+  set_pullup, write_byte, set_pullup.  Each ds2490 function will have
+  multiple USB bus transactions, except set_pullup where it only
+  requires transactions if the if the time value is different.  The
+  conversion could be reduced to w1_next_pullup, w1_reset_send which
+  would call set_pullup, reset_send. The reset_send would enable reset
+  and strong pullup (if enabled), write the data buffer, execute the
+  block I/O command, read status, and get data.  Currently I count 11
+  transactions required, this would reduce it to 4.
+- The hardware supports normal, flexable, and overdrive bus
+  communication speeds, but only the normal is supported.
+- The registered w1_bus_master functions don't define error
+  conditions.  If a bus search is in progress and the ds2490 is
+  removed it can produce a good amount of error output before the bus
+  search finishes.
+- The hardware supports detecting some error conditions, such as
+  short, alarming presence on reset, and no presence on reset, but the
+  driver doesn't query those values.
+- The ds2490 specification doesn't cover short bulk in reads in
+  detail, but my observation is if fewer bytes are requested than are
+  available, the bulk read will return an error and the hardware will
+  clear the entire bulk in buffer.  It would be possible to read the
+  maximum buffer size to not run into this error condition, only extra
+  bytes in the buffer is a logic error in the driver.  The code should
+  should match reads and writes as well as data sizes.  Reads and
+  writes are serialized and the status verifies that the chip is idle
+  (and data is available) before the read is executed, so it should
+  not happen.
+- Running x86_64 2.6.24 UHCI under qemu 0.9.0 under x86_64 2.6.22-rc6
+  with a OHCI controller, ds2490 running in the guest would operate
+  normally the first time the module was loaded after qemu attached
+  the ds2490 hardware, but if the module was unloaded, then reloaded
+  most of the time one of the bulk out or in, and usually the bulk in
+  would fail.  qemu sets a 50ms timeout and the bulk in would timeout
+  even when the status shows data available.  A bulk out write would
+  show a successful completion, but the ds2490 status register would
+  show 0 bytes written.  Detaching qemu from the ds2490 hardware and
+  reattaching would clear the problem.  usbmon output in the guest and
+  host did not explain the problem.  My guess is a bug in either qemu
+  or the host OS and more likely the host OS.
+-- 03-06-2008 David Fries <David@Fries.net>
-- 
1.4.4.4

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 13/33 replaces 14/35 and 15/35] W1: w1_slave_read_id multiple short read bug

[Evgeniy Polyakov]

Hi David.

On Sun, Apr 13, 2008 at 06:15:37PM -0500, David Fries (david@fries.net) wrote:
> This is more to point out the bug in w1_slave_read_id, the next patch
> rewrites the routine.
> 
> w1.c 1.15
... 
> w1.c 1.16


That is what I talked about. I agree with your analysis of the problem
and I agree with fixes. But I do not agree with how you created them:
why do we ever neen 1.15 version, since immediately after that we have
1.16 which completely rewrite what was done in previous one?
This applies to the whole patchset: there are essentially several bug
fixes and several features, so separate them into several patches, which
do not collide with each other in the above way.

-- 
	Evgeniy Polyakov

Re: [PATCH 0/35] W1: w1 core deadlock and oops fixes, ds2490 updates

[Evgeniy Polyakov]

Hi.

On Sun, Apr 13, 2008 at 05:56:32PM -0500, David Fries (david@fries.net) wrote:
> I dug out a panic before I started modifying the driver, see the end
> of the message.  There is still a remaining oops.  Register the same
> sensor with two masters.  The serial id of the sensor is assumed to be
> unique and the sysfs code will cause an oops when the second sensor is
> registered.  If you have two master devices that are both doing bus
> searching, let one detect the sensor, the move it to the other master.
> The second master will detect the device before the first one times it
> out.  None of my patches address this.

This is first time you show me the oops, so I we can start thinking
about bug itself. Getting that you already have a fix and changes which
I agree with, we can proceed with them. The only stuff I complain about
it how you organized your patchset. There is no need to put multiple
revisions of the same file in several patches when each new change just
overwrite previous one, this complicates code review and digging into
changeset changelogs in case of problems.

> Could you be more specific on "really wrong, like what you call a
> deadlock was intended"?  I only use 'intended' once, and that is is
> patch 1 in "The slave reconnect was incorrectly coded."  That
> paragraph is describing a infinite loop inserting a slave driver, not
> unloading a device driver.

I had a gpio driver which worked really bad, since current source was
very weak and shared with different resource. It was not pure luck to
get data from it, but it did not work reliably. So there are all those
loops you see in reading path, having them in userspace would provide a
great feeling of really broken hardware (like that broked rom data you
see blacklisted in the driver), while it just did not worked 100%
reliably. Because of that I did not allow to forcedly stop
'transaction', since getting it right took lots of time, so we do want
that data, so no interrupt, so removing module stops...
Yes, this is really ugly, but it works and was added intentionally.
Now I think that for better quality and simplicity of the code we can
drop this behaviour.

> If something is really wrong with the descriptions I'll fix it.
> 
> > There is another problem with your submission - please split it into 3-5
> > patches which are logically compound, so that there would not be things
> > like: patch1: replace A with B, patch2: remove B and the like.
> 
> You must be talking about patch 14 and patch 15 w1_slave_read_id sort
> read bug.  That's the only place it does that.
> 
> I debated on how to submit
> patch 14 "w1_slave_read_id multiple short read bug"
> and
> patch 15 "w1_slave_read_id from bin_attribute to device_attribute"
> and I did it that way to first show what he bug was and fix it, then
> show how simple the code was using a different method.  I'll submit it
> in one patch.
> 
> > You also have lots of codying style issues in the patchset.
> Such as?

if( without space
a=b without spaces and the like. Pretty trivial, but I will be frowned
upon if accept that :)

> This was the overview e-mail, and it was long enough without going
> into details on each of the issues.  But here they are.

I agree with what you pointed is not correct, including 'deadlock', but
I already described why I did it that way. It does work and is ugly and
is probably wrong, so I agreed to remove this 'feature'.

The same applies to other bug fixes you provided: I agreew ith them
(even though some of them I see first time)

> > Please reorganize your patches into something like this:
> > 1. threads changes
> > 2. master device removal changes (remove slaves which are on the bus
> > master being removed) and related things
> > 3. ds2490 changes
> > 4. cleanups
> > 
> > or something like that, reviewing 35 small patches which are removing
> > things past another is not a really good time...
> 
> You want me to take 35 patches and merge them down to 4?  There were a
> lot of issues in here, and I don't have an automagic patch reorder
> tool.  If you want to review fewer patches apply 1-6, do a diff and
> look at that.
> 
> Documentation/SubmittingPatches '3) Separate your changes.' It goes on
> to specifically separate out bug fixes and performance enhancements.

Your changes overwrite each other, and some times adds something or
remove something to accomplish requested goal. I specially pointed to
one of them in your last patchset, which you marked rfc and not for
merge. Change separation does not mean all commits should be sent as
independent patch, logical changes should be combined. When patch 2
overwrite patch1 and does something similar, combine them.
Bug fixes to bug fixes, features to features. If some overwrite fix a
bug, send that overwrite.

> You requested in another e-mail to merge patch 1-6.  The deadlocks in
> 1 is clearly a bug.  6 block (instead of sleeping with a timeout) is a
> performance enhancement.  5 might be a deadlock, but it's not at all
> related to 1.  2 and 3 have nothing to do with any of the above.
> Logically this come out to be a bunch of patches.

I roughly pointed how to organize them. My main intention was to
logically separate changes, not sending every single commit in different
patch.

> On Mon, Jan 21, 2008 at 06:56:35PM -0800, Andrew Morton wrote:
> > On Mon, 21 Jan 2008 20:47:19 -0600 David Fries <david@fries.net> wrote:
> > > Thanks.  Question, at what point should I just submit these one liner,
> > > but independent bugs in one patch?
> > 
> > You shouldn't, unless they are dependent in both directions.
> 
> If Andrew Morton is going to say no for independent one line bugs, I'm
> going to say No (for a bunch of complicated sometimes overlapping
> bugs, features, and performance enhancements) as well.  Besides I have
> something like 27KB in 664 lines of descriptions, if I tried to merge
> the changes down there's no way I could figure out which patch lines
> went with what descriptions, and no one else could either.

I think I have to note that again: if some feature overwrite fix the
bug, then just send that overwrite, since it was original goal. Instead
you send number of one-line fixes, and then overwrite them. Less changes
we have (presumbly they are logically separated and do not break each
others), less time we will spend reviewing code, searching for wrong
commit, checking for bugs.

I understand that you do not want to do that just because you already
have cvs sommit log which you send to me, but this also means that I
will need to do that for your changes and then resubmit.

> w1_master_driver w1_bus_master1: Family 21 for 21.34c0000039c5.ba is not registered.
> Failed to read 1-wire data from 0x02: err=-110.
> Failed to read 1-wire data from 0x02: err=-110.
> usb 2-3: USB disconnect, address 5
> w1_master_driver w1_bus_master1: Waiting for w1_bus_master1 to become free: refcnt=1.
> w1_slave_release: Releasing 81-00000020bbd0.
> w1_slave_release: Releasing 21-34c0000039c5.
> w1_slave_release: Releasing 28-0000000e84a2.
> w1_slave_release: Releasing 81-00000020bbd0.

> usb 2-3: new full speed USB device using ohci_hcd and address 8
> usb 2-3: device descriptor read/64, error -62
> hub 2-0:1.0: Cannot enable port 3.  Maybe the USB cable is bad?
> hub 2-0:1.0: Cannot enable port 3.  Maybe the USB cable is bad?
> hub 2-0:1.0: Cannot enable port 3.  Maybe the USB cable is bad?
> hub 2-0:1.0: Cannot enable port 3.  Maybe the USB cable is bad?
> usb 2-3: new full speed USB device using ohci_hcd and address 12
> usb 2-3: configuration #1 chosen from 1 choice
> usb 2-3: USB disconnect, address 12
> BUG: unable to handle kernel NULL pointer dereference at virtual address 0000000c

Does this happen only when you remove and plug ds9490 back to the
socket? We could fix it rather quickly, but I'm glad you fixed it
yourself :)




-- 
	Evgeniy Polyakov

Re: [PATCH 13/33 replaces 14/35 and 15/35] W1: w1_slave_read_id multiple short read bug

[David Fries]

On Mon, Apr 14, 2008 at 03:40:55PM +0400, Evgeniy Polyakov wrote:
> Hi David.
> 
> On Sun, Apr 13, 2008 at 06:15:37PM -0500, David Fries (david@fries.net) wrote:
> > This is more to point out the bug in w1_slave_read_id, the next patch
> > rewrites the routine.
> > 
> > w1.c 1.15
> ... 
> > w1.c 1.16
> 
> 
> That is what I talked about. I agree with your analysis of the problem
> and I agree with fixes. But I do not agree with how you created them:
> why do we ever neen 1.15 version, since immediately after that we have
> 1.16 which completely rewrite what was done in previous one?
> This applies to the whole patchset: there are essentially several bug
> fixes and several features, so separate them into several patches, which
> do not collide with each other in the above way.

The old patch 14/35 and 15/35 was the only place I know of where I
fixed the bug, then replaced it wholesale with something better.
Several bugs, a few features, lots of patches, go figure.

-- 
David Fries <david@fries.net>
http://fries.net/~david/ (PGP encryption key available)