From: Borislav Petkov <bp@alien8.de> To: Lucas De Marchi <lucas.demarchi@intel.com> Cc: x86@kernel.org, "Dave Hansen" <dave.hansen@linux.intel.com>, "Ingo Molnar" <mingo@redhat.com>, "Thomas Gleixner" <tglx@linutronix.de>, "Bjorn Helgaas" <bhelgaas@google.com>, linux-pci@vger.kernel.org, intel-gfx@lists.freedesktop.org, "Ville Syrjälä" <ville.syrjala@linux.intel.com>, "Matt Roper" <matthew.d.roper@intel.com>, "Rodrigo Vivi" <rodrigo.vivi@intel.com>, stable@vger.kernel.org Subject: Re: [PATCH v5 1/5] x86/quirks: Fix stolen detection with integrated + discrete GPU Date: Tue, 18 Jan 2022 18:26:48 +0100 [thread overview] Message-ID: <Yeb4WKOFNDNbx6tH@zn.tnic> (raw) In-Reply-To: <20220118163656.fzzkwubgoe5gz36k@ldmartin-desk2> On Tue, Jan 18, 2022 at 08:36:56AM -0800, Lucas De Marchi wrote: > I had the impression the subject/title should be imperative, with it > more relaxed in the body. It seems we have one more difference among > subsystems and I will adapt on next submissions to x86. We have written it down properly, in case it explains it better: "The tip tree maintainers set value on following these rules, especially on the request to write changelogs in imperative mood and not impersonating code or the execution of it. This is not just a whim of the maintainers. Changelogs written in abstract words are more precise and tend to be less confusing than those written in the form of novels." from Documentation/process/maintainer-tip.rst > > So I wonder: why can't you simply pass in a static struct chipset * > > pointer into the early_qrk[i].f function and in there you can set > > QFLAG_APPLIED or so, so that you can mark that the quirk is applied by > > using the nice, per-quirk flags someone has already added instead of > > this ugly static variable? > > It seems you prefer v1. See 20211218061313.100571-1-lucas.demarchi@intel.com I do? I don't see there: early_qrk[i].f(&early_qrk[i], num, slot, func) so that the ->f callback can set the flags. Or at least the flags passed in. If it is not clear what I mean, pls say so and I'll try to produce an example diff ontop. > Although in the review Bjorn suggested just splitting the commit, it was > also mentioned that the PCI subsystem has no such logic in its > equivalent pci_do_fixups(): a quirk/fixup needing that should instead > use a static local. Why? There's perfectly nice ->flags there for exactly stuff like that. static vars are ugly and should be avoided if possible. > What is special about patch 3? Nothing special. It is just ugly. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
WARNING: multiple messages have this Message-ID (diff)
From: Borislav Petkov <bp@alien8.de> To: Lucas De Marchi <lucas.demarchi@intel.com> Cc: Dave Hansen <dave.hansen@linux.intel.com>, linux-pci@vger.kernel.org, intel-gfx@lists.freedesktop.org, x86@kernel.org, stable@vger.kernel.org, Ingo Molnar <mingo@redhat.com>, Bjorn Helgaas <bhelgaas@google.com>, Thomas Gleixner <tglx@linutronix.de> Subject: Re: [Intel-gfx] [PATCH v5 1/5] x86/quirks: Fix stolen detection with integrated + discrete GPU Date: Tue, 18 Jan 2022 18:26:48 +0100 [thread overview] Message-ID: <Yeb4WKOFNDNbx6tH@zn.tnic> (raw) In-Reply-To: <20220118163656.fzzkwubgoe5gz36k@ldmartin-desk2> On Tue, Jan 18, 2022 at 08:36:56AM -0800, Lucas De Marchi wrote: > I had the impression the subject/title should be imperative, with it > more relaxed in the body. It seems we have one more difference among > subsystems and I will adapt on next submissions to x86. We have written it down properly, in case it explains it better: "The tip tree maintainers set value on following these rules, especially on the request to write changelogs in imperative mood and not impersonating code or the execution of it. This is not just a whim of the maintainers. Changelogs written in abstract words are more precise and tend to be less confusing than those written in the form of novels." from Documentation/process/maintainer-tip.rst > > So I wonder: why can't you simply pass in a static struct chipset * > > pointer into the early_qrk[i].f function and in there you can set > > QFLAG_APPLIED or so, so that you can mark that the quirk is applied by > > using the nice, per-quirk flags someone has already added instead of > > this ugly static variable? > > It seems you prefer v1. See 20211218061313.100571-1-lucas.demarchi@intel.com I do? I don't see there: early_qrk[i].f(&early_qrk[i], num, slot, func) so that the ->f callback can set the flags. Or at least the flags passed in. If it is not clear what I mean, pls say so and I'll try to produce an example diff ontop. > Although in the review Bjorn suggested just splitting the commit, it was > also mentioned that the PCI subsystem has no such logic in its > equivalent pci_do_fixups(): a quirk/fixup needing that should instead > use a static local. Why? There's perfectly nice ->flags there for exactly stuff like that. static vars are ugly and should be avoided if possible. > What is special about patch 3? Nothing special. It is just ugly. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
next prev parent reply other threads:[~2022-01-18 17:26 UTC|newest] Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-01-14 0:28 [PATCH v5 1/5] x86/quirks: Fix stolen detection with integrated + discrete GPU Lucas De Marchi 2022-01-14 0:28 ` [Intel-gfx] " Lucas De Marchi 2022-01-14 0:28 ` [PATCH v5 2/5] x86/quirks: Stop using QFLAG_APPLY_ONCE in via_bugs() Lucas De Marchi 2022-01-14 0:28 ` [Intel-gfx] " Lucas De Marchi 2022-01-14 0:28 ` [PATCH v5 3/5] x86/quirks: Stop using QFLAG_APPLY_ONCE in nvidia_bugs() Lucas De Marchi 2022-01-14 0:28 ` [Intel-gfx] " Lucas De Marchi 2022-01-14 0:28 ` [PATCH v5 4/5] x86/quirks: Remove unused logic for flags Lucas De Marchi 2022-01-14 0:28 ` [Intel-gfx] " Lucas De Marchi 2022-01-14 0:28 ` [PATCH v5 5/5] x86/quirks: Improve line wrap on quirk conditions Lucas De Marchi 2022-01-14 0:28 ` [Intel-gfx] " Lucas De Marchi 2022-01-14 1:25 ` [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v5,1/5] x86/quirks: Fix stolen detection with integrated + discrete GPU Patchwork 2022-01-14 2:30 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork 2022-01-18 9:40 ` [PATCH v5 1/5] " Borislav Petkov 2022-01-18 9:40 ` [Intel-gfx] " Borislav Petkov 2022-01-18 16:36 ` Lucas De Marchi 2022-01-18 16:36 ` [Intel-gfx] " Lucas De Marchi 2022-01-18 17:26 ` Borislav Petkov [this message] 2022-01-18 17:26 ` Borislav Petkov 2022-01-18 17:58 ` Bjorn Helgaas 2022-01-18 17:58 ` Bjorn Helgaas 2022-01-18 18:37 ` Borislav Petkov 2022-01-18 18:37 ` Borislav Petkov 2022-01-18 20:01 ` Bjorn Helgaas 2022-01-18 20:01 ` Bjorn Helgaas 2022-01-18 20:31 ` Borislav Petkov 2022-01-18 20:31 ` Borislav Petkov 2022-01-19 20:30 ` Lucas De Marchi 2022-01-19 20:30 ` Lucas De Marchi 2022-01-19 20:58 ` Bjorn Helgaas 2022-01-19 20:58 ` Bjorn Helgaas 2022-01-18 19:05 ` Lucas De Marchi 2022-01-18 19:05 ` [Intel-gfx] " Lucas De Marchi 2022-01-18 19:14 ` Borislav Petkov 2022-01-18 19:14 ` [Intel-gfx] " Borislav Petkov 2022-01-18 19:15 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for series starting with [v5,1/5] x86/quirks: Fix stolen detection with integrated + discrete GPU (rev2) Patchwork 2022-01-18 20:16 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v5,1/5] x86/quirks: Fix stolen detection with integrated + discrete GPU (rev3) Patchwork 2022-01-18 20:51 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork 2022-01-18 23:01 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=Yeb4WKOFNDNbx6tH@zn.tnic \ --to=bp@alien8.de \ --cc=bhelgaas@google.com \ --cc=dave.hansen@linux.intel.com \ --cc=intel-gfx@lists.freedesktop.org \ --cc=linux-pci@vger.kernel.org \ --cc=lucas.demarchi@intel.com \ --cc=matthew.d.roper@intel.com \ --cc=mingo@redhat.com \ --cc=rodrigo.vivi@intel.com \ --cc=stable@vger.kernel.org \ --cc=tglx@linutronix.de \ --cc=ville.syrjala@linux.intel.com \ --cc=x86@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.