Messages in this thread | | | Date | Wed, 29 Jun 2022 11:20:47 +0200 | Subject | Re: [PATCH v6 5/9] tpm, tpm_tis: Only handle supported interrupts | From | Lino Sanfilippo <> |
| |
On 28.06.22 01:09, Jarkko Sakkinen wrote: > On Sun, Jun 26, 2022 at 02:18:17PM +0200, Lino Sanfilippo wrote: >> On 26.06.22 at 08:40, Jarkko Sakkinen wrote: >>> >>> I would instead mask out bits and write a helper function >>> taking care of this: >>> >>> static u8 tpm_tis_filter_sts_mask(u8 int_mask, u8 sts_mask) >>> { >>> struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev); >>> >>> if (!(int_mask & TPM_INTF_STS_VALID_INT)) >>> sts_mask &= ~TPM_STS_VALID; >>> >>> if (!(int_mask & TPM_INTF_DATA_AVAIL_INT)) >>> sts_mask &= ~TPM_STS_DATA_AVAIL; >>> >>> if (!(int_mask & TPM_INTF_CMD_READY_INT)) >>> sts_mask &= ~TPM_STS_COMMAND_READY; >>> >>> return sts_mask; >>> } >>> >>> Less operations and imho somewhat cleaner structure. >>> >>> Add suggested-by if you want. >> >> I thought of a helper like this before but then decided to >> not introduce another function to keep the code changes minimal. But yes, >> it is indeed cleaner. I will do the change and resubmit the series. >> >> Thanks for the review! >> >> Regards, >> Lino > > Yeah, please don't add suggested-by, it's such a minor detail > in the overall patch :-)
I already created a separate patch which only contains moving the bit checks into the helper function. For that patch the Suggested-by is fully justified IMHO.
Thanks for taking time to fix these > glitches and also taking all the feedback into account (and > also being patient). >
No problem. Its always good to have some feedback from people that have a deeper insight into the code. Especially when it is as complex as the TPM subsystem and drivers.
Best regards, Lino
| |