Re: [E1000-devel] igb driver with Intel Atom Bay Trail issue

[Hans de G. <hde...@re...>]

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