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@xxxxxx>
> > ---
> > .../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 ?
> > + 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" ?
>
> > + - 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.
>
> > + 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.
> > 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@xxxxxx>
> > + */
> > +
> > +#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 */
--
Regards,
Laurent Pinchart
