Messages in this thread | | | Date | Thu, 15 Feb 2024 21:40:20 +0200 | From | Andy Shevchenko <> | Subject | Re: [PATCH v1 1/1] serial: 8250_bcm7271: Replace custom unit definitions |
| |
On Thu, Feb 15, 2024 at 11:27:39AM -0800, Florian Fainelli wrote: > On 2/15/24 11:21, Andy Shevchenko wrote: > > 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. > > OK, I suppose today's color is blue for the bike shed.
Sky is blue and sun is shining and I am happy person :-)
> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Thank you!
-- With Best Regards, Andy Shevchenko
| |