Skip to contentSkip to conversation

usb hid quirks for Masterkit MA901 usb radio

PATCH In review Mauro Carvalho Chehab 4 participants 13 messages Dec 28, 2012 – Mar 29, 2013 1 message not in the archive

Subsystems (2)
Media (V4L/DVB) HID
lore
Share & tools
lore

Mauro Carvalho Chehab

author
#0 lore raw

PATCH v2 2/3 usb hid quirks for Masterkit MA901 usb radio

Hi Jiri,

There's another radio device that it is incorrectly detected as an HID driver. As I'll be applying the driver's patch via the media tree, do you mind if I also apply this hid patch there?

Thanks!
Mauro

Forwarded message:

Date: Mon, 12 Nov 2012 07:57:03 +0100
From: Alexey Klimov <[email protected]>
To: [email protected]
Subject: [patch 02/03 v2] usb hid quirks for Masterkit MA901 usb radio

Don't let Masterkit MA901 USB radio be handled by usb hid drivers.

This device will be handled by radio-ma901.c driver.

Signed-off-by Alexey Klimov <[email protected]>
drivers/hid/hid-core.c +1 -0
index 5de3bb3..8e06569 100644
@@ -2025,6 +2025,7 @@ static const struct hid_device_id hid_ignore_list[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LD, USB_DEVICE_ID_LD_HYBRID) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LD, USB_DEVICE_ID_LD_HEATCONTROL) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_MADCATZ, USB_DEVICE_ID_MADCATZ_BEATPAD) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_MASTERKIT, USB_DEVICE_ID_MASTERKIT_MA901RADIO) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_MCC, USB_DEVICE_ID_MCC_PMD1024LS) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_MCC, USB_DEVICE_ID_MCC_PMD1208LS) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_PICKIT1) },
drivers/hid/hid-ids.h +3 -0
index 1dcb76f..17aa4f6 100644
@@ -533,6 +533,9 @@
 #define USB_VENDOR_ID_MADCATZ		0x0738
 #define USB_DEVICE_ID_MADCATZ_BEATPAD	0x4540
 
+#define USB_VENDOR_ID_MASTERKIT			0x16c0
+#define USB_DEVICE_ID_MASTERKIT_MA901RADIO	0x05df
+
 #define USB_VENDOR_ID_MCC		0x09db
 #define USB_DEVICE_ID_MCC_PMD1024LS	0x0076
 #define USB_DEVICE_ID_MCC_PMD1208LS	0x007a
Acked-by on v2 2/3
#1 lore raw
Quoted 5 lines · Mauro Carvalho Chehab

Hi Jiri,

There's another radio device that it is incorrectly detected as an HID driver. As I'll be applying the driver's patch via the media tree, do you mind if I also apply this hid patch there?

Hi Mauro,

please feel free to add

Acked-by Jiri Kosina <[email protected]>

and take the patch through your tree.

Thanks,

Signature · 2 lines
Jiri Kosina
SUSE Labs
author
#2 lore raw

Em Fri, 28 Dec 2012 14:27:56 +0100 (CET)
Jiri Kosina <[email protected]> escreveu:

Quoted 8 lines · Mauro Carvalho Chehab
Quoted 6 lines

Hi Jiri,

There's another radio device that it is incorrectly detected as an HID driver. As I'll be applying the driver's patch via the media tree, do you mind if I also apply this hid patch there?

Quoted 7 lines

Hi Mauro,

please feel free to add

Acked-by Jiri Kosina <[email protected]>

and take the patch through your tree.

Thank you, Jiri!

Regards,
Mauro

#3 lore raw
In reply to Mauro Carvalho Chehab

Hi Jiri and Mauro, all,

Quoted 41 lines · Mauro Carvalho Chehab

Hi Jiri,

There's another radio device that it is incorrectly detected as an HID driver. As I'll be applying the driver's patch via the media tree, do you mind if I also apply this hid patch there?

Thanks!
Mauro

Forwarded message:

Date: Mon, 12 Nov 2012 07:57:03 +0100
From: Alexey Klimov <[email protected]>
To: [email protected]
Subject: [patch 02/03 v2] usb hid quirks for Masterkit MA901 usb radio

Don't let Masterkit MA901 USB radio be handled by usb hid drivers.

This device will be handled by radio-ma901.c driver.

Signed-off-by Alexey Klimov <[email protected]>
drivers/hid/hid-core.c +1 -0
index 5de3bb3..8e06569 100644
@@ -2025,6 +2025,7 @@ static const struct hid_device_id hid_ignore_list[] = {
        { HID_USB_DEVICE(USB_VENDOR_ID_LD, USB_DEVICE_ID_LD_HYBRID) },
        { HID_USB_DEVICE(USB_VENDOR_ID_LD, USB_DEVICE_ID_LD_HEATCONTROL) },
        { HID_USB_DEVICE(USB_VENDOR_ID_MADCATZ, USB_DEVICE_ID_MADCATZ_BEATPAD) },
+       { HID_USB_DEVICE(USB_VENDOR_ID_MASTERKIT, USB_DEVICE_ID_MASTERKIT_MA901RADIO) },
        { HID_USB_DEVICE(USB_VENDOR_ID_MCC, USB_DEVICE_ID_MCC_PMD1024LS) },
        { HID_USB_DEVICE(USB_VENDOR_ID_MCC, USB_DEVICE_ID_MCC_PMD1208LS) },
        { HID_USB_DEVICE(USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_PICKIT1) },
drivers/hid/hid-ids.h +0 -0
index 1dcb76f..17aa4f6 100644

drivers/hid/hid-ids.h line 541

Quoted 9 lines
drivers/hid/hid-ids.h +3 -0
@@ -533,6 +533,9 @@
 #define USB_VENDOR_ID_MADCATZ          0x0738
 #define USB_DEVICE_ID_MADCATZ_BEATPAD  0x4540
 
+#define USB_VENDOR_ID_MASTERKIT                        0x16c0
+#define USB_DEVICE_ID_MASTERKIT_MA901RADIO     0x05df
+
 #define USB_VENDOR_ID_MCC              0x09db
 #define USB_DEVICE_ID_MCC_PMD1024LS    0x0076
 #define USB_DEVICE_ID_MCC_PMD1208LS    0x007a

Well, since patch also was pushed to stable trees like 3.5, 3.8, 3.2 and this fact made me look a little closer to usb ids. Actually, i googled these usb ids: 16c0 05df, link:
http://www.google.com/search?q=16c0+05df
and here comes some doubts.

For my eyes it looks like this usb radio consists of two chips: atmel tiny85 + actually fm tuner KT0830EG. It looks like tiny85 is used in many devices with the same usb ids like in our patch and people works with tiny85 using some software under linux. I don't know if linux software using hiddev/hidraw devices but this patch doesn't allow appearing of /dev/hiddev or /dev/hidraw files for any usb device with ids 0x16c0 0x05df, right? Is there any chance that using such patch we can break some linux software that uses /dev/hid* files and related functionality to communicate with tiny85?

Please note that i'm not expert in tiny85 chip and i don't have any deep knowledges on how usb ids are allocated for every device in the world.
Masterkit company changed (or was able to change?) only Manufacturer, Product, Serial fields in ma901 usb radio. I attached lsusb output in the end of letter. Bad thing here is that Masterkit has other usb not radio devices with the same usb ids based on tiny85 on the market.

Sorry if i'm over-alarmed, i just really dont want to break any userspace programs by this patch.

If everything above is correct then i can use some dev->product, dev->manufacturer, dev->serial checks in probe() function in radio-ma901.c driver in the way like it's done in radio-keene.c driver in probe function. If for example probe will discover that product doesn't match then i can return -ENODEV. It's just an idea. But i don't know if it is possible to do something with hid quirks: revert and put comments somewhere or workaround and additional checks? Well, any comments are welcome.

Signature · 64 lines
Best regards, Klimov Alexey



lsusb output:

Bus 003 Device 002: ID 16c0:05df VOTI
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               1.10
  bDeviceClass            0 (Defined at Interface level)
  bDeviceSubClass         0
  bDeviceProtocol         0
  bMaxPacketSize0         8
  idVendor           0x16c0 VOTI
  idProduct          0x05df
  bcdDevice            1.00
  iManufacturer           1 www.masterkit.ru
  iProduct                2 MA901
  iSerial                 3 SHS
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength           34
    bNumInterfaces          1
    bConfigurationValue     1
    iConfiguration          0
    bmAttributes         0x80
      (Bus Powered)
    MaxPower              250mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass         3 Human Interface Device
      bInterfaceSubClass      0 No Subclass
      bInterfaceProtocol      0 None
      iInterface              0
        HID Device Descriptor:
          bLength                 9
          bDescriptorType        33
          bcdHID               1.01
          bCountryCode            0 Not supported
          bNumDescriptors         1
          bDescriptorType        34 Report
          wDescriptorLength      22
         Report Descriptors:
           ** UNAVAILABLE **
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x81  EP 1 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0008  1x 8 bytes
        bInterval             100
Device Status:     0x0000
  (Bus Powered)

A message that is not in the archive (sent off-list or not yet received): [email protected]

#5 lore raw
Quoted 1 line · Dirk E. Wagner

Hi Alexey,

Hi Dirk, Mauro, Jiri

Quoted 8 lines

indeed your patch breaks Atmega applications which using V-USB
(http://www.obdev.at/products/vusb/index.html), because 0x16c0, 0x05df are the default Ids of V-USB.

Have a look at this FAQ

https://github.com/obdev/v-usb/blob/master/usbdrv/USB-ID-FAQ.txt

Quoted 8 lines

It seems that the Masterkit M901 also uses V-USB.

I'm using an IR remote control receiver based on Atmega8 with V-USB. Since Kernel 3.8.2 there is no more hidraw device for my receiver, so I had to change the Device-ID to 0x27d9. I think there are a lot of other V-USB applications with similar problems.

Dirk

Exactly. That's why i tried to point it out. Thanks for explaining this in simplier words.

It's difficult to answer on top posting emails.

I don't understand one thing about your letter. Did you put linux-media kernel list in bcc (hide copy)? Is there any reason for this? http://www.mail-archive.com/[email protected]/msg59714.html

Mauro, Jiri,
can we revert this patch? If you need any ack or sign from me i'm ready to send it.

I can contact people who cares about stable trees and ask them to revert this patch from stable trees.

During 3.9-rcX cycle i can try to figure out some fix or additional checks for radio-ma901.c driver.

Signature · 1 line
Thanks & best regards, Klimov Alexey
#6 lore raw
Quoted 30 lines · Alexey Klimov
Quoted 8 lines

indeed your patch breaks Atmega applications which using V-USB
(http://www.obdev.at/products/vusb/index.html), because 0x16c0, 0x05df are the default Ids of V-USB.

Have a look at this FAQ

https://github.com/obdev/v-usb/blob/master/usbdrv/USB-ID-FAQ.txt

Quoted 8 lines

It seems that the Masterkit M901 also uses V-USB.

I'm using an IR remote control receiver based on Atmega8 with V-USB. Since Kernel 3.8.2 there is no more hidraw device for my receiver, so I had to change the Device-ID to 0x27d9. I think there are a lot of other V-USB applications with similar problems.

Dirk

Exactly. That's why i tried to point it out. Thanks for explaining this in simplier words.

It's difficult to answer on top posting emails.

I don't understand one thing about your letter. Did you put linux-media kernel list in bcc (hide copy)? Is there any reason for this? http://www.mail-archive.com/[email protected]/msg59714.html

Mauro, Jiri,
can we revert this patch? If you need any ack or sign from me i'm ready to send it.

Quoted 5 lines

I can contact people who cares about stable trees and ask them to revert this patch from stable trees.

During 3.9-rcX cycle i can try to figure out some fix or additional checks for radio-ma901.c driver.

I can revert 0322bd3980 and push it out to Linus for 3.9 still, Ccing stable.

Or Mauro, as the original patch went in through your tree, are you handling that?

Also additional work will be needed later to properly detect the underlying device ... the best thing to do here is to put an entry into hid_ignore(), similar to what we do for example for Keene FM vs. Logitech AudioHub.

Signature · 2 lines
Jiri Kosina
SUSE Labs
#7 lore raw
Quoted 35 lines · Jiri Kosina
Quoted 33 lines · Alexey Klimov
Quoted 8 lines

indeed your patch breaks Atmega applications which using V-USB
(http://www.obdev.at/products/vusb/index.html), because 0x16c0, 0x05df are the default Ids of V-USB.

Have a look at this FAQ

https://github.com/obdev/v-usb/blob/master/usbdrv/USB-ID-FAQ.txt

Quoted 8 lines

It seems that the Masterkit M901 also uses V-USB.

I'm using an IR remote control receiver based on Atmega8 with V-USB. Since Kernel 3.8.2 there is no more hidraw device for my receiver, so I had to change the Device-ID to 0x27d9. I think there are a lot of other V-USB applications with similar problems.

Dirk

Exactly. That's why i tried to point it out. Thanks for explaining this in simplier words.

It's difficult to answer on top posting emails.

I don't understand one thing about your letter. Did you put linux-media kernel list in bcc (hide copy)? Is there any reason for this? http://www.mail-archive.com/[email protected]/msg59714.html

Mauro, Jiri,
can we revert this patch? If you need any ack or sign from me i'm ready to send it.

I can contact people who cares about stable trees and ask them to revert this patch from stable trees.

Quoted 8 lines
Quoted 2 lines

During 3.9-rcX cycle i can try to figure out some fix or additional checks for radio-ma901.c driver.

I can revert 0322bd3980 and push it out to Linus for 3.9 still, Ccing stable.

Or Mauro, as the original patch went in through your tree, are you handling that?

I think we really need to revert it before final release. It's already -rc3.

Quoted 4 lines

Also additional work will be needed later to properly detect the underlying device ... the best thing to do here is to put an entry into hid_ignore(), similar to what we do for example for Keene FM vs. Logitech AudioHub.

Yes, i just checked how hid_ignore() works and prepared dirty fix to test in almost the same way like it's done for Keene usb driver. I will send correct fix in next few days.

Thanks.
Best regards, Klimov Alexey

#8 lore raw
Quoted 4 lines · Alexey Klimov
Quoted 2 lines

Or Mauro, as the original patch went in through your tree, are you handling that?

I think we really need to revert it before final release. It's already -rc3.

If Mauro is currently too busy to handle this, I will take it. Please send it to me together with the appropriate hid_ignore() patch.

Thanks,

Signature · 2 lines
Jiri Kosina
SUSE Labs
#9 lore raw
In reply to Alexey Klimov
Quoted 3 lines · Alexey Klimov

Yes, i just checked how hid_ignore() works and prepared dirty fix to test in almost the same way like it's done for Keene usb driver. I will send correct fix in next few days.

Any news on this, please?

Signature · 2 lines
Jiri Kosina
SUSE Labs
#10 lore raw
Quoted 7 lines · Jiri Kosina
Quoted 3 lines · Alexey Klimov

Yes, i just checked how hid_ignore() works and prepared dirty fix to test in almost the same way like it's done for Keene usb driver. I will send correct fix in next few days.

Any news on this, please?

Hi Jiri,

I'm very very sorry (was busy because of life). I just sent two patches to you, Mauro and two mail lists:
[patch 1/2] hid: fix Masterkit MA901 hid quirks
[patch 2/2] media: radio-ma901: return ENODEV in probe if usb_device doesn't match

Please check. First one for hid layer, so maybe you can take it directly through your tree. I hope it's not too late. I think Mauro will take second patch.

I spend some time testing them trying to figure out right scenarios and i hope i did correct checks.
It will be nice if someone can test patches because i don't have any devices with same USB IDs as radio-ma901.

Thanks and best regards,
Alexey.

author Acked-by
#11 lore raw

Em Thu, 28 Mar 2013 00:33:29 +0400
Alexey Klimov <[email protected]> escreveu:

Quoted 17 lines · Jiri Kosina
Quoted 7 lines · Alexey Klimov
Quoted 4 lines

Yes, i just checked how hid_ignore() works and prepared dirty fix to test in almost the same way like it's done for Keene usb driver. I will send correct fix in next few days.

Any news on this, please?

Hi Jiri,

I'm very very sorry (was busy because of life). I just sent two patches to you, Mauro and two mail lists:
[patch 1/2] hid: fix Masterkit MA901 hid quirks
[patch 2/2] media: radio-ma901: return ENODEV in probe if usb_device doesn't match

Quoted 3 lines

Please check. First one for hid layer, so maybe you can take it directly through your tree. I hope it's not too late. I think Mauro will take second patch.

It is better to add both patches via the same tree. As it is badly affecting HID, it seems better if Jiri can apply both patches.

Also, there's no other patch for radio-ma901 on my tree. So, I don't expect any conflicts if those patches got merged via hid tree.

Jiri, could you please apply both patches on your tree?

For both:

Quoted 2 lines

[patch 1/2] hid: fix Masterkit MA901 hid quirks
[patch 2/2] media: radio-ma901: return ENODEV in probe if usb_device

Acked-by Mauro Carvalho Chehab <[email protected]>

Regards,
Mauro

#12 lore raw
Quoted 24 lines · Mauro Carvalho Chehab
Quoted 14 lines
Quoted 5 lines
Quoted 3 lines

Yes, i just checked how hid_ignore() works and prepared dirty fix to test in almost the same way like it's done for Keene usb driver. I will send correct fix in next few days.

Any news on this, please?

Hi Jiri,

I'm very very sorry (was busy because of life). I just sent two patches to you, Mauro and two mail lists:
[patch 1/2] hid: fix Masterkit MA901 hid quirks
[patch 2/2] media: radio-ma901: return ENODEV in probe if usb_device doesn't match

Quoted 3 lines

Please check. First one for hid layer, so maybe you can take it directly through your tree. I hope it's not too late. I think Mauro will take second patch.

It is better to add both patches via the same tree. As it is badly affecting HID, it seems better if Jiri can apply both patches.

Also, there's no other patch for radio-ma901 on my tree. So, I don't expect any conflicts if those patches got merged via hid tree.

Quoted 7 lines

Jiri, could you please apply both patches on your tree?

For both:

Quoted 2 lines

[patch 1/2] hid: fix Masterkit MA901 hid quirks
[patch 2/2] media: radio-ma901: return ENODEV in probe if usb_device

Acked-by Mauro Carvalho Chehab <[email protected]>

Absolutely. Will add your Ack and push it to Linus for 3.9 still.

Thanks,

Signature · 2 lines
Jiri Kosina
SUSE Labs
#13 lore raw
In reply to Alexey Klimov
Quoted 8 lines · Alexey Klimov

[patch 1/2] hid: fix Masterkit MA901 hid quirks
[patch 2/2] media: radio-ma901: return ENODEV in probe if usb_device doesn't match

I spend some time testing them trying to figure out right scenarios and i hope i did correct checks.
It will be nice if someone can test patches because i don't have any devices with same USB IDs as radio-ma901.

Thanks Alexey, I am using an infrared receiver running the same software Dirk uses, applied your [patch 1/2] against a vanilla kernel v3.4.38 on my vdr machine. The hidraw device is generated again as expected.

This is my USB device:

idVendor 0x16c0 VOTI
idProduct 0x05df
bcdDevice 1.08
iManufacturer 1 www.mikrocontroller.net/articles/USB_IR_Remote_Receiver
iProduct 2 USB IR Remote Receiver

Quoted 4 lines

Thanks and best regards,
Alexey.

Thanks,
Lars