| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <b914ea25-a9a8-f443-2ba0-615bdd6cc04f@roeck-us.net>
Date: Fri, 28 Oct 2022 08:11:59 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Cristian Marussi <cristian.marussi@....com>,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Cc: sudeep.holla@....com, Daniel Lezcano <daniel.lezcano@...aro.org>,
linux-hwmon@...r.kernel.org
Subject: Re: [PATCH 7/8] hwmon: (scmi) Register explicitly with Thermal
Framework
On 10/28/22 07:08, Cristian Marussi wrote:
> Available sensors are enumerated and reported by the SCMI platform server
> using a 16bit identification number; not all such sensors are of a type
> supported by hwmon subsystem and, among the supported ones, only a subset
> could be temperature sensors that have to be registered with the Thermal
> Framework.
> Potential clashes between hwmon channels indexes and the underlying real
> sensors IDs do not play well with the hwmon<-->thermal bridge automatic
> registration routines and could need a sensible number of fake dummy
> sensors to be made up in order to keep indexes and IDs in sync.
>
> Avoid to use the hwmon<-->thermal bridge dropping the HWMON_C_REGISTER_TZ
> attribute and instead explicit register temperature sensors directly with
> the Thermal Framework.
>
For my reference:
Reviewed-by: Guenter Roeck <linux@...ck-us.net>
$subject says "patch 7/8". Patches 1-6 are firmware patches. Does this patch
depend on the other patches of the series or can I apply it on its own ?
Additional comment inline below.
Thanks,
Guenter
> Cc: Daniel Lezcano <daniel.lezcano@...aro.org>
> Cc: Guenter Roeck <linux@...ck-us.net>
> Cc: linux-hwmon@...r.kernel.org
> Signed-off-by: Cristian Marussi <cristian.marussi@....com>
> ---
> drivers/hwmon/scmi-hwmon.c | 115 ++++++++++++++++++++++++++++++++-----
> 1 file changed, 102 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/hwmon/scmi-hwmon.c b/drivers/hwmon/scmi-hwmon.c
> index b1329a58ce40..124fe8ee1b9b 100644
> --- a/drivers/hwmon/scmi-hwmon.c
> +++ b/drivers/hwmon/scmi-hwmon.c
> @@ -20,6 +20,11 @@ struct scmi_sensors {
> const struct scmi_sensor_info **info[hwmon_max];
> };
>
> +struct scmi_thermal_sensor {
> + const struct scmi_protocol_handle *ph;
> + const struct scmi_sensor_info *info;
> +};
> +
> static inline u64 __pow10(u8 x)
> {
> u64 r = 1;
> @@ -64,16 +69,14 @@ static int scmi_hwmon_scale(const struct scmi_sensor_info *sensor, u64 *value)
> return 0;
> }
>
> -static int scmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> - u32 attr, int channel, long *val)
> +static int scmi_hwmon_read_scaled_value(const struct scmi_protocol_handle *ph,
> + const struct scmi_sensor_info *sensor,
> + long *val)
> {
> int ret;
> u64 value;
> - const struct scmi_sensor_info *sensor;
> - struct scmi_sensors *scmi_sensors = dev_get_drvdata(dev);
>
> - sensor = *(scmi_sensors->info[type] + channel);
> - ret = sensor_ops->reading_get(scmi_sensors->ph, sensor->id, &value);
> + ret = sensor_ops->reading_get(ph, sensor->id, &value);
> if (ret)
> return ret;
>
> @@ -84,6 +87,17 @@ static int scmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> return ret;
> }
>
> +static int scmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long *val)
> +{
> + const struct scmi_sensor_info *sensor;
> + struct scmi_sensors *scmi_sensors = dev_get_drvdata(dev);
> +
> + sensor = *(scmi_sensors->info[type] + channel);
> +
> + return scmi_hwmon_read_scaled_value(scmi_sensors->ph, sensor, val);
> +}
> +
> static int
> scmi_hwmon_read_string(struct device *dev, enum hwmon_sensor_types type,
> u32 attr, int channel, const char **str)
> @@ -122,6 +136,25 @@ static struct hwmon_chip_info scmi_chip_info = {
> .info = NULL,
> };
>
> +static int scmi_hwmon_thermal_get_temp(struct thermal_zone_device *tz,
> + int *temp)
> +{
> + int ret;
> + long value;
> + struct scmi_thermal_sensor *th_sensor = tz->devdata;
> +
> + ret = scmi_hwmon_read_scaled_value(th_sensor->ph, th_sensor->info,
> + &value);
> + if (!ret)
> + *temp = value;
> +
> + return ret;
> +}
> +
> +static const struct thermal_zone_device_ops scmi_hwmon_thermal_ops = {
> + .get_temp = scmi_hwmon_thermal_get_temp,
> +};
> +
> static int scmi_hwmon_add_chan_info(struct hwmon_channel_info *scmi_hwmon_chan,
> struct device *dev, int num,
> enum hwmon_sensor_types type, u32 config)
> @@ -149,7 +182,6 @@ static enum hwmon_sensor_types scmi_types[] = {
> };
>
> static u32 hwmon_attributes[hwmon_max] = {
> - [hwmon_chip] = HWMON_C_REGISTER_TZ,
> [hwmon_temp] = HWMON_T_INPUT | HWMON_T_LABEL,
> [hwmon_in] = HWMON_I_INPUT | HWMON_I_LABEL,
> [hwmon_curr] = HWMON_C_INPUT | HWMON_C_LABEL,
> @@ -157,6 +189,43 @@ static u32 hwmon_attributes[hwmon_max] = {
> [hwmon_energy] = HWMON_E_INPUT | HWMON_E_LABEL,
> };
>
> +static int scmi_thermal_sensor_register(struct device *dev,
> + const struct scmi_protocol_handle *ph,
> + const struct scmi_sensor_info *sensor)
> +{
> + struct scmi_thermal_sensor *th_sensor;
> + struct thermal_zone_device *tzd;
> +
> + th_sensor = devm_kzalloc(dev, sizeof(*th_sensor), GFP_KERNEL);
> + if (!th_sensor)
> + return -ENOMEM;
> +
> + th_sensor->ph = ph;
> + th_sensor->info = sensor;
> +
> + /*
> + * Try to register a temperature sensor with the Thermal Framework:
> + * skip sensors not defined as part of any thermal zone (-ENODEV) but
> + * report any other errors related to misconfigured zones/sensors.
> + */
> + tzd = devm_thermal_of_zone_register(dev, th_sensor->info->id, th_sensor,
> + &scmi_hwmon_thermal_ops);
> + if (IS_ERR(tzd)) {
> + devm_kfree(dev, th_sensor);
> +
> + if (PTR_ERR(tzd) != -ENODEV)
> + return PTR_ERR(tzd);
> +
> + dev_info(dev, "Sensor '%s' not attached to any thermal zone.\n",
> + sensor->name);
There were complaints about this message as it is noisy. If you send
another version, please drop it unless attaching each sensor to a thermal
zone is strongly expected. If you don't send another version, I'll drop it
while applying.
> + } else {
> + dev_dbg(dev, "Sensor '%s' attached to thermal zone ID:%d\n",
> + sensor->name, tzd->id);
> + }
> +
> + return 0;
> +}
> +
> static int scmi_hwmon_probe(struct scmi_device *sdev)
> {
> int i, idx;
> @@ -164,7 +233,7 @@ static int scmi_hwmon_probe(struct scmi_device *sdev)
> enum hwmon_sensor_types type;
> struct scmi_sensors *scmi_sensors;
> const struct scmi_sensor_info *sensor;
> - int nr_count[hwmon_max] = {0}, nr_types = 0;
> + int nr_count[hwmon_max] = {0}, nr_types = 0, nr_count_temp = 0;
> const struct hwmon_chip_info *chip_info;
> struct device *hwdev, *dev = &sdev->dev;
> struct hwmon_channel_info *scmi_hwmon_chan;
> @@ -208,10 +277,8 @@ static int scmi_hwmon_probe(struct scmi_device *sdev)
> }
> }
>
> - if (nr_count[hwmon_temp]) {
> - nr_count[hwmon_chip]++;
> - nr_types++;
> - }
> + if (nr_count[hwmon_temp])
> + nr_count_temp = nr_count[hwmon_temp];
>
> scmi_hwmon_chan = devm_kcalloc(dev, nr_types, sizeof(*scmi_hwmon_chan),
> GFP_KERNEL);
> @@ -262,8 +329,30 @@ static int scmi_hwmon_probe(struct scmi_device *sdev)
> hwdev = devm_hwmon_device_register_with_info(dev, "scmi_sensors",
> scmi_sensors, chip_info,
> NULL);
> + if (IS_ERR(hwdev))
> + return PTR_ERR(hwdev);
> +
> + for (i = 0; i < nr_count_temp; i++) {
> + int ret;
>
> - return PTR_ERR_OR_ZERO(hwdev);
> + sensor = *(scmi_sensors->info[hwmon_temp] + i);
> + if (!sensor)
> + continue;
> +
> + /*
> + * Warn on any misconfiguration related to thermal zones but
> + * bail out of probing only on memory errors.
> + */
> + ret = scmi_thermal_sensor_register(dev, ph, sensor);
> + if (ret == -ENOMEM)
> + return ret;
> + else if (ret)
> + dev_warn(dev,
> + "Thermal zone misconfigured for %s. err=%d\n",
> + sensor->name, ret);
> + }
> +
> + return 0;
> }
>
> static const struct scmi_device_id scmi_id_table[] = {
Powered by blists - more mailing lists