From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 831F9C001B0 for ; Mon, 7 Aug 2023 13:14:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231755AbjHGNOl (ORCPT ); Mon, 7 Aug 2023 09:14:41 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59956 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230367AbjHGNOl (ORCPT ); Mon, 7 Aug 2023 09:14:41 -0400 Received: from frasgout.his.huawei.com (frasgout.his.huawei.com [185.176.79.56]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 78287EB for ; Mon, 7 Aug 2023 06:14:39 -0700 (PDT) Received: from lhrpeml500005.china.huawei.com (unknown [172.18.147.226]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4RKGr46QBXz6802T; Mon, 7 Aug 2023 21:09:40 +0800 (CST) Received: from localhost (10.202.227.76) by lhrpeml500005.china.huawei.com (7.191.163.240) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.27; Mon, 7 Aug 2023 14:14:36 +0100 Date: Mon, 7 Aug 2023 14:14:35 +0100 From: Jonathan Cameron To: Jehoon Park CC: , , Alison Schofield , Vishal Verma , "Ira Weiny" , Dan Williams , "Dave Jiang" , Davidlohr Bueso , "Kyungsan Kim" , Junhyeok Im Subject: Re: [ndctl PATCH v2 2/3] libcxl: Fix accessors for temperature field to support negative value Message-ID: <20230807141435.00004eb0@Huawei.com> In-Reply-To: <20230807063549.5942-3-jehoon.park@samsung.com> References: <20230807063549.5942-1-jehoon.park@samsung.com> <20230807063549.5942-3-jehoon.park@samsung.com> Organization: Huawei Technologies Research and Development (UK) Ltd. X-Mailer: Claws Mail 4.1.0 (GTK 3.24.33; x86_64-w64-mingw32) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.202.227.76] X-ClientProxiedBy: lhrpeml500002.china.huawei.com (7.191.160.78) To lhrpeml500005.china.huawei.com (7.191.163.240) X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: linux-cxl@vger.kernel.org On Mon, 7 Aug 2023 15:35:48 +0900 Jehoon Park 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. > > Signed-off-by: Jehoon Park > --- > 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. > 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; > } >