Re: [PATCH] tpm_tis_core: Set TPM_CHIP_FLAG_IRQ before probing for interrupts

2019-09-07 Thread Jarkko Sakkinen 
On Fri, 2019-09-06 at 14:37 +0200, Jan Lübbe wrote:
> This is due to the SPI accesses performed by tis_int_handler (which
> will sleep). Switching to devm_request_threaded_irq fixes this and
> leads to a successful IRQ probe.

Aah, right through tpm_tis_read32/write32(). This is definitely a new
regression. Thanks for reporting this! This was completely missed when
the support for other than TCG MMIO was implemented for the TIS driver.

This should have a patch of it own with your reported-by unless you
care to send bug fix for it.

> But: It seems that the IRQ is not acked correctly, as the interrupt
> line stays low. I suspect this is because the tpm_chip_stop from
> http://git.infradead.org/users/jjs/linux-tpmdd.git/commitdiff/9b558deab2c5d7dc23d5f7a4064892ede482ad32
> happens before the threaded handler runs. I'm currently unable to
> verify that though, as my build machine's disk just died. :/

I cannot recall all the nyances related to interrupt probing but with a
quick look I wonder why it does not utilize int_queue. Then
tpm_chip_stop() could be done synchronously.

> Regards,
> Jan

/Jarkko

Re: [PATCH] tpm_tis_core: Set TPM_CHIP_FLAG_IRQ before probing for interrupts

2019-09-06 Thread Jan Lübbe 
On Thu, 2019-08-29 at 20:09 -0400, Stefan Berger wrote:
> From: Stefan Berger 
> 
> The tpm_tis_core has to set the TPM_CHIP_FLAG_IRQ before probing for
> interrupts since there is no other place in the code that would set
> it.

Thanks for this patch! I tested it to fix a
  [   13.198129] tpm tpm0: [Firmware Bug]: TPM interrupt not working, polling 
instead
we've been seeing, but received this scheduling error:
  [   13.241831] tpm_tis_spi spi0.0: 2.0 TPM (device-id 0x1B, rev-id 16)
  [   13.249450] BUG: scheduling while atomic: swapper/0/0/0x00010002
  [   13.255537] Modules linked in:
  [   13.258669] Preemption disabled at:
  [   13.258686] [] schedule_preempt_disabled+0x1c/0x20
  [   13.268050] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 
5.1.21-20190905-1-development #1
  [   13.276106] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
  [   13.282788] [] (unwind_backtrace) from [] 
(show_stack+0x10/0x14)
  [   13.290621] [] (show_stack) from [] 
(dump_stack+0x78/0x8c)
  [   13.297994] [] (dump_stack) from [] 
(__schedule_bug+0x88/0xd8)
  [   13.305711] [] (__schedule_bug) from [] 
(__schedule+0x5b0/0x750)
  [   13.313535] [] (__schedule) from [] 
(schedule+0x48/0xa0)
  [   13.320731] [] (schedule) from [] 
(schedule_timeout+0x170/0x2a8)
  [   13.328622] [] (schedule_timeout) from [] 
(wait_for_common+0xa4/0x160)
  [   13.336974] [] (wait_for_common) from [] 
(spi_imx_transfer+0xc4/0x5c4)
  [   13.345385] [] (spi_imx_transfer) from [] 
(spi_bitbang_transfer_one+0x50/0xa0)
  [   13.354496] [] (spi_bitbang_transfer_one) from [] 
(spi_transfer_one_message+0x18c/0x3cc)
  [   13.364474] [] (spi_transfer_one_message) from [] 
(__spi_pump_messages+0x350/0x474)
  [   13.374012] [] (__spi_pump_messages) from [] 
(__spi_sync+0x198/0x1a0)
  [   13.382275] [] (__spi_sync) from [] 
(tpm_tis_spi_transfer+0x124/0x300)
  [   13.390683] [] (tpm_tis_spi_transfer) from [] 
(tpm_tis_spi_read_bytes+0x14/0x1c)
  [   13.399957] [] (tpm_tis_spi_read_bytes) from [] 
(tpm_tis_spi_read32+0x30/0x58)
  [   13.409057] [] (tpm_tis_spi_read32) from [] 
(tis_int_handler+0x40/0x13c)
  [   13.417586] [] (tis_int_handler) from [] 
(__handle_irq_event_percpu+0x50/0x11c)
  [   13.426783] [] (__handle_irq_event_percpu) from [] 
(handle_irq_event_percpu+0x2c/0x7c)
  [   13.436582] [] (handle_irq_event_percpu) from [] 
(handle_irq_event+0x38/0x5c)
  [   13.445603] [] (handle_irq_event) from [] 
(handle_level_irq+0xcc/0x170)
  [   13.454098] [] (handle_level_irq) from [] 
(generic_handle_irq+0x24/0x34)
  [   13.462625] [] (generic_handle_irq) from [] 
(mxc_gpio_irq_handler+0x48/0x164)
  [   13.471643] [] (mxc_gpio_irq_handler) from [] 
(mx3_gpio_irq_handler+0x60/0xac)
  [   13.480748] [] (mx3_gpio_irq_handler) from [] 
(generic_handle_irq+0x24/0x34) 
  [   13.489678] [] (generic_handle_irq) from [] 
(__handle_domain_irq+0x7c/0xec)  
  [   13.498459] [] (__handle_domain_irq) from [] 
(gic_handle_irq+0x4c/0x90)
  [   13.506956] [] (gic_handle_irq) from [] 
(__irq_svc+0x6c/0xa8)
  [   13.514577] Exception stack(0xc0b01f30 to 0xc0b01f78)
  [   13.519708] 1f20:  9e08 
dfebb360 c0118e20
  [   13.528029] 1f40: e000 c0b09cf0 c0b09d30 0001 c0b3ff37 c08d0e78 
0001 c0a38a38
  [   13.536346] 1f60:  c0b01f80 c010988c c0109890 60010013 
  [   13.543047] [] (__irq_svc) from [] 
(arch_cpu_idle+0x38/0x3c)
  [   13.550590] [] (arch_cpu_idle) from [] 
(do_idle+0xe0/0x150)
  [   13.558046] [] (do_idle) from [] 
(cpu_startup_entry+0x18/0x1c)
  [   13.565699] [] (cpu_startup_entry) from [] 
(start_kernel+0x450/0x484)
  [   13.574017] [] (start_kernel) from [<>] (  (null))
  [   13.580513] bad: scheduling from the idle thread!

This is due to the SPI accesses performed by tis_int_handler (which
will sleep). Switching to devm_request_threaded_irq fixes this and
leads to a successful IRQ probe.

But: It seems that the IRQ is not acked correctly, as the interrupt
line stays low. I suspect this is because the tpm_chip_stop from
http://git.infradead.org/users/jjs/linux-tpmdd.git/commitdiff/9b558deab2c5d7dc23d5f7a4064892ede482ad32
happens before the threaded handler runs. I'm currently unable to
verify that though, as my build machine's disk just died. :/

Regards,
Jan

> Cc: linux-sta...@vger.kernel.org
> Fixes: 570a36097f30 ("tpm: drop 'irq' from struct
> tpm_vendor_specific")
> Signed-off-by: Stefan Berger 
> ---
>  drivers/char/tpm/tpm_tis_core.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/char/tpm/tpm_tis_core.c
> b/drivers/char/tpm/tpm_tis_core.c
> index ffa9048d8f6c..270f43acbb77 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -981,6 +981,7 @@ int tpm_tis_core_init(struct device *dev, struct
> tpm_tis_data *priv, int irq,
>   }
>  
>   tpm_chip_start(chip);
> + chip->flags |= TPM_CHIP_FLAG_IRQ;
>   if (irq) {
>   tpm_tis_probe_irq_single(chip

[PATCH] tpm_tis_core: Set TPM_CHIP_FLAG_IRQ before probing for interrupts

2019-08-29 Thread Stefan Berger 
From: Stefan Berger 

The tpm_tis_core has to set the TPM_CHIP_FLAG_IRQ before probing for
interrupts since there is no other place in the code that would set
it.

Cc: linux-sta...@vger.kernel.org
Fixes: 570a36097f30 ("tpm: drop 'irq' from struct tpm_vendor_specific")
Signed-off-by: Stefan Berger 
---
 drivers/char/tpm/tpm_tis_core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index ffa9048d8f6c..270f43acbb77 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -981,6 +981,7 @@ int tpm_tis_core_init(struct device *dev, struct 
tpm_tis_data *priv, int irq,
}
 
tpm_chip_start(chip);
+   chip->flags |= TPM_CHIP_FLAG_IRQ;
if (irq) {
tpm_tis_probe_irq_single(chip, intmask, IRQF_SHARED,
 irq);
-- 
2.14.5