[PATCH 1/3] rtl2832: add support for slave ts pid filter

[PATCH 1/3] rtl2832: add support for slave ts pid filter

[Benjamin Larsson]

Signed-off-by: Benjamin Larsson <benjamin@southpole.se>
---
 drivers/media/dvb-frontends/rtl2832.c      | 21 ++++++++++++++++++---
 drivers/media/dvb-frontends/rtl2832_priv.h |  1 +
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/media/dvb-frontends/rtl2832.c b/drivers/media/dvb-frontends/rtl2832.c
index 78b87b2..e054079 100644
--- a/drivers/media/dvb-frontends/rtl2832.c
+++ b/drivers/media/dvb-frontends/rtl2832.c
@@ -407,6 +407,7 @@ static int rtl2832_init(struct dvb_frontend *fe)
 	/* start statistics polling */
 	schedule_delayed_work(&dev->stat_work, msecs_to_jiffies(2000));
 	dev->sleeping = false;
+	dev->slave_ts = false;
 
 	return 0;
 err:
@@ -1122,6 +1123,8 @@ static int rtl2832_enable_slave_ts(struct i2c_client *client)
 	if (ret)
 		goto err;
 
+	dev->slave_ts = true;
+
 	return 0;
 err:
 	dev_dbg(&client->dev, "failed=%d\n", ret);
@@ -1143,7 +1146,11 @@ static int rtl2832_pid_filter_ctrl(struct dvb_frontend *fe, int onoff)
 	else
 		u8tmp = 0x00;
 
-	ret = rtl2832_update_bits(client, 0x061, 0xc0, u8tmp);
+	if (dev->slave_ts)
+		ret = rtl2832_update_bits(client, 0x021, 0xc0, u8tmp);
+	else
+		ret = rtl2832_update_bits(client, 0x061, 0xc0, u8tmp);
 	if (ret)
 		goto err;
 
@@ -1178,14 +1185,22 @@ static int rtl2832_pid_filter(struct dvb_frontend *fe, u8 index, u16 pid,
 	buf[1] = (dev->filters >>  8) & 0xff;
 	buf[2] = (dev->filters >> 16) & 0xff;
 	buf[3] = (dev->filters >> 24) & 0xff;
-	ret = rtl2832_bulk_write(client, 0x062, buf, 4);
+
+	if (dev->slave_ts)
+		ret = rtl2832_bulk_write(client, 0x022, buf, 4);
+	else
+		ret = rtl2832_bulk_write(client, 0x062, buf, 4);
 	if (ret)
 		goto err;
 
 	/* add PID */
 	buf[0] = (pid >> 8) & 0xff;
 	buf[1] = (pid >> 0) & 0xff;
-	ret = rtl2832_bulk_write(client, 0x066 + 2 * index, buf, 2);
+
+	if (dev->slave_ts)
+		ret = rtl2832_bulk_write(client, 0x026 + 2 * index, buf, 2);
+	else
+		ret = rtl2832_bulk_write(client, 0x066 + 2 * index, buf, 2);
 	if (ret)
 		goto err;
 
diff --git a/drivers/media/dvb-frontends/rtl2832_priv.h b/drivers/media/dvb-frontends/rtl2832_priv.h
index 5dcd3a4..efc230f 100644
--- a/drivers/media/dvb-frontends/rtl2832_priv.h
+++ b/drivers/media/dvb-frontends/rtl2832_priv.h
@@ -46,6 +46,7 @@ struct rtl2832_dev {
 	bool sleeping;
 	struct delayed_work i2c_gate_work;
 	unsigned long filters; /* PID filter */
+	bool slave_ts;
 };
 
 struct rtl2832_reg_entry {
-- 
2.1.4

[PATCH 2/3] mn88472: add work around for failing firmware loading

[Benjamin Larsson]

Sometimes the firmware fails to load because of an i2c error.
Work around that by adding retry logic. This kind of logic
also exist in the binary driver and failures have been observed
there also. Thus this seems to be a property of the hardware
and a fix like this is needed.

Signed-off-by: Benjamin Larsson <benjamin@southpole.se>
---
 drivers/staging/media/mn88472/mn88472.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/mn88472/mn88472.c b/drivers/staging/media/mn88472/mn88472.c
index cf2e96b..80c5807 100644
--- a/drivers/staging/media/mn88472/mn88472.c
+++ b/drivers/staging/media/mn88472/mn88472.c
@@ -282,7 +282,7 @@ static int mn88472_init(struct dvb_frontend *fe)
 	int ret, len, remaining;
 	const struct firmware *fw = NULL;
 	u8 *fw_file = MN88472_FIRMWARE;
-	unsigned int tmp;
+	unsigned int tmp, retry_cnt;
 
 	dev_dbg(&client->dev, "\n");
 
@@ -330,9 +330,22 @@ static int mn88472_init(struct dvb_frontend *fe)
 		if (len > (dev->i2c_wr_max - 1))
 			len = dev->i2c_wr_max - 1;
 
+		/* I2C transfers during firmware load might fail sometimes,
+		 * just retry in that case. 4 consecutive failures have
+		 * been observed thus a retry limit of 20 is used.
+		 */
+		retry_cnt = 20;
+retry:
 		ret = regmap_bulk_write(dev->regmap[0], 0xf6,
 				&fw->data[fw->size - remaining], len);
 		if (ret) {
+			if (retry_cnt) {
+				dev_dbg(&client->dev,
+				"i2c error, retry %d triggered\n", retry_cnt);
+				retry_cnt--;
+				usleep_range(200, 10000);
+				goto retry;
+			}
 			dev_err(&client->dev,
 					"firmware download failed=%d\n", ret);
 			goto firmware_release;
-- 
2.1.4

Re: [PATCH 2/3] mn88472: add work around for failing firmware loading

[Antti Palosaari]

Hello
I am not sure if problem is I2C adapter/bus or that demodulator I2C 
slave. If it is demod, then that workaround is correct place, but if it 
is not, then that is wrong and I2C adapter repeating logic should be used.

I did some testing again... Loading mn88472 firmware 1000 times, it failed:
61 times RC polling disabled
68 times RC polling enabled
83 times RC polling enabled, but repeated failed message due to that patch

I don't want apply that patch until I find some time myself to examine 
that problem - or someone else does some study to point out whats wrong. 
There is many things to test in order to get better understanding.

regards
Antti

On 11/29/2015 04:10 AM, Benjamin Larsson wrote:
> Sometimes the firmware fails to load because of an i2c error.
> Work around that by adding retry logic. This kind of logic
> also exist in the binary driver and failures have been observed
> there also. Thus this seems to be a property of the hardware
> and a fix like this is needed.
>
> Signed-off-by: Benjamin Larsson <benjamin@southpole.se>
> ---
>   drivers/staging/media/mn88472/mn88472.c | 15 ++++++++++++++-
>   1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/mn88472/mn88472.c b/drivers/staging/media/mn88472/mn88472.c
> index cf2e96b..80c5807 100644
> --- a/drivers/staging/media/mn88472/mn88472.c
> +++ b/drivers/staging/media/mn88472/mn88472.c
> @@ -282,7 +282,7 @@ static int mn88472_init(struct dvb_frontend *fe)
>   	int ret, len, remaining;
>   	const struct firmware *fw = NULL;
>   	u8 *fw_file = MN88472_FIRMWARE;
> -	unsigned int tmp;
> +	unsigned int tmp, retry_cnt;
>
>   	dev_dbg(&client->dev, "\n");
>
> @@ -330,9 +330,22 @@ static int mn88472_init(struct dvb_frontend *fe)
>   		if (len > (dev->i2c_wr_max - 1))
>   			len = dev->i2c_wr_max - 1;
>
> +		/* I2C transfers during firmware load might fail sometimes,
> +		 * just retry in that case. 4 consecutive failures have
> +		 * been observed thus a retry limit of 20 is used.
> +		 */
> +		retry_cnt = 20;
> +retry:
>   		ret = regmap_bulk_write(dev->regmap[0], 0xf6,
>   				&fw->data[fw->size - remaining], len);
>   		if (ret) {
> +			if (retry_cnt) {
> +				dev_dbg(&client->dev,
> +				"i2c error, retry %d triggered\n", retry_cnt);
> +				retry_cnt--;
> +				usleep_range(200, 10000);
> +				goto retry;
> +			}
>   			dev_err(&client->dev,
>   					"firmware download failed=%d\n", ret);
>   			goto firmware_release;
>

-- 
http://palosaari.fi/

Re: [PATCH 2/3] mn88472: add work around for failing firmware loading

[Benjamin Larsson]

On 12/21/2015 03:52 AM, Antti Palosaari wrote:
> Hello
> I am not sure if problem is I2C adapter/bus or that demodulator I2C
> slave. If it is demod, then that workaround is correct place, but if it
> is not, then that is wrong and I2C adapter repeating logic should be used.
>
> I did some testing again... Loading mn88472 firmware 1000 times, it failed:
> 61 times RC polling disabled
> 68 times RC polling enabled
> 83 times RC polling enabled, but repeated failed message due to that patch

At least this confirms there is an issue.

>
> I don't want apply that patch until I find some time myself to examine
> that problem - or someone else does some study to point out whats wrong.
> There is many things to test in order to get better understanding.
>
> regards
> Antti

I do have other hardware with with a mn88472 demod on it. A CX23102 
bridge and a dibcom (Xbox one tuner). I think that running the same test 
on those hardware will tell where the issue is.

I know that Olli have worked on the Xbox one tuner, do you have any 
support patches that could help testing this? And did you observe any 
issues with the mn88472 demod when working on the Xbox one tuner ?

I am quite sure that I saw this on the mn88473 version of this hardware 
also. I just haven't had the time to test it. But I will postpone that 
until the tests on the other bridges are done.

MvH
Benjamin Larsson

[PATCH 3/3] rtl28xxu: change Astrometa DVB-T2 to always use hardware pid filters

[Benjamin Larsson]

Signed-off-by: Benjamin Larsson <benjamin@southpole.se>
---
 drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 35 ++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
index 5a503a6..74201ec 100644
--- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
+++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
@@ -1848,6 +1848,39 @@ static const struct dvb_usb_device_properties rtl28xxu_props = {
 	},
 };
 
+static const struct dvb_usb_device_properties rtl28xxp_props = {
+	.driver_name = KBUILD_MODNAME,
+	.owner = THIS_MODULE,
+	.adapter_nr = adapter_nr,
+	.size_of_priv = sizeof(struct rtl28xxu_dev),
+
+	.identify_state = rtl28xxu_identify_state,
+	.power_ctrl = rtl28xxu_power_ctrl,
+	.frontend_ctrl = rtl28xxu_frontend_ctrl,
+	.i2c_algo = &rtl28xxu_i2c_algo,
+	.read_config = rtl28xxu_read_config,
+	.frontend_attach = rtl28xxu_frontend_attach,
+	.frontend_detach = rtl28xxu_frontend_detach,
+	.tuner_attach = rtl28xxu_tuner_attach,
+	.tuner_detach = rtl28xxu_tuner_detach,
+	.init = rtl28xxu_init,
+
+	.get_rc_config = rtl28xxu_get_rc_config,
+	.num_adapters = 1,
+	.adapter = {
+		{
+			.caps = DVB_USB_ADAP_NEED_PID_FILTERING |
+				DVB_USB_ADAP_PID_FILTER_CAN_BE_TURNED_OFF,
+
+			.pid_filter_count = 32,
+			.pid_filter_ctrl = rtl28xxu_pid_filter_ctrl,
+			.pid_filter = rtl28xxu_pid_filter,
+
+			.stream = DVB_USB_STREAM_BULK(0x81, 6, 8 * 512),
+		},
+	},
+};
+
 static const struct usb_device_id rtl28xxu_id_table[] = {
 	/* RTL2831U devices: */
 	{ DVB_USB_DEVICE(USB_VID_REALTEK, USB_PID_REALTEK_RTL2831U,
@@ -1919,7 +1952,7 @@ static const struct usb_device_id rtl28xxu_id_table[] = {
 
 	/* RTL2832P devices: */
 	{ DVB_USB_DEVICE(USB_VID_HANFTEK, 0x0131,
-		&rtl28xxu_props, "Astrometa DVB-T2", NULL) },
+		&rtl28xxp_props, "Astrometa DVB-T2", NULL) },
 	{ DVB_USB_DEVICE(0x5654, 0xca42,
 		&rtl28xxu_props, "GoTView MasterHD 3", NULL) },
 	{ }
-- 
2.1.4

Re: [PATCH 3/3] rtl28xxu: change Astrometa DVB-T2 to always use hardware pid filters

[Antti Palosaari]

Moikka!
I did some testing and I cannot see reason to force hw pid filter for 
that device. I assume you somehow think it does not work without 
filtering, but I think it does.

I tested streaming with mn88472 demod DVB-C and DVB-T2 modes without 
stream errors. DVB-T2 (live) datarate 45Mbps and DVB-C (modulator) 
datarate 50Mbps. Maximum DVB-T2 (8MHz) datarate is 50Mbps - in a real 
life it is bit less.

DVB-C
1fff 19870.47 p/s 29184.8 Kbps 207966 KB
TOT 34313.26 p/s 50397.6 Kbps 359127 KB

DVB-T2
1fff 2589.99 p/s 3804.0 Kbps 21400 KB
TOT 30346.18 p/s 44570.9 Kbps 250745 KB

So point me the reason hw PID filters need to be forced.

regards
Antti


On 11/29/2015 04:10 AM, Benjamin Larsson wrote:
> Signed-off-by: Benjamin Larsson <benjamin@southpole.se>
> ---
>   drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 35 ++++++++++++++++++++++++++++++++-
>   1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
> index 5a503a6..74201ec 100644
> --- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
> +++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
> @@ -1848,6 +1848,39 @@ static const struct dvb_usb_device_properties rtl28xxu_props = {
>   	},
>   };
>
> +static const struct dvb_usb_device_properties rtl28xxp_props = {
> +	.driver_name = KBUILD_MODNAME,
> +	.owner = THIS_MODULE,
> +	.adapter_nr = adapter_nr,
> +	.size_of_priv = sizeof(struct rtl28xxu_dev),
> +
> +	.identify_state = rtl28xxu_identify_state,
> +	.power_ctrl = rtl28xxu_power_ctrl,
> +	.frontend_ctrl = rtl28xxu_frontend_ctrl,
> +	.i2c_algo = &rtl28xxu_i2c_algo,
> +	.read_config = rtl28xxu_read_config,
> +	.frontend_attach = rtl28xxu_frontend_attach,
> +	.frontend_detach = rtl28xxu_frontend_detach,
> +	.tuner_attach = rtl28xxu_tuner_attach,
> +	.tuner_detach = rtl28xxu_tuner_detach,
> +	.init = rtl28xxu_init,
> +
> +	.get_rc_config = rtl28xxu_get_rc_config,
> +	.num_adapters = 1,
> +	.adapter = {
> +		{
> +			.caps = DVB_USB_ADAP_NEED_PID_FILTERING |
> +				DVB_USB_ADAP_PID_FILTER_CAN_BE_TURNED_OFF,
> +
> +			.pid_filter_count = 32,
> +			.pid_filter_ctrl = rtl28xxu_pid_filter_ctrl,
> +			.pid_filter = rtl28xxu_pid_filter,
> +
> +			.stream = DVB_USB_STREAM_BULK(0x81, 6, 8 * 512),
> +		},
> +	},
> +};
> +
>   static const struct usb_device_id rtl28xxu_id_table[] = {
>   	/* RTL2831U devices: */
>   	{ DVB_USB_DEVICE(USB_VID_REALTEK, USB_PID_REALTEK_RTL2831U,
> @@ -1919,7 +1952,7 @@ static const struct usb_device_id rtl28xxu_id_table[] = {
>
>   	/* RTL2832P devices: */
>   	{ DVB_USB_DEVICE(USB_VID_HANFTEK, 0x0131,
> -		&rtl28xxu_props, "Astrometa DVB-T2", NULL) },
> +		&rtl28xxp_props, "Astrometa DVB-T2", NULL) },
>   	{ DVB_USB_DEVICE(0x5654, 0xca42,
>   		&rtl28xxu_props, "GoTView MasterHD 3", NULL) },
>   	{ }
>

-- 
http://palosaari.fi/

Re: [PATCH 3/3] rtl28xxu: change Astrometa DVB-T2 to always use hardware pid filters

[Benjamin Larsson]

On 12/20/2015 04:36 AM, Antti Palosaari wrote:
> Moikka!
> I did some testing and I cannot see reason to force hw pid filter for
> that device. I assume you somehow think it does not work without
> filtering, but I think it does.
>

[... proof that I was wrong ...]

>
> So point me the reason hw PID filters need to be forced.

The only reason would be that I suspected that high rates would not be 
possible over the bridge and that this was the default mode for the 
binary driver but as you proved that the bridge is able to support the 
rate this patch can be nacked.

>
> regards
> Antti

MvH
Benjamin Larsson

Re: [PATCH 1/3] rtl2832: add support for slave ts pid filter

[Antti Palosaari]

Patch looks acceptable, but it is broken in a mean it does not apply :(

$ wget -O - https://patchwork.linuxtv.org/patch/32030/mbox/ | git am -3 -s
--2015-12-21 04:40:46--  https://patchwork.linuxtv.org/patch/32030/mbox/
Resolving patchwork.linuxtv.org (patchwork.linuxtv.org)... 130.149.80.248
Connecting to patchwork.linuxtv.org 
(patchwork.linuxtv.org)|130.149.80.248|:443... connected.
HTTP request sent, awaiting response... 200 OK
Length: unspecified [text/plain]
Saving to: ‘STDOUT’

-                                                   [ <=> 
 
               ]   2.73K  --.-KB/s   in 0s

2015-12-21 04:40:46 (60.4 MB/s) - written to stdout [2796]

Applying: rtl2832: add support for slave ts pid filter
fatal: corrupt patch at line 39
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 rtl2832: add support for slave ts pid filter
The copy of the patch that failed is found in:
    /home/crope/linuxtv/code/media_tree/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
[crope@localhost media_tree]$ patch -p1 < .git/rebase-apply/patch
patching file drivers/media/dvb-frontends/rtl2832.c
patch: **** malformed patch at line 39: @@ -1178,14 +1185,22 @@ static 
int rtl2832_pid_filter(struct dvb_frontend *fe, u8 index, u16 pid,

[crope@localhost media_tree]$ git am --abort
[crope@localhost media_tree]$ git am ~/\[PATCH\ 1_3\]\ rtl2832\:\ add\ 
support\ for\ slave\ ts\ pid\ filter.eml
Applying: rtl2832: add support for slave ts pid filter
fatal: corrupt patch at line 39
Patch failed at 0001 rtl2832: add support for slave ts pid filter
The copy of the patch that failed is found in:
    /home/crope/linuxtv/code/media_tree/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
[crope@localhost media_tree]$ git am --abort

Antti

On 11/29/2015 04:10 AM, Benjamin Larsson wrote:
> Signed-off-by: Benjamin Larsson <benjamin@southpole.se>
> ---
>   drivers/media/dvb-frontends/rtl2832.c      | 21 ++++++++++++++++++---
>   drivers/media/dvb-frontends/rtl2832_priv.h |  1 +
>   2 files changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/dvb-frontends/rtl2832.c b/drivers/media/dvb-frontends/rtl2832.c
> index 78b87b2..e054079 100644
> --- a/drivers/media/dvb-frontends/rtl2832.c
> +++ b/drivers/media/dvb-frontends/rtl2832.c
> @@ -407,6 +407,7 @@ static int rtl2832_init(struct dvb_frontend *fe)
>   	/* start statistics polling */
>   	schedule_delayed_work(&dev->stat_work, msecs_to_jiffies(2000));
>   	dev->sleeping = false;
> +	dev->slave_ts = false;
>
>   	return 0;
>   err:
> @@ -1122,6 +1123,8 @@ static int rtl2832_enable_slave_ts(struct i2c_client *client)
>   	if (ret)
>   		goto err;
>
> +	dev->slave_ts = true;
> +
>   	return 0;
>   err:
>   	dev_dbg(&client->dev, "failed=%d\n", ret);
> @@ -1143,7 +1146,11 @@ static int rtl2832_pid_filter_ctrl(struct dvb_frontend *fe, int onoff)
>   	else
>   		u8tmp = 0x00;
>
> -	ret = rtl2832_update_bits(client, 0x061, 0xc0, u8tmp);
> +	if (dev->slave_ts)
> +		ret = rtl2832_update_bits(client, 0x021, 0xc0, u8tmp);
> +	else
> +		ret = rtl2832_update_bits(client, 0x061, 0xc0, u8tmp);
>   	if (ret)
>   		goto err;
>
> @@ -1178,14 +1185,22 @@ static int rtl2832_pid_filter(struct dvb_frontend *fe, u8 index, u16 pid,
>   	buf[1] = (dev->filters >>  8) & 0xff;
>   	buf[2] = (dev->filters >> 16) & 0xff;
>   	buf[3] = (dev->filters >> 24) & 0xff;
> -	ret = rtl2832_bulk_write(client, 0x062, buf, 4);
> +
> +	if (dev->slave_ts)
> +		ret = rtl2832_bulk_write(client, 0x022, buf, 4);
> +	else
> +		ret = rtl2832_bulk_write(client, 0x062, buf, 4);
>   	if (ret)
>   		goto err;
>
>   	/* add PID */
>   	buf[0] = (pid >> 8) & 0xff;
>   	buf[1] = (pid >> 0) & 0xff;
> -	ret = rtl2832_bulk_write(client, 0x066 + 2 * index, buf, 2);
> +
> +	if (dev->slave_ts)
> +		ret = rtl2832_bulk_write(client, 0x026 + 2 * index, buf, 2);
> +	else
> +		ret = rtl2832_bulk_write(client, 0x066 + 2 * index, buf, 2);
>   	if (ret)
>   		goto err;
>
> diff --git a/drivers/media/dvb-frontends/rtl2832_priv.h b/drivers/media/dvb-frontends/rtl2832_priv.h
> index 5dcd3a4..efc230f 100644
> --- a/drivers/media/dvb-frontends/rtl2832_priv.h
> +++ b/drivers/media/dvb-frontends/rtl2832_priv.h
> @@ -46,6 +46,7 @@ struct rtl2832_dev {
>   	bool sleeping;
>   	struct delayed_work i2c_gate_work;
>   	unsigned long filters; /* PID filter */
> +	bool slave_ts;
>   };
>
>   struct rtl2832_reg_entry {
>

-- 
http://palosaari.fi/

Re: [PATCH 1/3] rtl2832: add support for slave ts pid filter

[Benjamin Larsson]

On 12/21/2015 03:42 AM, Antti Palosaari wrote:
> Patch looks acceptable, but it is broken in a mean it does not apply :(
>

I'll rebase on git://linuxtv.org/anttip/media_tree.git rtl2832u_pull.

MvH
Benjamin Larsson