On Mon, Aug 07, 2023 at 02:14:35PM +0100, Jonathan Cameron wrote: > On Mon, 7 Aug 2023 15:35:48 +0900 > Jehoon Park <[email protected]> wrote: > > > Add a new macro function to retrieve a signed value such as a temperature. > > Modify accessors for signed value to return INT_MAX when error occurs and > > set errno to corresponding errno codes. > > None of the callers have been modified to deal with INTMAX until next patch. > So I think you need to combine the two to avoid temporary breakage. > > Also you seem to be also changing the health status. That seems > to be unrelated to the negative temperature support so shouldn't > really be in same patch. >
Thank you for comments, I will re-organize these patches in the next revision. > > > > Signed-off-by: Jehoon Park <[email protected]> > > --- > > cxl/lib/libcxl.c | 32 +++++++++++++++++++++----------- > > 1 file changed, 21 insertions(+), 11 deletions(-) > > > > diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c > > index af4ca44..fc64de1 100644 > > --- a/cxl/lib/libcxl.c > > +++ b/cxl/lib/libcxl.c > > @@ -3661,11 +3661,23 @@ > > cxl_cmd_alert_config_get_life_used_prog_warn_threshold(struct cxl_cmd *cmd) > > life_used_prog_warn_threshold); > > } > > > > +#define cmd_get_field_s16(cmd, n, N, field) > > \ > > +do { > > \ > > + struct cxl_cmd_##n *c = \ > > + (struct cxl_cmd_##n *)cmd->send_cmd->out.payload; \ > > + int rc = cxl_cmd_validate_status(cmd, CXL_MEM_COMMAND_ID_##N); \ > > + if (rc) { \ > > + errno = -rc; \ > > + return INT_MAX; \ > > + } \ > > + return (int16_t)le16_to_cpu(c->field); \ > > +} while(0) > > + > > CXL_EXPORT int > > cxl_cmd_alert_config_get_dev_over_temperature_crit_alert_threshold( > > struct cxl_cmd *cmd) > > { > > - cmd_get_field_u16(cmd, get_alert_config, GET_ALERT_CONFIG, > > + cmd_get_field_s16(cmd, get_alert_config, GET_ALERT_CONFIG, > > dev_over_temperature_crit_alert_threshold); > > } > > > > @@ -3673,7 +3685,7 @@ CXL_EXPORT int > > cxl_cmd_alert_config_get_dev_under_temperature_crit_alert_threshold( > > struct cxl_cmd *cmd) > > { > > - cmd_get_field_u16(cmd, get_alert_config, GET_ALERT_CONFIG, > > + cmd_get_field_s16(cmd, get_alert_config, GET_ALERT_CONFIG, > > dev_under_temperature_crit_alert_threshold); > > } > > > > @@ -3681,7 +3693,7 @@ CXL_EXPORT int > > cxl_cmd_alert_config_get_dev_over_temperature_prog_warn_threshold( > > struct cxl_cmd *cmd) > > { > > - cmd_get_field_u16(cmd, get_alert_config, GET_ALERT_CONFIG, > > + cmd_get_field_s16(cmd, get_alert_config, GET_ALERT_CONFIG, > > dev_over_temperature_prog_warn_threshold); > > } > > > > @@ -3689,7 +3701,7 @@ CXL_EXPORT int > > cxl_cmd_alert_config_get_dev_under_temperature_prog_warn_threshold( > > struct cxl_cmd *cmd) > > { > > - cmd_get_field_u16(cmd, get_alert_config, GET_ALERT_CONFIG, > > + cmd_get_field_s16(cmd, get_alert_config, GET_ALERT_CONFIG, > > dev_under_temperature_prog_warn_threshold); > > } > > > > @@ -3905,8 +3917,6 @@ CXL_EXPORT int > > cxl_cmd_health_info_get_life_used(struct cxl_cmd *cmd) > > { > > int rc = health_info_get_life_used_raw(cmd); > > > > - if (rc < 0) > > - return rc; > > Why has this one changed? It's a u8 so not as far as I can see affected by > your new signed accessor. > > I removed it because it was unnecessary code. (No action after error checking) However, as you stated, this code cleaning is irrelevant to this patch. I will revert this in the next patch. > > if (rc == CXL_CMD_HEALTH_INFO_LIFE_USED_NOT_IMPL) > > return -EOPNOTSUPP; > > return rc; > > @@ -3914,7 +3924,7 @@ CXL_EXPORT int > > cxl_cmd_health_info_get_life_used(struct cxl_cmd *cmd) > > > > static int health_info_get_temperature_raw(struct cxl_cmd *cmd) > > { > > - cmd_get_field_u16(cmd, get_health_info, GET_HEALTH_INFO, > > + cmd_get_field_s16(cmd, get_health_info, GET_HEALTH_INFO, > > temperature); > > } > > > > @@ -3922,10 +3932,10 @@ CXL_EXPORT int > > cxl_cmd_health_info_get_temperature(struct cxl_cmd *cmd) > > { > > int rc = health_info_get_temperature_raw(cmd); > > > > - if (rc < 0) > > - return rc; > > - if (rc == CXL_CMD_HEALTH_INFO_TEMPERATURE_NOT_IMPL) > > - return -EOPNOTSUPP; > > + if (rc == CXL_CMD_HEALTH_INFO_TEMPERATURE_NOT_IMPL) { > > + errno = EOPNOTSUPP; > > + return INT_MAX; > > + } > > return rc; > > } > > >
