On Thu, Aug 10, 2023 at 06:06:48PM +0300, Jarkko Sakkinen wrote: > On Tue Aug 8, 2023 at 6:26 AM EEST, Linus Torvalds wrote: > > 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 > > What if I just revert the commit, apply the correct one, and send a PR? No, stop, please. We have it sorted already. There's a thread with Greg now about the stable backport you can weigh in on, though. Please just read all your emails from the last week, and then it'll be clear what's up. Do that catchup before taking further actions, please. Jason
