Messages in this thread | | | From | Geert Uytterhoeven <> | Date | Wed, 16 Dec 2020 15:55:20 +0100 | Subject | Re: [PATCH 01/18] arm64: dts: renesas: beacon kit: Configure programmable clocks |
| |
Hi Adam,
On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@gmail.com> wrote: > When the board was added, clock drivers were being updated done at > the same time to allow the versaclock driver to properly configure > the modes. Unforutnately, the updates were not applied to the board
Unfortunately
> files at the time they should have been, so do it now. > > Signed-off-by: Adam Ford <aford173@gmail.com>
Thanks for your patch!
> --- a/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi > +++ b/arch/arm64/boot/dts/renesas/beacon-renesom-baseboard.dtsi > @@ -5,6 +5,7 @@ > > #include <dt-bindings/gpio/gpio.h> > #include <dt-bindings/input/input.h> > +#include <dt-bindings/clk/versaclock.h> > > / { > backlight_lvds: backlight-lvds { > @@ -294,12 +295,12 @@ &du_out_rgb { > &ehci0 { > dr_mode = "otg"; > status = "okay"; > - clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>; > + clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>, <&versaclock5 3>;
Why this change? You said before you don't need this https://lore.kernel.org/linux-renesas-soc/CAHCN7xJWbP16SA-Ok-5syNnqOZAt8OFJo2_rtg5VrNVsN2-eiQ@mail.gmail.com/
BTW, something I missed in the earlier review: is there an override needed at all?
> }; > > &ehci1 { > status = "okay"; > - clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>; > + clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>, <&versaclock5 3>;
Same here.
BTW, something I missed in the earlier review: why did you override
clocks = <&cpg CPG_MOD 702>;
by
clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>;
?
> }; > > &hdmi0 { > @@ -373,12 +374,40 @@ versaclock6_bb: clock-controller@6a { > #clock-cells = <1>; > clocks = <&x304_clk>; > clock-names = "xin"; > - /* CSI0_MCLK, CSI1_MCLK, AUDIO_CLKIN, USB_HUB_MCLK_BB */ > + clock-output-names = "versaclock6_bb.out0_sel_i2cb", > + "versaclock6_bb.out1", > + "versaclock6_bb.out2", > + "versaclock6_bb.out3", > + "versaclock6_bb.out4";
Why? IIUIC, the driver doesn't parse clock-output-names (and it shouldn't).
> assigned-clocks = <&versaclock6_bb 1>, > <&versaclock6_bb 2>, > <&versaclock6_bb 3>, > <&versaclock6_bb 4>; > assigned-clock-rates = <24000000>, <24000000>, <24000000>, <24576000>; > + > + OUT1 { > + idt,mode = <VC5_CMOS>; > + idt,voltage-microvolts = <1800000>;
Oops. The DT bindings say "idt,voltage-microvolt", the example in the DT bindings says "idt,voltage-microvolts", and the driver parses "idt,voltage-microvolts".
According to Documentation/devicetree/bindings/property-units.txt, the property name should end in "microvolt".
Patch sent. https://lore.kernel.org/linux-clk/20201216145231.1344317-1-geert+renesas@glider.be/
> + idt,slew-percent = <100>; > + };
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
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
| |