Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1591083 > unrolled thread

[PATCH 2/2] HID: hiddev: store hiddev's minor number when hiddev is connected

Started byJaejoong Kim <climbbb.kim@gmail.com>
First post2017-03-02 14:50 +0100
Last post2017-03-03 08:20 +0100
Articles 3 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH 2/2] HID: hiddev: store hiddev's minor number when hiddev is connected Jaejoong Kim <climbbb.kim@gmail.com> - 2017-03-02 14:50 +0100
    Re: [PATCH 2/2] HID: hiddev: store hiddev's minor number when hiddev  is connected Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-03-02 17:10 +0100
      Re: [PATCH 2/2] HID: hiddev: store hiddev's minor number when hiddev  is connected Kim Jaejoong <climbbb.kim@gmail.com> - 2017-03-03 08:20 +0100

#1591083 — [PATCH 2/2] HID: hiddev: store hiddev's minor number when hiddev is connected

FromJaejoong Kim <climbbb.kim@gmail.com>
Date2017-03-02 14:50 +0100
Subject[PATCH 2/2] HID: hiddev: store hiddev's minor number when hiddev is connected
Message-ID<tgzhg-2VJ-5@gated-at.bofh.it>
The hid-core announces kernel message which driver is loaded if there is
a hiddev, but hiddev's minor number is always zero even though it's not
zero.

So, we need to store the minor number asked from usb core and do some
refactoring work(move from hiddev.c to hiddev.h) to access hiddev in
hid-core.

Signed-off-by: Jaejoong Kim <climbbb.kim@gmail.com>
---
 drivers/hid/hid-core.c      |  2 +-
 drivers/hid/usbhid/hiddev.c | 26 +++-----------------------
 include/linux/hiddev.h      | 24 ++++++++++++++++++++++++
 3 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index e9e87d3..1a0b910 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1695,7 +1695,7 @@ int hid_connect(struct hid_device *hdev, unsigned int connect_mask)
 		len += sprintf(buf + len, "input");
 	if (hdev->claimed & HID_CLAIMED_HIDDEV)
 		len += sprintf(buf + len, "%shiddev%d", len ? "," : "",
-				hdev->minor);
+				((struct hiddev *)hdev->hiddev)->minor);
 	if (hdev->claimed & HID_CLAIMED_HIDRAW)
 		len += sprintf(buf + len, "%shidraw%d", len ? "," : "",
 				((struct hidraw *)hdev->hidraw)->minor);
diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
index 5c2c489..ef83d68 100644
--- a/drivers/hid/usbhid/hiddev.c
+++ b/drivers/hid/usbhid/hiddev.c
@@ -44,29 +44,6 @@
 #define HIDDEV_MINOR_BASE	96
 #define HIDDEV_MINORS		16
 #endif
-#define HIDDEV_BUFFER_SIZE	2048
-
-struct hiddev {
-	int minor;
-	int exist;
-	int open;
-	struct mutex existancelock;
-	wait_queue_head_t wait;
-	struct hid_device *hid;
-	struct list_head list;
-	spinlock_t list_lock;
-};
-
-struct hiddev_list {
-	struct hiddev_usage_ref buffer[HIDDEV_BUFFER_SIZE];
-	int head;
-	int tail;
-	unsigned flags;
-	struct fasync_struct *fasync;
-	struct hiddev *hiddev;
-	struct list_head node;
-	struct mutex thread_lock;
-};
 
 /*
  * Find a report, given the report's type and ID.  The ID can be specified
@@ -911,6 +888,9 @@ int hiddev_connect(struct hid_device *hid, unsigned int force)
 		kfree(hiddev);
 		return -1;
 	}
+
+	hiddev->minor = usbhid->intf->minor;
+
 	return 0;
 }
 
diff --git a/include/linux/hiddev.h b/include/linux/hiddev.h
index a5dd814..ff3701b 100644
--- a/include/linux/hiddev.h
+++ b/include/linux/hiddev.h
@@ -32,6 +32,30 @@
  * In-kernel definitions.
  */
 
+#define HIDDEV_BUFFER_SIZE      2048
+
+struct hiddev {
+	int minor;
+	int exist;
+	int open;
+	struct mutex existancelock;
+	wait_queue_head_t wait;
+	struct hid_device *hid;
+	struct list_head list;
+	spinlock_t list_lock;
+};
+
+struct hiddev_list {
+	struct hiddev_usage_ref buffer[HIDDEV_BUFFER_SIZE];
+	int head;
+	int tail;
+	unsigned flags;
+	struct fasync_struct *fasync;
+	struct hiddev *hiddev;
+	struct list_head node;
+	struct mutex thread_lock;
+};
+
 struct hid_device;
 struct hid_usage;
 struct hid_field;
-- 
2.7.4

[toc] | [next] | [standalone]


#1591242 — Re: [PATCH 2/2] HID: hiddev: store hiddev's minor number when hiddev is connected

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2017-03-02 17:10 +0100
SubjectRe: [PATCH 2/2] HID: hiddev: store hiddev's minor number when hiddev is connected
Message-ID<tgBsK-4MC-29@gated-at.bofh.it>
In reply to#1591083
On Mar 02 2017 or thereabouts, Jaejoong Kim wrote:
> The hid-core announces kernel message which driver is loaded if there is
> a hiddev, but hiddev's minor number is always zero even though it's not
> zero.
> 
> So, we need to store the minor number asked from usb core and do some
> refactoring work(move from hiddev.c to hiddev.h) to access hiddev in
> hid-core.
> 
> Signed-off-by: Jaejoong Kim <climbbb.kim@gmail.com>
> ---
>  drivers/hid/hid-core.c      |  2 +-
>  drivers/hid/usbhid/hiddev.c | 26 +++-----------------------
>  include/linux/hiddev.h      | 24 ++++++++++++++++++++++++
>  3 files changed, 28 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index e9e87d3..1a0b910 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1695,7 +1695,7 @@ int hid_connect(struct hid_device *hdev, unsigned int connect_mask)
>  		len += sprintf(buf + len, "input");
>  	if (hdev->claimed & HID_CLAIMED_HIDDEV)
>  		len += sprintf(buf + len, "%shiddev%d", len ? "," : "",
> -				hdev->minor);
> +				((struct hiddev *)hdev->hiddev)->minor);
>  	if (hdev->claimed & HID_CLAIMED_HIDRAW)
>  		len += sprintf(buf + len, "%shidraw%d", len ? "," : "",
>  				((struct hidraw *)hdev->hidraw)->minor);
> diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
> index 5c2c489..ef83d68 100644
> --- a/drivers/hid/usbhid/hiddev.c
> +++ b/drivers/hid/usbhid/hiddev.c
> @@ -44,29 +44,6 @@
>  #define HIDDEV_MINOR_BASE	96
>  #define HIDDEV_MINORS		16
>  #endif
> -#define HIDDEV_BUFFER_SIZE	2048
> -
> -struct hiddev {
> -	int minor;
> -	int exist;
> -	int open;
> -	struct mutex existancelock;
> -	wait_queue_head_t wait;
> -	struct hid_device *hid;
> -	struct list_head list;
> -	spinlock_t list_lock;
> -};
> -
> -struct hiddev_list {
> -	struct hiddev_usage_ref buffer[HIDDEV_BUFFER_SIZE];
> -	int head;
> -	int tail;
> -	unsigned flags;
> -	struct fasync_struct *fasync;
> -	struct hiddev *hiddev;
> -	struct list_head node;
> -	struct mutex thread_lock;
> -};
>  
>  /*
>   * Find a report, given the report's type and ID.  The ID can be specified
> @@ -911,6 +888,9 @@ int hiddev_connect(struct hid_device *hid, unsigned int force)
>  		kfree(hiddev);
>  		return -1;
>  	}
> +
> +	hiddev->minor = usbhid->intf->minor;
> +
>  	return 0;
>  }
>  
> diff --git a/include/linux/hiddev.h b/include/linux/hiddev.h
> index a5dd814..ff3701b 100644
> --- a/include/linux/hiddev.h
> +++ b/include/linux/hiddev.h
> @@ -32,6 +32,30 @@
>   * In-kernel definitions.
>   */
>  
> +#define HIDDEV_BUFFER_SIZE      2048
> +
> +struct hiddev {
> +	int minor;
> +	int exist;
> +	int open;
> +	struct mutex existancelock;
> +	wait_queue_head_t wait;
> +	struct hid_device *hid;
> +	struct list_head list;
> +	spinlock_t list_lock;
> +};
> +
> +struct hiddev_list {
> +	struct hiddev_usage_ref buffer[HIDDEV_BUFFER_SIZE];
> +	int head;
> +	int tail;
> +	unsigned flags;
> +	struct fasync_struct *fasync;
> +	struct hiddev *hiddev;
> +	struct list_head node;
> +	struct mutex thread_lock;
> +};

Why do you need to also export struct hiddev_list? Unless I am missing
something we don't need it elsewhere but in hiddev.c, and so there is no
point exporting this struct to the world.

With this change amended, the end result looks good and the series
should be ready to be integrated IMO.

Cheers,
Benjamin

> +
>  struct hid_device;
>  struct hid_usage;
>  struct hid_field;
> -- 
> 2.7.4
> 

[toc] | [prev] | [next] | [standalone]


#1591709 — Re: [PATCH 2/2] HID: hiddev: store hiddev's minor number when hiddev is connected

FromKim Jaejoong <climbbb.kim@gmail.com>
Date2017-03-03 08:20 +0100
SubjectRe: [PATCH 2/2] HID: hiddev: store hiddev's minor number when hiddev is connected
Message-ID<tgPFn-62U-5@gated-at.bofh.it>
In reply to#1591242
2017-03-02 23:13 GMT+09:00 Benjamin Tissoires <benjamin.tissoires@redhat.com>:
> On Mar 02 2017 or thereabouts, Jaejoong Kim wrote:
>> The hid-core announces kernel message which driver is loaded if there is
>> a hiddev, but hiddev's minor number is always zero even though it's not
>> zero.
>>
>> So, we need to store the minor number asked from usb core and do some
>> refactoring work(move from hiddev.c to hiddev.h) to access hiddev in
>> hid-core.
>>
>> Signed-off-by: Jaejoong Kim <climbbb.kim@gmail.com>
>> ---
>>  drivers/hid/hid-core.c      |  2 +-
>>  drivers/hid/usbhid/hiddev.c | 26 +++-----------------------
>>  include/linux/hiddev.h      | 24 ++++++++++++++++++++++++
>>  3 files changed, 28 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>> index e9e87d3..1a0b910 100644
>> --- a/drivers/hid/hid-core.c
>> +++ b/drivers/hid/hid-core.c
>> @@ -1695,7 +1695,7 @@ int hid_connect(struct hid_device *hdev, unsigned int connect_mask)
>>               len += sprintf(buf + len, "input");
>>       if (hdev->claimed & HID_CLAIMED_HIDDEV)
>>               len += sprintf(buf + len, "%shiddev%d", len ? "," : "",
>> -                             hdev->minor);
>> +                             ((struct hiddev *)hdev->hiddev)->minor);
>>       if (hdev->claimed & HID_CLAIMED_HIDRAW)
>>               len += sprintf(buf + len, "%shidraw%d", len ? "," : "",
>>                               ((struct hidraw *)hdev->hidraw)->minor);
>> diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
>> index 5c2c489..ef83d68 100644
>> --- a/drivers/hid/usbhid/hiddev.c
>> +++ b/drivers/hid/usbhid/hiddev.c
>> @@ -44,29 +44,6 @@
>>  #define HIDDEV_MINOR_BASE    96
>>  #define HIDDEV_MINORS                16
>>  #endif
>> -#define HIDDEV_BUFFER_SIZE   2048
>> -
>> -struct hiddev {
>> -     int minor;
>> -     int exist;
>> -     int open;
>> -     struct mutex existancelock;
>> -     wait_queue_head_t wait;
>> -     struct hid_device *hid;
>> -     struct list_head list;
>> -     spinlock_t list_lock;
>> -};
>> -
>> -struct hiddev_list {
>> -     struct hiddev_usage_ref buffer[HIDDEV_BUFFER_SIZE];
>> -     int head;
>> -     int tail;
>> -     unsigned flags;
>> -     struct fasync_struct *fasync;
>> -     struct hiddev *hiddev;
>> -     struct list_head node;
>> -     struct mutex thread_lock;
>> -};
>>
>>  /*
>>   * Find a report, given the report's type and ID.  The ID can be specified
>> @@ -911,6 +888,9 @@ int hiddev_connect(struct hid_device *hid, unsigned int force)
>>               kfree(hiddev);
>>               return -1;
>>       }
>> +
>> +     hiddev->minor = usbhid->intf->minor;
>> +
>>       return 0;
>>  }
>>
>> diff --git a/include/linux/hiddev.h b/include/linux/hiddev.h
>> index a5dd814..ff3701b 100644
>> --- a/include/linux/hiddev.h
>> +++ b/include/linux/hiddev.h
>> @@ -32,6 +32,30 @@
>>   * In-kernel definitions.
>>   */
>>
>> +#define HIDDEV_BUFFER_SIZE      2048
>> +
>> +struct hiddev {
>> +     int minor;
>> +     int exist;
>> +     int open;
>> +     struct mutex existancelock;
>> +     wait_queue_head_t wait;
>> +     struct hid_device *hid;
>> +     struct list_head list;
>> +     spinlock_t list_lock;
>> +};
>> +
>> +struct hiddev_list {
>> +     struct hiddev_usage_ref buffer[HIDDEV_BUFFER_SIZE];
>> +     int head;
>> +     int tail;
>> +     unsigned flags;
>> +     struct fasync_struct *fasync;
>> +     struct hiddev *hiddev;
>> +     struct list_head node;
>> +     struct mutex thread_lock;
>> +};
>
> Why do you need to also export struct hiddev_list? Unless I am missing
> something we don't need it elsewhere but in hiddev.c, and so there is no
> point exporting this struct to the world.

You're right. I will export only struct hiddev.

>
> With this change amended, the end result looks good and the series
> should be ready to be integrated IMO.
>

I will resend v2 patchset your said.

Thanks,
jaejoong

> Cheers,
> Benjamin
>
>> +
>>  struct hid_device;
>>  struct hid_usage;
>>  struct hid_field;
>> --
>> 2.7.4
>>

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web