On Sun, Nov 10, 2019 at 11:17:46PM +0900, Akinobu Mita wrote: > +static int nvme_get_temp_thresh(struct nvme_ctrl *ctrl, int sensor, bool under, > + long *temp) > +{ > + unsigned int threshold = sensor << NVME_TEMP_THRESH_SELECT_SHIFT; > + int status; > + int ret; > + > + if (under) > + threshold |= NVME_TEMP_THRESH_TYPE_UNDER; > + > + ret = nvme_get_features(ctrl, NVME_FEAT_TEMP_THRESH, threshold, NULL, 0, > + &status); > + if (!ret) > + *temp = ((status & NVME_TEMP_THRESH_MASK) - 273) * 1000; > + > + return ret <= 0 ? ret : -EIO; This looks a little obsfucated. aI'd prefer something like: if (ret > 0) return -EIO; if (ret < 0) return ret; *temp = ((status & NVME_TEMP_THRESH_MASK) - 273) * 1000; return 0; > + return ret <= 0 ? ret : -EIO; Similarly here, something like: if (ret > 0) return -EIO; return ret; > + err = nvme_get_temp_thresh(data->ctrl, channel, false, val); > + if (err) > + *val = (data->ctrl->wctemp - 273) * 1000; Can we have a helper for this (x - 273) * 1000 conversion? It is repeated quite a bit over the code in this file.