lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20191003073155.4sl3jkmobnm53tm2@uno.localdomain>
Date:   Thu, 3 Oct 2019 09:31:55 +0200
From:   Jacopo Mondi <jacopo@...ndi.org>
To:     Benoit Parrot <bparrot@...com>
Cc:     Hans Verkuil <hverkuil@...all.nl>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        linux-media@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [Patch v2 3/3] media: ov5640: Make 2592x1944 mode only available
 at 15 fps

Hi Benoit,

On Wed, Oct 02, 2019 at 08:51:34AM -0500, Benoit Parrot wrote:
> The sensor data sheet clearly state that 2592x1944 only works at 15 fps
> make sure we don't try to miss configure the pll out of acceptable
> range.

The datasheet clearly indicates that 15 fps is the maximum achievable
rate with that resolution, so I guess preventing it from being set
to anything faster than that it's acceptable.
>
> Signed-off-by: Benoit Parrot <bparrot@...com>
> ---
>  drivers/media/i2c/ov5640.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 103a4e8f88e1..d5b0be2c7a0a 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -1613,6 +1613,11 @@ ov5640_find_mode(struct ov5640_dev *sensor, enum ov5640_frame_rate fr,
>  	    !(mode->hact == 640 && mode->vact == 480))
>  		return NULL;
>
> +	/* 2592x1944 only works at 15fps */
> +	if (fr != OV5640_15_FPS &&

As long as 15 fps is the lower framerate declared in
ov5640_framerates[] this is ok, but I would make this condition a
check for "fr  > OV5640_15_FPS" so that it's safe for future
extensions.

(And I would check for the resolution first then FPS, so you check
the most unlikely condition first, but that's really a minor
optimization).

With the above small details addressed
Reviewed-by: Jacopo Mondi <jacopo@...ndi.org>

Thanks
   j

> +	    (mode->hact == 2592 && mode->vact == 1944))
> +		return NULL;
> +
>  	return mode;
>  }
>
> --
> 2.17.1
>

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ