[PATCH] tty: serial: Replace deprecated PCI API

[PATCH] tty: serial: Replace deprecated PCI API

[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 <linux@roeck-us.net>
Link: https://lore.kernel.org/dri-devel/16cd212f-6ea0-471d-bf32-34f55d7292fe@roeck-us.net/
Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
@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(-)

diff --git a/drivers/tty/serial/8250/8250_pcilib.c b/drivers/tty/serial/8250/8250_pcilib.c
index d8d0ae0d7238..f98eb2ab1005 100644
--- a/drivers/tty/serial/8250/8250_pcilib.c
+++ b/drivers/tty/serial/8250/8250_pcilib.c
@@ -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.49.0

Re: [PATCH] tty: serial: Replace deprecated PCI API

[Guenter Roeck]

On 11/26/25 01:10, Philipp Stanner wrote:
> 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 <linux@roeck-us.net>
> Link: https://lore.kernel.org/dri-devel/16cd212f-6ea0-471d-bf32-34f55d7292fe@roeck-us.net/
> Signed-off-by: Philipp Stanner <phasta@kernel.org>
> ---
> @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 <linux@roeck-us.net>

> ---
>   drivers/tty/serial/8250/8250_pcilib.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_pcilib.c b/drivers/tty/serial/8250/8250_pcilib.c
> index d8d0ae0d7238..f98eb2ab1005 100644
> --- a/drivers/tty/serial/8250/8250_pcilib.c
> +++ b/drivers/tty/serial/8250/8250_pcilib.c
> @@ -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;

Re: [PATCH] tty: serial: Replace deprecated PCI API

[Philipp Stanner]

On Wed, 2025-11-26 at 09:02 -0800, Guenter Roeck wrote:
> On 11/26/25 01:10, Philipp Stanner wrote:
> > 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 <linux@roeck-us.net>
> > Link: https://lore.kernel.org/dri-devel/16cd212f-6ea0-471d-bf32-34f55d7292fe@roeck-us.net/
> > Signed-off-by: Philipp Stanner <phasta@kernel.org>
> > ---
> > @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 <linux@roeck-us.net>

@Greg:
Can you apply this?

P.


> 
> > ---
> >   drivers/tty/serial/8250/8250_pcilib.c | 9 +++++----
> >   1 file changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/tty/serial/8250/8250_pcilib.c b/drivers/tty/serial/8250/8250_pcilib.c
> > index d8d0ae0d7238..f98eb2ab1005 100644
> > --- a/drivers/tty/serial/8250/8250_pcilib.c
> > +++ b/drivers/tty/serial/8250/8250_pcilib.c
> > @@ -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;
> 

Re: [PATCH] tty: serial: Replace deprecated PCI API

[Greg Kroah-Hartman]

On Thu, Dec 11, 2025 at 02:57:46PM +0100, Philipp Stanner wrote:
> On Wed, 2025-11-26 at 09:02 -0800, Guenter Roeck wrote:
> > On 11/26/25 01:10, Philipp Stanner wrote:
> > > 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 <linux@roeck-us.net>
> > > Link: https://lore.kernel.org/dri-devel/16cd212f-6ea0-471d-bf32-34f55d7292fe@roeck-us.net/
> > > Signed-off-by: Philipp Stanner <phasta@kernel.org>
> > > ---
> > > @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 <linux@roeck-us.net>
> 
> @Greg:
> Can you apply this?

Does not apply at all to 6.19-rc1 :(

Can you please rebase and resend?

thanks,

greg k-h

Re: [PATCH] tty: serial: Replace deprecated PCI API

[Guenter Roeck]

On 12/17/25 06:06, Greg Kroah-Hartman wrote:
> On Thu, Dec 11, 2025 at 02:57:46PM +0100, Philipp Stanner wrote:
>> On Wed, 2025-11-26 at 09:02 -0800, Guenter Roeck wrote:
>>> On 11/26/25 01:10, Philipp Stanner wrote:
>>>> 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 <linux@roeck-us.net>
>>>> Link: https://lore.kernel.org/dri-devel/16cd212f-6ea0-471d-bf32-34f55d7292fe@roeck-us.net/
>>>> Signed-off-by: Philipp Stanner <phasta@kernel.org>
>>>> ---
>>>> @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 <linux@roeck-us.net>
>>
>> @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

Re: [PATCH] tty: serial: Replace deprecated PCI API

[Philipp Stanner]

+Cc Florian

On Wed, 2025-12-17 at 08:11 -0800, Guenter Roeck wrote:
> On 12/17/25 06:06, Greg Kroah-Hartman wrote:
> > On Thu, Dec 11, 2025 at 02:57:46PM +0100, Philipp Stanner wrote:
> > > On Wed, 2025-11-26 at 09:02 -0800, Guenter Roeck wrote:
> > > > On 11/26/25 01:10, Philipp Stanner wrote:
> > > > > 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 <linux@roeck-us.net>
> > > > > Link: https://lore.kernel.org/dri-devel/16cd212f-6ea0-471d-bf32-34f55d7292fe@roeck-us.net/
> > > > > Signed-off-by: Philipp Stanner <phasta@kernel.org>
> > > > > ---
> > > > > @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 <linux@roeck-us.net>
> > > 
> > > @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.

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():

diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index 152f914c599d..f0f13fdda2df 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -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.


> 
> Guenter
> 

Re: [PATCH] tty: serial: Replace deprecated PCI API

[Philipp Stanner]

On Thu, 2025-12-18 at 10:02 +0100, Philipp Stanner wrote:
> +Cc Florian
> 
> On Wed, 2025-12-17 at 08:11 -0800, Guenter Roeck wrote:
> > On 12/17/25 06:06, Greg Kroah-Hartman wrote:
> > > On Thu, Dec 11, 2025 at 02:57:46PM +0100, Philipp Stanner wrote:
> > > > On Wed, 2025-11-26 at 09:02 -0800, Guenter Roeck wrote:
> > > > > On 11/26/25 01:10, Philipp Stanner wrote:
> > > > > > 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 <linux@roeck-us.net>
> > > > > > Link: https://lore.kernel.org/dri-devel/16cd212f-6ea0-471d-bf32-34f55d7292fe@roeck-us.net/
> > > > > > Signed-off-by: Philipp Stanner <phasta@kernel.org>
> > > > > > ---
> > > > > > @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 <linux@roeck-us.net>
> > > > 
> > > > @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.
> 
> 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():
> 
> diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
> index 152f914c599d..f0f13fdda2df 100644
> --- a/drivers/tty/serial/8250/8250_pci.c
> +++ b/drivers/tty/serial/8250/8250_pci.c
> @@ -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.


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/20251218092819.149665-2-phasta@kernel.org/

Sorry for the noise

P.