[PATCH 2/2] hwmon: (ina3221) Add operating mode support

2018-10-09 Thread Nicolin Chen
The hwmon core now has a new optional mode interface. So this patch
just implements this mode support so that user space can check and
configure via sysfs node its operating modes: power-down, one-shot,
and continuous modes.

Signed-off-by: Nicolin Chen 
---
 drivers/hwmon/ina3221.c | 64 +
 1 file changed, 64 insertions(+)

diff --git a/drivers/hwmon/ina3221.c b/drivers/hwmon/ina3221.c
index d61688f04594..5218fd85506d 100644
--- a/drivers/hwmon/ina3221.c
+++ b/drivers/hwmon/ina3221.c
@@ -77,6 +77,28 @@ enum ina3221_channels {
INA3221_NUM_CHANNELS
 };
 
+enum ina3221_modes {
+   INA3221_MODE_POWERDOWN,
+   INA3221_MODE_ONESHOT,
+   INA3221_MODE_CONTINUOUS,
+   INA3221_NUM_MODES,
+};
+
+static const char *ina3221_mode_names[INA3221_NUM_MODES] = {
+   [INA3221_MODE_POWERDOWN] = "power-down",
+   [INA3221_MODE_ONESHOT] = "one-shot",
+   [INA3221_MODE_CONTINUOUS] = "continuous",
+};
+
+static const u16 ina3221_mode_val[] = {
+   [INA3221_MODE_POWERDOWN] = INA3221_CONFIG_MODE_POWERDOWN,
+   [INA3221_MODE_ONESHOT] = INA3221_CONFIG_MODE_SHUNT |
+INA3221_CONFIG_MODE_BUS,
+   [INA3221_MODE_CONTINUOUS] = INA3221_CONFIG_MODE_CONTINUOUS |
+   INA3221_CONFIG_MODE_SHUNT |
+   INA3221_CONFIG_MODE_BUS,
+};
+
 /**
  * struct ina3221_input - channel input source specific information
  * @label: label of channel input source
@@ -386,9 +408,51 @@ static const struct hwmon_ops ina3221_hwmon_ops = {
.write = ina3221_write,
 };
 
+static int ina3221_mode_get_index(struct device *dev, unsigned int *index)
+{
+   struct ina3221_data *ina = dev_get_drvdata(dev);
+   u16 mode = ina->reg_config & INA3221_CONFIG_MODE_MASK;
+
+   if (mode == INA3221_CONFIG_MODE_POWERDOWN)
+   *index = INA3221_MODE_POWERDOWN;
+   if (mode & INA3221_CONFIG_MODE_CONTINUOUS)
+   *index = INA3221_MODE_CONTINUOUS;
+   else
+   *index = INA3221_MODE_ONESHOT;
+
+   return 0;
+}
+
+static int ina3221_mode_set_index(struct device *dev, unsigned int index)
+{
+   struct ina3221_data *ina = dev_get_drvdata(dev);
+   int ret;
+
+   ret = regmap_update_bits(ina->regmap, INA3221_CONFIG,
+INA3221_CONFIG_MODE_MASK,
+ina3221_mode_val[index]);
+   if (ret)
+   return ret;
+
+   /* Cache the latest config register value */
+   ret = regmap_read(ina->regmap, INA3221_CONFIG, >reg_config);
+   if (ret)
+   return ret;
+
+   return 0;
+}
+
+static const struct hwmon_mode ina3221_hwmon_mode = {
+   .names = ina3221_mode_names,
+   .list_size = INA3221_NUM_MODES,
+   .get_index = ina3221_mode_get_index,
+   .set_index = ina3221_mode_set_index,
+};
+
 static const struct hwmon_chip_info ina3221_chip_info = {
.ops = _hwmon_ops,
.info = ina3221_info,
+   .mode = _hwmon_mode,
 };
 
 /* Extra attribute groups */
-- 
2.17.1



[PATCH 1/2] hwmon: (core) Add hwmon_mode structure and mode sysfs node

2018-10-09 Thread Nicolin Chen
There are a few hwmon sensors support different operating modes,
for example, one-shot and continuous modes. So it's probably not
a bad idea to abstract a mode sysfs node as a common feature in
the hwmon core.

Right beside the hwmon device name, this patch adds a new sysfs
attribute named "mode" and "available_modes" for user to check
and configure the operating mode. For hwmon device drivers that
implemented the _with_info API, the change also adds an optional
hwmon_mode structure in hwmon_chip_info structure so that those
drivers can pass mode related information.

Signed-off-by: Nicolin Chen 
---
 Documentation/hwmon/sysfs-interface | 15 +
 drivers/hwmon/hwmon.c   | 87 ++---
 include/linux/hwmon.h   | 24 
 3 files changed, 119 insertions(+), 7 deletions(-)

diff --git a/Documentation/hwmon/sysfs-interface 
b/Documentation/hwmon/sysfs-interface
index 2b9e1005d88b..48d6ca6b9bd4 100644
--- a/Documentation/hwmon/sysfs-interface
+++ b/Documentation/hwmon/sysfs-interface
@@ -92,6 +92,21 @@ name The chip name.
I2C devices get this attribute created automatically.
RO
 
+available_modes The available operating modes of the chip.
+   This should be short, lowercase string, not containing
+   whitespace, or the wildcard character '*'.
+   This attribute shows all the available of the operating modes,
+   for example, "power-down" "one-shot" and "continuous".
+   RO
+
+mode   The current operating mode of the chip.
+   This should be short, lowercase string, not containing
+   whitespace, or the wildcard character '*'.
+   This attribute shows the current operating mode of the chip.
+   Writing a valid string from the list of available_modes will
+   configure the chip to the corresponding operating mode.
+   RW
+
 update_intervalThe interval at which the chip will update readings.
Unit: millisecond
RW
diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
index 975c95169884..5a33b616284b 100644
--- a/drivers/hwmon/hwmon.c
+++ b/drivers/hwmon/hwmon.c
@@ -72,25 +72,88 @@ name_show(struct device *dev, struct device_attribute 
*attr, char *buf)
 }
 static DEVICE_ATTR_RO(name);
 
+static ssize_t available_modes_show(struct device *dev,
+   struct device_attribute *attr, char *buf)
+{
+   struct hwmon_device *hwdev = to_hwmon_device(dev);
+   const struct hwmon_chip_info *chip = hwdev->chip;
+   const struct hwmon_mode *mode = chip->mode;
+   int i;
+
+   for (i = 0; i < mode->list_size; i++)
+   snprintf(buf, PAGE_SIZE, "%s%s ", buf, mode->names[i]);
+
+   return snprintf(buf, PAGE_SIZE, "%s\n", buf);
+}
+static DEVICE_ATTR_RO(available_modes);
+
+static ssize_t mode_show(struct device *dev,
+struct device_attribute *attr, char *buf)
+{
+   struct hwmon_device *hwdev = to_hwmon_device(dev);
+   const struct hwmon_chip_info *chip = hwdev->chip;
+   const struct hwmon_mode *mode = chip->mode;
+   unsigned int index;
+   int ret;
+
+   ret = mode->get_index(dev, );
+   if (ret)
+   return ret;
+
+   return snprintf(buf, PAGE_SIZE, "%s\n", mode->names[index]);
+}
+
+static ssize_t mode_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+   struct hwmon_device *hwdev = to_hwmon_device(dev);
+   const struct hwmon_chip_info *chip = hwdev->chip;
+   const struct hwmon_mode *mode = chip->mode;
+   const char **names = mode->names;
+   unsigned int index;
+   int ret;
+
+   /* Get the corresponding mode index */
+   for (index = 0; index < mode->list_size; index++) {
+   if (!strncmp(buf, names[index], strlen(names[index])))
+   break;
+   }
+
+   if (index >= mode->list_size)
+   return -EINVAL;
+
+   ret = mode->set_index(dev, index);
+   if (ret)
+   return ret;
+
+   return count;
+}
+static DEVICE_ATTR_RW(mode);
+
 static struct attribute *hwmon_dev_attrs[] = {
-   _attr_name.attr,
+   _attr_name.attr,/* index = 0 */
+   _attr_available_modes.attr, /* index = 1 */
+   _attr_mode.attr,/* index = 2 */
NULL
 };
 
-static umode_t hwmon_dev_name_is_visible(struct kobject *kobj,
-struct attribute *attr, int n)
+static umode_t hwmon_dev_is_visible(struct kobject *kobj,
+   struct attribute *attr, int n)
 {
struct device *dev = container_of(kobj, struct device, kobj);
+   struct hwmon_device *hwdev = to_hwmon_device(dev);
 
-   if (to_hwmon_device(dev)->name == NULL)
-   return 0;
+   if (n == 0 

[PATCH 0/2] hwmon: Add operating mode support for core and ina3221

2018-10-09 Thread Nicolin Chen
Add operating mode support in the core and ina3221.

Nicolin Chen (2):
  hwmon: (core) Add hwmon_mode structure and mode sysfs node
  hwmon: (ina3221) Add operating mode support

 Documentation/hwmon/sysfs-interface | 15 +
 drivers/hwmon/hwmon.c   | 87 ++---
 drivers/hwmon/ina3221.c | 64 +
 include/linux/hwmon.h   | 24 
 4 files changed, 183 insertions(+), 7 deletions(-)

-- 
2.17.1



Re: [PATCH] hwmon: (npcm-750-pwm-fan): Change initial pwm target to 255

2018-10-09 Thread Kun Yi
Great, thanks!

On Tue, Oct 9, 2018 at 5:38 PM Guenter Roeck  wrote:
>
> On Mon, Oct 08, 2018 at 02:49:25PM -0700, Kun Yi wrote:
> > 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 
>
> Makes sense to me, and in line with other drivers. Applied to hwmon-next.
>
> Thanks,
> Guenter
>
> > ---
> >  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_NUM  255
> > -#define NPCM7XX_PWM_CMR_DEFAULT_NUM  127
> > +#define NPCM7XX_PWM_CMR_DEFAULT_NUM  255
> >  #define NPCM7XX_PWM_CMR_MAX  255
> >
> >  /* default all PWM channels PRESCALE2 = 1 */



-- 
Regards,
Kun


Re: [PATCH v2] hwmon: (ina3221) Use _info API to register hwmon device

2018-10-09 Thread Guenter Roeck
On Mon, Oct 08, 2018 at 01:14:24PM -0700, Nicolin Chen wrote:
> 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:
>text  data bss dec hex filename
>5114  1712   068261aaa drivers/hwmon/ina3221_before.o
>4456   440   048961320 drivers/hwmon/ina3221.o
> 
> Signed-off-by: Nicolin Chen 

Applied to hwmon-next.

Thanks,
Guenter

> ---
> 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];

Re: [PATCH] hwmon: (npcm-750-pwm-fan): Change initial pwm target to 255

2018-10-09 Thread Guenter Roeck
On Mon, Oct 08, 2018 at 02:49:25PM -0700, Kun Yi wrote:
> 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 

Makes sense to me, and in line with other drivers. Applied to hwmon-next.

Thanks,
Guenter

> ---
>  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_NUM  255
> -#define NPCM7XX_PWM_CMR_DEFAULT_NUM  127
> +#define NPCM7XX_PWM_CMR_DEFAULT_NUM  255
>  #define NPCM7XX_PWM_CMR_MAX  255
>  
>  /* default all PWM channels PRESCALE2 = 1 */


Re: [PATCH v2] hwmon: (ina3221) Validate shunt resistor value from DT

2018-10-09 Thread Guenter Roeck
On Mon, Oct 08, 2018 at 02:24:51PM -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 validates the value.
> 
> Signed-off-by: Nicolin Chen 

Applied.

Thanks,
Guenter

> ---
> 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;
>  }


Re: [PATCH] hwmon: (tmp421) make const array 'names' static

2018-10-09 Thread Guenter Roeck
On Tue, Oct 09, 2018 at 01:11:57PM +0100, Colin King wrote:
> From: Colin Ian King 
> 
> The const array 'names' can be made static, saves populating it on
> the stack and will make it read-only.
> 
> Signed-off-by: Colin Ian King 

Applied to hwmon-next.

Thanks,
Guenter

> ---
>  drivers/hwmon/tmp421.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> index e36399213324..8844c9565d2a 100644
> --- a/drivers/hwmon/tmp421.c
> +++ b/drivers/hwmon/tmp421.c
> @@ -226,8 +226,10 @@ static int tmp421_detect(struct i2c_client *client,
>  {
>   enum chips kind;
>   struct i2c_adapter *adapter = client->adapter;
> - const char * const names[] = { "TMP421", "TMP422", "TMP423",
> -"TMP441", "TMP442" };
> + static const char * const names[] = {
> + "TMP421", "TMP422", "TMP423",
> + "TMP441", "TMP442"
> + };
>   int addr = client->addr;
>   u8 reg;
>  


[PATCH v2] hwmon: (core) Add trace events to _attr_show/store functions

2018-10-09 Thread Nicolin Chen
Trace events are useful for people who collect data from the
Ftrace outputs. There're people who analyse the relationship
of cpufreq, thermal and hwmon (power/voltage/current) using
the convenient and timestamped Ftrace outputs, while unlike
cpufreq and thermal subsystems the hwmon does not have trace
events supported yet.

So this patch adds initial trace events for the hwmon core.
To call hwmon_attr_base() for aligned attr index numbers, it
also moves the function upward.

Ftrace outputs:
 ...: hwmon_attr_show_string: index=2, attr_name=in2_label, val=VDD_5V
 ...: hwmon_attr_show: index=2, attr_name=in2_input, val=5112
 ...: hwmon_attr_show: index=2, attr_name=curr2_input, val=440

Note that the _attr_show and _attr_store functions are tied
to the _with_info API. So a hwmon driver requiring the trace
events feature should use _with_info API to register a hwmon
device.

Signed-off-by: Nicolin Chen 
---
Changelog
v1->v2:
 * Added a descriptive reason for this change in the commit message

 MAINTAINERS  |  1 +
 drivers/hwmon/hwmon.c| 27 ++
 include/trace/events/hwmon.h | 71 
 3 files changed, 92 insertions(+), 7 deletions(-)
 create mode 100644 include/trace/events/hwmon.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 1640b9faa75e..589b32405bf4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6461,6 +6461,7 @@ F:Documentation/devicetree/bindings/hwmon/
 F: Documentation/hwmon/
 F: drivers/hwmon/
 F: include/linux/hwmon*.h
+F: include/trace/events/hwmon*.h
 
 HARDWARE RANDOM NUMBER GENERATOR CORE
 M: Matt Mackall 
diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
index ac1cdf88840f..975c95169884 100644
--- a/drivers/hwmon/hwmon.c
+++ b/drivers/hwmon/hwmon.c
@@ -24,6 +24,9 @@
 #include 
 #include 
 
+#define CREATE_TRACE_POINTS
+#include 
+
 #define HWMON_ID_PREFIX "hwmon"
 #define HWMON_ID_FORMAT HWMON_ID_PREFIX "%d"
 
@@ -171,6 +174,13 @@ static int hwmon_thermal_add_sensor(struct device *dev,
 }
 #endif /* IS_REACHABLE(CONFIG_THERMAL) && ... */
 
+static int hwmon_attr_base(enum hwmon_sensor_types type)
+{
+   if (type == hwmon_in)
+   return 0;
+   return 1;
+}
+
 /* sysfs attribute management */
 
 static ssize_t hwmon_attr_show(struct device *dev,
@@ -185,6 +195,9 @@ static ssize_t hwmon_attr_show(struct device *dev,
if (ret < 0)
return ret;
 
+   trace_hwmon_attr_show(hattr->index + hwmon_attr_base(hattr->type),
+ hattr->name, val);
+
return sprintf(buf, "%ld\n", val);
 }
 
@@ -193,6 +206,7 @@ static ssize_t hwmon_attr_show_string(struct device *dev,
  char *buf)
 {
struct hwmon_device_attribute *hattr = to_hwmon_attr(devattr);
+   enum hwmon_sensor_types type = hattr->type;
const char *s;
int ret;
 
@@ -201,6 +215,9 @@ static ssize_t hwmon_attr_show_string(struct device *dev,
if (ret < 0)
return ret;
 
+   trace_hwmon_attr_show_string(hattr->index + hwmon_attr_base(type),
+hattr->name, s);
+
return sprintf(buf, "%s\n", s);
 }
 
@@ -221,16 +238,12 @@ static ssize_t hwmon_attr_store(struct device *dev,
if (ret < 0)
return ret;
 
+   trace_hwmon_attr_store(hattr->index + hwmon_attr_base(hattr->type),
+  hattr->name, val);
+
return count;
 }
 
-static int hwmon_attr_base(enum hwmon_sensor_types type)
-{
-   if (type == hwmon_in)
-   return 0;
-   return 1;
-}
-
 static bool is_string_attr(enum hwmon_sensor_types type, u32 attr)
 {
return (type == hwmon_temp && attr == hwmon_temp_label) ||
diff --git a/include/trace/events/hwmon.h b/include/trace/events/hwmon.h
new file mode 100644
index ..d7a1d0ffb679
--- /dev/null
+++ b/include/trace/events/hwmon.h
@@ -0,0 +1,71 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM hwmon
+
+#if !defined(_TRACE_HWMON_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_HWMON_H
+
+#include 
+
+DECLARE_EVENT_CLASS(hwmon_attr_class,
+
+   TP_PROTO(int index, const char *attr_name, long val),
+
+   TP_ARGS(index, attr_name, val),
+
+   TP_STRUCT__entry(
+   __field(int, index)
+   __string(attr_name, attr_name)
+   __field(long, val)
+   ),
+
+   TP_fast_assign(
+   __entry->index = index;
+   __assign_str(attr_name, attr_name);
+   __entry->val = val;
+   ),
+
+   TP_printk("index=%d, attr_name=%s, val=%ld",
+ __entry->index,  __get_str(attr_name), __entry->val)
+);
+
+DEFINE_EVENT(hwmon_attr_class, hwmon_attr_show,
+
+   TP_PROTO(int index, const char *attr_name, long val),
+
+   TP_ARGS(index, attr_name, val)
+);
+
+DEFINE_EVENT(hwmon_attr_class, hwmon_attr_store,
+
+   TP_PROTO(int index, 

Re: [PATCH] hwmon: (core) Add trace events to _attr_show/store functions

2018-10-09 Thread Guenter Roeck
On Tue, Oct 09, 2018 at 04:42:42PM -0400, Steven Rostedt wrote:
> On Tue, 9 Oct 2018 13:39:37 -0700
> Guenter Roeck  wrote:
> 
> > > Personally a checkpatch warning bugs me more than having an extra
> > > line :)
> > >   
> > Same here. If we no longer believe in the 80-column limit, we should remove 
> > it,
> > not use it to hide other problems in the noise.
> 
> Yes, please, can we?
> 
> I personally hate the 80 character limit rule, because I like
> descriptive variables and function names, which itself causes the 80
> character limit to be broken. I find line breaks to avoid that limit
> just makes the code look worse. Or at least up it to 100 chars.
> 

It is a two-edged sword. Downside is that a longer limit also invites
deeply nested loops and conditionals, which I am sure will be used as
argument against raising it.

Feel free to submit a patch to raise the limit to 100; I'll be more
than happy to give it an enthusiastic Reviewed-by:. My personal desire
for conflict isn't strong enough to submit it myself, though.

Guenter


Re: [PATCH] hwmon: (core) Add trace events to _attr_show/store functions

2018-10-09 Thread Steven Rostedt
On Tue, 9 Oct 2018 13:39:37 -0700
Guenter Roeck  wrote:

> > Personally a checkpatch warning bugs me more than having an extra
> > line :)
> >   
> Same here. If we no longer believe in the 80-column limit, we should remove 
> it,
> not use it to hide other problems in the noise.

Note, this is also the main reason why I don't even bother using
checkpatch.

-- Steve


Re: [PATCH] hwmon: (core) Add trace events to _attr_show/store functions

2018-10-09 Thread Steven Rostedt
On Tue, 9 Oct 2018 13:39:37 -0700
Guenter Roeck  wrote:

> > Personally a checkpatch warning bugs me more than having an extra
> > line :)
> >   
> Same here. If we no longer believe in the 80-column limit, we should remove 
> it,
> not use it to hide other problems in the noise.

Yes, please, can we?

I personally hate the 80 character limit rule, because I like
descriptive variables and function names, which itself causes the 80
character limit to be broken. I find line breaks to avoid that limit
just makes the code look worse. Or at least up it to 100 chars.

-- Steve


Re: [PATCH] hwmon: (core) Add trace events to _attr_show/store functions

2018-10-09 Thread Guenter Roeck
On Tue, Oct 09, 2018 at 12:57:43PM -0700, Nicolin Chen wrote:
> On Tue, Oct 09, 2018 at 03:49:45PM -0400, Steven Rostedt wrote:
> > > > > Trace events are useful for people who collect data from the
> > > > > ftrace output. This patch adds initial trace events for the
> > > > > hwmon core. To call hwmon_attr_base() for aligned attr index
> > > > > numbers, this patch also moves the function upward.
> > > > > 
> > > > > Ftrace outputs:
> > > > >  ...: hwmon_attr_show_string: index=2, attr_name=in2_label, val=VDD_5V
> > > > >  ...: hwmon_attr_show: index=2, attr_name=in2_input, val=5112
> > > > >  ...: hwmon_attr_show: index=2, attr_name=curr2_input, val=440
> > > > > 
> > > > > Note that the _attr_show and _attr_store functions are tied
> > > > > to the _with_info API. So a hwmon driver requiring the trace
> > > > > events feature should use _with_info API to register a hwmon
> > > > > device.  
> > > > 
> > > > Hmm, this doesn't really explain why these trace events are needed.
> > > > They look to be attached to sysfs reads. What's the purpose of tracing
> > > > these?  
> > > 
> > > Our power folks analyse Ftrace outputs of cpufreq, thermal and
> > > hwmon (power/voltage/current) so as to get the relationship of
> > > them. The reason why we specifically need trace events is that
> > > it's convenient and timestamped, and because both cpufreq and
> > > thermal already have trace events.
> > > 
> > > I could add this to make the commit message more convincing if
> > > you'd prefer that.
> > 
> > I'm not against the patch, but yes, having this in the change log is
> > helpful.
> 
> I will add it in v2.
> 
> > > > > @@ -193,6 +206,7 @@ static ssize_t hwmon_attr_show_string(struct 
> > > > > device *dev,
> > > > > char *buf)
> > > > >  {
> > > > >   struct hwmon_device_attribute *hattr = to_hwmon_attr(devattr);
> > > > > + enum hwmon_sensor_types type = hattr->type;
> > > > >   const char *s;
> > > > >   int ret;
> > > > >  
> > > > > @@ -201,6 +215,9 @@ static ssize_t hwmon_attr_show_string(struct 
> > > > > device *dev,
> > > > >   if (ret < 0)
> > > > >   return ret;
> > > > >  
> > > > > + trace_hwmon_attr_show_string(hattr->index + 
> > > > > hwmon_attr_base(type),  
> > > > 
> > > > Also, the other to tracepoints use hattr->type, here you create a
> > > > separate variable. Is that just to keep within the 80 char limit?  
> > > 
> > > Yes. It looks clearer to me than wrapping the line here, since
> > > complier usually optimize the local variable away.
> > 
> > Or you can just break the 80 character limit ;-)
> 
> Personally a checkpatch warning bugs me more than having an extra
> line :)
> 
Same here. If we no longer believe in the 80-column limit, we should remove it,
not use it to hide other problems in the noise.

Thanks,
Guenter


Re: [PATCH] hwmon: (core) Add trace events to _attr_show/store functions

2018-10-09 Thread Nicolin Chen
On Tue, Oct 09, 2018 at 03:49:45PM -0400, Steven Rostedt wrote:
> > > > Trace events are useful for people who collect data from the
> > > > ftrace output. This patch adds initial trace events for the
> > > > hwmon core. To call hwmon_attr_base() for aligned attr index
> > > > numbers, this patch also moves the function upward.
> > > > 
> > > > Ftrace outputs:
> > > >  ...: hwmon_attr_show_string: index=2, attr_name=in2_label, val=VDD_5V
> > > >  ...: hwmon_attr_show: index=2, attr_name=in2_input, val=5112
> > > >  ...: hwmon_attr_show: index=2, attr_name=curr2_input, val=440
> > > > 
> > > > Note that the _attr_show and _attr_store functions are tied
> > > > to the _with_info API. So a hwmon driver requiring the trace
> > > > events feature should use _with_info API to register a hwmon
> > > > device.  
> > > 
> > > Hmm, this doesn't really explain why these trace events are needed.
> > > They look to be attached to sysfs reads. What's the purpose of tracing
> > > these?  
> > 
> > Our power folks analyse Ftrace outputs of cpufreq, thermal and
> > hwmon (power/voltage/current) so as to get the relationship of
> > them. The reason why we specifically need trace events is that
> > it's convenient and timestamped, and because both cpufreq and
> > thermal already have trace events.
> > 
> > I could add this to make the commit message more convincing if
> > you'd prefer that.
> 
> I'm not against the patch, but yes, having this in the change log is
> helpful.

I will add it in v2.

> > > > @@ -193,6 +206,7 @@ static ssize_t hwmon_attr_show_string(struct device 
> > > > *dev,
> > > >   char *buf)
> > > >  {
> > > > struct hwmon_device_attribute *hattr = to_hwmon_attr(devattr);
> > > > +   enum hwmon_sensor_types type = hattr->type;
> > > > const char *s;
> > > > int ret;
> > > >  
> > > > @@ -201,6 +215,9 @@ static ssize_t hwmon_attr_show_string(struct device 
> > > > *dev,
> > > > if (ret < 0)
> > > > return ret;
> > > >  
> > > > +   trace_hwmon_attr_show_string(hattr->index + 
> > > > hwmon_attr_base(type),  
> > > 
> > > Also, the other to tracepoints use hattr->type, here you create a
> > > separate variable. Is that just to keep within the 80 char limit?  
> > 
> > Yes. It looks clearer to me than wrapping the line here, since
> > complier usually optimize the local variable away.
> 
> Or you can just break the 80 character limit ;-)

Personally a checkpatch warning bugs me more than having an extra
line :)

Thank you
Nicolin


Re: [PATCH] hwmon: (core) Add trace events to _attr_show/store functions

2018-10-09 Thread Steven Rostedt
On Tue, 9 Oct 2018 12:11:11 -0700
Nicolin Chen  wrote:

> On Tue, Oct 09, 2018 at 02:57:21PM -0400, Steven Rostedt wrote:
> 
> > > Trace events are useful for people who collect data from the
> > > ftrace output. This patch adds initial trace events for the
> > > hwmon core. To call hwmon_attr_base() for aligned attr index
> > > numbers, this patch also moves the function upward.
> > > 
> > > Ftrace outputs:
> > >  ...: hwmon_attr_show_string: index=2, attr_name=in2_label, val=VDD_5V
> > >  ...: hwmon_attr_show: index=2, attr_name=in2_input, val=5112
> > >  ...: hwmon_attr_show: index=2, attr_name=curr2_input, val=440
> > > 
> > > Note that the _attr_show and _attr_store functions are tied
> > > to the _with_info API. So a hwmon driver requiring the trace
> > > events feature should use _with_info API to register a hwmon
> > > device.  
> > 
> > Hmm, this doesn't really explain why these trace events are needed.
> > They look to be attached to sysfs reads. What's the purpose of tracing
> > these?  
> 
> Our power folks analyse Ftrace outputs of cpufreq, thermal and
> hwmon (power/voltage/current) so as to get the relationship of
> them. The reason why we specifically need trace events is that
> it's convenient and timestamped, and because both cpufreq and
> thermal already have trace events.
> 
> I could add this to make the commit message more convincing if
> you'd prefer that.

I'm not against the patch, but yes, having this in the change log is
helpful.


> 
> > > @@ -193,6 +206,7 @@ static ssize_t hwmon_attr_show_string(struct device 
> > > *dev,
> > > char *buf)
> > >  {
> > >   struct hwmon_device_attribute *hattr = to_hwmon_attr(devattr);
> > > + enum hwmon_sensor_types type = hattr->type;
> > >   const char *s;
> > >   int ret;
> > >  
> > > @@ -201,6 +215,9 @@ static ssize_t hwmon_attr_show_string(struct device 
> > > *dev,
> > >   if (ret < 0)
> > >   return ret;
> > >  
> > > + trace_hwmon_attr_show_string(hattr->index + hwmon_attr_base(type),  
> > 
> > Also, the other to tracepoints use hattr->type, here you create a
> > separate variable. Is that just to keep within the 80 char limit?  
> 
> Yes. It looks clearer to me than wrapping the line here, since
> complier usually optimize the local variable away.
> 

Or you can just break the 80 character limit ;-)

-- Steve


Re: [PATCH] hwmon: (core) Add trace events to _attr_show/store functions

2018-10-09 Thread Nicolin Chen
On Tue, Oct 09, 2018 at 02:57:21PM -0400, Steven Rostedt wrote:

> > Trace events are useful for people who collect data from the
> > ftrace output. This patch adds initial trace events for the
> > hwmon core. To call hwmon_attr_base() for aligned attr index
> > numbers, this patch also moves the function upward.
> > 
> > Ftrace outputs:
> >  ...: hwmon_attr_show_string: index=2, attr_name=in2_label, val=VDD_5V
> >  ...: hwmon_attr_show: index=2, attr_name=in2_input, val=5112
> >  ...: hwmon_attr_show: index=2, attr_name=curr2_input, val=440
> > 
> > Note that the _attr_show and _attr_store functions are tied
> > to the _with_info API. So a hwmon driver requiring the trace
> > events feature should use _with_info API to register a hwmon
> > device.
> 
> Hmm, this doesn't really explain why these trace events are needed.
> They look to be attached to sysfs reads. What's the purpose of tracing
> these?

Our power folks analyse Ftrace outputs of cpufreq, thermal and
hwmon (power/voltage/current) so as to get the relationship of
them. The reason why we specifically need trace events is that
it's convenient and timestamped, and because both cpufreq and
thermal already have trace events.

I could add this to make the commit message more convincing if
you'd prefer that.

> > @@ -193,6 +206,7 @@ static ssize_t hwmon_attr_show_string(struct device 
> > *dev,
> >   char *buf)
> >  {
> > struct hwmon_device_attribute *hattr = to_hwmon_attr(devattr);
> > +   enum hwmon_sensor_types type = hattr->type;
> > const char *s;
> > int ret;
> >  
> > @@ -201,6 +215,9 @@ static ssize_t hwmon_attr_show_string(struct device 
> > *dev,
> > if (ret < 0)
> > return ret;
> >  
> > +   trace_hwmon_attr_show_string(hattr->index + hwmon_attr_base(type),
> 
> Also, the other to tracepoints use hattr->type, here you create a
> separate variable. Is that just to keep within the 80 char limit?

Yes. It looks clearer to me than wrapping the line here, since
complier usually optimize the local variable away.

Thanks
Nicolin


Re: [PATCH] hwmon: (core) Add trace events to _attr_show/store functions

2018-10-09 Thread Steven Rostedt
On Tue,  9 Oct 2018 11:26:02 -0700
Nicolin Chen  wrote:

> Trace events are useful for people who collect data from the
> ftrace output. This patch adds initial trace events for the
> hwmon core. To call hwmon_attr_base() for aligned attr index
> numbers, this patch also moves the function upward.
> 
> Ftrace outputs:
>  ...: hwmon_attr_show_string: index=2, attr_name=in2_label, val=VDD_5V
>  ...: hwmon_attr_show: index=2, attr_name=in2_input, val=5112
>  ...: hwmon_attr_show: index=2, attr_name=curr2_input, val=440
> 
> Note that the _attr_show and _attr_store functions are tied
> to the _with_info API. So a hwmon driver requiring the trace
> events feature should use _with_info API to register a hwmon
> device.

Hmm, this doesn't really explain why these trace events are needed.
They look to be attached to sysfs reads. What's the purpose of tracing
these?

> 
> Signed-off-by: Nicolin Chen 
> ---
>  MAINTAINERS  |  1 +
>  drivers/hwmon/hwmon.c| 27 ++
>  include/trace/events/hwmon.h | 71 
>  3 files changed, 92 insertions(+), 7 deletions(-)
>  create mode 100644 include/trace/events/hwmon.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1640b9faa75e..589b32405bf4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6461,6 +6461,7 @@ F:  Documentation/devicetree/bindings/hwmon/
>  F:   Documentation/hwmon/
>  F:   drivers/hwmon/
>  F:   include/linux/hwmon*.h
> +F:   include/trace/events/hwmon*.h
>  
>  HARDWARE RANDOM NUMBER GENERATOR CORE
>  M:   Matt Mackall 
> diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
> index ac1cdf88840f..975c95169884 100644
> --- a/drivers/hwmon/hwmon.c
> +++ b/drivers/hwmon/hwmon.c
> @@ -24,6 +24,9 @@
>  #include 
>  #include 
>  
> +#define CREATE_TRACE_POINTS
> +#include 
> +
>  #define HWMON_ID_PREFIX "hwmon"
>  #define HWMON_ID_FORMAT HWMON_ID_PREFIX "%d"
>  
> @@ -171,6 +174,13 @@ static int hwmon_thermal_add_sensor(struct device *dev,
>  }
>  #endif /* IS_REACHABLE(CONFIG_THERMAL) && ... */
>  
> +static int hwmon_attr_base(enum hwmon_sensor_types type)
> +{
> + if (type == hwmon_in)
> + return 0;
> + return 1;
> +}
> +
>  /* sysfs attribute management */
>  
>  static ssize_t hwmon_attr_show(struct device *dev,
> @@ -185,6 +195,9 @@ static ssize_t hwmon_attr_show(struct device *dev,
>   if (ret < 0)
>   return ret;
>  
> + trace_hwmon_attr_show(hattr->index + hwmon_attr_base(hattr->type),
> +   hattr->name, val);
> +
>   return sprintf(buf, "%ld\n", val);
>  }
>  
> @@ -193,6 +206,7 @@ static ssize_t hwmon_attr_show_string(struct device *dev,
> char *buf)
>  {
>   struct hwmon_device_attribute *hattr = to_hwmon_attr(devattr);
> + enum hwmon_sensor_types type = hattr->type;
>   const char *s;
>   int ret;
>  
> @@ -201,6 +215,9 @@ static ssize_t hwmon_attr_show_string(struct device *dev,
>   if (ret < 0)
>   return ret;
>  
> + trace_hwmon_attr_show_string(hattr->index + hwmon_attr_base(type),

Also, the other to tracepoints use hattr->type, here you create a
separate variable. Is that just to keep within the 80 char limit?

-- Steve

> +  hattr->name, s);
> +
>   return sprintf(buf, "%s\n", s);
>  }
>  
> @@ -221,16 +238,12 @@ static ssize_t hwmon_attr_store(struct device *dev,
>   if (ret < 0)
>   return ret;
>  
> + trace_hwmon_attr_store(hattr->index + hwmon_attr_base(hattr->type),
> +hattr->name, val);
> +
>   return count;
>  }
>  
> -static int hwmon_attr_base(enum hwmon_sensor_types type)
> -{
> - if (type == hwmon_in)
> - return 0;
> - return 1;
> -}
> -
>  static bool is_string_attr(enum hwmon_sensor_types type, u32 attr)
>  {
>   return (type == hwmon_temp && attr == hwmon_temp_label) ||



[PATCH] hwmon: (core) Add trace events to _attr_show/store functions

2018-10-09 Thread Nicolin Chen
Trace events are useful for people who collect data from the
ftrace output. This patch adds initial trace events for the
hwmon core. To call hwmon_attr_base() for aligned attr index
numbers, this patch also moves the function upward.

Ftrace outputs:
 ...: hwmon_attr_show_string: index=2, attr_name=in2_label, val=VDD_5V
 ...: hwmon_attr_show: index=2, attr_name=in2_input, val=5112
 ...: hwmon_attr_show: index=2, attr_name=curr2_input, val=440

Note that the _attr_show and _attr_store functions are tied
to the _with_info API. So a hwmon driver requiring the trace
events feature should use _with_info API to register a hwmon
device.

Signed-off-by: Nicolin Chen 
---
 MAINTAINERS  |  1 +
 drivers/hwmon/hwmon.c| 27 ++
 include/trace/events/hwmon.h | 71 
 3 files changed, 92 insertions(+), 7 deletions(-)
 create mode 100644 include/trace/events/hwmon.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 1640b9faa75e..589b32405bf4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6461,6 +6461,7 @@ F:Documentation/devicetree/bindings/hwmon/
 F: Documentation/hwmon/
 F: drivers/hwmon/
 F: include/linux/hwmon*.h
+F: include/trace/events/hwmon*.h
 
 HARDWARE RANDOM NUMBER GENERATOR CORE
 M: Matt Mackall 
diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
index ac1cdf88840f..975c95169884 100644
--- a/drivers/hwmon/hwmon.c
+++ b/drivers/hwmon/hwmon.c
@@ -24,6 +24,9 @@
 #include 
 #include 
 
+#define CREATE_TRACE_POINTS
+#include 
+
 #define HWMON_ID_PREFIX "hwmon"
 #define HWMON_ID_FORMAT HWMON_ID_PREFIX "%d"
 
@@ -171,6 +174,13 @@ static int hwmon_thermal_add_sensor(struct device *dev,
 }
 #endif /* IS_REACHABLE(CONFIG_THERMAL) && ... */
 
+static int hwmon_attr_base(enum hwmon_sensor_types type)
+{
+   if (type == hwmon_in)
+   return 0;
+   return 1;
+}
+
 /* sysfs attribute management */
 
 static ssize_t hwmon_attr_show(struct device *dev,
@@ -185,6 +195,9 @@ static ssize_t hwmon_attr_show(struct device *dev,
if (ret < 0)
return ret;
 
+   trace_hwmon_attr_show(hattr->index + hwmon_attr_base(hattr->type),
+ hattr->name, val);
+
return sprintf(buf, "%ld\n", val);
 }
 
@@ -193,6 +206,7 @@ static ssize_t hwmon_attr_show_string(struct device *dev,
  char *buf)
 {
struct hwmon_device_attribute *hattr = to_hwmon_attr(devattr);
+   enum hwmon_sensor_types type = hattr->type;
const char *s;
int ret;
 
@@ -201,6 +215,9 @@ static ssize_t hwmon_attr_show_string(struct device *dev,
if (ret < 0)
return ret;
 
+   trace_hwmon_attr_show_string(hattr->index + hwmon_attr_base(type),
+hattr->name, s);
+
return sprintf(buf, "%s\n", s);
 }
 
@@ -221,16 +238,12 @@ static ssize_t hwmon_attr_store(struct device *dev,
if (ret < 0)
return ret;
 
+   trace_hwmon_attr_store(hattr->index + hwmon_attr_base(hattr->type),
+  hattr->name, val);
+
return count;
 }
 
-static int hwmon_attr_base(enum hwmon_sensor_types type)
-{
-   if (type == hwmon_in)
-   return 0;
-   return 1;
-}
-
 static bool is_string_attr(enum hwmon_sensor_types type, u32 attr)
 {
return (type == hwmon_temp && attr == hwmon_temp_label) ||
diff --git a/include/trace/events/hwmon.h b/include/trace/events/hwmon.h
new file mode 100644
index ..d7a1d0ffb679
--- /dev/null
+++ b/include/trace/events/hwmon.h
@@ -0,0 +1,71 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM hwmon
+
+#if !defined(_TRACE_HWMON_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_HWMON_H
+
+#include 
+
+DECLARE_EVENT_CLASS(hwmon_attr_class,
+
+   TP_PROTO(int index, const char *attr_name, long val),
+
+   TP_ARGS(index, attr_name, val),
+
+   TP_STRUCT__entry(
+   __field(int, index)
+   __string(attr_name, attr_name)
+   __field(long, val)
+   ),
+
+   TP_fast_assign(
+   __entry->index = index;
+   __assign_str(attr_name, attr_name);
+   __entry->val = val;
+   ),
+
+   TP_printk("index=%d, attr_name=%s, val=%ld",
+ __entry->index,  __get_str(attr_name), __entry->val)
+);
+
+DEFINE_EVENT(hwmon_attr_class, hwmon_attr_show,
+
+   TP_PROTO(int index, const char *attr_name, long val),
+
+   TP_ARGS(index, attr_name, val)
+);
+
+DEFINE_EVENT(hwmon_attr_class, hwmon_attr_store,
+
+   TP_PROTO(int index, const char *attr_name, long val),
+
+   TP_ARGS(index, attr_name, val)
+);
+
+TRACE_EVENT(hwmon_attr_show_string,
+
+   TP_PROTO(int index, const char *attr_name, const char *s),
+
+   TP_ARGS(index, attr_name, s),
+
+   TP_STRUCT__entry(
+   __field(int, index)
+   __string(attr_name, 

[PATCH] hwmon: (tmp421) make const array 'names' static

2018-10-09 Thread Colin King
From: Colin Ian King 

The const array 'names' can be made static, saves populating it on
the stack and will make it read-only.

Signed-off-by: Colin Ian King 
---
 drivers/hwmon/tmp421.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
index e36399213324..8844c9565d2a 100644
--- a/drivers/hwmon/tmp421.c
+++ b/drivers/hwmon/tmp421.c
@@ -226,8 +226,10 @@ static int tmp421_detect(struct i2c_client *client,
 {
enum chips kind;
struct i2c_adapter *adapter = client->adapter;
-   const char * const names[] = { "TMP421", "TMP422", "TMP423",
-  "TMP441", "TMP442" };
+   static const char * const names[] = {
+   "TMP421", "TMP422", "TMP423",
+   "TMP441", "TMP442"
+   };
int addr = client->addr;
u8 reg;
 
-- 
2.17.1



[hwmon:watchdog-next 23/28] ERROR: "__udivdi3" [drivers/watchdog/armada_37xx_wdt.ko] undefined!

2018-10-09 Thread kbuild test robot
tree:   
https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git 
watchdog-next
head:   8de959ed4898d8f270fd990cbc551619799100ef
commit: 6f935e7eff6f4fd9da9d54f46901b19ffc15df27 [23/28] watchdog: Add support 
for Armada 37xx CPU watchdog
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
git checkout 6f935e7eff6f4fd9da9d54f46901b19ffc15df27
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

>> ERROR: "__udivdi3" [drivers/watchdog/armada_37xx_wdt.ko] undefined!

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip