diff mbox series

hid: intel-ish-hid: ipc: handle PIMR before ish_wakeup also clear PISR busy_clear bit

Message ID 1548119186-11503-1-git-send-email-hongyan.song@intel.com (mailing list archive)
State Mainlined
Commit 2edefc056e4f0e6ec9508dd1aca2c18fa320efef
Delegated to: Jiri Kosina
Headers show
Series hid: intel-ish-hid: ipc: handle PIMR before ish_wakeup also clear PISR busy_clear bit | expand

Commit Message

Song, Hongyan Jan. 22, 2019, 1:06 a.m. UTC
From: Song Hongyan <hongyan.song@intel.com>

Host driver should handle interrupt mask register earlier than wake up
ish FW else there will be conditions when FW interrupt comes, host
PIMR register still not set ready, so move the interrupt mask setting
before ish_wakeup.

Clear PISR busy_clear bit in ish_irq_handler. If not clear, there will
be conditions host driver received a busy_clear interrupt (before the
busy_clear mask bit is ready), it will return IRQ_NONE after
check_generated_interrupt, the interrupt will never be cleared,
causing the DEVICE not sending following IRQ.

Since PISR clear should not be called for the CHV device we do this
change. After the change, both ISH2HOST interrupt and busy_clear
interrupt will be considered as interrupt from ISH, busy_clear
interrupt will return IRQ_HANDLED from IPC_IS_BUSY check.

Signed-off-by: Song Hongyan <hongyan.song@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/hid/intel-ish-hid/ipc/ipc.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

srinivas pandruvada Jan. 22, 2019, 5:45 p.m. UTC | #1
On Tue, 2019-01-22 at 09:06 +0800, hongyan.song@intel.com wrote:
> From: Song Hongyan <hongyan.song@intel.com>
> 
> Host driver should handle interrupt mask register earlier than wake
> up
> ish FW else there will be conditions when FW interrupt comes, host
> PIMR register still not set ready, so move the interrupt mask setting
> before ish_wakeup.
> 
> Clear PISR busy_clear bit in ish_irq_handler. If not clear, there
> will
> be conditions host driver received a busy_clear interrupt (before the
> busy_clear mask bit is ready), it will return IRQ_NONE after
> check_generated_interrupt, the interrupt will never be cleared,
> causing the DEVICE not sending following IRQ.
> 
> Since PISR clear should not be called for the CHV device we do this
> change. After the change, both ISH2HOST interrupt and busy_clear
> interrupt will be considered as interrupt from ISH, busy_clear
> interrupt will return IRQ_HANDLED from IPC_IS_BUSY check.
> 
> Signed-off-by: Song Hongyan <hongyan.song@intel.com>
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

We can wait till next merge Window. This is not an urgent fix.

Thanks,
Srinivas


> ---
>  drivers/hid/intel-ish-hid/ipc/ipc.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hid/intel-ish-hid/ipc/ipc.c b/drivers/hid/intel-
> ish-hid/ipc/ipc.c
> index 742191b..45e33c7 100644
> --- a/drivers/hid/intel-ish-hid/ipc/ipc.c
> +++ b/drivers/hid/intel-ish-hid/ipc/ipc.c
> @@ -91,7 +91,10 @@ static bool check_generated_interrupt(struct
> ishtp_device *dev)
>  			IPC_INT_FROM_ISH_TO_HOST_CHV_AB(pisr_val);
>  	} else {
>  		pisr_val = ish_reg_read(dev, IPC_REG_PISR_BXT);
> -		interrupt_generated =
> IPC_INT_FROM_ISH_TO_HOST_BXT(pisr_val);
> +		interrupt_generated = !!pisr_val;
> +		/* only busy-clear bit is RW, others are RO */
> +		if (pisr_val)
> +			ish_reg_write(dev, IPC_REG_PISR_BXT, pisr_val);
>  	}
>  
>  	return interrupt_generated;
> @@ -839,11 +842,11 @@ int ish_hw_start(struct ishtp_device *dev)
>  {
>  	ish_set_host_rdy(dev);
>  
> +	set_host_ready(dev);
> +
>  	/* After that we can enable ISH DMA operation and wakeup ISHFW
> */
>  	ish_wakeup(dev);
>  
> -	set_host_ready(dev);
> -
>  	/* wait for FW-initiated reset flow */
>  	if (!dev->recvd_hw_ready)
>  		wait_event_interruptible_timeout(dev->wait_hw_ready,
Jiri Kosina Jan. 24, 2019, 12:11 p.m. UTC | #2
On Tue, 22 Jan 2019, hongyan.song@intel.com wrote:

> From: Song Hongyan <hongyan.song@intel.com>
> 
> Host driver should handle interrupt mask register earlier than wake up
> ish FW else there will be conditions when FW interrupt comes, host
> PIMR register still not set ready, so move the interrupt mask setting
> before ish_wakeup.
> 
> Clear PISR busy_clear bit in ish_irq_handler. If not clear, there will
> be conditions host driver received a busy_clear interrupt (before the
> busy_clear mask bit is ready), it will return IRQ_NONE after
> check_generated_interrupt, the interrupt will never be cleared,
> causing the DEVICE not sending following IRQ.
> 
> Since PISR clear should not be called for the CHV device we do this
> change. After the change, both ISH2HOST interrupt and busy_clear
> interrupt will be considered as interrupt from ISH, busy_clear
> interrupt will return IRQ_HANDLED from IPC_IS_BUSY check.
> 
> Signed-off-by: Song Hongyan <hongyan.song@intel.com>
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

Applied to for-5.1/ish. Thanks,
diff mbox series

Patch

diff --git a/drivers/hid/intel-ish-hid/ipc/ipc.c b/drivers/hid/intel-ish-hid/ipc/ipc.c
index 742191b..45e33c7 100644
--- a/drivers/hid/intel-ish-hid/ipc/ipc.c
+++ b/drivers/hid/intel-ish-hid/ipc/ipc.c
@@ -91,7 +91,10 @@  static bool check_generated_interrupt(struct ishtp_device *dev)
 			IPC_INT_FROM_ISH_TO_HOST_CHV_AB(pisr_val);
 	} else {
 		pisr_val = ish_reg_read(dev, IPC_REG_PISR_BXT);
-		interrupt_generated = IPC_INT_FROM_ISH_TO_HOST_BXT(pisr_val);
+		interrupt_generated = !!pisr_val;
+		/* only busy-clear bit is RW, others are RO */
+		if (pisr_val)
+			ish_reg_write(dev, IPC_REG_PISR_BXT, pisr_val);
 	}
 
 	return interrupt_generated;
@@ -839,11 +842,11 @@  int ish_hw_start(struct ishtp_device *dev)
 {
 	ish_set_host_rdy(dev);
 
+	set_host_ready(dev);
+
 	/* After that we can enable ISH DMA operation and wakeup ISHFW */
 	ish_wakeup(dev);
 
-	set_host_ready(dev);
-
 	/* wait for FW-initiated reset flow */
 	if (!dev->recvd_hw_ready)
 		wait_event_interruptible_timeout(dev->wait_hw_ready,