Re: [PATCH 1/3] mfd: apple-ibridge: Add Apple iBridge MFD driver.

From: Benjamin Tissoires
Date: Wed Apr 24 2019 - 10:18:40 EST


On Mon, Apr 22, 2019 at 5:13 AM Ronald TschalÃr <ronald@xxxxxxxxxxxxx> wrote:
>
> The iBridge device provides access to several devices, including:
> - the Touch Bar
> - the iSight webcam
> - the light sensor
> - the fingerprint sensor
>
> This driver provides the core support for managing the iBridge device
> and the access to the underlying devices. In particular, since the
> functionality for the touch bar and light sensor is exposed via USB HID
> interfaces, and the same HID device is used for multiple functions, this
> driver provides a multiplexing layer that allows multiple HID drivers to
> be registered for a given HID device. This allows the touch bar and ALS
> driver to be separated out into their own modules.

Sorry for coming late to the party, but IMO this series is far too
complex for what you need.

As I read this and the first comment of drivers/mfd/apple-ibridge.c,
you need to have a HID driver that multiplex 2 other sub drivers
through one USB communication.
For that, you are using MFD, platform driver and you own sauce instead
of creating a bus.

So, how about we reuse entirely the HID subsystem which already
provides the capability you need (assuming I am correct above).
hid-logitech-dj already does the same kind of stuff and you could:
- create drivers/hid/hid-ibridge.c that handles USB_ID_PRODUCT_IBRIDGE
- hid-ibridge will then register itself to the hid subsystem with a
call to hid_hw_start(hdev, HID_CONNECT_HIDRAW) and
hid_device_io_start(hdev) to enable the events (so you don't create
useless input nodes for it)
- then you add your 2 new devices by calling hid_allocate_device() and
then hid_add_device(). You can even create a new HID group
APPLE_IBRIDGE and allocate 2 new PIDs for them to distinguish them
from the actual USB device.
- then you have 2 brand new HID devices you can create their driver as
a regular ones.

hid-ibridge.c would just need to behave like any other hid transport
driver (see logi_dj_ll_driver in drivers/hid/hid-logitech-dj.c) and
you can get rid of at least the MFD and the platform part of your
drivers.

Does it makes sense or am I missing something obvious in the middle?


I have one other comment below.

Note that I haven't read the whole series as I'd like to first get
your feedback with my comment above.

>
> Signed-off-by: Ronald TschalÃr <ronald@xxxxxxxxxxxxx>
> ---
> drivers/mfd/Kconfig | 15 +
> drivers/mfd/Makefile | 1 +
> drivers/mfd/apple-ibridge.c | 883 ++++++++++++++++++++++++++++++
> include/linux/mfd/apple-ibridge.h | 39 ++
> 4 files changed, 938 insertions(+)
> create mode 100644 drivers/mfd/apple-ibridge.c
> create mode 100644 include/linux/mfd/apple-ibridge.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 76f9909cf396..d55fa77faacf 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1916,5 +1916,20 @@ config RAVE_SP_CORE
> Select this to get support for the Supervisory Processor
> device found on several devices in RAVE line of hardware.
>
> +config MFD_APPLE_IBRIDGE
> + tristate "Apple iBridge chip"
> + depends on ACPI
> + depends on USB_HID
> + depends on X86 || COMPILE_TEST
> + select MFD_CORE
> + help
> + This MFD provides the core support for the Apple iBridge chip
> + found on recent MacBookPro's. The drivers for the Touch Bar
> + (apple-ib-tb) and light sensor (apple-ib-als) need to be
> + enabled separately.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called apple-ibridge.
> +
> endmenu
> endif
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 12980a4ad460..c364e0e9d313 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -241,4 +241,5 @@ obj-$(CONFIG_MFD_MXS_LRADC) += mxs-lradc.o
> obj-$(CONFIG_MFD_SC27XX_PMIC) += sprd-sc27xx-spi.o
> obj-$(CONFIG_RAVE_SP_CORE) += rave-sp.o
> obj-$(CONFIG_MFD_ROHM_BD718XX) += rohm-bd718x7.o
> +obj-$(CONFIG_MFD_APPLE_IBRIDGE) += apple-ibridge.o
>
> diff --git a/drivers/mfd/apple-ibridge.c b/drivers/mfd/apple-ibridge.c
> new file mode 100644
> index 000000000000..56d325396961
> --- /dev/null
> +++ b/drivers/mfd/apple-ibridge.c
> @@ -0,0 +1,883 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Apple iBridge Driver
> + *
> + * Copyright (c) 2018 Ronald TschalÃr
> + */
> +
> +/**
> + * MacBookPro models with a Touch Bar (13,[23] and 14,[23]) have an Apple
> + * iBridge chip (also known as T1 chip) which exposes the touch bar,
> + * built-in webcam (iSight), ambient light sensor, and Secure Enclave
> + * Processor (SEP) for TouchID. It shows up in the system as a USB device
> + * with 3 configurations: 'Default iBridge Interfaces', 'Default iBridge
> + * Interfaces(OS X)', and 'Default iBridge Interfaces(Recovery)'. While
> + * the second one is used by MacOS to provide the fancy touch bar
> + * functionality with custom buttons etc, this driver just uses the first.
> + *
> + * In the first (default after boot) configuration, 4 usb interfaces are
> + * exposed: 2 related to the webcam, and 2 USB HID interfaces representing
> + * the touch bar and the ambient light sensor (and possibly the SEP,
> + * though at this point in time nothing is known about that). The webcam
> + * interfaces are already handled by the uvcvideo driver; furthermore, the
> + * handling of the input reports when "keys" on the touch bar are pressed
> + * is already handled properly by the generic USB HID core. This leaves
> + * the management of the touch bar modes (e.g. switching between function
> + * and special keys when the FN key is pressed), the touch bar display
> + * (dimming and turning off), the key-remapping when the FN key is
> + * pressed, and handling of the light sensor.
> + *
> + * This driver is implemented as an MFD driver, with the touch bar and ALS
> + * functions implemented by appropriate subdrivers (mfd cells). Because
> + * both those are basically hid drivers, but the current kernel driver
> + * structure does not allow more than one driver per device, this driver
> + * implements a demuxer for hid drivers: it registers itself as a hid
> + * driver with the core, and in turn it lets the subdrivers register
> + * themselves as hid drivers with this driver; the callbacks from the core
> + * are then forwarded to the subdrivers.
> + *
> + * Lastly, this driver also takes care of the power-management for the
> + * iBridge when suspending and resuming.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/device.h>
> +#include <linux/hid.h>
> +#include <linux/list.h>
> +#include <linux/mfd/apple-ibridge.h>
> +#include <linux/mfd/core.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/rculist.h>
> +#include <linux/slab.h>
> +#include <linux/srcu.h>
> +#include <linux/usb.h>
> +
> +#include "../hid/usbhid/usbhid.h"
> +
> +#define USB_ID_VENDOR_APPLE 0x05ac
> +#define USB_ID_PRODUCT_IBRIDGE 0x8600
> +
> +#define APPLETB_BASIC_CONFIG 1
> +
> +#define LOG_DEV(ib_dev) (&(ib_dev)->acpi_dev->dev)
> +
> +struct appleib_device {
> + struct acpi_device *acpi_dev;
> + acpi_handle asoc_socw;
> + struct list_head hid_drivers;
> + struct list_head hid_devices;
> + struct mfd_cell *subdevs;
> + struct mutex update_lock; /* protect updates to all lists */
> + struct srcu_struct lists_srcu;
> + bool in_hid_probe;
> +};
> +
> +struct appleib_hid_drv_info {
> + struct list_head entry;
> + struct hid_driver *driver;
> + void *driver_data;
> +};
> +
> +struct appleib_hid_dev_info {
> + struct list_head entry;
> + struct list_head drivers;
> + struct hid_device *device;
> + const struct hid_device_id *device_id;
> + bool started;
> +};
> +
> +static const struct mfd_cell appleib_subdevs[] = {
> + { .name = PLAT_NAME_IB_TB },
> + { .name = PLAT_NAME_IB_ALS },
> +};
> +
> +static struct appleib_device *appleib_dev;
> +
> +#define call_void_driver_func(drv_info, fn, ...) \
> + do { \
> + if ((drv_info)->driver->fn) \
> + (drv_info)->driver->fn(__VA_ARGS__); \
> + } while (0)
> +
> +#define call_driver_func(drv_info, fn, ret_type, ...) \
> + ({ \
> + ret_type rc = 0; \
> + \
> + if ((drv_info)->driver->fn) \
> + rc = (drv_info)->driver->fn(__VA_ARGS__); \
> + \
> + rc; \
> + })
> +
> +static void appleib_remove_driver(struct appleib_device *ib_dev,
> + struct appleib_hid_drv_info *drv_info,
> + struct appleib_hid_dev_info *dev_info)
> +{
> + list_del_rcu(&drv_info->entry);
> + synchronize_srcu(&ib_dev->lists_srcu);
> +
> + call_void_driver_func(drv_info, remove, dev_info->device);
> +
> + kfree(drv_info);
> +}
> +
> +static int appleib_probe_driver(struct appleib_hid_drv_info *drv_info,
> + struct appleib_hid_dev_info *dev_info)
> +{
> + struct appleib_hid_drv_info *d;
> + int rc = 0;
> +
> + rc = call_driver_func(drv_info, probe, int, dev_info->device,
> + dev_info->device_id);
> + if (rc)
> + return rc;
> +
> + d = kmemdup(drv_info, sizeof(*drv_info), GFP_KERNEL);
> + if (!d) {
> + call_void_driver_func(drv_info, remove, dev_info->device);
> + return -ENOMEM;
> + }
> +
> + list_add_tail_rcu(&d->entry, &dev_info->drivers);
> + return 0;
> +}
> +
> +static void appleib_remove_driver_attachments(struct appleib_device *ib_dev,
> + struct appleib_hid_dev_info *dev_info,
> + struct hid_driver *driver)
> +{
> + struct appleib_hid_drv_info *drv_info;
> + struct appleib_hid_drv_info *tmp;
> +
> + list_for_each_entry_safe(drv_info, tmp, &dev_info->drivers, entry) {
> + if (!driver || drv_info->driver == driver)
> + appleib_remove_driver(ib_dev, drv_info, dev_info);
> + }
> +}
> +
> +/*
> + * Find all devices that are attached to this driver and detach them.
> + *
> + * Note: this must be run with update_lock held.
> + */
> +static void appleib_detach_devices(struct appleib_device *ib_dev,
> + struct hid_driver *driver)
> +{
> + struct appleib_hid_dev_info *dev_info;
> +
> + list_for_each_entry(dev_info, &ib_dev->hid_devices, entry)
> + appleib_remove_driver_attachments(ib_dev, dev_info, driver);
> +}
> +
> +/*
> + * Find all drivers that are attached to this device and detach them.
> + *
> + * Note: this must be run with update_lock held.
> + */
> +static void appleib_detach_drivers(struct appleib_device *ib_dev,
> + struct appleib_hid_dev_info *dev_info)
> +{
> + appleib_remove_driver_attachments(ib_dev, dev_info, NULL);
> +}
> +
> +/**
> + * Unregister a previously registered HID driver from us.
> + * @ib_dev: the appleib_device from which to unregister the driver
> + * @driver: the driver to unregister
> + */
> +int appleib_unregister_hid_driver(struct appleib_device *ib_dev,
> + struct hid_driver *driver)
> +{
> + struct appleib_hid_drv_info *drv_info;
> +
> + mutex_lock(&ib_dev->update_lock);
> +
> + list_for_each_entry(drv_info, &ib_dev->hid_drivers, entry) {
> + if (drv_info->driver == driver) {
> + appleib_detach_devices(ib_dev, driver);
> + list_del_rcu(&drv_info->entry);
> + mutex_unlock(&ib_dev->update_lock);
> + synchronize_srcu(&ib_dev->lists_srcu);
> + kfree(drv_info);
> + dev_info(LOG_DEV(ib_dev), "unregistered driver '%s'\n",
> + driver->name);
> + return 0;
> + }
> + }
> +
> + mutex_unlock(&ib_dev->update_lock);
> +
> + return -ENOENT;
> +}
> +EXPORT_SYMBOL_GPL(appleib_unregister_hid_driver);
> +
> +static int appleib_start_hid_events(struct appleib_hid_dev_info *dev_info)
> +{
> + struct hid_device *hdev = dev_info->device;
> + int rc;
> +
> + rc = hid_connect(hdev, HID_CONNECT_DEFAULT);
> + if (rc) {
> + hid_err(hdev, "ib: hid connect failed (%d)\n", rc);
> + return rc;
> + }
> +
> + rc = hid_hw_open(hdev);
> + if (rc) {
> + hid_err(hdev, "ib: failed to open hid: %d\n", rc);
> + hid_disconnect(hdev);
> + }
> +
> + if (!rc)
> + dev_info->started = true;
> +
> + return rc;
> +}
> +
> +static void appleib_stop_hid_events(struct appleib_hid_dev_info *dev_info)
> +{
> + if (dev_info->started) {
> + hid_hw_close(dev_info->device);
> + hid_disconnect(dev_info->device);
> + dev_info->started = false;
> + }
> +}
> +
> +/**
> + * Register a HID driver with us.
> + * @ib_dev: the appleib_device with which to register the driver
> + * @driver: the driver to register
> + * @data: the driver-data to associate with the driver; this is available
> + * from appleib_get_drvdata(...).
> + */
> +int appleib_register_hid_driver(struct appleib_device *ib_dev,
> + struct hid_driver *driver, void *data)
> +{
> + struct appleib_hid_drv_info *drv_info;
> + struct appleib_hid_dev_info *dev_info;
> + int rc;
> +
> + if (!driver->probe)
> + return -EINVAL;
> +
> + drv_info = kzalloc(sizeof(*drv_info), GFP_KERNEL);
> + if (!drv_info)
> + return -ENOMEM;
> +
> + drv_info->driver = driver;
> + drv_info->driver_data = data;
> +
> + mutex_lock(&ib_dev->update_lock);
> +
> + list_add_tail_rcu(&drv_info->entry, &ib_dev->hid_drivers);
> +
> + list_for_each_entry(dev_info, &ib_dev->hid_devices, entry) {
> + appleib_stop_hid_events(dev_info);
> +
> + appleib_probe_driver(drv_info, dev_info);
> +
> + rc = appleib_start_hid_events(dev_info);
> + if (rc)
> + appleib_detach_drivers(ib_dev, dev_info);
> + }
> +
> + mutex_unlock(&ib_dev->update_lock);
> +
> + dev_info(LOG_DEV(ib_dev), "registered driver '%s'\n", driver->name);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(appleib_register_hid_driver);
> +
> +/**
> + * Get the driver-specific data associated with the given, previously
> + * registered HID driver (provided in the appleib_register_hid_driver()
> + * call).
> + * @ib_dev: the appleib_device with which the driver was registered
> + * @driver: the driver for which to get the data
> + */
> +void *appleib_get_drvdata(struct appleib_device *ib_dev,
> + struct hid_driver *driver)
> +{
> + struct appleib_hid_drv_info *drv_info;
> + void *drv_data = NULL;
> + int idx;
> +
> + idx = srcu_read_lock(&ib_dev->lists_srcu);
> +
> + list_for_each_entry_rcu(drv_info, &ib_dev->hid_drivers, entry) {
> + if (drv_info->driver == driver) {
> + drv_data = drv_info->driver_data;
> + break;
> + }
> + }
> +
> + srcu_read_unlock(&ib_dev->lists_srcu, idx);
> +
> + return drv_data;
> +}
> +EXPORT_SYMBOL_GPL(appleib_get_drvdata);
> +
> +/*
> + * Forward a hid-driver callback to all registered sub-drivers. This is for
> + * callbacks that return a status as an int.
> + * @hdev the hid-device
> + * @forward a function that calls the callback on the given driver
> + * @args arguments for the forward function
> + */
> +static int appleib_forward_int_op(struct hid_device *hdev,
> + int (*forward)(struct appleib_hid_drv_info *,
> + struct hid_device *, void *),
> + void *args)
> +{
> + struct appleib_device *ib_dev = hid_get_drvdata(hdev);
> + struct appleib_hid_dev_info *dev_info;
> + struct appleib_hid_drv_info *drv_info;
> + int idx;
> + int rc = 0;
> +
> + idx = srcu_read_lock(&ib_dev->lists_srcu);
> +
> + list_for_each_entry_rcu(dev_info, &ib_dev->hid_devices, entry) {
> + if (dev_info->device != hdev)
> + continue;
> +
> + list_for_each_entry_rcu(drv_info, &dev_info->drivers, entry) {
> + rc = forward(drv_info, hdev, args);
> + if (rc)
> + break;
> + }
> +
> + break;
> + }
> +
> + srcu_read_unlock(&ib_dev->lists_srcu, idx);
> +
> + return rc;
> +}
> +
> +struct appleib_hid_event_args {
> + struct hid_field *field;
> + struct hid_usage *usage;
> + __s32 value;
> +};
> +
> +static int appleib_hid_event_fwd(struct appleib_hid_drv_info *drv_info,
> + struct hid_device *hdev, void *args)
> +{
> + struct appleib_hid_event_args *evt_args = args;
> +
> + return call_driver_func(drv_info, event, int, hdev, evt_args->field,
> + evt_args->usage, evt_args->value);
> +}
> +
> +static int appleib_hid_event(struct hid_device *hdev, struct hid_field *field,
> + struct hid_usage *usage, __s32 value)
> +{
> + struct appleib_hid_event_args args = {
> + .field = field,
> + .usage = usage,
> + .value = value,
> + };
> +
> + return appleib_forward_int_op(hdev, appleib_hid_event_fwd, &args);
> +}
> +
> +static __u8 *appleib_report_fixup(struct hid_device *hdev, __u8 *rdesc,
> + unsigned int *rsize)
> +{
> + /* Some fields have a size of 64 bits, which according to HID 1.11
> + * Section 8.4 is not valid ("An item field cannot span more than 4
> + * bytes in a report"). Furthermore, hid_field_extract() complains

this must have been fixed in 94a9992f7dbdfb28976b565af220e0c4a117144a
which is part of v5.1, so not sure you actually need the report
descriptor fixup at all.

Cheers,
Benjamin

> + * when encountering such a field. So turn them into two 32-bit fields
> + * instead.
> + */
> +
> + if (*rsize == 634 &&
> + /* Usage Page 0xff12 (vendor defined) */
> + rdesc[212] == 0x06 && rdesc[213] == 0x12 && rdesc[214] == 0xff &&
> + /* Usage 0x51 */
> + rdesc[416] == 0x09 && rdesc[417] == 0x51 &&
> + /* report size 64 */
> + rdesc[432] == 0x75 && rdesc[433] == 64 &&
> + /* report count 1 */
> + rdesc[434] == 0x95 && rdesc[435] == 1) {
> + rdesc[433] = 32;
> + rdesc[435] = 2;
> + hid_dbg(hdev, "Fixed up first 64-bit field\n");
> + }
> +
> + if (*rsize == 634 &&
> + /* Usage Page 0xff12 (vendor defined) */
> + rdesc[212] == 0x06 && rdesc[213] == 0x12 && rdesc[214] == 0xff &&
> + /* Usage 0x51 */
> + rdesc[611] == 0x09 && rdesc[612] == 0x51 &&
> + /* report size 64 */
> + rdesc[627] == 0x75 && rdesc[628] == 64 &&
> + /* report count 1 */
> + rdesc[629] == 0x95 && rdesc[630] == 1) {
> + rdesc[628] = 32;
> + rdesc[630] = 2;
> + hid_dbg(hdev, "Fixed up second 64-bit field\n");
> + }
> +
> + return rdesc;
> +}
> +
> +static int appleib_input_configured_fwd(struct appleib_hid_drv_info *drv_info,
> + struct hid_device *hdev, void *args)
> +{
> + return call_driver_func(drv_info, input_configured, int, hdev,
> + (struct hid_input *)args);
> +}
> +
> +static int appleib_input_configured(struct hid_device *hdev,
> + struct hid_input *hidinput)
> +{
> + return appleib_forward_int_op(hdev, appleib_input_configured_fwd,
> + hidinput);
> +}
> +
> +#ifdef CONFIG_PM
> +static int appleib_hid_suspend_fwd(struct appleib_hid_drv_info *drv_info,
> + struct hid_device *hdev, void *args)
> +{
> + return call_driver_func(drv_info, suspend, int, hdev,
> + *(pm_message_t *)args);
> +}
> +
> +static int appleib_hid_suspend(struct hid_device *hdev, pm_message_t message)
> +{
> + return appleib_forward_int_op(hdev, appleib_hid_suspend_fwd, &message);
> +}
> +
> +static int appleib_hid_resume_fwd(struct appleib_hid_drv_info *drv_info,
> + struct hid_device *hdev, void *args)
> +{
> + return call_driver_func(drv_info, resume, int, hdev);
> +}
> +
> +static int appleib_hid_resume(struct hid_device *hdev)
> +{
> + return appleib_forward_int_op(hdev, appleib_hid_resume_fwd, NULL);
> +}
> +
> +static int appleib_hid_reset_resume_fwd(struct appleib_hid_drv_info *drv_info,
> + struct hid_device *hdev, void *args)
> +{
> + return call_driver_func(drv_info, reset_resume, int, hdev);
> +}
> +
> +static int appleib_hid_reset_resume(struct hid_device *hdev)
> +{
> + return appleib_forward_int_op(hdev, appleib_hid_reset_resume_fwd, NULL);
> +}
> +#endif /* CONFIG_PM */
> +
> +/**
> + * Find the field in the report with the given usage.
> + * @report: the report to search
> + * @field_usage: the usage of the field to search for
> + */
> +struct hid_field *appleib_find_report_field(struct hid_report *report,
> + unsigned int field_usage)
> +{
> + int f, u;
> +
> + for (f = 0; f < report->maxfield; f++) {
> + struct hid_field *field = report->field[f];
> +
> + if (field->logical == field_usage)
> + return field;
> +
> + for (u = 0; u < field->maxusage; u++) {
> + if (field->usage[u].hid == field_usage)
> + return field;
> + }
> + }
> +
> + return NULL;
> +}
> +EXPORT_SYMBOL_GPL(appleib_find_report_field);
> +
> +/**
> + * Search all the reports of the device for the field with the given usage.
> + * @hdev: the device whose reports to search
> + * @application: the usage of application collection that the field must
> + * belong to
> + * @field_usage: the usage of the field to search for
> + */
> +struct hid_field *appleib_find_hid_field(struct hid_device *hdev,
> + unsigned int application,
> + unsigned int field_usage)
> +{
> + static const int report_types[] = { HID_INPUT_REPORT, HID_OUTPUT_REPORT,
> + HID_FEATURE_REPORT };
> + struct hid_report *report;
> + struct hid_field *field;
> + int t;
> +
> + for (t = 0; t < ARRAY_SIZE(report_types); t++) {
> + struct list_head *report_list =
> + &hdev->report_enum[report_types[t]].report_list;
> + list_for_each_entry(report, report_list, list) {
> + if (report->application != application)
> + continue;
> +
> + field = appleib_find_report_field(report, field_usage);
> + if (field)
> + return field;
> + }
> + }
> +
> + return NULL;
> +}
> +EXPORT_SYMBOL_GPL(appleib_find_hid_field);
> +
> +/**
> + * Return whether we're currently inside a hid_device_probe or not.
> + * @ib_dev: the appleib_device
> + */
> +bool appleib_in_hid_probe(struct appleib_device *ib_dev)
> +{
> + return ib_dev->in_hid_probe;
> +}
> +EXPORT_SYMBOL_GPL(appleib_in_hid_probe);
> +
> +static struct appleib_hid_dev_info *
> +appleib_add_device(struct appleib_device *ib_dev, struct hid_device *hdev,
> + const struct hid_device_id *id)
> +{
> + struct appleib_hid_dev_info *dev_info;
> + struct appleib_hid_drv_info *drv_info;
> +
> + /* allocate device-info for this device */
> + dev_info = kzalloc(sizeof(*dev_info), GFP_KERNEL);
> + if (!dev_info)
> + return NULL;
> +
> + INIT_LIST_HEAD(&dev_info->drivers);
> + dev_info->device = hdev;
> + dev_info->device_id = id;
> +
> + /* notify all our sub drivers */
> + mutex_lock(&ib_dev->update_lock);
> +
> + ib_dev->in_hid_probe = true;
> +
> + list_for_each_entry(drv_info, &ib_dev->hid_drivers, entry) {
> + appleib_probe_driver(drv_info, dev_info);
> + }
> +
> + ib_dev->in_hid_probe = false;
> +
> + /* remember this new device */
> + list_add_tail_rcu(&dev_info->entry, &ib_dev->hid_devices);
> +
> + mutex_unlock(&ib_dev->update_lock);
> +
> + return dev_info;
> +}
> +
> +static void appleib_remove_device(struct appleib_device *ib_dev,
> + struct appleib_hid_dev_info *dev_info)
> +{
> + list_del_rcu(&dev_info->entry);
> + synchronize_srcu(&ib_dev->lists_srcu);
> +
> + appleib_detach_drivers(ib_dev, dev_info);
> +
> + kfree(dev_info);
> +}
> +
> +static int appleib_hid_probe(struct hid_device *hdev,
> + const struct hid_device_id *id)
> +{
> + struct appleib_device *ib_dev;
> + struct appleib_hid_dev_info *dev_info;
> + struct usb_device *udev;
> + int rc;
> +
> + /* check usb config first */
> + udev = hid_to_usb_dev(hdev);
> +
> + if (udev->actconfig->desc.bConfigurationValue != APPLETB_BASIC_CONFIG) {
> + rc = usb_driver_set_configuration(udev, APPLETB_BASIC_CONFIG);
> + return rc ? rc : -ENODEV;
> + }
> +
> + /* Assign the driver data */
> + ib_dev = appleib_dev;
> + hid_set_drvdata(hdev, ib_dev);
> +
> + /* initialize the report info */
> + rc = hid_parse(hdev);
> + if (rc) {
> + hid_err(hdev, "ib: hid parse failed (%d)\n", rc);
> + goto error;
> + }
> +
> + /* alloc bufs etc so probe's can send requests; but connect later */
> + rc = hid_hw_start(hdev, 0);
> + if (rc) {
> + hid_err(hdev, "ib: hw start failed (%d)\n", rc);
> + goto error;
> + }
> +
> + /* add this hdev to our device list */
> + dev_info = appleib_add_device(ib_dev, hdev, id);
> + if (!dev_info) {
> + rc = -ENOMEM;
> + goto stop_hw;
> + }
> +
> + /* start the hid */
> + rc = appleib_start_hid_events(dev_info);
> + if (rc)
> + goto remove_dev;
> +
> + return 0;
> +
> +remove_dev:
> + mutex_lock(&ib_dev->update_lock);
> + appleib_remove_device(ib_dev, dev_info);
> + mutex_unlock(&ib_dev->update_lock);
> +stop_hw:
> + hid_hw_stop(hdev);
> +error:
> + return rc;
> +}
> +
> +static void appleib_hid_remove(struct hid_device *hdev)
> +{
> + struct appleib_device *ib_dev = hid_get_drvdata(hdev);
> + struct appleib_hid_dev_info *dev_info;
> +
> + mutex_lock(&ib_dev->update_lock);
> +
> + list_for_each_entry(dev_info, &ib_dev->hid_devices, entry) {
> + if (dev_info->device == hdev) {
> + appleib_stop_hid_events(dev_info);
> + appleib_remove_device(ib_dev, dev_info);
> + break;
> + }
> + }
> +
> + mutex_unlock(&ib_dev->update_lock);
> +
> + hid_hw_stop(hdev);
> +}
> +
> +static const struct hid_device_id appleib_hid_devices[] = {
> + { HID_USB_DEVICE(USB_ID_VENDOR_APPLE, USB_ID_PRODUCT_IBRIDGE) },
> + { },
> +};
> +
> +static struct hid_driver appleib_hid_driver = {
> + .name = "apple-ibridge-hid",
> + .id_table = appleib_hid_devices,
> + .probe = appleib_hid_probe,
> + .remove = appleib_hid_remove,
> + .event = appleib_hid_event,
> + .report_fixup = appleib_report_fixup,
> + .input_configured = appleib_input_configured,
> +#ifdef CONFIG_PM
> + .suspend = appleib_hid_suspend,
> + .resume = appleib_hid_resume,
> + .reset_resume = appleib_hid_reset_resume,
> +#endif
> +};
> +
> +static struct appleib_device *appleib_alloc_device(struct acpi_device *acpi_dev)
> +{
> + struct appleib_device *ib_dev;
> + acpi_status sts;
> + int rc;
> +
> + /* allocate */
> + ib_dev = kzalloc(sizeof(*ib_dev), GFP_KERNEL);
> + if (!ib_dev)
> + return ERR_PTR(-ENOMEM);
> +
> + /* init structures */
> + INIT_LIST_HEAD(&ib_dev->hid_drivers);
> + INIT_LIST_HEAD(&ib_dev->hid_devices);
> + mutex_init(&ib_dev->update_lock);
> + init_srcu_struct(&ib_dev->lists_srcu);
> +
> + ib_dev->acpi_dev = acpi_dev;
> +
> + /* get iBridge acpi power control method */
> + sts = acpi_get_handle(acpi_dev->handle, "SOCW", &ib_dev->asoc_socw);
> + if (ACPI_FAILURE(sts)) {
> + dev_err(LOG_DEV(ib_dev),
> + "Error getting handle for ASOC.SOCW method: %s\n",
> + acpi_format_exception(sts));
> + rc = -ENXIO;
> + goto free_mem;
> + }
> +
> + /* ensure iBridge is powered on */
> + sts = acpi_execute_simple_method(ib_dev->asoc_socw, NULL, 1);
> + if (ACPI_FAILURE(sts))
> + dev_warn(LOG_DEV(ib_dev), "SOCW(1) failed: %s\n",
> + acpi_format_exception(sts));
> +
> + return ib_dev;
> +
> +free_mem:
> + kfree(ib_dev);
> + return ERR_PTR(rc);
> +}
> +
> +static int appleib_probe(struct acpi_device *acpi)
> +{
> + struct appleib_device *ib_dev;
> + struct appleib_platform_data *pdata;
> + int i;
> + int ret;
> +
> + if (appleib_dev)
> + return -EBUSY;
> +
> + ib_dev = appleib_alloc_device(acpi);
> + if (IS_ERR_OR_NULL(ib_dev))
> + return PTR_ERR(ib_dev);
> +
> + ib_dev->subdevs = kmemdup(appleib_subdevs, sizeof(appleib_subdevs),
> + GFP_KERNEL);
> + if (!ib_dev->subdevs) {
> + ret = -ENOMEM;
> + goto free_dev;
> + }
> +
> + pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
> + if (!pdata) {
> + ret = -ENOMEM;
> + goto free_subdevs;
> + }
> +
> + pdata->ib_dev = ib_dev;
> + pdata->log_dev = LOG_DEV(ib_dev);
> + for (i = 0; i < ARRAY_SIZE(appleib_subdevs); i++) {
> + ib_dev->subdevs[i].platform_data = pdata;
> + ib_dev->subdevs[i].pdata_size = sizeof(*pdata);
> + }
> +
> + ret = mfd_add_devices(&acpi->dev, PLATFORM_DEVID_NONE,
> + ib_dev->subdevs, ARRAY_SIZE(appleib_subdevs),
> + NULL, 0, NULL);
> + if (ret) {
> + dev_err(LOG_DEV(ib_dev), "Error adding MFD devices: %d\n", ret);
> + goto free_pdata;
> + }
> +
> + acpi->driver_data = ib_dev;
> + appleib_dev = ib_dev;
> +
> + ret = hid_register_driver(&appleib_hid_driver);
> + if (ret) {
> + dev_err(LOG_DEV(ib_dev), "Error registering hid driver: %d\n",
> + ret);
> + goto rem_mfd_devs;
> + }
> +
> + return 0;
> +
> +rem_mfd_devs:
> + mfd_remove_devices(&acpi->dev);
> +free_pdata:
> + kfree(pdata);
> +free_subdevs:
> + kfree(ib_dev->subdevs);
> +free_dev:
> + appleib_dev = NULL;
> + acpi->driver_data = NULL;
> + kfree(ib_dev);
> + return ret;
> +}
> +
> +static int appleib_remove(struct acpi_device *acpi)
> +{
> + struct appleib_device *ib_dev = acpi_driver_data(acpi);
> +
> + mfd_remove_devices(&acpi->dev);
> + hid_unregister_driver(&appleib_hid_driver);
> +
> + if (appleib_dev == ib_dev)
> + appleib_dev = NULL;
> +
> + kfree(ib_dev->subdevs[0].platform_data);
> + kfree(ib_dev->subdevs);
> + kfree(ib_dev);
> +
> + return 0;
> +}
> +
> +static int appleib_suspend(struct device *dev)
> +{
> + struct acpi_device *adev;
> + struct appleib_device *ib_dev;
> + int rc;
> +
> + adev = to_acpi_device(dev);
> + ib_dev = acpi_driver_data(adev);
> +
> + rc = acpi_execute_simple_method(ib_dev->asoc_socw, NULL, 0);
> + if (ACPI_FAILURE(rc))
> + dev_warn(LOG_DEV(ib_dev), "SOCW(0) failed: %s\n",
> + acpi_format_exception(rc));
> +
> + return 0;
> +}
> +
> +static int appleib_resume(struct device *dev)
> +{
> + struct acpi_device *adev;
> + struct appleib_device *ib_dev;
> + int rc;
> +
> + adev = to_acpi_device(dev);
> + ib_dev = acpi_driver_data(adev);
> +
> + rc = acpi_execute_simple_method(ib_dev->asoc_socw, NULL, 1);
> + if (ACPI_FAILURE(rc))
> + dev_warn(LOG_DEV(ib_dev), "SOCW(1) failed: %s\n",
> + acpi_format_exception(rc));
> +
> + return 0;
> +}
> +
> +static const struct dev_pm_ops appleib_pm = {
> + .suspend = appleib_suspend,
> + .resume = appleib_resume,
> + .restore = appleib_resume,
> +};
> +
> +static const struct acpi_device_id appleib_acpi_match[] = {
> + { "APP7777", 0 },
> + { },
> +};
> +
> +MODULE_DEVICE_TABLE(acpi, appleib_acpi_match);
> +
> +static struct acpi_driver appleib_driver = {
> + .name = "apple-ibridge",
> + .class = "topcase", /* ? */
> + .owner = THIS_MODULE,
> + .ids = appleib_acpi_match,
> + .ops = {
> + .add = appleib_probe,
> + .remove = appleib_remove,
> + },
> + .drv = {
> + .pm = &appleib_pm,
> + },
> +};
> +
> +module_acpi_driver(appleib_driver)
> +
> +MODULE_AUTHOR("Ronald TschalÃr");
> +MODULE_DESCRIPTION("Apple iBridge driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mfd/apple-ibridge.h b/include/linux/mfd/apple-ibridge.h
> new file mode 100644
> index 000000000000..d321714767f7
> --- /dev/null
> +++ b/include/linux/mfd/apple-ibridge.h
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Apple iBridge Driver
> + *
> + * Copyright (c) 2018 Ronald TschalÃr
> + */
> +
> +#ifndef __LINUX_MFD_APPLE_IBRDIGE_H
> +#define __LINUX_MFD_APPLE_IBRDIGE_H
> +
> +#include <linux/device.h>
> +#include <linux/hid.h>
> +
> +#define PLAT_NAME_IB_TB "apple-ib-tb"
> +#define PLAT_NAME_IB_ALS "apple-ib-als"
> +
> +struct appleib_device;
> +
> +struct appleib_platform_data {
> + struct appleib_device *ib_dev;
> + struct device *log_dev;
> +};
> +
> +int appleib_register_hid_driver(struct appleib_device *ib_dev,
> + struct hid_driver *driver, void *data);
> +int appleib_unregister_hid_driver(struct appleib_device *ib_dev,
> + struct hid_driver *driver);
> +
> +void *appleib_get_drvdata(struct appleib_device *ib_dev,
> + struct hid_driver *driver);
> +bool appleib_in_hid_probe(struct appleib_device *ib_dev);
> +
> +struct hid_field *appleib_find_report_field(struct hid_report *report,
> + unsigned int field_usage);
> +struct hid_field *appleib_find_hid_field(struct hid_device *hdev,
> + unsigned int application,
> + unsigned int field_usage);
> +
> +#endif
> --
> 2.20.1
>