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;
MIME-Version: 1.0
References: <20191102145530.16104-1-linux@roeck-us.net>
In-Reply-To: <20191102145530.16104-1-linux@roeck-us.net>
From:   Akinobu Mita <akinobu.mita@gmail.com>
Date:   Wed, 6 Nov 2019 00:38:39 +0900
Message-ID: <CAC5umyi9PuMTERNvNShfzu725bhBtkOZsD3NWtcxhKq5XGU2CQ@mail.gmail.com>
Subject: Re: [PATCH v4] nvme: Add hardware monitoring support
To:     Guenter Roeck <linux@roeck-us.net>
Cc:     Keith Busch <kbusch@kernel.org>, Chris Healy <cphealy@gmail.com>,
        Jens Axboe <axboe@fb.com>, Christoph Hellwig <hch@lst.de>,
        Sagi Grimberg <sagi@grimberg.me>,
        LKML <linux-kernel@vger.kernel.org>,
        linux-nvme@lists.infradead.org, Linux PM <linux-pm@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

2019=E5=B9=B411=E6=9C=882=E6=97=A5(=E5=9C=9F) 23:55 Guenter Roeck <linux@ro=
eck-us.net>:
> diff --git a/drivers/nvme/host/nvme-hwmon.c b/drivers/nvme/host/nvme-hwmo=
n.c
> new file mode 100644
> index 000000000000..28b4b7f43bb0
> --- /dev/null
> +++ b/drivers/nvme/host/nvme-hwmon.c
> @@ -0,0 +1,181 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * NVM Express hardware monitoring support
> + * Copyright (c) 2019, Guenter Roeck
> + */
> +
> +#include <linux/hwmon.h>
> +#include <asm/unaligned.h>
> +
> +#include "nvme.h"
> +
> +struct nvme_hwmon_data {
> +       struct nvme_ctrl *ctrl;
> +       struct nvme_smart_log log;
> +       struct mutex read_lock;
> +};
> +
> +static int nvme_hwmon_get_smart_log(struct nvme_hwmon_data *data)
> +{
> +       int ret;
> +
> +       ret =3D nvme_get_log(data->ctrl, NVME_NSID_ALL, NVME_LOG_SMART, 0=
,
> +                          &data->log, sizeof(data->log), 0);
> +
> +       return ret <=3D 0 ? ret : -EIO;
> +}
> +
> +static int nvme_hwmon_read(struct device *dev, enum hwmon_sensor_types t=
ype,
> +                          u32 attr, int channel, long *val)
> +{
> +       struct nvme_hwmon_data *data =3D dev_get_drvdata(dev);
> +       struct nvme_smart_log *log =3D &data->log;
> +       int temp;
> +       int err;
> +
> +       /*
> +        * First handle attributes which don't require us to read
> +        * the smart log.
> +        */
> +       switch (attr) {
> +       case hwmon_temp_max:
> +               *val =3D (data->ctrl->wctemp - 273) * 1000;
> +               return 0;
> +       case hwmon_temp_crit:
> +               *val =3D (data->ctrl->cctemp - 273) * 1000;

This attribute should be 'hwmon_temp_max_alarm' rather than
'hwmon_temp_crit_alarm'?

The 'hwmon_temp_crit_alarm' indicates that the temperature is greater
than CCTEMP.

But according to the description of the Critical Warning field in the NVMe
spec, the bit 1 is set to '1' when the temperature is greater than or
equal to an over temperature threshold.  The default value of the over
temperature threshold for Composite Temperature is WCTEMP.

That's why I think this attribute should be 'hwmon_temp_max_alarm' which
indicates that the temperature is greater than WCTEMP.