Re: [PATCH 1/5] i2c: Add SMBus alert support

[Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>]

Hi Jonathan,

On Mon, 15 Feb 2010 18:26:20 +0000, Jonathan Cameron wrote:
> I have a couple of parts I can test this on (connected to a pxa271)
> but it may be a little while before I get to it (so don't let me
> hold the patch up!)

That would be great. You can always get the latest version of the patch
either in my i2c tree or in linux-next.

> On tiny point below.  Worth changing if you are doing another roll of the patch
> as at least in my dozy Monday evening state it confused me for a few moments!
> ...
> > --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> > +++ linux-2.6.33-rc7/drivers/i2c/i2c-smbus.c	2010-02-12 16:11:34.000000000 +0100
> ...
> > +/*
> > + * The alert IRQ handler needs to hand work off to a task which can issue
> > + * SMBus calls, because those sleeping calls can't be made in IRQ context.
> > + */
> > +static void smbus_alert(struct work_struct *work)
> > +{
> > +	struct i2c_smbus_alert *data;
> > +	struct i2c_client *ara;
> > +	unsigned short prev_addr = 0;	/* Not a valid address */
> > +
> > +	data = container_of(work, struct i2c_smbus_alert, alert);
> > +	ara = data->ara;
> > +
> > +	for (;;) {
> > +		s32 status;
> > +		struct alert_data data;
> Can we change the name of data here.  From readability point of view it
> would be better not to have this reliance on scope (as data used for 
> struct i2c_smbus_alert *data above. (obviously changing it above would 
> work as well!) 

Oh, yeah, of course. Thanks for pointing this out. I wish we built the
kernel with -Wshadow so that gcc could tell us automatically...

I have changed all instances of struct i2c_smbus_alert * to name
"alert", this solves the problem and should make the code more
consistent and readable.

Thanks!
-- 
Jean Delvare