Current names are reported as "K750", "M705", and it can be misleading
for the users when they look at their input device list.
Prefixing the names with "Logitech " makes things better.
Signed-off-by: Benjamin Tissoires <[email protected]>
---
Changes in v2:
- renamed PREFIX_SIZE into PREFIX_LENGTH
- changed "name_length + PREFIX_LENGTH;" into "PREFIX_LENGTH + name_length;"
- rebased on Peter's last patch series
drivers/hid/hid-logitech-hidpp.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 2f420c0..274dbb7 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -282,6 +282,33 @@ static inline bool hidpp_report_is_connect_event(struct hidpp_report *report)
(report->rap.sub_id == 0x41);
}
+/**
+ * hidpp_prefix_name() prefixes the current given name with "Logitech ".
+ */
+static void hidpp_prefix_name(char **name, int name_length)
+{
+#define PREFIX_LENGTH 9 /* "Logitech " */
+
+ int new_length;
+ char *new_name;
+
+ if (name_length > PREFIX_LENGTH &&
+ strncmp(*name, "Logitech ", PREFIX_LENGTH) == 0)
+ /* The prefix has is already in the name */
+ return;
+
+ new_length = PREFIX_LENGTH + name_length;
+ new_name = kzalloc(new_length, GFP_KERNEL);
+ if (!new_name)
+ return;
+
+ snprintf(new_name, new_length, "Logitech %s", *name);
+
+ kfree(*name);
+
+ *name = new_name;
+}
+
/* -------------------------------------------------------------------------- */
/* HIDP++ 1.0 commands */
/* -------------------------------------------------------------------------- */
@@ -321,6 +348,10 @@ static char *hidpp_get_unifying_name(struct hidpp_device *hidpp_dev)
return NULL;
memcpy(name, &response.rap.params[2], len);
+
+ /* include the terminating '\0' */
+ hidpp_prefix_name(&name, len + 1);
+
return name;
}
@@ -498,6 +529,9 @@ static char *hidpp_get_device_name(struct hidpp_device *hidpp)
index += ret;
}
+ /* include the terminating '\0' */
+ hidpp_prefix_name(&name, __name_length + 1);
+
return name;
}
--
2.1.0
On Thursday 11 December 2014 17:39:59 Benjamin Tissoires wrote:
> Current names are reported as "K750", "M705", and it can be misleading
> for the users when they look at their input device list.
>
> Prefixing the names with "Logitech " makes things better.
>
> Signed-off-by: Benjamin Tissoires <[email protected]>
Reviewed-by: Peter Wu <[email protected]>
I have not tested this one, but the approach looks correct. What I have
also been thinking of is the possibility that Logitech adds "LOGITECH"
or "Logicool" (the Japanese trademark) before devices, but I think that
is unlikely so there is no need to check for other strings.
> ---
>
> Changes in v2:
> - renamed PREFIX_SIZE into PREFIX_LENGTH
> - changed "name_length + PREFIX_LENGTH;" into "PREFIX_LENGTH + name_length;"
> - rebased on Peter's last patch series
>
> drivers/hid/hid-logitech-hidpp.c | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index 2f420c0..274dbb7 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -282,6 +282,33 @@ static inline bool hidpp_report_is_connect_event(struct hidpp_report *report)
> (report->rap.sub_id == 0x41);
> }
>
> +/**
> + * hidpp_prefix_name() prefixes the current given name with "Logitech ".
> + */
> +static void hidpp_prefix_name(char **name, int name_length)
> +{
> +#define PREFIX_LENGTH 9 /* "Logitech " */
> +
> + int new_length;
> + char *new_name;
> +
> + if (name_length > PREFIX_LENGTH &&
> + strncmp(*name, "Logitech ", PREFIX_LENGTH) == 0)
> + /* The prefix has is already in the name */
> + return;
> +
> + new_length = PREFIX_LENGTH + name_length;
> + new_name = kzalloc(new_length, GFP_KERNEL);
> + if (!new_name)
> + return;
> +
> + snprintf(new_name, new_length, "Logitech %s", *name);
> +
> + kfree(*name);
> +
> + *name = new_name;
> +}
> +
> /* -------------------------------------------------------------------------- */
> /* HIDP++ 1.0 commands */
> /* -------------------------------------------------------------------------- */
> @@ -321,6 +348,10 @@ static char *hidpp_get_unifying_name(struct hidpp_device *hidpp_dev)
> return NULL;
>
> memcpy(name, &response.rap.params[2], len);
> +
> + /* include the terminating '\0' */
> + hidpp_prefix_name(&name, len + 1);
> +
> return name;
> }
>
> @@ -498,6 +529,9 @@ static char *hidpp_get_device_name(struct hidpp_device *hidpp)
> index += ret;
> }
>
> + /* include the terminating '\0' */
> + hidpp_prefix_name(&name, __name_length + 1);
> +
> return name;
> }
>
>
--
Kind regards,
Peter
https://lekensteyn.nl
On Thu, 11 Dec 2014, Benjamin Tissoires wrote:
> Current names are reported as "K750", "M705", and it can be misleading
> for the users when they look at their input device list.
>
> Prefixing the names with "Logitech " makes things better.
>
> Signed-off-by: Benjamin Tissoires <[email protected]>
> ---
>
> Changes in v2:
> - renamed PREFIX_SIZE into PREFIX_LENGTH
> - changed "name_length + PREFIX_LENGTH;" into "PREFIX_LENGTH + name_length;"
> - rebased on Peter's last patch series
Ok, looks reasonable. I was waiting whether someone from Logitech would
comment on the prefix check that could be used other than "Logitech".
But we can add that later.
Applied to for-3.20/logitech, thanks.
--
Jiri Kosina
SUSE Labs
On Dec 17 2014 or thereabouts, Jiri Kosina wrote:
> On Thu, 11 Dec 2014, Benjamin Tissoires wrote:
>
> > Current names are reported as "K750", "M705", and it can be misleading
> > for the users when they look at their input device list.
> >
> > Prefixing the names with "Logitech " makes things better.
> >
> > Signed-off-by: Benjamin Tissoires <[email protected]>
> > ---
> >
> > Changes in v2:
> > - renamed PREFIX_SIZE into PREFIX_LENGTH
> > - changed "name_length + PREFIX_LENGTH;" into "PREFIX_LENGTH + name_length;"
> > - rebased on Peter's last patch series
>
> Ok, looks reasonable. I was waiting whether someone from Logitech would
> comment on the prefix check that could be used other than "Logitech".
>
> But we can add that later.
>
> Applied to for-3.20/logitech, thanks.
>
Hi Jiri,
Thanks for applying most of the patches (2 are missing, I'll raise them
in your inbox :-P )
Regarding this one, I was wondering if we could not force it into 3.19,
or at least add a stable@ tag. I had requested this in the first
submission, and the rationale was to not change a third time the name of
the device (from "Logitech Unifying Device. Wireless PID:XXXX" to
"[TMK]XXX" to "Logitech [TMK]XXX").
Userspace would be grateful to have a reliable name.
Cheers,
Benjamin
On Wed, 17 Dec 2014, Benjamin Tissoires wrote:
> Thanks for applying most of the patches (2 are missing, I'll raise them
> in your inbox :-P )
>
> Regarding this one, I was wondering if we could not force it into 3.19,
> or at least add a stable@ tag. I had requested this in the first
> submission, and the rationale was to not change a third time the name of
> the device (from "Logitech Unifying Device. Wireless PID:XXXX" to
> "[TMK]XXX" to "Logitech [TMK]XXX").
>
> Userspace would be grateful to have a reliable name.
Fair enough. I've cherry-picked it from for-3.20/logitech into
for-3.19/upstream-fixes as well.
Thanks,
--
Jiri Kosina
SUSE Labs