Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
IronPort-SDR: n7YcpzwPFR4QuLG9U7ynn7m5YaaVbEH2hZVO0GBm+y+moUbi8pbvNc+J6J4J5VJjlv+r1f9Gep
 1CG+cWJlqWeXtQNuw8vXa/ZtS+DD2l8ztAgX4Qgta3xYpw53Zx9JjbpiT1jd1fdbbbM70Ws0Gr
 hrBpubCMtxPBTiSuGAYo2OMWE7zO3+0Lmk1F1kuJXKv/lem5uSum6Hf2RJ1wl1R0GqsbqUoYpt
 1YWKpww3Y6GBsCNDiTPyk4lOIu9rbzork+G34i3554CEr7oTFcmRt6NjGQMrYMaW1Kr8yi3N9X
 Wr8=
From:   Avri Altman <Avri.Altman@wdc.com>
To:     Guenter Roeck <linux@roeck-us.net>
CC:     Julian Calaby <julian.calaby@gmail.com>,
        Avi Shchislowski <Avi.Shchislowski@wdc.com>,
        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
Thread-Topic: [PATCH 0/5] scsi: ufs: ufs device as a temperature sensor
Thread-Index: AQHV2bYgyCYLCd8TvkGCDJX1L0irp6gISGeAgAEWVwCAAC+qAIAAZ98AgAANYYCAAIzHgIADb7iAgAAQtgCAAAb1sIAACmKAgAAMGmCAAChwAIAANn2QgAAbMYCAABlPEA==
Date:   Thu, 6 Feb 2020 22:21:10 +0000
Message-ID: <MN2PR04MB69913DA014B665CFAC3AF8FFFC1D0@MN2PR04MB6991.namprd04.prod.outlook.com>
References: <MN2PR04MB69910152F14A7D481029E4ECFC000@MN2PR04MB6991.namprd04.prod.outlook.com>
 <20200203214733.GA30898@roeck-us.net>
 <BY5PR04MB69809A3BEFD629A67FB563CDFC030@BY5PR04MB6980.namprd04.prod.outlook.com>
 <MN2PR04MB6190D9E63717D37285DADBB09A1D0@MN2PR04MB6190.namprd04.prod.outlook.com>
 <CAGRGNgWG2fvY33j0m00SkguU8N4TJttY4KeNtOxZ7HzTTXA=yw@mail.gmail.com>
 <MN2PR04MB6991848EBC8DED439FCD7C49FC1D0@MN2PR04MB6991.namprd04.prod.outlook.com>
 <CAGRGNgUA=LHbWqZY+hsYjfsTbyftc3uoGv6S3p8E4zPQyqsOGQ@mail.gmail.com>
 <MN2PR04MB699190E3474F82BEF9B91A58FC1D0@MN2PR04MB6991.namprd04.prod.outlook.com>
 <CAGRGNgWob+0t35AYXfzCqKtLjBgw=p8MhqDCKF=5_JGe5veqtQ@mail.gmail.com>
 <MN2PR04MB699192FB02C86DE567785A83FC1D0@MN2PR04MB6991.namprd04.prod.outlook.com>
 <20200206204218.GA23857@roeck-us.net>
In-Reply-To: <20200206204218.GA23857@roeck-us.net>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 3679e70e-5011-4ca4-d89e-08d7ab52de61
X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Feb 2020 22:21:10.0586
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: Lp1vCYBriIr4JIOG46QiAJ4CIdaSMmg7z9aZchE8zAMvFiTo24hkmava9JBUFwI4hNr3bwOMq4g0wbGoEW3c4A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR04MB5582
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

>=20
> 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@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 bu=
ilt-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 unre=
stricted
> > > 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 f=
rom
> 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 b=
e
> > > 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.
>=20
> 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.
>=20
> >
> > >
> > > 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 intui=
tive
> and simple way,
> > While the hwmon doesn't.
> >
> > We are indifference with respect of which subsystem to use.
>=20
> Not really. Quite the opposite; you are quite obviously heavily
> opposed to a hwmon driver.
>=20
> > 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?
> >
>=20
> 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.
>=20
> Why the insistence to _not_ use the hwmon subsystem ?
I am not. Please, guide me through it -=20
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 -=
=20
We are expected to pass such notification onward, for whatever governor/min=
d to  take an applicable action,
Should such mind exists.  How do I do that?

>=20
> Guenter