linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Robert Richter <rric-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
To: Liviu Dudau <Liviu.Dudau-5wv7dgnIgG8@public.gmane.org>
Cc: Bjorn Helgaas <bhelgaas-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>,
	Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>,
	Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Jason Gunthorpe
	<jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>,
	Benjamin Herrenschmidt
	<benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>,
	Catalin Marinas <Catalin.Marinas-5wv7dgnIgG8@public.gmane.org>,
	Will Deacon <Will.Deacon-5wv7dgnIgG8@public.gmane.org>,
	Russell King <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>,
	linux-pci <linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Linus Walleij
	<linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	Tanmay Inamdar <tinamdar-qTEPVZfXA3Y@public.gmane.org>,
	Grant Likely
	<grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>,
	Sinan Kaya <okaya-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>,
	Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>,
	Kukjin Kim <kgene.kim-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>,
	Suravee Suthikulanit
	<suravee.suthikulpanit-5C7GfCeVMHo@public.gmane.org>,
	linux-arch <linux-arch-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	LKML <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Device Tree ML
	<devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	LAKML
	<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>,
	"grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org"
	<grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>Sunil
	Goutham <s>
Subject: Re: [PATCH v11 08/10] OF: PCI: Add support for parsing PCI host bridge resources from DT
Date: Mon, 22 Sep 2014 14:15:47 +0200	[thread overview]
Message-ID: <20140922121547.GD20170@rric.localhost> (raw)
In-Reply-To: <20140922114317.GN1994-2JSQmVVBSi7ZROr8t4l/smS4ubULX0JqMm0uRHvK7Nw@public.gmane.org>

On 22.09.14 12:43:17, Liviu Dudau wrote:
> On Mon, Sep 22, 2014 at 10:32:28AM +0100, Robert Richter wrote:
> > On 18.09.14 02:30:23, Liviu Dudau wrote:
> > > +int of_pci_get_host_bridge_resources(struct device_node *dev,
> > > +			unsigned char busno, unsigned char bus_max,
> > > +			struct list_head *resources, resource_size_t *io_base)
> > > +{
> > > +	struct resource *res;
> > > +	struct resource *bus_range;
> > > +	struct of_pci_range range;
> > > +	struct of_pci_range_parser parser;
> > > +	char range_type[4];
> > > +	int err;
> > > +
> > > +	if (!io_base)
> > > +		return -EINVAL;
> > > +	*io_base = OF_BAD_ADDR;
> > 
> 
> Hi Robert,
> 
> > This breaks for mem-mapped pci host controllers. The patch below fixes
> > this.
> 
> I think you mean PCI host controller that have only memory mapped ranges,
> am I right? Initially I've read your reply as to mean that the host
> controller is accessed through some memory mapped area, which I believe is
> the case for all host controllers.

Right, that's meant here. Sorry for the misleading comment.

> 
> > 
> > This series was tested with the fix on top for Cavium Thunder.
> > 
> > Tested-by: Robert Richter <rrichter-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> 
> Thanks for that!
> 
> > 
> > -Robert
> > 
> > 
> > 
> > From e798af4fc2f664d1aff7e863489b8298f90e716e Mon Sep 17 00:00:00 2001
> > From: Robert Richter <rrichter-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> > Date: Mon, 22 Sep 2014 10:46:01 +0200
> > Subject: [PATCH] OF: PCI: Fix creation of mem-mapped pci host bridges
> > 
> > The pci host bridge was not created if io_base was not set when
> > calling of_pci_get_host_bridge_resources(). This is esp. the case for
> > mem-mapped io (IORESOURCE_MEM). This patch fixes this. Function
> > parameter io_base is optional now.
> 
> I think the message is misleading. What you want to do is make io_base
> optional for the case where the PCI host bridge only expects to have only
> IORESOURCE_MEM ranges and doesn't care about IORESOURCE_IO ones.
> 
> As I'm going to touch this area again to address a comment from Bjorn,
> do you mind if I roll this patch into mine with your Signed-off-by and
> the mention that you have made io_base optional?

Sure, fine with me.

Thanks,

-Robert


> 
> Best regards,
> Liviu
> 
> > 
> > Signed-off-by: Robert Richter <rrichter-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> > ---
> >  drivers/of/of_pci.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
> > index ffdb45ed8682..1f0e7c2505ee 100644
> > --- a/drivers/of/of_pci.c
> > +++ b/drivers/of/of_pci.c
> > @@ -182,9 +182,8 @@ int of_pci_get_host_bridge_resources(struct device_node *dev,
> >  	char range_type[4];
> >  	int err;
> >  
> > -	if (!io_base)
> > -		return -EINVAL;
> > -	*io_base = OF_BAD_ADDR;
> > +	if (io_base)
> > +		*io_base = OF_BAD_ADDR;
> >  
> >  	bus_range = kzalloc(sizeof(*bus_range), GFP_KERNEL);
> >  	if (!bus_range)
> > @@ -242,7 +241,7 @@ int of_pci_get_host_bridge_resources(struct device_node *dev,
> >  			goto parse_failed;
> >  		}
> >  
> > -		if (resource_type(res) == IORESOURCE_IO) {
> > +		if (io_base && resource_type(res) == IORESOURCE_IO) {
> >  			if (*io_base != OF_BAD_ADDR)
> >  				pr_warn("More than one I/O resource converted. CPU offset for old range lost!\n");
> >  			*io_base = range.cpu_addr;
> > -- 
> > 2.1.0
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 
> -- 
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Robert Richter <rric@kernel.org>
To: Liviu Dudau <Liviu.Dudau@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	Arnd Bergmann <arnd@arndb.de>, Rob Herring <robh+dt@kernel.org>,
	Jason Gunthorpe <jgunthorpe@obsidianresearch.com>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Catalin Marinas <Catalin.Marinas@arm.com>,
	Will Deacon <Will.Deacon@arm.com>,
	Russell King <linux@arm.linux.org.uk>,
	linux-pci <linux-pci@vger.kernel.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Tanmay Inamdar <tinamdar@apm.com>,
	Grant Likely <grant.likely@secretlab.ca>,
	Sinan Kaya <okaya@codeaurora.org>,
	Jingoo Han <jg1.han@samsung.com>,
	Kukjin Kim <kgene.kim@samsung.com>,
	Suravee Suthikulanit <suravee.suthikulpanit@amd.com>,
	linux-arch <linux-arch@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Device Tree ML <devicetree@vger.kernel.org>,
	LAKML <linux-arm-kernel@lists.infradead.org>,
	"grant.likely@linaro.org" <grant.likely@linaro.org>,
	Sunil Goutham <sunil.goutham@caviumnetworks.com>
Subject: Re: [PATCH v11 08/10] OF: PCI: Add support for parsing PCI host bridge resources from DT
Date: Mon, 22 Sep 2014 14:15:47 +0200	[thread overview]
Message-ID: <20140922121547.GD20170@rric.localhost> (raw)
Message-ID: <20140922121547.jlAnMqViBhf1gWI0Rtzo47cR9o5PZQBazCIpOW_yDHA@z> (raw)
In-Reply-To: <20140922114317.GN1994@e106497-lin.cambridge.arm.com>

On 22.09.14 12:43:17, Liviu Dudau wrote:
> On Mon, Sep 22, 2014 at 10:32:28AM +0100, Robert Richter wrote:
> > On 18.09.14 02:30:23, Liviu Dudau wrote:
> > > +int of_pci_get_host_bridge_resources(struct device_node *dev,
> > > +			unsigned char busno, unsigned char bus_max,
> > > +			struct list_head *resources, resource_size_t *io_base)
> > > +{
> > > +	struct resource *res;
> > > +	struct resource *bus_range;
> > > +	struct of_pci_range range;
> > > +	struct of_pci_range_parser parser;
> > > +	char range_type[4];
> > > +	int err;
> > > +
> > > +	if (!io_base)
> > > +		return -EINVAL;
> > > +	*io_base = OF_BAD_ADDR;
> > 
> 
> Hi Robert,
> 
> > This breaks for mem-mapped pci host controllers. The patch below fixes
> > this.
> 
> I think you mean PCI host controller that have only memory mapped ranges,
> am I right? Initially I've read your reply as to mean that the host
> controller is accessed through some memory mapped area, which I believe is
> the case for all host controllers.

Right, that's meant here. Sorry for the misleading comment.

> 
> > 
> > This series was tested with the fix on top for Cavium Thunder.
> > 
> > Tested-by: Robert Richter <rrichter@cavium.com>
> 
> Thanks for that!
> 
> > 
> > -Robert
> > 
> > 
> > 
> > From e798af4fc2f664d1aff7e863489b8298f90e716e Mon Sep 17 00:00:00 2001
> > From: Robert Richter <rrichter@cavium.com>
> > Date: Mon, 22 Sep 2014 10:46:01 +0200
> > Subject: [PATCH] OF: PCI: Fix creation of mem-mapped pci host bridges
> > 
> > The pci host bridge was not created if io_base was not set when
> > calling of_pci_get_host_bridge_resources(). This is esp. the case for
> > mem-mapped io (IORESOURCE_MEM). This patch fixes this. Function
> > parameter io_base is optional now.
> 
> I think the message is misleading. What you want to do is make io_base
> optional for the case where the PCI host bridge only expects to have only
> IORESOURCE_MEM ranges and doesn't care about IORESOURCE_IO ones.
> 
> As I'm going to touch this area again to address a comment from Bjorn,
> do you mind if I roll this patch into mine with your Signed-off-by and
> the mention that you have made io_base optional?

Sure, fine with me.

Thanks,

-Robert


> 
> Best regards,
> Liviu
> 
> > 
> > Signed-off-by: Robert Richter <rrichter@cavium.com>
> > ---
> >  drivers/of/of_pci.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
> > index ffdb45ed8682..1f0e7c2505ee 100644
> > --- a/drivers/of/of_pci.c
> > +++ b/drivers/of/of_pci.c
> > @@ -182,9 +182,8 @@ int of_pci_get_host_bridge_resources(struct device_node *dev,
> >  	char range_type[4];
> >  	int err;
> >  
> > -	if (!io_base)
> > -		return -EINVAL;
> > -	*io_base = OF_BAD_ADDR;
> > +	if (io_base)
> > +		*io_base = OF_BAD_ADDR;
> >  
> >  	bus_range = kzalloc(sizeof(*bus_range), GFP_KERNEL);
> >  	if (!bus_range)
> > @@ -242,7 +241,7 @@ int of_pci_get_host_bridge_resources(struct device_node *dev,
> >  			goto parse_failed;
> >  		}
> >  
> > -		if (resource_type(res) == IORESOURCE_IO) {
> > +		if (io_base && resource_type(res) == IORESOURCE_IO) {
> >  			if (*io_base != OF_BAD_ADDR)
> >  				pr_warn("More than one I/O resource converted. CPU offset for old range lost!\n");
> >  			*io_base = range.cpu_addr;
> > -- 
> > 2.1.0
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 
> -- 
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯
> 

  parent reply	other threads:[~2014-09-22 12:15 UTC|newest]

Thread overview: 75+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-18  1:30 [PATCH v11 00/10] Support for creating generic PCI host bridges from DT Liviu Dudau
2014-09-18  1:30 ` Liviu Dudau
2014-09-18  1:30 ` [PATCH v11 01/10] Fix ioport_map() for !CONFIG_GENERIC_IOMAP cases Liviu Dudau
2014-09-18  1:30   ` Liviu Dudau
2014-09-18  1:30 ` [PATCH v11 02/10] PCI: Introduce helper functions to deal with PCI I/O ranges Liviu Dudau
2014-09-18  1:30   ` Liviu Dudau
     [not found]   ` <1411003825-21521-3-git-send-email-Liviu.Dudau-5wv7dgnIgG8@public.gmane.org>
2014-09-19 20:48     ` Bjorn Helgaas
2014-09-19 20:48       ` Bjorn Helgaas
2014-09-18  1:30 ` [PATCH v11 03/10] ARM: Define PCI_IOBASE as the base of virtual PCI IO space Liviu Dudau
     [not found]   ` <1411003825-21521-4-git-send-email-Liviu.Dudau-5wv7dgnIgG8@public.gmane.org>
2014-09-20  2:14     ` Arnd Bergmann
2014-09-20  2:14       ` Arnd Bergmann
2014-09-18  1:30 ` [PATCH v11 04/10] PCI: OF: Fix the conversion of IO ranges into IO resources Liviu Dudau
2014-09-18  1:30   ` Liviu Dudau
2014-09-19 20:56   ` Bjorn Helgaas
2014-09-19 20:56     ` Bjorn Helgaas
2014-09-20 17:33   ` Rob Herring
2014-09-22 15:32     ` Liviu Dudau
2014-09-22 15:32       ` Liviu Dudau
2014-09-22 17:18       ` Rob Herring
2014-09-22 17:18         ` Rob Herring
2014-09-18  1:30 ` [PATCH v11 05/10] PCI: Create pci_host_bridge before its associated bus in pci_create_root_bus Liviu Dudau
2014-09-18  1:30   ` Liviu Dudau
2014-09-18  1:30 ` [PATCH v11 06/10] PCI: Introduce generic domain handling for PCI busses Liviu Dudau
2014-09-18  1:30   ` Liviu Dudau
2014-09-18  1:30 ` [PATCH v11 07/10] OF: Introduce helper function for getting PCI domain_nr Liviu Dudau
2014-09-18  1:30   ` Liviu Dudau
2014-09-19 21:03   ` Bjorn Helgaas
2014-09-19 21:03     ` Bjorn Helgaas
2014-09-22 11:05     ` Liviu Dudau
2014-09-22 11:05       ` Liviu Dudau
2014-09-22 15:25       ` Bjorn Helgaas
2014-09-22 15:25         ` Bjorn Helgaas
2014-09-22 15:33         ` Liviu Dudau
2014-09-22 15:33           ` Liviu Dudau
2014-09-20  2:24   ` Arnd Bergmann
2014-09-20  2:24     ` Arnd Bergmann
2014-09-22 15:20     ` Liviu Dudau
2014-09-22 15:20       ` Liviu Dudau
2014-09-20 17:52   ` Rob Herring
2014-09-18  1:30 ` [PATCH v11 08/10] OF: PCI: Add support for parsing PCI host bridge resources from DT Liviu Dudau
2014-09-18  1:30   ` Liviu Dudau
2014-09-19 21:06   ` Bjorn Helgaas
2014-09-19 21:06     ` Bjorn Helgaas
2014-09-20  0:28     ` Rob Herring
2014-09-22 17:55       ` Liviu Dudau
2014-09-22 17:55         ` Liviu Dudau
2014-09-22 22:11         ` Rob Herring
2014-09-22  9:32   ` Robert Richter
2014-09-22  9:32     ` Robert Richter
2014-09-22 11:43     ` Liviu Dudau
2014-09-22 11:43       ` Liviu Dudau
     [not found]       ` <20140922114317.GN1994-2JSQmVVBSi7ZROr8t4l/smS4ubULX0JqMm0uRHvK7Nw@public.gmane.org>
2014-09-22 12:15         ` Robert Richter [this message]
2014-09-22 12:15           ` Robert Richter
2014-09-23  7:56       ` Arnd Bergmann
2014-09-23  7:56         ` Arnd Bergmann
2014-09-23 10:49         ` Liviu Dudau
2014-09-23 10:49           ` Liviu Dudau
2014-09-23 13:30         ` Rob Herring
2014-09-23 13:30           ` Rob Herring
2014-09-23 13:58           ` Liviu Dudau
2014-09-23 13:58             ` Liviu Dudau
2014-09-18  1:30 ` [PATCH v11 09/10] PCI: Assign unassigned bus resources in pci_scan_root_bus() Liviu Dudau
2014-09-18  1:30   ` Liviu Dudau
2014-09-18  1:30 ` [PATCH v11 10/10] PCI: Introduce pci_remap_iospace() for remapping PCI I/O bus resources into CPU space Liviu Dudau
2014-09-18  1:30   ` Liviu Dudau
2014-09-19 21:54   ` Bjorn Helgaas
2014-09-19 21:54     ` Bjorn Helgaas
2014-09-20  2:20     ` Arnd Bergmann
2014-09-20  2:20       ` Arnd Bergmann
2014-09-22 15:29       ` Robert Richter
2014-09-20  0:34   ` Rob Herring
2014-09-20  0:34     ` Rob Herring
2014-09-19 21:59 ` [PATCH v11 00/10] Support for creating generic PCI host bridges from DT Bjorn Helgaas
2014-09-19 21:59   ` Bjorn Helgaas
2014-09-22 11:35   ` Liviu Dudau

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=20140922121547.GD20170@rric.localhost \
    --to=rric-dgejt+ai2ygdnm+yrofe0a@public.gmane.org \
    --cc=Catalin.Marinas-5wv7dgnIgG8@public.gmane.org \
    --cc=Liviu.Dudau-5wv7dgnIgG8@public.gmane.org \
    --cc=Will.Deacon-5wv7dgnIgG8@public.gmane.org \
    --cc=arnd-r2nGTMty4D4@public.gmane.org \
    --cc=benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org \
    --cc=bhelgaas-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org \
    --cc=jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org \
    --cc=jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org \
    --cc=kgene.kim-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org \
    --cc=linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=linux-arch-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org \
    --cc=linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=okaya-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
    --cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=suravee.suthikulpanit-5C7GfCeVMHo@public.gmane.org \
    --cc=tinamdar-qTEPVZfXA3Y@public.gmane.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 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).