drm/i2c: tda998x: Set CEC I2C address from TDA I2C address

drm/i2c: tda998x: Set CEC I2C address from TDA I2C address

[Andrew Jackson]

The two LSBs of the TDA998x's I2C address are set by pins on the
chip.  The associated CEC peripheral's I2C address is set in the
same way, so base the CEC address on that of the TDA998x.

Signed-off-by: Andrew Jackson <Andrew.Jackson@arm.com>

---
 drivers/gpu/drm/i2c/tda998x_drv.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index c2a690d..08a2a26 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -1260,6 +1260,7 @@ tda998x_encoder_init(struct i2c_client *client,
        struct device_node *np = client->dev.of_node;
        u32 video;
        int rev_lo, rev_hi, ret;
+       unsigned short cec_addr;

        priv = kzalloc(sizeof(*priv), GFP_KERNEL);
        if (!priv)
@@ -1274,9 +1275,9 @@ tda998x_encoder_init(struct i2c_client *client,

        priv->current_page = 0xff;
        priv->hdmi = client;
-       priv->cec = i2c_new_dummy(client->adapter, 0x34);
-       if (!priv->cec)
-               priv->cec = i2c_new_dummy(client->adapter, 0x35);
+       /* CEC I2C address bound to TDA998x I2C addr by configuration pins */
+       cec_addr = 0x34 + (client->addr & 3);
+       priv->cec = i2c_new_dummy(client->adapter, cec_addr);
        if (!priv->cec) {
                kfree(priv);
                return -ENODEV;
--
1.7.1

-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2548782

drm/i2c: tda998x: Set CEC I2C address from TDA I2C address

[Russell King - ARM Linux]

You missed me as the maintainer of the driver:

NXP TDA998X DRM DRIVER
M:      Russell King <rmk+kernel@arm.linux.org.uk>
S:      Supported
F:      drivers/gpu/drm/i2c/tda998x_drv.c
F:      include/drm/i2c/tda998x.h

On Wed, Jul 02, 2014 at 10:33:44AM +0100, Andrew Jackson wrote:
> @@ -1274,9 +1275,9 @@ tda998x_encoder_init(struct i2c_client *client,
> 
>         priv->current_page = 0xff;
>         priv->hdmi = client;
> -       priv->cec = i2c_new_dummy(client->adapter, 0x34);
> -       if (!priv->cec)
> -               priv->cec = i2c_new_dummy(client->adapter, 0x35);
> +       /* CEC I2C address bound to TDA998x I2C addr by configuration pins */
> +       cec_addr = 0x34 + (client->addr & 3);
> +       priv->cec = i2c_new_dummy(client->adapter, cec_addr);

We discussed the issue of the I2C CEC address when the DT binding first
came up...

Do we know for certain that the above is always true for all the devices
that this driver supports?

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

drm/i2c: tda998x: Set CEC I2C address from TDA I2C address

[Andrew Jackson]

On 07/02/14 11:25, Russell King - ARM Linux wrote:
> You missed me as the maintainer of the driver:

Sorry, an over-sight on my part.

> NXP TDA998X DRM DRIVER
> M:      Russell King <rmk+kernel@arm.linux.org.uk>
> S:      Supported
> F:      drivers/gpu/drm/i2c/tda998x_drv.c
> F:      include/drm/i2c/tda998x.h
>
> On Wed, Jul 02, 2014 at 10:33:44AM +0100, Andrew Jackson wrote:
>> @@ -1274,9 +1275,9 @@ tda998x_encoder_init(struct i2c_client *client,
>>
>>         priv->current_page = 0xff;
>>         priv->hdmi = client;
>> -       priv->cec = i2c_new_dummy(client->adapter, 0x34);
>> -       if (!priv->cec)
>> -               priv->cec = i2c_new_dummy(client->adapter, 0x35);
>> +       /* CEC I2C address bound to TDA998x I2C addr by configuration pins */
>> +       cec_addr = 0x34 + (client->addr & 3);
>> +       priv->cec = i2c_new_dummy(client->adapter, cec_addr);
>
> We discussed the issue of the I2C CEC address when the DT binding first
> came up...
>
> Do we know for certain that the above is always true for all the devices
> that this driver supports?

I know that it is true for the following:

        * TDA9989 (Both lower LSBs are zero)
        * TDA19988 (LSBs are set from configuration pins)
        * TDA19989 (Both lower LSBs are zero)

I don't know about the N2 variants of the various devices.

I have tested the patch on board containing two TDA19988s.


   Andrew


-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2548782

drm/i2c: tda998x: Set CEC I2C address from TDA I2C address

[Russell King - ARM Linux]

On Wed, Jul 02, 2014 at 11:45:56AM +0100, Andrew Jackson wrote:
> On 07/02/14 11:25, Russell King - ARM Linux wrote:
> > You missed me as the maintainer of the driver:
> 
> Sorry, an over-sight on my part.
> 
> > NXP TDA998X DRM DRIVER
> > M:      Russell King <rmk+kernel@arm.linux.org.uk>
> > S:      Supported
> > F:      drivers/gpu/drm/i2c/tda998x_drv.c
> > F:      include/drm/i2c/tda998x.h
> >
> > On Wed, Jul 02, 2014 at 10:33:44AM +0100, Andrew Jackson wrote:
> >> @@ -1274,9 +1275,9 @@ tda998x_encoder_init(struct i2c_client *client,
> >>
> >>         priv->current_page = 0xff;
> >>         priv->hdmi = client;
> >> -       priv->cec = i2c_new_dummy(client->adapter, 0x34);
> >> -       if (!priv->cec)
> >> -               priv->cec = i2c_new_dummy(client->adapter, 0x35);
> >> +       /* CEC I2C address bound to TDA998x I2C addr by configuration pins */
> >> +       cec_addr = 0x34 + (client->addr & 3);
> >> +       priv->cec = i2c_new_dummy(client->adapter, cec_addr);
> >
> > We discussed the issue of the I2C CEC address when the DT binding first
> > came up...
> >
> > Do we know for certain that the above is always true for all the devices
> > that this driver supports?
> 
> I know that it is true for the following:
> 
>         * TDA9989 (Both lower LSBs are zero)
>         * TDA19988 (LSBs are set from configuration pins)
>         * TDA19989 (Both lower LSBs are zero)
> 
> I don't know about the N2 variants of the various devices.
> 
> I have tested the patch on board containing two TDA19988s.

Thanks, that's useful information to have.  Also, it's worth pointing out
that the patch is whitespace damaged - tabs are converted to spaces,
which makes it unable to be applied.  There's other reasons (see below)
that makes us unable to apply this patch too...

As we all know, email clients know better than us humans how to format
email messages, which gives them carte blanche to screw up patches in
this way.

I'd recommend that you talk to others in ARM Ltd about how they get
around this problem.

> -- IMPORTANT NOTICE: The contents of this email and any attachments
> are confidential and may also be privileged. If you are not the
> intended recipient, please notify the sender immediately and do
> not disclose the contents to any other person, use it for any
> purpose, or store or copy the information in any medium.  Thank you.

This means that I can't take the patch, since applying it into a git
tree and then publishing it would violate this statement.  Doing so
copies and stores the information, and makes it available to people
who were not the intended recipient of your email.

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

drm/i2c: tda998x: Set CEC I2C address from TDA I2C address

[Andrew Jackson]

On 07/02/14 16:34, Russell King - ARM Linux wrote:
> On Wed, Jul 02, 2014 at 11:45:56AM +0100, Andrew Jackson wrote:
>> On 07/02/14 11:25, Russell King - ARM Linux wrote:
>>> On Wed, Jul 02, 2014 at 10:33:44AM +0100, Andrew Jackson wrote:

> Thanks, that's useful information to have.  Also, it's worth pointing out
> that the patch is whitespace damaged - tabs are converted to spaces,
> which makes it unable to be applied.  There's other reasons (see below)
> that makes us unable to apply this patch too...
> 
> As we all know, email clients know better than us humans how to format
> email messages, which gives them carte blanche to screw up patches in
> this way.
> 
> I'd recommend that you talk to others in ARM Ltd about how they get
> around this problem.
> 
>> -- IMPORTANT NOTICE: The contents of this email and any attachments
>> are confidential and may also be privileged. If you are not the
>> intended recipient, please notify the sender immediately and do
>> not disclose the contents to any other person, use it for any
>> purpose, or store or copy the information in any medium.  Thank you.
> 
> This means that I can't take the patch, since applying it into a git
> tree and then publishing it would violate this statement.  Doing so
> copies and stores the information, and makes it available to people
> who were not the intended recipient of your email.
> 

Indeed.  I've now defeated the signature issue, or so I assert, so I will re-submit a clean version of the patch with additional documentation regarding the information on the various TDA998x devices.

Thank you for your tolerance.

   Andrew