Re: [PATCH v5 9/9] arm64: dts: renesas: salvator: add a connection from adv748x codec (HDMI input) to the R-Car SoC

From: Alex Riesen <alexander.riesen@cetitec.com>
To: Kieran Bingham <kieran.bingham@ideasonboard.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Hans Verkuil <hverkuil-cisco@xs4all.nl>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>,
	devel@driverdev.osuosl.org, linux-media@vger.kernel.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org
Subject: Re: [PATCH v5 9/9] arm64: dts: renesas: salvator: add a connection from adv748x codec (HDMI input) to the R-Car SoC
Date: Mon, 14 Sep 2020 10:17:21 +0200	[thread overview]
Message-ID: <20200914081721.GA4705@pflmari> (raw)
In-Reply-To: <c7d93d6c-4893-8bd0-5859-87f30389b5d5@ideasonboard.com>

Hi Kieran,

Kieran Bingham, Tue, Aug 25, 2020 16:57:04 +0200:
> On 18/06/2020 17:32, Kieran Bingham wrote:
> > On 02/04/2020 19:35, Alex Riesen wrote:
> >> As all known variants of the Salvator board have the HDMI decoder
> >> chip (the ADV7482) connected to the SSI4 on R-Car SoC, the ADV7482
> >> endpoint and the connection definitions are placed in the common board
> >> file.
> >>
> >> For the same reason, the CLK_C clock line and I2C configuration (similar
> >> to the ak4613, on the same interface) are added into the common file.
> >> ...
> >> ---
> >>  .../boot/dts/renesas/r8a77950-salvator-x.dts  |  3 +-
> >>  arch/arm64/boot/dts/renesas/r8a77961.dtsi     |  1 +
> >>  .../boot/dts/renesas/salvator-common.dtsi     | 47 +++++++++++++++++--
> 
> Once again I'm back trying to test this series, and one issue I've had
> is that the board I have (r8a77951-salvator-xs.dts) isn't included in
> this DT update.
> 
> For v6, Should we include the relevant changes to all the following?

Ok. I shall add them as a separate patch though, as I have no way to verify
those boards (and some verification seem to be in order...)

> arch/arm64/boot/dts/renesas/r8a77950-salvator-x.dts
> arch/arm64/boot/dts/renesas/r8a77951-salvator-x.dts
> arch/arm64/boot/dts/renesas/r8a77960-salvator-x.dts
> arch/arm64/boot/dts/renesas/r8a77965-salvator-x.dts
> arch/arm64/boot/dts/renesas/salvator-x.dtsi
> 
> And perhaps handle the salvator-xs in a second (yet very similar) patch?
> 
> arch/arm64/boot/dts/renesas/r8a77951-salvator-xs.dts
> arch/arm64/boot/dts/renesas/r8a77960-salvator-xs.dts
> arch/arm64/boot/dts/renesas/r8a77961-salvator-xs.dts
> arch/arm64/boot/dts/renesas/r8a77965-salvator-xs.dts
> arch/arm64/boot/dts/renesas/salvator-xs.dtsi
> 
> I think I've added the relevant entries to my dtb, but I haven't
> successfully captured audio yet.
> 
> I can see the device being listed through arecord:
> 
> kbingham@salvator-xs:~$ arecord -l
> **** List of CAPTURE Hardware Devices ****
> card 0: rcarsound [rcar-sound], device 0: rsnd-dai.0-ak4613-hifi ak4613-hifi-0 []
>   Subdevices: 0/1
>   Subdevice #0: subdevice #0
> card 0: rcarsound [rcar-sound], device 3: rsnd-dai.3-adv748x-i2s adv748x.4-0070-3 []
>   Subdevices: 1/1
>   Subdevice #0: subdevice #0
> 
> But as yet, everything I try to record fails or is empty silence.
> 
> Debugging ...

Does it fail somewhere in the ASoC infrastructure? If so, how'd you find out
where exactly and what fails?

Asking, because when I was writing this code I ended up adding quite a bit of
tracing into the SoC core to figure that out, and I just hope there is a
better way to get at the diagnostics.

> >> diff --git a/arch/arm64/boot/dts/renesas/r8a77950-salvator-x.dts b/arch/arm64/boot/dts/renesas/r8a77950-salvator-x.dts
> >> index 2438825c9b22..e16c146808b6 100644
> >> --- a/arch/arm64/boot/dts/renesas/r8a77950-salvator-x.dts
> >> +++ b/arch/arm64/boot/dts/renesas/r8a77950-salvator-x.dts
> >> @@ -146,7 +146,8 @@ &sata {
> >>  &sound_card {
> >>  	dais = <&rsnd_port0	/* ak4613 */
> >>  		&rsnd_port1	/* HDMI0  */
> >> -		&rsnd_port2>;	/* HDMI1  */
> >> +		&rsnd_port2	/* HDMI1  */
> >> +		&rsnd_port3>;	/* adv7482 hdmi-in  */
> > 
> > Ah - that was confusing at first... but HDMI0 and HDMI1 are *outputs*,
> > where of course the adv7482 is an input ;-)

I shall add an "output" to HDMI0 and HDMI1.

> > Otherwise, I can't spot anything else yet so:
> > 
> > Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

Thanks!

> > But I fear there may have been some churn around here, so it would be
> > good to see a rebase too.

Of course, I shall rebase on top of linux-media/master.
Should I wait with submission until you get data out of your boards?

Regards,
Alex