Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 06 Feb 2012 11:07:17 -0600
From:      Nathan Whitehorn <nwhitehorn@freebsd.org>
To:        John Baldwin <jhb@freebsd.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r231026 - head/sys/powerpc/ofw
Message-ID:  <4F3008C5.8020104@freebsd.org>
In-Reply-To: <201202061106.59098.jhb@freebsd.org>
References:  <201202051654.q15GsQcc092137@svn.freebsd.org> <201202060753.43627.jhb@freebsd.org> <4F2FF486.5090507@freebsd.org> <201202061106.59098.jhb@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 02/06/12 10:06, John Baldwin wrote:
> On Monday, February 06, 2012 10:40:54 am Nathan Whitehorn wrote:
>> On 02/06/12 06:53, John Baldwin wrote:
>>> On Sunday, February 05, 2012 11:54:26 am Nathan Whitehorn wrote:
>>>> Author: nwhitehorn
>>>> Date: Sun Feb  5 16:54:26 2012
>>>> New Revision: 231026
>>>> URL: http://svn.freebsd.org/changeset/base/231026
>>>>
>>>> Log:
>>>>     Make sure to remap adjusted resources.
>>> Hmm, I had considered remapping adjusted resources an invalid operation (i.e.
>>> should fail with EINVAL).  I believe that the NEW_PCIB code should only apply
>>> this API to resources backing the resource windows in PCI-PCI bridges and that
>>> those resources should never have RF_ACTIVE set.  Are you seeing calls of it
>>> for active resources?
>>>
>> No, I was just trying to be safe here, since it wasn't clear that that
>> was invalid. I'm happy to replace the contents of the if with return
>> EINVAL or something.
> Yeah, I would do that, perhaps with a KASSERT() as well so it panics in HEAD.
>

How does this look? I've kept both the EINVAL and the KASSERT.
-Nathan

Index: ofw_pci.c
===================================================================
--- ofw_pci.c   (revision 231026)
+++ ofw_pci.c   (working copy)
@@ -431,8 +431,12 @@
  {
         struct rman *rm = NULL;
         struct ofw_pci_softc *sc = device_get_softc(bus);
-       int error;

+       KASSERT(!(rman_get_flags(res) & RF_ACTIVE),
+           ("active resources cannot be adjusted"));
+       if (rman_get_flags(res) & RF_ACTIVE)
+               return (EINVAL);
+
         switch (type) {
         case SYS_RES_MEMORY:
                 rm = &sc->sc_mem_rman;
@@ -447,21 +451,7 @@
         if (!rman_is_region_manager(res, rm))
                 return (EINVAL);

-       error = rman_adjust_resource(res, start, end);
-       if (error)
-               return (error);
-
-       if (rman_get_flags(res) & RF_ACTIVE) {
-               /* Remap memory resources */
-               error = ofw_pci_deactivate_resource(bus, child, type,
-                   rman_get_rid(res), res);
-               if (error)
-                       return (error);
-               error = ofw_pci_activate_resource(bus, child, type,
-                   rman_get_rid(res), res);
-       }
-
-       return (error);
+       return (rman_adjust_resource(res, start, end));
  }




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4F3008C5.8020104>