| Submitter | David Jedelsky |
|---|---|
| Date | 2014-02-06 09:45:07 |
| Message ID | <1391679907-17876-1-git-send-email-david.jedelsky@gmail.com> |
| Download | mbox | patch |
| Permalink | /patch/22003/ |
| State | Rejected |
| Headers | show |
Comments
Moi David On 06.02.2014 11:45, David Jedelsky wrote: > My TechniSat SkyStar 2 HD CI USB ID 14f7:0002 wasn't tuning DVB-S2 channels. > Investigation revealed that it doesn't read DVB-S2 registers out of stb0899. > Comparison with usb trafic of the Windows driver showed the correct > communication scheme. This patch implements it. > The question is, whether the changed communication doesn't break other devices. > But the read part of patched _stb0899_read_s2reg routine is now functinally > same as (not changed) stb0899_read_regs which reads ordinrary DVB-S registers. > Giving high chance that it should work correctly on other devices too. That changes I2C functionality from STOP + START to repeated START. Current functionality looks also very weird, as there is 5 messages sent, all with STOP condition. I am not surprised if actually bug is still in adapter... Somehow it should be first resolved how those messages are send, with repeated START or STOP. And fix I2C client or adapter or both. regards Antti > > Signed-off-by: David Jedelsky <david.jedelsky@gmail.com> > --- > drivers/media/dvb-frontends/stb0899_drv.c | 44 +++++++++++------------------ > 1 file changed, 17 insertions(+), 27 deletions(-) > > diff --git a/drivers/media/dvb-frontends/stb0899_drv.c b/drivers/media/dvb-frontends/stb0899_drv.c > index 07cd5ea..3084cb2 100644 > --- a/drivers/media/dvb-frontends/stb0899_drv.c > +++ b/drivers/media/dvb-frontends/stb0899_drv.c > @@ -305,19 +305,20 @@ u32 _stb0899_read_s2reg(struct stb0899_state *state, > .len = 6 > }; > > - struct i2c_msg msg_1 = { > - .addr = state->config->demod_address, > - .flags = 0, > - .buf = buf_1, > - .len = 2 > + struct i2c_msg msg[] = { > + { > + .addr = state->config->demod_address, > + .flags = 0, > + .buf = buf_1, > + .len = 2 > + }, { > + .addr = state->config->demod_address, > + .flags = I2C_M_RD, > + .buf = buf, > + .len = 4 > + } > }; > > - struct i2c_msg msg_r = { > - .addr = state->config->demod_address, > - .flags = I2C_M_RD, > - .buf = buf, > - .len = 4 > - }; > > tmpaddr = stb0899_reg_offset & 0xff00; > if (!(stb0899_reg_offset & 0x8)) > @@ -326,6 +327,7 @@ u32 _stb0899_read_s2reg(struct stb0899_state *state, > buf_1[0] = GETBYTE(tmpaddr, BYTE1); > buf_1[1] = GETBYTE(tmpaddr, BYTE0); > > + /* Write address */ > status = i2c_transfer(state->i2c, &msg_0, 1); > if (status < 1) { > if (status != -ERESTARTSYS) > @@ -336,28 +338,16 @@ u32 _stb0899_read_s2reg(struct stb0899_state *state, > } > > /* Dummy */ > - status = i2c_transfer(state->i2c, &msg_1, 1); > - if (status < 1) > - goto err; > - > - status = i2c_transfer(state->i2c, &msg_r, 1); > - if (status < 1) > + status = i2c_transfer(state->i2c, msg, 2); > + if (status < 2) > goto err; > > buf_1[0] = GETBYTE(stb0899_reg_offset, BYTE1); > buf_1[1] = GETBYTE(stb0899_reg_offset, BYTE0); > > /* Actual */ > - status = i2c_transfer(state->i2c, &msg_1, 1); > - if (status < 1) { > - if (status != -ERESTARTSYS) > - printk(KERN_ERR "%s ERR(2), Device=[0x%04x], Base address=[0x%08x], Offset=[0x%04x], Status=%d\n", > - __func__, stb0899_i2cdev, stb0899_base_addr, stb0899_reg_offset, status); > - goto err; > - } > - > - status = i2c_transfer(state->i2c, &msg_r, 1); > - if (status < 1) { > + status = i2c_transfer(state->i2c, msg, 2); > + if (status < 2) { > if (status != -ERESTARTSYS) > printk(KERN_ERR "%s ERR(3), Device=[0x%04x], Base address=[0x%08x], Offset=[0x%04x], Status=%d\n", > __func__, stb0899_i2cdev, stb0899_base_addr, stb0899_reg_offset, status); >
Hi David, On Thu, Feb 6, 2014 at 3:15 PM, David Jedelsky <david.jedelsky@gmail.com> wrote: > > My TechniSat SkyStar 2 HD CI USB ID 14f7:0002 wasn't tuning DVB-S2 channels. > Investigation revealed that it doesn't read DVB-S2 registers out of stb0899. > Comparison with usb trafic of the Windows driver showed the correct > communication scheme. This patch implements it. > The question is, whether the changed communication doesn't break other devices. > But the read part of patched _stb0899_read_s2reg routine is now functinally > same as (not changed) stb0899_read_regs which reads ordinrary DVB-S registers. > Giving high chance that it should work correctly on other devices too. The patch does not look correct. STB0899 has a well documented I2C access method, which involves dummy reads, prior to any other. Also, the S2 regs access are different from the standard register access. That's the first part of the register access. The second part, According to 7914335A, the output buffer is not updated until a new address is requested. The controller returns the same value given in the first read operation. Thus the current code. So, most likely, all your modified read_s2reg would be reading incorrect data out of the registers that you requested; ie could be likely reading a standard register, or could possibly be returning data for some other read. With the current code, Without any changes S2 access does work correctly with most bridges. It's extremely strange such a change can cause things to work for you. From what I can see, your USB I2C interface has an incorrect or a bad implementation (I have seen such weirdness with some I2C implementations), which could be a likely explanation for your symptoms. Generally, a bug with the USB host device firmware, or a bug with USB-I2C driver is the most probable case. Best Regards, Manu -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Feb 8, 2014 at 1:19 AM, David Jedelsky <david.jedelsky@gmail.com> wrote: >> That changes I2C functionality from STOP + START to repeated START. >> Current functionality looks also very weird, as there is 5 messages sent, >> all with STOP condition. I am not surprised if actually bug is still in >> adapter... Somehow it should be first resolved how those messages are send, >> with repeated START or STOP. And fix I2C client or adapter or both. >> >> regards >> Antti > > > > Manu, Antti, > > Thank you for your response. I agree that the code is somewhat peculiar and > it could be worthy to review it using documentation before I leave it as bug > in my hw. Unfortunately I don't own appropriate documentation. If you can > supply it I can look at it. I can assure you that the STB0899 driver works well for S2 with most USB bridges and PCI bridges, which brings me to the fact that the issue does not exist with the STB0899 driver. Regarding the documentation, I don't have any wrt to the USB bridge, but only for the demodulator, tuner. But my hands are tied on that front, due to NDA's and agreements. Looking further in my hardware museum, I did find a Technisat Skystar USB2 HD CI REV 2.0 The information on a white sticker on the PCB states: Model AD-SB301, Project ID: 6027 DVB-S2, CI, USB Box (on-line update) H/W Ver: A1, PID/VID: 14F7 / 0002 manufactured and sent to me by Azurewave. It has a broken ferrite cored inductor on it, which appears to be on the power line to the demodulator/tuner. The PID/VID looks exactly the same as yours. If you have a firmware bug, maybe it helps to update the firmware online ? (I guess the windows driver uses some stock Cypress driver, from what I can imagine ?) I had similar problems as you state, when I worked with a prototype version of the Mantis PCI chipset where it had some issues regarding repeated starts. I can't really remember the exact issue back then, but I do remember the issue being tuner related as well, since the write to the tuner would reach the very first tuner register alone. The communications to the tuner are through a repeater on the demodulator. This issue was addressed with an ECO Metal fix for the PCI bridge, but that did eventually result in a newer chip though. The problem could likely be similar with your USB bridge. Maybe it is a driver bug too .. I haven't looked deeply at the az6027 driver. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07.02.2014 22:54, Manu Abraham wrote: > On Sat, Feb 8, 2014 at 1:19 AM, David Jedelsky <david.jedelsky@gmail.com> wrote: >>> That changes I2C functionality from STOP + START to repeated START. >>> Current functionality looks also very weird, as there is 5 messages sent, >>> all with STOP condition. I am not surprised if actually bug is still in >>> adapter... Somehow it should be first resolved how those messages are send, >>> with repeated START or STOP. And fix I2C client or adapter or both. >>> >>> regards >>> Antti >> >> >> >> Manu, Antti, >> >> Thank you for your response. I agree that the code is somewhat peculiar and >> it could be worthy to review it using documentation before I leave it as bug >> in my hw. Unfortunately I don't own appropriate documentation. If you can >> supply it I can look at it. > > I can assure you that the STB0899 driver works well for S2 with most > USB bridges and PCI bridges, which brings me to the fact that the issue > does not exist with the STB0899 driver. > > Regarding the documentation, I don't have any wrt to the USB bridge, but > only for the demodulator, tuner. But my hands are tied on that front, due to > NDA's and agreements. > > Looking further in my hardware museum, I did find a > Technisat Skystar USB2 HD CI REV 2.0 > > The information on a white sticker on the PCB states: > Model AD-SB301, Project ID: 6027 > DVB-S2, CI, USB Box (on-line update) > H/W Ver: A1, PID/VID: 14F7 / 0002 > > manufactured and sent to me by Azurewave. > > It has a broken ferrite cored inductor on it, which appears to be on the > power line to the demodulator/tuner. > > The PID/VID looks exactly the same as yours. If you have a firmware bug, > maybe it helps to update the firmware online ? (I guess the windows driver > uses some stock Cypress driver, from what I can imagine ?) > > I had similar problems as you state, when I worked with a prototype version > of the Mantis PCI chipset where it had some issues regarding repeated > starts. I can't really remember the exact issue back then, but I do remember > the issue being tuner related as well, since the write to the tuner would reach > the very first tuner register alone. The communications to the tuner are > through a repeater on the demodulator. > > This issue was addressed with an ECO Metal fix for the PCI bridge, but that > did eventually result in a newer chip though. > > The problem could likely be similar with your USB bridge. Maybe it is a > driver bug too .. I haven't looked deeply at the az6027 driver. It is almost 100% sure I2C adapter or client bug. az6027 driver i2c adapter seems to have some weird looking things, it behaves differently according I2C slave address used. If I didn't read code wrong, in that case it does to branch "if (msg[i].addr == 0xd0)". And looking that logic reveals it supports only 2 I2C transfers: for reg read: START + write + REPEATED START + read + STOP for reg write: START + write + STOP So that read operation (START + read + STOP) used by STB0899 is not implemented at all. regards Antti
On Sat, Feb 8, 2014 at 2:41 AM, Antti Palosaari <crope@iki.fi> wrote: > On 07.02.2014 22:54, Manu Abraham wrote: >> >> On Sat, Feb 8, 2014 at 1:19 AM, David Jedelsky <david.jedelsky@gmail.com> >> wrote: >>>> >>>> That changes I2C functionality from STOP + START to repeated START. >>>> Current functionality looks also very weird, as there is 5 messages >>>> sent, >>>> all with STOP condition. I am not surprised if actually bug is still in >>>> adapter... Somehow it should be first resolved how those messages are >>>> send, >>>> with repeated START or STOP. And fix I2C client or adapter or both. >>>> >>>> regards >>>> Antti >>> >>> >>> >>> >>> Manu, Antti, >>> >>> Thank you for your response. I agree that the code is somewhat peculiar >>> and >>> it could be worthy to review it using documentation before I leave it as >>> bug >>> in my hw. Unfortunately I don't own appropriate documentation. If you can >>> supply it I can look at it. >> >> >> I can assure you that the STB0899 driver works well for S2 with most >> USB bridges and PCI bridges, which brings me to the fact that the issue >> does not exist with the STB0899 driver. >> >> Regarding the documentation, I don't have any wrt to the USB bridge, but >> only for the demodulator, tuner. But my hands are tied on that front, due >> to >> NDA's and agreements. >> >> Looking further in my hardware museum, I did find a >> Technisat Skystar USB2 HD CI REV 2.0 >> >> The information on a white sticker on the PCB states: >> Model AD-SB301, Project ID: 6027 >> DVB-S2, CI, USB Box (on-line update) >> H/W Ver: A1, PID/VID: 14F7 / 0002 >> >> manufactured and sent to me by Azurewave. >> >> It has a broken ferrite cored inductor on it, which appears to be on the >> power line to the demodulator/tuner. >> >> The PID/VID looks exactly the same as yours. If you have a firmware bug, >> maybe it helps to update the firmware online ? (I guess the windows driver >> uses some stock Cypress driver, from what I can imagine ?) >> >> I had similar problems as you state, when I worked with a prototype >> version >> of the Mantis PCI chipset where it had some issues regarding repeated >> starts. I can't really remember the exact issue back then, but I do >> remember >> the issue being tuner related as well, since the write to the tuner would >> reach >> the very first tuner register alone. The communications to the tuner are >> through a repeater on the demodulator. >> >> This issue was addressed with an ECO Metal fix for the PCI bridge, but >> that >> did eventually result in a newer chip though. >> >> The problem could likely be similar with your USB bridge. Maybe it is a >> driver bug too .. I haven't looked deeply at the az6027 driver. > > > It is almost 100% sure I2C adapter or client bug. az6027 driver i2c adapter > seems to have some weird looking things, it behaves differently according > I2C slave address used. If I didn't read code wrong, in that case it does to > branch "if (msg[i].addr == 0xd0)". And looking that logic reveals it > supports only 2 I2C transfers: ACK. I looked at the code, just now. The I2C interface code looks garbage! > for reg read: START + write + REPEATED START + read + STOP > for reg write: START + write + STOP > > So that read operation (START + read + STOP) used by STB0899 is not > implemented at all. To be a bit more specific; the STB0899 S2 part. The STB0899 has a different (but standard) I2C interface for the DVB-S demodulator and a different one with 16/32 bit registers for the DVB-S2 demodulator. The USB-I2C interface code for the bridge doesn't implement this interface. But I see some still more weirdness in there with comments; "/* demod 16 bit addr */". There's a knot in my head, right now. AFAICS, the overall I2C communication with the STB0899 is very standard, generic I2C according to the official I2C specifications and documentations. All STB0899 specific handling is done within the demodulator read/write routines. If the I2C host interface with the USB device works in a standard way, then the device should work as-is with no changes to any frontend drivers. Regards, Manu -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Moikka! On 08.02.2014 15:28, David Jedelsky wrote: > Manu, Antti, > > Thank you for explanation. Now I see that I patched wrong place. More > appropriate would be to concentrate on az6027 I2C code. > Maybe my device could be working with corrected I2C code. > And yes, I have to confirm that the current I2C interface code is very > strange. Especially that address checking, which really suggests that > there is something wrong. > > I looked at the PCB and there isn't too much information written there > ...just "SkyStar USB 2 HD CI REV 2.0". Here are images (taken little bit > from the side to be IC labels visible) > http://djed.cz/skystar-usb-2-hd-ci-bottom.jpg > http://djed.cz/skystar-usb-2-hd-ci-top.jpg > and closeup of stb0899 > http://djed.cz/skystar-usb-2-hd-ci-stb0899.jpg > > Regarding the firmware (btw. just for curiosity you can see from the > images that the actual controller is CY7C68013A). > At some point I have extracted the firmware sent by windows driver from > the USB communication and it was the same as the Linux one. I can try to > look for some updates. You likely already know, but lets say, CY7C68013A is general USB bridge having firmware which defines how it behaves. There is open DVB firmware for that chip somewhere on LinuxTV.org cvs. I hacked it once successfully for one device, relatively small changes were needed. STB0899 is demodulator. And that small chip near antenna connector is tuner. Altera is fpga, running custom logic. It implements common interface. LC245A near Altera is likely TS (transport stream, picture is here) switch. Parallel TS used. It routes TS via CI or directly to CY7C68013A. Cypress datasheets are rather open, you could easily check which are I2C pins. If you has hardware sniffer, Bus Pirate, oscilloscope, etc. you could easily sniff I2C bus and look if it uses repeated START condition for register reads. regards Antti > > Regards, > David > > > > On Fri, Feb 7, 2014 at 10:46 PM, Manu Abraham <abraham.manu@gmail.com > <mailto:abraham.manu@gmail.com>> wrote: > > On Sat, Feb 8, 2014 at 2:41 AM, Antti Palosaari <crope@iki.fi > <mailto:crope@iki.fi>> wrote: > > On 07.02.2014 22:54, Manu Abraham wrote: > >> > >> On Sat, Feb 8, 2014 at 1:19 AM, David Jedelsky > <david.jedelsky@gmail.com <mailto:david.jedelsky@gmail.com>> > >> wrote: > >>>> > >>>> That changes I2C functionality from STOP + START to repeated > START. > >>>> Current functionality looks also very weird, as there is 5 > messages > >>>> sent, > >>>> all with STOP condition. I am not surprised if actually bug is > still in > >>>> adapter... Somehow it should be first resolved how those > messages are > >>>> send, > >>>> with repeated START or STOP. And fix I2C client or adapter or > both. > >>>> > >>>> regards > >>>> Antti > >>> > >>> > >>> > >>> > >>> Manu, Antti, > >>> > >>> Thank you for your response. I agree that the code is somewhat > peculiar > >>> and > >>> it could be worthy to review it using documentation before I > leave it as > >>> bug > >>> in my hw. Unfortunately I don't own appropriate documentation. > If you can > >>> supply it I can look at it. > >> > >> > >> I can assure you that the STB0899 driver works well for S2 with most > >> USB bridges and PCI bridges, which brings me to the fact that > the issue > >> does not exist with the STB0899 driver. > >> > >> Regarding the documentation, I don't have any wrt to the USB > bridge, but > >> only for the demodulator, tuner. But my hands are tied on that > front, due > >> to > >> NDA's and agreements. > >> > >> Looking further in my hardware museum, I did find a > >> Technisat Skystar USB2 HD CI REV 2.0 > >> > >> The information on a white sticker on the PCB states: > >> Model AD-SB301, Project ID: 6027 > >> DVB-S2, CI, USB Box (on-line update) > >> H/W Ver: A1, PID/VID: 14F7 / 0002 > >> > >> manufactured and sent to me by Azurewave. > >> > >> It has a broken ferrite cored inductor on it, which appears to > be on the > >> power line to the demodulator/tuner. > >> > >> The PID/VID looks exactly the same as yours. If you have a > firmware bug, > >> maybe it helps to update the firmware online ? (I guess the > windows driver > >> uses some stock Cypress driver, from what I can imagine ?) > >> > >> I had similar problems as you state, when I worked with a prototype > >> version > >> of the Mantis PCI chipset where it had some issues regarding > repeated > >> starts. I can't really remember the exact issue back then, but I do > >> remember > >> the issue being tuner related as well, since the write to the > tuner would > >> reach > >> the very first tuner register alone. The communications to the > tuner are > >> through a repeater on the demodulator. > >> > >> This issue was addressed with an ECO Metal fix for the PCI > bridge, but > >> that > >> did eventually result in a newer chip though. > >> > >> The problem could likely be similar with your USB bridge. Maybe > it is a > >> driver bug too .. I haven't looked deeply at the az6027 driver. > > > > > > It is almost 100% sure I2C adapter or client bug. az6027 driver > i2c adapter > > seems to have some weird looking things, it behaves differently > according > > I2C slave address used. If I didn't read code wrong, in that case > it does to > > branch "if (msg[i].addr == 0xd0)". And looking that logic reveals it > > supports only 2 I2C transfers: > > > ACK. I looked at the code, just now. The I2C interface code looks > garbage! > > > > for reg read: START + write + REPEATED START + read + STOP > > for reg write: START + write + STOP > > > > So that read operation (START + read + STOP) used by STB0899 is not > > implemented at all. > > To be a bit more specific; the STB0899 S2 part. The STB0899 has a > different (but standard) I2C interface for the DVB-S demodulator and a > different one with 16/32 bit registers for the DVB-S2 demodulator. The > USB-I2C interface code for the bridge doesn't implement this interface. > > But I see some still more weirdness in there with comments; > "/* demod 16 bit addr */". There's a knot in my head, right now. > > AFAICS, the overall I2C communication with the STB0899 is very standard, > generic I2C according to the official I2C specifications and > documentations. > All STB0899 specific handling is done within the demodulator read/write > routines. If the I2C host interface with the USB device works in a > standard > way, then the device should work as-is with no changes to any frontend > drivers. > > Regards, > > Manu > >
Patch
diff --git a/drivers/media/dvb-frontends/stb0899_drv.c b/drivers/media/dvb-frontends/stb0899_drv.c index 07cd5ea..3084cb2 100644 --- a/drivers/media/dvb-frontends/stb0899_drv.c +++ b/drivers/media/dvb-frontends/stb0899_drv.c @@ -305,19 +305,20 @@ u32 _stb0899_read_s2reg(struct stb0899_state *state, .len = 6 }; - struct i2c_msg msg_1 = { - .addr = state->config->demod_address, - .flags = 0, - .buf = buf_1, - .len = 2 + struct i2c_msg msg[] = { + { + .addr = state->config->demod_address, + .flags = 0, + .buf = buf_1, + .len = 2 + }, { + .addr = state->config->demod_address, + .flags = I2C_M_RD, + .buf = buf, + .len = 4 + } }; - struct i2c_msg msg_r = { - .addr = state->config->demod_address, - .flags = I2C_M_RD, - .buf = buf, - .len = 4 - }; tmpaddr = stb0899_reg_offset & 0xff00; if (!(stb0899_reg_offset & 0x8)) @@ -326,6 +327,7 @@ u32 _stb0899_read_s2reg(struct stb0899_state *state, buf_1[0] = GETBYTE(tmpaddr, BYTE1); buf_1[1] = GETBYTE(tmpaddr, BYTE0); + /* Write address */ status = i2c_transfer(state->i2c, &msg_0, 1); if (status < 1) { if (status != -ERESTARTSYS) @@ -336,28 +338,16 @@ u32 _stb0899_read_s2reg(struct stb0899_state *state, } /* Dummy */ - status = i2c_transfer(state->i2c, &msg_1, 1); - if (status < 1) - goto err; - - status = i2c_transfer(state->i2c, &msg_r, 1); - if (status < 1) + status = i2c_transfer(state->i2c, msg, 2); + if (status < 2) goto err; buf_1[0] = GETBYTE(stb0899_reg_offset, BYTE1); buf_1[1] = GETBYTE(stb0899_reg_offset, BYTE0); /* Actual */ - status = i2c_transfer(state->i2c, &msg_1, 1); - if (status < 1) { - if (status != -ERESTARTSYS) - printk(KERN_ERR "%s ERR(2), Device=[0x%04x], Base address=[0x%08x], Offset=[0x%04x], Status=%d\n", - __func__, stb0899_i2cdev, stb0899_base_addr, stb0899_reg_offset, status); - goto err; - } - - status = i2c_transfer(state->i2c, &msg_r, 1); - if (status < 1) { + status = i2c_transfer(state->i2c, msg, 2); + if (status < 2) { if (status != -ERESTARTSYS) printk(KERN_ERR "%s ERR(3), Device=[0x%04x], Base address=[0x%08x], Offset=[0x%04x], Status=%d\n", __func__, stb0899_i2cdev, stb0899_base_addr, stb0899_reg_offset, status);
My TechniSat SkyStar 2 HD CI USB ID 14f7:0002 wasn't tuning DVB-S2 channels. Investigation revealed that it doesn't read DVB-S2 registers out of stb0899. Comparison with usb trafic of the Windows driver showed the correct communication scheme. This patch implements it. The question is, whether the changed communication doesn't break other devices. But the read part of patched _stb0899_read_s2reg routine is now functinally same as (not changed) stb0899_read_regs which reads ordinrary DVB-S registers. Giving high chance that it should work correctly on other devices too. Signed-off-by: David Jedelsky <david.jedelsky@gmail.com> --- drivers/media/dvb-frontends/stb0899_drv.c | 44 +++++++++++------------------ 1 file changed, 17 insertions(+), 27 deletions(-)