Hi Adam,
On Sun, Dec 13, 2020 at 7:38 PM Adam Ford <aford173@xxxxxxxxx> 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@xxxxxxxxx>
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@xxxxxxxxxxxxxx/
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@xxxxxxxxx/
> + idt,slew-percent = <100>;
> + };
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
