lkml.org 
[lkml]   [2008]   [Aug]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [patch 8/8] x86: apic - unify init_bsp_APIC
On Fri, 15 Aug 2008, Ingo Molnar wrote:

> > >> @@ -962,7 +962,8 @@ void __init init_bsp_APIC(void)
> > >> */
> > >> apic_write(APIC_LVT0, APIC_DM_EXTINT);
> > >> value = APIC_DM_NMI;
> > >> - if (!lapic_is_integrated()) /* 82489DX */
> > >> + /* discrete on 82489DX */
> > >> + if (!lapic_is_integrated())
> > >> value |= APIC_LVT_LEVEL_TRIGGER;
> > >> apic_write(APIC_LVT1, value);
> > >> }
> > >
> > > Please elaborate.
> > >
> > > Maciej
> > >
> >
> > Hi Maciej,
> >
> > don't really understand what do you mean. [...]
>
> i suspect the question might have been: 'why this change'.

Indeed -- the change just moves a comment around from the obvious place
(where it means: "the condition true applies to the 82489DX") to elsewhere
while rephrasing it in a way that makes me wonder: "What the hell is that
meant to mean?" Perhaps it is clearer to the others, but for me it is
just obfuscation.

I think the comment as it is is clear enough and is also clearly visible
when browsing through the source, when you want to spot all the odd bits
related to the discrete APIC. This is no longer true after the change.
And any more complete explanation belongs to the definition of
lapic_is_integrated().

> If that was the question, the answer would be: to unify apic_32.c and
> apic_64.c we first use tiny little changes to bring the two files in
> sync. Presumably, each such change is a NOP or at least very safe - and
> clearly bisectable in the worst-case.

If the obfuscation comes from apic_64.c, then the clean-up should be done
in th other direction IMO. Not everything that comes from the 64-bit
variation is better than its 32-bit counterpart.

Please note I do not question the semantic changes contained within this
patch, so the issue of bisectability or code bloat (I am assuming
lapic_is_integrated() expands to 0 on 64-bit; if not, this is clearly
asking for improvement) is irrelevant.

Maciej


\
 
 \ /
  Last update: 2008-08-15 14:19    [W:0.051 / U:0.240 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site