drivers/hwmon/pmbus: core extensions
cover drivers/hwmon/pmbus: core extensions
From: "Edward A. James" <[email protected]>
This series adds some functionality to the pmbus core.
The first two patches are fixes to enable the use of the STATUS_WORD register which was completely unused in existing pmbus. This allows the use of more default alarm/fault attributes for default pmbus sensors.
The third patch adds "status" attributes to each class of hwmon sensor created by pmbus. For example, in1_status and temp1_status. These will display the associated raw status register (STATUS_INPUT and STATUS_TEMPERATURE). These will probably be essential for the UCD driver, and certainly very convenient for the power supply driver.
An alternative I considered was breaking all the status registers out into bits, but some bits seem to be device-dependent and therefore couldn't be named correctly or at all in the pmbus core. This would also really make the hwmon directory messy, unless you have a separate directory for that, either in the device directory or under the hwmon directory (not sure if that's even possible).
Edward A. James (3):
drivers/hwmon/pmbus: Use STATUS_WORD by default and do correct access drivers/hmwon/pmbus: store STATUS_WORD in status registers drivers/hwmon/pmbus: Add sensor status to pmbus attributes
drivers/hwmon/pmbus/pmbus_core.c | 153 +++++++++++++++++++++++++++++++++------ 1 file changed, 130 insertions(+), 23 deletions(-)
Signature · 1 line
1.8.3.1
RFC 1/3 drivers/hwmon/pmbus: Use STATUS_WORD by default and do correct access
From: "Edward A. James" <[email protected]>
Pmbus core always read byte data from the status register, even if configured to use STATUS_WORD. Use a function pointer so we always do the correct byte/word access. Also switch to use STATUS_WORD by default.
--- drivers/hwmon/pmbus/pmbus_core.c | 69 +++++++++++++++++++++++++++++++--------- 1 file changed, 54 insertions(+), 15 deletions(-)
drivers/hwmon/pmbus/pmbus_core.c +54 -15
@@ -114,7 +114,8 @@ struct pmbus_data {* so we keep them all together. */ u8 status[PB_NUM_STATUS_REG ]; -u8 status_register; + +int (*read_status)(struct i2c_client *client,int page);u8 currpage; };
@@ -460,7 +461,7 @@ static int pmbus_check_status_cml(struct i2c_client *client)struct pmbus_data *data =i2c_get_clientdata (client);int status, status2; - status =_pmbus_read_byte_data (client,-1 , data->status_register); + status = data->read_status (client,-1 );if (status <0 || (status &PB_STATUS_CML )) { status2 =_pmbus_read_byte_data (client,-1 ,PMBUS_STATUS_CML );if (status2 <0 || (status2 &PB_CML_FAULT_INVALID_COMMAND ))
@@ -484,6 +485,36 @@ static bool pmbus_check_register(struct i2c_client *client,return rv >=0 ; } +/* Check specified page status register accessibility. Need a separate +* function rather than the two above so we can use the correct status +* register, and we can optimize out the second status register read. +*/ +static bool pmbus_check_status_register (struct i2c_client *client,int page) +{ +int status, rc =0 ; +struct pmbus_data *data =i2c_get_clientdata (client); + + status = data->read_status (client, page); +if (status <0 ) + goto out; + +if (!(data->flags &PMBUS_SKIP_STATUS_CHECK )) { +if (status &PB_STATUS_CML ) { + status =_pmbus_read_byte_data (client,-1 , +PMBUS_STATUS_CML ); +if (status <0 || + (status &PB_CML_FAULT_INVALID_COMMAND )) + goto out; + } + } + + rc =1 ; + +out: +pmbus_clear_fault_page (client,-1 ); +return rc; +} +bool pmbus_check_byte_register (struct i2c_client *client,int page,int reg) {return pmbus_check_register (client, _pmbus_read_byte_data, page, reg);
@@ -529,9 +560,8 @@ static struct pmbus_data *pmbus_update_device(struct device *dev)int i, j;for (i =0 ; i < info->pages; i++) { - data->status[PB_STATUS_BASE + i] - =_pmbus_read_byte_data (client, i, - data->status_register); + data->status[PB_STATUS_BASE + i] = + data->read_status (client, i);for (j =0 ; j <ARRAY_SIZE (pmbus_status); j++) {struct _pmbus_status *s = &pmbus_status[j];
@@ -1199,8 +1229,7 @@ static int pmbus_add_sensor_attrs_one(struct i2c_client *client,* the generic status register for this page is accessible. */ if (!ret && attr->gbit && -pmbus_check_byte_register (client, page, - data->status_register)) { +pmbus_check_status_register (client, page)) { ret = pmbus_add_boolean(data, name, "alarm", index, NULL, NULL, PB_STATUS_BASE + page,
@@ -1921,6 +1950,16 @@ static int pmbus_identify_common(struct i2c_client *client,return 0 ; } +static int pmbus_read_status_word (struct i2c_client *client,int page) +{ +return _pmbus_read_word_data (client, page,PMBUS_STATUS_WORD ); +} + +static int pmbus_read_status_byte (struct i2c_client *client,int page) +{ +return _pmbus_read_byte_data (client, page,PMBUS_STATUS_BYTE ); +} +static int pmbus_init_common (struct i2c_client *client,struct pmbus_data *data,struct pmbus_driver_info *info) {
@@ -1928,16 +1967,16 @@ static int pmbus_init_common(struct i2c_client *client, struct pmbus_data *data,int page, ret;/* -* Some PMBus chips don't support PMBUS_STATUS_BYTE, so try -* to use PMBUS_STATUS_WORD instead if that is the case. +* Some PMBus chips don't support PMBUS_STATUS_WORD, so try +* to use PMBUS_STATUS_BYTE instead if that is the case. * Bail out if both registers are not supported. */ - data->status_register =PMBUS_STATUS_BYTE ; - ret =i2c_smbus_read_byte_data (client,PMBUS_STATUS_BYTE ); -if (ret <0 || ret ==0xff ) { - data->status_register =PMBUS_STATUS_WORD ; - ret =i2c_smbus_read_word_data (client,PMBUS_STATUS_WORD ); -if (ret <0 || ret ==0xffff ) { + data->read_status = pmbus_read_status_word; + ret =i2c_smbus_read_word_data (client,PMBUS_STATUS_WORD ); +if (ret <0 || ret ==0xffff ) { + data->read_status = pmbus_read_status_byte; + ret =i2c_smbus_read_byte_data (client,PMBUS_STATUS_BYTE ); +if (ret <0 || ret ==0xff ) {dev_err (dev,"PMBus status register not found\n" );return -ENODEV ; }
Signature · 1 line
1.8.3.1
RFC 2/3 drivers/hmwon/pmbus: store STATUS_WORD in status registers
In reply to Eddie JamesFrom: "Edward A. James" <[email protected]>
Higher byte of the status register wasn't available for boolean alarms. Store the STATUS_WORD register if it's available, and read it out when queried by boolean attributes.
The method of storing and retrieving the status reg is a bit hacky but I couldn't work out another way without doubling the storage requirement of every pmbus device or tearing up more of the pmbus core code.
--- drivers/hwmon/pmbus/pmbus_core.c | 37 +++++++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 10 deletions(-)
drivers/hwmon/pmbus/pmbus_core.c +27 -10
@@ -43,7 +43,7 @@* Index into status register array, per status register group */ #define PB_STATUS_BASE 0 -#define PB_STATUS_VOUT_BASE (PB_STATUS_BASE + PMBUS_PAGES) +#define PB_STATUS_VOUT_BASE (PB_STATUS_BASE + (PMBUS_PAGES * 2))#define PB_STATUS_IOUT_BASE (PB_STATUS_VOUT_BASE + PMBUS_PAGES)#define PB_STATUS_FAN_BASE (PB_STATUS_IOUT_BASE + PMBUS_PAGES)#define PB_STATUS_FAN34_BASE (PB_STATUS_FAN_BASE + PMBUS_PAGES)
@@ -557,11 +557,14 @@ static struct pmbus_data *pmbus_update_device(struct device *dev)mutex_lock (&data->update_lock);if (time_after (jiffies, data->last_updated +HZ ) || !data->valid) { -int i, j; +int i, j, status;for (i =0 ; i < info->pages; i++) { - data->status[PB_STATUS_BASE + i] = - data->read_status (client, i); + status = data->read_status (client, i); + data->status[PB_STATUS_BASE + (i *2 )] = status; + data->status[PB_STATUS_BASE + (i *2 ) +1 ] = + status >>8 ; +for (j =0 ; j <ARRAY_SIZE (pmbus_status); j++) {struct _pmbus_status *s = &pmbus_status[j];
@@ -860,6 +863,18 @@ static u16 pmbus_data2reg(struct pmbus_data *data,return regval; } +static int pmbus_get_status (struct pmbus_data *data,int reg) +{ +int ret; + +if (reg <PB_STATUS_BASE +PMBUS_PAGES ) + ret = data->status[reg *2 ] | data->status[(reg *2 ) +1 ] <<8 ; +else + ret = data->status[reg]; + +return ret; +} +/* * Return boolean calculated from converted data. * <index> defines a status register index and mask.
@@ -888,12 +903,12 @@ static int pmbus_get_boolean(struct pmbus_data *data, struct pmbus_boolean *b, {struct pmbus_sensor *s1 = b->s1;struct pmbus_sensor *s2 = b->s2; -u16 reg = (index >>8 ) &0xffff ; -u8 mask = index &0xff ; +u16 reg = index >>16 ; +u16 mask = index &0xffff ;int ret, status;u8 regval; - status = data->status[reg]; + status =pmbus_get_status (data, reg);if (status <0 )return status;
@@ -1032,7 +1047,7 @@ static int pmbus_add_boolean(struct pmbus_data *data, const char *name, const char *type, int seq, struct pmbus_sensor *s1, struct pmbus_sensor *s2, - u16 reg, u8 mask) + u16 reg, u16 mask) { struct pmbus_boolean *boolean;struct sensor_device_attribute *a;
@@ -1048,7 +1063,7 @@ static int pmbus_add_boolean(struct pmbus_data *data, boolean->s1 = s1; boolean->s2 = s2;pmbus_attr_init (a, boolean->name,S_IRUGO , pmbus_show_boolean,NULL , - (reg <<8 ) | mask); + (reg <<16 ) | mask);return pmbus_add_attribute (data, &a->dev_attr.attr); }
@@ -1140,7 +1155,7 @@ struct pmbus_limit_attr {*/ struct pmbus_sensor_attr {u16 reg;/* sensor register */ -u8 gbit;/* generic status bit */ +u16 gbit;/* generic status bit */ u8 nlimit;/* # of limit registers */ enum pmbus_sensor_classes class;/* sensor class */ const char *label;/* sensor label */
@@ -1485,6 +1500,7 @@ static int pmbus_add_sensor_attrs(struct i2c_client *client, .func = PMBUS_HAVE_IIN, .sfunc = PMBUS_HAVE_STATUS_INPUT, .sbase = PB_STATUS_INPUT_BASE, + .gbit = PB_STATUS_INPUT, .limit = iin_limit_attrs, .nlimit = ARRAY_SIZE(iin_limit_attrs), }, {
@@ -1569,6 +1585,7 @@ static int pmbus_add_sensor_attrs(struct i2c_client *client, .func = PMBUS_HAVE_PIN, .sfunc = PMBUS_HAVE_STATUS_INPUT, .sbase = PB_STATUS_INPUT_BASE, + .gbit = PB_STATUS_INPUT, .limit = pin_limit_attrs, .nlimit = ARRAY_SIZE(pin_limit_attrs), }, {
Signature · 1 line
1.8.3.1
RFC 3/3 drivers/hwmon/pmbus: Add sensor status to pmbus attributes
In reply to Eddie JamesFrom: "Edward A. James" <[email protected]>
Extend the pmbus core to provide status registers for attributes.
--- drivers/hwmon/pmbus/pmbus_core.c | 51 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+)
drivers/hwmon/pmbus/pmbus_core.c +51 -0
@@ -87,6 +87,14 @@ struct pmbus_label {#define to_pmbus_label (_attr) \ container_of(_attr, struct pmbus_label, attribute) +struct pmbus_status { +char name[PMBUS_NAME_SIZE ];/* sysfs status name */ +struct device_attribute attribute; +int reg; +}; +#define to_pmbus_status (_attr) \ + container_of(_attr, struct pmbus_status, attribute) +struct pmbus_data {struct device *dev;struct device *hwmon_dev;
@@ -993,6 +1001,16 @@ static ssize_t pmbus_show_label(struct device *dev,return snprintf (buf,PAGE_SIZE ,"%s\n" , label->label); } +static ssize_t pmbus_show_status (struct device *dev, +struct device_attribute *da,char *buf) +{ +struct pmbus_status *status =to_pmbus_status (da); +struct pmbus_data *data =pmbus_update_device (dev); + +return snprintf (buf,PAGE_SIZE ,"%02x\n" , +pmbus_get_status (data, status->reg) &0xFF ); +} +static int pmbus_add_attribute (struct pmbus_data *data,struct attribute *attr) {if (data->num_attributes >= data->max_attributes -1 ) {
@@ -1131,6 +1149,25 @@ static int pmbus_add_label(struct pmbus_data *data,return pmbus_add_attribute (data, &a->attr); } +static int pmbus_add_status (struct pmbus_data *data,const char *name,int seq, +int reg) +{ +struct pmbus_status *status; +struct device_attribute *a; + + status =devm_kzalloc (data->dev,sizeof (*status),GFP_KERNEL ); +if (!status) +return -ENOMEM ; + + a = &status->attribute; + +snprintf (status->name,sizeof (status->name),"%s%d_status" , name, seq); + status->reg = reg; + +pmbus_dev_attr_init (a, status->name,S_IRUGO , pmbus_show_status,NULL ); +return pmbus_add_attribute (data, &a->attr); +} +/* * Search for attributes. Allocate sensors, booleans, and labels as needed. */
@@ -1253,6 +1290,16 @@ static int pmbus_add_sensor_attrs_one(struct i2c_client *client,return ret; } } +if (attr->sbase >0 && attr->sbase <PB_STATUS_VMON_BASE ) { +/* Add "status" attribute for physical status registers to dump +* the contents. This is pretty useful for hardware diagnostic +* purposes, as an alarm or fault bit doesn't necessarily +* provide all the info available. +*/ + ret =pmbus_add_status (data, name, index, attr->sbase + page); +if (ret) +return ret; + }return 0 ; }
@@ -1885,6 +1932,10 @@ static int pmbus_add_fan_attributes(struct i2c_client *client, PB_FAN_FAN1_FAULT >> (f & 1));if (ret)return ret; + ret =pmbus_add_status (data,"fan" , index, + base); +if (ret) +return ret; } index++; }
Signature · 1 line
1.8.3.1