* Re: [E1000-devel] igb driver with Intel Atom Bay Trail issue
[not found] ` <9B4A1B1917080E46B64F07F2989DADD69AF4DDE7@ORSMSX115.amr.corp.intel.com>
@ 2019-03-27 16:02 ` Hans de Goede
2019-03-27 16:47 ` Andy Shevchenko
0 siblings, 1 reply; 24+ messages in thread
From: Hans de Goede @ 2019-03-27 16:02 UTC (permalink / raw)
To: Fujinaka, Todd, Semyon Verchenko, Kirsher, Jeffrey T,
e1000-devel, Stephen Boyd, David Müller, linux-clk,
Michael Turquette, Stephen Boyd, Andy Shevchenko, linux-clk
Hi,
On 3/25/19 9:12 PM, Fujinaka, Todd wrote:
> This is going to take a bit of time to see what we need to do. The attachments were stripped but I think just figuring out what they had to change in the Realtek driver will tell us what we need to know.
I'm not sure fixing this in the ethernet driver is the best way to go,
the board in question is an embedded PC and I've also recently received
a bug report about a similar problem with the consumer not requesting
the pmc clocks causing a HSIC usb hub to not work.
I think that it might be better to restore the CLK_IS_CRITICAL workaround
for this, but then only on select boards, based on DMI matching.
I've added a bunch of relevant people / lists to the Cc.
Andy, Stephen, what is your take on this ?
I'm myself starting to believe the DMI based applying of the
CLK_IS_CRITICAL workaround is the best solution here.
Regards,
Hans
>
> Todd Fujinaka
> Software Application Engineer
> Datacenter Engineering Group
> Intel Corporation
> todd.fujinaka@intel.com
>
>
> -----Original Message-----
> From: Semyon Verchenko [mailto:semverchenko@factor-ts.ru]
> Sent: Monday, March 25, 2019 6:25 AM
> To: hdegoede@redhat.com; Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; e1000-devel@lists.sf.net
> Subject: [E1000-devel] igb driver with Intel Atom Bay Trail issue
>
> Dear Linux/igb maintainers,
>
> We've encountered problem with igb driver (both one that is distributed with Linux kernel and one which is downloadable from intel.com). After commit 648e921888ad ("clk: x86: Stop marking clocks as CLK_IS_CRITICAL") on machine with Intel(R) Atom(TM) CPU E3825 with 4 ethernet cards Intel
> I211 only one of cards probed correctly, other ones fail with error -2 (error -5 with driver from intel.com). I've rebuilt kernel with commit 648e921888ad reverted and all 4 interfaces had to probe correctly.
> Problem is reproducible at least with kernel 5.0.4 and kernels 4.14.y (actually firstly I've encountered this on 4.14.105 while it worked fine with 4.14.67 so I firstly started to build intermediate versions of kernel and found that problem started to appear on 4.14.77, but since it's reproducible in mainline kernel I think I should inform mainline kernel maintainers about this). Also it is reproducible with igb
> 5.3.5.22 from intel.com. I'm attaching kernel logs (journalctl.bad is from stock kernel and journalctl.badfix is from stock kernel with commit 648e921888ad reversed), lspci -vnn output (same about file names) and /proc/cpuinfo. The system is Arch Linux with kernel replaced to stock one (it is not Arch Linux-related problem as it appeared in another system with 4.14 kernel). I think something like Hans de Goede's r8169 patches is required for igb (and maybe other Intel drivers), but I'm not so good with driver developing so it seems too hard for me.
>
> Regards,
> Semyon Verchenko
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [E1000-devel] igb driver with Intel Atom Bay Trail issue
2019-03-27 16:02 ` [E1000-devel] igb driver with Intel Atom Bay Trail issue Hans de Goede
@ 2019-03-27 16:47 ` Andy Shevchenko
2019-03-27 17:31 ` David Müller
2019-03-28 14:48 ` Hans de Goede
0 siblings, 2 replies; 24+ messages in thread
From: Andy Shevchenko @ 2019-03-27 16:47 UTC (permalink / raw)
To: Hans de Goede
Cc: Fujinaka, Todd, Semyon Verchenko, Kirsher, Jeffrey T,
e1000-devel, Stephen Boyd, David Müller, linux-clk,
Michael Turquette, Stephen Boyd
On Wed, Mar 27, 2019 at 05:02:31PM +0100, Hans de Goede wrote:
> Hi,
>
> On 3/25/19 9:12 PM, Fujinaka, Todd wrote:
> > This is going to take a bit of time to see what we need to do. The attachments were stripped but I think just figuring out what they had to change in the Realtek driver will tell us what we need to know.
>
> I'm not sure fixing this in the ethernet driver is the best way to go,
> the board in question is an embedded PC and I've also recently received
> a bug report about a similar problem with the consumer not requesting
> the pmc clocks causing a HSIC usb hub to not work.
>
> I think that it might be better to restore the CLK_IS_CRITICAL workaround
> for this, but then only on select boards, based on DMI matching.
>
> I've added a bunch of relevant people / lists to the Cc.
>
> Andy, Stephen, what is your take on this ?
I'm afraid I forgot all details about that (semi-)famous issue. Though, looking
into your patch against r8169 and taking into account DT practice, it would be
not bad to fix a driver, we have by the way devm_clk_get_optional() now, so, it
would be not a big deal.
> I'm myself starting to believe the DMI based applying of the
> CLK_IS_CRITICAL workaround is the best solution here.
DMI quirk table tends to grow in mysterious ways. I would prefer in this case
logical solution — if platform has an optional clock, then use it.
>
> Regards,
>
> Hans
>
>
> >
> > Todd Fujinaka
> > Software Application Engineer
> > Datacenter Engineering Group
> > Intel Corporation
> > todd.fujinaka@intel.com
> >
> >
> > -----Original Message-----
> > From: Semyon Verchenko [mailto:semverchenko@factor-ts.ru]
> > Sent: Monday, March 25, 2019 6:25 AM
> > To: hdegoede@redhat.com; Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; e1000-devel@lists.sf.net
> > Subject: [E1000-devel] igb driver with Intel Atom Bay Trail issue
> >
> > Dear Linux/igb maintainers,
> >
> > We've encountered problem with igb driver (both one that is distributed with Linux kernel and one which is downloadable from intel.com). After commit 648e921888ad ("clk: x86: Stop marking clocks as CLK_IS_CRITICAL") on machine with Intel(R) Atom(TM) CPU E3825 with 4 ethernet cards Intel
> > I211 only one of cards probed correctly, other ones fail with error -2 (error -5 with driver from intel.com). I've rebuilt kernel with commit 648e921888ad reverted and all 4 interfaces had to probe correctly.
> > Problem is reproducible at least with kernel 5.0.4 and kernels 4.14.y (actually firstly I've encountered this on 4.14.105 while it worked fine with 4.14.67 so I firstly started to build intermediate versions of kernel and found that problem started to appear on 4.14.77, but since it's reproducible in mainline kernel I think I should inform mainline kernel maintainers about this). Also it is reproducible with igb
> > 5.3.5.22 from intel.com. I'm attaching kernel logs (journalctl.bad is from stock kernel and journalctl.badfix is from stock kernel with commit 648e921888ad reversed), lspci -vnn output (same about file names) and /proc/cpuinfo. The system is Arch Linux with kernel replaced to stock one (it is not Arch Linux-related problem as it appeared in another system with 4.14 kernel). I think something like Hans de Goede's r8169 patches is required for igb (and maybe other Intel drivers), but I'm not so good with driver developing so it seems too hard for me.
> >
> > Regards,
> > Semyon Verchenko
> >
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [E1000-devel] igb driver with Intel Atom Bay Trail issue
2019-03-27 16:47 ` Andy Shevchenko
@ 2019-03-27 17:31 ` David Müller
2019-03-27 19:19 ` Andy Shevchenko
2019-03-28 14:48 ` Hans de Goede
1 sibling, 1 reply; 24+ messages in thread
From: David Müller @ 2019-03-27 17:31 UTC (permalink / raw)
To: Andy Shevchenko, Hans de Goede
Cc: Fujinaka, Todd, Kirsher, Jeffrey T, e1000-devel, Stephen Boyd,
linux-clk, Michael Turquette, Stephen Boyd
Hello
Andy Shevchenko wrote:
> On Wed, Mar 27, 2019 at 05:02:31PM +0100, Hans de Goede wrote:
>> I think that it might be better to restore the CLK_IS_CRITICAL workaround
>> for this, but then only on select boards, based on DMI matching.
>>
>> I've added a bunch of relevant people / lists to the Cc.
>>
>> Andy, Stephen, what is your take on this ?
>
> I'm afraid I forgot all details about that (semi-)famous issue. Though, looking
> into your patch against r8169 and taking into account DT practice, it would be
> not bad to fix a driver, we have by the way devm_clk_get_optional() now, so, it
> would be not a big deal.
>
>> I'm myself starting to believe the DMI based applying of the
>> CLK_IS_CRITICAL workaround is the best solution here.
>
> DMI quirk table tends to grow in mysterious ways. I would prefer in this case
> logical solution — if platform has an optional clock, then use it.
The pmc_plt_clks may also be used for external hardware purposes without
the need for a driver involved. So I'm afraid a fix similar to the r8169
approach will not suit all needs. Please see
https://www.spinics.net/lists/linux-clk/msg35800.html for details.
I'm in the final phase of testing a DMI based patch on my hardware.
If all goes well, I could provide a first version of the patch for
review by tomorrow.
Dave
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [E1000-devel] igb driver with Intel Atom Bay Trail issue
2019-03-27 17:31 ` David Müller
@ 2019-03-27 19:19 ` Andy Shevchenko
2019-03-28 14:35 ` David Müller
0 siblings, 1 reply; 24+ messages in thread
From: Andy Shevchenko @ 2019-03-27 19:19 UTC (permalink / raw)
To: David Müller
Cc: Hans de Goede, Fujinaka, Todd, Kirsher, Jeffrey T, e1000-devel,
Stephen Boyd, linux-clk, Michael Turquette, Stephen Boyd
On Wed, Mar 27, 2019 at 06:31:19PM +0100, David Müller wrote:
> Andy Shevchenko wrote:
> > On Wed, Mar 27, 2019 at 05:02:31PM +0100, Hans de Goede wrote:
> > > I think that it might be better to restore the CLK_IS_CRITICAL workaround
> > > for this, but then only on select boards, based on DMI matching.
> > >
> > > I've added a bunch of relevant people / lists to the Cc.
> > >
> > > Andy, Stephen, what is your take on this ?
> >
> > I'm afraid I forgot all details about that (semi-)famous issue. Though, looking
> > into your patch against r8169 and taking into account DT practice, it would be
> > not bad to fix a driver, we have by the way devm_clk_get_optional() now, so, it
> > would be not a big deal.
> >
> > > I'm myself starting to believe the DMI based applying of the
> > > CLK_IS_CRITICAL workaround is the best solution here.
> >
> > DMI quirk table tends to grow in mysterious ways. I would prefer in this case
> > logical solution — if platform has an optional clock, then use it.
>
> The pmc_plt_clks may also be used for external hardware purposes without
> the need for a driver involved. So I'm afraid a fix similar to the r8169
> approach will not suit all needs. Please see
> https://www.spinics.net/lists/linux-clk/msg35800.html for details.
Any driver for device which is using PMC clock should take it into
consideration.
I don't see any issues with patching devices drivers based on case-by-case
approach.
Is there any other impediment that prevents us to update the driver in
question?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [E1000-devel] igb driver with Intel Atom Bay Trail issue
2019-03-27 19:19 ` Andy Shevchenko
@ 2019-03-28 14:35 ` David Müller
2019-03-28 14:58 ` Andy Shevchenko
0 siblings, 1 reply; 24+ messages in thread
From: David Müller @ 2019-03-28 14:35 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Hans de Goede, Fujinaka, Todd, Kirsher, Jeffrey T, e1000-devel,
Stephen Boyd, linux-clk, Michael Turquette, Stephen Boyd
Hello
Andy Shevchenko wrote:
> On Wed, Mar 27, 2019 at 06:31:19PM +0100, David Müller wrote:
>> The pmc_plt_clks may also be used for external hardware purposes without
>> the need for a driver involved. So I'm afraid a fix similar to the r8169
>> approach will not suit all needs. Please see
>> https://www.spinics.net/lists/linux-clk/msg35800.html for details.
>
> Any driver for device which is using PMC clock should take it into
> consideration.
I agree that each driver should properly request the clocks and other
resources needed.
But what if the PMC clock is used by hardware for which no driver is
being loaded or needed?
Dave
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [E1000-devel] igb driver with Intel Atom Bay Trail issue
2019-03-27 16:47 ` Andy Shevchenko
2019-03-27 17:31 ` David Müller
@ 2019-03-28 14:48 ` Hans de Goede
1 sibling, 0 replies; 24+ messages in thread
From: Hans de Goede @ 2019-03-28 14:48 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Fujinaka, Todd, Semyon Verchenko, Kirsher, Jeffrey T,
e1000-devel, Stephen Boyd, David Müller, linux-clk,
Michael Turquette, Stephen Boyd
Hi,
On 27-03-19 17:47, Andy Shevchenko wrote:
> On Wed, Mar 27, 2019 at 05:02:31PM +0100, Hans de Goede wrote:
>> Hi,
>>
>> On 3/25/19 9:12 PM, Fujinaka, Todd wrote:
>>> This is going to take a bit of time to see what we need to do. The attachments were stripped but I think just figuring out what they had to change in the Realtek driver will tell us what we need to know.
>>
>> I'm not sure fixing this in the ethernet driver is the best way to go,
>> the board in question is an embedded PC and I've also recently received
>> a bug report about a similar problem with the consumer not requesting
>> the pmc clocks causing a HSIC usb hub to not work.
>>
>> I think that it might be better to restore the CLK_IS_CRITICAL workaround
>> for this, but then only on select boards, based on DMI matching.
>>
>> I've added a bunch of relevant people / lists to the Cc.
>>
>> Andy, Stephen, what is your take on this ?
>
> I'm afraid I forgot all details about that (semi-)famous issue. Though, looking
> into your patch against r8169 and taking into account DT practice, it would be
> not bad to fix a driver, we have by the way devm_clk_get_optional() now, so, it
> would be not a big deal.
This assumes that the machine with the igb ethernet problem is using
the same pmc clk as the other device I fixed and that it is using one
and the same clock for all 4 of its ethernet controllers.
IMHO the not controlling of the clock as an ACPI resource on Bay Trail
models as is normal on x86 really is a firmware bug (shared by the
entire Bay Trail generation). I'm not in favor of adding workarounds to
drivers all over the place because of this.
And in case of the problem with David Müller's system, the problem
is that the clock is needed for an external USB hub connected via
HSIC. The USB subsystem assumes that HUBs will just work without
needing to request resources for them and in this case there really
is no good place to do the devm_clk_get_optional().
So I believe that we are going to need the DMI based approach for
David's case anyway, at which point we might just as well also fix
the igb problem that way, at least until more boards using the igb
driver and showing the same problem pop up.
The reasons I patched the r8169 driver for this are:
1) In principle I agree that this is the right thing to do
(but see above)
2) I expect more Cherry Trail based devices (yes this is a Cherry
Trail device, so here it really really is a firmware bug) to
use the r8169 (cheap SoC + cheap ethernet chip)
3) This is a laptop where being able to reach S0i3 is important,
which the CLK_IS_CRITICAL workaround will break
>> I'm myself starting to believe the DMI based applying of the
>> CLK_IS_CRITICAL workaround is the best solution here.
>
> DMI quirk table tends to grow in mysterious ways. I would prefer in this case
> logical solution — if platform has an optional clock, then use it.
I'm afraid that otherwise instead of a growing DMI table in a
single place we will get extra coded added for niche cases to
a bunch of different drivers, in which case I prefer the "central"
DMI quirk database.
Regards,
Hans
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [E1000-devel] igb driver with Intel Atom Bay Trail issue
2019-03-28 14:35 ` David Müller
@ 2019-03-28 14:58 ` Andy Shevchenko
2019-03-28 15:01 ` Hans de Goede
0 siblings, 1 reply; 24+ messages in thread
From: Andy Shevchenko @ 2019-03-28 14:58 UTC (permalink / raw)
To: David Müller
Cc: Hans de Goede, Fujinaka, Todd, Kirsher, Jeffrey T, e1000-devel,
Stephen Boyd, linux-clk, Michael Turquette, Stephen Boyd
On Thu, Mar 28, 2019 at 03:35:58PM +0100, David Müller wrote:
> Andy Shevchenko wrote:
> > On Wed, Mar 27, 2019 at 06:31:19PM +0100, David Müller wrote:
> >> The pmc_plt_clks may also be used for external hardware purposes without
> >> the need for a driver involved. So I'm afraid a fix similar to the r8169
> >> approach will not suit all needs. Please see
> >> https://www.spinics.net/lists/linux-clk/msg35800.html for details.
> >
> > Any driver for device which is using PMC clock should take it into
> > consideration.
>
> I agree that each driver should properly request the clocks and other
> resources needed.
>
> But what if the PMC clock is used by hardware for which no driver is
> being loaded or needed?
Perhaps I didn't get a full picture.
In the original message you referred to igb _driver_ and devices that are not
working properly after resume.
The igb driver patching can fix that, right?
If the driver is not loaded, then the clock is not in use and can be gated,
correct?
If there is a connection of this clock to the hardware which is served by
firmware, then its firmware's deal, no?
Can you elaborate a bit more the case you are talking about?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [E1000-devel] igb driver with Intel Atom Bay Trail issue
2019-03-28 14:58 ` Andy Shevchenko
@ 2019-03-28 15:01 ` Hans de Goede
2019-03-28 15:24 ` Andy Shevchenko
0 siblings, 1 reply; 24+ messages in thread
From: Hans de Goede @ 2019-03-28 15:01 UTC (permalink / raw)
To: Andy Shevchenko, David Müller
Cc: Fujinaka, Todd, Kirsher, Jeffrey T, e1000-devel, Stephen Boyd,
linux-clk, Michael Turquette, Stephen Boyd
Hi Andy,
On 28-03-19 15:58, Andy Shevchenko wrote:
> On Thu, Mar 28, 2019 at 03:35:58PM +0100, David Müller wrote:
>> Andy Shevchenko wrote:
>>> On Wed, Mar 27, 2019 at 06:31:19PM +0100, David Müller wrote:
>>>> The pmc_plt_clks may also be used for external hardware purposes without
>>>> the need for a driver involved. So I'm afraid a fix similar to the r8169
>>>> approach will not suit all needs. Please see
>>>> https://www.spinics.net/lists/linux-clk/msg35800.html for details.
>>>
>>> Any driver for device which is using PMC clock should take it into
>>> consideration.
>>
>> I agree that each driver should properly request the clocks and other
>> resources needed.
>>
>> But what if the PMC clock is used by hardware for which no driver is
>> being loaded or needed?
>
> Perhaps I didn't get a full picture.
>
> In the original message you referred to igb _driver_ and devices that are not
> working properly after resume.
>
> The igb driver patching can fix that, right?
>
> If the driver is not loaded, then the clock is not in use and can be gated,
> correct?
>
> If there is a connection of this clock to the hardware which is served by
> firmware, then its firmware's deal, no?
>
> Can you elaborate a bit more the case you are talking about?
In David's case we are talking about a USB hub which needs a pmc-clk
(yes really). Also see me other mail I send about this about 5 minutes
ago.
David's case is the reason why I believe we need a DMI quirk table; and
once we have that I think the board with igb ethernet controllers might
just as well be handled the same way (I already checked it has usable
DMI identifying info).
Regards,
Hans
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [E1000-devel] igb driver with Intel Atom Bay Trail issue
2019-03-28 15:01 ` Hans de Goede
@ 2019-03-28 15:24 ` Andy Shevchenko
2019-03-28 15:32 ` Hans de Goede
0 siblings, 1 reply; 24+ messages in thread
From: Andy Shevchenko @ 2019-03-28 15:24 UTC (permalink / raw)
To: Hans de Goede
Cc: David Müller, Fujinaka, Todd, Kirsher, Jeffrey T,
e1000-devel, Stephen Boyd, linux-clk, Michael Turquette,
Stephen Boyd
On Thu, Mar 28, 2019 at 04:01:37PM +0100, Hans de Goede wrote:
> On 28-03-19 15:58, Andy Shevchenko wrote:
> > On Thu, Mar 28, 2019 at 03:35:58PM +0100, David Müller wrote:
> > > Andy Shevchenko wrote:
> > > > On Wed, Mar 27, 2019 at 06:31:19PM +0100, David Müller wrote:
> > > > > The pmc_plt_clks may also be used for external hardware purposes without
> > > > > the need for a driver involved. So I'm afraid a fix similar to the r8169
> > > > > approach will not suit all needs. Please see
> > > > > https://www.spinics.net/lists/linux-clk/msg35800.html for details.
> > > >
> > > > Any driver for device which is using PMC clock should take it into
> > > > consideration.
> > >
> > > I agree that each driver should properly request the clocks and other
> > > resources needed.
> > >
> > > But what if the PMC clock is used by hardware for which no driver is
> > > being loaded or needed?
> >
> > Perhaps I didn't get a full picture.
> >
> > In the original message you referred to igb _driver_ and devices that are not
> > working properly after resume.
> >
> > The igb driver patching can fix that, right?
> >
> > If the driver is not loaded, then the clock is not in use and can be gated,
> > correct?
> >
> > If there is a connection of this clock to the hardware which is served by
> > firmware, then its firmware's deal, no?
> >
> > Can you elaborate a bit more the case you are talking about?
>
> In David's case we are talking about a USB hub which needs a pmc-clk
> (yes really).
OK, this is understandable (what a weird HW design)...
> Also see me other mail I send about this about 5 minutes
> ago.
>
> David's case is the reason why I believe we need a DMI quirk table; and
> once we have that I think the board with igb ethernet controllers might
> just as well be handled the same way (I already checked it has usable
> DMI identifying info).
But am I right that in the case of igb we will loose power at suspend? Wouldn't
be better to patch the driver?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [E1000-devel] igb driver with Intel Atom Bay Trail issue
2019-03-28 15:24 ` Andy Shevchenko
@ 2019-03-28 15:32 ` Hans de Goede
2019-03-28 15:49 ` Andy Shevchenko
0 siblings, 1 reply; 24+ messages in thread
From: Hans de Goede @ 2019-03-28 15:32 UTC (permalink / raw)
To: Andy Shevchenko
Cc: David Müller, Fujinaka, Todd, Kirsher, Jeffrey T,
e1000-devel, Stephen Boyd, linux-clk, Michael Turquette,
Stephen Boyd
Hi,
On 28-03-19 16:24, Andy Shevchenko wrote:
> On Thu, Mar 28, 2019 at 04:01:37PM +0100, Hans de Goede wrote:
>> On 28-03-19 15:58, Andy Shevchenko wrote:
>>> On Thu, Mar 28, 2019 at 03:35:58PM +0100, David Müller wrote:
>>>> Andy Shevchenko wrote:
>>>>> On Wed, Mar 27, 2019 at 06:31:19PM +0100, David Müller wrote:
>>>>>> The pmc_plt_clks may also be used for external hardware purposes without
>>>>>> the need for a driver involved. So I'm afraid a fix similar to the r8169
>>>>>> approach will not suit all needs. Please see
>>>>>> https://www.spinics.net/lists/linux-clk/msg35800.html for details.
>>>>>
>>>>> Any driver for device which is using PMC clock should take it into
>>>>> consideration.
>>>>
>>>> I agree that each driver should properly request the clocks and other
>>>> resources needed.
>>>>
>>>> But what if the PMC clock is used by hardware for which no driver is
>>>> being loaded or needed?
>>>
>>> Perhaps I didn't get a full picture.
>>>
>>> In the original message you referred to igb _driver_ and devices that are not
>>> working properly after resume.
>>>
>>> The igb driver patching can fix that, right?
>>>
>>> If the driver is not loaded, then the clock is not in use and can be gated,
>>> correct?
>>>
>>> If there is a connection of this clock to the hardware which is served by
>>> firmware, then its firmware's deal, no?
>>>
>>> Can you elaborate a bit more the case you are talking about?
>>
>> In David's case we are talking about a USB hub which needs a pmc-clk
>> (yes really).
>
> OK, this is understandable (what a weird HW design)...
>
>> Also see me other mail I send about this about 5 minutes
>> ago.
>>
>> David's case is the reason why I believe we need a DMI quirk table; and
>> once we have that I think the board with igb ethernet controllers might
>> just as well be handled the same way (I already checked it has usable
>> DMI identifying info).
>
> But am I right that in the case of igb we will loose power at suspend? Wouldn't
> be better to patch the driver?
This is an industrial embedded PC, so it is not running on battery and
I doubt it typically spends a lot of time in suspend at all.
Regards,
Hans
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [E1000-devel] igb driver with Intel Atom Bay Trail issue
2019-03-28 15:32 ` Hans de Goede
@ 2019-03-28 15:49 ` Andy Shevchenko
2019-03-29 4:46 ` Hisashi T Fujinaka
2019-03-29 12:30 ` Hans de Goede
0 siblings, 2 replies; 24+ messages in thread
From: Andy Shevchenko @ 2019-03-28 15:49 UTC (permalink / raw)
To: Hans de Goede
Cc: David Müller, Fujinaka, Todd, Kirsher, Jeffrey T,
e1000-devel, Stephen Boyd, linux-clk, Michael Turquette,
Stephen Boyd
On Thu, Mar 28, 2019 at 04:32:27PM +0100, Hans de Goede wrote:
> On 28-03-19 16:24, Andy Shevchenko wrote:
> > On Thu, Mar 28, 2019 at 04:01:37PM +0100, Hans de Goede wrote:
> > > On 28-03-19 15:58, Andy Shevchenko wrote:
> > > > On Thu, Mar 28, 2019 at 03:35:58PM +0100, David Müller wrote:
> > > > > Andy Shevchenko wrote:
> > > > > > On Wed, Mar 27, 2019 at 06:31:19PM +0100, David Müller wrote:
> > > > > > Any driver for device which is using PMC clock should take it into
> > > > > > consideration.
> > > > >
> > > > > I agree that each driver should properly request the clocks and other
> > > > > resources needed.
> > > > Can you elaborate a bit more the case you are talking about?
> > > I think the board with igb ethernet controllers might
> > > just as well be handled the same way (I already checked it has usable
> > > DMI identifying info).
> >
> > But am I right that in the case of igb we will loose power at suspend? Wouldn't
> > be better to patch the driver?
>
> This is an industrial embedded PC, so it is not running on battery and
> I doubt it typically spends a lot of time in suspend at all.
Okay, but still from logical point of view wouldn't be better to fix the driver
for such case? At least I see benefits out of this approach: a) less hackish,
less quirk code; b) if this happens on non-industrial case it would be better
to have in the driver due to power consumption.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [E1000-devel] igb driver with Intel Atom Bay Trail issue
2019-03-28 15:49 ` Andy Shevchenko
@ 2019-03-29 4:46 ` Hisashi T Fujinaka
2019-03-29 12:32 ` Hans de Goede
2019-03-29 12:30 ` Hans de Goede
1 sibling, 1 reply; 24+ messages in thread
From: Hisashi T Fujinaka @ 2019-03-29 4:46 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Hans de Goede, Stephen Boyd, Michael Turquette, Stephen Boyd,
e1000-devel, linux-clk
On Thu, 28 Mar 2019, Andy Shevchenko wrote:
> On Thu, Mar 28, 2019 at 04:32:27PM +0100, Hans de Goede wrote:
>> On 28-03-19 16:24, Andy Shevchenko wrote:
>>> On Thu, Mar 28, 2019 at 04:01:37PM +0100, Hans de Goede wrote:
>>>> On 28-03-19 15:58, Andy Shevchenko wrote:
>>>>> On Thu, Mar 28, 2019 at 03:35:58PM +0100, David M?ller wrote:
>>>>>> Andy Shevchenko wrote:
>>>>>>> On Wed, Mar 27, 2019 at 06:31:19PM +0100, David M?ller wrote:
>
>>>>>>> Any driver for device which is using PMC clock should take it into
>>>>>>> consideration.
>>>>>>
>>>>>> I agree that each driver should properly request the clocks and other
>>>>>> resources needed.
>
>>>>> Can you elaborate a bit more the case you are talking about?
>
>>>> I think the board with igb ethernet controllers might
>>>> just as well be handled the same way (I already checked it has usable
>>>> DMI identifying info).
>>>
>>> But am I right that in the case of igb we will loose power at suspend? Wouldn't
>>> be better to patch the driver?
>>
>> This is an industrial embedded PC, so it is not running on battery and
>> I doubt it typically spends a lot of time in suspend at all.
>
> Okay, but still from logical point of view wouldn't be better to fix the driver
> for such case? At least I see benefits out of this approach: a) less hackish,
> less quirk code; b) if this happens on non-industrial case it would be better
> to have in the driver due to power consumption.
Sorry for replying from home. It's either that or top-posting with
Outlook.
It sounds to me like Andriy's arguments are counter to what he suggests
since we'd have to fix the USB and the Ethernet and that would add more
special case code, right? However, I'm just the old igb bug fixer and
not a clock guy, and I've spent little time outside of the Ethernet
drivers.
It really sounded like there were going to be changes to the clock code
that would resolve the issue. Were those done? Do I still need to change
the igb driver to change the clocks used?
--
Todd Fujinaka <todd.fujinaka@intel.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [E1000-devel] igb driver with Intel Atom Bay Trail issue
2019-03-28 15:49 ` Andy Shevchenko
2019-03-29 4:46 ` Hisashi T Fujinaka
@ 2019-03-29 12:30 ` Hans de Goede
2019-03-29 13:59 ` Семен Верченко
1 sibling, 1 reply; 24+ messages in thread
From: Hans de Goede @ 2019-03-29 12:30 UTC (permalink / raw)
To: Andy Shevchenko
Cc: David Müller, Fujinaka, Todd, Kirsher, Jeffrey T,
e1000-devel, Stephen Boyd, linux-clk, Michael Turquette,
Stephen Boyd, Semyon Verchenko
Hi,
On 3/28/19 4:49 PM, Andy Shevchenko wrote:
> On Thu, Mar 28, 2019 at 04:32:27PM +0100, Hans de Goede wrote:
>> On 28-03-19 16:24, Andy Shevchenko wrote:
>>> On Thu, Mar 28, 2019 at 04:01:37PM +0100, Hans de Goede wrote:
>>>> On 28-03-19 15:58, Andy Shevchenko wrote:
>>>>> On Thu, Mar 28, 2019 at 03:35:58PM +0100, David Müller wrote:
>>>>>> Andy Shevchenko wrote:
>>>>>>> On Wed, Mar 27, 2019 at 06:31:19PM +0100, David Müller wrote:
>
>>>>>>> Any driver for device which is using PMC clock should take it into
>>>>>>> consideration.
>>>>>>
>>>>>> I agree that each driver should properly request the clocks and other
>>>>>> resources needed.
>
>>>>> Can you elaborate a bit more the case you are talking about?
>
>>>> I think the board with igb ethernet controllers might
>>>> just as well be handled the same way (I already checked it has usable
>>>> DMI identifying info).
>>>
>>> But am I right that in the case of igb we will loose power at suspend? Wouldn't
>>> be better to patch the driver?
>>
>> This is an industrial embedded PC, so it is not running on battery and
>> I doubt it typically spends a lot of time in suspend at all.
>
> Okay, but still from logical point of view wouldn't be better to fix the driver
> for such case? At least I see benefits out of this approach: a) less hackish,
> less quirk code; b) if this happens on non-industrial case it would be better
> to have in the driver due to power consumption.
Maybe, I guess we first need to figure out which platforms clock(s) is (are)
being used, if there is more then one; or it is a different one then in the
realtek ethernet case it might be better to go with the dmi quirk option.
Semyon Verchenko can you (as root) run the following command on a kernel
where the ethernet does work:
grep . /sys/kernel/debug/clk/pmc_plt_clk_?/flags
And then email us the output please?
Regards,
Hans
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [E1000-devel] igb driver with Intel Atom Bay Trail issue
2019-03-29 4:46 ` Hisashi T Fujinaka
@ 2019-03-29 12:32 ` Hans de Goede
0 siblings, 0 replies; 24+ messages in thread
From: Hans de Goede @ 2019-03-29 12:32 UTC (permalink / raw)
To: todd.fujinaka, Andy Shevchenko
Cc: Stephen Boyd, Michael Turquette, Stephen Boyd, e1000-devel, linux-clk
Hi Hisashi,
On 3/29/19 5:46 AM, Hisashi T Fujinaka wrote:
> On Thu, 28 Mar 2019, Andy Shevchenko wrote:
>
>> On Thu, Mar 28, 2019 at 04:32:27PM +0100, Hans de Goede wrote:
>>> On 28-03-19 16:24, Andy Shevchenko wrote:
>>>> On Thu, Mar 28, 2019 at 04:01:37PM +0100, Hans de Goede wrote:
>>>>> On 28-03-19 15:58, Andy Shevchenko wrote:
>>>>>> On Thu, Mar 28, 2019 at 03:35:58PM +0100, David M?ller wrote:
>>>>>>> Andy Shevchenko wrote:
>>>>>>>> On Wed, Mar 27, 2019 at 06:31:19PM +0100, David M?ller wrote:
>>
>>>>>>>> Any driver for device which is using PMC clock should take it into
>>>>>>>> consideration.
>>>>>>>
>>>>>>> I agree that each driver should properly request the clocks and other
>>>>>>> resources needed.
>>
>>>>>> Can you elaborate a bit more the case you are talking about?
>>
>>>>> I think the board with igb ethernet controllers might
>>>>> just as well be handled the same way (I already checked it has usable
>>>>> DMI identifying info).
>>>>
>>>> But am I right that in the case of igb we will loose power at suspend? Wouldn't
>>>> be better to patch the driver?
>>>
>>> This is an industrial embedded PC, so it is not running on battery and
>>> I doubt it typically spends a lot of time in suspend at all.
>>
>> Okay, but still from logical point of view wouldn't be better to fix the driver
>> for such case? At least I see benefits out of this approach: a) less hackish,
>> less quirk code; b) if this happens on non-industrial case it would be better
>> to have in the driver due to power consumption.
>
> Sorry for replying from home. It's either that or top-posting with
> Outlook.
>
> It sounds to me like Andriy's arguments are counter to what he suggests
> since we'd have to fix the USB and the Ethernet and that would add more
> special case code, right? However, I'm just the old igb bug fixer and
> not a clock guy, and I've spent little time outside of the Ethernet
> drivers.
>
> It really sounded like there were going to be changes to the clock code
> that would resolve the issue. Were those done? Do I still need to change
> the igb driver to change the clocks used?
Andy and I are still discussing this, we will get back to you when
we've a conclusion on how best to fix this.
Regards,
Hans
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [E1000-devel] igb driver with Intel Atom Bay Trail issue
2019-03-29 12:30 ` Hans de Goede
@ 2019-03-29 13:59 ` Семен Верченко
2019-03-29 15:53 ` Hans de Goede
0 siblings, 1 reply; 24+ messages in thread
From: Семен Верченко @ 2019-03-29 13:59 UTC (permalink / raw)
To: Hans de Goede, Andy Shevchenko
Cc: David Müller, Fujinaka, Todd, Kirsher, Jeffrey T,
e1000-devel, Stephen Boyd, linux-clk, Michael Turquette,
Stephen Boyd
On 29.03.2019 15:30, Hans de Goede wrote:
> Hi,
>
> On 3/28/19 4:49 PM, Andy Shevchenko wrote:
>> On Thu, Mar 28, 2019 at 04:32:27PM +0100, Hans de Goede wrote:
>>> On 28-03-19 16:24, Andy Shevchenko wrote:
>>>> On Thu, Mar 28, 2019 at 04:01:37PM +0100, Hans de Goede wrote:
>>>>> On 28-03-19 15:58, Andy Shevchenko wrote:
>>>>>> On Thu, Mar 28, 2019 at 03:35:58PM +0100, David Müller wrote:
>>>>>>> Andy Shevchenko wrote:
>>>>>>>> On Wed, Mar 27, 2019 at 06:31:19PM +0100, David Müller wrote:
>>
>>>>>>>> Any driver for device which is using PMC clock should take it into
>>>>>>>> consideration.
>>>>>>>
>>>>>>> I agree that each driver should properly request the clocks and
>>>>>>> other
>>>>>>> resources needed.
>>
>>>>>> Can you elaborate a bit more the case you are talking about?
>>
>>>>> I think the board with igb ethernet controllers might
>>>>> just as well be handled the same way (I already checked it has usable
>>>>> DMI identifying info).
>>>>
>>>> But am I right that in the case of igb we will loose power at
>>>> suspend? Wouldn't
>>>> be better to patch the driver?
>>>
>>> This is an industrial embedded PC, so it is not running on battery and
>>> I doubt it typically spends a lot of time in suspend at all.
>>
>> Okay, but still from logical point of view wouldn't be better to fix
>> the driver
>> for such case? At least I see benefits out of this approach: a) less
>> hackish,
>> less quirk code; b) if this happens on non-industrial case it would
>> be better
>> to have in the driver due to power consumption.
>
> Maybe, I guess we first need to figure out which platforms clock(s) is
> (are)
> being used, if there is more then one; or it is a different one then
> in the
> realtek ethernet case it might be better to go with the dmi quirk option.
>
> Semyon Verchenko can you (as root) run the following command on a kernel
> where the ethernet does work:
>
> grep . /sys/kernel/debug/clk/pmc_plt_clk_?/flags
>
> And then email us the output please?
>
> Regards,
>
> Hans
>
I don't have flags files in /sys/kernel/debug/clk/pmc_plt_clk_?; did you
mean clk_flags?
[root@archatom ~]# grep . /sys/kernel/debug/clk/pmc_plt_clk_?/flags
grep: /sys/kernel/debug/clk/pmc_plt_clk_?/flags: No such file or directory
[root@archatom ~]# grep . /sys/kernel/debug/clk/pmc_plt_clk_?/clk_flags
/sys/kernel/debug/clk/pmc_plt_clk_0/clk_flags:CLK_IS_CRITICAL
/sys/kernel/debug/clk/pmc_plt_clk_1/clk_flags:CLK_IS_CRITICAL
/sys/kernel/debug/clk/pmc_plt_clk_2/clk_flags:CLK_IS_CRITICAL
/sys/kernel/debug/clk/pmc_plt_clk_3/clk_flags:CLK_IS_CRITICAL
[root@archatom ~]# ls /sys/kernel/debug/clk
clk_dump clk_orphan_summary pll pmc_plt_clk_1
pmc_plt_clk_3 pmc_plt_clk_5
clk_orphan_dump clk_summary pmc_plt_clk_0 pmc_plt_clk_2
pmc_plt_clk_4 xtal
Kernel is 5.0.4 with commit 648e921888ad ("clk: x86: Stop marking clocks
as CLK_IS_CRITICAL") reversed, is it enough or I need to install older
kernel?
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [E1000-devel] igb driver with Intel Atom Bay Trail issue
2019-03-29 13:59 ` Семен Верченко
@ 2019-03-29 15:53 ` Hans de Goede
2019-04-04 14:43 ` Hans de Goede
0 siblings, 1 reply; 24+ messages in thread
From: Hans de Goede @ 2019-03-29 15:53 UTC (permalink / raw)
To: Семен
Верченко,
Andy Shevchenko
Cc: David Müller, Fujinaka, Todd, Kirsher, Jeffrey T,
e1000-devel, Stephen Boyd, linux-clk, Michael Turquette,
Stephen Boyd
Hi,
On 3/29/19 2:59 PM, Семен Верченко wrote:
>
> On 29.03.2019 15:30, Hans de Goede wrote:
>> Hi,
>>
>> On 3/28/19 4:49 PM, Andy Shevchenko wrote:
>>> On Thu, Mar 28, 2019 at 04:32:27PM +0100, Hans de Goede wrote:
>>>> On 28-03-19 16:24, Andy Shevchenko wrote:
>>>>> On Thu, Mar 28, 2019 at 04:01:37PM +0100, Hans de Goede wrote:
>>>>>> On 28-03-19 15:58, Andy Shevchenko wrote:
>>>>>>> On Thu, Mar 28, 2019 at 03:35:58PM +0100, David Müller wrote:
>>>>>>>> Andy Shevchenko wrote:
>>>>>>>>> On Wed, Mar 27, 2019 at 06:31:19PM +0100, David Müller wrote:
>>>
>>>>>>>>> Any driver for device which is using PMC clock should take it into
>>>>>>>>> consideration.
>>>>>>>>
>>>>>>>> I agree that each driver should properly request the clocks and other
>>>>>>>> resources needed.
>>>
>>>>>>> Can you elaborate a bit more the case you are talking about?
>>>
>>>>>> I think the board with igb ethernet controllers might
>>>>>> just as well be handled the same way (I already checked it has usable
>>>>>> DMI identifying info).
>>>>>
>>>>> But am I right that in the case of igb we will loose power at suspend? Wouldn't
>>>>> be better to patch the driver?
>>>>
>>>> This is an industrial embedded PC, so it is not running on battery and
>>>> I doubt it typically spends a lot of time in suspend at all.
>>>
>>> Okay, but still from logical point of view wouldn't be better to fix the driver
>>> for such case? At least I see benefits out of this approach: a) less hackish,
>>> less quirk code; b) if this happens on non-industrial case it would be better
>>> to have in the driver due to power consumption.
>>
>> Maybe, I guess we first need to figure out which platforms clock(s) is (are)
>> being used, if there is more then one; or it is a different one then in the
>> realtek ethernet case it might be better to go with the dmi quirk option.
>>
>> Semyon Verchenko can you (as root) run the following command on a kernel
>> where the ethernet does work:
>>
>> grep . /sys/kernel/debug/clk/pmc_plt_clk_?/flags
>>
>> And then email us the output please?
>>
>> Regards,
>>
>> Hans
>>
> I don't have flags files in /sys/kernel/debug/clk/pmc_plt_clk_?; did you mean clk_flags?
>
> [root@archatom ~]# grep . /sys/kernel/debug/clk/pmc_plt_clk_?/flags
> grep: /sys/kernel/debug/clk/pmc_plt_clk_?/flags: No such file or directory
> [root@archatom ~]# grep . /sys/kernel/debug/clk/pmc_plt_clk_?/clk_flags
> /sys/kernel/debug/clk/pmc_plt_clk_0/clk_flags:CLK_IS_CRITICAL
> /sys/kernel/debug/clk/pmc_plt_clk_1/clk_flags:CLK_IS_CRITICAL
> /sys/kernel/debug/clk/pmc_plt_clk_2/clk_flags:CLK_IS_CRITICAL
> /sys/kernel/debug/clk/pmc_plt_clk_3/clk_flags:CLK_IS_CRITICAL
> [root@archatom ~]# ls /sys/kernel/debug/clk
> clk_dump clk_orphan_summary pll pmc_plt_clk_1 pmc_plt_clk_3 pmc_plt_clk_5
> clk_orphan_dump clk_summary pmc_plt_clk_0 pmc_plt_clk_2 pmc_plt_clk_4 xtal
Hmm, so 4 ethernet cards and 4 enabled / marked as critical clocks.
Supporting this through get_clk is going to require a DMI table in the igb driver
combined with checking which PCI "slot" the card is to get the correct clock
for each ethernet controller.
I believe tht just restoring the old behavior to mark all clocks enabled
on boot as critical, but then limited to this system based on a dmi match,
is the best solution here.
Andy?
Regards,
Hans
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [E1000-devel] igb driver with Intel Atom Bay Trail issue
2019-03-29 15:53 ` Hans de Goede
@ 2019-04-04 14:43 ` Hans de Goede
[not found] ` <20190408172111.GX9224@smile.fi.intel.com>
0 siblings, 1 reply; 24+ messages in thread
From: Hans de Goede @ 2019-04-04 14:43 UTC (permalink / raw)
To: Семен
Верченко,
Andy Shevchenko
Cc: David Müller, Fujinaka, Todd, Kirsher, Jeffrey T,
e1000-devel, Stephen Boyd, linux-clk, Michael Turquette,
Stephen Boyd
Hi,
On 29-03-19 16:53, Hans de Goede wrote:
> Hi,
>
> On 3/29/19 2:59 PM, Семен Верченко wrote:
>>
>> On 29.03.2019 15:30, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 3/28/19 4:49 PM, Andy Shevchenko wrote:
>>>> On Thu, Mar 28, 2019 at 04:32:27PM +0100, Hans de Goede wrote:
>>>>> On 28-03-19 16:24, Andy Shevchenko wrote:
>>>>>> On Thu, Mar 28, 2019 at 04:01:37PM +0100, Hans de Goede wrote:
>>>>>>> On 28-03-19 15:58, Andy Shevchenko wrote:
>>>>>>>> On Thu, Mar 28, 2019 at 03:35:58PM +0100, David Müller wrote:
>>>>>>>>> Andy Shevchenko wrote:
>>>>>>>>>> On Wed, Mar 27, 2019 at 06:31:19PM +0100, David Müller wrote:
>>>>
>>>>>>>>>> Any driver for device which is using PMC clock should take it into
>>>>>>>>>> consideration.
>>>>>>>>>
>>>>>>>>> I agree that each driver should properly request the clocks and other
>>>>>>>>> resources needed.
>>>>
>>>>>>>> Can you elaborate a bit more the case you are talking about?
>>>>
>>>>>>> I think the board with igb ethernet controllers might
>>>>>>> just as well be handled the same way (I already checked it has usable
>>>>>>> DMI identifying info).
>>>>>>
>>>>>> But am I right that in the case of igb we will loose power at suspend? Wouldn't
>>>>>> be better to patch the driver?
>>>>>
>>>>> This is an industrial embedded PC, so it is not running on battery and
>>>>> I doubt it typically spends a lot of time in suspend at all.
>>>>
>>>> Okay, but still from logical point of view wouldn't be better to fix the driver
>>>> for such case? At least I see benefits out of this approach: a) less hackish,
>>>> less quirk code; b) if this happens on non-industrial case it would be better
>>>> to have in the driver due to power consumption.
>>>
>>> Maybe, I guess we first need to figure out which platforms clock(s) is (are)
>>> being used, if there is more then one; or it is a different one then in the
>>> realtek ethernet case it might be better to go with the dmi quirk option.
>>>
>>> Semyon Verchenko can you (as root) run the following command on a kernel
>>> where the ethernet does work:
>>>
>>> grep . /sys/kernel/debug/clk/pmc_plt_clk_?/flags
>>>
>>> And then email us the output please?
>>>
>>> Regards,
>>>
>>> Hans
>>>
>> I don't have flags files in /sys/kernel/debug/clk/pmc_plt_clk_?; did you mean clk_flags?
>>
>> [root@archatom ~]# grep . /sys/kernel/debug/clk/pmc_plt_clk_?/flags
>> grep: /sys/kernel/debug/clk/pmc_plt_clk_?/flags: No such file or directory
>> [root@archatom ~]# grep . /sys/kernel/debug/clk/pmc_plt_clk_?/clk_flags
>> /sys/kernel/debug/clk/pmc_plt_clk_0/clk_flags:CLK_IS_CRITICAL
>> /sys/kernel/debug/clk/pmc_plt_clk_1/clk_flags:CLK_IS_CRITICAL
>> /sys/kernel/debug/clk/pmc_plt_clk_2/clk_flags:CLK_IS_CRITICAL
>> /sys/kernel/debug/clk/pmc_plt_clk_3/clk_flags:CLK_IS_CRITICAL
>> [root@archatom ~]# ls /sys/kernel/debug/clk
>> clk_dump clk_orphan_summary pll pmc_plt_clk_1 pmc_plt_clk_3 pmc_plt_clk_5
>> clk_orphan_dump clk_summary pmc_plt_clk_0 pmc_plt_clk_2 pmc_plt_clk_4 xtal
>
> Hmm, so 4 ethernet cards and 4 enabled / marked as critical clocks.
>
> Supporting this through get_clk is going to require a DMI table in the igb driver
> combined with checking which PCI "slot" the card is to get the correct clock
> for each ethernet controller.
>
> I believe tht just restoring the old behavior to mark all clocks enabled
> on boot as critical, but then limited to this system based on a dmi match,
> is the best solution here.
>
> Andy?
Andy? Now that we've the patch ready for the other system which needs to
have the CLK_IS_CRITICAL workaround and enables this based on DMI info,
I believe the best fix for this system is to simply add it to that DMI
table?
Regards,
Hans
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [E1000-devel] igb driver with Intel Atom Bay Trail issue
[not found] ` <20190408172111.GX9224@smile.fi.intel.com>
@ 2019-04-08 18:43 ` Hans de Goede
2019-04-09 15:31 ` Andy Shevchenko
0 siblings, 1 reply; 24+ messages in thread
From: Hans de Goede @ 2019-04-08 18:43 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Семен
Верченко,
David Müller, Fujinaka, Todd, Kirsher, Jeffrey T,
e1000-devel, Stephen Boyd, linux-clk, Michael Turquette,
Stephen Boyd
Hi,
On 08-04-19 19:21, Andy Shevchenko wrote:
> On Thu, Apr 04, 2019 at 04:43:03PM +0200, Hans de Goede wrote:
>> On 29-03-19 16:53, Hans de Goede wrote:
>>> On 3/29/19 2:59 PM, Семен Верченко wrote:
>
>>> Hmm, so 4 ethernet cards and 4 enabled / marked as critical clocks.
>>>
>>> Supporting this through get_clk is going to require a DMI table in the igb driver
>>> combined with checking which PCI "slot" the card is to get the correct clock
>>> for each ethernet controller.
>>>
>>> I believe tht just restoring the old behavior to mark all clocks enabled
>>> on boot as critical, but then limited to this system based on a dmi match,
>>> is the best solution here.
>>>
>>> Andy?
>>
>> Andy? Now that we've the patch ready for the other system which needs to
>> have the CLK_IS_CRITICAL workaround and enables this based on DMI info,
>> I believe the best fix for this system is to simply add it to that DMI
>> table?
>
> I reviewed v4, supposed to go via CLK tree.
Right, but that patch adds the quirk for the system with the USB hub,
do you agree, that given that each ethernet controller seems to be
using its own clock, it is best to use a DMI quirk for this case too?
If you agree then someone needs to prepare a follow-up patch on top of
v4 which adds the DMI info for this board to the table.
Regards,
Hans
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [E1000-devel] igb driver with Intel Atom Bay Trail issue
2019-04-08 18:43 ` Hans de Goede
@ 2019-04-09 15:31 ` Andy Shevchenko
2019-04-18 13:09 ` Hans de Goede
0 siblings, 1 reply; 24+ messages in thread
From: Andy Shevchenko @ 2019-04-09 15:31 UTC (permalink / raw)
To: Hans de Goede
Cc: Семен
Верченко,
David Müller, Fujinaka, Todd, Kirsher, Jeffrey T,
e1000-devel, Stephen Boyd, linux-clk, Michael Turquette,
Stephen Boyd
On Mon, Apr 08, 2019 at 08:43:10PM +0200, Hans de Goede wrote:
> On 08-04-19 19:21, Andy Shevchenko wrote:
> > On Thu, Apr 04, 2019 at 04:43:03PM +0200, Hans de Goede wrote:
> > > On 29-03-19 16:53, Hans de Goede wrote:
> > > > On 3/29/19 2:59 PM, Семен Верченко wrote:
> >
> > > > Hmm, so 4 ethernet cards and 4 enabled / marked as critical clocks.
> > > >
> > > > Supporting this through get_clk is going to require a DMI table in the igb driver
> > > > combined with checking which PCI "slot" the card is to get the correct clock
> > > > for each ethernet controller.
> > > >
> > > > I believe tht just restoring the old behavior to mark all clocks enabled
> > > > on boot as critical, but then limited to this system based on a dmi match,
> > > > is the best solution here.
> > > >
> > > > Andy?
> > >
> > > Andy? Now that we've the patch ready for the other system which needs to
> > > have the CLK_IS_CRITICAL workaround and enables this based on DMI info,
> > > I believe the best fix for this system is to simply add it to that DMI
> > > table?
> >
> > I reviewed v4, supposed to go via CLK tree.
>
> Right, but that patch adds the quirk for the system with the USB hub,
> do you agree, that given that each ethernet controller seems to be
> using its own clock, it is best to use a DMI quirk for this case too?
>
> If you agree then someone needs to prepare a follow-up patch on top of
> v4 which adds the DMI info for this board to the table.
I hope we may find a better solution in the future, but for now as a quick fix
the proposed can be done.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [E1000-devel] igb driver with Intel Atom Bay Trail issue
2019-04-09 15:31 ` Andy Shevchenko
@ 2019-04-18 13:09 ` Hans de Goede
2019-04-18 13:26 ` Semyon Verchenko
0 siblings, 1 reply; 24+ messages in thread
From: Hans de Goede @ 2019-04-18 13:09 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Семен
Верченко,
David Müller, Fujinaka, Todd, Kirsher, Jeffrey T,
e1000-devel, Stephen Boyd, linux-clk, Michael Turquette,
Stephen Boyd
Hi,
On 09-04-19 17:31, Andy Shevchenko wrote:
> On Mon, Apr 08, 2019 at 08:43:10PM +0200, Hans de Goede wrote:
>> On 08-04-19 19:21, Andy Shevchenko wrote:
>>> On Thu, Apr 04, 2019 at 04:43:03PM +0200, Hans de Goede wrote:
>>>> On 29-03-19 16:53, Hans de Goede wrote:
>>>>> On 3/29/19 2:59 PM, Семен Верченко wrote:
>>>
>>>>> Hmm, so 4 ethernet cards and 4 enabled / marked as critical clocks.
>>>>>
>>>>> Supporting this through get_clk is going to require a DMI table in the igb driver
>>>>> combined with checking which PCI "slot" the card is to get the correct clock
>>>>> for each ethernet controller.
>>>>>
>>>>> I believe tht just restoring the old behavior to mark all clocks enabled
>>>>> on boot as critical, but then limited to this system based on a dmi match,
>>>>> is the best solution here.
>>>>>
>>>>> Andy?
>>>>
>>>> Andy? Now that we've the patch ready for the other system which needs to
>>>> have the CLK_IS_CRITICAL workaround and enables this based on DMI info,
>>>> I believe the best fix for this system is to simply add it to that DMI
>>>> table?
>>>
>>> I reviewed v4, supposed to go via CLK tree.
>>
>> Right, but that patch adds the quirk for the system with the USB hub,
>> do you agree, that given that each ethernet controller seems to be
>> using its own clock, it is best to use a DMI quirk for this case too?
>>
>> If you agree then someone needs to prepare a follow-up patch on top of
>> v4 which adds the DMI info for this board to the table.
>
> I hope we may find a better solution in the future, but for now as a quick fix
> the proposed can be done.
Ok.
Семен Верченко, this means that we are going to need DMI info from
the board in question. I thought we already had that, but I now see that
you original report did not have that a
Please run as root:
dmidecode &> dmidecode.log
And then reply to this email with the generated dmidecode.log file
attached. Once I have that file I can prepare a patch fixing this.
Regards,
Hans
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [E1000-devel] igb driver with Intel Atom Bay Trail issue
2019-04-18 13:09 ` Hans de Goede
@ 2019-04-18 13:26 ` Semyon Verchenko
2019-04-18 15:12 ` Hans de Goede
0 siblings, 1 reply; 24+ messages in thread
From: Semyon Verchenko @ 2019-04-18 13:26 UTC (permalink / raw)
To: Hans de Goede, Andy Shevchenko
Cc: David Müller, Fujinaka, Todd, Kirsher, Jeffrey T,
e1000-devel, Stephen Boyd, linux-clk, Michael Turquette,
Stephen Boyd
[-- Attachment #1: Type: text/plain, Size: 2171 bytes --]
On 18.04.2019 16:09, Hans de Goede wrote:
> Hi,
>
> On 09-04-19 17:31, Andy Shevchenko wrote:
>> On Mon, Apr 08, 2019 at 08:43:10PM +0200, Hans de Goede wrote:
>>> On 08-04-19 19:21, Andy Shevchenko wrote:
>>>> On Thu, Apr 04, 2019 at 04:43:03PM +0200, Hans de Goede wrote:
>>>>> On 29-03-19 16:53, Hans de Goede wrote:
>>>>>> On 3/29/19 2:59 PM, Семен Верченко wrote:
>>>>
>>>>>> Hmm, so 4 ethernet cards and 4 enabled / marked as critical clocks.
>>>>>>
>>>>>> Supporting this through get_clk is going to require a DMI table
>>>>>> in the igb driver
>>>>>> combined with checking which PCI "slot" the card is to get the
>>>>>> correct clock
>>>>>> for each ethernet controller.
>>>>>>
>>>>>> I believe tht just restoring the old behavior to mark all clocks
>>>>>> enabled
>>>>>> on boot as critical, but then limited to this system based on a
>>>>>> dmi match,
>>>>>> is the best solution here.
>>>>>>
>>>>>> Andy?
>>>>>
>>>>> Andy? Now that we've the patch ready for the other system which
>>>>> needs to
>>>>> have the CLK_IS_CRITICAL workaround and enables this based on DMI
>>>>> info,
>>>>> I believe the best fix for this system is to simply add it to that
>>>>> DMI
>>>>> table?
>>>>
>>>> I reviewed v4, supposed to go via CLK tree.
>>>
>>> Right, but that patch adds the quirk for the system with the USB hub,
>>> do you agree, that given that each ethernet controller seems to be
>>> using its own clock, it is best to use a DMI quirk for this case too?
>>>
>>> If you agree then someone needs to prepare a follow-up patch on top of
>>> v4 which adds the DMI info for this board to the table.
>>
>> I hope we may find a better solution in the future, but for now as a
>> quick fix
>> the proposed can be done.
>
> Ok.
>
> Семен Верченко, this means that we are going to need DMI info from
> the board in question. I thought we already had that, but I now see that
> you original report did not have that a
>
> Please run as root:
>
> dmidecode &> dmidecode.log
>
> And then reply to this email with the generated dmidecode.log file
> attached. Once I have that file I can prepare a patch fixing this.
>
> Regards,
>
> Hans
>
[-- Attachment #2: dmidecode.log --]
[-- Type: text/x-log, Size: 15787 bytes --]
# dmidecode 3.2
Getting SMBIOS data from sysfs.
SMBIOS 2.7 present.
51 structures occupying 2122 bytes.
Table at 0x000E6F40.
Handle 0x0000, DMI type 0, 24 bytes
BIOS Information
Vendor: INSYDE Corp.
Version: 3I380D A3
Release Date: 11/09/2015
Address: 0xF0000
Runtime Size: 64 kB
ROM Size: 3072 kB
Characteristics:
PCI is supported
BIOS is upgradeable
BIOS shadowing is allowed
Boot from CD is supported
Selectable boot is supported
EDD is supported
8042 keyboard services are supported (int 9h)
Serial services are supported (int 14h)
Printer services are supported (int 17h)
CGA/mono video services are supported (int 10h)
ACPI is supported
USB legacy is supported
ATAPI Zip drive boot is supported
BIOS boot specification is supported
Function key-initiated network boot is supported
Targeted content distribution is supported
BIOS Revision: 24.0
Firmware Revision: 0.0
Handle 0x0001, DMI type 1, 27 bytes
System Information
Manufacturer: Lex BayTrail
Product Name: 3I380D
Version:
Serial Number: BT-I C2II1
UUID: 00000001-0000-4000-0000-4c028910f18b
Wake-up Type: Power Switch
SKU Number:
Family:
Handle 0x0002, DMI type 2, 17 bytes
Base Board Information
Manufacturer: Lex
Product Name: 3I380D
Version:
Serial Number:
Asset Tag:
Features:
Board is a hosting board
Board is replaceable
Location In Chassis:
Chassis Handle: 0x0003
Type: Motherboard
Contained Object Handles: 0
Handle 0x0003, DMI type 3, 24 bytes
Chassis Information
Manufacturer: Lex
Type: Notebook
Lock: Not Present
Version:
Serial Number:
Asset Tag:
Boot-up State: Safe
Power Supply State: Safe
Thermal State: Safe
Security Status: None
OEM Information: 0x00000000
Height: Unspecified
Number Of Power Cords: 1
Contained Elements: 0
SKU Number: Not Specified
Handle 0x0004, DMI type 4, 42 bytes
Processor Information
Socket Designation: CPU 1
Type: Central Processor
Family: Atom
Manufacturer: Intel(R) Corporation
ID: 79 06 03 00 FF FB EB BF
Signature: Type 0, Family 6, Model 55, Stepping 9
Flags:
FPU (Floating-point unit on-chip)
VME (Virtual mode extension)
DE (Debugging extension)
PSE (Page size extension)
TSC (Time stamp counter)
MSR (Model specific registers)
PAE (Physical address extension)
MCE (Machine check exception)
CX8 (CMPXCHG8 instruction supported)
APIC (On-chip APIC hardware supported)
SEP (Fast system call)
MTRR (Memory type range registers)
PGE (Page global enable)
MCA (Machine check architecture)
CMOV (Conditional move instruction supported)
PAT (Page attribute table)
PSE-36 (36-bit page size extension)
CLFSH (CLFLUSH instruction supported)
DS (Debug store)
ACPI (ACPI supported)
MMX (MMX technology supported)
FXSR (FXSAVE and FXSTOR instructions supported)
SSE (Streaming SIMD extensions)
SSE2 (Streaming SIMD extensions 2)
SS (Self-snoop)
HTT (Multi-threading)
TM (Thermal monitor supported)
PBE (Pending break enabled)
Version: Intel(R) Atom(TM) CPU E3825 @ 1.33GHz
Voltage: 0.8 V
External Clock: 133 MHz
Max Speed: 1330 MHz
Current Speed: 1340 MHz
Status: Populated, Enabled
Upgrade: None
L1 Cache Handle: 0x0008
L2 Cache Handle: 0x0009
L3 Cache Handle: Not Provided
Serial Number: Not Specified
Asset Tag: Not Specified
Part Number: Not Specified
Core Count: 2
Core Enabled: 2
Thread Count: 1
Characteristics:
64-bit capable
Multi-Core
Execute Protection
Enhanced Virtualization
Power/Performance Control
Handle 0x0005, DMI type 6, 12 bytes
Memory Module Information
Socket Designation: DIMM0
Bank Connections: 0 0
Current Speed: Unknown
Type: None
Installed Size: 4096 MB (Single-bank Connection)
Enabled Size: 4096 MB (Single-bank Connection)
Error Status: OK
Handle 0x0006, DMI type 6, 12 bytes
Memory Module Information
Socket Designation: DIMM1
Bank Connections: 0 0
Current Speed: Unknown
Type: None
Installed Size: Not Installed
Enabled Size: Not Installed
Error Status: OK
Handle 0x0007, DMI type 7, 19 bytes
Cache Information
Socket Designation: L1 Cache
Configuration: Enabled, Not Socketed, Level 1
Operational Mode: Write Back
Location: Internal
Installed Size: 48 kB
Maximum Size: 48 kB
Supported SRAM Types:
Synchronous
Installed SRAM Type: Synchronous
Speed: Unknown
Error Correction Type: Single-bit ECC
System Type: Data
Associativity: Fully Associative
Handle 0x0008, DMI type 7, 19 bytes
Cache Information
Socket Designation: L1 Cache
Configuration: Enabled, Not Socketed, Level 1
Operational Mode: Write Back
Location: Internal
Installed Size: 64 kB
Maximum Size: 64 kB
Supported SRAM Types:
Synchronous
Installed SRAM Type: Synchronous
Speed: Unknown
Error Correction Type: Single-bit ECC
System Type: Instruction
Associativity: 8-way Set-associative
Handle 0x0009, DMI type 7, 19 bytes
Cache Information
Socket Designation: L2 Cache
Configuration: Enabled, Not Socketed, Level 2
Operational Mode: Write Back
Location: Internal
Installed Size: 1024 kB
Maximum Size: 1024 kB
Supported SRAM Types:
Synchronous
Installed SRAM Type: Synchronous
Speed: Unknown
Error Correction Type: Single-bit ECC
System Type: Unified
Associativity: 8-way Set-associative
Handle 0x000A, DMI type 8, 9 bytes
Port Connector Information
Internal Reference Designator: J1A2
Internal Connector Type: None
External Reference Designator: USB2.0
External Connector Type: Access Bus (USB)
Port Type: USB
Handle 0x000B, DMI type 8, 9 bytes
Port Connector Information
Internal Reference Designator: J1A2
Internal Connector Type: None
External Reference Designator: USB2.0
External Connector Type: Access Bus (USB)
Port Type: USB
Handle 0x000C, DMI type 8, 9 bytes
Port Connector Information
Internal Reference Designator: J2A2
Internal Connector Type: None
External Reference Designator: USB3.0 Port0
External Connector Type: Access Bus (USB)
Port Type: USB
Handle 0x000D, DMI type 8, 9 bytes
Port Connector Information
Internal Reference Designator: J9D1
Internal Connector Type: None
External Reference Designator: UART to Micro USB
External Connector Type: Access Bus (USB)
Port Type: USB
Handle 0x000E, DMI type 8, 9 bytes
Port Connector Information
Internal Reference Designator: CON6A1
Internal Connector Type: None
External Reference Designator: SD Card
External Connector Type: Other
Port Type: Other
Handle 0x000F, DMI type 8, 9 bytes
Port Connector Information
Internal Reference Designator: J9B1
Internal Connector Type: None
External Reference Designator: Keyboard
External Connector Type: PS/2
Port Type: Keyboard Port
Handle 0x0010, DMI type 8, 9 bytes
Port Connector Information
Internal Reference Designator: J9B1
Internal Connector Type: None
External Reference Designator: Mouse
External Connector Type: PS/2
Port Type: Mouse Port
Handle 0x0011, DMI type 8, 9 bytes
Port Connector Information
Internal Reference Designator: J5A1
Internal Connector Type: None
External Reference Designator: Microphone
External Connector Type: Mini Jack (headphones)
Port Type: Audio Port
Handle 0x0012, DMI type 8, 9 bytes
Port Connector Information
Internal Reference Designator: J5A1
Internal Connector Type: None
External Reference Designator: Line In
External Connector Type: Mini Jack (headphones)
Port Type: Audio Port
Handle 0x0013, DMI type 8, 9 bytes
Port Connector Information
Internal Reference Designator: J5A1
Internal Connector Type: None
External Reference Designator: Line Out
External Connector Type: Mini Jack (headphones)
Port Type: Audio Port
Handle 0x0014, DMI type 8, 9 bytes
Port Connector Information
Internal Reference Designator: J7A1
Internal Connector Type: None
External Reference Designator: Audio Jack
External Connector Type: Mini Jack (headphones)
Port Type: Audio Port
Handle 0x0015, DMI type 8, 9 bytes
Port Connector Information
Internal Reference Designator: J2A2
Internal Connector Type: None
External Reference Designator: Network Rj45 Jack
External Connector Type: RJ-45
Port Type: Network Port
Handle 0x0016, DMI type 8, 9 bytes
Port Connector Information
Internal Reference Designator: J4J1
Internal Connector Type: None
External Reference Designator: SATA
External Connector Type: SAS/SATA Plug Receptacle
Port Type: Other
Handle 0x0017, DMI type 8, 9 bytes
Port Connector Information
Internal Reference Designator: J4E2
Internal Connector Type: SAS/SATA Plug Receptacle
External Reference Designator: SATA Cable
External Connector Type: None
Port Type: Other
Handle 0x0018, DMI type 8, 9 bytes
Port Connector Information
Internal Reference Designator: J4A1
Internal Connector Type: None
External Reference Designator: VGA
External Connector Type: DB-15 female
Port Type: Video Port
Handle 0x0019, DMI type 8, 9 bytes
Port Connector Information
Internal Reference Designator: J1A1
Internal Connector Type: None
External Reference Designator: Display Port
External Connector Type: Other
Port Type: Video Port
Handle 0x001A, DMI type 8, 9 bytes
Port Connector Information
Internal Reference Designator: J3A1
Internal Connector Type: None
External Reference Designator: Display Port
External Connector Type: Other
Port Type: Video Port
Handle 0x001B, DMI type 8, 9 bytes
Port Connector Information
Internal Reference Designator: J9B4
Internal Connector Type: None
External Reference Designator: Serial Port
External Connector Type: DB-9 male
Port Type: Serial Port 16550A Compatible
Handle 0x001C, DMI type 8, 9 bytes
Port Connector Information
Internal Reference Designator: J2A1
Internal Connector Type: None
External Reference Designator: HDMI
External Connector Type: Other
Port Type: Other
Handle 0x001D, DMI type 9, 17 bytes
System Slot Information
Designation: J5C1
Type: x4 PCI Express x4
Current Usage: Available
Length: Other
ID: 0
Characteristics:
PME signal is supported
Hot-plug devices are supported
SMBus signal is supported
Bus Address: 0000:00:00.0
Handle 0x001E, DMI type 9, 17 bytes
System Slot Information
Designation: J4B3
Type: x1 PCI Express x1
Current Usage: Available
Length: Other
ID: 0
Characteristics:
PME signal is supported
Hot-plug devices are supported
SMBus signal is supported
Bus Address: 0000:00:00.0
Handle 0x001F, DMI type 9, 17 bytes
System Slot Information
Designation: J6C1
Type: x1 PCI Express x1
Current Usage: Available
Length: Other
ID: 0
Characteristics:
PME signal is supported
Hot-plug devices are supported
SMBus signal is supported
Bus Address: 0000:00:00.0
Handle 0x0020, DMI type 9, 17 bytes
System Slot Information
Designation: J6D1
Type: x1 PCI Express x1
Current Usage: Available
Length: Other
ID: 0
Characteristics:
PME signal is supported
Hot-plug devices are supported
SMBus signal is supported
Bus Address: 0000:00:00.0
Handle 0x0021, DMI type 11, 5 bytes
OEM Strings
String 1: Insyde Chipset
Handle 0x0022, DMI type 12, 5 bytes
System Configuration Options
Option 1: String1 for Type12 Equipment Manufacturer
Option 2: String2 for Type12 Equipment Manufacturer
Option 3: String3 for Type12 Equipment Manufacturer
Option 4: String4 for Type12 Equipment Manufacturer
Handle 0x0023, DMI type 13, 22 bytes
BIOS Language Information
Language Description Format: Long
Installable Languages: 4
en|US|iso8859-1
fr|CA|iso8859-1
zh|TW|unicode
ja|JP|unicode
Currently Installed Language: en|US|iso8859-1
Handle 0x0024, DMI type 15, 29 bytes
System Event Log
Area Length: 0 bytes
Header Start Offset: 0x0000
Header Length: 32 bytes
Data Start Offset: 0x0020
Access Method: General-purpose non-volatile data functions
Access Address: 0x0000
Status: Valid, Not Full
Change Token: 0x12345678
Header Format: OEM-specific
Supported Log Type Descriptors: 3
Descriptor 1: POST memory resize
Data Format 1: None
Descriptor 2: POST error
Data Format 2: POST results bitmap
Descriptor 3: Log area reset/cleared
Data Format 3: None
Handle 0x0025, DMI type 16, 15 bytes
Physical Memory Array
Location: System Board Or Motherboard
Use: System Memory
Error Correction Type: None
Maximum Capacity: 8 GB
Error Information Handle: No Error
Number Of Devices: 2
Handle 0x0026, DMI type 17, 34 bytes
Memory Device
Array Handle: 0x0025
Error Information Handle: No Error
Total Width: 64 bits
Data Width: 64 bits
Size: 4096 MB
Form Factor: SODIMM
Set: None
Locator: DIMM0
Bank Locator: BANK 0
Type: DDR3
Type Detail: Synchronous
Speed: 1066 MT/s
Manufacturer: 00
Serial Number: 00000000
Asset Tag: Unknown
Part Number:
Rank: Unknown
Configured Memory Speed: 1066 MT/s
Handle 0x0027, DMI type 17, 34 bytes
Memory Device
Array Handle: 0x0025
Error Information Handle: No Error
Total Width: Unknown
Data Width: Unknown
Size: No Module Installed
Form Factor: SODIMM
Set: None
Locator: DIMM1
Bank Locator: BANK 1
Type: Unknown
Type Detail: None
Speed: Unknown
Manufacturer: Not Specified
Serial Number: Not Specified
Asset Tag: Unknown
Part Number: Not Specified
Rank: Unknown
Configured Memory Speed: Unknown
Handle 0x0028, DMI type 19, 15 bytes
Memory Array Mapped Address
Starting Address: 0x00000000000
Ending Address: 0x000FFFFFFFF
Range Size: 4 GB
Physical Array Handle: 0x0025
Partition Width: 2
Handle 0x0029, DMI type 20, 19 bytes
Memory Device Mapped Address
Starting Address: 0x00000000000
Ending Address: 0x000FFFFFFFF
Range Size: 4 GB
Physical Device Handle: 0x0026
Memory Array Mapped Address Handle: 0x0028
Partition Row Position: Unknown
Handle 0x002A, DMI type 21, 7 bytes
Built-in Pointing Device
Type: Touch Pad
Interface: PS/2
Buttons: 2
Handle 0x002B, DMI type 22, 26 bytes
Portable Battery
Location: I2C2
Manufacturer: Intel SR 1
Manufacture Date: Date
Serial Number: 123456789
Name: SR Real Battery
Chemistry: Lithium Ion
Design Capacity: 0 mWh
Design Voltage: 3750 mV
SBDS Version: CRB Battery 0
Maximum Error: Unknown
OEM-specific Information: 0x00000000
Handle 0x002C, DMI type 26, 22 bytes
Voltage Probe
Description: Voltage Probe Description.
Location: <OUT OF SPEC>
Status: <OUT OF SPEC>
Maximum Value: Unknown
Minimum Value: Unknown
Resolution: Unknown
Tolerance: Unknown
Accuracy: Unknown
OEM-specific Information: 0x00000000
Nominal Value: Unknown
Handle 0x002D, DMI type 27, 15 bytes
Cooling Device
Type: Chip Fan
Status: OK
OEM-specific Information: 0x00000000
Nominal Speed: Unknown Or Non-rotating
Description: Cooling Device Description.
Handle 0x002E, DMI type 28, 22 bytes
Temperature Probe
Description: Temperature Probe Description.
Location: Unknown
Status: Unknown
Maximum Value: Unknown
Minimum Value: Unknown
Resolution: Unknown
Tolerance: Unknown
Accuracy: Unknown
OEM-specific Information: 0x00000000
Nominal Value: Unknown
Handle 0x002F, DMI type 32, 11 bytes
System Boot Information
Status: No errors detected
Handle 0x0030, DMI type 39, 22 bytes
System Power Supply
Location: OEM_Define0
Name: OEM_Define1
Manufacturer: OEM_Define2
Serial Number: OEM_Define3
Asset Tag: OEM_Define4
Model Part Number: OEM_Define5
Revision: OEM_Define6
Max Power Capacity: 75 W
Status: Present, OK
Type: Regulator
Input Voltage Range Switching: Auto-switch
Plugged: No
Hot Replaceable: No
Input Voltage Probe Handle: 0x002C
Cooling Device Handle: 0x002D
Handle 0x0031, DMI type 41, 11 bytes
Onboard Device
Reference Designation: IGD
Type: Video
Status: Disabled
Type Instance: 1
Bus Address: 0000:00:02.0
Handle 0xFEFF, DMI type 127, 4 bytes
End Of Table
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [E1000-devel] igb driver with Intel Atom Bay Trail issue
2019-04-18 13:26 ` Semyon Verchenko
@ 2019-04-18 15:12 ` Hans de Goede
2019-04-22 10:20 ` Semyon Verchenko
0 siblings, 1 reply; 24+ messages in thread
From: Hans de Goede @ 2019-04-18 15:12 UTC (permalink / raw)
To: Semyon Verchenko, Andy Shevchenko
Cc: David Müller, Fujinaka, Todd, Kirsher, Jeffrey T,
e1000-devel, Stephen Boyd, linux-clk, Michael Turquette,
Stephen Boyd
[-- Attachment #1: Type: text/plain, Size: 742 bytes --]
Hi Semyon,
On 18-04-19 15:26, Semyon Verchenko wrote:
>
> On 18.04.2019 16:09, Hans de Goede wrote:
>> Hi,
>>
>> On 09-04-19 17:31, Andy Shevchenko wrote:
<snip>
>> Ok.
>>
>> Семен Верченко, this means that we are going to need DMI info from
>> the board in question. I thought we already had that, but I now see that
>> you original report did not have that a
>>
>> Please run as root:
>>
>> dmidecode &> dmidecode.log
>>
>> And then reply to this email with the generated dmidecode.log file
>> attached. Once I have that file I can prepare a patch fixing this.
Thank you for the DMI decode.
Attached are 3 patches, can you please test if adding those 3 patches
to your kernel fixes the problem?
Thanks & Regards,
Hans
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-clk-x86-Add-system-specific-quirk-to-mark-clocks-as-.patch --]
[-- Type: text/x-patch; name="0001-clk-x86-Add-system-specific-quirk-to-mark-clocks-as-.patch", Size: 4991 bytes --]
From 7c2e07130090ae001a97a6b65597830d6815e93e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20M=C3=BCller?= <dave.mueller@gmx.ch>
Date: Mon, 8 Apr 2019 15:33:54 +0200
Subject: [PATCH] clk: x86: Add system specific quirk to mark clocks as
critical
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Since commit 648e921888ad ("clk: x86: Stop marking clocks as
CLK_IS_CRITICAL"), the pmc_plt_clocks of the Bay Trail SoC are
unconditionally gated off. Unfortunately this will break systems where these
clocks are used for external purposes beyond the kernel's knowledge. Fix it
by implementing a system specific quirk to mark the necessary pmc_plt_clks as
critical.
Fixes: 648e921888ad ("clk: x86: Stop marking clocks as CLK_IS_CRITICAL")
Signed-off-by: David Müller <dave.mueller@gmx.ch>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---
drivers/clk/x86/clk-pmc-atom.c | 14 ++++++++++---
drivers/platform/x86/pmc_atom.c | 21 +++++++++++++++++++
.../linux/platform_data/x86/clk-pmc-atom.h | 3 +++
3 files changed, 35 insertions(+), 3 deletions(-)
diff --git a/drivers/clk/x86/clk-pmc-atom.c b/drivers/clk/x86/clk-pmc-atom.c
index d977193842df..19174835693b 100644
--- a/drivers/clk/x86/clk-pmc-atom.c
+++ b/drivers/clk/x86/clk-pmc-atom.c
@@ -165,7 +165,7 @@ static const struct clk_ops plt_clk_ops = {
};
static struct clk_plt *plt_clk_register(struct platform_device *pdev, int id,
- void __iomem *base,
+ const struct pmc_clk_data *pmc_data,
const char **parent_names,
int num_parents)
{
@@ -184,9 +184,17 @@ static struct clk_plt *plt_clk_register(struct platform_device *pdev, int id,
init.num_parents = num_parents;
pclk->hw.init = &init;
- pclk->reg = base + PMC_CLK_CTL_OFFSET + id * PMC_CLK_CTL_SIZE;
+ pclk->reg = pmc_data->base + PMC_CLK_CTL_OFFSET + id * PMC_CLK_CTL_SIZE;
spin_lock_init(&pclk->lock);
+ /*
+ * On some systems, the pmc_plt_clocks already enabled by the
+ * firmware are being marked as critical to avoid them being
+ * gated by the clock framework.
+ */
+ if (pmc_data->critical && 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);
@@ -332,7 +340,7 @@ static int plt_clk_probe(struct platform_device *pdev)
return PTR_ERR(parent_names);
for (i = 0; i < PMC_CLK_NUM; i++) {
- data->clks[i] = plt_clk_register(pdev, i, pmc_data->base,
+ data->clks[i] = plt_clk_register(pdev, i, pmc_data,
parent_names, data->nparents);
if (IS_ERR(data->clks[i])) {
err = PTR_ERR(data->clks[i]);
diff --git a/drivers/platform/x86/pmc_atom.c b/drivers/platform/x86/pmc_atom.c
index 8f018b3f3cd4..eaec2d306481 100644
--- a/drivers/platform/x86/pmc_atom.c
+++ b/drivers/platform/x86/pmc_atom.c
@@ -17,6 +17,7 @@
#include <linux/debugfs.h>
#include <linux/device.h>
+#include <linux/dmi.h>
#include <linux/init.h>
#include <linux/io.h>
#include <linux/platform_data/x86/clk-pmc-atom.h>
@@ -391,11 +392,27 @@ static int pmc_dbgfs_register(struct pmc_dev *pmc)
}
#endif /* CONFIG_DEBUG_FS */
+/*
+ * Some systems need one or more of their pmc_plt_clks to be
+ * marked as critical.
+ */
+static const struct dmi_system_id critclk_systems[] __initconst = {
+ {
+ .ident = "MPL CEC1x",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "MPL AG"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "CEC10 Family"),
+ },
+ },
+ { /*sentinel*/ }
+};
+
static int pmc_setup_clks(struct pci_dev *pdev, void __iomem *pmc_regmap,
const struct pmc_data *pmc_data)
{
struct platform_device *clkdev;
struct pmc_clk_data *clk_data;
+ const struct dmi_system_id *d = dmi_first_match(critclk_systems);
clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
if (!clk_data)
@@ -403,6 +420,10 @@ static int pmc_setup_clks(struct pci_dev *pdev, void __iomem *pmc_regmap,
clk_data->base = pmc_regmap; /* offset is added by client */
clk_data->clks = pmc_data->clks;
+ if (d) {
+ clk_data->critical = true;
+ pr_info("%s critclks quirk enabled\n", d->ident);
+ }
clkdev = platform_device_register_data(&pdev->dev, "clk-pmc-atom",
PLATFORM_DEVID_NONE,
diff --git a/include/linux/platform_data/x86/clk-pmc-atom.h b/include/linux/platform_data/x86/clk-pmc-atom.h
index 3ab892208343..7a37ac27d0fb 100644
--- a/include/linux/platform_data/x86/clk-pmc-atom.h
+++ b/include/linux/platform_data/x86/clk-pmc-atom.h
@@ -35,10 +35,13 @@ struct pmc_clk {
*
* @base: PMC clock register base offset
* @clks: pointer to set of registered clocks, typically 0..5
+ * @critical: flag to indicate if firmware enabled pmc_plt_clks
+ * should be marked as critial or not
*/
struct pmc_clk_data {
void __iomem *base;
const struct pmc_clk *clks;
+ bool critical;
};
#endif /* __PLATFORM_DATA_X86_CLK_PMC_ATOM_H */
--
2.21.0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-platform-x86-pmc_atom-Drop-__initconst-on-dmi-table.patch --]
[-- Type: text/x-patch; name="0002-platform-x86-pmc_atom-Drop-__initconst-on-dmi-table.patch", Size: 1354 bytes --]
From b995dcca7cf12f208cfd95fd9d5768dca7cccec7 Mon Sep 17 00:00:00 2001
From: Stephen Boyd <sboyd@kernel.org>
Date: Thu, 11 Apr 2019 10:22:43 -0700
Subject: [PATCH] platform/x86: pmc_atom: Drop __initconst on dmi table
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
It's used by probe and that isn't an init function. Drop this so that we
don't get a section mismatch.
Reported-by: kbuild test robot <lkp@intel.com>
Cc: David Müller <dave.mueller@gmx.ch>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Fixes: 7c2e07130090 ("clk: x86: Add system specific quirk to mark clocks as critical")
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---
drivers/platform/x86/pmc_atom.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/platform/x86/pmc_atom.c b/drivers/platform/x86/pmc_atom.c
index eaec2d306481..c7039f52ad51 100644
--- a/drivers/platform/x86/pmc_atom.c
+++ b/drivers/platform/x86/pmc_atom.c
@@ -396,7 +396,7 @@ static int pmc_dbgfs_register(struct pmc_dev *pmc)
* Some systems need one or more of their pmc_plt_clks to be
* marked as critical.
*/
-static const struct dmi_system_id critclk_systems[] __initconst = {
+static const struct dmi_system_id critclk_systems[] = {
{
.ident = "MPL CEC1x",
.matches = {
--
2.21.0
[-- Attachment #4: 0003-platform-x86-Add-Lex-3I380D-industrial-PC-to-critclk.patch --]
[-- Type: text/x-patch, Size: 1480 bytes --]
From dcf861e0a71906903189baed4817a90e37dc78cc Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Thu, 18 Apr 2019 17:04:10 +0200
Subject: [PATCH] platform: x86: Add Lex 3I380D industrial PC to
critclk_systems DMI table
The Lex 3I380D industrial PC has 4 ethernet controllers on board
which need pmc_plt_clk0 - 3 to function, add it to the critclk_systems
DMI table, so that drivers/clk/x86/clk-pmc-atom.c will mark the clocks
as CLK_CRITICAL and they will not get turned off.
Reported-by: Semyon Verchenko <semverchenko@factor-ts.ru>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/platform/x86/pmc_atom.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/platform/x86/pmc_atom.c b/drivers/platform/x86/pmc_atom.c
index 3a635ea09b8a..2910845b7cdd 100644
--- a/drivers/platform/x86/pmc_atom.c
+++ b/drivers/platform/x86/pmc_atom.c
@@ -407,12 +407,21 @@ static int pmc_dbgfs_register(struct pmc_dev *pmc)
*/
static const struct dmi_system_id critclk_systems[] = {
{
+ /* pmc_plt_clk0 is used for an external HSIC USB HUB */
.ident = "MPL CEC1x",
.matches = {
DMI_MATCH(DMI_SYS_VENDOR, "MPL AG"),
DMI_MATCH(DMI_PRODUCT_NAME, "CEC10 Family"),
},
},
+ {
+ /* pmc_plt_clk0 - 3 are used for the 4 ethernet controllers */
+ .ident = "Lex 3I380D",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Lex BayTrail"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "3I380D"),
+ },
+ },
{ /*sentinel*/ }
};
--
2.21.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [E1000-devel] igb driver with Intel Atom Bay Trail issue
2019-04-18 15:12 ` Hans de Goede
@ 2019-04-22 10:20 ` Semyon Verchenko
2019-04-29 15:02 ` Hans de Goede
0 siblings, 1 reply; 24+ messages in thread
From: Semyon Verchenko @ 2019-04-22 10:20 UTC (permalink / raw)
To: Hans de Goede, Andy Shevchenko
Cc: David Müller, Fujinaka, Todd, Kirsher, Jeffrey T,
e1000-devel, Stephen Boyd, linux-clk, Michael Turquette,
Stephen Boyd
On 18.04.2019 18:12, Hans de Goede wrote:
> Hi Semyon,
>
> On 18-04-19 15:26, Semyon Verchenko wrote:
>>
>> On 18.04.2019 16:09, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 09-04-19 17:31, Andy Shevchenko wrote:
>
> <snip>
>
>>> Ok.
>>>
>>> Семен Верченко, this means that we are going to need DMI info from
>>> the board in question. I thought we already had that, but I now see
>>> that
>>> you original report did not have that a
>>>
>>> Please run as root:
>>>
>>> dmidecode &> dmidecode.log
>>>
>>> And then reply to this email with the generated dmidecode.log file
>>> attached. Once I have that file I can prepare a patch fixing this.
>
> Thank you for the DMI decode.
>
> Attached are 3 patches, can you please test if adding those 3 patches
> to your kernel fixes the problem?
>
> Thanks & Regards,
>
> Hans
>
Hi Hans,
It seems that these patches fix the problem (at least interfaces are
visible through ip addr and it's possible to ping something from them).
Thanks for fixing this issue.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [E1000-devel] igb driver with Intel Atom Bay Trail issue
2019-04-22 10:20 ` Semyon Verchenko
@ 2019-04-29 15:02 ` Hans de Goede
0 siblings, 0 replies; 24+ messages in thread
From: Hans de Goede @ 2019-04-29 15:02 UTC (permalink / raw)
To: Semyon Verchenko, Andy Shevchenko
Cc: David Müller, Fujinaka, Todd, Kirsher, Jeffrey T,
e1000-devel, Stephen Boyd, linux-clk, Michael Turquette,
Stephen Boyd
Hi,
On 22-04-19 12:20, Semyon Verchenko wrote:
>
> On 18.04.2019 18:12, Hans de Goede wrote:
>> Hi Semyon,
>>
>> On 18-04-19 15:26, Semyon Verchenko wrote:
>>>
>>> On 18.04.2019 16:09, Hans de Goede wrote:
>>>> Hi,
>>>>
>>>> On 09-04-19 17:31, Andy Shevchenko wrote:
>>
>> <snip>
>>
>>>> Ok.
>>>>
>>>> Семен Верченко, this means that we are going to need DMI info from
>>>> the board in question. I thought we already had that, but I now see that
>>>> you original report did not have that a
>>>>
>>>> Please run as root:
>>>>
>>>> dmidecode &> dmidecode.log
>>>>
>>>> And then reply to this email with the generated dmidecode.log file
>>>> attached. Once I have that file I can prepare a patch fixing this.
>>
>> Thank you for the DMI decode.
>>
>> Attached are 3 patches, can you please test if adding those 3 patches
>> to your kernel fixes the problem?
>>
>> Thanks & Regards,
>>
>> Hans
>>
> Hi Hans,
> It seems that these patches fix the problem (at least interfaces are visible through ip addr and it's possible to ping something from them).
>
> Thanks for fixing this issue.
Thank you for testing the fix, I've submitted the patch fixing this upstream.
Regards,
Hans
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2019-04-29 15:02 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1c433bef-055e-2ac3-990c-325aa2d3899e@factor-ts.ru>
[not found] ` <9B4A1B1917080E46B64F07F2989DADD69AF4DDE7@ORSMSX115.amr.corp.intel.com>
2019-03-27 16:02 ` [E1000-devel] igb driver with Intel Atom Bay Trail issue Hans de Goede
2019-03-27 16:47 ` Andy Shevchenko
2019-03-27 17:31 ` David Müller
2019-03-27 19:19 ` Andy Shevchenko
2019-03-28 14:35 ` David Müller
2019-03-28 14:58 ` Andy Shevchenko
2019-03-28 15:01 ` Hans de Goede
2019-03-28 15:24 ` Andy Shevchenko
2019-03-28 15:32 ` Hans de Goede
2019-03-28 15:49 ` Andy Shevchenko
2019-03-29 4:46 ` Hisashi T Fujinaka
2019-03-29 12:32 ` Hans de Goede
2019-03-29 12:30 ` Hans de Goede
2019-03-29 13:59 ` Семен Верченко
2019-03-29 15:53 ` Hans de Goede
2019-04-04 14:43 ` Hans de Goede
[not found] ` <20190408172111.GX9224@smile.fi.intel.com>
2019-04-08 18:43 ` Hans de Goede
2019-04-09 15:31 ` Andy Shevchenko
2019-04-18 13:09 ` Hans de Goede
2019-04-18 13:26 ` Semyon Verchenko
2019-04-18 15:12 ` Hans de Goede
2019-04-22 10:20 ` Semyon Verchenko
2019-04-29 15:02 ` Hans de Goede
2019-03-28 14:48 ` Hans de Goede
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.