Re: [ndctl PATCH V2 1/2] libcxl: add accessors for Get Alert Configuration CCI output

[Alison Schofield <alison.schofield@intel.com>]

On Mon, Oct 31, 2022 at 11:46:56PM +0000, Jonathan Zhang (Infra) wrote:
> 
> 
> > On Oct 31, 2022, at 11:57 AM, Alison Schofield <alison.schofield@intel.com> wrote:
> > 
> > > 
> > On Tue, Oct 25, 2022 at 01:43:19PM -0700, Jonathan Zhang wrote:
> >> From: Jonathan Zhang <jonzhang@fb.com>
> >> 
> >> CXL 3.0 spec section 8.2.9.8.3.2 "Get Alert Configuration
> >> (Opcode 4201h) defines the get-alert-config command to
> >> retrieve the devices's critical alert and programmable
> >> warning configurations.
> >> 
> >> Add the methods to issue the command and get the fields
> >> defined.
> >> 
> >> Signed-off-by: Jonathan Zhang <jonzhang@meta.com>
> > 
> > Consistency in typos are trickier to detect ;)
> > 
> >> +CXL_EXPORT int
> >> +cxl_cmd_alert_config_life_used_prog_warn_threshold_writable(struct cxl_cmd *cmd)
> >> +{
> >> +	cmd_alert_get_prog_alerts_field(
> >> +		cmd,
> >> +		CXL_CMD_ALERT_CONFIG_PROG_ALERTS_LIEF_USED_PROG_WARN_THRESHOLD_MASK);
                                                 ^^ LIFE ?

> >> +}
> >> +
> > 
> > --snip--
> > 
> >> +#define CXL_CMD_ALERT_CONFIG_PROG_ALERTS_LIEF_USED_PROG_WARN_THRESHOLD_MASK    \
> >> +	BIT(0)

                                               ^^ LIFE ?

> > 
> > --snip--
> >> 
> Thanks for the review. For the above snippets, I do not see any consistency issue though.

I'm sorry for being cryptic in my review comment.
Alison

> To save space, I have been using following abbreviations: 
> * prog, short for programmable
> * err, short for error
> * warn, short for warning
> * crit, short for critical
> * dev, short for device
> * config, short for configuration