Skip to contentSkip to conversation

tty: serial: Replace deprecated PCI API

PATCH In review Philipp Stanner 4 participants 7 messages Nov 26, 2025 – Dec 18, 2025 TTY layer and serial drivers

lore
Share & tools
lore

Philipp Stanner

author
#0 lore raw

PATCH 1/1 tty: serial: Replace deprecated PCI API

pcim_iomap_table() is deprecated. Moreover, its special usage in 8250, causes a WARN_ON to fire (in pcim_add_mapping_to_legacy_table()).

8250's function serial8250_pci_setup_port() effectively maps the same BAR multiple times and adds an offset to the start address. While mapping and adding offsets is not a bug, it can be achieved in a far more straightforward way by using the specialized function pcim_iomap_range().

pcim_iomap_range() does not add the mapping addresses to the deprecated iomap table - that's not a problem, however, because non of the users of serial8250_pci_setup_port() uses pcim_iomap_table().

Replace the deprecated PCI functions with pcim_iomap_range().

Cc Guenter Roeck <[email protected]>
Link https://lore.kernel.org/dri-devel/[email protected]/ open here
Signed-off-by Philipp Stanner <[email protected]>

---
@Guenther: Can you please test this? I hope it fixes your issue.

---
 drivers/tty/serial/8250/8250_pcilib.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
drivers/tty/serial/8250/8250_pcilib.c +5 -4
index d8d0ae0d7238..f98eb2ab1005 100644
@@ -28,13 +28,14 @@ int serial8250_pci_setup_port(struct pci_dev *dev, struct uart_8250_port *port,
 		return -EINVAL;
 
 	if (pci_resource_flags(dev, bar) & IORESOURCE_MEM) {
-		if (!pcim_iomap(dev, bar, 0) && !pcim_iomap_table(dev))
-			return -ENOMEM;
-
 		port->port.iotype = UPIO_MEM;
 		port->port.iobase = 0;
 		port->port.mapbase = pci_resource_start(dev, bar) + offset;
-		port->port.membase = pcim_iomap_table(dev)[bar] + offset;
+
+		port->port.membase = pcim_iomap_range(dev, bar, offset, 0);
+		if (IS_ERR(port->port.membase))
+			return PTR_ERR(port->port.membase);
+
 		port->port.regshift = regshift;
 	} else if (IS_ENABLED(CONFIG_HAS_IOPORT)) {
 		port->port.iotype = UPIO_PORT;
Signature · 1 line
2.49.0
Tested-by on v1 1/1
#1 lore raw
Quoted 13 lines · Philipp Stanner

pcim_iomap_table() is deprecated. Moreover, its special usage in 8250, causes a WARN_ON to fire (in pcim_add_mapping_to_legacy_table()).

8250's function serial8250_pci_setup_port() effectively maps the same BAR multiple times and adds an offset to the start address. While mapping and adding offsets is not a bug, it can be achieved in a far more straightforward way by using the specialized function pcim_iomap_range().

pcim_iomap_range() does not add the mapping addresses to the deprecated iomap table - that's not a problem, however, because non of the users of serial8250_pci_setup_port() uses pcim_iomap_table().

Quoted 7 lines

Replace the deprecated PCI functions with pcim_iomap_range().

Cc Guenter Roeck <[email protected]>
Link https://lore.kernel.org/dri-devel/[email protected]/ open here
Signed-off-by Philipp Stanner <[email protected]>

---
@Guenther: Can you please test this? I hope it fixes your issue.

Yes, it does. Thanks a lot for fixing this!

Tested-by Guenter Roeck <[email protected]>
Quoted 27 lines
---
  drivers/tty/serial/8250/8250_pcilib.c | 9 +++++----
  1 file changed, 5 insertions(+), 4 deletions(-)
drivers/tty/serial/8250/8250_pcilib.c +5 -4
index d8d0ae0d7238..f98eb2ab1005 100644
@@ -28,13 +28,14 @@ int serial8250_pci_setup_port(struct pci_dev *dev, struct uart_8250_port *port,
  		return -EINVAL;
 
  	if (pci_resource_flags(dev, bar) & IORESOURCE_MEM) {
-		if (!pcim_iomap(dev, bar, 0) && !pcim_iomap_table(dev))
-			return -ENOMEM;
-
  		port->port.iotype = UPIO_MEM;
  		port->port.iobase = 0;
  		port->port.mapbase = pci_resource_start(dev, bar) + offset;
-		port->port.membase = pcim_iomap_table(dev)[bar] + offset;
+
+		port->port.membase = pcim_iomap_range(dev, bar, offset, 0);
+		if (IS_ERR(port->port.membase))
+			return PTR_ERR(port->port.membase);
+
  		port->port.regshift = regshift;
  	} else if (IS_ENABLED(CONFIG_HAS_IOPORT)) {
  		port->port.iotype = UPIO_PORT;
#2 lore raw
Quoted 22 lines · Guenter Roeck
Quoted 13 lines · Philipp Stanner

pcim_iomap_table() is deprecated. Moreover, its special usage in 8250, causes a WARN_ON to fire (in pcim_add_mapping_to_legacy_table()).

8250's function serial8250_pci_setup_port() effectively maps the same BAR multiple times and adds an offset to the start address. While mapping and adding offsets is not a bug, it can be achieved in a far more straightforward way by using the specialized function pcim_iomap_range().

pcim_iomap_range() does not add the mapping addresses to the deprecated iomap table - that's not a problem, however, because non of the users of serial8250_pci_setup_port() uses pcim_iomap_table().

Quoted 7 lines

Replace the deprecated PCI functions with pcim_iomap_range().

Cc Guenter Roeck <[email protected]>
Link https://lore.kernel.org/dri-devel/[email protected]/ open here
Signed-off-by Philipp Stanner <[email protected]>

---
@Guenther: Can you please test this? I hope it fixes your issue.

Quoted 3 lines

Yes, it does. Thanks a lot for fixing this!

Tested-by Guenter Roeck <[email protected]>

@Greg:
Can you apply this?

P.

Quoted 29 lines
Quoted 28 lines

---
  drivers/tty/serial/8250/8250_pcilib.c | 9 +++++----

  1 file changed, 5 insertions(+), 4 deletions(-)
drivers/tty/serial/8250/8250_pcilib.c +0 -0
index d8d0ae0d7238..f98eb2ab1005 100644
@@ -28,13 +28,14 @@ int serial8250_pci_setup_port(struct pci_dev *dev, struct uart_8250_port *port,
   		return -EINVAL;


   if (pci_resource_flags(dev, bar) & IORESOURCE_MEM) {

drivers/tty/serial/8250/8250_pcilib.c +0 -3
-		if (!pcim_iomap(dev, bar, 0) && !pcim_iomap_table(dev))
-			return -ENOMEM;
-

   port->port.iotype = UPIO_MEM;
   port->port.iobase = 0;
   port->port.mapbase = pci_resource_start(dev, bar) + offset;

drivers/tty/serial/8250/8250_pcilib.c +5 -1
-		port->port.membase = pcim_iomap_table(dev)[bar] + offset;
+
+		port->port.membase = pcim_iomap_range(dev, bar, offset, 0);
+		if (IS_ERR(port->port.membase))
+			return PTR_ERR(port->port.membase);
+

   port->port.regshift = regshift; } else if (IS_ENABLED(CONFIG_HAS_IOPORT)) { port->port.iotype = UPIO_PORT;

maintainer
#3 lore raw
Quoted 23 lines · Philipp Stanner
Quoted 22 lines · Guenter Roeck
Quoted 20 lines · Philipp Stanner

pcim_iomap_table() is deprecated. Moreover, its special usage in 8250, causes a WARN_ON to fire (in pcim_add_mapping_to_legacy_table()).

8250's function serial8250_pci_setup_port() effectively maps the same BAR multiple times and adds an offset to the start address. While mapping and adding offsets is not a bug, it can be achieved in a far more straightforward way by using the specialized function pcim_iomap_range().

pcim_iomap_range() does not add the mapping addresses to the deprecated iomap table - that's not a problem, however, because non of the users of serial8250_pci_setup_port() uses pcim_iomap_table().

Replace the deprecated PCI functions with pcim_iomap_range().

Cc Guenter Roeck <[email protected]>
Link https://lore.kernel.org/dri-devel/[email protected]/ open here
Signed-off-by Philipp Stanner <[email protected]>

---
@Guenther: Can you please test this? I hope it fixes your issue.

Quoted 6 lines
Quoted 3 lines

Yes, it does. Thanks a lot for fixing this!

Tested-by Guenter Roeck <[email protected]>

@Greg:
Can you apply this?

Does not apply at all to 6.19-rc1 :(

Can you please rebase and resend?

thanks,

greg k-h

#4 lore raw
Quoted 24 lines · Greg Kroah-Hartman
Quoted 23 lines · Philipp Stanner
Quoted 22 lines · Guenter Roeck
Quoted 20 lines · Philipp Stanner

pcim_iomap_table() is deprecated. Moreover, its special usage in 8250, causes a WARN_ON to fire (in pcim_add_mapping_to_legacy_table()).

8250's function serial8250_pci_setup_port() effectively maps the same BAR multiple times and adds an offset to the start address. While mapping and adding offsets is not a bug, it can be achieved in a far more straightforward way by using the specialized function pcim_iomap_range().

pcim_iomap_range() does not add the mapping addresses to the deprecated iomap table - that's not a problem, however, because non of the users of serial8250_pci_setup_port() uses pcim_iomap_table().

Replace the deprecated PCI functions with pcim_iomap_range().

Cc Guenter Roeck <[email protected]>
Link https://lore.kernel.org/dri-devel/[email protected]/ open here
Signed-off-by Philipp Stanner <[email protected]>

---
@Guenther: Can you please test this? I hope it fixes your issue.

Quoted 9 lines
Quoted 6 lines
Quoted 3 lines

Yes, it does. Thanks a lot for fixing this!

Tested-by Guenter Roeck <[email protected]>

@Greg:
Can you apply this?

Does not apply at all to 6.19-rc1 :(

It conflicts with commit b7cefdb663382 ("serial: 8250_pcilib: Replace deprecated PCI functions"). Unfortunately that commit does not fix the problem; I still see it with v6.19-rc1.

Guenter

#5 lore raw

+Cc Florian

Quoted 32 lines · Guenter Roeck
Quoted 31 lines · Greg Kroah-Hartman
Quoted 29 lines · Philipp Stanner
Quoted 22 lines · Guenter Roeck
Quoted 13 lines · Philipp Stanner

pcim_iomap_table() is deprecated. Moreover, its special usage in 8250, causes a WARN_ON to fire (in pcim_add_mapping_to_legacy_table()).

8250's function serial8250_pci_setup_port() effectively maps the same BAR multiple times and adds an offset to the start address. While mapping and adding offsets is not a bug, it can be achieved in a far more straightforward way by using the specialized function pcim_iomap_range().

pcim_iomap_range() does not add the mapping addresses to the deprecated iomap table - that's not a problem, however, because non of the users of serial8250_pci_setup_port() uses pcim_iomap_table().

Quoted 7 lines

Replace the deprecated PCI functions with pcim_iomap_range().

Cc Guenter Roeck <[email protected]>
Link https://lore.kernel.org/dri-devel/[email protected]/ open here
Signed-off-by Philipp Stanner <[email protected]>

---
@Guenther: Can you please test this? I hope it fixes your issue.

Quoted 3 lines

Yes, it does. Thanks a lot for fixing this!

Tested-by Guenter Roeck <[email protected]>

@Greg:
Can you apply this?

Quoted 6 lines
Quoted 2 lines

Does not apply at all to 6.19-rc1 :(

It conflicts with commit b7cefdb663382 ("serial: 8250_pcilib: Replace deprecated PCI functions"). Unfortunately that commit does not fix the problem; I still see it with v6.19-rc1.

Without me being too familiar with the code, it seems that that commit didn't do what it promised, which is getting rid of the multiple mappings of the same BAR. Presumably it is mapped multiple times through setup_port():

drivers/tty/serial/8250/8250_pci.c +9 -1
index 152f914c599d..f0f13fdda2df 100644
@@ -165,7 +165,15 @@ static int
 setup_port(struct serial_private *priv, struct uart_8250_port *port,
           u8 bar, unsigned int offset, int regshift)
 {
-       return serial8250_pci_setup_port(priv->dev, port, bar, offset, regshift);
+       void __iomem *iomem = NULL;
+
+       if (pci_resource_flags(priv->dev, bar) & IORESOURCE_MEM) {
+               iomem = pcim_iomap(priv->dev, bar, 0);
+               if (!iomem)
+                       return -ENOMEM;
+       }
+
+       return serial8250_pci_setup_port(priv->dev, port, bar, offset, regshift, iomem);
 }

Note that pcim_iomap() is the party that is *adding* the mappings to the legacy table, whereas pcim_iomap_table() was just used to access those.

In any case, I'm quite confident that this description of intended behavior from the commit message

"the remapping should only be called once via 'pcim_iomap()'. Therefore the remapping moved to the caller of 'serial8250_pci_setup_port()'."

has not been implemented in the actual code. Otherwise the WARN_ON couldn't fire.

My personal opinion is that mapping the same BAR multiple times is not a bug and there is no real practical reason why it shouldn't be done when it can make sense. pci_iomap_range() exists for that reason since quite some time.

So my suggested fix for this would be to revert b7cefdb663382 and apply my patch here instead.

P.

Quoted 3 lines

Guenter

#6 lore raw
Quoted 82 lines · Philipp Stanner

+Cc Florian

Quoted 32 lines · Guenter Roeck
Quoted 31 lines · Greg Kroah-Hartman
Quoted 29 lines · Philipp Stanner
Quoted 22 lines · Guenter Roeck
Quoted 13 lines · Philipp Stanner

pcim_iomap_table() is deprecated. Moreover, its special usage in 8250, causes a WARN_ON to fire (in pcim_add_mapping_to_legacy_table()).

8250's function serial8250_pci_setup_port() effectively maps the same BAR multiple times and adds an offset to the start address. While mapping and adding offsets is not a bug, it can be achieved in a far more straightforward way by using the specialized function pcim_iomap_range().

pcim_iomap_range() does not add the mapping addresses to the deprecated iomap table - that's not a problem, however, because non of the users of serial8250_pci_setup_port() uses pcim_iomap_table().

Quoted 7 lines

Replace the deprecated PCI functions with pcim_iomap_range().

Cc Guenter Roeck <[email protected]>
Link https://lore.kernel.org/dri-devel/[email protected]/ open here
Signed-off-by Philipp Stanner <[email protected]>

---
@Guenther: Can you please test this? I hope it fixes your issue.

Quoted 3 lines

Yes, it does. Thanks a lot for fixing this!

Tested-by Guenter Roeck <[email protected]>

@Greg:
Can you apply this?

Quoted 6 lines
Quoted 2 lines

Does not apply at all to 6.19-rc1 :(

It conflicts with commit b7cefdb663382 ("serial: 8250_pcilib: Replace deprecated PCI functions"). Unfortunately that commit does not fix the problem; I still see it with v6.19-rc1.

Without me being too familiar with the code, it seems that that commit didn't do what it promised, which is getting rid of the multiple mappings of the same BAR. Presumably it is mapped multiple times through setup_port():

drivers/tty/serial/8250/8250_pci.c +9 -1
index 152f914c599d..f0f13fdda2df 100644
@@ -165,7 +165,15 @@ static int
  setup_port(struct serial_private *priv, struct uart_8250_port *port,
            u8 bar, unsigned int offset, int regshift)
  {
-       return serial8250_pci_setup_port(priv->dev, port, bar, offset, regshift);
+       void __iomem *iomem = NULL;
+
+       if (pci_resource_flags(priv->dev, bar) & IORESOURCE_MEM) {
+               iomem = pcim_iomap(priv->dev, bar, 0);
+               if (!iomem)
+                       return -ENOMEM;
+       }
+
+       return serial8250_pci_setup_port(priv->dev, port, bar, offset, regshift, iomem);
  }

Note that pcim_iomap() is the party that is *adding* the mappings to the legacy table, whereas pcim_iomap_table() was just used to access those.

In any case, I'm quite confident that this description of intended behavior from the commit message

"the remapping should only be called once via 'pcim_iomap()'. Therefore the remapping moved to the caller of 'serial8250_pci_setup_port()'."

has not been implemented in the actual code. Otherwise the WARN_ON couldn't fire.

Quoted 7 lines

My personal opinion is that mapping the same BAR multiple times is not a bug and there is no real practical reason why it shouldn't be done when it can make sense. pci_iomap_range() exists for that reason since quite some time.

So my suggested fix for this would be to revert b7cefdb663382 and apply my patch here instead.

Ah, nevermind. I thought about it and it's probably safe to say that that WARN_ON is useless anyways. So since Florian has already done the work of getting rid of the deprecated function, I think the clean thing to do is to remove the WARN_ON from PCI.

I proposed this to Bjorn today:
https://lore.kernel.org/all/[email protected]/

Sorry for the noise

P.