| Message ID | 1344551101-16700-4-git-send-email-crope@iki.fi (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Em 09-08-2012 19:25, Antti Palosaari escreveu: > Demodulator cannot perform statistic IOCTLs when it is not tuned. > Return -EPERM in such case. While, in general, doing it makes sense, -EPERM is a very bad return code. It is used to indicate when accessing some resources would require root access. > > Signed-off-by: Antti Palosaari <crope@iki.fi> > --- > drivers/media/dvb/dvb-core/dvb_frontend.c | 34 +++++++++++++++++++++++-------- > 1 file changed, 25 insertions(+), 9 deletions(-) > > diff --git a/drivers/media/dvb/dvb-core/dvb_frontend.c b/drivers/media/dvb/dvb-core/dvb_frontend.c > index 4fc11eb..40efcde 100644 > --- a/drivers/media/dvb/dvb-core/dvb_frontend.c > +++ b/drivers/media/dvb/dvb-core/dvb_frontend.c > @@ -2157,27 +2157,43 @@ static int dvb_frontend_ioctl_legacy(struct file *file, > err = fe->ops.read_status(fe, status); > break; > } > + > case FE_READ_BER: > - if (fe->ops.read_ber) > - err = fe->ops.read_ber(fe, (__u32*) parg); > + if (fe->ops.read_ber) { > + if (fepriv->thread) > + err = fe->ops.read_ber(fe, (__u32 *) parg); > + else > + err = -EPERM; > + } > break; > > case FE_READ_SIGNAL_STRENGTH: > - if (fe->ops.read_signal_strength) > - err = fe->ops.read_signal_strength(fe, (__u16*) parg); > + if (fe->ops.read_signal_strength) { > + if (fepriv->thread) > + err = fe->ops.read_signal_strength(fe, (__u16 *) parg); > + else > + err = -EPERM; > + } > break; Signal strength can still be available even without locking. > > case FE_READ_SNR: > - if (fe->ops.read_snr) > - err = fe->ops.read_snr(fe, (__u16*) parg); > + if (fe->ops.read_snr) { > + if (fepriv->thread) > + err = fe->ops.read_snr(fe, (__u16 *) parg); > + else > + err = -EPERM; > + } > break; > > case FE_READ_UNCORRECTED_BLOCKS: > - if (fe->ops.read_ucblocks) > - err = fe->ops.read_ucblocks(fe, (__u32*) parg); > + if (fe->ops.read_ucblocks) { > + if (fepriv->thread) > + err = fe->ops.read_ucblocks(fe, (__u32 *) parg); > + else > + err = -EPERM; > + } > break; > > - > case FE_DISEQC_RESET_OVERLOAD: > if (fe->ops.diseqc_reset_overload) { > err = fe->ops.diseqc_reset_overload(fe); > 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
On 08/12/2012 06:28 PM, Mauro Carvalho Chehab wrote: > Em 09-08-2012 19:25, Antti Palosaari escreveu: >> Demodulator cannot perform statistic IOCTLs when it is not tuned. >> Return -EPERM in such case. > > While, in general, doing it makes sense, -EPERM is a very bad return code. > It is used to indicate when accessing some resources would require root access. OK, makes sense. As I mentioned in coder letter I selected that due to V4L2 usage to keep consistent. VIDIOC_DECODER_CMD, VIDIOC_TRY_DECODER_CMD VIDIOC_ENCODER_CMD, VIDIOC_TRY_ENCODER_CMD Cover letter also lists all the other error codes I found suitable. Which one you prefer? >> Signed-off-by: Antti Palosaari <crope@iki.fi> >> --- >> drivers/media/dvb/dvb-core/dvb_frontend.c | 34 +++++++++++++++++++++++-------- >> 1 file changed, 25 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/media/dvb/dvb-core/dvb_frontend.c b/drivers/media/dvb/dvb-core/dvb_frontend.c >> index 4fc11eb..40efcde 100644 >> --- a/drivers/media/dvb/dvb-core/dvb_frontend.c >> +++ b/drivers/media/dvb/dvb-core/dvb_frontend.c >> @@ -2157,27 +2157,43 @@ static int dvb_frontend_ioctl_legacy(struct file *file, >> err = fe->ops.read_status(fe, status); >> break; >> } >> + >> case FE_READ_BER: >> - if (fe->ops.read_ber) >> - err = fe->ops.read_ber(fe, (__u32*) parg); >> + if (fe->ops.read_ber) { >> + if (fepriv->thread) >> + err = fe->ops.read_ber(fe, (__u32 *) parg); >> + else >> + err = -EPERM; >> + } >> break; >> >> case FE_READ_SIGNAL_STRENGTH: >> - if (fe->ops.read_signal_strength) >> - err = fe->ops.read_signal_strength(fe, (__u16*) parg); >> + if (fe->ops.read_signal_strength) { >> + if (fepriv->thread) >> + err = fe->ops.read_signal_strength(fe, (__u16 *) parg); >> + else >> + err = -EPERM; >> + } >> break; > > Signal strength can still be available even without locking. So it is correct. :) It checks if frontend thread is running, not demodulator lock flags. Actually, my original plan was to use demod lock flags but after looking various demod drivers I ended-up conclusion it is not wise. Many demod drivers just set all flags at the same time when there is full lock gained and never provide more accurate info - all or nothing. Anyhow, that solution prevents I/O errors when demod is so deep sleep state (like reset) it cannot even answer at all. >> case FE_READ_SNR: >> - if (fe->ops.read_snr) >> - err = fe->ops.read_snr(fe, (__u16*) parg); >> + if (fe->ops.read_snr) { >> + if (fepriv->thread) >> + err = fe->ops.read_snr(fe, (__u16 *) parg); >> + else >> + err = -EPERM; >> + } >> break; >> >> case FE_READ_UNCORRECTED_BLOCKS: >> - if (fe->ops.read_ucblocks) >> - err = fe->ops.read_ucblocks(fe, (__u32*) parg); >> + if (fe->ops.read_ucblocks) { >> + if (fepriv->thread) >> + err = fe->ops.read_ucblocks(fe, (__u32 *) parg); >> + else >> + err = -EPERM; >> + } >> break; >> >> - >> case FE_DISEQC_RESET_OVERLOAD: >> if (fe->ops.diseqc_reset_overload) { >> err = fe->ops.diseqc_reset_overload(fe); >> > > Regards, > Mauro regards Antti
diff --git a/drivers/media/dvb/dvb-core/dvb_frontend.c b/drivers/media/dvb/dvb-core/dvb_frontend.c index 4fc11eb..40efcde 100644 --- a/drivers/media/dvb/dvb-core/dvb_frontend.c +++ b/drivers/media/dvb/dvb-core/dvb_frontend.c @@ -2157,27 +2157,43 @@ static int dvb_frontend_ioctl_legacy(struct file *file, err = fe->ops.read_status(fe, status); break; } + case FE_READ_BER: - if (fe->ops.read_ber) - err = fe->ops.read_ber(fe, (__u32*) parg); + if (fe->ops.read_ber) { + if (fepriv->thread) + err = fe->ops.read_ber(fe, (__u32 *) parg); + else + err = -EPERM; + } break; case FE_READ_SIGNAL_STRENGTH: - if (fe->ops.read_signal_strength) - err = fe->ops.read_signal_strength(fe, (__u16*) parg); + if (fe->ops.read_signal_strength) { + if (fepriv->thread) + err = fe->ops.read_signal_strength(fe, (__u16 *) parg); + else + err = -EPERM; + } break; case FE_READ_SNR: - if (fe->ops.read_snr) - err = fe->ops.read_snr(fe, (__u16*) parg); + if (fe->ops.read_snr) { + if (fepriv->thread) + err = fe->ops.read_snr(fe, (__u16 *) parg); + else + err = -EPERM; + } break; case FE_READ_UNCORRECTED_BLOCKS: - if (fe->ops.read_ucblocks) - err = fe->ops.read_ucblocks(fe, (__u32*) parg); + if (fe->ops.read_ucblocks) { + if (fepriv->thread) + err = fe->ops.read_ucblocks(fe, (__u32 *) parg); + else + err = -EPERM; + } break; - case FE_DISEQC_RESET_OVERLOAD: if (fe->ops.diseqc_reset_overload) { err = fe->ops.diseqc_reset_overload(fe);
Demodulator cannot perform statistic IOCTLs when it is not tuned. Return -EPERM in such case. Signed-off-by: Antti Palosaari <crope@iki.fi> --- drivers/media/dvb/dvb-core/dvb_frontend.c | 34 +++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 9 deletions(-)