| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20210114101711.GM3975472@dell>
Date: Thu, 14 Jan 2021 10:17:11 +0000
From: Lee Jones <lee.jones@...aro.org>
To: Jeff LaBundy <jeff@...undy.com>
Cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCH 5/6] mfd: iqs62x: Do not poll during ATI
On Sun, 03 Jan 2021, Jeff LaBundy wrote:
> After loading firmware, the driver triggers ATI (calibration) with
> the newly loaded register configuration in place. Next, the driver
> polls a register field to ensure ATI completed in a timely fashion
> and that the device is ready to sense.
>
> However, communicating with the device over I2C while ATI is under-
This doesn't line-up with all of the others! ;)
> way may induce noise in the device and cause ATI to fail. As such,
> the vendor recommends not to poll the device during ATI.
>
> To solve this problem, let the device naturally signal to the host
> that ATI is complete by way of an interrupt. A completion prevents
> the sub-devices from being registered until this happens.
>
> The former logic that scaled ATI timeout and filter settling delay
> is not carried forward with the new implementation, as it produces
> overly conservative delays at lower clock rates. Instead, a single
> pair of delays that covers all cases is used.
>
> Signed-off-by: Jeff LaBundy <jeff@...undy.com>
> ---
> drivers/mfd/iqs62x.c | 103 ++++++++++++++++++++++++++++++---------------
> include/linux/mfd/iqs62x.h | 6 ++-
> 2 files changed, 73 insertions(+), 36 deletions(-)
[...]
> @@ -567,6 +600,12 @@ static void iqs62x_firmware_load(const struct firmware *fw, void *context)
> goto err_out;
> }
>
> + if (!wait_for_completion_timeout(&iqs62x->ati_done,
> + msecs_to_jiffies(2000))) {
> + dev_err(&client->dev, "Failed to complete ATI\n");
> + goto err_out;
> + }
> +
> ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
> iqs62x->dev_desc->sub_devs,
> iqs62x->dev_desc->num_sub_devs,
> @@ -763,7 +802,6 @@ static const struct iqs62x_dev_desc iqs62x_devs[] = {
> .prox_settings = IQS620_PROX_SETTINGS_4,
> .hall_flags = IQS620_HALL_FLAGS,
>
> - .clk_div = 4,
No unnecessary double-line spacings please.
> .fw_name = "iqs620a.bin",
> .event_regs = &iqs620a_event_regs[IQS62X_UI_PROX],
> },
> @@ -784,7 +822,6 @@ static const struct iqs62x_dev_desc iqs62x_devs[] = {
> .prox_settings = IQS620_PROX_SETTINGS_4,
> .hall_flags = IQS620_HALL_FLAGS,
>
> - .clk_div = 4,
As above.
> .fw_name = "iqs620a.bin",
> .event_regs = &iqs620a_event_regs[IQS62X_UI_PROX],
> },
> @@ -808,7 +845,6 @@ static const struct iqs62x_dev_desc iqs62x_devs[] = {
> .hall_flags = IQS621_HALL_FLAGS,
> .hyst_shift = 5,
>
> - .clk_div = 2,
As above.
> .fw_name = "iqs621.bin",
> .event_regs = &iqs621_event_regs[IQS62X_UI_PROX],
> },
> @@ -830,7 +866,6 @@ static const struct iqs62x_dev_desc iqs62x_devs[] = {
> .als_flags = IQS622_ALS_FLAGS,
> .hall_flags = IQS622_HALL_FLAGS,
>
> - .clk_div = 2,
As above.
> .fw_name = "iqs622.bin",
> .event_regs = &iqs622_event_regs[IQS62X_UI_PROX],
> },
> @@ -845,7 +880,6 @@ static const struct iqs62x_dev_desc iqs62x_devs[] = {
> .interval = IQS624_INTERVAL_NUM,
> .interval_div = 3,
>
> - .clk_div = 2,
As above.
> .fw_name = "iqs624.bin",
> .event_regs = &iqs624_event_regs[IQS62X_UI_PROX],
> },
> @@ -860,7 +894,6 @@ static const struct iqs62x_dev_desc iqs62x_devs[] = {
> .interval = IQS625_INTERVAL_NUM,
> .interval_div = 10,
>
> - .clk_div = 2,
As above.
> .fw_name = "iqs625.bin",
> .event_regs = &iqs625_event_regs[IQS62X_UI_PROX],
> },
> @@ -890,6 +923,8 @@ static int iqs62x_probe(struct i2c_client *client)
>
> BLOCKING_INIT_NOTIFIER_HEAD(&iqs62x->nh);
> INIT_LIST_HEAD(&iqs62x->fw_blk_head);
> +
> + init_completion(&iqs62x->ati_done);
> init_completion(&iqs62x->fw_done);
>
> iqs62x->regmap = devm_regmap_init_i2c(client, &iqs62x_regmap_config);
> diff --git a/include/linux/mfd/iqs62x.h b/include/linux/mfd/iqs62x.h
> index 043d3b6..0b71173 100644
> --- a/include/linux/mfd/iqs62x.h
> +++ b/include/linux/mfd/iqs62x.h
> @@ -28,7 +28,7 @@
> #define IQS620_GLBL_EVENT_MASK_PMU BIT(6)
>
> #define IQS62X_NUM_KEYS 16
> -#define IQS62X_NUM_EVENTS (IQS62X_NUM_KEYS + 5)
> +#define IQS62X_NUM_EVENTS (IQS62X_NUM_KEYS + 6)
>
> #define IQS62X_EVENT_SIZE 10
>
> @@ -78,6 +78,7 @@ enum iqs62x_event_flag {
>
> /* everything else */
> IQS62X_EVENT_SYS_RESET,
> + IQS62X_EVENT_SYS_ATI,
> };
>
> struct iqs62x_event_data {
> @@ -119,7 +120,6 @@ struct iqs62x_dev_desc {
> u8 interval;
> u8 interval_div;
>
> - u8 clk_div;
As above.
> const char *fw_name;
> const enum iqs62x_event_reg (*event_regs)[IQS62X_EVENT_SIZE];
> };
> @@ -130,8 +130,10 @@ struct iqs62x_core {
> struct regmap *regmap;
> struct blocking_notifier_head nh;
> struct list_head fw_blk_head;
> + struct completion ati_done;
> struct completion fw_done;
> enum iqs62x_ui_sel ui_sel;
> + unsigned long event_cache;
> };
>
> extern const struct iqs62x_event_desc iqs62x_events[IQS62X_NUM_EVENTS];
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
Powered by blists - more mailing lists