lkml.org 
[lkml]   [2020]   [Mar]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH 8/8] arm64: dts: renesas: salvator: add a connection from adv748x codec (HDMI input) to the R-Car SoC
    Hi Alex,

    Thanks for your patch!

    On Mon, Jan 13, 2020 at 3:24 PM Alex Riesen
    <alexander.riesen@cetitec.com> wrote:
    > Not sure if all variants of the Salvator board have the HDMI decoder
    > chip (the ADV7482) connected to the SSI4 on R-Car SoC, as it is on
    > Salvator-X ES1, so the the ADV7482 endpoint and connection definitions
    > are placed in the board file.

    Both Salvator-X and Salvator-XS have SSI4 wired to the ADV7482.

    > I do assume though that all Salvator variants have the CLK_C clock line
    > hard-wired to the ADV7482 HDMI decoder, and remove it from the list of
    > clocks provided by the R-Car sound system.

    Yes, both Salvator-X and Salvator-XS have it wired that way. But please
    see below.

    > The I2C wiring is also likely to persist across the variants (similar
    > to ak4613, connected to the same interface), so that is in the common
    > file.
    >
    > Signed-off-by: Alexander Riesen <alexander.riesen@cetitec.com>

    Below are my comments w.r.t. the board-specific wiring.
    I'll defer to the multimedia people for commenting on the audio parts.

    BTW, what is the status of the other patches in this series?

    > --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
    > +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
    > @@ -322,6 +322,10 @@
    > clock-frequency = <22579200>;
    > };
    >
    > +&audio_clk_c {
    > + clock-frequency = <12288000>;
    > +};

    Does the ADV7482 always generate a 12.288 MHz clock signal?
    Or is this programmable?

    > +
    > &avb {
    > pinctrl-0 = <&avb_pins>;
    > pinctrl-names = "default";
    > @@ -471,12 +475,14 @@
    >
    > #address-cells = <1>;
    > #size-cells = <0>;
    > + #sound-dai-cells = <0>;
    >
    > interrupt-parent = <&gpio6>;
    > interrupt-names = "intrq1", "intrq2";
    > interrupts = <30 IRQ_TYPE_LEVEL_LOW>,
    > <31 IRQ_TYPE_LEVEL_LOW>;
    > -
    > + clocks = <&rcar_sound 3>, <&audio_clk_c>;
    > + clock-names = "clk-hdmi-video", "clk-hdmi-i2s-mclk";

    The above declares the Audio CLK C to be a clock input of the ADV7482, while
    it is an output.
    Furthermore, the DT bindings do not document that clocks can be specified.

    > port@7 {
    > reg = <7>;
    >
    > @@ -512,6 +518,14 @@
    > remote-endpoint = <&csi20_in>;
    > };
    > };
    > +
    > + port@c {
    > + reg = <12>;
    > +
    > + adv7482_i2s: endpoint {
    > + /* remote-endpoint defined in the board file */
    > + };
    > + };
    > };
    >
    > csa_vdd: adc@7c {
    > @@ -686,7 +700,8 @@
    > };
    >
    > sound_pins: sound {
    > - groups = "ssi01239_ctrl", "ssi0_data", "ssi1_data_a";
    > + groups = "ssi01239_ctrl", "ssi0_data", "ssi1_data_a",
    > + "ssi4_data";

    Missing "ss4_ctrl", for the SCK4 and WS4 pins.

    > function = "ssi";
    > };
    >
    > @@ -735,8 +750,8 @@
    > pinctrl-0 = <&sound_pins &sound_clk_pins>;
    > pinctrl-names = "default";
    >
    > - /* Single DAI */
    > - #sound-dai-cells = <0>;
    > + /* multi DAI */
    > + #sound-dai-cells = <1>;
    >
    > /* audio_clkout0/1/2/3 */
    > #clock-cells = <1>;
    > @@ -760,8 +775,18 @@
    > <&cpg CPG_MOD 1020>, <&cpg CPG_MOD 1021>,
    > <&cpg CPG_MOD 1019>, <&cpg CPG_MOD 1018>,
    > <&audio_clk_a>, <&cs2000>,
    > - <&audio_clk_c>,

    Why remove it? This is the list of clock inputs, not outputs.

    > <&cpg CPG_CORE CPG_AUDIO_CLK_I>;
    > + clock-names = "ssi-all",
    > + "ssi.9", "ssi.8", "ssi.7", "ssi.6",
    > + "ssi.5", "ssi.4", "ssi.3", "ssi.2",
    > + "ssi.1", "ssi.0",
    > + "src.9", "src.8", "src.7", "src.6",
    > + "src.5", "src.4", "src.3", "src.2",
    > + "src.1", "src.0",
    > + "mix.1", "mix.0",
    > + "ctu.1", "ctu.0",
    > + "dvc.0", "dvc.1",
    > + "clk_a", "clk_b", "clk_i";
    >
    > ports {
    > #address-cells = <1>;
    > --
    > 2.24.1.508.g91d2dafee0

    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-03-02 13:28    [W:3.116 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site