Hi
2021. január 2., szombat 23:31 keltezéssel, Roderick Colenbrander írta:
> From: Roderick Colenbrander <roderick.colenbrander@xxxxxxxx>
>
> 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@xxxxxxxx>
>
> [...]
> +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
