[PATCH RFC v4 4/8] drm/i2c: tda998x: Add support of a DT graph of ports

[Rob Herring]

On Thu, Sep 24, 2015 at 5:36 AM, Jean-Francois Moine <moinejf at free.fr> wrote:
> On Mon, 21 Sep 2015 10:19:18 -0500
> Rob Herring <robh at kernel.org> wrote:
>
>> On 09/18/2015 06:06 AM, Jyri Sarha wrote:
>> > From: Jean-Francois Moine <moinejf at free.fr>
>> >
>> > Two kinds of ports may be declared in a DT graph of ports: video and audio.
>> > This patch accepts the port value from a video port as an alternative
>> > to the video-ports property.
>> > It also accepts audio ports in the case the transmitter is not used as
>> > a slave encoder.
>> > The new file include/sound/tda998x.h prepares to the definition of
>> > a tda998x CODEC.
>> >
>> > Signed-off-by: Jean-Francois Moine <moinejf at free.fr>
>> > Signed-off-by: Jyri Sarha <jsarha at ti.com>
>> > ---
>         [snip]
>> > diff --git a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt
>> > index e9e4bce..35f6a80 100644
>> > --- a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt
>> > +++ b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt
>> > @@ -16,6 +16,35 @@ Optional properties:
>> >
>> >    - video-ports: 24 bits value which defines how the video controller
>> >     output is wired to the TDA998x input - default: <0x230145>
>> > +   This property is not used when ports are defined.
>> > +
>> > +Optional nodes:
>> > +
>> > +  - port: up to three ports.
>> > +   The ports are defined according to [1].
>> > +
>> > +    Video port.
>> > +   There may be only one video port.
>> > +   This one must contain the following property:
>> > +
>> > +   - port-type: must be "rgb"
>>
>> Why do you need this if there is no other choice? The port number should
>> define which one is video.
>
> There is no specific port number.
> One of the ports is video and two other ones are audio.

I saying you should define the port numbering in the binding. Port 0
is video, Port 1 is i2s, etc.

>
>> > +
>> > +   and may contain the optional property:
>> > +
>> > +   - reg: 24 bits value which defines how the video controller
>> > +           output is wired to the TDA998x input (video pins)
>> > +           When absent, the default value is <0x230145>.
>>
>> I'm failing to decode how this value works. In any case, I would keep
>> this as a vendor specific property rather than abusing reg.
>
> This has been explained in
> https://lkml.org/lkml/2014/1/20/86

Okay, so that explains how it works, but still it should not be in reg.

>
>> > +
>> > +    Audio ports.
>> > +   There may be one or two audio ports.
>> > +   These ones must contain the following properties:
>> > +
>> > +   - port-type: must be "i2s" or "spdif"
>> > +
>> > +   - reg: 8 bits value which defines how the audio controller
>> > +           output is wired to the TDA998x input (audio pins)
>>
>> reg is 32-bits.
>
> This port has only 8 significant bits as explained in
> https://lkml.org/lkml/2015/1/7/362

Maybe only 8-bits are used, but the size of the data in the DTB is
32-bits unless you add 8-bit annotation. Again, reg is not appropriate
to use here.

Rob