[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ZeWr06YWj5cDHfWL@smile.fi.intel.com>
Date: Mon, 4 Mar 2024 13:09:07 +0200
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Thomas Gleixner <tglx@...utronix.de>, linux-kernel@...r.kernel.org,
linux-serial@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-aspeed@...ts.ozlabs.org, linux-rpi-kernel@...ts.infradead.org,
linux-mips@...r.kernel.org, linux-tegra@...r.kernel.org,
Jiri Slaby <jirislaby@...nel.org>, Joel Stanley <joel@....id.au>,
Andrew Jeffery <andrew@...econstruct.com.au>,
Florian Fainelli <florian.fainelli@...adcom.com>,
Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>,
Ray Jui <rjui@...adcom.com>, Scott Branden <sbranden@...adcom.com>,
Al Cooper <alcooperx@...il.com>,
Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
Paul Cercueil <paul@...pouillou.net>,
Vladimir Zapolskiy <vz@...ia.com>,
Thierry Reding <thierry.reding@...il.com>,
Jonathan Hunter <jonathanh@...dia.com>,
Kunihiko Hayashi <hayashi.kunihiko@...ionext.com>,
Masami Hiramatsu <mhiramat@...nel.org>
Subject: Re: [PATCH v2 03/14] serial: port: Introduce a common helper to read
properties
On Sat, Mar 02, 2024 at 09:58:53PM +0100, Greg Kroah-Hartman wrote:
> On Mon, Feb 26, 2024 at 04:19:19PM +0200, Andy Shevchenko wrote:
..
> > + * uart_read_port_properties - read firmware properties of the given UART port
>
> I like, but:
>
> > + * @port: corresponding port
> > + * @use_defaults: apply defaults (when %true) or validate the values (when %false)
>
> Using random booleans in a function is horrid. Every time you see the
> function call, or want to call it, you need to go and look up what the
> boolean is and means.
>
> Make 2 public functions here, one that does it with use_defaults=true
> and one =false and then have them both call this one static function,
> that way the function names themselves are easy to read and understand
> and maintain over time.
Okay! I'll redo that.
..
> > +EXPORT_SYMBOL(uart_read_port_properties);
>
> EXPORT_SYMBOL_GPL()? I have to ask :)
No clue, the rest in this file is EXPORT_SYMBOL, but I admit I followed the
cargo cult. I'll check the modified code and see if I may use _GPL version.
Thank you for review!
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists