On Wed, May 27, 2020 at 03:34:36PM +0530, Anshuman Khandual wrote: > +/* > + * get_arm64_ftr_reg - Looks up a feature register entry using > + * its sys_reg() encoding. This calls get_arm64_ftr_reg_nowarn(). > + * > + * returns - Upon success, matching ftr_reg entry for id. > + * - NULL on failure but with an WARN_ON(). > + */ > +static struct arm64_ftr_reg *get_arm64_ftr_reg(u32 sys_id) > +{ > + struct arm64_ftr_reg *reg; > + > + reg = get_arm64_ftr_reg_nowarn(sys_id); > + > + /* > + * Can not really proceed when the search fails here. > + * Requesting for a non existent register search will > + * be an error. Warn but let it continue for now. > + */ > + WARN_ON(!reg); > + return reg;
I find the comment here slightly confusing: cannot proceed but continue. Maybe something like: /* * Requesting a non-existent register search is an error. Warn * and let the caller handle it. */ Otherwise it looks fine: Reviewed-by: Catalin Marinas <catalin.mari...@arm.com>