Re: [ndctl PATCH RESEND 1/2] cxl: Update a revision by CXL 3.0 specification

[Jehoon Park <jehoon.park@samsung.com>]

[-- Attachment #1: Type: text/plain, Size: 1963 bytes --]

On Mon, Jul 17, 2023 at 08:18:55AM -0500, Nathan Fontenot wrote:
> On 7/17/23 01:29, Jehoon Park wrote:
> > Update the value of device temperature field when it is not implemented.
> > (CXL 3.0 8.2.9.8.3.1)
> > 
> > Signed-off-by: Jehoon Park <jehoon.park@samsung.com>
> > ---
> >  cxl/json.c        | 2 +-
> >  cxl/lib/private.h | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/cxl/json.c b/cxl/json.c
> > index 9a4b5c7..3661eb9 100644
> > --- a/cxl/json.c
> > +++ b/cxl/json.c
> > @@ -155,7 +155,7 @@ static struct json_object *util_cxl_memdev_health_to_json(
> >  	}
> >  
> >  	field = cxl_cmd_health_info_get_temperature(cmd);
> > -	if (field != 0xffff) {
> > +	if (field != 0x7fff) {
> 
> Should you also update this field check to use CXL_CMD_HEALTH_INFO_TEMPERATURE_NOT_IMPL
> instead of using 0x7fff directly?
> 
> -Nathan
>

Hi, Nathan

I agree with your suggestion since it is more understandable. However, the
constant macro is defined in "cxl/lib/private.h" which should be included only
under "cxl/lib/" (as I understand properly). To use the macro in json.c,
we have to define it somewhere under "cxl/" e.g. libcxl.h, json.h, ...

I'm not sure about this approach is right, so I followed existing
implementation that used NOT_IMPL value directly.

Jehoon

> >  		jobj = json_object_new_int(field);
> >  		if (jobj)
> >  			json_object_object_add(jhealth, "temperature", jobj);
> > diff --git a/cxl/lib/private.h b/cxl/lib/private.h
> > index d49b560..e92592d 100644
> > --- a/cxl/lib/private.h
> > +++ b/cxl/lib/private.h
> > @@ -324,7 +324,7 @@ struct cxl_cmd_set_partition {
> >  #define CXL_CMD_HEALTH_INFO_EXT_CORRECTED_PERSISTENT_WARNING		(1)
> >  
> >  #define CXL_CMD_HEALTH_INFO_LIFE_USED_NOT_IMPL				0xff
> > -#define CXL_CMD_HEALTH_INFO_TEMPERATURE_NOT_IMPL			0xffff
> > +#define CXL_CMD_HEALTH_INFO_TEMPERATURE_NOT_IMPL			0x7fff
> >  
> >  static inline int check_kmod(struct kmod_ctx *kmod_ctx)
> >  {

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]