joshua stein writes:

> On Thu, 05 Aug 2021 at 12:41:56 -0600, Aaron Bieber wrote:
>> 
>> Martin Pieuchot writes:
>> 
>> > On 05/08/21(Thu) 09:41, Aaron Bieber wrote:
>> >> 
>> >> Stuart Henderson writes:
>> >> 
>> >> > On 2021/07/13 01:29, Anindya Mukherjee wrote:
>> >> >> I have a Cyber Power CP1500PFCLCDa UPS and get exactly the same crash 
>> >> >> on the
>> >> >> latest snapshot if the USB cable is unplugged. My dmesg is very 
>> >> >> similar so I've
>> >> >> omitted it, but I'd also be happy to help debug this issue.
>> >> >> 
>> >> >> Regards,
>> >> >> Anindya
>> >> >> 
>> >> >
>> >> > First try backing out the "Allow uhidev child devices to claim selective
>> >> > report ids" commit from March.
>> >> 
>> >> Here is a diff that reverts:
>> >> 
>> >> 2021-03-08 6545f693 jcs       Add another Type Cover device
>> >> 2021-03-08 1f85050a jcs       regen
>> >> 2021-03-08 fc9d2605 jcs       Add Surface Pro Type Cover
>> >> 2021-03-08 f31b43ce jcs       Allow uhidev child devices to claim 
>> >> selective report ids
>> >> 
>> >> With them reverted, my UPS(s) no longer trigger panics on disconnect.
>> >
>> > The bugs lies in upd_match() and has been introduced with the change
>> > containing UHIDEV_CLAIM_MULTIPLE_REPORTID.
>> >
>> > upd(4) owns all the reportID of a USB device, so a "uha.claimed" logic
>> > similar to what has been written for umt_match() is missing.
>> >
>> 
>> The following diff fixes it for me:
>> 
>> diff 2f84ac1381d01579f2b25c83b3dec5f908503765 /usr/src
>> blob - ad65b77718b50851c1537a7048eb7b54aba91203
>> file + sys/dev/usb/upd.c
>> --- sys/dev/usb/upd.c
>> +++ sys/dev/usb/upd.c
>> @@ -167,6 +167,7 @@ upd_match(struct device *parent, void *match, void *au
>>              if (upd_lookup_usage_entry(desc, size,
>>                  upd_usage_roots + i, &item)) {
>>                      ret = UMATCH_VENDOR_PRODUCT;
>> +                    uha->claimed[item.report_ID] = 1;
>>                      break;
>>              }
>
> I think it needs to loop through all of the report IDs like it does 
> in upd_attach_sensor_tree to mark them claimed, so it should not 
> break after the first result.

Ah, ty! Version with break removed:

diff --git a/sys/dev/usb/upd.c b/sys/dev/usb/upd.c
index ad65b77718b..cfffab765a3 100644
--- a/sys/dev/usb/upd.c
+++ b/sys/dev/usb/upd.c
@@ -167,7 +167,7 @@ upd_match(struct device *parent, void *match, void *aux)
 		if (upd_lookup_usage_entry(desc, size,
 		    upd_usage_roots + i, &item)) {
 			ret = UMATCH_VENDOR_PRODUCT;
-			break;
+			uha->claimed[item.report_ID] = 1;
 		}
 
 	return (ret);

Reply via email to