On Wed, Sep 28, 2022 at 05:10:25AM +0000, Joel Stanley wrote:
> On Thu, 8 Sept 2022 at 13:53, Eddie James <eajames@xxxxxxxxxxxxx> wrote:
> >
> >
> > On 9/8/22 00:22, Jarkko Sakkinen wrote:
> > > On Wed, Sep 07, 2022 at 11:43:17AM -0500, Eddie James wrote:
> > >> The check for cancelled request depends on the VID of the chip, but
> > >> some chips share VID which shouldn't share their cancellation
> > >> behavior. This is the case for the Nuvoton NPCT75X, which should use
> > >> the default cancellation check, not the Winbond one.
> > >> To avoid changing the existing behavior, add a new flag to indicate
> > >> that the chip should use the default cancellation check and set it
> > >> for the I2C TPM2 TIS driver.
> > >>
> > >> Signed-off-by: Eddie James <eajames@xxxxxxxxxxxxx>
> > >> ---
> > >> drivers/char/tpm/tpm_tis_core.c | 18 ++++++++++--------
> > >> drivers/char/tpm/tpm_tis_core.h | 1 +
> > >> drivers/char/tpm/tpm_tis_i2c.c | 1 +
> > >> 3 files changed, 12 insertions(+), 8 deletions(-)
> > >>
> > >> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> > >> index 757623bacfd5..175e75337395 100644
> > >> --- a/drivers/char/tpm/tpm_tis_core.c
> > >> +++ b/drivers/char/tpm/tpm_tis_core.c
> > >> @@ -682,15 +682,17 @@ static bool tpm_tis_req_canceled(struct tpm_chip *chip, u8 status)
> > >> {
> > >> struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
> > >>
> > >> - switch (priv->manufacturer_id) {
> > >> - case TPM_VID_WINBOND:
> > >> - return ((status == TPM_STS_VALID) ||
> > >> - (status == (TPM_STS_VALID | TPM_STS_COMMAND_READY)));
> > >> - case TPM_VID_STM:
> > >> - return (status == (TPM_STS_VALID | TPM_STS_COMMAND_READY));
> > >> - default:
> > >> - return (status == TPM_STS_COMMAND_READY);
> > >> + if (!test_bit(TPM_TIS_DEFAULT_CANCELLATION, &priv->flags)) {
> > >> + switch (priv->manufacturer_id) {
> > >> + case TPM_VID_WINBOND:
> > >> + return ((status == TPM_STS_VALID) ||
> > >> + (status == (TPM_STS_VALID | TPM_STS_COMMAND_READY)));
> > >> + case TPM_VID_STM:
> > >> + return (status == (TPM_STS_VALID | TPM_STS_COMMAND_READY));
> > >> + }
> > > Why there is no default: ?
> >
> >
> > Well I didn't want to duplicate the line "status ==
> > TPM_STS_COMMAND_READY" in the default case and for the flagged case. So
> > now the switch just falls through for default. I can add default: break
> > instead
>
> This code was in the original patch series submitted by Nuvoton:
>
> https://lore.kernel.org/r/20211104140211.6258-3-amirmizi6@xxxxxxxxx
>
> Perhaps something like that would be better?
The current patch could have
default:
/* fall-through */
break;
BR, Jarkko
