UFS3.0 allows using the ufs device as a temperature sensor. The
purpose of this feature is to provide notification to the host of the
UFS device case temperature. It allows reading of a rough estimate
(+-10 degrees centigrade) of the current case temperature, And
setting a lower and upper temperature bounds, in which the device
will trigger an applicable exception event.
We added the capability of responding to such notifications, while
notifying the kernel's thermal core, which further exposes the thermal
zone attributes to user space. UFS temperature attributes are all
read-only, so only thermal read ops (.get_xxx) can be implemented.
Avi Shchislowski (5):
scsi: ufs: Add ufs thermal support
scsi: ufs: export ufshcd_enable_ee
scsi: ufs: enable thermal exception event
scsi: ufs-thermal: implement thermal file ops
scsi: ufs: temperature atrributes add to ufs_sysfs
drivers/scsi/ufs/Kconfig | 11 ++
drivers/scsi/ufs/Makefile | 1 +
drivers/scsi/ufs/ufs-sysfs.c | 6 +
drivers/scsi/ufs/ufs-thermal.c | 247 +++++++++++++++++++++++++++++++++++++++++
drivers/scsi/ufs/ufs-thermal.h | 25 +++++
drivers/scsi/ufs/ufs.h | 20 +++-
drivers/scsi/ufs/ufshcd.c | 9 +-
drivers/scsi/ufs/ufshcd.h | 12 ++
8 files changed, 329 insertions(+), 2 deletions(-)
create mode 100644 drivers/scsi/ufs/ufs-thermal.c
create mode 100644 drivers/scsi/ufs/ufs-thermal.h
--
1.9.1
From: Avi Shchislowski <[email protected]>
Signed-off-by: Uri Yanai <[email protected]>
Signed-off-by: Avi Shchislowski <[email protected]>
---
drivers/scsi/ufs/ufs-sysfs.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c
index dbdf8b0..180f4db 100644
--- a/drivers/scsi/ufs/ufs-sysfs.c
+++ b/drivers/scsi/ufs/ufs-sysfs.c
@@ -667,6 +667,9 @@ static DEVICE_ATTR_RO(_name)
UFS_ATTRIBUTE(ffu_status, _FFU_STATUS);
UFS_ATTRIBUTE(psa_state, _PSA_STATE);
UFS_ATTRIBUTE(psa_data_size, _PSA_DATA_SIZE);
+UFS_ATTRIBUTE(rough_temp, _ROUGH_TEMP);
+UFS_ATTRIBUTE(too_high_temp, _TOO_HIGH_TEMP);
+UFS_ATTRIBUTE(too_low_temp, _TOO_LOW_TEMP);
static struct attribute *ufs_sysfs_attributes[] = {
&dev_attr_boot_lun_enabled.attr,
@@ -685,6 +688,9 @@ static DEVICE_ATTR_RO(_name)
&dev_attr_ffu_status.attr,
&dev_attr_psa_state.attr,
&dev_attr_psa_data_size.attr,
+ &dev_attr_rough_temp.attr,
+ &dev_attr_too_high_temp.attr,
+ &dev_attr_too_low_temp.attr,
NULL,
};
--
1.9.1
On Sun, Feb 02, 2020 at 12:46:54PM +0200, Avi Shchislowski wrote:
> UFS3.0 allows using the ufs device as a temperature sensor. The
> purpose of this feature is to provide notification to the host of the
> UFS device case temperature. It allows reading of a rough estimate
> (+-10 degrees centigrade) of the current case temperature, And
> setting a lower and upper temperature bounds, in which the device
> will trigger an applicable exception event.
>
> We added the capability of responding to such notifications, while
> notifying the kernel's thermal core, which further exposes the thermal
> zone attributes to user space. UFS temperature attributes are all
> read-only, so only thermal read ops (.get_xxx) can be implemented.
>
Can you add an explanation why this can't be added to the just-introduced
'drivetemp' driver in the hwmon subsystem, and why it make sense to
have proprietary attributes for temperature and temperature limits ?
Thanks,
Guenter
> Avi Shchislowski (5):
> scsi: ufs: Add ufs thermal support
> scsi: ufs: export ufshcd_enable_ee
> scsi: ufs: enable thermal exception event
> scsi: ufs-thermal: implement thermal file ops
> scsi: ufs: temperature atrributes add to ufs_sysfs
attributes
Hi, Avi
Please add "resend" prefix or version in your subject when you re-send your patches for the next time,
Such make us easier follow your changes.
Thanks,
//Bean
> Subject: [EXT] [PATCH 0/5] scsi: ufs: ufs device as a temperature sensor
>
> Avi Shchislowski (5):
> scsi: ufs: Add ufs thermal support
> scsi: ufs: export ufshcd_enable_ee
> scsi: ufs: enable thermal exception event
> scsi: ufs-thermal: implement thermal file ops
> scsi: ufs: temperature atrributes add to ufs_sysfs
>
> drivers/scsi/ufs/Kconfig | 11 ++
> drivers/scsi/ufs/Makefile | 1 +
> drivers/scsi/ufs/ufs-sysfs.c | 6 +
> drivers/scsi/ufs/ufs-thermal.c | 247
> +++++++++++++++++++++++++++++++++++++++++
> drivers/scsi/ufs/ufs-thermal.h | 25 +++++
> drivers/scsi/ufs/ufs.h | 20 +++-
> drivers/scsi/ufs/ufshcd.c | 9 +-
> drivers/scsi/ufs/ufshcd.h | 12 ++
> 8 files changed, 329 insertions(+), 2 deletions(-) create mode 100644
> drivers/scsi/ufs/ufs-thermal.c create mode 100644 drivers/scsi/ufs/ufs-
> thermal.h
>
> --
> 1.9.1
> -----Original Message-----
> From: Guenter Roeck <[email protected]> On Behalf Of Guenter Roeck
> Sent: Sunday, February 2, 2020 9:21 PM
> To: Avi Shchislowski <[email protected]>
> Cc: Alim Akhtar <[email protected]>; Avri Altman
> <[email protected]>; James E.J. Bottomley <[email protected]>;
> Martin K. Petersen <[email protected]>; linux-
> [email protected]; [email protected]
> Subject: Re: [PATCH 0/5] scsi: ufs: ufs device as a temperature sensor
>
> On Sun, Feb 02, 2020 at 12:46:54PM +0200, Avi Shchislowski wrote:
> > UFS3.0 allows using the ufs device as a temperature sensor. The
> > purpose of this feature is to provide notification to the host of the
> > UFS device case temperature. It allows reading of a rough estimate
> > (+-10 degrees centigrade) of the current case temperature, And setting
> > a lower and upper temperature bounds, in which the device will trigger
> > an applicable exception event.
> >
> > We added the capability of responding to such notifications, while
> > notifying the kernel's thermal core, which further exposes the thermal
> > zone attributes to user space. UFS temperature attributes are all
> > read-only, so only thermal read ops (.get_xxx) can be implemented.
> >
>
> Can you add an explanation why this can't be added to the just-introduced
> 'drivetemp' driver in the hwmon subsystem, and why it make sense to have
> proprietary attributes for temperature and temperature limits ?
>
> Thanks,
> Guenter
>
Hi Guenter
Thank you for your comment
The ufs device is not a temperature sensor per-se. It is, first and foremost, a storage device.
Reporting the device case temperature is a feature added in a recently released UFS spec (UFS3.0).
Therefore, adding a thermal-core module, in opposed to hwmon module, seemed more appropriate.
Registering a hwmon device look excessive, as no other hw-monitoring attribute is available - aside temperature.
Using Martin's tree, I wasn't able to locate the 'drivetemp' module, nor any reference to it in the hwmon documentation.
> > Avi Shchislowski (5):
> > scsi: ufs: Add ufs thermal support
> > scsi: ufs: export ufshcd_enable_ee
> > scsi: ufs: enable thermal exception event
> > scsi: ufs-thermal: implement thermal file ops
> > scsi: ufs: temperature atrributes add to ufs_sysfs
>
> attributes
On 2/3/20 3:57 AM, Avi Shchislowski wrote:
>
>
>> -----Original Message-----
>> From: Guenter Roeck <[email protected]> On Behalf Of Guenter Roeck
>> Sent: Sunday, February 2, 2020 9:21 PM
>> To: Avi Shchislowski <[email protected]>
>> Cc: Alim Akhtar <[email protected]>; Avri Altman
>> <[email protected]>; James E.J. Bottomley <[email protected]>;
>> Martin K. Petersen <[email protected]>; linux-
>> [email protected]; [email protected]
>> Subject: Re: [PATCH 0/5] scsi: ufs: ufs device as a temperature sensor
>>
>
>> On Sun, Feb 02, 2020 at 12:46:54PM +0200, Avi Shchislowski wrote:
>>> UFS3.0 allows using the ufs device as a temperature sensor. The
>>> purpose of this feature is to provide notification to the host of the
>>> UFS device case temperature. It allows reading of a rough estimate
>>> (+-10 degrees centigrade) of the current case temperature, And setting
>>> a lower and upper temperature bounds, in which the device will trigger
>>> an applicable exception event.
>>>
>>> We added the capability of responding to such notifications, while
>>> notifying the kernel's thermal core, which further exposes the thermal
>>> zone attributes to user space. UFS temperature attributes are all
>>> read-only, so only thermal read ops (.get_xxx) can be implemented.
>>>
>>
>> Can you add an explanation why this can't be added to the just-introduced
>> 'drivetemp' driver in the hwmon subsystem, and why it make sense to have
>> proprietary attributes for temperature and temperature limits ?
>>
>> Thanks,
>> Guenter
>>
> Hi Guenter
>
> Thank you for your comment
>
> The ufs device is not a temperature sensor per-se. It is, first and foremost, a storage device.
Huh ? Many non-temperature-sensor devices in the kernel report their temperature
through the hardware monitoring subsystem.
> Reporting the device case temperature is a feature added in a recently released UFS spec (UFS3.0).
> Therefore, adding a thermal-core module, in opposed to hwmon module, seemed more appropriate.
> Registering a hwmon device look excessive, as no other hw-monitoring attribute is available - aside temperature.
>
Really ? Interesting position. Are you saying that instantiating a thermal core
module, plus providing non-standard sysfs attributes to report the temperature,
is less complex ? Are you sure ? Can you provide evidence that this is indeed
the case ?
> Using Martin's tree, I wasn't able to locate the 'drivetemp' module, nor any reference to it in the hwmon documentation.
>
You might want to consider looking in the mainline kernel.
Guenter
> >> Can you add an explanation why this can't be added to the just-
> introduced
> >> 'drivetemp' driver in the hwmon subsystem, and why it make sense to
> have
> >> proprietary attributes for temperature and temperature limits ?
Guenter hi,
Yeah - I see your point. But here is the thing -
UFS devices support only a subset of scsi commands.
It does not support ATA_16 nor SMART attributes.
Moreover, you can't read UFS attributes using any other scsi/ATA/SATA
Commands, nor it obey the ATA temperature sensing conventions.
So unless you want to totally break the newly born drivetemp -
Better to leave ufs devices out of it.
Another option is to put a ufs module under hwmon.
Do you see why would that be more advantageous to using the thermal core?
Provided that you are not going to deprecate it (Intel's wifi card is still using it)...
As for adding those attributes to ufs-sysfs -
this is just a supplementary as all other attributes are exposed this way.
Thanks,
Avri
On Mon, Feb 03, 2020 at 09:29:57PM +0000, Avri Altman wrote:
> > >> Can you add an explanation why this can't be added to the just-
> > introduced
> > >> 'drivetemp' driver in the hwmon subsystem, and why it make sense to
> > have
> > >> proprietary attributes for temperature and temperature limits ?
>
>
> Guenter hi,
> Yeah - I see your point. But here is the thing -
> UFS devices support only a subset of scsi commands.
> It does not support ATA_16 nor SMART attributes.
> Moreover, you can't read UFS attributes using any other scsi/ATA/SATA
> Commands, nor it obey the ATA temperature sensing conventions.
> So unless you want to totally break the newly born drivetemp -
> Better to leave ufs devices out of it.
>
drivetemp is written with extensibility in mind. For example, Martin has a
prototype enhancement which supports SCSI drive temperature sensors.
As long as a device can be identified as ufs device, and as long as there
is a means to pass-through commands, adding a new type would be easy.
> Another option is to put a ufs module under hwmon.
> Do you see why would that be more advantageous to using the thermal core?
> Provided that you are not going to deprecate it (Intel's wifi card is still using it)...
>
Deprecate what, and what does this discussion have to do with Intel's wifi
card ?
Either case, the hardware monitoring subsystem provides standard attributes,
and it provides a bridge to the thermal subsystem. The question should be
why _not_ to use the hwmon subsystem, and this question should be answered
as part of this patch series.
Guenter
> On Mon, Feb 03, 2020 at 09:29:57PM +0000, Avri Altman wrote:
> > > >> Can you add an explanation why this can't be added to the just-
> > > introduced
> > > >> 'drivetemp' driver in the hwmon subsystem, and why it make sense to
> > > have
> > > >> proprietary attributes for temperature and temperature limits ?
> >
> >
> > Guenter hi,
> > Yeah - I see your point. But here is the thing -
> > UFS devices support only a subset of scsi commands.
> > It does not support ATA_16 nor SMART attributes.
> > Moreover, you can't read UFS attributes using any other scsi/ATA/SATA
> > Commands, nor it obey the ATA temperature sensing conventions.
> > So unless you want to totally break the newly born drivetemp -
> > Better to leave ufs devices out of it.
> >
>
> drivetemp is written with extensibility in mind. For example, Martin has a
> prototype enhancement which supports SCSI drive temperature sensors.
> As long as a device can be identified as ufs device, and as long as there
The ufs device does not identifies as such, e.g. by INQUIRY or other.
> is a means to pass-through commands, adding a new type would be easy.
I am unaware of any such option.
Device management commands are privet to the ufs driver.
Thanks,
Avri
As it become evident that the hwmon is not a viable option to implement ufs thermal notification, I would appreciate some concrete comments of this series.
Thanks,
Avi
>
> > On Mon, Feb 03, 2020 at 09:29:57PM +0000, Avri Altman wrote:
> > > > >> Can you add an explanation why this can't be added to the just-
> > > > introduced
> > > > >> 'drivetemp' driver in the hwmon subsystem, and why it make
> > > > >> sense to
> > > > have
> > > > >> proprietary attributes for temperature and temperature limits ?
> > >
> > >
> > > Guenter hi,
> > > Yeah - I see your point. But here is the thing - UFS devices support
> > > only a subset of scsi commands.
> > > It does not support ATA_16 nor SMART attributes.
> > > Moreover, you can't read UFS attributes using any other
> > > scsi/ATA/SATA Commands, nor it obey the ATA temperature sensing
> conventions.
> > > So unless you want to totally break the newly born drivetemp -
> > > Better to leave ufs devices out of it.
> > >
> >
> > drivetemp is written with extensibility in mind. For example, Martin
> > has a prototype enhancement which supports SCSI drive temperature
> sensors.
> > As long as a device can be identified as ufs device, and as long as
> > there
> The ufs device does not identifies as such, e.g. by INQUIRY or other.
>
> > is a means to pass-through commands, adding a new type would be easy.
> I am unaware of any such option.
> Device management commands are privet to the ufs driver.
>
> Thanks,
> Avri
Hi Avi,
On Thu, Feb 6, 2020 at 9:48 PM Avi Shchislowski
<[email protected]> wrote:
>
> As it become evident that the hwmon is not a viable option to implement ufs thermal notification, I would appreciate some concrete comments of this series.
That isn't my reading of this thread.
You have two options:
1. extend drivetemp if that makes sense for this particular application.
2. follow the model of other devices that happen to have a built-in
temperature sensor and expose the hwmon compatible attributes as a
subdevice
It appears that option 1 isn't viable, so what about option 2?
Thanks,
--
Julian Calaby
Email: [email protected]
Profile: http://www.google.com/profiles/julian.calaby/