diff mbox series

[1/2] dvbsky: add support for "Mygica T230C v2"

Message ID 20190616003929.GE4518@jpvw.nl (mailing list archive)
State New, archived
Headers show
Series [1/2] dvbsky: add support for "Mygica T230C v2" | expand

Commit Message

Jan Pieter van Woerkom June 16, 2019, 12:39 a.m. UTC 
From: Jan Pieter van Woerkom <jp@jpvw.nl>

Adds support for the "Mygica T230C v2" into the "dvbsky" driver.
A small enhancement is also needed in the si2168 demodulator
driver, and a USB device ID in dvb-usb-ids.h .

This is v3.3 of the proposed patch, based on feedback from Sean
Young and Antti Palosaari.
Tested by patch author on a T230C v2.
Tested by Frank Rysanek on a T230C v2: can tune into locally
available DVB-T and DVB-T2 muxes, video and audio playback works.
Applies cleanly against Linux 5.1.10 .

The T230C v2 hardware needs a mode of the si2168 chip to be
set for which the si2168 driver previously had no support.
This patch uses a specific measure to configure this on the
T230C v2 hardware only - see the flag passed via the ts_mode
attribute and its dependency on USB_PID_MYGICA_T230C2. Other
devices using the si2168 demodulator driver are not affected
in any way.

Signed-off-by: Jan Pieter van Woerkom <jp@jpvw.nl>
Tested-by: Frank Rysanek <Frantisek.Rysanek@post.cz>
---

Comments

Jan Pieter van Woerkom June 16, 2019, 12:44 a.m. UTC | #1 
From: Jan Pieter van Woerkom <jp@jpvw.nl>

Adds support for the "Mygica T230C v2" into the "dvbsky" driver.

Signed-off-by: Jan Pieter van Woerkom <jp@jpvw.nl>
Tested-by: Frank Rysanek <Frantisek.Rysanek@post.cz>
---
diff -ru a/drivers/media/usb/dvb-usb-v2/dvbsky.c b/drivers/media/usb/dvb-usb-v2/dvbsky.c
--- a/drivers/media/usb/dvb-usb-v2/dvbsky.c	2019-06-04 07:59:45.000000000 +0200
+++ b/drivers/media/usb/dvb-usb-v2/dvbsky.c	2019-06-08 19:49:01.688181437 +0200
@@ -560,6 +560,8 @@
 	si2168_config.i2c_adapter = &i2c_adapter;
 	si2168_config.fe = &adap->fe[0];
 	si2168_config.ts_mode = SI2168_TS_PARALLEL;
+	if (le16_to_cpu(d->udev->descriptor.idProduct) == USB_PID_MYGICA_T230C2)
+		si2168_config.ts_mode |= SI2168_TS_CLK_MANUAL;
 	si2168_config.ts_clock_inv = 1;
 
 	state->i2c_client_demod = dvb_module_probe("si2168", NULL,
@@ -799,6 +801,9 @@
 	{ DVB_USB_DEVICE(USB_VID_CONEXANT, USB_PID_MYGICA_T230C,
 		&mygica_t230c_props, "MyGica Mini DVB-T2 USB Stick T230C",
 		RC_MAP_TOTAL_MEDIA_IN_HAND_02) },
+	{ DVB_USB_DEVICE(USB_VID_CONEXANT, USB_PID_MYGICA_T230C2,
+		&mygica_t230c_props, "MyGica Mini DVB-T2 USB Stick T230C v2",
+		RC_MAP_TOTAL_MEDIA_IN_HAND_02) },
 	{ }
 };
 MODULE_DEVICE_TABLE(usb, dvbsky_id_table);
diff -ru a/include/media/dvb-usb-ids.h b/include/media/dvb-usb-ids.h
--- a/include/media/dvb-usb-ids.h	2019-06-04 07:59:45.000000000 +0200
+++ b/include/media/dvb-usb-ids.h	2019-06-07 23:38:06.679688490 +0200
@@ -387,6 +387,7 @@
 #define USB_PID_MYGICA_D689				0xd811
 #define USB_PID_MYGICA_T230				0xc688
 #define USB_PID_MYGICA_T230C				0xc689
+#define USB_PID_MYGICA_T230C2				0xc68a
 #define USB_PID_ELGATO_EYETV_DIVERSITY			0x0011
 #define USB_PID_ELGATO_EYETV_DTT			0x0021
 #define USB_PID_ELGATO_EYETV_DTT_2			0x003f
Sean Young June 25, 2019, 11:16 a.m. UTC | #2 
On Sun, Jun 16, 2019 at 02:39:29AM +0200, Jan Pieter van Woerkom wrote:
> From: Jan Pieter van Woerkom <jp@jpvw.nl>
> 
> Adds support for the "Mygica T230C v2" into the "dvbsky" driver.
> A small enhancement is also needed in the si2168 demodulator
> driver, and a USB device ID in dvb-usb-ids.h .
> 
> This is v3.3 of the proposed patch, based on feedback from Sean
> Young and Antti Palosaari.
> Tested by patch author on a T230C v2.
> Tested by Frank Rysanek on a T230C v2: can tune into locally
> available DVB-T and DVB-T2 muxes, video and audio playback works.
> Applies cleanly against Linux 5.1.10 .
> 
> The T230C v2 hardware needs a mode of the si2168 chip to be
> set for which the si2168 driver previously had no support.
> This patch uses a specific measure to configure this on the
> T230C v2 hardware only - see the flag passed via the ts_mode
> attribute and its dependency on USB_PID_MYGICA_T230C2. Other
> devices using the si2168 demodulator driver are not affected
> in any way.
> 
> Signed-off-by: Jan Pieter van Woerkom <jp@jpvw.nl>
> Tested-by: Frank Rysanek <Frantisek.Rysanek@post.cz>
> ---
> diff -ru a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
> --- a/drivers/media/dvb-frontends/si2168.c	2019-06-04 07:59:45.000000000 +0200
> +++ b/drivers/media/dvb-frontends/si2168.c	2019-06-08 19:47:32.385526558 +0200
> @@ -91,8 +91,18 @@
>  
>  	dev_dbg(&client->dev, "%s acquire: %d\n", __func__, acquire);
>  
> +	/* set manual value */
> +	if (dev->ts_mode | SI2168_TS_CLK_MANUAL) {

This looks wrong. Should it not be "dev->ts_mode & SI2168_TS_CLK_MANUAL"?
Now the expression is always true.


> +		memcpy(cmd.args, "\x14\x00\x0d\x10\xe8\x03", 6);
> +		cmd.wlen = 6;
> +		cmd.rlen = 4;
> +		ret = si2168_cmd_execute(client, &cmd);
> +		if (ret)
> +			return ret;
> +		}
>  	/* set TS_MODE property */
> -	memcpy(cmd.args, "\x14\x00\x01\x10\x10\x00", 6);
> +	memcpy(cmd.args, "\x14\x00\x01\x10\x00\x00", 6);
> +	cmd.args[4] = dev->ts_mode & (SI2168_TS_CLK_AUTO|SI2168_TS_CLK_MANUAL);
>  	if (acquire)
>  		cmd.args[4] |= dev->ts_mode;
>  	else
> diff -ru a/drivers/media/dvb-frontends/si2168.h b/drivers/media/dvb-frontends/si2168.h
> --- a/drivers/media/dvb-frontends/si2168.h	2019-06-04 07:59:45.000000000 +0200
> +++ b/drivers/media/dvb-frontends/si2168.h	2019-06-08 19:32:52.400320490 +0200
> @@ -39,6 +39,8 @@
>  #define SI2168_TS_PARALLEL	0x06
>  #define SI2168_TS_SERIAL	0x03
>  #define SI2168_TS_TRISTATE	0x00
> +#define SI2168_TS_CLK_AUTO	0x10
> +#define SI2168_TS_CLK_MANUAL	0x20
>  	u8 ts_mode;
>  
>  	/* TS clock inverted */

Thanks,
Sean
Jan Pieter van Woerkom June 25, 2019, 3:41 p.m. UTC | #3 
On 6/25/19 1:16 PM, Sean Young wrote:
> On Sun, Jun 16, 2019 at 02:39:29AM +0200, Jan Pieter van Woerkom wrote:
>> From: Jan Pieter van Woerkom <jp@jpvw.nl>
>>
>> Adds support for the "Mygica T230C v2" into the "dvbsky" driver.
>> A small enhancement is also needed in the si2168 demodulator
>> driver, and a USB device ID in dvb-usb-ids.h .
>>
>> This is v3.3 of the proposed patch, based on feedback from Sean
>> Young and Antti Palosaari.
>> Tested by patch author on a T230C v2.
>> Tested by Frank Rysanek on a T230C v2: can tune into locally
>> available DVB-T and DVB-T2 muxes, video and audio playback works.
>> Applies cleanly against Linux 5.1.10 .
>>
>> The T230C v2 hardware needs a mode of the si2168 chip to be
>> set for which the si2168 driver previously had no support.
>> This patch uses a specific measure to configure this on the
>> T230C v2 hardware only - see the flag passed via the ts_mode
>> attribute and its dependency on USB_PID_MYGICA_T230C2. Other
>> devices using the si2168 demodulator driver are not affected
>> in any way.
>>
>> Signed-off-by: Jan Pieter van Woerkom <jp@jpvw.nl>
>> Tested-by: Frank Rysanek <Frantisek.Rysanek@post.cz>
>> ---
>> diff -ru a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
>> --- a/drivers/media/dvb-frontends/si2168.c	2019-06-04 07:59:45.000000000 +0200
>> +++ b/drivers/media/dvb-frontends/si2168.c	2019-06-08 19:47:32.385526558 +0200
>> @@ -91,8 +91,18 @@
>>   
>>   	dev_dbg(&client->dev, "%s acquire: %d\n", __func__, acquire);
>>   
>> +	/* set manual value */
>> +	if (dev->ts_mode | SI2168_TS_CLK_MANUAL) {
> This looks wrong. Should it not be "dev->ts_mode & SI2168_TS_CLK_MANUAL"?
> Now the expression is always true.
You're absolutely right. Silly me.

What now? Correct and repost?


>> +		memcpy(cmd.args, "\x14\x00\x0d\x10\xe8\x03", 6);
>> +		cmd.wlen = 6;
>> +		cmd.rlen = 4;
>> +		ret = si2168_cmd_execute(client, &cmd);
>> +		if (ret)
>> +			return ret;
>> +		}
>>   	/* set TS_MODE property */
>> -	memcpy(cmd.args, "\x14\x00\x01\x10\x10\x00", 6);
>> +	memcpy(cmd.args, "\x14\x00\x01\x10\x00\x00", 6);
>> +	cmd.args[4] = dev->ts_mode & (SI2168_TS_CLK_AUTO|SI2168_TS_CLK_MANUAL);
>>   	if (acquire)
>>   		cmd.args[4] |= dev->ts_mode;
>>   	else
>> diff -ru a/drivers/media/dvb-frontends/si2168.h b/drivers/media/dvb-frontends/si2168.h
>> --- a/drivers/media/dvb-frontends/si2168.h	2019-06-04 07:59:45.000000000 +0200
>> +++ b/drivers/media/dvb-frontends/si2168.h	2019-06-08 19:32:52.400320490 +0200
>> @@ -39,6 +39,8 @@
>>   #define SI2168_TS_PARALLEL	0x06
>>   #define SI2168_TS_SERIAL	0x03
>>   #define SI2168_TS_TRISTATE	0x00
>> +#define SI2168_TS_CLK_AUTO	0x10
>> +#define SI2168_TS_CLK_MANUAL	0x20
>>   	u8 ts_mode;
>>   
>>   	/* TS clock inverted */
> Thanks,
> Sean
>
Thank you,
Jan Pieter.
Antti Palosaari June 25, 2019, 3:54 p.m. UTC | #4 
On 6/25/19 6:41 PM, JP wrote:
> 
> 
> On 6/25/19 1:16 PM, Sean Young wrote:
>> On Sun, Jun 16, 2019 at 02:39:29AM +0200, Jan Pieter van Woerkom wrote:
>>> From: Jan Pieter van Woerkom <jp@jpvw.nl>
>>>
>>> Adds support for the "Mygica T230C v2" into the "dvbsky" driver.
>>> A small enhancement is also needed in the si2168 demodulator
>>> driver, and a USB device ID in dvb-usb-ids.h .
>>>
>>> This is v3.3 of the proposed patch, based on feedback from Sean
>>> Young and Antti Palosaari.
>>> Tested by patch author on a T230C v2.
>>> Tested by Frank Rysanek on a T230C v2: can tune into locally
>>> available DVB-T and DVB-T2 muxes, video and audio playback works.
>>> Applies cleanly against Linux 5.1.10 .
>>>
>>> The T230C v2 hardware needs a mode of the si2168 chip to be
>>> set for which the si2168 driver previously had no support.
>>> This patch uses a specific measure to configure this on the
>>> T230C v2 hardware only - see the flag passed via the ts_mode
>>> attribute and its dependency on USB_PID_MYGICA_T230C2. Other
>>> devices using the si2168 demodulator driver are not affected
>>> in any way.
>>>
>>> Signed-off-by: Jan Pieter van Woerkom <jp@jpvw.nl>
>>> Tested-by: Frank Rysanek <Frantisek.Rysanek@post.cz>
>>> ---
>>> diff -ru a/drivers/media/dvb-frontends/si2168.c 
>>> b/drivers/media/dvb-frontends/si2168.c
>>> --- a/drivers/media/dvb-frontends/si2168.c    2019-06-04 
>>> 07:59:45.000000000 +0200
>>> +++ b/drivers/media/dvb-frontends/si2168.c    2019-06-08 
>>> 19:47:32.385526558 +0200
>>> @@ -91,8 +91,18 @@
>>>       dev_dbg(&client->dev, "%s acquire: %d\n", __func__, acquire);
>>> +    /* set manual value */
>>> +    if (dev->ts_mode | SI2168_TS_CLK_MANUAL) {
>> This looks wrong. Should it not be "dev->ts_mode & SI2168_TS_CLK_MANUAL"?
>> Now the expression is always true.
> You're absolutely right. Silly me.
> 
> What now? Correct and repost?

yes, and next indentation looks also wrong

> 
> 
>>> +        memcpy(cmd.args, "\x14\x00\x0d\x10\xe8\x03", 6);
>>> +        cmd.wlen = 6;
>>> +        cmd.rlen = 4;
>>> +        ret = si2168_cmd_execute(client, &cmd);
>>> +        if (ret)
>>> +            return ret;
>>> +        }
>>>       /* set TS_MODE property */
>>> -    memcpy(cmd.args, "\x14\x00\x01\x10\x10\x00", 6);
>>> +    memcpy(cmd.args, "\x14\x00\x01\x10\x00\x00", 6);
>>> +    cmd.args[4] = dev->ts_mode & 
>>> (SI2168_TS_CLK_AUTO|SI2168_TS_CLK_MANUAL);
>>>       if (acquire)
>>>           cmd.args[4] |= dev->ts_mode;
>>>       else
>>> diff -ru a/drivers/media/dvb-frontends/si2168.h 
>>> b/drivers/media/dvb-frontends/si2168.h
>>> --- a/drivers/media/dvb-frontends/si2168.h    2019-06-04 
>>> 07:59:45.000000000 +0200
>>> +++ b/drivers/media/dvb-frontends/si2168.h    2019-06-08 
>>> 19:32:52.400320490 +0200
>>> @@ -39,6 +39,8 @@
>>>   #define SI2168_TS_PARALLEL    0x06
>>>   #define SI2168_TS_SERIAL    0x03
>>>   #define SI2168_TS_TRISTATE    0x00
>>> +#define SI2168_TS_CLK_AUTO    0x10
>>> +#define SI2168_TS_CLK_MANUAL    0x20
>>>       u8 ts_mode;
>>>       /* TS clock inverted */
>> Thanks,
>> Sean
>>
> Thank you,
> Jan Pieter.

regards
Antti
Sean Young July 8, 2019, 10:04 a.m. UTC | #5 
Hallo Jan-Pieter,

> On 6/25/19 1:16 PM, Sean Young wrote:
> > On Sun, Jun 16, 2019 at 02:39:29AM +0200, Jan Pieter van Woerkom wrote:
> > > From: Jan Pieter van Woerkom <jp@jpvw.nl>
> > > 
> > > Adds support for the "Mygica T230C v2" into the "dvbsky" driver.
> > > A small enhancement is also needed in the si2168 demodulator
> > > driver, and a USB device ID in dvb-usb-ids.h .
> > > 
> > > This is v3.3 of the proposed patch, based on feedback from Sean
> > > Young and Antti Palosaari.
> > > Tested by patch author on a T230C v2.
> > > Tested by Frank Rysanek on a T230C v2: can tune into locally
> > > available DVB-T and DVB-T2 muxes, video and audio playback works.
> > > Applies cleanly against Linux 5.1.10 .
> > > 
> > > The T230C v2 hardware needs a mode of the si2168 chip to be
> > > set for which the si2168 driver previously had no support.
> > > This patch uses a specific measure to configure this on the
> > > T230C v2 hardware only - see the flag passed via the ts_mode
> > > attribute and its dependency on USB_PID_MYGICA_T230C2. Other
> > > devices using the si2168 demodulator driver are not affected
> > > in any way.
> > > 
> > > Signed-off-by: Jan Pieter van Woerkom <jp@jpvw.nl>
> > > Tested-by: Frank Rysanek <Frantisek.Rysanek@post.cz>
> > > ---
> > > diff -ru a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
> > > --- a/drivers/media/dvb-frontends/si2168.c	2019-06-04 07:59:45.000000000 +0200
> > > +++ b/drivers/media/dvb-frontends/si2168.c	2019-06-08 19:47:32.385526558 +0200
> > > @@ -91,8 +91,18 @@
> > >   	dev_dbg(&client->dev, "%s acquire: %d\n", __func__, acquire);
> > > +	/* set manual value */
> > > +	if (dev->ts_mode | SI2168_TS_CLK_MANUAL) {
> > This looks wrong. Should it not be "dev->ts_mode & SI2168_TS_CLK_MANUAL"?
> > Now the expression is always true.
> You're absolutely right. Silly me.
> 
> What now? Correct and repost?

Yes, please. I don't have the hardware to test so it would be great if
you could repost a tested version, so we can merged that.

Thanks,

Sean
Jan Pieter van Woerkom July 8, 2019, 11:08 p.m. UTC | #6 
On 7/8/19 12:04 PM, Sean Young wrote:
> Hallo Jan-Pieter,
>
>> On 6/25/19 1:16 PM, Sean Young wrote:
>>> On Sun, Jun 16, 2019 at 02:39:29AM +0200, Jan Pieter van Woerkom wrote:
>>>> From: Jan Pieter van Woerkom <jp@jpvw.nl>
>>>>
>>>> Adds support for the "Mygica T230C v2" into the "dvbsky" driver.
>>>> A small enhancement is also needed in the si2168 demodulator
>>>> driver, and a USB device ID in dvb-usb-ids.h .
>>>>
>>>> This is v3.3 of the proposed patch, based on feedback from Sean
>>>> Young and Antti Palosaari.
>>>> Tested by patch author on a T230C v2.
>>>> Tested by Frank Rysanek on a T230C v2: can tune into locally
>>>> available DVB-T and DVB-T2 muxes, video and audio playback works.
>>>> Applies cleanly against Linux 5.1.10 .
>>>>
>>>> The T230C v2 hardware needs a mode of the si2168 chip to be
>>>> set for which the si2168 driver previously had no support.
>>>> This patch uses a specific measure to configure this on the
>>>> T230C v2 hardware only - see the flag passed via the ts_mode
>>>> attribute and its dependency on USB_PID_MYGICA_T230C2. Other
>>>> devices using the si2168 demodulator driver are not affected
>>>> in any way.
>>>>
>>>> Signed-off-by: Jan Pieter van Woerkom <jp@jpvw.nl>
>>>> Tested-by: Frank Rysanek <Frantisek.Rysanek@post.cz>
>>>> ---
>>>> diff -ru a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
>>>> --- a/drivers/media/dvb-frontends/si2168.c	2019-06-04 07:59:45.000000000 +0200
>>>> +++ b/drivers/media/dvb-frontends/si2168.c	2019-06-08 19:47:32.385526558 +0200
>>>> @@ -91,8 +91,18 @@
>>>>    	dev_dbg(&client->dev, "%s acquire: %d\n", __func__, acquire);
>>>> +	/* set manual value */
>>>> +	if (dev->ts_mode | SI2168_TS_CLK_MANUAL) {
>>> This looks wrong. Should it not be "dev->ts_mode & SI2168_TS_CLK_MANUAL"?
>>> Now the expression is always true.
>> You're absolutely right. Silly me.
>>
>> What now? Correct and repost?
> Yes, please. I don't have the hardware to test so it would be great if
> you could repost a tested version, so we can merged that.
Done,  sent as new post to the linux-media list. I messed up one
subject line, 2/1 instead of 1/2.

Also, I made a another change (as per Antti's recommendation):
instead of piggybacking the magic value on the ts_mode variable,
define own boolean flag and use that when this special si2168
mode is needed.

> Thanks,
>
> Sean
>
Thank you,

Jan Pieter
diff mbox series

Patch

diff -ru a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
--- a/drivers/media/dvb-frontends/si2168.c	2019-06-04 07:59:45.000000000 +0200
+++ b/drivers/media/dvb-frontends/si2168.c	2019-06-08 19:47:32.385526558 +0200
@@ -91,8 +91,18 @@ 
 
 	dev_dbg(&client->dev, "%s acquire: %d\n", __func__, acquire);
 
+	/* set manual value */
+	if (dev->ts_mode | SI2168_TS_CLK_MANUAL) {
+		memcpy(cmd.args, "\x14\x00\x0d\x10\xe8\x03", 6);
+		cmd.wlen = 6;
+		cmd.rlen = 4;
+		ret = si2168_cmd_execute(client, &cmd);
+		if (ret)
+			return ret;
+		}
 	/* set TS_MODE property */
-	memcpy(cmd.args, "\x14\x00\x01\x10\x10\x00", 6);
+	memcpy(cmd.args, "\x14\x00\x01\x10\x00\x00", 6);
+	cmd.args[4] = dev->ts_mode & (SI2168_TS_CLK_AUTO|SI2168_TS_CLK_MANUAL);
 	if (acquire)
 		cmd.args[4] |= dev->ts_mode;
 	else
diff -ru a/drivers/media/dvb-frontends/si2168.h b/drivers/media/dvb-frontends/si2168.h
--- a/drivers/media/dvb-frontends/si2168.h	2019-06-04 07:59:45.000000000 +0200
+++ b/drivers/media/dvb-frontends/si2168.h	2019-06-08 19:32:52.400320490 +0200
@@ -39,6 +39,8 @@ 
 #define SI2168_TS_PARALLEL	0x06
 #define SI2168_TS_SERIAL	0x03
 #define SI2168_TS_TRISTATE	0x00
+#define SI2168_TS_CLK_AUTO	0x10
+#define SI2168_TS_CLK_MANUAL	0x20
 	u8 ts_mode;
 
 	/* TS clock inverted */