Re: [PATCH] i2c-smbus: Don't report duplicate alerts

[Jean Delvare <jdelvare@suse.de>]

Le Thursday 03 March 2016 à 21:57 +0100, Wolfram Sang a écrit :
> On Tue, Jan 19, 2016 at 11:00:48AM -0600, minyard@acm.org wrote:
> > From: Corey Minyard <cminyard@mvista.com>
> > 
> > Getting the same alert twice in a row is legal and normal,
> > especially on a fast device (like running in qemu).  Kind of
> > like interrupts.  So don't report duplicate alerts, and deliver
> > them normally.
> > 
> > Signed-off-by: Corey Minyard <cminyard@mvista.com>
> 
> Looks plausible to me, but I never used SMBALERT myself. Any chance this
> can cause a regression? Jean, what do you think?

I'm afraid I had a good reason to add this check back then. I'll test
with my ADM1032 evaluation board when I get back home (tomorrow at the
earliest.) Maybe my hardware was misbehaving, in which case I agree any
filtering should be done at the device driver level. But I must double
check what the SMBus specification says too.

Either way the patch's subject is misleading. Should be "Don't filter
out duplicate alerts" or something like that.

> > ---
> >  drivers/i2c/i2c-smbus.c | 7 -------
> >  1 file changed, 7 deletions(-)
> > 
> > diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
> > index 94765a8..cecd423 100644
> > --- a/drivers/i2c/i2c-smbus.c
> > +++ b/drivers/i2c/i2c-smbus.c
> > @@ -75,7 +75,6 @@ static void smbus_alert(struct work_struct *work)
> >  {
> >  	struct i2c_smbus_alert *alert;
> >  	struct i2c_client *ara;
> > -	unsigned short prev_addr = 0;	/* Not a valid address */
> >  
> >  	alert = container_of(work, struct i2c_smbus_alert, alert);
> >  	ara = alert->ara;
> > @@ -99,18 +98,12 @@ static void smbus_alert(struct work_struct *work)
> >  		data.flag = status & 1;
> >  		data.addr = status >> 1;
> >  
> > -		if (data.addr == prev_addr) {
> > -			dev_warn(&ara->dev, "Duplicate SMBALERT# from dev "
> > -				"0x%02x, skipping\n", data.addr);
> > -			break;
> > -		}
> >  		dev_dbg(&ara->dev, "SMBALERT# from dev 0x%02x, flag %d\n",
> >  			data.addr, data.flag);
> >  
> >  		/* Notify driver for the device which issued the alert */
> >  		device_for_each_child(&ara->adapter->dev, &data,
> >  				      smbus_do_alert);
> > -		prev_addr = data.addr;
> >  	}
> >  
> >  	/* We handled all alerts; re-enable level-triggered IRQs */


-- 
Jean Delvare
SUSE L3 Support