Re: [PATCH 08/10] V4L/DVB: tda18271: allow restricting max out to 4 bytes

2010-09-30 Thread Michael Krufky
On Tue, Sep 28, 2010 at 2:46 PM, Mauro Carvalho Chehab
mche...@redhat.com wrote:
 By default, tda18271 tries to optimize I2C bus by updating all registers
 at the same time. Unfortunately, some devices doesn't support it.

 The current logic has a problem when small_i2c is equal to 8, since there
 are some transfers using 11 + 1 bytes.

 Fix the problem by enforcing the max size at the right place, and allows
 reducing it to max = 3 + 1.

 Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com

This looks to me as if it is working around a problem on the i2c
master.  I believe that a fix like this really belongs in the i2c
master driver, it should be able to break the i2c transactions down
into transactions that the i2c master can handle.

I wouldn't want to merge this without a better explanation of why it
is necessary in the tda18271 driver.  It seems to be a band-aid to
cover up a problem in the i2c master device driver code.

Regards,

Mike Krufky
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 08/10] V4L/DVB: tda18271: allow restricting max out to 4 bytes

2010-09-30 Thread Mauro Carvalho Chehab
Hi Michael,

Em 30-09-2010 15:52, Michael Krufky escreveu:
 On Tue, Sep 28, 2010 at 2:46 PM, Mauro Carvalho Chehab
 mche...@redhat.com wrote:
 By default, tda18271 tries to optimize I2C bus by updating all registers
 at the same time. Unfortunately, some devices doesn't support it.

 The current logic has a problem when small_i2c is equal to 8, since there
 are some transfers using 11 + 1 bytes.

 Fix the problem by enforcing the max size at the right place, and allows
 reducing it to max = 3 + 1.

 Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
 
 This looks to me as if it is working around a problem on the i2c
 master.  I believe that a fix like this really belongs in the i2c
 master driver, it should be able to break the i2c transactions down
 into transactions that the i2c master can handle.
 
 I wouldn't want to merge this without a better explanation of why it
 is necessary in the tda18271 driver.  It seems to be a band-aid to
 cover up a problem in the i2c master device driver code.

In the specific case of cx231xx the hardware don't support any I2C transactions 
with more than 4 bytes. It is common to find this kind of limit on other types
of hardware as well.

In the specific case of tda18271, the workaround for tda18271 is already there:

enum tda18271_small_i2c {
TDA18271_39_BYTE_CHUNK_INIT = 0,
TDA18271_16_BYTE_CHUNK_INIT = 1,
TDA18271_08_BYTE_CHUNK_INIT = 2,
};
(from upstream tda18271.h header)

Yet, it is currently broken. In thesis, if you use small_i2c, it should limit 
the 
transactions size to a certain value, but, if you try to limit to 8, you'll 
still 
see some transactions with size=11, since the code that honors small_i2c only 
works
for the initial dump of the 39 registers, as there are some cases where writes 
to
EP3 registers are done with:
tda18271_write_regs(fe, R_EP3, 11);

and the current code for tda18271_write_regs don't check it.

So, if there's a code there that allows limiting small_i2c:
1) this code should work for all transactions, not only to the initial 
init;
2) if there is such code, why not allow specifying a max size of 4 
bytes?

Another aspect is that doing such kind or restriction at the i2c adapter would 
require
a very ugly logic, as some devices like tda18271 have only 8 bits registers per 
i2c address,
and a write (or read) with length  1 byte update/read the next consecutive 
registers,
while other devices like xc3028 have one single I2C address for multi-byte 
operations like
updating the firmware.

If this logic would be moved into the bridge drivers, they would need to have 
some ugly
tests, checking for each specific i2c sub-driver at their core drivers.

Cheers,
Mauro.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 08/10] V4L/DVB: tda18271: allow restricting max out to 4 bytes

2010-09-30 Thread Michael Krufky
On Thu, Sep 30, 2010 at 3:12 PM, Mauro Carvalho Chehab
mche...@redhat.com wrote:
 Hi Michael,

 Em 30-09-2010 15:52, Michael Krufky escreveu:
 On Tue, Sep 28, 2010 at 2:46 PM, Mauro Carvalho Chehab
 mche...@redhat.com wrote:
 By default, tda18271 tries to optimize I2C bus by updating all registers
 at the same time. Unfortunately, some devices doesn't support it.

 The current logic has a problem when small_i2c is equal to 8, since there
 are some transfers using 11 + 1 bytes.

 Fix the problem by enforcing the max size at the right place, and allows
 reducing it to max = 3 + 1.

 Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com

 This looks to me as if it is working around a problem on the i2c
 master.  I believe that a fix like this really belongs in the i2c
 master driver, it should be able to break the i2c transactions down
 into transactions that the i2c master can handle.

 I wouldn't want to merge this without a better explanation of why it
 is necessary in the tda18271 driver.  It seems to be a band-aid to
 cover up a problem in the i2c master device driver code.

 In the specific case of cx231xx the hardware don't support any I2C 
 transactions
 with more than 4 bytes. It is common to find this kind of limit on other types
 of hardware as well.

 In the specific case of tda18271, the workaround for tda18271 is already 
 there:

 enum tda18271_small_i2c {
        TDA18271_39_BYTE_CHUNK_INIT = 0,
        TDA18271_16_BYTE_CHUNK_INIT = 1,
        TDA18271_08_BYTE_CHUNK_INIT = 2,
 };
 (from upstream tda18271.h header)

 Yet, it is currently broken. In thesis, if you use small_i2c, it should limit 
 the
 transactions size to a certain value, but, if you try to limit to 8, you'll 
 still
 see some transactions with size=11, since the code that honors small_i2c only 
 works
 for the initial dump of the 39 registers, as there are some cases where 
 writes to
 EP3 registers are done with:
        tda18271_write_regs(fe, R_EP3, 11);

 and the current code for tda18271_write_regs don't check it.

 So, if there's a code there that allows limiting small_i2c:
        1) this code should work for all transactions, not only to the initial 
 init;
        2) if there is such code, why not allow specifying a max size of 4 
 bytes?

 Another aspect is that doing such kind or restriction at the i2c adapter 
 would require
 a very ugly logic, as some devices like tda18271 have only 8 bits registers 
 per i2c address,
 and a write (or read) with length  1 byte update/read the next consecutive 
 registers,
 while other devices like xc3028 have one single I2C address for multi-byte 
 operations like
 updating the firmware.

 If this logic would be moved into the bridge drivers, they would need to have 
 some ugly
 tests, checking for each specific i2c sub-driver at their core drivers.

Hmm... If you don't mind, would you allow me to think about this a
bit, and run some tests of my own?

I have already seen the tda18271 work properly on the cx231xx with 8
byte transactions, the issue that I saw was actually a 12-byte
transaction limit, so the 11 byte transfer didn't cause a problem.
I'd like to test this again myself using the cx231xx device that I
have on hand.

However, this transaction in particular:

tda18271_write_regs(fe, R_EP3, 11);

...is not supposed to be broken down to smaller transaction -- this
particular transaction is supposed to be a one shot change to all 11
regs at once.  Straying from this could result in unpredictable
behavior.

I'd just like to review some other options and retest this before we
merge.  Is that okay with you?

Regards,

Mike Krufky
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 08/10] V4L/DVB: tda18271: allow restricting max out to 4 bytes

2010-09-30 Thread Mauro Carvalho Chehab
Em 30-09-2010 16:18, Michael Krufky escreveu:
 On Thu, Sep 30, 2010 at 3:12 PM, Mauro Carvalho Chehab
 mche...@redhat.com wrote:
 Hi Michael,

 Em 30-09-2010 15:52, Michael Krufky escreveu:
 On Tue, Sep 28, 2010 at 2:46 PM, Mauro Carvalho Chehab
 mche...@redhat.com wrote:
 By default, tda18271 tries to optimize I2C bus by updating all registers
 at the same time. Unfortunately, some devices doesn't support it.

 The current logic has a problem when small_i2c is equal to 8, since there
 are some transfers using 11 + 1 bytes.

 Fix the problem by enforcing the max size at the right place, and allows
 reducing it to max = 3 + 1.

 Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com

 This looks to me as if it is working around a problem on the i2c
 master.  I believe that a fix like this really belongs in the i2c
 master driver, it should be able to break the i2c transactions down
 into transactions that the i2c master can handle.

 I wouldn't want to merge this without a better explanation of why it
 is necessary in the tda18271 driver.  It seems to be a band-aid to
 cover up a problem in the i2c master device driver code.

 In the specific case of cx231xx the hardware don't support any I2C 
 transactions
 with more than 4 bytes. It is common to find this kind of limit on other 
 types
 of hardware as well.

 In the specific case of tda18271, the workaround for tda18271 is already 
 there:

 enum tda18271_small_i2c {
TDA18271_39_BYTE_CHUNK_INIT = 0,
TDA18271_16_BYTE_CHUNK_INIT = 1,
TDA18271_08_BYTE_CHUNK_INIT = 2,
 };
 (from upstream tda18271.h header)

 Yet, it is currently broken. In thesis, if you use small_i2c, it should 
 limit the
 transactions size to a certain value, but, if you try to limit to 8, you'll 
 still
 see some transactions with size=11, since the code that honors small_i2c 
 only works
 for the initial dump of the 39 registers, as there are some cases where 
 writes to
 EP3 registers are done with:
tda18271_write_regs(fe, R_EP3, 11);

 and the current code for tda18271_write_regs don't check it.

 So, if there's a code there that allows limiting small_i2c:
1) this code should work for all transactions, not only to the 
 initial init;
2) if there is such code, why not allow specifying a max size of 4 
 bytes?

 Another aspect is that doing such kind or restriction at the i2c adapter 
 would require
 a very ugly logic, as some devices like tda18271 have only 8 bits registers 
 per i2c address,
 and a write (or read) with length  1 byte update/read the next consecutive 
 registers,
 while other devices like xc3028 have one single I2C address for multi-byte 
 operations like
 updating the firmware.

 If this logic would be moved into the bridge drivers, they would need to 
 have some ugly
 tests, checking for each specific i2c sub-driver at their core drivers.
 
 Hmm... If you don't mind, would you allow me to think about this a
 bit, and run some tests of my own?
 
 I have already seen the tda18271 work properly on the cx231xx with 8
 byte transactions, the issue that I saw was actually a 12-byte
 transaction limit, so the 11 byte transfer didn't cause a problem.
 I'd like to test this again myself using the cx231xx device that I
 have on hand.

Maybe the cx231xx have different limits per I2C bus. The device I'm currently
handling uses I2C channel 2 (instead of channel 1) for the tuner. The only
device on this bus is tda18271. The original driver uses just one byte for every
transactions. On my tests, _all_ I2C transactions to i2c channel #2 with 
wLength  4 bytes fail.

So, I'm pretty confident that it won't support more than 4 bytes of payload. 

 However, this transaction in particular:
 
 tda18271_write_regs(fe, R_EP3, 11);
 
 ...is not supposed to be broken down to smaller transaction -- this
 particular transaction is supposed to be a one shot change to all 11
 regs at once.  Straying from this could result in unpredictable
 behavior.

Well, if the device doesn't support large transactions, what other options do
we have?

 I'd just like to review some other options and retest this before we
 merge.  Is that okay with you?

Yeah, sure.

Cheers,
Mauro.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 08/10] V4L/DVB: tda18271: allow restricting max out to 4 bytes

2010-09-30 Thread Antti Palosaari

On 09/30/2010 09:52 PM, Michael Krufky wrote:

On Tue, Sep 28, 2010 at 2:46 PM, Mauro Carvalho Chehab
mche...@redhat.com  wrote:

By default, tda18271 tries to optimize I2C bus by updating all registers
at the same time. Unfortunately, some devices doesn't support it.

The current logic has a problem when small_i2c is equal to 8, since there
are some transfers using 11 + 1 bytes.

Fix the problem by enforcing the max size at the right place, and allows
reducing it to max = 3 + 1.

Signed-off-by: Mauro Carvalho Chehabmche...@redhat.com


This looks to me as if it is working around a problem on the i2c
master.  I believe that a fix like this really belongs in the i2c
master driver, it should be able to break the i2c transactions down
into transactions that the i2c master can handle.

I wouldn't want to merge this without a better explanation of why it
is necessary in the tda18271 driver.  It seems to be a band-aid to
cover up a problem in the i2c master device driver code.


Yes it is I2C provider limitation, but I think almost all I2C adapters 
have some limit. I suggest to set param for each tuner and demod driver 
which splits reads and writes to len adapter can handle. I did that for 
tda18218 write.


But there is one major point you don't see. It is not simple to add this 
splitting limit to the provider. Provider does not have knowledge which 
is meaning of bytes it transfers to the bus. Without knowledge it breaks 
functionality surely in some point. There is commonly seen 1, 2 and 4 
byte register address and same for register values. Also some chips like 
to send data as register-value pairs.


regards,
Antti
--
http://palosaari.fi/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 08/10] V4L/DVB: tda18271: allow restricting max out to 4 bytes

2010-09-30 Thread Michael Krufky
On Thu, Sep 30, 2010 at 6:00 PM, Antti Palosaari cr...@iki.fi wrote:
 On 09/30/2010 09:52 PM, Michael Krufky wrote:

 On Tue, Sep 28, 2010 at 2:46 PM, Mauro Carvalho Chehab
 mche...@redhat.com  wrote:

 By default, tda18271 tries to optimize I2C bus by updating all registers
 at the same time. Unfortunately, some devices doesn't support it.

 The current logic has a problem when small_i2c is equal to 8, since there
 are some transfers using 11 + 1 bytes.

 Fix the problem by enforcing the max size at the right place, and allows
 reducing it to max = 3 + 1.

 Signed-off-by: Mauro Carvalho Chehabmche...@redhat.com

 This looks to me as if it is working around a problem on the i2c
 master.  I believe that a fix like this really belongs in the i2c
 master driver, it should be able to break the i2c transactions down
 into transactions that the i2c master can handle.

 I wouldn't want to merge this without a better explanation of why it
 is necessary in the tda18271 driver.  It seems to be a band-aid to
 cover up a problem in the i2c master device driver code.

 Yes it is I2C provider limitation, but I think almost all I2C adapters have
 some limit. I suggest to set param for each tuner and demod driver which
 splits reads and writes to len adapter can handle. I did that for tda18218
 write.

 But there is one major point you don't see. It is not simple to add this
 splitting limit to the provider. Provider does not have knowledge which is
 meaning of bytes it transfers to the bus. Without knowledge it breaks
 functionality surely in some point. There is commonly seen 1, 2 and 4 byte
 register address and same for register values. Also some chips like to send
 data as register-value pairs.

Yes, I understand.  We will likely merge Mauro's patch, I just want to
test it on my own hardware first, and I'd like to verify the cx231xx
i2c xfer limit issue that Mauro is reporting.  I'll try to get back to
this early next week, or hopefully over this weekend.

Thanks for the input :-)

Regards,

Mike Krufky
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 08/10] V4L/DVB: tda18271: allow restricting max out to 4 bytes

2010-09-30 Thread Mauro Carvalho Chehab
Em 30-09-2010 19:07, Michael Krufky escreveu:
 On Thu, Sep 30, 2010 at 6:00 PM, Antti Palosaari cr...@iki.fi wrote:
 On 09/30/2010 09:52 PM, Michael Krufky wrote:

 On Tue, Sep 28, 2010 at 2:46 PM, Mauro Carvalho Chehab
 mche...@redhat.com  wrote:

 By default, tda18271 tries to optimize I2C bus by updating all registers
 at the same time. Unfortunately, some devices doesn't support it.

 The current logic has a problem when small_i2c is equal to 8, since there
 are some transfers using 11 + 1 bytes.

 Fix the problem by enforcing the max size at the right place, and allows
 reducing it to max = 3 + 1.

 Signed-off-by: Mauro Carvalho Chehabmche...@redhat.com

 This looks to me as if it is working around a problem on the i2c
 master.  I believe that a fix like this really belongs in the i2c
 master driver, it should be able to break the i2c transactions down
 into transactions that the i2c master can handle.

 I wouldn't want to merge this without a better explanation of why it
 is necessary in the tda18271 driver.  It seems to be a band-aid to
 cover up a problem in the i2c master device driver code.

 Yes it is I2C provider limitation, but I think almost all I2C adapters have
 some limit. I suggest to set param for each tuner and demod driver which
 splits reads and writes to len adapter can handle. I did that for tda18218
 write.

 But there is one major point you don't see. It is not simple to add this
 splitting limit to the provider. Provider does not have knowledge which is
 meaning of bytes it transfers to the bus. Without knowledge it breaks
 functionality surely in some point. There is commonly seen 1, 2 and 4 byte
 register address and same for register values. Also some chips like to send
 data as register-value pairs.
 
 Yes, I understand.  We will likely merge Mauro's patch, I just want to
 test it on my own hardware first, and I'd like to verify the cx231xx
 i2c xfer limit issue that Mauro is reporting.  I'll try to get back to
 this early next week, or hopefully over this weekend.

Feel free to test. Just remember that all Hauppauge devices at Devin's polaris4
tree uses I2C Channel #1. This seems to be the default on all Conexant SDK 
also. 
The device I'm working with has the tuner at channel #2, and the ISDB-T demod
at channel #1. So, the limits may differ.

On my device, everything seem to work fine with max size = 4 
(both analog and digital modes).

Cheers,
Mauro

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 08/10] V4L/DVB: tda18271: allow restricting max out to 4 bytes

2010-09-29 Thread Devin Heitmueller
On Tue, Sep 28, 2010 at 2:46 PM, Mauro Carvalho Chehab
mche...@redhat.com wrote:
 By default, tda18271 tries to optimize I2C bus by updating all registers
 at the same time. Unfortunately, some devices doesn't support it.

 The current logic has a problem when small_i2c is equal to 8, since there
 are some transfers using 11 + 1 bytes.

 Fix the problem by enforcing the max size at the right place, and allows
 reducing it to max = 3 + 1.

 Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com

 diff --git a/drivers/media/common/tuners/tda18271-common.c 
 b/drivers/media/common/tuners/tda18271-common.c
 index e1f6782..195b30e 100644
 --- a/drivers/media/common/tuners/tda18271-common.c
 +++ b/drivers/media/common/tuners/tda18271-common.c
 @@ -193,20 +193,46 @@ int tda18271_write_regs(struct dvb_frontend *fe, int 
 idx, int len)
        unsigned char *regs = priv-tda18271_regs;
        unsigned char buf[TDA18271_NUM_REGS + 1];
        struct i2c_msg msg = { .addr = priv-i2c_props.addr, .flags = 0,
 -                              .buf = buf, .len = len + 1 };
 -       int i, ret;
 +                              .buf = buf };
 +       int i, ret = 1, max;

        BUG_ON((len == 0) || (idx + len  sizeof(buf)));

 -       buf[0] = idx;
 -       for (i = 1; i = len; i++)
 -               buf[i] = regs[idx - 1 + i];
 +
 +       switch (priv-small_i2c) {
 +       case TDA18271_03_BYTE_CHUNK_INIT:
 +               max = 3;
 +               break;
 +       case TDA18271_08_BYTE_CHUNK_INIT:
 +               max = 8;
 +               break;
 +       case TDA18271_16_BYTE_CHUNK_INIT:
 +               max = 16;
 +               break;
 +       case TDA18271_39_BYTE_CHUNK_INIT:
 +       default:
 +               max = 39;
 +       }

        tda18271_i2c_gate_ctrl(fe, 1);
 +       while (len) {
 +               if (max  len)
 +                       max = len;

 -       /* write registers */
 -       ret = i2c_transfer(priv-i2c_props.adap, msg, 1);
 +               buf[0] = idx;
 +               for (i = 1; i = max; i++)
 +                       buf[i] = regs[idx - 1 + i];

 +               msg.len = max + 1;
 +
 +               /* write registers */
 +               ret = i2c_transfer(priv-i2c_props.adap, msg, 1);
 +               if (ret != 1)
 +                       break;
 +
 +               idx += max;
 +               len -= max;
 +       }
        tda18271_i2c_gate_ctrl(fe, 0);

        if (ret != 1)
 @@ -326,24 +352,7 @@ int tda18271_init_regs(struct dvb_frontend *fe)
        regs[R_EB22] = 0x48;
        regs[R_EB23] = 0xb0;

 -       switch (priv-small_i2c) {
 -       case TDA18271_08_BYTE_CHUNK_INIT:
 -               tda18271_write_regs(fe, 0x00, 0x08);
 -               tda18271_write_regs(fe, 0x08, 0x08);
 -               tda18271_write_regs(fe, 0x10, 0x08);
 -               tda18271_write_regs(fe, 0x18, 0x08);
 -               tda18271_write_regs(fe, 0x20, 0x07);
 -               break;
 -       case TDA18271_16_BYTE_CHUNK_INIT:
 -               tda18271_write_regs(fe, 0x00, 0x10);
 -               tda18271_write_regs(fe, 0x10, 0x10);
 -               tda18271_write_regs(fe, 0x20, 0x07);
 -               break;
 -       case TDA18271_39_BYTE_CHUNK_INIT:
 -       default:
 -               tda18271_write_regs(fe, 0x00, TDA18271_NUM_REGS);
 -               break;
 -       }
 +       tda18271_write_regs(fe, 0x00, TDA18271_NUM_REGS);

        /* setup agc1 gain */
        regs[R_EB17] = 0x00;
 diff --git a/drivers/media/common/tuners/tda18271.h 
 b/drivers/media/common/tuners/tda18271.h
 index d7fcc36..3abb221 100644
 --- a/drivers/media/common/tuners/tda18271.h
 +++ b/drivers/media/common/tuners/tda18271.h
 @@ -80,8 +80,9 @@ enum tda18271_output_options {

  enum tda18271_small_i2c {
        TDA18271_39_BYTE_CHUNK_INIT = 0,
 -       TDA18271_16_BYTE_CHUNK_INIT = 1,
 -       TDA18271_08_BYTE_CHUNK_INIT = 2,
 +       TDA18271_16_BYTE_CHUNK_INIT = 16,
 +       TDA18271_08_BYTE_CHUNK_INIT = 8,
 +       TDA18271_03_BYTE_CHUNK_INIT = 3,
  };

  struct tda18271_config {
 diff --git a/drivers/media/video/tuner-core.c 
 b/drivers/media/video/tuner-core.c
 index fa406b9..1a047c5 100644
 --- a/drivers/media/video/tuner-core.c
 +++ b/drivers/media/video/tuner-core.c
 @@ -428,7 +428,7 @@ static void set_type(struct i2c_client *c, unsigned int 
 type,
        {
                struct tda18271_config cfg = {
                        .config = t-config,
 -                       .small_i2c = TDA18271_08_BYTE_CHUNK_INIT,
 +                       .small_i2c = TDA18271_03_BYTE_CHUNK_INIT,
                };

                if (!dvb_attach(tda18271_attach, t-fe, t-i2c-addr,
 --
 1.7.1




Adding the maintainer for the 18271 driver to the CC.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 08/10] V4L/DVB: tda18271: allow restricting max out to 4 bytes

2010-09-28 Thread Mauro Carvalho Chehab
By default, tda18271 tries to optimize I2C bus by updating all registers
at the same time. Unfortunately, some devices doesn't support it.

The current logic has a problem when small_i2c is equal to 8, since there
are some transfers using 11 + 1 bytes.

Fix the problem by enforcing the max size at the right place, and allows
reducing it to max = 3 + 1.

Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com

diff --git a/drivers/media/common/tuners/tda18271-common.c 
b/drivers/media/common/tuners/tda18271-common.c
index e1f6782..195b30e 100644
--- a/drivers/media/common/tuners/tda18271-common.c
+++ b/drivers/media/common/tuners/tda18271-common.c
@@ -193,20 +193,46 @@ int tda18271_write_regs(struct dvb_frontend *fe, int idx, 
int len)
unsigned char *regs = priv-tda18271_regs;
unsigned char buf[TDA18271_NUM_REGS + 1];
struct i2c_msg msg = { .addr = priv-i2c_props.addr, .flags = 0,
-  .buf = buf, .len = len + 1 };
-   int i, ret;
+  .buf = buf };
+   int i, ret = 1, max;
 
BUG_ON((len == 0) || (idx + len  sizeof(buf)));
 
-   buf[0] = idx;
-   for (i = 1; i = len; i++)
-   buf[i] = regs[idx - 1 + i];
+
+   switch (priv-small_i2c) {
+   case TDA18271_03_BYTE_CHUNK_INIT:
+   max = 3;
+   break;
+   case TDA18271_08_BYTE_CHUNK_INIT:
+   max = 8;
+   break;
+   case TDA18271_16_BYTE_CHUNK_INIT:
+   max = 16;
+   break;
+   case TDA18271_39_BYTE_CHUNK_INIT:
+   default:
+   max = 39;
+   }
 
tda18271_i2c_gate_ctrl(fe, 1);
+   while (len) {
+   if (max  len)
+   max = len;
 
-   /* write registers */
-   ret = i2c_transfer(priv-i2c_props.adap, msg, 1);
+   buf[0] = idx;
+   for (i = 1; i = max; i++)
+   buf[i] = regs[idx - 1 + i];
 
+   msg.len = max + 1;
+
+   /* write registers */
+   ret = i2c_transfer(priv-i2c_props.adap, msg, 1);
+   if (ret != 1)
+   break;
+
+   idx += max;
+   len -= max;
+   }
tda18271_i2c_gate_ctrl(fe, 0);
 
if (ret != 1)
@@ -326,24 +352,7 @@ int tda18271_init_regs(struct dvb_frontend *fe)
regs[R_EB22] = 0x48;
regs[R_EB23] = 0xb0;
 
-   switch (priv-small_i2c) {
-   case TDA18271_08_BYTE_CHUNK_INIT:
-   tda18271_write_regs(fe, 0x00, 0x08);
-   tda18271_write_regs(fe, 0x08, 0x08);
-   tda18271_write_regs(fe, 0x10, 0x08);
-   tda18271_write_regs(fe, 0x18, 0x08);
-   tda18271_write_regs(fe, 0x20, 0x07);
-   break;
-   case TDA18271_16_BYTE_CHUNK_INIT:
-   tda18271_write_regs(fe, 0x00, 0x10);
-   tda18271_write_regs(fe, 0x10, 0x10);
-   tda18271_write_regs(fe, 0x20, 0x07);
-   break;
-   case TDA18271_39_BYTE_CHUNK_INIT:
-   default:
-   tda18271_write_regs(fe, 0x00, TDA18271_NUM_REGS);
-   break;
-   }
+   tda18271_write_regs(fe, 0x00, TDA18271_NUM_REGS);
 
/* setup agc1 gain */
regs[R_EB17] = 0x00;
diff --git a/drivers/media/common/tuners/tda18271.h 
b/drivers/media/common/tuners/tda18271.h
index d7fcc36..3abb221 100644
--- a/drivers/media/common/tuners/tda18271.h
+++ b/drivers/media/common/tuners/tda18271.h
@@ -80,8 +80,9 @@ enum tda18271_output_options {
 
 enum tda18271_small_i2c {
TDA18271_39_BYTE_CHUNK_INIT = 0,
-   TDA18271_16_BYTE_CHUNK_INIT = 1,
-   TDA18271_08_BYTE_CHUNK_INIT = 2,
+   TDA18271_16_BYTE_CHUNK_INIT = 16,
+   TDA18271_08_BYTE_CHUNK_INIT = 8,
+   TDA18271_03_BYTE_CHUNK_INIT = 3,
 };
 
 struct tda18271_config {
diff --git a/drivers/media/video/tuner-core.c b/drivers/media/video/tuner-core.c
index fa406b9..1a047c5 100644
--- a/drivers/media/video/tuner-core.c
+++ b/drivers/media/video/tuner-core.c
@@ -428,7 +428,7 @@ static void set_type(struct i2c_client *c, unsigned int 
type,
{
struct tda18271_config cfg = {
.config = t-config,
-   .small_i2c = TDA18271_08_BYTE_CHUNK_INIT,
+   .small_i2c = TDA18271_03_BYTE_CHUNK_INIT,
};
 
if (!dvb_attach(tda18271_attach, t-fe, t-i2c-addr,
-- 
1.7.1


--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html