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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Thu, Sep 24, 2015 at 5:36 AM, Jean-Francois Moine <moinejf@xxxxxxx> wrote:
> On Mon, 21 Sep 2015 10:19:18 -0500
> Rob Herring <robh@xxxxxxxxxx> wrote:
>
>> On 09/18/2015 06:06 AM, Jyri Sarha wrote:
>> > From: Jean-Francois Moine <moinejf@xxxxxxx>
>> >
>> > 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@xxxxxxx>
>> > Signed-off-by: Jyri Sarha <jsarha@xxxxxx>
>> > ---
>         [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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux