> From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > Sent: Sunday, December 11, 2022 2:45 AM > To: Kumaravel Thiagarajan - I21417 > <Kumaravel.Thiagarajan@xxxxxxxxxxxxx> > Subject: Re: [PATCH v8 tty-next 2/4] serial: 8250_pci1xxxx: Add driver for > quad-uart support > > > + 4,/* PCI1p3 */ > > + }; > > If you move this outside of the function you may use static_assert(), see > below why. If you move this outside of the function -> Do you suggest to move the array outside the function and make it global? > > if (subsys_dev <= ARRAY_SIZE(max_port)) > return max_port[subsys_dev]; > > (in this case you can make sure it is the same as the above using > static_assert(), so it won't compile otherwise) I am not getting this. You suggest doing something like this: static_assert(subsys_dev <= ARRAY_SIZE(max_port)) ? > > + priv->membase = pcim_iomap(pdev, 0, 0); > > As you said there will be no hardware that uses IO ports, why do you need > pci_iomap()? I guess what you may use pci_ioremap_bar(). > > > + if (!priv->membase) > > + return -ENOMEM; Okay, I will use pci_ioremap_bar(pdev, 0) > > + priv->nr = nr_ports; > > + pci_set_master(pdev); > > + max_vec_reqd = pci1xxxx_get_max_port(subsys_dev); > > The above needs a bit of reshuffling and perhaps a blank line or lines. > Make it ordered and grouped more logically. Okay. I will do something like this: pci_set_master(pdev); <NL> priv->pdev = pdev; priv->nr = nr_ports; <NL> subsys_dev = priv->pdev->subsystem_device; max_vec_reqd = pci1xxxx_get_max_port(subsys_dev); <NL> num_vectors = pci_alloc_irq_vectors(pdev, 1, max_vec_reqd, PCI_IRQ_ALL_TYPES); if (num_vectors < 0) return num_vectors; > Can be simplified a bit by PCI_VDEVICE(). Okay. Thanks, Tharun Kumar P
