lkml.org 
[lkml]   [2020]   [Dec]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [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

    \
     
     \ /
      Last update: 2020-12-16 15:57    [W:4.732 / U:0.048 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site