lkml.org 
[lkml]   [2005]   [Jun]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH]MTRR suspend/resume cleanup
    From
    Date
    On Tue, 2005-06-28 at 13:24 +0800, Andrew Morton wrote:
    > > @@ -332,6 +332,7 @@ int mtrr_add_page(unsigned long base, un
    > >
    > > error = -EINVAL;
    > >
    > > + lock_cpu_hotplug();
    > > /* Search for existing MTRR */
    > > down(&main_lock);
    > > for (i = 0; i < num_var_ranges; ++i) {
    >
    > Is this new locking? If so, please describe it.
    > > +
    > > +static int __init mtrr_init_finialize(void)
    > > +{
    > > + if (!mtrr_if)
    > > + return 0;
    > > + if (use_intel())
    > > + mtrr_state_warn();
    > > + else
    > > + /* The CPUs haven't MTRR and seemes not support SMP. They have
    > specific
    > > + * drivers, we use a tricky method to support suspend/resume
    > for them
    > > + * TBD: is there any system with such CPU which supports
    > suspend/resume?
    > > + * if no, we should remove the code.
    > > + */
    > > + sysdev_driver_register(&cpu_sysdev_class,
    > > + &mtrr_sysdev_driver);
    > > + return 0;
    > > +}
    >
    > That's pretty hard to read. Braces will improve it:
    >
    > if (use_intel()) {
    > mtrr_state_warn();
    > } else {
    > /*
    > * The CPUs haven't MTRR and seemes not support SMP.
    > They have
    > * specific drivers, we use a tricky method to support
    > * suspend/resume for them.
    > * TBD: is there any system with such CPU which
    > supports
    > * suspend/resume? If no, we should remove the code.
    > */
    > sysdev_driver_register(&cpu_sysdev_class,
    > &mtrr_sysdev_driver);
    > }
    >
    > > +#ifdef CONFIG_MTRR
    > > +extern int mtrr_ap_init(void);
    > > +extern int mtrr_bp_init(void);
    > > +#else
    > > +#define mtrr_ap_init()
    > > +#define mtrr_bp_init()
    > > +#endif
    >
    > If this works, it means we're always ignoring the return values and
    > these
    > things should return void.
    >
    > Otherwise the !CONFIG_MTRR stubs should be returning an integer. So
    > we
    > need either:
    >
    > #ifdef CONFIG_MTRR
    > extern void mtrr_ap_init(void);
    > extern void mtrr_bp_init(void);
    > #else
    > #define mtrr_ap_init() do {} while (0) (or static inlines)
    > #define mtrr_bp_init() do {} while (0)
    > #endif
    >
    > or
    >
    > #ifdef CONFIG_MTRR
    > extern int mtrr_ap_init(void);
    > extern int mtrr_bp_init(void);
    > #else
    > static inline int mtrr_ap_init(void) { return 0; }
    > static inline int mtrr_bp_init(void) { return 0; }
    > #endif
    > > +#ifdef CONFIG_MTRR
    > > +extern int mtrr_ap_init(void);
    > > +extern int mtrr_bp_init(void);
    > > +#else
    > > +#define mtrr_ap_init()
    > > +#define mtrr_bp_init();
    > > +#endif
    > Ditto.
    Thanks for your feedback. I will update the patch when I collect all
    comments.

    Thanks,
    Shaohua

    -
    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/

    \
     
     \ /
      Last update: 2005-06-28 09:58    [from the cache]
    ©2003-2014 Jasper Spaans. hosted at Digital Ocean