| Message ID | 20140320092639.48F68A6279@smtp3-g21.free.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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(+)