Re: [PATCH 0/5] scsi: ufs: ufs device as a temperature sensor

From: Julian Calaby <julian.calaby@gmail.com>
To: Avri Altman <Avri.Altman@wdc.com>
Cc: Avi Shchislowski <Avi.Shchislowski@wdc.com>,
	Guenter Roeck <linux@roeck-us.net>,
	Alim Akhtar <alim.akhtar@samsung.com>,
	"James E.J. Bottomley" <jejb@linux.ibm.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>
Subject: Re: [PATCH 0/5] scsi: ufs: ufs device as a temperature sensor
Date: Fri, 7 Feb 2020 02:49:58 +1100	[thread overview]
Message-ID: <CAGRGNgWob+0t35AYXfzCqKtLjBgw=p8MhqDCKF=5_JGe5veqtQ@mail.gmail.com> (raw)
In-Reply-To: <MN2PR04MB699190E3474F82BEF9B91A58FC1D0@MN2PR04MB6991.namprd04.prod.outlook.com>

Hi Avri,

On Fri, Feb 7, 2020 at 12:41 AM Avri Altman <Avri.Altman@wdc.com> wrote:
>
> >
> > Hi Avri,
> >
> > On Thu, Feb 6, 2020 at 11:08 PM Avri Altman <Avri.Altman@wdc.com>
> > wrote:
> > >
> > >
> > > >
> > > > Hi Avi,
> > > >
> > > > On Thu, Feb 6, 2020 at 9:48 PM Avi Shchislowski
> > > > <Avi.Shchislowski@wdc.com> 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?
> > > This will require to export the ufs device management commands,
> > > Which is privet to the ufs driver.
> > >
> > > This is not a viable option as well, because it will allow unrestricted access
> > > (Including format etc.) to the storage device.
> > >
> > > Sorry for not making it clearer before.
> >
> > I should have clarified further: I meant having the UFS device
> > register a HWMON driver using this API:
> > https://www.kernel.org/doc/html/latest/hwmon/hwmon-kernel-api.html
> >
> > *Not* writing a separate HWMON driver that uses some private interface.
> Ok.
> Just one last question:
> The ufs spec requires to be able to react upon an exception event from the device.
> The thermal core provides an api in the form of thermal_notify_framework().
> What would be the hwmon equivalent for that?

My understanding is that HWMON is just a standardised way to report
hardware sensor data to userspace. There are "alarm" files that can be
used to report fault conditions, so any action taken would have to be
either managed by userspace or configured using thermal zones
configured in the hardware's devicetree.

thermal_notify_framework() is a way to notify the "other side" of a
thermal zone to do something to reduce the temperature of that zone.
E.g. spin up a fan or switch to a lower-power state to cool a CPU.
Looking at your code, you're only implementing the "sensor" side of
the thermal zone functionality, so your calls to
thermal_notify_framework() won't do anything.

Thanks,

--
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/