* [PATCH] pseries: asm/pci-bridge.h CONFIG_ minor cleanup
@ 2007-05-21 23:18 Linas Vepstas
2007-05-22 0:40 ` Michael Ellerman
0 siblings, 1 reply; 6+ messages in thread
From: Linas Vepstas @ 2007-05-21 23:18 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev
Use the correct CONFIG_ option to mark off the EEH bits.
Move the EEH bits to the bottom of the struct.
Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
----
include/asm-powerpc/pci-bridge.h | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
Index: linux-2.6.22-rc1/include/asm-powerpc/pci-bridge.h
===================================================================
--- linux-2.6.22-rc1.orig/include/asm-powerpc/pci-bridge.h 2007-05-21 18:13:00.000000000 -0500
+++ linux-2.6.22-rc1/include/asm-powerpc/pci-bridge.h 2007-05-21 18:13:36.000000000 -0500
@@ -70,19 +70,20 @@ struct pci_dn {
int devfn; /* pci device and function number */
int class_code; /* pci device class */
-#ifdef CONFIG_PPC_PSERIES
- int eeh_mode; /* See eeh.h for possible EEH_MODEs */
- int eeh_config_addr;
- int eeh_pe_config_addr; /* new-style partition endpoint address */
- int eeh_check_count; /* # times driver ignored error */
- int eeh_freeze_count; /* # times this device froze up. */
-#endif
int pci_ext_config_space; /* for pci devices */
struct pci_controller *phb; /* for pci devices */
struct iommu_table *iommu_table; /* for phb's or bridges */
struct pci_dev *pcidev; /* back-pointer to the pci device */
struct device_node *node; /* back-pointer to the device_node */
+
+#ifdef CONFIG_EEH
+ int eeh_mode; /* See eeh.h for possible EEH_MODEs */
+ int eeh_config_addr;
+ int eeh_pe_config_addr; /* new-style partition endpoint address */
+ int eeh_check_count; /* # times driver ignored error */
+ int eeh_freeze_count; /* # times this device froze up. */
u32 config_space[16]; /* saved PCI config space */
+#endif
};
/* Get the pointer to a device_node's pci_dn */
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] pseries: asm/pci-bridge.h CONFIG_ minor cleanup
2007-05-21 23:18 [PATCH] pseries: asm/pci-bridge.h CONFIG_ minor cleanup Linas Vepstas
@ 2007-05-22 0:40 ` Michael Ellerman
2007-05-22 17:00 ` Linas Vepstas
0 siblings, 1 reply; 6+ messages in thread
From: Michael Ellerman @ 2007-05-22 0:40 UTC (permalink / raw)
To: Linas Vepstas; +Cc: linuxppc-dev, Paul Mackerras
[-- Attachment #1: Type: text/plain, Size: 2325 bytes --]
On Mon, 2007-05-21 at 18:18 -0500, Linas Vepstas wrote:
> Use the correct CONFIG_ option to mark off the EEH bits.
> Move the EEH bits to the bottom of the struct.
>
> Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
>
> ----
> include/asm-powerpc/pci-bridge.h | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> Index: linux-2.6.22-rc1/include/asm-powerpc/pci-bridge.h
> ===================================================================
> --- linux-2.6.22-rc1.orig/include/asm-powerpc/pci-bridge.h 2007-05-21 18:13:00.000000000 -0500
> +++ linux-2.6.22-rc1/include/asm-powerpc/pci-bridge.h 2007-05-21 18:13:36.000000000 -0500
> @@ -70,19 +70,20 @@ struct pci_dn {
> int devfn; /* pci device and function number */
> int class_code; /* pci device class */
>
> -#ifdef CONFIG_PPC_PSERIES
> - int eeh_mode; /* See eeh.h for possible EEH_MODEs */
> - int eeh_config_addr;
> - int eeh_pe_config_addr; /* new-style partition endpoint address */
> - int eeh_check_count; /* # times driver ignored error */
> - int eeh_freeze_count; /* # times this device froze up. */
> -#endif
You're making the struct 4 bytes larger by creating a hole here :(
> int pci_ext_config_space; /* for pci devices */
> struct pci_controller *phb; /* for pci devices */
> struct iommu_table *iommu_table; /* for phb's or bridges */
> struct pci_dev *pcidev; /* back-pointer to the pci device */
> struct device_node *node; /* back-pointer to the device_node */
> +
> +#ifdef CONFIG_EEH
> + int eeh_mode; /* See eeh.h for possible EEH_MODEs */
> + int eeh_config_addr;
> + int eeh_pe_config_addr; /* new-style partition endpoint address */
> + int eeh_check_count; /* # times driver ignored error */
> + int eeh_freeze_count; /* # times this device froze up. */
> u32 config_space[16]; /* saved PCI config space */
> +#endif
It looks correct, but I think it's worth mentioning in the changelog
that config_space was previously unconditionally defined, but is now
within CONFIG_EEH.
cheers
--
Michael Ellerman
OzLabs, IBM Australia Development Lab
wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)
We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] pseries: asm/pci-bridge.h CONFIG_ minor cleanup
2007-05-22 0:40 ` Michael Ellerman
@ 2007-05-22 17:00 ` Linas Vepstas
2007-05-22 18:18 ` [PATCH] (revised) " Linas Vepstas
2007-05-23 1:11 ` [PATCH] " Michael Ellerman
0 siblings, 2 replies; 6+ messages in thread
From: Linas Vepstas @ 2007-05-22 17:00 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, Paul Mackerras
On Tue, May 22, 2007 at 10:40:59AM +1000, Michael Ellerman wrote:
> On Mon, 2007-05-21 at 18:18 -0500, Linas Vepstas wrote:
> > Use the correct CONFIG_ option to mark off the EEH bits.
>
> You're making the struct 4 bytes larger by creating a hole here :(
Hmm. Perhaps I could shuffle one of the ints down ...
I figured there might be some vague cache-line benefits to
getting the eeh stuff out of the way.
> > u32 config_space[16]; /* saved PCI config space */
> > +#endif
>
> It looks correct, but I think it's worth mentioning in the changelog
> that config_space was previously unconditionally defined, but is now
> within CONFIG_EEH.
I could just tell that someone would remark on this; why I didn't
mention it, I don't know.
--linas
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] (revised) pseries: asm/pci-bridge.h CONFIG_ minor cleanup
2007-05-22 17:00 ` Linas Vepstas
@ 2007-05-22 18:18 ` Linas Vepstas
2007-05-23 1:11 ` [PATCH] " Michael Ellerman
1 sibling, 0 replies; 6+ messages in thread
From: Linas Vepstas @ 2007-05-22 18:18 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev
Use the correct CONFIG_ option to mark off the EEH bits.
Move the EEH bits to the bottom of the struct.
The config_space array is used by EEH only; it does not
need to be part of the struct for non-pseries machines.
Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
----
Revised patch, per commments from Michael Ellerman.
include/asm-powerpc/pci-bridge.h | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
Index: linux-2.6.22-rc1/include/asm-powerpc/pci-bridge.h
===================================================================
--- linux-2.6.22-rc1.orig/include/asm-powerpc/pci-bridge.h 2007-05-21 18:13:00.000000000 -0500
+++ linux-2.6.22-rc1/include/asm-powerpc/pci-bridge.h 2007-05-22 12:03:50.000000000 -0500
@@ -70,19 +70,21 @@ struct pci_dn {
int devfn; /* pci device and function number */
int class_code; /* pci device class */
-#ifdef CONFIG_PPC_PSERIES
+ struct pci_controller *phb; /* for pci devices */
+ struct iommu_table *iommu_table; /* for phb's or bridges */
+ struct pci_dev *pcidev; /* back-pointer to the pci device */
+ struct device_node *node; /* back-pointer to the device_node */
+
+ int pci_ext_config_space; /* for pci devices */
+
+#ifdef CONFIG_EEH
int eeh_mode; /* See eeh.h for possible EEH_MODEs */
int eeh_config_addr;
int eeh_pe_config_addr; /* new-style partition endpoint address */
int eeh_check_count; /* # times driver ignored error */
int eeh_freeze_count; /* # times this device froze up. */
-#endif
- int pci_ext_config_space; /* for pci devices */
- struct pci_controller *phb; /* for pci devices */
- struct iommu_table *iommu_table; /* for phb's or bridges */
- struct pci_dev *pcidev; /* back-pointer to the pci device */
- struct device_node *node; /* back-pointer to the device_node */
u32 config_space[16]; /* saved PCI config space */
+#endif
};
/* Get the pointer to a device_node's pci_dn */
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] pseries: asm/pci-bridge.h CONFIG_ minor cleanup
2007-05-22 17:00 ` Linas Vepstas
2007-05-22 18:18 ` [PATCH] (revised) " Linas Vepstas
@ 2007-05-23 1:11 ` Michael Ellerman
2007-05-23 20:51 ` Linas Vepstas
1 sibling, 1 reply; 6+ messages in thread
From: Michael Ellerman @ 2007-05-23 1:11 UTC (permalink / raw)
To: Linas Vepstas; +Cc: linuxppc-dev, Paul Mackerras
[-- Attachment #1: Type: text/plain, Size: 1310 bytes --]
On Tue, 2007-05-22 at 12:00 -0500, Linas Vepstas wrote:
> On Tue, May 22, 2007 at 10:40:59AM +1000, Michael Ellerman wrote:
> > On Mon, 2007-05-21 at 18:18 -0500, Linas Vepstas wrote:
> > > Use the correct CONFIG_ option to mark off the EEH bits.
> >
> > You're making the struct 4 bytes larger by creating a hole here :(
>
> Hmm. Perhaps I could shuffle one of the ints down ...
>
> I figured there might be some vague cache-line benefits to
> getting the eeh stuff out of the way.
Yeah who knows. The hole doesn't really matter unless the struct gets
bigger, having the EEH stuff at the bottom makes it read better which is
probably more important.
> > > u32 config_space[16]; /* saved PCI config space */
> > > +#endif
> >
> > It looks correct, but I think it's worth mentioning in the changelog
> > that config_space was previously unconditionally defined, but is now
> > within CONFIG_EEH.
>
> I could just tell that someone would remark on this; why I didn't
> mention it, I don't know.
Me neither :)
cheers
--
Michael Ellerman
OzLabs, IBM Australia Development Lab
wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)
We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] pseries: asm/pci-bridge.h CONFIG_ minor cleanup
2007-05-23 1:11 ` [PATCH] " Michael Ellerman
@ 2007-05-23 20:51 ` Linas Vepstas
0 siblings, 0 replies; 6+ messages in thread
From: Linas Vepstas @ 2007-05-23 20:51 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, Paul Mackerras
On Wed, May 23, 2007 at 11:11:32AM +1000, Michael Ellerman wrote:
>
> Yeah who knows. The hole doesn't really matter unless the struct gets
> bigger, having the EEH stuff at the bottom makes it read better which is
> probably more important.
Havin become proficient at adding hex addrs in my head, I have also
come to appreciate structs which have all the important/hot stuff
at the top, instead of large filler holes that one has to skip over
during debugging.
--linas
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2007-05-23 20:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-05-21 23:18 [PATCH] pseries: asm/pci-bridge.h CONFIG_ minor cleanup Linas Vepstas
2007-05-22 0:40 ` Michael Ellerman
2007-05-22 17:00 ` Linas Vepstas
2007-05-22 18:18 ` [PATCH] (revised) " Linas Vepstas
2007-05-23 1:11 ` [PATCH] " Michael Ellerman
2007-05-23 20:51 ` Linas Vepstas
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).