* [PATCH] x86, apicdef: Fix checkpatch issues in apicdef.h
@ 2009-10-31 9:29 Rakib Mullick
2009-10-31 9:44 ` Cyrill Gorcunov
0 siblings, 1 reply; 4+ messages in thread
From: Rakib Mullick @ 2009-10-31 9:29 UTC (permalink / raw)
To: Ingo Molnar, LKML; +Cc: Cyrill Gorcunov, Andrew Morton
In apicdef.h - we have a checkpatch issue - needs to be fixed.
We were warned by the following checkpatch warning:
ERROR: space prohibited before that ':' (ctx:WxW)
#276: FILE: arch/x86/include/asm/apicdef.h:376:
+ hi : 1;
^
---
Signed-off-by: Rakib Mullick <rakib.mullick@gmail.com>
--- linus/arch/x86/include/asm/apicdef.h 2009-10-17 13:12:59.000000000 +0600
+++ rakib/arch/x86/include/asm/apicdef.h 2009-10-31 16:27:49.000000000 +0600
@@ -170,18 +170,18 @@ struct local_apic {
/*010*/ struct { u32 __reserved[4]; } __reserved_02;
/*020*/ struct { /* APIC ID Register */
- u32 __reserved_1 : 24,
- phys_apic_id : 4,
- __reserved_2 : 4;
+ u32 __reserved_1:24,
+ phys_apic_id:4,
+ __reserved_2:4;
u32 __reserved[3];
} id;
/*030*/ const
struct { /* APIC Version Register */
- u32 version : 8,
- __reserved_1 : 8,
- max_lvt : 8,
- __reserved_2 : 8;
+ u32 version:8,
+ __reserved_1:8,
+ max_lvt:8,
+ __reserved_2:8;
u32 __reserved[3];
} version;
@@ -194,22 +194,22 @@ struct local_apic {
/*070*/ struct { u32 __reserved[4]; } __reserved_06;
/*080*/ struct { /* Task Priority Register */
- u32 priority : 8,
- __reserved_1 : 24;
+ u32 priority:8,
+ __reserved_1:24;
u32 __reserved_2[3];
} tpr;
/*090*/ const
struct { /* Arbitration Priority Register */
- u32 priority : 8,
- __reserved_1 : 24;
+ u32 priority:8,
+ __reserved_1:24;
u32 __reserved_2[3];
} apr;
/*0A0*/ const
struct { /* Processor Priority Register */
- u32 priority : 8,
- __reserved_1 : 24;
+ u32 priority:8,
+ __reserved_1:24;
u32 __reserved_2[3];
} ppr;
@@ -221,51 +221,55 @@ struct local_apic {
/*0C0*/ struct { u32 __reserved[4]; } __reserved_07;
/*0D0*/ struct { /* Logical Destination Register */
- u32 __reserved_1 : 24,
- logical_dest : 8;
+ u32 __reserved_1:24,
+ logical_dest:8;
u32 __reserved_2[3];
} ldr;
/*0E0*/ struct { /* Destination Format Register */
- u32 __reserved_1 : 28,
- model : 4;
+ const u32 __reserved_1:28;
+ u32 model:4;
u32 __reserved_2[3];
} dfr;
/*0F0*/ struct { /* Spurious Interrupt Vector Register */
- u32 spurious_vector : 8,
- apic_enabled : 1,
- focus_cpu : 1,
- __reserved_2 : 22;
+ u32 spurious_vector:8;
+ const u32 apic_enabled:1,
+ focus_cpu:1,
+ __reserved_2:22;
u32 __reserved_3[3];
} svr;
-/*100*/ struct { /* In Service Register */
-/*170*/ u32 bitfield;
+/*100*/ const
+/*170*/ struct { /* In Service Register */
+ u32 bitfield;
u32 __reserved[3];
} isr [8];
-/*180*/ struct { /* Trigger Mode Register */
-/*1F0*/ u32 bitfield;
+/*180*/ const
+/*1F0*/ struct { /* Trigger Mode Register */
+ u32 bitfield;
u32 __reserved[3];
} tmr [8];
-/*200*/ struct { /* Interrupt Request Register */
-/*270*/ u32 bitfield;
+/*200*/ const
+/*270*/ struct { /* Interrupt Request Register */
+ u32 bitfield;
u32 __reserved[3];
} irr [8];
-/*280*/ union { /* Error Status Register */
+/*280*/ const
+ union { /* Error Status Register */
struct {
- u32 send_cs_error : 1,
- receive_cs_error : 1,
- send_accept_error : 1,
- receive_accept_error : 1,
- __reserved_1 : 1,
- send_illegal_vector : 1,
- receive_illegal_vector : 1,
- illegal_register_address : 1,
- __reserved_2 : 24;
+ u32 send_cs_error:1,
+ receive_cs_error:1,
+ send_accept_error:1,
+ receive_accept_error:1,
+ __reserved_1:1,
+ send_illegal_vector:1,
+ receive_illegal_vector:1,
+ illegal_register_address:1,
+ __reserved_2:24;
u32 __reserved_3[3];
} error_bits;
struct {
@@ -289,96 +293,96 @@ struct local_apic {
/*2F0*/ struct { u32 __reserved[4]; } __reserved_14;
/*300*/ struct { /* Interrupt Command Register 1 */
- u32 vector : 8,
- delivery_mode : 3,
- destination_mode : 1,
- delivery_status : 1,
- __reserved_1 : 1,
- level : 1,
- trigger : 1,
- __reserved_2 : 2,
- shorthand : 2,
- __reserved_3 : 12;
+ u32 vector:8,
+ delivery_mode:3,
+ destination_mode:1,
+ delivery_status:1,
+ __reserved_1:1,
+ level:1,
+ trigger:1,
+ __reserved_2:2,
+ shorthand:2,
+ __reserved_3:12;
u32 __reserved_4[3];
} icr1;
/*310*/ struct { /* Interrupt Command Register 2 */
union {
- u32 __reserved_1 : 24,
- phys_dest : 4,
- __reserved_2 : 4;
- u32 __reserved_3 : 24,
- logical_dest : 8;
+ u32 __reserved_1:24,
+ phys_dest:4,
+ __reserved_2:4;
+ u32 __reserved_3:24,
+ logical_dest:8;
} dest;
u32 __reserved_4[3];
} icr2;
/*320*/ struct { /* LVT - Timer */
- u32 vector : 8,
- __reserved_1 : 4,
- delivery_status : 1,
- __reserved_2 : 3,
- mask : 1,
- timer_mode : 1,
- __reserved_3 : 14;
+ u32 vector:8,
+ __reserved_1:4,
+ delivery_status:1,
+ __reserved_2:3,
+ mask:1,
+ timer_mode:1,
+ __reserved_3:14;
u32 __reserved_4[3];
} lvt_timer;
/*330*/ struct { /* LVT - Thermal Sensor */
- u32 vector : 8,
- delivery_mode : 3,
- __reserved_1 : 1,
- delivery_status : 1,
- __reserved_2 : 3,
- mask : 1,
- __reserved_3 : 15;
+ u32 vector:8,
+ delivery_mode:3,
+ __reserved_1:1,
+ delivery_status:1,
+ __reserved_2:3,
+ mask:1,
+ __reserved_3:15;
u32 __reserved_4[3];
} lvt_thermal;
/*340*/ struct { /* LVT - Performance Counter */
- u32 vector : 8,
- delivery_mode : 3,
- __reserved_1 : 1,
- delivery_status : 1,
- __reserved_2 : 3,
- mask : 1,
- __reserved_3 : 15;
+ u32 vector:8,
+ delivery_mode:3,
+ __reserved_1:1,
+ delivery_status:1,
+ __reserved_2:3,
+ mask:1,
+ __reserved_3:15;
u32 __reserved_4[3];
} lvt_pc;
/*350*/ struct { /* LVT - LINT0 */
- u32 vector : 8,
- delivery_mode : 3,
- __reserved_1 : 1,
- delivery_status : 1,
- polarity : 1,
- remote_irr : 1,
- trigger : 1,
- mask : 1,
- __reserved_2 : 15;
+ u32 vector:8,
+ delivery_mode:3,
+ __reserved_1:1,
+ delivery_status:1,
+ polarity:1,
+ remote_irr:1,
+ trigger:1,
+ mask:1,
+ __reserved_2:15;
u32 __reserved_3[3];
} lvt_lint0;
/*360*/ struct { /* LVT - LINT1 */
- u32 vector : 8,
- delivery_mode : 3,
- __reserved_1 : 1,
- delivery_status : 1,
- polarity : 1,
- remote_irr : 1,
- trigger : 1,
- mask : 1,
- __reserved_2 : 15;
+ u32 vector:8,
+ delivery_mode:3,
+ __reserved_1:1,
+ delivery_status:1,
+ polarity:1,
+ remote_irr:1,
+ trigger:1,
+ mask:1,
+ __reserved_2:15;
u32 __reserved_3[3];
} lvt_lint1;
/*370*/ struct { /* LVT - Error */
- u32 vector : 8,
- __reserved_1 : 4,
- delivery_status : 1,
- __reserved_2 : 3,
- mask : 1,
- __reserved_3 : 15;
+ u32 vector:8,
+ __reserved_1:4,
+ delivery_status:1,
+ __reserved_2:3,
+ mask:1,
+ __reserved_3:15;
u32 __reserved_4[3];
} lvt_error;
@@ -402,8 +406,8 @@ struct local_apic {
/*3D0*/ struct { u32 __reserved[4]; } __reserved_19;
/*3E0*/ struct { /* Timer Divide Configuration Register */
- u32 divisor : 4,
- __reserved_1 : 28;
+ u32 divisor:4,
+ __reserved_1:28;
u32 __reserved_2[3];
} timer_dcr;
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] x86, apicdef: Fix checkpatch issues in apicdef.h
2009-10-31 9:29 [PATCH] x86, apicdef: Fix checkpatch issues in apicdef.h Rakib Mullick
@ 2009-10-31 9:44 ` Cyrill Gorcunov
2009-10-31 9:59 ` Cyrill Gorcunov
0 siblings, 1 reply; 4+ messages in thread
From: Cyrill Gorcunov @ 2009-10-31 9:44 UTC (permalink / raw)
To: Rakib Mullick; +Cc: Ingo Molnar, LKML, Andrew Morton
[Rakib Mullick - Sat, Oct 31, 2009 at 03:29:32PM +0600]
|
| In apicdef.h - we have a checkpatch issue - needs to be fixed.
| We were warned by the following checkpatch warning:
|
| ERROR: space prohibited before that ':' (ctx:WxW)
| #276: FILE: arch/x86/include/asm/apicdef.h:376:
| + hi : 1;
| ^
| ---
| Signed-off-by: Rakib Mullick <rakib.mullick@gmail.com>
|
Hi Rakib,
personally I would not change it even having checkpatch
issue. Really, it's easier to read this bitfield width
aligned rather then moved left. But this is a personal
opinion only (sorry).
-- Cyrill
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] x86, apicdef: Fix checkpatch issues in apicdef.h
2009-10-31 9:44 ` Cyrill Gorcunov
@ 2009-10-31 9:59 ` Cyrill Gorcunov
2009-11-03 23:34 ` Maciej W. Rozycki
0 siblings, 1 reply; 4+ messages in thread
From: Cyrill Gorcunov @ 2009-10-31 9:59 UTC (permalink / raw)
To: Rakib Mullick, Ingo Molnar, LKML, Andrew Morton,
Maciej W. Rozycki, Yinghai Lu
[Cyrill Gorcunov - Sat, Oct 31, 2009 at 12:44:31PM +0300]
| [Rakib Mullick - Sat, Oct 31, 2009 at 03:29:32PM +0600]
| |
| | In apicdef.h - we have a checkpatch issue - needs to be fixed.
| | We were warned by the following checkpatch warning:
| |
| | ERROR: space prohibited before that ':' (ctx:WxW)
| | #276: FILE: arch/x86/include/asm/apicdef.h:376:
| | + hi : 1;
| | ^
| | ---
| | Signed-off-by: Rakib Mullick <rakib.mullick@gmail.com>
| |
|
| Hi Rakib,
|
| personally I would not change it even having checkpatch
| issue. Really, it's easier to read this bitfield width
| aligned rather then moved left. But this is a personal
| opinion only (sorry).
|
| -- Cyrill
On the other hands -- I wonder if we need this structure
at all. Perhaps there was an idea to use it with suspend/resume
actions? Ingo, Maciej, Yinghai?
-- Cyrill
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] x86, apicdef: Fix checkpatch issues in apicdef.h
2009-10-31 9:59 ` Cyrill Gorcunov
@ 2009-11-03 23:34 ` Maciej W. Rozycki
0 siblings, 0 replies; 4+ messages in thread
From: Maciej W. Rozycki @ 2009-11-03 23:34 UTC (permalink / raw)
To: Cyrill Gorcunov
Cc: Rakib Mullick, Ingo Molnar, LKML, Andrew Morton, Yinghai Lu
On Sat, 31 Oct 2009, Cyrill Gorcunov wrote:
> [Cyrill Gorcunov - Sat, Oct 31, 2009 at 12:44:31PM +0300]
> | [Rakib Mullick - Sat, Oct 31, 2009 at 03:29:32PM +0600]
> | |
> | | In apicdef.h - we have a checkpatch issue - needs to be fixed.
> | | We were warned by the following checkpatch warning:
> | |
> | | ERROR: space prohibited before that ':' (ctx:WxW)
> | | #276: FILE: arch/x86/include/asm/apicdef.h:376:
> | | + hi : 1;
> | | ^
> | | ---
> | | Signed-off-by: Rakib Mullick <rakib.mullick@gmail.com>
> | |
> |
> | personally I would not change it even having checkpatch
> | issue. Really, it's easier to read this bitfield width
> | aligned rather then moved left. But this is a personal
> | opinion only (sorry).
I agree -- don't fix what ain't broke!
> On the other hands -- I wonder if we need this structure
> at all. Perhaps there was an idea to use it with suspend/resume
> actions? Ingo, Maciej, Yinghai?
The existence of this structure is purely documentary as it's considered
unsafe to map device registers using bitfields -- first: the C language
standard does not guarantee the layout of bitfield struct members, second:
the compiler may choose to use machine instructions such as BT that are
unsafe for MMIO, so all accesses should be made via the appropriate
accessors.
Then even as such, the contents are out of date and as such may be
misleading, especially to the newcomers, so I think the choice is either
to fix the declarations or to dump them altogether and given I couldn't be
bothered to do the former, my vote is for the latter.
Maciej
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-11-03 23:34 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-10-31 9:29 [PATCH] x86, apicdef: Fix checkpatch issues in apicdef.h Rakib Mullick
2009-10-31 9:44 ` Cyrill Gorcunov
2009-10-31 9:59 ` Cyrill Gorcunov
2009-11-03 23:34 ` Maciej W. Rozycki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox