linux-kernel - Re: [PATCH 5/6] mfd: iqs62x: Do not poll during ATI

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 14 Jan 2021 22:01:34 -0600
From:   Jeff LaBundy <jeff@...undy.com>
To:     Lee Jones <lee.jones@...aro.org>
Cc:     linux-kernel@...r.kernel.org
Subject: Re: [PATCH 5/6] mfd: iqs62x: Do not poll during ATI

Hi Lee,

Thank you for taking a look at the series.

On Thu, Jan 14, 2021 at 10:17:11AM +0000, Lee Jones wrote:
> 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.

Not a problem, v2 on the way. There are a couple nearby instances of
double-spacing within these same structs so I'll nuke those as well.

[...]

Kind regards,
Jeff LaBundy