Re: [PATCH 04/13] HID: playstation: add DualSense touchpad support.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi


2020. december 19., szombat 7:23 keltezéssel, Roderick Colenbrander írta:

> [...]
> +static struct input_dev *ps_touchpad_create(struct hid_device *hdev, int width, int height,
> +		int num_contacts)

Very minor thing, but `input_mt_init_slots()` takes an `unsigned int`, so
wouldn't it be better if `num_contacts` was an `unsigned int`?


> +{
> +	struct input_dev *touchpad;
> +	int ret;
> +
> +	touchpad = ps_allocate_input_dev(hdev, "Touchpad");
> +	if (IS_ERR(touchpad))
> +		return ERR_PTR(-ENOMEM);
> +
> +	/* Map button underneath touchpad to BTN_LEFT. */
> +	input_set_capability(touchpad, EV_KEY, BTN_LEFT);
> +	__set_bit(INPUT_PROP_BUTTONPAD, touchpad->propbit);
> +
> +	input_set_abs_params(touchpad, ABS_MT_POSITION_X, 0, width, 0, 0);
> +	input_set_abs_params(touchpad, ABS_MT_POSITION_Y, 0, height, 0, 0);

Shouldn't it be `width - 1` and `height - 1`? Or what am I missing?


> +
> +	ret = input_mt_init_slots(touchpad, num_contacts, INPUT_MT_POINTER);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	ret = input_register_device(touchpad);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	return touchpad;
> +}
> +
>  static int dualsense_get_mac_address(struct dualsense *ds)
>  {
>  	uint8_t *buf;
> @@ -271,6 +304,7 @@ static int dualsense_parse_report(struct ps_device *ps_dev, struct hid_report *r
>  	uint8_t battery_data, battery_capacity, charging_status, value;
>  	int battery_status;
>  	unsigned long flags;
> +	int i;
>
>  	/* DualSense in USB uses the full HID report for reportID 1, but
>  	 * Bluetooth uses a minimal HID report for reportID 1 and reports
> @@ -311,6 +345,25 @@ static int dualsense_parse_report(struct ps_device *ps_dev, struct hid_report *r
>  	input_report_key(ds->gamepad, BTN_MODE, ds_report->buttons[2] & DS_BUTTONS2_PS_HOME);
>  	input_sync(ds->gamepad);
>
> +	input_report_key(ds->touchpad, BTN_LEFT, ds_report->buttons[2] & DS_BUTTONS2_TOUCHPAD);
> +	for (i = 0; i < 2; i++) {
> +		bool active = (ds_report->points[i].contact & 0x80) ? false : true;
> [...]

I believe it'd be preferable to give a name to that 0x80.


Regards,
Barnabás Pőcze




[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux