linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).