* [PATCH] ipack: tpci200: change pci_iounmap to iounmap
@ 2021-08-27 9:43 Dongliang Mu
2021-08-27 10:00 ` Greg Kroah-Hartman
0 siblings, 1 reply; 6+ messages in thread
From: Dongliang Mu @ 2021-08-27 9:43 UTC (permalink / raw)
To: Samuel Iglesias Gonsalvez, Jens Taprogge, Greg Kroah-Hartman,
Dongliang Mu, Lv Yunlong, Aditya Srivastava
Cc: Randy Dunlap, industrypack-devel, linux-kernel
The deallocation api for ioremap should be iounmap, other than
pci_iounmap.
Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
---
drivers/ipack/carriers/tpci200.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/ipack/carriers/tpci200.c b/drivers/ipack/carriers/tpci200.c
index cbfdadecb23b..bf2ae2be5eb5 100644
--- a/drivers/ipack/carriers/tpci200.c
+++ b/drivers/ipack/carriers/tpci200.c
@@ -88,7 +88,7 @@ static void tpci200_unregister(struct tpci200_board *tpci200)
{
free_irq(tpci200->info->pdev->irq, (void *) tpci200);
- pci_iounmap(tpci200->info->pdev, tpci200->info->interface_regs);
+ iounmap(tpci200->info->interface_regs);
pci_release_region(tpci200->info->pdev, TPCI200_IP_INTERFACE_BAR);
pci_release_region(tpci200->info->pdev, TPCI200_IO_ID_INT_SPACES_BAR);
@@ -347,7 +347,7 @@ static int tpci200_register(struct tpci200_board *tpci200)
return 0;
err_interface_regs:
- pci_iounmap(tpci200->info->pdev, tpci200->info->interface_regs);
+ iounmap(tpci200->info->interface_regs);
err_mem16_space_bar:
pci_release_region(tpci200->info->pdev, TPCI200_MEM16_SPACE_BAR);
err_mem8_space_bar:
@@ -596,7 +596,7 @@ static int tpci200_pci_probe(struct pci_dev *pdev,
err_tpci200_install:
tpci200_uninstall(tpci200);
err_cfg_regs:
- pci_iounmap(tpci200->info->pdev, tpci200->info->cfg_regs);
+ iounmap(tpci200->info->cfg_regs);
err_request_region:
pci_release_region(pdev, TPCI200_CFG_MEM_BAR);
err_tpci200_info:
@@ -612,7 +612,7 @@ static void __tpci200_pci_remove(struct tpci200_board *tpci200)
ipack_bus_unregister(tpci200->info->ipack_bus);
tpci200_uninstall(tpci200);
- pci_iounmap(tpci200->info->pdev, tpci200->info->cfg_regs);
+ iounmap(tpci200->info->cfg_regs);
pci_release_region(tpci200->info->pdev, TPCI200_CFG_MEM_BAR);
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] ipack: tpci200: change pci_iounmap to iounmap
2021-08-27 9:43 [PATCH] ipack: tpci200: change pci_iounmap to iounmap Dongliang Mu
@ 2021-08-27 10:00 ` Greg Kroah-Hartman
2021-08-27 10:33 ` Dongliang Mu
0 siblings, 1 reply; 6+ messages in thread
From: Greg Kroah-Hartman @ 2021-08-27 10:00 UTC (permalink / raw)
To: Dongliang Mu
Cc: Samuel Iglesias Gonsalvez, Jens Taprogge, Lv Yunlong,
Aditya Srivastava, Randy Dunlap, industrypack-devel,
linux-kernel
On Fri, Aug 27, 2021 at 05:43:47PM +0800, Dongliang Mu wrote:
> The deallocation api for ioremap should be iounmap, other than
> pci_iounmap.
why?
Isn't this a pci device?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ipack: tpci200: change pci_iounmap to iounmap
2021-08-27 10:00 ` Greg Kroah-Hartman
@ 2021-08-27 10:33 ` Dongliang Mu
2021-08-27 14:24 ` Greg Kroah-Hartman
0 siblings, 1 reply; 6+ messages in thread
From: Dongliang Mu @ 2021-08-27 10:33 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Samuel Iglesias Gonsalvez, Jens Taprogge, Lv Yunlong,
Aditya Srivastava, Randy Dunlap, industrypack-devel,
linux-kernel
On Fri, Aug 27, 2021 at 6:00 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Fri, Aug 27, 2021 at 05:43:47PM +0800, Dongliang Mu wrote:
> > The deallocation api for ioremap should be iounmap, other than
> > pci_iounmap.
>
> why?
Because the tpci200->info->cfg_regs/interface_regs is allocated by
ioremap. From my understanding, ioremap and iounmap are in pairs,
other than pci_iounmap.
See the code below.
tpci200->info->interface_regs =
ioremap(pci_resource_start(tpci200->info->pdev,
TPCI200_IP_INTERFACE_BAR),
TPCI200_IFACE_SIZE);
https://elixir.bootlin.com/linux/latest/source/drivers/ipack/carriers/tpci200.c#L297
tpci200->info->cfg_regs = ioremap(
pci_resource_start(pdev, TPCI200_CFG_MEM_BAR),
pci_resource_len(pdev, TPCI200_CFG_MEM_BAR));
https://elixir.bootlin.com/linux/latest/source/drivers/ipack/carriers/tpci200.c#L546
If there is any issue, please let me know
>
> Isn't this a pci device?
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ipack: tpci200: change pci_iounmap to iounmap
2021-08-27 10:33 ` Dongliang Mu
@ 2021-08-27 14:24 ` Greg Kroah-Hartman
2021-08-30 1:22 ` Dongliang Mu
0 siblings, 1 reply; 6+ messages in thread
From: Greg Kroah-Hartman @ 2021-08-27 14:24 UTC (permalink / raw)
To: Dongliang Mu
Cc: Samuel Iglesias Gonsalvez, Jens Taprogge, Lv Yunlong,
Aditya Srivastava, Randy Dunlap, industrypack-devel,
linux-kernel
On Fri, Aug 27, 2021 at 06:33:46PM +0800, Dongliang Mu wrote:
> On Fri, Aug 27, 2021 at 6:00 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Fri, Aug 27, 2021 at 05:43:47PM +0800, Dongliang Mu wrote:
> > > The deallocation api for ioremap should be iounmap, other than
> > > pci_iounmap.
> >
> > why?
>
> Because the tpci200->info->cfg_regs/interface_regs is allocated by
> ioremap. From my understanding, ioremap and iounmap are in pairs,
> other than pci_iounmap.
> See the code below.
>
> tpci200->info->interface_regs =
> ioremap(pci_resource_start(tpci200->info->pdev,
> TPCI200_IP_INTERFACE_BAR),
> TPCI200_IFACE_SIZE);
>
> https://elixir.bootlin.com/linux/latest/source/drivers/ipack/carriers/tpci200.c#L297
>
> tpci200->info->cfg_regs = ioremap(
> pci_resource_start(pdev, TPCI200_CFG_MEM_BAR),
> pci_resource_len(pdev, TPCI200_CFG_MEM_BAR));
>
> https://elixir.bootlin.com/linux/latest/source/drivers/ipack/carriers/tpci200.c#L546
>
> If there is any issue, please let me know
The point is that the driver should be calling pci_iomap_bar() instead
of ioremap(), and then the call to pci_ioumap() is correct here.
Please make that change instead.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ipack: tpci200: change pci_iounmap to iounmap
2021-08-27 14:24 ` Greg Kroah-Hartman
@ 2021-08-30 1:22 ` Dongliang Mu
2021-08-30 2:32 ` Dongliang Mu
0 siblings, 1 reply; 6+ messages in thread
From: Dongliang Mu @ 2021-08-30 1:22 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Samuel Iglesias Gonsalvez, Jens Taprogge, Lv Yunlong,
Aditya Srivastava, Randy Dunlap, industrypack-devel,
linux-kernel
On Fri, Aug 27, 2021 at 10:25 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Fri, Aug 27, 2021 at 06:33:46PM +0800, Dongliang Mu wrote:
> > On Fri, Aug 27, 2021 at 6:00 PM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Fri, Aug 27, 2021 at 05:43:47PM +0800, Dongliang Mu wrote:
> > > > The deallocation api for ioremap should be iounmap, other than
> > > > pci_iounmap.
> > >
> > > why?
> >
> > Because the tpci200->info->cfg_regs/interface_regs is allocated by
> > ioremap. From my understanding, ioremap and iounmap are in pairs,
> > other than pci_iounmap.
> > See the code below.
> >
> > tpci200->info->interface_regs =
> > ioremap(pci_resource_start(tpci200->info->pdev,
> > TPCI200_IP_INTERFACE_BAR),
> > TPCI200_IFACE_SIZE);
> >
> > https://elixir.bootlin.com/linux/latest/source/drivers/ipack/carriers/tpci200.c#L297
> >
> > tpci200->info->cfg_regs = ioremap(
> > pci_resource_start(pdev, TPCI200_CFG_MEM_BAR),
> > pci_resource_len(pdev, TPCI200_CFG_MEM_BAR));
> >
> > https://elixir.bootlin.com/linux/latest/source/drivers/ipack/carriers/tpci200.c#L546
> >
> > If there is any issue, please let me know
>
> The point is that the driver should be calling pci_iomap_bar() instead
> of ioremap(), and then the call to pci_ioumap() is correct here.
>
> Please make that change instead.
No problem. I will modify the patch and send a v2 patch afterwards.
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ipack: tpci200: change pci_iounmap to iounmap
2021-08-30 1:22 ` Dongliang Mu
@ 2021-08-30 2:32 ` Dongliang Mu
0 siblings, 0 replies; 6+ messages in thread
From: Dongliang Mu @ 2021-08-30 2:32 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Samuel Iglesias Gonsalvez, Jens Taprogge, Lv Yunlong,
Aditya Srivastava, Randy Dunlap, industrypack-devel,
linux-kernel
On Mon, Aug 30, 2021 at 9:22 AM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
>
> On Fri, Aug 27, 2021 at 10:25 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Fri, Aug 27, 2021 at 06:33:46PM +0800, Dongliang Mu wrote:
> > > On Fri, Aug 27, 2021 at 6:00 PM Greg Kroah-Hartman
> > > <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Fri, Aug 27, 2021 at 05:43:47PM +0800, Dongliang Mu wrote:
> > > > > The deallocation api for ioremap should be iounmap, other than
> > > > > pci_iounmap.
> > > >
> > > > why?
> > >
> > > Because the tpci200->info->cfg_regs/interface_regs is allocated by
> > > ioremap. From my understanding, ioremap and iounmap are in pairs,
> > > other than pci_iounmap.
> > > See the code below.
> > >
> > > tpci200->info->interface_regs =
> > > ioremap(pci_resource_start(tpci200->info->pdev,
> > > TPCI200_IP_INTERFACE_BAR),
> > > TPCI200_IFACE_SIZE);
> > >
> > > https://elixir.bootlin.com/linux/latest/source/drivers/ipack/carriers/tpci200.c#L297
> > >
> > > tpci200->info->cfg_regs = ioremap(
> > > pci_resource_start(pdev, TPCI200_CFG_MEM_BAR),
> > > pci_resource_len(pdev, TPCI200_CFG_MEM_BAR));
> > >
> > > https://elixir.bootlin.com/linux/latest/source/drivers/ipack/carriers/tpci200.c#L546
> > >
> > > If there is any issue, please let me know
> >
> > The point is that the driver should be calling pci_iomap_bar() instead
> > of ioremap(), and then the call to pci_ioumap() is correct here.
Hi gregkh,
For cfg_regs [1], it is fine to change from:
tpci200->info->cfg_regs = ioremap(
pci_resource_start(pdev, TPCI200_CFG_MEM_BAR),
pci_resource_len(pdev, TPCI200_CFG_MEM_BAR));
to :
tpci200->info->cfg_regs = pci_ioremap_bar(pdev, TPCI200_CFG_MEM_BAR);
But for interface_regs [2], I am not sure as TPCI200_IFACE_SIZE is a
totally separate definition, not calculated by resouce_size or
pci_resource_len(tpci200->info->pdev, TPCI200_IP_INTERFACE_BAR).
tpci200->info->interface_regs =
ioremap(pci_resource_start(tpci200->info->pdev,
TPCI200_IP_INTERFACE_BAR),
TPCI200_IFACE_SIZE);
#define TPCI200_IFACE_SIZE 0x100
Any comment here?
Note that, it is pci_ioremap_bar, not pci_iomap_bar.
[1] https://elixir.bootlin.com/linux/v5.14/source/drivers/ipack/carriers/tpci200.c#L544
[2] https://elixir.bootlin.com/linux/v5.14/source/drivers/ipack/carriers/tpci200.c#L295
> >
> > Please make that change instead.
>
> No problem. I will modify the patch and send a v2 patch afterwards.
>
> >
> > thanks,
> >
> > greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-08-30 2:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-27 9:43 [PATCH] ipack: tpci200: change pci_iounmap to iounmap Dongliang Mu
2021-08-27 10:00 ` Greg Kroah-Hartman
2021-08-27 10:33 ` Dongliang Mu
2021-08-27 14:24 ` Greg Kroah-Hartman
2021-08-30 1:22 ` Dongliang Mu
2021-08-30 2:32 ` Dongliang Mu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.