2021-05-22 18:08:40

by José Expósito

[permalink] [raw]
Subject: [PATCH v2 1/5] HID: magicmouse: register power supply

Unlike the Apple Magic Mouse 1 and the Apple Magic Trackpad 1, the
second generation of the devices don't report their battery status
automatically.

This patchset adds support for reporting the battery capacity and
charging status for the Apple Magic Mouse 2 and Apple Magic Trackpad
2 both over bluetooth and USB.

This patch:

Register the required power supply structs for the Apple Magic Mouse 2
and the Apple Magic Trackpad 2 to be able to report battery capacity
and status in future patches.

Signed-off-by: José Expósito <[email protected]>

---

v2: Add depends on USB_HID to Kconfig
---
drivers/hid/hid-magicmouse.c | 90 ++++++++++++++++++++++++++++++++++++
1 file changed, 90 insertions(+)

diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index 2bb473d8c424..0f766bce4537 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -112,6 +112,9 @@ MODULE_PARM_DESC(report_undeciphered, "Report undeciphered multi-touch state fie
* @scroll_jiffies: Time of last scroll motion.
* @touches: Most recent data for a touch, indexed by tracking ID.
* @tracking_ids: Mapping of current touch input data to @touches.
+ * @battery: Required data to report the battery status of the Apple Magic
+ * Mouse 2 and Apple Magic Trackpad 2. Battery is reported automatically on the
+ * first generation of the devices.
*/
struct magicmouse_sc {
struct input_dev *input;
@@ -132,8 +135,89 @@ struct magicmouse_sc {

struct hid_device *hdev;
struct delayed_work work;
+
+ struct {
+ struct power_supply *ps;
+ struct power_supply_desc ps_desc;
+ } battery;
+};
+
+static enum power_supply_property magicmouse_ps_props[] = {
+ POWER_SUPPLY_PROP_PRESENT,
+ POWER_SUPPLY_PROP_SCOPE,
};

+static bool magicmouse_can_report_battery(struct magicmouse_sc *msc)
+{
+ return (msc->input->id.product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2) ||
+ (msc->input->id.product == USB_DEVICE_ID_APPLE_MAGICMOUSE2);
+}
+
+static int magicmouse_battery_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct magicmouse_sc *msc = power_supply_get_drvdata(psy);
+ int ret = 0;
+
+ if (!magicmouse_can_report_battery(msc))
+ return -EINVAL;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_PRESENT:
+ val->intval = 1;
+ break;
+ case POWER_SUPPLY_PROP_SCOPE:
+ val->intval = POWER_SUPPLY_SCOPE_DEVICE;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ return ret;
+}
+
+static int magicmouse_battery_probe(struct hid_device *hdev)
+{
+ struct magicmouse_sc *msc = hid_get_drvdata(hdev);
+ struct power_supply *ps = NULL;
+ struct power_supply_config ps_cfg = { .drv_data = msc };
+ int ret;
+
+ if (!magicmouse_can_report_battery(msc))
+ return 0;
+
+ msc->battery.ps_desc.type = POWER_SUPPLY_TYPE_BATTERY;
+ msc->battery.ps_desc.properties = magicmouse_ps_props;
+ msc->battery.ps_desc.num_properties = ARRAY_SIZE(magicmouse_ps_props);
+ msc->battery.ps_desc.get_property = magicmouse_battery_get_property;
+ msc->battery.ps_desc.name = kasprintf(GFP_KERNEL, "magic_trackpad_2_%s",
+ msc->input->uniq);
+ if (!msc->battery.ps_desc.name) {
+ hid_err(hdev, "unable to register ps_desc name, ENOMEM\n");
+ return -ENOMEM;
+ }
+
+ ps = devm_power_supply_register(&hdev->dev, &msc->battery.ps_desc,
+ &ps_cfg);
+ if (IS_ERR(ps)) {
+ ret = PTR_ERR(ps);
+ hid_err(hdev, "unable to register battery device: %d\n", ret);
+ return ret;
+ }
+
+ msc->battery.ps = ps;
+
+ ret = power_supply_powers(msc->battery.ps, &hdev->dev);
+ if (ret) {
+ hid_err(hdev, "unable to activate battery device: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
static int magicmouse_firm_touch(struct magicmouse_sc *msc)
{
int touch = -1;
@@ -726,6 +810,12 @@ static int magicmouse_probe(struct hid_device *hdev,
goto err_stop_hw;
}

+ ret = magicmouse_battery_probe(hdev);
+ if (ret) {
+ hid_err(hdev, "battery not registered\n");
+ goto err_stop_hw;
+ }
+
if (id->product == USB_DEVICE_ID_APPLE_MAGICMOUSE)
report = hid_register_report(hdev, HID_INPUT_REPORT,
MOUSE_REPORT_ID, 0);
--
2.25.1


2021-05-22 18:08:48

by José Expósito

[permalink] [raw]
Subject: [PATCH v2 3/5] HID: magicmouse: Magic Trackpad 2 USB battery capacity

Report the battery capacity percentage for the Apple Magic Trackpad 2
when it is connected over USB.

Signed-off-by: José Expósito <[email protected]>
---
drivers/hid/Kconfig | 2 +-
drivers/hid/hid-magicmouse.c | 136 +++++++++++++++++++++++++++++++++++
2 files changed, 137 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 4bf263c2d61a..f4856e5f5aa4 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -640,7 +640,7 @@ config LOGIWHEELS_FF

config HID_MAGICMOUSE
tristate "Apple Magic Mouse/Trackpad multi-touch support"
- depends on HID
+ depends on USB_HID
help
Support for the Apple Magic Mouse/Trackpad multi-touch.

diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index d4a58dd6d2b8..ea8a85767c39 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -16,6 +16,7 @@
#include <linux/input/mt.h>
#include <linux/module.h>
#include <linux/slab.h>
+#include <linux/usb.h>
#include <linux/workqueue.h>

#include "hid-ids.h"
@@ -58,6 +59,7 @@ MODULE_PARM_DESC(report_undeciphered, "Report undeciphered multi-touch state fie
#define MOUSE2_REPORT_ID 0x12
#define DOUBLE_REPORT_ID 0xf7
#define BT_BATTERY_REPORT_ID 0x90
+#define USB_BATTERY_EP_ADDR 0x81

/* These definitions are not precise, but they're close enough. (Bits
* 0x03 seem to indicate the aspect ratio of the touch, bits 0x70 seem
@@ -142,6 +144,10 @@ struct magicmouse_sc {
struct power_supply *ps;
struct power_supply_desc ps_desc;
int capacity;
+ struct urb *urb;
+ u8 *urb_buf;
+ int urb_buf_size;
+ dma_addr_t urb_buf_dma;
} battery;
};

@@ -231,6 +237,112 @@ static int magicmouse_battery_get_property(struct power_supply *psy,
return ret;
}

+static void magicmouse_battery_usb_urb_complete(struct urb *urb)
+{
+ struct magicmouse_sc *msc = urb->context;
+ int ret;
+
+ switch (urb->status) {
+ case 0:
+ msc->battery.capacity = msc->battery.urb_buf[2];
+ break;
+ case -EOVERFLOW:
+ hid_err(msc->hdev, "URB overflow\n");
+ fallthrough;
+ case -ECONNRESET:
+ case -ENOENT:
+ case -ESHUTDOWN:
+ return;
+ default:
+ break;
+ }
+
+ ret = usb_submit_urb(msc->battery.urb, GFP_ATOMIC);
+ if (ret)
+ hid_err(msc->hdev, "unable to submit URB, %d\n", ret);
+}
+
+static int magicmouse_battery_usb_probe(struct magicmouse_sc *msc)
+{
+ struct usb_interface *iface = to_usb_interface(msc->hdev->dev.parent);
+ struct usb_device *usbdev = interface_to_usbdev(iface);
+ struct usb_host_endpoint *endpoint = NULL;
+ u8 ep_address;
+ unsigned int pipe = 0;
+ int i, ret;
+
+ if (!magicmouse_can_report_battery_vendor(msc, USB_VENDOR_ID_APPLE))
+ return -EINVAL;
+
+ for (i = 0; i < sizeof(usbdev->ep_in); i++) {
+ endpoint = usbdev->ep_in[i];
+ if (endpoint) {
+ ep_address = endpoint->desc.bEndpointAddress;
+ if (ep_address == USB_BATTERY_EP_ADDR)
+ break;
+ }
+ }
+
+ if (!endpoint) {
+ hid_err(msc->hdev, "endpoint with address %d not found\n",
+ USB_BATTERY_EP_ADDR);
+ ret = -EIO;
+ goto exit;
+ }
+
+ msc->battery.urb = usb_alloc_urb(0, GFP_ATOMIC);
+ if (!msc->battery.urb) {
+ hid_err(msc->hdev, "unable to alloc URB, ENOMEM\n");
+ ret = -ENOMEM;
+ goto exit;
+ }
+
+ pipe = usb_rcvintpipe(usbdev, endpoint->desc.bEndpointAddress);
+ if (pipe == 0) {
+ hid_err(msc->hdev, "unable to create USB rcvintpipe\n");
+ ret = -EIO;
+ goto err_free_urb;
+ }
+
+ msc->battery.urb_buf_size = endpoint->desc.wMaxPacketSize;
+ msc->battery.urb_buf_dma = msc->battery.urb->transfer_dma;
+ msc->battery.urb_buf = usb_alloc_coherent(usbdev,
+ msc->battery.urb_buf_size, GFP_ATOMIC,
+ &msc->battery.urb_buf_dma);
+ if (!msc->battery.urb_buf) {
+ hid_err(msc->hdev, "unable to alloc URB buffer, ENOMEM\n");
+ ret = -ENOMEM;
+ goto err_free_urb;
+ }
+
+ usb_fill_int_urb(msc->battery.urb, usbdev, pipe, msc->battery.urb_buf,
+ msc->battery.urb_buf_size,
+ magicmouse_battery_usb_urb_complete, msc,
+ endpoint->desc.bInterval);
+
+ ret = usb_submit_urb(msc->battery.urb, GFP_ATOMIC);
+ if (ret) {
+ hid_err(msc->hdev, "unable to submit URB, %d\n", ret);
+ goto err_free_urb_buf;
+ }
+
+ return 0;
+
+err_free_urb_buf:
+ usb_free_coherent(usbdev, msc->battery.urb_buf_size,
+ msc->battery.urb_buf, msc->battery.urb_buf_dma);
+
+err_free_urb:
+ usb_free_urb(msc->battery.urb);
+
+exit:
+ msc->battery.urb = NULL;
+ msc->battery.urb_buf = NULL;
+ msc->battery.urb_buf_size = 0;
+
+ return ret;
+}
+
static int magicmouse_battery_probe(struct hid_device *hdev)
{
struct magicmouse_sc *msc = hid_get_drvdata(hdev);
@@ -269,6 +381,12 @@ static int magicmouse_battery_probe(struct hid_device *hdev)
return ret;
}

+ if (msc->input->id.vendor == USB_VENDOR_ID_APPLE) {
+ ret = magicmouse_battery_usb_probe(msc);
+ if (ret)
+ return ret;
+ }
+
return 0;
}

@@ -923,7 +1041,25 @@ static int magicmouse_probe(struct hid_device *hdev,
static void magicmouse_remove(struct hid_device *hdev)
{
struct magicmouse_sc *msc = hid_get_drvdata(hdev);
+ struct usb_interface *iface;
+ struct usb_device *usbdev;
cancel_delayed_work_sync(&msc->work);
+
+ if (msc &&
+ magicmouse_can_report_battery_vendor(msc, USB_VENDOR_ID_APPLE) &&
+ msc->battery.urb && msc->battery.urb_buf) {
+ iface = to_usb_interface(hdev->dev.parent);
+ usbdev = interface_to_usbdev(iface);
+
+ usb_kill_urb(msc->battery.urb);
+
+ usb_free_coherent(usbdev, msc->battery.urb_buf_size,
+ msc->battery.urb_buf,
+ msc->battery.urb_buf_dma);
+
+ usb_free_urb(msc->battery.urb);
+ }
+
hid_hw_stop(hdev);
}

--
2.25.1

2021-05-22 18:09:30

by José Expósito

[permalink] [raw]
Subject: [PATCH v2 5/5] HID: magicmouse: report battery status

Report the battery charging status for the Apple Magic Mouse 2
and the Apple Magic Trackpad 2.

Signed-off-by: José Expósito <[email protected]>
---
drivers/hid/hid-magicmouse.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index 53e8a10f0551..4085b6698f2c 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -155,6 +155,7 @@ static enum power_supply_property magicmouse_ps_props[] = {
POWER_SUPPLY_PROP_PRESENT,
POWER_SUPPLY_PROP_SCOPE,
POWER_SUPPLY_PROP_CAPACITY,
+ POWER_SUPPLY_PROP_STATUS,
};

static bool magicmouse_can_report_battery(struct magicmouse_sc *msc)
@@ -229,6 +230,15 @@ static int magicmouse_battery_get_property(struct power_supply *psy,

val->intval = msc->battery.capacity;
break;
+ case POWER_SUPPLY_PROP_STATUS:
+ if (msc->input->id.vendor == BT_VENDOR_ID_APPLE) {
+ val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+ } else { /* USB_VENDOR_ID_APPLE */
+ val->intval = (msc->battery.capacity == 100) ?
+ POWER_SUPPLY_STATUS_FULL :
+ POWER_SUPPLY_STATUS_CHARGING;
+ }
+ break;
default:
ret = -EINVAL;
break;
--
2.25.1

2021-05-22 18:10:42

by José Expósito

[permalink] [raw]
Subject: [PATCH v2 4/5] HID: magicmouse: Magic Mouse 2 USB battery capacity

Report the battery capacity percentage for the Apple Magic Mouse 2
when it is connected over USB.

Signed-off-by: José Expósito <[email protected]>
---
drivers/hid/hid-magicmouse.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index ea8a85767c39..53e8a10f0551 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -911,8 +911,17 @@ static int magicmouse_enable_multitouch(struct hid_device *hdev)
feature = feature_mt_trackpad2_usb;
}
} else if (hdev->product == USB_DEVICE_ID_APPLE_MAGICMOUSE2) {
- feature_size = sizeof(feature_mt_mouse2);
- feature = feature_mt_mouse2;
+ if (hdev->vendor == BT_VENDOR_ID_APPLE) {
+ feature_size = sizeof(feature_mt_mouse2);
+ feature = feature_mt_mouse2;
+ } else { /* USB_VENDOR_ID_APPLE */
+ /*
+ * The Magic Mouse 2 has the lightning connector on the
+ * bottom, making impossible to use it when it is
+ * charging.
+ */
+ return 0;
+ }
} else {
feature_size = sizeof(feature_mt);
feature = feature_mt;
@@ -947,7 +956,8 @@ static int magicmouse_probe(struct hid_device *hdev,
int ret;

if (id->vendor == USB_VENDOR_ID_APPLE &&
- id->product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2 &&
+ (id->product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2 ||
+ id->product == USB_DEVICE_ID_APPLE_MAGICMOUSE2) &&
hdev->type != HID_TYPE_USBMOUSE)
return 0;

@@ -1068,6 +1078,8 @@ static const struct hid_device_id magic_mice[] = {
USB_DEVICE_ID_APPLE_MAGICMOUSE), .driver_data = 0 },
{ HID_BLUETOOTH_DEVICE(BT_VENDOR_ID_APPLE,
USB_DEVICE_ID_APPLE_MAGICMOUSE2), .driver_data = 0 },
+ { HID_USB_DEVICE(USB_VENDOR_ID_APPLE,
+ USB_DEVICE_ID_APPLE_MAGICMOUSE2), .driver_data = 0 },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE,
USB_DEVICE_ID_APPLE_MAGICTRACKPAD), .driver_data = 0 },
{ HID_BLUETOOTH_DEVICE(BT_VENDOR_ID_APPLE,
--
2.25.1

2021-06-24 13:35:30

by Jiri Kosina

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] HID: magicmouse: register power supply

On Sat, 22 May 2021, José Expósito wrote:

> Unlike the Apple Magic Mouse 1 and the Apple Magic Trackpad 1, the
> second generation of the devices don't report their battery status
> automatically.
>
> This patchset adds support for reporting the battery capacity and
> charging status for the Apple Magic Mouse 2 and Apple Magic Trackpad
> 2 both over bluetooth and USB.
>
> This patch:
>
> Register the required power supply structs for the Apple Magic Mouse 2
> and the Apple Magic Trackpad 2 to be able to report battery capacity
> and status in future patches.
>
> Signed-off-by: José Expósito <[email protected]>
>
> ---
>
> v2: Add depends on USB_HID to Kconfig

Hmm, why is this dependency needed in the first place, please? I think
trying to keep the drivers independent on transport drivers (especially in
cases like this, where more variants of physical transports actually
really do exist) is worth trying.

Thanks,

--
Jiri Kosina
SUSE Labs

2021-06-25 17:10:19

by José Expósito

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] HID: magicmouse: register power supply

Hi Jiri,

First of all, thank you for taking the time to review my patches.

On Thu, Jun 24, 2021 at 03:33:39PM +0200, Jiri Kosina wrote:
> On Sat, 22 May 2021, Jos? Exp?sito wrote:
>
> > [...]
> > v2: Add depends on USB_HID to Kconfig
>
> Hmm, why is this dependency needed in the first place, please? I think
> trying to keep the drivers independent on transport drivers (especially in
> cases like this, where more variants of physical transports actually
> really do exist) is worth trying.

Sorry, that's something I should have explained in the changelog.

Intel's test bot reported compilation errors on the first version of the patch
when USB support wasn't configured:
https://lore.kernel.org/patchwork/patch/1425313/

I was kindly pointed to a similar error and its fix, but, maybe in this case this
is not the right fix?
Maybe there is a macro that I can use to wrap the USB related code in an #ifdef?

Thanks,
Jose

2021-07-28 09:36:24

by Jiri Kosina

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] HID: magicmouse: register power supply

On Fri, 25 Jun 2021, Jos? Exp?sito wrote:

> > > [...]
> > > v2: Add depends on USB_HID to Kconfig
> >
> > Hmm, why is this dependency needed in the first place, please? I think
> > trying to keep the drivers independent on transport drivers (especially in
> > cases like this, where more variants of physical transports actually
> > really do exist) is worth trying.
>
> Sorry, that's something I should have explained in the changelog.
>
> Intel's test bot reported compilation errors on the first version of the patch
> when USB support wasn't configured:
> https://lore.kernel.org/patchwork/patch/1425313/
>
> I was kindly pointed to a similar error and its fix, but, maybe in this case this
> is not the right fix?
> Maybe there is a macro that I can use to wrap the USB related code in an #ifdef?

It can certainly be wrapped, but looking into the code now, it probably
wouldn't really bring more clarity. I will apply the series with adding
the USB_HID dependency for now.

Thanks,

--
Jiri Kosina
SUSE Labs


2021-07-28 09:59:01

by José Expósito

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] HID: magicmouse: register power supply

On Wed, Jul 28, 2021 at 11:35:20AM +0200, Jiri Kosina wrote:
> On Fri, 25 Jun 2021, Jos? Exp?sito wrote:
>
> > > > [...]
> > > > v2: Add depends on USB_HID to Kconfig
> > >
> > > Hmm, why is this dependency needed in the first place, please? I think
> > > trying to keep the drivers independent on transport drivers (especially in
> > > cases like this, where more variants of physical transports actually
> > > really do exist) is worth trying.
> >
> > Sorry, that's something I should have explained in the changelog.
> >
> > Intel's test bot reported compilation errors on the first version of the patch
> > when USB support wasn't configured:
> > https://lore.kernel.org/patchwork/patch/1425313/
> >
> > I was kindly pointed to a similar error and its fix, but, maybe in this case this
> > is not the right fix?
> > Maybe there is a macro that I can use to wrap the USB related code in an #ifdef?
>
> It can certainly be wrapped, but looking into the code now, it probably
> wouldn't really bring more clarity. I will apply the series with adding
> the USB_HID dependency for now.

Hi Jiri,

I've been investigating a bit about this issue and I think this might not be the
righ solution for the problem.

John Chen's patch (9de07a4e8d4cb269f9876b2ffa282b5ffd09e05b):
https://lore.kernel.org/lkml/[email protected]/

Already adds battery reporting over bluetooth, so my patch is redundant... And worse
than his, I should add.

I was investigating how to do something similar over USB, but I couldn't finish a patch yet.

So, if you don't mind, I'd prefer not to apply this patchset yet until I figure out
a better solution on v3.

Thanks,
Jose

2021-07-28 10:08:09

by Jiri Kosina

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] HID: magicmouse: register power supply

On Wed, 28 Jul 2021, Jos? Exp?sito wrote:

> So, if you don't mind, I'd prefer not to apply this patchset yet until I
> figure out a better solution on v3.

Thanks for the heads up. I am dropping it for now.

--
Jiri Kosina
SUSE Labs