[PATCH] ASoC: tda998x: Fix lack of required reg in DT documentation

[PATCH] ASoC: tda998x: Fix lack of required reg in DT documentation

[Jean-Francois Moine]

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>
+
 Optional properties:
   - interrupts: interrupt number and trigger type
 	default: polling
-- 
1.9.1

Re: [PATCH] ASoC: tda998x: Fix lack of required reg in DT documentation

[Sebastian Hesselbarth]

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-GANU6spQydw@public.gmane.org>
> ---
> 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ASoC: tda998x: Fix lack of required reg in DT documentation

[Russell King - ARM Linux]

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.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Re: [PATCH] ASoC: tda998x: Fix lack of required reg in DT documentation

[Jean-Francois Moine]

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)

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] ASoC: tda998x: Fix lack of required reg in DT documentation

[Jean-Francois Moine]

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?

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] ASoC: tda998x: Fix lack of required reg in DT documentation

[Russell King - ARM Linux]

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.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Re: [PATCH] ASoC: tda998x: Fix lack of required reg in DT documentation

[Sebastian Hesselbarth]

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

Re: [PATCH] ASoC: tda998x: Fix lack of required reg in DT documentation

[Jean-Francois Moine]

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?

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] ASoC: tda998x: Fix lack of required reg in DT documentation

[Sebastian Hesselbarth]

On 03/20/2014 02:52 PM, Jean-Francois Moine wrote:
> On Thu, 20 Mar 2014 14:32:18 +0100
> Sebastian Hesselbarth <sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ASoC: tda998x: Fix lack of required reg in DT documentation

[Russell King - ARM Linux]

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.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Re: [PATCH] ASoC: tda998x: Fix lack of required reg in DT documentation

[Jean-Francois Moine]

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...

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] ASoC: tda998x: Fix lack of required reg in DT documentation

[Robert Nelson]

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,

-- 
Robert Nelson
http://www.rcn-ee.com/

Re: [PATCH] ASoC: tda998x: Fix lack of required reg in DT documentation

[Russell King - ARM Linux]

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.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

Re: [PATCH] ASoC: tda998x: Fix lack of required reg in DT documentation

[Jean-Francois Moine]

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'.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] ASoC: tda998x: Fix lack of required reg in DT documentation

[Russell King - ARM Linux]

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.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.