platform/x86: pmc_atom: Match all Beckhoff Automation baytrail boards with critclk_systems DMI table
PATCH 1/1 platform/x86: pmc_atom: Match all Beckhoff Automation baytrail boards with critclk_systems DMI table
From: Steffen Dirkwinkel <[email protected]>
pmc_plt_clk* clocks are used for ethernet controllers so need to stay turned on. This adds the affected board family to critclk_systems DMI table so the clocks are marked as CLK_CRITICAL and not turned off.
This replaces the previosly listed boards with a match for the whole device family. There are new affected boards that would otherwise need to be listed. There are only few unaffected boards in the family and having the clocks turned on is not an issue on those.
--- drivers/platform/x86/pmc_atom.c | 28 ++-------------------------- 1 file changed, 2 insertions(+), 26 deletions(-)
drivers/platform/x86/pmc_atom.c +2 -26
@@ -393,34 +393,10 @@ static const struct dmi_system_id critclk_systems[] = { }, {/* pmc_plt_clk* - are used for ethernet controllers */ - .ident = "Beckhoff CB3163", + .ident = "Beckhoff Baytrail", .matches = {DMI_MATCH (DMI_SYS_VENDOR ,"Beckhoff Automation" ), - DMI_MATCH(DMI_BOARD_NAME, "CB3163"), - }, - }, - { -/* pmc_plt_clk* - are used for ethernet controllers */ - .ident ="Beckhoff CB4063" , - .matches = { -DMI_MATCH (DMI_SYS_VENDOR ,"Beckhoff Automation" ), - DMI_MATCH(DMI_BOARD_NAME, "CB4063"), - }, - }, - { -/* pmc_plt_clk* - are used for ethernet controllers */ - .ident ="Beckhoff CB6263" , - .matches = { -DMI_MATCH (DMI_SYS_VENDOR ,"Beckhoff Automation" ), - DMI_MATCH(DMI_BOARD_NAME, "CB6263"), - }, - }, - { -/* pmc_plt_clk* - are used for ethernet controllers */ - .ident ="Beckhoff CB6363" , - .matches = { -DMI_MATCH (DMI_SYS_VENDOR ,"Beckhoff Automation" ), - DMI_MATCH(DMI_BOARD_NAME, "CB6363"), + DMI_MATCH(DMI_PRODUCT_FAMILY, "CBxx63"), }, }, {
Signature · 1 line
2.31.1
Quoted 8 lines · Steffen Dirkwinkel
From: Steffen Dirkwinkel <[email protected]>
pmc_plt_clk* clocks are used for ethernet controllers so need to stay turned on. This adds the affected board family to
critclk_systemsDMI table so the clocks are marked asCLK_CRITICALand not turned off.This replaces the previosly listed boards with a match for the whole
"...previously..."
Quoted 2 lines
device family. There are new affected boards that would otherwise need to be listed. There are only few unaffected boards in the family and
"...only a few..."
Quoted 1 line
having the clocks turned on is not an issue on those.
"...not an issue."
Quoted 2 lines
Fixes 648e921888ad ("clk: x86: Stop marking clocks as CLK_IS_CRITICAL")Signed-off-by Steffen Dirkwinkel <[email protected]>
I'm afraid it's a bit too much. Is there any guarantee all the boards based on x86 will be Baytrail only?
Signature · 2 lines
With Best Regards, Andy Shevchenko
Quoted 6 lines · Andy Shevchenko
Quoted 3 lines · Steffen Dirkwinkel
From: Steffen Dirkwinkel <[email protected]>
Quoted 7 lines
Quoted 5 lines
pmc_plt_clk* clocks are used for ethernet controllers so need to stay turned on. This adds the affected board family to
critclk_systemsDMI table so the clocks are marked asCLK_CRITICALand not turned off.This replaces the previosly listed boards with a match for the whole
"...previously..."
thanks
Quoted 5 lines
Quoted 3 lines
device family. There are new affected boards that would otherwise need to be listed. There are only few unaffected boards in the family and
"...only a few..."
will drop the phrase
Quoted 4 lines
Quoted 2 lines
having the clocks turned on is not an issue on those.
"...not an issue."
Not an issue for these industrial PCs as sleep is an unusual use case. Having no ethernet after boot/sleep is worse.
Quoted 7 lines
Quoted 3 lines
Fixes 648e921888ad ("clk: x86: Stop marking clocks as CLK_IS_CRITICAL")Signed-off-by Steffen Dirkwinkel <[email protected]>I'm afraid it's a bit too much. Is there any guarantee all the boards based on x86 will be Baytrail only?
Sorry, I guess I should make this clearer in the message. All boards with "CBxx63" are Baytrail.
Quoted 4 lines
--
With Best Regards,
Andy Shevchenko
Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff Registered office: Verl, Germany | Register court: Guetersloh HRA 7075
Quoted 3 lines · linux-kernel-dev
Quoted 2 lines · Andy Shevchenko
On Mon, Apr 12, 2021 at 12:29 PM Steffen Dirkwinkel <[email protected]> wrote:
...
Quoted 5 lines
Quoted 3 lines
I'm afraid it's a bit too much. Is there any guarantee all the boards based on x86 will be Baytrail only?
Sorry, I guess I should make this clearer in the message. All boards with "CBxx63" are Baytrail.
Exactly! And this supports my idea that this shouldn't be done like in this patch.
Are you guaranteeing that *all x86-based* boards produced by your company will be Baytrail only?
Above tells that the answer is rather "no". So, I think we can't apply this patch in its current form.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Quoted 17 lines · Andy Shevchenko
CAUTION: External Email!!
Quoted 3 lines · linux-kernel-dev
Quoted 2 lines · Andy Shevchenko
On Mon, Apr 12, 2021 at 12:29 PM Steffen Dirkwinkel <[email protected]> wrote:
...
Quoted 5 lines
Quoted 3 lines
I'm afraid it's a bit too much. Is there any guarantee all the boards based on x86 will be Baytrail only?
Sorry, I guess I should make this clearer in the message. All boards with "CBxx63" are Baytrail.
Quoted 6 lines
Exactly! And this supports my idea that this shouldn't be done like in this patch.
Are you guaranteeing that *all x86-based* boards produced by your company will be Baytrail only?
Above tells that the answer is rather "no". So, I think we can't apply this patch in its current form.
All boards with DMI_PRODUCT_FAMILY="CBxx63" are Baytrail boards. We do produce other x86 boards but the family is exclusive to Baytrail.
I might be misunderstanding how the matching works. Does this match anything other than CBxx63?
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "Beckhoff Automation"),DMI_MATCH(DMI_PRODUCT_FAMILY, "CBxx63"), },
I can switch it to DMI_EXACT_MATCH but even substring matching works.
Quoted 5 lines
--
With Best Regards,
Andy Shevchenko
Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff Registered office: Verl, Germany | Register court: Guetersloh HRA 7075
Andy Shevchenko
Reviewed-byQuoted 7 lines · linux-kernel-dev
Quoted 6 lines · Andy Shevchenko
CAUTION: External Email!!
Quoted 3 lines · linux-kernel-dev
Quoted 2 lines · Andy Shevchenko
On Mon, Apr 12, 2021 at 12:29 PM Steffen Dirkwinkel <[email protected]> wrote:
...
Quoted 17 lines
Quoted 6 lines
Quoted 5 lines
Quoted 3 lines
I'm afraid it's a bit too much. Is there any guarantee all the boards based on x86 will be Baytrail only?
Sorry, I guess I should make this clearer in the message. All boards with "CBxx63" are Baytrail.
Quoted 6 lines
Exactly! And this supports my idea that this shouldn't be done like in this patch.
Are you guaranteeing that *all x86-based* boards produced by your company will be Baytrail only?
Above tells that the answer is rather "no". So, I think we can't apply this patch in its current form.All boards with DMI_PRODUCT_FAMILY="CBxx63" are Baytrail boards. We do produce other x86 boards but the family is exclusive to Baytrail.
I might be misunderstanding how the matching works. Does this match anything other than CBxx63?
Quoted 6 lines
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "Beckhoff Automation"),DMI_MATCH(DMI_PRODUCT_FAMILY, "CBxx63"), },I can switch it to
DMI_EXACT_MATCHbut even substring matching works.
Sorry, I missed the second match line. Yes, looks correct to me now.
Signature · 2 lines
With Best Regards, Andy Shevchenko
PATCH v2 1/1 platform/x86: pmc_atom: Match all Beckhoff Automation baytrail boards with critclk_systems DMI table
From: Steffen Dirkwinkel <[email protected]>
pmc_plt_clk* clocks are used for ethernet controllers, so need to stay turned on. This adds the affected board family to critclk_systems DMI table, so the clocks are marked as CLK_CRITICAL and not turned off.
This replaces the previously listed boards with a match for the whole device family CBxx63. CBxx63 matches only baytrail devices. There are new affected boards that would otherwise need to be listed. There are unaffected boards in the family, but having the clocks turned on is not an issue.
--- drivers/platform/x86/pmc_atom.c | 28 ++-------------------------- 1 file changed, 2 insertions(+), 26 deletions(-)
drivers/platform/x86/pmc_atom.c +2 -26
@@ -393,34 +393,10 @@ static const struct dmi_system_id critclk_systems[] = { }, {/* pmc_plt_clk* - are used for ethernet controllers */ - .ident = "Beckhoff CB3163", + .ident = "Beckhoff Baytrail", .matches = {DMI_MATCH (DMI_SYS_VENDOR ,"Beckhoff Automation" ), - DMI_MATCH(DMI_BOARD_NAME, "CB3163"), - }, - }, - { -/* pmc_plt_clk* - are used for ethernet controllers */ - .ident ="Beckhoff CB4063" , - .matches = { -DMI_MATCH (DMI_SYS_VENDOR ,"Beckhoff Automation" ), - DMI_MATCH(DMI_BOARD_NAME, "CB4063"), - }, - }, - { -/* pmc_plt_clk* - are used for ethernet controllers */ - .ident ="Beckhoff CB6263" , - .matches = { -DMI_MATCH (DMI_SYS_VENDOR ,"Beckhoff Automation" ), - DMI_MATCH(DMI_BOARD_NAME, "CB6263"), - }, - }, - { -/* pmc_plt_clk* - are used for ethernet controllers */ - .ident ="Beckhoff CB6363" , - .matches = { -DMI_MATCH (DMI_SYS_VENDOR ,"Beckhoff Automation" ), - DMI_MATCH(DMI_BOARD_NAME, "CB6363"), + DMI_MATCH(DMI_PRODUCT_FAMILY, "CBxx63"), }, }, {
Signature · 1 line
2.31.1
Hi,
Quoted 12 lines · Steffen Dirkwinkel
From: Steffen Dirkwinkel <[email protected]>
pmc_plt_clk* clocks are used for ethernet controllers, so need to stay turned on. This adds the affected board family to
critclk_systemsDMI table, so the clocks are marked asCLK_CRITICALand not turned off.This replaces the previously listed boards with a match for the whole device family CBxx63. CBxx63 matches only baytrail devices. There are new affected boards that would otherwise need to be listed. There are unaffected boards in the family, but having the clocks turned on is not an issue.
Quoted 3 lines
Fixes 648e921888ad ("clk: x86: Stop marking clocks as CLK_IS_CRITICAL")Reviewed-by Andy Shevchenko <[email protected]>Signed-off-by Steffen Dirkwinkel <[email protected]>
Thank you for your patch, I've applied this patch to my review-hans branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
Note it will show up in my review-hans branch once I've pushed my local branch there, which might take a while.
Once I've run some tests on this branch the patches there will be added to the platform-drivers-x86/for-next branch and eventually will be included in the pdx86 pull-request to Linus for the next merge-window.
Regards,
Hans
Quoted 46 lines
--- drivers/platform/x86/pmc_atom.c | 28 ++-------------------------- 1 file changed, 2 insertions(+), 26 deletions(-)drivers/platform/x86/pmc_atom.c +2
-26@@ -393,34 +393,10 @@ static const struct dmi_system_id critclk_systems[] = { }, {/* pmc_plt_clk* - are used for ethernet controllers */ - .ident = "Beckhoff CB3163", + .ident = "Beckhoff Baytrail", .matches = {DMI_MATCH (DMI_SYS_VENDOR ,"Beckhoff Automation" ), - DMI_MATCH(DMI_BOARD_NAME, "CB3163"), - }, - }, - { -/* pmc_plt_clk* - are used for ethernet controllers */ - .ident ="Beckhoff CB4063" , - .matches = { -DMI_MATCH (DMI_SYS_VENDOR ,"Beckhoff Automation" ), - DMI_MATCH(DMI_BOARD_NAME, "CB4063"), - }, - }, - { -/* pmc_plt_clk* - are used for ethernet controllers */ - .ident ="Beckhoff CB6263" , - .matches = { -DMI_MATCH (DMI_SYS_VENDOR ,"Beckhoff Automation" ), - DMI_MATCH(DMI_BOARD_NAME, "CB6263"), - }, - }, - { -/* pmc_plt_clk* - are used for ethernet controllers */ - .ident ="Beckhoff CB6363" , - .matches = { -DMI_MATCH (DMI_SYS_VENDOR ,"Beckhoff Automation" ), - DMI_MATCH(DMI_BOARD_NAME, "CB6363"), + DMI_MATCH(DMI_PRODUCT_FAMILY, "CBxx63"), }, }, {