Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Sender: bbhatt=codeaurora.org@mg.codeaurora.org
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date:   Mon, 12 Oct 2020 17:03:56 -0700
From:   Bhaumik Bhatt <bbhatt@codeaurora.org>
To:     Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
        mgautam@codeaurora.org
Cc:     linux-arm-msm@vger.kernel.org, hemantk@codeaurora.org,
        jhugo@codeaurora.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 05/10] bus: mhi: core: Disable IRQs when powering down
Organization: Qualcomm Innovation Center, Inc.
Reply-To: bbhatt@codeaurora.org
Mail-Reply-To: bbhatt@codeaurora.org
In-Reply-To: <20201009160216.GF4810@Mani-XPS-13-9360>
References: <1600480955-16827-1-git-send-email-bbhatt@codeaurora.org>
 <1600480955-16827-6-git-send-email-bbhatt@codeaurora.org>
 <20201009160216.GF4810@Mani-XPS-13-9360>
Message-ID: <8b49f421f81ff850e58d14795376337b@codeaurora.org>
User-Agent: Roundcube Webmail/1.3.9
Precedence: bulk

On 2020-10-09 09:02, Manivannan Sadhasivam wrote:
> On Fri, Sep 18, 2020 at 07:02:30PM -0700, Bhaumik Bhatt wrote:
>> While powering down, the device may or may not acknowledge the MHI
>> RESET issued by host for graceful shutdown scenario which can lead
>> to a rogue device sending an interrupt after the clean-up has been
>> done. This can result in a tasklet being scheduled after it has
>> been killed and access already freed memory causing a NULL pointer
>> exception. Avoid this corner case by disabling the interrupts as a
>> part of host clean up.
>> 
>> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
>> ---
>>  drivers/bus/mhi/core/pm.c | 1 +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c
>> index 1862960..3462d82 100644
>> --- a/drivers/bus/mhi/core/pm.c
>> +++ b/drivers/bus/mhi/core/pm.c
>> @@ -517,6 +517,7 @@ static void mhi_pm_disable_transition(struct 
>> mhi_controller *mhi_cntrl,
>>  	for (i = 0; i < mhi_cntrl->total_ev_rings; i++, mhi_event++) {
>>  		if (mhi_event->offload_ev)
>>  			continue;
>> +		disable_irq(mhi_cntrl->irq[mhi_event->irq]);
> 
> No need to disable irq[0]?
> 
> Thanks,
> Mani
> 
>>  		tasklet_kill(&mhi_event->task);
>>  	}
>> 
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
>> Forum,
>> a Linux Foundation Collaborative Project
>> 

This patch would disable the IRQ line and if IRQ lines are shared 
between BHI
and MHI, we would not see handling of BHI related work happen.

Discussed this with Hemant and and as I am dropping the previous patch, 
will update
this one to make it free_irq() instead which removes the IRQ handler and 
does not
disable the interrupt line.

The function mhi_deinit_free_irq() will not be called from 
mhi_power_down() and
instead, only a free for the main IRQ handler will be called.

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum,
a Linux Foundation Collaborative Project