From: Szymon Janc <szymon.janc@codecoup.pl>
To: Bastien Nocera <hadess@hadess.net>
Cc: linux-bluetooth@vger.kernel.org
Subject: Re: [PATCH 1/9] plugins/sixaxis: Remove LEDs handling
Date: Wed, 27 Sep 2017 11:07:43 +0200 [thread overview]
Message-ID: <11501873.MWAtCWdrlt@ix> (raw)
In-Reply-To: <20170904181212.5639-1-hadess@hadess.net>
Hi Bastien,
On Monday, 4 September 2017 20:12:04 CEST Bastien Nocera wrote:
> It's done in the kernel since 2014 in linux kernel commit
> 8025087acf9d2b941bae93b3e0967560e7e03e87
> ---
> plugins/sixaxis.c | 295
> +----------------------------------------------------- 1 file changed, 5
> insertions(+), 290 deletions(-)
>
> diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c
> index 7e3c950b4..7d3a8f3ac 100644
> --- a/plugins/sixaxis.c
> +++ b/plugins/sixaxis.c
> @@ -71,17 +71,6 @@ static const struct {
> },
> };
>
> -struct leds_data {
> - char *syspath_prefix;
> - uint8_t bitmap;
> -};
> -
> -static void leds_data_destroy(struct leds_data *data)
> -{
> - free(data->syspath_prefix);
> - free(data);
> -}
> -
> static struct udev *ctx = NULL;
> static struct udev_monitor *monitor = NULL;
> static guint watch_id = 0;
> @@ -146,115 +135,6 @@ static int set_master_bdaddr(int fd, const bdaddr_t
> *bdaddr) return ret;
> }
>
> -static uint8_t calc_leds_bitmap(int number)
> -{
> - uint8_t bitmap = 0;
> -
> - /* TODO we could support up to 10 (1 + 2 + 3 + 4) */
> - if (number > 7)
> - return bitmap;
> -
> - if (number > 4) {
> - bitmap |= 0x10;
> - number -= 4;
> - }
> -
> - bitmap |= 0x01 << number;
> -
> - return bitmap;
> -}
> -
> -static void set_leds_hidraw(int fd, uint8_t leds_bitmap)
> -{
> - /*
> - * the total time the led is active (0xff means forever)
> - * | duty_length: cycle time in deciseconds (0 - "blink very fast")
> - * | | ??? (Maybe a phase shift or duty_length multiplier?)
> - * | | | % of duty_length led is off (0xff means 100%)
> - * | | | | % of duty_length led is on (0xff means 100%)
> - * | | | | |
> - * 0xff, 0x27, 0x10, 0x00, 0x32,
> - */
> - uint8_t leds_report[] = {
> - 0x01,
> - 0x00, 0x00, 0x00, 0x00, 0x00, /* rumble values TBD */
> - 0x00, 0x00, 0x00, 0x00, 0x00, /* LED_1=0x02, LED_2=0x04 ... */
> - 0xff, 0x27, 0x10, 0x00, 0x32, /* LED_4 */
> - 0xff, 0x27, 0x10, 0x00, 0x32, /* LED_3 */
> - 0xff, 0x27, 0x10, 0x00, 0x32, /* LED_2 */
> - 0xff, 0x27, 0x10, 0x00, 0x32, /* LED_1 */
> - 0x00, 0x00, 0x00, 0x00, 0x00,
> - };
> - int ret;
> -
> - leds_report[10] = leds_bitmap;
> -
> - ret = write(fd, leds_report, sizeof(leds_report));
> - if (ret == sizeof(leds_report))
> - return;
> -
> - if (ret < 0)
> - error("sixaxis: failed to set LEDS (%s)", strerror(errno));
> - else
> - error("sixaxis: failed to set LEDS (%d bytes written)", ret);
> -}
> -
> -static bool set_leds_sysfs(struct leds_data *data)
> -{
> - int i;
> -
> - if (!data->syspath_prefix)
> - return false;
> -
> - /* start from 1, LED0 is never used */
> - for (i = 1; i <= 4; i++) {
> - char path[PATH_MAX] = { 0 };
> - char buf[2] = { 0 };
> - int fd;
> - int ret;
> -
> - snprintf(path, PATH_MAX, "%s%d/brightness",
> - data->syspath_prefix, i);
> -
> - fd = open(path, O_WRONLY);
> - if (fd < 0) {
> - error("sixaxis: cannot open %s (%s)", path,
> - strerror(errno));
> - return false;
> - }
> -
> - buf[0] = '0' + !!(data->bitmap & (1 << i));
> - ret = write(fd, buf, sizeof(buf));
> - close(fd);
> - if (ret != sizeof(buf))
> - return false;
> - }
> -
> - return true;
> -}
> -
> -static gboolean setup_leds(GIOChannel *channel, GIOCondition cond,
> - gpointer user_data)
> -{
> - struct leds_data *data = user_data;
> -
> - if (!data)
> - return FALSE;
> -
> - if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL))
> - goto out;
> -
> - if(!set_leds_sysfs(data)) {
> - int fd = g_io_channel_unix_get_fd(channel);
> - set_leds_hidraw(fd, data->bitmap);
> - }
> -
> -out:
> - leds_data_destroy(data);
> -
> - return FALSE;
> -}
> -
> static bool setup_device(int fd, int index, struct btd_adapter *adapter)
> {
> char device_addr[18], master_addr[18], adapter_addr[18];
> @@ -269,8 +149,7 @@ static bool setup_device(int fd, int index, struct
> btd_adapter *adapter) return false;
>
> /* This can happen if controller was plugged while already connected
> - * eg. to charge up battery.
> - * Don't set LEDs in that case, hence return false */
> + * eg. to charge up battery. */
> device = btd_adapter_find_device(adapter, &device_bdaddr,
> BDADDR_BREDR);
> if (device && btd_device_is_connected(device))
> @@ -307,152 +186,6 @@ static bool setup_device(int fd, int index, struct
> btd_adapter *adapter) return true;
> }
>
> -static int get_js_number(struct udev_device *udevice)
> -{
> - struct udev_list_entry *devices, *dev_list_entry;
> - struct udev_enumerate *enumerate;
> - struct udev_device *hid_parent;
> - const char *hidraw_node;
> - const char *hid_id;
> - int number = 0;
> -
> - hid_parent = udev_device_get_parent_with_subsystem_devtype(udevice,
> - "hid", NULL);
> -
> - /*
> - * Look for HID_UNIQ first for the correct behavior via BT, if
> - * HID_UNIQ is not available it means the USB bus is being used and we
> - * can rely on HID_PHYS.
> - */
> - hid_id = udev_device_get_property_value(hid_parent, "HID_UNIQ");
> - if (!hid_id)
> - hid_id = udev_device_get_property_value(hid_parent,
> - "HID_PHYS");
> -
> - hidraw_node = udev_device_get_devnode(udevice);
> - if (!hid_id || !hidraw_node)
> - return 0;
> -
> - enumerate = udev_enumerate_new(udev_device_get_udev(udevice));
> - udev_enumerate_add_match_sysname(enumerate, "js*");
> - udev_enumerate_scan_devices(enumerate);
> - devices = udev_enumerate_get_list_entry(enumerate);
> -
> - udev_list_entry_foreach(dev_list_entry, devices) {
> - struct udev_device *input_parent;
> - struct udev_device *js_dev;
> - const char *input_id;
> - const char *devname;
> -
> - devname = udev_list_entry_get_name(dev_list_entry);
> - js_dev = udev_device_new_from_syspath(
> - udev_device_get_udev(udevice),
> - devname);
> -
> - input_parent = udev_device_get_parent_with_subsystem_devtype(
> - js_dev, "input", NULL);
> - if (!input_parent)
> - goto next;
> -
> - /* check if this is the joystick relative to the hidraw device
> - * above */
> - input_id = udev_device_get_sysattr_value(input_parent, "uniq");
> -
> - /*
> - * A strlen() check is needed because input device over USB
> - * have the UNIQ attribute defined but with an empty value.
> - */
> - if (!input_id || strlen(input_id) == 0)
> - input_id = udev_device_get_sysattr_value(input_parent,
> - "phys");
> -
> - if (!input_id)
> - goto next;
> -
> - if (!strcmp(input_id, hid_id)) {
> - number = atoi(udev_device_get_sysnum(js_dev));
> -
> - /* joystick numbers start from 0, leds from 1 */
> - number++;
> -
> - udev_device_unref(js_dev);
> - break;
> - }
> -next:
> - udev_device_unref(js_dev);
> - }
> -
> - udev_enumerate_unref(enumerate);
> -
> - return number;
> -}
> -
> -static char *get_leds_syspath_prefix(struct udev_device *udevice)
> -{
> - struct udev_list_entry *dev_list_entry;
> - struct udev_enumerate *enumerate;
> - struct udev_device *hid_parent;
> - const char *syspath;
> - char *syspath_prefix;
> -
> - hid_parent = udev_device_get_parent_with_subsystem_devtype(udevice,
> - "hid", NULL);
> -
> - enumerate = udev_enumerate_new(udev_device_get_udev(udevice));
> - udev_enumerate_add_match_parent(enumerate, hid_parent);
> - udev_enumerate_add_match_subsystem(enumerate, "leds");
> - udev_enumerate_scan_devices(enumerate);
> -
> - dev_list_entry = udev_enumerate_get_list_entry(enumerate);
> - if (!dev_list_entry) {
> - syspath_prefix = NULL;
> - goto out;
> - }
> -
> - syspath = udev_list_entry_get_name(dev_list_entry);
> -
> - /*
> - * All the sysfs paths of the LEDs have the same structure, just the
> - * number changes, so strip it and store only the common prefix.
> - *
> - * Subtracting 1 here means assuming that the LED number is a single
> - * digit, this is safe as the kernel driver only exposes 4 LEDs.
> - */
> - syspath_prefix = strndup(syspath, strlen(syspath) - 1);
> -
> -out:
> - udev_enumerate_unref(enumerate);
> -
> - return syspath_prefix;
> -}
> -
> -static struct leds_data *get_leds_data(struct udev_device *udevice)
> -{
> - struct leds_data *data;
> - int number;
> -
> - number = get_js_number(udevice);
> - DBG("number %d", number);
> -
> - data = malloc0(sizeof(*data));
> - if (!data)
> - return NULL;
> -
> - data->bitmap = calc_leds_bitmap(number);
> - if (data->bitmap == 0) {
> - leds_data_destroy(data);
> - return NULL;
> - }
> -
> - /*
> - * It's OK if this fails, set_leds_hidraw() will be used in
> - * case data->syspath_prefix is NULL.
> - */
> - data->syspath_prefix = get_leds_syspath_prefix(udevice);
> -
> - return data;
> -}
> -
> static int get_supported_device(struct udev_device *udevice, uint16_t *bus)
> {
> struct udev_device *hid_parent;
> @@ -481,7 +214,6 @@ static int get_supported_device(struct udev_device
> *udevice, uint16_t *bus) static void device_added(struct udev_device
> *udevice)
> {
> struct btd_adapter *adapter;
> - GIOChannel *io;
> uint16_t bus;
> int index;
> int fd;
> @@ -493,6 +225,8 @@ static void device_added(struct udev_device *udevice)
> index = get_supported_device(udevice, &bus);
> if (index < 0)
> return;
> + if (bus != BUS_USB)
> + return;
>
> info("sixaxis: compatible device connected: %s (%04X:%04X)",
> devices[index].name, devices[index].vid,
> @@ -502,27 +236,8 @@ static void device_added(struct udev_device *udevice)
> if (fd < 0)
> return;
>
> - io = g_io_channel_unix_new(fd);
> -
> - switch (bus) {
> - case BUS_USB:
> - if (!setup_device(fd, index, adapter))
> - break;
> -
> - /* fall through */
> - case BUS_BLUETOOTH:
> - /* wait for events before setting leds */
> - g_io_add_watch(io, G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
> - setup_leds, get_leds_data(udevice));
> -
> - break;
> - default:
> - DBG("uknown bus type (%u)", bus);
> - break;
> - }
> -
> - g_io_channel_set_close_on_unref(io, TRUE);
> - g_io_channel_unref(io);
> + setup_device(fd, index, adapter);
> + close(fd);
> }
>
> static gboolean monitor_watch(GIOChannel *source, GIOCondition condition,
This patch is now applied, thanks.
--
pozdrawiam
Szymon Janc
prev parent reply other threads:[~2017-09-27 9:07 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-04 18:12 [PATCH 1/9] plugins/sixaxis: Remove LEDs handling Bastien Nocera
2017-09-04 18:12 ` [PATCH 2/9] adapter: Add btd_request_authorization_cable_configured() Bastien Nocera
2017-09-04 18:12 ` [PATCH 3/9] sixaxis: Ask user whether cable configuration should be allowed Bastien Nocera
2017-09-27 9:12 ` Szymon Janc
2017-10-18 1:51 ` Bastien Nocera
2017-09-27 9:14 ` Szymon Janc
2017-10-04 12:38 ` Bastien Nocera
2017-09-04 18:12 ` [PATCH 4/9] plugins/sixaxis: Move device discovery to shared header Bastien Nocera
2017-09-05 9:03 ` Bastien Nocera
2017-09-05 9:13 ` Szymon Janc
2017-09-05 10:37 ` Bastien Nocera
2017-09-04 18:12 ` [PATCH 5/9] profiles/input: Use sixaxis header to simplify device detection Bastien Nocera
2017-09-04 18:12 ` [PATCH 6/9] profiles/input: Add DS4 devices to the shared header Bastien Nocera
2017-09-04 18:12 ` [PATCH 7/9] plugins/sixaxis: Rename sixaxis specific functions Bastien Nocera
2017-09-04 18:12 ` [PATCH 8/9] plugins/sixaxis: Add support for DualShock 4/PS4 cable pairing Bastien Nocera
2017-09-27 9:12 ` Szymon Janc
2017-10-04 12:40 ` Bastien Nocera
2017-10-18 1:51 ` Bastien Nocera
2017-09-04 18:12 ` [PATCH 9/9] plugins/sixaxis: Cancel cable pairing if unplugged Bastien Nocera
2017-09-27 9:07 ` Szymon Janc [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=11501873.MWAtCWdrlt@ix \
--to=szymon.janc@codecoup.pl \
--cc=hadess@hadess.net \
--cc=linux-bluetooth@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).