Re: [PATCH RFC 3/3] dvb_frontend: do not allow statistic IOCTLs when sleeping

2012-08-12 Thread Mauro Carvalho Chehab 
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 cr...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC 3/3] dvb_frontend: do not allow statistic IOCTLs when sleeping

2012-08-12 Thread Antti Palosaari 

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 cr...@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


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

[PATCH RFC 3/3] dvb_frontend: do not allow statistic IOCTLs when sleeping

2012-08-09 Thread Antti Palosaari 
Demodulator cannot perform statistic IOCTLs when it is not tuned.
Return -EPERM in such case.

Signed-off-by: Antti Palosaari cr...@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;
 
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);
-- 
1.7.11.2

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