On Mon, 7 Aug 2023 at 17:39, Jason A. Donenfeld <Jason@xxxxxxxxx> wrote:
>
> I'm not sure what's best or what Linus prefers. Linus - Jarkko sent you
> the wrong version patch. Do you want a fixup patch that accounts for the
> difference, and then I'll address the stable@ metadata deficiency
> manually by talking to Greg, or would you rather some merge commit
> magic, or something else?
Either works for me, whatever ends up being easiest.
However, looking at that v3 patch, that "should we enable/disable the
hwrng" is now repeated *three* times, and that first one is
if (!IS_ENABLED(CONFIG_HW_RANDOM_TPM) || tpm_is_firmware_upgrade(chip) ||
- tpm_amd_is_rng_defective(chip))
+ chip->flags & TPM_CHIP_FLAG_HWRNG_DISABLED)
and wants fixing anyway: you want parenthesis around the '&'.
Yes, yes, it works (because bitwise ops have higher precedence than
logical ones), but let's not do that.
But more importantly, can we just have a single helper inline function
for this and *not* repeat the same multi-line expression three times
(just in negated and then 2x non-negated format)?
That test is ugly anyway. Why is "tpm_is_firmware_upgrade()" a wrapper
function around testing "chip->flags", but then right next to it it
tests them explicitly.
So if we have to re-do this all, let's re-do it properly. Ok?
Thinking about it, I do guess that makes it easier to just send an
incremental patch on top.
Linus
