[{"id":1761152,"web_url":"http://patchwork.ozlabs.org/comment/1761152/","msgid":"<2b79cfa6-7dd3-553f-6925-eb127a9577cd@linux.vnet.ibm.com>","list_archive_url":null,"date":"2017-08-31T16:11:43","subject":"Re: [PATCH linux dev-4.10 v6 2/3] hwmon: (ucd9000) Add gpio chip\n\tinterface","submitter":{"id":70582,"url":"http://patchwork.ozlabs.org/api/people/70582/","name":"Matt Spinler","email":"mspinler@linux.vnet.ibm.com"},"content":"Tested-by: Matt Spinler mspinler@linux.vnet.ibm.com\n\n\nOn 8/30/2017 4:49 PM, Christopher Bostic wrote:\n> Add a struct gpio_chip and define some methods so that this device's\n> I/O can be accessed via /sys/class/gpio.\n>\n> Present requirements only call for retrieving current state of pin\n> values and their direction.  No requirement at this time to switch\n> modes between output and input within the device driver.\n>\n> Signed-off-by: Christopher Bostic <cbostic@linux.vnet.ibm.com>\n> ---\n> v6 - Break out sysfs clear logged faults into separate patch\n>     - Fix logic bug that was bitwise inverting instead of !\n>     - Add further comments about assumptions related to pin mux.\n> v5 - Clean up remaining branch return statements to !! non\n>       branching method.\n>     - Clean up white space issues.\n>     - Add return codes to error messages.\n>     - Add comments describing assumptions of ucd90160 type.\n>     - Define gpio direction set methods.\n>     - Add unique id for each ucd9000 in system for gpio chip.\n> v4 - Change status check from branch to a !! non branching method\n>     - Remove usage comments on libgpiod for the struct gpio_chip\n>       methods.\n>     - Clean up some text formatting.\n> v3 - Correct bug in gpio_chip get method.  Wasn't retrieving\n>       gpio config information correctly.\n>     - Remove old debugfs flag from previous pmbus core changes.\n>     - Remove all sysfs files for mfr_status command.\n>     - Add comments on direct i2c_smbus calls to clarify that no page\n>       set is required.\n> v2 - Remove clear_faults file - redundant since all other sysfs\n>       core accesses result in an automatic clear fault.\n>     - Removed local status_word and status_vout register dumps and\n>       use the new pmbus core status facilities instead.\n>     - Rename gpi<x>_fault to gpi<x>_alarm to better match core naming\n>       conventions.\n>     - Add full register dump for mfr_status.\n>     - Add gpio chip to device structure and use gpio interfaces.\n> ---\n>   drivers/hwmon/pmbus/ucd9000.c | 206 ++++++++++++++++++++++++++++++++++++++++++\n>   1 file changed, 206 insertions(+)\n>\n> diff --git a/drivers/hwmon/pmbus/ucd9000.c b/drivers/hwmon/pmbus/ucd9000.c\n> index c401e5b..61abd3a 100644\n> --- a/drivers/hwmon/pmbus/ucd9000.c\n> +++ b/drivers/hwmon/pmbus/ucd9000.c\n> @@ -28,6 +28,7 @@\n>   #include <linux/i2c/pmbus.h>\n>   #include <linux/sysfs.h>\n>   #include <linux/hwmon-sysfs.h>\n> +#include <linux/gpio.h>\n>   #include \"pmbus.h\"\n>\n>   enum chips { ucd9000, ucd90120, ucd90124, ucd90160, ucd9090, ucd90910 };\n> @@ -37,8 +38,18 @@\n>   #define UCD9000_FAN_CONFIG_INDEX\t0xe7\n>   #define UCD9000_FAN_CONFIG\t\t0xe8\n>   #define UCD9000_LOGGED_FAULTS\t\t0xea\n> +#define UCD9000_GPIO_SELECT\t\t0xfa\n> +#define UCD9000_GPIO_CONFIG\t\t0xfb\n>   #define UCD9000_DEVICE_ID\t\t0xfd\n>\n> +/* GPIO CONFIG bits */\n> +#define UCD9000_GPIO_CONFIG_ENABLE\tBIT(0)\n> +#define UCD9000_GPIO_CONFIG_OUT_ENABLE\tBIT(1)\n> +#define UCD9000_GPIO_CONFIG_OUT_VALUE\tBIT(2)\n> +#define UCD9000_GPIO_CONFIG_STATUS\tBIT(3)\n> +#define UCD9000_GPIO_INPUT\t\t0\n> +#define UCD9000_GPIO_OUTPUT\t\t1\n> +\n>   #define UCD9000_MON_TYPE(x)\t(((x) >> 5) & 0x07)\n>   #define UCD9000_MON_PAGE(x)\t((x) & 0x0f)\n>\n> @@ -49,9 +60,13 @@\n>\n>   #define UCD9000_NUM_FAN\t\t4\n>\n> +#define UCD9000_GPIO_NAME_LEN\t16\n> +#define UCD90160_NUM_GPIOS\t26\n> +\n>   struct ucd9000_data {\n>   \tu8 fan_data[UCD9000_NUM_FAN][I2C_SMBUS_BLOCK_MAX];\n>   \tstruct pmbus_driver_info info;\n> +\tstruct gpio_chip gpio;\n>   };\n>   #define to_ucd9000_data(_info) container_of(_info, struct ucd9000_data, info)\n>\n> @@ -122,6 +137,166 @@ static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg)\n>   };\n>   MODULE_DEVICE_TABLE(i2c, ucd9000_id);\n>\n> +static int ucd9000_gpio_read_config(struct i2c_client *client,\n> +\t\t\t\tunsigned int offset)\n> +{\n> +\tint ret;\n> +\n> +\t/* No page set required */\n> +\tret = i2c_smbus_write_byte_data(client, UCD9000_GPIO_SELECT, offset);\n> +\tif (ret < 0) {\n> +\t\tdev_err(&client->dev, \"Failed to select GPIO %d: %d\\n\", offset,\n> +\t\t\tret);\n> +\n> +\t\treturn ret;\n> +\t}\n> +\n> +\treturn i2c_smbus_read_byte_data(client, UCD9000_GPIO_CONFIG);\n> +}\n> +\n> +static int ucd9000_gpio_get(struct gpio_chip *gc, unsigned int offset)\n> +{\n> +\tstruct i2c_client *client  = gpiochip_get_data(gc);\n> +\tint ret;\n> +\n> +\tret = ucd9000_gpio_read_config(client, offset);\n> +\tif (ret < 0) {\n> +\t\tdev_err(&client->dev, \"failed to read GPIO %d config: %d\\n\",\n> +\t\t\toffset, ret);\n> +\n> +\t\treturn ret;\n> +\t}\n> +\n> +\treturn !!(ret & UCD9000_GPIO_CONFIG_STATUS);\n> +}\n> +\n> +static void ucd9000_gpio_set(struct gpio_chip *gc, unsigned int offset,\n> +\t\t\tint value)\n> +{\n> +\tstruct i2c_client *client = gpiochip_get_data(gc);\n> +\tint ret;\n> +\n> +\tret = ucd9000_gpio_read_config(client, offset);\n> +\tif (ret < 0) {\n> +\t\tdev_err(&client->dev, \"failed to read GPIO %d config: %d\\n\",\n> +\t\t\toffset, ret);\n> +\n> +\t\treturn;\n> +\t}\n> +\n> +\tif (value) {\n> +\t\tif (ret & UCD9000_GPIO_CONFIG_STATUS)\n> +\t\t\treturn;\n> +\n> +\t\tret |= UCD9000_GPIO_CONFIG_STATUS;\n> +\t} else {\n> +\t\tif (!(ret & UCD9000_GPIO_CONFIG_STATUS))\n> +\t\t\treturn;\n> +\n> +\t\tret &= ~UCD9000_GPIO_CONFIG_STATUS;\n> +\t}\n> +\n> +\tret |= UCD9000_GPIO_CONFIG_ENABLE;\n> +\n> +\t/* Page set not required */\n> +\tret = i2c_smbus_write_byte_data(client, UCD9000_GPIO_CONFIG, ret);\n> +\tif (ret < 0) {\n> +\t\tdev_err(&client->dev, \"Failed to write GPIO %d config: %d\\n\",\n> +\t\t\toffset, ret);\n> +\n> +\t\treturn;\n> +\t}\n> +\n> +\tret &= ~UCD9000_GPIO_CONFIG_ENABLE;\n> +\n> +\tret = i2c_smbus_write_byte_data(client, UCD9000_GPIO_CONFIG, ret);\n> +\tif (ret < 0)\n> +\t\tdev_err(&client->dev, \"Failed to write GPIO %d config: %d\\n\",\n> +\t\t\toffset, ret);\n> +}\n> +\n> +static int ucd9000_gpio_get_direction(struct gpio_chip *gc,\n> +\t\t\t\t\tunsigned int offset)\n> +{\n> +\tstruct i2c_client *client = gpiochip_get_data(gc);\n> +\tint ret;\n> +\n> +\tret = ucd9000_gpio_read_config(client, offset);\n> +\tif (ret < 0) {\n> +\t\tdev_err(&client->dev, \"failed to read GPIO %d config: %d\\n\",\n> +\t\t\toffset, ret);\n> +\n> +\t\treturn ret;\n> +\t}\n> +\n> +\treturn !(!!(ret & UCD9000_GPIO_CONFIG_OUT_ENABLE));\n> +}\n> +\n> +static int ucd9000_gpio_set_direction(struct gpio_chip *gc, unsigned int offset,\n> +\t\t\t\t\tbool direction_out, int requested_out)\n> +{\n> +\tstruct i2c_client *client = gpiochip_get_data(gc);\n> +\tint ret, config, out_val;\n> +\n> +\n> +\tret = ucd9000_gpio_read_config(client, offset);\n> +\tif (ret < 0) {\n> +\t\tdev_err(&client->dev, \"failed to read GPIO %d config: %d\\n\",\n> +\t\t\toffset, ret);\n> +\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tif (direction_out) {\n> +\t\tout_val = requested_out ? UCD9000_GPIO_CONFIG_OUT_VALUE : 0;\n> +\n> +\t\tif (ret & UCD9000_GPIO_CONFIG_OUT_ENABLE) {\n> +\t\t\tif ((ret & UCD9000_GPIO_CONFIG_OUT_VALUE) == out_val)\n> +\t\t\t\treturn 0;\n> +\t\t} else\n> +\t\t\tret |= UCD9000_GPIO_CONFIG_OUT_ENABLE;\n> +\n> +\t\tif (out_val)\n> +\t\t\tret |= UCD9000_GPIO_CONFIG_OUT_VALUE;\n> +\t\telse\n> +\t\t\tret &= ~UCD9000_GPIO_CONFIG_OUT_VALUE;\n> +\n> +\t} else {\n> +\t\tif (!(ret & UCD9000_GPIO_CONFIG_OUT_ENABLE))\n> +\t\t\treturn 0;\n> +\n> +\t\tret &= ~UCD9000_GPIO_CONFIG_OUT_ENABLE;\n> +\t}\n> +\n> +\tret |= UCD9000_GPIO_CONFIG_ENABLE;\n> +\tconfig = ret;\n> +\n> +\t/* Page set not required */\n> +\tret = i2c_smbus_write_byte_data(client, UCD9000_GPIO_CONFIG, config);\n> +\tif (ret < 0) {\n> +\t\tdev_err(&client->dev, \"Failed to write GPIO %d config: %d\\n\",\n> +\t\t\toffset, ret);\n> +\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tconfig &= ~UCD9000_GPIO_CONFIG_ENABLE;\n> +\n> +\treturn i2c_smbus_write_byte_data(client, UCD9000_GPIO_CONFIG, config);\n> +}\n> +\n> +static int ucd9000_gpio_direction_input(struct gpio_chip *gc,\n> +\t\t\t\t\tunsigned int offset)\n> +{\n> +\treturn ucd9000_gpio_set_direction(gc, offset, UCD9000_GPIO_INPUT, 0);\n> +}\n> +\n> +static int ucd9000_gpio_direction_output(struct gpio_chip *gc,\n> +\t\t\t\t\tunsigned int offset, int val)\n> +{\n> +\treturn ucd9000_gpio_set_direction(gc, offset, UCD9000_GPIO_OUTPUT, val);\n> +}\n> +\n>   static ssize_t ucd9000_clear_logged_faults(struct device *dev,\n>   \t\t\t\tstruct device_attribute *attr, const char *buf,\n>   \t\t\t\tsize_t count)\n> @@ -261,6 +436,37 @@ static int ucd9000_probe(struct i2c_client *client,\n>   \t\t  | PMBUS_HAVE_FAN34 | PMBUS_HAVE_STATUS_FAN34;\n>   \t}\n>\n> +\t/*\n> +\t * Note:\n> +\t *\n> +\t * Pinmux support has not been added to the new gpio_chip.\n> +\t * This support should be added when possible given the mux\n> +\t * behavior of these IO devices.\n> +\t */\n> +\tdata->gpio.label = (const char *)&client->name;\n> +\tdata->gpio.get_direction = ucd9000_gpio_get_direction;\n> +\tdata->gpio.direction_input = ucd9000_gpio_direction_input;\n> +\tdata->gpio.direction_output = ucd9000_gpio_direction_output;\n> +\tdata->gpio.get = ucd9000_gpio_get;\n> +\tdata->gpio.set = ucd9000_gpio_set;\n> +\tdata->gpio.can_sleep = 1;\n> +\tdata->gpio.base = -1;\n> +\n> +\t/*\n> +\t * TODO: set ngpio for ucd9000 devs that aren't 90160 type\n> +\t */\n> +\tif (mid->driver_data == ucd90160)\n> +\t\tdata->gpio.ngpio = UCD90160_NUM_GPIOS;\n> +\tdata->gpio.parent = &client->dev;\n> +\tdata->gpio.owner = THIS_MODULE;\n> +\n> +\tret = devm_gpiochip_add_data(&client->dev, &data->gpio, client);\n> +\tif (ret) {\n> +\t\tdata->gpio.parent = NULL;\n> +\t\tdev_warn(&client->dev, \"Could not add gpiochip: %d\\n\", ret);\n> +\t\treturn ret;\n> +\t}\n> +\n>   \tret = sysfs_create_group(&client->dev.kobj, &ucd9000_attr_group);\n>   \tif (ret < 0) {\n>   \t\tdev_warn(&client->dev, \"Failed to add sysfs files: %d\\n\", ret);","headers":{"Return-Path":"<openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org>","X-Original-To":["incoming@patchwork.ozlabs.org","openbmc@lists.ozlabs.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","openbmc@lists.ozlabs.org"],"Received":["from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\t(using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xjnSV0TBHz9sPm\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri,  1 Sep 2017 02:11:54 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xjnST6ndkzDqXp\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri,  1 Sep 2017 02:11:53 +1000 (AEST)","from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com\n\t[148.163.156.1])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\tby lists.ozlabs.org (Postfix) with ESMTPS id 3xjnSP2K79zDqGX\n\tfor <openbmc@lists.ozlabs.org>; Fri,  1 Sep 2017 02:11:49 +1000 (AEST)","from pps.filterd (m0098409.ppops.net [127.0.0.1])\n\tby mx0a-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id\n\tv7VGALdE078535\n\tfor <openbmc@lists.ozlabs.org>; Thu, 31 Aug 2017 12:11:47 -0400","from e13.ny.us.ibm.com (e13.ny.us.ibm.com [129.33.205.203])\n\tby mx0a-001b2d01.pphosted.com with ESMTP id 2cphwwxsw7-1\n\t(version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT)\n\tfor <openbmc@lists.ozlabs.org>; Thu, 31 Aug 2017 12:11:46 -0400","from localhost\n\tby e13.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use\n\tOnly! Violators will be prosecuted\n\tfor <openbmc@lists.ozlabs.org> from <mspinler@linux.vnet.ibm.com>;\n\tThu, 31 Aug 2017 12:11:45 -0400","from b01cxnp22036.gho.pok.ibm.com (9.57.198.26)\n\tby e13.ny.us.ibm.com (146.89.104.200) with IBM ESMTP SMTP Gateway:\n\tAuthorized Use Only! Violators will be prosecuted; \n\tThu, 31 Aug 2017 12:11:43 -0400","from b01ledav005.gho.pok.ibm.com (b01ledav005.gho.pok.ibm.com\n\t[9.57.199.110])\n\tby b01cxnp22036.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP\n\tid v7VGBg6R28573778; Thu, 31 Aug 2017 16:11:43 GMT","from b01ledav005.gho.pok.ibm.com (unknown [127.0.0.1])\n\tby IMSVA (Postfix) with ESMTP id BB6D5AE04E;\n\tThu, 31 Aug 2017 12:12:07 -0400 (EDT)","from [9.10.99.183] (unknown [9.10.99.183])\n\tby b01ledav005.gho.pok.ibm.com (Postfix) with ESMTP id 85789AE03C;\n\tThu, 31 Aug 2017 12:12:07 -0400 (EDT)"],"Subject":"Re: [PATCH linux dev-4.10 v6 2/3] hwmon: (ucd9000) Add gpio chip\n\tinterface","To":"Christopher Bostic <cbostic@linux.vnet.ibm.com>","References":"<20170830214953.73327-1-cbostic@linux.vnet.ibm.com>\n\t<20170830214953.73327-3-cbostic@linux.vnet.ibm.com>","From":"Matt Spinler <mspinler@linux.vnet.ibm.com>","Date":"Thu, 31 Aug 2017 11:11:43 -0500","User-Agent":"Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101\n\tThunderbird/45.8.0","MIME-Version":"1.0","In-Reply-To":"<20170830214953.73327-3-cbostic@linux.vnet.ibm.com>","Content-Type":"text/plain; charset=windows-1252; format=flowed","Content-Transfer-Encoding":"7bit","X-TM-AS-GCONF":"00","x-cbid":"17083116-0008-0000-0000-000002776EFC","X-IBM-SpamModules-Scores":"","X-IBM-SpamModules-Versions":"BY=3.00007642; HX=3.00000241; KW=3.00000007;\n\tPH=3.00000004; SC=3.00000226; SDB=6.00910273; UDB=6.00456616;\n\tIPR=6.00690553; \n\tBA=6.00005563; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009;\n\tZB=6.00000000; \n\tZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00016945;\n\tXFM=3.00000015; UTC=2017-08-31 16:11:45","X-IBM-AV-DETECTION":"SAVI=unused REMOTE=unused XFE=unused","x-cbparentid":"17083116-0009-0000-0000-0000368CDDF9","Message-Id":"<2b79cfa6-7dd3-553f-6925-eb127a9577cd@linux.vnet.ibm.com>","X-Proofpoint-Virus-Version":"vendor=fsecure engine=2.50.10432:, ,\n\tdefinitions=2017-08-31_06:, , signatures=0","X-Proofpoint-Spam-Details":"rule=outbound_notspam policy=outbound score=0\n\tspamscore=0 suspectscore=0\n\tmalwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam\n\tadjust=0 reason=mlx scancount=1 engine=8.0.1-1707230000\n\tdefinitions=main-1708310239","X-BeenThere":"openbmc@lists.ozlabs.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"Development list for OpenBMC <openbmc.lists.ozlabs.org>","List-Unsubscribe":"<https://lists.ozlabs.org/options/openbmc>,\n\t<mailto:openbmc-request@lists.ozlabs.org?subject=unsubscribe>","List-Archive":"<http://lists.ozlabs.org/pipermail/openbmc/>","List-Post":"<mailto:openbmc@lists.ozlabs.org>","List-Help":"<mailto:openbmc-request@lists.ozlabs.org?subject=help>","List-Subscribe":"<https://lists.ozlabs.org/listinfo/openbmc>,\n\t<mailto:openbmc-request@lists.ozlabs.org?subject=subscribe>","Cc":"openbmc@lists.ozlabs.org","Errors-To":"openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"openbmc\"\n\t<openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org>"}}]