All of lore.kernel.org
 help / color / mirror / Atom feed
From: Will Deacon <will.deacon@arm.com>
To: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	linux-pci@vger.kernel.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] pci: Add support for unbinding the generic PCI host controller
Date: Wed, 22 Jun 2016 07:06:39 +0100	[thread overview]
Message-ID: <20160622060638.GY29165@arm.com> (raw)
In-Reply-To: <57698276.6050606@siemens.com>

Hi Jan,

On Tue, Jun 21, 2016 at 08:07:50PM +0200, Jan Kiszka wrote:
> Particularly useful when working in virtual environments where the
> controller may come and go, but possibly not only there.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  drivers/pci/ecam.h                  |  1 +
>  drivers/pci/host/pci-host-common.c  | 13 +++++++++++++
>  drivers/pci/host/pci-host-generic.c |  1 +
>  3 files changed, 15 insertions(+)
> 
> diff --git a/drivers/pci/ecam.h b/drivers/pci/ecam.h
> index 9878beb..5a5f607 100644
> --- a/drivers/pci/ecam.h
> +++ b/drivers/pci/ecam.h
> @@ -63,5 +63,6 @@ extern struct pci_ecam_ops pci_generic_ecam_ops;
>  /* for DT-based PCI controllers that support ECAM */
>  int pci_host_common_probe(struct platform_device *pdev,
>  			  struct pci_ecam_ops *ops);
> +int pci_host_common_remove(struct platform_device *pdev);
>  #endif
>  #endif
> diff --git a/drivers/pci/host/pci-host-common.c b/drivers/pci/host/pci-host-common.c
> index 8cba7ab..c0ff4b1 100644
> --- a/drivers/pci/host/pci-host-common.c
> +++ b/drivers/pci/host/pci-host-common.c
> @@ -164,6 +164,19 @@ int pci_host_common_probe(struct platform_device *pdev,
>  	}
>  
>  	pci_bus_add_devices(bus);
> +	platform_set_drvdata(pdev, bus);
> +	return 0;
> +}
> +
> +int pci_host_common_remove(struct platform_device *pdev)
> +{
> +	struct pci_bus *bus = platform_get_drvdata(pdev);
> +
> +	pci_lock_rescan_remove();
> +	pci_stop_root_bus(bus);
> +	pci_remove_root_bus(bus);
> +	pci_unlock_rescan_remove();

A couple of comments/questions about this:

  - The probe path seems to have some stateful operations outside of PCI
    resources. For example, kzalloc'ing the bus_range resource in
    of_pci_get_host_bridge_resources. Do we need to clean these up
    explicitly?

  - Similarly, we don't seem to tear-down the config space mappings and
    data structures for that, so we leak VA space afaict.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: will.deacon@arm.com (Will Deacon)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] pci: Add support for unbinding the generic PCI host controller
Date: Wed, 22 Jun 2016 07:06:39 +0100	[thread overview]
Message-ID: <20160622060638.GY29165@arm.com> (raw)
In-Reply-To: <57698276.6050606@siemens.com>

Hi Jan,

On Tue, Jun 21, 2016 at 08:07:50PM +0200, Jan Kiszka wrote:
> Particularly useful when working in virtual environments where the
> controller may come and go, but possibly not only there.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  drivers/pci/ecam.h                  |  1 +
>  drivers/pci/host/pci-host-common.c  | 13 +++++++++++++
>  drivers/pci/host/pci-host-generic.c |  1 +
>  3 files changed, 15 insertions(+)
> 
> diff --git a/drivers/pci/ecam.h b/drivers/pci/ecam.h
> index 9878beb..5a5f607 100644
> --- a/drivers/pci/ecam.h
> +++ b/drivers/pci/ecam.h
> @@ -63,5 +63,6 @@ extern struct pci_ecam_ops pci_generic_ecam_ops;
>  /* for DT-based PCI controllers that support ECAM */
>  int pci_host_common_probe(struct platform_device *pdev,
>  			  struct pci_ecam_ops *ops);
> +int pci_host_common_remove(struct platform_device *pdev);
>  #endif
>  #endif
> diff --git a/drivers/pci/host/pci-host-common.c b/drivers/pci/host/pci-host-common.c
> index 8cba7ab..c0ff4b1 100644
> --- a/drivers/pci/host/pci-host-common.c
> +++ b/drivers/pci/host/pci-host-common.c
> @@ -164,6 +164,19 @@ int pci_host_common_probe(struct platform_device *pdev,
>  	}
>  
>  	pci_bus_add_devices(bus);
> +	platform_set_drvdata(pdev, bus);
> +	return 0;
> +}
> +
> +int pci_host_common_remove(struct platform_device *pdev)
> +{
> +	struct pci_bus *bus = platform_get_drvdata(pdev);
> +
> +	pci_lock_rescan_remove();
> +	pci_stop_root_bus(bus);
> +	pci_remove_root_bus(bus);
> +	pci_unlock_rescan_remove();

A couple of comments/questions about this:

  - The probe path seems to have some stateful operations outside of PCI
    resources. For example, kzalloc'ing the bus_range resource in
    of_pci_get_host_bridge_resources. Do we need to clean these up
    explicitly?

  - Similarly, we don't seem to tear-down the config space mappings and
    data structures for that, so we leak VA space afaict.

Will

WARNING: multiple messages have this Message-ID (diff)
From: Will Deacon <will.deacon@arm.com>
To: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	linux-pci@vger.kernel.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] pci: Add support for unbinding the generic PCI host controller
Date: Wed, 22 Jun 2016 07:06:39 +0100	[thread overview]
Message-ID: <20160622060638.GY29165@arm.com> (raw)
In-Reply-To: <57698276.6050606@siemens.com>

Hi Jan,

On Tue, Jun 21, 2016 at 08:07:50PM +0200, Jan Kiszka wrote:
> Particularly useful when working in virtual environments where the
> controller may come and go, but possibly not only there.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  drivers/pci/ecam.h                  |  1 +
>  drivers/pci/host/pci-host-common.c  | 13 +++++++++++++
>  drivers/pci/host/pci-host-generic.c |  1 +
>  3 files changed, 15 insertions(+)
> 
> diff --git a/drivers/pci/ecam.h b/drivers/pci/ecam.h
> index 9878beb..5a5f607 100644
> --- a/drivers/pci/ecam.h
> +++ b/drivers/pci/ecam.h
> @@ -63,5 +63,6 @@ extern struct pci_ecam_ops pci_generic_ecam_ops;
>  /* for DT-based PCI controllers that support ECAM */
>  int pci_host_common_probe(struct platform_device *pdev,
>  			  struct pci_ecam_ops *ops);
> +int pci_host_common_remove(struct platform_device *pdev);
>  #endif
>  #endif
> diff --git a/drivers/pci/host/pci-host-common.c b/drivers/pci/host/pci-host-common.c
> index 8cba7ab..c0ff4b1 100644
> --- a/drivers/pci/host/pci-host-common.c
> +++ b/drivers/pci/host/pci-host-common.c
> @@ -164,6 +164,19 @@ int pci_host_common_probe(struct platform_device *pdev,
>  	}
>  
>  	pci_bus_add_devices(bus);
> +	platform_set_drvdata(pdev, bus);
> +	return 0;
> +}
> +
> +int pci_host_common_remove(struct platform_device *pdev)
> +{
> +	struct pci_bus *bus = platform_get_drvdata(pdev);
> +
> +	pci_lock_rescan_remove();
> +	pci_stop_root_bus(bus);
> +	pci_remove_root_bus(bus);
> +	pci_unlock_rescan_remove();

A couple of comments/questions about this:

  - The probe path seems to have some stateful operations outside of PCI
    resources. For example, kzalloc'ing the bus_range resource in
    of_pci_get_host_bridge_resources. Do we need to clean these up
    explicitly?

  - Similarly, we don't seem to tear-down the config space mappings and
    data structures for that, so we leak VA space afaict.

Will

  reply	other threads:[~2016-06-22  6:06 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-21 18:07 [PATCH] pci: Add support for unbinding the generic PCI host controller Jan Kiszka
2016-06-21 18:07 ` Jan Kiszka
2016-06-22  6:06 ` Will Deacon [this message]
2016-06-22  6:06   ` Will Deacon
2016-06-22  6:06   ` Will Deacon
2016-06-23 17:31   ` Jan Kiszka
2016-06-23 17:31     ` Jan Kiszka
2016-06-24  6:12     ` Will Deacon
2016-06-24  6:12       ` Will Deacon
2016-06-24  6:39       ` Jan Kiszka
2016-06-24  6:39         ` Jan Kiszka
2016-06-24  6:50         ` Will Deacon
2016-06-24  6:50           ` Will Deacon
2016-08-22 17:05 ` Bjorn Helgaas
2016-08-22 17:05   ` Bjorn Helgaas
2016-08-22 17:23   ` Jan Kiszka
2016-08-22 17:23     ` Jan Kiszka
2016-08-22 17:23     ` Jan Kiszka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160622060638.GY29165@arm.com \
    --to=will.deacon@arm.com \
    --cc=bhelgaas@google.com \
    --cc=jan.kiszka@siemens.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
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.