Re: [PATCH 6/6] net: bcmgenet: reduce severity of missing clock warnings

[Nicolas Saenz Julienne <nsaenzjulienne@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 3238 bytes --]

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?

> BTW: For DT, is part of the problem here that the videocore mailbox has 
> a clock management method?

I don't think it'll be the case for these clocks. We try to only use the
mailbox interface if access to the clock is shared with videocore's firmware.
The only example for now is 'pllb' which drives the CPU. See clk-raspberrypi.c
for the firmware part and clk-bcm2835.c for the rest.

Note that the firmware interface has some shortcomings, it isn't fine grained
nor provides a full clock tree to work with, also some clock changes, from
videocore's point of view, might change multiple plls behind your back. See for
example the ARM clock, at offset 0x3[1]: if you don't explicitly disable turbo
mode, it'll change both pllb and pllc. Affecting a whole lot of peripherals.

In an Ideal world I'd love to see them implement ARM's SCMI[2]. It would make
our lives easier.

> For ACPI one of the paths of investigation is to write AML which just
> interfaces to that mailbox interface for clock control here. (there is also
> SCMII to be considered).

As we're on the topic of integrating the mailbox interfaces with ACPI, have you
looked at VCHIQ in the staging directory? It serves as an interface to
videocore for the camera, HDMI audio and video codec drivers. It ultimately
depends on the mailbox interface mentioned above. It might be interesting for
you to look into it before writing the AML interface to the mailbox.

Regards,
Nicolas

[1] https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface
[2] https://github.com/raspberrypi/firmware/issues/1139


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]