2017-03-20 21:04 GMT+09:00 walter harms <wharms@xxxxxx>:
>
>
> Am 20.03.2017 11:59, schrieb Daeseok Youn:
>> If v4l2_subdev_call() gets the global frame interval values,
>> it returned 0 and it could be checked whether numerator is zero or not.
>>
>> If the numerator is not zero, the fps could be calculated in this function.
>> If not, it just returns 0.
>>
>> Signed-off-by: Daeseok Youn <daeseok.youn@xxxxxxxxx>
>> ---
>> .../media/atomisp/pci/atomisp2/atomisp_cmd.c | 22 ++++++++++------------
>> 1 file changed, 10 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
>> index 8bdb224..6bdd19e 100644
>> --- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
>> +++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
>> @@ -153,20 +153,18 @@ struct atomisp_acc_pipe *atomisp_to_acc_pipe(struct video_device *dev)
>>
>> static unsigned short atomisp_get_sensor_fps(struct atomisp_sub_device *asd)
>> {
>> - struct v4l2_subdev_frame_interval frame_interval;
>> + struct v4l2_subdev_frame_interval fi;
>> struct atomisp_device *isp = asd->isp;
>> - unsigned short fps;
>>
>> - if (v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
>> - video, g_frame_interval, &frame_interval)) {
>> - fps = 0;
>> - } else {
>> - if (frame_interval.interval.numerator)
>> - fps = frame_interval.interval.denominator /
>> - frame_interval.interval.numerator;
>> - else
>> - fps = 0;
>> - }
>> + unsigned short fps = 0;
>> + int ret;
>> +
>> + ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
>> + video, g_frame_interval, &fi);
>> +
>> + if (!ret && fi.interval.numerator)
>> + fps = fi.interval.denominator / fi.interval.numerator;
>> +
>> return fps;
>> }
>
>
>
> do you need to check ret at all ? if an error occurs can fi.interval.numerator
> be something else than 0 ?
the return value from the v4l2_subdev_call() function is zero when it
is done without any error. and also I checked
the ret value whether is 0 or not. if the ret is 0 then the value of
numerator should be checked to avoid for dividing by 0.
>
> if ret is an ERRNO it would be wise to return ret not fps, but this may require
> changes at other places also.
hmm.., yes, you are right. but I think it is ok because the
atomisp_get_sensor_fps() function is needed to get fps value.
(originally, zero or calculated fps value was returned.)
>
> re,
> wh
>
>>
