diff mbox series

[v1,04/10] bus: mhi: core: Use the IRQF_ONESHOT flag for the BHI interrupt line

Message ID 1600480955-16827-5-git-send-email-bbhatt@codeaurora.org
State New
Headers show
Series Bug fixes and improvements for MHI power operations | expand

Commit Message

Bhaumik Bhatt Sept. 19, 2020, 2:02 a.m. UTC
Remove the IRQF_SHARED flag as it is not needed for the BHI interrupt
and replace it with IRQF_ONESHOT so that host can be sure that the
next BHI hard interrupt is triggered only when the threaded interrupt
handler has returned. This is to avoid any race conditions we may run
into if BHI interrupts fire one after another.

Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
---
 drivers/bus/mhi/core/init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Manivannan Sadhasivam Oct. 9, 2020, 3:57 p.m. UTC | #1
On Fri, Sep 18, 2020 at 07:02:29PM -0700, Bhaumik Bhatt wrote:
> Remove the IRQF_SHARED flag as it is not needed for the BHI interrupt
> and replace it with IRQF_ONESHOT so that host can be sure that the
> next BHI hard interrupt is triggered only when the threaded interrupt
> handler has returned. This is to avoid any race conditions we may run
> into if BHI interrupts fire one after another.
> 
> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
> ---
>  drivers/bus/mhi/core/init.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
> index ca32563..9ae4c19 100644
> --- a/drivers/bus/mhi/core/init.c
> +++ b/drivers/bus/mhi/core/init.c
> @@ -167,7 +167,7 @@ int mhi_init_irq_setup(struct mhi_controller *mhi_cntrl)
>  	/* Setup BHI_INTVEC IRQ */
>  	ret = request_threaded_irq(mhi_cntrl->irq[0], mhi_intvec_handler,
>  				   mhi_intvec_threaded_handler,
> -				   IRQF_SHARED | IRQF_NO_SUSPEND,
> +				   IRQF_ONESHOT | IRQF_NO_SUSPEND,

Jeff, I believe you're the one requested for shared irq during initial push.
Are you okay with this?

Thanks,
Mani

>  				   "bhi", mhi_cntrl);
>  	if (ret)
>  		return ret;
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
Jeffrey Hugo Oct. 9, 2020, 4:04 p.m. UTC | #2
On 10/9/2020 9:57 AM, Manivannan Sadhasivam wrote:
> On Fri, Sep 18, 2020 at 07:02:29PM -0700, Bhaumik Bhatt wrote:
>> Remove the IRQF_SHARED flag as it is not needed for the BHI interrupt
>> and replace it with IRQF_ONESHOT so that host can be sure that the
>> next BHI hard interrupt is triggered only when the threaded interrupt
>> handler has returned. This is to avoid any race conditions we may run
>> into if BHI interrupts fire one after another.
>>
>> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
>> ---
>>   drivers/bus/mhi/core/init.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
>> index ca32563..9ae4c19 100644
>> --- a/drivers/bus/mhi/core/init.c
>> +++ b/drivers/bus/mhi/core/init.c
>> @@ -167,7 +167,7 @@ int mhi_init_irq_setup(struct mhi_controller *mhi_cntrl)
>>   	/* Setup BHI_INTVEC IRQ */
>>   	ret = request_threaded_irq(mhi_cntrl->irq[0], mhi_intvec_handler,
>>   				   mhi_intvec_threaded_handler,
>> -				   IRQF_SHARED | IRQF_NO_SUSPEND,
>> +				   IRQF_ONESHOT | IRQF_NO_SUSPEND,
> 
> Jeff, I believe you're the one requested for shared irq during initial push.
> Are you okay with this?

Nope  :)

AIC100 has a single interrupt for BHI/MHI activity, so this needs to be 
shared.
Bhaumik Bhatt Oct. 12, 2020, 11:52 p.m. UTC | #3
On 2020-10-09 09:04, Jeffrey Hugo wrote:
> On 10/9/2020 9:57 AM, Manivannan Sadhasivam wrote:
>> On Fri, Sep 18, 2020 at 07:02:29PM -0700, Bhaumik Bhatt wrote:
>>> Remove the IRQF_SHARED flag as it is not needed for the BHI interrupt
>>> and replace it with IRQF_ONESHOT so that host can be sure that the
>>> next BHI hard interrupt is triggered only when the threaded interrupt
>>> handler has returned. This is to avoid any race conditions we may run
>>> into if BHI interrupts fire one after another.
>>> 
>>> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
>>> ---
>>>   drivers/bus/mhi/core/init.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/drivers/bus/mhi/core/init.c 
>>> b/drivers/bus/mhi/core/init.c
>>> index ca32563..9ae4c19 100644
>>> --- a/drivers/bus/mhi/core/init.c
>>> +++ b/drivers/bus/mhi/core/init.c
>>> @@ -167,7 +167,7 @@ int mhi_init_irq_setup(struct mhi_controller 
>>> *mhi_cntrl)
>>>   	/* Setup BHI_INTVEC IRQ */
>>>   	ret = request_threaded_irq(mhi_cntrl->irq[0], mhi_intvec_handler,
>>>   				   mhi_intvec_threaded_handler,
>>> -				   IRQF_SHARED | IRQF_NO_SUSPEND,
>>> +				   IRQF_ONESHOT | IRQF_NO_SUSPEND,
>> 
>> Jeff, I believe you're the one requested for shared irq during initial 
>> push.
>> Are you okay with this?
> 
> Nope  :)
> 
> AIC100 has a single interrupt for BHI/MHI activity, so this needs to be 
> shared.

Did not account for this. Will drop this patch.

Thanks,
Bhaumik
diff mbox series

Patch

diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
index ca32563..9ae4c19 100644
--- a/drivers/bus/mhi/core/init.c
+++ b/drivers/bus/mhi/core/init.c
@@ -167,7 +167,7 @@  int mhi_init_irq_setup(struct mhi_controller *mhi_cntrl)
 	/* Setup BHI_INTVEC IRQ */
 	ret = request_threaded_irq(mhi_cntrl->irq[0], mhi_intvec_handler,
 				   mhi_intvec_threaded_handler,
-				   IRQF_SHARED | IRQF_NO_SUSPEND,
+				   IRQF_ONESHOT | IRQF_NO_SUSPEND,
 				   "bhi", mhi_cntrl);
 	if (ret)
 		return ret;