[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.DEB.2.21.1904221101420.964@vps.pmeerw.net>
Date: Mon, 22 Apr 2019 11:17:27 +0200 (CEST)
From: Peter Meerwald-Stadler <pmeerw@...erw.net>
To: Ronald Tschalär <ronald@...ovation.ch>
cc: Jiri Kosina <jikos@...nel.org>,
Benjamin Tissoires <benjamin.tissoires@...hat.com>,
Jonathan Cameron <jic23@...nel.org>,
Hartmut Knaack <knaack.h@....de>,
Lars-Peter Clausen <lars@...afoo.de>,
Lee Jones <lee.jones@...aro.org>, linux-input@...r.kernel.org,
linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] iio: light: apple-ib-als: Add driver for ALS on
iBridge chip.
On Sun, 21 Apr 2019, Ronald Tschalär wrote:
> On 2016/2017 MacBook Pro's with a Touch Bar the ALS is attached to,
> and exposed via the iBridge device. This provides the driver for that
> sensor.
some comments below inline
> Signed-off-by: Ronald Tschalär <ronald@...ovation.ch>
> ---
> drivers/iio/light/Kconfig | 12 +
> drivers/iio/light/Makefile | 1 +
> drivers/iio/light/apple-ib-als.c | 694 +++++++++++++++++++++++++++++++
> 3 files changed, 707 insertions(+)
> create mode 100644 drivers/iio/light/apple-ib-als.c
>
> diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
> index 36f458433480..49159fab1c0e 100644
> --- a/drivers/iio/light/Kconfig
> +++ b/drivers/iio/light/Kconfig
> @@ -64,6 +64,18 @@ config APDS9960
> To compile this driver as a module, choose M here: the
> module will be called apds9960
>
> +config APPLE_IBRIDGE_ALS
> + tristate "Apple iBridge ambient light sensor"
> + select IIO_BUFFER
> + select IIO_TRIGGERED_BUFFER
> + depends on MFD_APPLE_IBRIDGE
> + help
> + Say Y here to build the driver for the Apple iBridge ALS
> + sensor.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called apple-ib-als.
> +
> config BH1750
> tristate "ROHM BH1750 ambient light sensor"
> depends on I2C
> diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
> index 286bf3975372..144d918917f7 100644
> --- a/drivers/iio/light/Makefile
> +++ b/drivers/iio/light/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_ADJD_S311) += adjd_s311.o
> obj-$(CONFIG_AL3320A) += al3320a.o
> obj-$(CONFIG_APDS9300) += apds9300.o
> obj-$(CONFIG_APDS9960) += apds9960.o
> +obj-$(CONFIG_APPLE_IBRIDGE_ALS) += apple-ib-als.o
> obj-$(CONFIG_BH1750) += bh1750.o
> obj-$(CONFIG_BH1780) += bh1780.o
> obj-$(CONFIG_CM32181) += cm32181.o
> diff --git a/drivers/iio/light/apple-ib-als.c b/drivers/iio/light/apple-ib-als.c
> new file mode 100644
> index 000000000000..1718fcbe304f
> --- /dev/null
> +++ b/drivers/iio/light/apple-ib-als.c
> @@ -0,0 +1,694 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Apple Ambient Light Sensor Driver
> + *
> + * Copyright (c) 2017-2018 Ronald Tschalär
> + */
> +
> +/*
> + * MacBookPro models with an iBridge chip (13,[23] and 14,[23]) have an
> + * ambient light sensor that is exposed via one of the USB interfaces on
> + * the iBridge as a standard HID light sensor. However, we cannot use the
> + * existing hid-sensor-als driver, for two reasons:
> + *
> + * 1. The hid-sensor-als driver is part of the hid-sensor-hub which in turn
> + * is a hid driver, but you can't have more than one hid driver per hid
> + * device, which is a problem because the touch bar also needs to
> + * register as a driver for this hid device.
> + *
> + * 2. While the hid-sensors-als driver stores sensor readings received via
> + * interrupt in an iio buffer, reads on the sysfs
> + * .../iio:deviceX/in_illuminance_YYY attribute result in a get of the
> + * feature report; however, in the case of this sensor here the
> + * illuminance field of that report is always 0. Instead, the input
> + * report needs to be requested.
> + */
> +
> +#define dev_fmt(fmt) "als: " fmt
> +
> +#include <linux/device.h>
> +#include <linux/hid.h>
> +#include <linux/hid-sensor-ids.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/mfd/apple-ibridge.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#define APPLEALS_DYN_SENS 0 /* our dynamic sensitivity */
> +#define APPLEALS_DEF_CHANGE_SENS APPLEALS_DYN_SENS
> +
> +struct appleals_device {
> + struct appleib_device *ib_dev;
> + struct device *log_dev;
> + struct hid_device *hid_dev;
> + struct hid_report *cfg_report;
> + struct hid_field *illum_field;
> + struct iio_dev *iio_dev;
> + struct iio_trigger *iio_trig;
> + int cur_sensitivity;
> + int cur_hysteresis;
> + bool events_enabled;
> +};
> +
> +static struct hid_driver appleals_hid_driver;
> +
> +/*
> + * This is a primitive way to get a relative sensitivity, one where we get
> + * notified when the value changes by a certain percentage rather than some
> + * absolute value. MacOS somehow manages to configure the sensor to work this
> + * way (with a 15% relative sensitivity), but I haven't been able to figure
> + * out how so far. So until we do, this provides a less-than-perfect
> + * simulation.
> + *
> + * When the brightness value is within one of the ranges, the sensitivity is
> + * set to that range's sensitivity. But in order to reduce flapping when the
> + * brightness is right on the border between two ranges, the ranges overlap
> + * somewhat (by at least one sensitivity), and sensitivity is only changed if
> + * the value leaves the current sensitivity's range.
> + *
> + * The values chosen for the map are somewhat arbitrary: a compromise of not
> + * too many ranges (and hence changing the sensitivity) but not too small or
> + * large of a percentage of the min and max values in the range (currently
> + * from 7.5% to 30%, i.e. within a factor of 2 of 15%), as well as just plain
> + * "this feels reasonable to me".
> + */
> +struct appleals_sensitivity_map {
> + int sensitivity;
> + int illum_low;
> + int illum_high;
> +};
> +
> +static struct appleals_sensitivity_map appleals_sensitivity_map[] = {
const?
> + { 1, 0, 14 },
> + { 3, 10, 40 },
> + { 9, 30, 120 },
> + { 27, 90, 360 },
> + { 81, 270, 1080 },
> + { 243, 810, 3240 },
> + { 729, 2430, 9720 },
> +};
> +
> +static int appleals_compute_sensitivity(int cur_illum, int cur_sens)
> +{
> + struct appleals_sensitivity_map *entry;
> + int i;
> +
> + /* see if we're still in current range */
> + for (i = 0; i < ARRAY_SIZE(appleals_sensitivity_map); i++) {
> + entry = &appleals_sensitivity_map[i];
> +
> + if (entry->sensitivity == cur_sens &&
> + entry->illum_low <= cur_illum &&
> + entry->illum_high >= cur_illum)
> + return cur_sens;
> + else if (entry->sensitivity > cur_sens)
> + break;
> + }
> +
> + /* not in current range, so find new sensitivity */
> + for (i = 0; i < ARRAY_SIZE(appleals_sensitivity_map); i++) {
> + entry = &appleals_sensitivity_map[i];
> +
> + if (entry->illum_low <= cur_illum &&
> + entry->illum_high >= cur_illum)
> + return entry->sensitivity;
> + }
> +
> + /* hmm, not in table, so assume we are above highest range */
> + i = ARRAY_SIZE(appleals_sensitivity_map) - 1;
> + return appleals_sensitivity_map[i].sensitivity;
> +}
> +
> +static int appleals_get_field_value_for_usage(struct hid_field *field,
> + unsigned int usage)
> +{
> + int u;
> +
> + if (!field)
> + return -1;
> +
> + for (u = 0; u < field->maxusage; u++) {
> + if (field->usage[u].hid == usage)
> + return u + field->logical_minimum;
> + }
> +
> + return -1;
> +}
> +
> +static __s32 appleals_get_field_value(struct appleals_device *als_dev,
> + struct hid_field *field)
> +{
> + hid_hw_request(als_dev->hid_dev, field->report, HID_REQ_GET_REPORT);
> + hid_hw_wait(als_dev->hid_dev);
> +
> + return field->value[0];
> +}
> +
> +static void appleals_set_field_value(struct appleals_device *als_dev,
> + struct hid_field *field, __s32 value)
> +{
> + hid_set_field(field, 0, value);
> + hid_hw_request(als_dev->hid_dev, field->report, HID_REQ_SET_REPORT);
> +}
> +
> +static int appleals_get_config(struct appleals_device *als_dev,
> + unsigned int field_usage, __s32 *value)
> +{
> + struct hid_field *field;
> +
> + field = appleib_find_report_field(als_dev->cfg_report, field_usage);
> + if (!field)
> + return -EINVAL;
> +
> + *value = appleals_get_field_value(als_dev, field);
> +
> + return 0;
> +}
> +
> +static int appleals_set_config(struct appleals_device *als_dev,
> + unsigned int field_usage, __s32 value)
> +{
> + struct hid_field *field;
> +
> + field = appleib_find_report_field(als_dev->cfg_report, field_usage);
> + if (!field)
> + return -EINVAL;
> +
> + appleals_set_field_value(als_dev, field, value);
> +
> + return 0;
> +}
> +
> +static int appleals_set_enum_config(struct appleals_device *als_dev,
> + unsigned int field_usage,
> + unsigned int value_usage)
> +{
> + struct hid_field *field;
> + int value;
> +
> + field = appleib_find_report_field(als_dev->cfg_report, field_usage);
> + if (!field)
> + return -EINVAL;
> +
> + value = appleals_get_field_value_for_usage(field, value_usage);
can return -1, not checked
> +
> + appleals_set_field_value(als_dev, field, value);
> +
> + return 0;
> +}
> +
> +static void appleals_update_dyn_sensitivity(struct appleals_device *als_dev,
> + __s32 value)
> +{
> + int new_sens;
> + int rc;
> +
> + new_sens = appleals_compute_sensitivity(value,
> + als_dev->cur_sensitivity);
> + if (new_sens != als_dev->cur_sensitivity) {
> + rc = appleals_set_config(als_dev,
> + HID_USAGE_SENSOR_LIGHT_ILLUM |
> + HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_ABS,
> + new_sens);
> + if (!rc)
> + als_dev->cur_sensitivity = new_sens;
> + }
> +}
> +
> +static void appleals_push_new_value(struct appleals_device *als_dev,
> + __s32 value)
> +{
> + __s32 buf[2] = { value, value };
> +
> + iio_push_to_buffers(als_dev->iio_dev, buf);
> +
> + if (als_dev->cur_hysteresis == APPLEALS_DYN_SENS)
> + appleals_update_dyn_sensitivity(als_dev, value);
> +}
> +
> +static int appleals_hid_event(struct hid_device *hdev, struct hid_field *field,
> + struct hid_usage *usage, __s32 value)
> +{
> + struct appleals_device *als_dev =
> + appleib_get_drvdata(hid_get_drvdata(hdev),
> + &appleals_hid_driver);
> + int rc = 0;
> +
> + if ((usage->hid & HID_USAGE_PAGE) != HID_UP_SENSOR)
> + return 0;
> +
> + if (usage->hid == HID_USAGE_SENSOR_LIGHT_ILLUM) {
> + appleals_push_new_value(als_dev, value);
> + rc = 1;
> + }
> +
> + return rc;
> +}
> +
> +static int appleals_enable_events(struct iio_trigger *trig, bool enable)
> +{
> + struct appleals_device *als_dev = iio_trigger_get_drvdata(trig);
> + int value;
> +
> + /* set the sensor's reporting state */
> + appleals_set_enum_config(als_dev, HID_USAGE_SENSOR_PROP_REPORT_STATE,
> + enable ? HID_USAGE_SENSOR_PROP_REPORTING_STATE_ALL_EVENTS_ENUM :
> + HID_USAGE_SENSOR_PROP_REPORTING_STATE_NO_EVENTS_ENUM);
> + als_dev->events_enabled = enable;
> +
> + /* if the sensor was enabled, push an initial value */
> + if (enable) {
> + value = appleals_get_field_value(als_dev, als_dev->illum_field);
> + appleals_push_new_value(als_dev, value);
> + }
> +
> + return 0;
> +}
> +
> +static int appleals_read_raw(struct iio_dev *iio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct appleals_device *als_dev =
> + *(struct appleals_device **)iio_priv(iio_dev);
> + __s32 value;
> + int rc;
> +
> + *val = 0;
> + *val2 = 0;
no need to set these here
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + case IIO_CHAN_INFO_PROCESSED:
> + *val = appleals_get_field_value(als_dev, als_dev->illum_field);
> + return IIO_VAL_INT;
> +
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + rc = appleals_get_config(als_dev,
> + HID_USAGE_SENSOR_PROP_REPORT_INTERVAL,
> + &value);
> + if (rc)
> + return rc;
> +
> + /* interval is in ms; val is in HZ, val2 in µHZ */
> + value = 1000000000 / value;
> + *val = value / 1000000;
> + *val2 = value - (*val * 1000000);
> +
> + return IIO_VAL_INT_PLUS_MICRO;
> +
> + case IIO_CHAN_INFO_HYSTERESIS:
> + if (als_dev->cur_hysteresis == APPLEALS_DYN_SENS) {
> + *val = als_dev->cur_hysteresis;
> + return IIO_VAL_INT;
> + }
> +
> + rc = appleals_get_config(als_dev,
> + HID_USAGE_SENSOR_LIGHT_ILLUM |
> + HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_ABS,
> + val);
> + if (!rc) {
> + als_dev->cur_sensitivity = *val;
> + als_dev->cur_hysteresis = *val;
> + }
> + return rc ? rc : IIO_VAL_INT;
> +
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int appleals_write_raw(struct iio_dev *iio_dev,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long mask)
> +{
> + struct appleals_device *als_dev =
> + *(struct appleals_device **)iio_priv(iio_dev);
> + __s32 illum;
> + int rc;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + rc = appleals_set_config(als_dev,
> + HID_USAGE_SENSOR_PROP_REPORT_INTERVAL,
> + 1000000000 / (val * 1000000 + val2));
> + break;
maybe return directly instead of at the end (matter of taste);
here and in the other cases below
> +
> + case IIO_CHAN_INFO_HYSTERESIS:
> + if (val == APPLEALS_DYN_SENS) {
> + if (als_dev->cur_hysteresis != APPLEALS_DYN_SENS) {
> + als_dev->cur_hysteresis = val;
> + illum = appleals_get_field_value(als_dev,
> + als_dev->illum_field);
> + appleals_update_dyn_sensitivity(als_dev, illum);
> + }
> + rc = 0;
> + break;
> + }
> +
> + rc = appleals_set_config(als_dev,
> + HID_USAGE_SENSOR_LIGHT_ILLUM |
> + HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_ABS,
> + val);
> + if (!rc) {
> + als_dev->cur_sensitivity = val;
> + als_dev->cur_hysteresis = val;
> + }
> + break;
> +
> + default:
> + rc = -EINVAL;
> + }
> +
> + return rc;
> +}
> +
> +static const struct iio_chan_spec appleals_channels[] = {
> + {
> + .type = IIO_INTENSITY,
> + .modified = 1,
> + .channel2 = IIO_MOD_LIGHT_BOTH,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
> + BIT(IIO_CHAN_INFO_RAW),
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> + BIT(IIO_CHAN_INFO_HYSTERESIS),
> + .scan_type = {
> + .sign = 'u',
> + .realbits = 32,
> + .storagebits = 32,
> + },
> + .scan_index = 0,
> + },
> + {
> + .type = IIO_LIGHT,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
> + BIT(IIO_CHAN_INFO_RAW),
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> + BIT(IIO_CHAN_INFO_HYSTERESIS),
> + .scan_type = {
> + .sign = 'u',
> + .realbits = 32,
> + .storagebits = 32,
> + },
> + .scan_index = 1,
> + }
> +};
> +
> +static const struct iio_trigger_ops appleals_trigger_ops = {
> + .set_trigger_state = &appleals_enable_events,
> +};
> +
> +static const struct iio_info appleals_info = {
> + .read_raw = &appleals_read_raw,
> + .write_raw = &appleals_write_raw,
> +};
> +
> +static void appleals_config_sensor(struct appleals_device *als_dev,
> + bool events_enabled, int sensitivity)
> +{
> + struct hid_field *field;
> + __s32 val;
> +
> + /*
> + * We're (often) in a probe here, so need to enable input processing
> + * in that case, but only in that case.
> + */
> + if (appleib_in_hid_probe(als_dev->ib_dev))
> + hid_device_io_start(als_dev->hid_dev);
> +
> + /* power on the sensor */
> + field = appleib_find_report_field(als_dev->cfg_report,
> + HID_USAGE_SENSOR_PROY_POWER_STATE);
> + val = appleals_get_field_value_for_usage(field,
> + HID_USAGE_SENSOR_PROP_POWER_STATE_D0_FULL_POWER_ENUM);
what if -1?
> + hid_set_field(field, 0, val);
> +
> + /* configure reporting of change events */
> + field = appleib_find_report_field(als_dev->cfg_report,
> + HID_USAGE_SENSOR_PROP_REPORT_STATE);
> + val = appleals_get_field_value_for_usage(field,
> + events_enabled ?
> + HID_USAGE_SENSOR_PROP_REPORTING_STATE_ALL_EVENTS_ENUM :
> + HID_USAGE_SENSOR_PROP_REPORTING_STATE_NO_EVENTS_ENUM);
> + hid_set_field(field, 0, val);
> +
> + /* report change events asap */
> + field = appleib_find_report_field(als_dev->cfg_report,
> + HID_USAGE_SENSOR_PROP_REPORT_INTERVAL);
> + hid_set_field(field, 0, field->logical_minimum);
> +
> + /*
> + * Set initial change sensitivity; if dynamic, enabling trigger will set
> + * it instead.
> + */
> + if (sensitivity != APPLEALS_DYN_SENS) {
> + field = appleib_find_report_field(als_dev->cfg_report,
> + HID_USAGE_SENSOR_LIGHT_ILLUM |
> + HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_ABS);
> +
> + hid_set_field(field, 0, sensitivity);
> + }
> +
> + /* write the new config to the sensor */
> + hid_hw_request(als_dev->hid_dev, als_dev->cfg_report,
> + HID_REQ_SET_REPORT);
> +
> + if (appleib_in_hid_probe(als_dev->ib_dev))
> + hid_device_io_stop(als_dev->hid_dev);
> +};
no semicolon at the end of a function please
> +
> +static int appleals_config_iio(struct appleals_device *als_dev)
> +{
> + struct iio_dev *iio_dev;
> + struct iio_trigger *iio_trig;
> + int rc;
> +
> + /* create and register iio device */
> + iio_dev = iio_device_alloc(sizeof(als_dev));
how about using the devm_ variants?
> + if (!iio_dev)
> + return -ENOMEM;
> +
> + *(struct appleals_device **)iio_priv(iio_dev) = als_dev;
> +
> + iio_dev->channels = appleals_channels;
> + iio_dev->num_channels = ARRAY_SIZE(appleals_channels);
> + iio_dev->dev.parent = &als_dev->hid_dev->dev;
> + iio_dev->info = &appleals_info;
> + iio_dev->name = "als";
> + iio_dev->modes = INDIO_DIRECT_MODE;
> +
> + rc = iio_triggered_buffer_setup(iio_dev, &iio_pollfunc_store_time, NULL,
> + NULL);
> + if (rc) {
> + dev_err(als_dev->log_dev, "failed to set up iio triggers: %d\n",
just one trigger?
> + rc);
> + goto free_iio_dev;
> + }
> +
> + iio_trig = iio_trigger_alloc("%s-dev%d", iio_dev->name, iio_dev->id);
> + if (!iio_trig) {
> + rc = -ENOMEM;
> + goto clean_trig_buf;
> + }
> +
> + iio_trig->dev.parent = &als_dev->hid_dev->dev;
> + iio_trig->ops = &appleals_trigger_ops;
> + iio_trigger_set_drvdata(iio_trig, als_dev);
> +
> + rc = iio_trigger_register(iio_trig);
> + if (rc) {
> + dev_err(als_dev->log_dev, "failed to register iio trigger: %d\n",
some messages start lowercase, some uppercase (nitpicking)
> + rc);
> + goto free_iio_trig;
> + }
> +
> + als_dev->iio_trig = iio_trig;
> +
> + rc = iio_device_register(iio_dev);
> + if (rc) {
> + dev_err(als_dev->log_dev, "failed to register iio device: %d\n",
> + rc);
> + goto unreg_iio_trig;
> + }
> +
> + als_dev->iio_dev = iio_dev;
> +
> + return 0;
> +
> +unreg_iio_trig:
> + iio_trigger_unregister(iio_trig);
> +free_iio_trig:
> + iio_trigger_free(iio_trig);
> + als_dev->iio_trig = NULL;
> +clean_trig_buf:
> + iio_triggered_buffer_cleanup(iio_dev);
> +free_iio_dev:
> + iio_device_free(iio_dev);
> +
> + return rc;
> +}
> +
> +static int appleals_probe(struct hid_device *hdev,
> + const struct hid_device_id *id)
> +{
> + struct appleals_device *als_dev =
> + appleib_get_drvdata(hid_get_drvdata(hdev),
> + &appleals_hid_driver);
> + struct hid_field *state_field;
> + struct hid_field *illum_field;
> + int rc;
> +
> + /* find als fields and reports */
> + state_field = appleib_find_hid_field(hdev, HID_USAGE_SENSOR_ALS,
> + HID_USAGE_SENSOR_PROP_REPORT_STATE);
> + illum_field = appleib_find_hid_field(hdev, HID_USAGE_SENSOR_ALS,
> + HID_USAGE_SENSOR_LIGHT_ILLUM);
> + if (!state_field || !illum_field)
> + return -ENODEV;
> +
> + if (als_dev->hid_dev) {
> + dev_warn(als_dev->log_dev,
> + "Found duplicate ambient light sensor - ignoring\n");
> + return -EBUSY;
> + }
> +
> + dev_info(als_dev->log_dev, "Found ambient light sensor\n");
in general avoid logging for the OK case, it just clutters the log
> +
> + /* initialize device */
> + als_dev->hid_dev = hdev;
> + als_dev->cfg_report = state_field->report;
> + als_dev->illum_field = illum_field;
> +
> + als_dev->cur_hysteresis = APPLEALS_DEF_CHANGE_SENS;
> + als_dev->cur_sensitivity = APPLEALS_DEF_CHANGE_SENS;
> + appleals_config_sensor(als_dev, false, als_dev->cur_sensitivity);
> +
> + rc = appleals_config_iio(als_dev);
> + if (rc)
> + return rc;
> +
> + return 0;
> +}
> +
> +static void appleals_remove(struct hid_device *hdev)
> +{
> + struct appleals_device *als_dev =
> + appleib_get_drvdata(hid_get_drvdata(hdev),
> + &appleals_hid_driver);
> +
could be a lot less if devm_ were used?
> + if (als_dev->iio_dev) {
> + iio_device_unregister(als_dev->iio_dev);
> +
> + iio_trigger_unregister(als_dev->iio_trig);
> + iio_trigger_free(als_dev->iio_trig);
> + als_dev->iio_trig = NULL;
> +
> + iio_triggered_buffer_cleanup(als_dev->iio_dev);
> + iio_device_free(als_dev->iio_dev);
> + als_dev->iio_dev = NULL;
> + }
> +
> + als_dev->hid_dev = NULL;
> +}
> +
> +#ifdef CONFIG_PM
> +static int appleals_reset_resume(struct hid_device *hdev)
> +{
> + struct appleals_device *als_dev =
> + appleib_get_drvdata(hid_get_drvdata(hdev),
> + &appleals_hid_driver);
> +
> + appleals_config_sensor(als_dev, als_dev->events_enabled,
> + als_dev->cur_sensitivity);
> +
> + return 0;
> +}
> +#endif
> +
> +static struct hid_driver appleals_hid_driver = {
> + .name = "apple-ib-als",
> + .probe = appleals_probe,
> + .remove = appleals_remove,
> + .event = appleals_hid_event,
> +#ifdef CONFIG_PM
> + .reset_resume = appleals_reset_resume,
> +#endif
> +};
> +
> +static int appleals_platform_probe(struct platform_device *pdev)
> +{
> + struct appleib_platform_data *pdata = pdev->dev.platform_data;
> + struct appleib_device *ib_dev = pdata->ib_dev;
> + struct appleals_device *als_dev;
> + int rc;
> +
> + als_dev = kzalloc(sizeof(*als_dev), GFP_KERNEL);
> + if (!als_dev)
> + return -ENOMEM;
> +
> + als_dev->ib_dev = ib_dev;
> + als_dev->log_dev = pdata->log_dev;
> +
> + rc = appleib_register_hid_driver(ib_dev, &appleals_hid_driver, als_dev);
> + if (rc) {
> + dev_err(als_dev->log_dev, "Error registering hid driver: %d\n",
> + rc);
> + goto error;
> + }
> +
> + platform_set_drvdata(pdev, als_dev);
> +
> + return 0;
> +
> +error:
> + kfree(als_dev);
> + return rc;
> +}
> +
> +static int appleals_platform_remove(struct platform_device *pdev)
> +{
> + struct appleib_platform_data *pdata = pdev->dev.platform_data;
> + struct appleib_device *ib_dev = pdata->ib_dev;
> + struct appleals_device *als_dev = platform_get_drvdata(pdev);
> + int rc;
> +
> + rc = appleib_unregister_hid_driver(ib_dev, &appleals_hid_driver);
> + if (rc) {
> + dev_err(als_dev->log_dev,
> + "Error unregistering hid driver: %d\n", rc);
> + goto error;
> + }
> +
> + kfree(als_dev);
> +
> + return 0;
> +
> +error:
> + return rc;
> +}
> +
> +static const struct platform_device_id appleals_platform_ids[] = {
> + { .name = PLAT_NAME_IB_ALS },
> + { }
> +};
> +MODULE_DEVICE_TABLE(platform, appleals_platform_ids);
> +
> +static struct platform_driver appleals_platform_driver = {
> + .id_table = appleals_platform_ids,
> + .driver = {
> + .name = "apple-ib-als",
> + },
> + .probe = appleals_platform_probe,
> + .remove = appleals_platform_remove,
> +};
> +
> +module_platform_driver(appleals_platform_driver);
> +
> +MODULE_AUTHOR("Ronald Tschalär");
> +MODULE_DESCRIPTION("Apple iBridge ALS driver");
> +MODULE_LICENSE("GPL v2");
>
--
Peter Meerwald-Stadler
Mobile: +43 664 24 44 418
Powered by blists - more mailing lists