| Message ID | 1571631083-4962-3-git-send-email-jeff@labundy.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add support for Azoteq IQS620A/621/622/624/625 | expand |
On Sun, 20 Oct 2019, Jeff LaBundy wrote: > This patch adds support for core functions common to all six-channel > members of the Azoteq ProxFusion family of sensor devices. > > Signed-off-by: Jeff LaBundy <jeff@labundy.com> > --- > drivers/mfd/Kconfig | 13 + > drivers/mfd/Makefile | 2 + > drivers/mfd/iqs62x-core.c | 638 ++++++++++++++++++++++++++++++++++++++++++++ > drivers/mfd/iqs62x-tables.c | 424 +++++++++++++++++++++++++++++ > include/linux/mfd/iqs62x.h | 148 ++++++++++ > 5 files changed, 1225 insertions(+) > create mode 100644 drivers/mfd/iqs62x-core.c > create mode 100644 drivers/mfd/iqs62x-tables.c > create mode 100644 include/linux/mfd/iqs62x.h > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > index ae24d3e..df391f7 100644 > --- a/drivers/mfd/Kconfig > +++ b/drivers/mfd/Kconfig > @@ -642,6 +642,19 @@ config MFD_IPAQ_MICRO > AT90LS8535 microcontroller flashed with a special iPAQ > firmware using the custom protocol implemented in this driver. > > +config MFD_IQS62X > + tristate "Azoteq IQS620A/621/622/624/625 core support" > + depends on I2C > + select MFD_CORE > + select REGMAP_I2C > + help > + Say Y here if you want to build support for six-channel members of > + the Azoteq ProxFusion family of sensor devices. Additional options > + must be selected to enable device-specific functions. > + > + To compile this driver as a module, choose M here: the module will > + be called iqs62x. > + > config MFD_JANZ_CMODIO > tristate "Janz CMOD-IO PCI MODULbus Carrier Board" > select MFD_CORE > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > index c1067ea..23dd71c6 100644 > --- a/drivers/mfd/Makefile > +++ b/drivers/mfd/Makefile > @@ -256,3 +256,5 @@ obj-$(CONFIG_MFD_ROHM_BD70528) += rohm-bd70528.o > obj-$(CONFIG_MFD_ROHM_BD718XX) += rohm-bd718x7.o > obj-$(CONFIG_MFD_STMFX) += stmfx.o > > +iqs62x-objs := iqs62x-core.o iqs62x-tables.o > +obj-$(CONFIG_MFD_IQS62X) += iqs62x.o > diff --git a/drivers/mfd/iqs62x-core.c b/drivers/mfd/iqs62x-core.c > new file mode 100644 > index 0000000..e2200c8 > --- /dev/null > +++ b/drivers/mfd/iqs62x-core.c > @@ -0,0 +1,638 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Azoteq IQS620A/621/622/624/625 ProxFusion Sensor Family > + * > + * Copyright (C) 2019 Needs a company or person's name. > + * Author: Jeff LaBundy <jeff@labundy.com> > + * > + * These devices rely on application-specific register settings and calibration > + * data developed in and exported from a suite of GUIs offered by the vendor. A > + * separate tool converts the GUIs' ASCII-based output into a standard firmware > + * file parsed by the driver. This troubles me somewhat. So here we take a C header file which is the output of a vendor supplied configuration tool, convert it to a bespoke file format using a Python script authored by yourself, which we masquerade as Linux firmware in order to set-up the hardware. Is that correct? What is preventing a very naughty person from providing their own register map (firmware) in order to read/write unsuitable registers from kernel context for their own gains; simply by swapping out a file contained in userspace? It would probably be a better idea to compile the register definitions with the kernel/module to be safe. You can use Device Tree for run-time configuration changes. > + * Link to data sheets and GUIs: https://www.azoteq.com/products/proxfusion/ > + * > + * Link to conversion tool: https://github.com/jlabundy/iqs62x-h2bin.git This is unlikely to stand the test of time. Probably best to just list the name of the tool in the description above. > + */ > + > +#include <linux/completion.h> > +#include <linux/delay.h> > +#include <linux/device.h> > +#include <linux/err.h> > +#include <linux/firmware.h> > +#include <linux/i2c.h> > +#include <linux/interrupt.h> > +#include <linux/kernel.h> > +#include <linux/list.h> > +#include <linux/mfd/core.h> > +#include <linux/module.h> > +#include <linux/notifier.h> > +#include <linux/of_device.h> > +#include <linux/property.h> > +#include <linux/regmap.h> > +#include <linux/slab.h> > +#include <asm/unaligned.h> [...] > +static int iqs62x_fw_parse(struct iqs62x_core *iqs62x, > + const struct firmware *fw) > +{ > + struct i2c_client *client = iqs62x->client; > + struct iqs62x_fw_rec *fw_rec; > + struct iqs62x_fw_blk *fw_blk; > + unsigned int hall_cal_index = 0; > + size_t pos = 0; > + int error = 0; > + u8 mask, len; > + u8 *data; > + > + while (pos < fw->size) { > + if (pos + sizeof(*fw_rec) > fw->size) { > + error = -EINVAL; > + break; > + } > + fw_rec = (struct iqs62x_fw_rec *)(fw->data + pos); > + pos += sizeof(*fw_rec); > + > + if (pos + fw_rec->len - 1 > fw->size) { > + error = -EINVAL; > + break; > + } > + pos += fw_rec->len - 1; > + > + switch (fw_rec->type) { > + case IQS62X_FW_REC_TYPE_INFO: > + continue; > + > + case IQS62X_FW_REC_TYPE_PROD: > + if (fw_rec->data == iqs62x->dev_desc->prod_num) > + continue; > + > + dev_err(&client->dev, > + "Incompatible product number: 0x%02X\n", > + fw_rec->data); > + error = -EINVAL; > + break; > + > + case IQS62X_FW_REC_TYPE_HALL: > + if (!hall_cal_index) { > + error = regmap_write(iqs62x->map, > + IQS62X_OTP_CMD, > + IQS62X_OTP_CMD_FG3); > + if (error) > + break; > + > + error = regmap_read(iqs62x->map, > + IQS62X_OTP_DATA, > + &hall_cal_index); > + if (error) > + break; > + > + hall_cal_index &= IQS62X_HALL_CAL_MASK; > + if (!hall_cal_index) { > + dev_err(&client->dev, > + "Uncalibrated device\n"); > + error = -ENODATA; > + break; > + } > + } > + > + if (hall_cal_index > fw_rec->len) { > + error = -EINVAL; > + break; > + } > + > + mask = 0; > + data = &fw_rec->data + hall_cal_index - 1; > + len = sizeof(*data); > + break; > + > + case IQS62X_FW_REC_TYPE_MASK: > + if (fw_rec->len < (sizeof(mask) + sizeof(*data))) { > + error = -EINVAL; > + break; > + } > + > + mask = fw_rec->data; > + data = &fw_rec->data + sizeof(mask); > + len = sizeof(*data); > + break; > + > + case IQS62X_FW_REC_TYPE_DATA: > + mask = 0; > + data = &fw_rec->data; > + len = fw_rec->len; > + break; > + > + default: > + dev_err(&client->dev, > + "Unrecognized record type: 0x%02X\n", > + fw_rec->type); > + error = -EINVAL; > + } > + > + if (error) > + break; > + > + fw_blk = devm_kzalloc(&client->dev, > + struct_size(fw_blk, data, len), > + GFP_KERNEL); > + if (!fw_blk) { > + error = -ENOMEM; > + break; > + } > + > + fw_blk->addr = fw_rec->addr; > + fw_blk->mask = mask; > + fw_blk->len = len; > + memcpy(fw_blk->data, data, len); > + > + list_add(&fw_blk->list, &iqs62x->fw_blk_head); > + } > + > + release_firmware(fw); > + > + return error; > +} > + > +static irqreturn_t iqs62x_irq(int irq, void *context) > +{ > + struct iqs62x_core *iqs62x = context; > + struct iqs62x_event_data event_data; > + struct iqs62x_event_desc event_desc; > + enum iqs62x_event_reg event_reg; > + unsigned long event_flags = 0; > + int error, i, j; > + u8 event_map[IQS62X_EVENT_SIZE]; > + > + /* > + * The device asserts the RDY output to signal the beginning of a > + * communication window, which is closed by an I2C stop condition. > + * As such, all interrupt status is captured in a single read and > + * broadcast to any interested sub-device drivers. > + */ > + error = regmap_raw_read(iqs62x->map, IQS62X_SYS_FLAGS, > + event_map, sizeof(event_map)); > + if (error) > + return IRQ_NONE; > + > + for (i = 0; i < sizeof(event_map); i++) { > + event_reg = iqs62x->dev_desc->event_regs[iqs62x->ui_sel][i]; > + > + switch (event_reg) { > + case IQS62X_EVENT_UI_LO: > + event_data.ui_data = get_unaligned_le16(&event_map[i]); > + /* fall through */ > + case IQS62X_EVENT_UI_HI: > + case IQS62X_EVENT_NONE: > + case IQS62X_EVENT_GLBL: > + continue; > + > + case IQS62X_EVENT_TEMP: > + event_data.temp_flags = event_map[i]; > + continue; > + > + case IQS62X_EVENT_ALS: > + event_data.als_flags = event_map[i]; > + continue; > + > + case IQS62X_EVENT_IR: > + event_data.ir_flags = event_map[i]; > + continue; > + > + case IQS62X_EVENT_INTER: > + event_data.interval = event_map[i]; > + continue; > + > + case IQS62X_EVENT_HYST: > + event_map[i] <<= iqs62x->dev_desc->hyst_shift; > + /* fall through */ > + case IQS62X_EVENT_WHEEL: > + case IQS62X_EVENT_HALL: > + case IQS62X_EVENT_PROX: > + case IQS62X_EVENT_SYS: > + break; > + } > + > + for (j = 0; j < IQS62X_NUM_EVENTS; j++) { > + event_desc = iqs62x_events[j]; > + > + if (event_desc.reg != event_reg) > + continue; > + > + if ((event_map[i] & event_desc.mask) == event_desc.val) > + event_flags |= BIT(j); > + } > + } > + > + /* > + * The device resets itself in response to the I2C master stalling > + * communication beyond a timeout. In this case, all registers are > + * restored and any interested sub-device drivers are notified. > + */ > + if (event_flags & BIT(IQS62X_EVENT_SYS_RESET)) { > + dev_err(&iqs62x->client->dev, "Unexpected device reset\n"); > + > + error = iqs62x_dev_init(iqs62x); Is it safe to re-initialise the entire device in IRQ context? > + if (error) { > + dev_err(&iqs62x->client->dev, > + "Failed to re-initialize device: %d\n", error); > + return IRQ_NONE; > + } > + } > + > + error = blocking_notifier_call_chain(&iqs62x->nh, event_flags, > + &event_data); > + if (error & NOTIFY_STOP_MASK) > + return IRQ_NONE; > + > + /* > + * Once the communication window is closed, a small delay is added to > + * ensure the device's RDY output has been deasserted by the time the > + * interrupt handler returns. > + */ > + usleep_range(50, 100); > + > + return IRQ_HANDLED; > +} [...] > +static int iqs62x_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct iqs62x_core *iqs62x; > + struct iqs62x_info info; > + unsigned int val; > + int error, i, j; Nit: It's more common to use 'ret' or 'err' - my preference is 'ret'. > + const char *fw_file = NULL; > + > + iqs62x = devm_kzalloc(&client->dev, sizeof(*iqs62x), GFP_KERNEL); > + if (!iqs62x) > + return -ENOMEM; > + > + i2c_set_clientdata(client, iqs62x); > + iqs62x->client = client; > + > + BLOCKING_INIT_NOTIFIER_HEAD(&iqs62x->nh); > + INIT_LIST_HEAD(&iqs62x->fw_blk_head); > + init_completion(&iqs62x->fw_done); > + > + iqs62x->map = devm_regmap_init_i2c(client, &iqs62x_map_config); > + if (IS_ERR(iqs62x->map)) { > + error = PTR_ERR(iqs62x->map); > + dev_err(&client->dev, "Failed to initialize register map: %d\n", > + error); > + return error; > + } > + > + error = regmap_raw_read(iqs62x->map, IQS62X_PROD_NUM, &info, > + sizeof(info)); > + if (error) > + return error; > + > + for (i = 0; i < IQS62X_NUM_DEV; i++) { > + if (info.prod_num == iqs62x_devs[i].prod_num) > + iqs62x->dev_desc = &iqs62x_devs[i]; > + else > + continue; Reads better without the else: if (info.prod_num != iqs62x_devs[i].prod_num) continue; > + if (info.sw_num >= iqs62x->dev_desc->sw_num) > + iqs62x->sw_num = info.sw_num; > + else > + continue; Same as above. Do you still need to be in this loop at this point? > + for (j = 0; j < iqs62x->dev_desc->num_cal_regs; j++) { What are you doing here? Please provide a comment. > + error = regmap_read(iqs62x->map, > + iqs62x->dev_desc->cal_regs[j], > + &val); > + if (error) > + return error; > + > + if (!val) > + break; > + } > + > + if (j == iqs62x->dev_desc->num_cal_regs) > + break; Is there a reason not to break here? If the product number matched once, can it match for a second time? > + } > + > + if (!iqs62x->dev_desc) { > + dev_err(&client->dev, "Unrecognized product number: 0x%02X\n", > + info.prod_num); > + return -EINVAL; > + } > + > + if (!iqs62x->sw_num) { > + dev_err(&client->dev, "Unrecognized software number: 0x%02X\n", > + info.sw_num); > + return -EINVAL; > + } > + > + if (i == IQS62X_NUM_DEV) { > + dev_err(&client->dev, "Uncalibrated device\n"); > + return -ENODATA; > + } > + > + error = regmap_write(iqs62x->map, IQS62X_SYS_SETTINGS, > + IQS62X_SYS_SETTINGS_SOFT_RESET); > + if (error) > + return error; > + usleep_range(10000, 10100); > + > + device_property_read_string(&client->dev, "linux,fw-file", &fw_file); > + > + error = request_firmware_nowait(THIS_MODULE, FW_ACTION_HOTPLUG, > + fw_file ? : iqs62x->dev_desc->fw_file, > + &client->dev, GFP_KERNEL, iqs62x, > + iqs62x_fw_load); > + if (error) > + dev_err(&client->dev, "Failed to request firmware: %d\n", > + error); > + > + return error; > +} > + > +static int iqs62x_remove(struct i2c_client *client) > +{ > + struct iqs62x_core *iqs62x = i2c_get_clientdata(client); > + > + wait_for_completion(&iqs62x->fw_done); > + > + return 0; > +} Please move the suspend/resume calls down to here. > +static const struct i2c_device_id iqs62x_id[] = { > + { "iqs620a", 0 }, > + { "iqs621", 1 }, > + { "iqs622", 2 }, > + { "iqs624", 3 }, > + { "iqs625", 4 }, Better to define these. In fact, do you even use the .ids? If not, you can use the new I2C probe function and drop this table. > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, iqs62x_id); > + > +static const struct of_device_id iqs62x_of_match[] = { > + { .compatible = "azoteq,iqs620a" }, > + { .compatible = "azoteq,iqs621" }, > + { .compatible = "azoteq,iqs622" }, > + { .compatible = "azoteq,iqs624" }, > + { .compatible = "azoteq,iqs625" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, iqs62x_of_match); > + > +static struct i2c_driver iqs62x_i2c_driver = { > + .driver = { > + .name = "iqs62x", > + .of_match_table = iqs62x_of_match, > + .pm = &iqs62x_pm, > + }, > + .id_table = iqs62x_id, > + .probe = iqs62x_probe, > + .remove = iqs62x_remove, > +}; > +module_i2c_driver(iqs62x_i2c_driver); > + > +MODULE_AUTHOR("Jeff LaBundy <jeff@labundy.com>"); > +MODULE_DESCRIPTION("Azoteq IQS620A/621/622/624/625 ProxFusion Sensor Family"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/mfd/iqs62x-tables.c b/drivers/mfd/iqs62x-tables.c > new file mode 100644 > index 0000000..12300b7 > --- /dev/null > +++ b/drivers/mfd/iqs62x-tables.c > @@ -0,0 +1,424 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Azoteq IQS620A/621/622/624/625 ProxFusion Sensor Family > + * > + * Copyright (C) 2019 > + * Author: Jeff LaBundy <jeff@labundy.com> > + */ > + > +#include <linux/kernel.h> > +#include <linux/mfd/core.h> > +#include <linux/mfd/iqs62x.h> > + > +static const struct mfd_cell iqs620at_sub_devs[] = { > + { > + .name = IQS62X_DRV_NAME_KEYS, > + .of_compatible = "azoteq,iqs620a-keys", > + }, > + { > + .name = IQS620_DRV_NAME_PWM, > + .of_compatible = "azoteq,iqs620a-pwm", > + }, > + { > + .name = IQS620_DRV_NAME_TEMP, > + }, > +}; > + > +static const struct mfd_cell iqs620a_sub_devs[] = { > + { > + .name = IQS62X_DRV_NAME_KEYS, > + .of_compatible = "azoteq,iqs620a-keys", > + }, > + { > + .name = IQS620_DRV_NAME_PWM, > + .of_compatible = "azoteq,iqs620a-pwm", > + }, > +}; > + > +static const struct mfd_cell iqs621_sub_devs[] = { > + { > + .name = IQS62X_DRV_NAME_KEYS, > + .of_compatible = "azoteq,iqs621-keys", > + }, > + { > + .name = IQS621_DRV_NAME_ALS, > + }, > +}; > + > +static const struct mfd_cell iqs622_sub_devs[] = { > + { > + .name = IQS62X_DRV_NAME_KEYS, > + .of_compatible = "azoteq,iqs622-keys", > + }, > + { > + .name = IQS622_DRV_NAME_PROX, > + .of_compatible = "azoteq,iqs622-prox", > + }, > +}; > + > +static const struct mfd_cell iqs624_sub_devs[] = { > + { > + .name = IQS62X_DRV_NAME_KEYS, > + .of_compatible = "azoteq,iqs624-keys", > + }, > + { > + .name = IQS624_DRV_NAME_POS, > + }, > +}; > + > +static const struct mfd_cell iqs625_sub_devs[] = { > + { > + .name = IQS62X_DRV_NAME_KEYS, > + .of_compatible = "azoteq,iqs625-keys", > + }, > + { > + .name = IQS624_DRV_NAME_POS, > + }, > +}; These should be moved into the core driver. > +static const u8 iqs620at_cal_regs[] = { 0xC2, 0xC3, 0xC4, }; > +static const u8 iqs621_cal_regs[] = { 0x82, 0x83, }; What do these mean? Probably best of do define them, so people can read them. [...]
On Thu, Oct 31, 2019 at 01:44:10PM +0000, Lee Jones wrote: > On Sun, 20 Oct 2019, Jeff LaBundy wrote: > > + * Author: Jeff LaBundy <jeff@labundy.com> > > + * > > + * These devices rely on application-specific register settings and calibration > > + * data developed in and exported from a suite of GUIs offered by the vendor. A > > + * separate tool converts the GUIs' ASCII-based output into a standard firmware > > + * file parsed by the driver. > > This troubles me somewhat. So here we take a C header file which is > the output of a vendor supplied configuration tool, convert it to a > bespoke file format using a Python script authored by yourself, which > we masquerade as Linux firmware in order to set-up the hardware. > > Is that correct? > > What is preventing a very naughty person from providing their own > register map (firmware) in order to read/write unsuitable registers > from kernel context for their own gains; simply by swapping out a file > contained in userspace? How is it different from, let's say, Intel WiFi firmware that you load into the kernel and then into the card every time you boot up your laptop? Thanks.
Hi Lee, Thank you for your thorough review. On Thu, Oct 31, 2019 at 01:44:10PM +0000, Lee Jones wrote: > On Sun, 20 Oct 2019, Jeff LaBundy wrote: > > > This patch adds support for core functions common to all six-channel > > members of the Azoteq ProxFusion family of sensor devices. > > > > Signed-off-by: Jeff LaBundy <jeff@labundy.com> > > --- > > drivers/mfd/Kconfig | 13 + > > drivers/mfd/Makefile | 2 + > > drivers/mfd/iqs62x-core.c | 638 ++++++++++++++++++++++++++++++++++++++++++++ > > drivers/mfd/iqs62x-tables.c | 424 +++++++++++++++++++++++++++++ > > include/linux/mfd/iqs62x.h | 148 ++++++++++ > > 5 files changed, 1225 insertions(+) > > create mode 100644 drivers/mfd/iqs62x-core.c > > create mode 100644 drivers/mfd/iqs62x-tables.c > > create mode 100644 include/linux/mfd/iqs62x.h > > > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > > index ae24d3e..df391f7 100644 > > --- a/drivers/mfd/Kconfig > > +++ b/drivers/mfd/Kconfig > > @@ -642,6 +642,19 @@ config MFD_IPAQ_MICRO > > AT90LS8535 microcontroller flashed with a special iPAQ > > firmware using the custom protocol implemented in this driver. > > > > +config MFD_IQS62X > > + tristate "Azoteq IQS620A/621/622/624/625 core support" > > + depends on I2C > > + select MFD_CORE > > + select REGMAP_I2C > > + help > > + Say Y here if you want to build support for six-channel members of > > + the Azoteq ProxFusion family of sensor devices. Additional options > > + must be selected to enable device-specific functions. > > + > > + To compile this driver as a module, choose M here: the module will > > + be called iqs62x. > > + > > config MFD_JANZ_CMODIO > > tristate "Janz CMOD-IO PCI MODULbus Carrier Board" > > select MFD_CORE > > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > > index c1067ea..23dd71c6 100644 > > --- a/drivers/mfd/Makefile > > +++ b/drivers/mfd/Makefile > > @@ -256,3 +256,5 @@ obj-$(CONFIG_MFD_ROHM_BD70528) += rohm-bd70528.o > > obj-$(CONFIG_MFD_ROHM_BD718XX) += rohm-bd718x7.o > > obj-$(CONFIG_MFD_STMFX) += stmfx.o > > > > +iqs62x-objs := iqs62x-core.o iqs62x-tables.o > > +obj-$(CONFIG_MFD_IQS62X) += iqs62x.o > > diff --git a/drivers/mfd/iqs62x-core.c b/drivers/mfd/iqs62x-core.c > > new file mode 100644 > > index 0000000..e2200c8 > > --- /dev/null > > +++ b/drivers/mfd/iqs62x-core.c > > @@ -0,0 +1,638 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Azoteq IQS620A/621/622/624/625 ProxFusion Sensor Family > > + * > > + * Copyright (C) 2019 > > Needs a company or person's name. > Sure thing; will do. > > + * Author: Jeff LaBundy <jeff@labundy.com> > > + * > > + * These devices rely on application-specific register settings and calibration > > + * data developed in and exported from a suite of GUIs offered by the vendor. A > > + * separate tool converts the GUIs' ASCII-based output into a standard firmware > > + * file parsed by the driver. > > This troubles me somewhat. So here we take a C header file which is > the output of a vendor supplied configuration tool, convert it to a > bespoke file format using a Python script authored by yourself, which > we masquerade as Linux firmware in order to set-up the hardware. > > Is that correct? Correct on all counts. > > What is preventing a very naughty person from providing their own > register map (firmware) in order to read/write unsuitable registers > from kernel context for their own gains; simply by swapping out a file > contained in userspace? > I would argue that if someone is willing to go that length, they likely understand that their dock switch sensitivity may change or their ambient light sensor may no longer function properly. > It would probably be a better idea to compile the register definitions > with the kernel/module to be safe. You can use Device Tree for > run-time configuration changes. > Taking the IQS620A as an example, there are over 100 individual fields that need configured. Forcing customers to manually transfer the values derived within the GUI to a corresponding collection of device tree bindings would be prohibitively complex. To complicate matters, many registers change meaning or restrict their available values based on the values stored in other registers. Duplicating all of the de- pendencies and restrictions comprehended by the vendor's tool in the driver and/ or the bindings document would not be practical. Just to clarify, we're not storing register definitions (i.e. addresses) in this "firmware"; rather, we're storing application-specific register values that don't belong in the driver source. The C header file that comes from the vendor's tool comprises dozens of #define's that map a register name to a value written to the address bearing said arbitrary name, e.g.: #define HYSTERESIS_UI_SETTINGS_0 0xA2 My tool associates the address of HYSTERESIS_UI_SETTINGS_0 with its physical ad- dress (0x80) which will be written with 0xA2. Note that common registers may oc- cupy different addresses for each device, and my tool comprehends this. The file may also contain one or more byte arrays that represent calibration data that the driver selectively applies based on values in the device's OTP memory. We simply package all of these elements as address/data "records" for the driver to apply as efficiently as possible. This "register map as firmware" concept is not new. The widely used wm_adsp frame- work [0] does the exact same thing, where .bin (coefficient) files represent data to be written directly to register-mapped DSP memory. Dmitry has highlighted some valid examples as well. The intent here is to offer a customer-friendly solution to a complex problem. And although it's a Python tool I wrote myself, I've been fastidious about its documen- tation [1], and I'm committed to maintaining it as with anything else. > > + * Link to data sheets and GUIs: https://www.azoteq.com/products/proxfusion/ > > + * > > + * Link to conversion tool: https://github.com/jlabundy/iqs62x-h2bin.git > > This is unlikely to stand the test of time. Probably best to just list > the name of the tool in the description above. > My only concern here is that doing so forces customers to hunt for it using a web search, or to email me. And while I'm happy to field such emails, an email address is subject to the same risk of changing over time. I think the onus is simply on me to send a patch if the host or owner of this tool changes, just as I would if my email address changes. Please let me know if you do not agree. > > + */ > > + > > +#include <linux/completion.h> > > +#include <linux/delay.h> > > +#include <linux/device.h> > > +#include <linux/err.h> > > +#include <linux/firmware.h> > > +#include <linux/i2c.h> > > +#include <linux/interrupt.h> > > +#include <linux/kernel.h> > > +#include <linux/list.h> > > +#include <linux/mfd/core.h> > > +#include <linux/module.h> > > +#include <linux/notifier.h> > > +#include <linux/of_device.h> > > +#include <linux/property.h> > > +#include <linux/regmap.h> > > +#include <linux/slab.h> > > +#include <asm/unaligned.h> > > [...] > > > +static int iqs62x_fw_parse(struct iqs62x_core *iqs62x, > > + const struct firmware *fw) > > +{ > > + struct i2c_client *client = iqs62x->client; > > + struct iqs62x_fw_rec *fw_rec; > > + struct iqs62x_fw_blk *fw_blk; > > + unsigned int hall_cal_index = 0; > > + size_t pos = 0; > > + int error = 0; > > + u8 mask, len; > > + u8 *data; > > + > > + while (pos < fw->size) { > > + if (pos + sizeof(*fw_rec) > fw->size) { > > + error = -EINVAL; > > + break; > > + } > > + fw_rec = (struct iqs62x_fw_rec *)(fw->data + pos); > > + pos += sizeof(*fw_rec); > > + > > + if (pos + fw_rec->len - 1 > fw->size) { > > + error = -EINVAL; > > + break; > > + } > > + pos += fw_rec->len - 1; > > + > > + switch (fw_rec->type) { > > + case IQS62X_FW_REC_TYPE_INFO: > > + continue; > > + > > + case IQS62X_FW_REC_TYPE_PROD: > > + if (fw_rec->data == iqs62x->dev_desc->prod_num) > > + continue; > > + > > + dev_err(&client->dev, > > + "Incompatible product number: 0x%02X\n", > > + fw_rec->data); > > + error = -EINVAL; > > + break; > > + > > + case IQS62X_FW_REC_TYPE_HALL: > > + if (!hall_cal_index) { > > + error = regmap_write(iqs62x->map, > > + IQS62X_OTP_CMD, > > + IQS62X_OTP_CMD_FG3); > > + if (error) > > + break; > > + > > + error = regmap_read(iqs62x->map, > > + IQS62X_OTP_DATA, > > + &hall_cal_index); > > + if (error) > > + break; > > + > > + hall_cal_index &= IQS62X_HALL_CAL_MASK; > > + if (!hall_cal_index) { > > + dev_err(&client->dev, > > + "Uncalibrated device\n"); > > + error = -ENODATA; > > + break; > > + } > > + } > > + > > + if (hall_cal_index > fw_rec->len) { > > + error = -EINVAL; > > + break; > > + } > > + > > + mask = 0; > > + data = &fw_rec->data + hall_cal_index - 1; > > + len = sizeof(*data); > > + break; > > + > > + case IQS62X_FW_REC_TYPE_MASK: > > + if (fw_rec->len < (sizeof(mask) + sizeof(*data))) { > > + error = -EINVAL; > > + break; > > + } > > + > > + mask = fw_rec->data; > > + data = &fw_rec->data + sizeof(mask); > > + len = sizeof(*data); > > + break; > > + > > + case IQS62X_FW_REC_TYPE_DATA: > > + mask = 0; > > + data = &fw_rec->data; > > + len = fw_rec->len; > > + break; > > + > > + default: > > + dev_err(&client->dev, > > + "Unrecognized record type: 0x%02X\n", > > + fw_rec->type); > > + error = -EINVAL; > > + } > > + > > + if (error) > > + break; > > + > > + fw_blk = devm_kzalloc(&client->dev, > > + struct_size(fw_blk, data, len), > > + GFP_KERNEL); > > + if (!fw_blk) { > > + error = -ENOMEM; > > + break; > > + } > > + > > + fw_blk->addr = fw_rec->addr; > > + fw_blk->mask = mask; > > + fw_blk->len = len; > > + memcpy(fw_blk->data, data, len); > > + > > + list_add(&fw_blk->list, &iqs62x->fw_blk_head); > > + } > > + > > + release_firmware(fw); > > + > > + return error; > > +} > > + > > +static irqreturn_t iqs62x_irq(int irq, void *context) > > +{ > > + struct iqs62x_core *iqs62x = context; > > + struct iqs62x_event_data event_data; > > + struct iqs62x_event_desc event_desc; > > + enum iqs62x_event_reg event_reg; > > + unsigned long event_flags = 0; > > + int error, i, j; > > + u8 event_map[IQS62X_EVENT_SIZE]; > > + > > + /* > > + * The device asserts the RDY output to signal the beginning of a > > + * communication window, which is closed by an I2C stop condition. > > + * As such, all interrupt status is captured in a single read and > > + * broadcast to any interested sub-device drivers. > > + */ > > + error = regmap_raw_read(iqs62x->map, IQS62X_SYS_FLAGS, > > + event_map, sizeof(event_map)); > > + if (error) > > + return IRQ_NONE; > > + > > + for (i = 0; i < sizeof(event_map); i++) { > > + event_reg = iqs62x->dev_desc->event_regs[iqs62x->ui_sel][i]; > > + > > + switch (event_reg) { > > + case IQS62X_EVENT_UI_LO: > > + event_data.ui_data = get_unaligned_le16(&event_map[i]); > > + /* fall through */ > > + case IQS62X_EVENT_UI_HI: > > + case IQS62X_EVENT_NONE: > > + case IQS62X_EVENT_GLBL: > > + continue; > > + > > + case IQS62X_EVENT_TEMP: > > + event_data.temp_flags = event_map[i]; > > + continue; > > + > > + case IQS62X_EVENT_ALS: > > + event_data.als_flags = event_map[i]; > > + continue; > > + > > + case IQS62X_EVENT_IR: > > + event_data.ir_flags = event_map[i]; > > + continue; > > + > > + case IQS62X_EVENT_INTER: > > + event_data.interval = event_map[i]; > > + continue; > > + > > + case IQS62X_EVENT_HYST: > > + event_map[i] <<= iqs62x->dev_desc->hyst_shift; > > + /* fall through */ > > + case IQS62X_EVENT_WHEEL: > > + case IQS62X_EVENT_HALL: > > + case IQS62X_EVENT_PROX: > > + case IQS62X_EVENT_SYS: > > + break; > > + } > > + > > + for (j = 0; j < IQS62X_NUM_EVENTS; j++) { > > + event_desc = iqs62x_events[j]; > > + > > + if (event_desc.reg != event_reg) > > + continue; > > + > > + if ((event_map[i] & event_desc.mask) == event_desc.val) > > + event_flags |= BIT(j); > > + } > > + } > > + > > + /* > > + * The device resets itself in response to the I2C master stalling > > + * communication beyond a timeout. In this case, all registers are > > + * restored and any interested sub-device drivers are notified. > > + */ > > + if (event_flags & BIT(IQS62X_EVENT_SYS_RESET)) { > > + dev_err(&iqs62x->client->dev, "Unexpected device reset\n"); > > + > > + error = iqs62x_dev_init(iqs62x); > > Is it safe to re-initialise the entire device in IRQ context? > Here, we are simply re-writing several registers from memory. This is a threaded interrupt handler, so it should be safe to do so. But if I've misunderstood your concern, please let me know. > > + if (error) { > > + dev_err(&iqs62x->client->dev, > > + "Failed to re-initialize device: %d\n", error); > > + return IRQ_NONE; > > + } > > + } > > + > > + error = blocking_notifier_call_chain(&iqs62x->nh, event_flags, > > + &event_data); > > + if (error & NOTIFY_STOP_MASK) > > + return IRQ_NONE; > > + > > + /* > > + * Once the communication window is closed, a small delay is added to > > + * ensure the device's RDY output has been deasserted by the time the > > + * interrupt handler returns. > > + */ > > + usleep_range(50, 100); > > + > > + return IRQ_HANDLED; > > +} > > [...] > > > +static int iqs62x_probe(struct i2c_client *client, > > + const struct i2c_device_id *id) > > +{ > > + struct iqs62x_core *iqs62x; > > + struct iqs62x_info info; > > + unsigned int val; > > + int error, i, j; > > Nit: It's more common to use 'ret' or 'err' - my preference is 'ret'. > I think there are valid arguments both ways, but in my experience, the preference is not consistent across the audience of this patch series. Unless this is a deal breaker, I'd like to leave it as 'error' simply for consistency. > > + const char *fw_file = NULL; > > + > > + iqs62x = devm_kzalloc(&client->dev, sizeof(*iqs62x), GFP_KERNEL); > > + if (!iqs62x) > > + return -ENOMEM; > > + > > + i2c_set_clientdata(client, iqs62x); > > + iqs62x->client = client; > > + > > + BLOCKING_INIT_NOTIFIER_HEAD(&iqs62x->nh); > > + INIT_LIST_HEAD(&iqs62x->fw_blk_head); > > + init_completion(&iqs62x->fw_done); > > + > > + iqs62x->map = devm_regmap_init_i2c(client, &iqs62x_map_config); > > + if (IS_ERR(iqs62x->map)) { > > + error = PTR_ERR(iqs62x->map); > > + dev_err(&client->dev, "Failed to initialize register map: %d\n", > > + error); > > + return error; > > + } > > + > > + error = regmap_raw_read(iqs62x->map, IQS62X_PROD_NUM, &info, > > + sizeof(info)); > > + if (error) > > + return error; > > + > > + for (i = 0; i < IQS62X_NUM_DEV; i++) { > > + if (info.prod_num == iqs62x_devs[i].prod_num) > > + iqs62x->dev_desc = &iqs62x_devs[i]; > > + else > > + continue; > > Reads better without the else: > > if (info.prod_num != iqs62x_devs[i].prod_num) > continue; > Sure thing; will do. > > + if (info.sw_num >= iqs62x->dev_desc->sw_num) > > + iqs62x->sw_num = info.sw_num; > > + else > > + continue; > > Same as above. > Sure thing; will do. > Do you still need to be in this loop at this point? > Yes, that's because we need to determine whether the newly identified device is calibrated or not (more on that next). > > + for (j = 0; j < iqs62x->dev_desc->num_cal_regs; j++) { > > What are you doing here? Please provide a comment. > The search process here is as follows: 1. Check if the product number (device ID) is recognized, and if so: 2. Check that the software number (FW revision) is valid, and if so: 3. Check that the device's calibration (OTP) registers are non-zero (i.e. programmed) in which case some additional functionality is awarded, or the device is bad. For example, the IQS620A device can report its absolute die temperature if its scale/offset registers (0xC2 through 0xC4) have been programmed at the factory. In that case, we're actually talking to an IQS620AT device and we load an additional hwmon (soon to be iio) driver. However if they're blank, we're talking to a plain IQS620A device and stick to input and pwm drivers. In another example, the IQS621 (which includes an ambient light sensor) is _only_ sold in a calibrated version. If we happen to come across a device with empty calibration registers, its lux output will be garbage. In this case we don't register the device at all. I would be happy to add some comments here to explain what's happening. > > + error = regmap_read(iqs62x->map, > > + iqs62x->dev_desc->cal_regs[j], > > + &val); > > + if (error) > > + return error; > > + > > + if (!val) > > + break; > > + } > > + > > + if (j == iqs62x->dev_desc->num_cal_regs) > > + break; > > Is there a reason not to break here? If the product number matched > once, can it match for a second time? > It can in the case of the aforementioned IQS620A (no 'T') device. The driver first looks for the IQS620AT which defines 3 calibration registers. If we're talking to an IQS620A, the first pass of the loop (i = 0) will find 0xC2 = 0, then j < num_cal_regs and the outer loop will wind forward (i = 1). At that point, the second pass of the outer loop will check for an IQS620A, which has the same product number but defines num_cal_regs as zero. In that case, j = num_cal_regs = 0 and the outer loop will break. After the outer loop finishes, if i < NUM_DEV then we know the following: 1. The product number is recognized, and: 2. The software number is valid, and: 3. All calibration registers, if any, are nonzero. Again, this process warrants some comments and I would be happy to add some. > > + } > > + > > + if (!iqs62x->dev_desc) { > > + dev_err(&client->dev, "Unrecognized product number: 0x%02X\n", > > + info.prod_num); > > + return -EINVAL; > > + } > > + > > + if (!iqs62x->sw_num) { > > + dev_err(&client->dev, "Unrecognized software number: 0x%02X\n", > > + info.sw_num); > > + return -EINVAL; > > + } > > + > > + if (i == IQS62X_NUM_DEV) { > > + dev_err(&client->dev, "Uncalibrated device\n"); > > + return -ENODATA; > > + } > > + > > + error = regmap_write(iqs62x->map, IQS62X_SYS_SETTINGS, > > + IQS62X_SYS_SETTINGS_SOFT_RESET); > > + if (error) > > + return error; > > + usleep_range(10000, 10100); > > + > > + device_property_read_string(&client->dev, "linux,fw-file", &fw_file); > > + > > + error = request_firmware_nowait(THIS_MODULE, FW_ACTION_HOTPLUG, > > + fw_file ? : iqs62x->dev_desc->fw_file, > > + &client->dev, GFP_KERNEL, iqs62x, > > + iqs62x_fw_load); > > + if (error) > > + dev_err(&client->dev, "Failed to request firmware: %d\n", > > + error); > > + > > + return error; > > +} > > + > > +static int iqs62x_remove(struct i2c_client *client) > > +{ > > + struct iqs62x_core *iqs62x = i2c_get_clientdata(client); > > + > > + wait_for_completion(&iqs62x->fw_done); > > + > > + return 0; > > +} > > Please move the suspend/resume calls down to here. > Sure thing; will do. > > +static const struct i2c_device_id iqs62x_id[] = { > > + { "iqs620a", 0 }, > > + { "iqs621", 1 }, > > + { "iqs622", 2 }, > > + { "iqs624", 3 }, > > + { "iqs625", 4 }, > > Better to define these. In fact, do you even use the .ids? > > If not, you can use the new I2C probe function and drop this table. > Sure thing; will do. I don't use the .ids, so I'll implement a probe_new callback instead. > > + { } > > +}; > > +MODULE_DEVICE_TABLE(i2c, iqs62x_id); > > + > > +static const struct of_device_id iqs62x_of_match[] = { > > + { .compatible = "azoteq,iqs620a" }, > > + { .compatible = "azoteq,iqs621" }, > > + { .compatible = "azoteq,iqs622" }, > > + { .compatible = "azoteq,iqs624" }, > > + { .compatible = "azoteq,iqs625" }, > > + { } > > +}; > > +MODULE_DEVICE_TABLE(of, iqs62x_of_match); > > + > > +static struct i2c_driver iqs62x_i2c_driver = { > > + .driver = { > > + .name = "iqs62x", > > + .of_match_table = iqs62x_of_match, > > + .pm = &iqs62x_pm, > > + }, > > + .id_table = iqs62x_id, > > + .probe = iqs62x_probe, > > + .remove = iqs62x_remove, > > +}; > > +module_i2c_driver(iqs62x_i2c_driver); > > + > > +MODULE_AUTHOR("Jeff LaBundy <jeff@labundy.com>"); > > +MODULE_DESCRIPTION("Azoteq IQS620A/621/622/624/625 ProxFusion Sensor Family"); > > +MODULE_LICENSE("GPL"); > > diff --git a/drivers/mfd/iqs62x-tables.c b/drivers/mfd/iqs62x-tables.c > > new file mode 100644 > > index 0000000..12300b7 > > --- /dev/null > > +++ b/drivers/mfd/iqs62x-tables.c > > @@ -0,0 +1,424 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Azoteq IQS620A/621/622/624/625 ProxFusion Sensor Family > > + * > > + * Copyright (C) 2019 > > + * Author: Jeff LaBundy <jeff@labundy.com> > > + */ > > + > > +#include <linux/kernel.h> > > +#include <linux/mfd/core.h> > > +#include <linux/mfd/iqs62x.h> > > + > > +static const struct mfd_cell iqs620at_sub_devs[] = { > > + { > > + .name = IQS62X_DRV_NAME_KEYS, > > + .of_compatible = "azoteq,iqs620a-keys", > > + }, > > + { > > + .name = IQS620_DRV_NAME_PWM, > > + .of_compatible = "azoteq,iqs620a-pwm", > > + }, > > + { > > + .name = IQS620_DRV_NAME_TEMP, > > + }, > > +}; > > + > > +static const struct mfd_cell iqs620a_sub_devs[] = { > > + { > > + .name = IQS62X_DRV_NAME_KEYS, > > + .of_compatible = "azoteq,iqs620a-keys", > > + }, > > + { > > + .name = IQS620_DRV_NAME_PWM, > > + .of_compatible = "azoteq,iqs620a-pwm", > > + }, > > +}; > > + > > +static const struct mfd_cell iqs621_sub_devs[] = { > > + { > > + .name = IQS62X_DRV_NAME_KEYS, > > + .of_compatible = "azoteq,iqs621-keys", > > + }, > > + { > > + .name = IQS621_DRV_NAME_ALS, > > + }, > > +}; > > + > > +static const struct mfd_cell iqs622_sub_devs[] = { > > + { > > + .name = IQS62X_DRV_NAME_KEYS, > > + .of_compatible = "azoteq,iqs622-keys", > > + }, > > + { > > + .name = IQS622_DRV_NAME_PROX, > > + .of_compatible = "azoteq,iqs622-prox", > > + }, > > +}; > > + > > +static const struct mfd_cell iqs624_sub_devs[] = { > > + { > > + .name = IQS62X_DRV_NAME_KEYS, > > + .of_compatible = "azoteq,iqs624-keys", > > + }, > > + { > > + .name = IQS624_DRV_NAME_POS, > > + }, > > +}; > > + > > +static const struct mfd_cell iqs625_sub_devs[] = { > > + { > > + .name = IQS62X_DRV_NAME_KEYS, > > + .of_compatible = "azoteq,iqs625-keys", > > + }, > > + { > > + .name = IQS624_DRV_NAME_POS, > > + }, > > +}; > > These should be moved into the core driver. > The reason they're placed here is because they're referenced in the iqs62x_devs array, members of which are then referenced by devm_mfd_add_devices in the core driver. If the mfd_cell arrays move to the core driver (where they're not used directly), I think I'd have to make them extern. I think it's cleaner to limit the scope of any given element to the minimum level that is necessary. However if I have misunderstood or I could possibly make this more clear with a comment or two, please let me know. > > +static const u8 iqs620at_cal_regs[] = { 0xC2, 0xC3, 0xC4, }; > > +static const u8 iqs621_cal_regs[] = { 0x82, 0x83, }; > > What do these mean? > These are the register addresses that correspond to the aforementioned calibration (OTP) registers used for device identification. > Probably best of do define them, so people can read them. > Sure thing; will do. > [...] > > -- > Lee Jones [李琼斯] > Linaro Services Technical Lead > Linaro.org │ Open source software for ARM SoCs > Follow Linaro: Facebook | Twitter | Blog > [0] https://github.com/torvalds/linux/blob/master/sound/soc/codecs/wm_adsp.c [1] https://github.com/jlabundy/iqs62x-h2bin/blob/master/README.md Kind regards, Jeff LaBundy
On Thu, 31 Oct 2019, Jeff LaBundy wrote: > On Thu, Oct 31, 2019 at 01:44:10PM +0000, Lee Jones wrote: > > On Sun, 20 Oct 2019, Jeff LaBundy wrote: > > > > > This patch adds support for core functions common to all six-channel > > > members of the Azoteq ProxFusion family of sensor devices. > > > > > > Signed-off-by: Jeff LaBundy <jeff@labundy.com> > > > --- > > > drivers/mfd/Kconfig | 13 + > > > drivers/mfd/Makefile | 2 + > > > drivers/mfd/iqs62x-core.c | 638 ++++++++++++++++++++++++++++++++++++++++++++ > > > drivers/mfd/iqs62x-tables.c | 424 +++++++++++++++++++++++++++++ > > > include/linux/mfd/iqs62x.h | 148 ++++++++++ > > > 5 files changed, 1225 insertions(+) > > > create mode 100644 drivers/mfd/iqs62x-core.c > > > create mode 100644 drivers/mfd/iqs62x-tables.c > > > create mode 100644 include/linux/mfd/iqs62x.h > > > > > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > > > index ae24d3e..df391f7 100644 > > > --- a/drivers/mfd/Kconfig > > > +++ b/drivers/mfd/Kconfig > > > @@ -642,6 +642,19 @@ config MFD_IPAQ_MICRO > > > AT90LS8535 microcontroller flashed with a special iPAQ > > > firmware using the custom protocol implemented in this driver. [...] > > What is preventing a very naughty person from providing their own > > register map (firmware) in order to read/write unsuitable registers > > from kernel context for their own gains; simply by swapping out a file > > contained in userspace? > > I would argue that if someone is willing to go that length, they likely understand > that their dock switch sensitivity may change or their ambient light sensor may no > longer function properly. > > > It would probably be a better idea to compile the register definitions > > with the kernel/module to be safe. You can use Device Tree for > > run-time configuration changes. > > Taking the IQS620A as an example, there are over 100 individual fields that need > configured. Forcing customers to manually transfer the values derived within the > GUI to a corresponding collection of device tree bindings would be prohibitively > complex. > > To complicate matters, many registers change meaning or restrict their available > values based on the values stored in other registers. Duplicating all of the de- > pendencies and restrictions comprehended by the vendor's tool in the driver and/ > or the bindings document would not be practical. > > Just to clarify, we're not storing register definitions (i.e. addresses) in this > "firmware"; rather, we're storing application-specific register values that don't > belong in the driver source. Okay, this allays my fears. I was under the impression that you could manipulate addresses in the firmware in order to read/write from non-expected registers in kernel context. [...] > > > + /* > > > + * The device resets itself in response to the I2C master stalling > > > + * communication beyond a timeout. In this case, all registers are > > > + * restored and any interested sub-device drivers are notified. > > > + */ > > > + if (event_flags & BIT(IQS62X_EVENT_SYS_RESET)) { > > > + dev_err(&iqs62x->client->dev, "Unexpected device reset\n"); > > > + > > > + error = iqs62x_dev_init(iqs62x); > > > > Is it safe to re-initialise the entire device in IRQ context? > > > > Here, we are simply re-writing several registers from memory. This is a threaded > interrupt handler, so it should be safe to do so. But if I've misunderstood your > concern, please let me know. My intent here is to ensure it's been thought about. I see that you are in a threaded handler, so it should be save to read/write register and sleep. > > > + if (error) { > > > + dev_err(&iqs62x->client->dev, > > > + "Failed to re-initialize device: %d\n", error); > > > + return IRQ_NONE; > > > + } > > > + } > > > + > > > + error = blocking_notifier_call_chain(&iqs62x->nh, event_flags, > > > + &event_data); > > > + if (error & NOTIFY_STOP_MASK) > > > + return IRQ_NONE; > > > + > > > + /* > > > + * Once the communication window is closed, a small delay is added to > > > + * ensure the device's RDY output has been deasserted by the time the > > > + * interrupt handler returns. > > > + */ > > > + usleep_range(50, 100); > > > + > > > + return IRQ_HANDLED; > > > +} > > > > [...] > > > > > +static int iqs62x_probe(struct i2c_client *client, > > > + const struct i2c_device_id *id) > > > +{ > > > + struct iqs62x_core *iqs62x; > > > + struct iqs62x_info info; > > > + unsigned int val; > > > + int error, i, j; > > > > Nit: It's more common to use 'ret' or 'err' - my preference is 'ret'. > > > > I think there are valid arguments both ways, but in my experience, the preference > is not consistent across the audience of this patch series. Unless this is a deal > breaker, I'd like to leave it as 'error' simply for consistency. The difference is *very* significant, more than an order of magnitude: $ git grep "int.* ret[;\|,]" | wc -l 40549 $ git grep "int.* err[;\|,]" | wc -l 18558 $ git grep "int.* error[;\|,]" | wc -l 3381 [...] > > > + for (j = 0; j < iqs62x->dev_desc->num_cal_regs; j++) { > > > > What are you doing here? Please provide a comment. > > The search process here is as follows: > > 1. Check if the product number (device ID) is recognized, and if so: > 2. Check that the software number (FW revision) is valid, and if so: > 3. Check that the device's calibration (OTP) registers are non-zero (i.e. > programmed) in which case some additional functionality is awarded, or > the device is bad. > > For example, the IQS620A device can report its absolute die temperature if > its scale/offset registers (0xC2 through 0xC4) have been programmed at the > factory. In that case, we're actually talking to an IQS620AT device and we > load an additional hwmon (soon to be iio) driver. However if they're blank, > we're talking to a plain IQS620A device and stick to input and pwm drivers. > > In another example, the IQS621 (which includes an ambient light sensor) is > _only_ sold in a calibrated version. If we happen to come across a device > with empty calibration registers, its lux output will be garbage. In this > case we don't register the device at all. > > I would be happy to add some comments here to explain what's happening. Please. > > > + error = regmap_read(iqs62x->map, > > > + iqs62x->dev_desc->cal_regs[j], > > > + &val); > > > + if (error) > > > + return error; > > > + > > > + if (!val) > > > + break; > > > + } > > > + > > > + if (j == iqs62x->dev_desc->num_cal_regs) > > > + break; > > > > Is there a reason not to break here? If the product number matched > > once, can it match for a second time? > > > > It can in the case of the aforementioned IQS620A (no 'T') device. The driver > first looks for the IQS620AT which defines 3 calibration registers. If we're > talking to an IQS620A, the first pass of the loop (i = 0) will find 0xC2 = 0, > then j < num_cal_regs and the outer loop will wind forward (i = 1). > > At that point, the second pass of the outer loop will check for an IQS620A, > which has the same product number but defines num_cal_regs as zero. In that > case, j = num_cal_regs = 0 and the outer loop will break. > > After the outer loop finishes, if i < NUM_DEV then we know the following: > > 1. The product number is recognized, and: > 2. The software number is valid, and: > 3. All calibration registers, if any, are nonzero. > > Again, this process warrants some comments and I would be happy to add some. Great, thanks. [...] > > > +static const struct mfd_cell iqs625_sub_devs[] = { > > > + { > > > + .name = IQS62X_DRV_NAME_KEYS, > > > + .of_compatible = "azoteq,iqs625-keys", > > > + }, > > > + { > > > + .name = IQS624_DRV_NAME_POS, > > > + }, > > > +}; > > > > These should be moved into the core driver. > > > > The reason they're placed here is because they're referenced in the iqs62x_devs > array, members of which are then referenced by devm_mfd_add_devices in the core > driver. > > If the mfd_cell arrays move to the core driver (where they're not used directly), > I think I'd have to make them extern. I think it's cleaner to limit the scope of > any given element to the minimum level that is necessary. > > However if I have misunderstood or I could possibly make this more clear with a > comment or two, please let me know. Leave them where they are for now. I still need to do a review of this file. It's strange to see such an odd weave of; registers, masks, files, values and names bundled up in structure arrays like this. It may take a little time. [...]
Hi Lee, On Fri, Nov 01, 2019 at 08:56:12AM +0000, Lee Jones wrote: > On Thu, 31 Oct 2019, Jeff LaBundy wrote: > > On Thu, Oct 31, 2019 at 01:44:10PM +0000, Lee Jones wrote: > > > On Sun, 20 Oct 2019, Jeff LaBundy wrote: > > > > > > > This patch adds support for core functions common to all six-channel > > > > members of the Azoteq ProxFusion family of sensor devices. > > > > > > > > Signed-off-by: Jeff LaBundy <jeff@labundy.com> > > > > --- > > > > drivers/mfd/Kconfig | 13 + > > > > drivers/mfd/Makefile | 2 + > > > > drivers/mfd/iqs62x-core.c | 638 ++++++++++++++++++++++++++++++++++++++++++++ > > > > drivers/mfd/iqs62x-tables.c | 424 +++++++++++++++++++++++++++++ > > > > include/linux/mfd/iqs62x.h | 148 ++++++++++ > > > > 5 files changed, 1225 insertions(+) > > > > create mode 100644 drivers/mfd/iqs62x-core.c > > > > create mode 100644 drivers/mfd/iqs62x-tables.c > > > > create mode 100644 include/linux/mfd/iqs62x.h > > > > > > > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > > > > index ae24d3e..df391f7 100644 > > > > --- a/drivers/mfd/Kconfig > > > > +++ b/drivers/mfd/Kconfig > > > > @@ -642,6 +642,19 @@ config MFD_IPAQ_MICRO > > > > AT90LS8535 microcontroller flashed with a special iPAQ > > > > firmware using the custom protocol implemented in this driver. > > [...] > > > > What is preventing a very naughty person from providing their own > > > register map (firmware) in order to read/write unsuitable registers > > > from kernel context for their own gains; simply by swapping out a file > > > contained in userspace? > > > > I would argue that if someone is willing to go that length, they likely understand > > that their dock switch sensitivity may change or their ambient light sensor may no > > longer function properly. > > > > > It would probably be a better idea to compile the register definitions > > > with the kernel/module to be safe. You can use Device Tree for > > > run-time configuration changes. > > > > Taking the IQS620A as an example, there are over 100 individual fields that need > > configured. Forcing customers to manually transfer the values derived within the > > GUI to a corresponding collection of device tree bindings would be prohibitively > > complex. > > > > To complicate matters, many registers change meaning or restrict their available > > values based on the values stored in other registers. Duplicating all of the de- > > pendencies and restrictions comprehended by the vendor's tool in the driver and/ > > or the bindings document would not be practical. > > > > Just to clarify, we're not storing register definitions (i.e. addresses) in this > > "firmware"; rather, we're storing application-specific register values that don't > > belong in the driver source. > > Okay, this allays my fears. I was under the impression that you could > manipulate addresses in the firmware in order to read/write from > non-expected registers in kernel context. > My apologies, as my response was misleading. We're not defining register addresses in this firmware the way a driver might (e.g. #define IQS62X_SYS_FLAGS 0x10). But we are in fact storing the address to which the vendor's GUI requests that one or more data bytes be written. The firmware in this case is largely based off Intel hex format, with some extensions to handle the device family's specific calibration needs. Specifically, in some cases a field within a register is populated by OTP memory and the remaining fields must be modified using a R/M/W operation. The tool comprehends all such registers. And just like Intel hex, if a user _really_ wanted to be ornery, that user could edit the address field of a record and re-route the record's data to an unused or reserved register address. This is also the case for the aforementioned wm_adsp framework that stores register address offsets in a .bin file which is also handled as "firmware." Any any rate--I only clarify this to be transparent, and to point out that we're not doing anything new. If there is any further information I can provide, please let me know. > [...] > > > > > + /* > > > > + * The device resets itself in response to the I2C master stalling > > > > + * communication beyond a timeout. In this case, all registers are > > > > + * restored and any interested sub-device drivers are notified. > > > > + */ > > > > + if (event_flags & BIT(IQS62X_EVENT_SYS_RESET)) { > > > > + dev_err(&iqs62x->client->dev, "Unexpected device reset\n"); > > > > + > > > > + error = iqs62x_dev_init(iqs62x); > > > > > > Is it safe to re-initialise the entire device in IRQ context? > > > > > > > Here, we are simply re-writing several registers from memory. This is a threaded > > interrupt handler, so it should be safe to do so. But if I've misunderstood your > > concern, please let me know. > > My intent here is to ensure it's been thought about. I see that you > are in a threaded handler, so it should be save to read/write register > and sleep. > > > > > + if (error) { > > > > + dev_err(&iqs62x->client->dev, > > > > + "Failed to re-initialize device: %d\n", error); > > > > + return IRQ_NONE; > > > > + } > > > > + } > > > > + > > > > + error = blocking_notifier_call_chain(&iqs62x->nh, event_flags, > > > > + &event_data); > > > > + if (error & NOTIFY_STOP_MASK) > > > > + return IRQ_NONE; > > > > + > > > > + /* > > > > + * Once the communication window is closed, a small delay is added to > > > > + * ensure the device's RDY output has been deasserted by the time the > > > > + * interrupt handler returns. > > > > + */ > > > > + usleep_range(50, 100); > > > > + > > > > + return IRQ_HANDLED; > > > > +} > > > > > > [...] > > > > > > > +static int iqs62x_probe(struct i2c_client *client, > > > > + const struct i2c_device_id *id) > > > > +{ > > > > + struct iqs62x_core *iqs62x; > > > > + struct iqs62x_info info; > > > > + unsigned int val; > > > > + int error, i, j; > > > > > > Nit: It's more common to use 'ret' or 'err' - my preference is 'ret'. > > > > > > > I think there are valid arguments both ways, but in my experience, the preference > > is not consistent across the audience of this patch series. Unless this is a deal > > breaker, I'd like to leave it as 'error' simply for consistency. > > The difference is *very* significant, more than an order of magnitude: > > $ git grep "int.* ret[;\|,]" | wc -l > 40549 > $ git grep "int.* err[;\|,]" | wc -l > 18558 > $ git grep "int.* error[;\|,]" | wc -l > 3381 > > [...] > I'm happy to change to 'ret' here; I'd simply like to remain consistent within this patch series. @Dmitry: I know that 'error' tends to be preferred in input and the reasoning makes sense to me, but I'd like to change 'error' to 'ret' in patch [3/8] as well. If you have any objection, please let me know. > > > > + for (j = 0; j < iqs62x->dev_desc->num_cal_regs; j++) { > > > > > > What are you doing here? Please provide a comment. > > > > The search process here is as follows: > > > > 1. Check if the product number (device ID) is recognized, and if so: > > 2. Check that the software number (FW revision) is valid, and if so: > > 3. Check that the device's calibration (OTP) registers are non-zero (i.e. > > programmed) in which case some additional functionality is awarded, or > > the device is bad. > > > > For example, the IQS620A device can report its absolute die temperature if > > its scale/offset registers (0xC2 through 0xC4) have been programmed at the > > factory. In that case, we're actually talking to an IQS620AT device and we > > load an additional hwmon (soon to be iio) driver. However if they're blank, > > we're talking to a plain IQS620A device and stick to input and pwm drivers. > > > > In another example, the IQS621 (which includes an ambient light sensor) is > > _only_ sold in a calibrated version. If we happen to come across a device > > with empty calibration registers, its lux output will be garbage. In this > > case we don't register the device at all. > > > > I would be happy to add some comments here to explain what's happening. > > Please. > > > > > + error = regmap_read(iqs62x->map, > > > > + iqs62x->dev_desc->cal_regs[j], > > > > + &val); > > > > + if (error) > > > > + return error; > > > > + > > > > + if (!val) > > > > + break; > > > > + } > > > > + > > > > + if (j == iqs62x->dev_desc->num_cal_regs) > > > > + break; > > > > > > Is there a reason not to break here? If the product number matched > > > once, can it match for a second time? > > > > > > > It can in the case of the aforementioned IQS620A (no 'T') device. The driver > > first looks for the IQS620AT which defines 3 calibration registers. If we're > > talking to an IQS620A, the first pass of the loop (i = 0) will find 0xC2 = 0, > > then j < num_cal_regs and the outer loop will wind forward (i = 1). > > > > At that point, the second pass of the outer loop will check for an IQS620A, > > which has the same product number but defines num_cal_regs as zero. In that > > case, j = num_cal_regs = 0 and the outer loop will break. > > > > After the outer loop finishes, if i < NUM_DEV then we know the following: > > > > 1. The product number is recognized, and: > > 2. The software number is valid, and: > > 3. All calibration registers, if any, are nonzero. > > > > Again, this process warrants some comments and I would be happy to add some. > > Great, thanks. > > [...] > > > > > +static const struct mfd_cell iqs625_sub_devs[] = { > > > > + { > > > > + .name = IQS62X_DRV_NAME_KEYS, > > > > + .of_compatible = "azoteq,iqs625-keys", > > > > + }, > > > > + { > > > > + .name = IQS624_DRV_NAME_POS, > > > > + }, > > > > +}; > > > > > > These should be moved into the core driver. > > > > > > > The reason they're placed here is because they're referenced in the iqs62x_devs > > array, members of which are then referenced by devm_mfd_add_devices in the core > > driver. > > > > If the mfd_cell arrays move to the core driver (where they're not used directly), > > I think I'd have to make them extern. I think it's cleaner to limit the scope of > > any given element to the minimum level that is necessary. > > > > However if I have misunderstood or I could possibly make this more clear with a > > comment or two, please let me know. > > Leave them where they are for now. I still need to do a review of this > file. It's strange to see such an odd weave of; registers, masks, > files, values and names bundled up in structure arrays like this. It > may take a little time. > Sure thing; will do. The reason for this arrangement is that this family of devices exposes status bits at (mostly) the same bit locations within a given register, but the addresses of those particular registers may vary per device. The structures and arrays in this file help keep common functions simple and generic, with all device- specific details in a separate location that's easy to maintain. > [...] > > -- > Lee Jones [李琼斯] > Linaro Services Technical Lead > Linaro.org │ Open source software for ARM SoCs > Follow Linaro: Facebook | Twitter | Blog > Kind regards, Jeff LaBundy
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index ae24d3e..df391f7 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -642,6 +642,19 @@ config MFD_IPAQ_MICRO AT90LS8535 microcontroller flashed with a special iPAQ firmware using the custom protocol implemented in this driver. +config MFD_IQS62X + tristate "Azoteq IQS620A/621/622/624/625 core support" + depends on I2C + select MFD_CORE + select REGMAP_I2C + help + Say Y here if you want to build support for six-channel members of + the Azoteq ProxFusion family of sensor devices. Additional options + must be selected to enable device-specific functions. + + To compile this driver as a module, choose M here: the module will + be called iqs62x. + config MFD_JANZ_CMODIO tristate "Janz CMOD-IO PCI MODULbus Carrier Board" select MFD_CORE diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index c1067ea..23dd71c6 100644 --- a/drivers/mfd/Makefile +++ b/drivers/mfd/Makefile @@ -256,3 +256,5 @@ obj-$(CONFIG_MFD_ROHM_BD70528) += rohm-bd70528.o obj-$(CONFIG_MFD_ROHM_BD718XX) += rohm-bd718x7.o obj-$(CONFIG_MFD_STMFX) += stmfx.o +iqs62x-objs := iqs62x-core.o iqs62x-tables.o +obj-$(CONFIG_MFD_IQS62X) += iqs62x.o diff --git a/drivers/mfd/iqs62x-core.c b/drivers/mfd/iqs62x-core.c new file mode 100644 index 0000000..e2200c8 --- /dev/null +++ b/drivers/mfd/iqs62x-core.c @@ -0,0 +1,638 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Azoteq IQS620A/621/622/624/625 ProxFusion Sensor Family + * + * Copyright (C) 2019 + * Author: Jeff LaBundy <jeff@labundy.com> + * + * These devices rely on application-specific register settings and calibration + * data developed in and exported from a suite of GUIs offered by the vendor. A + * separate tool converts the GUIs' ASCII-based output into a standard firmware + * file parsed by the driver. + * + * Link to data sheets and GUIs: https://www.azoteq.com/products/proxfusion/ + * + * Link to conversion tool: https://github.com/jlabundy/iqs62x-h2bin.git + */ + +#include <linux/completion.h> +#include <linux/delay.h> +#include <linux/device.h> +#include <linux/err.h> +#include <linux/firmware.h> +#include <linux/i2c.h> +#include <linux/interrupt.h> +#include <linux/kernel.h> +#include <linux/list.h> +#include <linux/mfd/core.h> +#include <linux/module.h> +#include <linux/notifier.h> +#include <linux/of_device.h> +#include <linux/property.h> +#include <linux/regmap.h> +#include <linux/slab.h> +#include <asm/unaligned.h> + +#include <linux/mfd/iqs62x.h> + +#define IQS62X_PROD_NUM 0x00 + +#define IQS62X_SYS_FLAGS 0x10 +#define IQS62X_SYS_FLAGS_IN_ATI BIT(2) + +#define IQS622_PROX_SETTINGS_4 0x48 +#define IQS620_PROX_SETTINGS_4 0x50 +#define IQS620_PROX_SETTINGS_4_SAR_EN BIT(7) + +#define IQS62X_SYS_SETTINGS 0xD0 +#define IQS62X_SYS_SETTINGS_SOFT_RESET BIT(7) +#define IQS62X_SYS_SETTINGS_ACK_RESET BIT(6) +#define IQS62X_SYS_SETTINGS_EVENT_MODE BIT(5) +#define IQS62X_SYS_SETTINGS_REDO_ATI BIT(1) + +#define IQS62X_PWR_SETTINGS 0xD2 +#define IQS62X_PWR_SETTINGS_DIS_AUTO BIT(5) +#define IQS62X_PWR_SETTINGS_PWR_MODE_MASK (BIT(4) | BIT(3)) +#define IQS62X_PWR_SETTINGS_PWR_MODE_HALT (BIT(4) | BIT(3)) +#define IQS62X_PWR_SETTINGS_PWR_MODE_NORM 0 + +#define IQS62X_OTP_CMD 0xF0 +#define IQS62X_OTP_CMD_FG3 0x13 +#define IQS62X_OTP_DATA 0xF1 +#define IQS62X_MAX_REG 0xFF + +#define IQS62X_HALL_CAL_MASK 0x0F + +#define IQS62X_ATI_TIMEOUT 10 + +#define IQS62X_FW_REC_TYPE_INFO 0 +#define IQS62X_FW_REC_TYPE_PROD 1 +#define IQS62X_FW_REC_TYPE_HALL 2 +#define IQS62X_FW_REC_TYPE_MASK 3 +#define IQS62X_FW_REC_TYPE_DATA 4 + +struct iqs62x_fw_rec { + u8 type; + u8 addr; + u8 len; + u8 data; +} __packed; + +struct iqs62x_fw_blk { + struct list_head list; + u8 addr; + u8 mask; + u8 len; + u8 data[]; +}; + +struct iqs62x_info { + u8 prod_num; + u8 sw_num; + u8 hw_num; +} __packed; + +static int iqs62x_dev_init(struct iqs62x_core *iqs62x) +{ + struct iqs62x_fw_blk *fw_blk; + unsigned int val; + int error, i; + + list_for_each_entry(fw_blk, &iqs62x->fw_blk_head, list) { + if (fw_blk->mask) + error = regmap_update_bits(iqs62x->map, fw_blk->addr, + fw_blk->mask, *fw_blk->data); + else + error = regmap_raw_write(iqs62x->map, fw_blk->addr, + fw_blk->data, fw_blk->len); + if (error) + return error; + } + + switch (iqs62x->dev_desc->prod_num) { + case IQS620_PROD_NUM: + case IQS622_PROD_NUM: + error = regmap_read(iqs62x->map, + iqs62x->dev_desc->prod_num == + IQS620_PROD_NUM ? IQS620_PROX_SETTINGS_4 : + IQS622_PROX_SETTINGS_4, + &val); + if (error) + return error; + + if (val & IQS620_PROX_SETTINGS_4_SAR_EN) + iqs62x->ui_sel = IQS62X_UI_SAR1; + /* fall through */ + + case IQS621_PROD_NUM: + error = regmap_write(iqs62x->map, IQS620_GLBL_EVENT_MASK, + IQS620_GLBL_EVENT_MASK_PMU | + iqs62x->dev_desc->prox_mask | + iqs62x->dev_desc->sar_mask | + iqs62x->dev_desc->hall_mask | + iqs62x->dev_desc->hyst_mask | + iqs62x->dev_desc->temp_mask | + iqs62x->dev_desc->als_mask | + iqs62x->dev_desc->ir_mask); + if (error) + return error; + break; + + default: + error = regmap_write(iqs62x->map, IQS624_HALL_UI, + IQS624_HALL_UI_WHL_EVENT | + IQS624_HALL_UI_INT_EVENT | + IQS624_HALL_UI_AUTO_CAL); + if (error) + return error; + + error = regmap_read(iqs62x->map, IQS624_INTERVAL_DIV, &val); + if (error) + return error; + + if (val >= iqs62x->dev_desc->interval_div) + break; + + error = regmap_write(iqs62x->map, IQS624_INTERVAL_DIV, + iqs62x->dev_desc->interval_div); + if (error) + return error; + } + + error = regmap_update_bits(iqs62x->map, IQS62X_SYS_SETTINGS, + IQS62X_SYS_SETTINGS_ACK_RESET | + IQS62X_SYS_SETTINGS_EVENT_MODE | + IQS62X_SYS_SETTINGS_REDO_ATI, 0xFF); + if (error) + return error; + + for (i = 0; i < IQS62X_ATI_TIMEOUT; i++) { + msleep(50); + + error = regmap_read(iqs62x->map, IQS62X_SYS_FLAGS, &val); + if (error) + return error; + + if (!(val & IQS62X_SYS_FLAGS_IN_ATI)) + break; + } + + if (i == IQS62X_ATI_TIMEOUT) + return -ETIME; + + /* + * The following delay accommodates the post-ATI stabilization time + * specified in the data sheet (with additional margin). + */ + msleep(150); + + return 0; +} + +static int iqs62x_fw_parse(struct iqs62x_core *iqs62x, + const struct firmware *fw) +{ + struct i2c_client *client = iqs62x->client; + struct iqs62x_fw_rec *fw_rec; + struct iqs62x_fw_blk *fw_blk; + unsigned int hall_cal_index = 0; + size_t pos = 0; + int error = 0; + u8 mask, len; + u8 *data; + + while (pos < fw->size) { + if (pos + sizeof(*fw_rec) > fw->size) { + error = -EINVAL; + break; + } + fw_rec = (struct iqs62x_fw_rec *)(fw->data + pos); + pos += sizeof(*fw_rec); + + if (pos + fw_rec->len - 1 > fw->size) { + error = -EINVAL; + break; + } + pos += fw_rec->len - 1; + + switch (fw_rec->type) { + case IQS62X_FW_REC_TYPE_INFO: + continue; + + case IQS62X_FW_REC_TYPE_PROD: + if (fw_rec->data == iqs62x->dev_desc->prod_num) + continue; + + dev_err(&client->dev, + "Incompatible product number: 0x%02X\n", + fw_rec->data); + error = -EINVAL; + break; + + case IQS62X_FW_REC_TYPE_HALL: + if (!hall_cal_index) { + error = regmap_write(iqs62x->map, + IQS62X_OTP_CMD, + IQS62X_OTP_CMD_FG3); + if (error) + break; + + error = regmap_read(iqs62x->map, + IQS62X_OTP_DATA, + &hall_cal_index); + if (error) + break; + + hall_cal_index &= IQS62X_HALL_CAL_MASK; + if (!hall_cal_index) { + dev_err(&client->dev, + "Uncalibrated device\n"); + error = -ENODATA; + break; + } + } + + if (hall_cal_index > fw_rec->len) { + error = -EINVAL; + break; + } + + mask = 0; + data = &fw_rec->data + hall_cal_index - 1; + len = sizeof(*data); + break; + + case IQS62X_FW_REC_TYPE_MASK: + if (fw_rec->len < (sizeof(mask) + sizeof(*data))) { + error = -EINVAL; + break; + } + + mask = fw_rec->data; + data = &fw_rec->data + sizeof(mask); + len = sizeof(*data); + break; + + case IQS62X_FW_REC_TYPE_DATA: + mask = 0; + data = &fw_rec->data; + len = fw_rec->len; + break; + + default: + dev_err(&client->dev, + "Unrecognized record type: 0x%02X\n", + fw_rec->type); + error = -EINVAL; + } + + if (error) + break; + + fw_blk = devm_kzalloc(&client->dev, + struct_size(fw_blk, data, len), + GFP_KERNEL); + if (!fw_blk) { + error = -ENOMEM; + break; + } + + fw_blk->addr = fw_rec->addr; + fw_blk->mask = mask; + fw_blk->len = len; + memcpy(fw_blk->data, data, len); + + list_add(&fw_blk->list, &iqs62x->fw_blk_head); + } + + release_firmware(fw); + + return error; +} + +static irqreturn_t iqs62x_irq(int irq, void *context) +{ + struct iqs62x_core *iqs62x = context; + struct iqs62x_event_data event_data; + struct iqs62x_event_desc event_desc; + enum iqs62x_event_reg event_reg; + unsigned long event_flags = 0; + int error, i, j; + u8 event_map[IQS62X_EVENT_SIZE]; + + /* + * The device asserts the RDY output to signal the beginning of a + * communication window, which is closed by an I2C stop condition. + * As such, all interrupt status is captured in a single read and + * broadcast to any interested sub-device drivers. + */ + error = regmap_raw_read(iqs62x->map, IQS62X_SYS_FLAGS, + event_map, sizeof(event_map)); + if (error) + return IRQ_NONE; + + for (i = 0; i < sizeof(event_map); i++) { + event_reg = iqs62x->dev_desc->event_regs[iqs62x->ui_sel][i]; + + switch (event_reg) { + case IQS62X_EVENT_UI_LO: + event_data.ui_data = get_unaligned_le16(&event_map[i]); + /* fall through */ + case IQS62X_EVENT_UI_HI: + case IQS62X_EVENT_NONE: + case IQS62X_EVENT_GLBL: + continue; + + case IQS62X_EVENT_TEMP: + event_data.temp_flags = event_map[i]; + continue; + + case IQS62X_EVENT_ALS: + event_data.als_flags = event_map[i]; + continue; + + case IQS62X_EVENT_IR: + event_data.ir_flags = event_map[i]; + continue; + + case IQS62X_EVENT_INTER: + event_data.interval = event_map[i]; + continue; + + case IQS62X_EVENT_HYST: + event_map[i] <<= iqs62x->dev_desc->hyst_shift; + /* fall through */ + case IQS62X_EVENT_WHEEL: + case IQS62X_EVENT_HALL: + case IQS62X_EVENT_PROX: + case IQS62X_EVENT_SYS: + break; + } + + for (j = 0; j < IQS62X_NUM_EVENTS; j++) { + event_desc = iqs62x_events[j]; + + if (event_desc.reg != event_reg) + continue; + + if ((event_map[i] & event_desc.mask) == event_desc.val) + event_flags |= BIT(j); + } + } + + /* + * The device resets itself in response to the I2C master stalling + * communication beyond a timeout. In this case, all registers are + * restored and any interested sub-device drivers are notified. + */ + if (event_flags & BIT(IQS62X_EVENT_SYS_RESET)) { + dev_err(&iqs62x->client->dev, "Unexpected device reset\n"); + + error = iqs62x_dev_init(iqs62x); + if (error) { + dev_err(&iqs62x->client->dev, + "Failed to re-initialize device: %d\n", error); + return IRQ_NONE; + } + } + + error = blocking_notifier_call_chain(&iqs62x->nh, event_flags, + &event_data); + if (error & NOTIFY_STOP_MASK) + return IRQ_NONE; + + /* + * Once the communication window is closed, a small delay is added to + * ensure the device's RDY output has been deasserted by the time the + * interrupt handler returns. + */ + usleep_range(50, 100); + + return IRQ_HANDLED; +} + +static void iqs62x_fw_load(const struct firmware *fw, void *context) +{ + struct iqs62x_core *iqs62x = context; + struct i2c_client *client = iqs62x->client; + int error; + + if (fw) { + error = iqs62x_fw_parse(iqs62x, fw); + if (error) { + dev_err(&client->dev, "Failed to parse firmware: %d\n", + error); + goto err_out; + } + } + + error = iqs62x_dev_init(iqs62x); + if (error) { + dev_err(&client->dev, "Failed to initialize device: %d\n", + error); + goto err_out; + } + + error = devm_request_threaded_irq(&client->dev, client->irq, + NULL, iqs62x_irq, IRQF_ONESHOT, + client->name, iqs62x); + if (error) { + dev_err(&client->dev, "Failed to request IRQ: %d\n", error); + goto err_out; + } + + error = devm_mfd_add_devices(&client->dev, -1, + iqs62x->dev_desc->sub_devs, + iqs62x->dev_desc->num_sub_devs, + NULL, 0, NULL); + if (error) + dev_err(&client->dev, "Failed to add devices: %d\n", error); + +err_out: + complete_all(&iqs62x->fw_done); +} + +static int __maybe_unused iqs62x_suspend(struct device *dev) +{ + struct iqs62x_core *iqs62x = dev_get_drvdata(dev); + int error; + + wait_for_completion(&iqs62x->fw_done); + + /* + * As per the data sheet, automatic mode switching must be disabled + * before the device is placed in or taken out of halt mode. + */ + error = regmap_update_bits(iqs62x->map, IQS62X_PWR_SETTINGS, + IQS62X_PWR_SETTINGS_DIS_AUTO, + IQS62X_PWR_SETTINGS_DIS_AUTO); + if (error) + return error; + + return regmap_update_bits(iqs62x->map, IQS62X_PWR_SETTINGS, + IQS62X_PWR_SETTINGS_PWR_MODE_MASK, + IQS62X_PWR_SETTINGS_PWR_MODE_HALT); +} + +static int __maybe_unused iqs62x_resume(struct device *dev) +{ + struct iqs62x_core *iqs62x = dev_get_drvdata(dev); + int error; + + error = regmap_update_bits(iqs62x->map, IQS62X_PWR_SETTINGS, + IQS62X_PWR_SETTINGS_PWR_MODE_MASK, + IQS62X_PWR_SETTINGS_PWR_MODE_NORM); + if (error) + return error; + + return regmap_update_bits(iqs62x->map, IQS62X_PWR_SETTINGS, + IQS62X_PWR_SETTINGS_DIS_AUTO, 0); +} + +static SIMPLE_DEV_PM_OPS(iqs62x_pm, iqs62x_suspend, iqs62x_resume); + +static const struct regmap_config iqs62x_map_config = { + .reg_bits = 8, + .val_bits = 8, + .max_register = IQS62X_MAX_REG, +}; + +static int iqs62x_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct iqs62x_core *iqs62x; + struct iqs62x_info info; + unsigned int val; + int error, i, j; + const char *fw_file = NULL; + + iqs62x = devm_kzalloc(&client->dev, sizeof(*iqs62x), GFP_KERNEL); + if (!iqs62x) + return -ENOMEM; + + i2c_set_clientdata(client, iqs62x); + iqs62x->client = client; + + BLOCKING_INIT_NOTIFIER_HEAD(&iqs62x->nh); + INIT_LIST_HEAD(&iqs62x->fw_blk_head); + init_completion(&iqs62x->fw_done); + + iqs62x->map = devm_regmap_init_i2c(client, &iqs62x_map_config); + if (IS_ERR(iqs62x->map)) { + error = PTR_ERR(iqs62x->map); + dev_err(&client->dev, "Failed to initialize register map: %d\n", + error); + return error; + } + + error = regmap_raw_read(iqs62x->map, IQS62X_PROD_NUM, &info, + sizeof(info)); + if (error) + return error; + + for (i = 0; i < IQS62X_NUM_DEV; i++) { + if (info.prod_num == iqs62x_devs[i].prod_num) + iqs62x->dev_desc = &iqs62x_devs[i]; + else + continue; + + if (info.sw_num >= iqs62x->dev_desc->sw_num) + iqs62x->sw_num = info.sw_num; + else + continue; + + for (j = 0; j < iqs62x->dev_desc->num_cal_regs; j++) { + error = regmap_read(iqs62x->map, + iqs62x->dev_desc->cal_regs[j], + &val); + if (error) + return error; + + if (!val) + break; + } + + if (j == iqs62x->dev_desc->num_cal_regs) + break; + } + + if (!iqs62x->dev_desc) { + dev_err(&client->dev, "Unrecognized product number: 0x%02X\n", + info.prod_num); + return -EINVAL; + } + + if (!iqs62x->sw_num) { + dev_err(&client->dev, "Unrecognized software number: 0x%02X\n", + info.sw_num); + return -EINVAL; + } + + if (i == IQS62X_NUM_DEV) { + dev_err(&client->dev, "Uncalibrated device\n"); + return -ENODATA; + } + + error = regmap_write(iqs62x->map, IQS62X_SYS_SETTINGS, + IQS62X_SYS_SETTINGS_SOFT_RESET); + if (error) + return error; + usleep_range(10000, 10100); + + device_property_read_string(&client->dev, "linux,fw-file", &fw_file); + + error = request_firmware_nowait(THIS_MODULE, FW_ACTION_HOTPLUG, + fw_file ? : iqs62x->dev_desc->fw_file, + &client->dev, GFP_KERNEL, iqs62x, + iqs62x_fw_load); + if (error) + dev_err(&client->dev, "Failed to request firmware: %d\n", + error); + + return error; +} + +static int iqs62x_remove(struct i2c_client *client) +{ + struct iqs62x_core *iqs62x = i2c_get_clientdata(client); + + wait_for_completion(&iqs62x->fw_done); + + return 0; +} + +static const struct i2c_device_id iqs62x_id[] = { + { "iqs620a", 0 }, + { "iqs621", 1 }, + { "iqs622", 2 }, + { "iqs624", 3 }, + { "iqs625", 4 }, + { } +}; +MODULE_DEVICE_TABLE(i2c, iqs62x_id); + +static const struct of_device_id iqs62x_of_match[] = { + { .compatible = "azoteq,iqs620a" }, + { .compatible = "azoteq,iqs621" }, + { .compatible = "azoteq,iqs622" }, + { .compatible = "azoteq,iqs624" }, + { .compatible = "azoteq,iqs625" }, + { } +}; +MODULE_DEVICE_TABLE(of, iqs62x_of_match); + +static struct i2c_driver iqs62x_i2c_driver = { + .driver = { + .name = "iqs62x", + .of_match_table = iqs62x_of_match, + .pm = &iqs62x_pm, + }, + .id_table = iqs62x_id, + .probe = iqs62x_probe, + .remove = iqs62x_remove, +}; +module_i2c_driver(iqs62x_i2c_driver); + +MODULE_AUTHOR("Jeff LaBundy <jeff@labundy.com>"); +MODULE_DESCRIPTION("Azoteq IQS620A/621/622/624/625 ProxFusion Sensor Family"); +MODULE_LICENSE("GPL"); diff --git a/drivers/mfd/iqs62x-tables.c b/drivers/mfd/iqs62x-tables.c new file mode 100644 index 0000000..12300b7 --- /dev/null +++ b/drivers/mfd/iqs62x-tables.c @@ -0,0 +1,424 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Azoteq IQS620A/621/622/624/625 ProxFusion Sensor Family + * + * Copyright (C) 2019 + * Author: Jeff LaBundy <jeff@labundy.com> + */ + +#include <linux/kernel.h> +#include <linux/mfd/core.h> +#include <linux/mfd/iqs62x.h> + +static const struct mfd_cell iqs620at_sub_devs[] = { + { + .name = IQS62X_DRV_NAME_KEYS, + .of_compatible = "azoteq,iqs620a-keys", + }, + { + .name = IQS620_DRV_NAME_PWM, + .of_compatible = "azoteq,iqs620a-pwm", + }, + { + .name = IQS620_DRV_NAME_TEMP, + }, +}; + +static const struct mfd_cell iqs620a_sub_devs[] = { + { + .name = IQS62X_DRV_NAME_KEYS, + .of_compatible = "azoteq,iqs620a-keys", + }, + { + .name = IQS620_DRV_NAME_PWM, + .of_compatible = "azoteq,iqs620a-pwm", + }, +}; + +static const struct mfd_cell iqs621_sub_devs[] = { + { + .name = IQS62X_DRV_NAME_KEYS, + .of_compatible = "azoteq,iqs621-keys", + }, + { + .name = IQS621_DRV_NAME_ALS, + }, +}; + +static const struct mfd_cell iqs622_sub_devs[] = { + { + .name = IQS62X_DRV_NAME_KEYS, + .of_compatible = "azoteq,iqs622-keys", + }, + { + .name = IQS622_DRV_NAME_PROX, + .of_compatible = "azoteq,iqs622-prox", + }, +}; + +static const struct mfd_cell iqs624_sub_devs[] = { + { + .name = IQS62X_DRV_NAME_KEYS, + .of_compatible = "azoteq,iqs624-keys", + }, + { + .name = IQS624_DRV_NAME_POS, + }, +}; + +static const struct mfd_cell iqs625_sub_devs[] = { + { + .name = IQS62X_DRV_NAME_KEYS, + .of_compatible = "azoteq,iqs625-keys", + }, + { + .name = IQS624_DRV_NAME_POS, + }, +}; + +static const u8 iqs620at_cal_regs[] = { 0xC2, 0xC3, 0xC4, }; +static const u8 iqs621_cal_regs[] = { 0x82, 0x83, }; + +static const enum iqs62x_event_reg iqs620a_event_regs[][IQS62X_EVENT_SIZE] = { + [IQS62X_UI_PROX] = { + IQS62X_EVENT_SYS, /* 0x10 */ + IQS62X_EVENT_GLBL, /* 0x11 */ + IQS62X_EVENT_PROX, /* 0x12 */ + IQS62X_EVENT_HYST, /* 0x13 */ + IQS62X_EVENT_NONE, + IQS62X_EVENT_NONE, + IQS62X_EVENT_HALL, /* 0x16 */ + IQS62X_EVENT_NONE, + IQS62X_EVENT_NONE, + IQS62X_EVENT_TEMP, /* 0x19 */ + IQS62X_EVENT_UI_LO, /* 0x1A */ + IQS62X_EVENT_UI_HI, /* 0x1B */ + }, + [IQS62X_UI_SAR1] = { + IQS62X_EVENT_SYS, /* 0x10 */ + IQS62X_EVENT_GLBL, /* 0x11 */ + IQS62X_EVENT_NONE, + IQS62X_EVENT_HYST, /* 0x13 */ + IQS62X_EVENT_NONE, + IQS62X_EVENT_NONE, + IQS62X_EVENT_HALL, /* 0x16 */ + IQS62X_EVENT_NONE, + IQS62X_EVENT_NONE, + IQS62X_EVENT_TEMP, /* 0x19 */ + IQS62X_EVENT_UI_LO, /* 0x1A */ + IQS62X_EVENT_UI_HI, /* 0x1B */ + }, +}; + +static const enum iqs62x_event_reg iqs621_event_regs[][IQS62X_EVENT_SIZE] = { + [IQS62X_UI_PROX] = { + IQS62X_EVENT_SYS, /* 0x10 */ + IQS62X_EVENT_GLBL, /* 0x11 */ + IQS62X_EVENT_PROX, /* 0x12 */ + IQS62X_EVENT_HYST, /* 0x13 */ + IQS62X_EVENT_NONE, + IQS62X_EVENT_NONE, + IQS62X_EVENT_ALS, /* 0x16 */ + IQS62X_EVENT_UI_LO, /* 0x17 */ + IQS62X_EVENT_UI_HI, /* 0x18 */ + IQS62X_EVENT_HALL, /* 0x19 */ + IQS62X_EVENT_NONE, + IQS62X_EVENT_NONE, + }, +}; + +static const enum iqs62x_event_reg iqs622_event_regs[][IQS62X_EVENT_SIZE] = { + [IQS62X_UI_PROX] = { + IQS62X_EVENT_SYS, /* 0x10 */ + IQS62X_EVENT_GLBL, /* 0x11 */ + IQS62X_EVENT_PROX, /* 0x12 */ + IQS62X_EVENT_NONE, + IQS62X_EVENT_NONE, + IQS62X_EVENT_NONE, + IQS62X_EVENT_IR, /* 0x16 */ + IQS62X_EVENT_UI_LO, /* 0x17 */ + IQS62X_EVENT_UI_HI, /* 0x18 */ + IQS62X_EVENT_HALL, /* 0x19 */ + IQS62X_EVENT_NONE, + IQS62X_EVENT_NONE, + }, + [IQS62X_UI_SAR1] = { + IQS62X_EVENT_SYS, /* 0x10 */ + IQS62X_EVENT_GLBL, /* 0x11 */ + IQS62X_EVENT_NONE, + IQS62X_EVENT_HYST, /* 0x13 */ + IQS62X_EVENT_NONE, + IQS62X_EVENT_NONE, + IQS62X_EVENT_IR, /* 0x16 */ + IQS62X_EVENT_UI_LO, /* 0x17 */ + IQS62X_EVENT_UI_HI, /* 0x18 */ + IQS62X_EVENT_HALL, /* 0x19 */ + IQS62X_EVENT_NONE, + IQS62X_EVENT_NONE, + }, +}; + +static const enum iqs62x_event_reg iqs624_event_regs[][IQS62X_EVENT_SIZE] = { + [IQS62X_UI_PROX] = { + IQS62X_EVENT_SYS, /* 0x10 */ + IQS62X_EVENT_NONE, + IQS62X_EVENT_PROX, /* 0x12 */ + IQS62X_EVENT_NONE, + IQS62X_EVENT_WHEEL, /* 0x14 */ + IQS62X_EVENT_NONE, + IQS62X_EVENT_UI_LO, /* 0x16 */ + IQS62X_EVENT_UI_HI, /* 0x17 */ + IQS62X_EVENT_INTER, /* 0x18 */ + IQS62X_EVENT_NONE, + IQS62X_EVENT_NONE, + IQS62X_EVENT_NONE, + }, +}; + +static const enum iqs62x_event_reg iqs625_event_regs[][IQS62X_EVENT_SIZE] = { + [IQS62X_UI_PROX] = { + IQS62X_EVENT_SYS, /* 0x10 */ + IQS62X_EVENT_PROX, /* 0x11 */ + IQS62X_EVENT_INTER, /* 0x12 */ + IQS62X_EVENT_NONE, + IQS62X_EVENT_NONE, + IQS62X_EVENT_NONE, + IQS62X_EVENT_NONE, + IQS62X_EVENT_NONE, + IQS62X_EVENT_NONE, + IQS62X_EVENT_NONE, + IQS62X_EVENT_NONE, + IQS62X_EVENT_NONE, + }, +}; + +enum { + IQS620AT_DEV, + IQS620A_DEV, + IQS621_DEV, + IQS622_DEV, + IQS624_DEV, + IQS625_DEV, +}; + +const struct iqs62x_dev_desc iqs62x_devs[IQS62X_NUM_DEV] = { + [IQS620AT_DEV] = { + .dev_name = "iqs620at", + .sub_devs = iqs620at_sub_devs, + .num_sub_devs = ARRAY_SIZE(iqs620at_sub_devs), + + .prod_num = IQS620_PROD_NUM, + .sw_num = 0x08, + .cal_regs = iqs620at_cal_regs, + .num_cal_regs = ARRAY_SIZE(iqs620at_cal_regs), + + .prox_mask = BIT(0), + .sar_mask = BIT(1) | BIT(7), + .hall_mask = BIT(2), + .hyst_mask = BIT(3), + .temp_mask = BIT(4), + + .hall_flags = 0x16, + + .fw_file = "iqs620a.bin", + .event_regs = &iqs620a_event_regs[IQS62X_UI_PROX], + }, + [IQS620A_DEV] = { + .dev_name = "iqs620a", + .sub_devs = iqs620a_sub_devs, + .num_sub_devs = ARRAY_SIZE(iqs620a_sub_devs), + + .prod_num = IQS620_PROD_NUM, + .sw_num = 0x08, + + .prox_mask = BIT(0), + .sar_mask = BIT(1) | BIT(7), + .hall_mask = BIT(2), + .hyst_mask = BIT(3), + .temp_mask = BIT(4), + + .hall_flags = 0x16, + + .fw_file = "iqs620a.bin", + .event_regs = &iqs620a_event_regs[IQS62X_UI_PROX], + }, + [IQS621_DEV] = { + .dev_name = "iqs621", + .sub_devs = iqs621_sub_devs, + .num_sub_devs = ARRAY_SIZE(iqs621_sub_devs), + + .prod_num = IQS621_PROD_NUM, + .sw_num = 0x09, + .cal_regs = iqs621_cal_regs, + .num_cal_regs = ARRAY_SIZE(iqs621_cal_regs), + + .prox_mask = BIT(0), + .hall_mask = BIT(1), + .als_mask = BIT(2), + .hyst_mask = BIT(3), + .temp_mask = BIT(4), + + .hall_flags = 0x19, + .hyst_shift = 5, + + .fw_file = "iqs621.bin", + .event_regs = &iqs621_event_regs[IQS62X_UI_PROX], + }, + [IQS622_DEV] = { + .dev_name = "iqs622", + .sub_devs = iqs622_sub_devs, + .num_sub_devs = ARRAY_SIZE(iqs622_sub_devs), + + .prod_num = IQS622_PROD_NUM, + .sw_num = 0x06, + + .prox_mask = BIT(0), + .sar_mask = BIT(1), + .hall_mask = BIT(2), + .als_mask = BIT(3), + .ir_mask = BIT(4), + + .hall_flags = 0x19, + + .fw_file = "iqs622.bin", + .event_regs = &iqs622_event_regs[IQS62X_UI_PROX], + }, + [IQS624_DEV] = { + .dev_name = "iqs624", + .sub_devs = iqs624_sub_devs, + .num_sub_devs = ARRAY_SIZE(iqs624_sub_devs), + + .prod_num = IQS624_PROD_NUM, + .sw_num = 0x0B, + + .interval = 0x18, + .interval_div = 3, + + .fw_file = "iqs624.bin", + .event_regs = &iqs624_event_regs[IQS62X_UI_PROX], + }, + [IQS625_DEV] = { + .dev_name = "iqs625", + .sub_devs = iqs625_sub_devs, + .num_sub_devs = ARRAY_SIZE(iqs625_sub_devs), + + .prod_num = IQS625_PROD_NUM, + .sw_num = 0x0B, + + .interval = 0x12, + .interval_div = 10, + + .fw_file = "iqs625.bin", + .event_regs = &iqs625_event_regs[IQS62X_UI_PROX], + }, +}; +EXPORT_SYMBOL_GPL(iqs62x_devs); + +const struct iqs62x_event_desc iqs62x_events[IQS62X_NUM_EVENTS] = { + [IQS62X_EVENT_PROX_CH0_T] = { + .reg = IQS62X_EVENT_PROX, + .mask = BIT(4), + .val = BIT(4), + }, + [IQS62X_EVENT_PROX_CH0_P] = { + .reg = IQS62X_EVENT_PROX, + .mask = BIT(0), + .val = BIT(0), + }, + [IQS62X_EVENT_PROX_CH1_T] = { + .reg = IQS62X_EVENT_PROX, + .mask = BIT(5), + .val = BIT(5), + }, + [IQS62X_EVENT_PROX_CH1_P] = { + .reg = IQS62X_EVENT_PROX, + .mask = BIT(1), + .val = BIT(1), + }, + [IQS62X_EVENT_PROX_CH2_T] = { + .reg = IQS62X_EVENT_PROX, + .mask = BIT(6), + .val = BIT(6), + }, + [IQS62X_EVENT_PROX_CH2_P] = { + .reg = IQS62X_EVENT_PROX, + .mask = BIT(2), + .val = BIT(2), + }, + [IQS62X_EVENT_HYST_POS_T] = { + .reg = IQS62X_EVENT_HYST, + .mask = BIT(6) | BIT(7), + .val = BIT(6), + }, + [IQS62X_EVENT_HYST_POS_P] = { + .reg = IQS62X_EVENT_HYST, + .mask = BIT(5) | BIT(7), + .val = BIT(5), + }, + [IQS62X_EVENT_HYST_NEG_T] = { + .reg = IQS62X_EVENT_HYST, + .mask = BIT(6) | BIT(7), + .val = BIT(6) | BIT(7), + }, + [IQS62X_EVENT_HYST_NEG_P] = { + .reg = IQS62X_EVENT_HYST, + .mask = BIT(5) | BIT(7), + .val = BIT(5) | BIT(7), + }, + [IQS62X_EVENT_SAR1_ACT] = { + .reg = IQS62X_EVENT_HYST, + .mask = BIT(4), + .val = BIT(4), + }, + [IQS62X_EVENT_SAR1_QRD] = { + .reg = IQS62X_EVENT_HYST, + .mask = BIT(2), + .val = BIT(2), + }, + [IQS62X_EVENT_SAR1_MOVE] = { + .reg = IQS62X_EVENT_HYST, + .mask = BIT(1), + .val = BIT(1), + }, + [IQS62X_EVENT_SAR1_HALT] = { + .reg = IQS62X_EVENT_HYST, + .mask = BIT(0), + .val = BIT(0), + }, + [IQS62X_EVENT_WHEEL_UP] = { + .reg = IQS62X_EVENT_WHEEL, + .mask = BIT(7) | BIT(6), + .val = BIT(7), + }, + [IQS62X_EVENT_WHEEL_DN] = { + .reg = IQS62X_EVENT_WHEEL, + .mask = BIT(7) | BIT(6), + .val = BIT(7) | BIT(6), + }, + [IQS62X_EVENT_HALL_N_T] = { + .reg = IQS62X_EVENT_HALL, + .mask = BIT(2) | BIT(0), + .val = BIT(2), + }, + [IQS62X_EVENT_HALL_N_P] = { + .reg = IQS62X_EVENT_HALL, + .mask = BIT(1) | BIT(0), + .val = BIT(1), + }, + [IQS62X_EVENT_HALL_S_T] = { + .reg = IQS62X_EVENT_HALL, + .mask = BIT(2) | BIT(0), + .val = BIT(2) | BIT(0), + }, + [IQS62X_EVENT_HALL_S_P] = { + .reg = IQS62X_EVENT_HALL, + .mask = BIT(1) | BIT(0), + .val = BIT(1) | BIT(0), + }, + [IQS62X_EVENT_SYS_RESET] = { + .reg = IQS62X_EVENT_SYS, + .mask = BIT(7), + .val = BIT(7), + }, +}; +EXPORT_SYMBOL_GPL(iqs62x_events); diff --git a/include/linux/mfd/iqs62x.h b/include/linux/mfd/iqs62x.h new file mode 100644 index 0000000..01d0f5b --- /dev/null +++ b/include/linux/mfd/iqs62x.h @@ -0,0 +1,148 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Azoteq IQS620A/621/622/624/625 ProxFusion Sensor Family + * + * Copyright (C) 2019 + * Author: Jeff LaBundy <jeff@labundy.com> + */ + +#ifndef __LINUX_MFD_IQS62X_H +#define __LINUX_MFD_IQS62X_H + +#define IQS620_PROD_NUM 0x41 +#define IQS621_PROD_NUM 0x46 +#define IQS622_PROD_NUM 0x42 +#define IQS624_PROD_NUM 0x43 +#define IQS625_PROD_NUM 0x4E + +#define IQS624_HALL_UI 0x70 +#define IQS624_HALL_UI_WHL_EVENT BIT(4) +#define IQS624_HALL_UI_INT_EVENT BIT(3) +#define IQS624_HALL_UI_AUTO_CAL BIT(2) + +#define IQS624_INTERVAL_DIV 0x7D + +#define IQS620_GLBL_EVENT_MASK 0xD7 +#define IQS620_GLBL_EVENT_MASK_PMU BIT(6) + +#define IQS62X_NUM_DEV 6 +#define IQS62X_NUM_KEYS 16 +#define IQS62X_NUM_EVENTS (IQS62X_NUM_KEYS + 5) + +#define IQS62X_EVENT_SIZE 12 + +#define IQS62X_DRV_NAME_KEYS "iqs62x-keys" +#define IQS620_DRV_NAME_TEMP "iqs620at-temp" +#define IQS620_DRV_NAME_PWM "iqs620a-pwm" +#define IQS621_DRV_NAME_ALS "iqs621-als" +#define IQS622_DRV_NAME_PROX "iqs622-prox" +#define IQS624_DRV_NAME_POS "iqs624-pos" + +enum iqs62x_ui_sel { + IQS62X_UI_PROX, + IQS62X_UI_SAR1, +}; + +enum iqs62x_event_reg { + IQS62X_EVENT_NONE, + IQS62X_EVENT_SYS, + IQS62X_EVENT_GLBL, + IQS62X_EVENT_PROX, + IQS62X_EVENT_HYST, + IQS62X_EVENT_HALL, + IQS62X_EVENT_TEMP, + IQS62X_EVENT_ALS, + IQS62X_EVENT_IR, + IQS62X_EVENT_WHEEL, + IQS62X_EVENT_INTER, + IQS62X_EVENT_UI_LO, + IQS62X_EVENT_UI_HI, +}; + +enum iqs62x_event_flag { + /* keys */ + IQS62X_EVENT_PROX_CH0_T, + IQS62X_EVENT_PROX_CH0_P, + IQS62X_EVENT_PROX_CH1_T, + IQS62X_EVENT_PROX_CH1_P, + IQS62X_EVENT_PROX_CH2_T, + IQS62X_EVENT_PROX_CH2_P, + IQS62X_EVENT_HYST_POS_T, + IQS62X_EVENT_HYST_POS_P, + IQS62X_EVENT_HYST_NEG_T, + IQS62X_EVENT_HYST_NEG_P, + IQS62X_EVENT_SAR1_ACT, + IQS62X_EVENT_SAR1_QRD, + IQS62X_EVENT_SAR1_MOVE, + IQS62X_EVENT_SAR1_HALT, + IQS62X_EVENT_WHEEL_UP, + IQS62X_EVENT_WHEEL_DN, + + /* switches */ + IQS62X_EVENT_HALL_N_T, + IQS62X_EVENT_HALL_N_P, + IQS62X_EVENT_HALL_S_T, + IQS62X_EVENT_HALL_S_P, + + /* everything else */ + IQS62X_EVENT_SYS_RESET, +}; + +struct iqs62x_event_data { + u16 ui_data; + union { + u8 temp_flags; + u8 als_flags; + u8 ir_flags; + u8 interval; + }; +}; + +struct iqs62x_event_desc { + enum iqs62x_event_reg reg; + u8 mask; + u8 val; +}; + +struct iqs62x_dev_desc { + const char *dev_name; + const struct mfd_cell *sub_devs; + int num_sub_devs; + + u8 prod_num; + u8 sw_num; + const u8 *cal_regs; + int num_cal_regs; + + u8 prox_mask; + u8 sar_mask; + u8 hall_mask; + u8 hyst_mask; + u8 temp_mask; + u8 als_mask; + u8 ir_mask; + + u8 hall_flags; + u8 hyst_shift; + u8 interval; + u8 interval_div; + + const char *fw_file; + const enum iqs62x_event_reg (*event_regs)[IQS62X_EVENT_SIZE]; +}; + +struct iqs62x_core { + const struct iqs62x_dev_desc *dev_desc; + struct i2c_client *client; + struct regmap *map; + struct blocking_notifier_head nh; + struct list_head fw_blk_head; + struct completion fw_done; + enum iqs62x_ui_sel ui_sel; + u8 sw_num; +}; + +extern const struct iqs62x_dev_desc iqs62x_devs[IQS62X_NUM_DEV]; +extern const struct iqs62x_event_desc iqs62x_events[IQS62X_NUM_EVENTS]; + +#endif /* __LINUX_MFD_IQS62X_H */
This patch adds support for core functions common to all six-channel members of the Azoteq ProxFusion family of sensor devices. Signed-off-by: Jeff LaBundy <jeff@labundy.com> --- drivers/mfd/Kconfig | 13 + drivers/mfd/Makefile | 2 + drivers/mfd/iqs62x-core.c | 638 ++++++++++++++++++++++++++++++++++++++++++++ drivers/mfd/iqs62x-tables.c | 424 +++++++++++++++++++++++++++++ include/linux/mfd/iqs62x.h | 148 ++++++++++ 5 files changed, 1225 insertions(+) create mode 100644 drivers/mfd/iqs62x-core.c create mode 100644 drivers/mfd/iqs62x-tables.c create mode 100644 include/linux/mfd/iqs62x.h