On Fri, 3 Jan 2014 16:03:46 -0500 (EST) Frank Praznik <[email protected]> wrote:
> This patch builds on the previous patch and adds controls for the light Just use "Add controls for ..." > bar on the Dualshock 4. The light bar contains a red, green and blue LED > that can independently vary in brightness from 0 to 255. The LED system > in the module had to be extended to support a full byte per led for > storing the brightness level as well as storing a count of LEDs on each > device. Like the Sixaxis, LED status is set in the worker function by > setting certain bytes in the data packet. > > Like the force feedback patch, this meant to be applied against the > for-3.14/sony branch in jiros/hid.git > Annotation :) Ah and remember to CC Jiri Kosina when you send the v2, you can use scripts/get_maintainer.pl to get some _hints_ about where to send a patch. Some more nitpicking below. > Signed-off-by: Frank Praznik <[email protected]> > > --- > drivers/hid/hid-sony.c | 77 > ++++++++++++++++++++++++++++++++------------------ > 1 file changed, 49 insertions(+), 28 deletions(-) > > diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c > index f42c866..445b5f2 100644 > --- a/drivers/hid/hid-sony.c > +++ b/drivers/hid/hid-sony.c > @@ -40,7 +40,9 @@ > #define PS3REMOTE BIT(4) > #define DUALSHOCK4_CONTROLLER BIT(5) > > -#define SONY_LED_SUPPORT (SIXAXIS_CONTROLLER_USB | BUZZ_CONTROLLER) > +#define SONY_LED_SUPPORT (SIXAXIS_CONTROLLER_USB | BUZZ_CONTROLLER | > DUALSHOCK4_CONTROLLER) > + > +#define MAX_LEDS 4 > > static const u8 sixaxis_rdesc_fixup[] = { > 0x95, 0x13, 0x09, 0x01, 0x81, 0x02, 0x95, 0x0C, > @@ -227,7 +229,7 @@ static const unsigned int buzz_keymap[] = { > > struct sony_sc { > struct hid_device *hdev; > - struct led_classdev *leds[4]; > + struct led_classdev *leds[MAX_LEDS]; > unsigned long quirks; > struct work_struct state_worker; > > @@ -236,7 +238,8 @@ struct sony_sc { > __u8 right; > #endif > > - __u8 led_state; > + __u8 led_state[MAX_LEDS]; > + __u8 led_count; > }; > > static __u8 *ps3remote_fixup(struct hid_device *hdev, __u8 *rdesc, > @@ -447,7 +450,7 @@ static int sixaxis_set_operational_bt(struct hid_device > *hdev) > return hdev->hid_output_raw_report(hdev, buf, sizeof(buf), > HID_FEATURE_REPORT); > } > > -static void buzz_set_leds(struct hid_device *hdev, int leds) > +static void buzz_set_leds(struct hid_device *hdev, const __u8 *leds) > { > struct list_head *report_list = > &hdev->report_enum[HID_OUTPUT_REPORT].report_list; > @@ -456,23 +459,27 @@ static void buzz_set_leds(struct hid_device *hdev, int > leds) > __s32 *value = report->field[0]->value; > > value[0] = 0x00; > - value[1] = (leds & 1) ? 0xff : 0x00; > - value[2] = (leds & 2) ? 0xff : 0x00; > - value[3] = (leds & 4) ? 0xff : 0x00; > - value[4] = (leds & 8) ? 0xff : 0x00; > + value[1] = leds[0] ? 0xff : 0x00; > + value[2] = leds[1] ? 0xff : 0x00; > + value[3] = leds[2] ? 0xff : 0x00; > + value[4] = leds[3] ? 0xff : 0x00; > value[5] = 0x00; > value[6] = 0x00; > hid_hw_request(hdev, report, HID_REQ_SET_REPORT); > } > > -static void sony_set_leds(struct hid_device *hdev, __u8 leds) > +static void sony_set_leds(struct hid_device *hdev, const __u8 *leds, int > count) > { > struct sony_sc *drv_data = hid_get_drvdata(hdev); > - > - if (drv_data->quirks & BUZZ_CONTROLLER) { > + int n; > + > + BUG_ON(count > MAX_LEDS); > + > + if (drv_data->quirks & BUZZ_CONTROLLER && count == 4) { > buzz_set_leds(hdev, leds); > - } else if (drv_data->quirks & SIXAXIS_CONTROLLER_USB) { > - drv_data->led_state = leds; > + } else if ((drv_data->quirks & SIXAXIS_CONTROLLER_USB) || > (drv_data->quirks & DUALSHOCK4_CONTROLLER)) { > + for(n = 0; n < count; n++) > + drv_data->led_state[n] = leds[n]; > schedule_work(&drv_data->state_worker); > } > } > @@ -492,15 +499,11 @@ static void sony_led_set_brightness(struct led_classdev > *led, > return; > } > > - for (n = 0; n < 4; n++) { > + for (n = 0; n < drv_data->led_count; n++) { > if (led == drv_data->leds[n]) { > - int on = !!(drv_data->led_state & (1 << n)); > - if (value == LED_OFF && on) { > - drv_data->led_state &= ~(1 << n); > - sony_set_leds(hdev, drv_data->led_state); > - } else if (value != LED_OFF && !on) { > - drv_data->led_state |= (1 << n); > - sony_set_leds(hdev, drv_data->led_state); > + if (value != drv_data->led_state[n]) { > + drv_data->led_state[n] = value; > + sony_set_leds(hdev, drv_data->led_state, > drv_data->led_count); > } > break; > } > @@ -522,9 +525,9 @@ static enum led_brightness sony_led_get_brightness(struct > led_classdev *led) > return LED_OFF; > } > > - for (n = 0; n < 4; n++) { > + for (n = 0; n < drv_data->led_count; n++) { > if (led == drv_data->leds[n]) { > - on = !!(drv_data->led_state & (1 << n)); > + on = !!(drv_data->led_state[n]); > break; > } > } > @@ -541,7 +544,7 @@ static void sony_leds_remove(struct hid_device *hdev) > drv_data = hid_get_drvdata(hdev); > BUG_ON(!(drv_data->quirks & SONY_LED_SUPPORT)); > > - for (n = 0; n < 4; n++) { > + for (n = 0; n < drv_data->led_count; n++) { > led = drv_data->leds[n]; > drv_data->leds[n] = NULL; > if (!led) > @@ -549,17 +552,21 @@ static void sony_leds_remove(struct hid_device *hdev) > led_classdev_unregister(led); > kfree(led); > } > + > + drv_data->led_count = 0; > } > > static int sony_leds_init(struct hid_device *hdev) > { > struct sony_sc *drv_data; > int n, ret = 0; > + int max_brightness = 1; you could avoid initialization here and set the value in the check below, it'd look more symmetrical to me. > struct led_classdev *led; > size_t name_sz; > char *name; > size_t name_len; > const char *name_fmt; > + static const __u8 initial_values[MAX_LEDS] = { 0x00, 0x00, 0x00, 0x00 }; > > drv_data = hid_get_drvdata(hdev); > BUG_ON(!(drv_data->quirks & SONY_LED_SUPPORT)); > @@ -574,15 +581,22 @@ static int sony_leds_init(struct hid_device *hdev) > name_len = strlen("::sony#"); > name_fmt = "%s::sony%d"; > } > + > + if (drv_data->quirks & DUALSHOCK4_CONTROLLER) { > + drv_data->led_count = 3; > + max_brightness = 255; > + } > + else > + drv_data->led_count = 4; > Also the else block requires curly braces, see Documentation/CodingStyle maybe scripts/checkpatch.pl catches these issues. > /* Clear LEDs as we have no way of reading their initial state. This is > * only relevant if the driver is loaded after somebody actively set the > * LEDs to on */ > - sony_set_leds(hdev, 0x00); > + sony_set_leds(hdev, initial_values, drv_data->led_count); > > name_sz = strlen(dev_name(&hdev->dev)) + name_len + 1; > > - for (n = 0; n < 4; n++) { > + for (n = 0; n < drv_data->led_count; n++) { > led = kzalloc(sizeof(struct led_classdev) + name_sz, > GFP_KERNEL); > if (!led) { > hid_err(hdev, "Couldn't allocate memory for LED %d\n", > n); > @@ -594,7 +608,7 @@ static int sony_leds_init(struct hid_device *hdev) > snprintf(name, name_sz, name_fmt, dev_name(&hdev->dev), n + 1); > led->name = name; > led->brightness = 0; > - led->max_brightness = 1; > + led->max_brightness = max_brightness; > led->brightness_get = sony_led_get_brightness; > led->brightness_set = sony_led_set_brightness; > > @@ -635,7 +649,10 @@ static void sixaxis_state_worker(struct work_struct > *work) > buf[5] = sc->left; > #endif > > - buf[10] |= (sc->led_state & 0xf) << 1; > + buf[10] |= sc->led_state[0] << 1; > + buf[10] |= sc->led_state[1] << 2; > + buf[10] |= sc->led_state[2] << 3; > + buf[10] |= sc->led_state[3] << 4; > > sc->hdev->hid_output_raw_report(sc->hdev, buf, sizeof(buf), > HID_OUTPUT_REPORT); > @@ -660,6 +677,10 @@ static void dualshock4_state_worker(struct work_struct > *work) > buf[5] = sc->left; > #endif > > + buf[6] = sc->led_state[0]; > + buf[7] = sc->led_state[1]; > + buf[8] = sc->led_state[2]; > + > sc->hdev->hid_output_raw_report(sc->hdev, buf, sizeof(buf), > HID_OUTPUT_REPORT); > } > -- > 1.8.3.2 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-input" in > the body of a message to [email protected] > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- Antonio Ospite http://ao2.it A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
