Re: [PATCH v4] dvb-usb/friio, dvb-usb-v2/gl861: decompose friio and merge with gl861

2018-03-31 Thread Akihiro TSUKADA 
> I don't understand those control message parts and it is bit too hard to
> read i2c adapter implementation to get understanding. Could you offer
> simple 2 sniff examples, register write to demod and register write to
> tuner.

Here is the part of a packet log.

1. write to demod (addr:0x18)

reg:0x76 val:0c
===
[46264 ms]  >>>  URB 146 going down  >>> 
-- URB_FUNCTION_VENDOR_DEVICE:
  TransferFlags  =  (USBD_TRANSFER_DIRECTION_OUT, ~USBD_SHORT_TR
ANSFER_OK)
  TransferBufferLength = 
  TransferBuffer   = 8609d21e
  TransferBufferMDL= 
  UrbLink = 
  RequestTypeReservedBits = 
  Request = 0001
  Value   = 300c
  Index   = 0076
[46266 ms] UsbSnoop - MyInternalIOCTLCompletion(f79b7db0) : fido=, Irp=8
58f2938, Context=858c4ed8, IRQL=2
[46266 ms]  <<<  URB 146 coming back  <<< 
-- URB_FUNCTION_CONTROL_TRANSFER:
  PipeHandle   = 86239260
  TransferFlags= 000a (USBD_TRANSFER_DIRECTION_OUT, USBD_SHORT_TRANS
FER_OK)
  TransferBufferLength = 
  TransferBuffer   = 8609d21e
  TransferBufferMDL= 
  UrbLink  = 
  SetupPacket  =
: 40 01 0c 30 76 00 00 00
===


2. write to tuner (addr:0x60)

write [0f 7b b2 08] to addr 0x60
===
[47267 ms]  >>>  URB 147 going down  >>> 
-- URB_FUNCTION_VENDOR_DEVICE:
  TransferFlags  =  (USBD_TRANSFER_DIRECTION_OUT, ~USBD_SHORT_TR
ANSFER_OK)
  TransferBufferLength = 0005
  TransferBuffer   = 8581c7d6
  TransferBufferMDL= 
: c0 0f 7b b2 08
  UrbLink = 
  RequestTypeReservedBits = 
  Request = 0003
  Value   = 3000
  Index   = 00fe
[47270 ms] UsbSnoop - MyInternalIOCTLCompletion(f79b7db0) : fido=, Irp=8
58f2008, Context=86275258, IRQL=2
[47270 ms]  <<<  URB 147 coming back  <<< 
-- URB_FUNCTION_CONTROL_TRANSFER:
  PipeHandle   = 86239260
  TransferFlags= 000a (USBD_TRANSFER_DIRECTION_OUT, USBD_SHORT_TRANS
FER_OK)
  TransferBufferLength = 0005
  TransferBuffer   = 8581c7d6
  TransferBufferMDL= 855f7760
  UrbLink  = 
  SetupPacket  =
: 40 03 00 30 fe 00 05 00
===

3. read from tuner

read one byte from addr 0x60
(2 USB packets)
===
[46036 ms]  >>>  URB 26 going down  >>> 
-- URB_FUNCTION_VENDOR_DEVICE:
  TransferFlags  =  (USBD_TRANSFER_DIRECTION_OUT, ~USBD_SHORT_TR
ANSFER_OK)
  TransferBufferLength = 0001
  TransferBuffer   = 8609d21e
  TransferBufferMDL= 
: c1
  UrbLink = 
  RequestTypeReservedBits = 
  Request = 0003
  Value   = 3000
  Index   = 00fe
[46038 ms] UsbSnoop - MyInternalIOCTLCompletion(f79b7db0) : fido=, Irp=8
58f2938, Context=858ccea0, IRQL=2
[46038 ms]  <<<  URB 26 coming back  <<< 
-- URB_FUNCTION_CONTROL_TRANSFER:
  PipeHandle   = 86239260
  TransferFlags= 000a (USBD_TRANSFER_DIRECTION_OUT, USBD_SHORT_TRANS
FER_OK)
  TransferBufferLength = 0001
  TransferBuffer   = 8609d21e
  TransferBufferMDL= 855f7760
  UrbLink  = 
  SetupPacket  =
: 40 03 00 30 fe 00 01 00


[46038 ms]  >>>  URB 27 going down  >>> 
-- URB_FUNCTION_VENDOR_DEVICE:
  TransferFlags  = 0001 (USBD_TRANSFER_DIRECTION_IN, ~USBD_SHORT_TRA
NSFER_OK)
  TransferBufferLength = 0001
  TransferBuffer   = 8609d21e
  TransferBufferMDL= 
  UrbLink = 
  RequestTypeReservedBits = 
  Request = 0002
  Value   = 3000
  Index   = 0100
[46040 ms] UsbSnoop - MyInternalIOCTLCompletion(f79b7db0) : fido=, Irp=8
58f2938, Context=86366778, IRQL=2
[46040 ms]  <<<  URB 27 coming back  <<< 
-- URB_FUNCTION_CONTROL_TRANSFER:
  PipeHandle   = 86239260
  TransferFlags= 000b (USBD_TRANSFER_DIRECTION_IN, USBD_SHORT_TRANSF
ER_OK)
  TransferBufferLength = 0001
  TransferBuffer   = 8609d21e
  TransferBufferMDL= 855f7760
: 7c
  UrbLink  = 
  SetupPacket  =
: c0 02 00 30 00 01 01 00


Note: In log 2 & 3, "Request" parameter value is different from log 1. 

regards,
Akihiro

Re: [PATCH v4] dvb-usb/friio, dvb-usb-v2/gl861: decompose friio and merge with gl861

2018-03-30 Thread Antti Palosaari 



On 03/30/2018 04:21 PM, Akihiro TSUKADA wrote:

I simply cannot see why it cannot work. Just add i2c adapter and
suitable logic there. Transaction on your example is simply and there is
no problem to implement that kind of logic to demod i2c adapter.


I might be totally wrong, but...

i2c transactions to a tuner must use:
1. usb_control_msg(request:3) for the first half (write) of reads
2. usb_control_msg(request:1) for the other writes
3. usb_control_msg(request:2) for (all) reads

How can the demod driver control the 'request' argument of USB messages
that are sent to its parent (not to the demod itself),
when the bridge of tc90522 cannot be limited to gl861 (or even to USB) ?


I don't understand those control message parts and it is bit too hard to 
read i2c adapter implementation to get understanding. Could you offer 
simple 2 sniff examples, register write to demod and register write to 
tuner.


Anyhow, demod i2c adapter gets request from tuner and then does some 
demod specific i2c algo stuff and then pass proper request to usb-bridge 
i2c adapter.


IIR it was somehing like

write_tuner_reg(0xaa, 0xbb);
 ==> demod i2c algo:
 * write_demod_reg(0xfe, 0x60) // set tuner i2c addr + start i2c write
 * write_demod_reg(0xaa, 0xbb)

so those command now goes to i2c-bridge i2c algo which uses gl861 i2c algo


regards
Antti

--
http://palosaari.fi/

Re: [PATCH v4] dvb-usb/friio, dvb-usb-v2/gl861: decompose friio and merge with gl861

2018-03-30 Thread Akihiro TSUKADA 
> I simply cannot see why it cannot work. Just add i2c adapter and
> suitable logic there. Transaction on your example is simply and there is
> no problem to implement that kind of logic to demod i2c adapter.

I might be totally wrong, but...

i2c transactions to a tuner must use:
1. usb_control_msg(request:3) for the first half (write) of reads
2. usb_control_msg(request:1) for the other writes
3. usb_control_msg(request:2) for (all) reads

How can the demod driver control the 'request' argument of USB messages
that are sent to its parent (not to the demod itself),
when the bridge of tc90522 cannot be limited to gl861 (or even to USB) ?

Akihiro

Re: [PATCH v4] dvb-usb/friio, dvb-usb-v2/gl861: decompose friio and merge with gl861

2018-03-28 Thread Antti Palosaari 



On 03/28/2018 03:37 PM, Akihiro TSUKADA wrote:

Hi,
thanks for the comment.


You should implement i2c adapter to demod driver and not add such glue
to that USB-bridge. I mean that "relayed" stuff, i2c communication to
tuner via demod. I2C-mux may not work I think as there is no gate-style
multiplexing so you probably need plain i2c adapter. There is few
examples already on some demod drivers.


I am afraid that the glue is actually necessary.

host - USB -> gl861 - I2C(1) -> tc90522 (addr:X)
   \- I2C(2) -> tua6034 (addr:Y)

To send an i2c read message to tua6034,
one has to issue two transactions:
  1. write via I2C(1) to addr:X, [ reg:0xfe, val: Y ]
  2. read via I2C(1) from addr:X, [ out_data0, out_data1, ]

The problem is that the transaction 1 is (somehow) implemented with
the different USB request than the other i2c transactions on I2C(1).
(this is confirmed by a packet capture on Windows box).

Although tc90522 already creats the i2c adapter for I2C(2),
tc90522 cannot know/control the USB implementation of I2C(1),
only the bridge driver can do this.


I simply cannot see why it cannot work. Just add i2c adapter and 
suitable logic there. Transaction on your example is simply and there is 
no problem to implement that kind of logic to demod i2c adapter.


If gl861 driver i2c adapter logic is broken it can be fixed easily too. 
It seems to support only i2c writes with len 1 and 2 bytes, but fixing 
it should be easy if you has some sniffs.




Antti

--
http://palosaari.fi/

Re: [PATCH v4] dvb-usb/friio, dvb-usb-v2/gl861: decompose friio and merge with gl861

2018-03-28 Thread kbuild test robot 
Hi Akihiro,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.16-rc7 next-20180328]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/tskd08-gmail-com/dvb-usb-friio-dvb-usb-v2-gl861-decompose-friio-and-merge-with-gl861/20180329-001436
base:   git://linuxtv.org/media_tree.git master
config: x86_64-randconfig-x019-201812 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

>> drivers/media/usb/dvb-usb-v2/gl861.c:203:24: error: field 'tuner_cfg' has 
>> incomplete type
 struct dvb_pll_config tuner_cfg;
   ^
>> drivers/media/usb/dvb-usb-v2/gl861.c:210:3: error: field name not in record 
>> or union initializer
  .desc_id = DVB_PLL_TUA6034_FRIIO,
  ^
   drivers/media/usb/dvb-usb-v2/gl861.c:210:3: note: (near initialization for 
'friio_config.tuner_cfg')
>> drivers/media/usb/dvb-usb-v2/gl861.c:210:14: error: 'DVB_PLL_TUA6034_FRIIO' 
>> undeclared here (not in a function); did you mean 'DVB_PLL_TUA6034'?
  .desc_id = DVB_PLL_TUA6034_FRIIO,
 ^
 DVB_PLL_TUA6034
   drivers/media/usb/dvb-usb-v2/gl861.c: In function 'friio_tuner_attach':
>> drivers/media/usb/dvb-usb-v2/gl861.c:414:24: error: storage size of 'cfg' 
>> isn't known
 struct dvb_pll_config cfg;
   ^~~
   drivers/media/usb/dvb-usb-v2/gl861.c:414:24: warning: unused variable 'cfg' 
[-Wunused-variable]

vim +/tuner_cfg +203 drivers/media/usb/dvb-usb-v2/gl861.c

   197  
   198  struct friio_config {
   199  struct i2c_board_info demod_info;
   200  struct tc90522_config demod_cfg;
   201  
   202  struct i2c_board_info tuner_info;
 > 203  struct dvb_pll_config tuner_cfg;
   204  };
   205  
   206  static const struct friio_config friio_config = {
   207  .demod_info = { I2C_BOARD_INFO(TC90522_I2C_DEV_TER, 0x18), },
   208  .tuner_info = { I2C_BOARD_INFO("dvb_pll", 0x60), },
   209  .tuner_cfg = {
 > 210  .desc_id = DVB_PLL_TUA6034_FRIIO,
   211  },
   212  };
   213  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v4] dvb-usb/friio, dvb-usb-v2/gl861: decompose friio and merge with gl861

2018-03-28 Thread Akihiro TSUKADA 
Hi,
thanks for the comment.

> You should implement i2c adapter to demod driver and not add such glue
> to that USB-bridge. I mean that "relayed" stuff, i2c communication to
> tuner via demod. I2C-mux may not work I think as there is no gate-style
> multiplexing so you probably need plain i2c adapter. There is few
> examples already on some demod drivers.

I am afraid that the glue is actually necessary.

host - USB -> gl861 - I2C(1) -> tc90522 (addr:X)
  \- I2C(2) -> tua6034 (addr:Y)

To send an i2c read message to tua6034,
one has to issue two transactions:
 1. write via I2C(1) to addr:X, [ reg:0xfe, val: Y ]
 2. read via I2C(1) from addr:X, [ out_data0, out_data1, ]

The problem is that the transaction 1 is (somehow) implemented with
the different USB request than the other i2c transactions on I2C(1).
(this is confirmed by a packet capture on Windows box).

Although tc90522 already creats the i2c adapter for I2C(2),
tc90522 cannot know/control the USB implementation of I2C(1),
only the bridge driver can do this.

regards,
Akihiro

Re: [PATCH v4] dvb-usb/friio, dvb-usb-v2/gl861: decompose friio and merge with gl861

2018-03-27 Thread Antti Palosaari 

On 03/27/2018 08:47 PM, tsk...@gmail.com wrote:

From: Akihiro Tsukada 

Friio device contains "gl861" bridge and "tc90522" demod,
for which the separate drivers are already in the kernel.
But friio driver was monolithic and did not use them,
practically copying those features.
This patch decomposes friio driver into sub drivers and
re-uses existing ones, thus reduces some code.

It adds some features to gl861,
to support the friio-specific init/config of the devices
and implement i2c communications to the tuner via demod
with USB vendor requests.


You should implement i2c adapter to demod driver and not add such glue 
to that USB-bridge. I mean that "relayed" stuff, i2c communication to 
tuner via demod. I2C-mux may not work I think as there is no gate-style 
multiplexing so you probably need plain i2c adapter. There is few 
examples already on some demod drivers.


regards
Antti

--
http://palosaari.fi/

[PATCH v4] dvb-usb/friio, dvb-usb-v2/gl861: decompose friio and merge with gl861

2018-03-27 Thread tskd08 
From: Akihiro Tsukada 

Friio device contains "gl861" bridge and "tc90522" demod,
for which the separate drivers are already in the kernel.
But friio driver was monolithic and did not use them,
practically copying those features.
This patch decomposes friio driver into sub drivers and
re-uses existing ones, thus reduces some code.

It adds some features to gl861,
to support the friio-specific init/config of the devices
and implement i2c communications to the tuner via demod
with USB vendor requests.

Signed-off-by: Akihiro Tsukada 
---
Changes since v3:
 - make dvb_usb_device_properties static

Changes since v2:
(patch #27928, dvb-usb-friio: split and merge into dvb-usbv2-gl861)
 - used the new i2c binding helpers instead of my own one
 - merged gl861-friio.c with gl861.c

 drivers/media/usb/dvb-usb-v2/Kconfig |   5 +-
 drivers/media/usb/dvb-usb-v2/gl861.c | 468 ++-
 drivers/media/usb/dvb-usb-v2/gl861.h |   1 +
 drivers/media/usb/dvb-usb/Kconfig|   6 -
 drivers/media/usb/dvb-usb/Makefile   |   3 -
 drivers/media/usb/dvb-usb/friio-fe.c | 441 -
 drivers/media/usb/dvb-usb/friio.c| 522 ---
 drivers/media/usb/dvb-usb/friio.h|  99 ---
 8 files changed, 465 insertions(+), 1080 deletions(-)
 delete mode 100644 drivers/media/usb/dvb-usb/friio-fe.c
 delete mode 100644 drivers/media/usb/dvb-usb/friio.c
 delete mode 100644 drivers/media/usb/dvb-usb/friio.h

diff --git a/drivers/media/usb/dvb-usb-v2/Kconfig 
b/drivers/media/usb/dvb-usb-v2/Kconfig
index 0e4944b2b0f..e0a1f377295 100644
--- a/drivers/media/usb/dvb-usb-v2/Kconfig
+++ b/drivers/media/usb/dvb-usb-v2/Kconfig
@@ -95,10 +95,13 @@ config DVB_USB_GL861
tristate "Genesys Logic GL861 USB2.0 support"
depends on DVB_USB_V2
select DVB_ZL10353 if MEDIA_SUBDRV_AUTOSELECT
+   select DVB_TC90522 if MEDIA_SUBDRV_AUTOSELECT
select MEDIA_TUNER_QT1010 if MEDIA_SUBDRV_AUTOSELECT
+   select DVB_PLL if MEDIA_SUBDRV_AUTOSELECT
help
  Say Y here to support the MSI Megasky 580 (55801) DVB-T USB2.0
- receiver with USB ID 0db0:5581.
+ receiver with USB ID 0db0:5581, Friio White ISDB-T receiver
+ with USB ID 0x7a69:0001.
 
 config DVB_USB_LME2510
tristate "LME DM04/QQBOX DVB-S USB2.0 support"
diff --git a/drivers/media/usb/dvb-usb-v2/gl861.c 
b/drivers/media/usb/dvb-usb-v2/gl861.c
index b1b09c54786..ef644117f84 100644
--- a/drivers/media/usb/dvb-usb-v2/gl861.c
+++ b/drivers/media/usb/dvb-usb-v2/gl861.c
@@ -10,6 +10,8 @@
 
 #include "zl10353.h"
 #include "qt1010.h"
+#include "tc90522.h"
+#include "dvb-pll.h"
 
 DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);
 
@@ -49,6 +51,80 @@ static int gl861_i2c_msg(struct dvb_usb_device *d, u8 addr,
   value, index, rbuf, rlen, 2000);
 }
 
+/* Friio specific I2C read/write */
+/* special USB request is used in Friio's init/config */
+static int
+gl861_i2c_rawwrite(struct dvb_usb_device *d, u8 addr, u8 *wbuf, u16 wlen)
+{
+   u8 *buf;
+   int ret;
+
+   buf = kmalloc(wlen, GFP_KERNEL);
+   if (!buf)
+   return -ENOMEM;
+
+   usleep_range(1000, 2000); /* avoid I2C errors */
+   memcpy(buf, wbuf, wlen);
+   ret = usb_control_msg(d->udev, usb_sndctrlpipe(d->udev, 0),
+GL861_REQ_I2C_RAW, GL861_WRITE,
+addr << (8 + 1), 0x0100, buf, wlen, 2000);
+   kfree(buf);
+   return ret;
+}
+
+/*
+ * In Friio,
+ * I2C commnucations to the tuner are relay'ed via the demod (via the bridge),
+ * so its encapsulation to USB message is different from the one to the demod.
+ */
+static int
+gl861_i2c_rawread(struct dvb_usb_device *d, u8 addr, u8 *rbuf, u16 rlen)
+{
+   u8 *buf;
+   int ret;
+
+   buf = kmalloc(rlen, GFP_KERNEL);
+   if (!buf)
+   return -ENOMEM;
+
+   usleep_range(1000, 2000); /* avoid I2C errors */
+
+   ret = usb_control_msg(d->udev, usb_rcvctrlpipe(d->udev, 0),
+GL861_REQ_I2C_READ, GL861_READ,
+addr << (8 + 1), 0x0100, buf, rlen, 2000);
+   if (ret > 0 && rbuf)
+   memcpy(rbuf, buf, rlen);
+   kfree(buf);
+
+   return ret;
+}
+
+static int
+gl861_i2c_relay_write(struct dvb_usb_device *d, struct i2c_msg *msg)
+{
+   u8 *buf;
+   int ret;
+
+   if (msg->flags & I2C_M_RD)
+   return -EINVAL;
+   if (msg->len < 2)
+   return -EINVAL;
+
+   buf = kmalloc(msg->len - 1, GFP_KERNEL);
+   if (!buf)
+   return -ENOMEM;
+   memcpy(buf, msg->buf + 1, msg->len - 1);
+
+   usleep_range(1000, 2000); /* avoid I2C errors */
+
+   ret = usb_control_msg(d->udev, usb_sndctrlpipe(d->udev, 0),
+GL861_REQ_I2C_RAW, GL861_WRITE,
+msg->addr << (8 + 1), msg->buf[0],