2018-09-12 14:38:43

by Hans de Goede

[permalink] [raw]
Subject: [PATCH v2 0/3] r8169 (x86) clk fixes to fix S0ix not being reached

Hi David,

This series adds code to the r8169 ethernet driver to get and enable an
external clock if present, avoiding the need for a hack in the
clk-pmc-atom driver where that clock was left on continuesly causing x86
some devices to not reach deep power saving states (S0ix) when suspended
causing to them to quickly drain their battery while suspended.

The 3 commits in this series need to be merged in order to avoid
regressions while bisecting. The clk-pmc-atom driver does not see much
changes (it was last touched over a year ago). So the clk maintainers
have agreed with merging all 3 patches through the net tree.
All 3 patches have Stephen Boyd's Acked-by for this purpose.

This v2 of the series only had some minor tweaks done to the commit
messages and is ready for merging through the net tree now.

Thanks & Regards,

Hans


2018-09-12 14:38:49

by Hans de Goede

[permalink] [raw]
Subject: [PATCH v2 2/3] r8169: Get and enable optional ether_clk clock

On some boards a platform clock is used as clock for the r8169 chip,
this commit adds support for getting and enabling this clock (assuming
it has an "ether_clk" alias set on it).

This is related to commit d31fd43c0f9a ("clk: x86: Do not gate clocks
enabled by the firmware") which is a previous attempt to fix this for some
x86 boards, but this causes all Cherry Trail SoC using boards to not reach
there lowest power states when suspending.

This commit (together with an atom-pmc-clk driver commit adding the alias)
fixes things properly by making the r8169 get the clock and enable it when
it needs it.

Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=193891#c102
Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=196861
Cc: Johannes Stezenbach <[email protected]>
Cc: Carlo Caione <[email protected]>
Reported-by: Johannes Stezenbach <[email protected]>
Acked-by: Stephen Boyd <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Hans de Goede <[email protected]>
---
Changes in v2:
-Tweaked the commit msg a bit
-Added: Stephen's Acked-by, Andy's Reviewed-by
---
drivers/net/ethernet/realtek/r8169.c | 33 ++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index b08d51bf7a20..474229548731 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -13,6 +13,7 @@
#include <linux/pci.h>
#include <linux/netdevice.h>
#include <linux/etherdevice.h>
+#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/ethtool.h>
#include <linux/phy.h>
@@ -665,6 +666,7 @@ struct rtl8169_private {

u16 event_slow;
const struct rtl_coalesce_info *coalesce_info;
+ struct clk *clk;

struct mdio_ops {
void (*write)(struct rtl8169_private *, int, int);
@@ -7254,6 +7256,11 @@ static int rtl_jumbo_max(struct rtl8169_private *tp)
}
}

+static void rtl_disable_clk(void *data)
+{
+ clk_disable_unprepare(data);
+}
+
static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
{
const struct rtl_cfg_info *cfg = rtl_cfg_infos + ent->driver_data;
@@ -7274,6 +7281,32 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
tp->msg_enable = netif_msg_init(debug.msg_enable, R8169_MSG_DEFAULT);
tp->supports_gmii = cfg->has_gmii;

+ /* Get the *optional* external "ether_clk" used on some boards */
+ tp->clk = devm_clk_get(&pdev->dev, "ether_clk");
+ if (IS_ERR(tp->clk)) {
+ rc = PTR_ERR(tp->clk);
+ if (rc == -ENOENT) {
+ /* clk-core allows NULL (for suspend / resume) */
+ tp->clk = NULL;
+ } else if (rc == -EPROBE_DEFER) {
+ return rc;
+ } else {
+ dev_err(&pdev->dev, "failed to get clk: %d\n", rc);
+ return rc;
+ }
+ } else {
+ rc = clk_prepare_enable(tp->clk);
+ if (rc) {
+ dev_err(&pdev->dev, "failed to enable clk: %d\n", rc);
+ return rc;
+ }
+
+ rc = devm_add_action_or_reset(&pdev->dev, rtl_disable_clk,
+ tp->clk);
+ if (rc)
+ return rc;
+ }
+
/* enable device (incl. PCI PM wakeup and hotplug setup) */
rc = pcim_enable_device(pdev);
if (rc < 0) {
--
2.19.0.rc0

2018-09-12 14:38:50

by Hans de Goede

[permalink] [raw]
Subject: [PATCH v2 3/3] clk: x86: Stop marking clocks as CLK_IS_CRITICAL

Commit d31fd43c0f9a ("clk: x86: Do not gate clocks enabled by the
firmware"), which added the code to mark clocks as CLK_IS_CRITICAL, causes
all unclaimed PMC clocks on Cherry Trail devices to be on all the time,
resulting on the device not being able to reach S0i3 when suspended.

The reason for this commit is that on some Bay Trail / Cherry Trail devices
the r8169 ethernet controller uses pmc_plt_clk_4. Now that the clk-pmc-atom
driver exports an "ether_clk" alias for pmc_plt_clk_4 and the r8169 driver
has been modified to get and enable this clock (if present) the marking of
the clocks as CLK_IS_CRITICAL is no longer necessary.

This commit removes the CLK_IS_CRITICAL marking, fixing Cherry Trail
devices not being able to reach S0i3 greatly decreasing their battery
drain when suspended.

Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=193891#c102
Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=196861
Cc: Johannes Stezenbach <[email protected]>
Cc: Carlo Caione <[email protected]>
Reported-by: Johannes Stezenbach <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Acked-by: Stephen Boyd <[email protected]>
Signed-off-by: Hans de Goede <[email protected]>
---
Changes in v2:
-Tweaked the commit msg a bit
-Added: Stephen's Acked-by, Andy's Reviewed-by
---
drivers/clk/x86/clk-pmc-atom.c | 7 -------
1 file changed, 7 deletions(-)

diff --git a/drivers/clk/x86/clk-pmc-atom.c b/drivers/clk/x86/clk-pmc-atom.c
index 75151901ff7d..d977193842df 100644
--- a/drivers/clk/x86/clk-pmc-atom.c
+++ b/drivers/clk/x86/clk-pmc-atom.c
@@ -187,13 +187,6 @@ static struct clk_plt *plt_clk_register(struct platform_device *pdev, int id,
pclk->reg = base + PMC_CLK_CTL_OFFSET + id * PMC_CLK_CTL_SIZE;
spin_lock_init(&pclk->lock);

- /*
- * If the clock was already enabled by the firmware mark it as critical
- * to avoid it being gated by the clock framework if no driver owns it.
- */
- if (plt_clk_is_enabled(&pclk->hw))
- init.flags |= CLK_IS_CRITICAL;
-
ret = devm_clk_hw_register(&pdev->dev, &pclk->hw);
if (ret) {
pclk = ERR_PTR(ret);
--
2.19.0.rc0

2018-09-12 14:38:46

by Hans de Goede

[permalink] [raw]
Subject: [PATCH v2 1/3] clk: x86: add "ether_clk" alias for Bay Trail / Cherry Trail

Commit d31fd43c0f9a ("clk: x86: Do not gate clocks enabled by the
firmware") causes all unclaimed PMC clocks on Cherry Trail devices to be on
all the time, resulting on the device not being able to reach S0i2 or S0i3
when suspended.

The reason for this commit is that on some Bay Trail / Cherry Trail devices
the ethernet controller uses pmc_plt_clk_4. This commit adds an "ether_clk"
alias, so that the relevant ethernet drivers can try to (optionally) use
this, without needing X86 specific code / hacks, thus fixing ethernet on
these devices without breaking S0i3 support.

This commit uses clkdev_hw_create() to create the alias, mirroring the code
for the already existing "mclk" alias for pmc_plt_clk_3.

Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=193891#c102
Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=196861
Cc: Johannes Stezenbach <[email protected]>
Cc: Carlo Caione <[email protected]>
Reported-by: Johannes Stezenbach <[email protected]>
Acked-by: Stephen Boyd <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Hans de Goede <[email protected]>
---
Changes in v2:
-Tweaked the commit msg a bit
-Added: Stephen's Acked-by, Andy's Reviewed-by
---
drivers/clk/x86/clk-pmc-atom.c | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/drivers/clk/x86/clk-pmc-atom.c b/drivers/clk/x86/clk-pmc-atom.c
index 08ef69945ffb..75151901ff7d 100644
--- a/drivers/clk/x86/clk-pmc-atom.c
+++ b/drivers/clk/x86/clk-pmc-atom.c
@@ -55,6 +55,7 @@ struct clk_plt_data {
u8 nparents;
struct clk_plt *clks[PMC_CLK_NUM];
struct clk_lookup *mclk_lookup;
+ struct clk_lookup *ether_clk_lookup;
};

/* Return an index in parent table */
@@ -351,11 +352,20 @@ static int plt_clk_probe(struct platform_device *pdev)
goto err_unreg_clk_plt;
}

+ data->ether_clk_lookup = clkdev_hw_create(&data->clks[4]->hw,
+ "ether_clk", NULL);
+ if (!data->ether_clk_lookup) {
+ err = -ENOMEM;
+ goto err_drop_mclk;
+ }
+
plt_clk_free_parent_names_loop(parent_names, data->nparents);

platform_set_drvdata(pdev, data);
return 0;

+err_drop_mclk:
+ clkdev_drop(data->mclk_lookup);
err_unreg_clk_plt:
plt_clk_unregister_loop(data, i);
plt_clk_unregister_parents(data);
@@ -369,6 +379,7 @@ static int plt_clk_remove(struct platform_device *pdev)

data = platform_get_drvdata(pdev);

+ clkdev_drop(data->ether_clk_lookup);
clkdev_drop(data->mclk_lookup);
plt_clk_unregister_loop(data, PMC_CLK_NUM);
plt_clk_unregister_parents(data);
--
2.19.0.rc0

2018-09-18 07:18:57

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] r8169 (x86) clk fixes to fix S0ix not being reached

From: Hans de Goede <[email protected]>
Date: Wed, 12 Sep 2018 11:34:53 +0200

> This series adds code to the r8169 ethernet driver to get and enable an
> external clock if present, avoiding the need for a hack in the
> clk-pmc-atom driver where that clock was left on continuesly causing x86
> some devices to not reach deep power saving states (S0ix) when suspended
> causing to them to quickly drain their battery while suspended.
>
> The 3 commits in this series need to be merged in order to avoid
> regressions while bisecting. The clk-pmc-atom driver does not see much
> changes (it was last touched over a year ago). So the clk maintainers
> have agreed with merging all 3 patches through the net tree.
> All 3 patches have Stephen Boyd's Acked-by for this purpose.
>
> This v2 of the series only had some minor tweaks done to the commit
> messages and is ready for merging through the net tree now.

Thanks for all of that useful information.

Series applied, thanks.

2022-07-24 21:18:38

by Matwey V. Kornilov

[permalink] [raw]
Subject: [BISECTED] igb initialization failure on Bay Trail

Hello,

I've just found that the following commit

648e921888ad ("clk: x86: Stop marking clocks as CLK_IS_CRITICAL")

breaks the ethernet on my Lex 3I380CW (Atom E3845) motherboard. The board is
equipped with dual Intel I211 based 1Gbps copper ethernet.

Before the commit I see the following:

igb 0000:01:00.0: added PHC on eth0
igb 0000:01:00.0: Intel(R) Gigabit Ethernet Network Connection
igb 0000:01:00.0: eth0: (PCIe:2.5Gb/s:Width x1) 4c:02:89:10:02:e4
igb 0000:01:00.0: eth0: PBA No: FFFFFF-0FF
igb 0000:01:00.0: Using MSI-X interrupts. 2 rx queue(s), 2 tx queue(s)
igb 0000:02:00.0: added PHC on eth1
igb 0000:02:00.0: Intel(R) Gigabit Ethernet Network Connection
igb 0000:02:00.0: eth1: (PCIe:2.5Gb/s:Width x1) 4c:02:89:10:02:e5
igb 0000:02:00.0: eth1: PBA No: FFFFFF-0FF
igb 0000:02:00.0: Using MSI-X interrupts. 2 rx queue(s), 2 tx queue(s)

while when the commit is applied I see the following:

igb 0000:01:00.0: added PHC on eth0
igb 0000:01:00.0: Intel(R) Gigabit Ethernet Network Connection
igb 0000:01:00.0: eth0: (PCIe:2.5Gb/s:Width x1) 4c:02:89:10:02:e4
igb 0000:01:00.0: eth0: PBA No: FFFFFF-0FF
igb 0000:01:00.0: Using MSI-X interrupts. 2 rx queue(s), 2 tx queue(s)
igb: probe of 0000:02:00.0 failed with error -2

Please note, that the second ethernet initialization is failed.


See also: http://www.lex.com.tw/products/pdf/3I380A&3I380CW.pdf

2022-07-25 17:11:10

by Pierre-Louis Bossart

[permalink] [raw]
Subject: Re: [BISECTED] igb initialization failure on Bay Trail



On 7/24/22 16:00, Matwey V. Kornilov wrote:
> Hello,
>
> I've just found that the following commit
>
> 648e921888ad ("clk: x86: Stop marking clocks as CLK_IS_CRITICAL")
>
> breaks the ethernet on my Lex 3I380CW (Atom E3845) motherboard. The board is
> equipped with dual Intel I211 based 1Gbps copper ethernet.

It's not going to be simple, it's 4 yr old commit that fixes other
issues with S0i3...

>
> Before the commit I see the following:
>
> igb 0000:01:00.0: added PHC on eth0
> igb 0000:01:00.0: Intel(R) Gigabit Ethernet Network Connection
> igb 0000:01:00.0: eth0: (PCIe:2.5Gb/s:Width x1) 4c:02:89:10:02:e4
> igb 0000:01:00.0: eth0: PBA No: FFFFFF-0FF
> igb 0000:01:00.0: Using MSI-X interrupts. 2 rx queue(s), 2 tx queue(s)
> igb 0000:02:00.0: added PHC on eth1
> igb 0000:02:00.0: Intel(R) Gigabit Ethernet Network Connection
> igb 0000:02:00.0: eth1: (PCIe:2.5Gb/s:Width x1) 4c:02:89:10:02:e5
> igb 0000:02:00.0: eth1: PBA No: FFFFFF-0FF
> igb 0000:02:00.0: Using MSI-X interrupts. 2 rx queue(s), 2 tx queue(s)
>
> while when the commit is applied I see the following:
>
> igb 0000:01:00.0: added PHC on eth0
> igb 0000:01:00.0: Intel(R) Gigabit Ethernet Network Connection
> igb 0000:01:00.0: eth0: (PCIe:2.5Gb/s:Width x1) 4c:02:89:10:02:e4
> igb 0000:01:00.0: eth0: PBA No: FFFFFF-0FF
> igb 0000:01:00.0: Using MSI-X interrupts. 2 rx queue(s), 2 tx queue(s)
> igb: probe of 0000:02:00.0 failed with error -2
>
> Please note, that the second ethernet initialization is failed.
>
>
> See also: http://www.lex.com.tw/products/pdf/3I380A&3I380CW.pdf

2022-07-26 09:22:42

by Matwey V. Kornilov

[permalink] [raw]
Subject: Re: [BISECTED] igb initialization failure on Bay Trail

пн, 25 июл. 2022 г. в 20:08, Pierre-Louis Bossart
<[email protected]>:
>
>
>
> On 7/24/22 16:00, Matwey V. Kornilov wrote:
> > Hello,
> >
> > I've just found that the following commit
> >
> > 648e921888ad ("clk: x86: Stop marking clocks as CLK_IS_CRITICAL")
> >
> > breaks the ethernet on my Lex 3I380CW (Atom E3845) motherboard. The board is
> > equipped with dual Intel I211 based 1Gbps copper ethernet.
>
> It's not going to be simple, it's 4 yr old commit that fixes other
> issues with S0i3...

Additionally, it seems that the issue appears only when CONFIG_IGB=m
is used. When CONFIG_IGB=y then both ethernets are initialized
correctly.
However, most (if not all) kernel configs in Linux distros use CONFIG_IGB=m

>
> >
> > Before the commit I see the following:
> >
> > igb 0000:01:00.0: added PHC on eth0
> > igb 0000:01:00.0: Intel(R) Gigabit Ethernet Network Connection
> > igb 0000:01:00.0: eth0: (PCIe:2.5Gb/s:Width x1) 4c:02:89:10:02:e4
> > igb 0000:01:00.0: eth0: PBA No: FFFFFF-0FF
> > igb 0000:01:00.0: Using MSI-X interrupts. 2 rx queue(s), 2 tx queue(s)
> > igb 0000:02:00.0: added PHC on eth1
> > igb 0000:02:00.0: Intel(R) Gigabit Ethernet Network Connection
> > igb 0000:02:00.0: eth1: (PCIe:2.5Gb/s:Width x1) 4c:02:89:10:02:e5
> > igb 0000:02:00.0: eth1: PBA No: FFFFFF-0FF
> > igb 0000:02:00.0: Using MSI-X interrupts. 2 rx queue(s), 2 tx queue(s)
> >
> > while when the commit is applied I see the following:
> >
> > igb 0000:01:00.0: added PHC on eth0
> > igb 0000:01:00.0: Intel(R) Gigabit Ethernet Network Connection
> > igb 0000:01:00.0: eth0: (PCIe:2.5Gb/s:Width x1) 4c:02:89:10:02:e4
> > igb 0000:01:00.0: eth0: PBA No: FFFFFF-0FF
> > igb 0000:01:00.0: Using MSI-X interrupts. 2 rx queue(s), 2 tx queue(s)
> > igb: probe of 0000:02:00.0 failed with error -2
> >
> > Please note, that the second ethernet initialization is failed.
> >
> >
> > See also: http://www.lex.com.tw/products/pdf/3I380A&3I380CW.pdf



--
With best regards,
Matwey V. Kornilov

2022-07-27 12:28:53

by Hans de Goede

[permalink] [raw]
Subject: Re: [BISECTED] igb initialization failure on Bay Trail

Hi Paul,

On 7/24/22 23:00, Matwey V. Kornilov wrote:
> Hello,
>
> I've just found that the following commit
>
> 648e921888ad ("clk: x86: Stop marking clocks as CLK_IS_CRITICAL")
>
> breaks the ethernet on my Lex 3I380CW (Atom E3845) motherboard. The board is
> equipped with dual Intel I211 based 1Gbps copper ethernet.
>
> Before the commit I see the following:
>
> igb 0000:01:00.0: added PHC on eth0
> igb 0000:01:00.0: Intel(R) Gigabit Ethernet Network Connection
> igb 0000:01:00.0: eth0: (PCIe:2.5Gb/s:Width x1) 4c:02:89:10:02:e4
> igb 0000:01:00.0: eth0: PBA No: FFFFFF-0FF
> igb 0000:01:00.0: Using MSI-X interrupts. 2 rx queue(s), 2 tx queue(s)
> igb 0000:02:00.0: added PHC on eth1
> igb 0000:02:00.0: Intel(R) Gigabit Ethernet Network Connection
> igb 0000:02:00.0: eth1: (PCIe:2.5Gb/s:Width x1) 4c:02:89:10:02:e5
> igb 0000:02:00.0: eth1: PBA No: FFFFFF-0FF
> igb 0000:02:00.0: Using MSI-X interrupts. 2 rx queue(s), 2 tx queue(s)
>
> while when the commit is applied I see the following:
>
> igb 0000:01:00.0: added PHC on eth0
> igb 0000:01:00.0: Intel(R) Gigabit Ethernet Network Connection
> igb 0000:01:00.0: eth0: (PCIe:2.5Gb/s:Width x1) 4c:02:89:10:02:e4
> igb 0000:01:00.0: eth0: PBA No: FFFFFF-0FF
> igb 0000:01:00.0: Using MSI-X interrupts. 2 rx queue(s), 2 tx queue(s)
> igb: probe of 0000:02:00.0 failed with error -2
>
> Please note, that the second ethernet initialization is failed.
>
>
> See also: http://www.lex.com.tw/products/pdf/3I380A&3I380CW.pdf

Yes some boards use more then 1 clk for the ethernet and do not take
care of enabling/disabling the clk in their ACPI.

As Pierre-Louis mentioned already the disabling of the clocks is necessary
to make 100-s of different (tablet) models suspend properly.

Unfortunately this is known to break ethernet on some boards. As a workaround
we use DMI quirks on those few boards to keep the clocks enabled there, see:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/platform/x86/pmc_atom.c#n381

If you can submit a patch adding your board to this DMI table, then I will
merge it and get it on its way to Linus Torvalds asap.

If you instead want me to write the patch for you, please run:

sudo dmidecode > dmidecode.txt

And attach the generated dmidecode.txt file to your next email.

Regards,

Hans

2022-07-27 15:40:38

by Matwey V. Kornilov

[permalink] [raw]
Subject: [PATCH] platform/x86: pmc_atom: Add DMI quirk for Lex 3I380A/CW boards

Lex 3I380A/CW (Atom E3845) motherboards are equipped with dual Intel I211
based 1Gbps copper ethernet:

http://www.lex.com.tw/products/pdf/3I380A&3I380CW.pdf

This patch is to fix the issue with broken "LAN2" port. Before the
patch, only one ethernet port is initialized:

igb 0000:01:00.0: added PHC on eth0
igb 0000:01:00.0: Intel(R) Gigabit Ethernet Network Connection
igb 0000:01:00.0: eth0: (PCIe:2.5Gb/s:Width x1) 4c:02:89:10:02:e4
igb 0000:01:00.0: eth0: PBA No: FFFFFF-0FF
igb 0000:01:00.0: Using MSI-X interrupts. 2 rx queue(s), 2 tx queue(s)
igb: probe of 0000:02:00.0 failed with error -2

With this patch, both ethernet ports are available:

igb 0000:01:00.0: added PHC on eth0
igb 0000:01:00.0: Intel(R) Gigabit Ethernet Network Connection
igb 0000:01:00.0: eth0: (PCIe:2.5Gb/s:Width x1) 4c:02:89:10:02:e4
igb 0000:01:00.0: eth0: PBA No: FFFFFF-0FF
igb 0000:01:00.0: Using MSI-X interrupts. 2 rx queue(s), 2 tx queue(s)
igb 0000:02:00.0: added PHC on eth1
igb 0000:02:00.0: Intel(R) Gigabit Ethernet Network Connection
igb 0000:02:00.0: eth1: (PCIe:2.5Gb/s:Width x1) 4c:02:89:10:02:e5
igb 0000:02:00.0: eth1: PBA No: FFFFFF-0FF
igb 0000:02:00.0: Using MSI-X interrupts. 2 rx queue(s), 2 tx queue(s)

The issue was observed at 3I380A board with BIOS version "A4 01/15/2016"
and 3I380CW board with BIOS version "A3 09/29/2014".

Reference: https://lore.kernel.org/netdev/[email protected]/
Fixes: 648e921888ad ("clk: x86: Stop marking clocks as CLK_IS_CRITICAL")
Cc: <[email protected]> # v4.19+
Signed-off-by: Matwey V. Kornilov <[email protected]>
---
drivers/platform/x86/pmc_atom.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/drivers/platform/x86/pmc_atom.c b/drivers/platform/x86/pmc_atom.c
index b8b1ed1406de..5dc82667907b 100644
--- a/drivers/platform/x86/pmc_atom.c
+++ b/drivers/platform/x86/pmc_atom.c
@@ -388,6 +388,24 @@ static const struct dmi_system_id critclk_systems[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "CEC10 Family"),
},
},
+ {
+ /* pmc_plt_clk* - are used for ethernet controllers */
+ .ident = "Lex 3I380A",
+ .callback = dmi_callback,
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Lex BayTrail"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "3I380A"),
+ },
+ },
+ {
+ /* pmc_plt_clk* - are used for ethernet controllers */
+ .ident = "Lex 3I380CW",
+ .callback = dmi_callback,
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Lex BayTrail"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "3I380CW"),
+ },
+ },
{
/* pmc_plt_clk0 - 3 are used for the 4 ethernet controllers */
.ident = "Lex 3I380D",
--
2.35.3

2022-07-28 18:40:37

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH] platform/x86: pmc_atom: Add DMI quirk for Lex 3I380A/CW boards

Hi,

On 7/27/22 17:32, Matwey V. Kornilov wrote:
> Lex 3I380A/CW (Atom E3845) motherboards are equipped with dual Intel I211
> based 1Gbps copper ethernet:
>
> http://www.lex.com.tw/products/pdf/3I380A&3I380CW.pdf
>
> This patch is to fix the issue with broken "LAN2" port. Before the
> patch, only one ethernet port is initialized:
>
> igb 0000:01:00.0: added PHC on eth0
> igb 0000:01:00.0: Intel(R) Gigabit Ethernet Network Connection
> igb 0000:01:00.0: eth0: (PCIe:2.5Gb/s:Width x1) 4c:02:89:10:02:e4
> igb 0000:01:00.0: eth0: PBA No: FFFFFF-0FF
> igb 0000:01:00.0: Using MSI-X interrupts. 2 rx queue(s), 2 tx queue(s)
> igb: probe of 0000:02:00.0 failed with error -2
>
> With this patch, both ethernet ports are available:
>
> igb 0000:01:00.0: added PHC on eth0
> igb 0000:01:00.0: Intel(R) Gigabit Ethernet Network Connection
> igb 0000:01:00.0: eth0: (PCIe:2.5Gb/s:Width x1) 4c:02:89:10:02:e4
> igb 0000:01:00.0: eth0: PBA No: FFFFFF-0FF
> igb 0000:01:00.0: Using MSI-X interrupts. 2 rx queue(s), 2 tx queue(s)
> igb 0000:02:00.0: added PHC on eth1
> igb 0000:02:00.0: Intel(R) Gigabit Ethernet Network Connection
> igb 0000:02:00.0: eth1: (PCIe:2.5Gb/s:Width x1) 4c:02:89:10:02:e5
> igb 0000:02:00.0: eth1: PBA No: FFFFFF-0FF
> igb 0000:02:00.0: Using MSI-X interrupts. 2 rx queue(s), 2 tx queue(s)
>
> The issue was observed at 3I380A board with BIOS version "A4 01/15/2016"
> and 3I380CW board with BIOS version "A3 09/29/2014".
>
> Reference: https://lore.kernel.org/netdev/[email protected]/
> Fixes: 648e921888ad ("clk: x86: Stop marking clocks as CLK_IS_CRITICAL")
> Cc: <[email protected]> # v4.19+
> Signed-off-by: Matwey V. Kornilov <[email protected]>


Thank you for the patch.

The last week I have received 2 different patches adding
a total of 3 new "Lex BayTrail" entries to critclk_systems[]
on top of the existing 2.

Looking at: https://www.lex.com.tw/products/embedded-ipc-board/
we can see that Lex BayTrail makes many embedded boards with
multiple ethernet boards and none of their products are battery
powered so we don't need to worry (too much) about power consumption
when suspended.

So instead of adding 3 new entries I've written a patch to
simply disable the turning off of the clocks on all
systems which have "Lex BayTrail" as their DMI sys_vendor:

https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/commit/?h=review-hans&id=c9d959fc32a5f9312282817052d8986614f2dc08

I've added a Reported-by tag to give you credit for the work
you have done on this.

I will send this alternative fix to Linus as part of
the other pdx86 patches for 5.21.

Regards,

Hans




> ---
> drivers/platform/x86/pmc_atom.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/platform/x86/pmc_atom.c b/drivers/platform/x86/pmc_atom.c
> index b8b1ed1406de..5dc82667907b 100644
> --- a/drivers/platform/x86/pmc_atom.c
> +++ b/drivers/platform/x86/pmc_atom.c
> @@ -388,6 +388,24 @@ static const struct dmi_system_id critclk_systems[] = {
> DMI_MATCH(DMI_PRODUCT_NAME, "CEC10 Family"),
> },
> },
> + {
> + /* pmc_plt_clk* - are used for ethernet controllers */
> + .ident = "Lex 3I380A",
> + .callback = dmi_callback,
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Lex BayTrail"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "3I380A"),
> + },
> + },
> + {
> + /* pmc_plt_clk* - are used for ethernet controllers */
> + .ident = "Lex 3I380CW",
> + .callback = dmi_callback,
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Lex BayTrail"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "3I380CW"),
> + },
> + },
> {
> /* pmc_plt_clk0 - 3 are used for the 4 ethernet controllers */
> .ident = "Lex 3I380D",

2022-07-28 18:42:31

by Matwey V. Kornilov

[permalink] [raw]
Subject: Re: [PATCH] platform/x86: pmc_atom: Add DMI quirk for Lex 3I380A/CW boards

чт, 28 июл. 2022 г. в 21:33, Hans de Goede <[email protected]>:
>
> Hi,
>
> On 7/27/22 17:32, Matwey V. Kornilov wrote:
> > Lex 3I380A/CW (Atom E3845) motherboards are equipped with dual Intel I211
> > based 1Gbps copper ethernet:
> >
> > http://www.lex.com.tw/products/pdf/3I380A&3I380CW.pdf
> >
> > This patch is to fix the issue with broken "LAN2" port. Before the
> > patch, only one ethernet port is initialized:
> >
> > igb 0000:01:00.0: added PHC on eth0
> > igb 0000:01:00.0: Intel(R) Gigabit Ethernet Network Connection
> > igb 0000:01:00.0: eth0: (PCIe:2.5Gb/s:Width x1) 4c:02:89:10:02:e4
> > igb 0000:01:00.0: eth0: PBA No: FFFFFF-0FF
> > igb 0000:01:00.0: Using MSI-X interrupts. 2 rx queue(s), 2 tx queue(s)
> > igb: probe of 0000:02:00.0 failed with error -2
> >
> > With this patch, both ethernet ports are available:
> >
> > igb 0000:01:00.0: added PHC on eth0
> > igb 0000:01:00.0: Intel(R) Gigabit Ethernet Network Connection
> > igb 0000:01:00.0: eth0: (PCIe:2.5Gb/s:Width x1) 4c:02:89:10:02:e4
> > igb 0000:01:00.0: eth0: PBA No: FFFFFF-0FF
> > igb 0000:01:00.0: Using MSI-X interrupts. 2 rx queue(s), 2 tx queue(s)
> > igb 0000:02:00.0: added PHC on eth1
> > igb 0000:02:00.0: Intel(R) Gigabit Ethernet Network Connection
> > igb 0000:02:00.0: eth1: (PCIe:2.5Gb/s:Width x1) 4c:02:89:10:02:e5
> > igb 0000:02:00.0: eth1: PBA No: FFFFFF-0FF
> > igb 0000:02:00.0: Using MSI-X interrupts. 2 rx queue(s), 2 tx queue(s)
> >
> > The issue was observed at 3I380A board with BIOS version "A4 01/15/2016"
> > and 3I380CW board with BIOS version "A3 09/29/2014".
> >
> > Reference: https://lore.kernel.org/netdev/[email protected]/
> > Fixes: 648e921888ad ("clk: x86: Stop marking clocks as CLK_IS_CRITICAL")
> > Cc: <[email protected]> # v4.19+
> > Signed-off-by: Matwey V. Kornilov <[email protected]>
>
>
> Thank you for the patch.
>
> The last week I have received 2 different patches adding
> a total of 3 new "Lex BayTrail" entries to critclk_systems[]
> on top of the existing 2.
>
> Looking at: https://www.lex.com.tw/products/embedded-ipc-board/
> we can see that Lex BayTrail makes many embedded boards with
> multiple ethernet boards and none of their products are battery
> powered so we don't need to worry (too much) about power consumption
> when suspended.
>
> So instead of adding 3 new entries I've written a patch to
> simply disable the turning off of the clocks on all
> systems which have "Lex BayTrail" as their DMI sys_vendor:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/commit/?h=review-hans&id=c9d959fc32a5f9312282817052d8986614f2dc08
>
> I've added a Reported-by tag to give you credit for the work
> you have done on this.
>
> I will send this alternative fix to Linus as part of
> the other pdx86 patches for 5.21.

Thank you. Will your fix also appear in stable/lts kernels?

>
> Regards,
>
> Hans
>
>
>
>
> > ---
> > drivers/platform/x86/pmc_atom.c | 18 ++++++++++++++++++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git a/drivers/platform/x86/pmc_atom.c b/drivers/platform/x86/pmc_atom.c
> > index b8b1ed1406de..5dc82667907b 100644
> > --- a/drivers/platform/x86/pmc_atom.c
> > +++ b/drivers/platform/x86/pmc_atom.c
> > @@ -388,6 +388,24 @@ static const struct dmi_system_id critclk_systems[] = {
> > DMI_MATCH(DMI_PRODUCT_NAME, "CEC10 Family"),
> > },
> > },
> > + {
> > + /* pmc_plt_clk* - are used for ethernet controllers */
> > + .ident = "Lex 3I380A",
> > + .callback = dmi_callback,
> > + .matches = {
> > + DMI_MATCH(DMI_SYS_VENDOR, "Lex BayTrail"),
> > + DMI_MATCH(DMI_PRODUCT_NAME, "3I380A"),
> > + },
> > + },
> > + {
> > + /* pmc_plt_clk* - are used for ethernet controllers */
> > + .ident = "Lex 3I380CW",
> > + .callback = dmi_callback,
> > + .matches = {
> > + DMI_MATCH(DMI_SYS_VENDOR, "Lex BayTrail"),
> > + DMI_MATCH(DMI_PRODUCT_NAME, "3I380CW"),
> > + },
> > + },
> > {
> > /* pmc_plt_clk0 - 3 are used for the 4 ethernet controllers */
> > .ident = "Lex 3I380D",
>


--
With best regards,
Matwey V. Kornilov

2022-07-28 18:43:38

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH] platform/x86: pmc_atom: Add DMI quirk for Lex 3I380A/CW boards

Hi,

On 7/28/22 20:39, Matwey V. Kornilov wrote:
> чт, 28 июл. 2022 г. в 21:33, Hans de Goede <[email protected]>:
>>
>> Hi,
>>
>> On 7/27/22 17:32, Matwey V. Kornilov wrote:
>>> Lex 3I380A/CW (Atom E3845) motherboards are equipped with dual Intel I211
>>> based 1Gbps copper ethernet:
>>>
>>> http://www.lex.com.tw/products/pdf/3I380A&3I380CW.pdf
>>>
>>> This patch is to fix the issue with broken "LAN2" port. Before the
>>> patch, only one ethernet port is initialized:
>>>
>>> igb 0000:01:00.0: added PHC on eth0
>>> igb 0000:01:00.0: Intel(R) Gigabit Ethernet Network Connection
>>> igb 0000:01:00.0: eth0: (PCIe:2.5Gb/s:Width x1) 4c:02:89:10:02:e4
>>> igb 0000:01:00.0: eth0: PBA No: FFFFFF-0FF
>>> igb 0000:01:00.0: Using MSI-X interrupts. 2 rx queue(s), 2 tx queue(s)
>>> igb: probe of 0000:02:00.0 failed with error -2
>>>
>>> With this patch, both ethernet ports are available:
>>>
>>> igb 0000:01:00.0: added PHC on eth0
>>> igb 0000:01:00.0: Intel(R) Gigabit Ethernet Network Connection
>>> igb 0000:01:00.0: eth0: (PCIe:2.5Gb/s:Width x1) 4c:02:89:10:02:e4
>>> igb 0000:01:00.0: eth0: PBA No: FFFFFF-0FF
>>> igb 0000:01:00.0: Using MSI-X interrupts. 2 rx queue(s), 2 tx queue(s)
>>> igb 0000:02:00.0: added PHC on eth1
>>> igb 0000:02:00.0: Intel(R) Gigabit Ethernet Network Connection
>>> igb 0000:02:00.0: eth1: (PCIe:2.5Gb/s:Width x1) 4c:02:89:10:02:e5
>>> igb 0000:02:00.0: eth1: PBA No: FFFFFF-0FF
>>> igb 0000:02:00.0: Using MSI-X interrupts. 2 rx queue(s), 2 tx queue(s)
>>>
>>> The issue was observed at 3I380A board with BIOS version "A4 01/15/2016"
>>> and 3I380CW board with BIOS version "A3 09/29/2014".
>>>
>>> Reference: https://lore.kernel.org/netdev/[email protected]/
>>> Fixes: 648e921888ad ("clk: x86: Stop marking clocks as CLK_IS_CRITICAL")
>>> Cc: <[email protected]> # v4.19+
>>> Signed-off-by: Matwey V. Kornilov <[email protected]>
>>
>>
>> Thank you for the patch.
>>
>> The last week I have received 2 different patches adding
>> a total of 3 new "Lex BayTrail" entries to critclk_systems[]
>> on top of the existing 2.
>>
>> Looking at: https://www.lex.com.tw/products/embedded-ipc-board/
>> we can see that Lex BayTrail makes many embedded boards with
>> multiple ethernet boards and none of their products are battery
>> powered so we don't need to worry (too much) about power consumption
>> when suspended.
>>
>> So instead of adding 3 new entries I've written a patch to
>> simply disable the turning off of the clocks on all
>> systems which have "Lex BayTrail" as their DMI sys_vendor:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/commit/?h=review-hans&id=c9d959fc32a5f9312282817052d8986614f2dc08
>>
>> I've added a Reported-by tag to give you credit for the work
>> you have done on this.
>>
>> I will send this alternative fix to Linus as part of
>> the other pdx86 patches for 5.21.
>
> Thank you. Will your fix also appear in stable/lts kernels?

Yes it has the same Fixes tag as your patch did, this will
cause it to automatically get cherry-picked into kernels
which have the fixed commit hash.

Regards,

Hans


>>> ---
>>> drivers/platform/x86/pmc_atom.c | 18 ++++++++++++++++++
>>> 1 file changed, 18 insertions(+)
>>>
>>> diff --git a/drivers/platform/x86/pmc_atom.c b/drivers/platform/x86/pmc_atom.c
>>> index b8b1ed1406de..5dc82667907b 100644
>>> --- a/drivers/platform/x86/pmc_atom.c
>>> +++ b/drivers/platform/x86/pmc_atom.c
>>> @@ -388,6 +388,24 @@ static const struct dmi_system_id critclk_systems[] = {
>>> DMI_MATCH(DMI_PRODUCT_NAME, "CEC10 Family"),
>>> },
>>> },
>>> + {
>>> + /* pmc_plt_clk* - are used for ethernet controllers */
>>> + .ident = "Lex 3I380A",
>>> + .callback = dmi_callback,
>>> + .matches = {
>>> + DMI_MATCH(DMI_SYS_VENDOR, "Lex BayTrail"),
>>> + DMI_MATCH(DMI_PRODUCT_NAME, "3I380A"),
>>> + },
>>> + },
>>> + {
>>> + /* pmc_plt_clk* - are used for ethernet controllers */
>>> + .ident = "Lex 3I380CW",
>>> + .callback = dmi_callback,
>>> + .matches = {
>>> + DMI_MATCH(DMI_SYS_VENDOR, "Lex BayTrail"),
>>> + DMI_MATCH(DMI_PRODUCT_NAME, "3I380CW"),
>>> + },
>>> + },
>>> {
>>> /* pmc_plt_clk0 - 3 are used for the 4 ethernet controllers */
>>> .ident = "Lex 3I380D",
>>
>
>