Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190422031251.11968-1-ronald@innovation.ch> <20190422031251.11968-2-ronald@innovation.ch>
In-Reply-To: <20190422031251.11968-2-ronald@innovation.ch>
From:   Benjamin Tissoires <benjamin.tissoires@redhat.com>
Date:   Wed, 24 Apr 2019 16:18:23 +0200
Message-ID: <CAO-hwJLMZvBGheEtH_iU+mBtE4T3OsT6W+=1x-ewzGGP5Fxp4Q@mail.gmail.com>
Subject: Re: [PATCH 1/3] mfd: apple-ibridge: Add Apple iBridge MFD driver.
To:     =?UTF-8?Q?Ronald_Tschal=C3=A4r?= <ronald@innovation.ch>
Cc:     Jiri Kosina <jikos@kernel.org>,
        Jonathan Cameron <jic23@kernel.org>,
        Hartmut Knaack <knaack.h@gmx.de>,
        Lars-Peter Clausen <lars@metafoo.de>,
        Peter Meerwald-Stadler <pmeerw@pmeerw.net>,
        Lee Jones <lee.jones@linaro.org>,
        "open list:HID CORE LAYER" <linux-input@vger.kernel.org>,
        linux-iio@vger.kernel.org, lkml <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Apr 22, 2019 at 5:13 AM Ronald Tschalär <ronald@innovation.ch> 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@innovation.ch>
> ---
>  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
>