Hello Pavel, thanks for review. On Thursday 22 of October 2020 13:39:52 Pavel Machek wrote: > Hi! > > > @@ -12,4 +12,13 @@ config CAN_CTUCANFD > > > > if CAN_CTUCANFD > > > > +config CAN_CTUCANFD_PCI > > + tristate "CTU CAN-FD IP core PCI/PCIe driver" > > + depends on PCI > > + help > > + This driver adds PCI/PCIe support for CTU CAN-FD IP core. > > + The project providing FPGA design for Intel EP4CGX15 based DB4CGX15 > > + PCIe board with PiKRON.com designed transceiver riser shield is > > available + at https://gitlab.fel.cvut.cz/canbus/pcie-ctu_can_fd . > > + > > endif > > Ok, now the if in the first patch makes sense. It can stay. > > And it is separate module, so EXPORT_SYMBOLs make sense. Ok. Great. > > +#ifndef PCI_VENDOR_ID_TEDIA > > +#define PCI_VENDOR_ID_TEDIA 0x1760 > > +#endif > > > > +#define PCI_DEVICE_ID_ALTERA_CTUCAN_TEST 0xCAFD > > +#define PCI_DEVICE_ID_TEDIA_CTUCAN_VER21 0xff00 > > These should go elsewhere. They should propagate somehow from https://pci-ids.ucw.cz/read/PC/1760/ff00 We have registered them long time ago. I am not sure what is right mechanism. > > +#ifndef PCI_VENDOR_ID_TEDIA > > +#define PCI_VENDOR_ID_TEDIA 0x1760 > > +#endif So this one should be known to kernel globally, but I would be happy if driver build even if global process to introduce define did not proceed end even backports would be required for long time until kernel including CTU CAN FD propagates into distributions, and industrial systems distributions lag often a lot > > +#define PCI_DEVICE_ID_ALTERA_CTUCAN_TEST 0xCAFD We drop this, I hope we have no system running old test version of the core integration before Tedia offered us to reserve some IDs (promissed that they would never use them in future) for us. > > +#define PCI_DEVICE_ID_TEDIA_CTUCAN_VER21 0xff00 This should propagate into kernel from registry or at least match registry. > > +static bool use_msi = 1; > > +static bool pci_use_second = 1; > > true? Done Best wishes, Pavel