| Message ID | cover.1712846025.git.pnewman@connecttech.com |
|---|---|
| Headers | show |
| Series | serial: exar: add Connect Tech serial cards to Exar driver | expand |
On Thu, Apr 11, 2024 at 11:29:27AM -0400, parker@finest.io wrote: > From: Parker Newman <pnewman@connecttech.com> > > This patch adds support for most of Connect Tech's (CTI) PCI/PCIe > serial cards to the 8250_exar driver. > > Major changes: > - Added (optional) board_setup function called once during probe > - Added exar_get_nr_ports function > - Removed old Connect Tech setup functions > - Added new UART family based CTI setup functions > - Added PCI IDs entries for Exar XR17V25x and CTI FPGA based cards > - Added MPIO functions for configuring/setting a single MPIO > - Added support for reading from the Exar EEPROM When you need to list the different things you did in a single patch, that's a huge hint it needs to be broken up into smaller, easier to review, pieces. > > Signed-off-by: Parker Newman <pnewman@connecttech.com> > --- > drivers/tty/serial/8250/8250_exar.c | 1089 +++++++++++++++++++++++++-- > 1 file changed, 1019 insertions(+), 70 deletions(-) Reviewing 1000 lines of changes at once that does different things, isn't easy. Please break this up into smaller, one-logical-step-at-a-time patches. thanks, greg k-h
On Thu, 11 Apr 2024 17:55:54 +0200 Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > On Thu, Apr 11, 2024 at 11:29:27AM -0400, parker@finest.io wrote: > > From: Parker Newman <pnewman@connecttech.com> > > > > This patch adds support for most of Connect Tech's (CTI) PCI/PCIe > > serial cards to the 8250_exar driver. > > > > Major changes: > > - Added (optional) board_setup function called once during probe > > - Added exar_get_nr_ports function > > - Removed old Connect Tech setup functions > > - Added new UART family based CTI setup functions > > - Added PCI IDs entries for Exar XR17V25x and CTI FPGA based cards > > - Added MPIO functions for configuring/setting a single MPIO > > - Added support for reading from the Exar EEPROM > > When you need to list the different things you did in a single patch, > that's a huge hint it needs to be broken up into smaller, easier to > review, pieces. > > > > > Signed-off-by: Parker Newman <pnewman@connecttech.com> > > --- > > drivers/tty/serial/8250/8250_exar.c | 1089 +++++++++++++++++++++++++-- > > 1 file changed, 1019 insertions(+), 70 deletions(-) > > Reviewing 1000 lines of changes at once that does different things, > isn't easy. Please break this up into smaller, > one-logical-step-at-a-time patches. > > thanks, > > greg k-h I will try to split it up better and submit a v2. Thank you, - Parker
On Thu, Apr 11, 2024 at 12:22:48PM -0400, Parker Newman wrote: > On Thu, 11 Apr 2024 17:54:45 +0200 > Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > > On Thu, Apr 11, 2024 at 11:29:26AM -0400, parker@finest.io wrote: > > > From: Parker Newman <pnewman@connecttech.com> > > > > > > - Added missing CTI serial car PCI IDs > > > - Added missing Exar XR17V25X PCI IDs > > > - Moved XR17V4358 and XR17V8358 PCI ID defines to pci_ids.h > > > > Did you read the top of the pci_ids.h file? Don't add new ids there > > unless it is going to be needed in multiple files please. > > > > thanks, > > > > greg k-h > No sorry I missed that... I was just trying to consolidate them all > in one place. > > Would it be better to move all the Connect Tech sub-IDs from > pci_ids.h into 8250_exar.c? Or should I just add the missing ones > to 8250_exar.c instead of pci_ids.h? Just add the missing ones if you don't want to touch the pci_ids.h file, that way it's a simpler change. thanks, greg k-h
From: Parker Newman <pnewman@connecttech.com> Hello, These patches add proper support for most of Connect Tech's (CTI) Exar based serial cards. Previously, only a subset of CTI's cards would work with the Exar driver while the rest required the CTI out-of-tree driver. These patches are intended to phase out the out-of-tree driver. I am new to the mailing lists and contributing to the kernel so please let me know if I have made any mistakes or if you have any feedback. Thank you, Parker Newman (2): serial: exar: add missing CTI/Exar PCI IDs to include/linux/pci_ids.h serial: exar: adding CTI PCI/PCIe serial port support to 8250_exar drivers/tty/serial/8250/8250_exar.c | 1092 +++++++++++++++++++++++++-- include/linux/pci_ids.h | 104 ++- 2 files changed, 1094 insertions(+), 102 deletions(-) -- 2.43.2