> -----Original Message----- > From: Borislav Petkov [mailto:b...@suse.de] > Sent: Monday, March 27, 2017 1:27 PM > To: Ghannam, Yazen <yazen.ghan...@amd.com>
... > > static void > > -__log_error(unsigned int bank, bool deferred_err, bool threshold_err, > > u64 misc) > > +__log_error(unsigned int bank, bool deferred_err, bool use_smca_destat, > > + bool threshold_err, u64 misc) > > So I had to paste the function signature in a separate vim window and keep > looking between those arguments' names and the function calls. > > Because if I look at: > > __log_error(bank, true, false, false, 0); > > I absolutely have no clue what that code does. So we need to think of > something better. From the looks of it, I guess we dealt with a single > __log_error() function long enough. Perhaps it is time for separation: > > log_error_deferred > log_error_smca > log_error... > > and have a function __log_error() which all three call to do the work which > all > three share. > > That should make the code readable again IMO. __log_error() as it is now is > hard to follow anyway. > Okay, will do. > > @@ -832,25 +823,29 @@ asmlinkage __visible void __irq_entry > smp_trace_deferred_error_interrupt(void) > > exiting_ack_irq(); > > } > > > > +static inline bool check_deferred_status(u64 status) > > This function name does not tell me anything. > > if (is_deferred_error(status)) > > tells me more. > Okay. Thanks, Yazen
