Re: [PATCH] HID: logitech-dj: issue udev change event on device connection

[Hans de Goede]

Hi,

On 3/18/20 6:15 PM, Mario Limonciello wrote:
> On Wed, Mar 18, 2020 at 11:19 AM Filipe LaÃns <lains@xxxxxxxxxxxxx> wrote:
> > As discussed in the mailing list:
> >
> > > Right now the hid-logitech-dj driver will export one node for each
> > > connected device, even when the device is not connected. That causes
> > > some trouble because in userspace we don't have have any way to know if
> > > the device is connected or not, so when we try to communicate, if the
> > > device is disconnected it will fail.
> >
> > The solution reached to solve this issue is to trigger an udev change
> > event when the device connects, this way userspace can just wait on
> > those connections instead of trying to ping the device.
> >
> > Signed-off-by: Filipe LaÃns <lains@xxxxxxxxxxxxx>
> > ---
> >   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 48dff5d6b605..fcd481a0be1f 100644
> > --- a/drivers/hid/hid-logitech-dj.c
> > +++ b/drivers/hid/hid-logitech-dj.c
> > @@ -1464,6 +1464,8 @@ static int logi_dj_dj_event(struct hid_device *hdev,
> >                  if (dj_report->report_params[CONNECTION_STATUS_PARAM_STATUS] ==
> >                      STATUS_LINKLOSS) {
> >                          logi_dj_recv_forward_null_report(djrcv_dev, dj_report);
> > +               } else {
> > +                       kobject_uevent(&hdev->dev.kobj, KOBJ_CHANGE);
> >                  }
> >                  break;
> >          default:
> > --
> > 2.25.1
>
> The problem that will remain here is the transition period for
> userspace to start to rely upon
> this.  It will have no idea whether the kernel is expected to send
> events or not.  What do you
> think about adding a syfs attribute to indicate that events are being
> sent?  Or something similar?

Then we would need to support that attribute forever. IMHO the best
option is to just make a uname call and check the kernel version, with
the code marked to be removed in the future when kernels older then
$version are no longer something we want to support.

Regards,

Hans