Re: [PATCH] add support for the dvb-t part of CT-3650 v3

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/23/2011 06:41 PM, Jose Alberto Reguero wrote:
On Sábado, 23 de Julio de 2011 12:37:53 Antti Palosaari escribió:
On 07/23/2011 01:21 PM, Jose Alberto Reguero wrote:
On Sábado, 23 de Julio de 2011 11:42:58 Antti Palosaari escribió:
On 07/23/2011 11:26 AM, Jose Alberto Reguero wrote:
The problem is in i2c read in tda827x_probe_version. Without the fix
sometimes, when changing the code the tuner is detected as  tda827xo
instead of tda827xa. That is because the variable where i2c read should
store the value is initialized, and sometimes it works.

struct i2c_msg msg = { .addr = priv->i2c_addr, .flags = I2C_M_RD,

			       .buf =&data, .len = 1 };

rc = tuner_transfer(fe,&msg, 1);

:-( Could you read what I write. It is a little bit annoying to find out

everything for you. You just answer every time something like it does
not work and I should always find out what's problem.

As I pointed out read will never work since I2C adapter supports only
read done in WRITE+READ combination. Driver uses read which is single
READ without write.

You should implement new read. You can look example from af9015 or other
drivers using tda827x

This have been never worked thus I Cc Guy Martin who have added DVB-C
support for that device.


regards
Antti

I don't understand you. I think that you don' see the fix, but the old
code. Old code:

read = i+1<    num&&    (msg[i+1].flags&    I2C_M_RD);

Fix:

read1 = i+1<   num&&   (msg[i+1].flags&   I2C_M_RD); for the tda10023 and
tda10048 read2 = msg[i].flags&   I2C_M_RD; for the tda827x

Jose Alberto

First of all I must apologize of blaming you about that I2C adapter,
sorry, I should going to shame now. It was me who doesn't read your
changes as should :/

Your changes are logically OK and implements correctly single reading as
needed. Some comments still;
* consider renaming read1 and read2 for example write_read and read
* obuf[1] contains WRITE len. your code sets that now as READ len.
Probably it should be 0 always in single write since no bytes written.
* remove useless checks from end of the "if (foo) if (foo)";
if (read1 || read2) {
	if (read1) {
[...]
	} else if (read2)

If you store some variables at the beginning, olen, ilen, obuf, ibuf,
you can increase i++ for write+read and rest of the code in function can
be same (no more if read or write + read). But maybe it is safe to keep
closer original than change such much.


regards
Antti

There are a second i2c read, but less important.It is in:

tda827xa_set_params

............
         buf[0] = 0xa0;
         buf[1] = 0x40;
         msg.len = 2;
         rc = tuner_transfer(fe,&msg, 1);
         if (rc<  0)
                 goto err;

         msleep(11);
         msg.flags = I2C_M_RD;
         rc = tuner_transfer(fe,&msg, 1);
         if (rc<  0)
                 goto err;
         msg.flags = 0;

         buf[1]>>= 4;
............
I supposed that buf[0] is the register to read and they read the value in
buf[1]. The code now seem to work ok but perhaps is wrong.

This one is as translated to "normal" C we usually use;
write_reg(0xa0, 0x40); // write one reg
read_regs(2); // read 2 regs

example from the sniff
 AA B0 31 05 C2 02 00 A0 40                        ª°1.Â.. @
 55 B0 31 03 C2 02 00 4A 44 08 00 00 00 71 AC EC   U°1.Â..JD....q¬ì
 AA B1 31 05 C2 02 00 30 11                        ª±1.Â..0.
 55 B1 31 03 C2 02 00 4A 44 08 00 00 00 71 AC EC   U±1.Â..JD....q¬ì


AA USB direction to device
B1 USB msg seq
31 USB cmd
05 USB data len (4+5=9, 4=hdr len, 5=data len, 9=total)
C2 I2C addr (addr << 1)
02 I2C write len
00 I2C read len
30 I2C data [0]
11 I2C data [1]

So it seems actually to write 30 11 and then read 4a 44 as reply. But if you read driver code it does not write "30 11" instead just reads. Maybe buggy I2C adap implementation or buggy tuner driver (Linux driver or driver where sniff taken). Try to read without write and with write and compare if there is any difference.


regards
Antti

--
http://palosaari.fi/
--
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


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux