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: Thu, 15 Feb 2024 21:21:49 +0200
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Florian Fainelli <florian.fainelli@...adcom.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-serial@...r.kernel.org, linux-kernel@...r.kernel.org,
	Al Cooper <alcooperx@...il.com>,
	Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>,
	Jiri Slaby <jirislaby@...nel.org>
Subject: Re: [PATCH v1 1/1] serial: 8250_bcm7271: Replace custom unit
 definitions

On Thu, Feb 15, 2024 at 10:35:11AM -0800, Florian Fainelli wrote:
> On 2/15/24 08:02, Andy Shevchenko wrote:

..

> > -#define KHZ    1000
> > -#define MHZ(x) ((x) * KHZ * KHZ)
> >   static const u32 brcmstb_rate_table[] = {
> > -	MHZ(81),
> > -	MHZ(108),
> > -	MHZ(64),		/* Actually 64285715 for some chips */
> > -	MHZ(48),
> > +	81 * HZ_PER_MHZ,
> > +	108 * HZ_PER_MHZ,
> > +	64 * HZ_PER_MHZ,		/* Actually 64285715 for some chips */
> > +	48 * HZ_PER_MHZ,
> 
> The previous notation was IMHO more readable,

I tend to disagree as we read in plain text "frequency is 64 MHz",
the patch follows natural language.

> can we meet in the middle and do:
> 
> #define MHZ(x)	((x) * HZ_PER_MHZ
> 
> and avoid touching the tables entirely?

I don't like the intermediate layer which hides the implementation of MHZ().
What does it do exactly? You need to look at the internals, with the patch
applied you immediately see that these are just constants.

-- 
With Best Regards,
Andy Shevchenko



Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ