Hi,
I finally managed to dig through your patch. You did a great job.
Hopefully the changed behaviour will not cause too much trouble.
Wolfgang Rohdewald wrote:
> diff -up dvb-cvs/dvb-kernel/linux/drivers/media/dvb/ttpci/av7110_av.c linux/drivers/media/dvb/ttpci/av7110_av.c
> ...
> @@ -974,7 +986,7 @@ static int dvb_video_ioctl(struct inode
> ...
> case VIDEO_PLAY:
> ...
> if (av7110->videostate.stream_source == VIDEO_SOURCE_MEMORY) {
> if (av7110->playing == RP_AV) {
> - av7110_fw_cmd(av7110, COMTYPE_REC_PLAY, __Stop, 0);
> + ret = av7110_fw_cmd(av7110, COMTYPE_REC_PLAY, __Stop, 0);
> + if (ret)
> + break;
> av7110->playing &= ~RP_VIDEO;
> }
> - av7110_av_start_play(av7110, RP_VIDEO);
> - vidcom(av7110, VIDEO_CMD_PLAY, 0);
> + ret = av7110_av_start_play(av7110, RP_VIDEO);
> + if (!ret)
> + ret = vidcom(av7110, VIDEO_CMD_PLAY, 0);
> } else {
> - //av7110_av_stop(av7110, RP_VIDEO);
> - vidcom(av7110, VIDEO_CMD_PLAY, 0);
> + // ret = av7110_av_stop(av7110, RP_VIDEO);
> + if (!ret)
> + ret = vidcom(av7110, VIDEO_CMD_PLAY, 0);
> }
@all:
Can we remove the commented-out code?
vidcom() is executed for 'if' and 'else', so the else clause can be
removed.
case VIDEO_CLEAR_BUFFER:
> @@ -1136,18 +1167,19 @@ static int dvb_video_ioctl(struct inode
> av7110_ipack_reset(&av7110->ipack[1]);
>
> if (av7110->playing == RP_AV) {
> - av7110_fw_cmd(av7110, COMTYPE_REC_PLAY,
> + ret = av7110_fw_cmd(av7110, COMTYPE_REC_PLAY,
> __Play, 2, AV_PES, 0);
Add something like
if(ret)
break;
here.
> if (av7110->trickmode == TRICK_FAST)
> - av7110_fw_cmd(av7110, COMTYPE_REC_PLAY,
> + ret = av7110_fw_cmd(av7110, COMTYPE_REC_PLAY,
> __Scan_I, 2, AV_PES, 0);
> if (av7110->trickmode == TRICK_SLOW) {
> ...
> diff -up dvb-cvs/dvb-kernel/linux/drivers/media/dvb/ttpci/av7110.c linux/drivers/media/dvb/ttpci/av7110.c
> ...
> @@ -856,10 +875,12 @@ static int StopHWFilter(struct dvb_demux
> dprintk(4, "%p\n", av7110);
>
> handle = dvbdmxfilter->hw_handle;
> + if (handle == 0xffff) /* the filter has not really been started */
> + return 0;
Hmm - is it normal to get here with handle==0xffff?
Wouldn't it be better to have a log entry as it was before?
> @@ -1119,18 +1166,16 @@ static int av7110_set_tone(struct dvb_fr
>
> switch (tone) {
> case SEC_TONE_ON:
> - Set22K(av7110, 1);
> + return Set22K(av7110, 1);
> break;
You should remove the 'break' here.
> case SEC_TONE_OFF:
> - Set22K(av7110, 0);
> + return Set22K(av7110, 0);
> break;
ditto
av7110_fe_lock_fix() was (and is) broken. It would have never be retried
after -ERESTARTSYS. For a fix see below.
> -static void av7110_fe_lock_fix(struct av7110* av7110, fe_status_t status)
> +static int av7110_fe_lock_fix(struct av7110* av7110, fe_status_t status)
> {
> + int ret = 0;
> int synced = (status & FE_HAS_LOCK) ? 1 : 0;
>
> av7110->fe_status = status;
>
> if (av7110->fe_synced == synced)
> - return;
> + return 0;
>
> av7110->fe_synced = synced;
Delete the line above, and move it to the end.
> if (av7110->playing)
> - return;
> + return 0;
>
> if (down_interruptible(&av7110->pid_mutex))
> - return;
> + return -ERESTARTSYS;
>
> if (av7110->fe_synced) {
replace by 'if (synced) {'
> - SetPIDs(av7110, av7110->pids[DMX_PES_VIDEO],
> + ret = SetPIDs(av7110, av7110->pids[DMX_PES_VIDEO],
> av7110->pids[DMX_PES_AUDIO],
> av7110->pids[DMX_PES_TELETEXT], 0,
> av7110->pids[DMX_PES_PCR]);
> - av7110_fw_cmd(av7110, COMTYPE_PIDFILTER, Scan, 0);
> + if (!ret)
> + ret = av7110_fw_cmd(av7110, COMTYPE_PIDFILTER, Scan, 0);
> } else {
> - SetPIDs(av7110, 0, 0, 0, 0, 0);
> - av7110_fw_cmd(av7110, COMTYPE_PID_FILTER, FlushTSQueue, 0);
> - av7110_wait_msgstate(av7110, GPMQBusy);
> + ret = SetPIDs(av7110, 0, 0, 0, 0, 0);
> + if (!ret) {
> + ret = av7110_fw_cmd(av7110, COMTYPE_PID_FILTER, FlushTSQueue, 0);
> + if (!ret)
> + ret = av7110_wait_msgstate(av7110, GPMQBusy);
> + }
> }
>
Add here:
if (!ret)
av7110->fe_synced = synced;
> up(&av7110->pid_mutex);
>
> + return ret;
> }
> diff -up dvb-cvs/dvb-kernel/linux/drivers/media/dvb/ttpci/av7110_hw.h linux/drivers/media/dvb/ttpci/av7110_hw.h
> --- dvb-cvs/dvb-kernel/linux/drivers/media/dvb/ttpci/av7110_hw.h 2005-04-24 00:46:40.000000000 +0200
> +++ linux/drivers/media/dvb/ttpci/av7110_hw.h 2005-06-18 00:30:09.000000000 +0200
> @@ -453,14 +453,14 @@ static inline void ARM_ClearIrq(struct a
> * Firmware commands
> ****************************************************************************/
>
> -static inline int SendDAC(struct av7110 *av7110, u8 addr, u8 data)
> +static int inline SendDAC(struct av7110 *av7110, u8 addr, u8 data)
Hmm?
> -static inline void av7710_set_video_mode(struct av7110 *av7110, int mode)
> +static inline int av7710_set_video_mode(struct av7110 *av7110, int mode)
>
> ...
>
> -static inline void Set22K(struct av7110 *av7110, int state)
> +static int inline Set22K(struct av7110 *av7110, int state)
Either use 'inline int' or 'int inline'.
Imho we should keep the changes minimal.
Oliver
--
--------------------------------------------------------
VDR Remote Plugin available at
http://www.escape-edv.de/endriss/vdr/
--------------------------------------------------------
