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

[Jeremy Linton]

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:

BTW: For DT, is part of the problem here that the videocore mailbox has 
a clock management method? 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).


> diff --git a/arch/arm/boot/dts/bcm2711.dtsi b/arch/arm/boot/dts/bcm2711.dtsi
> index 961bed8..d4ff370 100644
> --- a/arch/arm/boot/dts/bcm2711.dtsi
> +++ b/arch/arm/boot/dts/bcm2711.dtsi
> @@ -338,6 +338,8 @@
>  ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ reg = <0x0 0x7d580000 0x10000>;
>  ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ #address-cells = <0x1>;
>  ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ #size-cells = <0x1>;
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ clocks = <&clocks BCM2711_CLOCK_GENET250>;
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ clock-names = "enet";
>  ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ interrupts = <GIC_SPI 157 IRQ_TYPE_LEVEL_HIGH>,
>  ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ <GIC_SPI 158 IRQ_TYPE_LEVEL_HIGH>;
>  ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ status = "disabled";
> diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
> index ded13cc..627f1b1 100644
> --- a/drivers/clk/bcm/clk-bcm2835.c
> +++ b/drivers/clk/bcm/clk-bcm2835.c
> @@ -116,6 +116,10 @@
>  Â#define CM_EMMCDIVÂÂÂÂÂÂÂÂÂÂÂÂ 0x1c4
>  Â#define CM_EMMC2CTLÂÂÂÂÂÂÂÂÂÂÂ 0x1d0
>  Â#define CM_EMMC2DIVÂÂÂÂÂÂÂÂÂÂÂ 0x1d4
> +#define CM_GENET250CTLÂÂÂÂÂÂÂÂ 0x1e8
> +#define CM_GENET250DIVÂÂÂÂÂÂÂÂ 0x1ec
> +#define CM_GENET125CTLÂÂÂÂÂÂÂÂ 0x210
> +#define CM_GENET125DIVÂÂÂÂÂÂÂÂ 0x214
>   
>  Â/* General bits for the CM_*CTL regs */
>  Â# define CM_ENABLEÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ BIT(4)
> @@ -2021,6 +2025,25 @@ static const struct bcm2835_clk_desc
> clk_desc_array[] = {
>  ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ .frac_bits = 8,
>  ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ .tcnt_mux = 42),
>   
> +ÂÂÂÂÂÂ /* GENET clocks (only available for BCM2711) */
> +ÂÂÂÂÂÂ [BCM2711_CLOCK_GENET250]ÂÂÂÂÂÂÂ = REGISTER_PER_CLK(
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ SOC_BCM2711,
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ .name = "genet250",
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ .ctl_reg = CM_GENET250CTL,
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ .div_reg = CM_GENET250DIV,
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ .int_bits = 4,
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ .frac_bits = 8,
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ .tcnt_mux = 45),
> +
> +ÂÂÂÂÂÂ [BCM2711_CLOCK_GENET125]ÂÂÂÂÂÂÂ = REGISTER_PER_CLK(
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ SOC_BCM2711,
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ .name = "genet125",
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ .ctl_reg = CM_GENET125CTL,
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ .div_reg = CM_GENET125DIV,
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ .int_bits = 4,
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ .frac_bits = 8,
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ .tcnt_mux = 50),
> +
>  ÂÂÂÂÂÂÂ /* General purpose (GPIO) clocks */
>  ÂÂÂÂÂÂÂ [BCM2835_CLOCK_GP0]ÂÂÂÂ = REGISTER_PER_CLK(
>  ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ SOC_ALL,
> diff --git a/include/dt-bindings/clock/bcm2835.h
> b/include/dt-bindings/clock/bcm2835.h
> index b60c0343..fca65ab 100644
> --- a/include/dt-bindings/clock/bcm2835.h
> +++ b/include/dt-bindings/clock/bcm2835.h
> @@ -60,3 +60,5 @@
>  Â#define BCM2835_CLOCK_DSI1PÂÂÂÂÂÂÂÂÂÂÂ 50
>   
>  Â#define BCM2711_CLOCK_EMMC2ÂÂÂÂÂÂÂÂÂÂÂ 51
> +#define BCM2711_CLOCK_GENET250ÂÂÂÂÂÂÂÂ 52
> +#define BCM2711_CLOCK_GENET125ÂÂÂÂÂÂÂÂ 53