On Fri, Apr 12, 2024 at 11:19:26AM -0400, Parker Newman wrote: > On Fri, 12 Apr 2024 13:57:01 +0300 (EEST) > Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx> wrote: > > > On Thu, 11 Apr 2024, parker@xxxxxxxxx wrote: > > > > > From: Parker Newman <pnewman@xxxxxxxxxxxxxxx> > > > > > > - Removed old port setup function and replaced with UART specific ones > > > - Added board setup functions for CTI boards > > > - Replaced CONNECT_DEVICE macro with CTI_EXAR_DEVICE and CTI_PCI_DEVICE > > > > In general, you should try to do refactoring in a preparatory patch (one > > refactoring thing at a time) and add new stuff in another patch in > > the series. I didn't go to figure out how much it applies to those three > > items because you likely know the answer immediately. > > > > > - Moved "generic rs485" support up in the file > > > > Please do this in a separate patch. > > > > Will do. > > > > > Another general level problem with your series is that it adds functions > > x, y, etc. without users, whereas the expected way of doing things would > > be to add the functions in the change they are getting used so it's easier > > to follow what's going on. > > > > I believe if you separate the refactoring & moving code around into own > > changes (no functional change type patches), the new stuff is much > > smaller so there is no need to split that illogically into incomplete > > fragments in some patches. > > > > -- > > i. > > > > Thanks for the feedback, I am new to the mailing lists and am trying to balance > what you mention above with not having giant patches. It's a fine line, and takes a while to learn, but as a first cut, this was pretty good, I didn't have any major problems with the structure of it, so nice work. thanks, greg k-h
