Re: [PATCH v2 1/2] hwmon: (ucd9000) Add gpio chip interface

2018-03-15 Thread kbuild test robot 
Hi Christopher,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.16-rc5 next-20180315]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Eddie-James/hwmon-ucd9000-Add-gpio-and-debugfs-interfaces/20180316-125048
config: x86_64-randconfig-x019-201810 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

>> drivers/hwmon/pmbus/ucd9000.c:69:19: error: field 'gpio' has incomplete type
 struct gpio_chip gpio;
  ^~~~
   drivers/hwmon/pmbus/ucd9000.c: In function 'ucd9000_gpio_get':
>> drivers/hwmon/pmbus/ucd9000.c:184:31: error: implicit declaration of 
>> function 'gpiochip_get_data'; did you mean 'gpio_get_value'? 
>> [-Werror=implicit-function-declaration]
 struct i2c_client *client  = gpiochip_get_data(gc);
  ^
  gpio_get_value
>> drivers/hwmon/pmbus/ucd9000.c:184:31: warning: initialization makes pointer 
>> from integer without a cast [-Wint-conversion]
   drivers/hwmon/pmbus/ucd9000.c: In function 'ucd9000_gpio_set':
   drivers/hwmon/pmbus/ucd9000.c:197:30: warning: initialization makes pointer 
from integer without a cast [-Wint-conversion]
 struct i2c_client *client = gpiochip_get_data(gc);
 ^
   drivers/hwmon/pmbus/ucd9000.c: In function 'ucd9000_gpio_get_direction':
   drivers/hwmon/pmbus/ucd9000.c:240:30: warning: initialization makes pointer 
from integer without a cast [-Wint-conversion]
 struct i2c_client *client = gpiochip_get_data(gc);
 ^
   drivers/hwmon/pmbus/ucd9000.c: In function 'ucd9000_gpio_set_direction':
   drivers/hwmon/pmbus/ucd9000.c:254:30: warning: initialization makes pointer 
from integer without a cast [-Wint-conversion]
 struct i2c_client *client = gpiochip_get_data(gc);
 ^
   drivers/hwmon/pmbus/ucd9000.c: In function 'ucd9000_probe':
>> drivers/hwmon/pmbus/ucd9000.c:460:9: error: implicit declaration of function 
>> 'devm_gpiochip_add_data'; did you mean 'devm_gpio_request'? 
>> [-Werror=implicit-function-declaration]
  ret = devm_gpiochip_add_data(>dev, >gpio,
^~
devm_gpio_request
   cc1: some warnings being treated as errors

vim +/gpio +69 drivers/hwmon/pmbus/ucd9000.c

65  
66  struct ucd9000_data {
67  u8 fan_data[UCD9000_NUM_FAN][I2C_SMBUS_BLOCK_MAX];
68  struct pmbus_driver_info info;
  > 69  struct gpio_chip gpio;
70  };
71  #define to_ucd9000_data(_info) container_of(_info, struct ucd9000_data, 
info)
72  
73  static int ucd9000_get_fan_config(struct i2c_client *client, int fan)
74  {
75  int fan_config = 0;
76  struct ucd9000_data *data
77= to_ucd9000_data(pmbus_get_driver_info(client));
78  
79  if (data->fan_data[fan][3] & 1)
80  fan_config |= PB_FAN_2_INSTALLED;   /* Use lower bit 
position */
81  
82  /* Pulses/revolution */
83  fan_config |= (data->fan_data[fan][3] & 0x06) >> 1;
84  
85  return fan_config;
86  }
87  
88  static int ucd9000_read_byte_data(struct i2c_client *client, int page, 
int reg)
89  {
90  int ret = 0;
91  int fan_config;
92  
93  switch (reg) {
94  case PMBUS_FAN_CONFIG_12:
95  if (page > 0)
96  return -ENXIO;
97  
98  ret = ucd9000_get_fan_config(client, 0);
99  if (ret < 0)
   100  return ret;
   101  fan_config = ret << 4;
   102  ret = ucd9000_get_fan_config(client, 1);
   103  if (ret < 0)
   104  return ret;
   105  fan_config |= ret;
   106  ret = fan_config;
   107  break;
   108  case PMBUS_FAN_CONFIG_34:
   109  if (page > 0)
   110  return -ENXIO;
   111  
   112  ret = ucd9000_get_fan_config(client, 2);
   113  if (ret < 0)
   114  return ret;
   115  fan_config = ret << 4;
   116  ret = ucd9000_get_fan_config(client, 3);
   117  if (ret < 0)
   118  return ret;
   119  fan_config |= ret;
   120  r

[PATCH v5 2/2] hwmon: (ucd9000) Add debugfs attributes to provide mfr_status

2018-03-15 Thread Eddie James 
From: Christopher Bostic 

Expose the gpiN_fault fields of mfr_status as individual debugfs
attributes. This provides a way for users to be easily notified of gpi
faults. Also provide the whole mfr_status register in debugfs.

Signed-off-by: Christopher Bostic 
Signed-off-by: Andrew Jeffery 
Signed-off-by: Eddie James 
---
 drivers/hwmon/pmbus/ucd9000.c | 137 +-
 1 file changed, 136 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/pmbus/ucd9000.c b/drivers/hwmon/pmbus/ucd9000.c
index a34ffc4..c1a1560 100644
--- a/drivers/hwmon/pmbus/ucd9000.c
+++ b/drivers/hwmon/pmbus/ucd9000.c
@@ -19,6 +19,7 @@
  * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -36,6 +37,7 @@
 #define UCD9000_NUM_PAGES  0xd6
 #define UCD9000_FAN_CONFIG_INDEX   0xe7
 #define UCD9000_FAN_CONFIG 0xe8
+#define UCD9000_MFR_STATUS 0xf3
 #define UCD9000_GPIO_SELECT0xfa
 #define UCD9000_GPIO_CONFIG0xfb
 #define UCD9000_DEVICE_ID  0xfd
@@ -63,13 +65,22 @@
 #define UCD901XX_NUM_GPIOS 26
 #define UCD90910_NUM_GPIOS 26
 
+#define UCD9000_DEBUGFS_NAME_LEN   24
+#define UCD9000_GPI_COUNT  8
+
 struct ucd9000_data {
u8 fan_data[UCD9000_NUM_FAN][I2C_SMBUS_BLOCK_MAX];
struct pmbus_driver_info info;
struct gpio_chip gpio;
+   struct dentry *debugfs;
 };
 #define to_ucd9000_data(_info) container_of(_info, struct ucd9000_data, info)
 
+struct ucd9000_debugfs_entry {
+   struct i2c_client *client;
+   u8 index;
+};
+
 static int ucd9000_get_fan_config(struct i2c_client *client, int fan)
 {
int fan_config = 0;
@@ -306,6 +317,121 @@ static int ucd9000_gpio_direction_output(struct gpio_chip 
*gc,
  val);
 }
 
+#ifdef CONFIG_DEBUG_FS
+static int ucd9000_get_mfr_status(struct i2c_client *client, u8 *buffer)
+{
+   int ret = pmbus_set_page(client, 0);
+
+   if (ret < 0)
+   return ret;
+
+   return i2c_smbus_read_block_data(client, UCD9000_MFR_STATUS, buffer);
+}
+
+static int ucd9000_debugfs_show_mfr_status_bit(void *data, u64 *val)
+{
+   struct ucd9000_debugfs_entry *entry = data;
+   struct i2c_client *client = entry->client;
+   u8 buffer[I2C_SMBUS_BLOCK_MAX];
+   int ret;
+
+   ret = ucd9000_get_mfr_status(client, buffer);
+   if (ret < 0)
+   return ret;
+
+   /*
+* Attribute only created for devices with gpi fault bits at bits
+* 16-23, which is the second byte of the response.
+*/
+   *val = !!(buffer[1] & BIT(entry->index));
+
+   return 0;
+}
+DEFINE_DEBUGFS_ATTRIBUTE(ucd9000_debugfs_mfr_status_bit,
+ucd9000_debugfs_show_mfr_status_bit, NULL, "%1lld\n");
+
+static ssize_t ucd9000_debugfs_read_mfr_status(struct file *file,
+  char __user *buf, size_t count,
+  loff_t *ppos)
+{
+   struct i2c_client *client = file->private_data;
+   u8 buffer[I2C_SMBUS_BLOCK_MAX] = { 0 };
+   char str[(I2C_SMBUS_BLOCK_MAX * 2) + 2] = { 0 };
+   char *res;
+   int rc;
+
+   rc = ucd9000_get_mfr_status(client, buffer);
+   if (rc < 0)
+   return rc;
+
+   res = bin2hex(str, buffer, rc);
+   *res++ = '\n';
+
+   return simple_read_from_buffer(buf, count, ppos, str, (res - str) + 1);
+}
+
+static const struct file_operations ucd9000_debugfs_show_mfr_status_fops = {
+   .llseek = noop_llseek,
+   .read = ucd9000_debugfs_read_mfr_status,
+   .open = simple_open,
+};
+
+static int ucd9000_init_debugfs(struct i2c_client *client,
+   const struct i2c_device_id *mid,
+   struct ucd9000_data *data)
+{
+   struct dentry *debugfs;
+   struct ucd9000_debugfs_entry *entries;
+   int i;
+   char name[UCD9000_DEBUGFS_NAME_LEN];
+
+   debugfs = pmbus_get_debugfs_dir(client);
+   if (!debugfs)
+   return -ENOENT;
+
+   data->debugfs = debugfs_create_dir(client->name, debugfs);
+   if (!data->debugfs)
+   return -ENOENT;
+
+   /*
+* Of the chips this driver supports, only the UCD9090, UCD90160,
+* and UCD90910 report GPI faults in their MFR_STATUS register, so only
+* create the GPI fault debugfs attributes for those chips.
+*/
+   if (mid->driver_data == ucd9090 || mid->driver_data == ucd90160 ||
+   mid->driver_data == ucd90910) {
+   entries = devm_kzalloc(>dev,
+  sizeof(*entries) * UCD9000_GPI_COUNT,
+  GFP_KERNEL);
+   if (!entries)
+   return -ENOMEM;
+
+

[PATCH v5 0/2] hwmon: (ucd9000) Add gpio and debugfs interfaces

2018-03-15 Thread Eddie James 
The ucd9000 series chips have gpio pins. Add a gpio chip interface to the ucd
device so that users can query and set the state of the gpio pins.

Add a debugfs interface using the existing pmbus debugfs directory to provide
MFR_STATUS and the status of the gpi faults to users.

Changes since v4:
 - max-sized buffers for smbus transfers
 - used bin2hex instead of my own code

Changes since v3:
 - remove setting of gpio_chip->owner
 - format the mfr_status data
 - switch to #ifdef rather than #if IS_ENABLED for debugfs

Changes since v2:
 - split the gpio registration into it's own function

Changes since v1:
 - dropped dev_err messages
 - made gpio chip registration conditional on having gpio pins
 - made mfr_status debugfs attribute more simple

Christopher Bostic (2):
  hwmon: (ucd9000) Add gpio chip interface
  hwmon: (ucd9000) Add debugfs attributes to provide mfr_status

 drivers/hwmon/pmbus/ucd9000.c | 338 +-
 1 file changed, 337 insertions(+), 1 deletion(-)

-- 
1.8.3.1

--
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 -next] hwmon: (scmi) return -EINVAL when sensor information is unavailable

2018-03-15 Thread Guenter Roeck 

On 03/15/2018 11:08 AM, Sudeep Holla wrote:

Passing NULL pointer to PTR_ERR will result in return value of 0
indicating success which is clearly not what it is intended here.

This patch returns -EINVAL instead when the sensor information is not
available.

Fixes: b23688aefb8b ("hwmon: add support for sensors exported via ARM SCMI")
Reported-by: Dan Carpenter 
Cc: Guenter Roeck 
Cc: linux-hwmon@vger.kernel.org
Signed-off-by: Sudeep Holla 


Acked-by: Guenter Roeck 


---
  drivers/hwmon/scmi-hwmon.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

Hi Guenter,

I will try to push this via ARM-SoC if possible before v4.17 merge window.
So please provide Ack if you are fine.

Regards,
Sudeep

diff --git a/drivers/hwmon/scmi-hwmon.c b/drivers/hwmon/scmi-hwmon.c
index 32e750373ced..363bf56eb0f2 100644
--- a/drivers/hwmon/scmi-hwmon.c
+++ b/drivers/hwmon/scmi-hwmon.c
@@ -138,7 +138,7 @@ static int scmi_hwmon_probe(struct scmi_device *sdev)
for (i = 0; i < nr_sensors; i++) {
sensor = handle->sensor_ops->info_get(handle, i);
if (!sensor)
-   return PTR_ERR(sensor);
+   return -EINVAL;
  
  		switch (sensor->type) {

case TEMPERATURE_C:



--
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 -next] hwmon: (scmi) return -EINVAL when sensor information is unavailable

2018-03-15 Thread Sudeep Holla 
Passing NULL pointer to PTR_ERR will result in return value of 0
indicating success which is clearly not what it is intended here.

This patch returns -EINVAL instead when the sensor information is not
available.

Fixes: b23688aefb8b ("hwmon: add support for sensors exported via ARM SCMI")
Reported-by: Dan Carpenter 
Cc: Guenter Roeck 
Cc: linux-hwmon@vger.kernel.org
Signed-off-by: Sudeep Holla 
---
 drivers/hwmon/scmi-hwmon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Hi Guenter,

I will try to push this via ARM-SoC if possible before v4.17 merge window.
So please provide Ack if you are fine.

Regards,
Sudeep

diff --git a/drivers/hwmon/scmi-hwmon.c b/drivers/hwmon/scmi-hwmon.c
index 32e750373ced..363bf56eb0f2 100644
--- a/drivers/hwmon/scmi-hwmon.c
+++ b/drivers/hwmon/scmi-hwmon.c
@@ -138,7 +138,7 @@ static int scmi_hwmon_probe(struct scmi_device *sdev)
for (i = 0; i < nr_sensors; i++) {
sensor = handle->sensor_ops->info_get(handle, i);
if (!sensor)
-   return PTR_ERR(sensor);
+   return -EINVAL;
 
switch (sensor->type) {
case TEMPERATURE_C:
-- 
2.7.4

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