Hi Alex,
Thanks for your patch!
On Mon, Jan 13, 2020 at 3:24 PM Alex Riesen
<alexander.riesen@xxxxxxxxxxx> 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@xxxxxxxxxxx>
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@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
