Re: [PATCH v2 12/13] HID: playstation: DualSense set LEDs to default player id.

["Barnabás Pőcze" <pobrn@protonmail.com>]

Hi


2021. január 2., szombat 23:31 keltezéssel, Roderick Colenbrander írta:

> From: Roderick Colenbrander <roderick.colenbrander@sony.com>
>
> Add a ID allocator to assign player ids to ps_device instances.
> Utilize the player id to set a default color on the DualSense its
> player LED strip.
>
> Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com>
>
> [...]
> +static void ps_device_release_player_id(struct ps_device *dev)
> +{
> +	ida_free(&ps_player_id_allocator, dev->player_id);
> +
> +	dev->player_id = -1;

A minor thing, but I believe U32_MAX would be better here. I'd avoid
(especially) negative magic numbers for an unsigned value. You could even
  #define PS_PLAYER_ID_INVALID U32_MAX
or something similar.


> +}
> +
>  static struct input_dev *ps_allocate_input_dev(struct hid_device *hdev, const char *name_suffix)
>  {
>  	struct input_dev *input_dev;
> @@ -1102,6 +1125,28 @@  static int dualsense_reset_leds(struct dualsense *ds)
>  	return 0;
>  }
>
> +static void dualsense_set_player_leds(struct dualsense *ds)
> +{
> +	/* The DualSense controller has a row of 5 LEDs used for player ids.
> +	 * Behavior on the PlayStation 5 console is to center the player id
> +	 * across the LEDs, so e.g. player 1 would be "--x--" with x being 'on'.
> +	 * Follow a similar mapping here.
> +	 */
> +	int player_ids[5] = {
> +		BIT(2),
> +		BIT(3) | BIT(1),
> +		BIT(4) | BIT(2) | BIT(0),
> +		BIT(4) | BIT(3) | BIT(1) | BIT(0),
> +		BIT(4) | BIT(3) | BIT(2) | BIT(1) | BIT(0)
> +	};
> +
> +	uint8_t player_id = ds->base.player_id % 5;

I'd write `ds->base.player_id % ARRAY_SIZE(player_ids)` here.


> +
> +	ds->update_player_leds = true;
> +	ds->player_leds_state = player_ids[player_id];
> +	schedule_work(&ds->output_worker);
> +}
> [...]


Regards,
Barnabás Pőcze