Messages in this thread | | | Subject | Re: [PATCH 6/6] net: bcmgenet: reduce severity of missing clock warnings | From | Stefan Wahren <> | Date | Wed, 5 Feb 2020 19:42:42 +0100 |
| |
Hi Florian,
Am 03.02.20 um 22:21 schrieb Florian Fainelli: > On 2/3/20 11:08 AM, Stefan Wahren wrote: >> Hi, >> >> Am 03.02.20 um 19:36 schrieb Nicolas Saenz Julienne: >>> Hi, >>> BTW the patch looks good to me too: >>> >>> Reviewed-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> >>> >>> On Sat, 2020-02-01 at 13:27 -0600, Jeremy Linton wrote: >>>> Hi, >>>> >>>> First, thanks for looking at this! >>>> >>>> On 2/1/20 10:44 AM, Stefan Wahren wrote: >>>>> Hi Jeremy, >>>>> >>>>> [add Nicolas as BCM2835 maintainer] >>>>> >>>>> Am 01.02.20 um 08:46 schrieb Jeremy Linton: >>>>>> If one types "failed to get enet clock" or similar into google >>>>>> there are ~370k hits. The vast majority are people debugging >>>>>> problems unrelated to this adapter, or bragging about their >>>>>> rpi's. Given that its not a fatal situation with common DT based >>>>>> systems, lets reduce the severity so people aren't seeing failure >>>>>> messages in everyday operation. >>>>>> >>>>> i'm fine with your patch, since the clocks are optional according to the >>>>> binding. But instead of hiding of those warning, it would be better to >>>>> fix the root cause (missing clocks). Unfortunately i don't have the >>>>> necessary documentation, just some answers from the RPi guys. >>>> The DT case just added to my ammunition here :) >>>> >>>> But really, I'm fixing an ACPI problem because the ACPI power management >>>> methods are also responsible for managing the clocks. Which means if I >>>> don't lower the severity (or otherwise tweak the code path) these errors >>>> are going to happen on every ACPI boot. >>>> >>>>> This is what i got so far: >>> Stefan, Apart from the lack of documentation (and maybe also time), is there >>> any specific reason you didn't sent the genet clock patch yet? It should be OK >>> functionally isn't it? >> last time i tried to specify the both clocks as suggest by the binding >> document (took genet125 for wol, not sure this is correct), but this >> caused an abort on the BCM2711. In the lack of documentation i stopped >> further investigations. As i saw that Jeremy send this patch, i wanted >> to share my current results and retestet it with this version which >> doesn't crash. I don't know the reason why both clocks should be >> specified, but this patch should be acceptable since the RPi 4 doesn't >> support wake on LAN. > Your clock changes look correct, but there is also a CLKGEN register > block which has separate clocks for the GENET controller, which lives at > register offset 0x7d5e0048 and which has the following layout: > > bit 0: GENET_CLK_250_CLOCK_ENABLE > bit 1: GENET_EEE_CLOCK_ENABLE > bit 2: GENET_GISB_CLOCK_ENABLE > bit 3: GENET_GMII_CLOCK_ENABLE > bit 4: GENET_HFB_CLOCK_ENABLE > bit 5: GENET_L2_INTR_CLOCK_ENABLE > bit 6: GENET_SCB_CLOCK_ENABLE > bit 7: GENET_UNIMAC_SYS_RX_CLOCK_ENABLE > bit 8: GENET_UNIMAC_SYS_TX_CLOCK_ENABLE > > you will need all of those clocks turned on for normal operation minus > EEE, unless EEE is desirable which is why it is a separate clock. Those > clocks default to ON unless turned off, and the main gate that you > control is probably enough. so you suggest to add these clock gate(s) to the clk-bcm2835 or introduce a "clk-genet" from DT perspective? > > The Pi4 could support Wake-on-LAN with appropriate VPU firmware changes, > but I do not believe there is any interest in doing that. I would not > "bend" the clock representation just so as to please the driver with its > clocking needs.
| |