Re: [PATCH] Revert "[Bluetooth] Eliminate checks for impossible conditionsin IRQ handler"

[Robert Hancock]

On 09/13/2009 11:18 PM, Mike Frysinger wrote:
> Commit ac019360fe3 changed the irq handler logic to BUG_ON rather than
> returning IRQ_NONE when the incoming argument is invalid.  While this
> works in most cases, it doesn't work when the IRQ is shared with other
> devices (or when DEBUG_SHIRQ is enabled).

Something doesn't add up here. It shouldn't be possible for the incoming 
argument to be invalid. I'd think that if it is it means that the IRQ 
handler is being registered too soon, before the data structures it 
requires are set up fully. If that's the case, reverting the change just 
partially papers over the bug.

> So revert the previous change and replace the warning message with a
> comment.
>
> Signed-off-by: Michael Hennerich<michael.hennerich-OyLXuOCK7orQT0dZR+AlfA@xxxxxxxxxxxxxxxx>
> Signed-off-by: Mike Frysinger<vapier-aBrp7R+bbdUdnm+yROfE0A@xxxxxxxxxxxxxxxx>
> ---
>   drivers/bluetooth/bluecard_cs.c |    4 +++-
>   drivers/bluetooth/bt3c_cs.c     |    4 +++-
>   drivers/bluetooth/btuart_cs.c   |    4 +++-
>   drivers/bluetooth/dtl1_cs.c     |    4 +++-
>   4 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/bluetooth/bluecard_cs.c b/drivers/bluetooth/bluecard_cs.c
> index b0e569b..a3a8f4d 100644
> --- a/drivers/bluetooth/bluecard_cs.c
> +++ b/drivers/bluetooth/bluecard_cs.c
> @@ -503,7 +503,9 @@ static irqreturn_t bluecard_interrupt(int irq, void *dev_inst)
>   	unsigned int iobase;
>   	unsigned char reg;
>
> -	BUG_ON(!info->hdev);
> +	if (!info || !info->hdev)
> +		/* our irq handler is shared */
> +		return IRQ_NONE;
>
>   	if (!test_bit(CARD_READY,&(info->hw_state)))
>   		return IRQ_HANDLED;
> diff --git a/drivers/bluetooth/bt3c_cs.c b/drivers/bluetooth/bt3c_cs.c
> index d58e22b..9a7578e 100644
> --- a/drivers/bluetooth/bt3c_cs.c
> +++ b/drivers/bluetooth/bt3c_cs.c
> @@ -345,7 +345,9 @@ static irqreturn_t bt3c_interrupt(int irq, void *dev_inst)
>   	int iir;
>   	irqreturn_t r = IRQ_NONE;
>
> -	BUG_ON(!info->hdev);
> +	if (!info || !info->hdev)
> +		/* our irq handler is shared */
> +		return IRQ_NONE;
>
>   	iobase = info->p_dev->io.BasePort1;
>
> diff --git a/drivers/bluetooth/btuart_cs.c b/drivers/bluetooth/btuart_cs.c
> index efd689a..cd9cb6c 100644
> --- a/drivers/bluetooth/btuart_cs.c
> +++ b/drivers/bluetooth/btuart_cs.c
> @@ -295,7 +295,9 @@ static irqreturn_t btuart_interrupt(int irq, void *dev_inst)
>   	int iir, lsr;
>   	irqreturn_t r = IRQ_NONE;
>
> -	BUG_ON(!info->hdev);
> +	if (!info || !info->hdev)
> +		/* our irq handler is shared */
> +		return IRQ_NONE;
>
>   	iobase = info->p_dev->io.BasePort1;
>
> diff --git a/drivers/bluetooth/dtl1_cs.c b/drivers/bluetooth/dtl1_cs.c
> index 2cc7b32..588678a 100644
> --- a/drivers/bluetooth/dtl1_cs.c
> +++ b/drivers/bluetooth/dtl1_cs.c
> @@ -299,7 +299,9 @@ static irqreturn_t dtl1_interrupt(int irq, void *dev_inst)
>   	int iir, lsr;
>   	irqreturn_t r = IRQ_NONE;
>
> -	BUG_ON(!info->hdev);
> +	if (!info || !info->hdev)
> +		/* our irq handler is shared */
> +		return IRQ_NONE;
>
>   	iobase = info->p_dev->io.BasePort1;


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/