Received-SPF: pass (google.com: domain of linux-kernel+bounces-110555-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <e6994ff1-6c32-4dac-846a-5f4f18b2f996@gmail.com>
Date: Thu, 21 Mar 2024 20:31:33 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 13/13] ASoC: ti: davinci-i2s: Opitonally drive DX pin
 during capture streams
To: Bastien Curutchet <bastien.curutchet@bootlin.com>,
 Liam Girdwood <lgirdwood@gmail.com>, Mark Brown <broonie@kernel.org>,
 Rob Herring <robh+dt@kernel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
 Conor Dooley <conor+dt@kernel.org>, Jaroslav Kysela <perex@perex.cz>,
 Takashi Iwai <tiwai@suse.com>
Cc: linux-sound@vger.kernel.org, devicetree@vger.kernel.org,
 linux-kernel@vger.kernel.org, alsa-devel@alsa-project.org,
 Thomas Petazzoni <thomas.petazzoni@bootlin.com>, herve.codina@bootlin.com,
 christophercordahi@nanometrics.ca
References: <20240315112745.63230-1-bastien.curutchet@bootlin.com>
 <20240315112745.63230-14-bastien.curutchet@bootlin.com>
 <00182d1d-ef29-457f-9e3e-6e9b57592118@gmail.com>
 <0bb26153-8bcb-475f-8892-5eb925fec538@bootlin.com>
 <7925bbe5-17e8-42cb-a5f0-4f3e06810a90@gmail.com>
 <d20dc37c-13fa-43d1-af52-dce8d8dcdd75@gmail.com>
 <5467add6-0099-4e54-a4f7-12aa149d03c7@bootlin.com>
Content-Language: en-US
From: =?UTF-8?Q?P=C3=A9ter_Ujfalusi?= <peter.ujfalusi@gmail.com>
In-Reply-To: <5467add6-0099-4e54-a4f7-12aa149d03c7@bootlin.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Hi Bastien,

On 3/21/24 17:14, Bastien Curutchet wrote:
>>> I think the definition of the 'ti,drive-dx' is somehow odd. It allows
>>> you to set it to 0x1234 and the DX pin will show 0x1234 when you capture
>>> 32bit. If you capture 16bit then it will transmit 0x12 (or 0x34?), no?
>>> If you have 4 channel capture then I won't speculate what will be on the
>>> DX pin ;)
>>>
>>> Would not be better to say that the DX pin will be driven low or high
>>> during capture _and_ disable the playback support?
>>
>> After some thinking, it might be still better to use the DX pin as GPIO
>> and either have a custom machine driver which would handle it (set low
>> when a capture trigger happens) or connect it in DAPM as a supply, bias
>> or something and ASoC would handle it automagically.
>>
>> I think that would be cleaner in many ways. What do you think?
>>
> I agree, that would be cleaner. I ran a few tests to see if that would
> work on my hardware. It doesn't ... So I looked back to the schematics
> and found two reasons :
>  * the DX pin needs to be in sync with the clock.

I'm not sure what this means, sync with which clock?

>  * the DX pin needs to be in a high-impedance state between two frames
>    so a pull-up can drive it back up. Actually, the DX pin is also
>    linked to the FSR pin so it provides the frame clock to the capture
>    stream.

Hrm, you are using the DX pin as FSR for the capture? Why not McBSP.FSR pin?

Looking back to the patch, one thing stood out: you are setting the
XDATDLY to 2.
You have some sort of T1 framing on the bus? The pullup will make the DX
line high in for the framing bit, right?
Or you simulate another FSR line with T1 framing DX?

The 'ti,drive-dx' sounds like a bad property for sure, you have T1
framing and driving the DX to certain level.
It is like DSP_A (1 bit delay) playing constant 0x2 ?

Can you use aplay /dev/zero and a DT property to select T1 framing for
the playback? Or that would be too coarse for timing the start of
playback and capture?

-- 
Péter