| 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 |
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,
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 --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,