Message ID | 20140119195844.0c975b90@armhf (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Hi, On Sun, Jan 19, 2014 at 10:58 AM, Jean-Francois Moine <moinejf@free.fr> wrote: > Signed-off-by: Jean-Francois Moine <moinejf@free.fr> > --- > .../devicetree/bindings/drm/i2c/tda998x.txt | 24 ++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > create mode 100644 Documentation/devicetree/bindings/drm/i2c/tda998x.txt Please cc bindings for review to devicetree@vger.kernel.org (cc:d here now) > diff --git a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt > new file mode 100644 > index 0000000..72da71d > --- /dev/null > +++ b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt > @@ -0,0 +1,24 @@ > +Device-Tree bindings for the NXP TDA998x HDMI transmitter > + > +Required properties; > + - compatible: must be "nxp,tda998x" > + > +Optional properties: > + - interrupts: interrupt number for HDMI exchanges - default: by polling What are HDMI exchanges, and how do they differ from other interrupts? > + > + - pinctrl-0: pin control group to be used for this controller (IRQ). > + > + - pinctrl-names: must contain a "default" entry. > + > + - video-ports: 24 bits value - default: <0x230145> What is this? -Olof
On Sun, 19 Jan 2014 20:06:09 -0800 Olof Johansson <olof@lixom.net> wrote: > Hi, > > On Sun, Jan 19, 2014 at 10:58 AM, Jean-Francois Moine <moinejf@free.fr> wrote: > > Signed-off-by: Jean-Francois Moine <moinejf@free.fr> > > --- > > .../devicetree/bindings/drm/i2c/tda998x.txt | 24 ++++++++++++++++++++++ > > 1 file changed, 24 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/drm/i2c/tda998x.txt > > Please cc bindings for review to devicetree@vger.kernel.org (cc:d here now) > > > diff --git a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt > > new file mode 100644 > > index 0000000..72da71d > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt > > @@ -0,0 +1,24 @@ > > +Device-Tree bindings for the NXP TDA998x HDMI transmitter > > + > > +Required properties; > > + - compatible: must be "nxp,tda998x" > > + > > +Optional properties: > > + - interrupts: interrupt number for HDMI exchanges - default: by polling > > What are HDMI exchanges, and how do they differ from other interrupts? The used HDMI interrupt events are screen plug/unplug and EDID read. There are also CEC read/write which are not yet implemented in the tda998x driver. There is no difference from normal interrupts, except that the events may be get by polling, so, the interrupt number is optional. > > + > > + - pinctrl-0: pin control group to be used for this controller (IRQ). > > + > > + - pinctrl-names: must contain a "default" entry. > > + > > + - video-ports: 24 bits value - default: <0x230145> > > What is this? The video-ports value defines how the video controller is connected to the tda998x chip. Each 4 bits value tells from which input pins comes the video data and if there is any bit inversion. Each byte of this video-ports is used to load the VIP_CNTRL_{0,1,2} registers. These ones are described in the TDA9983B documentation which is the closer available document about the TDA998x family. The default value is the one defined for TI boards. A known other value is <0x234501> which is used for Russell's Armada DRM driver in the Cubox (Marvell A510), but this driver has no DT support.
On Mon, Jan 20, 2014 at 1:54 AM, Jean-Francois Moine <moinejf@free.fr> wrote: > On Sun, 19 Jan 2014 20:06:09 -0800 > Olof Johansson <olof@lixom.net> wrote: > >> Hi, >> >> On Sun, Jan 19, 2014 at 10:58 AM, Jean-Francois Moine <moinejf@free.fr> wrote: >> > Signed-off-by: Jean-Francois Moine <moinejf@free.fr> >> > --- >> > .../devicetree/bindings/drm/i2c/tda998x.txt | 24 ++++++++++++++++++++++ >> > 1 file changed, 24 insertions(+) >> > create mode 100644 Documentation/devicetree/bindings/drm/i2c/tda998x.txt >> >> Please cc bindings for review to devicetree@vger.kernel.org (cc:d here now) >> >> > diff --git a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt >> > new file mode 100644 >> > index 0000000..72da71d >> > --- /dev/null >> > +++ b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt >> > @@ -0,0 +1,24 @@ >> > +Device-Tree bindings for the NXP TDA998x HDMI transmitter >> > + >> > +Required properties; >> > + - compatible: must be "nxp,tda998x" >> > + >> > +Optional properties: >> > + - interrupts: interrupt number for HDMI exchanges - default: by polling >> >> What are HDMI exchanges, and how do they differ from other interrupts? > > The used HDMI interrupt events are screen plug/unplug and EDID read. > There are also CEC read/write which are not yet implemented in the > tda998x driver. > > There is no difference from normal interrupts, except that the events > may be get by polling, so, the interrupt number is optional.] Ok, then it looks like the property description is a little confusing. I'd remove the mentioning of HDMI exchanges from it. >> > + >> > + - pinctrl-0: pin control group to be used for this controller (IRQ). >> > + >> > + - pinctrl-names: must contain a "default" entry. >> > + >> > + - video-ports: 24 bits value - default: <0x230145> >> >> What is this? > > The video-ports value defines how the video controller is connected to > the tda998x chip. Each 4 bits value tells from which input pins comes > the video data and if there is any bit inversion. Each byte of this > video-ports is used to load the VIP_CNTRL_{0,1,2} registers. These ones > are described in the TDA9983B documentation which is the closer > available document about the TDA998x family. > > The default value is the one defined for TI boards. > A known other value is <0x234501> which is used for Russell's Armada > DRM driver in the Cubox (Marvell A510), but this driver has no DT > support. Ok, this is a classic case where the binding should describe how things are configured/wired up instead of hardcoding a register value like this. From the data sheet there seems to be a _lot_ of settings, so selecting the needed subset for now might be acceptable (and go with defaults on the rest). -Olof
On Tue, Jan 21, 2014 at 10:17:56AM -0800, Olof Johansson wrote: > Ok, this is a classic case where the binding should describe how > things are configured/wired up instead of hardcoding a register value > like this. From the data sheet there seems to be a _lot_ of settings, > so selecting the needed subset for now might be acceptable (and go > with defaults on the rest). The issue is not that simple, because the register is kind of backwards. Rather than describing what is on each nibble of the input bus, you specify how each nibble of the internal bus maps to the external world. The internal bus is mostly fixed as to which nibbles are red, green and blue (there is internal support for swapping red/blue on the internal bus, but we don't use that - it just adds extra unnecessary headaches to the mapping.) The way I ended up describing it in platform data was via the individual nibble settings for the register: .swap_a = 2, .swap_b = 3, .swap_c = 4, .swap_d = 5, .swap_e = 0, .swap_f = 1, which gives you a way to refer back to the data sheet. There's also the control whether the nibble is bit-big-endian or bit-little-endian. So basically, swap_a controls which nibble of the input bus is mapped to the internal bus bits 23:20, and mirror_a whether it's bit 0123 -> 0123 or 0123 -> 3210. This means it's possible to tell the chip that each nibble of the interal bus comes from the same four pins! If we want to describe what each nibble set on the input pins does, it's going to make for some complex code to convert that from "forward" notation (which we'll have to invent) to the "reverse" notation that the chip needs.
diff --git a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt new file mode 100644 index 0000000..72da71d --- /dev/null +++ b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt @@ -0,0 +1,24 @@ +Device-Tree bindings for the NXP TDA998x HDMI transmitter + +Required properties; + - compatible: must be "nxp,tda998x" + +Optional properties: + - interrupts: interrupt number for HDMI exchanges - default: by polling + + - pinctrl-0: pin control group to be used for this controller (IRQ). + + - pinctrl-names: must contain a "default" entry. + + - video-ports: 24 bits value - default: <0x230145> + +Example: + + tda998x: hdmi-encoder { + compatible = "nxp,tda998x"; + reg = <0x70>; + interrupt-parent = <&gpio0>; + interrupts = <27 2>; /* falling edge */ + pinctrl-0 = <&pmx_camera>; + pinctrl-names = "default"; + };
Signed-off-by: Jean-Francois Moine <moinejf@free.fr> --- .../devicetree/bindings/drm/i2c/tda998x.txt | 24 ++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 Documentation/devicetree/bindings/drm/i2c/tda998x.txt