[PATCH v3 5/6] dt-bindings: display: sii902x: Add HDMI audio bindings

Wed Mar 13 17:52:08 UTC 2019

On 13/03/2019 18:47, Laurent Pinchart wrote:
> Hello again,
> 
> On Wed, Mar 13, 2019 at 06:29:19PM +0200, Laurent Pinchart wrote:
>> On Wed, Mar 13, 2019 at 06:01:07PM +0200, Jyri Sarha wrote:
>>> The sii902x chip family supports also HDMI audio. Add binding for
>>> describing the necessary i2s and mclk wiring for it.
>>>
>>> Signed-off-by: Jyri Sarha <jsarha at ti.com>
>>> ---
>>>  .../bindings/display/bridge/sii902x.txt       | 34 +++++++++++++++++++
>>>  include/dt-bindings/sound/sii902x-audio.h     | 17 ++++++++++
>>>  2 files changed, 51 insertions(+)
>>>  create mode 100644 include/dt-bindings/sound/sii902x-audio.h
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/bridge/sii902x.txt b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
>>> index c4c1855ca654..977756841193 100644
>>> --- a/Documentation/devicetree/bindings/display/bridge/sii902x.txt
>>> +++ b/Documentation/devicetree/bindings/display/bridge/sii902x.txt
>>> @@ -8,6 +8,29 @@ Optional properties:
>>>  	- interrupts: describe the interrupt line used to inform the host
>>>  	  about hotplug events.
>>>  	- reset-gpios: OF device-tree gpio specification for RST_N pin.
>>> +	- sil,i2s-fifo-routing: Array of exactly 4 integers indicating i2s
>>> +	  pins for audio fifo routing. First integer defines routing to
>>> +	  fifo 0 and second to fifo 1, etc. Integers can be filled with
>>> +	  definitions from: include/dt-bindings/sound/sii902x-audio.h
>>> +	  The available definitions are:
>>> +	  - ENABLE_BIT:	enable this audio fifo
>>> +	  - CONNECT_SD#: route audio input from SD0, SD1, SD2, or SD3 i2s
>>> +			 data input pin
>>> +	  - LEFT_RIGHT_SWAP_BIT: swap i2s input channels for this fifo
>>
>> Are all combinations valid ? For instance, could we have D1 routed to
>> the third FIFO, and all other FIFOs disabled ?
> 
> I found the answer to this question in the datasheet:
> 
> "Note that no gaps are allowed when mapping channels to FIFOs – SD pins
> must be mapped to FIFO#0 and FIFO#1 before mapping a channel to FIFO#2,
> and so on."
> 
> I think we can thus simplify the bindings, and use an approach similar
> to the one taken by the data-lanes property for CSI-2. Furthermore, I
> think this should be standardized, not left device-specific.
> 
> How about an sd-lanes property (better names are welcome) that would
> store an array of N integers, where each sd-lanes[i] tells which SD pin
> the i-th FIFO is connected to ?

I agree otherwise, but I would still rather use i2s than sd, because it
is more explicit. SD overlaps with so many other acronyms. So how would
i2s-lanes sound?

> 
>>> +	  I2S HDMI audio is configured only if this property is found.
>>> +	- clocks: phandle mclk
>>
>> Maybe "clocks: phandle and clock specifier for each clock listed in the clock-names property" ?
>>

Ok

>>> +	- clock-names: "mclk"
>>> +	    Describes SII902x MCLK input. MCLK is used to produce
>>> +	    HDMI audio CTS values. This property is required if
>>> +	    "i2s-fifo-routing"-property is present. This property follows
>>
>> The property is named sil,i2s-fifo-routing.
>>

Ok

>>> +	    Documentation/devicetree/bindings/clock/clock-bindings.txt
>>> +	    consumer binding.
>>> +	- #sound-dai-cells = <0>: ASoC codec dai available for simple-card
>>> +	    If audio properties are present sii902x provides an ASoC
>>> +	    codec component driver that can be used by other ASoC
>>> +	    components like simple-card. See binding document for
>>> +	    details:
>>> +	    Documentation/devicetree/bindings/sound/simple-card.txt
>>>  
>>>  Optional subnodes:
>>>  	- video input: this subnode can contain a video input port node
>>> @@ -21,6 +44,17 @@ Example:
>>>  		compatible = "sil,sii9022";
>>>  		reg = <0x39>;
>>>  		reset-gpios = <&pioA 1 0>;
>>> +
>>> +		#sound-dai-cells = <0>;
>>> +		sil,i2s-fifo-routing = <
>>> +			(ENABLE_BIT|CONNECT_SD0)
>>> +			0
>>> +			0
>>> +			0
>>> +		>;
>>> +		clocks = <&mclk>;
>>> +		clock-names = "mclk";
>>> +
>>>  		ports {
>>>  			#address-cells = <1>;
>>>  			#size-cells = <0>;
> 
> I also forgot to mention that the audio connection should be modeled
> using OF graph too.
> 

With the audio configuration sii902x becomes an ASoC codec component.
There are current more than one way to describe the connections between
ASoC components (see [1] and [2] for details). The individual ASoC DAI
drivers (digital audio interface) do not need to know about the audio
system topology, that is taken care of by the machine driver (or card
driver if you will), for instance simple-card or audio-graph-card.

However, since sii902x device is agnostic about the way the connections
are described, I don't think I should go into too many details about how
the possible bindings. However, I should add a reference at least to
audio-graph-card too.

[1] Documentation/devicetree/bindings/sound/simple-card.txt
[2] Documentation/devicetree/bindings/sound/audio-graph-card.txt

>>> diff --git a/include/dt-bindings/sound/sii902x-audio.h b/include/dt-bindings/sound/sii902x-audio.h
>>> new file mode 100644
>>> index 000000000000..0a849904754b
>>> --- /dev/null
>>> +++ b/include/dt-bindings/sound/sii902x-audio.h
>>> @@ -0,0 +1,17 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
>>> + * Author: Jyri Sarha <jsarha at ti.com>
>>> + */
>>> +
>>> +#ifndef __DT_SII9022_AUDIO_H
>>> +#define __DT_SII9022_AUDIO_H
>>> +
>>> +#define ENABLE_BIT			0x80
>>> +#define CONNECT_SD0			0x00
>>> +#define CONNECT_SD1			0x10
>>> +#define CONNECT_SD2			0x20
>>> +#define CONNECT_SD3			0x30
>>> +#define LEFT_RIGHT_SWAP_BIT		0x04
>>
>> This is fairly generic, should you prefix the macros with SII9022 ?
>>
>>> +
>>> +#endif /* __DT_SII9022_AUDIO_H */
> 

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki