From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
To: Guenter Roeck <linux@roeck-us.net>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"suravee.suthikulpanit@amd.com" <suravee.suthikulpanit@amd.com>,
Will Deacon <Will.Deacon@arm.com>
Subject: Re: [PATCH] PCI: Only enable IO window if supported
Date: Fri, 19 Jun 2015 17:24:13 +0100 [thread overview]
Message-ID: <20150619162413.GA21533@red-moon> (raw)
In-Reply-To: <20150618180103.GA23809@roeck-us.net>
Hi Guenter,
On Thu, Jun 18, 2015 at 07:01:03PM +0100, Guenter Roeck wrote:
> On Thu, May 28, 2015 at 07:41:12AM -0500, Bjorn Helgaas wrote:
> >
> > > > I'd like res->flags to reflect the capabilities of the hardware, not
> > > > whether the window is currently enabled.
> > > >
> > > Flag bits seem to be all taken. Could we use IORESOURCE_DISABLED for that
> > > purpose, or could that cause conflicts elsewhere ?
> >
> > Yes, I think IORESOURCE_DISABLED would be appropriate for any I/O windows
> > below a host bridge that doesn't support I/O space.
> >
> I integrated Lorenzo's patch and tried to get this working.
Thanks. How do you want to proceed with this ? Are you taking my patch
and post it along with your updated series ? We need to extend test
coverage to platforms we could not test on, as you know my series
affects all archs but SPARC (I mean it should *not* affect them, this
has to be tested though, I do not have the HW needed, your coverage
for x86 and PowerPC is great but I do not think it can be deemed
sufficient).
Please let me know, thanks !
Lorenzo
> Problem is that the use of a resource is widely checked with "!res->flags"
> throughout the code. That would have to be changed to something like
> "(!res->flags || (res->flags & IORESOURCE_DISABLED))" whereever it is used.
>
> I tried going with "!res->flags" instead, but have not been able to get it
> to work realiably; it is just very difficult to distinguish if "!res->flags"
> means that the resource has not yet been assigned or if it means that it is not
> supported.
>
> The correct approach, in my opinion, would be to go with IORESOURCE_DISABLED
> and make the necessary changes whereever needed. Effectively this means to
> replace the "!res->flags" check with something like pci_res_used() [ pick
> your preferred name ] and define it as
>
> #define pci_res_used(res) ((res)->flags && !((res)->flags & IORESOURCE_DISABLED))
>
> Is that the right direction ?
>
> Thanks,
> Guenter
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
WARNING: multiple messages have this Message-ID (diff)
From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
To: Guenter Roeck <linux@roeck-us.net>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"suravee.suthikulpanit@amd.com" <suravee.suthikulpanit@amd.com>,
Will Deacon <Will.Deacon@arm.com>
Subject: Re: [PATCH] PCI: Only enable IO window if supported
Date: Fri, 19 Jun 2015 17:24:13 +0100 [thread overview]
Message-ID: <20150619162413.GA21533@red-moon> (raw)
In-Reply-To: <20150618180103.GA23809@roeck-us.net>
Hi Guenter,
On Thu, Jun 18, 2015 at 07:01:03PM +0100, Guenter Roeck wrote:
> On Thu, May 28, 2015 at 07:41:12AM -0500, Bjorn Helgaas wrote:
> >
> > > > I'd like res->flags to reflect the capabilities of the hardware, not
> > > > whether the window is currently enabled.
> > > >
> > > Flag bits seem to be all taken. Could we use IORESOURCE_DISABLED for that
> > > purpose, or could that cause conflicts elsewhere ?
> >
> > Yes, I think IORESOURCE_DISABLED would be appropriate for any I/O windows
> > below a host bridge that doesn't support I/O space.
> >
> I integrated Lorenzo's patch and tried to get this working.
Thanks. How do you want to proceed with this ? Are you taking my patch
and post it along with your updated series ? We need to extend test
coverage to platforms we could not test on, as you know my series
affects all archs but SPARC (I mean it should *not* affect them, this
has to be tested though, I do not have the HW needed, your coverage
for x86 and PowerPC is great but I do not think it can be deemed
sufficient).
Please let me know, thanks !
Lorenzo
> Problem is that the use of a resource is widely checked with "!res->flags"
> throughout the code. That would have to be changed to something like
> "(!res->flags || (res->flags & IORESOURCE_DISABLED))" whereever it is used.
>
> I tried going with "!res->flags" instead, but have not been able to get it
> to work realiably; it is just very difficult to distinguish if "!res->flags"
> means that the resource has not yet been assigned or if it means that it is not
> supported.
>
> The correct approach, in my opinion, would be to go with IORESOURCE_DISABLED
> and make the necessary changes whereever needed. Effectively this means to
> replace the "!res->flags" check with something like pci_res_used() [ pick
> your preferred name ] and define it as
>
> #define pci_res_used(res) ((res)->flags && !((res)->flags & IORESOURCE_DISABLED))
>
> Is that the right direction ?
>
> Thanks,
> Guenter
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at http://www.tux.org/lkml/
next prev parent reply other threads:[~2015-06-19 16:23 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-23 0:52 [PATCH] PCI: Only enable IO window if supported Guenter Roeck
2015-05-27 21:04 ` Bjorn Helgaas
2015-05-28 2:23 ` Guenter Roeck
2015-05-28 12:41 ` Bjorn Helgaas
2015-06-18 18:01 ` Guenter Roeck
2015-06-18 19:51 ` Bjorn Helgaas
2015-06-18 20:53 ` Guenter Roeck
2015-06-19 16:24 ` Lorenzo Pieralisi [this message]
2015-06-19 16:24 ` Lorenzo Pieralisi
2015-07-07 14:40 ` Lorenzo Pieralisi
2015-07-07 15:01 ` Guenter Roeck
2015-07-07 17:28 ` Lorenzo Pieralisi
2015-07-07 18:13 ` Guenter Roeck
2015-06-02 14:55 ` Lorenzo Pieralisi
2015-06-02 16:32 ` Bjorn Helgaas
2015-06-02 17:02 ` Guenter Roeck
2015-06-02 19:58 ` Bjorn Helgaas
2015-06-03 15:15 ` Guenter Roeck
2015-06-03 10:32 ` Lorenzo Pieralisi
2015-06-03 15:12 ` Guenter Roeck
2015-06-03 16:55 ` Lorenzo Pieralisi
2015-06-03 18:07 ` Guenter Roeck
2015-06-23 22:46 ` Benjamin Herrenschmidt
2015-06-23 23:02 ` Bjorn Helgaas
2015-06-23 23:14 ` Benjamin Herrenschmidt
2015-06-25 11:27 ` Lorenzo Pieralisi
2015-07-08 8:38 ` Lorenzo Pieralisi
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=20150619162413.GA21533@red-moon \
--to=lorenzo.pieralisi@arm.com \
--cc=Will.Deacon@arm.com \
--cc=bhelgaas@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=suravee.suthikulpanit@amd.com \
/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.