| Message ID | 20220427224526.35657-2-vi@endrift.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/6] HID: hid-playstation: Allow removal of touchpad | expand |
Hi Vicki, On Thu, Apr 28, 2022 at 12:52 AM Vicki Pfau <vi@endrift.com> wrote: > > Add parameter "touchpad_mouse" to enable disabling or re-enabling exposing the > touchpad input_dev, which can be changed while the module is loaded. What's the point of exposing this new parameter? Patch 3/6 automatically disables touchpad when hidraw is opened, so who will be the user of this parameter? The problem I have with kernel parameter is that they are effectively kernel API, and we need to keep them forever, so I'd rather have good arguments on why this is needed. Cheers, Benjamin > > Signed-off-by: Vicki Pfau <vi@endrift.com> > --- > drivers/hid/hid-playstation.c | 41 +++++++++++++++++++++++++++++++++++ > 1 file changed, 41 insertions(+) > > diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c > index f859a8dd8a2e..ad0da4470615 100644 > --- a/drivers/hid/hid-playstation.c > +++ b/drivers/hid/hid-playstation.c > @@ -23,6 +23,8 @@ static LIST_HEAD(ps_devices_list); > > static DEFINE_IDA(ps_player_id_allocator); > > +static bool touchpad_mouse = true; > + > #define HID_PLAYSTATION_VERSION_PATCH 0x8000 > > /* Base class for playstation devices. */ > @@ -1343,6 +1345,45 @@ static void ps_remove(struct hid_device *hdev) > hid_hw_stop(hdev); > } > > +static int ps_param_set_touchpad_mouse(const char *val, > + const struct kernel_param *kp) > +{ > + struct ps_device *dev; > + struct dualsense *ds; > + struct input_dev *touchpad; > + int ret; > + > + ret = param_set_bool(val, kp); > + if (ret) > + return ret; > + > + mutex_lock(&ps_devices_lock); > + list_for_each_entry(dev, &ps_devices_list, list) { > + mutex_lock(&dev->mutex); > + if (dev->hdev->product == USB_DEVICE_ID_SONY_PS5_CONTROLLER) { > + ds = container_of(dev, struct dualsense, base); > + if (touchpad_mouse) { > + touchpad = ps_touchpad_create(dev->hdev, DS_TOUCHPAD_WIDTH, DS_TOUCHPAD_HEIGHT, 2); > + if (IS_ERR(touchpad)) > + continue; > + rcu_assign_pointer(ds->touchpad, touchpad); > + } else > + dualsense_unregister_touchpad(ds); > + } > + mutex_unlock(&dev->mutex); > + } > + mutex_unlock(&ps_devices_lock); > + return 0; > +} > + > +static const struct kernel_param_ops ps_touchpad_mouse_ops = { > + .set = ps_param_set_touchpad_mouse, > + .get = param_get_bool, > +}; > + > +module_param_cb(touchpad_mouse, &ps_touchpad_mouse_ops, &touchpad_mouse, 0644); > +MODULE_PARM_DESC(touchpad_mouse, "Enable mouse emulation using the touchpad"); > + > static const struct hid_device_id ps_devices[] = { > { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS5_CONTROLLER) }, > { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS5_CONTROLLER) }, > -- > 2.36.0 >
Hello, On 5/5/22 01:52, Benjamin Tissoires wrote: > Hi Vicki, > > On Thu, Apr 28, 2022 at 12:52 AM Vicki Pfau <vi@endrift.com> wrote: >> >> Add parameter "touchpad_mouse" to enable disabling or re-enabling exposing the >> touchpad input_dev, which can be changed while the module is loaded. > > What's the point of exposing this new parameter? > Patch 3/6 automatically disables touchpad when hidraw is opened, so > who will be the user of this parameter? Generally speaking, the touchpad shows up as a mouse, even though on other OSes it is only presented via gamepad APIs. This is, well, very frustrating for a lot of users, and having to tell libinput or evdev or xinput or whatever to ignore it every time you plug it in (if you're not using the hidraw directly) is...suboptimal. This was an attempt at a way to just generically say "don't do that". > > The problem I have with kernel parameter is that they are effectively > kernel API, and we need to keep them forever, so I'd rather have good > arguments on why this is needed. Ah, this is probably not a good enough argument to justify that then. Some other way will probably be preferred. > > Cheers, > Benjamin > >> >> Signed-off-by: Vicki Pfau <vi@endrift.com> >> --- >> drivers/hid/hid-playstation.c | 41 +++++++++++++++++++++++++++++++++++ >> 1 file changed, 41 insertions(+) >> >> diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c >> index f859a8dd8a2e..ad0da4470615 100644 >> --- a/drivers/hid/hid-playstation.c >> +++ b/drivers/hid/hid-playstation.c >> @@ -23,6 +23,8 @@ static LIST_HEAD(ps_devices_list); >> >> static DEFINE_IDA(ps_player_id_allocator); >> >> +static bool touchpad_mouse = true; >> + >> #define HID_PLAYSTATION_VERSION_PATCH 0x8000 >> >> /* Base class for playstation devices. */ >> @@ -1343,6 +1345,45 @@ static void ps_remove(struct hid_device *hdev) >> hid_hw_stop(hdev); >> } >> >> +static int ps_param_set_touchpad_mouse(const char *val, >> + const struct kernel_param *kp) >> +{ >> + struct ps_device *dev; >> + struct dualsense *ds; >> + struct input_dev *touchpad; >> + int ret; >> + >> + ret = param_set_bool(val, kp); >> + if (ret) >> + return ret; >> + >> + mutex_lock(&ps_devices_lock); >> + list_for_each_entry(dev, &ps_devices_list, list) { >> + mutex_lock(&dev->mutex); >> + if (dev->hdev->product == USB_DEVICE_ID_SONY_PS5_CONTROLLER) { >> + ds = container_of(dev, struct dualsense, base); >> + if (touchpad_mouse) { >> + touchpad = ps_touchpad_create(dev->hdev, DS_TOUCHPAD_WIDTH, DS_TOUCHPAD_HEIGHT, 2); >> + if (IS_ERR(touchpad)) >> + continue; >> + rcu_assign_pointer(ds->touchpad, touchpad); >> + } else >> + dualsense_unregister_touchpad(ds); >> + } >> + mutex_unlock(&dev->mutex); >> + } >> + mutex_unlock(&ps_devices_lock); >> + return 0; >> +} >> + >> +static const struct kernel_param_ops ps_touchpad_mouse_ops = { >> + .set = ps_param_set_touchpad_mouse, >> + .get = param_get_bool, >> +}; >> + >> +module_param_cb(touchpad_mouse, &ps_touchpad_mouse_ops, &touchpad_mouse, 0644); >> +MODULE_PARM_DESC(touchpad_mouse, "Enable mouse emulation using the touchpad"); >> + >> static const struct hid_device_id ps_devices[] = { >> { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS5_CONTROLLER) }, >> { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS5_CONTROLLER) }, >> -- >> 2.36.0 >> > Vicki
diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c index f859a8dd8a2e..ad0da4470615 100644 --- a/drivers/hid/hid-playstation.c +++ b/drivers/hid/hid-playstation.c @@ -23,6 +23,8 @@ static LIST_HEAD(ps_devices_list); static DEFINE_IDA(ps_player_id_allocator); +static bool touchpad_mouse = true; + #define HID_PLAYSTATION_VERSION_PATCH 0x8000 /* Base class for playstation devices. */ @@ -1343,6 +1345,45 @@ static void ps_remove(struct hid_device *hdev) hid_hw_stop(hdev); } +static int ps_param_set_touchpad_mouse(const char *val, + const struct kernel_param *kp) +{ + struct ps_device *dev; + struct dualsense *ds; + struct input_dev *touchpad; + int ret; + + ret = param_set_bool(val, kp); + if (ret) + return ret; + + mutex_lock(&ps_devices_lock); + list_for_each_entry(dev, &ps_devices_list, list) { + mutex_lock(&dev->mutex); + if (dev->hdev->product == USB_DEVICE_ID_SONY_PS5_CONTROLLER) { + ds = container_of(dev, struct dualsense, base); + if (touchpad_mouse) { + touchpad = ps_touchpad_create(dev->hdev, DS_TOUCHPAD_WIDTH, DS_TOUCHPAD_HEIGHT, 2); + if (IS_ERR(touchpad)) + continue; + rcu_assign_pointer(ds->touchpad, touchpad); + } else + dualsense_unregister_touchpad(ds); + } + mutex_unlock(&dev->mutex); + } + mutex_unlock(&ps_devices_lock); + return 0; +} + +static const struct kernel_param_ops ps_touchpad_mouse_ops = { + .set = ps_param_set_touchpad_mouse, + .get = param_get_bool, +}; + +module_param_cb(touchpad_mouse, &ps_touchpad_mouse_ops, &touchpad_mouse, 0644); +MODULE_PARM_DESC(touchpad_mouse, "Enable mouse emulation using the touchpad"); + static const struct hid_device_id ps_devices[] = { { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS5_CONTROLLER) }, { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS5_CONTROLLER) },
Add parameter "touchpad_mouse" to enable disabling or re-enabling exposing the touchpad input_dev, which can be changed while the module is loaded. Signed-off-by: Vicki Pfau <vi@endrift.com> --- drivers/hid/hid-playstation.c | 41 +++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+)