| Message ID | 20200113192300.2482096-1-lains@archlinux.org (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Benjamin Tissoires |
| Headers | show |
| Series | HID: logitech-dj: add debug msg when exporting a HID++ report descriptors | expand |
Hi Filipe, On Tue, Jan 14, 2020 at 5:23 AM Filipe Laíns <lains@archlinux.org> wrote: > > When exporting all other types of report descriptors we print a debug > message. Not doing so for HID++ descriptors makes unaware users think > that no HID++ descriptor was exported. Unless I am mistaken, those dbg_hid() calls are not displayed by default on any distribution. So I am not sure what is the benefit to add this one here when we are already not showing the rest to the users by default. There is a tiny improvement to have some code symmetry, but here, honestly, it doesn't feel that required. Cheers, Benjamin > > Signed-off-by: Filipe Laíns <lains@archlinux.org> > --- > drivers/hid/hid-logitech-dj.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c > index cc7fc71d8b05..8f17a29b5a94 100644 > --- a/drivers/hid/hid-logitech-dj.c > +++ b/drivers/hid/hid-logitech-dj.c > @@ -1368,6 +1368,8 @@ static int logi_dj_ll_parse(struct hid_device *hid) > } > > if (djdev->reports_supported & HIDPP) { > + dbg_hid("%s: sending a HID++ descriptor, reports_supported: %llx\n", > + __func__, djdev->reports_supported); > rdcat(rdesc, &rsize, hidpp_descriptor, > sizeof(hidpp_descriptor)); > } > -- > 2.24.1 >
On Tue, 2020-01-14 at 11:24 +1000, Benjamin Tissoires wrote: > Hi Filipe, > > On Tue, Jan 14, 2020 at 5:23 AM Filipe Laíns <lains@archlinux.org> wrote: > > When exporting all other types of report descriptors we print a debug > > message. Not doing so for HID++ descriptors makes unaware users think > > that no HID++ descriptor was exported. > > Unless I am mistaken, those dbg_hid() calls are not displayed by > default on any distribution. So I am not sure what is the benefit to > add this one here when we are already not showing the rest to the > users by default. There is a tiny improvement to have some code > symmetry, but here, honestly, it doesn't feel that required. > > Cheers, > Benjamin Hi Benjamin, As far as I know, they aren't. This was just a nitpick. As I already had the code locally I decided to just go ahead and submit the patch. But noted, I won't submit any more of this nitpick patches. > > Signed-off-by: Filipe Laíns <lains@archlinux.org> > > --- > > drivers/hid/hid-logitech-dj.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c > > index cc7fc71d8b05..8f17a29b5a94 100644 > > --- a/drivers/hid/hid-logitech-dj.c > > +++ b/drivers/hid/hid-logitech-dj.c > > @@ -1368,6 +1368,8 @@ static int logi_dj_ll_parse(struct hid_device *hid) > > } > > > > if (djdev->reports_supported & HIDPP) { > > + dbg_hid("%s: sending a HID++ descriptor, reports_supported: %llx\n", > > + __func__, djdev->reports_supported); > > rdcat(rdesc, &rsize, hidpp_descriptor, > > sizeof(hidpp_descriptor)); > > } > > -- > > 2.24.1 > > Thank you, Filipe Laíns
On Mon, 13 Jan 2020, Filipe Laíns wrote: > When exporting all other types of report descriptors we print a debug > message. Not doing so for HID++ descriptors makes unaware users think > that no HID++ descriptor was exported. > > Signed-off-by: Filipe Laíns <lains@archlinux.org> > --- > drivers/hid/hid-logitech-dj.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c > index cc7fc71d8b05..8f17a29b5a94 100644 > --- a/drivers/hid/hid-logitech-dj.c > +++ b/drivers/hid/hid-logitech-dj.c > @@ -1368,6 +1368,8 @@ static int logi_dj_ll_parse(struct hid_device *hid) > } > > if (djdev->reports_supported & HIDPP) { > + dbg_hid("%s: sending a HID++ descriptor, reports_supported: %llx\n", > + __func__, djdev->reports_supported); > rdcat(rdesc, &rsize, hidpp_descriptor, > sizeof(hidpp_descriptor)); Applied, thanks.
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c index cc7fc71d8b05..8f17a29b5a94 100644 --- a/drivers/hid/hid-logitech-dj.c +++ b/drivers/hid/hid-logitech-dj.c @@ -1368,6 +1368,8 @@ static int logi_dj_ll_parse(struct hid_device *hid) } if (djdev->reports_supported & HIDPP) { + dbg_hid("%s: sending a HID++ descriptor, reports_supported: %llx\n", + __func__, djdev->reports_supported); rdcat(rdesc, &rsize, hidpp_descriptor, sizeof(hidpp_descriptor)); }
When exporting all other types of report descriptors we print a debug message. Not doing so for HID++ descriptors makes unaware users think that no HID++ descriptor was exported. Signed-off-by: Filipe Laíns <lains@archlinux.org> --- drivers/hid/hid-logitech-dj.c | 2 ++ 1 file changed, 2 insertions(+)