platform-driver-x86.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] platform/x86: pmc_atom: Match all Beckhoff Automation baytrail boards with critclk_systems DMI table
@ 2021-04-12  9:04 Steffen Dirkwinkel
  2021-04-12  9:43 ` Andy Shevchenko
  0 siblings, 1 reply; 8+ messages in thread
From: Steffen Dirkwinkel @ 2021-04-12  9:04 UTC (permalink / raw)
  Cc: Andy Shevchenko, Hans de Goede, Mark Gross, platform-driver-x86,
	linux-kernel, Steffen Dirkwinkel

From: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>

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.

Fixes: 648e921888ad ("clk: x86: Stop marking clocks as CLK_IS_CRITICAL")
Signed-off-by: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>
---
 drivers/platform/x86/pmc_atom.c | 28 ++--------------------------
 1 file changed, 2 insertions(+), 26 deletions(-)

diff --git a/drivers/platform/x86/pmc_atom.c b/drivers/platform/x86/pmc_atom.c
index ca684ed760d1..a9d2a4b98e57 100644
--- a/drivers/platform/x86/pmc_atom.c
+++ b/drivers/platform/x86/pmc_atom.c
@@ -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"),
 		},
 	},
 	{
-- 
2.31.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] platform/x86: pmc_atom: Match all Beckhoff Automation baytrail boards with critclk_systems DMI table
  2021-04-12  9:04 [PATCH] platform/x86: pmc_atom: Match all Beckhoff Automation baytrail boards with critclk_systems DMI table Steffen Dirkwinkel
@ 2021-04-12  9:43 ` Andy Shevchenko
  2021-04-12 10:39   ` linux-kernel-dev
  0 siblings, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2021-04-12  9:43 UTC (permalink / raw)
  To: Steffen Dirkwinkel
  Cc: Andy Shevchenko, Hans de Goede, Mark Gross, Platform Driver,
	Linux Kernel Mailing List, Steffen Dirkwinkel

On Mon, Apr 12, 2021 at 12:29 PM Steffen Dirkwinkel
<linux-kernel-dev@beckhoff.com> wrote:
>
> From: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>
>
> 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

"...previously..."

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

> having the clocks turned on is not an issue on those.

"...not an issue."

> Fixes: 648e921888ad ("clk: x86: Stop marking clocks as CLK_IS_CRITICAL")
> Signed-off-by: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>

I'm afraid it's a bit too much. Is there any guarantee all the boards
based on x86 will be Baytrail only?

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] platform/x86: pmc_atom: Match all Beckhoff Automation baytrail boards with critclk_systems DMI table
  2021-04-12  9:43 ` Andy Shevchenko
@ 2021-04-12 10:39   ` linux-kernel-dev
  2021-04-12 10:54     ` Andy Shevchenko
  0 siblings, 1 reply; 8+ messages in thread
From: linux-kernel-dev @ 2021-04-12 10:39 UTC (permalink / raw)
  To: andy.shevchenko
  Cc: hdegoede, andriy.shevchenko, mgross, platform-driver-x86, linux-kernel

On Mo, 2021-04-12 at 12:43 +0300, Andy Shevchenko wrote:
>
> On Mon, Apr 12, 2021 at 12:29 PM Steffen Dirkwinkel
> <linux-kernel-dev@beckhoff.com> wrote:
> >
> > From: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>
> >
> > 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
>
> "...previously..."
thanks

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

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

>
> > Fixes: 648e921888ad ("clk: x86: Stop marking clocks as CLK_IS_CRITICAL")
> > Signed-off-by: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>
>
> 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.


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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] platform/x86: pmc_atom: Match all Beckhoff Automation baytrail boards with critclk_systems DMI table
  2021-04-12 10:39   ` linux-kernel-dev
@ 2021-04-12 10:54     ` Andy Shevchenko
  2021-04-12 11:07       ` linux-kernel-dev
  0 siblings, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2021-04-12 10:54 UTC (permalink / raw)
  To: linux-kernel-dev
  Cc: hdegoede, andriy.shevchenko, mgross, platform-driver-x86, linux-kernel

On Mon, Apr 12, 2021 at 1:39 PM linux-kernel-dev
<linux-kernel-dev@beckhoff.com> wrote:
> On Mo, 2021-04-12 at 12:43 +0300, Andy Shevchenko wrote:
> > On Mon, Apr 12, 2021 at 12:29 PM Steffen Dirkwinkel
> > <linux-kernel-dev@beckhoff.com> wrote:

...

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

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] platform/x86: pmc_atom: Match all Beckhoff Automation baytrail boards with critclk_systems DMI table
  2021-04-12 10:54     ` Andy Shevchenko
@ 2021-04-12 11:07       ` linux-kernel-dev
  2021-04-12 11:23         ` Andy Shevchenko
  0 siblings, 1 reply; 8+ messages in thread
From: linux-kernel-dev @ 2021-04-12 11:07 UTC (permalink / raw)
  To: andy.shevchenko
  Cc: hdegoede, andriy.shevchenko, mgross, platform-driver-x86, linux-kernel

On Mo, 2021-04-12 at 13:54 +0300, Andy Shevchenko wrote:
> CAUTION: External Email!!
>
>
> On Mon, Apr 12, 2021 at 1:39 PM linux-kernel-dev
> <linux-kernel-dev@beckhoff.com> wrote:
> > On Mo, 2021-04-12 at 12:43 +0300, Andy Shevchenko wrote:
> > > On Mon, Apr 12, 2021 at 12:29 PM Steffen Dirkwinkel
> > > <linux-kernel-dev@beckhoff.com> wrote:
>
> ...
>
> > > 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.

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.

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] platform/x86: pmc_atom: Match all Beckhoff Automation baytrail boards with critclk_systems DMI table
  2021-04-12 11:07       ` linux-kernel-dev
@ 2021-04-12 11:23         ` Andy Shevchenko
  2021-04-12 13:30           ` [PATCH v2] " Steffen Dirkwinkel
  0 siblings, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2021-04-12 11:23 UTC (permalink / raw)
  To: linux-kernel-dev
  Cc: hdegoede, andriy.shevchenko, mgross, platform-driver-x86, linux-kernel

On Mon, Apr 12, 2021 at 2:07 PM linux-kernel-dev
<linux-kernel-dev@beckhoff.com> wrote:
> On Mo, 2021-04-12 at 13:54 +0300, Andy Shevchenko wrote:
> > CAUTION: External Email!!
> > On Mon, Apr 12, 2021 at 1:39 PM linux-kernel-dev
> > <linux-kernel-dev@beckhoff.com> wrote:
> > > On Mo, 2021-04-12 at 12:43 +0300, Andy Shevchenko wrote:
> > > > On Mon, Apr 12, 2021 at 12:29 PM Steffen Dirkwinkel
> > > > <linux-kernel-dev@beckhoff.com> wrote:

...

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

Sorry, I missed the second match line. Yes, looks correct to me now.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] platform/x86: pmc_atom: Match all Beckhoff Automation baytrail boards with critclk_systems DMI table
  2021-04-12 11:23         ` Andy Shevchenko
@ 2021-04-12 13:30           ` Steffen Dirkwinkel
  2021-04-13  8:15             ` Hans de Goede
  0 siblings, 1 reply; 8+ messages in thread
From: Steffen Dirkwinkel @ 2021-04-12 13:30 UTC (permalink / raw)
  Cc: Andy Shevchenko, Hans de Goede, Mark Gross, platform-driver-x86,
	linux-kernel, Steffen Dirkwinkel

From: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>

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.

Fixes: 648e921888ad ("clk: x86: Stop marking clocks as CLK_IS_CRITICAL")
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>
---
 drivers/platform/x86/pmc_atom.c | 28 ++--------------------------
 1 file changed, 2 insertions(+), 26 deletions(-)

diff --git a/drivers/platform/x86/pmc_atom.c b/drivers/platform/x86/pmc_atom.c
index ca684ed760d1..a9d2a4b98e57 100644
--- a/drivers/platform/x86/pmc_atom.c
+++ b/drivers/platform/x86/pmc_atom.c
@@ -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"),
 		},
 	},
 	{
-- 
2.31.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] platform/x86: pmc_atom: Match all Beckhoff Automation baytrail boards with critclk_systems DMI table
  2021-04-12 13:30           ` [PATCH v2] " Steffen Dirkwinkel
@ 2021-04-13  8:15             ` Hans de Goede
  0 siblings, 0 replies; 8+ messages in thread
From: Hans de Goede @ 2021-04-13  8:15 UTC (permalink / raw)
  To: Steffen Dirkwinkel
  Cc: Andy Shevchenko, Mark Gross, platform-driver-x86, linux-kernel,
	Steffen Dirkwinkel

Hi,

On 4/12/21 3:30 PM, Steffen Dirkwinkel wrote:
> From: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>
> 
> 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.
> 
> Fixes: 648e921888ad ("clk: x86: Stop marking clocks as CLK_IS_CRITICAL")
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Steffen Dirkwinkel <s.dirkwinkel@beckhoff.com>

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




> ---
>  drivers/platform/x86/pmc_atom.c | 28 ++--------------------------
>  1 file changed, 2 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/platform/x86/pmc_atom.c b/drivers/platform/x86/pmc_atom.c
> index ca684ed760d1..a9d2a4b98e57 100644
> --- a/drivers/platform/x86/pmc_atom.c
> +++ b/drivers/platform/x86/pmc_atom.c
> @@ -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"),
>  		},
>  	},
>  	{
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-04-13  8:15 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-12  9:04 [PATCH] platform/x86: pmc_atom: Match all Beckhoff Automation baytrail boards with critclk_systems DMI table Steffen Dirkwinkel
2021-04-12  9:43 ` Andy Shevchenko
2021-04-12 10:39   ` linux-kernel-dev
2021-04-12 10:54     ` Andy Shevchenko
2021-04-12 11:07       ` linux-kernel-dev
2021-04-12 11:23         ` Andy Shevchenko
2021-04-12 13:30           ` [PATCH v2] " Steffen Dirkwinkel
2021-04-13  8:15             ` Hans de Goede

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).