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
