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]
Date: Wed, 14 Feb 2024 11:14:35 +0000
From: Sakari Ailus <sakari.ailus@...ux.intel.com>
To: Tommaso Merciai <tomm.merciai@...il.com>
Cc: Laurent Pinchart <laurent.pinchart@...asonboard.com>,
	linuxfancy@...glegroups.com, martin.hecht@...et.eu,
	Mauro Carvalho Chehab <mchehab@...nel.org>,
	linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 5/5] media: i2c: alvium: fix req_fr check into
 alvium_s_frame_interval()

Hi Tommaso,

On Wed, Dec 20, 2023 at 02:52:53PM +0100, Tommaso Merciai wrote:
> Hi Laurent,
> 
> On Wed, Dec 20, 2023 at 03:02:36PM +0200, Laurent Pinchart wrote:
> > Hi Tommaso,
> > 
> > Thank you for the patch.
> > 
> > On Wed, Dec 20, 2023 at 01:40:23PM +0100, Tommaso Merciai wrote:
> > > Actually req_fr check into alvium_s_frame_interval() is wrong.
> > > In particular req_fr can't be >=max and <= min at the same time.
> > > Fix this using clamp and remove dft_fr parameter from
> > > alvium_get_frame_interval() not more used.
> > 
> > The commit message should have explained why clamping is better than
> > picking a default value, as that's a functional change. If you propose
> > an updated commit message in a reply, I think Sakari can update the
> > patch when applying the series to his tree, there's no need for a v4.
> 
> What about:
> 
> Actually req_fr check into alvium_s_frame_interval() is wrong.
> In particular req_fr can't be >=max and <= min at the same time.
> Fix this using clamp and remove dft_fr parameter from
> alvium_get_frame_interval() not more used.
> 
> Clamp function make sure that if the setted value exceeds the limits is
> replaced with min_fr/max_fr instead of setting the value readed back
> from the hw.
> 
> What do you think?

I used this, hopefully it's ok:

media: i2c: alvium: fix req_fr check in alvium_s_frame_interval()

req_fr check in alvium_s_frame_interval() is incorrect. In particular
req_fr can't be >=max and <= min at the same time. Ensure the requested
frame rate remains within the supported range between min_fr and max_fr by
clamping it.

Also remove the unused dft_fr argument of alvium_get_frame_interval().

-- 
Kind regards,

Sakari Ailus

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ