[{"id":1759839,"web_url":"http://patchwork.ozlabs.org/comment/1759839/","msgid":"<1504064653.5933.16.camel@aj.id.au>","list_archive_url":null,"date":"2017-08-30T03:44:13","subject":"Re: [PATCH linux dev-4.10 v5 1/2] hwmon: (ucd9000) Add gpio chip\n\tinterface and clear logged faults","submitter":{"id":68332,"url":"http://patchwork.ozlabs.org/api/people/68332/","name":"Andrew Jeffery","email":"andrew@aj.id.au"},"content":"Hi, Chris,\n\nOn Tue, 2017-08-29 at 15:08 -0500, 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> Add capability to clear logged faults via sysfs.\n> \n> > Signed-off-by: Christopher Bostic \n> ---\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_fault to gpi_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 | 250 +++++++++++++++++++++++++++++++++++++++++-\n>  1 file changed, 248 insertions(+), 2 deletions(-)\n> \n> diff --git a/drivers/hwmon/pmbus/ucd9000.c b/drivers/hwmon/pmbus/ucd9000.c\n> index 3e3aa95..cfd7703 100644\n> --- a/drivers/hwmon/pmbus/ucd9000.c\n> +++ b/drivers/hwmon/pmbus/ucd9000.c\n> @@ -26,6 +26,9 @@\n>  #include \n>  #include \n>  #include \n> +#include \n> +#include \n> +#include \n>  #include \"pmbus.h\"\n>  \n>  enum chips { ucd9000, ucd90120, ucd90124, ucd90160, ucd9090, ucd90910 };\n> @@ -34,8 +37,19 @@\n> >  #define UCD9000_NUM_PAGES\t\t0xd6\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> @@ -46,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> @@ -119,6 +137,197 @@ 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\nWhy are we doing bit-wise negation after boolean double negation? That looks\nwrong. I would expect to only do the boolean double negation.\n\nThe documentation says a return value of 0 is out, 1 is in, and negative is an\nerror. With the bit-wise negation this would give values of either -1 or -2,\nboth of which are errors.\n\n\t2 13:07:24 andrew@keelia:~$ cat /tmp/bits.c\n\t#include \n\n\tint main(void) {\n\t\tprintf(\"%d, %d\\n\", ~((int) 0), ~((int) 1));\n\n\t\treturn 0;\n\t}\n\t2 13:07:29 andrew@keelia:~$ make /tmp/bits\n\tcc     /tmp/bits.c   -o /tmp/bits\n\t2 13:07:31 andrew@keelia:~$ /tmp/bits\n\t-1, -2\n\t2 13:07:36 andrew@keelia:~$\n\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\nThanks for implementing the direction switch. With the early exits it is a bit\nmore involved than I was anticipating, but certainly we want to avoid the\ndouble-write if we can.\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> +{\n> > +\tstruct i2c_client *client = to_i2c_client(dev);\n> > +\tint ret;\n> +\n> > +\t/* No page set required */\n> > +\tret = i2c_smbus_write_byte_data(client, UCD9000_LOGGED_FAULTS, 0);\n> > +\tif (ret) {\n> > +\t\tdev_err(&client->dev, \"Failed to clear logged faults: %d\\n\",\n> > +\t\t\tret);\n> +\n> > +\t\treturn ret;\n> > +\t}\n> +\n> > +\treturn count;\n> +}\n> +\n> +static DEVICE_ATTR(clear_logged_faults, 0200, NULL,\n> > +\t\tucd9000_clear_logged_faults);\n> +\n> +static struct attribute *ucd9000_attributes[] = {\n> > +\t&dev_attr_clear_logged_faults.attr,\n> > +\tNULL\n> +};\n> +\n> +static const struct attribute_group ucd9000_attr_group = {\n> > +\t.attrs = ucd9000_attributes,\n> +};\n\nWe discussed splitting ucd9000_clear_logged_faults() and associated bits out\ninto a separate patch so it could be sent upstream for review. Can you talk\nabout why it's still included?\n\n> +\n>  static int ucd9000_probe(struct i2c_client *client,\n> >  \t\t\t const struct i2c_device_id *id)\n>  {\n> @@ -227,7 +436,44 @@ static int ucd9000_probe(struct i2c_client *client,\n> >  \t\t  | PMBUS_HAVE_FAN34 | PMBUS_HAVE_STATUS_FAN34;\n> >  \t}\n>  \n> > -\treturn pmbus_do_probe(client, mid, info);\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);\n> +\n> > +\t\treturn ret;\n> > +\t}\n> +\n> > +\treturn  pmbus_do_probe(client, mid, info);\n> +}\n> +\n> +static int ucd9000_remove(struct i2c_client *client)\n> +{\n> > +\tsysfs_remove_group(&client->dev.kobj, &ucd9000_attr_group);\n> > +\treturn pmbus_do_remove(client);\n>  }\n>  \n>  /* This is the driver that will be inserted */\n> @@ -236,7 +482,7 @@ static int ucd9000_probe(struct i2c_client *client,\n> >  \t\t.name = \"ucd9000\",\n> >  \t},\n> >  \t.probe = ucd9000_probe,\n> > -\t.remove = pmbus_do_remove,\n> > +\t.remove = ucd9000_remove,\n> >  \t.id_table = ucd9000_id,\n>  };\n>  \n\nThere's no comment about pinmux, which is another one of the assumptions we're\nmaking with this patch. In general I was trying to make the point that we\nshould document anything we're assuming. The mux state is another such thing.\n\nCheers,\n\nAndrew","headers":{"Return-Path":"","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 3xhrwb54fjz9sNr\n\tfor ;\n\tWed, 30 Aug 2017 13:44:31 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xhrwb3Q40zDqJ9\n\tfor ;\n\tWed, 30 Aug 2017 13:44:31 +1000 (AEST)","from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com\n\t[66.111.4.26])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby lists.ozlabs.org (Postfix) with ESMTPS id 3xhrwR3y9xzDqGG\n\tfor ; Wed, 30 Aug 2017 13:44:23 +1000 (AEST)","from compute4.internal (compute4.nyi.internal [10.202.2.44])\n\tby mailout.nyi.internal (Postfix) with ESMTP id EAA1420F78;\n\tTue, 29 Aug 2017 23:44:20 -0400 (EDT)","from frontend2 ([10.202.2.161])\n\tby compute4.internal (MEProxy); Tue, 29 Aug 2017 23:44:20 -0400","from keelia (unknown [203.0.153.9])\n\tby mail.messagingengine.com (Postfix) with ESMTPA id 0321624311;\n\tTue, 29 Aug 2017 23:44:18 -0400 (EDT)"],"Authentication-Results":["ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"WoiYr87X\";\n\tdkim=pass (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"VLz8dvfi\"; \n\tdkim-atps=neutral","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"WoiYr87X\";\n\tdkim=pass (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"VLz8dvfi\"; \n\tdkim-atps=neutral","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"WoiYr87X\";\n\tdkim=pass (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com\n\theader.b=\"VLz8dvfi\"; dkim-atps=neutral"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed; d=aj.id.au; h=cc\n\t:content-type:date:from:in-reply-to:message-id:mime-version\n\t:references:subject:to:x-me-sender:x-me-sender:x-sasl-enc\n\t:x-sasl-enc; s=fm1; bh=0aNRSaeQWB/s/YR5cAc6XFIvl1luu/v/PJM4oGQTu\n\tUI=; b=WoiYr87XJ4sX3kGaaCZyvwc5Y+A0XGVB216PGuTfrheLKhL+YmpbV9ZvK\n\tohLlP0jqW85fHxlEw6o/GzsypYEzQu4Hn7G+cKVpXfm1nHkzAhXRoBli0hKQBpH6\n\teby4IG4WDNJ0TPAjs8NXJhN2l08gLUhmqsOwvxS5uCBFCKXUIV83HK+3hDjbqFnm\n\tF6K4SaSKFbfgpNL66zFSBQ4Ds9UvLyWckaDzjWUwYu6csxyY2ii8bwpX2npP+XlQ\n\tz8fTTUV5bHP9KU2H1orgTqmLWaYf60wxCOZYLx4lrBVA3JcCpuycvrle79ebo+zK\n\tdkY4J2kseV3l9t0JuzKX24mGSTszQ==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=\n\tmessagingengine.com; h=cc:content-type:date:from:in-reply-to\n\t:message-id:mime-version:references:subject:to:x-me-sender\n\t:x-me-sender:x-sasl-enc:x-sasl-enc; s=fm1; bh=0aNRSaeQWB/s/YR5cA\n\tc6XFIvl1luu/v/PJM4oGQTuUI=; b=VLz8dvficFVdyBsdAMgCfNPVwTaAWQdHhL\n\trWa+DsZcrxRDyaMHK4MEOGqK63RcJjNM5j0Lad1TddfoTvT09VI8ucGAE6tHZc7H\n\tmClM15mUGiXkb9AooAFs67uv6i5QhCgGTDzi4ecMr8eHeaE5KCPEkE2/1SfvhRq6\n\tgxVZNi87no6rhXsMCtl5zz9lLuEZP8MuMfhTObIUy8rw+GJd55O+HvwmkfDeszAc\n\t3Jbr2vDlh0u0PoPOKOfpgL7OneyqPDPLnOk+EHwX4mBuYwkXT1VhtoWO00yyZjHf\n\t4sKE24J7Ri8/r/9lYLDbz2YZiLpa7r2PeJIFVirfUD8WkSnfczvA=="],"X-ME-Sender":"","X-Sasl-enc":"5yI+ydeNRNgu9Io5xFkUnZk7IFGifN7R0lqa2Hm75mj9 1504064660","Message-ID":"<1504064653.5933.16.camel@aj.id.au>","Subject":"Re: [PATCH linux dev-4.10 v5 1/2] hwmon: (ucd9000) Add gpio chip\n\tinterface and clear logged faults","From":"Andrew Jeffery ","To":"Christopher Bostic , joel@jms.id.au","Date":"Wed, 30 Aug 2017 13:14:13 +0930","In-Reply-To":"<20170829200813.66010-2-cbostic@linux.vnet.ibm.com>","References":"<20170829200813.66010-1-cbostic@linux.vnet.ibm.com>\n\t<20170829200813.66010-2-cbostic@linux.vnet.ibm.com>","Content-Type":"multipart/signed; micalg=\"pgp-sha512\";\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"=-i37srN1KaP7rJ5Wo+6In\"","X-Mailer":"Evolution 3.22.6-1ubuntu1 ","Mime-Version":"1.0","X-BeenThere":"openbmc@lists.ozlabs.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"Development list for OpenBMC ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"openbmc@lists.ozlabs.org","Errors-To":"openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"openbmc\"\n\t"}},{"id":1760425,"web_url":"http://patchwork.ozlabs.org/comment/1760425/","msgid":"","list_archive_url":null,"date":"2017-08-30T19:29:00","subject":"Re: [PATCH linux dev-4.10 v5 1/2] hwmon: (ucd9000) Add gpio chip\n\tinterface and clear logged faults","submitter":{"id":70879,"url":"http://patchwork.ozlabs.org/api/people/70879/","name":"Christopher Bostic","email":"cbostic@linux.vnet.ibm.com"},"content":"On 8/29/17 10:44 PM, Andrew Jeffery wrote:\n> Hi, Chris,\n>\n> On Tue, 2017-08-29 at 15:08 -0500, 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>> Add capability to clear logged faults via sysfs.\n>> \n>>> Signed-off-by: Christopher Bostic \n>> ---\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_fault to gpi_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 | 250 +++++++++++++++++++++++++++++++++++++++++-\n>> 1 file changed, 248 insertions(+), 2 deletions(-)\n>> \n>> diff --git a/drivers/hwmon/pmbus/ucd9000.c b/drivers/hwmon/pmbus/ucd9000.c\n>> index 3e3aa95..cfd7703 100644\n>> --- a/drivers/hwmon/pmbus/ucd9000.c\n>> +++ b/drivers/hwmon/pmbus/ucd9000.c\n>> @@ -26,6 +26,9 @@\n>> #include \n>> #include \n>> #include \n>> +#include \n>> +#include \n>> +#include \n>> #include \"pmbus.h\"\n>> \n>> enum chips { ucd9000, ucd90120, ucd90124, ucd90160, ucd9090, ucd90910 };\n>> @@ -34,8 +37,19 @@\n>>> #define UCD9000_NUM_PAGES\t\t0xd6\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>> @@ -46,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>> @@ -119,6 +137,197 @@ 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> Why are we doing bit-wise negation after boolean double negation? That looks\n> wrong. I would expect to only do the boolean double negation.\n>\n> The documentation says a return value of 0 is out, 1 is in, and negative is an\n> error. With the bit-wise negation this would give values of either -1 or -2,\n> both of which are errors.\n>\n> \t2 13:07:24 andrew@keelia:~$ cat /tmp/bits.c\n> \t#include \n>\n> \tint main(void) {\n> \t\tprintf(\"%d, %d\\n\", ~((int) 0), ~((int) 1));\n>\n> \t\treturn 0;\n> \t}\n> \t2 13:07:29 andrew@keelia:~$ make /tmp/bits\n> \tcc /tmp/bits.c -o /tmp/bits\n> \t2 13:07:31 andrew@keelia:~$ /tmp/bits\n> \t-1, -2\n> \t2 13:07:36 andrew@keelia:~$\n>\nHi Andrew,\n\nYeah this is a problem. My tests passed but clearly this is wrong. \nWill fix.\n\n\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> Thanks for implementing the direction switch. With the early exits it is a bit\n> more involved than I was anticipating, but certainly we want to avoid the\n> double-write if we can.\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>> +{\n>>> +\tstruct i2c_client *client = to_i2c_client(dev);\n>>> +\tint ret;\n>> +\n>>> +\t/* No page set required */\n>>> +\tret = i2c_smbus_write_byte_data(client, UCD9000_LOGGED_FAULTS, 0);\n>>> +\tif (ret) {\n>>> +\t\tdev_err(&client->dev, \"Failed to clear logged faults: %d\\n\",\n>>> +\t\t\tret);\n>> +\n>>> +\t\treturn ret;\n>>> +\t}\n>> +\n>>> +\treturn count;\n>> +}\n>> +\n>> +static DEVICE_ATTR(clear_logged_faults, 0200, NULL,\n>>> +\t\tucd9000_clear_logged_faults);\n>> +\n>> +static struct attribute *ucd9000_attributes[] = {\n>>> +\t&dev_attr_clear_logged_faults.attr,\n>>> +\tNULL\n>> +};\n>> +\n>> +static const struct attribute_group ucd9000_attr_group = {\n>>> +\t.attrs = ucd9000_attributes,\n>> +};\n> We discussed splitting ucd9000_clear_logged_faults() and associated bits out\n> into a separate patch so it could be sent upstream for review. Can you talk\n> about why it's still included?\n\nI broke out the ucd9000_clear_logged_faults( ) in the community patch \nbut not for this set. I will do so for next version.\n>> +\n>> static int ucd9000_probe(struct i2c_client *client,\n>>> \t\t\t const struct i2c_device_id *id)\n>> {\n>> @@ -227,7 +436,44 @@ static int ucd9000_probe(struct i2c_client *client,\n>>> \t\t | PMBUS_HAVE_FAN34 | PMBUS_HAVE_STATUS_FAN34;\n>>> \t}\n>> \n>>> -\treturn pmbus_do_probe(client, mid, info);\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);\n>> +\n>>> +\t\treturn ret;\n>>> +\t}\n>> +\n>>> +\treturn pmbus_do_probe(client, mid, info);\n>> +}\n>> +\n>> +static int ucd9000_remove(struct i2c_client *client)\n>> +{\n>>> +\tsysfs_remove_group(&client->dev.kobj, &ucd9000_attr_group);\n>>> +\treturn pmbus_do_remove(client);\n>> }\n>> \n>> /* This is the driver that will be inserted */\n>> @@ -236,7 +482,7 @@ static int ucd9000_probe(struct i2c_client *client,\n>>> \t\t.name = \"ucd9000\",\n>>> \t},\n>>> \t.probe = ucd9000_probe,\n>>> -\t.remove = pmbus_do_remove,\n>>> +\t.remove = ucd9000_remove,\n>>> \t.id_table = ucd9000_id,\n>> };\n>> \n> There's no comment about pinmux, which is another one of the assumptions we're\n> making with this patch. In general I was trying to make the point that we\n> should document anything we're assuming. The mux state is another such thing.\nWill add that to the documented assumptions.\n\nThanks,\nChris\n\n>\n> Cheers,\n>\n> Andrew","headers":{"Return-Path":"","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 3xjFtq1zXmz9sP5\n\tfor ;\n\tThu, 31 Aug 2017 05:29:23 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xjFtq0vmhzDqTt\n\tfor ;\n\tThu, 31 Aug 2017 05:29:23 +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 3xjFtc6ljRzDqN4\n\tfor ; Thu, 31 Aug 2017 05:29:12 +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\tv7UJT4j4023548\n\tfor ; Wed, 30 Aug 2017 15:29:09 -0400","from e12.ny.us.ibm.com (e12.ny.us.ibm.com [129.33.205.202])\n\tby mx0a-001b2d01.pphosted.com with ESMTP id 2cnyn8wcsp-1\n\t(version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT)\n\tfor ; Wed, 30 Aug 2017 15:29:09 -0400","from localhost\n\tby e12.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use\n\tOnly! Violators will be prosecuted\n\tfor from ;\n\tWed, 30 Aug 2017 15:29:07 -0400","from b01cxnp22036.gho.pok.ibm.com (9.57.198.26)\n\tby e12.ny.us.ibm.com (146.89.104.199) with IBM ESMTP SMTP Gateway:\n\tAuthorized Use Only! Violators will be prosecuted; \n\tWed, 30 Aug 2017 15:29:05 -0400","from b01ledav003.gho.pok.ibm.com (b01ledav003.gho.pok.ibm.com\n\t[9.57.199.108])\n\tby b01cxnp22036.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP\n\tid v7UJT58p31522870; Wed, 30 Aug 2017 19:29:05 GMT","from b01ledav003.gho.pok.ibm.com (unknown [127.0.0.1])\n\tby IMSVA (Postfix) with ESMTP id 26BDDB2046;\n\tWed, 30 Aug 2017 15:26:29 -0400 (EDT)","from christophersmbp.austin.ibm.com (unknown [9.83.1.36])\n\tby b01ledav003.gho.pok.ibm.com (Postfix) with ESMTP id 7AA5FB204E;\n\tWed, 30 Aug 2017 15:26:26 -0400 (EDT)"],"Subject":"Re: [PATCH linux dev-4.10 v5 1/2] hwmon: (ucd9000) Add gpio chip\n\tinterface and clear logged faults","To":"Andrew Jeffery , joel@jms.id.au","References":"<20170829200813.66010-1-cbostic@linux.vnet.ibm.com>\n\t<20170829200813.66010-2-cbostic@linux.vnet.ibm.com>\n\t<1504064653.5933.16.camel@aj.id.au>","From":"Christopher Bostic ","Date":"Wed, 30 Aug 2017 14:29:00 -0500","User-Agent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:45.0)\n\tGecko/20100101 Thunderbird/45.7.0","MIME-Version":"1.0","In-Reply-To":"<1504064653.5933.16.camel@aj.id.au>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"7bit","X-TM-AS-GCONF":"00","x-cbid":"17083019-0048-0000-0000-000001DBE228","X-IBM-SpamModules-Scores":"","X-IBM-SpamModules-Versions":"BY=3.00007637; HX=3.00000241; KW=3.00000007;\n\tPH=3.00000004; SC=3.00000226; SDB=6.00909859; UDB=6.00456367;\n\tIPR=6.00690143; \n\tBA=6.00005562; 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.00016931;\n\tXFM=3.00000015; UTC=2017-08-30 19:29:07","X-IBM-AV-DETECTION":"SAVI=unused REMOTE=unused XFE=unused","x-cbparentid":"17083019-0049-0000-0000-000042667C46","Message-Id":"","X-Proofpoint-Virus-Version":"vendor=fsecure engine=2.50.10432:, ,\n\tdefinitions=2017-08-30_08:, , 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-1708300296","X-BeenThere":"openbmc@lists.ozlabs.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"Development list for OpenBMC ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"openbmc@lists.ozlabs.org","Errors-To":"openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"openbmc\"\n\t"}}]