[PATCH] hwmon: (npcm-750-pwm-fan): Change initial pwm target to 255
Change initial PWM target to 255 to prevent overheating, for example when BMC hangs in userspace or when userspace fan control application is not implemented yet. Signed-off-by: Kun Yi --- drivers/hwmon/npcm750-pwm-fan.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hwmon/npcm750-pwm-fan.c b/drivers/hwmon/npcm750-pwm-fan.c index 96634fd54e0b..c0fab54c0094 100644 --- a/drivers/hwmon/npcm750-pwm-fan.c +++ b/drivers/hwmon/npcm750-pwm-fan.c @@ -52,7 +52,7 @@ /* Define the Counter Register, value = 100 for match 100% */ #define NPCM7XX_PWM_COUNTER_DEFAULT_NUM255 -#define NPCM7XX_PWM_CMR_DEFAULT_NUM127 +#define NPCM7XX_PWM_CMR_DEFAULT_NUM255 #define NPCM7XX_PWM_CMR_MAX255 /* default all PWM channels PRESCALE2 = 1 */ -- 2.19.0.605.g01d371f741-goog
[PATCH v2] hwmon: (ina3221) Validate shunt resistor value from DT
The input->shunt_resistor is int type while the value from DT is
unsigned int. Meanwhile, a divide-by-zero error would happen if
the value is 0. So this patch just simply validates the value.
Signed-off-by: Nicolin Chen
---
Changelog
v1->v2:
* Validate the value and error out
drivers/hwmon/ina3221.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/hwmon/ina3221.c b/drivers/hwmon/ina3221.c
index c3a497aed345..4f3ed24efe8e 100644
--- a/drivers/hwmon/ina3221.c
+++ b/drivers/hwmon/ina3221.c
@@ -536,8 +536,14 @@ static int ina3221_probe_child_from_dt(struct device *dev,
of_property_read_string(child, "label", >label);
/* Overwrite default shunt resistor value optionally */
- if (!of_property_read_u32(child, "shunt-resistor-micro-ohms", ))
+ if (!of_property_read_u32(child, "shunt-resistor-micro-ohms", )) {
+ if (val < 1 || val > INT_MAX) {
+ dev_err(dev, "invalid shunt resistor value %u of %s\n",
+ val, child->name);
+ return -EINVAL;
+ }
input->shunt_resistor = val;
+ }
return 0;
}
--
2.17.1
Re: [PATCH] hwmon: (ina3221) Clamp shunt resistor value from DT
On Mon, Oct 08, 2018 at 01:05:37PM -0700, Nicolin Chen wrote:
> The input->shunt_resistor is int type while the value from DT is
> unsigned int. Meanwhile, a divide-by-zero error would happen if
> the value is 0. So this patch just simply clamps the value.
>
I think it would be better to validate the numbers here instead of
silently accepting bad values, and return -EINVAL if the property value
is bad.
Thanks,
Guenter
> Signed-off-by: Nicolin Chen
> ---
> drivers/hwmon/ina3221.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hwmon/ina3221.c b/drivers/hwmon/ina3221.c
> index c3a497aed345..ce8f0a8c4982 100644
> --- a/drivers/hwmon/ina3221.c
> +++ b/drivers/hwmon/ina3221.c
> @@ -536,8 +536,10 @@ static int ina3221_probe_child_from_dt(struct device
> *dev,
> of_property_read_string(child, "label", >label);
>
> /* Overwrite default shunt resistor value optionally */
> - if (!of_property_read_u32(child, "shunt-resistor-micro-ohms", ))
> + if (!of_property_read_u32(child, "shunt-resistor-micro-ohms", )) {
> + val = clamp_val(val, 1, INT_MAX);
> input->shunt_resistor = val;
> + }
>
> return 0;
> }
> --
> 2.17.1
>
[PATCH v2] hwmon: (ina3221) Use _info API to register hwmon device
The hwmon core has a newer API which abstracts most of common
things in the core so as to simplify the hwmon device drivers.
This patch implements this _info API to ina3221 hwmon driver.
It also reduces the binary size:
textdata bss dec hex filename
51141712 068261aaa drivers/hwmon/ina3221_before.o
4456 440 048961320 drivers/hwmon/ina3221.o
Signed-off-by: Nicolin Chen
---
Changelog
v1->v2:
* Added a binary size comparison in commit message
* Dropped unnecessary checks
* Dropped misleading ID conversion in the comments
* Added two comments to indicate 0-align channel indexes
* Used "fall through" instead of "fallthrough"
drivers/hwmon/ina3221.c | 520 ++--
1 file changed, 236 insertions(+), 284 deletions(-)
diff --git a/drivers/hwmon/ina3221.c b/drivers/hwmon/ina3221.c
index ce8f0a8c4982..973fc4af0ddf 100644
--- a/drivers/hwmon/ina3221.c
+++ b/drivers/hwmon/ina3221.c
@@ -77,21 +77,6 @@ enum ina3221_channels {
INA3221_NUM_CHANNELS
};
-static const unsigned int register_channel[] = {
- [INA3221_BUS1] = INA3221_CHANNEL1,
- [INA3221_BUS2] = INA3221_CHANNEL2,
- [INA3221_BUS3] = INA3221_CHANNEL3,
- [INA3221_SHUNT1] = INA3221_CHANNEL1,
- [INA3221_SHUNT2] = INA3221_CHANNEL2,
- [INA3221_SHUNT3] = INA3221_CHANNEL3,
- [INA3221_CRIT1] = INA3221_CHANNEL1,
- [INA3221_CRIT2] = INA3221_CHANNEL2,
- [INA3221_CRIT3] = INA3221_CHANNEL3,
- [INA3221_WARN1] = INA3221_CHANNEL1,
- [INA3221_WARN2] = INA3221_CHANNEL2,
- [INA3221_WARN3] = INA3221_CHANNEL3,
-};
-
/**
* struct ina3221_input - channel input source specific information
* @label: label of channel input source
@@ -123,58 +108,6 @@ static inline bool ina3221_is_enabled(struct ina3221_data
*ina, int channel)
return ina->reg_config & INA3221_CONFIG_CHx_EN(channel);
}
-static ssize_t ina3221_show_label(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct sensor_device_attribute *sd_attr = to_sensor_dev_attr(attr);
- struct ina3221_data *ina = dev_get_drvdata(dev);
- unsigned int channel = sd_attr->index;
- struct ina3221_input *input = >inputs[channel];
-
- return snprintf(buf, PAGE_SIZE, "%s\n", input->label);
-}
-
-static ssize_t ina3221_show_enable(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct sensor_device_attribute *sd_attr = to_sensor_dev_attr(attr);
- struct ina3221_data *ina = dev_get_drvdata(dev);
- unsigned int channel = sd_attr->index;
-
- return snprintf(buf, PAGE_SIZE, "%d\n",
- ina3221_is_enabled(ina, channel));
-}
-
-static ssize_t ina3221_set_enable(struct device *dev,
- struct device_attribute *attr,
- const char *buf, size_t count)
-{
- struct sensor_device_attribute *sd_attr = to_sensor_dev_attr(attr);
- struct ina3221_data *ina = dev_get_drvdata(dev);
- unsigned int channel = sd_attr->index;
- u16 config, mask = INA3221_CONFIG_CHx_EN(channel);
- bool enable;
- int ret;
-
- ret = kstrtobool(buf, );
- if (ret)
- return ret;
-
- config = enable ? mask : 0;
-
- /* Enable or disable the channel */
- ret = regmap_update_bits(ina->regmap, INA3221_CONFIG, mask, config);
- if (ret)
- return ret;
-
- /* Cache the latest config register value */
- ret = regmap_read(ina->regmap, INA3221_CONFIG, >reg_config);
- if (ret)
- return ret;
-
- return count;
-}
-
static int ina3221_read_value(struct ina3221_data *ina, unsigned int reg,
int *val)
{
@@ -190,94 +123,104 @@ static int ina3221_read_value(struct ina3221_data *ina,
unsigned int reg,
return 0;
}
-static ssize_t ina3221_show_bus_voltage(struct device *dev,
- struct device_attribute *attr,
- char *buf)
+static const u8 ina3221_in_reg[] = {
+ INA3221_BUS1,
+ INA3221_BUS2,
+ INA3221_BUS3,
+ INA3221_SHUNT1,
+ INA3221_SHUNT2,
+ INA3221_SHUNT3,
+};
+
+static int ina3221_read_in(struct device *dev, u32 attr, int channel, long
*val)
{
- struct sensor_device_attribute *sd_attr = to_sensor_dev_attr(attr);
+ const bool is_shunt = channel > INA3221_CHANNEL3;
struct ina3221_data *ina = dev_get_drvdata(dev);
- unsigned int reg = sd_attr->index;
- unsigned int channel = register_channel[reg];
- int val, voltage_mv, ret;
+ u8 reg = ina3221_in_reg[channel];
+ int regval, ret;
- /* No data for read-only attribute if channel is disabled */
- if (!attr->store && !ina3221_is_enabled(ina, channel))
- return
[PATCH] hwmon: (ina3221) Clamp shunt resistor value from DT
The input->shunt_resistor is int type while the value from DT is
unsigned int. Meanwhile, a divide-by-zero error would happen if
the value is 0. So this patch just simply clamps the value.
Signed-off-by: Nicolin Chen
---
drivers/hwmon/ina3221.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/hwmon/ina3221.c b/drivers/hwmon/ina3221.c
index c3a497aed345..ce8f0a8c4982 100644
--- a/drivers/hwmon/ina3221.c
+++ b/drivers/hwmon/ina3221.c
@@ -536,8 +536,10 @@ static int ina3221_probe_child_from_dt(struct device *dev,
of_property_read_string(child, "label", >label);
/* Overwrite default shunt resistor value optionally */
- if (!of_property_read_u32(child, "shunt-resistor-micro-ohms", ))
+ if (!of_property_read_u32(child, "shunt-resistor-micro-ohms", )) {
+ val = clamp_val(val, 1, INT_MAX);
input->shunt_resistor = val;
+ }
return 0;
}
--
2.17.1
Re: [PATCH v5] scsi: Add hwmon support for SMART temperature sensors
On 10/7/18 10:49 PM, Linus Walleij wrote: S.M.A.R.T. temperature sensors have been supported for years by userspace tools such as smarttools. This adds support to read it from the kernel and adds a temperature zone for the drive. The temperature readout is however also a good fit for Linux' hwmon subsystem. By adding a hwmon interface to dig out SMART parameters, we can expose the drive temperature as a standard hwmon sensor. The idea came about when experimenting with NAS enclosures that lack their own on-board sensors but instead piggy-back the sensor found in the harddrive, if any, to decide on a policy for driving the on-board fan. The kernel thermal subsystem supports defining a thermal policy for the enclosure using the device tree, see e.g.: arch/arm/boot/dts/gemini-dlink-dns-313.dts but this requires a proper hwmon sensor integrated with the kernel. With this driver, the hard disk temperatur can be read from sysfs: > cd /sys/class/hwmon/hwmon0/ > cat temp1_input 38 If the harddrive supports one of the detected vendor extensions for providing min/max temperatures we also provide attributes for displaying that. This means that they can also be handled by userspace tools such as lm_sensors in a uniform way without need for any special tools such as "hddtemp" (which seems dormant). This driver does not block any simultaneous use of other SMART userspace tools, it's a both/and approach, not either/or. Reviewed-by: Guenter Roeck # HWMON Signed-off-by: Linus Walleij --- Hmm. I might be getting something wrong here, but the main problem with SMART values is that they are _not_ really standardized; plus any drive is free to implement whatever they want. Plus there are tons of SMART attribute values, and decoding just one seems to be a bit poor. At the same time having them in the kernel will just require us to implement lots of decoding, which I really would leave to userland. So overall I'm not sure if that the right approach. Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
