All of lore.kernel.org
 help / color / mirror / Atom feed
* How to proceed ? => [PATCH ?] EM28xx driver ?
@ 2018-04-23  7:18 mjs
  2018-04-23  9:09 ` Geert Stappers
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: mjs @ 2018-04-23  7:18 UTC (permalink / raw)
  To: 3 linux-media@vger.kernel.org; +Cc: Stappers (Geert)

Hello,

4 years ago I started to try to make a dvb-t ":ZOLID Hybrid TV Stick" to work with linux.

Today there is success at least for the digital part, not the analog or remote part.

If and how to proceed adding this patch to the global source.

Greatings,
  Marcel Stork


Git result:

From ccea81c2a3c6e937d304a87fad823dd557937968 Mon Sep 17 00:00:00 2001
From: mjs <mjs@toshiba>
Date: Sun, 1 Apr 2018 19:10:33 +0200
Subject: [PATCH]  Committer: mjs <mjs@toshiba>  On branch master
 zolid-em28xx-driver  Changes to be committed: 	modified:   em28xx-cards.c 
 modified:   em28xx-dvb.c 	modified:   em28xx.h

---
 em28xx-cards.c | 32 +++++++++++++++++++++++++++++---
 em28xx-dvb.c   |  1 +
 em28xx.h       |  1 +
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/em28xx-cards.c b/em28xx-cards.c
index e397f54..5024dd7 100644
--- a/em28xx-cards.c
+++ b/em28xx-cards.c
@@ -92,6 +92,20 @@ static struct em28xx_reg_seq default_digital[] = {
 	{	-1,		-1,	-1,		-1},
 };
 
+static struct em28xx_reg_seq zolid_tuner[] = {
+	{EM2820_R08_GPIO_CTRL,		0xfd,		0xff,	100},
+	{EM2820_R08_GPIO_CTRL,		0xfe,		0xff,	100},
+	{		-1,					-1,			-1,		 -1},
+};
+
+static struct em28xx_reg_seq zolid_digital[] = {
+	{EM2820_R08_GPIO_CTRL,		0x6a,		0xff,	100},
+	{EM2820_R08_GPIO_CTRL,		0x7a,		0xff,	100},
+	{EM2880_R04_GPO,			0x04,		0xff,	100},
+	{EM2880_R04_GPO,			0x0c,		0xff,	100},
+	{	-1,						-1,			-1,		 -1},
+};
+
 /* Board Hauppauge WinTV HVR 900 analog */
 static struct em28xx_reg_seq hauppauge_wintv_hvr_900_analog[] = {
 	{EM2820_R08_GPIO_CTRL,	0x2d,	~EM_GPIO_4,	10},
@@ -629,6 +643,17 @@ static struct em28xx_led hauppauge_dualhd_leds[] = {
  *  Board definitions
  */
 struct em28xx_board em28xx_boards[] = {
+
+	[EM2882_BOARD_ZOLID_HYBRID_TV_STICK] = {
+		.name			= ":ZOLID HYBRID TV STICK",
+		.tuner_type		= TUNER_XC2028,
+		.tuner_gpio		= zolid_tuner,
+		.decoder		= EM28XX_TVP5150,
+		.xclk			= EM28XX_XCLK_FREQUENCY_12MHZ,
+		.mts_firmware	= 1,
+		.has_dvb		= 1,
+		.dvb_gpio		= zolid_digital,
+	},
 	[EM2750_BOARD_UNKNOWN] = {
 		.name          = "EM2710/EM2750/EM2751 webcam grabber",
 		.xclk          = EM28XX_XCLK_FREQUENCY_20MHZ,
@@ -2421,7 +2446,7 @@ struct usb_device_id em28xx_id_table[] = {
 	{ USB_DEVICE(0xeb1a, 0x2881),
 			.driver_info = EM2820_BOARD_UNKNOWN },
 	{ USB_DEVICE(0xeb1a, 0x2883),
-			.driver_info = EM2820_BOARD_UNKNOWN },
+			.driver_info = EM2882_BOARD_ZOLID_HYBRID_TV_STICK },
 	{ USB_DEVICE(0xeb1a, 0x2868),
 			.driver_info = EM2820_BOARD_UNKNOWN },
 	{ USB_DEVICE(0xeb1a, 0x2875),
@@ -2599,6 +2624,7 @@ static struct em28xx_hash_table em28xx_eeprom_hash[] = {
 	{0xb8846b20, EM2881_BOARD_PINNACLE_HYBRID_PRO, TUNER_XC2028},
 	{0x63f653bd, EM2870_BOARD_REDDO_DVB_C_USB_BOX, TUNER_ABSENT},
 	{0x4e913442, EM2882_BOARD_DIKOM_DK300, TUNER_XC2028},
+	{0x85dd871e, EM2882_BOARD_ZOLID_HYBRID_TV_STICK, TUNER_XC2028},
 };
 
 /* I2C devicelist hash table for devices with generic USB IDs */
@@ -2610,6 +2636,7 @@ static struct em28xx_hash_table em28xx_i2c_hash[] = {
 	{0xc51200e3, EM2820_BOARD_GADMEI_TVR200, TUNER_LG_PAL_NEW_TAPC},
 	{0x4ba50080, EM2861_BOARD_GADMEI_UTV330PLUS, TUNER_TNF_5335MF},
 	{0x6b800080, EM2874_BOARD_LEADERSHIP_ISDBT, TUNER_ABSENT},
+	{0x27e10080, EM2882_BOARD_ZOLID_HYBRID_TV_STICK, TUNER_XC2028},
 };
 
 /* NOTE: introduce a separate hash table for devices with 16 bit eeproms */
@@ -3063,8 +3090,7 @@ void em28xx_setup_xc3028(struct em28xx *dev, struct xc2028_ctrl *ctl)
 	case EM2880_BOARD_EMPIRE_DUAL_TV:
 	case EM2880_BOARD_HAUPPAUGE_WINTV_HVR_900:
 	case EM2882_BOARD_TERRATEC_HYBRID_XS:
-		ctl->demod = XC3028_FE_ZARLINK456;
-		break;
+	case EM2882_BOARD_ZOLID_HYBRID_TV_STICK:
 	case EM2880_BOARD_TERRATEC_HYBRID_XS:
 	case EM2880_BOARD_TERRATEC_HYBRID_XS_FR:
 	case EM2881_BOARD_PINNACLE_HYBRID_PRO:
diff --git a/em28xx-dvb.c b/em28xx-dvb.c
index 1a5c012..e18d048 100644
--- a/em28xx-dvb.c
+++ b/em28xx-dvb.c
@@ -1197,6 +1197,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
 	case EM2880_BOARD_HAUPPAUGE_WINTV_HVR_900:
 	case EM2882_BOARD_TERRATEC_HYBRID_XS:
 	case EM2880_BOARD_EMPIRE_DUAL_TV:
+	case EM2882_BOARD_ZOLID_HYBRID_TV_STICK:
 		dvb->fe[0] = dvb_attach(zl10353_attach,
 					   &em28xx_zl10353_xc3028_no_i2c_gate,
 					   &dev->i2c_adap[dev->def_i2c_bus]);
diff --git a/em28xx.h b/em28xx.h
index d148463..d786bfa 100644
--- a/em28xx.h
+++ b/em28xx.h
@@ -147,6 +147,7 @@
 #define EM2884_BOARD_ELGATO_EYETV_HYBRID_2008     97
 #define EM28178_BOARD_PLEX_PX_BCUD                98
 #define EM28174_BOARD_HAUPPAUGE_WINTV_DUALHD_DVB  99
+#define EM2882_BOARD_ZOLID_HYBRID_TV_STICK	  100
 
 /* Limits minimum and default number of buffers */
 #define EM28XX_MIN_BUF 4
-- 
2.11.0

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

* Re: How to proceed ? => [PATCH ?] EM28xx driver ?
  2018-04-23  7:18 How to proceed ? => [PATCH ?] EM28xx driver ? mjs
@ 2018-04-23  9:09 ` Geert Stappers
  2018-04-23 16:35 ` Alexandre-Xavier Labonté-Lamoureux
  2018-04-26 19:59 ` [Solved] " mjs
  2 siblings, 0 replies; 7+ messages in thread
From: Geert Stappers @ 2018-04-23  9:09 UTC (permalink / raw)
  To: mjs; +Cc: 3 linux-media@vger.kernel.org

On Mon, Apr 23, 2018 at 09:18:01AM +0200, mjs wrote:
> to try to make a dvb-t ":ZOLID Hybrid TV Stick" to work with linux.
> 
> --- a/em28xx-cards.c
> +++ b/em28xx-cards.c
       ...
> @@ -3063,8 +3090,7 @@ void em28xx_setup_xc3028(struct em28xx *dev, struct xc2028_ctrl *ctl)
>  	case EM2880_BOARD_EMPIRE_DUAL_TV:
>  	case EM2880_BOARD_HAUPPAUGE_WINTV_HVR_900:
>  	case EM2882_BOARD_TERRATEC_HYBRID_XS:
> -		ctl->demod = XC3028_FE_ZARLINK456;
> -		break;
> +	case EM2882_BOARD_ZOLID_HYBRID_TV_STICK:
>  	case EM2880_BOARD_TERRATEC_HYBRID_XS:
>  	case EM2880_BOARD_TERRATEC_HYBRID_XS_FR:
>  	case EM2881_BOARD_PINNACLE_HYBRID_PRO:

The extra line makes sense,
the removed lines shall break  TERRATEC_HYBRID_XS


My appology for not telling you this earlier.

Cheers
Geert Stappers

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

* Re: How to proceed ? => [PATCH ?] EM28xx driver ?
  2018-04-23  7:18 How to proceed ? => [PATCH ?] EM28xx driver ? mjs
  2018-04-23  9:09 ` Geert Stappers
@ 2018-04-23 16:35 ` Alexandre-Xavier Labonté-Lamoureux
  2018-04-23 17:15   ` Mauro Carvalho Chehab
  2018-04-26 19:59 ` [Solved] " mjs
  2 siblings, 1 reply; 7+ messages in thread
From: Alexandre-Xavier Labonté-Lamoureux @ 2018-04-23 16:35 UTC (permalink / raw)
  To: mjs; +Cc: 3 linux-media@vger.kernel.org

This should give you an idea of what you need to do:
https://linuxtv.org/wiki/index.php/Development:_How_to_submit_patches


On Mon, Apr 23, 2018 at 3:18 AM, mjs <mjstork@gmail.com> wrote:
> Hello,
>
> 4 years ago I started to try to make a dvb-t ":ZOLID Hybrid TV Stick" to work with linux.
>
> Today there is success at least for the digital part, not the analog or remote part.
>
> If and how to proceed adding this patch to the global source.
>
> Greatings,
>   Marcel Stork
>
>
> Git result:
>
> From ccea81c2a3c6e937d304a87fad823dd557937968 Mon Sep 17 00:00:00 2001
> From: mjs <mjs@toshiba>
> Date: Sun, 1 Apr 2018 19:10:33 +0200
> Subject: [PATCH]  Committer: mjs <mjs@toshiba>  On branch master
>  zolid-em28xx-driver  Changes to be committed:  modified:   em28xx-cards.c
>  modified:   em28xx-dvb.c       modified:   em28xx.h
>
> ---
>  em28xx-cards.c | 32 +++++++++++++++++++++++++++++---
>  em28xx-dvb.c   |  1 +
>  em28xx.h       |  1 +
>  3 files changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/em28xx-cards.c b/em28xx-cards.c
> index e397f54..5024dd7 100644
> --- a/em28xx-cards.c
> +++ b/em28xx-cards.c
> @@ -92,6 +92,20 @@ static struct em28xx_reg_seq default_digital[] = {
>         {       -1,             -1,     -1,             -1},
>  };
>
> +static struct em28xx_reg_seq zolid_tuner[] = {
> +       {EM2820_R08_GPIO_CTRL,          0xfd,           0xff,   100},
> +       {EM2820_R08_GPIO_CTRL,          0xfe,           0xff,   100},
> +       {               -1,                                     -1,                     -1,              -1},
> +};
> +
> +static struct em28xx_reg_seq zolid_digital[] = {
> +       {EM2820_R08_GPIO_CTRL,          0x6a,           0xff,   100},
> +       {EM2820_R08_GPIO_CTRL,          0x7a,           0xff,   100},
> +       {EM2880_R04_GPO,                        0x04,           0xff,   100},
> +       {EM2880_R04_GPO,                        0x0c,           0xff,   100},
> +       {       -1,                                             -1,                     -1,              -1},
> +};
> +
>  /* Board Hauppauge WinTV HVR 900 analog */
>  static struct em28xx_reg_seq hauppauge_wintv_hvr_900_analog[] = {
>         {EM2820_R08_GPIO_CTRL,  0x2d,   ~EM_GPIO_4,     10},
> @@ -629,6 +643,17 @@ static struct em28xx_led hauppauge_dualhd_leds[] = {
>   *  Board definitions
>   */
>  struct em28xx_board em28xx_boards[] = {
> +
> +       [EM2882_BOARD_ZOLID_HYBRID_TV_STICK] = {
> +               .name                   = ":ZOLID HYBRID TV STICK",
> +               .tuner_type             = TUNER_XC2028,
> +               .tuner_gpio             = zolid_tuner,
> +               .decoder                = EM28XX_TVP5150,
> +               .xclk                   = EM28XX_XCLK_FREQUENCY_12MHZ,
> +               .mts_firmware   = 1,
> +               .has_dvb                = 1,
> +               .dvb_gpio               = zolid_digital,
> +       },
>         [EM2750_BOARD_UNKNOWN] = {
>                 .name          = "EM2710/EM2750/EM2751 webcam grabber",
>                 .xclk          = EM28XX_XCLK_FREQUENCY_20MHZ,
> @@ -2421,7 +2446,7 @@ struct usb_device_id em28xx_id_table[] = {
>         { USB_DEVICE(0xeb1a, 0x2881),
>                         .driver_info = EM2820_BOARD_UNKNOWN },
>         { USB_DEVICE(0xeb1a, 0x2883),
> -                       .driver_info = EM2820_BOARD_UNKNOWN },
> +                       .driver_info = EM2882_BOARD_ZOLID_HYBRID_TV_STICK },
>         { USB_DEVICE(0xeb1a, 0x2868),
>                         .driver_info = EM2820_BOARD_UNKNOWN },
>         { USB_DEVICE(0xeb1a, 0x2875),
> @@ -2599,6 +2624,7 @@ static struct em28xx_hash_table em28xx_eeprom_hash[] = {
>         {0xb8846b20, EM2881_BOARD_PINNACLE_HYBRID_PRO, TUNER_XC2028},
>         {0x63f653bd, EM2870_BOARD_REDDO_DVB_C_USB_BOX, TUNER_ABSENT},
>         {0x4e913442, EM2882_BOARD_DIKOM_DK300, TUNER_XC2028},
> +       {0x85dd871e, EM2882_BOARD_ZOLID_HYBRID_TV_STICK, TUNER_XC2028},
>  };
>
>  /* I2C devicelist hash table for devices with generic USB IDs */
> @@ -2610,6 +2636,7 @@ static struct em28xx_hash_table em28xx_i2c_hash[] = {
>         {0xc51200e3, EM2820_BOARD_GADMEI_TVR200, TUNER_LG_PAL_NEW_TAPC},
>         {0x4ba50080, EM2861_BOARD_GADMEI_UTV330PLUS, TUNER_TNF_5335MF},
>         {0x6b800080, EM2874_BOARD_LEADERSHIP_ISDBT, TUNER_ABSENT},
> +       {0x27e10080, EM2882_BOARD_ZOLID_HYBRID_TV_STICK, TUNER_XC2028},
>  };
>
>  /* NOTE: introduce a separate hash table for devices with 16 bit eeproms */
> @@ -3063,8 +3090,7 @@ void em28xx_setup_xc3028(struct em28xx *dev, struct xc2028_ctrl *ctl)
>         case EM2880_BOARD_EMPIRE_DUAL_TV:
>         case EM2880_BOARD_HAUPPAUGE_WINTV_HVR_900:
>         case EM2882_BOARD_TERRATEC_HYBRID_XS:
> -               ctl->demod = XC3028_FE_ZARLINK456;
> -               break;
> +       case EM2882_BOARD_ZOLID_HYBRID_TV_STICK:
>         case EM2880_BOARD_TERRATEC_HYBRID_XS:
>         case EM2880_BOARD_TERRATEC_HYBRID_XS_FR:
>         case EM2881_BOARD_PINNACLE_HYBRID_PRO:
> diff --git a/em28xx-dvb.c b/em28xx-dvb.c
> index 1a5c012..e18d048 100644
> --- a/em28xx-dvb.c
> +++ b/em28xx-dvb.c
> @@ -1197,6 +1197,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
>         case EM2880_BOARD_HAUPPAUGE_WINTV_HVR_900:
>         case EM2882_BOARD_TERRATEC_HYBRID_XS:
>         case EM2880_BOARD_EMPIRE_DUAL_TV:
> +       case EM2882_BOARD_ZOLID_HYBRID_TV_STICK:
>                 dvb->fe[0] = dvb_attach(zl10353_attach,
>                                            &em28xx_zl10353_xc3028_no_i2c_gate,
>                                            &dev->i2c_adap[dev->def_i2c_bus]);
> diff --git a/em28xx.h b/em28xx.h
> index d148463..d786bfa 100644
> --- a/em28xx.h
> +++ b/em28xx.h
> @@ -147,6 +147,7 @@
>  #define EM2884_BOARD_ELGATO_EYETV_HYBRID_2008     97
>  #define EM28178_BOARD_PLEX_PX_BCUD                98
>  #define EM28174_BOARD_HAUPPAUGE_WINTV_DUALHD_DVB  99
> +#define EM2882_BOARD_ZOLID_HYBRID_TV_STICK       100
>
>  /* Limits minimum and default number of buffers */
>  #define EM28XX_MIN_BUF 4
> --
> 2.11.0

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

* Re: How to proceed ? => [PATCH ?] EM28xx driver ?
  2018-04-23 16:35 ` Alexandre-Xavier Labonté-Lamoureux
@ 2018-04-23 17:15   ` Mauro Carvalho Chehab
  2018-04-23 20:21     ` mjs
  0 siblings, 1 reply; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2018-04-23 17:15 UTC (permalink / raw)
  To: Alexandre-Xavier Labonté-Lamoureux
  Cc: mjs, 3 linux-media@vger.kernel.org

Em Mon, 23 Apr 2018 12:35:06 -0400
Alexandre-Xavier Labonté-Lamoureux         <axdoomer@gmail.com> escreveu:

> This should give you an idea of what you need to do:
> https://linuxtv.org/wiki/index.php/Development:_How_to_submit_patches

Yes.

In particular, you should send it against the upstream development Kernel,
using "git diff", and add a proper description.

Anyway, let review a few things. See below.
> 
> 
> On Mon, Apr 23, 2018 at 3:18 AM, mjs <mjstork@gmail.com> wrote:
> > Hello,
> >
> > 4 years ago I started to try to make a dvb-t ":ZOLID Hybrid TV Stick" to work with linux.
> >
> > Today there is success at least for the digital part, not the analog or remote part.
> >
> > If and how to proceed adding this patch to the global source.
> >
> > Greatings,
> >   Marcel Stork
> >
> >
> > Git result:
> >
> > From ccea81c2a3c6e937d304a87fad823dd557937968 Mon Sep 17 00:00:00 2001
> > From: mjs <mjs@toshiba>
> > Date: Sun, 1 Apr 2018 19:10:33 +0200
> > Subject: [PATCH]  Committer: mjs <mjs@toshiba>  On branch master
> >  zolid-em28xx-driver  Changes to be committed:  modified:   em28xx-cards.c
> >  modified:   em28xx-dvb.c       modified:   em28xx.h
> >
> > ---
> >  em28xx-cards.c | 32 +++++++++++++++++++++++++++++---
> >  em28xx-dvb.c   |  1 +
> >  em28xx.h       |  1 +
> >  3 files changed, 31 insertions(+), 3 deletions(-)
> >
> > diff --git a/em28xx-cards.c b/em28xx-cards.c
> > index e397f54..5024dd7 100644
> > --- a/em28xx-cards.c
> > +++ b/em28xx-cards.c
> > @@ -92,6 +92,20 @@ static struct em28xx_reg_seq default_digital[] = {
> >         {       -1,             -1,     -1,             -1},
> >  };
> >
> > +static struct em28xx_reg_seq zolid_tuner[] = {
> > +       {EM2820_R08_GPIO_CTRL,          0xfd,           0xff,   100},
> > +       {EM2820_R08_GPIO_CTRL,          0xfe,           0xff,   100},
> > +       {               -1,                                     -1,                     -1,              -1},
> > +};
> > +
> > +static struct em28xx_reg_seq zolid_digital[] = {
> > +       {EM2820_R08_GPIO_CTRL,          0x6a,           0xff,   100},
> > +       {EM2820_R08_GPIO_CTRL,          0x7a,           0xff,   100},
> > +       {EM2880_R04_GPO,                        0x04,           0xff,   100},
> > +       {EM2880_R04_GPO,                        0x0c,           0xff,   100},
> > +       {       -1,                                             -1,                     -1,              -1},
> > +};
> > +
> >  /* Board Hauppauge WinTV HVR 900 analog */
> >  static struct em28xx_reg_seq hauppauge_wintv_hvr_900_analog[] = {
> >         {EM2820_R08_GPIO_CTRL,  0x2d,   ~EM_GPIO_4,     10},
> > @@ -629,6 +643,17 @@ static struct em28xx_led hauppauge_dualhd_leds[] = {
> >   *  Board definitions
> >   */
> >  struct em28xx_board em28xx_boards[] = {
> > +
> > +       [EM2882_BOARD_ZOLID_HYBRID_TV_STICK] = {
> > +               .name                   = ":ZOLID HYBRID TV STICK",
> > +               .tuner_type             = TUNER_XC2028,
> > +               .tuner_gpio             = zolid_tuner,
> > +               .decoder                = EM28XX_TVP5150,
> > +               .xclk                   = EM28XX_XCLK_FREQUENCY_12MHZ,
> > +               .mts_firmware   = 1,
> > +               .has_dvb                = 1,
> > +               .dvb_gpio               = zolid_digital,
> > +       },
> >         [EM2750_BOARD_UNKNOWN] = {
> >                 .name          = "EM2710/EM2750/EM2751 webcam grabber",
> >                 .xclk          = EM28XX_XCLK_FREQUENCY_20MHZ,
> > @@ -2421,7 +2446,7 @@ struct usb_device_id em28xx_id_table[] = {
> >         { USB_DEVICE(0xeb1a, 0x2881),
> >                         .driver_info = EM2820_BOARD_UNKNOWN },
> >         { USB_DEVICE(0xeb1a, 0x2883),
> > -                       .driver_info = EM2820_BOARD_UNKNOWN },
> > +                       .driver_info = EM2882_BOARD_ZOLID_HYBRID_TV_STICK },

This is actually wrong. The USB address EB1A:2883 is the generic one
with is hardcoded at the em28xx chip. All devices without eeprom 
(or with an eeprom that doesn't set a vendor-specific ID) will
announce this address.

So, instead of hardcoding, there are 3 alternatives:

1) if the board has an eeprom (whose USB is not filled), there's
a hint code that uses the eeprom to produce a hash;
2) if the board doesn't have, it could hint based on the devices
reported via an i2c scan;
3) if none as above works for this board, the user has to force a
modprobe parameter via card=<number>.

> >         { USB_DEVICE(0xeb1a, 0x2868),
> >                         .driver_info = EM2820_BOARD_UNKNOWN },
> >         { USB_DEVICE(0xeb1a, 0x2875),
> > @@ -2599,6 +2624,7 @@ static struct em28xx_hash_table em28xx_eeprom_hash[] = {
> >         {0xb8846b20, EM2881_BOARD_PINNACLE_HYBRID_PRO, TUNER_XC2028},
> >         {0x63f653bd, EM2870_BOARD_REDDO_DVB_C_USB_BOX, TUNER_ABSENT},
> >         {0x4e913442, EM2882_BOARD_DIKOM_DK300, TUNER_XC2028},
> > +       {0x85dd871e, EM2882_BOARD_ZOLID_HYBRID_TV_STICK, TUNER_XC2028},
> >  };
> >
> >  /* I2C devicelist hash table for devices with generic USB IDs */
> > @@ -2610,6 +2636,7 @@ static struct em28xx_hash_table em28xx_i2c_hash[] = {
> >         {0xc51200e3, EM2820_BOARD_GADMEI_TVR200, TUNER_LG_PAL_NEW_TAPC},
> >         {0x4ba50080, EM2861_BOARD_GADMEI_UTV330PLUS, TUNER_TNF_5335MF},
> >         {0x6b800080, EM2874_BOARD_LEADERSHIP_ISDBT, TUNER_ABSENT},
> > +       {0x27e10080, EM2882_BOARD_ZOLID_HYBRID_TV_STICK, TUNER_XC2028},
> >  };
> >
> >  /* NOTE: introduce a separate hash table for devices with 16 bit eeproms */
> > @@ -3063,8 +3090,7 @@ void em28xx_setup_xc3028(struct em28xx *dev, struct xc2028_ctrl *ctl)
> >         case EM2880_BOARD_EMPIRE_DUAL_TV:
> >         case EM2880_BOARD_HAUPPAUGE_WINTV_HVR_900:
> >         case EM2882_BOARD_TERRATEC_HYBRID_XS:
> > -               ctl->demod = XC3028_FE_ZARLINK456;
> > -               break;

Why are you removing it? You will break support for the above boards.

> > +       case EM2882_BOARD_ZOLID_HYBRID_TV_STICK:
> >         case EM2880_BOARD_TERRATEC_HYBRID_XS:
> >         case EM2880_BOARD_TERRATEC_HYBRID_XS_FR:
> >         case EM2881_BOARD_PINNACLE_HYBRID_PRO:
> > diff --git a/em28xx-dvb.c b/em28xx-dvb.c
> > index 1a5c012..e18d048 100644
> > --- a/em28xx-dvb.c
> > +++ b/em28xx-dvb.c
> > @@ -1197,6 +1197,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
> >         case EM2880_BOARD_HAUPPAUGE_WINTV_HVR_900:
> >         case EM2882_BOARD_TERRATEC_HYBRID_XS:
> >         case EM2880_BOARD_EMPIRE_DUAL_TV:
> > +       case EM2882_BOARD_ZOLID_HYBRID_TV_STICK:
> >                 dvb->fe[0] = dvb_attach(zl10353_attach,
> >                                            &em28xx_zl10353_xc3028_no_i2c_gate,
> >                                            &dev->i2c_adap[dev->def_i2c_bus]);
> > diff --git a/em28xx.h b/em28xx.h
> > index d148463..d786bfa 100644
> > --- a/em28xx.h
> > +++ b/em28xx.h
> > @@ -147,6 +147,7 @@
> >  #define EM2884_BOARD_ELGATO_EYETV_HYBRID_2008     97
> >  #define EM28178_BOARD_PLEX_PX_BCUD                98
> >  #define EM28174_BOARD_HAUPPAUGE_WINTV_DUALHD_DVB  99
> > +#define EM2882_BOARD_ZOLID_HYBRID_TV_STICK       100
> >
> >  /* Limits minimum and default number of buffers */
> >  #define EM28XX_MIN_BUF 4
> > --
> > 2.11.0  



Thanks,
Mauro

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

* Re: How to proceed ? => [PATCH ?] EM28xx driver ?
  2018-04-23 17:15   ` Mauro Carvalho Chehab
@ 2018-04-23 20:21     ` mjs
  2018-04-23 20:49       ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 7+ messages in thread
From: mjs @ 2018-04-23 20:21 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Alexandre-Xavier Labonté-Lamoureux,
	3 linux-media@vger.kernel.org, Stappers (Geert)

Op Mon, 23 Apr 2018 14:15:59 -0300
Mauro Carvalho Chehab <mchehab@s-opensource.com> schreef:

> Em Mon, 23 Apr 2018 12:35:06 -0400
> Alexandre-Xavier Labonté-Lamoureux         <axdoomer@gmail.com> escreveu:
> 
> > This should give you an idea of what you need to do:
> > https://linuxtv.org/wiki/index.php/Development:_How_to_submit_patches  
> 
> Yes.
> 
> In particular, you should send it against the upstream development Kernel,
> using "git diff", and add a proper description.


Thanks, I will read and use it.


> 
> Anyway, let review a few things. See below.
> > 
> > 
> > On Mon, Apr 23, 2018 at 3:18 AM, mjs <mjstork@gmail.com> wrote:  
> > > Hello,
> > >
> > > 4 years ago I started to try to make a dvb-t ":ZOLID Hybrid TV Stick" to work with linux.
> > >
> > > Today there is success at least for the digital part, not the analog or remote part.
> > >
> > > If and how to proceed adding this patch to the global source.
> > >
> > > Greatings,
> > >   Marcel Stork
> > >
> > >
> > > Git result:
> > >
> > > From ccea81c2a3c6e937d304a87fad823dd557937968 Mon Sep 17 00:00:00 2001
> > > From: mjs <mjs@toshiba>
> > > Date: Sun, 1 Apr 2018 19:10:33 +0200
> > > Subject: [PATCH]  Committer: mjs <mjs@toshiba>  On branch master
> > >  zolid-em28xx-driver  Changes to be committed:  modified:   em28xx-cards.c
> > >  modified:   em28xx-dvb.c       modified:   em28xx.h
> > >
> > > ---
> > >  em28xx-cards.c | 32 +++++++++++++++++++++++++++++---
> > >  em28xx-dvb.c   |  1 +
> > >  em28xx.h       |  1 +
> > >  3 files changed, 31 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/em28xx-cards.c b/em28xx-cards.c
> > > index e397f54..5024dd7 100644
> > > --- a/em28xx-cards.c
> > > +++ b/em28xx-cards.c
> > > @@ -92,6 +92,20 @@ static struct em28xx_reg_seq default_digital[] = {
> > >         {       -1,             -1,     -1,             -1},
> > >  };
> > >
> > > +static struct em28xx_reg_seq zolid_tuner[] = {
> > > +       {EM2820_R08_GPIO_CTRL,          0xfd,           0xff,   100},
> > > +       {EM2820_R08_GPIO_CTRL,          0xfe,           0xff,   100},
> > > +       {               -1,                                     -1,                     -1,              -1},
> > > +};
> > > +
> > > +static struct em28xx_reg_seq zolid_digital[] = {
> > > +       {EM2820_R08_GPIO_CTRL,          0x6a,           0xff,   100},
> > > +       {EM2820_R08_GPIO_CTRL,          0x7a,           0xff,   100},
> > > +       {EM2880_R04_GPO,                        0x04,           0xff,   100},
> > > +       {EM2880_R04_GPO,                        0x0c,           0xff,   100},
> > > +       {       -1,                                             -1,                     -1,              -1},
> > > +};
> > > +
> > >  /* Board Hauppauge WinTV HVR 900 analog */
> > >  static struct em28xx_reg_seq hauppauge_wintv_hvr_900_analog[] = {
> > >         {EM2820_R08_GPIO_CTRL,  0x2d,   ~EM_GPIO_4,     10},
> > > @@ -629,6 +643,17 @@ static struct em28xx_led hauppauge_dualhd_leds[] = {
> > >   *  Board definitions
> > >   */
> > >  struct em28xx_board em28xx_boards[] = {
> > > +
> > > +       [EM2882_BOARD_ZOLID_HYBRID_TV_STICK] = {
> > > +               .name                   = ":ZOLID HYBRID TV STICK",
> > > +               .tuner_type             = TUNER_XC2028,
> > > +               .tuner_gpio             = zolid_tuner,
> > > +               .decoder                = EM28XX_TVP5150,
> > > +               .xclk                   = EM28XX_XCLK_FREQUENCY_12MHZ,
> > > +               .mts_firmware   = 1,
> > > +               .has_dvb                = 1,
> > > +               .dvb_gpio               = zolid_digital,
> > > +       },
> > >         [EM2750_BOARD_UNKNOWN] = {
> > >                 .name          = "EM2710/EM2750/EM2751 webcam grabber",
> > >                 .xclk          = EM28XX_XCLK_FREQUENCY_20MHZ,
> > > @@ -2421,7 +2446,7 @@ struct usb_device_id em28xx_id_table[] = {
> > >         { USB_DEVICE(0xeb1a, 0x2881),
> > >                         .driver_info = EM2820_BOARD_UNKNOWN },
> > >         { USB_DEVICE(0xeb1a, 0x2883),
> > > -                       .driver_info = EM2820_BOARD_UNKNOWN },
> > > +                       .driver_info = EM2882_BOARD_ZOLID_HYBRID_TV_STICK },  
> 
> This is actually wrong. The USB address EB1A:2883 is the generic one
> with is hardcoded at the em28xx chip. All devices without eeprom 
> (or with an eeprom that doesn't set a vendor-specific ID) will
> announce this address.

The eeprom has this vid-pid ID (according to the em2882 data sheet).
em28xx #0: i2c eeprom 00: 1a eb 67 95 1a eb 83 28 d0 12 65 00 6a 22 8c 10
em28xx #0: i2c eeprom 10: 00 00 24 57 4e 37 01 00 60 00 00 00 02 00 00 00
and so on .....

> 
> So, instead of hardcoding, there are 3 alternatives:
> 
> 1) if the board has an eeprom (whose USB is not filled), there's
> a hint code that uses the eeprom to produce a hash;
> 2) if the board doesn't have, it could hint based on the devices
> reported via an i2c scan;
> 3) if none as above works for this board, the user has to force a
> modprobe parameter via card=<number>.

Option 1 and 2 are available (see below).
I have to try option 3 again with the knowledge of today (dit not work 4 years ago)

> 
> > >         { USB_DEVICE(0xeb1a, 0x2868),
> > >                         .driver_info = EM2820_BOARD_UNKNOWN },
> > >         { USB_DEVICE(0xeb1a, 0x2875),
> > > @@ -2599,6 +2624,7 @@ static struct em28xx_hash_table em28xx_eeprom_hash[] = {
> > >         {0xb8846b20, EM2881_BOARD_PINNACLE_HYBRID_PRO, TUNER_XC2028},
> > >         {0x63f653bd, EM2870_BOARD_REDDO_DVB_C_USB_BOX, TUNER_ABSENT},
> > >         {0x4e913442, EM2882_BOARD_DIKOM_DK300, TUNER_XC2028},
> > > +       {0x85dd871e, EM2882_BOARD_ZOLID_HYBRID_TV_STICK, TUNER_XC2028},
> > >  };
> > >
> > >  /* I2C devicelist hash table for devices with generic USB IDs */
> > > @@ -2610,6 +2636,7 @@ static struct em28xx_hash_table em28xx_i2c_hash[] = {
> > >         {0xc51200e3, EM2820_BOARD_GADMEI_TVR200, TUNER_LG_PAL_NEW_TAPC},
> > >         {0x4ba50080, EM2861_BOARD_GADMEI_UTV330PLUS, TUNER_TNF_5335MF},
> > >         {0x6b800080, EM2874_BOARD_LEADERSHIP_ISDBT, TUNER_ABSENT},
> > > +       {0x27e10080, EM2882_BOARD_ZOLID_HYBRID_TV_STICK, TUNER_XC2028},
> > >  };
> > >
> > >  /* NOTE: introduce a separate hash table for devices with 16 bit eeproms */
> > > @@ -3063,8 +3090,7 @@ void em28xx_setup_xc3028(struct em28xx *dev, struct xc2028_ctrl *ctl)
> > >         case EM2880_BOARD_EMPIRE_DUAL_TV:
> > >         case EM2880_BOARD_HAUPPAUGE_WINTV_HVR_900:
> > >         case EM2882_BOARD_TERRATEC_HYBRID_XS:
> > > -               ctl->demod = XC3028_FE_ZARLINK456;
> > > -               break;  
> 
> Why are you removing it? You will break support for the above boards.
> 

My line of thinking:

My original source is like:
	switch (dev->model) {
	case EM2880_BOARD_EMPIRE_DUAL_TV:
	case EM2880_BOARD_HAUPPAUGE_WINTV_HVR_900:
	case EM2882_BOARD_TERRATEC_HYBRID_XS:
		ctl->demod = XC3028_FE_ZARLINK456;
		break;
	case EM2880_BOARD_TERRATEC_HYBRID_XS:
	case EM2880_BOARD_TERRATEC_HYBRID_XS_FR:
	case EM2881_BOARD_PINNACLE_HYBRID_PRO:
		ctl->demod = XC3028_FE_ZARLINK456;
		break;

Deleting the two lines would give:
	switch (dev->model) {
	case EM2880_BOARD_EMPIRE_DUAL_TV:
	case EM2880_BOARD_HAUPPAUGE_WINTV_HVR_900:
	case EM2882_BOARD_TERRATEC_HYBRID_XS:
	case EM2880_BOARD_TERRATEC_HYBRID_XS:
	case EM2880_BOARD_TERRATEC_HYBRID_XS_FR:
	case EM2881_BOARD_PINNACLE_HYBRID_PRO:
		ctl->demod = XC3028_FE_ZARLINK456;
		break;

Put the two blobs together to get a reduction of lines because they seems to have the same result.
I was convinced not breaking some line of code unless there is a maximum of putting a number of "case......." together.
I think in this case the git-diff does not show the whole picture, additional explanation from my side would have been better.


> > > +       case EM2882_BOARD_ZOLID_HYBRID_TV_STICK:
> > >         case EM2880_BOARD_TERRATEC_HYBRID_XS:
> > >         case EM2880_BOARD_TERRATEC_HYBRID_XS_FR:
> > >         case EM2881_BOARD_PINNACLE_HYBRID_PRO:
> > > diff --git a/em28xx-dvb.c b/em28xx-dvb.c
> > > index 1a5c012..e18d048 100644
> > > --- a/em28xx-dvb.c
> > > +++ b/em28xx-dvb.c
> > > @@ -1197,6 +1197,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
> > >         case EM2880_BOARD_HAUPPAUGE_WINTV_HVR_900:
> > >         case EM2882_BOARD_TERRATEC_HYBRID_XS:
> > >         case EM2880_BOARD_EMPIRE_DUAL_TV:
> > > +       case EM2882_BOARD_ZOLID_HYBRID_TV_STICK:
> > >                 dvb->fe[0] = dvb_attach(zl10353_attach,
> > >                                            &em28xx_zl10353_xc3028_no_i2c_gate,
> > >                                            &dev->i2c_adap[dev->def_i2c_bus]);
> > > diff --git a/em28xx.h b/em28xx.h
> > > index d148463..d786bfa 100644
> > > --- a/em28xx.h
> > > +++ b/em28xx.h
> > > @@ -147,6 +147,7 @@
> > >  #define EM2884_BOARD_ELGATO_EYETV_HYBRID_2008     97
> > >  #define EM28178_BOARD_PLEX_PX_BCUD                98
> > >  #define EM28174_BOARD_HAUPPAUGE_WINTV_DUALHD_DVB  99
> > > +#define EM2882_BOARD_ZOLID_HYBRID_TV_STICK       100
> > >
> > >  /* Limits minimum and default number of buffers */
> > >  #define EM28XX_MIN_BUF 4
> > > --
> > > 2.11.0    
> 

I'll do my best.
Thanks,

 Marcel

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

* Re: How to proceed ? => [PATCH ?] EM28xx driver ?
  2018-04-23 20:21     ` mjs
@ 2018-04-23 20:49       ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2018-04-23 20:49 UTC (permalink / raw)
  To: mjs
  Cc: Alexandre-Xavier Labonté-Lamoureux,
	3 linux-media@vger.kernel.org, Stappers (Geert)

Em Mon, 23 Apr 2018 22:21:29 +0200
mjs <mjstork@gmail.com> escreveu:

> Op Mon, 23 Apr 2018 14:15:59 -0300
> Mauro Carvalho Chehab <mchehab@s-opensource.com> schreef:
> 
> > Em Mon, 23 Apr 2018 12:35:06 -0400
> > Alexandre-Xavier Labonté-Lamoureux         <axdoomer@gmail.com> escreveu:
> >   
> > > This should give you an idea of what you need to do:
> > > https://linuxtv.org/wiki/index.php/Development:_How_to_submit_patches    
> > 
> > Yes.
> > 
> > In particular, you should send it against the upstream development Kernel,
> > using "git diff", and add a proper description.  
> 
> 
> Thanks, I will read and use it.
> 
> 
> > 
> > Anyway, let review a few things. See below.  
> > > 
> > > 
> > > On Mon, Apr 23, 2018 at 3:18 AM, mjs <mjstork@gmail.com> wrote:    
> > > > Hello,
> > > >
> > > > 4 years ago I started to try to make a dvb-t ":ZOLID Hybrid TV Stick" to work with linux.
> > > >
> > > > Today there is success at least for the digital part, not the analog or remote part.
> > > >
> > > > If and how to proceed adding this patch to the global source.
> > > >
> > > > Greatings,
> > > >   Marcel Stork
> > > >
> > > >
> > > > Git result:
> > > >
> > > > From ccea81c2a3c6e937d304a87fad823dd557937968 Mon Sep 17 00:00:00 2001
> > > > From: mjs <mjs@toshiba>
> > > > Date: Sun, 1 Apr 2018 19:10:33 +0200
> > > > Subject: [PATCH]  Committer: mjs <mjs@toshiba>  On branch master
> > > >  zolid-em28xx-driver  Changes to be committed:  modified:   em28xx-cards.c
> > > >  modified:   em28xx-dvb.c       modified:   em28xx.h
> > > >
> > > > ---
> > > >  em28xx-cards.c | 32 +++++++++++++++++++++++++++++---
> > > >  em28xx-dvb.c   |  1 +
> > > >  em28xx.h       |  1 +
> > > >  3 files changed, 31 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/em28xx-cards.c b/em28xx-cards.c
> > > > index e397f54..5024dd7 100644
> > > > --- a/em28xx-cards.c
> > > > +++ b/em28xx-cards.c
> > > > @@ -92,6 +92,20 @@ static struct em28xx_reg_seq default_digital[] = {
> > > >         {       -1,             -1,     -1,             -1},
> > > >  };
> > > >
> > > > +static struct em28xx_reg_seq zolid_tuner[] = {
> > > > +       {EM2820_R08_GPIO_CTRL,          0xfd,           0xff,   100},
> > > > +       {EM2820_R08_GPIO_CTRL,          0xfe,           0xff,   100},
> > > > +       {               -1,                                     -1,                     -1,              -1},
> > > > +};
> > > > +
> > > > +static struct em28xx_reg_seq zolid_digital[] = {
> > > > +       {EM2820_R08_GPIO_CTRL,          0x6a,           0xff,   100},
> > > > +       {EM2820_R08_GPIO_CTRL,          0x7a,           0xff,   100},
> > > > +       {EM2880_R04_GPO,                        0x04,           0xff,   100},
> > > > +       {EM2880_R04_GPO,                        0x0c,           0xff,   100},
> > > > +       {       -1,                                             -1,                     -1,              -1},
> > > > +};
> > > > +
> > > >  /* Board Hauppauge WinTV HVR 900 analog */
> > > >  static struct em28xx_reg_seq hauppauge_wintv_hvr_900_analog[] = {
> > > >         {EM2820_R08_GPIO_CTRL,  0x2d,   ~EM_GPIO_4,     10},
> > > > @@ -629,6 +643,17 @@ static struct em28xx_led hauppauge_dualhd_leds[] = {
> > > >   *  Board definitions
> > > >   */
> > > >  struct em28xx_board em28xx_boards[] = {
> > > > +
> > > > +       [EM2882_BOARD_ZOLID_HYBRID_TV_STICK] = {
> > > > +               .name                   = ":ZOLID HYBRID TV STICK",
> > > > +               .tuner_type             = TUNER_XC2028,
> > > > +               .tuner_gpio             = zolid_tuner,
> > > > +               .decoder                = EM28XX_TVP5150,
> > > > +               .xclk                   = EM28XX_XCLK_FREQUENCY_12MHZ,
> > > > +               .mts_firmware   = 1,
> > > > +               .has_dvb                = 1,
> > > > +               .dvb_gpio               = zolid_digital,
> > > > +       },
> > > >         [EM2750_BOARD_UNKNOWN] = {
> > > >                 .name          = "EM2710/EM2750/EM2751 webcam grabber",
> > > >                 .xclk          = EM28XX_XCLK_FREQUENCY_20MHZ,
> > > > @@ -2421,7 +2446,7 @@ struct usb_device_id em28xx_id_table[] = {
> > > >         { USB_DEVICE(0xeb1a, 0x2881),
> > > >                         .driver_info = EM2820_BOARD_UNKNOWN },
> > > >         { USB_DEVICE(0xeb1a, 0x2883),
> > > > -                       .driver_info = EM2820_BOARD_UNKNOWN },
> > > > +                       .driver_info = EM2882_BOARD_ZOLID_HYBRID_TV_STICK },    
> > 
> > This is actually wrong. The USB address EB1A:2883 is the generic one
> > with is hardcoded at the em28xx chip. All devices without eeprom 
> > (or with an eeprom that doesn't set a vendor-specific ID) will
> > announce this address.  
> 
> The eeprom has this vid-pid ID (according to the em2882 data sheet).
> em28xx #0: i2c eeprom 00: 1a eb 67 95 1a eb 83 28 d0 12 65 00 6a 22 8c 10
> em28xx #0: i2c eeprom 10: 00 00 24 57 4e 37 01 00 60 00 00 00 02 00 00 00
> and so on .....

Lazy vendor, with either didn't buy his own USB ID block or didn't
care enough to write it at the device's eeprom.

> 
> > 
> > So, instead of hardcoding, there are 3 alternatives:
> > 
> > 1) if the board has an eeprom (whose USB is not filled), there's
> > a hint code that uses the eeprom to produce a hash;
> > 2) if the board doesn't have, it could hint based on the devices
> > reported via an i2c scan;
> > 3) if none as above works for this board, the user has to force a
> > modprobe parameter via card=<number>.  
> 
> Option 1 and 2 are available (see below).

Ah, OK! Then, just drop the above hunk from the patch.

> I have to try option 3 again with the knowledge of today (dit not work 4 years ago)

Option 3 is always available. You can always force the driver to try use
a different model at probing.

> 
> >   
> > > >         { USB_DEVICE(0xeb1a, 0x2868),
> > > >                         .driver_info = EM2820_BOARD_UNKNOWN },
> > > >         { USB_DEVICE(0xeb1a, 0x2875),
> > > > @@ -2599,6 +2624,7 @@ static struct em28xx_hash_table em28xx_eeprom_hash[] = {
> > > >         {0xb8846b20, EM2881_BOARD_PINNACLE_HYBRID_PRO, TUNER_XC2028},
> > > >         {0x63f653bd, EM2870_BOARD_REDDO_DVB_C_USB_BOX, TUNER_ABSENT},
> > > >         {0x4e913442, EM2882_BOARD_DIKOM_DK300, TUNER_XC2028},
> > > > +       {0x85dd871e, EM2882_BOARD_ZOLID_HYBRID_TV_STICK, TUNER_XC2028},
> > > >  };
> > > >
> > > >  /* I2C devicelist hash table for devices with generic USB IDs */
> > > > @@ -2610,6 +2636,7 @@ static struct em28xx_hash_table em28xx_i2c_hash[] = {
> > > >         {0xc51200e3, EM2820_BOARD_GADMEI_TVR200, TUNER_LG_PAL_NEW_TAPC},
> > > >         {0x4ba50080, EM2861_BOARD_GADMEI_UTV330PLUS, TUNER_TNF_5335MF},
> > > >         {0x6b800080, EM2874_BOARD_LEADERSHIP_ISDBT, TUNER_ABSENT},
> > > > +       {0x27e10080, EM2882_BOARD_ZOLID_HYBRID_TV_STICK, TUNER_XC2028},
> > > >  };
> > > >
> > > >  /* NOTE: introduce a separate hash table for devices with 16 bit eeproms */
> > > > @@ -3063,8 +3090,7 @@ void em28xx_setup_xc3028(struct em28xx *dev, struct xc2028_ctrl *ctl)
> > > >         case EM2880_BOARD_EMPIRE_DUAL_TV:
> > > >         case EM2880_BOARD_HAUPPAUGE_WINTV_HVR_900:
> > > >         case EM2882_BOARD_TERRATEC_HYBRID_XS:
> > > > -               ctl->demod = XC3028_FE_ZARLINK456;
> > > > -               break;    
> > 
> > Why are you removing it? You will break support for the above boards.
> >   
> 
> My line of thinking:
> 
> My original source is like:
> 	switch (dev->model) {
> 	case EM2880_BOARD_EMPIRE_DUAL_TV:
> 	case EM2880_BOARD_HAUPPAUGE_WINTV_HVR_900:
> 	case EM2882_BOARD_TERRATEC_HYBRID_XS:
> 		ctl->demod = XC3028_FE_ZARLINK456;
> 		break;
> 	case EM2880_BOARD_TERRATEC_HYBRID_XS:
> 	case EM2880_BOARD_TERRATEC_HYBRID_XS_FR:
> 	case EM2881_BOARD_PINNACLE_HYBRID_PRO:
> 		ctl->demod = XC3028_FE_ZARLINK456;
> 		break;
> 
> Deleting the two lines would give:
> 	switch (dev->model) {
> 	case EM2880_BOARD_EMPIRE_DUAL_TV:
> 	case EM2880_BOARD_HAUPPAUGE_WINTV_HVR_900:
> 	case EM2882_BOARD_TERRATEC_HYBRID_XS:
> 	case EM2880_BOARD_TERRATEC_HYBRID_XS:
> 	case EM2880_BOARD_TERRATEC_HYBRID_XS_FR:
> 	case EM2881_BOARD_PINNACLE_HYBRID_PRO:
> 		ctl->demod = XC3028_FE_ZARLINK456;
> 		break;
> 
> Put the two blobs together to get a reduction of lines because they seems to have the same result.

Ok, but then send it as a separate patch, as it has nothing to do with this
board's addition.

> I was convinced not breaking some line of code unless there is a maximum of putting a number of "case......." together.

There's no restriction. It can have as many case... as needed.

> I think in this case the git-diff does not show the whole picture, additional explanation from my side would have been better.

Actually, a separate patch :-)

The way it was sent, it appeared that some lines were just removed
by mistake.

> 
> 
> > > > +       case EM2882_BOARD_ZOLID_HYBRID_TV_STICK:
> > > >         case EM2880_BOARD_TERRATEC_HYBRID_XS:
> > > >         case EM2880_BOARD_TERRATEC_HYBRID_XS_FR:
> > > >         case EM2881_BOARD_PINNACLE_HYBRID_PRO:
> > > > diff --git a/em28xx-dvb.c b/em28xx-dvb.c
> > > > index 1a5c012..e18d048 100644
> > > > --- a/em28xx-dvb.c
> > > > +++ b/em28xx-dvb.c
> > > > @@ -1197,6 +1197,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
> > > >         case EM2880_BOARD_HAUPPAUGE_WINTV_HVR_900:
> > > >         case EM2882_BOARD_TERRATEC_HYBRID_XS:
> > > >         case EM2880_BOARD_EMPIRE_DUAL_TV:
> > > > +       case EM2882_BOARD_ZOLID_HYBRID_TV_STICK:
> > > >                 dvb->fe[0] = dvb_attach(zl10353_attach,
> > > >                                            &em28xx_zl10353_xc3028_no_i2c_gate,
> > > >                                            &dev->i2c_adap[dev->def_i2c_bus]);
> > > > diff --git a/em28xx.h b/em28xx.h
> > > > index d148463..d786bfa 100644
> > > > --- a/em28xx.h
> > > > +++ b/em28xx.h
> > > > @@ -147,6 +147,7 @@
> > > >  #define EM2884_BOARD_ELGATO_EYETV_HYBRID_2008     97
> > > >  #define EM28178_BOARD_PLEX_PX_BCUD                98
> > > >  #define EM28174_BOARD_HAUPPAUGE_WINTV_DUALHD_DVB  99
> > > > +#define EM2882_BOARD_ZOLID_HYBRID_TV_STICK       100
> > > >
> > > >  /* Limits minimum and default number of buffers */
> > > >  #define EM28XX_MIN_BUF 4
> > > > --
> > > > 2.11.0      
> >   
> 
> I'll do my best.
> Thanks,
> 
>  Marcel



Thanks,
Mauro

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

* [Solved] How to proceed ? => [PATCH ?] EM28xx driver ?
  2018-04-23  7:18 How to proceed ? => [PATCH ?] EM28xx driver ? mjs
  2018-04-23  9:09 ` Geert Stappers
  2018-04-23 16:35 ` Alexandre-Xavier Labonté-Lamoureux
@ 2018-04-26 19:59 ` mjs
  2 siblings, 0 replies; 7+ messages in thread
From: mjs @ 2018-04-26 19:59 UTC (permalink / raw)
  To: mjstork, 3 linux-media@vger.kernel.org

Solved.
See msg 130325.

Thanks,
  Marcel

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

end of thread, other threads:[~2018-04-26 19:59 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-23  7:18 How to proceed ? => [PATCH ?] EM28xx driver ? mjs
2018-04-23  9:09 ` Geert Stappers
2018-04-23 16:35 ` Alexandre-Xavier Labonté-Lamoureux
2018-04-23 17:15   ` Mauro Carvalho Chehab
2018-04-23 20:21     ` mjs
2018-04-23 20:49       ` Mauro Carvalho Chehab
2018-04-26 19:59 ` [Solved] " mjs

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.