Re: [PATCH 2/6] ARM: dts: r7s72100: Add SPIBSC clocks

[Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>]

Hi Chris,

On Tue, Dec 3, 2019 at 4:46 AM Chris Brandt <chris.brandt@xxxxxxxxxxx> wrote:
> The SPIBSC-0 clock is marked as critical because for XIP systems, this
> is the SPI flash controller it will boot from and the kernel will also
> be running from so it cannot be turned off.
>
> Signed-off-by: Chris Brandt <chris.brandt@xxxxxxxxxxx>

Thanks for your patch!

> --- a/arch/arm/boot/dts/r7s72100.dtsi
> +++ b/arch/arm/boot/dts/r7s72100.dtsi
> @@ -101,6 +101,26 @@
>                 #size-cells = <1>;
>                 ranges;
>
> +               spibsc0: spi@3fefa000 {
> +                       compatible = "renesas,r7s72100-spibsc", "renesas,spibsc";
> +                       reg = <0x3fefa000 0x100>, <0x18000000 0x4000000>;

The second region conflicts with the XIP flash@18000000 in
arch/arm/boot/dts/r7s72100-gr-peach.dts.
Yes, I know it is the same device ;-)

> +                       clocks = <&mstp9_clks R7S72100_CLK_SPIBSC0>;
> +                       power-domains = <&cpg_clocks>;
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       status = "disabled";
> +               };
> +
> +               spibsc1: spi@3fefb000 {
> +                       compatible = "renesas,r7s72100-spibsc", "renesas,spibsc";
> +                       reg = <0x3fefb000 0x100>, <0x1c000000 0x4000000>;
> +                       clocks = <&mstp9_clks R7S72100_CLK_SPIBSC1>;
> +                       power-domains = <&cpg_clocks>;
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +                       status = "disabled";
> +               };
> +
>                 L2: cache-controller@3ffff000 {
>                         compatible = "arm,pl310-cache";
>                         reg = <0x3ffff000 0x1000>;
> @@ -467,11 +487,13 @@
>                         #clock-cells = <1>;
>                         compatible = "renesas,r7s72100-mstp-clocks", "renesas,cpg-mstp-clocks";
>                         reg = <0xfcfe0438 4>;
> -                       clocks = <&p0_clk>, <&p0_clk>, <&p0_clk>, <&p0_clk>;
> +                       clocks = <&p0_clk>, <&p0_clk>, <&p0_clk>, <&p0_clk>, <&b_clk>, <&b_clk>;
>                         clock-indices = <
>                                 R7S72100_CLK_I2C0 R7S72100_CLK_I2C1 R7S72100_CLK_I2C2 R7S72100_CLK_I2C3
> +                               R7S72100_CLK_SPIBSC0 R7S72100_CLK_SPIBSC1
>                         >;
> -                       clock-output-names = "i2c0", "i2c1", "i2c2", "i2c3";
> +                       clock-output-names = "i2c0", "i2c1", "i2c2", "i2c3", "spibsc0", "spibsc1";
> +                       clock-critical = <4>; /* spibsc0 */

Iff we go this clock-critical route, I think this should be specified in the
board-specific .dts instead of in the SoC-specific .dtsi.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds