> > On Thu, Feb 06, 2020 at 07:32:12PM +0000, Avri Altman wrote: > > Hi Julian, > > > > > > > > > > > Hi Avri, > > > > > > On Fri, Feb 7, 2020 at 12:41 AM Avri Altman <Avri.Altman@xxxxxxx> > wrote: > > > > > > > > > > > > > > Hi Avri, > > > > > > > > > > On Thu, Feb 6, 2020 at 11:08 PM Avri Altman > <Avri.Altman@xxxxxxx> > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Avi, > > > > > > > > > > > > > > On Thu, Feb 6, 2020 at 9:48 PM Avi Shchislowski > > > > > > > <Avi.Shchislowski@xxxxxxx> 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. > > Those "alarms" are implemented as part of the modules under > drivers/hwmon/ isn't it? > > We already established that this is not an option for the ufs driver. > > You have established nothing. What exactly is not an option ? > To create alarm attributes ? No one forces you to create any of those > if you don't want to. > > > > > > > > > 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. > > Right. The thermal core allows to react to such notifications, > > Provided that the thermal zone device has a governor defined, > > And/or notify ops etc. > > > > Should the current patches implement those callbacks or not, > > Can be discussed during their review process. > > But the important thing is that the thermal core support it in an intuitive > and simple way, > > While the hwmon doesn't. > > > > We are indifference with respect of which subsystem to use. > > Not really. Quite the opposite; you are quite obviously heavily > opposed to a hwmon driver. > > > The thermal core was our first choice because we bumped into it, > > Looking for a way to raise thermal exceptions. > > It provides the functionality we need, and other devices uses it, > > Why the insistence not to use it? > > > > As mentioned before, the hwmon subsystem lets you create a bridge > to the thermal subsystem, it creates standard attributes to report > temperatures instead of the private ones your patch provides, > and it would result in simpler code. > > Why the insistence to _not_ use the hwmon subsystem ? I am not. Please, guide me through it - We'll register a hwmon device and implement its read ops. Since read is confined to the ufs driver, we'll still need to have the ufs-thermal module that we've created. Next we need to respond to a thermal exception event raised by the device - We are expected to pass such notification onward, for whatever governor/mind to take an applicable action, Should such mind exists. How do I do that? > > Guenter
