Messages in this thread | | | From | Linus Torvalds <> | Date | Fri, 6 Jan 2023 10:59:50 -0800 | Subject | Re: [PATCH v2] tpm: Allow system suspend to continue when TPM suspend fails |
| |
On Thu, Jan 5, 2023 at 7:02 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > In lieu of actually fixing the underlying bug, just allow system suspend > to continue, so that laptops still go to sleep fine. Later, this can be > reverted when the real bug is fixed.
So the patch looks fine to me, but since there's still the ChromeOS discussion pending I'll wait for that to finish.
Perhaps re-send or at least remind me if/when it does?
Also, a query about the printout:
> + if (rc) > + pr_err("Unable to suspend tpm-%d (error %d), but continuing system suspend\n", > + chip->dev_num, rc);
so I suspect that 99% of the time the dev_num isn't actually all that useful, but what *might* be useful is which tpm driver it is.
Just comparing the error dmesg output you had:
.. tpm tpm0: Error (28) sending savestate before suspend tpm_tis 00:08: PM: __pnp_bus_suspend(): tpm_pm_suspend+0x0/0x80 returns 28 ..
that "tpm tpm0" output is kind of useless compared to the "tpm_tis 00:08" one.
So I think "dev_err(dev, ...)" would be more useful here.
Finally - and maybe I'm just being difficult here, I will note here again that TPM2 devices don't have this issue, because the TPM2 path for suspend doesn't do any of this at all.
It just does
tpm_transmit_cmd(..);
with a TPM2_CC_SHUTDOWN TPM_SU_STATE command, and doesn't even check the return value. In fact, the tpm2 code *used* to have this comment:
/* In places where shutdown command is sent there's no much we can do * except print the error code on a system failure. */ if (rc < 0 && rc != -EPIPE) dev_warn(&chip->dev, "transmit returned %d while stopping the TPM", rc);
but it was summarily removed when doing some re-organization around buffer handling.
So just by looking at what tpm2 does, I'm not 100% convinced that tpm1 should do this dance at all.
But having a dev_err() is probably a good idea at least as a transitional thing.
Linus
| |