Hi Michael,
Em 30-09-2010 15:52, Michael Krufky escreveu:
> On Tue, Sep 28, 2010 at 2:46 PM, Mauro Carvalho Chehab
> <mchehab@xxxxxxxxxx> 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 <mchehab@xxxxxxxxxx>
>
> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
