[PATCH resend] platform/x86: intel-uncore-freq: add Emerald Rapids support

[PATCH resend] platform/x86: intel-uncore-freq: add Emerald Rapids support

[Artem Bityutskiy]

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.

 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);
-- 
2.37.3

Re: [PATCH resend] platform/x86: intel-uncore-freq: add Emerald Rapids support

[Hans de Goede]

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);

Re: [PATCH resend] platform/x86: intel-uncore-freq: add Emerald Rapids support

[Artem Bityutskiy]

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!

Re: [PATCH resend] platform/x86: intel-uncore-freq: add Emerald Rapids support

[Hans de Goede]

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

Re: [PATCH resend] platform/x86: intel-uncore-freq: add Emerald Rapids support

[Rafael J. Wysocki]

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!

Re: [PATCH resend] platform/x86: intel-uncore-freq: add Emerald Rapids support

[Rafael J. Wysocki]

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?

Re: [PATCH resend] platform/x86: intel-uncore-freq: add Emerald Rapids support

[Hans de Goede]

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

Re: [PATCH resend] platform/x86: intel-uncore-freq: add Emerald Rapids support

[Rafael J. Wysocki]

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!

Re: [PATCH resend] platform/x86: intel-uncore-freq: add Emerald Rapids support

[srinivas pandruvada]

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
> 
> 

Re: [PATCH resend] platform/x86: intel-uncore-freq: add Emerald Rapids support

[Hans de Goede]

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

Re: [PATCH resend] platform/x86: intel-uncore-freq: add Emerald Rapids support

[Artem Bityutskiy]

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.