| Message ID | 20221122070014.3639277-1-dedekind1@gmail.com (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
| Series | [resend] platform/x86: intel-uncore-freq: add Emerald Rapids support | expand |
Hi, On 11/22/22 08:00, Artem Bityutskiy wrote: > From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com> > > Make Intel uncore frequency driver support Emerald Rapids by adding its CPU > model to the match table. Emerald Rapids uncore frequency control is the same > as in Sapphire Rapids. > > Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com> > Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> > --- > > Re-sending the same patch, but added X86 platform maintainers. There are 3 different issues with this patch, next time please check your patch a bit more thorough before submitting it: 1. This is the first time I see this, or that the platform-driver-x86@vger.kernel.org list sees this. Next time please make sure you address the patch to the right people the first time you send it: [hans@shalem platform-drivers-x86]$ scripts/get_maintainer.pl -f drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> (maintainer:INTEL UNCORE FREQUENCY CONTROL) Hans de Goede <hdegoede@redhat.com> (maintainer:X86 PLATFORM DRIVERS) Mark Gross <markgross@kernel.org> (maintainer:X86 PLATFORM DRIVERS) platform-driver-x86@vger.kernel.org (open list:INTEL UNCORE FREQUENCY CONTROL) linux-kernel@vger.kernel.org (open list) 2. This has checkpatch warnings which are easily fixable: [hans@shalem platform-drivers-x86]$ scripts/checkpatch.pl 0001-platform-x86-intel-uncore-freq-add-Emerald-Rapids-su.patch WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line) #7: model to the match table. Emerald Rapids uncore frequency control is the same total: 0 errors, 1 warnings, 7 lines checked NOTE: For some of the reported defects, checkpatch may be able to mechanically convert to the typical style using --fix or --fix-inplace. 0001-platform-x86-intel-uncore-freq-add-Emerald-Rapids-su.patch has style problems, please review. NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS. 3. This fails to build on top of: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=for-next which is the branch on which this needs to be applied to get upstream, so this is also the branch on which you should base the patch (and build test it) before submitting it upstream: In file included from drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c:21: ./arch/x86/include/asm/cpu_device_id.h:161:46: error: ‘INTEL_FAM6_EMERALDRAPIDS_X’ undeclared here (not in a function); did you mean ‘INTEL_FAM6_SAPPHIRERAPIDS_X’? 161 | X86_MATCH_VENDOR_FAM_MODEL(INTEL, 6, INTEL_FAM6_##model, data) | ^~~~~~~~~~~ ./arch/x86/include/asm/cpu_device_id.h:46:27: note: in definition of macro ‘X86_MATCH_VENDOR_FAM_MODEL_STEPPINGS_FEATURE’ 46 | .model = _model, \ | ^~~~~~ ./arch/x86/include/asm/cpu_device_id.h:129:9: note: in expansion of macro ‘X86_MATCH_VENDOR_FAM_MODEL_FEATURE’ 129 | X86_MATCH_VENDOR_FAM_MODEL_FEATURE(vendor, family, model, \ | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ./arch/x86/include/asm/cpu_device_id.h:161:9: note: in expansion of macro ‘X86_MATCH_VENDOR_FAM_MODEL’ 161 | X86_MATCH_VENDOR_FAM_MODEL(INTEL, 6, INTEL_FAM6_##model, data) | ^~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c:206:9: note: in expansion of macro ‘X86_MATCH_INTEL_FAM6_MODEL’ 206 | X86_MATCH_INTEL_FAM6_MODEL(EMERALDRAPIDS_X, NULL), | ^~~~~~~~~~~~~~~~~~~~~~~~~~ make[6]: *** [scripts/Makefile.build:250: drivers/platform/x86/intel/uncore-frequency/uncore-frequency.o] Error 1 Regards, Hans > > drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c > index 8f9c571d7257..00ac7e381441 100644 > --- a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c > +++ b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c > @@ -203,6 +203,7 @@ static const struct x86_cpu_id intel_uncore_cpu_ids[] = { > X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_X, NULL), > X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_D, NULL), > X86_MATCH_INTEL_FAM6_MODEL(SAPPHIRERAPIDS_X, NULL), > + X86_MATCH_INTEL_FAM6_MODEL(EMERALDRAPIDS_X, NULL), > {} > }; > MODULE_DEVICE_TABLE(x86cpu, intel_uncore_cpu_ids);
Hello Hans, On Tue, 2022-11-22 at 16:30 +0100, Hans de Goede wrote: There are 3 different issues with this patch, next time please check your patch a bit more thorough before submitting it: 1. This is the first time I see this, or that the platform-driver-x86@vger.kernel.org list sees this. Next time please make sure you address the patch to the right people the first time you send it: sure, thanks. 2. This has checkpatch warnings which are easily fixable: [hans@shalem platform-drivers-x86]$ scripts/checkpatch.pl 0001-platform-x86- intel-uncore-freq-add-Emerald-Rapids-su.patch WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line) OK. 3. This fails to build on top of: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=for-next OK, thanks for the pointer. I'd need platfrom-drivers-x86 git tree to include this upstream commit: 7beade0dd41d x86/cpu: Add several Intel server CPU model numbers Would you please consider updating? Thanks!
Hi, On 11/23/22 09:45, Artem Bityutskiy wrote: > Hello Hans, > > On Tue, 2022-11-22 at 16:30 +0100, Hans de Goede wrote: > There are 3 different issues with this patch, next time please > check your patch a bit more thorough before submitting it: > > 1. This is the first time I see this, or that the > platform-driver-x86@vger.kernel.org > list sees this. Next time please make sure you address the patch to the right > people the first time you send it: > > sure, thanks. > > 2. This has checkpatch warnings which are easily fixable: > > [hans@shalem platform-drivers-x86]$ scripts/checkpatch.pl 0001-platform-x86- > intel-uncore-freq-add-Emerald-Rapids-su.patch > WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per > line) > > OK. > > 3. This fails to build on top of: > https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=for-next > > OK, thanks for the pointer. I'd need platfrom-drivers-x86 git tree to include > this upstream commit: > > 7beade0dd41d x86/cpu: Add several Intel server CPU model numbers > > Would you please consider updating? Ugh, no, *NO*! I really expect Intel to do better here! As I repeated explained with the "platform/x86/intel: pmc/core: Add Raptor Lake support to pmc core driver" patch I cannot just go and cherry-pick random patches merged through other trees because that may cause conflicts and will cause the merge to look really funky. There are proper ways to do this and this is not it! This is something which Intel really *must* do correctly next time because having this discussion over and over again is becoming very tiresome! So the proper way to do starts with realizing *beforehand* that things will not build on top of pdx86/for-next. By like actually doing a build-test based on top of pdx86/for-next instead of this nonsense of repeatedly sending me broken patches. Once you realize this there are a couple of options, these all start with identifying which tree has the patch on which the other patch depends iow through which tree / subsystem was "7beade0dd41d x86/cpu: Add several Intel server CPU model numbers" merged? Based on the name I'm going to assume this is done through the x86 tree. Once you have: 1. Realized the patch actually won't build on top of pdx86/for-next 2. Identified which tree has the commit your patch depends on Then there are several options how to proceed: 1. Since this is a one-liner and it touches a driver which has seen no other recent changes, you can submit the patch to the x86/tip tree maintainers instead of to the pdx86 subsystem. The x86/tip tree maintainers will likely want my ack for merging this through their tree. For the next version which you should submit to the x86/tip tree folks, not to the pdx86 list! Feel free to add my: Acked-by: Hans de Goede <hdegoede@redhat.com> and you will want to add a cover-letter explaining why this is being submitted to the x86/tip tree and my ack is there to indicate that I believe it is ok for the patch to go through another tree. What you would normally do is realize beforehand you want to go this route and directly submit to the x86/tip maintainers with me in the Cc asking for my ack for merging through another tree. Or what you could have done is: 2. Ask the x86 maintainers to create an immutable branch based on the last rc1 + just the patch adding the defines (which means realizing before hand you will need this patch in other subsystems and ask them to do this when *submitting* the patch. Then I could have merged the immutable-branch and then cleanly apply your patch on top. ### What you can *NOT* do is just submit the patch to me and expect me to somehow magically fix the cross subsystem dependency problems for you. As the patch submitter any cross subsystem dependency problems arr *your problem* not mine. Regards, Hans
On Wed, Nov 23, 2022 at 3:37 PM Hans de Goede <hdegoede@redhat.com> wrote: > > Hi, > > On 11/23/22 09:45, Artem Bityutskiy wrote: > > Hello Hans, > > > > On Tue, 2022-11-22 at 16:30 +0100, Hans de Goede wrote: > > There are 3 different issues with this patch, next time please > > check your patch a bit more thorough before submitting it: > > > > 1. This is the first time I see this, or that the > > platform-driver-x86@vger.kernel.org > > list sees this. Next time please make sure you address the patch to the right > > people the first time you send it: > > > > sure, thanks. > > > > 2. This has checkpatch warnings which are easily fixable: > > > > [hans@shalem platform-drivers-x86]$ scripts/checkpatch.pl 0001-platform-x86- > > intel-uncore-freq-add-Emerald-Rapids-su.patch > > WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per > > line) > > > > OK. > > > > 3. This fails to build on top of: > > https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=for-next > > > > OK, thanks for the pointer. I'd need platfrom-drivers-x86 git tree to include > > this upstream commit: > > > > 7beade0dd41d x86/cpu: Add several Intel server CPU model numbers > > > > Would you please consider updating? > > Ugh, no, *NO*! I really expect Intel to do better here! > > As I repeated explained with the > > "platform/x86/intel: pmc/core: Add Raptor Lake support to pmc core driver" > > patch I cannot just go and cherry-pick random patches merged through other trees > because that may cause conflicts and will cause the merge to look really > funky. I don't think this is about requesting a cherry-pick though. > There are proper ways to do this and this is not it! > > This is something which Intel really *must* do correctly next time because > having this discussion over and over again is becoming very tiresome! > > So the proper way to do starts with realizing *beforehand* that things > will not build on top of pdx86/for-next. By like actually doing > a build-test based on top of pdx86/for-next instead of this nonsense of > repeatedly sending me broken patches. This patch is based on the mainline. The requisite commit has been included into the Linus' tree since at least 6.1-rc4 AFAICS and I suppose that it has been tested on top of that. You could in principle create a temporary branch based on 6.1-rc4 (or a later -rc), apply the patch on top of it, merge your current branch on top of that and merge it back into your current branch (that should result in a fast-forward merge, so the temporary branch can be deleted after it). I do such things on a regular basis and no complaints so far. Cheers!
On Wed, Nov 23, 2022 at 3:59 PM Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Wed, Nov 23, 2022 at 3:37 PM Hans de Goede <hdegoede@redhat.com> wrote: > > > > Hi, > > > > On 11/23/22 09:45, Artem Bityutskiy wrote: > > > Hello Hans, > > > > > > On Tue, 2022-11-22 at 16:30 +0100, Hans de Goede wrote: > > > There are 3 different issues with this patch, next time please > > > check your patch a bit more thorough before submitting it: > > > > > > 1. This is the first time I see this, or that the > > > platform-driver-x86@vger.kernel.org > > > list sees this. Next time please make sure you address the patch to the right > > > people the first time you send it: > > > > > > sure, thanks. > > > > > > 2. This has checkpatch warnings which are easily fixable: > > > > > > [hans@shalem platform-drivers-x86]$ scripts/checkpatch.pl 0001-platform-x86- > > > intel-uncore-freq-add-Emerald-Rapids-su.patch > > > WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per > > > line) > > > > > > OK. > > > > > > 3. This fails to build on top of: > > > https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=for-next > > > > > > OK, thanks for the pointer. I'd need platfrom-drivers-x86 git tree to include > > > this upstream commit: > > > > > > 7beade0dd41d x86/cpu: Add several Intel server CPU model numbers > > > > > > Would you please consider updating? > > > > Ugh, no, *NO*! I really expect Intel to do better here! > > > > As I repeated explained with the > > > > "platform/x86/intel: pmc/core: Add Raptor Lake support to pmc core driver" > > > > patch I cannot just go and cherry-pick random patches merged through other trees > > because that may cause conflicts and will cause the merge to look really > > funky. > > I don't think this is about requesting a cherry-pick though. > > > There are proper ways to do this and this is not it! > > > > This is something which Intel really *must* do correctly next time because > > having this discussion over and over again is becoming very tiresome! > > > > So the proper way to do starts with realizing *beforehand* that things > > will not build on top of pdx86/for-next. By like actually doing > > a build-test based on top of pdx86/for-next instead of this nonsense of > > repeatedly sending me broken patches. > > This patch is based on the mainline. The requisite commit has been > included into the Linus' tree since at least 6.1-rc4 AFAICS and I > suppose that it has been tested on top of that. > > You could in principle create a temporary branch based on 6.1-rc4 (or > a later -rc), apply the patch on top of it, merge your current branch > on top of that and merge it back into your current branch (that should > result in a fast-forward merge, so the temporary branch can be deleted > after it). > > I do such things on a regular basis and no complaints so far. Alternatively, if you'd rather not do that, I can merge the Artem's patch through the PM tree (it is PM-related after all). I suppose that your ACK would be applicable for that too?
Hi Rafael, On 11/23/22 15:59, Rafael J. Wysocki wrote: > On Wed, Nov 23, 2022 at 3:37 PM Hans de Goede <hdegoede@redhat.com> wrote: >> >> Hi, >> >> On 11/23/22 09:45, Artem Bityutskiy wrote: >>> Hello Hans, >>> >>> On Tue, 2022-11-22 at 16:30 +0100, Hans de Goede wrote: >>> There are 3 different issues with this patch, next time please >>> check your patch a bit more thorough before submitting it: >>> >>> 1. This is the first time I see this, or that the >>> platform-driver-x86@vger.kernel.org >>> list sees this. Next time please make sure you address the patch to the right >>> people the first time you send it: >>> >>> sure, thanks. >>> >>> 2. This has checkpatch warnings which are easily fixable: >>> >>> [hans@shalem platform-drivers-x86]$ scripts/checkpatch.pl 0001-platform-x86- >>> intel-uncore-freq-add-Emerald-Rapids-su.patch >>> WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per >>> line) >>> >>> OK. >>> >>> 3. This fails to build on top of: >>> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=for-next >>> >>> OK, thanks for the pointer. I'd need platfrom-drivers-x86 git tree to include >>> this upstream commit: >>> >>> 7beade0dd41d x86/cpu: Add several Intel server CPU model numbers >>> >>> Would you please consider updating? >> >> Ugh, no, *NO*! I really expect Intel to do better here! >> >> As I repeated explained with the >> >> "platform/x86/intel: pmc/core: Add Raptor Lake support to pmc core driver" >> >> patch I cannot just go and cherry-pick random patches merged through other trees >> because that may cause conflicts and will cause the merge to look really >> funky. > > I don't think this is about requesting a cherry-pick though. > >> There are proper ways to do this and this is not it! >> >> This is something which Intel really *must* do correctly next time because >> having this discussion over and over again is becoming very tiresome! >> >> So the proper way to do starts with realizing *beforehand* that things >> will not build on top of pdx86/for-next. By like actually doing >> a build-test based on top of pdx86/for-next instead of this nonsense of >> repeatedly sending me broken patches. > > This patch is based on the mainline. The requisite commit has been > included into the Linus' tree since at least 6.1-rc4 AFAICS and I > suppose that it has been tested on top of that. Ah, I did not know that; and that is typically info which I would have expected to be explicitly mentioned in the non-existing cover-letter for this patch. > > You could in principle create a temporary branch based on 6.1-rc4 (or > a later -rc), apply the patch on top of it, merge your current branch > on top of that and merge it back into your current branch (that should > result in a fast-forward merge, so the temporary branch can be deleted > after it). Yes I could merge rc4 into my for-next, but I'm not really a big fan of back-merges like this. I try to keep my for-next history linear based on the last rc1, because I find seeing what is going on a lot easier that way. But if this happens more often I guess I may need to get used to doing back-merges more often then just after rc1 is out. What I don't understand is why this patch was not send as a part of the series starting which also had the "7beade0dd41d x86/cpu: Add several Intel server CPU model numbers" patch. That patch just adds a couple #define-s presumably there were more patches in that series actually using those defines. Things would have been cleaner / easier if this patch had simply been a part of that series and if it was merged in one go with that series... Btw this new CPU ID is also missing from: drivers/platform/x86/intel/pmc/core.c drivers/platform/x86/intel/ifs/core.c At least I assume it will need to be added there too, although I guess it might not be as simple as only adding the CPU-id match there ? > Alternatively, if you'd rather not do that, I can merge the Artem's > patch through the PM tree (it is PM-related after all). If you can do that, that would be great, thank you. > I suppose that your ACK would be applicable for that too? Yes. Regards, Hans
On Wed, Nov 23, 2022 at 4:55 PM Hans de Goede <hdegoede@redhat.com> wrote: > > Hi Rafael, > > On 11/23/22 15:59, Rafael J. Wysocki wrote: > > On Wed, Nov 23, 2022 at 3:37 PM Hans de Goede <hdegoede@redhat.com> wrote: > >> > >> Hi, > >> > >> On 11/23/22 09:45, Artem Bityutskiy wrote: > >>> Hello Hans, > >>> > >>> On Tue, 2022-11-22 at 16:30 +0100, Hans de Goede wrote: > >>> There are 3 different issues with this patch, next time please > >>> check your patch a bit more thorough before submitting it: > >>> > >>> 1. This is the first time I see this, or that the > >>> platform-driver-x86@vger.kernel.org > >>> list sees this. Next time please make sure you address the patch to the right > >>> people the first time you send it: > >>> > >>> sure, thanks. > >>> > >>> 2. This has checkpatch warnings which are easily fixable: > >>> > >>> [hans@shalem platform-drivers-x86]$ scripts/checkpatch.pl 0001-platform-x86- > >>> intel-uncore-freq-add-Emerald-Rapids-su.patch > >>> WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per > >>> line) > >>> > >>> OK. > >>> > >>> 3. This fails to build on top of: > >>> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=for-next > >>> > >>> OK, thanks for the pointer. I'd need platfrom-drivers-x86 git tree to include > >>> this upstream commit: > >>> > >>> 7beade0dd41d x86/cpu: Add several Intel server CPU model numbers > >>> > >>> Would you please consider updating? > >> > >> Ugh, no, *NO*! I really expect Intel to do better here! > >> > >> As I repeated explained with the > >> > >> "platform/x86/intel: pmc/core: Add Raptor Lake support to pmc core driver" > >> > >> patch I cannot just go and cherry-pick random patches merged through other trees > >> because that may cause conflicts and will cause the merge to look really > >> funky. > > > > I don't think this is about requesting a cherry-pick though. > > > >> There are proper ways to do this and this is not it! > >> > >> This is something which Intel really *must* do correctly next time because > >> having this discussion over and over again is becoming very tiresome! > >> > >> So the proper way to do starts with realizing *beforehand* that things > >> will not build on top of pdx86/for-next. By like actually doing > >> a build-test based on top of pdx86/for-next instead of this nonsense of > >> repeatedly sending me broken patches. > > > > This patch is based on the mainline. The requisite commit has been > > included into the Linus' tree since at least 6.1-rc4 AFAICS and I > > suppose that it has been tested on top of that. > > Ah, I did not know that; and that is typically info which I would > have expected to be explicitly mentioned in the non-existing cover-letter > for this patch. > > > > > You could in principle create a temporary branch based on 6.1-rc4 (or > > a later -rc), apply the patch on top of it, merge your current branch > > on top of that and merge it back into your current branch (that should > > result in a fast-forward merge, so the temporary branch can be deleted > > after it). > > Yes I could merge rc4 into my for-next, but I'm not really a big fan > of back-merges like this. I try to keep my for-next history linear > based on the last rc1, because I find seeing what is going on > a lot easier that way. But if this happens more often I guess > I may need to get used to doing back-merges more often then > just after rc1 is out. > > What I don't understand is why this patch was not send as a part of > the series starting which also had the > "7beade0dd41d x86/cpu: Add several Intel server CPU model numbers" > patch. That patch just adds a couple #define-s presumably there > were more patches in that series actually using those defines. > > Things would have been cleaner / easier if this patch had simply > been a part of that series and if it was merged in one go with > that series... > > Btw this new CPU ID is also missing from: > drivers/platform/x86/intel/pmc/core.c > drivers/platform/x86/intel/ifs/core.c > > At least I assume it will need to be added there too, although > I guess it might not be as simple as only adding the CPU-id > match there ? > > > Alternatively, if you'd rather not do that, I can merge the Artem's > > patch through the PM tree (it is PM-related after all). > > If you can do that, that would be great, thank you. No problem. > > I suppose that your ACK would be applicable for that too? > > Yes. Thanks!
Hi Hans, > > > [...] > > > Ugh, no, *NO*! I really expect Intel to do better here! > > > Sorry, I didn't realize the CPUID is not added to rc1. Our internal tree constantly gets rebased. So difficult to catch. As you rule, I will communicate internally that apply on top of https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=for-next If doesn't build atleast add that to the patch notes. BTW, I send my PULL from this tree and branch always. Thanks, Srinivas > > > As I repeated explained with the > > > > > > "platform/x86/intel: pmc/core: Add Raptor Lake support to pmc core > > > driver" > > > > > > patch I cannot just go and cherry-pick random patches merged > > > through other trees > > > because that may cause conflicts and will cause the merge to look > > > really > > > funky. > > > > I don't think this is about requesting a cherry-pick though. > > > > > There are proper ways to do this and this is not it! > > > > > > This is something which Intel really *must* do correctly next time > > > because > > > having this discussion over and over again is becoming very > > > tiresome! > > > > > > So the proper way to do starts with realizing *beforehand* that > > > things > > > will not build on top of pdx86/for-next. By like actually doing > > > a build-test based on top of pdx86/for-next instead of this > > > nonsense of > > > repeatedly sending me broken patches. > > > > This patch is based on the mainline. The requisite commit has been > > included into the Linus' tree since at least 6.1-rc4 AFAICS and I > > suppose that it has been tested on top of that. > > Ah, I did not know that; and that is typically info which I would > have expected to be explicitly mentioned in the non-existing cover- > letter > for this patch. > > > > > You could in principle create a temporary branch based on 6.1-rc4 (or > > a later -rc), apply the patch on top of it, merge your current branch > > on top of that and merge it back into your current branch (that > > should > > result in a fast-forward merge, so the temporary branch can be > > deleted > > after it). > > Yes I could merge rc4 into my for-next, but I'm not really a big fan > of back-merges like this. I try to keep my for-next history linear > based on the last rc1, because I find seeing what is going on > a lot easier that way. But if this happens more often I guess > I may need to get used to doing back-merges more often then > just after rc1 is out. > > What I don't understand is why this patch was not send as a part of > the series starting which also had the > "7beade0dd41d x86/cpu: Add several Intel server CPU model numbers" > patch. That patch just adds a couple #define-s presumably there > were more patches in that series actually using those defines. > > Things would have been cleaner / easier if this patch had simply > been a part of that series and if it was merged in one go with > that series... > > Btw this new CPU ID is also missing from: > drivers/platform/x86/intel/pmc/core.c > drivers/platform/x86/intel/ifs/core.c > > At least I assume it will need to be added there too, although > I guess it might not be as simple as only adding the CPU-id > match there ? > > > Alternatively, if you'd rather not do that, I can merge the Artem's > > patch through the PM tree (it is PM-related after all). > > If you can do that, that would be great, thank you. > > > I suppose that your ACK would be applicable for that too? > > Yes. > > Regards, > > Hans > >
Hi Srinivas, On 11/23/22 18:25, srinivas pandruvada wrote: > Hi Hans, > >>>> > > [...] > >>>> Ugh, no, *NO*! I really expect Intel to do better here! >>>> > Sorry, I didn't realize the CPUID is not added to rc1. Our internal > tree constantly gets rebased. So difficult to catch. > > As you rule, I will communicate internally that apply on top of > https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=for-next Thank you and no worries. These simple CPUID patches seem to have been causing some (minor) merging issues, as I mentioned there was a similar issue with "platform/x86/intel: pmc/core: Add Raptor Lake support to pmc core driver" last cycle. It would be nice / good if Intel could try to get patches adding new CPU-ids #defines to land in rc1, rather then in a post rc1 pull-req as has now happened the last 2 cycles. I believe that the CPU-ids #defines landing in rc1 (as you thought they did) would make things easier for everyone. Regards, Hans
Hi Hans, On Wed, 2022-11-23 at 15:37 +0100, Hans de Goede wrote: > Ugh, no, *NO*! I really expect Intel to do better here! ... > patch I cannot just go and cherry-pick random patches merged through other trees > because that may cause conflicts and will cause the merge to look really > funky. I did not suggest or imply to cherry-pick. Back when I was a kernel subsystem maintainer, I did merge Linus' master sometimes, when there was a good reason. And this is what I implied by asking if you'd consider updating: the referenced patch is in Linus' tree. Also, just to be clear. I did accept the criticism in your first reply. This e- mail seems to partially repeat the criticism, so let me do better job explicitly addressing it. 1. I apologize for sending this patch against a wrong tree. It was my mistake. This caused confusion. Sorry for this, and I do mean it. I do realize this caused troubles and you wasted your time because of this. 2. I apologize for the commit message with more than 75 characters per line. I acknowledge that I should have followed checkpatch.pl suggestion. Personally, I do not think it is a big deal, but I do understand that it is not that difficult to just follow checkpatch.pl suggestions. I did not participate in kernel community much for the last 7 or so years, and some of my knowledge/skills are rust/out-of-date. I acknowledge that too. But basics like "won't cherry pick random patches" I do understand. > Acked-by: Hans de Goede <hdegoede@redhat.com> Thank you, this is appreciated. Artem.
diff --git a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c index 8f9c571d7257..00ac7e381441 100644 --- a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c +++ b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency.c @@ -203,6 +203,7 @@ static const struct x86_cpu_id intel_uncore_cpu_ids[] = { X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_X, NULL), X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_D, NULL), X86_MATCH_INTEL_FAM6_MODEL(SAPPHIRERAPIDS_X, NULL), + X86_MATCH_INTEL_FAM6_MODEL(EMERALDRAPIDS_X, NULL), {} }; MODULE_DEVICE_TABLE(x86cpu, intel_uncore_cpu_ids);