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

[Jyri Sarha <jsarha@ti.com>]

On 13/03/2019 20:12, Laurent Pinchart wrote:
> Hi Jyri,
> 
> On Wed, Mar 13, 2019 at 07:52:08PM +0200, Jyri Sarha wrote:
>> On 13/03/2019 18:47, Laurent Pinchart wrote:
>>> 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@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?
> Sounds good to me. It's a better name, so it's welcome :-) I don't know
> what terminology is usually used in the audio world for this, so I was
> pretty sure my initial name proposal was bad.
> 
> Is there a risk of needing to describe the clock lane separately in the
> future (for this or another I2S-related chip) ? If so, maybe
> i2s-data-lanes, or just data-lanes, would be a better pick.
> 

Usually (or always AFAIK) there is only one bit clock and one frame
clock lane, and 1 - n data lanes. But still being more explicit does not
hurt, let's make it i2s-data-lanes.

Thanks for the prompt review. I'll try to make the changes for tomorrow.

Best regards,
Jyri

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel