diff mbox

[v7,03/10] i2c: i2c-smbus: Use threaded irq for smbalert

Message ID 1497535178-12001-4-git-send-email-preid@electromag.com.au (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Phil Reid June 15, 2017, 1:59 p.m. UTC
Prior to this commit the smbalert_irq was handling in the hard irq
context. This change switch to using a thread irq which avoids the need
for the work thread. Using threaded irq also removes the need for the
edge_triggered flag as the enabling / disabling of the hard irq for level
triggered interrupts will be handled by the irq core.

Without this change have an irq connected to something like an i2c gpio
resulted in a null ptr deferences. Specifically handle_nested_irq calls
the threaded irq handler.

There are currently 3 in tree drivers affected by this change.

i2c-parport driver calls i2c_handle_smbus_alert in a hard irq context.
This driver use edge trigger interrupts which skip the enable / disable
calls. But it still need to handle the smbus transaction on a thread. So
the work thread is kept for this driver.

i2c-parport-light & i2c-thunderx-pcidrv provide the irq number in the
setup which will result in the thread irq being used.

i2c-parport-light is edge trigger so the enable / disable call was
skipped as well.

i2c-thunderx-pcidrv is getting the edge / level trigger setting from of
data and was setting the flag as required. However the irq core should
handle this automatically.

Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 drivers/i2c/busses/i2c-parport-light.c   |  1 -
 drivers/i2c/busses/i2c-parport.c         |  1 -
 drivers/i2c/busses/i2c-thunderx-pcidrv.c |  6 -----
 drivers/i2c/i2c-smbus.c                  | 41 +++++++++++++-------------------
 include/linux/i2c-smbus.h                |  1 -
 5 files changed, 17 insertions(+), 33 deletions(-)

Comments

Wolfram Sang June 19, 2017, 3:27 p.m. UTC | #1
On Thu, Jun 15, 2017 at 09:59:31PM +0800, Phil Reid wrote:
> Prior to this commit the smbalert_irq was handling in the hard irq
> context. This change switch to using a thread irq which avoids the need
> for the work thread. Using threaded irq also removes the need for the
> edge_triggered flag as the enabling / disabling of the hard irq for level
> triggered interrupts will be handled by the irq core.
> 
> Without this change have an irq connected to something like an i2c gpio
> resulted in a null ptr deferences. Specifically handle_nested_irq calls
> the threaded irq handler.
> 
> There are currently 3 in tree drivers affected by this change.
> 
> i2c-parport driver calls i2c_handle_smbus_alert in a hard irq context.
> This driver use edge trigger interrupts which skip the enable / disable
> calls. But it still need to handle the smbus transaction on a thread. So
> the work thread is kept for this driver.
> 
> i2c-parport-light & i2c-thunderx-pcidrv provide the irq number in the
> setup which will result in the thread irq being used.
> 
> i2c-parport-light is edge trigger so the enable / disable call was
> skipped as well.
> 
> i2c-thunderx-pcidrv is getting the edge / level trigger setting from of
> data and was setting the flag as required. However the irq core should
> handle this automatically.
> 
> Signed-off-by: Phil Reid <preid@electromag.com.au>

CCing Benjamin for smbus changes (like last time). Please CC him in the
future in case we need another revision of this series.

> ---
>  drivers/i2c/busses/i2c-parport-light.c   |  1 -
>  drivers/i2c/busses/i2c-parport.c         |  1 -
>  drivers/i2c/busses/i2c-thunderx-pcidrv.c |  6 -----
>  drivers/i2c/i2c-smbus.c                  | 41 +++++++++++++-------------------
>  include/linux/i2c-smbus.h                |  1 -
>  5 files changed, 17 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-parport-light.c b/drivers/i2c/busses/i2c-parport-light.c
> index 1bcdd10..e6e22b8 100644
> --- a/drivers/i2c/busses/i2c-parport-light.c
> +++ b/drivers/i2c/busses/i2c-parport-light.c
> @@ -123,7 +123,6 @@ static int parport_getsda(void *data)
>  
>  /* SMBus alert support */
>  static struct i2c_smbus_alert_setup alert_data = {
> -	.alert_edge_triggered	= 1,
>  };
>  static struct i2c_client *ara;
>  static struct lineop parport_ctrl_irq = {
> diff --git a/drivers/i2c/busses/i2c-parport.c b/drivers/i2c/busses/i2c-parport.c
> index a8e54df..319209a 100644
> --- a/drivers/i2c/busses/i2c-parport.c
> +++ b/drivers/i2c/busses/i2c-parport.c
> @@ -237,7 +237,6 @@ static void i2c_parport_attach(struct parport *port)
>  
>  	/* Setup SMBus alert if supported */
>  	if (adapter_parm[type].smbus_alert) {
> -		adapter->alert_data.alert_edge_triggered = 1;
>  		adapter->ara = i2c_setup_smbus_alert(&adapter->adapter,
>  						     &adapter->alert_data);
>  		if (adapter->ara)
> diff --git a/drivers/i2c/busses/i2c-thunderx-pcidrv.c b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
> index 1d4c2be..c27a50f 100644
> --- a/drivers/i2c/busses/i2c-thunderx-pcidrv.c
> +++ b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
> @@ -112,8 +112,6 @@ static void thunder_i2c_clock_disable(struct device *dev, struct clk *clk)
>  static int thunder_i2c_smbus_setup_of(struct octeon_i2c *i2c,
>  				      struct device_node *node)
>  {
> -	u32 type;
> -
>  	if (!node)
>  		return -EINVAL;
>  
> @@ -121,10 +119,6 @@ static int thunder_i2c_smbus_setup_of(struct octeon_i2c *i2c,
>  	if (!i2c->alert_data.irq)
>  		return -EINVAL;
>  
> -	type = irqd_get_trigger_type(irq_get_irq_data(i2c->alert_data.irq));
> -	i2c->alert_data.alert_edge_triggered =
> -		(type & IRQ_TYPE_LEVEL_MASK) ? 1 : 0;
> -
>  	i2c->ara = i2c_setup_smbus_alert(&i2c->adap, &i2c->alert_data);
>  	if (!i2c->ara)
>  		return -ENODEV;
> diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
> index f9271c7..d4af270 100644
> --- a/drivers/i2c/i2c-smbus.c
> +++ b/drivers/i2c/i2c-smbus.c
> @@ -25,8 +25,6 @@
>  #include <linux/workqueue.h>
>  
>  struct i2c_smbus_alert {
> -	unsigned int		alert_edge_triggered:1;
> -	int			irq;
>  	struct work_struct	alert;
>  	struct i2c_client	*ara;		/* Alert response address */
>  };
> @@ -72,13 +70,12 @@ static int smbus_do_alert(struct device *dev, void *addrp)
>   * 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)
> +static irqreturn_t smbus_alert(int irq, void *d)
>  {
> -	struct i2c_smbus_alert *alert;
> +	struct i2c_smbus_alert *alert = d;
>  	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;
>  
>  	for (;;) {
> @@ -115,21 +112,17 @@ static void smbus_alert(struct work_struct *work)
>  		prev_addr = data.addr;
>  	}
>  
> -	/* We handled all alerts; re-enable level-triggered IRQs */
> -	if (!alert->alert_edge_triggered)
> -		enable_irq(alert->irq);
> +	return IRQ_HANDLED;
>  }
>  
> -static irqreturn_t smbalert_irq(int irq, void *d)
> +static void smbalert_work(struct work_struct *work)
>  {
> -	struct i2c_smbus_alert *alert = d;
> +	struct i2c_smbus_alert *alert;
> +
> +	alert = container_of(work, struct i2c_smbus_alert, alert);
>  
> -	/* Disable level-triggered IRQs until we handle them */
> -	if (!alert->alert_edge_triggered)
> -		disable_irq_nosync(irq);
> +	smbus_alert(0, alert);
>  
> -	schedule_work(&alert->alert);
> -	return IRQ_HANDLED;
>  }
>  
>  /* Setup SMBALERT# infrastructure */
> @@ -139,28 +132,28 @@ static int smbalert_probe(struct i2c_client *ara,
>  	struct i2c_smbus_alert_setup *setup = dev_get_platdata(&ara->dev);
>  	struct i2c_smbus_alert *alert;
>  	struct i2c_adapter *adapter = ara->adapter;
> -	int res;
> +	int res, irq;
>  
>  	alert = devm_kzalloc(&ara->dev, sizeof(struct i2c_smbus_alert),
>  			     GFP_KERNEL);
>  	if (!alert)
>  		return -ENOMEM;
>  
> -	alert->alert_edge_triggered = setup->alert_edge_triggered;
> -	alert->irq = setup->irq;
> -	INIT_WORK(&alert->alert, smbus_alert);
> +	irq = setup->irq;
> +	INIT_WORK(&alert->alert, smbalert_work);
>  	alert->ara = ara;
>  
> -	if (setup->irq > 0) {
> -		res = devm_request_irq(&ara->dev, setup->irq, smbalert_irq,
> -				       0, "smbus_alert", alert);
> +	if (irq > 0) {
> +		res = devm_request_threaded_irq(&ara->dev, irq,
> +						NULL, smbus_alert,
> +						IRQF_SHARED | IRQF_ONESHOT,
> +						"smbus_alert", alert);
>  		if (res)
>  			return res;
>  	}
>  
>  	i2c_set_clientdata(ara, alert);
> -	dev_info(&adapter->dev, "supports SMBALERT#, %s trigger\n",
> -		 setup->alert_edge_triggered ? "edge" : "level");
> +	dev_info(&adapter->dev, "supports SMBALERT#\n");
>  
>  	return 0;
>  }
> diff --git a/include/linux/i2c-smbus.h b/include/linux/i2c-smbus.h
> index a138502..19efbd1 100644
> --- a/include/linux/i2c-smbus.h
> +++ b/include/linux/i2c-smbus.h
> @@ -42,7 +42,6 @@
>   * properly set.
>   */
>  struct i2c_smbus_alert_setup {
> -	unsigned int		alert_edge_triggered:1;
>  	int			irq;
>  };
>  
> -- 
> 1.8.3.1
>
Phil Reid June 20, 2017, 2:30 a.m. UTC | #2
On 19/06/2017 23:27, Wolfram Sang wrote:
> On Thu, Jun 15, 2017 at 09:59:31PM +0800, Phil Reid wrote:
>> Prior to this commit the smbalert_irq was handling in the hard irq
>> context. This change switch to using a thread irq which avoids the need
>> for the work thread. Using threaded irq also removes the need for the
>> edge_triggered flag as the enabling / disabling of the hard irq for level
>> triggered interrupts will be handled by the irq core.
>>
>> Without this change have an irq connected to something like an i2c gpio
>> resulted in a null ptr deferences. Specifically handle_nested_irq calls
>> the threaded irq handler.
>>
>> There are currently 3 in tree drivers affected by this change.
>>
>> i2c-parport driver calls i2c_handle_smbus_alert in a hard irq context.
>> This driver use edge trigger interrupts which skip the enable / disable
>> calls. But it still need to handle the smbus transaction on a thread. So
>> the work thread is kept for this driver.
>>
>> i2c-parport-light & i2c-thunderx-pcidrv provide the irq number in the
>> setup which will result in the thread irq being used.
>>
>> i2c-parport-light is edge trigger so the enable / disable call was
>> skipped as well.
>>
>> i2c-thunderx-pcidrv is getting the edge / level trigger setting from of
>> data and was setting the flag as required. However the irq core should
>> handle this automatically.
>>
>> Signed-off-by: Phil Reid <preid@electromag.com.au>
> 
> CCing Benjamin for smbus changes (like last time). Please CC him in the
> future in case we need another revision of this series.
>

Sorry forgot he was added manually.
I just used the get maintainers script to get the list.
And he wasn't listed against the i2c / smbus paths.
Benjamin Tissoires June 23, 2017, 12:11 p.m. UTC | #3
Hi,

On Jun 19 2017 or thereabouts, Wolfram Sang wrote:
> On Thu, Jun 15, 2017 at 09:59:31PM +0800, Phil Reid wrote:
> > Prior to this commit the smbalert_irq was handling in the hard irq
> > context. This change switch to using a thread irq which avoids the need
> > for the work thread. Using threaded irq also removes the need for the
> > edge_triggered flag as the enabling / disabling of the hard irq for level
> > triggered interrupts will be handled by the irq core.
> > 
> > Without this change have an irq connected to something like an i2c gpio
> > resulted in a null ptr deferences. Specifically handle_nested_irq calls
> > the threaded irq handler.
> > 
> > There are currently 3 in tree drivers affected by this change.
> > 
> > i2c-parport driver calls i2c_handle_smbus_alert in a hard irq context.
> > This driver use edge trigger interrupts which skip the enable / disable
> > calls. But it still need to handle the smbus transaction on a thread. So
> > the work thread is kept for this driver.
> > 
> > i2c-parport-light & i2c-thunderx-pcidrv provide the irq number in the
> > setup which will result in the thread irq being used.
> > 
> > i2c-parport-light is edge trigger so the enable / disable call was
> > skipped as well.
> > 
> > i2c-thunderx-pcidrv is getting the edge / level trigger setting from of
> > data and was setting the flag as required. However the irq core should
> > handle this automatically.
> > 
> > Signed-off-by: Phil Reid <preid@electromag.com.au>
> 
> CCing Benjamin for smbus changes (like last time). Please CC him in the
> future in case we need another revision of this series.

Thanks Wolfram.

I wonder if we can not have something similar than what we do for host notify
for removing the worker all together:

In host notify, we request a new IRQ number, and when the Host Notify
function is called, we simply call generic_handle_irq(). Switching
i2c-parport to something similar could help us remove this worker
entirely. But given this change would require to actually have a device
handled by i2c-parport, we might postpone this later.

Other than that, the patch looks good to me. The commit message is a
little bit messy IMO, and it took me a while to understand all the
subtleties (most of the issues raised in the messaged are simpler to
understand when reading the code).

Anyway, not a big deal, this one is:
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Cheers,
Benjamin

> 
> > ---
> >  drivers/i2c/busses/i2c-parport-light.c   |  1 -
> >  drivers/i2c/busses/i2c-parport.c         |  1 -
> >  drivers/i2c/busses/i2c-thunderx-pcidrv.c |  6 -----
> >  drivers/i2c/i2c-smbus.c                  | 41 +++++++++++++-------------------
> >  include/linux/i2c-smbus.h                |  1 -
> >  5 files changed, 17 insertions(+), 33 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-parport-light.c b/drivers/i2c/busses/i2c-parport-light.c
> > index 1bcdd10..e6e22b8 100644
> > --- a/drivers/i2c/busses/i2c-parport-light.c
> > +++ b/drivers/i2c/busses/i2c-parport-light.c
> > @@ -123,7 +123,6 @@ static int parport_getsda(void *data)
> >  
> >  /* SMBus alert support */
> >  static struct i2c_smbus_alert_setup alert_data = {
> > -	.alert_edge_triggered	= 1,
> >  };
> >  static struct i2c_client *ara;
> >  static struct lineop parport_ctrl_irq = {
> > diff --git a/drivers/i2c/busses/i2c-parport.c b/drivers/i2c/busses/i2c-parport.c
> > index a8e54df..319209a 100644
> > --- a/drivers/i2c/busses/i2c-parport.c
> > +++ b/drivers/i2c/busses/i2c-parport.c
> > @@ -237,7 +237,6 @@ static void i2c_parport_attach(struct parport *port)
> >  
> >  	/* Setup SMBus alert if supported */
> >  	if (adapter_parm[type].smbus_alert) {
> > -		adapter->alert_data.alert_edge_triggered = 1;
> >  		adapter->ara = i2c_setup_smbus_alert(&adapter->adapter,
> >  						     &adapter->alert_data);
> >  		if (adapter->ara)
> > diff --git a/drivers/i2c/busses/i2c-thunderx-pcidrv.c b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
> > index 1d4c2be..c27a50f 100644
> > --- a/drivers/i2c/busses/i2c-thunderx-pcidrv.c
> > +++ b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
> > @@ -112,8 +112,6 @@ static void thunder_i2c_clock_disable(struct device *dev, struct clk *clk)
> >  static int thunder_i2c_smbus_setup_of(struct octeon_i2c *i2c,
> >  				      struct device_node *node)
> >  {
> > -	u32 type;
> > -
> >  	if (!node)
> >  		return -EINVAL;
> >  
> > @@ -121,10 +119,6 @@ static int thunder_i2c_smbus_setup_of(struct octeon_i2c *i2c,
> >  	if (!i2c->alert_data.irq)
> >  		return -EINVAL;
> >  
> > -	type = irqd_get_trigger_type(irq_get_irq_data(i2c->alert_data.irq));
> > -	i2c->alert_data.alert_edge_triggered =
> > -		(type & IRQ_TYPE_LEVEL_MASK) ? 1 : 0;
> > -
> >  	i2c->ara = i2c_setup_smbus_alert(&i2c->adap, &i2c->alert_data);
> >  	if (!i2c->ara)
> >  		return -ENODEV;
> > diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
> > index f9271c7..d4af270 100644
> > --- a/drivers/i2c/i2c-smbus.c
> > +++ b/drivers/i2c/i2c-smbus.c
> > @@ -25,8 +25,6 @@
> >  #include <linux/workqueue.h>
> >  
> >  struct i2c_smbus_alert {
> > -	unsigned int		alert_edge_triggered:1;
> > -	int			irq;
> >  	struct work_struct	alert;
> >  	struct i2c_client	*ara;		/* Alert response address */
> >  };
> > @@ -72,13 +70,12 @@ static int smbus_do_alert(struct device *dev, void *addrp)
> >   * 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)
> > +static irqreturn_t smbus_alert(int irq, void *d)
> >  {
> > -	struct i2c_smbus_alert *alert;
> > +	struct i2c_smbus_alert *alert = d;
> >  	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;
> >  
> >  	for (;;) {
> > @@ -115,21 +112,17 @@ static void smbus_alert(struct work_struct *work)
> >  		prev_addr = data.addr;
> >  	}
> >  
> > -	/* We handled all alerts; re-enable level-triggered IRQs */
> > -	if (!alert->alert_edge_triggered)
> > -		enable_irq(alert->irq);
> > +	return IRQ_HANDLED;
> >  }
> >  
> > -static irqreturn_t smbalert_irq(int irq, void *d)
> > +static void smbalert_work(struct work_struct *work)
> >  {
> > -	struct i2c_smbus_alert *alert = d;
> > +	struct i2c_smbus_alert *alert;
> > +
> > +	alert = container_of(work, struct i2c_smbus_alert, alert);
> >  
> > -	/* Disable level-triggered IRQs until we handle them */
> > -	if (!alert->alert_edge_triggered)
> > -		disable_irq_nosync(irq);
> > +	smbus_alert(0, alert);
> >  
> > -	schedule_work(&alert->alert);
> > -	return IRQ_HANDLED;
> >  }
> >  
> >  /* Setup SMBALERT# infrastructure */
> > @@ -139,28 +132,28 @@ static int smbalert_probe(struct i2c_client *ara,
> >  	struct i2c_smbus_alert_setup *setup = dev_get_platdata(&ara->dev);
> >  	struct i2c_smbus_alert *alert;
> >  	struct i2c_adapter *adapter = ara->adapter;
> > -	int res;
> > +	int res, irq;
> >  
> >  	alert = devm_kzalloc(&ara->dev, sizeof(struct i2c_smbus_alert),
> >  			     GFP_KERNEL);
> >  	if (!alert)
> >  		return -ENOMEM;
> >  
> > -	alert->alert_edge_triggered = setup->alert_edge_triggered;
> > -	alert->irq = setup->irq;
> > -	INIT_WORK(&alert->alert, smbus_alert);
> > +	irq = setup->irq;
> > +	INIT_WORK(&alert->alert, smbalert_work);
> >  	alert->ara = ara;
> >  
> > -	if (setup->irq > 0) {
> > -		res = devm_request_irq(&ara->dev, setup->irq, smbalert_irq,
> > -				       0, "smbus_alert", alert);
> > +	if (irq > 0) {
> > +		res = devm_request_threaded_irq(&ara->dev, irq,
> > +						NULL, smbus_alert,
> > +						IRQF_SHARED | IRQF_ONESHOT,
> > +						"smbus_alert", alert);
> >  		if (res)
> >  			return res;
> >  	}
> >  
> >  	i2c_set_clientdata(ara, alert);
> > -	dev_info(&adapter->dev, "supports SMBALERT#, %s trigger\n",
> > -		 setup->alert_edge_triggered ? "edge" : "level");
> > +	dev_info(&adapter->dev, "supports SMBALERT#\n");
> >  
> >  	return 0;
> >  }
> > diff --git a/include/linux/i2c-smbus.h b/include/linux/i2c-smbus.h
> > index a138502..19efbd1 100644
> > --- a/include/linux/i2c-smbus.h
> > +++ b/include/linux/i2c-smbus.h
> > @@ -42,7 +42,6 @@
> >   * properly set.
> >   */
> >  struct i2c_smbus_alert_setup {
> > -	unsigned int		alert_edge_triggered:1;
> >  	int			irq;
> >  };
> >  
> > -- 
> > 1.8.3.1
> >
diff mbox

Patch

diff --git a/drivers/i2c/busses/i2c-parport-light.c b/drivers/i2c/busses/i2c-parport-light.c
index 1bcdd10..e6e22b8 100644
--- a/drivers/i2c/busses/i2c-parport-light.c
+++ b/drivers/i2c/busses/i2c-parport-light.c
@@ -123,7 +123,6 @@  static int parport_getsda(void *data)
 
 /* SMBus alert support */
 static struct i2c_smbus_alert_setup alert_data = {
-	.alert_edge_triggered	= 1,
 };
 static struct i2c_client *ara;
 static struct lineop parport_ctrl_irq = {
diff --git a/drivers/i2c/busses/i2c-parport.c b/drivers/i2c/busses/i2c-parport.c
index a8e54df..319209a 100644
--- a/drivers/i2c/busses/i2c-parport.c
+++ b/drivers/i2c/busses/i2c-parport.c
@@ -237,7 +237,6 @@  static void i2c_parport_attach(struct parport *port)
 
 	/* Setup SMBus alert if supported */
 	if (adapter_parm[type].smbus_alert) {
-		adapter->alert_data.alert_edge_triggered = 1;
 		adapter->ara = i2c_setup_smbus_alert(&adapter->adapter,
 						     &adapter->alert_data);
 		if (adapter->ara)
diff --git a/drivers/i2c/busses/i2c-thunderx-pcidrv.c b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
index 1d4c2be..c27a50f 100644
--- a/drivers/i2c/busses/i2c-thunderx-pcidrv.c
+++ b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
@@ -112,8 +112,6 @@  static void thunder_i2c_clock_disable(struct device *dev, struct clk *clk)
 static int thunder_i2c_smbus_setup_of(struct octeon_i2c *i2c,
 				      struct device_node *node)
 {
-	u32 type;
-
 	if (!node)
 		return -EINVAL;
 
@@ -121,10 +119,6 @@  static int thunder_i2c_smbus_setup_of(struct octeon_i2c *i2c,
 	if (!i2c->alert_data.irq)
 		return -EINVAL;
 
-	type = irqd_get_trigger_type(irq_get_irq_data(i2c->alert_data.irq));
-	i2c->alert_data.alert_edge_triggered =
-		(type & IRQ_TYPE_LEVEL_MASK) ? 1 : 0;
-
 	i2c->ara = i2c_setup_smbus_alert(&i2c->adap, &i2c->alert_data);
 	if (!i2c->ara)
 		return -ENODEV;
diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c
index f9271c7..d4af270 100644
--- a/drivers/i2c/i2c-smbus.c
+++ b/drivers/i2c/i2c-smbus.c
@@ -25,8 +25,6 @@ 
 #include <linux/workqueue.h>
 
 struct i2c_smbus_alert {
-	unsigned int		alert_edge_triggered:1;
-	int			irq;
 	struct work_struct	alert;
 	struct i2c_client	*ara;		/* Alert response address */
 };
@@ -72,13 +70,12 @@  static int smbus_do_alert(struct device *dev, void *addrp)
  * 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)
+static irqreturn_t smbus_alert(int irq, void *d)
 {
-	struct i2c_smbus_alert *alert;
+	struct i2c_smbus_alert *alert = d;
 	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;
 
 	for (;;) {
@@ -115,21 +112,17 @@  static void smbus_alert(struct work_struct *work)
 		prev_addr = data.addr;
 	}
 
-	/* We handled all alerts; re-enable level-triggered IRQs */
-	if (!alert->alert_edge_triggered)
-		enable_irq(alert->irq);
+	return IRQ_HANDLED;
 }
 
-static irqreturn_t smbalert_irq(int irq, void *d)
+static void smbalert_work(struct work_struct *work)
 {
-	struct i2c_smbus_alert *alert = d;
+	struct i2c_smbus_alert *alert;
+
+	alert = container_of(work, struct i2c_smbus_alert, alert);
 
-	/* Disable level-triggered IRQs until we handle them */
-	if (!alert->alert_edge_triggered)
-		disable_irq_nosync(irq);
+	smbus_alert(0, alert);
 
-	schedule_work(&alert->alert);
-	return IRQ_HANDLED;
 }
 
 /* Setup SMBALERT# infrastructure */
@@ -139,28 +132,28 @@  static int smbalert_probe(struct i2c_client *ara,
 	struct i2c_smbus_alert_setup *setup = dev_get_platdata(&ara->dev);
 	struct i2c_smbus_alert *alert;
 	struct i2c_adapter *adapter = ara->adapter;
-	int res;
+	int res, irq;
 
 	alert = devm_kzalloc(&ara->dev, sizeof(struct i2c_smbus_alert),
 			     GFP_KERNEL);
 	if (!alert)
 		return -ENOMEM;
 
-	alert->alert_edge_triggered = setup->alert_edge_triggered;
-	alert->irq = setup->irq;
-	INIT_WORK(&alert->alert, smbus_alert);
+	irq = setup->irq;
+	INIT_WORK(&alert->alert, smbalert_work);
 	alert->ara = ara;
 
-	if (setup->irq > 0) {
-		res = devm_request_irq(&ara->dev, setup->irq, smbalert_irq,
-				       0, "smbus_alert", alert);
+	if (irq > 0) {
+		res = devm_request_threaded_irq(&ara->dev, irq,
+						NULL, smbus_alert,
+						IRQF_SHARED | IRQF_ONESHOT,
+						"smbus_alert", alert);
 		if (res)
 			return res;
 	}
 
 	i2c_set_clientdata(ara, alert);
-	dev_info(&adapter->dev, "supports SMBALERT#, %s trigger\n",
-		 setup->alert_edge_triggered ? "edge" : "level");
+	dev_info(&adapter->dev, "supports SMBALERT#\n");
 
 	return 0;
 }
diff --git a/include/linux/i2c-smbus.h b/include/linux/i2c-smbus.h
index a138502..19efbd1 100644
--- a/include/linux/i2c-smbus.h
+++ b/include/linux/i2c-smbus.h
@@ -42,7 +42,6 @@ 
  * properly set.
  */
 struct i2c_smbus_alert_setup {
-	unsigned int		alert_edge_triggered:1;
 	int			irq;
 };