[PATCH] [media] drxk: Switch the delivery system on FE_SET_PROPERTY

[PATCH] [media] drxk: Switch the delivery system on FE_SET_PROPERTY

[Mauro Carvalho Chehab]

The DRX-K doesn't change the delivery system at set_properties,
but do it at frontend init. This causes problems on programs like
w_scan that, by default, opens both frontends.

Instead, explicitly set the format when set_parameters callback is
called.

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
 drivers/media/dvb/frontends/drxk_hard.c |   32 ++++++++++++++++++++++--------
 drivers/media/dvb/frontends/drxk_hard.h |    2 +
 2 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/drivers/media/dvb/frontends/drxk_hard.c b/drivers/media/dvb/frontends/drxk_hard.c
index 95cbc98..c8e0921 100644
--- a/drivers/media/dvb/frontends/drxk_hard.c
+++ b/drivers/media/dvb/frontends/drxk_hard.c
@@ -1847,6 +1847,7 @@ static int SetOperationMode(struct drxk_state *state,
 		*/
 	switch (oMode) {
 	case OM_DVBT:
+		dprintk(1, ": DVB-T\n");
 		state->m_OperationMode = oMode;
 		status = SetDVBTStandard(state, oMode);
 		if (status < 0)
@@ -1854,6 +1855,8 @@ static int SetOperationMode(struct drxk_state *state,
 		break;
 	case OM_QAM_ITU_A:	/* fallthrough */
 	case OM_QAM_ITU_C:
+		dprintk(1, ": DVB-C Annex %c\n",
+			(state->m_OperationMode == OM_QAM_ITU_A) ? 'A' : 'C');
 		state->m_OperationMode = oMode;
 		status = SetQAMStandard(state, oMode);
 		if (status < 0)
@@ -6183,7 +6186,10 @@ static int drxk_c_init(struct dvb_frontend *fe)
 	dprintk(1, "\n");
 	if (mutex_trylock(&state->ctlock) == 0)
 		return -EBUSY;
-	SetOperationMode(state, OM_QAM_ITU_A);
+	if (state->m_itut_annex_c)
+		SetOperationMode(state, OM_QAM_ITU_C);
+	else
+		SetOperationMode(state, OM_QAM_ITU_A);
 	return 0;
 }
 
@@ -6219,14 +6225,6 @@ static int drxk_set_parameters(struct dvb_frontend *fe,
 		return -EINVAL;
 	}
 
-	if (state->m_OperationMode == OM_QAM_ITU_A ||
-	    state->m_OperationMode == OM_QAM_ITU_C) {
-		if (fe->dtv_property_cache.rolloff == ROLLOFF_13)
-			state->m_OperationMode = OM_QAM_ITU_C;
-		else
-			state->m_OperationMode = OM_QAM_ITU_A;
-	}
-
 	if (fe->ops.i2c_gate_ctrl)
 		fe->ops.i2c_gate_ctrl(fe, 1);
 	if (fe->ops.tuner_ops.set_params)
@@ -6235,6 +6233,22 @@ static int drxk_set_parameters(struct dvb_frontend *fe,
 		fe->ops.i2c_gate_ctrl(fe, 0);
 	state->param = *p;
 	fe->ops.tuner_ops.get_if_frequency(fe, &IF);
+
+	/*
+	 * Make sure that the frontend is on the right state
+	 */
+
+	if (fe->ops.info.type == FE_QAM) {
+		if (fe->dtv_property_cache.rolloff == ROLLOFF_13) {
+			state->m_itut_annex_c = true;
+			SetOperationMode(state, OM_QAM_ITU_C);
+		} else {
+			state->m_itut_annex_c = false;
+			SetOperationMode(state, OM_QAM_ITU_A);
+		}
+	} else
+		SetOperationMode(state, OM_DVBT);
+
 	Start(state, 0, IF);
 
 	/* printk(KERN_DEBUG "drxk: %s IF=%d done\n", __func__, IF); */
diff --git a/drivers/media/dvb/frontends/drxk_hard.h b/drivers/media/dvb/frontends/drxk_hard.h
index a05c32e..85a423f 100644
--- a/drivers/media/dvb/frontends/drxk_hard.h
+++ b/drivers/media/dvb/frontends/drxk_hard.h
@@ -263,6 +263,8 @@ struct drxk_state {
 	u8     m_TSDataStrength;
 	u8     m_TSClockkStrength;
 
+	bool   m_itut_annex_c;      /* If true, uses ITU-T DVB-C Annex C, instead of Annex A */
+
 	enum DRXMPEGStrWidth_t  m_widthSTR;    /**< MPEG start width */
 	u32    m_mpegTsStaticBitrate;          /**< Maximum bitrate in b/s in case
 						    static clockrate is selected */
-- 
1.7.8

Re: [PATCH] [media] drxk: Switch the delivery system on FE_SET_PROPERTY

[Antti Palosaari]

On 12/09/2011 08:20 PM, Mauro Carvalho Chehab wrote:
> The DRX-K doesn't change the delivery system at set_properties,
> but do it at frontend init. This causes problems on programs like
> w_scan that, by default, opens both frontends.
>
> Instead, explicitly set the format when set_parameters callback is
> called.

May I ask why you don't use mfe_shared flag instead?

regards
Antti

-- 
http://palosaari.fi/

Re: [PATCH] [media] drxk: Switch the delivery system on FE_SET_PROPERTY

[Mauro Carvalho Chehab]

On 09-12-2011 16:26, Antti Palosaari wrote:
> On 12/09/2011 08:20 PM, Mauro Carvalho Chehab wrote:
>> The DRX-K doesn't change the delivery system at set_properties,
>> but do it at frontend init. This causes problems on programs like
>> w_scan that, by default, opens both frontends.
>>
>> Instead, explicitly set the format when set_parameters callback is
>> called.
>
> May I ask why you don't use mfe_shared flag instead?

Tested with it. Works. It takes a little more time to switch, but the
solution will be cleaner. version 2 will follow.

Regards,
Mauro

[PATCHv2] [media] drxk: Switch the delivery system on FE_SET_PROPERTY

[Mauro Carvalho Chehab]

The DRX-K doesn't change the delivery system at set_properties,
but do it at frontend init. This causes problems on programs like
w_scan that, by default, opens both frontends.

Use adap->mfe_shared in order to prevent this, and be sure that Annex A
or C are properly selected.

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---

v2: Use mfe_shared

 drivers/media/dvb/frontends/drxk_hard.c |   16 ++++++++++------
 drivers/media/dvb/frontends/drxk_hard.h |    2 ++
 drivers/media/video/em28xx/em28xx-dvb.c |    4 ++++
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/media/dvb/frontends/drxk_hard.c b/drivers/media/dvb/frontends/drxk_hard.c
index 95cbc98..388b815 100644
--- a/drivers/media/dvb/frontends/drxk_hard.c
+++ b/drivers/media/dvb/frontends/drxk_hard.c
@@ -1847,6 +1847,7 @@ static int SetOperationMode(struct drxk_state *state,
 		*/
 	switch (oMode) {
 	case OM_DVBT:
+		dprintk(1, ": DVB-T\n");
 		state->m_OperationMode = oMode;
 		status = SetDVBTStandard(state, oMode);
 		if (status < 0)
@@ -1854,6 +1855,8 @@ static int SetOperationMode(struct drxk_state *state,
 		break;
 	case OM_QAM_ITU_A:	/* fallthrough */
 	case OM_QAM_ITU_C:
+		dprintk(1, ": DVB-C Annex %c\n",
+			(state->m_OperationMode == OM_QAM_ITU_A) ? 'A' : 'C');
 		state->m_OperationMode = oMode;
 		status = SetQAMStandard(state, oMode);
 		if (status < 0)
@@ -6183,7 +6186,10 @@ static int drxk_c_init(struct dvb_frontend *fe)
 	dprintk(1, "\n");
 	if (mutex_trylock(&state->ctlock) == 0)
 		return -EBUSY;
-	SetOperationMode(state, OM_QAM_ITU_A);
+	if (state->m_itut_annex_c)
+		SetOperationMode(state, OM_QAM_ITU_C);
+	else
+		SetOperationMode(state, OM_QAM_ITU_A);
 	return 0;
 }
 
@@ -6219,13 +6225,11 @@ static int drxk_set_parameters(struct dvb_frontend *fe,
 		return -EINVAL;
 	}
 
-	if (state->m_OperationMode == OM_QAM_ITU_A ||
-	    state->m_OperationMode == OM_QAM_ITU_C) {
+	if (fe->ops.info.type == FE_QAM) {
 		if (fe->dtv_property_cache.rolloff == ROLLOFF_13)
-			state->m_OperationMode = OM_QAM_ITU_C;
+			state->m_itut_annex_c = true;
 		else
-			state->m_OperationMode = OM_QAM_ITU_A;
-	}
+			state->m_itut_annex_c = false;
 
 	if (fe->ops.i2c_gate_ctrl)
 		fe->ops.i2c_gate_ctrl(fe, 1);
diff --git a/drivers/media/dvb/frontends/drxk_hard.h b/drivers/media/dvb/frontends/drxk_hard.h
index a05c32e..85a423f 100644
--- a/drivers/media/dvb/frontends/drxk_hard.h
+++ b/drivers/media/dvb/frontends/drxk_hard.h
@@ -263,6 +263,8 @@ struct drxk_state {
 	u8     m_TSDataStrength;
 	u8     m_TSClockkStrength;
 
+	bool   m_itut_annex_c;      /* If true, uses ITU-T DVB-C Annex C, instead of Annex A */
+
 	enum DRXMPEGStrWidth_t  m_widthSTR;    /**< MPEG start width */
 	u32    m_mpegTsStaticBitrate;          /**< Maximum bitrate in b/s in case
 						    static clockrate is selected */
diff --git a/drivers/media/video/em28xx/em28xx-dvb.c b/drivers/media/video/em28xx/em28xx-dvb.c
index 7f0592c..3868c1e 100644
--- a/drivers/media/video/em28xx/em28xx-dvb.c
+++ b/drivers/media/video/em28xx/em28xx-dvb.c
@@ -899,6 +899,8 @@ static int em28xx_dvb_init(struct em28xx *dev)
 		       &dvb->fe[0]->ops.tuner_ops,
 		       sizeof(dvb->fe[0]->ops.tuner_ops));
 
+		mfe_shared = 1;
+
 		break;
 	}
 	case EM2884_BOARD_TERRATEC_H5:
@@ -935,6 +937,8 @@ static int em28xx_dvb_init(struct em28xx *dev)
 		       &dvb->fe[0]->ops.tuner_ops,
 		       sizeof(dvb->fe[0]->ops.tuner_ops));
 
+		mfe_shared = 1;
+
 		break;
 	case EM28174_BOARD_PCTV_460E:
 		/* attach demod */
-- 
1.7.8

Re: [PATCH] [media] drxk: Switch the delivery system on FE_SET_PROPERTY

[Antti Palosaari]

On 12/09/2011 08:58 PM, Mauro Carvalho Chehab wrote:
> On 09-12-2011 16:26, Antti Palosaari wrote:
>> On 12/09/2011 08:20 PM, Mauro Carvalho Chehab wrote:
>>> The DRX-K doesn't change the delivery system at set_properties,
>>> but do it at frontend init. This causes problems on programs like
>>> w_scan that, by default, opens both frontends.
>>>
>>> Instead, explicitly set the format when set_parameters callback is
>>> called.
>>
>> May I ask why you don't use mfe_shared flag instead?
>
> Tested with it. Works. It takes a little more time to switch, but the
> solution will be cleaner. version 2 will follow.

Yes, there is kind of loop timer which tries to take FE and swithing 
takes second or two because it waits FE is freed. I looked it when I did 
MFE and I did not understood why it was done like that.

cxd2820r has earlier simple lock (inside of demod driver) that just 
returned error immediately when busy. It is a little bit mystery for me 
why mfe_shared has that kind of waiting mechanism. Could someone explain 
reason for that?

regards
Antti

-- 
http://palosaari.fi/

Re: [PATCHv2] [media] drxk: Switch the delivery system on FE_SET_PROPERTY

[Eddi De Pieri]

Hi,

> v2: Use mfe_shared

on hvr930c this patch solve the commutation issue of frontend.

still persist same issue like Fredrik on w_scan. scan still works perfectly...


root@depieri1lnx:~# w_scan -f t -c IT
w_scan version 20110616 (compiled for DVB API 5.3)
using settings for ITALY
DVB aerial
DVB-T Europe
frontend_type DVB-T, channellist 4
output format vdr-1.6
output charset 'UTF-8', use -C <charset> to override
Info: using DVB adapter auto detection.
	/dev/dvb/adapter0/frontend0 -> DVB-C "DRXK DVB-C": specified was
DVB-T -> SEARCH NEXT ONE.
	/dev/dvb/adapter0/frontend1 -> DVB-T "DRXK DVB-T": good :-)
Using DVB-T frontend (adapter /dev/dvb/adapter0/frontend1)
-_-_-_-_ Getting frontend capabilities-_-_-_-_
Using DVB API 5.4
frontend 'DRXK DVB-T' supports
INVERSION_AUTO
QAM_AUTO
TRANSMISSION_MODE_AUTO
GUARD_INTERVAL_AUTO
HIERARCHY_AUTO
FEC_AUTO
FREQ (47.12MHz ... 865.00MHz)
-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_
Scanning 7MHz frequencies...
[...]
858000: (time: 03:10) (time: 03:13)

ERROR: Sorry - i couldn't get any working frequency/transponder
 Nothing to scan!!


this verbose mode seems interesting but I don't figure out why i get timeout.

177500: (time: 00:04) set_frontend: using DVB API 5.4
(time: 00:06) set_frontend: using DVB API 5.4
signal ok:
	QAM_AUTO f = 177500 kHz I999B7C999D999T999G999Y999
NIT (actual TS)
	new transponder:
	   (QAM_64   f = 1600000 kHz I999B8C34D0T8G16Y0)


Info: NIT(other) filter timeout

Re: [PATCHv2] [media] drxk: Switch the delivery system on FE_SET_PROPERTY

[Mauro Carvalho Chehab]

On 09-12-2011 18:04, Eddi De Pieri wrote:
> Hi,
>
>> v2: Use mfe_shared
>
> on hvr930c this patch solve the commutation issue of frontend.

Ok, good. One issue solved.

> still persist same issue like Fredrik on w_scan. scan still works perfectly...
>
>
> root@depieri1lnx:~# w_scan -f t -c IT
> w_scan version 20110616 (compiled for DVB API 5.3)
> using settings for ITALY
> DVB aerial
> DVB-T Europe
> frontend_type DVB-T, channellist 4
> output format vdr-1.6
> output charset 'UTF-8', use -C<charset>  to override
> Info: using DVB adapter auto detection.
> 	/dev/dvb/adapter0/frontend0 ->  DVB-C "DRXK DVB-C": specified was
> DVB-T ->  SEARCH NEXT ONE.
> 	/dev/dvb/adapter0/frontend1 ->  DVB-T "DRXK DVB-T": good :-)
> Using DVB-T frontend (adapter /dev/dvb/adapter0/frontend1)
> -_-_-_-_ Getting frontend capabilities-_-_-_-_
> Using DVB API 5.4
> frontend 'DRXK DVB-T' supports
> INVERSION_AUTO
> QAM_AUTO
> TRANSMISSION_MODE_AUTO
> GUARD_INTERVAL_AUTO
> HIERARCHY_AUTO
> FEC_AUTO
> FREQ (47.12MHz ... 865.00MHz)
> -_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_
> Scanning 7MHz frequencies...
> [...]
> 858000: (time: 03:10) (time: 03:13)
>
> ERROR: Sorry - i couldn't get any working frequency/transponder
>   Nothing to scan!!
>
>
> this verbose mode seems interesting but I don't figure out why i get timeout.
>
> 177500: (time: 00:04) set_frontend: using DVB API 5.4
> (time: 00:06) set_frontend: using DVB API 5.4
> signal ok:
> 	QAM_AUTO f = 177500 kHz I999B7C999D999T999G999Y999
> NIT (actual TS)
> 	new transponder:
> 	   (QAM_64   f = 1600000 kHz I999B8C34D0T8G16Y0)
>
>
> Info: NIT(other) filter timeout

Try to use w_scan with "-F" parameter, to increase the timeout. Maybe this
issue is due to a smaller timeout on w_scan, when compared with scan.

If this doesn't help, then you'll need to figure out what w_scan is doing
different than scan.

Regards,
Mauro.


> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] [media] drxk: Switch the delivery system on FE_SET_PROPERTY

[Mauro Carvalho Chehab]

On 09-12-2011 17:08, Antti Palosaari wrote:
> On 12/09/2011 08:58 PM, Mauro Carvalho Chehab wrote:
>> On 09-12-2011 16:26, Antti Palosaari wrote:
>>> On 12/09/2011 08:20 PM, Mauro Carvalho Chehab wrote:
>>>> The DRX-K doesn't change the delivery system at set_properties,
>>>> but do it at frontend init. This causes problems on programs like
>>>> w_scan that, by default, opens both frontends.
>>>>
>>>> Instead, explicitly set the format when set_parameters callback is
>>>> called.
>>>
>>> May I ask why you don't use mfe_shared flag instead?
>>
>> Tested with it. Works. It takes a little more time to switch, but the
>> solution will be cleaner. version 2 will follow.
>
> Yes, there is kind of loop timer which tries to take FE and swithing takes second or two
>because it waits FE is freed. I looked it when I did MFE and I did not understood why
>it was done like that.
>
> cxd2820r has earlier simple lock (inside of demod driver) that just
> returned error immediately when busy. It is a little bit mystery for
> me why mfe_shared has that kind of waiting mechanism.

Still, it doesn't make much sense, at least on w_scan, as the only thing that is
called with each adapter is FE_GET_INFO:


open("/dev/dvb/adapter0/frontend0", O_RDWR|O_NONBLOCK) = 3
ioctl(3, FE_GET_INFO, 0x635120)         = 0
write(2, "\t/dev/dvb/adapter0/frontend0 -> "..., 92	/dev/dvb/adapter0/frontend0 -> DVB-C "DRXK DVB-C": specified was DVB-T -> SEARCH NEXT ONE.
) = 92
close(3)                                = 0
open("/dev/dvb/adapter0/frontend1", O_RDWR|O_NONBLOCK) = 3
ioctl(3, FE_GET_INFO, 0x635120)         = 0
write(2, "\t/dev/dvb/adapter0/frontend1 -> "..., 52	/dev/dvb/adapter0/frontend1 -> DVB-T "DRXK DVB-T": ) = 52
write(2, "good :-)\n", 9good :-)
)               = 9
close(3)                                = 0

Still, the second open takes about 4 seconds to complete, _even_ with O_NONBLOCK.

There's something bad there, as it is violating POSIX.

>Could someone explain reason for that?

I dunno, but I think this needs to be fixed, at least when the frontend
is opened with O_NONBLOCK.

Regards,
Mauro.

Re: [PATCH] [media] drxk: Switch the delivery system on FE_SET_PROPERTY

[Devin Heitmueller]

On Fri, Dec 9, 2011 at 5:11 PM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
>> Could someone explain reason for that?
>
>
> I dunno, but I think this needs to be fixed, at least when the frontend
> is opened with O_NONBLOCK.

Are you doing the drx-k firmware load on dvb_init()?  That could
easily take 4 seconds.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: [PATCH] [media] drxk: Switch the delivery system on FE_SET_PROPERTY

[Mauro Carvalho Chehab]

On 09-12-2011 20:33, Devin Heitmueller wrote:
> On Fri, Dec 9, 2011 at 5:11 PM, Mauro Carvalho Chehab
> <mchehab@redhat.com>  wrote:
>>> Could someone explain reason for that?
>>
>>
>> I dunno, but I think this needs to be fixed, at least when the frontend
>> is opened with O_NONBLOCK.
>
> Are you doing the drx-k firmware load on dvb_init()?  That could
> easily take 4 seconds.

No. The firmware were opened previously.

This is what happens at the driver:

	(frontend0 open - DVB-C)
[ 5177.932326] drxk: drxk_c_init
[ 5177.932330] drxk: SetOperationMode
[ 5177.932691] drxk: drxk_gate_ctrlenable
[ 5177.932695] drxk: ConfigureI2CBridge
[ 5177.932697] xc5000: xc5000_init()
[ 5177.936565] xc5000: xc5000_is_firmware_loaded() returns True id = 0x1388
[ 5177.943306] xc5000: xc_initialize()
[ 5178.187199] xc5000: *** ADC envelope (0-1023) = 4
[ 5178.192569] xc5000: *** Frequency error = 0 Hz
[ 5178.197566] xc5000: *** Lock status (0-Wait, 1-Locked, 2-No-signal) = 1
[ 5178.205291] xc5000: *** HW: V03.02, FW: V01.06.0072
[ 5178.210662] xc5000: *** Horizontal sync frequency = 15473 Hz
[ 5178.216909] xc5000: *** Frame lines = 789
[ 5178.221659] xc5000: *** Quality (0:<8dB, 7:>56dB) = 9
[ 5178.226753] drxk: drxk_gate_ctrldisable
[ 5178.226755] drxk: ConfigureI2CBridge
	(frontend1 open - DVB-T)
[ 5181.224873] drxk: drxk_gate_ctrlenable
[ 5181.224877] drxk: ConfigureI2CBridge
[ 5181.224880] xc5000: xc5000_sleep()
[ 5181.228327] xc5000: xc5000_TunerReset()
[ 5181.232204] drxk: drxk_gate_ctrldisable
[ 5181.232205] drxk: ConfigureI2CBridge
[ 5181.232207] drxk: drxk_c_sleep
[ 5181.232209] drxk: ShutDown
[ 5181.232211] drxk: MPEGTSStop
[ 5181.731673] drxk: drxk_t_init
[ 5181.731677] drxk: SetOperationMode
[ 5181.732101] drxk: MPEGTSStop
[ 5181.734075] drxk: PowerDownQAM
[ 5181.735075] drxk: scu_command
[ 5181.737970] drxk: SetIqmAf
[ 5181.738948] drxk: SetOperationMode: DVB-T
[ 5181.738950] drxk: SetDVBTStandard
[ 5181.738952] drxk: PowerUpDVBT
[ 5181.738954] drxk: CtrlPowerMode
[ 5181.738956] drxk: PowerUpDevice
[ 5181.740321] drxk: DVBTEnableOFDMTokenRing
[ 5181.741947] drxk: SwitchAntennaToDVBT
[ 5181.741949] drxk: scu_command
[ 5181.744718] drxk: scu_command
[ 5181.750317] drxk: SetIqmAf
[ 5181.755439] drxk: BLChainCmd
[ 5181.760710] drxk: ADCSynchronization
[ 5181.760713] drxk: ADCSyncMeasurement
[ 5181.763596] drxk: SetPreSaw
[ 5181.764309] drxk: SetAgcRf
[ 5181.766433] drxk: SetAgcIf
[ 5181.773183] drxk: MPEGTSDtoSetup
[ 5181.777948] drxk: DVBTActivatePresets
[ 5181.777951] drxk: DVBTCtrlSetIncEnable
[ 5181.778301] drxk: DVBTCtrlSetFrEnable
[ 5181.778703] drxk: DVBTCtrlSetEchoThreshold
[ 5181.779697] drxk: DVBTCtrlSetEchoThreshold
[ 5181.781053] drxk: drxk_gate_ctrlenable
[ 5181.781056] drxk: ConfigureI2CBridge
[ 5181.781058] xc5000: xc5000_init()
[ 5181.785050] xc5000: xc5000_is_firmware_loaded() returns True id = 0x1388
[ 5181.791790] xc5000: xc_initialize()
[ 5182.041187] xc5000: *** ADC envelope (0-1023) = 4
[ 5182.046559] xc5000: *** Frequency error = 0 Hz
[ 5182.051557] xc5000: *** Lock status (0-Wait, 1-Locked, 2-No-signal) = 1
[ 5182.059448] xc5000: *** HW: V03.02, FW: V01.06.0072
[ 5182.065154] xc5000: *** Horizontal sync frequency = 14817 Hz
[ 5182.071424] xc5000: *** Frame lines = 1283
[ 5182.076273] xc5000: *** Quality (0:<8dB, 7:>56dB) = 9
[ 5182.081366] drxk: drxk_gate_ctrldisable
[ 5182.081368] drxk: ConfigureI2CBridge
[ 5185.079823] drxk: drxk_gate_ctrlenable
[ 5185.079827] drxk: ConfigureI2CBridge
[ 5185.079830] xc5000: xc5000_sleep()
[ 5185.083276] xc5000: xc5000_TunerReset()
[ 5185.087104] drxk: drxk_gate_ctrldisable
[ 5185.087107] drxk: ConfigureI2CBridge
[ 5185.087111] drxk: drxk_t_sleep
[ 5185.087323] drxk: drxk_c_init
[ 5185.087326] drxk: SetOperationMode
[ 5185.087778] drxk: MPEGTSStop
[ 5185.089993] drxk: PowerDownDVBT
[ 5185.090780] drxk: scu_command
[ 5185.094100] drxk: scu_command
[ 5185.098219] drxk: SetIqmAf
[ 5185.099221] drxk: CtrlPowerMode
[ 5185.099223] drxk: MPEGTSStop
[ 5185.101218] drxk: PowerDownDVBT
[ 5185.101854] drxk: scu_command
[ 5185.105090] drxk: scu_command
[ 5185.109215] drxk: SetIqmAf
[ 5185.110215] drxk: DVBTEnableOFDMTokenRing
[ 5185.112566] drxk: SetOperationMode: DVB-C Annex C
[ 5185.112568] drxk: SetQAMStandard
[ 5185.112570] drxk: SwitchAntennaToQAM
[ 5185.112572] drxk: PowerUpQAM
[ 5185.112574] drxk: CtrlPowerMode
[ 5185.112575] drxk: QAMResetQAM
[ 5185.112962] drxk: scu_command
[ 5185.116838] drxk: BLChainCmd
[ 5185.127954] drxk: SetIqmAf
[ 5185.129306] drxk: ADCSynchronization
[ 5185.129308] drxk: ADCSyncMeasurement
[ 5185.132949] drxk: InitAGC
[ 5185.149315] drxk: SetPreSaw
[ 5185.149721] drxk: SetAgcRf
[ 5185.151720] drxk: SetAgcIf
[ 5185.155817] drxk: drxk_gate_ctrlenable
[ 5185.155820] drxk: ConfigureI2CBridge
[ 5185.155822] xc5000: xc5000_init()
[ 5185.159694] xc5000: xc5000_is_firmware_loaded() returns True id = 0x1388
[ 5185.166432] xc5000: xc_initialize()
[ 5185.416305] xc5000: *** ADC envelope (0-1023) = 4
[ 5185.421593] xc5000: *** Frequency error = 0 Hz
[ 5185.426676] xc5000: *** Lock status (0-Wait, 1-Locked, 2-No-signal) = 1
[ 5185.434622] xc5000: *** HW: V03.02, FW: V01.06.0072
[ 5185.440270] xc5000: *** Horizontal sync frequency = 14374 Hz
[ 5185.446579] xc5000: *** Frame lines = 1283
[ 5185.451389] xc5000: *** Quality (0:<8dB, 7:>56dB) = 9
[ 5185.456475] drxk: drxk_gate_ctrldisable
[ 5185.456477] drxk: ConfigureI2CBridge
[ 5185.456614] drxk: drxk_c_get_tune_settings
[ 5185.456773] drxk: drxk_set_parameters
[ 5185.456776] drxk: drxk_gate_ctrlenable
[ 5185.456778] drxk: ConfigureI2CBridge
[ 5185.457576] xc5000: xc5000_is_firmware_loaded() returns True id = 0x1388
[ 5185.464311] xc5000: xc5000_set_params() frequency=57000000 (Hz)
[ 5185.470268] xc5000: xc5000_set_params() QAM modulation
[ 5185.475439] xc5000: xc5000_set_params() Bandwidth 6MHz (5999550)
[ 5185.481473] xc5000: xc5000_set_params() frequency=55250000 (compensated)
[ 5185.488202] xc5000: xc_SetSignalSource(1) Source = CABLE
[ 5185.494524] xc5000: xc_SetTVStandard(0x8002,0x00c0)
[ 5185.499435] xc5000: xc_SetTVStandard() Standard = DTV6
[ 5185.513360] xc5000: xc_set_IF_frequency(freq_khz = 4000) freq_code = 0x1000
[ 5185.528244] xc5000: xc_tune_channel(55250000)
[ 5185.532643] xc5000: xc_set_RF_frequency(55250000)
[ 5185.729144] xc5000: *** ADC envelope (0-1023) = 744
[ 5185.734762] xc5000: *** Frequency error = 0 Hz
[ 5185.739739] xc5000: *** Lock status (0-Wait, 1-Locked, 2-No-signal) = 1
[ 5185.747612] xc5000: *** HW: V03.02, FW: V01.06.0072
[ 5185.753107] xc5000: *** Horizontal sync frequency = 15038 Hz
[ 5185.759410] xc5000: *** Frame lines = 65535
[ 5185.764408] xc5000: *** Quality (0:<8dB, 7:>56dB) = 5
[ 5185.769503] drxk: drxk_gate_ctrldisable
[ 5185.769505] drxk: ConfigureI2CBridge
[ 5185.769507] xc5000: xc5000_get_if_frequency()
[ 5185.773902] drxk: Start
[ 5185.773904] drxk: SetQAM
[ 5185.774845] drxk: QAMResetQAM
[ 5185.775218] drxk: scu_command
[ 5185.778738] drxk: QAMSetSymbolrate
[ 5185.779969] drxk: scu_command
[ 5185.783737] drxk: SCU_RESULT_INVPAR while sending cmd 0x0203 with params:
[ 5185.790388] drxk: 02 00 00 00 10 00 05 00 03 02                    ..........
[ 5185.790581] drxk: scu_command
[ 5185.793857] drxk: scu_command
[ 5185.797729] drxk: SetFrequencyShifter
[ 5185.798209] drxk: SetQAMMeasurement
[ 5185.808601] drxk: SetQAM64
[ 5185.829359] drxk: MPEGTSDtoSetup
[ 5185.836588] drxk: scu_command
	(I've aborted w_scan to avoid generating a too big dump)
[ 5188.837828] drxk: drxk_gate_ctrlenable
[ 5188.837833] drxk: ConfigureI2CBridge
[ 5188.837835] xc5000: xc5000_sleep()
[ 5188.841280] xc5000: xc5000_TunerReset()
[ 5188.845155] drxk: drxk_gate_ctrldisable
[ 5188.845157] drxk: ConfigureI2CBridge
[ 5188.845159] drxk: drxk_c_sleep
[ 5188.845160] drxk: ShutDown
[ 5188.845162] drxk: MPEGTSStop

Re: [PATCH] [media] drxk: Switch the delivery system on FE_SET_PROPERTY

[Mauro Carvalho Chehab]

On 09-12-2011 21:37, Mauro Carvalho Chehab wrote:
> On 09-12-2011 20:33, Devin Heitmueller wrote:
>> On Fri, Dec 9, 2011 at 5:11 PM, Mauro Carvalho Chehab
>> <mchehab@redhat.com> wrote:
>>>> Could someone explain reason for that?
>>>
>>>
>>> I dunno, but I think this needs to be fixed, at least when the frontend
>>> is opened with O_NONBLOCK.
>>
>> Are you doing the drx-k firmware load on dvb_init()? That could
>> easily take 4 seconds.
>
> No. The firmware were opened previously.

Maybe the delay is due to this part of dvb_frontend.c:

static int dvb_mfe_wait_time = 5;
...
                         int mferetry = (dvb_mfe_wait_time << 1);

                         mutex_unlock (&adapter->mfe_lock);
                         while (mferetry-- && (mfedev->users != -1 ||
                                         mfepriv->thread != NULL)) {
                                 if(msleep_interruptible(500)) {
                                         if(signal_pending(current))
                                                 return -EINTR;
                                 }
                         }


If I set this modprobe parameter to 1, the delay reduces drastically:

[ 5975.865162] drxk: ConfigureI2CBridge
[ 5975.865164] xc5000: xc5000_init()
[ 5975.869257] xc5000: xc5000_is_firmware_loaded() returns True id = 0x1388
[ 5975.876009] xc5000: xc_initialize()
[ 5976.120891] xc5000: *** ADC envelope (0-1023) = 4
[ 5976.126260] xc5000: *** Frequency error = 0 Hz
[ 5976.131260] xc5000: *** Lock status (0-Wait, 1-Locked, 2-No-signal) = 1
[ 5976.139111] xc5000: *** HW: V03.02, FW: V01.06.0072
[ 5976.144733] xc5000: *** Horizontal sync frequency = 11292 Hz
[ 5976.150976] xc5000: *** Frame lines = 1442
[ 5976.155850] xc5000: *** Quality (0:<8dB, 7:>56dB) = 9
[ 5976.160937] drxk: drxk_gate_ctrldisable
[ 5976.160939] drxk: ConfigureI2CBridge
[ 5977.161897] drxk: drxk_c_get_tune_settings
[ 5977.162085] drxk: drxk_c_init
[ 5977.162089] drxk: drxk_gate_ctrlenable
[ 5977.162091] drxk: ConfigureI2CBridge
[ 5977.162094] xc5000: xc5000_init()
[ 5977.166095] xc5000: xc5000_is_firmware_loaded() returns True id = 0x1388
[ 5977.172836] xc5000: xc_initialize()
[ 5977.422213] xc5000: *** ADC envelope (0-1023) = 4
[ 5977.427706] xc5000: *** Frequency error = 0 Hz
[ 5977.432832] xc5000: *** Lock status (0-Wait, 1-Locked, 2-No-signal) = 1
[ 5977.440682] xc5000: *** HW: V03.02, FW: V01.06.0072
[ 5977.446177] xc5000: *** Horizontal sync frequency = 10460 Hz
[ 5977.452482] xc5000: *** Frame lines = 1442
[ 5977.457296] xc5000: *** Quality (0:<8dB, 7:>56dB) = 9
[ 5977.462385] drxk: drxk_gate_ctrldisable
[ 5977.462388] drxk: ConfigureI2CBridge
[ 5977.462390] drxk: drxk_set_parameters
[ 5977.462392] drxk: drxk_gate_ctrlenable
[ 5977.462394] drxk: ConfigureI2CBridge
[ 5977.463043] xc5000: xc5000_is_firmware_loaded() returns True id = 0x1388
[ 5977.469781] xc5000: xc5000_set_params() frequency=57000000 (Hz)
[ 5977.475740] xc5000: xc5000_set_params() QAM modulation
[ 5977.480912] xc5000: xc5000_set_params() Bandwidth 6MHz (5999550)
[ 5977.486948] xc5000: xc5000_set_params() frequency=55250000 (compensated)
[ 5977.493677] xc5000: xc_SetSignalSource(1) Source = CABLE
[ 5977.500024] xc5000: xc_SetTVStandard(0x8002,0x00c0)
[ 5977.504930] xc5000: xc_SetTVStandard() Standard = DTV6
[ 5977.518267] xc5000: xc_set_IF_frequency(freq_khz = 4000) freq_code = 0x1000
[ 5977.527135] xc5000: xc_tune_channel(55250000)
[ 5977.531530] xc5000: xc_set_RF_frequency(55250000)
[ 5977.728050] xc5000: *** ADC envelope (0-1023) = 768
[ 5977.733671] xc5000: *** Frequency error = 0 Hz
[ 5977.738649] xc5000: *** Lock status (0-Wait, 1-Locked, 2-No-signal) = 1
[ 5977.746523] xc5000: *** HW: V03.02, FW: V01.06.0072
[ 5977.752017] xc5000: *** Horizontal sync frequency = 14970 Hz
[ 5977.758288] xc5000: *** Frame lines = 65535
[ 5977.763137] xc5000: *** Quality (0:<8dB, 7:>56dB) = 5
[ 5977.768224] drxk: drxk_gate_ctrldisable
[ 5977.768226] drxk: ConfigureI2CBridge
[ 5977.768228] xc5000: xc5000_get_if_frequency()
[ 5977.772624] drxk: Start
[ 5977.772626] drxk: SetQAM
[ 5977.773530] drxk: QAMResetQAM
[ 5977.773880] drxk: scu_command
[ 5977.777653] drxk: QAMSetSymbolrate
[ 5977.778880] drxk: scu_command
[ 5977.782647] drxk: SCU_RESULT_INVPAR while sending cmd 0x0203 with params:
[ 5977.789298] drxk: 02 00 00 00 10 00 05 00 03 02                    ..........
[ 5977.789490] drxk: scu_command
[ 5977.792644] drxk: scu_command
[ 5977.795641] drxk: SetFrequencyShifter
[ 5977.796119] drxk: SetQAMMeasurement
[ 5977.806489] drxk: SetQAM64
[ 5977.827502] drxk: MPEGTSDtoSetup
[ 5977.834621] drxk: scu_command
[ 5978.161550] drxk: drxk_read_status
[ 5978.161554] drxk: GetLockStatus
[ 5978.161556] drxk: GetQAMLockStatus
[ 5978.161558] drxk: scu_command
[ 5978.315220] drxk: drxk_read_status
[ 5978.315223] drxk: GetLockStatus
[ 5978.315225] drxk: GetQAMLockStatus
[ 5978.315227] drxk: scu_command
[ 5978.469137] drxk: drxk_read_status
[ 5978.469141] drxk: GetLockStatus
[ 5978.469143] drxk: GetQAMLockStatus
[ 5978.469144] drxk: scu_command
[ 5978.623043] drxk: drxk_read_status
[ 5978.623046] drxk: GetLockStatus
[ 5978.623048] drxk: GetQAMLockStatus
[ 5978.623050] drxk: scu_command
[ 5978.776974] drxk: drxk_read_status
[ 5978.776977] drxk: GetLockStatus
[ 5978.776979] drxk: GetQAMLockStatus
[ 5978.776981] drxk: scu_command
[ 5978.930891] drxk: drxk_read_status
[ 5978.930894] drxk: GetLockStatus
[ 5978.930896] drxk: GetQAMLockStatus
[ 5978.930898] drxk: scu_command
[ 5979.084814] drxk: drxk_read_status
[ 5979.084817] drxk: GetLockStatus
[ 5979.084819] drxk: GetQAMLockStatus
[ 5979.084821] drxk: scu_command
[ 5979.238727] drxk: drxk_read_status
[ 5979.238730] drxk: GetLockStatus
[ 5979.238732] drxk: GetQAMLockStatus
[ 5979.238734] drxk: scu_command
[ 5979.392643] drxk: drxk_read_status
[ 5979.392646] drxk: GetLockStatus
[ 5979.392648] drxk: GetQAMLockStatus
[ 5979.392650] drxk: scu_command
[ 5979.546595] drxk: drxk_read_status
[ 5979.546598] drxk: GetLockStatus
[ 5979.546601] drxk: GetQAMLockStatus
[ 5979.546602] drxk: scu_command
[ 5979.700506] drxk: drxk_c_get_tune_settings
[ 5979.700683] drxk: drxk_set_parameters
[ 5979.700687] drxk: drxk_gate_ctrlenable
[ 5979.700689] drxk: ConfigureI2CBridge
[ 5979.701382] xc5000: xc5000_is_firmware_loaded() returns True id = 0x1388
[ 5979.708099] xc5000: xc5000_set_params() frequency=57000000 (Hz)
[ 5979.714055] xc5000: xc5000_set_params() QAM modulation
[ 5979.719230] xc5000: xc5000_set_params() Bandwidth 6MHz (5929400)
[ 5979.725267] xc5000: xc5000_set_params() frequency=55250000 (compensated)
[ 5979.731996] xc5000: xc_SetSignalSource(1) Source = CABLE
[ 5979.738262] xc5000: xc_SetTVStandard(0x8002,0x00c0)
[ 5979.743170] xc5000: xc_SetTVStandard() Standard = DTV6
[ 5979.757100] xc5000: xc_set_IF_frequency(freq_khz = 4000) freq_code = 0x1000
[ 5979.765968] xc5000: xc_tune_channel(55250000)
[ 5979.770364] xc5000: xc_set_RF_frequency(55250000)
[ 5979.966886] xc5000: *** ADC envelope (0-1023) = 816
[ 5979.972506] xc5000: *** Frequency error = 0 Hz
[ 5979.977483] xc5000: *** Lock status (0-Wait, 1-Locked, 2-No-signal) = 1
[ 5979.985482] xc5000: *** HW: V03.02, FW: V01.06.0072
[ 5979.990975] xc5000: *** Horizontal sync frequency = 15023 Hz
[ 5979.997496] xc5000: *** Frame lines = 65535
[ 5980.002347] xc5000: *** Quality (0:<8dB, 7:>56dB) = 0
[ 5980.007428] drxk: drxk_gate_ctrldisable
[ 5980.007430] drxk: ConfigureI2CBridge
[ 5980.007432] xc5000: xc5000_get_if_frequency()
[ 5980.011820] drxk: Start
[ 5980.011821] drxk: SetQAM
[ 5980.012714] drxk: QAMResetQAM
[ 5980.013111] drxk: scu_command
[ 5980.016483] drxk: QAMSetSymbolrate
[ 5980.017713] drxk: scu_command
[ 5980.021482] drxk: SCU_RESULT_INVPAR while sending cmd 0x0203 with params:
[ 5980.028126] drxk: 02 00 00 00 10 00 05 00 03 02                    ..........
[ 5980.028316] drxk: scu_command
[ 5980.031475] drxk: scu_command
[ 5980.034475] drxk: SetFrequencyShifter
[ 5980.034953] drxk: SetQAMMeasurement
[ 5980.045136] drxk: SetQAM64
[ 5980.066075] drxk: MPEGTSDtoSetup
[ 5980.073329] drxk: scu_command
[ 5980.700153] drxk: drxk_read_status
[ 5980.700157] drxk: GetLockStatus
[ 5980.700159] drxk: GetQAMLockStatus
[ 5980.700161] drxk: scu_command
[ 5980.853772] drxk: drxk_read_status
[ 5980.853775] drxk: GetLockStatus
[ 5980.853777] drxk: GetQAMLockStatus
[ 5980.853779] drxk: scu_command
[ 5981.007797] drxk: drxk_read_status
[ 5981.007801] drxk: GetLockStatus
[ 5981.007803] drxk: GetQAMLockStatus
[ 5981.007805] drxk: scu_command
[ 5981.161681] drxk: drxk_read_status
[ 5981.161684] drxk: GetLockStatus
[ 5981.161686] drxk: GetQAMLockStatus
[ 5981.161688] drxk: scu_command
[ 5981.315635] drxk: drxk_read_status
[ 5981.315638] drxk: GetLockStatus
[ 5981.315640] drxk: GetQAMLockStatus
[ 5981.315642] drxk: scu_command
[ 5981.469555] drxk: drxk_read_status
[ 5981.469558] drxk: GetLockStatus
[ 5981.469561] drxk: GetQAMLockStatus
[ 5981.469562] drxk: scu_command
[ 5981.623468] drxk: drxk_read_status
[ 5981.623472] drxk: GetLockStatus
[ 5981.623474] drxk: GetQAMLockStatus
[ 5981.623476] drxk: scu_command
[ 5981.777388] drxk: drxk_read_status
[ 5981.777391] drxk: GetLockStatus
[ 5981.777393] drxk: GetQAMLockStatus
[ 5981.777395] drxk: scu_command
[ 5981.931307] drxk: drxk_read_status
[ 5981.931311] drxk: GetLockStatus
[ 5981.931313] drxk: GetQAMLockStatus
[ 5981.931314] drxk: scu_command
[ 5982.085228] drxk: drxk_read_status
[ 5982.085232] drxk: GetLockStatus
[ 5982.085234] drxk: GetQAMLockStatus
[ 5982.085236] drxk: scu_command
[ 5982.239174] drxk: drxk_c_get_tune_settings
[ 5982.239356] drxk: drxk_set_parameters
[ 5982.239360] drxk: drxk_gate_ctrlenable
[ 5982.239362] drxk: ConfigureI2CBridge
[ 5982.240066] xc5000: xc5000_is_firmware_loaded() returns True id = 0x1388
[ 5982.246785] xc5000: xc5000_set_params() frequency=57000000 (Hz)
[ 5982.252740] xc5000: xc5000_set_params() QAM modulation
[ 5982.257912] xc5000: xc5000_set_params() Bandwidth 6MHz (5750000)
[ 5982.263949] xc5000: xc5000_set_params() frequency=55250000 (compensated)
[ 5982.270679] xc5000: xc_SetSignalSource(1) Source = CABLE
[ 5982.276941] xc5000: xc_SetTVStandard(0x8002,0x00c0)
[ 5982.281849] xc5000: xc_SetTVStandard() Standard = DTV6
[ 5982.295776] xc5000: xc_set_IF_frequency(freq_khz = 4000) freq_code = 0x1000
[ 5982.304771] xc5000: xc_tune_channel(55250000)
[ 5982.309169] xc5000: xc_set_RF_frequency(55250000)
[ 5982.506561] xc5000: *** ADC envelope (0-1023) = 724
[ 5982.512186] xc5000: *** Frequency error = 0 Hz
[ 5982.517408] xc5000: *** Lock status (0-Wait, 1-Locked, 2-No-signal) = 1
[ 5982.525178] xc5000: *** HW: V03.02, FW: V01.06.0072
[ 5982.530799] xc5000: *** Horizontal sync frequency = 14954 Hz
[ 5982.537149] xc5000: *** Frame lines = 65535
[ 5982.542022] xc5000: *** Quality (0:<8dB, 7:>56dB) = 5
[ 5982.547109] drxk: drxk_gate_ctrldisable
[ 5982.547111] drxk: ConfigureI2CBridge
[ 5982.547113] xc5000: xc5000_get_if_frequency()
[ 5982.551512] drxk: Start
[ 5982.551514] drxk: SetQAM
[ 5982.552390] drxk: QAMResetQAM
[ 5982.552769] drxk: scu_command
[ 5982.556160] drxk: QAMSetSymbolrate
[ 5982.557390] drxk: scu_command
[ 5982.561160] drxk: SCU_RESULT_INVPAR while sending cmd 0x0203 with params:
[ 5982.567809] drxk: 02 00 00 00 10 00 05 00 03 02                    ..........
[ 5982.568001] drxk: scu_command
[ 5982.571154] drxk: scu_command
[ 5982.574150] drxk: SetFrequencyShifter
[ 5982.574630] drxk: SetQAMMeasurement
[ 5982.584376] drxk: SetQAM64
[ 5982.604258] drxk: MPEGTSDtoSetup
[ 5982.611361] drxk: scu_command
[ 5982.615007] drxk: drxk_gate_ctrlenable
[ 5982.615010] drxk: ConfigureI2CBridge
[ 5982.615012] xc5000: xc5000_sleep()
[ 5982.618436] xc5000: xc5000_TunerReset()
[ 5982.622313] drxk: drxk_gate_ctrldisable
[ 5982.622315] drxk: ConfigureI2CBridge
[ 5982.622317] drxk: drxk_c_sleep
[ 5982.622319] drxk: ShutDown
[ 5982.622321] drxk: MPEGTSStop



Regards,
Mauro.

[PATCH] DVB: dvb_frontend: fix delayed thread exit

[Andreas Oberritter]

On 10.12.2011 00:43, Mauro Carvalho Chehab wrote:
> On 09-12-2011 21:37, Mauro Carvalho Chehab wrote:
>> On 09-12-2011 20:33, Devin Heitmueller wrote:
>>> On Fri, Dec 9, 2011 at 5:11 PM, Mauro Carvalho Chehab
>>> <mchehab@redhat.com> wrote:
>>>>> Could someone explain reason for that?
>>>>
>>>>
>>>> I dunno, but I think this needs to be fixed, at least when the frontend
>>>> is opened with O_NONBLOCK.
>>>
>>> Are you doing the drx-k firmware load on dvb_init()? That could
>>> easily take 4 seconds.
>>
>> No. The firmware were opened previously.
> 
> Maybe the delay is due to this part of dvb_frontend.c:
> 
> static int dvb_mfe_wait_time = 5;
> ...
>                         int mferetry = (dvb_mfe_wait_time << 1);
> 
>                         mutex_unlock (&adapter->mfe_lock);
>                         while (mferetry-- && (mfedev->users != -1 ||
>                                         mfepriv->thread != NULL)) {
>                                 if(msleep_interruptible(500)) {
>                                         if(signal_pending(current))
>                                                 return -EINTR;
>                                 }
>                         }

I haven't looked at the mfe code, but in case it's waiting for the
frontend thread to exit, there's a problem that causes the thread
not to exit immediately. Here's a patch that's been sitting in my
queue for a while:

---

Signed-off-by: Andreas Oberritter <obi@linuxtv.org>

diff --git a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c b/linux/drivers/media/dvb/dvb-core/dvb_frontend.c
index 7784d74..6823c2b 100644
--- a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c	2011-09-07 12:32:24.000000000 +0200
+++ a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c	2011-09-13 15:55:48.865742791 +0200
@@ -514,7 +514,7 @@
 		return 1;
 
 	if (fepriv->dvbdev->writers == 1)
-		if (time_after(jiffies, fepriv->release_jiffies +
+		if (time_after_eq(jiffies, fepriv->release_jiffies +
 				  dvb_shutdown_timeout * HZ))
 			return 1;
 
@@ -2070,12 +2070,15 @@
 
 	dprintk ("%s\n", __func__);
 
-	if ((file->f_flags & O_ACCMODE) != O_RDONLY)
+	if ((file->f_flags & O_ACCMODE) != O_RDONLY) {
 		fepriv->release_jiffies = jiffies;
+		mb();
+	}
 
 	ret = dvb_generic_release (inode, file);
 
 	if (dvbdev->users == -1) {
+		wake_up(&fepriv->wait_queue);
 		if (fepriv->exit != DVB_FE_NO_EXIT) {
 			fops_put(file->f_op);
 			file->f_op = NULL;

Re: [PATCH] DVB: dvb_frontend: fix delayed thread exit

[Devin Heitmueller]

On Fri, Dec 9, 2011 at 8:37 PM, Andreas Oberritter <obi@linuxtv.org> wrote:
> On 10.12.2011 00:43, Mauro Carvalho Chehab wrote:
>> On 09-12-2011 21:37, Mauro Carvalho Chehab wrote:
>>> On 09-12-2011 20:33, Devin Heitmueller wrote:
>>>> On Fri, Dec 9, 2011 at 5:11 PM, Mauro Carvalho Chehab
>>>> <mchehab@redhat.com> wrote:
>>>>>> Could someone explain reason for that?
>>>>>
>>>>>
>>>>> I dunno, but I think this needs to be fixed, at least when the frontend
>>>>> is opened with O_NONBLOCK.
>>>>
>>>> Are you doing the drx-k firmware load on dvb_init()? That could
>>>> easily take 4 seconds.
>>>
>>> No. The firmware were opened previously.
>>
>> Maybe the delay is due to this part of dvb_frontend.c:
>>
>> static int dvb_mfe_wait_time = 5;
>> ...
>>                         int mferetry = (dvb_mfe_wait_time << 1);
>>
>>                         mutex_unlock (&adapter->mfe_lock);
>>                         while (mferetry-- && (mfedev->users != -1 ||
>>                                         mfepriv->thread != NULL)) {
>>                                 if(msleep_interruptible(500)) {
>>                                         if(signal_pending(current))
>>                                                 return -EINTR;
>>                                 }
>>                         }
>
> I haven't looked at the mfe code, but in case it's waiting for the
> frontend thread to exit, there's a problem that causes the thread
> not to exit immediately. Here's a patch that's been sitting in my
> queue for a while:
>
> ---
>
> Signed-off-by: Andreas Oberritter <obi@linuxtv.org>
>
> diff --git a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c b/linux/drivers/media/dvb/dvb-core/dvb_frontend.c
> index 7784d74..6823c2b 100644
> --- a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c   2011-09-07 12:32:24.000000000 +0200
> +++ a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c   2011-09-13 15:55:48.865742791 +0200
> @@ -514,7 +514,7 @@
>                return 1;
>
>        if (fepriv->dvbdev->writers == 1)
> -               if (time_after(jiffies, fepriv->release_jiffies +
> +               if (time_after_eq(jiffies, fepriv->release_jiffies +
>                                  dvb_shutdown_timeout * HZ))
>                        return 1;
>
> @@ -2070,12 +2070,15 @@
>
>        dprintk ("%s\n", __func__);
>
> -       if ((file->f_flags & O_ACCMODE) != O_RDONLY)
> +       if ((file->f_flags & O_ACCMODE) != O_RDONLY) {
>                fepriv->release_jiffies = jiffies;
> +               mb();
> +       }
>
>        ret = dvb_generic_release (inode, file);
>
>        if (dvbdev->users == -1) {
> +               wake_up(&fepriv->wait_queue);
>                if (fepriv->exit != DVB_FE_NO_EXIT) {
>                        fops_put(file->f_op);
>                        file->f_op = NULL;

This patch needs to have a much better explanation of exactly what it
does and what problem it solves.  We have a history of race conditions
in dvb_frontend.c, and it's patches like this with virtually no
details just makes it worse.

I'm not arguing the actual merits of the code change - it *may* be
correct.  But without the appropriate background there is no real way
of knowing...

Mauro, this patch should be NACK'd and resubmitted with a detailed
explanation of the current behavior, what the problem is, and how the
code changes proposed solve that problem.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: [PATCH] DVB: dvb_frontend: fix delayed thread exit

[Andreas Oberritter]

On 10.12.2011 02:59, Devin Heitmueller wrote:
> On Fri, Dec 9, 2011 at 8:37 PM, Andreas Oberritter <obi@linuxtv.org> wrote:
>> On 10.12.2011 00:43, Mauro Carvalho Chehab wrote:
>>> On 09-12-2011 21:37, Mauro Carvalho Chehab wrote:
>>>> On 09-12-2011 20:33, Devin Heitmueller wrote:
>>>>> On Fri, Dec 9, 2011 at 5:11 PM, Mauro Carvalho Chehab
>>>>> <mchehab@redhat.com> wrote:
>>>>>>> Could someone explain reason for that?
>>>>>>
>>>>>>
>>>>>> I dunno, but I think this needs to be fixed, at least when the frontend
>>>>>> is opened with O_NONBLOCK.
>>>>>
>>>>> Are you doing the drx-k firmware load on dvb_init()? That could
>>>>> easily take 4 seconds.
>>>>
>>>> No. The firmware were opened previously.
>>>
>>> Maybe the delay is due to this part of dvb_frontend.c:
>>>
>>> static int dvb_mfe_wait_time = 5;
>>> ...
>>>                         int mferetry = (dvb_mfe_wait_time << 1);
>>>
>>>                         mutex_unlock (&adapter->mfe_lock);
>>>                         while (mferetry-- && (mfedev->users != -1 ||
>>>                                         mfepriv->thread != NULL)) {
>>>                                 if(msleep_interruptible(500)) {
>>>                                         if(signal_pending(current))
>>>                                                 return -EINTR;
>>>                                 }
>>>                         }
>>
>> I haven't looked at the mfe code, but in case it's waiting for the
>> frontend thread to exit, there's a problem that causes the thread
>> not to exit immediately. Here's a patch that's been sitting in my
>> queue for a while:
>>
>> ---
>>
>> Signed-off-by: Andreas Oberritter <obi@linuxtv.org>
>>
>> diff --git a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c b/linux/drivers/media/dvb/dvb-core/dvb_frontend.c
>> index 7784d74..6823c2b 100644
>> --- a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c   2011-09-07 12:32:24.000000000 +0200
>> +++ a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c   2011-09-13 15:55:48.865742791 +0200
>> @@ -514,7 +514,7 @@
>>                return 1;
>>
>>        if (fepriv->dvbdev->writers == 1)
>> -               if (time_after(jiffies, fepriv->release_jiffies +
>> +               if (time_after_eq(jiffies, fepriv->release_jiffies +
>>                                  dvb_shutdown_timeout * HZ))
>>                        return 1;
>>
>> @@ -2070,12 +2070,15 @@
>>
>>        dprintk ("%s\n", __func__);
>>
>> -       if ((file->f_flags & O_ACCMODE) != O_RDONLY)
>> +       if ((file->f_flags & O_ACCMODE) != O_RDONLY) {
>>                fepriv->release_jiffies = jiffies;
>> +               mb();
>> +       }
>>
>>        ret = dvb_generic_release (inode, file);
>>
>>        if (dvbdev->users == -1) {
>> +               wake_up(&fepriv->wait_queue);
>>                if (fepriv->exit != DVB_FE_NO_EXIT) {
>>                        fops_put(file->f_op);
>>                        file->f_op = NULL;
> 
> This patch needs to have a much better explanation of exactly what it
> does and what problem it solves.  We have a history of race conditions
> in dvb_frontend.c, and it's patches like this with virtually no
> details just makes it worse.
> 
> I'm not arguing the actual merits of the code change - it *may* be
> correct.  But without the appropriate background there is no real way
> of knowing...
> 
> Mauro, this patch should be NACK'd and resubmitted with a detailed
> explanation of the current behavior, what the problem is, and how the
> code changes proposed solve that problem.

WTF, Devin, you again? I haven't asked anyone to upstream it. Feel free
to analyze the code and resubmit it.

Re: [PATCH] DVB: dvb_frontend: fix delayed thread exit

[Devin Heitmueller]

Hello Andreas,

On Fri, Dec 9, 2011 at 9:06 PM, Andreas Oberritter <obi@linuxtv.org> wrote:
> WTF, Devin, you again? I haven't asked anyone to upstream it. Feel free
> to analyze the code and resubmit it.

1.  It's marked with a subject line that starts with "[PATCH]"
2.  It has your SIgned-Off-By line.
3.  it was sent to the mailing list.
4.  It doesn't have any keywords like "RFC" or "proposed".

If you don't intend for it to go upstream then don't do all of the above.

I'm not sure if your "WTF, Devin, you again?" is some indication that
I'm annoying you.  The last patch you submitted that touches the
threading in dvb_frontend.c had a host of problems and was clearly not
well researched (i.e. "DVB: dvb_frontend: convert semaphore to
mutex").  As in this case, there is no background indicating that this
patch has been fully thought out and due diligence has been done.

Maybe you have thoroughly researched the change, taken the time to
fully understand its effects, and tested it with a variety of boards
and scenarios.  Without a good patch description, there is no way to
know.

I apologize if you're inconvenienced if I'm making an active effort to
prevent poorly documented changes from getting merged (which often
result in regressions).  Oh wait, I'm not sorry at all.  Nevermind.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: [PATCHv2] [media] drxk: Switch the delivery system on FE_SET_PROPERTY

[Oliver Endriss]

On Friday 09 December 2011 20:00:12 Mauro Carvalho Chehab wrote:
> The DRX-K doesn't change the delivery system at set_properties,
> but do it at frontend init. This causes problems on programs like
> w_scan that, by default, opens both frontends.
> 
> Use adap->mfe_shared in order to prevent this, and be sure that Annex A
> or C are properly selected.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
> ---
> 
> v2: Use mfe_shared
> 
>  drivers/media/dvb/frontends/drxk_hard.c |   16 ++++++++++------
>  drivers/media/dvb/frontends/drxk_hard.h |    2 ++
>  drivers/media/video/em28xx/em28xx-dvb.c |    4 ++++
>  3 files changed, 16 insertions(+), 6 deletions(-)
...

Please commit Manu's patch to 'Query DVB frontend delivery capabilities'.
Then you will no longer have to struggle with multi-frontend problems.

We could finally get rid of having 2 mutual-exclusive frontends, which
is just an ugly workaround, barely covered by the API spec...

CU
Oliver

-- 
----------------------------------------------------------------
VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/
4 MByte Mod: http://www.escape-edv.de/endriss/dvb-mem-mod/
Full-TS Mod: http://www.escape-edv.de/endriss/dvb-full-ts-mod/
----------------------------------------------------------------
Oliver Endriss                         ESCAPE GmbH
e-mail:  o.endriss@escape-edv.de       EDV-Loesungen
phone:   +49 (0)7722 21504             Birkenweg 9
fax:     +49 (0)7722 21510             D-78098 Triberg
----------------------------------------------------------------

Re: [PATCH] DVB: dvb_frontend: fix delayed thread exit

[Mauro Carvalho Chehab]

On 10-12-2011 00:25, Devin Heitmueller wrote:
> Hello Andreas,
>
> On Fri, Dec 9, 2011 at 9:06 PM, Andreas Oberritter<obi@linuxtv.org>  wrote:
>> WTF, Devin, you again? I haven't asked anyone to upstream it. Feel free
>> to analyze the code and resubmit it.
>
> 1.  It's marked with a subject line that starts with "[PATCH]"
> 2.  It has your SIgned-Off-By line.
> 3.  it was sent to the mailing list.
> 4.  It doesn't have any keywords like "RFC" or "proposed".

Devin,

You're over-reacting. This patch were a reply from Andreas to a thread,
and not a separate patch submission.

Patches like are generally handled as RFC, especially since it doesn't
contain a description.

> If you don't intend for it to go upstream then don't do all of the above.
>
> I'm not sure if your "WTF, Devin, you again?" is some indication that
> I'm annoying you.  The last patch you submitted that touches the
> threading in dvb_frontend.c had a host of problems and was clearly not
> well researched (i.e. "DVB: dvb_frontend: convert semaphore to
> mutex").  As in this case, there is no background indicating that this
> patch has been fully thought out and due diligence has been done.
>
> Maybe you have thoroughly researched the change, taken the time to
> fully understand its effects, and tested it with a variety of boards
> and scenarios.  Without a good patch description, there is no way to
> know.
>
> I apologize if you're inconvenienced if I'm making an active effort to
> prevent poorly documented changes from getting merged (which often
> result in regressions).  Oh wait, I'm not sorry at all.  Nevermind.
>
> Devin
>

Regards,
Mauro

Re: [PATCH] DVB: dvb_frontend: fix delayed thread exit

[Mauro Carvalho Chehab]

On 09-12-2011 23:37, Andreas Oberritter wrote:
> On 10.12.2011 00:43, Mauro Carvalho Chehab wrote:
>> On 09-12-2011 21:37, Mauro Carvalho Chehab wrote:
>>> On 09-12-2011 20:33, Devin Heitmueller wrote:
>>>> On Fri, Dec 9, 2011 at 5:11 PM, Mauro Carvalho Chehab
>>>> <mchehab@redhat.com>  wrote:
>>>>>> Could someone explain reason for that?
>>>>>
>>>>>
>>>>> I dunno, but I think this needs to be fixed, at least when the frontend
>>>>> is opened with O_NONBLOCK.
>>>>
>>>> Are you doing the drx-k firmware load on dvb_init()? That could
>>>> easily take 4 seconds.
>>>
>>> No. The firmware were opened previously.
>>
>> Maybe the delay is due to this part of dvb_frontend.c:
>>
>> static int dvb_mfe_wait_time = 5;
>> ...
>>                          int mferetry = (dvb_mfe_wait_time<<  1);
>>
>>                          mutex_unlock (&adapter->mfe_lock);
>>                          while (mferetry--&&  (mfedev->users != -1 ||
>>                                          mfepriv->thread != NULL)) {
>>                                  if(msleep_interruptible(500)) {
>>                                          if(signal_pending(current))
>>                                                  return -EINTR;
>>                                  }
>>                          }
>
> I haven't looked at the mfe code, but in case it's waiting for the
> frontend thread to exit, there's a problem that causes the thread
> not to exit immediately. Here's a patch that's been sitting in my
> queue for a while:
>
> ---
>
> Signed-off-by: Andreas Oberritter<obi@linuxtv.org>

Andreas,

Thanks for the patch!

Devin,

> diff --git a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c b/linux/drivers/media/dvb/dvb-core/dvb_frontend.c
> index 7784d74..6823c2b 100644
> --- a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c	2011-09-07 12:32:24.000000000 +0200
> +++ a/linux/drivers/media/dvb/dvb-core/dvb_frontend.c	2011-09-13 15:55:48.865742791 +0200
> @@ -514,7 +514,7 @@
>   		return 1;
>
>   	if (fepriv->dvbdev->writers == 1)
> -		if (time_after(jiffies, fepriv->release_jiffies +
> +		if (time_after_eq(jiffies, fepriv->release_jiffies +
>   				  dvb_shutdown_timeout * HZ))

The only change here is that it will now use dvb_shutdown_timeout instead of
(dvb_shutdown_timeout * HZ + 1).

This makes sense.

>   			return 1;
>
> @@ -2070,12 +2070,15 @@
>
>   	dprintk ("%s\n", __func__);
>
> -	if ((file->f_flags&  O_ACCMODE) != O_RDONLY)
> +	if ((file->f_flags&  O_ACCMODE) != O_RDONLY) {
>   		fepriv->release_jiffies = jiffies;
> +		mb();

This is just a memory barrier to warrant that all CPU's will consider the new value for release_jiffies.
Probably Andreas added it because he noticed some race condition.

In any case, this won't cause any regressions.

> +	}
>
>   	ret = dvb_generic_release (inode, file);
>
>   	if (dvbdev->users == -1) {
> +		wake_up(&fepriv->wait_queue);

This is the only hook that changes the core behavior.

>   		if (fepriv->exit != DVB_FE_NO_EXIT) {
>   			fops_put(file->f_op);
>   			file->f_op = NULL;

With this change, the current code at dvb_frontend_release() wil; be:

         ret = dvb_generic_release (inode, file);

      	if (dvbdev->users == -1) {
		wake_up(&fepriv->wait_queue);
                 if (fepriv->exit != DVB_FE_NO_EXIT) {
                        	fops_put(file->f_op);
                         file->f_op = NULL;
                         wake_up(&dvbdev->wait_queue);
                 }
                	if (fe->ops.ts_bus_ctrl)
                         fe->ops.ts_bus_ctrl(fe, 0);
         }

The addition of a wake_up there is that the wake_up thread will be called
also when fepriv->exit == DVB_FE_NO_EXIT. This seems to make sense, as
dvb_frontend_thread() explicitly tests for it at:

                 wait_event_interruptible_timeout(fepriv->wait_queue,
                         dvb_frontend_should_wakeup(fe) || kthread_should_stop()
                                 || freezing(current),
                         fepriv->delay);

as dvb_frontend_should_wakeup(fe) is defined (after applying this patch) as:

static int dvb_frontend_is_exiting(struct dvb_frontend *fe)
{
         struct dvb_frontend_private *fepriv = fe->frontend_priv;

         if (fepriv->exit != DVB_FE_NO_EXIT)
                 return 1;

         if (fepriv->dvbdev->writers == 1)
                 if (time_after_eq(jiffies, fepriv->release_jiffies +
                                   dvb_shutdown_timeout * HZ))
                        	return 1;

        	return 0;
}

static int dvb_frontend_should_wakeup(struct dvb_frontend *fe)
{
         struct dvb_frontend_private *fepriv = fe->frontend_priv;

         if (fepriv->wakeup) {
                 fepriv->wakeup = 0;
                 return 1;
         }
	return dvb_frontend_is_exiting(fe);
}

So, this code makes sense to me. Btw, a wait queue can wait even without
an explicit call, since it is just something like [1]:

	do schedule() while (!condition);

So, all this patch would hurt would be to increase the chance for us to
detect a bug that it is already there.

Devin,

I'll do some tests here with a few devices, but, in principle, I don't see
any reason why not applying this patch. So, except if I detect something wrong
on my tests, of if you you point us for a regression caused by this change,
I'll apply it.

Of course, it would be nice if Andreas could add some comments, but if he doesn't,
I can write something. It won't be the first patch that the maintainer would
need to insert some description.

Regards,
Mauro.

[1] The actual implementation is a more complex than that loop. In this
specific case, as it uses the interruptible version, any signal would
also wake this thread.

Re: [PATCHv2] [media] drxk: Switch the delivery system on FE_SET_PROPERTY

[Mauro Carvalho Chehab]

On 10-12-2011 02:00, Oliver Endriss wrote:
> On Friday 09 December 2011 20:00:12 Mauro Carvalho Chehab wrote:
>> The DRX-K doesn't change the delivery system at set_properties,
>> but do it at frontend init. This causes problems on programs like
>> w_scan that, by default, opens both frontends.
>>
>> Use adap->mfe_shared in order to prevent this, and be sure that Annex A
>> or C are properly selected.
>>
>> Signed-off-by: Mauro Carvalho Chehab<mchehab@redhat.com>
>> ---
>>
>> v2: Use mfe_shared
>>
>>   drivers/media/dvb/frontends/drxk_hard.c |   16 ++++++++++------
>>   drivers/media/dvb/frontends/drxk_hard.h |    2 ++
>>   drivers/media/video/em28xx/em28xx-dvb.c |    4 ++++
>>   3 files changed, 16 insertions(+), 6 deletions(-)
> ...
>
> Please commit Manu's patch to 'Query DVB frontend delivery capabilities'.
> Then you will no longer have to struggle with multi-frontend problems.

I was waiting for him to submit the new version, as there were several
comments on the last series. Just checked that he submitted the new
version today. That will be a great improvement!

> We could finally get rid of having 2 mutual-exclusive frontends, which
> is just an ugly workaround, barely covered by the API spec...

Agreed.

Regards,
Mauro.

Re: [PATCH] DVB: dvb_frontend: fix delayed thread exit

[Devin Heitmueller]

Hello Mauro,

On Sat, Dec 10, 2011 at 5:28 AM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> Devin,
>
> You're over-reacting. This patch were a reply from Andreas to a thread,
> and not a separate patch submission.
>
> Patches like are generally handled as RFC, especially since it doesn't
> contain a description.

Any email that starts with "WTF, Devin, you again?" will probably not
get a very polite response.

I agree there's been some overreaction, but it hasn't been on my part.
 He took the time to split it onto a new thread, add the subject line
"PATCH", as well as adding an SOB.  Even if his intent was only to get
it reviewed, why should I waste half an hour of my time analyzing his
patch to try to figure out his intent if he isn't willing to simply
document it?

You have a history of blindly accepting such patches without review.
My only intent was to flag this patch to ensure that this didn't
happen here.  I've spent way more time than I should have to fixing
obscure race conditions in dvb core.  If the author of a patch cannot
take the time to document his findings to provide context then the
patch should be rejected without review until he does so.

And why isn't this broken into a patch series?  Even after you
analyzed the patch you still don't understand what the changes do and
why there are being made.  Your explanation for why he added the
"mb()" call was because "Probably Andreas added it because he noticed
some race condition".  What is the race condition?  Did he find
multiple race conditions?  Is this patch multiple fixes for a race
condition and some other crap at the same time?

If a developer wants a patch reviewed (as Andreas suggested was the
case here after-the-fact), then here's my feedback:  break this into a
series of small incremental patches which *in detail* describe the
problem that was found and how each patch addresses the issue.  Once
we have that, the maintainer can do a more in-depth analysis of
whether the patch should be accepted.  Code whose function cannot be
explicitly justified but simply 'looks better' should not be mixed in
with real functional changes.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: [PATCH] DVB: dvb_frontend: fix delayed thread exit

[Mauro Carvalho Chehab]

On 10-12-2011 11:43, Devin Heitmueller wrote:
> Hello Mauro,
>
> On Sat, Dec 10, 2011 at 5:28 AM, Mauro Carvalho Chehab
> <mchehab@redhat.com>  wrote:
>> Devin,
>>
>> You're over-reacting. This patch were a reply from Andreas to a thread,
>> and not a separate patch submission.
>>
>> Patches like are generally handled as RFC, especially since it doesn't
>> contain a description.
>
> Any email that starts with "WTF, Devin, you again?" will probably not
> get a very polite response.
>
> I agree there's been some overreaction, but it hasn't been on my part.
>   He took the time to split it onto a new thread, add the subject line
> "PATCH", as well as adding an SOB.  Even if his intent was only to get
> it reviewed, why should I waste half an hour of my time analyzing his
> patch to try to figure out his intent if he isn't willing to simply
> document it?

Both overacted, but this doesn't bring anything good.

> You have a history of blindly accepting such patches without review.

No. I always review all patches I receive. Yeah, I have to confess:
I'm not a robot, I'm not infallible ;) (well, even a robot would
hardly be infallible, anyway).

> My only intent was to flag this patch to ensure that this didn't
> happen here.  I've spent way more time than I should have to fixing
> obscure race conditions in dvb core.  If the author of a patch cannot
> take the time to document his findings to provide context then the
> patch should be rejected without review until he does so.
>
> And why isn't this broken into a patch series?  Even after you
> analyzed the patch you still don't understand what the changes do and
> why there are being made.  Your explanation for why he added the
> "mb()" call was because "Probably Andreas added it because he noticed
> some race condition".  What is the race condition?  Did he find
> multiple race conditions?  Is this patch multiple fixes for a race
> condition and some other crap at the same time?
>
> If a developer wants a patch reviewed (as Andreas suggested was the
> case here after-the-fact), then here's my feedback:  break this into a
> series of small incremental patches which *in detail* describe the
> problem that was found and how each patch addresses the issue.  Once
> we have that, the maintainer can do a more in-depth analysis of
> whether the patch should be accepted.  Code whose function cannot be
> explicitly justified but simply 'looks better' should not be mixed in
> with real functional changes.

I understand that you want patches better documented, so do I, and it
would be great if this patch had a better description since the beginning.

Yet, I don't agree that this patch should be split. It does just one
thing: it fixes the timeout handling for the dvb core frontend thread.

Regards,
Mauro