All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] [media] cx231xx: Initial support Evromedia USB Full Hybrid Full HD
@ 2017-01-09 15:23 Oleh Kravchenko
  2017-01-09 16:59 ` Antti Palosaari
  2017-01-09 20:08 ` kbuild test robot
  0 siblings, 2 replies; 14+ messages in thread
From: Oleh Kravchenko @ 2017-01-09 15:23 UTC (permalink / raw)
  To: linux-media, hverkuil; +Cc: Oleh Kravchenko

This patch provide only digital support.

The device is based on Si2168 30-demodulator,
Si2158-20 tuner and CX23102-11Z chipset;
USB id: 1b80:d3b2.

Status:
- DVB-T2 works fine;
- Composite and SVideo works fine;
- Analog not implemented.

Signed-off-by: Oleh Kravchenko <oleg@kaa.org.ua>
Tested-by: Oleh Kravchenko <oleg@kaa.org.ua>
---
 drivers/media/usb/cx231xx/Kconfig         |  1 +
 drivers/media/usb/cx231xx/cx231xx-cards.c | 29 +++++++++++++
 drivers/media/usb/cx231xx/cx231xx-dvb.c   | 71 +++++++++++++++++++++++++++++++
 drivers/media/usb/cx231xx/cx231xx-i2c.c   | 37 ++++++++++++++++
 drivers/media/usb/cx231xx/cx231xx.h       |  1 +
 5 files changed, 139 insertions(+)

diff --git a/drivers/media/usb/cx231xx/Kconfig b/drivers/media/usb/cx231xx/Kconfig
index 0cced3e..58de80b 100644
--- a/drivers/media/usb/cx231xx/Kconfig
+++ b/drivers/media/usb/cx231xx/Kconfig
@@ -50,6 +50,7 @@ config VIDEO_CX231XX_DVB
 	select DVB_LGDT3306A if MEDIA_SUBDRV_AUTOSELECT
 	select DVB_TDA18271C2DD if MEDIA_SUBDRV_AUTOSELECT
 	select DVB_SI2165 if MEDIA_SUBDRV_AUTOSELECT
+	select DVB_SI2168 if MEDIA_SUBDRV_AUTOSELECT
 	select MEDIA_TUNER_SI2157 if MEDIA_SUBDRV_AUTOSELECT
 
 	---help---
diff --git a/drivers/media/usb/cx231xx/cx231xx-cards.c b/drivers/media/usb/cx231xx/cx231xx-cards.c
index 36bc254..9b1df5a 100644
--- a/drivers/media/usb/cx231xx/cx231xx-cards.c
+++ b/drivers/media/usb/cx231xx/cx231xx-cards.c
@@ -841,6 +841,33 @@ struct cx231xx_board cx231xx_boards[] = {
 			.gpio = NULL,
 		} },
 	},
+	[CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD] = {
+		.name = "Evromedia USB Full Hybrid Full HD",
+		.tuner_type = TUNER_ABSENT,
+		.demod_addr = 0xc8 >> 1,
+		.demod_i2c_master = I2C_1_MUX_3,
+		.has_dvb = 1,
+		.ir_i2c_master = I2C_0,
+		.norm = V4L2_STD_PAL,
+		.output_mode = OUT_MODE_VIP11,
+		.tuner_addr = 0xc0 >> 1,
+		.tuner_i2c_master = I2C_2,
+		.input = {{
+			.type = CX231XX_VMUX_TELEVISION,
+			.vmux = 0,
+			.amux = CX231XX_AMUX_VIDEO,
+		}, {
+			.type = CX231XX_VMUX_COMPOSITE1,
+			.vmux = CX231XX_VIN_2_1,
+			.amux = CX231XX_AMUX_LINE_IN,
+		}, {
+			.type = CX231XX_VMUX_SVIDEO,
+			.vmux = CX231XX_VIN_1_1 |
+				(CX231XX_VIN_1_2 << 8) |
+				CX25840_SVIDEO_ON,
+			.amux = CX231XX_AMUX_LINE_IN,
+		} },
+	},
 };
 const unsigned int cx231xx_bcount = ARRAY_SIZE(cx231xx_boards);
 
@@ -908,6 +935,8 @@ struct usb_device_id cx231xx_id_table[] = {
 	 .driver_info = CX231XX_BOARD_OTG102},
 	{USB_DEVICE(USB_VID_TERRATEC, 0x00a6),
 	 .driver_info = CX231XX_BOARD_TERRATEC_GRABBY},
+	{USB_DEVICE(0x1b80, 0xd3b2),
+	.driver_info = CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD},
 	{},
 };
 
diff --git a/drivers/media/usb/cx231xx/cx231xx-dvb.c b/drivers/media/usb/cx231xx/cx231xx-dvb.c
index 1417515..08472a3 100644
--- a/drivers/media/usb/cx231xx/cx231xx-dvb.c
+++ b/drivers/media/usb/cx231xx/cx231xx-dvb.c
@@ -33,6 +33,7 @@
 #include "s5h1411.h"
 #include "lgdt3305.h"
 #include "si2165.h"
+#include "si2168.h"
 #include "mb86a20s.h"
 #include "si2157.h"
 #include "lgdt3306a.h"
@@ -949,6 +950,76 @@ static int dvb_init(struct cx231xx *dev)
 			   &pv_tda18271_config);
 		break;
 
+	case CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD:
+	{
+		struct si2157_config si2157_config = {};
+		struct si2168_config si2168_config = {};
+		struct i2c_board_info info = {};
+		struct i2c_client *client;
+		struct i2c_adapter *adapter;
+
+		/* attach demodulator chip */
+		si2168_config.ts_mode = SI2168_TS_SERIAL; /* from *.inf file */
+		si2168_config.fe = &dev->dvb->frontend;
+		si2168_config.i2c_adapter = &adapter;
+		si2168_config.ts_clock_inv = true;
+
+		strlcpy(info.type, "si2168", info.type);
+		info.addr = dev->board.demod_addr;
+		info.platform_data = &si2168_config;
+
+		request_module(info.type);
+		client = i2c_new_device(demod_i2c, &info);
+
+		if (client == NULL || client->dev.driver == NULL || dev->dvb->frontend == NULL) {
+			dev_err(dev->dev, "Failed to attach Si2168 front end\n");
+			result = -EINVAL;
+			goto out_free;
+		}
+
+		if (!try_module_get(client->dev.driver->owner)) {
+			i2c_unregister_device(client);
+			result = -ENODEV;
+			goto out_free;
+		}
+
+		dvb->i2c_client_demod = client;
+		dev->dvb->frontend->ops.i2c_gate_ctrl = NULL;
+		dvb->frontend->callback = cx231xx_tuner_callback;
+
+		/* attach tuner chip */
+		si2157_config.fe = dev->dvb->frontend;
+#ifdef CONFIG_MEDIA_CONTROLLER_DVB
+		si2157_config.mdev = dev->media_dev;
+#endif
+		si2157_config.if_port = 1;
+		si2157_config.inversion = false;
+
+		memset(&info, 0, sizeof(info));
+		strlcpy(info.type, "si2157", info.type);
+		info.addr = dev->board.tuner_addr;
+		info.platform_data = &si2157_config;
+
+		request_module("si2157");
+		client = i2c_new_device(tuner_i2c, &info);
+
+		if (client == NULL || client->dev.driver == NULL) {
+			dvb_frontend_detach(dev->dvb->frontend);
+			result = -ENODEV;
+			goto out_free;
+		}
+
+		if (!try_module_get(client->dev.driver->owner)) {
+			i2c_unregister_device(client);
+			dvb_frontend_detach(dev->dvb->frontend);
+			result = -ENODEV;
+			goto out_free;
+		}
+
+		dev->cx231xx_reset_analog_tuner = NULL;
+		dev->dvb->i2c_client_tuner = client;
+		break;
+	}
 	default:
 		dev_err(dev->dev,
 			"%s/2: The frontend of your DVB/ATSC card isn't supported yet\n",
diff --git a/drivers/media/usb/cx231xx/cx231xx-i2c.c b/drivers/media/usb/cx231xx/cx231xx-i2c.c
index 35e9acf..60412ec 100644
--- a/drivers/media/usb/cx231xx/cx231xx-i2c.c
+++ b/drivers/media/usb/cx231xx/cx231xx-i2c.c
@@ -171,6 +171,43 @@ static int cx231xx_i2c_send_bytes(struct i2c_adapter *i2c_adap,
 		bus->i2c_nostop = 0;
 		bus->i2c_reserve = 0;
 
+	} else if (dev->model == CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD
+		&& msg->addr == dev->tuner_addr
+		&& msg->len > 4) {
+		/* special case for Evromedia USB Full Hybrid Full HD tuner chip */
+		size = msg->len;
+		saddr_len = 1;
+
+		/* adjust the length to correct length */
+		size -= saddr_len;
+
+		buf_ptr = (u8*)(msg->buf + 1);
+
+		do {
+			/* prepare xfer_data struct */
+			req_data.dev_addr = msg->addr;
+			req_data.direction = msg->flags;
+			req_data.saddr_len = saddr_len;
+			req_data.saddr_dat = msg->buf[0];
+			req_data.buf_size = size > 4 ? 4 : size;
+			req_data.p_buffer = (u8*)(buf_ptr + loop * 4);
+
+			bus->i2c_nostop = (size > 4) ? 1 : 0;
+			bus->i2c_reserve = (loop == 0) ? 0 : 1;
+
+			/* usb send command */
+			status = dev->cx231xx_send_usb_command(bus, &req_data);
+			loop++;
+
+			if (size >= 4) {
+				size -= 4;
+			} else {
+				size = 0;
+			}
+		} while (size > 0);
+
+		bus->i2c_nostop = 0;
+		bus->i2c_reserve = 0;
 	} else {		/* regular case */
 
 		/* prepare xfer_data struct */
diff --git a/drivers/media/usb/cx231xx/cx231xx.h b/drivers/media/usb/cx231xx/cx231xx.h
index 90c8676..d9792ea 100644
--- a/drivers/media/usb/cx231xx/cx231xx.h
+++ b/drivers/media/usb/cx231xx/cx231xx.h
@@ -78,6 +78,7 @@
 #define CX231XX_BOARD_HAUPPAUGE_930C_HD_1114xx 20
 #define CX231XX_BOARD_HAUPPAUGE_955Q 21
 #define CX231XX_BOARD_TERRATEC_GRABBY 22
+#define CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD 23
 
 /* Limits minimum and default number of buffers */
 #define CX231XX_MIN_BUF                 4
-- 
2.10.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] [media] cx231xx: Initial support Evromedia USB Full Hybrid Full HD
  2017-01-09 15:23 [PATCH] [media] cx231xx: Initial support Evromedia USB Full Hybrid Full HD Oleh Kravchenko
@ 2017-01-09 16:59 ` Antti Palosaari
  2017-01-09 21:49   ` Oleh Kravchenko
  2017-01-09 20:08 ` kbuild test robot
  1 sibling, 1 reply; 14+ messages in thread
From: Antti Palosaari @ 2017-01-09 16:59 UTC (permalink / raw)
  To: Oleh Kravchenko, linux-media, hverkuil



On 01/09/2017 05:23 PM, Oleh Kravchenko wrote:
> This patch provide only digital support.
>
> The device is based on Si2168 30-demodulator,
> Si2158-20 tuner and CX23102-11Z chipset;
> USB id: 1b80:d3b2.
>
> Status:
> - DVB-T2 works fine;
> - Composite and SVideo works fine;
> - Analog not implemented.
>
> Signed-off-by: Oleh Kravchenko <oleg@kaa.org.ua>
> Tested-by: Oleh Kravchenko <oleg@kaa.org.ua>
> ---
>  drivers/media/usb/cx231xx/Kconfig         |  1 +
>  drivers/media/usb/cx231xx/cx231xx-cards.c | 29 +++++++++++++
>  drivers/media/usb/cx231xx/cx231xx-dvb.c   | 71 +++++++++++++++++++++++++++++++
>  drivers/media/usb/cx231xx/cx231xx-i2c.c   | 37 ++++++++++++++++
>  drivers/media/usb/cx231xx/cx231xx.h       |  1 +
>  5 files changed, 139 insertions(+)
>
> diff --git a/drivers/media/usb/cx231xx/Kconfig b/drivers/media/usb/cx231xx/Kconfig
> index 0cced3e..58de80b 100644
> --- a/drivers/media/usb/cx231xx/Kconfig
> +++ b/drivers/media/usb/cx231xx/Kconfig
> @@ -50,6 +50,7 @@ config VIDEO_CX231XX_DVB
>  	select DVB_LGDT3306A if MEDIA_SUBDRV_AUTOSELECT
>  	select DVB_TDA18271C2DD if MEDIA_SUBDRV_AUTOSELECT
>  	select DVB_SI2165 if MEDIA_SUBDRV_AUTOSELECT
> +	select DVB_SI2168 if MEDIA_SUBDRV_AUTOSELECT
>  	select MEDIA_TUNER_SI2157 if MEDIA_SUBDRV_AUTOSELECT
>
>  	---help---
> diff --git a/drivers/media/usb/cx231xx/cx231xx-cards.c b/drivers/media/usb/cx231xx/cx231xx-cards.c
> index 36bc254..9b1df5a 100644
> --- a/drivers/media/usb/cx231xx/cx231xx-cards.c
> +++ b/drivers/media/usb/cx231xx/cx231xx-cards.c
> @@ -841,6 +841,33 @@ struct cx231xx_board cx231xx_boards[] = {
>  			.gpio = NULL,
>  		} },
>  	},
> +	[CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD] = {
> +		.name = "Evromedia USB Full Hybrid Full HD",
> +		.tuner_type = TUNER_ABSENT,
> +		.demod_addr = 0xc8 >> 1,
> +		.demod_i2c_master = I2C_1_MUX_3,
> +		.has_dvb = 1,
> +		.ir_i2c_master = I2C_0,
> +		.norm = V4L2_STD_PAL,
> +		.output_mode = OUT_MODE_VIP11,
> +		.tuner_addr = 0xc0 >> 1,

These "8-bit" I2C addresses looks funny, but if that's used by cx231xx 
driver then leave...


> +		.tuner_i2c_master = I2C_2,
> +		.input = {{
> +			.type = CX231XX_VMUX_TELEVISION,
> +			.vmux = 0,
> +			.amux = CX231XX_AMUX_VIDEO,
> +		}, {
> +			.type = CX231XX_VMUX_COMPOSITE1,
> +			.vmux = CX231XX_VIN_2_1,
> +			.amux = CX231XX_AMUX_LINE_IN,
> +		}, {
> +			.type = CX231XX_VMUX_SVIDEO,
> +			.vmux = CX231XX_VIN_1_1 |
> +				(CX231XX_VIN_1_2 << 8) |
> +				CX25840_SVIDEO_ON,
> +			.amux = CX231XX_AMUX_LINE_IN,
> +		} },
> +	},
>  };
>  const unsigned int cx231xx_bcount = ARRAY_SIZE(cx231xx_boards);
>
> @@ -908,6 +935,8 @@ struct usb_device_id cx231xx_id_table[] = {
>  	 .driver_info = CX231XX_BOARD_OTG102},
>  	{USB_DEVICE(USB_VID_TERRATEC, 0x00a6),
>  	 .driver_info = CX231XX_BOARD_TERRATEC_GRABBY},
> +	{USB_DEVICE(0x1b80, 0xd3b2),
> +	.driver_info = CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD},
>  	{},
>  };
>
> diff --git a/drivers/media/usb/cx231xx/cx231xx-dvb.c b/drivers/media/usb/cx231xx/cx231xx-dvb.c
> index 1417515..08472a3 100644
> --- a/drivers/media/usb/cx231xx/cx231xx-dvb.c
> +++ b/drivers/media/usb/cx231xx/cx231xx-dvb.c
> @@ -33,6 +33,7 @@
>  #include "s5h1411.h"
>  #include "lgdt3305.h"
>  #include "si2165.h"
> +#include "si2168.h"
>  #include "mb86a20s.h"
>  #include "si2157.h"
>  #include "lgdt3306a.h"
> @@ -949,6 +950,76 @@ static int dvb_init(struct cx231xx *dev)
>  			   &pv_tda18271_config);
>  		break;
>
> +	case CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD:
> +	{
> +		struct si2157_config si2157_config = {};
> +		struct si2168_config si2168_config = {};
> +		struct i2c_board_info info = {};
> +		struct i2c_client *client;
> +		struct i2c_adapter *adapter;
> +
> +		/* attach demodulator chip */
> +		si2168_config.ts_mode = SI2168_TS_SERIAL; /* from *.inf file */
> +		si2168_config.fe = &dev->dvb->frontend;
> +		si2168_config.i2c_adapter = &adapter;
> +		si2168_config.ts_clock_inv = true;
> +
> +		strlcpy(info.type, "si2168", info.type);
> +		info.addr = dev->board.demod_addr;
> +		info.platform_data = &si2168_config;
> +
> +		request_module(info.type);
> +		client = i2c_new_device(demod_i2c, &info);
> +
> +		if (client == NULL || client->dev.driver == NULL || dev->dvb->frontend == NULL) {

No need to check frontend here, or at least I cannot see why it should? 
Does the cx231xx initialize with some special value before calling 
si2168 probe - which will set it? client and driver will be null in case 
si2168 probe fails. Also, frontend pointer is not set if si2168 probe fails.

> +			dev_err(dev->dev, "Failed to attach Si2168 front end\n");
> +			result = -EINVAL;
> +			goto out_free;
> +		}
> +
> +		if (!try_module_get(client->dev.driver->owner)) {
> +			i2c_unregister_device(client);
> +			result = -ENODEV;
> +			goto out_free;
> +		}
> +
> +		dvb->i2c_client_demod = client;
> +		dev->dvb->frontend->ops.i2c_gate_ctrl = NULL;

si2168 does not use nor set i2c_gate_ctrl callback. No need to nullify 
it in any case.

> +		dvb->frontend->callback = cx231xx_tuner_callback;
> +
> +		/* attach tuner chip */
> +		si2157_config.fe = dev->dvb->frontend;
> +#ifdef CONFIG_MEDIA_CONTROLLER_DVB
> +		si2157_config.mdev = dev->media_dev;
> +#endif
> +		si2157_config.if_port = 1;
> +		si2157_config.inversion = false;
> +
> +		memset(&info, 0, sizeof(info));
> +		strlcpy(info.type, "si2157", info.type);
> +		info.addr = dev->board.tuner_addr;
> +		info.platform_data = &si2157_config;
> +
> +		request_module("si2157");
> +		client = i2c_new_device(tuner_i2c, &info);
> +
> +		if (client == NULL || client->dev.driver == NULL) {
> +			dvb_frontend_detach(dev->dvb->frontend);
> +			result = -ENODEV;
> +			goto out_free;
> +		}

Does this error handling work? Have you tested it? I suspect it will 
not. You should likely decrease si2168 module reference counter and then 
unregister si2168 driver.

> +
> +		if (!try_module_get(client->dev.driver->owner)) {
> +			i2c_unregister_device(client);
> +			dvb_frontend_detach(dev->dvb->frontend);
> +			result = -ENODEV;
> +			goto out_free;
> +		}
> +
> +		dev->cx231xx_reset_analog_tuner = NULL;
> +		dev->dvb->i2c_client_tuner = client;
> +		break;
> +	}
>  	default:
>  		dev_err(dev->dev,
>  			"%s/2: The frontend of your DVB/ATSC card isn't supported yet\n",
> diff --git a/drivers/media/usb/cx231xx/cx231xx-i2c.c b/drivers/media/usb/cx231xx/cx231xx-i2c.c
> index 35e9acf..60412ec 100644
> --- a/drivers/media/usb/cx231xx/cx231xx-i2c.c
> +++ b/drivers/media/usb/cx231xx/cx231xx-i2c.c
> @@ -171,6 +171,43 @@ static int cx231xx_i2c_send_bytes(struct i2c_adapter *i2c_adap,
>  		bus->i2c_nostop = 0;
>  		bus->i2c_reserve = 0;
>
> +	} else if (dev->model == CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD
> +		&& msg->addr == dev->tuner_addr
> +		&& msg->len > 4) {
> +		/* special case for Evromedia USB Full Hybrid Full HD tuner chip */
> +		size = msg->len;
> +		saddr_len = 1;
> +
> +		/* adjust the length to correct length */
> +		size -= saddr_len;
> +
> +		buf_ptr = (u8*)(msg->buf + 1);
> +
> +		do {
> +			/* prepare xfer_data struct */
> +			req_data.dev_addr = msg->addr;
> +			req_data.direction = msg->flags;
> +			req_data.saddr_len = saddr_len;
> +			req_data.saddr_dat = msg->buf[0];
> +			req_data.buf_size = size > 4 ? 4 : size;
> +			req_data.p_buffer = (u8*)(buf_ptr + loop * 4);
> +
> +			bus->i2c_nostop = (size > 4) ? 1 : 0;
> +			bus->i2c_reserve = (loop == 0) ? 0 : 1;
> +
> +			/* usb send command */
> +			status = dev->cx231xx_send_usb_command(bus, &req_data);
> +			loop++;
> +
> +			if (size >= 4) {
> +				size -= 4;
> +			} else {
> +				size = 0;
> +			}
> +		} while (size > 0);
> +
> +		bus->i2c_nostop = 0;
> +		bus->i2c_reserve = 0;

I don't follow (and I looked only that patch, not whole cx231xx driver) 
why this I2C adapter logic is limited to that device only. si2168 and 
si2157 drivers applies just standard multibyte I2C read and write 
operations - no write+read op used at all.

These I2C adapter routines should be same for every device. Maybe 
original logic is somehow wrong and it should be fixed instead of adding 
new device specific logic.


>  	} else {		/* regular case */
>
>  		/* prepare xfer_data struct */
> diff --git a/drivers/media/usb/cx231xx/cx231xx.h b/drivers/media/usb/cx231xx/cx231xx.h
> index 90c8676..d9792ea 100644
> --- a/drivers/media/usb/cx231xx/cx231xx.h
> +++ b/drivers/media/usb/cx231xx/cx231xx.h
> @@ -78,6 +78,7 @@
>  #define CX231XX_BOARD_HAUPPAUGE_930C_HD_1114xx 20
>  #define CX231XX_BOARD_HAUPPAUGE_955Q 21
>  #define CX231XX_BOARD_TERRATEC_GRABBY 22
> +#define CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD 23
>
>  /* Limits minimum and default number of buffers */
>  #define CX231XX_MIN_BUF                 4
>

regards
Antti

-- 
http://palosaari.fi/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] [media] cx231xx: Initial support Evromedia USB Full Hybrid Full HD
  2017-01-09 15:23 [PATCH] [media] cx231xx: Initial support Evromedia USB Full Hybrid Full HD Oleh Kravchenko
  2017-01-09 16:59 ` Antti Palosaari
@ 2017-01-09 20:08 ` kbuild test robot
  1 sibling, 0 replies; 14+ messages in thread
From: kbuild test robot @ 2017-01-09 20:08 UTC (permalink / raw)
  To: Oleh Kravchenko; +Cc: kbuild-all, linux-media, hverkuil, Oleh Kravchenko

[-- Attachment #1: Type: text/plain, Size: 4587 bytes --]

Hi Oleh,

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.10-rc3 next-20170106]
[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/Oleh-Kravchenko/cx231xx-Initial-support-Evromedia-USB-Full-Hybrid-Full-HD/20170110-033153
base:   git://linuxtv.org/media_tree.git master
config: x86_64-rhel-7.2 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/media/usb/cx231xx/cx231xx-dvb.c: In function 'dvb_init':
>> drivers/media/usb/cx231xx/cx231xx-dvb.c:967:32: warning: passing argument 3 of 'strlcpy' makes integer from pointer without a cast [-Wint-conversion]
      strlcpy(info.type, "si2168", info.type);
                                   ^~~~
   In file included from include/linux/bitmap.h:8:0,
                    from include/linux/cpumask.h:11,
                    from arch/x86/include/asm/cpumask.h:4,
                    from arch/x86/include/asm/msr.h:10,
                    from arch/x86/include/asm/processor.h:20,
                    from arch/x86/include/asm/cpufeature.h:4,
                    from arch/x86/include/asm/thread_info.h:52,
                    from include/linux/thread_info.h:25,
                    from arch/x86/include/asm/preempt.h:6,
                    from include/linux/preempt.h:59,
                    from include/linux/spinlock.h:50,
                    from include/linux/seqlock.h:35,
                    from include/linux/time.h:5,
                    from include/linux/videodev2.h:59,
                    from drivers/media/usb/cx231xx/cx231xx.h:25,
                    from drivers/media/usb/cx231xx/cx231xx-dvb.c:22:
   include/linux/string.h:27:8: note: expected 'size_t {aka long unsigned int}' but argument is of type 'char *'
    size_t strlcpy(char *, const char *, size_t);
           ^~~~~~~
   drivers/media/usb/cx231xx/cx231xx-dvb.c:999:32: warning: passing argument 3 of 'strlcpy' makes integer from pointer without a cast [-Wint-conversion]
      strlcpy(info.type, "si2157", info.type);
                                   ^~~~
   In file included from include/linux/bitmap.h:8:0,
                    from include/linux/cpumask.h:11,
                    from arch/x86/include/asm/cpumask.h:4,
                    from arch/x86/include/asm/msr.h:10,
                    from arch/x86/include/asm/processor.h:20,
                    from arch/x86/include/asm/cpufeature.h:4,
                    from arch/x86/include/asm/thread_info.h:52,
                    from include/linux/thread_info.h:25,
                    from arch/x86/include/asm/preempt.h:6,
                    from include/linux/preempt.h:59,
                    from include/linux/spinlock.h:50,
                    from include/linux/seqlock.h:35,
                    from include/linux/time.h:5,
                    from include/linux/videodev2.h:59,
                    from drivers/media/usb/cx231xx/cx231xx.h:25,
                    from drivers/media/usb/cx231xx/cx231xx-dvb.c:22:
   include/linux/string.h:27:8: note: expected 'size_t {aka long unsigned int}' but argument is of type 'char *'
    size_t strlcpy(char *, const char *, size_t);
           ^~~~~~~

vim +/strlcpy +967 drivers/media/usb/cx231xx/cx231xx-dvb.c

   951			break;
   952	
   953		case CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD:
   954		{
   955			struct si2157_config si2157_config = {};
   956			struct si2168_config si2168_config = {};
   957			struct i2c_board_info info = {};
   958			struct i2c_client *client;
   959			struct i2c_adapter *adapter;
   960	
   961			/* attach demodulator chip */
   962			si2168_config.ts_mode = SI2168_TS_SERIAL; /* from *.inf file */
   963			si2168_config.fe = &dev->dvb->frontend;
   964			si2168_config.i2c_adapter = &adapter;
   965			si2168_config.ts_clock_inv = true;
   966	
 > 967			strlcpy(info.type, "si2168", info.type);
   968			info.addr = dev->board.demod_addr;
   969			info.platform_data = &si2168_config;
   970	
   971			request_module(info.type);
   972			client = i2c_new_device(demod_i2c, &info);
   973	
   974			if (client == NULL || client->dev.driver == NULL || dev->dvb->frontend == NULL) {
   975				dev_err(dev->dev, "Failed to attach Si2168 front end\n");

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38277 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] [media] cx231xx: Initial support Evromedia USB Full Hybrid Full HD
  2017-01-09 16:59 ` Antti Palosaari
@ 2017-01-09 21:49   ` Oleh Kravchenko
  2017-01-09 22:35     ` Antti Palosaari
  0 siblings, 1 reply; 14+ messages in thread
From: Oleh Kravchenko @ 2017-01-09 21:49 UTC (permalink / raw)
  To: Antti Palosaari, linux-media, hverkuil

Hello!

On 09.01.17 18:59, Antti Palosaari wrote:
> 
> 
> On 01/09/2017 05:23 PM, Oleh Kravchenko wrote:
>> This patch provide only digital support.
>>
>> The device is based on Si2168 30-demodulator,
>> Si2158-20 tuner and CX23102-11Z chipset;
>> USB id: 1b80:d3b2.
>>
>> Status:
>> - DVB-T2 works fine;
>> - Composite and SVideo works fine;
>> - Analog not implemented.
>>
>> Signed-off-by: Oleh Kravchenko <oleg@kaa.org.ua>
>> Tested-by: Oleh Kravchenko <oleg@kaa.org.ua>
>> ---
>>  drivers/media/usb/cx231xx/Kconfig         |  1 +
>>  drivers/media/usb/cx231xx/cx231xx-cards.c | 29 +++++++++++++
>>  drivers/media/usb/cx231xx/cx231xx-dvb.c   | 71 +++++++++++++++++++++++++++++++
>>  drivers/media/usb/cx231xx/cx231xx-i2c.c   | 37 ++++++++++++++++
>>  drivers/media/usb/cx231xx/cx231xx.h       |  1 +
>>  5 files changed, 139 insertions(+)
>>
>> diff --git a/drivers/media/usb/cx231xx/Kconfig b/drivers/media/usb/cx231xx/Kconfig
>> index 0cced3e..58de80b 100644
>> --- a/drivers/media/usb/cx231xx/Kconfig
>> +++ b/drivers/media/usb/cx231xx/Kconfig
>> @@ -50,6 +50,7 @@ config VIDEO_CX231XX_DVB
>>      select DVB_LGDT3306A if MEDIA_SUBDRV_AUTOSELECT
>>      select DVB_TDA18271C2DD if MEDIA_SUBDRV_AUTOSELECT
>>      select DVB_SI2165 if MEDIA_SUBDRV_AUTOSELECT
>> +    select DVB_SI2168 if MEDIA_SUBDRV_AUTOSELECT
>>      select MEDIA_TUNER_SI2157 if MEDIA_SUBDRV_AUTOSELECT
>>
>>      ---help---
>> diff --git a/drivers/media/usb/cx231xx/cx231xx-cards.c b/drivers/media/usb/cx231xx/cx231xx-cards.c
>> index 36bc254..9b1df5a 100644
>> --- a/drivers/media/usb/cx231xx/cx231xx-cards.c
>> +++ b/drivers/media/usb/cx231xx/cx231xx-cards.c
>> @@ -841,6 +841,33 @@ struct cx231xx_board cx231xx_boards[] = {
>>              .gpio = NULL,
>>          } },
>>      },
>> +    [CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD] = {
>> +        .name = "Evromedia USB Full Hybrid Full HD",
>> +        .tuner_type = TUNER_ABSENT,
>> +        .demod_addr = 0xc8 >> 1,
>> +        .demod_i2c_master = I2C_1_MUX_3,
>> +        .has_dvb = 1,
>> +        .ir_i2c_master = I2C_0,
>> +        .norm = V4L2_STD_PAL,
>> +        .output_mode = OUT_MODE_VIP11,
>> +        .tuner_addr = 0xc0 >> 1,
> 
> These "8-bit" I2C addresses looks funny, but if that's used by cx231xx driver then leave...

To avoid misunderstanding, Windows driver use shifted i2c addresses.
 
>> +        .tuner_i2c_master = I2C_2,
>> +        .input = {{
>> +            .type = CX231XX_VMUX_TELEVISION,
>> +            .vmux = 0,
>> +            .amux = CX231XX_AMUX_VIDEO,
>> +        }, {
>> +            .type = CX231XX_VMUX_COMPOSITE1,
>> +            .vmux = CX231XX_VIN_2_1,
>> +            .amux = CX231XX_AMUX_LINE_IN,
>> +        }, {
>> +            .type = CX231XX_VMUX_SVIDEO,
>> +            .vmux = CX231XX_VIN_1_1 |
>> +                (CX231XX_VIN_1_2 << 8) |
>> +                CX25840_SVIDEO_ON,
>> +            .amux = CX231XX_AMUX_LINE_IN,
>> +        } },
>> +    },
>>  };
>>  const unsigned int cx231xx_bcount = ARRAY_SIZE(cx231xx_boards);
>>
>> @@ -908,6 +935,8 @@ struct usb_device_id cx231xx_id_table[] = {
>>       .driver_info = CX231XX_BOARD_OTG102},
>>      {USB_DEVICE(USB_VID_TERRATEC, 0x00a6),
>>       .driver_info = CX231XX_BOARD_TERRATEC_GRABBY},
>> +    {USB_DEVICE(0x1b80, 0xd3b2),
>> +    .driver_info = CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD},
>>      {},
>>  };
>>
>> diff --git a/drivers/media/usb/cx231xx/cx231xx-dvb.c b/drivers/media/usb/cx231xx/cx231xx-dvb.c
>> index 1417515..08472a3 100644
>> --- a/drivers/media/usb/cx231xx/cx231xx-dvb.c
>> +++ b/drivers/media/usb/cx231xx/cx231xx-dvb.c
>> @@ -33,6 +33,7 @@
>>  #include "s5h1411.h"
>>  #include "lgdt3305.h"
>>  #include "si2165.h"
>> +#include "si2168.h"
>>  #include "mb86a20s.h"
>>  #include "si2157.h"
>>  #include "lgdt3306a.h"
>> @@ -949,6 +950,76 @@ static int dvb_init(struct cx231xx *dev)
>>                 &pv_tda18271_config);
>>          break;
>>
>> +    case CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD:
>> +    {
>> +        struct si2157_config si2157_config = {};
>> +        struct si2168_config si2168_config = {};
>> +        struct i2c_board_info info = {};
>> +        struct i2c_client *client;
>> +        struct i2c_adapter *adapter;
>> +
>> +        /* attach demodulator chip */
>> +        si2168_config.ts_mode = SI2168_TS_SERIAL; /* from *.inf file */
>> +        si2168_config.fe = &dev->dvb->frontend;
>> +        si2168_config.i2c_adapter = &adapter;
>> +        si2168_config.ts_clock_inv = true;
>> +
>> +        strlcpy(info.type, "si2168", info.type);
>> +        info.addr = dev->board.demod_addr;
>> +        info.platform_data = &si2168_config;
>> +
>> +        request_module(info.type);
>> +        client = i2c_new_device(demod_i2c, &info);
>> +
>> +        if (client == NULL || client->dev.driver == NULL || dev->dvb->frontend == NULL) {
> 
> No need to check frontend here, or at least I cannot see why it should? Does the cx231xx initialize with some special value before calling si2168 probe - which will set it? client and driver will be null in case si2168 probe fails. Also, frontend pointer is not set if si2168 probe fails.
> 

I just copy code from CX231XX_BOARD_HAUPPAUGE_930C_HD_1114xx case, are you sure about it?

>> +            dev_err(dev->dev, "Failed to attach Si2168 front end\n");
>> +            result = -EINVAL;
>> +            goto out_free;
>> +        }
>> +
>> +        if (!try_module_get(client->dev.driver->owner)) {
>> +            i2c_unregister_device(client);
>> +            result = -ENODEV;
>> +            goto out_free;
>> +        }
>> +
>> +        dvb->i2c_client_demod = client;
>> +        dev->dvb->frontend->ops.i2c_gate_ctrl = NULL;
> 
> si2168 does not use nor set i2c_gate_ctrl callback. No need to nullify it in any case.

Done!
 
>> +        dvb->frontend->callback = cx231xx_tuner_callback;
>> +
>> +        /* attach tuner chip */
>> +        si2157_config.fe = dev->dvb->frontend;
>> +#ifdef CONFIG_MEDIA_CONTROLLER_DVB
>> +        si2157_config.mdev = dev->media_dev;
>> +#endif
>> +        si2157_config.if_port = 1;
>> +        si2157_config.inversion = false;
>> +
>> +        memset(&info, 0, sizeof(info));
>> +        strlcpy(info.type, "si2157", info.type);
>> +        info.addr = dev->board.tuner_addr;
>> +        info.platform_data = &si2157_config;
>> +
>> +        request_module("si2157");
>> +        client = i2c_new_device(tuner_i2c, &info);
>> +
>> +        if (client == NULL || client->dev.driver == NULL) {
>> +            dvb_frontend_detach(dev->dvb->frontend);
>> +            result = -ENODEV;
>> +            goto out_free;
>> +        }
> 
> Does this error handling work? Have you tested it? I suspect it will not. You should likely decrease si2168 module reference counter and then unregister si2168 driver.
> 

The same error handling for CX231XX_BOARD_HAUPPAUGE_930C_HD_1114xx, why it bad?

>> +
>> +        if (!try_module_get(client->dev.driver->owner)) {
>> +            i2c_unregister_device(client);
>> +            dvb_frontend_detach(dev->dvb->frontend);
>> +            result = -ENODEV;
>> +            goto out_free;
>> +        }
>> +
>> +        dev->cx231xx_reset_analog_tuner = NULL;
>> +        dev->dvb->i2c_client_tuner = client;
>> +        break;
>> +    }
>>      default:
>>          dev_err(dev->dev,
>>              "%s/2: The frontend of your DVB/ATSC card isn't supported yet\n",
>> diff --git a/drivers/media/usb/cx231xx/cx231xx-i2c.c b/drivers/media/usb/cx231xx/cx231xx-i2c.c
>> index 35e9acf..60412ec 100644
>> --- a/drivers/media/usb/cx231xx/cx231xx-i2c.c
>> +++ b/drivers/media/usb/cx231xx/cx231xx-i2c.c
>> @@ -171,6 +171,43 @@ static int cx231xx_i2c_send_bytes(struct i2c_adapter *i2c_adap,
>>          bus->i2c_nostop = 0;
>>          bus->i2c_reserve = 0;
>>
>> +    } else if (dev->model == CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD
>> +        && msg->addr == dev->tuner_addr
>> +        && msg->len > 4) {
>> +        /* special case for Evromedia USB Full Hybrid Full HD tuner chip */
>> +        size = msg->len;
>> +        saddr_len = 1;
>> +
>> +        /* adjust the length to correct length */
>> +        size -= saddr_len;
>> +
>> +        buf_ptr = (u8*)(msg->buf + 1);
>> +
>> +        do {
>> +            /* prepare xfer_data struct */
>> +            req_data.dev_addr = msg->addr;
>> +            req_data.direction = msg->flags;
>> +            req_data.saddr_len = saddr_len;
>> +            req_data.saddr_dat = msg->buf[0];
>> +            req_data.buf_size = size > 4 ? 4 : size;
>> +            req_data.p_buffer = (u8*)(buf_ptr + loop * 4);
>> +
>> +            bus->i2c_nostop = (size > 4) ? 1 : 0;
>> +            bus->i2c_reserve = (loop == 0) ? 0 : 1;
>> +
>> +            /* usb send command */
>> +            status = dev->cx231xx_send_usb_command(bus, &req_data);
>> +            loop++;
>> +
>> +            if (size >= 4) {
>> +                size -= 4;
>> +            } else {
>> +                size = 0;
>> +            }
>> +        } while (size > 0);
>> +
>> +        bus->i2c_nostop = 0;
>> +        bus->i2c_reserve = 0;
> 
> I don't follow (and I looked only that patch, not whole cx231xx driver) why this I2C adapter logic is limited to that device only. si2168 and si2157 drivers applies just standard multibyte I2C read and write operations - no write+read op used at all.
> 
> These I2C adapter routines should be same for every device. Maybe original logic is somehow wrong and it should be fixed instead of adding new device specific logic.

I can rewrite it, but who will test it? :)
 
>>      } else {        /* regular case */
>>
>>          /* prepare xfer_data struct */
>> diff --git a/drivers/media/usb/cx231xx/cx231xx.h b/drivers/media/usb/cx231xx/cx231xx.h
>> index 90c8676..d9792ea 100644
>> --- a/drivers/media/usb/cx231xx/cx231xx.h
>> +++ b/drivers/media/usb/cx231xx/cx231xx.h
>> @@ -78,6 +78,7 @@
>>  #define CX231XX_BOARD_HAUPPAUGE_930C_HD_1114xx 20
>>  #define CX231XX_BOARD_HAUPPAUGE_955Q 21
>>  #define CX231XX_BOARD_TERRATEC_GRABBY 22
>> +#define CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD 23
>>
>>  /* Limits minimum and default number of buffers */
>>  #define CX231XX_MIN_BUF                 4
>>
> 
> regards
> Antti
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] [media] cx231xx: Initial support Evromedia USB Full Hybrid Full HD
  2017-01-09 21:49   ` Oleh Kravchenko
@ 2017-01-09 22:35     ` Antti Palosaari
  0 siblings, 0 replies; 14+ messages in thread
From: Antti Palosaari @ 2017-01-09 22:35 UTC (permalink / raw)
  To: Oleh Kravchenko, linux-media, hverkuil



On 01/09/2017 11:49 PM, Oleh Kravchenko wrote:
> Hello!
>
> On 09.01.17 18:59, Antti Palosaari wrote:
>>
>>
>> On 01/09/2017 05:23 PM, Oleh Kravchenko wrote:
>>> This patch provide only digital support.
>>>
>>> The device is based on Si2168 30-demodulator,
>>> Si2158-20 tuner and CX23102-11Z chipset;
>>> USB id: 1b80:d3b2.
>>>
>>> Status:
>>> - DVB-T2 works fine;
>>> - Composite and SVideo works fine;
>>> - Analog not implemented.
>>>
>>> Signed-off-by: Oleh Kravchenko <oleg@kaa.org.ua>
>>> Tested-by: Oleh Kravchenko <oleg@kaa.org.ua>
>>> ---
>>>  drivers/media/usb/cx231xx/Kconfig         |  1 +
>>>  drivers/media/usb/cx231xx/cx231xx-cards.c | 29 +++++++++++++
>>>  drivers/media/usb/cx231xx/cx231xx-dvb.c   | 71 +++++++++++++++++++++++++++++++
>>>  drivers/media/usb/cx231xx/cx231xx-i2c.c   | 37 ++++++++++++++++
>>>  drivers/media/usb/cx231xx/cx231xx.h       |  1 +
>>>  5 files changed, 139 insertions(+)
>>>
>>> diff --git a/drivers/media/usb/cx231xx/Kconfig b/drivers/media/usb/cx231xx/Kconfig
>>> index 0cced3e..58de80b 100644
>>> --- a/drivers/media/usb/cx231xx/Kconfig
>>> +++ b/drivers/media/usb/cx231xx/Kconfig
>>> @@ -50,6 +50,7 @@ config VIDEO_CX231XX_DVB
>>>      select DVB_LGDT3306A if MEDIA_SUBDRV_AUTOSELECT
>>>      select DVB_TDA18271C2DD if MEDIA_SUBDRV_AUTOSELECT
>>>      select DVB_SI2165 if MEDIA_SUBDRV_AUTOSELECT
>>> +    select DVB_SI2168 if MEDIA_SUBDRV_AUTOSELECT
>>>      select MEDIA_TUNER_SI2157 if MEDIA_SUBDRV_AUTOSELECT
>>>
>>>      ---help---
>>> diff --git a/drivers/media/usb/cx231xx/cx231xx-cards.c b/drivers/media/usb/cx231xx/cx231xx-cards.c
>>> index 36bc254..9b1df5a 100644
>>> --- a/drivers/media/usb/cx231xx/cx231xx-cards.c
>>> +++ b/drivers/media/usb/cx231xx/cx231xx-cards.c
>>> @@ -841,6 +841,33 @@ struct cx231xx_board cx231xx_boards[] = {
>>>              .gpio = NULL,
>>>          } },
>>>      },
>>> +    [CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD] = {
>>> +        .name = "Evromedia USB Full Hybrid Full HD",
>>> +        .tuner_type = TUNER_ABSENT,
>>> +        .demod_addr = 0xc8 >> 1,
>>> +        .demod_i2c_master = I2C_1_MUX_3,
>>> +        .has_dvb = 1,
>>> +        .ir_i2c_master = I2C_0,
>>> +        .norm = V4L2_STD_PAL,
>>> +        .output_mode = OUT_MODE_VIP11,
>>> +        .tuner_addr = 0xc0 >> 1,
>>
>> These "8-bit" I2C addresses looks funny, but if that's used by cx231xx driver then leave...
>
> To avoid misunderstanding, Windows driver use shifted i2c addresses.

eh, I2C addresses are 7-bit + lsb which is bit read/write. Just use 
correct addresses. I assume you saw "8-bit" addresses on usb sniffs - 
that is very commonly used for usb hardware api...

>
>>> +        .tuner_i2c_master = I2C_2,
>>> +        .input = {{
>>> +            .type = CX231XX_VMUX_TELEVISION,
>>> +            .vmux = 0,
>>> +            .amux = CX231XX_AMUX_VIDEO,
>>> +        }, {
>>> +            .type = CX231XX_VMUX_COMPOSITE1,
>>> +            .vmux = CX231XX_VIN_2_1,
>>> +            .amux = CX231XX_AMUX_LINE_IN,
>>> +        }, {
>>> +            .type = CX231XX_VMUX_SVIDEO,
>>> +            .vmux = CX231XX_VIN_1_1 |
>>> +                (CX231XX_VIN_1_2 << 8) |
>>> +                CX25840_SVIDEO_ON,
>>> +            .amux = CX231XX_AMUX_LINE_IN,
>>> +        } },
>>> +    },
>>>  };
>>>  const unsigned int cx231xx_bcount = ARRAY_SIZE(cx231xx_boards);
>>>
>>> @@ -908,6 +935,8 @@ struct usb_device_id cx231xx_id_table[] = {
>>>       .driver_info = CX231XX_BOARD_OTG102},
>>>      {USB_DEVICE(USB_VID_TERRATEC, 0x00a6),
>>>       .driver_info = CX231XX_BOARD_TERRATEC_GRABBY},
>>> +    {USB_DEVICE(0x1b80, 0xd3b2),
>>> +    .driver_info = CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD},
>>>      {},
>>>  };
>>>
>>> diff --git a/drivers/media/usb/cx231xx/cx231xx-dvb.c b/drivers/media/usb/cx231xx/cx231xx-dvb.c
>>> index 1417515..08472a3 100644
>>> --- a/drivers/media/usb/cx231xx/cx231xx-dvb.c
>>> +++ b/drivers/media/usb/cx231xx/cx231xx-dvb.c
>>> @@ -33,6 +33,7 @@
>>>  #include "s5h1411.h"
>>>  #include "lgdt3305.h"
>>>  #include "si2165.h"
>>> +#include "si2168.h"
>>>  #include "mb86a20s.h"
>>>  #include "si2157.h"
>>>  #include "lgdt3306a.h"
>>> @@ -949,6 +950,76 @@ static int dvb_init(struct cx231xx *dev)
>>>                 &pv_tda18271_config);
>>>          break;
>>>
>>> +    case CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD:
>>> +    {
>>> +        struct si2157_config si2157_config = {};
>>> +        struct si2168_config si2168_config = {};
>>> +        struct i2c_board_info info = {};
>>> +        struct i2c_client *client;
>>> +        struct i2c_adapter *adapter;
>>> +
>>> +        /* attach demodulator chip */
>>> +        si2168_config.ts_mode = SI2168_TS_SERIAL; /* from *.inf file */
>>> +        si2168_config.fe = &dev->dvb->frontend;
>>> +        si2168_config.i2c_adapter = &adapter;
>>> +        si2168_config.ts_clock_inv = true;
>>> +
>>> +        strlcpy(info.type, "si2168", info.type);
>>> +        info.addr = dev->board.demod_addr;
>>> +        info.platform_data = &si2168_config;
>>> +
>>> +        request_module(info.type);
>>> +        client = i2c_new_device(demod_i2c, &info);
>>> +
>>> +        if (client == NULL || client->dev.driver == NULL || dev->dvb->frontend == NULL) {
>>
>> No need to check frontend here, or at least I cannot see why it should? Does the cx231xx initialize with some special value before calling si2168 probe - which will set it? client and driver will be null in case si2168 probe fails. Also, frontend pointer is not set if si2168 probe fails.
>>
>
> I just copy code from CX231XX_BOARD_HAUPPAUGE_930C_HD_1114xx case, are you sure about it?

I am pretty sure. But I am not any familiar with cx231xx driver. It is 
your work to explain why it is needed. No idea why it is used for 
hauppauge card, but it simply looks wrong for my eyes.

>
>>> +            dev_err(dev->dev, "Failed to attach Si2168 front end\n");
>>> +            result = -EINVAL;
>>> +            goto out_free;
>>> +        }
>>> +
>>> +        if (!try_module_get(client->dev.driver->owner)) {
>>> +            i2c_unregister_device(client);
>>> +            result = -ENODEV;
>>> +            goto out_free;
>>> +        }
>>> +
>>> +        dvb->i2c_client_demod = client;
>>> +        dev->dvb->frontend->ops.i2c_gate_ctrl = NULL;
>>
>> si2168 does not use nor set i2c_gate_ctrl callback. No need to nullify it in any case.
>
> Done!
>
>>> +        dvb->frontend->callback = cx231xx_tuner_callback;
>>> +
>>> +        /* attach tuner chip */
>>> +        si2157_config.fe = dev->dvb->frontend;
>>> +#ifdef CONFIG_MEDIA_CONTROLLER_DVB
>>> +        si2157_config.mdev = dev->media_dev;
>>> +#endif
>>> +        si2157_config.if_port = 1;
>>> +        si2157_config.inversion = false;
>>> +
>>> +        memset(&info, 0, sizeof(info));
>>> +        strlcpy(info.type, "si2157", info.type);
>>> +        info.addr = dev->board.tuner_addr;
>>> +        info.platform_data = &si2157_config;
>>> +
>>> +        request_module("si2157");
>>> +        client = i2c_new_device(tuner_i2c, &info);
>>> +
>>> +        if (client == NULL || client->dev.driver == NULL) {
>>> +            dvb_frontend_detach(dev->dvb->frontend);
>>> +            result = -ENODEV;
>>> +            goto out_free;
>>> +        }
>>
>> Does this error handling work? Have you tested it? I suspect it will not. You should likely decrease si2168 module reference counter and then unregister si2168 driver.
>>
>
> The same error handling for CX231XX_BOARD_HAUPPAUGE_930C_HD_1114xx, why it bad?

Because frontend or tuner is not attached and you call 
dvb_frontend_detach() which will likely do nothing. You are only 
registered si2168 driver and increased module reference count.

Test error handling by hacking si2168 and si2157 driver returning some 
error code from the probe. What I think what happens when for example 
si2157 driver probe fails it leaves si2168 driver there registered and 
module use count increased => you cannot remove si2168 driver from 
memory (rmmod).

>
>>> +
>>> +        if (!try_module_get(client->dev.driver->owner)) {
>>> +            i2c_unregister_device(client);
>>> +            dvb_frontend_detach(dev->dvb->frontend);
>>> +            result = -ENODEV;
>>> +            goto out_free;
>>> +        }
>>> +
>>> +        dev->cx231xx_reset_analog_tuner = NULL;
>>> +        dev->dvb->i2c_client_tuner = client;
>>> +        break;
>>> +    }
>>>      default:
>>>          dev_err(dev->dev,
>>>              "%s/2: The frontend of your DVB/ATSC card isn't supported yet\n",
>>> diff --git a/drivers/media/usb/cx231xx/cx231xx-i2c.c b/drivers/media/usb/cx231xx/cx231xx-i2c.c
>>> index 35e9acf..60412ec 100644
>>> --- a/drivers/media/usb/cx231xx/cx231xx-i2c.c
>>> +++ b/drivers/media/usb/cx231xx/cx231xx-i2c.c
>>> @@ -171,6 +171,43 @@ static int cx231xx_i2c_send_bytes(struct i2c_adapter *i2c_adap,
>>>          bus->i2c_nostop = 0;
>>>          bus->i2c_reserve = 0;
>>>
>>> +    } else if (dev->model == CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD
>>> +        && msg->addr == dev->tuner_addr
>>> +        && msg->len > 4) {
>>> +        /* special case for Evromedia USB Full Hybrid Full HD tuner chip */
>>> +        size = msg->len;
>>> +        saddr_len = 1;
>>> +
>>> +        /* adjust the length to correct length */
>>> +        size -= saddr_len;
>>> +
>>> +        buf_ptr = (u8*)(msg->buf + 1);
>>> +
>>> +        do {
>>> +            /* prepare xfer_data struct */
>>> +            req_data.dev_addr = msg->addr;
>>> +            req_data.direction = msg->flags;
>>> +            req_data.saddr_len = saddr_len;
>>> +            req_data.saddr_dat = msg->buf[0];
>>> +            req_data.buf_size = size > 4 ? 4 : size;
>>> +            req_data.p_buffer = (u8*)(buf_ptr + loop * 4);
>>> +
>>> +            bus->i2c_nostop = (size > 4) ? 1 : 0;
>>> +            bus->i2c_reserve = (loop == 0) ? 0 : 1;
>>> +
>>> +            /* usb send command */
>>> +            status = dev->cx231xx_send_usb_command(bus, &req_data);
>>> +            loop++;
>>> +
>>> +            if (size >= 4) {
>>> +                size -= 4;
>>> +            } else {
>>> +                size = 0;
>>> +            }
>>> +        } while (size > 0);
>>> +
>>> +        bus->i2c_nostop = 0;
>>> +        bus->i2c_reserve = 0;
>>
>> I don't follow (and I looked only that patch, not whole cx231xx driver) why this I2C adapter logic is limited to that device only. si2168 and si2157 drivers applies just standard multibyte I2C read and write operations - no write+read op used at all.
>>
>> These I2C adapter routines should be same for every device. Maybe original logic is somehow wrong and it should be fixed instead of adding new device specific logic.
>
> I can rewrite it, but who will test it? :)

gah, there seems to be a lot of existing hacks on that I2C adapter 
implementation :/ Now it is very hard to fix properly... dunno what to do.


>
>>>      } else {        /* regular case */
>>>
>>>          /* prepare xfer_data struct */
>>> diff --git a/drivers/media/usb/cx231xx/cx231xx.h b/drivers/media/usb/cx231xx/cx231xx.h
>>> index 90c8676..d9792ea 100644
>>> --- a/drivers/media/usb/cx231xx/cx231xx.h
>>> +++ b/drivers/media/usb/cx231xx/cx231xx.h
>>> @@ -78,6 +78,7 @@
>>>  #define CX231XX_BOARD_HAUPPAUGE_930C_HD_1114xx 20
>>>  #define CX231XX_BOARD_HAUPPAUGE_955Q 21
>>>  #define CX231XX_BOARD_TERRATEC_GRABBY 22
>>> +#define CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD 23
>>>
>>>  /* Limits minimum and default number of buffers */
>>>  #define CX231XX_MIN_BUF                 4
>>>
>>
>> regards
>> Antti
>>

-- 
http://palosaari.fi/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] [media] cx231xx: Initial support Evromedia USB Full Hybrid Full HD
@ 2017-01-29 18:03 Oleh Kravchenko
  0 siblings, 0 replies; 14+ messages in thread
From: Oleh Kravchenko @ 2017-01-29 18:03 UTC (permalink / raw)
  To: Steven Toth, Linux Media Mailing List, Jacob Johan Verkuil,
	Antti Palosaari
  Cc: Oleh Kravchenko

This patch provide only digital support.
The device is based on Si2168 30-demodulator,
Si2158-20 tuner and CX23102-11Z chipset;
USB id: 1b80:d3b2.

Status:
- DVB-T2 works fine;
- Composite and SVideo works fine;
- Analog not implemented.

Signed-off-by: Oleh Kravchenko <oleg@kaa.org.ua>
Tested-by: Oleh Kravchenko <oleg@kaa.org.ua>
---
 drivers/media/usb/cx231xx/Kconfig         |  1 +
 drivers/media/usb/cx231xx/cx231xx-cards.c | 29 +++++++++++++
 drivers/media/usb/cx231xx/cx231xx-dvb.c   | 70 +++++++++++++++++++++++++++++++
 drivers/media/usb/cx231xx/cx231xx.h       |  1 +
 4 files changed, 101 insertions(+)

diff --git a/drivers/media/usb/cx231xx/Kconfig b/drivers/media/usb/cx231xx/Kconfig
index 0cced3e..58de80b 100644
--- a/drivers/media/usb/cx231xx/Kconfig
+++ b/drivers/media/usb/cx231xx/Kconfig
@@ -50,6 +50,7 @@ config VIDEO_CX231XX_DVB
 	select DVB_LGDT3306A if MEDIA_SUBDRV_AUTOSELECT
 	select DVB_TDA18271C2DD if MEDIA_SUBDRV_AUTOSELECT
 	select DVB_SI2165 if MEDIA_SUBDRV_AUTOSELECT
+	select DVB_SI2168 if MEDIA_SUBDRV_AUTOSELECT
 	select MEDIA_TUNER_SI2157 if MEDIA_SUBDRV_AUTOSELECT
 
 	---help---
diff --git a/drivers/media/usb/cx231xx/cx231xx-cards.c b/drivers/media/usb/cx231xx/cx231xx-cards.c
index 36bc254..f730fdb 100644
--- a/drivers/media/usb/cx231xx/cx231xx-cards.c
+++ b/drivers/media/usb/cx231xx/cx231xx-cards.c
@@ -841,6 +841,33 @@ struct cx231xx_board cx231xx_boards[] = {
 			.gpio = NULL,
 		} },
 	},
+	[CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD] = {
+		.name = "Evromedia USB Full Hybrid Full HD",
+		.tuner_type = TUNER_ABSENT,
+		.demod_addr = 0x64, /* 0xc8 >> 1 */
+		.demod_i2c_master = I2C_1_MUX_3,
+		.has_dvb = 1,
+		.ir_i2c_master = I2C_0,
+		.norm = V4L2_STD_PAL,
+		.output_mode = OUT_MODE_VIP11,
+		.tuner_addr = 0x60, /* 0xc0 >> 1 */
+		.tuner_i2c_master = I2C_2,
+		.input = {{
+			.type = CX231XX_VMUX_TELEVISION,
+			.vmux = 0,
+			.amux = CX231XX_AMUX_VIDEO,
+		}, {
+			.type = CX231XX_VMUX_COMPOSITE1,
+			.vmux = CX231XX_VIN_2_1,
+			.amux = CX231XX_AMUX_LINE_IN,
+		}, {
+			.type = CX231XX_VMUX_SVIDEO,
+			.vmux = CX231XX_VIN_1_1 |
+				(CX231XX_VIN_1_2 << 8) |
+				CX25840_SVIDEO_ON,
+			.amux = CX231XX_AMUX_LINE_IN,
+		} },
+	},
 };
 const unsigned int cx231xx_bcount = ARRAY_SIZE(cx231xx_boards);
 
@@ -908,6 +935,8 @@ struct usb_device_id cx231xx_id_table[] = {
 	 .driver_info = CX231XX_BOARD_OTG102},
 	{USB_DEVICE(USB_VID_TERRATEC, 0x00a6),
 	 .driver_info = CX231XX_BOARD_TERRATEC_GRABBY},
+	{USB_DEVICE(0x1b80, 0xd3b2),
+	.driver_info = CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD},
 	{},
 };
 
diff --git a/drivers/media/usb/cx231xx/cx231xx-dvb.c b/drivers/media/usb/cx231xx/cx231xx-dvb.c
index 2868546..46427fd 100644
--- a/drivers/media/usb/cx231xx/cx231xx-dvb.c
+++ b/drivers/media/usb/cx231xx/cx231xx-dvb.c
@@ -33,6 +33,7 @@
 #include "s5h1411.h"
 #include "lgdt3305.h"
 #include "si2165.h"
+#include "si2168.h"
 #include "mb86a20s.h"
 #include "si2157.h"
 #include "lgdt3306a.h"
@@ -949,6 +950,75 @@ static int dvb_init(struct cx231xx *dev)
 			   &pv_tda18271_config);
 		break;
 
+	case CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD:
+	{
+		struct si2157_config si2157_config = {};
+		struct si2168_config si2168_config = {};
+		struct i2c_board_info info = {};
+		struct i2c_client *client;
+		struct i2c_adapter *adapter;
+
+		/* attach demodulator chip */
+		si2168_config.ts_mode = SI2168_TS_SERIAL; /* from *.inf file */
+		si2168_config.fe = &dev->dvb->frontend;
+		si2168_config.i2c_adapter = &adapter;
+		si2168_config.ts_clock_inv = true;
+
+		strlcpy(info.type, "si2168", sizeof(info.type));
+		info.addr = dev->board.demod_addr;
+		info.platform_data = &si2168_config;
+
+		request_module(info.type);
+		client = i2c_new_device(demod_i2c, &info);
+
+		if (client == NULL || client->dev.driver == NULL) {
+			result = -ENODEV;
+			goto out_free;
+		}
+
+		if (!try_module_get(client->dev.driver->owner)) {
+			i2c_unregister_device(client);
+			result = -ENODEV;
+			goto out_free;
+		}
+
+		dvb->i2c_client_demod = client;
+
+		/* attach tuner chip */
+		si2157_config.fe = dev->dvb->frontend;
+#ifdef CONFIG_MEDIA_CONTROLLER_DVB
+		si2157_config.mdev = dev->media_dev;
+#endif
+		si2157_config.if_port = 1;
+		si2157_config.inversion = false;
+
+		memset(&info, 0, sizeof(info));
+		strlcpy(info.type, "si2157", sizeof(info.type));
+		info.addr = dev->board.tuner_addr;
+		info.platform_data = &si2157_config;
+
+		request_module(info.type);
+		client = i2c_new_device(tuner_i2c, &info);
+
+		if (client == NULL || client->dev.driver == NULL) {
+			module_put(dvb->i2c_client_demod->dev.driver->owner);
+			i2c_unregister_device(dvb->i2c_client_demod);
+			result = -ENODEV;
+			goto out_free;
+		}
+
+		if (!try_module_get(client->dev.driver->owner)) {
+			i2c_unregister_device(client);
+			module_put(dvb->i2c_client_demod->dev.driver->owner);
+			i2c_unregister_device(dvb->i2c_client_demod);
+			result = -ENODEV;
+			goto out_free;
+		}
+
+		dev->cx231xx_reset_analog_tuner = NULL;
+		dev->dvb->i2c_client_tuner = client;
+		break;
+	}
 	default:
 		dev_err(dev->dev,
 			"%s/2: The frontend of your DVB/ATSC card isn't supported yet\n",
diff --git a/drivers/media/usb/cx231xx/cx231xx.h b/drivers/media/usb/cx231xx/cx231xx.h
index 90c8676..d9792ea 100644
--- a/drivers/media/usb/cx231xx/cx231xx.h
+++ b/drivers/media/usb/cx231xx/cx231xx.h
@@ -78,6 +78,7 @@
 #define CX231XX_BOARD_HAUPPAUGE_930C_HD_1114xx 20
 #define CX231XX_BOARD_HAUPPAUGE_955Q 21
 #define CX231XX_BOARD_TERRATEC_GRABBY 22
+#define CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD 23
 
 /* Limits minimum and default number of buffers */
 #define CX231XX_MIN_BUF                 4
-- 
2.10.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] [media] cx231xx: Initial support Evromedia USB Full Hybrid Full HD
  2017-01-11 14:00   ` Oleh Kravchenko
@ 2017-01-11 14:13     ` Steven Toth
  0 siblings, 0 replies; 14+ messages in thread
From: Steven Toth @ 2017-01-11 14:13 UTC (permalink / raw)
  To: Oleh Kravchenko
  Cc: Linux Media Mailing List, Jacob Johan (Hans) Verkuil, Antti Palosaari

On Wed, Jan 11, 2017 at 9:00 AM, Oleh Kravchenko <oleg@kaa.org.ua> wrote:
> On 11.01.17 15:53, Steven Toth wrote:
>>> diff --git a/drivers/media/usb/cx231xx/cx231xx-i2c.c b/drivers/media/usb/cx231xx/cx231xx-i2c.c
>>> index 35e9acf..60412ec 100644
>>> --- a/drivers/media/usb/cx231xx/cx231xx-i2c.c
>>> +++ b/drivers/media/usb/cx231xx/cx231xx-i2c.c
>>> @@ -171,6 +171,43 @@ static int cx231xx_i2c_send_bytes(struct i2c_adapter *i2c_adap,
>>>                 bus->i2c_nostop = 0;
>>>                 bus->i2c_reserve = 0;
>>>
>>> +       } else if (dev->model == CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD
>>> +               && msg->addr == dev->tuner_addr
>>> +               && msg->len > 4) {
>>> +               /* special case for Evromedia USB Full Hybrid Full HD tuner chip */
>>> +               size = msg->len;
>>> +               saddr_len = 1;
>>> +
>>> +               /* adjust the length to correct length */
>>> +               size -= saddr_len;
>>> +
>>> +               buf_ptr = (u8*)(msg->buf + 1);
>>> +
>>> +               do {
>>> +                       /* prepare xfer_data struct */
>>> +                       req_data.dev_addr = msg->addr;
>>> +                       req_data.direction = msg->flags;
>>> +                       req_data.saddr_len = saddr_len;
>>> +                       req_data.saddr_dat = msg->buf[0];
>>> +                       req_data.buf_size = size > 4 ? 4 : size;
>>> +                       req_data.p_buffer = (u8*)(buf_ptr + loop * 4);
>>> +
>>> +                       bus->i2c_nostop = (size > 4) ? 1 : 0;
>>> +                       bus->i2c_reserve = (loop == 0) ? 0 : 1;
>>> +
>>> +                       /* usb send command */
>>> +                       status = dev->cx231xx_send_usb_command(bus, &req_data);
>>> +                       loop++;
>>> +
>>> +                       if (size >= 4) {
>>> +                               size -= 4;
>>> +                       } else {
>>> +                               size = 0;
>>> +                       }
>>> +               } while (size > 0);
>>> +
>>> +               bus->i2c_nostop = 0;
>>> +               bus->i2c_reserve = 0;
>>>         } else {                /* regular case */
>>>
>>>                 /* prepare xfer_data struct */
>> If the i2c functionality is broken in some way, I suggest its fixed
>> first, along with a correct patch description, as a separate piece of
>> work. Lets not group this in with a board profile.
>>
>> Almost certainly we should never see a "if board == X" in any i2c
>> implementation without proper discussion.
>>
> I'm interested in accepting this patch :) What I should do?

Lets start the conversion with what's wrong with the current
implementation that prevents it from working with the new design.
Lets discuss the I2C needs of the parts you need to communicate with,
and why the current design is broken.

Then, lets think about a better solution.

A BOARD == X is the wrong solution to the problem.


-- 
Steven Toth - Kernel Labs
http://www.kernellabs.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] [media] cx231xx: Initial support Evromedia USB Full Hybrid Full HD
  2017-01-11 13:53 ` Steven Toth
@ 2017-01-11 14:00   ` Oleh Kravchenko
  2017-01-11 14:13     ` Steven Toth
  0 siblings, 1 reply; 14+ messages in thread
From: Oleh Kravchenko @ 2017-01-11 14:00 UTC (permalink / raw)
  To: Steven Toth
  Cc: Linux Media Mailing List, Jacob Johan (Hans) Verkuil, Antti Palosaari

On 11.01.17 15:53, Steven Toth wrote:
>> diff --git a/drivers/media/usb/cx231xx/cx231xx-i2c.c b/drivers/media/usb/cx231xx/cx231xx-i2c.c
>> index 35e9acf..60412ec 100644
>> --- a/drivers/media/usb/cx231xx/cx231xx-i2c.c
>> +++ b/drivers/media/usb/cx231xx/cx231xx-i2c.c
>> @@ -171,6 +171,43 @@ static int cx231xx_i2c_send_bytes(struct i2c_adapter *i2c_adap,
>>                 bus->i2c_nostop = 0;
>>                 bus->i2c_reserve = 0;
>>
>> +       } else if (dev->model == CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD
>> +               && msg->addr == dev->tuner_addr
>> +               && msg->len > 4) {
>> +               /* special case for Evromedia USB Full Hybrid Full HD tuner chip */
>> +               size = msg->len;
>> +               saddr_len = 1;
>> +
>> +               /* adjust the length to correct length */
>> +               size -= saddr_len;
>> +
>> +               buf_ptr = (u8*)(msg->buf + 1);
>> +
>> +               do {
>> +                       /* prepare xfer_data struct */
>> +                       req_data.dev_addr = msg->addr;
>> +                       req_data.direction = msg->flags;
>> +                       req_data.saddr_len = saddr_len;
>> +                       req_data.saddr_dat = msg->buf[0];
>> +                       req_data.buf_size = size > 4 ? 4 : size;
>> +                       req_data.p_buffer = (u8*)(buf_ptr + loop * 4);
>> +
>> +                       bus->i2c_nostop = (size > 4) ? 1 : 0;
>> +                       bus->i2c_reserve = (loop == 0) ? 0 : 1;
>> +
>> +                       /* usb send command */
>> +                       status = dev->cx231xx_send_usb_command(bus, &req_data);
>> +                       loop++;
>> +
>> +                       if (size >= 4) {
>> +                               size -= 4;
>> +                       } else {
>> +                               size = 0;
>> +                       }
>> +               } while (size > 0);
>> +
>> +               bus->i2c_nostop = 0;
>> +               bus->i2c_reserve = 0;
>>         } else {                /* regular case */
>>
>>                 /* prepare xfer_data struct */
> If the i2c functionality is broken in some way, I suggest its fixed
> first, along with a correct patch description, as a separate piece of
> work. Lets not group this in with a board profile.
>
> Almost certainly we should never see a "if board == X" in any i2c
> implementation without proper discussion.
>
I'm interested in accepting this patch :) What I should do?

-- 
Best regards,
Oleh Kravchenko

Senior Software Developer, CMS | skype: oleg_krava | Email: oleg@kaa.org.ua


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] [media] cx231xx: Initial support Evromedia USB Full Hybrid Full HD
  2017-01-11 10:08 Oleh Kravchenko
@ 2017-01-11 13:53 ` Steven Toth
  2017-01-11 14:00   ` Oleh Kravchenko
  0 siblings, 1 reply; 14+ messages in thread
From: Steven Toth @ 2017-01-11 13:53 UTC (permalink / raw)
  To: Oleh Kravchenko
  Cc: Linux Media Mailing List, Jacob Johan (Hans) Verkuil, Antti Palosaari

> diff --git a/drivers/media/usb/cx231xx/cx231xx-i2c.c b/drivers/media/usb/cx231xx/cx231xx-i2c.c
> index 35e9acf..60412ec 100644
> --- a/drivers/media/usb/cx231xx/cx231xx-i2c.c
> +++ b/drivers/media/usb/cx231xx/cx231xx-i2c.c
> @@ -171,6 +171,43 @@ static int cx231xx_i2c_send_bytes(struct i2c_adapter *i2c_adap,
>                 bus->i2c_nostop = 0;
>                 bus->i2c_reserve = 0;
>
> +       } else if (dev->model == CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD
> +               && msg->addr == dev->tuner_addr
> +               && msg->len > 4) {
> +               /* special case for Evromedia USB Full Hybrid Full HD tuner chip */
> +               size = msg->len;
> +               saddr_len = 1;
> +
> +               /* adjust the length to correct length */
> +               size -= saddr_len;
> +
> +               buf_ptr = (u8*)(msg->buf + 1);
> +
> +               do {
> +                       /* prepare xfer_data struct */
> +                       req_data.dev_addr = msg->addr;
> +                       req_data.direction = msg->flags;
> +                       req_data.saddr_len = saddr_len;
> +                       req_data.saddr_dat = msg->buf[0];
> +                       req_data.buf_size = size > 4 ? 4 : size;
> +                       req_data.p_buffer = (u8*)(buf_ptr + loop * 4);
> +
> +                       bus->i2c_nostop = (size > 4) ? 1 : 0;
> +                       bus->i2c_reserve = (loop == 0) ? 0 : 1;
> +
> +                       /* usb send command */
> +                       status = dev->cx231xx_send_usb_command(bus, &req_data);
> +                       loop++;
> +
> +                       if (size >= 4) {
> +                               size -= 4;
> +                       } else {
> +                               size = 0;
> +                       }
> +               } while (size > 0);
> +
> +               bus->i2c_nostop = 0;
> +               bus->i2c_reserve = 0;
>         } else {                /* regular case */
>
>                 /* prepare xfer_data struct */

If the i2c functionality is broken in some way, I suggest its fixed
first, along with a correct patch description, as a separate piece of
work. Lets not group this in with a board profile.

Almost certainly we should never see a "if board == X" in any i2c
implementation without proper discussion.

-- 
Steven Toth - Kernel Labs
http://www.kernellabs.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] [media] cx231xx: Initial support Evromedia USB Full Hybrid Full HD
@ 2017-01-11 10:08 Oleh Kravchenko
  2017-01-11 13:53 ` Steven Toth
  0 siblings, 1 reply; 14+ messages in thread
From: Oleh Kravchenko @ 2017-01-11 10:08 UTC (permalink / raw)
  To: linux-media, hverkuil, crope; +Cc: Oleh Kravchenko

This patch provide only digital support.

The device is based on Si2168 30-demodulator,
Si2158-20 tuner and CX23102-11Z chipset;
USB id: 1b80:d3b2.

Status:
- DVB-T2 works fine;
- Composite and SVideo works fine;
- Analog not implemented.

Signed-off-by: Oleh Kravchenko <oleg@kaa.org.ua>
Tested-by: Oleh Kravchenko <oleg@kaa.org.ua>
---
 drivers/media/usb/cx231xx/Kconfig         |  1 +
 drivers/media/usb/cx231xx/cx231xx-cards.c | 29 +++++++++++++
 drivers/media/usb/cx231xx/cx231xx-dvb.c   | 70 +++++++++++++++++++++++++++++++
 drivers/media/usb/cx231xx/cx231xx-i2c.c   | 37 ++++++++++++++++
 drivers/media/usb/cx231xx/cx231xx.h       |  1 +
 5 files changed, 138 insertions(+)

diff --git a/drivers/media/usb/cx231xx/Kconfig b/drivers/media/usb/cx231xx/Kconfig
index 0cced3e..58de80b 100644
--- a/drivers/media/usb/cx231xx/Kconfig
+++ b/drivers/media/usb/cx231xx/Kconfig
@@ -50,6 +50,7 @@ config VIDEO_CX231XX_DVB
 	select DVB_LGDT3306A if MEDIA_SUBDRV_AUTOSELECT
 	select DVB_TDA18271C2DD if MEDIA_SUBDRV_AUTOSELECT
 	select DVB_SI2165 if MEDIA_SUBDRV_AUTOSELECT
+	select DVB_SI2168 if MEDIA_SUBDRV_AUTOSELECT
 	select MEDIA_TUNER_SI2157 if MEDIA_SUBDRV_AUTOSELECT
 
 	---help---
diff --git a/drivers/media/usb/cx231xx/cx231xx-cards.c b/drivers/media/usb/cx231xx/cx231xx-cards.c
index 36bc254..f730fdb 100644
--- a/drivers/media/usb/cx231xx/cx231xx-cards.c
+++ b/drivers/media/usb/cx231xx/cx231xx-cards.c
@@ -841,6 +841,33 @@ struct cx231xx_board cx231xx_boards[] = {
 			.gpio = NULL,
 		} },
 	},
+	[CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD] = {
+		.name = "Evromedia USB Full Hybrid Full HD",
+		.tuner_type = TUNER_ABSENT,
+		.demod_addr = 0x64, /* 0xc8 >> 1 */
+		.demod_i2c_master = I2C_1_MUX_3,
+		.has_dvb = 1,
+		.ir_i2c_master = I2C_0,
+		.norm = V4L2_STD_PAL,
+		.output_mode = OUT_MODE_VIP11,
+		.tuner_addr = 0x60, /* 0xc0 >> 1 */
+		.tuner_i2c_master = I2C_2,
+		.input = {{
+			.type = CX231XX_VMUX_TELEVISION,
+			.vmux = 0,
+			.amux = CX231XX_AMUX_VIDEO,
+		}, {
+			.type = CX231XX_VMUX_COMPOSITE1,
+			.vmux = CX231XX_VIN_2_1,
+			.amux = CX231XX_AMUX_LINE_IN,
+		}, {
+			.type = CX231XX_VMUX_SVIDEO,
+			.vmux = CX231XX_VIN_1_1 |
+				(CX231XX_VIN_1_2 << 8) |
+				CX25840_SVIDEO_ON,
+			.amux = CX231XX_AMUX_LINE_IN,
+		} },
+	},
 };
 const unsigned int cx231xx_bcount = ARRAY_SIZE(cx231xx_boards);
 
@@ -908,6 +935,8 @@ struct usb_device_id cx231xx_id_table[] = {
 	 .driver_info = CX231XX_BOARD_OTG102},
 	{USB_DEVICE(USB_VID_TERRATEC, 0x00a6),
 	 .driver_info = CX231XX_BOARD_TERRATEC_GRABBY},
+	{USB_DEVICE(0x1b80, 0xd3b2),
+	.driver_info = CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD},
 	{},
 };
 
diff --git a/drivers/media/usb/cx231xx/cx231xx-dvb.c b/drivers/media/usb/cx231xx/cx231xx-dvb.c
index 1417515..cf75c0a 100644
--- a/drivers/media/usb/cx231xx/cx231xx-dvb.c
+++ b/drivers/media/usb/cx231xx/cx231xx-dvb.c
@@ -33,6 +33,7 @@
 #include "s5h1411.h"
 #include "lgdt3305.h"
 #include "si2165.h"
+#include "si2168.h"
 #include "mb86a20s.h"
 #include "si2157.h"
 #include "lgdt3306a.h"
@@ -949,6 +950,75 @@ static int dvb_init(struct cx231xx *dev)
 			   &pv_tda18271_config);
 		break;
 
+	case CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD:
+	{
+		struct si2157_config si2157_config = {};
+		struct si2168_config si2168_config = {};
+		struct i2c_board_info info = {};
+		struct i2c_client *client;
+		struct i2c_adapter *adapter;
+
+		/* attach demodulator chip */
+		si2168_config.ts_mode = SI2168_TS_SERIAL; /* from *.inf file */
+		si2168_config.fe = &dev->dvb->frontend;
+		si2168_config.i2c_adapter = &adapter;
+		si2168_config.ts_clock_inv = true;
+
+		strlcpy(info.type, "si2168", sizeof(info.type));
+		info.addr = dev->board.demod_addr;
+		info.platform_data = &si2168_config;
+
+		request_module(info.type);
+		client = i2c_new_device(demod_i2c, &info);
+
+		if (client == NULL || client->dev.driver == NULL) {
+			result = -ENODEV;
+			goto out_free;
+		}
+
+		if (!try_module_get(client->dev.driver->owner)) {
+			i2c_unregister_device(client);
+			result = -ENODEV;
+			goto out_free;
+		}
+
+		dvb->i2c_client_demod = client;
+
+		/* attach tuner chip */
+		si2157_config.fe = dev->dvb->frontend;
+#ifdef CONFIG_MEDIA_CONTROLLER_DVB
+		si2157_config.mdev = dev->media_dev;
+#endif
+		si2157_config.if_port = 1;
+		si2157_config.inversion = false;
+
+		memset(&info, 0, sizeof(info));
+		strlcpy(info.type, "si2157", sizeof(info.type));
+		info.addr = dev->board.tuner_addr;
+		info.platform_data = &si2157_config;
+
+		request_module(info.type);
+		client = i2c_new_device(tuner_i2c, &info);
+
+		if (client == NULL || client->dev.driver == NULL) {
+			module_put(dvb->i2c_client_demod->dev.driver->owner);
+			i2c_unregister_device(dvb->i2c_client_demod);
+			result = -ENODEV;
+			goto out_free;
+		}
+
+		if (!try_module_get(client->dev.driver->owner)) {
+			i2c_unregister_device(client);
+			module_put(dvb->i2c_client_demod->dev.driver->owner);
+			i2c_unregister_device(dvb->i2c_client_demod);
+			result = -ENODEV;
+			goto out_free;
+		}
+
+		dev->cx231xx_reset_analog_tuner = NULL;
+		dev->dvb->i2c_client_tuner = client;
+		break;
+	}
 	default:
 		dev_err(dev->dev,
 			"%s/2: The frontend of your DVB/ATSC card isn't supported yet\n",
diff --git a/drivers/media/usb/cx231xx/cx231xx-i2c.c b/drivers/media/usb/cx231xx/cx231xx-i2c.c
index 35e9acf..60412ec 100644
--- a/drivers/media/usb/cx231xx/cx231xx-i2c.c
+++ b/drivers/media/usb/cx231xx/cx231xx-i2c.c
@@ -171,6 +171,43 @@ static int cx231xx_i2c_send_bytes(struct i2c_adapter *i2c_adap,
 		bus->i2c_nostop = 0;
 		bus->i2c_reserve = 0;
 
+	} else if (dev->model == CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD
+		&& msg->addr == dev->tuner_addr
+		&& msg->len > 4) {
+		/* special case for Evromedia USB Full Hybrid Full HD tuner chip */
+		size = msg->len;
+		saddr_len = 1;
+
+		/* adjust the length to correct length */
+		size -= saddr_len;
+
+		buf_ptr = (u8*)(msg->buf + 1);
+
+		do {
+			/* prepare xfer_data struct */
+			req_data.dev_addr = msg->addr;
+			req_data.direction = msg->flags;
+			req_data.saddr_len = saddr_len;
+			req_data.saddr_dat = msg->buf[0];
+			req_data.buf_size = size > 4 ? 4 : size;
+			req_data.p_buffer = (u8*)(buf_ptr + loop * 4);
+
+			bus->i2c_nostop = (size > 4) ? 1 : 0;
+			bus->i2c_reserve = (loop == 0) ? 0 : 1;
+
+			/* usb send command */
+			status = dev->cx231xx_send_usb_command(bus, &req_data);
+			loop++;
+
+			if (size >= 4) {
+				size -= 4;
+			} else {
+				size = 0;
+			}
+		} while (size > 0);
+
+		bus->i2c_nostop = 0;
+		bus->i2c_reserve = 0;
 	} else {		/* regular case */
 
 		/* prepare xfer_data struct */
diff --git a/drivers/media/usb/cx231xx/cx231xx.h b/drivers/media/usb/cx231xx/cx231xx.h
index 90c8676..d9792ea 100644
--- a/drivers/media/usb/cx231xx/cx231xx.h
+++ b/drivers/media/usb/cx231xx/cx231xx.h
@@ -78,6 +78,7 @@
 #define CX231XX_BOARD_HAUPPAUGE_930C_HD_1114xx 20
 #define CX231XX_BOARD_HAUPPAUGE_955Q 21
 #define CX231XX_BOARD_TERRATEC_GRABBY 22
+#define CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD 23
 
 /* Limits minimum and default number of buffers */
 #define CX231XX_MIN_BUF                 4
-- 
2.10.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] [media] cx231xx: Initial support Evromedia USB Full Hybrid Full HD
  2017-01-11  0:16 ` Antti Palosaari
@ 2017-01-11  7:28   ` Oleh Kravchenko
  0 siblings, 0 replies; 14+ messages in thread
From: Oleh Kravchenko @ 2017-01-11  7:28 UTC (permalink / raw)
  To: Antti Palosaari, linux-media, hverkuil



On 11.01.17 02:16, Antti Palosaari wrote:
> On 01/10/2017 11:41 PM, Oleh Kravchenko wrote:
>> This patch provide only digital support.
>>
>> The device is based on Si2168 30-demodulator,
>> Si2158-20 tuner and CX23102-11Z chipset;
>> USB id: 1b80:d3b2.
>>
>> Status:
>> - DVB-T2 works fine;
>> - Composite and SVideo works fine;
>> - Analog not implemented.
>>
>> Signed-off-by: Oleh Kravchenko <oleg@kaa.org.ua>
>> Tested-by: Oleh Kravchenko <oleg@kaa.org.ua>
>> ---
>>  drivers/media/usb/cx231xx/Kconfig         |  1 +
>>  drivers/media/usb/cx231xx/cx231xx-cards.c | 29 +++++++++++++
>>  drivers/media/usb/cx231xx/cx231xx-dvb.c   | 71 +++++++++++++++++++++++++++++++
>>  drivers/media/usb/cx231xx/cx231xx-i2c.c   | 37 ++++++++++++++++
>>  drivers/media/usb/cx231xx/cx231xx.h       |  1 +
>>  5 files changed, 139 insertions(+)
>>
>> diff --git a/drivers/media/usb/cx231xx/Kconfig b/drivers/media/usb/cx231xx/Kconfig
>> index 0cced3e..58de80b 100644
>> --- a/drivers/media/usb/cx231xx/Kconfig
>> +++ b/drivers/media/usb/cx231xx/Kconfig
>> @@ -50,6 +50,7 @@ config VIDEO_CX231XX_DVB
>>      select DVB_LGDT3306A if MEDIA_SUBDRV_AUTOSELECT
>>      select DVB_TDA18271C2DD if MEDIA_SUBDRV_AUTOSELECT
>>      select DVB_SI2165 if MEDIA_SUBDRV_AUTOSELECT
>> +    select DVB_SI2168 if MEDIA_SUBDRV_AUTOSELECT
>>      select MEDIA_TUNER_SI2157 if MEDIA_SUBDRV_AUTOSELECT
>>
>>      ---help---
>> diff --git a/drivers/media/usb/cx231xx/cx231xx-cards.c b/drivers/media/usb/cx231xx/cx231xx-cards.c
>> index 36bc254..f730fdb 100644
>> --- a/drivers/media/usb/cx231xx/cx231xx-cards.c
>> +++ b/drivers/media/usb/cx231xx/cx231xx-cards.c
>> @@ -841,6 +841,33 @@ struct cx231xx_board cx231xx_boards[] = {
>>              .gpio = NULL,
>>          } },
>>      },
>> +    [CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD] = {
>> +        .name = "Evromedia USB Full Hybrid Full HD",
>> +        .tuner_type = TUNER_ABSENT,
>> +        .demod_addr = 0x64, /* 0xc8 >> 1 */
>> +        .demod_i2c_master = I2C_1_MUX_3,
>> +        .has_dvb = 1,
>> +        .ir_i2c_master = I2C_0,
>> +        .norm = V4L2_STD_PAL,
>> +        .output_mode = OUT_MODE_VIP11,
>> +        .tuner_addr = 0x60, /* 0xc0 >> 1 */
> 
> Looks good. I looked the existing file and all the other entries were also using correct 7-bit addresses.
> 
>> +        .tuner_i2c_master = I2C_2,
>> +        .input = {{
>> +            .type = CX231XX_VMUX_TELEVISION,
>> +            .vmux = 0,
>> +            .amux = CX231XX_AMUX_VIDEO,
>> +        }, {
>> +            .type = CX231XX_VMUX_COMPOSITE1,
>> +            .vmux = CX231XX_VIN_2_1,
>> +            .amux = CX231XX_AMUX_LINE_IN,
>> +        }, {
>> +            .type = CX231XX_VMUX_SVIDEO,
>> +            .vmux = CX231XX_VIN_1_1 |
>> +                (CX231XX_VIN_1_2 << 8) |
>> +                CX25840_SVIDEO_ON,
>> +            .amux = CX231XX_AMUX_LINE_IN,
>> +        } },
>> +    },
>>  };
>>  const unsigned int cx231xx_bcount = ARRAY_SIZE(cx231xx_boards);
>>
>> @@ -908,6 +935,8 @@ struct usb_device_id cx231xx_id_table[] = {
>>       .driver_info = CX231XX_BOARD_OTG102},
>>      {USB_DEVICE(USB_VID_TERRATEC, 0x00a6),
>>       .driver_info = CX231XX_BOARD_TERRATEC_GRABBY},
>> +    {USB_DEVICE(0x1b80, 0xd3b2),
>> +    .driver_info = CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD},
>>      {},
>>  };
>>
>> diff --git a/drivers/media/usb/cx231xx/cx231xx-dvb.c b/drivers/media/usb/cx231xx/cx231xx-dvb.c
>> index 1417515..ecd3528 100644
>> --- a/drivers/media/usb/cx231xx/cx231xx-dvb.c
>> +++ b/drivers/media/usb/cx231xx/cx231xx-dvb.c
>> @@ -33,6 +33,7 @@
>>  #include "s5h1411.h"
>>  #include "lgdt3305.h"
>>  #include "si2165.h"
>> +#include "si2168.h"
>>  #include "mb86a20s.h"
>>  #include "si2157.h"
>>  #include "lgdt3306a.h"
>> @@ -949,6 +950,76 @@ static int dvb_init(struct cx231xx *dev)
>>                 &pv_tda18271_config);
>>          break;
>>
>> +    case CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD:
>> +    {
>> +        struct si2157_config si2157_config = {};
>> +        struct si2168_config si2168_config = {};
>> +        struct i2c_board_info info = {};
> 
> Personally I don't like initializing variables to zero at the declaration, but it was Hans who asked to do it.
> 
>> +        struct i2c_client *client;
>> +        struct i2c_adapter *adapter;
>> +
>> +        /* attach demodulator chip */
>> +        si2168_config.ts_mode = SI2168_TS_SERIAL; /* from *.inf file */
>> +        si2168_config.fe = &dev->dvb->frontend;
>> +        si2168_config.i2c_adapter = &adapter;
>> +        si2168_config.ts_clock_inv = true;
>> +
>> +        strlcpy(info.type, "si2168", sizeof(info.type));
>> +        info.addr = dev->board.demod_addr;
>> +        info.platform_data = &si2168_config;
>> +
>> +        request_module(info.type);
>> +        client = i2c_new_device(demod_i2c, &info);
>> +
>> +        if (client == NULL || client->dev.driver == NULL) {
>> +            result = -ENODEV;
>> +            goto out_free;
>> +        }
>> +
>> +        if (!try_module_get(client->dev.driver->owner)) {
>> +            i2c_unregister_device(client);
>> +            result = -ENODEV;
>> +            goto out_free;
>> +        }
>> +
>> +        dvb->i2c_client_demod = client;
>> +        dvb->frontend->callback = cx231xx_tuner_callback;
> 
> Drop that callback assignment. It is not needed. It is used only for TDA18271 and XC5000 tuners. (History: whole callback is used only(?) for gpios which could be implemented via kernel gpio api).

Done
 
>> +
>> +        /* attach tuner chip */
>> +        si2157_config.fe = dev->dvb->frontend;
>> +#ifdef CONFIG_MEDIA_CONTROLLER_DVB
>> +        si2157_config.mdev = dev->media_dev;
>> +#endif
>> +        si2157_config.if_port = 1;
>> +        si2157_config.inversion = false;
>> +
>> +        memset(&info, 0, sizeof(info));
>> +        strlcpy(info.type, "si2157", sizeof(info.type));
>> +        info.addr = dev->board.tuner_addr;
>> +        info.platform_data = &si2157_config;
>> +
>> +        request_module(info.type);
>> +        client = i2c_new_device(tuner_i2c, &info);
>> +
>> +        if (client == NULL || client->dev.driver == NULL) {
>> +            module_put(dvb->i2c_client_demod->dev.driver->owner);
>> +            i2c_unregister_device(dvb->i2c_client_demod);
>> +            result = -ENODEV;
>> +            goto out_free;
>> +        }
>> +
>> +        if (!try_module_get(client->dev.driver->owner)) {
>> +            i2c_unregister_device(client);
>> +            module_put(dvb->i2c_client_demod->dev.driver->owner);
>> +            i2c_unregister_device(dvb->i2c_client_demod);
>> +            result = -ENODEV;
>> +            goto out_free;
>> +        }
>> +
>> +        dev->cx231xx_reset_analog_tuner = NULL;
> 
> A bit weird that this function is only for XC5000 tuner and the rest should disable it that way. But it is not problem with that patch.
> 
> 
>> +        dev->dvb->i2c_client_tuner = client;
>> +        break;
>> +    }
>>      default:
>>          dev_err(dev->dev,
>>              "%s/2: The frontend of your DVB/ATSC card isn't supported yet\n",
>> diff --git a/drivers/media/usb/cx231xx/cx231xx-i2c.c b/drivers/media/usb/cx231xx/cx231xx-i2c.c
>> index 35e9acf..60412ec 100644
>> --- a/drivers/media/usb/cx231xx/cx231xx-i2c.c
>> +++ b/drivers/media/usb/cx231xx/cx231xx-i2c.c
>> @@ -171,6 +171,43 @@ static int cx231xx_i2c_send_bytes(struct i2c_adapter *i2c_adap,
>>          bus->i2c_nostop = 0;
>>          bus->i2c_reserve = 0;
>>
>> +    } else if (dev->model == CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD
>> +        && msg->addr == dev->tuner_addr
>> +        && msg->len > 4) {
>> +        /* special case for Evromedia USB Full Hybrid Full HD tuner chip */
>> +        size = msg->len;
>> +        saddr_len = 1;
>> +
>> +        /* adjust the length to correct length */
>> +        size -= saddr_len;
>> +
>> +        buf_ptr = (u8*)(msg->buf + 1);
>> +
>> +        do {
>> +            /* prepare xfer_data struct */
>> +            req_data.dev_addr = msg->addr;
>> +            req_data.direction = msg->flags;
>> +            req_data.saddr_len = saddr_len;
>> +            req_data.saddr_dat = msg->buf[0];
>> +            req_data.buf_size = size > 4 ? 4 : size;
>> +            req_data.p_buffer = (u8*)(buf_ptr + loop * 4);
>> +
>> +            bus->i2c_nostop = (size > 4) ? 1 : 0;
>> +            bus->i2c_reserve = (loop == 0) ? 0 : 1;
>> +
>> +            /* usb send command */
>> +            status = dev->cx231xx_send_usb_command(bus, &req_data);
>> +            loop++;
>> +
>> +            if (size >= 4) {
>> +                size -= 4;
>> +            } else {
>> +                size = 0;
>> +            }
>> +        } while (size > 0);
>> +
>> +        bus->i2c_nostop = 0;
>> +        bus->i2c_reserve = 0;
> 
> 
> I looked that cx231xx_i2c_send_bytes() function more carefully. Currently it is implemented like:
> if "tuner is XC5000"
>   * do some special hacks
>   * send 16 byte i2c messages
> else
>   * send i2c message (~unlimited size)
> 
> Now this adds special case for this device too. For my eyes it looks like you just try to split i2c message to multiple 4 byte messages. I wonder if there is really 4 byte limit on that chip as XC5000 branch still sends 16 bytes as one go. Are you really sure?
> 
> Could you test what is maximum limit of one usb i2c message and use that value to split messages? It does not sound correct on XC5000 case you could send 16 bytes as a one go, but with si2158 only 4 bytes. si2168/si2157 are sending maximum ~15 byte messages as one go and I wonder if that i2c adapter could handle it without even splitting.
> 
> All-in-all: if XC5000 could send 16 bytes and all the rest has no limitation, it does not sound correct you have to split long messages to only 4 byte len.

I tested with 5, 8, 16 bytes, and get this error:
[ 2757.119866] si2168 9-0064: downloading firmware from file 'dvb-demod-si2168-a30-01.fw'
[ 2761.109662] si2168 9-0064: firmware version: A 3.0.19
# si2158 firmware uploading..
[ 2761.113098] cx231xx 3-1:1.1: cx231xx_send_usb_command: failed with status --32

[ 2985.263378] si2168 9-0064: downloading firmware from file 'dvb-demod-si2168-a30-01.fw'
[ 2989.230173] si2168 9-0064: firmware version: A 3.0.19
# si2158 firmware uploading..
[ 2989.233141] cx231xx 3-1:1.1: cx231xx_send_usb_command: failed with status --32
[ 2989.233513] cx231xx 3-1:1.1: cx231xx_send_usb_command: failed with status --32
[ 2989.233876] cx231xx 3-1:1.1: cx231xx_send_usb_command: failed with status --32

And when I split messages to 4 bytes, it succeeds:
[ 3037.816936] si2168 9-0064: downloading firmware from file 'dvb-demod-si2168-a30-01.fw'
[ 3041.806845] si2168 9-0064: firmware version: A 3.0.19
[ 3041.814467] si2157 7-0060: found a 'Silicon Labs Si2158-A20'
[ 3041.814513] si2157 7-0060: downloading firmware from file 'dvb-tuner-si2158-a20-01.fw'
[ 3042.408588] si2157 7-0060: firmware version: 2.1.9

 
>>      } else {        /* regular case */
>>
>>          /* prepare xfer_data struct */
>> diff --git a/drivers/media/usb/cx231xx/cx231xx.h b/drivers/media/usb/cx231xx/cx231xx.h
>> index 90c8676..d9792ea 100644
>> --- a/drivers/media/usb/cx231xx/cx231xx.h
>> +++ b/drivers/media/usb/cx231xx/cx231xx.h
>> @@ -78,6 +78,7 @@
>>  #define CX231XX_BOARD_HAUPPAUGE_930C_HD_1114xx 20
>>  #define CX231XX_BOARD_HAUPPAUGE_955Q 21
>>  #define CX231XX_BOARD_TERRATEC_GRABBY 22
>> +#define CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD 23
>>
>>  /* Limits minimum and default number of buffers */
>>  #define CX231XX_MIN_BUF                 4
>>
> 
> Otherwise than I2C send it looks good for my eyes. I am waiting your explanation for need of i2c message splitting.
> 
> regards
> Antti
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] [media] cx231xx: Initial support Evromedia USB Full Hybrid Full HD
  2017-01-10 21:41 Oleh Kravchenko
@ 2017-01-11  0:16 ` Antti Palosaari
  2017-01-11  7:28   ` Oleh Kravchenko
  0 siblings, 1 reply; 14+ messages in thread
From: Antti Palosaari @ 2017-01-11  0:16 UTC (permalink / raw)
  To: Oleh Kravchenko, linux-media, hverkuil

On 01/10/2017 11:41 PM, Oleh Kravchenko wrote:
> This patch provide only digital support.
>
> The device is based on Si2168 30-demodulator,
> Si2158-20 tuner and CX23102-11Z chipset;
> USB id: 1b80:d3b2.
>
> Status:
> - DVB-T2 works fine;
> - Composite and SVideo works fine;
> - Analog not implemented.
>
> Signed-off-by: Oleh Kravchenko <oleg@kaa.org.ua>
> Tested-by: Oleh Kravchenko <oleg@kaa.org.ua>
> ---
>  drivers/media/usb/cx231xx/Kconfig         |  1 +
>  drivers/media/usb/cx231xx/cx231xx-cards.c | 29 +++++++++++++
>  drivers/media/usb/cx231xx/cx231xx-dvb.c   | 71 +++++++++++++++++++++++++++++++
>  drivers/media/usb/cx231xx/cx231xx-i2c.c   | 37 ++++++++++++++++
>  drivers/media/usb/cx231xx/cx231xx.h       |  1 +
>  5 files changed, 139 insertions(+)
>
> diff --git a/drivers/media/usb/cx231xx/Kconfig b/drivers/media/usb/cx231xx/Kconfig
> index 0cced3e..58de80b 100644
> --- a/drivers/media/usb/cx231xx/Kconfig
> +++ b/drivers/media/usb/cx231xx/Kconfig
> @@ -50,6 +50,7 @@ config VIDEO_CX231XX_DVB
>  	select DVB_LGDT3306A if MEDIA_SUBDRV_AUTOSELECT
>  	select DVB_TDA18271C2DD if MEDIA_SUBDRV_AUTOSELECT
>  	select DVB_SI2165 if MEDIA_SUBDRV_AUTOSELECT
> +	select DVB_SI2168 if MEDIA_SUBDRV_AUTOSELECT
>  	select MEDIA_TUNER_SI2157 if MEDIA_SUBDRV_AUTOSELECT
>
>  	---help---
> diff --git a/drivers/media/usb/cx231xx/cx231xx-cards.c b/drivers/media/usb/cx231xx/cx231xx-cards.c
> index 36bc254..f730fdb 100644
> --- a/drivers/media/usb/cx231xx/cx231xx-cards.c
> +++ b/drivers/media/usb/cx231xx/cx231xx-cards.c
> @@ -841,6 +841,33 @@ struct cx231xx_board cx231xx_boards[] = {
>  			.gpio = NULL,
>  		} },
>  	},
> +	[CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD] = {
> +		.name = "Evromedia USB Full Hybrid Full HD",
> +		.tuner_type = TUNER_ABSENT,
> +		.demod_addr = 0x64, /* 0xc8 >> 1 */
> +		.demod_i2c_master = I2C_1_MUX_3,
> +		.has_dvb = 1,
> +		.ir_i2c_master = I2C_0,
> +		.norm = V4L2_STD_PAL,
> +		.output_mode = OUT_MODE_VIP11,
> +		.tuner_addr = 0x60, /* 0xc0 >> 1 */

Looks good. I looked the existing file and all the other entries were 
also using correct 7-bit addresses.

> +		.tuner_i2c_master = I2C_2,
> +		.input = {{
> +			.type = CX231XX_VMUX_TELEVISION,
> +			.vmux = 0,
> +			.amux = CX231XX_AMUX_VIDEO,
> +		}, {
> +			.type = CX231XX_VMUX_COMPOSITE1,
> +			.vmux = CX231XX_VIN_2_1,
> +			.amux = CX231XX_AMUX_LINE_IN,
> +		}, {
> +			.type = CX231XX_VMUX_SVIDEO,
> +			.vmux = CX231XX_VIN_1_1 |
> +				(CX231XX_VIN_1_2 << 8) |
> +				CX25840_SVIDEO_ON,
> +			.amux = CX231XX_AMUX_LINE_IN,
> +		} },
> +	},
>  };
>  const unsigned int cx231xx_bcount = ARRAY_SIZE(cx231xx_boards);
>
> @@ -908,6 +935,8 @@ struct usb_device_id cx231xx_id_table[] = {
>  	 .driver_info = CX231XX_BOARD_OTG102},
>  	{USB_DEVICE(USB_VID_TERRATEC, 0x00a6),
>  	 .driver_info = CX231XX_BOARD_TERRATEC_GRABBY},
> +	{USB_DEVICE(0x1b80, 0xd3b2),
> +	.driver_info = CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD},
>  	{},
>  };
>
> diff --git a/drivers/media/usb/cx231xx/cx231xx-dvb.c b/drivers/media/usb/cx231xx/cx231xx-dvb.c
> index 1417515..ecd3528 100644
> --- a/drivers/media/usb/cx231xx/cx231xx-dvb.c
> +++ b/drivers/media/usb/cx231xx/cx231xx-dvb.c
> @@ -33,6 +33,7 @@
>  #include "s5h1411.h"
>  #include "lgdt3305.h"
>  #include "si2165.h"
> +#include "si2168.h"
>  #include "mb86a20s.h"
>  #include "si2157.h"
>  #include "lgdt3306a.h"
> @@ -949,6 +950,76 @@ static int dvb_init(struct cx231xx *dev)
>  			   &pv_tda18271_config);
>  		break;
>
> +	case CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD:
> +	{
> +		struct si2157_config si2157_config = {};
> +		struct si2168_config si2168_config = {};
> +		struct i2c_board_info info = {};

Personally I don't like initializing variables to zero at the 
declaration, but it was Hans who asked to do it.

> +		struct i2c_client *client;
> +		struct i2c_adapter *adapter;
> +
> +		/* attach demodulator chip */
> +		si2168_config.ts_mode = SI2168_TS_SERIAL; /* from *.inf file */
> +		si2168_config.fe = &dev->dvb->frontend;
> +		si2168_config.i2c_adapter = &adapter;
> +		si2168_config.ts_clock_inv = true;
> +
> +		strlcpy(info.type, "si2168", sizeof(info.type));
> +		info.addr = dev->board.demod_addr;
> +		info.platform_data = &si2168_config;
> +
> +		request_module(info.type);
> +		client = i2c_new_device(demod_i2c, &info);
> +
> +		if (client == NULL || client->dev.driver == NULL) {
> +			result = -ENODEV;
> +			goto out_free;
> +		}
> +
> +		if (!try_module_get(client->dev.driver->owner)) {
> +			i2c_unregister_device(client);
> +			result = -ENODEV;
> +			goto out_free;
> +		}
> +
> +		dvb->i2c_client_demod = client;
> +		dvb->frontend->callback = cx231xx_tuner_callback;

Drop that callback assignment. It is not needed. It is used only for 
TDA18271 and XC5000 tuners. (History: whole callback is used only(?) for 
gpios which could be implemented via kernel gpio api).

> +
> +		/* attach tuner chip */
> +		si2157_config.fe = dev->dvb->frontend;
> +#ifdef CONFIG_MEDIA_CONTROLLER_DVB
> +		si2157_config.mdev = dev->media_dev;
> +#endif
> +		si2157_config.if_port = 1;
> +		si2157_config.inversion = false;
> +
> +		memset(&info, 0, sizeof(info));
> +		strlcpy(info.type, "si2157", sizeof(info.type));
> +		info.addr = dev->board.tuner_addr;
> +		info.platform_data = &si2157_config;
> +
> +		request_module(info.type);
> +		client = i2c_new_device(tuner_i2c, &info);
> +
> +		if (client == NULL || client->dev.driver == NULL) {
> +			module_put(dvb->i2c_client_demod->dev.driver->owner);
> +			i2c_unregister_device(dvb->i2c_client_demod);
> +			result = -ENODEV;
> +			goto out_free;
> +		}
> +
> +		if (!try_module_get(client->dev.driver->owner)) {
> +			i2c_unregister_device(client);
> +			module_put(dvb->i2c_client_demod->dev.driver->owner);
> +			i2c_unregister_device(dvb->i2c_client_demod);
> +			result = -ENODEV;
> +			goto out_free;
> +		}
> +
> +		dev->cx231xx_reset_analog_tuner = NULL;

A bit weird that this function is only for XC5000 tuner and the rest 
should disable it that way. But it is not problem with that patch.


> +		dev->dvb->i2c_client_tuner = client;
> +		break;
> +	}
>  	default:
>  		dev_err(dev->dev,
>  			"%s/2: The frontend of your DVB/ATSC card isn't supported yet\n",
> diff --git a/drivers/media/usb/cx231xx/cx231xx-i2c.c b/drivers/media/usb/cx231xx/cx231xx-i2c.c
> index 35e9acf..60412ec 100644
> --- a/drivers/media/usb/cx231xx/cx231xx-i2c.c
> +++ b/drivers/media/usb/cx231xx/cx231xx-i2c.c
> @@ -171,6 +171,43 @@ static int cx231xx_i2c_send_bytes(struct i2c_adapter *i2c_adap,
>  		bus->i2c_nostop = 0;
>  		bus->i2c_reserve = 0;
>
> +	} else if (dev->model == CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD
> +		&& msg->addr == dev->tuner_addr
> +		&& msg->len > 4) {
> +		/* special case for Evromedia USB Full Hybrid Full HD tuner chip */
> +		size = msg->len;
> +		saddr_len = 1;
> +
> +		/* adjust the length to correct length */
> +		size -= saddr_len;
> +
> +		buf_ptr = (u8*)(msg->buf + 1);
> +
> +		do {
> +			/* prepare xfer_data struct */
> +			req_data.dev_addr = msg->addr;
> +			req_data.direction = msg->flags;
> +			req_data.saddr_len = saddr_len;
> +			req_data.saddr_dat = msg->buf[0];
> +			req_data.buf_size = size > 4 ? 4 : size;
> +			req_data.p_buffer = (u8*)(buf_ptr + loop * 4);
> +
> +			bus->i2c_nostop = (size > 4) ? 1 : 0;
> +			bus->i2c_reserve = (loop == 0) ? 0 : 1;
> +
> +			/* usb send command */
> +			status = dev->cx231xx_send_usb_command(bus, &req_data);
> +			loop++;
> +
> +			if (size >= 4) {
> +				size -= 4;
> +			} else {
> +				size = 0;
> +			}
> +		} while (size > 0);
> +
> +		bus->i2c_nostop = 0;
> +		bus->i2c_reserve = 0;


I looked that cx231xx_i2c_send_bytes() function more carefully. 
Currently it is implemented like:
if "tuner is XC5000"
   * do some special hacks
   * send 16 byte i2c messages
else
   * send i2c message (~unlimited size)

Now this adds special case for this device too. For my eyes it looks 
like you just try to split i2c message to multiple 4 byte messages. I 
wonder if there is really 4 byte limit on that chip as XC5000 branch 
still sends 16 bytes as one go. Are you really sure?

Could you test what is maximum limit of one usb i2c message and use that 
value to split messages? It does not sound correct on XC5000 case you 
could send 16 bytes as a one go, but with si2158 only 4 bytes. 
si2168/si2157 are sending maximum ~15 byte messages as one go and I 
wonder if that i2c adapter could handle it without even splitting.

All-in-all: if XC5000 could send 16 bytes and all the rest has no 
limitation, it does not sound correct you have to split long messages to 
only 4 byte len.

>  	} else {		/* regular case */
>
>  		/* prepare xfer_data struct */
> diff --git a/drivers/media/usb/cx231xx/cx231xx.h b/drivers/media/usb/cx231xx/cx231xx.h
> index 90c8676..d9792ea 100644
> --- a/drivers/media/usb/cx231xx/cx231xx.h
> +++ b/drivers/media/usb/cx231xx/cx231xx.h
> @@ -78,6 +78,7 @@
>  #define CX231XX_BOARD_HAUPPAUGE_930C_HD_1114xx 20
>  #define CX231XX_BOARD_HAUPPAUGE_955Q 21
>  #define CX231XX_BOARD_TERRATEC_GRABBY 22
> +#define CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD 23
>
>  /* Limits minimum and default number of buffers */
>  #define CX231XX_MIN_BUF                 4
>

Otherwise than I2C send it looks good for my eyes. I am waiting your 
explanation for need of i2c message splitting.

regards
Antti

-- 
http://palosaari.fi/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] [media] cx231xx: Initial support Evromedia USB Full Hybrid Full HD
@ 2017-01-10 21:41 Oleh Kravchenko
  2017-01-11  0:16 ` Antti Palosaari
  0 siblings, 1 reply; 14+ messages in thread
From: Oleh Kravchenko @ 2017-01-10 21:41 UTC (permalink / raw)
  To: linux-media, hverkuil, crope; +Cc: Oleh Kravchenko

This patch provide only digital support.

The device is based on Si2168 30-demodulator,
Si2158-20 tuner and CX23102-11Z chipset;
USB id: 1b80:d3b2.

Status:
- DVB-T2 works fine;
- Composite and SVideo works fine;
- Analog not implemented.

Signed-off-by: Oleh Kravchenko <oleg@kaa.org.ua>
Tested-by: Oleh Kravchenko <oleg@kaa.org.ua>
---
 drivers/media/usb/cx231xx/Kconfig         |  1 +
 drivers/media/usb/cx231xx/cx231xx-cards.c | 29 +++++++++++++
 drivers/media/usb/cx231xx/cx231xx-dvb.c   | 71 +++++++++++++++++++++++++++++++
 drivers/media/usb/cx231xx/cx231xx-i2c.c   | 37 ++++++++++++++++
 drivers/media/usb/cx231xx/cx231xx.h       |  1 +
 5 files changed, 139 insertions(+)

diff --git a/drivers/media/usb/cx231xx/Kconfig b/drivers/media/usb/cx231xx/Kconfig
index 0cced3e..58de80b 100644
--- a/drivers/media/usb/cx231xx/Kconfig
+++ b/drivers/media/usb/cx231xx/Kconfig
@@ -50,6 +50,7 @@ config VIDEO_CX231XX_DVB
 	select DVB_LGDT3306A if MEDIA_SUBDRV_AUTOSELECT
 	select DVB_TDA18271C2DD if MEDIA_SUBDRV_AUTOSELECT
 	select DVB_SI2165 if MEDIA_SUBDRV_AUTOSELECT
+	select DVB_SI2168 if MEDIA_SUBDRV_AUTOSELECT
 	select MEDIA_TUNER_SI2157 if MEDIA_SUBDRV_AUTOSELECT
 
 	---help---
diff --git a/drivers/media/usb/cx231xx/cx231xx-cards.c b/drivers/media/usb/cx231xx/cx231xx-cards.c
index 36bc254..f730fdb 100644
--- a/drivers/media/usb/cx231xx/cx231xx-cards.c
+++ b/drivers/media/usb/cx231xx/cx231xx-cards.c
@@ -841,6 +841,33 @@ struct cx231xx_board cx231xx_boards[] = {
 			.gpio = NULL,
 		} },
 	},
+	[CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD] = {
+		.name = "Evromedia USB Full Hybrid Full HD",
+		.tuner_type = TUNER_ABSENT,
+		.demod_addr = 0x64, /* 0xc8 >> 1 */
+		.demod_i2c_master = I2C_1_MUX_3,
+		.has_dvb = 1,
+		.ir_i2c_master = I2C_0,
+		.norm = V4L2_STD_PAL,
+		.output_mode = OUT_MODE_VIP11,
+		.tuner_addr = 0x60, /* 0xc0 >> 1 */
+		.tuner_i2c_master = I2C_2,
+		.input = {{
+			.type = CX231XX_VMUX_TELEVISION,
+			.vmux = 0,
+			.amux = CX231XX_AMUX_VIDEO,
+		}, {
+			.type = CX231XX_VMUX_COMPOSITE1,
+			.vmux = CX231XX_VIN_2_1,
+			.amux = CX231XX_AMUX_LINE_IN,
+		}, {
+			.type = CX231XX_VMUX_SVIDEO,
+			.vmux = CX231XX_VIN_1_1 |
+				(CX231XX_VIN_1_2 << 8) |
+				CX25840_SVIDEO_ON,
+			.amux = CX231XX_AMUX_LINE_IN,
+		} },
+	},
 };
 const unsigned int cx231xx_bcount = ARRAY_SIZE(cx231xx_boards);
 
@@ -908,6 +935,8 @@ struct usb_device_id cx231xx_id_table[] = {
 	 .driver_info = CX231XX_BOARD_OTG102},
 	{USB_DEVICE(USB_VID_TERRATEC, 0x00a6),
 	 .driver_info = CX231XX_BOARD_TERRATEC_GRABBY},
+	{USB_DEVICE(0x1b80, 0xd3b2),
+	.driver_info = CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD},
 	{},
 };
 
diff --git a/drivers/media/usb/cx231xx/cx231xx-dvb.c b/drivers/media/usb/cx231xx/cx231xx-dvb.c
index 1417515..ecd3528 100644
--- a/drivers/media/usb/cx231xx/cx231xx-dvb.c
+++ b/drivers/media/usb/cx231xx/cx231xx-dvb.c
@@ -33,6 +33,7 @@
 #include "s5h1411.h"
 #include "lgdt3305.h"
 #include "si2165.h"
+#include "si2168.h"
 #include "mb86a20s.h"
 #include "si2157.h"
 #include "lgdt3306a.h"
@@ -949,6 +950,76 @@ static int dvb_init(struct cx231xx *dev)
 			   &pv_tda18271_config);
 		break;
 
+	case CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD:
+	{
+		struct si2157_config si2157_config = {};
+		struct si2168_config si2168_config = {};
+		struct i2c_board_info info = {};
+		struct i2c_client *client;
+		struct i2c_adapter *adapter;
+
+		/* attach demodulator chip */
+		si2168_config.ts_mode = SI2168_TS_SERIAL; /* from *.inf file */
+		si2168_config.fe = &dev->dvb->frontend;
+		si2168_config.i2c_adapter = &adapter;
+		si2168_config.ts_clock_inv = true;
+
+		strlcpy(info.type, "si2168", sizeof(info.type));
+		info.addr = dev->board.demod_addr;
+		info.platform_data = &si2168_config;
+
+		request_module(info.type);
+		client = i2c_new_device(demod_i2c, &info);
+
+		if (client == NULL || client->dev.driver == NULL) {
+			result = -ENODEV;
+			goto out_free;
+		}
+
+		if (!try_module_get(client->dev.driver->owner)) {
+			i2c_unregister_device(client);
+			result = -ENODEV;
+			goto out_free;
+		}
+
+		dvb->i2c_client_demod = client;
+		dvb->frontend->callback = cx231xx_tuner_callback;
+
+		/* attach tuner chip */
+		si2157_config.fe = dev->dvb->frontend;
+#ifdef CONFIG_MEDIA_CONTROLLER_DVB
+		si2157_config.mdev = dev->media_dev;
+#endif
+		si2157_config.if_port = 1;
+		si2157_config.inversion = false;
+
+		memset(&info, 0, sizeof(info));
+		strlcpy(info.type, "si2157", sizeof(info.type));
+		info.addr = dev->board.tuner_addr;
+		info.platform_data = &si2157_config;
+
+		request_module(info.type);
+		client = i2c_new_device(tuner_i2c, &info);
+
+		if (client == NULL || client->dev.driver == NULL) {
+			module_put(dvb->i2c_client_demod->dev.driver->owner);
+			i2c_unregister_device(dvb->i2c_client_demod);
+			result = -ENODEV;
+			goto out_free;
+		}
+
+		if (!try_module_get(client->dev.driver->owner)) {
+			i2c_unregister_device(client);
+			module_put(dvb->i2c_client_demod->dev.driver->owner);
+			i2c_unregister_device(dvb->i2c_client_demod);
+			result = -ENODEV;
+			goto out_free;
+		}
+
+		dev->cx231xx_reset_analog_tuner = NULL;
+		dev->dvb->i2c_client_tuner = client;
+		break;
+	}
 	default:
 		dev_err(dev->dev,
 			"%s/2: The frontend of your DVB/ATSC card isn't supported yet\n",
diff --git a/drivers/media/usb/cx231xx/cx231xx-i2c.c b/drivers/media/usb/cx231xx/cx231xx-i2c.c
index 35e9acf..60412ec 100644
--- a/drivers/media/usb/cx231xx/cx231xx-i2c.c
+++ b/drivers/media/usb/cx231xx/cx231xx-i2c.c
@@ -171,6 +171,43 @@ static int cx231xx_i2c_send_bytes(struct i2c_adapter *i2c_adap,
 		bus->i2c_nostop = 0;
 		bus->i2c_reserve = 0;
 
+	} else if (dev->model == CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD
+		&& msg->addr == dev->tuner_addr
+		&& msg->len > 4) {
+		/* special case for Evromedia USB Full Hybrid Full HD tuner chip */
+		size = msg->len;
+		saddr_len = 1;
+
+		/* adjust the length to correct length */
+		size -= saddr_len;
+
+		buf_ptr = (u8*)(msg->buf + 1);
+
+		do {
+			/* prepare xfer_data struct */
+			req_data.dev_addr = msg->addr;
+			req_data.direction = msg->flags;
+			req_data.saddr_len = saddr_len;
+			req_data.saddr_dat = msg->buf[0];
+			req_data.buf_size = size > 4 ? 4 : size;
+			req_data.p_buffer = (u8*)(buf_ptr + loop * 4);
+
+			bus->i2c_nostop = (size > 4) ? 1 : 0;
+			bus->i2c_reserve = (loop == 0) ? 0 : 1;
+
+			/* usb send command */
+			status = dev->cx231xx_send_usb_command(bus, &req_data);
+			loop++;
+
+			if (size >= 4) {
+				size -= 4;
+			} else {
+				size = 0;
+			}
+		} while (size > 0);
+
+		bus->i2c_nostop = 0;
+		bus->i2c_reserve = 0;
 	} else {		/* regular case */
 
 		/* prepare xfer_data struct */
diff --git a/drivers/media/usb/cx231xx/cx231xx.h b/drivers/media/usb/cx231xx/cx231xx.h
index 90c8676..d9792ea 100644
--- a/drivers/media/usb/cx231xx/cx231xx.h
+++ b/drivers/media/usb/cx231xx/cx231xx.h
@@ -78,6 +78,7 @@
 #define CX231XX_BOARD_HAUPPAUGE_930C_HD_1114xx 20
 #define CX231XX_BOARD_HAUPPAUGE_955Q 21
 #define CX231XX_BOARD_TERRATEC_GRABBY 22
+#define CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD 23
 
 /* Limits minimum and default number of buffers */
 #define CX231XX_MIN_BUF                 4
-- 
2.10.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH] [media] cx231xx: Initial support Evromedia USB Full Hybrid Full HD
@ 2017-01-09 21:41 Oleh Kravchenko
  0 siblings, 0 replies; 14+ messages in thread
From: Oleh Kravchenko @ 2017-01-09 21:41 UTC (permalink / raw)
  To: linux-media, hverkuil, crope; +Cc: Oleh Kravchenko

This patch provide only digital support.

The device is based on Si2168 30-demodulator,
Si2158-20 tuner and CX23102-11Z chipset;
USB id: 1b80:d3b2.

Status:
- DVB-T2 works fine;
- Composite and SVideo works fine;
- Analog not implemented.

Signed-off-by: Oleh Kravchenko <oleg@kaa.org.ua>
Tested-by: Oleh Kravchenko <oleg@kaa.org.ua>
---
 drivers/media/usb/cx231xx/Kconfig         |  1 +
 drivers/media/usb/cx231xx/cx231xx-cards.c | 29 +++++++++++++
 drivers/media/usb/cx231xx/cx231xx-dvb.c   | 70 +++++++++++++++++++++++++++++++
 drivers/media/usb/cx231xx/cx231xx-i2c.c   | 37 ++++++++++++++++
 drivers/media/usb/cx231xx/cx231xx.h       |  1 +
 5 files changed, 138 insertions(+)

diff --git a/drivers/media/usb/cx231xx/Kconfig b/drivers/media/usb/cx231xx/Kconfig
index 0cced3e..58de80b 100644
--- a/drivers/media/usb/cx231xx/Kconfig
+++ b/drivers/media/usb/cx231xx/Kconfig
@@ -50,6 +50,7 @@ config VIDEO_CX231XX_DVB
 	select DVB_LGDT3306A if MEDIA_SUBDRV_AUTOSELECT
 	select DVB_TDA18271C2DD if MEDIA_SUBDRV_AUTOSELECT
 	select DVB_SI2165 if MEDIA_SUBDRV_AUTOSELECT
+	select DVB_SI2168 if MEDIA_SUBDRV_AUTOSELECT
 	select MEDIA_TUNER_SI2157 if MEDIA_SUBDRV_AUTOSELECT
 
 	---help---
diff --git a/drivers/media/usb/cx231xx/cx231xx-cards.c b/drivers/media/usb/cx231xx/cx231xx-cards.c
index 36bc254..9b1df5a 100644
--- a/drivers/media/usb/cx231xx/cx231xx-cards.c
+++ b/drivers/media/usb/cx231xx/cx231xx-cards.c
@@ -841,6 +841,33 @@ struct cx231xx_board cx231xx_boards[] = {
 			.gpio = NULL,
 		} },
 	},
+	[CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD] = {
+		.name = "Evromedia USB Full Hybrid Full HD",
+		.tuner_type = TUNER_ABSENT,
+		.demod_addr = 0xc8 >> 1,
+		.demod_i2c_master = I2C_1_MUX_3,
+		.has_dvb = 1,
+		.ir_i2c_master = I2C_0,
+		.norm = V4L2_STD_PAL,
+		.output_mode = OUT_MODE_VIP11,
+		.tuner_addr = 0xc0 >> 1,
+		.tuner_i2c_master = I2C_2,
+		.input = {{
+			.type = CX231XX_VMUX_TELEVISION,
+			.vmux = 0,
+			.amux = CX231XX_AMUX_VIDEO,
+		}, {
+			.type = CX231XX_VMUX_COMPOSITE1,
+			.vmux = CX231XX_VIN_2_1,
+			.amux = CX231XX_AMUX_LINE_IN,
+		}, {
+			.type = CX231XX_VMUX_SVIDEO,
+			.vmux = CX231XX_VIN_1_1 |
+				(CX231XX_VIN_1_2 << 8) |
+				CX25840_SVIDEO_ON,
+			.amux = CX231XX_AMUX_LINE_IN,
+		} },
+	},
 };
 const unsigned int cx231xx_bcount = ARRAY_SIZE(cx231xx_boards);
 
@@ -908,6 +935,8 @@ struct usb_device_id cx231xx_id_table[] = {
 	 .driver_info = CX231XX_BOARD_OTG102},
 	{USB_DEVICE(USB_VID_TERRATEC, 0x00a6),
 	 .driver_info = CX231XX_BOARD_TERRATEC_GRABBY},
+	{USB_DEVICE(0x1b80, 0xd3b2),
+	.driver_info = CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD},
 	{},
 };
 
diff --git a/drivers/media/usb/cx231xx/cx231xx-dvb.c b/drivers/media/usb/cx231xx/cx231xx-dvb.c
index 1417515..1925329 100644
--- a/drivers/media/usb/cx231xx/cx231xx-dvb.c
+++ b/drivers/media/usb/cx231xx/cx231xx-dvb.c
@@ -33,6 +33,7 @@
 #include "s5h1411.h"
 #include "lgdt3305.h"
 #include "si2165.h"
+#include "si2168.h"
 #include "mb86a20s.h"
 #include "si2157.h"
 #include "lgdt3306a.h"
@@ -949,6 +950,75 @@ static int dvb_init(struct cx231xx *dev)
 			   &pv_tda18271_config);
 		break;
 
+	case CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD:
+	{
+		struct si2157_config si2157_config = {};
+		struct si2168_config si2168_config = {};
+		struct i2c_board_info info = {};
+		struct i2c_client *client;
+		struct i2c_adapter *adapter;
+
+		/* attach demodulator chip */
+		si2168_config.ts_mode = SI2168_TS_SERIAL; /* from *.inf file */
+		si2168_config.fe = &dev->dvb->frontend;
+		si2168_config.i2c_adapter = &adapter;
+		si2168_config.ts_clock_inv = true;
+
+		strlcpy(info.type, "si2168", sizeof(info.type));
+		info.addr = dev->board.demod_addr;
+		info.platform_data = &si2168_config;
+
+		request_module(info.type);
+		client = i2c_new_device(demod_i2c, &info);
+
+		if (client == NULL || client->dev.driver == NULL || dev->dvb->frontend == NULL) {
+			dev_err(dev->dev, "Failed to attach Si2168 front end\n");
+			result = -EINVAL;
+			goto out_free;
+		}
+
+		if (!try_module_get(client->dev.driver->owner)) {
+			i2c_unregister_device(client);
+			result = -ENODEV;
+			goto out_free;
+		}
+
+		dvb->i2c_client_demod = client;
+		dvb->frontend->callback = cx231xx_tuner_callback;
+
+		/* attach tuner chip */
+		si2157_config.fe = dev->dvb->frontend;
+#ifdef CONFIG_MEDIA_CONTROLLER_DVB
+		si2157_config.mdev = dev->media_dev;
+#endif
+		si2157_config.if_port = 1;
+		si2157_config.inversion = false;
+
+		memset(&info, 0, sizeof(info));
+		strlcpy(info.type, "si2157", sizeof(info.type));
+		info.addr = dev->board.tuner_addr;
+		info.platform_data = &si2157_config;
+
+		request_module("si2157");
+		client = i2c_new_device(tuner_i2c, &info);
+
+		if (client == NULL || client->dev.driver == NULL) {
+			dvb_frontend_detach(dev->dvb->frontend);
+			result = -ENODEV;
+			goto out_free;
+		}
+
+		if (!try_module_get(client->dev.driver->owner)) {
+			i2c_unregister_device(client);
+			dvb_frontend_detach(dev->dvb->frontend);
+			result = -ENODEV;
+			goto out_free;
+		}
+
+		dev->cx231xx_reset_analog_tuner = NULL;
+		dev->dvb->i2c_client_tuner = client;
+		break;
+	}
 	default:
 		dev_err(dev->dev,
 			"%s/2: The frontend of your DVB/ATSC card isn't supported yet\n",
diff --git a/drivers/media/usb/cx231xx/cx231xx-i2c.c b/drivers/media/usb/cx231xx/cx231xx-i2c.c
index 35e9acf..60412ec 100644
--- a/drivers/media/usb/cx231xx/cx231xx-i2c.c
+++ b/drivers/media/usb/cx231xx/cx231xx-i2c.c
@@ -171,6 +171,43 @@ static int cx231xx_i2c_send_bytes(struct i2c_adapter *i2c_adap,
 		bus->i2c_nostop = 0;
 		bus->i2c_reserve = 0;
 
+	} else if (dev->model == CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD
+		&& msg->addr == dev->tuner_addr
+		&& msg->len > 4) {
+		/* special case for Evromedia USB Full Hybrid Full HD tuner chip */
+		size = msg->len;
+		saddr_len = 1;
+
+		/* adjust the length to correct length */
+		size -= saddr_len;
+
+		buf_ptr = (u8*)(msg->buf + 1);
+
+		do {
+			/* prepare xfer_data struct */
+			req_data.dev_addr = msg->addr;
+			req_data.direction = msg->flags;
+			req_data.saddr_len = saddr_len;
+			req_data.saddr_dat = msg->buf[0];
+			req_data.buf_size = size > 4 ? 4 : size;
+			req_data.p_buffer = (u8*)(buf_ptr + loop * 4);
+
+			bus->i2c_nostop = (size > 4) ? 1 : 0;
+			bus->i2c_reserve = (loop == 0) ? 0 : 1;
+
+			/* usb send command */
+			status = dev->cx231xx_send_usb_command(bus, &req_data);
+			loop++;
+
+			if (size >= 4) {
+				size -= 4;
+			} else {
+				size = 0;
+			}
+		} while (size > 0);
+
+		bus->i2c_nostop = 0;
+		bus->i2c_reserve = 0;
 	} else {		/* regular case */
 
 		/* prepare xfer_data struct */
diff --git a/drivers/media/usb/cx231xx/cx231xx.h b/drivers/media/usb/cx231xx/cx231xx.h
index 90c8676..d9792ea 100644
--- a/drivers/media/usb/cx231xx/cx231xx.h
+++ b/drivers/media/usb/cx231xx/cx231xx.h
@@ -78,6 +78,7 @@
 #define CX231XX_BOARD_HAUPPAUGE_930C_HD_1114xx 20
 #define CX231XX_BOARD_HAUPPAUGE_955Q 21
 #define CX231XX_BOARD_TERRATEC_GRABBY 22
+#define CX231XX_BOARD_EVROMEDIA_FULL_HYBRID_FULLHD 23
 
 /* Limits minimum and default number of buffers */
 #define CX231XX_MIN_BUF                 4
-- 
2.10.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2017-01-29 18:04 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-09 15:23 [PATCH] [media] cx231xx: Initial support Evromedia USB Full Hybrid Full HD Oleh Kravchenko
2017-01-09 16:59 ` Antti Palosaari
2017-01-09 21:49   ` Oleh Kravchenko
2017-01-09 22:35     ` Antti Palosaari
2017-01-09 20:08 ` kbuild test robot
2017-01-09 21:41 Oleh Kravchenko
2017-01-10 21:41 Oleh Kravchenko
2017-01-11  0:16 ` Antti Palosaari
2017-01-11  7:28   ` Oleh Kravchenko
2017-01-11 10:08 Oleh Kravchenko
2017-01-11 13:53 ` Steven Toth
2017-01-11 14:00   ` Oleh Kravchenko
2017-01-11 14:13     ` Steven Toth
2017-01-29 18:03 Oleh Kravchenko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.