All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] HID: hiddev: move hiddev's minor number and refactoring
@ 2017-03-02 13:45 ` Jaejoong Kim
  0 siblings, 0 replies; 13+ messages in thread
From: Jaejoong Kim @ 2017-03-02 13:45 UTC (permalink / raw)
  To: jikos, benjamin.tissoires
  Cc: linux-input, linux-kernel, linux-usb, Jaejoong Kim

Hi all,

I found hiddev's minor number is always zero in struct hid_device. So,
store the minor number asked from usb core in struct hid_device.

This is my first approach.

But after reviewed from Bendjamin, he suggested that it would make sense
to store a minor number in struct hiddev like hidraw if it neeeded.

So, I move the minor number from hid_device to hiddev and do some refactoring
to access struct hiddev in hid-core

Jaejoong Kim (2):
  HID: hiddev: move hiddev's minor number from struct hid_device to
    hiddev
  HID: hiddev: store hiddev's minor number when hiddev is connected

 drivers/hid/hid-core.c      |  2 +-
 drivers/hid/usbhid/hiddev.c | 25 +++----------------------
 include/linux/hid.h         |  1 -
 include/linux/hiddev.h      | 24 ++++++++++++++++++++++++
 4 files changed, 28 insertions(+), 24 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 0/2] HID: hiddev: move hiddev's minor number and refactoring
@ 2017-03-02 13:45 ` Jaejoong Kim
  0 siblings, 0 replies; 13+ messages in thread
From: Jaejoong Kim @ 2017-03-02 13:45 UTC (permalink / raw)
  To: jikos-DgEjT+Ai2ygdnm+yROfE0A, benjamin.tissoires-H+wXaHxf7aLQT0dZR+AlfA
  Cc: linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, Jaejoong Kim

Hi all,

I found hiddev's minor number is always zero in struct hid_device. So,
store the minor number asked from usb core in struct hid_device.

This is my first approach.

But after reviewed from Bendjamin, he suggested that it would make sense
to store a minor number in struct hiddev like hidraw if it neeeded.

So, I move the minor number from hid_device to hiddev and do some refactoring
to access struct hiddev in hid-core

Jaejoong Kim (2):
  HID: hiddev: move hiddev's minor number from struct hid_device to
    hiddev
  HID: hiddev: store hiddev's minor number when hiddev is connected

 drivers/hid/hid-core.c      |  2 +-
 drivers/hid/usbhid/hiddev.c | 25 +++----------------------
 include/linux/hid.h         |  1 -
 include/linux/hiddev.h      | 24 ++++++++++++++++++++++++
 4 files changed, 28 insertions(+), 24 deletions(-)

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/2] HID: hiddev: move hiddev's minor number from struct hid_device to hiddev
  2017-03-02 13:45 ` Jaejoong Kim
  (?)
@ 2017-03-02 13:45 ` Jaejoong Kim
  2017-03-02 14:10   ` Benjamin Tissoires
                     ` (2 more replies)
  -1 siblings, 3 replies; 13+ messages in thread
From: Jaejoong Kim @ 2017-03-02 13:45 UTC (permalink / raw)
  To: jikos, benjamin.tissoires
  Cc: linux-input, linux-kernel, linux-usb, Jaejoong Kim

We need to store the minor number each drivers. In case of hidraw, it's
minor number stores in struct hidraw. But hiddev's minor is located in
struct hid_device.

So reallocates for hiddev's minor number.

Signed-off-by: Jaejoong Kim <climbbb.kim@gmail.com>
---
 drivers/hid/usbhid/hiddev.c | 1 +
 include/linux/hid.h         | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
index 700145b..5c2c489 100644
--- a/drivers/hid/usbhid/hiddev.c
+++ b/drivers/hid/usbhid/hiddev.c
@@ -47,6 +47,7 @@
 #define HIDDEV_BUFFER_SIZE	2048
 
 struct hiddev {
+	int minor;
 	int exist;
 	int open;
 	struct mutex existancelock;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 28f38e2b8..643c017 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -541,7 +541,6 @@ struct hid_device {							/* device report descriptor */
 	struct list_head inputs;					/* The list of inputs */
 	void *hiddev;							/* The hiddev structure */
 	void *hidraw;
-	int minor;							/* Hiddev minor number */
 
 	int open;							/* is the device open by anyone? */
 	char name[128];							/* Device name */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/2] HID: hiddev: store hiddev's minor number when hiddev is connected
  2017-03-02 13:45 ` Jaejoong Kim
  (?)
  (?)
@ 2017-03-02 13:45 ` Jaejoong Kim
  2017-03-02 14:13     ` Benjamin Tissoires
  -1 siblings, 1 reply; 13+ messages in thread
From: Jaejoong Kim @ 2017-03-02 13:45 UTC (permalink / raw)
  To: jikos, benjamin.tissoires
  Cc: linux-input, linux-kernel, linux-usb, Jaejoong Kim

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] HID: hiddev: move hiddev's minor number from struct hid_device to hiddev
  2017-03-02 13:45 ` [PATCH 1/2] HID: hiddev: move hiddev's minor number from struct hid_device to hiddev Jaejoong Kim
@ 2017-03-02 14:10   ` Benjamin Tissoires
  2017-03-03  7:00     ` Kim Jaejoong
  2017-03-03 14:47     ` kbuild test robot
  2017-03-03 15:28     ` kbuild test robot
  2 siblings, 1 reply; 13+ messages in thread
From: Benjamin Tissoires @ 2017-03-02 14:10 UTC (permalink / raw)
  To: Jaejoong Kim; +Cc: jikos, linux-input, linux-kernel, linux-usb

On Mar 02 2017 or thereabouts, Jaejoong Kim wrote:
> We need to store the minor number each drivers. In case of hidraw, it's
> minor number stores in struct hidraw. But hiddev's minor is located in
> struct hid_device.
> 
> So reallocates for hiddev's minor number.
> 

There is not a real need to have this one in a separate patch. Also, it
depends on the patch "[PATCH] HID: cp2112: use proper hidraw name with
minor number", so better include this cp2112 in this series (as I
mentioned in the cp2112 patch).

I'd say simply squash this patch with 2/2 and have the cp2112 as 1/2.

Cheers,
Benjamin

> Signed-off-by: Jaejoong Kim <climbbb.kim@gmail.com>
> ---
>  drivers/hid/usbhid/hiddev.c | 1 +
>  include/linux/hid.h         | 1 -
>  2 files changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
> index 700145b..5c2c489 100644
> --- a/drivers/hid/usbhid/hiddev.c
> +++ b/drivers/hid/usbhid/hiddev.c
> @@ -47,6 +47,7 @@
>  #define HIDDEV_BUFFER_SIZE	2048
>  
>  struct hiddev {
> +	int minor;
>  	int exist;
>  	int open;
>  	struct mutex existancelock;
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index 28f38e2b8..643c017 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -541,7 +541,6 @@ struct hid_device {							/* device report descriptor */
>  	struct list_head inputs;					/* The list of inputs */
>  	void *hiddev;							/* The hiddev structure */
>  	void *hidraw;
> -	int minor;							/* Hiddev minor number */
>  
>  	int open;							/* is the device open by anyone? */
>  	char name[128];							/* Device name */
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] HID: hiddev: store hiddev's minor number when hiddev is connected
@ 2017-03-02 14:13     ` Benjamin Tissoires
  0 siblings, 0 replies; 13+ messages in thread
From: Benjamin Tissoires @ 2017-03-02 14:13 UTC (permalink / raw)
  To: Jaejoong Kim; +Cc: jikos, linux-input, linux-kernel, linux-usb

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
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] HID: hiddev: store hiddev's minor number when hiddev is connected
@ 2017-03-02 14:13     ` Benjamin Tissoires
  0 siblings, 0 replies; 13+ messages in thread
From: Benjamin Tissoires @ 2017-03-02 14:13 UTC (permalink / raw)
  To: Jaejoong Kim
  Cc: jikos-DgEjT+Ai2ygdnm+yROfE0A, linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA

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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  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
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] HID: hiddev: move hiddev's minor number from struct hid_device to hiddev
  2017-03-02 14:10   ` Benjamin Tissoires
@ 2017-03-03  7:00     ` Kim Jaejoong
  0 siblings, 0 replies; 13+ messages in thread
From: Kim Jaejoong @ 2017-03-03  7:00 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: jikos, linux-input, linux-kernel, linux-usb

2017-03-02 23:10 GMT+09:00 Benjamin Tissoires <benjamin.tissoires@redhat.com>:
> On Mar 02 2017 or thereabouts, Jaejoong Kim wrote:
>> We need to store the minor number each drivers. In case of hidraw, it's
>> minor number stores in struct hidraw. But hiddev's minor is located in
>> struct hid_device.
>>
>> So reallocates for hiddev's minor number.
>>
>
> There is not a real need to have this one in a separate patch. Also, it
> depends on the patch "[PATCH] HID: cp2112: use proper hidraw name with
> minor number", so better include this cp2112 in this series (as I
> mentioned in the cp2112 patch).
>
> I'd say simply squash this patch with 2/2 and have the cp2112 as 1/2.

Ok. I will resend v2 patchset.

Thanks.
jaejoong

>
> Cheers,
> Benjamin
>
>> Signed-off-by: Jaejoong Kim <climbbb.kim@gmail.com>
>> ---
>>  drivers/hid/usbhid/hiddev.c | 1 +
>>  include/linux/hid.h         | 1 -
>>  2 files changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
>> index 700145b..5c2c489 100644
>> --- a/drivers/hid/usbhid/hiddev.c
>> +++ b/drivers/hid/usbhid/hiddev.c
>> @@ -47,6 +47,7 @@
>>  #define HIDDEV_BUFFER_SIZE   2048
>>
>>  struct hiddev {
>> +     int minor;
>>       int exist;
>>       int open;
>>       struct mutex existancelock;
>> diff --git a/include/linux/hid.h b/include/linux/hid.h
>> index 28f38e2b8..643c017 100644
>> --- a/include/linux/hid.h
>> +++ b/include/linux/hid.h
>> @@ -541,7 +541,6 @@ struct hid_device {                                                       /* device report descriptor */
>>       struct list_head inputs;                                        /* The list of inputs */
>>       void *hiddev;                                                   /* The hiddev structure */
>>       void *hidraw;
>> -     int minor;                                                      /* Hiddev minor number */
>>
>>       int open;                                                       /* is the device open by anyone? */
>>       char name[128];                                                 /* Device name */
>> --
>> 2.7.4
>>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] HID: hiddev: store hiddev's minor number when hiddev is connected
  2017-03-02 14:13     ` Benjamin Tissoires
  (?)
@ 2017-03-03  7:05     ` Kim Jaejoong
  -1 siblings, 0 replies; 13+ messages in thread
From: Kim Jaejoong @ 2017-03-03  7:05 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: jikos, linux-input, linux-kernel, linux-usb

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
>>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] HID: hiddev: move hiddev's minor number from struct hid_device to hiddev
  2017-03-02 13:45 ` [PATCH 1/2] HID: hiddev: move hiddev's minor number from struct hid_device to hiddev Jaejoong Kim
@ 2017-03-03 14:47     ` kbuild test robot
  2017-03-03 14:47     ` kbuild test robot
  2017-03-03 15:28     ` kbuild test robot
  2 siblings, 0 replies; 13+ messages in thread
From: kbuild test robot @ 2017-03-03 14:47 UTC (permalink / raw)
  To: Jaejoong Kim
  Cc: kbuild-all, jikos, benjamin.tissoires, linux-input, linux-kernel,
	linux-usb, Jaejoong Kim

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

Hi Jaejoong,

[auto build test ERROR on hid/for-next]
[also build test ERROR on v4.10 next-20170303]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Jaejoong-Kim/HID-hiddev-move-hiddev-s-minor-number-and-refactoring/20170303-222428
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git for-next
config: x86_64-randconfig-x016-201709 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/hid/hid-core.c: In function 'hid_connect':
>> drivers/hid/hid-core.c:1698:9: error: 'struct hid_device' has no member named 'minor'; did you mean 'vendor'?
        hdev->minor);
            ^~
--
   drivers/hid/hid-cp2112.c: In function 'cp2112_probe':
>> drivers/hid/hid-cp2112.c:1300:43: error: 'struct hid_device' has no member named 'minor'; did you mean 'vendor'?
       "CP2112 SMBus Bridge on hiddev%d", hdev->minor);
                                              ^~

vim +1698 drivers/hid/hid-core.c

93c10132 Jiri Slaby 2008-06-27  1692  
93c10132 Jiri Slaby 2008-06-27  1693  	len = 0;
93c10132 Jiri Slaby 2008-06-27  1694  	if (hdev->claimed & HID_CLAIMED_INPUT)
93c10132 Jiri Slaby 2008-06-27  1695  		len += sprintf(buf + len, "input");
93c10132 Jiri Slaby 2008-06-27  1696  	if (hdev->claimed & HID_CLAIMED_HIDDEV)
93c10132 Jiri Slaby 2008-06-27  1697  		len += sprintf(buf + len, "%shiddev%d", len ? "," : "",
93c10132 Jiri Slaby 2008-06-27 @1698  				hdev->minor);
93c10132 Jiri Slaby 2008-06-27  1699  	if (hdev->claimed & HID_CLAIMED_HIDRAW)
93c10132 Jiri Slaby 2008-06-27  1700  		len += sprintf(buf + len, "%shidraw%d", len ? "," : "",
93c10132 Jiri Slaby 2008-06-27  1701  				((struct hidraw *)hdev->hidraw)->minor);

:::::: The code at line 1698 was first introduced by commit
:::::: 93c10132a7ac160df3175b53f7ee857625412165 HID: move connect quirks

:::::: TO: Jiri Slaby <jirislaby@gmail.com>
:::::: CC: Jiri Kosina <jkosina@suse.cz>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28058 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] HID: hiddev: move hiddev's minor number from struct hid_device to hiddev
@ 2017-03-03 14:47     ` kbuild test robot
  0 siblings, 0 replies; 13+ messages in thread
From: kbuild test robot @ 2017-03-03 14:47 UTC (permalink / raw)
  Cc: kbuild-all, jikos, benjamin.tissoires, linux-input, linux-kernel,
	linux-usb, Jaejoong Kim

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

Hi Jaejoong,

[auto build test ERROR on hid/for-next]
[also build test ERROR on v4.10 next-20170303]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Jaejoong-Kim/HID-hiddev-move-hiddev-s-minor-number-and-refactoring/20170303-222428
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git for-next
config: x86_64-randconfig-x016-201709 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/hid/hid-core.c: In function 'hid_connect':
>> drivers/hid/hid-core.c:1698:9: error: 'struct hid_device' has no member named 'minor'; did you mean 'vendor'?
        hdev->minor);
            ^~
--
   drivers/hid/hid-cp2112.c: In function 'cp2112_probe':
>> drivers/hid/hid-cp2112.c:1300:43: error: 'struct hid_device' has no member named 'minor'; did you mean 'vendor'?
       "CP2112 SMBus Bridge on hiddev%d", hdev->minor);
                                              ^~

vim +1698 drivers/hid/hid-core.c

93c10132 Jiri Slaby 2008-06-27  1692  
93c10132 Jiri Slaby 2008-06-27  1693  	len = 0;
93c10132 Jiri Slaby 2008-06-27  1694  	if (hdev->claimed & HID_CLAIMED_INPUT)
93c10132 Jiri Slaby 2008-06-27  1695  		len += sprintf(buf + len, "input");
93c10132 Jiri Slaby 2008-06-27  1696  	if (hdev->claimed & HID_CLAIMED_HIDDEV)
93c10132 Jiri Slaby 2008-06-27  1697  		len += sprintf(buf + len, "%shiddev%d", len ? "," : "",
93c10132 Jiri Slaby 2008-06-27 @1698  				hdev->minor);
93c10132 Jiri Slaby 2008-06-27  1699  	if (hdev->claimed & HID_CLAIMED_HIDRAW)
93c10132 Jiri Slaby 2008-06-27  1700  		len += sprintf(buf + len, "%shidraw%d", len ? "," : "",
93c10132 Jiri Slaby 2008-06-27  1701  				((struct hidraw *)hdev->hidraw)->minor);

:::::: The code at line 1698 was first introduced by commit
:::::: 93c10132a7ac160df3175b53f7ee857625412165 HID: move connect quirks

:::::: TO: Jiri Slaby <jirislaby@gmail.com>
:::::: CC: Jiri Kosina <jkosina@suse.cz>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28058 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] HID: hiddev: move hiddev's minor number from struct hid_device to hiddev
  2017-03-02 13:45 ` [PATCH 1/2] HID: hiddev: move hiddev's minor number from struct hid_device to hiddev Jaejoong Kim
@ 2017-03-03 15:28     ` kbuild test robot
  2017-03-03 14:47     ` kbuild test robot
  2017-03-03 15:28     ` kbuild test robot
  2 siblings, 0 replies; 13+ messages in thread
From: kbuild test robot @ 2017-03-03 15:28 UTC (permalink / raw)
  To: Jaejoong Kim
  Cc: kbuild-all, jikos, benjamin.tissoires, linux-input, linux-kernel,
	linux-usb, Jaejoong Kim

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

Hi Jaejoong,

[auto build test ERROR on hid/for-next]
[also build test ERROR on v4.10 next-20170303]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Jaejoong-Kim/HID-hiddev-move-hiddev-s-minor-number-and-refactoring/20170303-222428
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git for-next
config: i386-randconfig-b0-03032110 (attached as .config)
compiler: gcc-5 (Debian 5.4.1-2) 5.4.1 20160904
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/hid/hid-cp2112.c: In function 'cp2112_probe':
>> drivers/hid/hid-cp2112.c:1300:43: error: 'struct hid_device' has no member named 'minor'
       "CP2112 SMBus Bridge on hiddev%d", hdev->minor);
                                              ^

vim +1300 drivers/hid/hid-cp2112.c

e932d817 David Barksdale 2014-02-04  1294  	dev->adap.owner		= THIS_MODULE;
e932d817 David Barksdale 2014-02-04  1295  	dev->adap.class		= I2C_CLASS_HWMON;
e932d817 David Barksdale 2014-02-04  1296  	dev->adap.algo		= &smbus_algorithm;
e932d817 David Barksdale 2014-02-04  1297  	dev->adap.algo_data	= dev;
e932d817 David Barksdale 2014-02-04  1298  	dev->adap.dev.parent	= &hdev->dev;
e932d817 David Barksdale 2014-02-04  1299  	snprintf(dev->adap.name, sizeof(dev->adap.name),
e932d817 David Barksdale 2014-02-04 @1300  		 "CP2112 SMBus Bridge on hiddev%d", hdev->minor);
44eda784 Ellen Wang      2015-07-09  1301  	dev->hwversion = buf[2];
e932d817 David Barksdale 2014-02-04  1302  	init_waitqueue_head(&dev->wait);
e932d817 David Barksdale 2014-02-04  1303  

:::::: The code at line 1300 was first introduced by commit
:::::: e932d817866770d456815c9a84b7ed94f0589d80 HID: add hid-cp2112 driver

:::::: TO: David Barksdale <dbarksdale@uplogix.com>
:::::: CC: Jiri Kosina <jkosina@suse.cz>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28029 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] HID: hiddev: move hiddev's minor number from struct hid_device to hiddev
@ 2017-03-03 15:28     ` kbuild test robot
  0 siblings, 0 replies; 13+ messages in thread
From: kbuild test robot @ 2017-03-03 15:28 UTC (permalink / raw)
  Cc: kbuild-all, jikos, benjamin.tissoires, linux-input, linux-kernel,
	linux-usb, Jaejoong Kim

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

Hi Jaejoong,

[auto build test ERROR on hid/for-next]
[also build test ERROR on v4.10 next-20170303]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Jaejoong-Kim/HID-hiddev-move-hiddev-s-minor-number-and-refactoring/20170303-222428
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git for-next
config: i386-randconfig-b0-03032110 (attached as .config)
compiler: gcc-5 (Debian 5.4.1-2) 5.4.1 20160904
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/hid/hid-cp2112.c: In function 'cp2112_probe':
>> drivers/hid/hid-cp2112.c:1300:43: error: 'struct hid_device' has no member named 'minor'
       "CP2112 SMBus Bridge on hiddev%d", hdev->minor);
                                              ^

vim +1300 drivers/hid/hid-cp2112.c

e932d817 David Barksdale 2014-02-04  1294  	dev->adap.owner		= THIS_MODULE;
e932d817 David Barksdale 2014-02-04  1295  	dev->adap.class		= I2C_CLASS_HWMON;
e932d817 David Barksdale 2014-02-04  1296  	dev->adap.algo		= &smbus_algorithm;
e932d817 David Barksdale 2014-02-04  1297  	dev->adap.algo_data	= dev;
e932d817 David Barksdale 2014-02-04  1298  	dev->adap.dev.parent	= &hdev->dev;
e932d817 David Barksdale 2014-02-04  1299  	snprintf(dev->adap.name, sizeof(dev->adap.name),
e932d817 David Barksdale 2014-02-04 @1300  		 "CP2112 SMBus Bridge on hiddev%d", hdev->minor);
44eda784 Ellen Wang      2015-07-09  1301  	dev->hwversion = buf[2];
e932d817 David Barksdale 2014-02-04  1302  	init_waitqueue_head(&dev->wait);
e932d817 David Barksdale 2014-02-04  1303  

:::::: The code at line 1300 was first introduced by commit
:::::: e932d817866770d456815c9a84b7ed94f0589d80 HID: add hid-cp2112 driver

:::::: TO: David Barksdale <dbarksdale@uplogix.com>
:::::: CC: Jiri Kosina <jkosina@suse.cz>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28029 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2017-03-03 16:27 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-02 13:45 [PATCH 0/2] HID: hiddev: move hiddev's minor number and refactoring Jaejoong Kim
2017-03-02 13:45 ` Jaejoong Kim
2017-03-02 13:45 ` [PATCH 1/2] HID: hiddev: move hiddev's minor number from struct hid_device to hiddev Jaejoong Kim
2017-03-02 14:10   ` Benjamin Tissoires
2017-03-03  7:00     ` Kim Jaejoong
2017-03-03 14:47   ` kbuild test robot
2017-03-03 14:47     ` kbuild test robot
2017-03-03 15:28   ` kbuild test robot
2017-03-03 15:28     ` kbuild test robot
2017-03-02 13:45 ` [PATCH 2/2] HID: hiddev: store hiddev's minor number when hiddev is connected Jaejoong Kim
2017-03-02 14:13   ` Benjamin Tissoires
2017-03-02 14:13     ` Benjamin Tissoires
2017-03-03  7:05     ` Kim Jaejoong

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.