Re: [PATCH 4/4] x86/mce/AMD: Add comments for easier understanding

[Aravind Gopalakrishnan]

On 2/26/2016 11:44 AM, Borislav Petkov wrote:
> threshold_restart_bank() reprograms the MISC MSR after sanity-checking
> the fields supplied for that MSR. store_threshold_limit() sets the error
> count, store_interrupt_enable() enables/disables the interrupt and both
> call threshold_restart_bank() to do that.
>
> But this is basically spelling the code now - I don't think we need to
> comment in that detail.

Ok, Have dropped this for V2.

> /*
>   * Called via smp_call_function_single(), must be called with correct
>   * cpu affinity.
>   */
>
> is also useless.

Will remove these as well.

> > "This function provides user with capabilities to re-program the
> > 'thresold_limit' and 'interrupt_enable' sysfs attributes"
> No sorry, I don't want to be explaining every line. Just say: "Reprogram
> the MISC MSR behind this threshold bank."

Ok, Will do that.

Btw, included comments around struct threshold_block to describethe members.
Do let me know if this seems OK-

 struct threshold_block {
-       unsigned int            block;
-       unsigned int            bank;
-       unsigned int            cpu;
-       u32                     address;
-       u16                     interrupt_enable;
-       bool                    interrupt_capable;
-       u16                     threshold_limit;
-       struct kobject          kobj;
-       struct list_head        miscj;
+       unsigned int            block;                  /* Threshold 
block number within bank */
+       unsigned int            bank;                   /* MCA bank the 
block belongs to */
+       unsigned int            cpu;                    /* CPU which 
controls the MCA bank */
+       u32                     address;                /* MSR address 
for the block */
+       u16                     interrupt_enable;       /* Enable/ 
Disable APIC interrupt upon threshold error */
+       bool                    interrupt_capable;      /* Specifies if 
interrupt is possible from the block */
+       u16                     threshold_limit;        /* Value upon 
which threshold interrupt is generated */
+       struct kobject          kobj;                   /* sysfs object */
+       struct list_head        miscj;                  /* Add multiple 
threshold blocks within a bank to the list */
 };

Thanks,
-Aravind.