On 9/1/21 5:37 AM, Avri Altman wrote:
+What: /sys/bus/platform/drivers/ufshcd/*/attributes/case_rough_temp
+Date: September 2021
+Contact: Avri Altman <avri.altman@xxxxxxx>
+Description: The device case rough temperature (bDeviceCaseRoughTemperature
+ attribute). It is termed "rough" due to the inherent inaccuracy
+ of the temperature sensor inside a semiconductor device,
+ e.g. +- 10 degrees centigrade error range.
My understanding is that the word Celsius is more common than centigrade
so please use Celsius instead of centigrade. See also
https://www.brannan.co.uk/celsius-centigrade-and-fahrenheit/
+ allowable range is [-79..170].
+ The temperature readings are in decimal degrees Celsius.
+
+ Please note that the Tcase validity depends on the state of the
+ wExceptionEventControl attribute: it is up to the user to
+ verify that the applicable mask (TOO_HIGH_TEMP_EN, and / or
+ TOO_LOW_TEMP_EN) is set for the exception handling control.
+ This can be either done by ufs-bsg or ufs-debugfs.
Instead of making the user verify whether case_rough_temp is valid,
please modify the kernel code such that case_rough_temp only reports a
value if that value is valid. One possible approach is to make the show
method return an error code if case_rough_temp is not valid. Another and
probably better approach is to define a sysfs attribute group and to
make case_rough_temp visible only if it is valid.
diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c
index 5c405ff7b6ea..a9abe33c40e4 100644
--- a/drivers/scsi/ufs/ufs-sysfs.c
+++ b/drivers/scsi/ufs/ufs-sysfs.c
@@ -1047,6 +1047,86 @@ static inline bool ufshcd_is_wb_attrs(enum attr_idn idn)
idn <= QUERY_ATTR_IDN_CURR_WB_BUFF_SIZE;
}
+static inline bool ufshcd_is_temp_attrs(enum attr_idn idn)
+{
+ return idn >= QUERY_ATTR_IDN_CASE_ROUGH_TEMP &&
+ idn <= QUERY_ATTR_IDN_LOW_TEMP_BOUND;
+}
Modern compilers are good at deciding when to inline a function so
please leave out the 'inline' keyword from the above function.
+static bool ufshcd_case_temp_legal(struct ufs_hba *hba)\
Please use another word than "legal" since the primary meaning of
"legal" is "of or relating to law".
+ ufshcd_rpm_get_sync(hba);
+ ret = ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_READ_ATTR,
+ QUERY_ATTR_IDN_EE_CONTROL, 0, 0, &ee_mask);
+ ufshcd_rpm_put_sync(hba);
Are there any ufshcd_query_attr() calls that are not surrounded by
ufshcd_rpm_{get,put}_sync()? If not, please move the
ufshcd_rpm_{get,put}_sync() calls into ufshcd_query_attr().
Thanks,
Bart.
