Le 01/10/2022 à 08:15, Kumaravel Thiagarajan a écrit :
pci1xxxx is a PCIe switch with a multi-function endpoint on one of its
downstream ports. Quad-uart is one of the functions in the
multi-function endpoint. This driver loads for the quad-uart and
enumerates single or multiple instances of uart based on the PCIe
subsystem device ID.
Signed-off-by: Kumaravel Thiagarajan <kumaravel.thiagarajan@xxxxxxxxxxxxx>
---
[...]
+static int pci1xxxx_setup_port(struct pci1xxxx_8250 *priv, struct uart_8250_port *port,
+ int offset)
+{
+ struct pci_dev *dev = priv->dev;
+
+ if (pci_resource_flags(dev, 0) & IORESOURCE_MEM) {
+ if (!pcim_iomap(dev, 0, 0) && !pcim_iomap_table(dev))
+ return -ENOMEM;
+
+ port->port.iotype = UPIO_MEM;
+ port->port.iobase = 0;
+ port->port.mapbase = pci_resource_start(dev, 0) + offset;
+ port->port.membase = pcim_iomap_table(dev)[0] + offset;
Hi,
Is it needed to call pcim_iomap_table(dev) twice? (here and a few lines
above in the 'if')
+ port->port.regshift = 0;
+ } else {
+ port->port.iotype = UPIO_PORT;
+ port->port.iobase = pci_resource_start(dev, 0) + offset;
+ port->port.mapbase = 0;
+ port->port.membase = NULL;
+ port->port.regshift = 0;
+ }
+
+ return 0;
+}
[...]
+static int pci1xxxx_serial_probe(struct pci_dev *dev,
+ const struct pci_device_id *ent)
+{
+ struct pci1xxxx_8250 *priv;
+ struct uart_8250_port uart;
+ unsigned int nr_ports, i;
+ int num_vectors = 0;
+ int rc;
+
+ rc = pcim_enable_device(dev);
+ if (rc)
+ return rc;
+
+ nr_ports = pci1xxxx_get_num_ports(dev);
+
+ priv = devm_kzalloc(&dev->dev, struct_size(priv, line, nr_ports), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->membase = pcim_iomap(dev, 0, 0);
+ priv->dev = dev;
+ priv->nr = nr_ports;
+
+ pci_set_master(dev);
+
+ num_vectors = pci_alloc_irq_vectors(dev, 1, 4, PCI_IRQ_ALL_TYPES);
+ if (num_vectors < 0)
+ return num_vectors;
+
+ memset(&uart, 0, sizeof(uart));
+ uart.port.flags = UPF_SHARE_IRQ | UPF_FIXED_TYPE | UPF_FIXED_PORT;
+ uart.port.uartclk = 62500000;
+ uart.port.dev = &dev->dev;
+
+ if (num_vectors == 4)
+ writeb(UART_PCI_CTRL_SET_MULTIPLE_MSI, (priv->membase + UART_PCI_CTRL_REG));
+ else
+ uart.port.irq = pci_irq_vector(dev, 0);
+
+ for (i = 0; i < nr_ports; i++) {
+ if (num_vectors == 4)
+ pci1xxxx_irq_assign(priv, &uart, i);
+ priv->line[i] = -ENOSPC;
+ rc = pci1xxxx_setup(priv, &uart, i);
+ if (rc) {
+ dev_err(&dev->dev, "Failed to setup port %u\n", i);
+ break;
+ }
+ priv->line[i] = serial8250_register_8250_port(&uart);
In case of error, this should be undone in an error handling path in the
probe, as done in the remove() function below.
If we break, we still continue and return success. But the last
priv->line[i] are still 0. Is it an issue when pci1xxxx_serial_remove()
is called?
+
+ if (priv->line[i] < 0) {
+ dev_err(&dev->dev,
+ "Couldn't register serial port %lx, irq %d, type %d, error %d\n",
+ uart.port.iobase, uart.port.irq,
+ uart.port.iotype, priv->line[i]);
+ break;
+ }
+ }
+
+ pci_set_drvdata(dev, priv);
+
+ return 0;
+}
+
+static void pci1xxxx_serial_remove(struct pci_dev *dev)
+{
+ struct pci1xxxx_8250 *priv = pci_get_drvdata(dev);
+ int i;
+
+ for (i = 0; i < priv->nr; i++) {
+ if (priv->line[i] >= 0)
+ serial8250_unregister_port(priv->line[i]);
+ }
+}
+
[...]
