date:20170421

Re: [PATCH] hwmon (pmbus): Add client driver for IR35221

2017-04-21 Thread Patrick Williams

Sam,

On Fri, Apr 21, 2017 at 02:25:05PM +1000, Samuel Mendoza-Jonas wrote:
> IR35221 is a Digital DC-DC Multiphase Converter
> 
> Signed-off-by: Samuel Mendoza-Jonas 
> ---
> - Tested on a ppc64 system which includes several of these devices.
> - This patch re-implements the linear reg2data/data2reg functions from
>   pmbus-core like some other drivers in order to scale some results. Is
>   this something that would be better off being made generic for pmbus
>   drivers to call?
> - The resolution of iout0 is apparently configurable between two values,
>   however the documentation I have access to does not specify how this is
>   actually configured - currently it is left at the default resolution in
>   the driver.
> 
>  Documentation/hwmon/ir35221   |  87 
>  drivers/hwmon/pmbus/Kconfig   |  11 ++
>  drivers/hwmon/pmbus/Makefile  |   1 +
>  drivers/hwmon/pmbus/ir35221.c | 322 
> ++
>  4 files changed, 421 insertions(+)
>  create mode 100644 Documentation/hwmon/ir35221
>  create mode 100644 drivers/hwmon/pmbus/ir35221.c

Thanks for working on this.

Would it be possible to enhance it in a follow up to support an
'inN_target' also?  I see this is not documented by the sysfs-interface
for hwmon but we likely need support for this.  There are voltage
slewing operations we do in system manufacturing.  Without an hwmon
interface for programming the target voltage we're going to have to
unbind the device and manually do the i2c operations.

Guenter,

I mention 'inN_target' to match 'fanN_target'.  Do you have
any opposition to either formally adding that as an option for voltage
readings or adding it to this specific driver?

-- 
Patrick Williams

signature.asc
Description: Digital signature

Re: [PATCH] hwmon: (ibmpowernv) Add min/max attributes and current sensors

2017-04-21 Thread Cédric Le Goater

Hello Shilpasri,

On 04/21/2017 06:31 AM, Shilpasri G Bhat wrote:
> Add support for adding min/max values for the inband sensors copied by
> OCC to main memory. And also add current(mA) sensors to the list.
> 
> Signed-off-by: Shilpasri G Bhat 
> ---
>  drivers/hwmon/ibmpowernv.c | 55 
> --
>  1 file changed, 53 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwmon/ibmpowernv.c b/drivers/hwmon/ibmpowernv.c
> index 6d2e660..1f329fa8 100644
> --- a/drivers/hwmon/ibmpowernv.c
> +++ b/drivers/hwmon/ibmpowernv.c
> @@ -50,6 +50,7 @@ enum sensors {
>   TEMP,
>   POWER_SUPPLY,
>   POWER_INPUT,
> + CURRENT,
>   MAX_SENSOR_TYPE,
>  };
>  
> @@ -65,7 +66,8 @@ enum sensors {
>   {"fan", "ibm,opal-sensor-cooling-fan"},
>   {"temp", "ibm,opal-sensor-amb-temp"},
>   {"in", "ibm,opal-sensor-power-supply"},
> - {"power", "ibm,opal-sensor-power"}
> + {"power", "ibm,opal-sensor-power"},
> + {"curr"}, /* Follows newer device tree compatible ibm,opal-sensor */

why isn't there a compatible string ? 

>  };
>  
>  struct sensor_data {
> @@ -287,6 +289,7 @@ static int populate_attr_groups(struct platform_device 
> *pdev)
>   opal = of_find_node_by_path("/ibm,opal/sensors");
>   for_each_child_of_node(opal, np) {
>   const char *label;
> + int len;
>  
>   if (np->name == NULL)
>   continue;
> @@ -298,10 +301,14 @@ static int populate_attr_groups(struct platform_device 
> *pdev)
>   sensor_groups[type].attr_count++;
>  
>   /*
> -  * add a new attribute for labels
> +  * add attributes for labels, min and max
>*/
>   if (!of_property_read_string(np, "label", ))
>   sensor_groups[type].attr_count++;

We are negating of_property_read_string() above, but not below.
I wonder why ? 
 
> + if (of_find_property(np, "sensor-data-min", ))
> + sensor_groups[type].attr_count++;
> + if (of_find_property(np, "sensor-data-max", ))
> + sensor_groups[type].attr_count++;
>   }
>  
>   of_node_put(opal);
> @@ -428,6 +435,50 @@ static int create_device_attrs(struct platform_device 
> *pdev)
>   pgroups[type]->attrs[sensor_groups[type].attr_count++] =
>   [count++].dev_attr.attr;
>   }
> +
> + if (!of_property_read_u32(np, "sensor-data-max", _id)) {
> + switch (type) {
> + case POWER_INPUT:
> + attr_name = "input_highest";
> + break;
> + case TEMP:
> + attr_name = "max";
> + break;
> + default:
> + attr_name = "highest";
> + break;
> + }

May be we could use a converting routine here ? create_device_attrs()
is getting big.

> + sdata[count].id = sensor_id;
> + sdata[count].type = type;
> + sdata[count].hwmon_index = sdata[count - 1].hwmon_index;
> + create_hwmon_attr([count], attr_name,
> +   show_sensor);
> + pgroups[type]->attrs[sensor_groups[type].attr_count++] =
> + [count++].dev_attr.attr;
> + }
> +
> + if (!of_property_read_u32(np, "sensor-data-min", _id)) {
> + switch (type) {
> + case POWER_INPUT:
> + attr_name = "input_lowest";
> + break;
> + case TEMP:
> + attr_name = "min";
> + break;
> + default:
> + attr_name = "lowest";
> + break;
> + }

same here.

Thanks,

C.
> + sdata[count].id = sensor_id;
> + sdata[count].type = type;
> + sdata[count].hwmon_index = sdata[count - 1].hwmon_index;
> + create_hwmon_attr([count], attr_name,
> +   show_sensor);
> + pgroups[type]->attrs[sensor_groups[type].attr_count++] =
> + [count++].dev_attr.attr;
> + }
>   }
>  
>  exit_put_node:
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: hwmon: ina209: Handled signed registers

2017-04-21 Thread Guenter Roeck

On Tue, Apr 18, 2017 at 07:15:50AM -0500, Joe Schaack wrote:
> The shunt voltage and current registers are signed 16-bit values so
> handle them as such.
> 
> Signed-off-by: Joe Schaack 
> Reviewed-by: Aaron Sierra 

Applied.

Thanks,
Guenter

> ---
>  drivers/hwmon/ina209.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwmon/ina209.c b/drivers/hwmon/ina209.c
> index 5378fde..9fa975d 100644
> --- a/drivers/hwmon/ina209.c
> +++ b/drivers/hwmon/ina209.c
> @@ -117,7 +117,7 @@ static long ina209_from_reg(const u8 reg, const u16 val)
>   case INA209_SHUNT_VOLTAGE_POS_WARN:
>   case INA209_SHUNT_VOLTAGE_NEG_WARN:
>   /* LSB=10 uV. Convert to mV. */
> - return DIV_ROUND_CLOSEST(val, 100);
> + return DIV_ROUND_CLOSEST((s16)val, 100);
>  
>   case INA209_BUS_VOLTAGE:
>   case INA209_BUS_VOLTAGE_MAX_PEAK:
> @@ -146,7 +146,7 @@ static long ina209_from_reg(const u8 reg, const u16 val)
>  
>   case INA209_CURRENT:
>   /* LSB=1 mA (selected). Is in mA */
> - return val;
> + return (s16)val;
>   }
>  
>   /* programmer goofed */
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] hwmon: (adt7475) set start bit in probe

2017-04-21 Thread Chris Packham

The ADT7475 and ADT7476 have the STRT bit cleared by default[1]. Before any
monitoring activities the STRT bit needs to be set. Logically this needs
to happen before any of the sensors are read so the probe() function
seems the best place for it.

[1] - https://www.onsemi.com/pub/Collateral/ADT7475-D.PDF

Signed-off-by: Chris Packham 
---
 drivers/hwmon/adt7475.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
index fcfa48222145..c803e3c5fcd4 100644
--- a/drivers/hwmon/adt7475.c
+++ b/drivers/hwmon/adt7475.c
@@ -59,6 +59,8 @@
 #define REG_VENDID 0x3E
 #define REG_DEVID2 0x3F
 
+#define REG_CONFIG10x40
+
 #define REG_STATUS10x41
 #define REG_STATUS20x42
 
@@ -1371,6 +1373,17 @@ static int adt7475_probe(struct i2c_client *client,
for (i = 0; i < ADT7475_PWM_COUNT; i++)
adt7475_read_pwm(client, i);
 
+   /* Start monitoring */
+   switch (chip) {
+   case adt7475:
+   case adt7476:
+   i2c_smbus_write_byte_data(client, REG_CONFIG1,
+ adt7475_read(REG_CONFIG1) | 0x01);
+   break;
+   default:
+   break;
+   }
+
ret = sysfs_create_group(>dev.kobj, _attr_group);
if (ret)
return ret;
-- 
2.11.0.24.ge6920cf

--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] hwmon (pmbus): Add client driver for IR35221

2017-04-21 Thread Guenter Roeck


On 04/20/2017 09:25 PM, Samuel Mendoza-Jonas wrote:

IR35221 is a Digital DC-DC Multiphase Converter

Signed-off-by: Samuel Mendoza-Jonas 
---
- Tested on a ppc64 system which includes several of these devices.
- This patch re-implements the linear reg2data/data2reg functions from
  pmbus-core like some other drivers in order to scale some results. Is
  this something that would be better off being made generic for pmbus
  drivers to call?


That might make sense. The only other driver is zl6100, or am I missing
something ?

Trying to understand the code, the data format seems to be linear11 with
an added exponent on top of it. Is that correct ? If so, I wonder if we should
make that configurable, ie store the additional exponent in R and handle it
in the pmbus core. Would that help ?

If so, and if you have time, feel free to provide patches the modify the
pmbus core code accordingly.


- The resolution of iout0 is apparently configurable between two values,
  however the documentation I have access to does not specify how this is
  actually configured - currently it is left at the default resolution in
  the driver.



Nothing we can do about that, but it might make sense to add a note somewhere
in the driver.


 Documentation/hwmon/ir35221   |  87 
 drivers/hwmon/pmbus/Kconfig   |  11 ++
 drivers/hwmon/pmbus/Makefile  |   1 +
 drivers/hwmon/pmbus/ir35221.c | 322 ++
 4 files changed, 421 insertions(+)
 create mode 100644 Documentation/hwmon/ir35221
 create mode 100644 drivers/hwmon/pmbus/ir35221.c

diff --git a/Documentation/hwmon/ir35221 b/Documentation/hwmon/ir35221
new file mode 100644
index ..f7e112752c04
--- /dev/null
+++ b/Documentation/hwmon/ir35221
@@ -0,0 +1,87 @@
+Kernel driver ir35221
+=
+
+Supported chips:
+  * Infinion IR35221
+Prefix: 'ir35221'
+Addresses scanned: -
+Datasheet: Datasheet is not publicly available.


The chip doesn't even publicly exist, which is really annoying :-(.


+
+Author: Samuel Mendoza-Jonas 
+
+
+Description
+---
+
+IR35221 is a Digital DC-DC Multiphase Converter
+
+
+Usage Notes
+---
+
+This driver does not probe for PMBus devices. You will have to instantiate
+devices explicitly.
+
+Example: the following commands will load the driver for an IR35221
+at address 0x70 on I2C bus #4:
+
+# modprobe ir35221
+# echo ir35221 0x70 > /sys/bus/i2c/devices/i2c-4/new_device
+
+
+Sysfs attributes
+
+
+curr1_label"iin"
+curr1_inputMeasured input current
+curr1_max  Maximum current
+curr1_max_alarmCurrent high alarm
+
+curr[2-3]_label"iout[1-2]"
+curr[2-3]_inputMeasured output current
+curr[2-3]_crit Critical maximum current
+curr[2-3]_crit_alarm   Current critical high alarm
+curr[2-3]_highest  Highest output current
+curr[2-3]_lowest   Lowest output current
+curr[2-3]_max  Maximum current
+curr[2-3]_max_alarmCurrent high alarm
+
+in1_label  "vin"
+in1_input  Measured input voltage
+in1_crit   Critical maximum input voltage
+in1_crit_alarm Input voltage critical high alarm
+in1_highestHighest input voltage
+in1_lowest Lowest input voltage
+in1_minMinimum input voltage
+in1_min_alarm  Input voltage low alarm
+
+in[2-3]_label  "vout[1-2]"
+in[2-3]_input  Measured output voltage
+in[2-3]_lcrit  Critical minimum output voltage
+in[2-3]_lcrit_alarmOutput voltage critical low alarm
+in[2-3]_crit   Critical maximum output voltage
+in[2-3]_crit_alarm Output voltage critical high alarm
+in[2-3]_highestHighest output voltage
+in[2-3]_lowest Lowest output voltage
+in[2-3]_maxMaximum output voltage
+in[2-3]_max_alarm  Output voltage high alarm
+in[2-3]_minMinimum output voltage
+in[2-3]_min_alarm  Output voltage low alarm
+
+power1_label   "pin"
+power1_input   Measured input power
+power1_alarm   Input power high alarm
+power1_max Input power limit
+
+power[2-3]_label   "pout[1-2]"
+power[2-3]_input   Measured output power
+power[2-3]_max Output power limit
+power[2-3]_max_alarm   Output power high alarm
+
+temp[1-2]_inputMeasured temperature
+temp[1-2]_crit Critical high temperature
+temp[1-2]_crit_alarm   Chip temperature critical high alarm
+temp[1-2]_highest  Highest temperature
+temp[1-2]_lowest   Lowest temperature
+temp[1-2]_max  Maximum temperature
+temp[1-2]_max_alarmChip temperature high alarm
diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index cad1229b7e17..88c7d6b116f8 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -159,4 +159,15 @@ config

Re: [PATCH] hwmon: (ibmpowernv) Add min/max attributes and current sensors

2017-04-21 Thread Shilpasri G Bhat

Hi Cedric,

On 04/21/2017 05:17 PM, Cédric Le Goater wrote:
> Hello Shilpasri,
> 
> On 04/21/2017 06:31 AM, Shilpasri G Bhat wrote:
>> Add support for adding min/max values for the inband sensors copied by
>> OCC to main memory. And also add current(mA) sensors to the list.
>>
>> Signed-off-by: Shilpasri G Bhat 
>> ---
>>  drivers/hwmon/ibmpowernv.c | 55 
>> --
>>  1 file changed, 53 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/hwmon/ibmpowernv.c b/drivers/hwmon/ibmpowernv.c
>> index 6d2e660..1f329fa8 100644
>> --- a/drivers/hwmon/ibmpowernv.c
>> +++ b/drivers/hwmon/ibmpowernv.c
>> @@ -50,6 +50,7 @@ enum sensors {
>>  TEMP,
>>  POWER_SUPPLY,
>>  POWER_INPUT,
>> +CURRENT,
>>  MAX_SENSOR_TYPE,
>>  };
>>  
>> @@ -65,7 +66,8 @@ enum sensors {
>>  {"fan", "ibm,opal-sensor-cooling-fan"},
>>  {"temp", "ibm,opal-sensor-amb-temp"},
>>  {"in", "ibm,opal-sensor-power-supply"},
>> -{"power", "ibm,opal-sensor-power"}
>> +{"power", "ibm,opal-sensor-power"},
>> +{"curr"}, /* Follows newer device tree compatible ibm,opal-sensor */
> 
> why isn't there a compatible string ? 
> 

Skiboot exports "ibm,opal-sensor" as compatible string for all the inband
sensors. Now if I define that as the compatible string here, then all the
sensors would get identified as "curr" type of sensor by the driver.

>>  };
>>  
>>  struct sensor_data {
>> @@ -287,6 +289,7 @@ static int populate_attr_groups(struct platform_device 
>> *pdev)
>>  opal = of_find_node_by_path("/ibm,opal/sensors");
>>  for_each_child_of_node(opal, np) {
>>  const char *label;
>> +int len;
>>  
>>  if (np->name == NULL)
>>  continue;
>> @@ -298,10 +301,14 @@ static int populate_attr_groups(struct platform_device 
>> *pdev)
>>  sensor_groups[type].attr_count++;
>>  
>>  /*
>> - * add a new attribute for labels
>> + * add attributes for labels, min and max
>>   */
>>  if (!of_property_read_string(np, "label", ))
>>  sensor_groups[type].attr_count++;
> 
> We are negating of_property_read_string() above, but not below.
> I wonder why ? 

of_find_property() returns 'struct property *' while of_property_read_string()
returns 0 on success.

> 
>> +if (of_find_property(np, "sensor-data-min", ))
>> +sensor_groups[type].attr_count++;
>> +if (of_find_property(np, "sensor-data-max", ))
>> +sensor_groups[type].attr_count++;
>>  }
>>  
>>  of_node_put(opal);
>> @@ -428,6 +435,50 @@ static int create_device_attrs(struct platform_device 
>> *pdev)
>>  pgroups[type]->attrs[sensor_groups[type].attr_count++] =
>>  [count++].dev_attr.attr;
>>  }
>> +
>> +if (!of_property_read_u32(np, "sensor-data-max", _id)) {
>> +switch (type) {
>> +case POWER_INPUT:
>> +attr_name = "input_highest";
>> +break;
>> +case TEMP:
>> +attr_name = "max";
>> +break;
>> +default:
>> +attr_name = "highest";
>> +break;
>> +}
> 
> May be we could use a converting routine here ? create_device_attrs()
> is getting big.

Okay will do.

> 
>> +sdata[count].id = sensor_id;
>> +sdata[count].type = type;
>> +sdata[count].hwmon_index = sdata[count - 1].hwmon_index;
>> +create_hwmon_attr([count], attr_name,
>> +  show_sensor);
>> +pgroups[type]->attrs[sensor_groups[type].attr_count++] =
>> +[count++].dev_attr.attr;
>> +}
>> +
>> +if (!of_property_read_u32(np, "sensor-data-min", _id)) {
>> +switch (type) {
>> +case POWER_INPUT:
>> +attr_name = "input_lowest";
>> +break;
>> +case TEMP:
>> +attr_name = "min";
>> +break;
>> +default:
>> +attr_name = "lowest";
>> +break;
>> +}
> 
> same here.

Sure.

Thanks and Regards,
Shilpa

> 
> Thanks,
> 
> C.
>> +sdata[count].id = sensor_id;
>> +sdata[count].type = type;
>> +sdata[count].hwmon_index = sdata[count - 1].hwmon_index;
>> +create_hwmon_attr([count], attr_name,
>> +  show_sensor);
>> +

Re: [PATCH] hwmon (pmbus): Add client driver for IR35221

Re: [PATCH] hwmon: (ibmpowernv) Add min/max attributes and current sensors

Re: hwmon: ina209: Handled signed registers

[PATCH] hwmon: (adt7475) set start bit in probe

Re: [PATCH] hwmon (pmbus): Add client driver for IR35221

Re: [PATCH] hwmon: (ibmpowernv) Add min/max attributes and current sensors

6 matches

Site Navigation

Mail list logo

Footer information