Re: [PATCH] [19/48] Suspend2 2.1.9.8 for 2.6.12: 510-version-specific-mac.patch

Nigel Cunningham
2005-07-06T13:57:55Z

Requested changes applied.

Thanks!

Nigel

On Wed, 2005-07-06 at 15:58, Pekka Enberg wrote:
> On 7/6/05, Nigel Cunningham <nigel@suspend2.net> wrote:
> > diff -ruNp 520-version-specific-x86_64.patch-old/include/asm-x86_64/suspend2.h 520-version-specific-x86_64.patch-new/include/asm-x86_64/suspend2.h
> > --- 520-version-specific-x86_64.patch-old/include/asm-x86_64/suspend2.h 1970-01-01 10:00:00.000000000 +1000
> > +++ 520-version-specific-x86_64.patch-new/include/asm-x86_64/suspend2.h 2005-07-05 23:56:15.000000000 +1000
> > @@ -0,0 +1,432 @@
> > +#undef inline
> > +#define inline __inline__ __attribute__((always_inline))
>
> Please drop this macro. <linux/compiler.h> takes care of it already.
>
> > +
> > +/* image of the saved processor states */
> > +struct suspend2_saved_context {
> > + unsigned long eax, ebx, ecx, edx;
> > + unsigned long esp, ebp, esi, edi;
> > + unsigned long r8, r9, r10, r11;
> > + unsigned long r12, r13, r14, r15;
> > +
> > +#if 0
> > + u16 es, fs, gs, ss;
> > + u32 cr0, cr2, cr3, cr4;
> > + u16 gdt_pad;
> > + u16 gdt_limit;
> > + u32 gdt_base;
> > + u16 idt_pad;
> > + u16 idt_limit;
> > + u32 idt_base;
> > + u16 ldt;
> > + u16 tss;
> > + u32 tr;
> > + u32 safety;
> > + u32 return_address;
> > +#endif
>
> Please drop the #ifdef
>
> > + unsigned long eflags;
> > +} __attribute__((packed));
> > +
> > +extern struct suspend2_saved_context suspend2_saved_context; /* temporary storage */
>
> Please move the comment above the declaration (looks as if you're
> breaking 80 columns).
>
> > +
> > +#ifdef CONFIG_MTRR
> > +/* MTRR functions */
> > +extern int mtrr_save(void);
> > +extern int mtrr_restore_one_cpu(void);
> > +extern void mtrr_restore_finish(void);
> > +#else
> > +#define mtrr_save() do { } while(0)
> > +#define mtrr_restore_one_cpu() do { } while(0)
> > +#define mtrr_restore_finish() do { } while(0)
>
> Empty static inline functions are preferred.
>
> > +#endif
> > +
> > +#ifndef CONFIG_SMP
> > +#undef cpu_clear
> > +#define cpu_clear(a, b) do { } while(0)
>
> Same here.
>
> > +#endif
> > +
> > +extern struct suspend2_saved_context suspend2_saved_context; /* temporary storage */
>
> Move comment up.
>
> > +static void fix_processor_context(void)
> > +{
> > + int nr = _smp_processor_id();
> > + struct tss_struct * t = &per_cpu(init_tss,nr);
> > +
> > + set_tss_desc(nr,t); /* This just modifies memory; should not be neccessary. But... This is neccessary, because 386 hardware has concept of busy tsc or some similar stupidity. */
>
> Please move comment before function call and indent it properly.
>
> > +/*
> > + * END of IRQ affinity code, based on LKCD code.
> > + * -----------------------------------------------------------------
> > + */
> > +#else
> > +#define save_and_set_irq_affinity() do { } while(0)
> > +#define reset_irq_affinity() do { } while(0)
>
> Empty static inlines please.
>
> > diff -ruNp 520-version-specific-x86_64.patch-old/include/asm-x86_64/suspend.h 520-version-specific-x86_64.patch-new/include/asm-x86_64/suspend.h
> > --- 520-version-specific-x86_64.patch-old/include/asm-x86_64/suspend.h 2005-06-20 11:47:28.000000000 +1000
> > +++ 520-version-specific-x86_64.patch-new/include/asm-x86_64/suspend.h 2005-07-04 23:14:19.000000000 +1000
> > @@ -43,7 +43,7 @@ extern unsigned long saved_context_eflag
> > : /* no output */ \
> > :"r" ((thread)->debugreg##register))
> >
> > -extern void fix_processor_context(void);
> > +/* extern void fix_processor_context(void); */
>
> Please drop commented out code.
>
> >
> > #ifdef CONFIG_ACPI_SLEEP
> > extern unsigned long saved_eip;
> >
> > -
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
> >
>
--
Evolution.
Enumerate the requirements.
Consider the interdependencies.
Calculate the probabilities.
Be amazed that people believe it happened.