diff mbox

Introduce ppc_pci_flags accessors

Message ID 20081210191148.GA1769@yoda.jdub.homelinux.org (mailing list archive)
State Changes Requested, archived
Headers show

Commit Message

Josh Boyer Dec. 10, 2008, 7:11 p.m. UTC
Currently there are a number of platforms that open code access to
the ppc_pci_flags global variable.  However, that variable is not
present if CONFIG_PCI is not set, which can lead to a build break.

This introduces a number of accessor functions that are defined
to be empty in the case of CONFIG_PCI being disabled.  The
various platform files in the kernel are updated to use these.

Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com>

---

Comments

Michael Ellerman Dec. 10, 2008, 11:46 p.m. UTC | #1
On Wed, 2008-12-10 at 14:11 -0500, Josh Boyer wrote:
> Currently there are a number of platforms that open code access to
> the ppc_pci_flags global variable.  However, that variable is not
> present if CONFIG_PCI is not set, which can lead to a build break.
> 
> This introduces a number of accessor functions that are defined
> to be empty in the case of CONFIG_PCI being disabled.  The
> various platform files in the kernel are updated to use these.
> 
> Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com>
> 
> ---
> 
> diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
> index fa8b3b7..8f2c7ca 100644
> --- a/arch/powerpc/include/asm/pci-bridge.h
> +++ b/arch/powerpc/include/asm/pci-bridge.h
> @@ -13,7 +13,6 @@
>  
>  struct device_node;
>  
> -extern unsigned int ppc_pci_flags;
>  enum {
>  	/* Force re-assigning all resources (ignore firmware
>  	 * setup completely)
> @@ -36,6 +35,16 @@ enum {
>  	/* ... except for domain 0 */
>  	PPC_PCI_COMPAT_DOMAIN_0		= 0x00000020,
>  };
> +#ifdef CONFIG_PCI
> +extern unsigned int ppc_pci_flags;
> +#define ppc_pci_set_flags(flags) ppc_pci_flags = (flags)
> +#define ppc_pci_add_flags(flags) ppc_pci_flags |= (flags)
> +#define ppc_pci_flag_is_set(flag) (ppc_pci_flags & (flag))
> +#else
> +#define ppc_pci_set_flags(flags) do {} while (0)
> +#define ppc_pci_add_flags(flags) do {} while (0)
> +#define ppc_pci_flag_is_set(flag) (0)
> +#endif

I hate to be picky, but I don't see any reason why these shouldn't be
static inlines.

cheers
Josh Boyer Dec. 10, 2008, 11:54 p.m. UTC | #2
On Thu, 11 Dec 2008 10:46:28 +1100
Michael Ellerman <michael@ellerman.id.au> wrote:

> On Wed, 2008-12-10 at 14:11 -0500, Josh Boyer wrote:
> > Currently there are a number of platforms that open code access to
> > the ppc_pci_flags global variable.  However, that variable is not
> > present if CONFIG_PCI is not set, which can lead to a build break.
> > 
> > This introduces a number of accessor functions that are defined
> > to be empty in the case of CONFIG_PCI being disabled.  The
> > various platform files in the kernel are updated to use these.
> > 
> > Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com>
> > 
> > ---
> > 
> > diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
> > index fa8b3b7..8f2c7ca 100644
> > --- a/arch/powerpc/include/asm/pci-bridge.h
> > +++ b/arch/powerpc/include/asm/pci-bridge.h
> > @@ -13,7 +13,6 @@
> >  
> >  struct device_node;
> >  
> > -extern unsigned int ppc_pci_flags;
> >  enum {
> >  	/* Force re-assigning all resources (ignore firmware
> >  	 * setup completely)
> > @@ -36,6 +35,16 @@ enum {
> >  	/* ... except for domain 0 */
> >  	PPC_PCI_COMPAT_DOMAIN_0		= 0x00000020,
> >  };
> > +#ifdef CONFIG_PCI
> > +extern unsigned int ppc_pci_flags;
> > +#define ppc_pci_set_flags(flags) ppc_pci_flags = (flags)
> > +#define ppc_pci_add_flags(flags) ppc_pci_flags |= (flags)
> > +#define ppc_pci_flag_is_set(flag) (ppc_pci_flags & (flag))
> > +#else
> > +#define ppc_pci_set_flags(flags) do {} while (0)
> > +#define ppc_pci_add_flags(flags) do {} while (0)
> > +#define ppc_pci_flag_is_set(flag) (0)
> > +#endif
> 
> I hate to be picky, but I don't see any reason why these shouldn't be
> static inlines.

There's a perfectly good reason.  I AM LAZY.

That aside, it doesn't matter to me either way.  If the general idea
seems fine and the naming of the functions is acceptable, I'd be happy
to respin.

josh
Michael Ellerman Dec. 11, 2008, 12:04 a.m. UTC | #3
On Wed, 2008-12-10 at 18:54 -0500, Josh Boyer wrote:
> On Thu, 11 Dec 2008 10:46:28 +1100
> Michael Ellerman <michael@ellerman.id.au> wrote:
> 
> > On Wed, 2008-12-10 at 14:11 -0500, Josh Boyer wrote:
> > > Currently there are a number of platforms that open code access to
> > > the ppc_pci_flags global variable.  However, that variable is not
> > > present if CONFIG_PCI is not set, which can lead to a build break.
> > > 
> > > This introduces a number of accessor functions that are defined
> > > to be empty in the case of CONFIG_PCI being disabled.  The
> > > various platform files in the kernel are updated to use these.
> > > 
> > > Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com>
> > > 
> > > ---
> > > 
> > > diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
> > > index fa8b3b7..8f2c7ca 100644
> > > --- a/arch/powerpc/include/asm/pci-bridge.h
> > > +++ b/arch/powerpc/include/asm/pci-bridge.h
> > > @@ -13,7 +13,6 @@
> > >  
> > >  struct device_node;
> > >  
> > > -extern unsigned int ppc_pci_flags;
> > >  enum {
> > >  	/* Force re-assigning all resources (ignore firmware
> > >  	 * setup completely)
> > > @@ -36,6 +35,16 @@ enum {
> > >  	/* ... except for domain 0 */
> > >  	PPC_PCI_COMPAT_DOMAIN_0		= 0x00000020,
> > >  };
> > > +#ifdef CONFIG_PCI
> > > +extern unsigned int ppc_pci_flags;
> > > +#define ppc_pci_set_flags(flags) ppc_pci_flags = (flags)
> > > +#define ppc_pci_add_flags(flags) ppc_pci_flags |= (flags)
> > > +#define ppc_pci_flag_is_set(flag) (ppc_pci_flags & (flag))
> > > +#else
> > > +#define ppc_pci_set_flags(flags) do {} while (0)
> > > +#define ppc_pci_add_flags(flags) do {} while (0)
> > > +#define ppc_pci_flag_is_set(flag) (0)
> > > +#endif
> > 
> > I hate to be picky, but I don't see any reason why these shouldn't be
> > static inlines.
> 
> There's a perfectly good reason.  I AM LAZY.

Fair enough, I'll do the typing for you :)

#ifdef CONFIG_PCI
extern unsigned int ppc_pci_flags;

static inline void ppc_pci_set_flags(int flags)
{
	ppc_pci_flags = flags;
}

static inline void ppc_pci_add_flags(int flags)
{
	ppc_pci_flags |= flags;
}

static inline int ppc_pci_flag_is_set(int flag)
{
	return (ppc_pci_flags & flag);
}
#else
static inline void ppc_pci_set_flags(int flags) { }
static inline void ppc_pci_add_flags(int flags) { }
static inline int ppc_pci_flag_is_set(int flag)
{
	return 0;
}
#endif


One other thought, should the check routine be
ppc_pci_flags_are_set(flags)? 

cheers
Trent Piepho Dec. 11, 2008, 12:17 a.m. UTC | #4
On Wed, 10 Dec 2008, Josh Boyer wrote:
> On Thu, 11 Dec 2008 10:46:28 +1100
>>> +#ifdef CONFIG_PCI
>>> +extern unsigned int ppc_pci_flags;
>>> +#define ppc_pci_set_flags(flags) ppc_pci_flags = (flags)
>>> +#define ppc_pci_add_flags(flags) ppc_pci_flags |= (flags)
>>> +#define ppc_pci_flag_is_set(flag) (ppc_pci_flags & (flag))
>>> +#else
>>> +#define ppc_pci_set_flags(flags) do {} while (0)
>>> +#define ppc_pci_add_flags(flags) do {} while (0)
>>> +#define ppc_pci_flag_is_set(flag) (0)
>>> +#endif
>>
>> I hate to be picky, but I don't see any reason why these shouldn't be
>> static inlines.
>
> There's a perfectly good reason.  I AM LAZY.
>
> That aside, it doesn't matter to me either way.  If the general idea
> seems fine and the naming of the functions is acceptable, I'd be happy
> to respin.

If were allowed to be picky, I think ppc_pci_has_flag() is a better name
than ppc_pci_flag_is_set().  Matches the other function names better, and a
quick grep of the kernel source shows bar_has_foo() is much more common
than bar_foo_is_set().
Josh Boyer Dec. 11, 2008, 12:47 a.m. UTC | #5
On Thu, 11 Dec 2008 11:04:05 +1100
Michael Ellerman <michael@ellerman.id.au> wrote:

> On Wed, 2008-12-10 at 18:54 -0500, Josh Boyer wrote:
> > On Thu, 11 Dec 2008 10:46:28 +1100
> > Michael Ellerman <michael@ellerman.id.au> wrote:
> > 
> > > On Wed, 2008-12-10 at 14:11 -0500, Josh Boyer wrote:
> > > > Currently there are a number of platforms that open code access to
> > > > the ppc_pci_flags global variable.  However, that variable is not
> > > > present if CONFIG_PCI is not set, which can lead to a build break.
> > > > 
> > > > This introduces a number of accessor functions that are defined
> > > > to be empty in the case of CONFIG_PCI being disabled.  The
> > > > various platform files in the kernel are updated to use these.
> > > > 
> > > > Signed-off-by: Josh Boyer <jwboyer@linux.vnet.ibm.com>
> > > > 
> > > > ---
> > > > 
> > > > diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
> > > > index fa8b3b7..8f2c7ca 100644
> > > > --- a/arch/powerpc/include/asm/pci-bridge.h
> > > > +++ b/arch/powerpc/include/asm/pci-bridge.h
> > > > @@ -13,7 +13,6 @@
> > > >  
> > > >  struct device_node;
> > > >  
> > > > -extern unsigned int ppc_pci_flags;
> > > >  enum {
> > > >  	/* Force re-assigning all resources (ignore firmware
> > > >  	 * setup completely)
> > > > @@ -36,6 +35,16 @@ enum {
> > > >  	/* ... except for domain 0 */
> > > >  	PPC_PCI_COMPAT_DOMAIN_0		= 0x00000020,
> > > >  };
> > > > +#ifdef CONFIG_PCI
> > > > +extern unsigned int ppc_pci_flags;
> > > > +#define ppc_pci_set_flags(flags) ppc_pci_flags = (flags)
> > > > +#define ppc_pci_add_flags(flags) ppc_pci_flags |= (flags)
> > > > +#define ppc_pci_flag_is_set(flag) (ppc_pci_flags & (flag))
> > > > +#else
> > > > +#define ppc_pci_set_flags(flags) do {} while (0)
> > > > +#define ppc_pci_add_flags(flags) do {} while (0)
> > > > +#define ppc_pci_flag_is_set(flag) (0)
> > > > +#endif
> > > 
> > > I hate to be picky, but I don't see any reason why these shouldn't be
> > > static inlines.
> > 
> > There's a perfectly good reason.  I AM LAZY.
> 
> Fair enough, I'll do the typing for you :)

Catering to my laziness... not sure if that is good or bad :).

> #ifdef CONFIG_PCI
> extern unsigned int ppc_pci_flags;
> 
> static inline void ppc_pci_set_flags(int flags)
> {
> 	ppc_pci_flags = flags;
> }
> 
> static inline void ppc_pci_add_flags(int flags)
> {
> 	ppc_pci_flags |= flags;
> }
> 
> static inline int ppc_pci_flag_is_set(int flag)
> {
> 	return (ppc_pci_flags & flag);
> }
> #else
> static inline void ppc_pci_set_flags(int flags) { }
> static inline void ppc_pci_add_flags(int flags) { }
> static inline int ppc_pci_flag_is_set(int flag)
> {
> 	return 0;
> }
> #endif
> 
> 
> One other thought, should the check routine be
> ppc_pci_flags_are_set(flags)? 

Likely, yes.  I can change that and I'll respin.

josh
Josh Boyer Dec. 11, 2008, 12:53 a.m. UTC | #6
On Wed, 10 Dec 2008 16:17:13 -0800 (PST)
Trent Piepho <tpiepho@freescale.com> wrote:

> On Wed, 10 Dec 2008, Josh Boyer wrote:
> > On Thu, 11 Dec 2008 10:46:28 +1100
> >>> +#ifdef CONFIG_PCI
> >>> +extern unsigned int ppc_pci_flags;
> >>> +#define ppc_pci_set_flags(flags) ppc_pci_flags = (flags)
> >>> +#define ppc_pci_add_flags(flags) ppc_pci_flags |= (flags)
> >>> +#define ppc_pci_flag_is_set(flag) (ppc_pci_flags & (flag))
> >>> +#else
> >>> +#define ppc_pci_set_flags(flags) do {} while (0)
> >>> +#define ppc_pci_add_flags(flags) do {} while (0)
> >>> +#define ppc_pci_flag_is_set(flag) (0)
> >>> +#endif
> >>
> >> I hate to be picky, but I don't see any reason why these shouldn't be
> >> static inlines.
> >
> > There's a perfectly good reason.  I AM LAZY.
> >
> > That aside, it doesn't matter to me either way.  If the general idea
> > seems fine and the naming of the functions is acceptable, I'd be happy
> > to respin.
> 
> If were allowed to be picky, I think ppc_pci_has_flag() is a better name
> than ppc_pci_flag_is_set().  Matches the other function names better, and a
> quick grep of the kernel source shows bar_has_foo() is much more common
> than bar_foo_is_set().

That's fine too.  I think you can Michael can have a virtual
arm-wrestling match to decide whether ppc_pci_has_flag or
ppc_pci_flags_are_set wins ;)

josh
Benjamin Herrenschmidt Dec. 11, 2008, 11:06 a.m. UTC | #7
On Wed, 2008-12-10 at 19:53 -0500, Josh Boyer wrote:
> 
> That's fine too.  I think you can Michael can have a virtual
> arm-wrestling match to decide whether ppc_pci_has_flag or
> ppc_pci_flags_are_set wins ;)

I vote for _has_flag() :-)

I even wonder if we need the word "flag" in there at all, but then I'm
just being anal !

Cheers,
Ben.
diff mbox

Patch

diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
index fa8b3b7..8f2c7ca 100644
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h
@@ -13,7 +13,6 @@ 
 
 struct device_node;
 
-extern unsigned int ppc_pci_flags;
 enum {
 	/* Force re-assigning all resources (ignore firmware
 	 * setup completely)
@@ -36,6 +35,16 @@  enum {
 	/* ... except for domain 0 */
 	PPC_PCI_COMPAT_DOMAIN_0		= 0x00000020,
 };
+#ifdef CONFIG_PCI
+extern unsigned int ppc_pci_flags;
+#define ppc_pci_set_flags(flags) ppc_pci_flags = (flags)
+#define ppc_pci_add_flags(flags) ppc_pci_flags |= (flags)
+#define ppc_pci_flag_is_set(flag) (ppc_pci_flags & (flag))
+#else
+#define ppc_pci_set_flags(flags) do {} while (0)
+#define ppc_pci_add_flags(flags) do {} while (0)
+#define ppc_pci_flag_is_set(flag) (0)
+#endif
 
 
 /*
diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
index 1c721a6..535ccb6 100644
--- a/arch/powerpc/include/asm/pci.h
+++ b/arch/powerpc/include/asm/pci.h
@@ -38,8 +38,8 @@  struct pci_dev;
  * Set this to 1 if you want the kernel to re-assign all PCI
  * bus numbers (don't do that on ppc64 yet !)
  */
-#define pcibios_assign_all_busses()    	(ppc_pci_flags & \
-					 PPC_PCI_REASSIGN_ALL_BUS)
+#define pcibios_assign_all_busses() \
+	(ppc_pci_flag_is_set(PPC_PCI_REASSIGN_ALL_BUS))
 #define pcibios_scan_all_fns(a, b)	0
 
 static inline void pcibios_set_master(struct pci_dev *dev)
diff --git a/arch/powerpc/platforms/40x/ep405.c b/arch/powerpc/platforms/40x/ep405.c
index ae2e7f6..4058fd1 100644
--- a/arch/powerpc/platforms/40x/ep405.c
+++ b/arch/powerpc/platforms/40x/ep405.c
@@ -100,7 +100,7 @@  static void __init ep405_setup_arch(void)
 	/* Find & init the BCSR CPLD */
 	ep405_init_bcsr();
 
-	ppc_pci_flags = PPC_PCI_REASSIGN_ALL_RSRC;
+	ppc_pci_set_flags(PPC_PCI_REASSIGN_ALL_RSRC);
 }
 
 static int __init ep405_probe(void)
diff --git a/arch/powerpc/platforms/40x/kilauea.c b/arch/powerpc/platforms/40x/kilauea.c
index 1dd24ff..fd7d934 100644
--- a/arch/powerpc/platforms/40x/kilauea.c
+++ b/arch/powerpc/platforms/40x/kilauea.c
@@ -44,7 +44,7 @@  static int __init kilauea_probe(void)
 	if (!of_flat_dt_is_compatible(root, "amcc,kilauea"))
 		return 0;
 
-	ppc_pci_flags = PPC_PCI_REASSIGN_ALL_RSRC;
+	ppc_pci_set_flags(PPC_PCI_REASSIGN_ALL_RSRC);
 
 	return 1;
 }
diff --git a/arch/powerpc/platforms/40x/ppc40x_simple.c b/arch/powerpc/platforms/40x/ppc40x_simple.c
index 4498a86..f40ac9b 100644
--- a/arch/powerpc/platforms/40x/ppc40x_simple.c
+++ b/arch/powerpc/platforms/40x/ppc40x_simple.c
@@ -61,7 +61,7 @@  static int __init ppc40x_probe(void)
 
 	for (i = 0; i < ARRAY_SIZE(board); i++) {
 		if (of_flat_dt_is_compatible(root, board[i])) {
-			ppc_pci_flags = PPC_PCI_REASSIGN_ALL_RSRC;
+			ppc_pci_set_flags(PPC_PCI_REASSIGN_ALL_RSRC);
 			return 1;
 		}
 	}
diff --git a/arch/powerpc/platforms/44x/ebony.c b/arch/powerpc/platforms/44x/ebony.c
index a0e8fe4..88b9117 100644
--- a/arch/powerpc/platforms/44x/ebony.c
+++ b/arch/powerpc/platforms/44x/ebony.c
@@ -54,7 +54,7 @@  static int __init ebony_probe(void)
 	if (!of_flat_dt_is_compatible(root, "ibm,ebony"))
 		return 0;
 
-	ppc_pci_flags = PPC_PCI_REASSIGN_ALL_RSRC;
+	ppc_pci_set_flags(PPC_PCI_REASSIGN_ALL_RSRC);
 
 	return 1;
 }
diff --git a/arch/powerpc/platforms/44x/ppc44x_simple.c b/arch/powerpc/platforms/44x/ppc44x_simple.c
index 2967126..76fdc51 100644
--- a/arch/powerpc/platforms/44x/ppc44x_simple.c
+++ b/arch/powerpc/platforms/44x/ppc44x_simple.c
@@ -69,7 +69,7 @@  static int __init ppc44x_probe(void)
 
 	for (i = 0; i < ARRAY_SIZE(board); i++) {
 		if (of_flat_dt_is_compatible(root, board[i])) {
-			ppc_pci_flags = PPC_PCI_REASSIGN_ALL_RSRC;
+			ppc_pci_set_flags(PPC_PCI_REASSIGN_ALL_RSRC);
 			return 1;
 		}
 	}
diff --git a/arch/powerpc/platforms/44x/sam440ep.c b/arch/powerpc/platforms/44x/sam440ep.c
index 47f10e6..a78e8eb 100644
--- a/arch/powerpc/platforms/44x/sam440ep.c
+++ b/arch/powerpc/platforms/44x/sam440ep.c
@@ -51,7 +51,7 @@  static int __init sam440ep_probe(void)
 	if (!of_flat_dt_is_compatible(root, "acube,sam440ep"))
 		return 0;
 
-	ppc_pci_flags = PPC_PCI_REASSIGN_ALL_RSRC;
+	ppc_pci_set_flags(PPC_PCI_REASSIGN_ALL_RSRC);
 
 	return 1;
 }
diff --git a/arch/powerpc/platforms/52xx/mpc52xx_pci.c b/arch/powerpc/platforms/52xx/mpc52xx_pci.c
index b49a185..c3f2c21 100644
--- a/arch/powerpc/platforms/52xx/mpc52xx_pci.c
+++ b/arch/powerpc/platforms/52xx/mpc52xx_pci.c
@@ -375,7 +375,7 @@  mpc52xx_add_bridge(struct device_node *node)
 
 	pr_debug("Adding MPC52xx PCI host bridge %s\n", node->full_name);
 
-	ppc_pci_flags |= PPC_PCI_REASSIGN_ALL_BUS;
+	ppc_pci_add_flags(PPC_PCI_REASSIGN_ALL_BUS);
 
 	if (of_address_to_resource(node, 0, &rsrc) != 0) {
 		printk(KERN_ERR "Can't get %s resources\n", node->full_name);
diff --git a/arch/powerpc/platforms/82xx/pq2.c b/arch/powerpc/platforms/82xx/pq2.c
index 1b75902..9761a59 100644
--- a/arch/powerpc/platforms/82xx/pq2.c
+++ b/arch/powerpc/platforms/82xx/pq2.c
@@ -53,7 +53,7 @@  static void __init pq2_pci_add_bridge(struct device_node *np)
 	if (of_address_to_resource(np, 0, &r) || r.end - r.start < 0x10b)
 		goto err;
 
-	ppc_pci_flags |= PPC_PCI_REASSIGN_ALL_BUS;
+	ppc_pci_add_flags(PPC_PCI_REASSIGN_ALL_BUS);
 
 	hose = pcibios_alloc_controller(np);
 	if (!hose)
diff --git a/arch/powerpc/platforms/chrp/pci.c b/arch/powerpc/platforms/chrp/pci.c
index d3cde6b..e0392e2 100644
--- a/arch/powerpc/platforms/chrp/pci.c
+++ b/arch/powerpc/platforms/chrp/pci.c
@@ -198,7 +198,7 @@  static void __init setup_peg2(struct pci_controller *hose, struct device_node *d
 		printk ("RTAS supporting Pegasos OF not found, please upgrade"
 			" your firmware\n");
 	}
-	ppc_pci_flags |= PPC_PCI_REASSIGN_ALL_BUS;
+	ppc_pci_add_flags(PPC_PCI_REASSIGN_ALL_BUS);
 	/* keep the reference to the root node */
 }
 
diff --git a/arch/powerpc/platforms/powermac/pci.c b/arch/powerpc/platforms/powermac/pci.c
index bcf50d7..61f1a39 100644
--- a/arch/powerpc/platforms/powermac/pci.c
+++ b/arch/powerpc/platforms/powermac/pci.c
@@ -729,7 +729,7 @@  static void __init setup_bandit(struct pci_controller *hose,
 static int __init setup_uninorth(struct pci_controller *hose,
 				 struct resource *addr)
 {
-	ppc_pci_flags |= PPC_PCI_REASSIGN_ALL_BUS;
+	ppc_pci_add_flags(PPC_PCI_REASSIGN_ALL_BUS);
 	has_uninorth = 1;
 	hose->ops = &macrisc_pci_ops;
 	hose->cfg_addr = ioremap(addr->start + 0x800000, 0x1000);
@@ -996,7 +996,7 @@  void __init pmac_pci_init(void)
 	struct device_node *np, *root;
 	struct device_node *ht = NULL;
 
-	ppc_pci_flags = PPC_PCI_CAN_SKIP_ISA_ALIGN;
+	ppc_pci_set_flags(PPC_PCI_CAN_SKIP_ISA_ALIGN);
 
 	root = of_find_node_by_path("/");
 	if (root == NULL) {
@@ -1055,7 +1055,7 @@  void __init pmac_pci_init(void)
 	 * some offset between bus number and domains for now when we
 	 * assign all busses should help for now
 	 */
-	if (ppc_pci_flags & PPC_PCI_REASSIGN_ALL_BUS)
+	if (ppc_pci_flag_is_set(PPC_PCI_REASSIGN_ALL_BUS))
 		pcibios_assign_bus_offset = 0x10;
 #endif
 }
diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index 5b264eb..d5f9ae0 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -187,7 +187,7 @@  int __init fsl_add_bridge(struct device_node *dev, int is_primary)
 		printk(KERN_WARNING "Can't get bus-range for %s, assume"
 			" bus 0\n", dev->full_name);
 
-	ppc_pci_flags |= PPC_PCI_REASSIGN_ALL_BUS;
+	ppc_pci_add_flags(PPC_PCI_REASSIGN_ALL_BUS);
 	hose = pcibios_alloc_controller(dev);
 	if (!hose)
 		return -ENOMEM;
@@ -300,7 +300,7 @@  int __init mpc83xx_add_bridge(struct device_node *dev)
 		       " bus 0\n", dev->full_name);
 	}
 
-	ppc_pci_flags |= PPC_PCI_REASSIGN_ALL_BUS;
+	ppc_pci_add_flags(PPC_PCI_REASSIGN_ALL_BUS);
 	hose = pcibios_alloc_controller(dev);
 	if (!hose)
 		return -ENOMEM;
diff --git a/arch/powerpc/sysdev/grackle.c b/arch/powerpc/sysdev/grackle.c
index d502927..5da37c2 100644
--- a/arch/powerpc/sysdev/grackle.c
+++ b/arch/powerpc/sysdev/grackle.c
@@ -57,7 +57,7 @@  void __init setup_grackle(struct pci_controller *hose)
 {
 	setup_indirect_pci(hose, 0xfec00000, 0xfee00000, 0);
 	if (machine_is_compatible("PowerMac1,1"))
-		ppc_pci_flags |= PPC_PCI_REASSIGN_ALL_BUS;
+		ppc_pci_add_flags(PPC_PCI_REASSIGN_ALL_BUS);
 	if (machine_is_compatible("AAPL,PowerBook1998"))
 		grackle_set_loop_snoop(hose, 1);
 #if 0	/* Disabled for now, HW problems ??? */