Received-SPF: pass (google.com: domain of linux-kernel+bounces-67567-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Thu, 15 Feb 2024 21:21:49 +0200
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Florian Fainelli <florian.fainelli@broadcom.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org,
	Al Cooper <alcooperx@gmail.com>,
	Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>,
	Jiri Slaby <jirislaby@kernel.org>
Subject: Re: [PATCH v1 1/1] serial: 8250_bcm7271: Replace custom unit
 definitions
Message-ID: <Zc5kTdkTqXEE6cA3@smile.fi.intel.com>
References: <20240215160234.653305-1-andriy.shevchenko@linux.intel.com>
 <7132d989-533f-47ef-803b-6001de3aff33@broadcom.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <7132d989-533f-47ef-803b-6001de3aff33@broadcom.com>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo

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