Re: [PATCH 1/3] picolcd: driver for PicoLCD HID device

["Bruno Prémont" <bonbons@linux-vserver.org>]

On Thu, 25 February 2010 Jiri Kosina <jkosina@suse.cz> wrote:
> On Thu, 25 Feb 2010, Bruno Prémont wrote:
> 
> > For the two sysfs attributes I currently use, the 'reset' one shall 
> > probably be moved to debugfs (I would like to place it under 
> > /sys/kernel/debug/hid/$device/ next to rdesc and events).
> 
> Yes, that would make sense.

Hm, that works only when driver module gets loaded when USBHID device
is already plugged in.
When driver is registered and has it's probe function called on hotplug
it runs right before hid-core registers the debugfs entries.
As such hid_device's debug_dir is still NULL.

To get around this, in hid_add_device(), hid_debug_register() would
have to be called before device_add() instead of right after.

Is something like below acceptable? (from error handling point of view
it makes no difference...)

Thanks,
Bruno

file: drivers/hid/hid-core.c

 int hid_add_device(struct hid_device *hdev)
 {
         static atomic_t id = ATOMIC_INIT(0);
         int ret;
 
         if (WARN_ON(hdev->status & HID_STAT_ADDED))
                 return -EBUSY;
 
         /* we need to kill them here, otherwise they will stay allocated to
          * wait for coming driver */
         if (hid_ignore(hdev))
                 return -ENODEV;
 
         /* XXX hack, any other cleaner solution after the driver core
          * is converted to allow more than 20 bytes as the device name? */
         dev_set_name(&hdev->dev, "%04X:%04X:%04X.%04X", hdev->bus,
                      hdev->vendor, hdev->product, atomic_inc_return(&id));
 
+        hid_debug_register(hdev, dev_name(&hdev->dev));
         ret = device_add(&hdev->dev);
         if (!ret)
                 hdev->status |= HID_STAT_ADDED;
 
-        hid_debug_register(hdev, dev_name(&hdev->dev));
 
         return ret;
 }
 EXPORT_SYMBOL_GPL(hid_add_device);