| Message ID | 20140320092639.48F68A6279@smtp3-g21.free.fr |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 03/20/2014 09:58 AM, Jean-Francois Moine wrote: > The I2C address (reg) is required for the TDA998x driver to be loaded > and initialized. > > Signed-off-by: Jean-Francois Moine <moinejf@free.fr> > --- > This patch applies to linux-next. > --- > Documentation/devicetree/bindings/drm/i2c/tda998x.txt | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt > index d7df01c..fc7effa 100644 > --- a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt > +++ b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt > @@ -3,6 +3,8 @@ Device-Tree bindings for the NXP TDA998x HDMI transmitter > Required properties; > - compatible: must be "nxp,tda998x" > > + - reg: I2C address - must be <0x70> TDA9983b datasheet says: "Bits A0 and A1 of the I2C-bus device address are externally selected by pins A0 and A1." Therefore, 0x70, 0x71, 0x72, and 0x73 are valid i2c addresses. Sebastian > Optional properties: > - interrupts: interrupt number and trigger type > default: polling > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Mar 20, 2014 at 01:32:24PM +0100, Sebastian Hesselbarth wrote: > On 03/20/2014 09:58 AM, Jean-Francois Moine wrote: >> The I2C address (reg) is required for the TDA998x driver to be loaded >> and initialized. >> >> Signed-off-by: Jean-Francois Moine <moinejf@free.fr> >> --- >> This patch applies to linux-next. >> --- >> Documentation/devicetree/bindings/drm/i2c/tda998x.txt | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt >> index d7df01c..fc7effa 100644 >> --- a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt >> +++ b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt >> @@ -3,6 +3,8 @@ Device-Tree bindings for the NXP TDA998x HDMI transmitter >> Required properties; >> - compatible: must be "nxp,tda998x" >> >> + - reg: I2C address - must be <0x70> > > TDA9983b datasheet says: > > "Bits A0 and A1 of the I2C-bus device address are externally selected > by pins A0 and A1." > > Therefore, 0x70, 0x71, 0x72, and 0x73 are valid i2c addresses. Thanks Sebastian, those kinds of details are very important to get right when it comes to the binding doc, much more so than what's in the code. It probably makes more sense not to specify what the address actually is. After all, that's a specification by the data sheet and software shouldn't really care what that is.
On Thu, 20 Mar 2014 13:32:24 +0100 Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> wrote: > > + - reg: I2C address - must be <0x70> > > TDA9983b datasheet says: > > "Bits A0 and A1 of the I2C-bus device address are externally selected > by pins A0 and A1." > > Therefore, 0x70, 0x71, 0x72, and 0x73 are valid i2c addresses. Sebastian, That's interesting! For the Cubox and the AMX33XX boards, the I2C address of the HDMI registers is 0x70, and the I2C address of the CEC registers is 0x34. For other boards using the TDA998x family, if the I2C address is different from 0x70, have you an idea about what can be the CEC I2C address? (this value is actually hard-coded in the TDA998x driver)
On Thu, 20 Mar 2014 14:01:56 +0100 Jean-Francois Moine <moinejf@free.fr> wrote: > For other boards using the TDA998x family, if the I2C address is > different from 0x70, have you an idea about what can be the CEC I2C > address? (this value is actually hard-coded in the TDA998x driver) I had a look again on the tda998x driver from NXP and the linux tda998x driver: - the NXP driver handles the TDAs 19989, 19988, 9984, 9983 and 9981. It accesses the tda registers only at I2C address 0x70 and 0x34 (hard-coded values in the driver). - the linux tda998x driver handles only the TDAs 19988 and 19989. As we have no documentation about the chips TDA 19988 and 19989, may the HDMI I2C addresses of these chips be different from 0x70?
On Thu, Mar 20, 2014 at 02:01:56PM +0100, Jean-Francois Moine wrote: > On Thu, 20 Mar 2014 13:32:24 +0100 > Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> wrote: > > > > + - reg: I2C address - must be <0x70> > > > > TDA9983b datasheet says: > > > > "Bits A0 and A1 of the I2C-bus device address are externally selected > > by pins A0 and A1." > > > > Therefore, 0x70, 0x71, 0x72, and 0x73 are valid i2c addresses. > > Sebastian, > > That's interesting! > > For the Cubox and the AMX33XX boards, the I2C address of the HDMI > registers is 0x70, and the I2C address of the CEC registers is 0x34. > > For other boards using the TDA998x family, if the I2C address is > different from 0x70, have you an idea about what can be the CEC I2C > address? (this value is actually hard-coded in the TDA998x driver) For the TDA9983, it's configurable as Sebastian says. For the TDA19988, they're fixed at 0x70 and 0x34. To put it another way: it is chip specific, and is specified by the data sheet, not by the software, and should not be specified as an "required value" with any particular value in the DT binding specification.
On 03/20/2014 02:01 PM, Jean-Francois Moine wrote: > On Thu, 20 Mar 2014 13:32:24 +0100 > Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> wrote: > >>> + - reg: I2C address - must be <0x70> >> >> TDA9983b datasheet says: >> >> "Bits A0 and A1 of the I2C-bus device address are externally selected >> by pins A0 and A1." >> >> Therefore, 0x70, 0x71, 0x72, and 0x73 are valid i2c addresses. > > Sebastian, > > That's interesting! > > For the Cubox and the AMX33XX boards, the I2C address of the HDMI > registers is 0x70, and the I2C address of the CEC registers is 0x34. > > For other boards using the TDA998x family, if the I2C address is > different from 0x70, have you an idea about what can be the CEC I2C > address? (this value is actually hard-coded in the TDA998x driver) > Ok, I had another round of google'ing and found this: http://hipstercircuits.com/wp-content/uploads/2013/05/TDA19988.pdf There, the datasheet specifically for TDA19988 only states 0x70 and 0x34 as the two i2c addresses. Therefore, TDA19988 has fixed i2c addresses while TDA9983b has configurable (main) i2c address. Not as easy as we thought ;) I suggest reword the reg property to: "- reg: shall be set to the I2C address" and optionally list all known addresses for each TDA[1]998x in the binding. Sebastian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 20 Mar 2014 14:32:18 +0100 Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> wrote: > Ok, I had another round of google'ing and found this: > http://hipstercircuits.com/wp-content/uploads/2013/05/TDA19988.pdf > > There, the datasheet specifically for TDA19988 only states 0x70 and > 0x34 as the two i2c addresses. Therefore, TDA19988 has fixed i2c > addresses while TDA9983b has configurable (main) i2c address. > > Not as easy as we thought ;) > > I suggest reword the reg property to: > "- reg: shall be set to the I2C address" > > and optionally list all known addresses for each TDA[1]998x in the > binding. Thanks for the link. OK, then, as the linux tda998x driver handles only the tda 19988 and 19989 chips, the HDMI I2C address is always 0x70. So, question: Russell and Sebastian, do you still want an other patch? Other question: the CEC address is hard-coded to 0x34 in the driver. Should it be configurable in the DT?
On 03/20/2014 02:52 PM, Jean-Francois Moine wrote: > On Thu, 20 Mar 2014 14:32:18 +0100 > Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> wrote: > >> Ok, I had another round of google'ing and found this: >> http://hipstercircuits.com/wp-content/uploads/2013/05/TDA19988.pdf >> >> There, the datasheet specifically for TDA19988 only states 0x70 and >> 0x34 as the two i2c addresses. Therefore, TDA19988 has fixed i2c >> addresses while TDA9983b has configurable (main) i2c address. >> >> Not as easy as we thought ;) >> >> I suggest reword the reg property to: >> "- reg: shall be set to the I2C address" >> >> and optionally list all known addresses for each TDA[1]998x in the >> binding. > > Thanks for the link. > > OK, then, as the linux tda998x driver handles only the tda 19988 and > 19989 chips, the HDMI I2C address is always 0x70. > > So, question: Russell and Sebastian, do you still want an other patch? Up to Russell, but reg property is required for i2c slaves and should be documented. > Other question: the CEC address is hard-coded to 0x34 in the driver. > Should it be configurable in the DT? Looking at nxp's website, detailed information about TDA[1]998x have vanished. Luckily, there are some hints in the parametric search: There is TDA998[14] without any CEC support and TDA1998[89] with CEC support. Both TDA19988 and TDA19989 have fixed i2c addresses 0x70 and 0x34 respectively. So, the answer is: Let the driver access CEC i2c slave only if it is TDA1998[89]. The HDMI part should be quite compatible with TDA998x, so if anyone has it on his board, he is invited to add proper support. For the binding, reg address should contain reg property to the HDMI i2c slave. Add proper compatibles/i2c_ids for nxp,tda19988 and nxp,tda19989 and check that to add the CEC i2c slave on 0x34. If, someday there will be a tda19990 with configurable addresses, I am sure you can derive it from i2c main address, e.g. 0x70<>0x34, 0x71<>0x35, aso. No need to take care of configurable CEC slave address now, neither in the driver nor binding. Sebastian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Mar 20, 2014 at 02:52:21PM +0100, Jean-Francois Moine wrote: > Thanks for the link. > > OK, then, as the linux tda998x driver handles only the tda 19988 and > 19989 chips, the HDMI I2C address is always 0x70. > > So, question: Russell and Sebastian, do you still want an other patch? > > Other question: the CEC address is hard-coded to 0x34 in the driver. > Should it be configurable in the DT? As we haven't had a mainline non-rc kernel release with this in yet, we have more scope in what we can do to sort this out. What I'd suggest is: 1. change the DT compatible strings the driver has to accept both nxp,tda19988 and nxp,tda19989, and set the appropriate device in the DT file (tda19988). I'm a bit nervous about using "nxp,tda1998x" in case we're clashing with devices with different characteristics. 2. specify that the i2c reg address must exist, but not specify what it should be - leave that open. 3. assume that there's a CEC at 0x34 for these two devices. If we wish to extend support to tda998x, then we'd need to modify the driver quite a bit anyway.
On Thu, 20 Mar 2014 14:31:10 +0000 Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > 1. change the DT compatible strings the driver has to accept both > nxp,tda19988 and nxp,tda19989, and set the appropriate device > in the DT file (tda19988). I'm a bit nervous about using > "nxp,tda1998x" in case we're clashing with devices with different > characteristics. The Cubox is sold with either the TDA19988 or the TDA19989 (I don't know about the AMX33XX boards). Then, setting the exact type in the DT would ask for 2 differents DTs or for having two tda998x definitions in a same DT...
On Thu, Mar 20, 2014 at 9:59 AM, Jean-Francois Moine <moinejf@free.fr> wrote: > On Thu, 20 Mar 2014 14:31:10 +0000 > Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > >> 1. change the DT compatible strings the driver has to accept both >> nxp,tda19988 and nxp,tda19989, and set the appropriate device >> in the DT file (tda19988). I'm a bit nervous about using >> "nxp,tda1998x" in case we're clashing with devices with different >> characteristics. > > The Cubox is sold with either the TDA19988 or the TDA19989 (I don't > know about the AMX33XX boards). Then, setting the exact type in the DT > would ask for 2 differents DTs or for having two tda998x definitions in > a same DT... fyi: On the AM335x (BeagleBone Black), the NXP's chip ink shows, 88BHN so TDA19988 Regards,
On Thu, Mar 20, 2014 at 03:59:35PM +0100, Jean-Francois Moine wrote: > On Thu, 20 Mar 2014 14:31:10 +0000 > Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > > > 1. change the DT compatible strings the driver has to accept both > > nxp,tda19988 and nxp,tda19989, and set the appropriate device > > in the DT file (tda19988). I'm a bit nervous about using > > "nxp,tda1998x" in case we're clashing with devices with different > > characteristics. > > The Cubox is sold with either the TDA19988 or the TDA19989 (I don't > know about the AMX33XX boards). Then, setting the exact type in the DT > would ask for 2 differents DTs or for having two tda998x definitions in > a same DT... I'm not saying that it has to match the physical device fitted - I'm merely suggesting not using nxp,tda1998x which could (and as Sebastian has found, does) conflict with other devices with different properties. We still auto-detect the exact device type by reading the ID register because that's the most reliable way to detect exactly what kind of device is fitted to the board.
On Thu, 20 Mar 2014 15:19:34 +0000 Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > I'm not saying that it has to match the physical device fitted - I'm > merely suggesting not using nxp,tda1998x which could (and as Sebastian > has found, does) conflict with other devices with different properties. > > We still auto-detect the exact device type by reading the ID register > because that's the most reliable way to detect exactly what kind of > device is fitted to the board. I don't see the problem. Actually the driver handles the tda9989, tda19988 and tda19989 (2 variants). If some board has, for example, the tda9983 and if the driver is extended to handle this chip (i.e. mainly ignore the CEC part), setting 'nxp,tda998x' in the associated DT will still work. There could be a problem if somebody would write a specific driver for, say, a tda9985. But, then, the compatible would be 'nxp,tda9985'.
On Thu, Mar 20, 2014 at 04:54:40PM +0100, Jean-Francois Moine wrote: > On Thu, 20 Mar 2014 15:19:34 +0000 > Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > > > I'm not saying that it has to match the physical device fitted - I'm > > merely suggesting not using nxp,tda1998x which could (and as Sebastian > > has found, does) conflict with other devices with different properties. > > > > We still auto-detect the exact device type by reading the ID register > > because that's the most reliable way to detect exactly what kind of > > device is fitted to the board. > > I don't see the problem. > > Actually the driver handles the tda9989, tda19988 and tda19989 (2 > variants). If some board has, for example, the tda9983 and if the > driver is extended to handle this chip (i.e. mainly ignore the CEC > part), setting 'nxp,tda998x' in the associated DT will still work. So you have to encode in the driver that if you see a tda9983 device, you don't touch the CEC part. Now think about how you'd handle a tda998x compatible device but with the CEC stuff at a different I2C address.
diff --git a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt index d7df01c..fc7effa 100644 --- a/Documentation/devicetree/bindings/drm/i2c/tda998x.txt +++ b/Documentation/devicetree/bindings/drm/i2c/tda998x.txt @@ -3,6 +3,8 @@ Device-Tree bindings for the NXP TDA998x HDMI transmitter Required properties; - compatible: must be "nxp,tda998x" + - reg: I2C address - must be <0x70> + Optional properties: - interrupts: interrupt number and trigger type default: polling
The I2C address (reg) is required for the TDA998x driver to be loaded and initialized. Signed-off-by: Jean-Francois Moine <moinejf@free.fr> --- This patch applies to linux-next. --- Documentation/devicetree/bindings/drm/i2c/tda998x.txt | 2 ++ 1 file changed, 2 insertions(+)