Re: [PATCH 2/6] HID: hid-playstation: Add touchpad_mouse param

[Benjamin Tissoires <benjamin.tissoires@redhat.com>]

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
>