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
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit
http://communities.intel.com/community/wired
