[{"id":1763507,"web_url":"http://patchwork.ozlabs.org/comment/1763507/","msgid":"<20170905170002.GG11478@roeck-us.net>","list_archive_url":null,"date":"2017-09-05T17:00:02","subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","submitter":{"id":21889,"url":"http://patchwork.ozlabs.org/api/people/21889/","name":"Guenter Roeck","email":"linux@roeck-us.net"},"content":"On Tue, Sep 05, 2017 at 05:01:32PM +1000, Andrew Jeffery wrote:\n> Some functions exposed by pmbus core conflated errors that occurred when\n> setting the page to access with errors that occurred when accessing\n> registers in a page. In some cases, this caused legitimate errors to be\n> hidden under the guise of the register not being supported.\n> \n\nI'll have to look into this. If I recall correctly, the reason for not returning\nerrors from the \"clear faults\" command was that some early chips did not support\nit, or did not support it on all pages. Those chips would now always fail.\n\nOn a higher level, I am not sure if it is a good idea to return an error\nfrom a function intended to clear faults (and nothing else). That seems\ncounterproductive.\n\nIs this a problem you have actually observed ? If so, what is the chip ?\n\nThanks,\nGuenter\n\n> Signed-off-by: Andrew Jeffery \n> ---\n> Documentation/hwmon/pmbus-core | 12 ++--\n> drivers/hwmon/pmbus/pmbus.h | 6 +-\n> drivers/hwmon/pmbus/pmbus_core.c | 115 +++++++++++++++++++++++++++++----------\n> 3 files changed, 95 insertions(+), 38 deletions(-)\n> \n> diff --git a/Documentation/hwmon/pmbus-core b/Documentation/hwmon/pmbus-core\n> index 8ed10e9ddfb5..3e9f41bb756f 100644\n> --- a/Documentation/hwmon/pmbus-core\n> +++ b/Documentation/hwmon/pmbus-core\n> @@ -218,17 +218,17 @@ Specifically, it provides the following information.\n> This function calls the device specific write_byte function if defined.\n> Therefore, it must _not_ be called from that function.\n> \n> - bool pmbus_check_byte_register(struct i2c_client *client, int page, int reg);\n> + int pmbus_check_byte_register(struct i2c_client *client, int page, int reg);\n> \n> - Check if byte register exists. Return true if the register exists, false\n> - otherwise.\n> + Check if byte register exists. Returns 1 if the register exists, 0 if it does\n> + not, and less than zero on an unexpected error.\n> This function calls the device specific write_byte function if defined to\n> obtain the chip status. Therefore, it must _not_ be called from that function.\n> \n> - bool pmbus_check_word_register(struct i2c_client *client, int page, int reg);\n> + int pmbus_check_word_register(struct i2c_client *client, int page, int reg);\n> \n> - Check if word register exists. Return true if the register exists, false\n> - otherwise.\n> + Check if word register exists. Returns 1 if the register exists, 0 if it does\n> + not, and less than zero on an unexpected error.\n> This function calls the device specific write_byte function if defined to\n> obtain the chip status. Therefore, it must _not_ be called from that function.\n> \n> diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h\n> index bfcb13bae34b..c53032a04a6f 100644\n> --- a/drivers/hwmon/pmbus/pmbus.h\n> +++ b/drivers/hwmon/pmbus/pmbus.h\n> @@ -413,9 +413,9 @@ int pmbus_write_byte_data(struct i2c_client *client, int page, u8 reg,\n> \t\t\t u8 value);\n> int pmbus_update_byte_data(struct i2c_client *client, int page, u8 reg,\n> \t\t\t u8 mask, u8 value);\n> -void pmbus_clear_faults(struct i2c_client *client);\n> -bool pmbus_check_byte_register(struct i2c_client *client, int page, int reg);\n> -bool pmbus_check_word_register(struct i2c_client *client, int page, int reg);\n> +int pmbus_clear_faults(struct i2c_client *client);\n> +int pmbus_check_byte_register(struct i2c_client *client, int page, int reg);\n> +int pmbus_check_word_register(struct i2c_client *client, int page, int reg);\n> int pmbus_do_probe(struct i2c_client *client, const struct i2c_device_id *id,\n> \t\t struct pmbus_driver_info *info);\n> int pmbus_do_remove(struct i2c_client *client);\n> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c\n> index f1eff6b6c798..153700e35431 100644\n> --- a/drivers/hwmon/pmbus/pmbus_core.c\n> +++ b/drivers/hwmon/pmbus/pmbus_core.c\n> @@ -304,18 +304,24 @@ static int _pmbus_read_byte_data(struct i2c_client *client, int page, int reg)\n> \treturn pmbus_read_byte_data(client, page, reg);\n> }\n> \n> -static void pmbus_clear_fault_page(struct i2c_client *client, int page)\n> +static int pmbus_clear_fault_page(struct i2c_client *client, int page)\n> {\n> -\t_pmbus_write_byte(client, page, PMBUS_CLEAR_FAULTS);\n> +\treturn _pmbus_write_byte(client, page, PMBUS_CLEAR_FAULTS);\n> }\n> \n> -void pmbus_clear_faults(struct i2c_client *client)\n> +int pmbus_clear_faults(struct i2c_client *client)\n> {\n> \tstruct pmbus_data *data = i2c_get_clientdata(client);\n> +\tint rv;\n> \tint i;\n> \n> -\tfor (i = 0; i < data->info->pages; i++)\n> -\t\tpmbus_clear_fault_page(client, i);\n> +\tfor (i = 0; i < data->info->pages; i++) {\n> +\t\trv = pmbus_clear_fault_page(client, i);\n> +\t\tif (rv)\n> +\t\t\treturn rv;\n> +\t}\n> +\n> +\treturn 0;\n> }\n> EXPORT_SYMBOL_GPL(pmbus_clear_faults);\n> \n> @@ -333,28 +339,45 @@ static int pmbus_check_status_cml(struct i2c_client *client)\n> \treturn 0;\n> }\n> \n> -static bool pmbus_check_register(struct i2c_client *client,\n> +static int pmbus_check_register(struct i2c_client *client,\n> \t\t\t\t int (*func)(struct i2c_client *client,\n> \t\t\t\t\t int page, int reg),\n> \t\t\t\t int page, int reg)\n> {\n> +\tstruct pmbus_data *data;\n> +\tint check;\n> \tint rv;\n> -\tstruct pmbus_data *data = i2c_get_clientdata(client);\n> \n> -\trv = func(client, page, reg);\n> -\tif (rv >= 0 && !(data->flags & PMBUS_SKIP_STATUS_CHECK))\n> -\t\trv = pmbus_check_status_cml(client);\n> -\tpmbus_clear_fault_page(client, -1);\n> -\treturn rv >= 0;\n> +\tdata = i2c_get_clientdata(client);\n> +\n> +\t/*\n> +\t * pmbus_set_page() guards transactions on the requested page matching\n> +\t * the current page. This may be done in the execution of func(), but\n> +\t * at that point a set-page error is conflated with accessing a\n> +\t * non-existent register.\n> +\t */\n> +\trv = pmbus_set_page(client, page);\n> +\tif (rv < 0)\n> +\t\treturn rv;\n> +\n> +\tcheck = func(client, page, reg);\n> +\tif (check >= 0 && !(data->flags & PMBUS_SKIP_STATUS_CHECK))\n> +\t\tcheck = pmbus_check_status_cml(client);\n> +\n> +\trv = pmbus_clear_fault_page(client, -1);\n> +\tif (rv < 0)\n> +\t\treturn rv;\n> +\n> +\treturn check >= 0;\n> }\n> \n> -bool pmbus_check_byte_register(struct i2c_client *client, int page, int reg)\n> +int pmbus_check_byte_register(struct i2c_client *client, int page, int reg)\n> {\n> \treturn pmbus_check_register(client, _pmbus_read_byte_data, page, reg);\n> }\n> EXPORT_SYMBOL_GPL(pmbus_check_byte_register);\n> \n> -bool pmbus_check_word_register(struct i2c_client *client, int page, int reg)\n> +int pmbus_check_word_register(struct i2c_client *client, int page, int reg)\n> {\n> \treturn pmbus_check_register(client, _pmbus_read_word_data, page, reg);\n> }\n> @@ -390,7 +413,7 @@ static struct pmbus_data *pmbus_update_device(struct device *dev)\n> \n> \tmutex_lock(&data->update_lock);\n> \tif (time_after(jiffies, data->last_updated + HZ) || !data->valid) {\n> -\t\tint i, j;\n> +\t\tint i, j, ret;\n> \n> \t\tfor (i = 0; i < info->pages; i++) {\n> \t\t\tdata->status[PB_STATUS_BASE + i]\n> @@ -424,7 +447,13 @@ static struct pmbus_data *pmbus_update_device(struct device *dev)\n> \t\t\t\t\t\t\t sensor->page,\n> \t\t\t\t\t\t\t sensor->reg);\n> \t\t}\n> -\t\tpmbus_clear_faults(client);\n> +\n> +\t\tret = pmbus_clear_faults(client);\n> +\t\tif (ret < 0) {\n> +\t\t\tmutex_unlock(&data->update_lock);\n> +\t\t\treturn ERR_PTR(ret);\n> +\t\t}\n> +\n> \t\tdata->last_updated = jiffies;\n> \t\tdata->valid = 1;\n> \t}\n> @@ -754,6 +783,9 @@ static ssize_t pmbus_show_boolean(struct device *dev,\n> \tstruct pmbus_data *data = pmbus_update_device(dev);\n> \tint val;\n> \n> +\tif (IS_ERR(data))\n> +\t\treturn PTR_ERR(data);\n> +\n> \tval = pmbus_get_boolean(data, boolean, attr->index);\n> \tif (val < 0)\n> \t\treturn val;\n> @@ -766,6 +798,9 @@ static ssize_t pmbus_show_sensor(struct device *dev,\n> \tstruct pmbus_data *data = pmbus_update_device(dev);\n> \tstruct pmbus_sensor *sensor = to_pmbus_sensor(devattr);\n> \n> +\tif (IS_ERR(data))\n> +\t\treturn PTR_ERR(data);\n> +\n> \tif (sensor->data < 0)\n> \t\treturn sensor->data;\n> \n> @@ -995,7 +1030,11 @@ static int pmbus_add_limit_attrs(struct i2c_client *client,\n> \tstruct pmbus_sensor *curr;\n> \n> \tfor (i = 0; i < nlimit; i++) {\n> -\t\tif (pmbus_check_word_register(client, page, l->reg)) {\n> +\t\tret = pmbus_check_word_register(client, page, l->reg);\n> +\t\tif (ret < 0)\n> +\t\t\treturn ret;\n> +\n> +\t\tif (ret) {\n> \t\t\tcurr = pmbus_add_sensor(data, name, l->attr, index,\n> \t\t\t\t\t\tpage, l->reg, attr->class,\n> \t\t\t\t\t\tattr->update || l->update,\n> @@ -1041,6 +1080,8 @@ static int pmbus_add_sensor_attrs_one(struct i2c_client *client,\n> \tif (!base)\n> \t\treturn -ENOMEM;\n> \tif (attr->sfunc) {\n> +\t\tint check;\n> +\n> \t\tret = pmbus_add_limit_attrs(client, data, info, name,\n> \t\t\t\t\t index, page, base, attr);\n> \t\tif (ret < 0)\n> @@ -1050,9 +1091,13 @@ static int pmbus_add_sensor_attrs_one(struct i2c_client *client,\n> \t\t * alarm attributes, if there is a global alarm bit, and if\n> \t\t * the generic status register for this page is accessible.\n> \t\t */\n> -\t\tif (!ret && attr->gbit &&\n> -\t\t pmbus_check_byte_register(client, page,\n> -\t\t\t\t\t data->status_register)) {\n> +\n> +\t\tcheck = pmbus_check_byte_register(client, page,\n> +\t\t\t\t\t\t data->status_register);\n> +\t\tif (check < 0)\n> +\t\t\treturn check;\n> +\n> +\t\tif (!ret && attr->gbit && check) {\n> \t\t\tret = pmbus_add_boolean(data, name, \"alarm\", index,\n> \t\t\t\t\t\tNULL, NULL,\n> \t\t\t\t\t\tPB_STATUS_BASE + page,\n> @@ -1604,8 +1649,12 @@ static int pmbus_add_fan_attributes(struct i2c_client *client,\n> \t\t\tif (!(info->func[page] & pmbus_fan_flags[f]))\n> \t\t\t\tbreak;\n> \n> -\t\t\tif (!pmbus_check_word_register(client, page,\n> -\t\t\t\t\t\t pmbus_fan_registers[f]))\n> +\t\t\tret = pmbus_check_word_register(client, page,\n> +\t\t\t\t\t\t pmbus_fan_registers[f]);\n> +\t\t\tif (ret < 0)\n> +\t\t\t\treturn ret;\n> +\n> +\t\t\tif (!ret)\n> \t\t\t\tbreak;\n> \n> \t\t\t/*\n> @@ -1628,9 +1677,13 @@ static int pmbus_add_fan_attributes(struct i2c_client *client,\n> \t\t\t * Each fan status register covers multiple fans,\n> \t\t\t * so we have to do some magic.\n> \t\t\t */\n> -\t\t\tif ((info->func[page] & pmbus_fan_status_flags[f]) &&\n> -\t\t\t pmbus_check_byte_register(client,\n> -\t\t\t\t\tpage, pmbus_fan_status_registers[f])) {\n> +\t\t\tret = pmbus_check_byte_register(client, page,\n> +\t\t\t\t\t\tpmbus_fan_status_registers[f]);\n> +\t\t\tif (ret < 0)\n> +\t\t\t\treturn ret;\n> +\n> +\t\t\tif ((info->func[page] & pmbus_fan_status_flags[f])\n> +\t\t\t\t\t&& ret) {\n> \t\t\t\tint base;\n> \n> \t\t\t\tif (f > 1)\t/* fan 3, 4 */\n> @@ -1696,10 +1749,13 @@ static int pmbus_identify_common(struct i2c_client *client,\n> \t\t\t\t struct pmbus_data *data, int page)\n> {\n> \tint vout_mode = -1;\n> +\tint rv;\n> \n> -\tif (pmbus_check_byte_register(client, page, PMBUS_VOUT_MODE))\n> +\trv = pmbus_check_byte_register(client, page, PMBUS_VOUT_MODE);\n> +\tif (rv == 1)\n> \t\tvout_mode = _pmbus_read_byte_data(client, page,\n> \t\t\t\t\t\t PMBUS_VOUT_MODE);\n> +\n> \tif (vout_mode >= 0 && vout_mode != 0xff) {\n> \t\t/*\n> \t\t * Not all chips support the VOUT_MODE command,\n> @@ -1725,8 +1781,7 @@ static int pmbus_identify_common(struct i2c_client *client,\n> \t\t}\n> \t}\n> \n> -\tpmbus_clear_fault_page(client, page);\n> -\treturn 0;\n> +\treturn pmbus_clear_fault_page(client, page);\n> }\n> \n> static int pmbus_init_common(struct i2c_client *client, struct pmbus_data *data,\n> @@ -1756,7 +1811,9 @@ static int pmbus_init_common(struct i2c_client *client, struct pmbus_data *data,\n> \tif (ret >= 0 && (ret & PB_CAPABILITY_ERROR_CHECK))\n> \t\tclient->flags |= I2C_CLIENT_PEC;\n> \n> -\tpmbus_clear_faults(client);\n> +\tret = pmbus_clear_faults(client);\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> \n> \tif (info->identify) {\n> \t\tret = (*info->identify)(client, info);\n> -- \n> 2.11.0\n>","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 [103.22.144.68])\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 3xmtJ42qzbz9sPt\n\tfor ;\n\tWed, 6 Sep 2017 03:00:20 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xmtJ34XlnzDrJZ\n\tfor ;\n\tWed, 6 Sep 2017 03:00:19 +1000 (AEST)","from mail-pg0-x242.google.com (mail-pg0-x242.google.com\n\t[IPv6:2607:f8b0:400e:c05::242])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128\n\tbits)) (No client certificate requested)\n\tby lists.ozlabs.org (Postfix) with ESMTPS id 3xmtHq58ByzDrFM\n\tfor ; Wed, 6 Sep 2017 03:00:06 +1000 (AEST)","by mail-pg0-x242.google.com with SMTP id v82so1626781pgb.1\n\tfor ; Tue, 05 Sep 2017 10:00:06 -0700 (PDT)","from localhost (108-223-40-66.lightspeed.sntcca.sbcglobal.net.\n\t[108.223.40.66]) by smtp.gmail.com with ESMTPSA id\n\tw7sm2785537pfb.124.2017.09.05.10.00.02\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tTue, 05 Sep 2017 10:00:03 -0700 (PDT)"],"Authentication-Results":["ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"Vn+PrP62\"; dkim-atps=neutral","lists.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"Vn+PrP62\"; dkim-atps=neutral","ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=gmail.com\n\t(client-ip=2607:f8b0:400e:c05::242; helo=mail-pg0-x242.google.com;\n\tenvelope-from=groeck7@gmail.com; receiver=)","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"Vn+PrP62\"; dkim-atps=neutral"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=sender:date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=YUkpPZymb37n3Kpz/fn6SpqrHX3utGFTAg7FXKqRbgs=;\n\tb=Vn+PrP62XkTsQcxcqq71FLUt2y239TwYsLNdu31zzr/82upAGq3/TcYkCieMytDbhg\n\tpGCgEa9mG2uodmvcyrUeeA3goFIOus3uwJs7xtUvjL1Utj3siU1Qnd5dYsGxuj+ypgTa\n\t27g095F9II79NmJQv0zxwDEQc6gVPyXEwxbfHTMlUzkJOFl7BjeWbhPEbeJYMSC69Djb\n\t0oajr1/S1GRUoygCMnE5v7/4J4oWf/zzPJEXmy8ov0onHMGMVsEkPtzIkncX15o8BQNn\n\tRQM9gE0SMcjbe7caRNfWim4nWJrY3SGpRy+I8xsWCqrGf4rlYmySJx3MDbtwqKsGbiGl\n\tnUXg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:sender:date:from:to:cc:subject:message-id\n\t:references:mime-version:content-disposition:in-reply-to:user-agent; \n\tbh=YUkpPZymb37n3Kpz/fn6SpqrHX3utGFTAg7FXKqRbgs=;\n\tb=glSA/UJeQG+GUPwynv/sv+d4RiXmEzgHjAFtnCialKHqZGOHjlIBf98xuc1RpLaDqO\n\tkF0v5WDsEpMy26JxKDuqQHsNjIsbsrrFzaWkH8j+7y18yvoX6hzULK7IZJ9Er05TNiLI\n\tP26w9NoTXWduX8wHvBxTBe7LrTuJ2JPipqBymqCPTGkdnQe8UjWHnxSlnzt+afAaTrpx\n\tu7Z6LP/G6ZWPeb0QsQvSQ5TETERMZeOUu+zJTMzQYBqLHmIGzBMtgfCOmqMS4UuRM1c6\n\tAzgywxfeXHJb14rmCpAL2yczerhRnz/flwdBEpUuOTrrOaOrURqkW7S25dl7wi8qStw6\n\truvw==","X-Gm-Message-State":"AHPjjUhBTgSWj6YlpxItJO5WD9WEwemKuJeXm3bmCEvPj2jPeLVHc+7B\n\tohVHiADJnGpvzA==","X-Google-Smtp-Source":"ADKCNb7hggh0R13t494JnSV8rwWdrfW6HecNNApSmRZoiwO5a+85K1ein/Xc3PkgzQ3DcIFqit3CGA==","X-Received":"by 10.84.211.42 with SMTP id b39mr5021741pli.150.1504630803860; \n\tTue, 05 Sep 2017 10:00:03 -0700 (PDT)","Date":"Tue, 5 Sep 2017 10:00:02 -0700","From":"Guenter Roeck ","To":"Andrew Jeffery ","Subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","Message-ID":"<20170905170002.GG11478@roeck-us.net>","References":"<20170905070132.17682-1-andrew@aj.id.au>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170905070132.17682-1-andrew@aj.id.au>","User-Agent":"Mutt/1.5.24 (2015-08-30)","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":"linux-hwmon@vger.kernel.org, jdelvare@suse.com, linux-doc@vger.kernel.org,\n\topenbmc@lists.ozlabs.org, corbet@lwn.net, linux-kernel@vger.kernel.org","Errors-To":"openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"openbmc\"\n\t"}},{"id":1763736,"web_url":"http://patchwork.ozlabs.org/comment/1763736/","msgid":"<1504657417.28363.8.camel@aj.id.au>","list_archive_url":null,"date":"2017-09-06T00:23:37","subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","submitter":{"id":68332,"url":"http://patchwork.ozlabs.org/api/people/68332/","name":"Andrew Jeffery","email":"andrew@aj.id.au"},"content":"On Tue, 2017-09-05 at 10:00 -0700, Guenter Roeck wrote:\n> On Tue, Sep 05, 2017 at 05:01:32PM +1000, Andrew Jeffery wrote:\n> > Some functions exposed by pmbus core conflated errors that occurred when\n> > setting the page to access with errors that occurred when accessing\n> > registers in a page. In some cases, this caused legitimate errors to be\n> > hidden under the guise of the register not being supported.\n> > \n> \n> I'll have to look into this. If I recall correctly, the reason for not returning\n> errors from the \"clear faults\" command was that some early chips did not support\n> it, or did not support it on all pages. Those chips would now always fail.\n\nYes, that is a concern.\n\nHowever, shouldn't the lack of support for CLEAR_FAULTS be a described\nproperty of the chip or page?\n\nRegardless, the issue here is the PAGE command is sometimes failing\n(more below). This means we won't have issued a CLEAR_FAULTS against\nthe page even if the page supports CLEAR_FAULTS. That to me is\nsomething that should be propagated back up the call chain, as faults\nthat can be cleared will not have been.\n\n> \n> On a higher level, I am not sure if it is a good idea to return an error\n> from a function intended to clear faults (and nothing else). That seems\n> counterproductive.\n> \n> Is this a problem you have actually observed ? \n\nUnfortunately yes. I was trying to reproduce some issues that we are\nseeing in userspace but wasn't having much luck (getting -EIO on\nreading a hwmon attribute). I ended up instrumenting our I2C bus\ndriver, and found that the problem was more prevalent than what the\nread failures in userspace were indicating. The paths that were\ntriggering these unreported conditions all traced through the check\nregister and clear fault functions, which on analysis were swallowing\nthe error.\n\n> If so, what is the chip ?\n\nWell, the chip that I'm *communicating* with is the Maxim MAX31785\nIntelligent Fan Controller. As to whether that's what is *causing* the\nPAGE command to fail is still up in the air. I would not be surprised\nif we have other issues in the hardware design.\n\nI haven't sent a follow-up to the series introducing the MAX31785\ndriver because I've been chasing down communication issues. I felt it\nwas important to understand whether the device has quirks that need to\nbe worked around in the driver, or if our hardware design has bigger\nproblems that should be handled in other ways. I've been in touch with\nMaxim who have asserted that some of the problems we're seeing cannot\nbe caused by their chip.\n\nAndrew\n\n> \n> Thanks,\n> Guenter\n> \n> > > > Signed-off-by: Andrew Jeffery \n> > ---\n> >  Documentation/hwmon/pmbus-core   |  12 ++--\n> >  drivers/hwmon/pmbus/pmbus.h      |   6 +-\n> >  drivers/hwmon/pmbus/pmbus_core.c | 115 +++++++++++++++++++++++++++++----------\n> >  3 files changed, 95 insertions(+), 38 deletions(-)\n> > \n> > diff --git a/Documentation/hwmon/pmbus-core b/Documentation/hwmon/pmbus-core\n> > index 8ed10e9ddfb5..3e9f41bb756f 100644\n> > --- a/Documentation/hwmon/pmbus-core\n> > +++ b/Documentation/hwmon/pmbus-core\n> > @@ -218,17 +218,17 @@ Specifically, it provides the following information.\n> >    This function calls the device specific write_byte function if defined.\n> >    Therefore, it must _not_ be called from that function.\n> >  \n> > -  bool pmbus_check_byte_register(struct i2c_client *client, int page, int reg);\n> > +  int pmbus_check_byte_register(struct i2c_client *client, int page, int reg);\n> >  \n> > -  Check if byte register exists. Return true if the register exists, false\n> > -  otherwise.\n> > +  Check if byte register exists. Returns 1 if the register exists, 0 if it does\n> > +  not, and less than zero on an unexpected error.\n> >    This function calls the device specific write_byte function if defined to\n> >    obtain the chip status. Therefore, it must _not_ be called from that function.\n> >  \n> > -  bool pmbus_check_word_register(struct i2c_client *client, int page, int reg);\n> > +  int pmbus_check_word_register(struct i2c_client *client, int page, int reg);\n> >  \n> > -  Check if word register exists. Return true if the register exists, false\n> > -  otherwise.\n> > +  Check if word register exists. Returns 1 if the register exists, 0 if it does\n> > +  not, and less than zero on an unexpected error.\n> >    This function calls the device specific write_byte function if defined to\n> >    obtain the chip status. Therefore, it must _not_ be called from that function.\n> >  \n> > diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h\n> > index bfcb13bae34b..c53032a04a6f 100644\n> > --- a/drivers/hwmon/pmbus/pmbus.h\n> > +++ b/drivers/hwmon/pmbus/pmbus.h\n> > @@ -413,9 +413,9 @@ int pmbus_write_byte_data(struct i2c_client *client, int page, u8 reg,\n> > > >  \t\t\t  u8 value);\n> >  int pmbus_update_byte_data(struct i2c_client *client, int page, u8 reg,\n> > > >  \t\t\t   u8 mask, u8 value);\n> > -void pmbus_clear_faults(struct i2c_client *client);\n> > -bool pmbus_check_byte_register(struct i2c_client *client, int page, int reg);\n> > -bool pmbus_check_word_register(struct i2c_client *client, int page, int reg);\n> > +int pmbus_clear_faults(struct i2c_client *client);\n> > +int pmbus_check_byte_register(struct i2c_client *client, int page, int reg);\n> > +int pmbus_check_word_register(struct i2c_client *client, int page, int reg);\n> >  int pmbus_do_probe(struct i2c_client *client, const struct i2c_device_id *id,\n> > > >  \t\t   struct pmbus_driver_info *info);\n> >  int pmbus_do_remove(struct i2c_client *client);\n> > diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c\n> > index f1eff6b6c798..153700e35431 100644\n> > --- a/drivers/hwmon/pmbus/pmbus_core.c\n> > +++ b/drivers/hwmon/pmbus/pmbus_core.c\n> > @@ -304,18 +304,24 @@ static int _pmbus_read_byte_data(struct i2c_client *client, int page, int reg)\n> > > >  \treturn pmbus_read_byte_data(client, page, reg);\n> >  }\n> >  \n> > -static void pmbus_clear_fault_page(struct i2c_client *client, int page)\n> > +static int pmbus_clear_fault_page(struct i2c_client *client, int page)\n> >  {\n> > > > -\t_pmbus_write_byte(client, page, PMBUS_CLEAR_FAULTS);\n> > > > +\treturn _pmbus_write_byte(client, page, PMBUS_CLEAR_FAULTS);\n> >  }\n> >  \n> > -void pmbus_clear_faults(struct i2c_client *client)\n> > +int pmbus_clear_faults(struct i2c_client *client)\n> >  {\n> > > >  \tstruct pmbus_data *data = i2c_get_clientdata(client);\n> > > > +\tint rv;\n> > > >  \tint i;\n> >  \n> > > > -\tfor (i = 0; i < data->info->pages; i++)\n> > > > -\t\tpmbus_clear_fault_page(client, i);\n> > > > +\tfor (i = 0; i < data->info->pages; i++) {\n> > > > +\t\trv = pmbus_clear_fault_page(client, i);\n> > > > +\t\tif (rv)\n> > > > +\t\t\treturn rv;\n> > > > +\t}\n> > +\n> > > > +\treturn 0;\n> >  }\n> >  EXPORT_SYMBOL_GPL(pmbus_clear_faults);\n> >  \n> > @@ -333,28 +339,45 @@ static int pmbus_check_status_cml(struct i2c_client *client)\n> > > >  \treturn 0;\n> >  }\n> >  \n> > -static bool pmbus_check_register(struct i2c_client *client,\n> > +static int pmbus_check_register(struct i2c_client *client,\n> > > >  \t\t\t\t int (*func)(struct i2c_client *client,\n> > > >  \t\t\t\t\t     int page, int reg),\n> > > >  \t\t\t\t int page, int reg)\n> >  {\n> > > > +\tstruct pmbus_data *data;\n> > > > +\tint check;\n> > > >  \tint rv;\n> > > > -\tstruct pmbus_data *data = i2c_get_clientdata(client);\n> >  \n> > > > -\trv = func(client, page, reg);\n> > > > -\tif (rv >= 0 && !(data->flags & PMBUS_SKIP_STATUS_CHECK))\n> > > > -\t\trv = pmbus_check_status_cml(client);\n> > > > -\tpmbus_clear_fault_page(client, -1);\n> > > > -\treturn rv >= 0;\n> > > > +\tdata = i2c_get_clientdata(client);\n> > +\n> > > > +\t/*\n> > > > +\t * pmbus_set_page() guards transactions on the requested page matching\n> > > > +\t * the current page. This may be done in the execution of func(), but\n> > > > +\t * at that point a set-page error is conflated with accessing a\n> > > > +\t * non-existent register.\n> > > > +\t */\n> > > > +\trv = pmbus_set_page(client, page);\n> > > > +\tif (rv < 0)\n> > > > +\t\treturn rv;\n> > +\n> > > > +\tcheck = func(client, page, reg);\n> > > > +\tif (check >= 0 && !(data->flags & PMBUS_SKIP_STATUS_CHECK))\n> > > > +\t\tcheck = pmbus_check_status_cml(client);\n> > +\n> > > > +\trv = pmbus_clear_fault_page(client, -1);\n> > > > +\tif (rv < 0)\n> > > > +\t\treturn rv;\n> > +\n> > > > +\treturn check >= 0;\n> >  }\n> >  \n> > -bool pmbus_check_byte_register(struct i2c_client *client, int page, int reg)\n> > +int pmbus_check_byte_register(struct i2c_client *client, int page, int reg)\n> >  {\n> > > >  \treturn pmbus_check_register(client, _pmbus_read_byte_data, page, reg);\n> >  }\n> >  EXPORT_SYMBOL_GPL(pmbus_check_byte_register);\n> >  \n> > -bool pmbus_check_word_register(struct i2c_client *client, int page, int reg)\n> > +int pmbus_check_word_register(struct i2c_client *client, int page, int reg)\n> >  {\n> > > >  \treturn pmbus_check_register(client, _pmbus_read_word_data, page, reg);\n> >  }\n> > @@ -390,7 +413,7 @@ static struct pmbus_data *pmbus_update_device(struct device *dev)\n> >  \n> > > >  \tmutex_lock(&data->update_lock);\n> > > >  \tif (time_after(jiffies, data->last_updated + HZ) || !data->valid) {\n> > > > -\t\tint i, j;\n> > > > +\t\tint i, j, ret;\n> >  \n> > > >  \t\tfor (i = 0; i < info->pages; i++) {\n> > > >  \t\t\tdata->status[PB_STATUS_BASE + i]\n> > @@ -424,7 +447,13 @@ static struct pmbus_data *pmbus_update_device(struct device *dev)\n> > > >  \t\t\t\t\t\t\t    sensor->page,\n> > > >  \t\t\t\t\t\t\t    sensor->reg);\n> > > >  \t\t}\n> > > > -\t\tpmbus_clear_faults(client);\n> > +\n> > > > +\t\tret = pmbus_clear_faults(client);\n> > > > +\t\tif (ret < 0) {\n> > > > +\t\t\tmutex_unlock(&data->update_lock);\n> > > > +\t\t\treturn ERR_PTR(ret);\n> > > > +\t\t}\n> > +\n> > > >  \t\tdata->last_updated = jiffies;\n> > > >  \t\tdata->valid = 1;\n> > > >  \t}\n> > @@ -754,6 +783,9 @@ static ssize_t pmbus_show_boolean(struct device *dev,\n> > > >  \tstruct pmbus_data *data = pmbus_update_device(dev);\n> > > >  \tint val;\n> >  \n> > > > +\tif (IS_ERR(data))\n> > > > +\t\treturn PTR_ERR(data);\n> > +\n> > > >  \tval = pmbus_get_boolean(data, boolean, attr->index);\n> > > >  \tif (val < 0)\n> > > >  \t\treturn val;\n> > @@ -766,6 +798,9 @@ static ssize_t pmbus_show_sensor(struct device *dev,\n> > > >  \tstruct pmbus_data *data = pmbus_update_device(dev);\n> > > >  \tstruct pmbus_sensor *sensor = to_pmbus_sensor(devattr);\n> >  \n> > > > +\tif (IS_ERR(data))\n> > > > +\t\treturn PTR_ERR(data);\n> > +\n> > > >  \tif (sensor->data < 0)\n> > > >  \t\treturn sensor->data;\n> >  \n> > @@ -995,7 +1030,11 @@ static int pmbus_add_limit_attrs(struct i2c_client *client,\n> > > >  \tstruct pmbus_sensor *curr;\n> >  \n> > > >  \tfor (i = 0; i < nlimit; i++) {\n> > > > -\t\tif (pmbus_check_word_register(client, page, l->reg)) {\n> > > > +\t\tret = pmbus_check_word_register(client, page, l->reg);\n> > > > +\t\tif (ret < 0)\n> > > > +\t\t\treturn ret;\n> > +\n> > > > +\t\tif (ret) {\n> > > >  \t\t\tcurr = pmbus_add_sensor(data, name, l->attr, index,\n> > > >  \t\t\t\t\t\tpage, l->reg, attr->class,\n> > > >  \t\t\t\t\t\tattr->update || l->update,\n> > @@ -1041,6 +1080,8 @@ static int pmbus_add_sensor_attrs_one(struct i2c_client *client,\n> > > >  \tif (!base)\n> > > >  \t\treturn -ENOMEM;\n> > > >  \tif (attr->sfunc) {\n> > > > +\t\tint check;\n> > +\n> > > >  \t\tret = pmbus_add_limit_attrs(client, data, info, name,\n> > > >  \t\t\t\t\t    index, page, base, attr);\n> > > >  \t\tif (ret < 0)\n> > @@ -1050,9 +1091,13 @@ static int pmbus_add_sensor_attrs_one(struct i2c_client *client,\n> > > >  \t\t * alarm attributes, if there is a global alarm bit, and if\n> > > >  \t\t * the generic status register for this page is accessible.\n> > > >  \t\t */\n> > > > -\t\tif (!ret && attr->gbit &&\n> > > > -\t\t    pmbus_check_byte_register(client, page,\n> > > > -\t\t\t\t\t      data->status_register)) {\n> > +\n> > > > +\t\tcheck = pmbus_check_byte_register(client, page,\n> > > > +\t\t\t\t\t\t  data->status_register);\n> > > > +\t\tif (check < 0)\n> > > > +\t\t\treturn check;\n> > +\n> > > > +\t\tif (!ret && attr->gbit && check) {\n> > > >  \t\t\tret = pmbus_add_boolean(data, name, \"alarm\", index,\n> > > >  \t\t\t\t\t\tNULL, NULL,\n> > > >  \t\t\t\t\t\tPB_STATUS_BASE + page,\n> > @@ -1604,8 +1649,12 @@ static int pmbus_add_fan_attributes(struct i2c_client *client,\n> > > >  \t\t\tif (!(info->func[page] & pmbus_fan_flags[f]))\n> > > >  \t\t\t\tbreak;\n> >  \n> > > > -\t\t\tif (!pmbus_check_word_register(client, page,\n> > > > -\t\t\t\t\t\t       pmbus_fan_registers[f]))\n> > > > +\t\t\tret = pmbus_check_word_register(client, page,\n> > > > +\t\t\t\t\t\t       pmbus_fan_registers[f]);\n> > > > +\t\t\tif (ret < 0)\n> > > > +\t\t\t\treturn ret;\n> > +\n> > > > +\t\t\tif (!ret)\n> > > >  \t\t\t\tbreak;\n> >  \n> > > >  \t\t\t/*\n> > @@ -1628,9 +1677,13 @@ static int pmbus_add_fan_attributes(struct i2c_client *client,\n> > > >  \t\t\t * Each fan status register covers multiple fans,\n> > > >  \t\t\t * so we have to do some magic.\n> > > >  \t\t\t */\n> > > > -\t\t\tif ((info->func[page] & pmbus_fan_status_flags[f]) &&\n> > > > -\t\t\t    pmbus_check_byte_register(client,\n> > > > -\t\t\t\t\tpage, pmbus_fan_status_registers[f])) {\n> > > > +\t\t\tret =  pmbus_check_byte_register(client, page,\n> > > > +\t\t\t\t\t\tpmbus_fan_status_registers[f]);\n> > > > +\t\t\tif (ret < 0)\n> > > > +\t\t\t\treturn ret;\n> > +\n> > > > +\t\t\tif ((info->func[page] & pmbus_fan_status_flags[f])\n> > > > +\t\t\t\t\t&& ret) {\n> > > >  \t\t\t\tint base;\n> >  \n> > > > > >  \t\t\t\tif (f > 1)\t/* fan 3, 4 */\n> > @@ -1696,10 +1749,13 @@ static int pmbus_identify_common(struct i2c_client *client,\n> > > >  \t\t\t\t struct pmbus_data *data, int page)\n> >  {\n> > > >  \tint vout_mode = -1;\n> > > > +\tint rv;\n> >  \n> > > > -\tif (pmbus_check_byte_register(client, page, PMBUS_VOUT_MODE))\n> > > > +\trv = pmbus_check_byte_register(client, page, PMBUS_VOUT_MODE);\n> > > > +\tif (rv == 1)\n> > > >  \t\tvout_mode = _pmbus_read_byte_data(client, page,\n> > > >  \t\t\t\t\t\t  PMBUS_VOUT_MODE);\n> > +\n> > > >  \tif (vout_mode >= 0 && vout_mode != 0xff) {\n> > > >  \t\t/*\n> > > >  \t\t * Not all chips support the VOUT_MODE command,\n> > @@ -1725,8 +1781,7 @@ static int pmbus_identify_common(struct i2c_client *client,\n> > > >  \t\t}\n> > > >  \t}\n> >  \n> > > > -\tpmbus_clear_fault_page(client, page);\n> > > > -\treturn 0;\n> > > > +\treturn pmbus_clear_fault_page(client, page);\n> >  }\n> >  \n> >  static int pmbus_init_common(struct i2c_client *client, struct pmbus_data *data,\n> > @@ -1756,7 +1811,9 @@ static int pmbus_init_common(struct i2c_client *client, struct pmbus_data *data,\n> > > >  \tif (ret >= 0 && (ret & PB_CAPABILITY_ERROR_CHECK))\n> > > >  \t\tclient->flags |= I2C_CLIENT_PEC;\n> >  \n> > > > -\tpmbus_clear_faults(client);\n> > > > +\tret = pmbus_clear_faults(client);\n> > > > +\tif (ret < 0)\n> > > > +\t\treturn ret;\n> >  \n> > > >  \tif (info->identify) {\n> > > >  \t\tret = (*info->identify)(client, info);\n> > -- \n> > 2.11.0\n> >","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 3xn4840lZvz9t3P\n\tfor ;\n\tWed, 6 Sep 2017 10:24:04 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xn4834sFbzDrJm\n\tfor ;\n\tWed, 6 Sep 2017 10:24:03 +1000 (AEST)","from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com\n\t[66.111.4.25])\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 3xn47n3jxtzDrFT\n\tfor ; Wed, 6 Sep 2017 10:23:48 +1000 (AEST)","from compute4.internal (compute4.nyi.internal [10.202.2.44])\n\tby mailout.nyi.internal (Postfix) with ESMTP id E56EB2119E;\n\tTue, 5 Sep 2017 20:23:43 -0400 (EDT)","from frontend1 ([10.202.2.160])\n\tby compute4.internal (MEProxy); Tue, 05 Sep 2017 20:23:43 -0400","from keelia (unknown [122.99.82.10])\n\tby mail.messagingengine.com (Postfix) with ESMTPA id 3FB1A7F96C;\n\tTue, 5 Sep 2017 20:23:41 -0400 (EDT)"],"Authentication-Results":["ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"mQlN4c7l\";\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"XlxGHv47\"; \n\tdkim-atps=neutral","lists.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"mQlN4c7l\";\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"XlxGHv47\"; \n\tdkim-atps=neutral","ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=aj.id.au\n\t(client-ip=66.111.4.25; helo=out1-smtp.messagingengine.com;\n\tenvelope-from=andrew@aj.id.au; receiver=)","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"mQlN4c7l\";\n\tdkim=pass (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com\n\theader.b=\"XlxGHv47\"; 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=/OMOGyKGs+KIrCO0Ze0VWQOEPVaYzs+3LRyWjmDFs\n\t5s=; b=mQlN4c7l5gBi8R0qV7dbFOpEuv3MSSWUe1n3+hGfBdTMbNK5ssXBOAT6T\n\tNsojkNmApFVXfjB+w/TbIf/8Lz0HCPFupbJ2BwAwp9cOfwgvhRaXFgX33uELN56F\n\t9ENWgKGuJYW6dRbhYsAKAFy8EYAFH1JMtkNm3yLnk0faTnQTU1ctH27gf2xy5lYE\n\t3aT8u3znHinN2rLsa9KDD4jleottK9Ot7wzn/Uu61416aKAHPSIMCF2qIBSdAYyJ\n\tCXnACePOEF6nla/m38UYx6PJB8f62TQymhhiUB+K0f6Y+/bxv8P9ydK+hZhgAfeQ\n\tfIPNh4jFOW6HMu2GMAxTP+Qpe7Xsg==","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=/OMOGyKGs+KIrCO0Ze\n\t0VWQOEPVaYzs+3LRyWjmDFs5s=; b=XlxGHv478d3PTcD1USc9cMJo1lGAZJ/sYn\n\tj/mxZKeTxAEbOmVEEgF0UvofZoq2w/upEx0dbRmZYKCZZLXRIO0NyKDX1TRITw8+\n\t3+XkMKCU3+PIyW9F35h4ucoEnQBvMVYmHPIwT3OrxlVJ5zcjBzieXE41bSz8frgG\n\tUYYYTMFS3S6DCkaaKHmv9590wgxVDreP7tb9XtIjvT8scjhx8KVMV38xUfAjwhOr\n\tFENRmxtC8d24rmcZ3jELCwJ30wo7ACRYUsWtIGPkU0K8wypZHOy845gAkYkCg+oi\n\t35ovKSmFp3jCmVp9RCoQr9CCQNUzWPt6lIbaXWi9b5EcqyqmY9AA=="],"X-ME-Sender":"","X-Sasl-enc":"7/8C0uDUpx+6N9m7+k3AnljsMfR/WwIkfYLtaimMZbbW 1504657423","Message-ID":"<1504657417.28363.8.camel@aj.id.au>","Subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","From":"Andrew Jeffery ","To":"Guenter Roeck ","Date":"Wed, 06 Sep 2017 10:23:37 +1000","In-Reply-To":"<20170905170002.GG11478@roeck-us.net>","References":"<20170905070132.17682-1-andrew@aj.id.au>\n\t<20170905170002.GG11478@roeck-us.net>","Content-Type":"multipart/signed; micalg=\"pgp-sha512\";\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"=-LOiTEbHXDgJsBfl24LQo\"","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":"linux-hwmon@vger.kernel.org, jdelvare@suse.com, linux-doc@vger.kernel.org,\n\topenbmc@lists.ozlabs.org, corbet@lwn.net, linux-kernel@vger.kernel.org","Errors-To":"openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"openbmc\"\n\t"}},{"id":1764423,"web_url":"http://patchwork.ozlabs.org/comment/1764423/","msgid":"<20170906225102.GA32210@roeck-us.net>","list_archive_url":null,"date":"2017-09-06T22:51:02","subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","submitter":{"id":21889,"url":"http://patchwork.ozlabs.org/api/people/21889/","name":"Guenter Roeck","email":"linux@roeck-us.net"},"content":"On Wed, Sep 06, 2017 at 10:23:37AM +1000, Andrew Jeffery wrote:\n> On Tue, 2017-09-05 at 10:00 -0700, Guenter Roeck wrote:\n> > On Tue, Sep 05, 2017 at 05:01:32PM +1000, Andrew Jeffery wrote:\n> > > Some functions exposed by pmbus core conflated errors that occurred when\n> > > setting the page to access with errors that occurred when accessing\n> > > registers in a page. In some cases, this caused legitimate errors to be\n> > > hidden under the guise of the register not being supported.\n> > > \n> > \n> > I'll have to look into this. If I recall correctly, the reason for not returning\n> > errors from the \"clear faults\" command was that some early chips did not support\n> > it, or did not support it on all pages. Those chips would now always fail.\n> \n> Yes, that is a concern.\n> \n> However, shouldn't the lack of support for CLEAR_FAULTS be a described\n> property of the chip or page?\n> \n> Regardless, the issue here is the PAGE command is sometimes failing\n> (more below). This means we won't have issued a CLEAR_FAULTS against\n> the page even if the page supports CLEAR_FAULTS. That to me is\n> something that should be propagated back up the call chain, as faults\n> that can be cleared will not have been.\n> \nWe could also consider\n- retry\n- Add a marker indicating that faults (specifically command errors)\n are unreliable after clearing faults failed\n\nIf we can't reliably clear faults, all bets are off.\n\n> > \n> > On a higher level, I am not sure if it is a good idea to return an error\n> > from a function intended to clear faults (and nothing else). That seems\n> > counterproductive.\n> > \n> > Is this a problem you have actually observed ? \n> \n> Unfortunately yes. I was trying to reproduce some issues that we are\n> seeing in userspace but wasn't having much luck (getting -EIO on\n> reading a hwmon attribute). I ended up instrumenting our I2C bus\n> driver, and found that the problem was more prevalent than what the\n> read failures in userspace were indicating. The paths that were\n> triggering these unreported conditions all traced through the check\n> register and clear fault functions, which on analysis were swallowing\n> the error.\n> \n> > If so, what is the chip ?\n> \n> Well, the chip that I'm *communicating* with is the Maxim MAX31785\n> Intelligent Fan Controller. As to whether that's what is *causing* the\n> PAGE command to fail is still up in the air. I would not be surprised\n> if we have other issues in the hardware design.\n> \n> I haven't sent a follow-up to the series introducing the MAX31785\n> driver because I've been chasing down communication issues. I felt it\n> was important to understand whether the device has quirks that need to\n> be worked around in the driver, or if our hardware design has bigger\n> problems that should be handled in other ways. I've been in touch with\n> Maxim who have asserted that some of the problems we're seeing cannot\n> be caused by their chip.\n> \n\nGuess I need to dig up my eval board and see if I can reproduce the problem.\nSeems you are saying that the problem is always seen when issuing a sequence\nof \"clear faults\" commands on multiple pages ?\n\nThe MAX31785 is microcode based, so I would not be entirely surprised if\nit sometimes fails to reply if a sequence of \ncommands is executed.\n\nIf possible, you might try reducing the i2c clock. I have not seen that with\nMaxim chips, but some other PMBus chips don't operate reliably at 400 KHz.\n\nGuenter\n\n> > \n> > > > > Signed-off-by: Andrew Jeffery \n> > > ---\n> > >  Documentation/hwmon/pmbus-core   |  12 ++--\n> > >  drivers/hwmon/pmbus/pmbus.h      |   6 +-\n> > >  drivers/hwmon/pmbus/pmbus_core.c | 115 +++++++++++++++++++++++++++++----------\n> > >  3 files changed, 95 insertions(+), 38 deletions(-)\n> > > \n> > > diff --git a/Documentation/hwmon/pmbus-core b/Documentation/hwmon/pmbus-core\n> > > index 8ed10e9ddfb5..3e9f41bb756f 100644\n> > > --- a/Documentation/hwmon/pmbus-core\n> > > +++ b/Documentation/hwmon/pmbus-core\n> > > @@ -218,17 +218,17 @@ Specifically, it provides the following information.\n> > >    This function calls the device specific write_byte function if defined.\n> > >    Therefore, it must _not_ be called from that function.\n> > >  \n> > > -  bool pmbus_check_byte_register(struct i2c_client *client, int page, int reg);\n> > > +  int pmbus_check_byte_register(struct i2c_client *client, int page, int reg);\n> > >  \n> > > -  Check if byte register exists. Return true if the register exists, false\n> > > -  otherwise.\n> > > +  Check if byte register exists. Returns 1 if the register exists, 0 if it does\n> > > +  not, and less than zero on an unexpected error.\n> > >    This function calls the device specific write_byte function if defined to\n> > >    obtain the chip status. Therefore, it must _not_ be called from that function.\n> > >  \n> > > -  bool pmbus_check_word_register(struct i2c_client *client, int page, int reg);\n> > > +  int pmbus_check_word_register(struct i2c_client *client, int page, int reg);\n> > >  \n> > > -  Check if word register exists. Return true if the register exists, false\n> > > -  otherwise.\n> > > +  Check if word register exists. Returns 1 if the register exists, 0 if it does\n> > > +  not, and less than zero on an unexpected error.\n> > >    This function calls the device specific write_byte function if defined to\n> > >    obtain the chip status. Therefore, it must _not_ be called from that function.\n> > >  \n> > > diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h\n> > > index bfcb13bae34b..c53032a04a6f 100644\n> > > --- a/drivers/hwmon/pmbus/pmbus.h\n> > > +++ b/drivers/hwmon/pmbus/pmbus.h\n> > > @@ -413,9 +413,9 @@ int pmbus_write_byte_data(struct i2c_client *client, int page, u8 reg,\n> > > > >  \t\t\t  u8 value);\n> > >  int pmbus_update_byte_data(struct i2c_client *client, int page, u8 reg,\n> > > > >  \t\t\t   u8 mask, u8 value);\n> > > -void pmbus_clear_faults(struct i2c_client *client);\n> > > -bool pmbus_check_byte_register(struct i2c_client *client, int page, int reg);\n> > > -bool pmbus_check_word_register(struct i2c_client *client, int page, int reg);\n> > > +int pmbus_clear_faults(struct i2c_client *client);\n> > > +int pmbus_check_byte_register(struct i2c_client *client, int page, int reg);\n> > > +int pmbus_check_word_register(struct i2c_client *client, int page, int reg);\n> > >  int pmbus_do_probe(struct i2c_client *client, const struct i2c_device_id *id,\n> > > > >  \t\t   struct pmbus_driver_info *info);\n> > >  int pmbus_do_remove(struct i2c_client *client);\n> > > diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c\n> > > index f1eff6b6c798..153700e35431 100644\n> > > --- a/drivers/hwmon/pmbus/pmbus_core.c\n> > > +++ b/drivers/hwmon/pmbus/pmbus_core.c\n> > > @@ -304,18 +304,24 @@ static int _pmbus_read_byte_data(struct i2c_client *client, int page, int reg)\n> > > > >  \treturn pmbus_read_byte_data(client, page, reg);\n> > >  }\n> > >  \n> > > -static void pmbus_clear_fault_page(struct i2c_client *client, int page)\n> > > +static int pmbus_clear_fault_page(struct i2c_client *client, int page)\n> > >  {\n> > > > > -\t_pmbus_write_byte(client, page, PMBUS_CLEAR_FAULTS);\n> > > > > +\treturn _pmbus_write_byte(client, page, PMBUS_CLEAR_FAULTS);\n> > >  }\n> > >  \n> > > -void pmbus_clear_faults(struct i2c_client *client)\n> > > +int pmbus_clear_faults(struct i2c_client *client)\n> > >  {\n> > > > >  \tstruct pmbus_data *data = i2c_get_clientdata(client);\n> > > > > +\tint rv;\n> > > > >  \tint i;\n> > >  \n> > > > > -\tfor (i = 0; i < data->info->pages; i++)\n> > > > > -\t\tpmbus_clear_fault_page(client, i);\n> > > > > +\tfor (i = 0; i < data->info->pages; i++) {\n> > > > > +\t\trv = pmbus_clear_fault_page(client, i);\n> > > > > +\t\tif (rv)\n> > > > > +\t\t\treturn rv;\n> > > > > +\t}\n> > > +\n> > > > > +\treturn 0;\n> > >  }\n> > >  EXPORT_SYMBOL_GPL(pmbus_clear_faults);\n> > >  \n> > > @@ -333,28 +339,45 @@ static int pmbus_check_status_cml(struct i2c_client *client)\n> > > > >  \treturn 0;\n> > >  }\n> > >  \n> > > -static bool pmbus_check_register(struct i2c_client *client,\n> > > +static int pmbus_check_register(struct i2c_client *client,\n> > > > >  \t\t\t\t int (*func)(struct i2c_client *client,\n> > > > >  \t\t\t\t\t     int page, int reg),\n> > > > >  \t\t\t\t int page, int reg)\n> > >  {\n> > > > > +\tstruct pmbus_data *data;\n> > > > > +\tint check;\n> > > > >  \tint rv;\n> > > > > -\tstruct pmbus_data *data = i2c_get_clientdata(client);\n> > >  \n> > > > > -\trv = func(client, page, reg);\n> > > > > -\tif (rv >= 0 && !(data->flags & PMBUS_SKIP_STATUS_CHECK))\n> > > > > -\t\trv = pmbus_check_status_cml(client);\n> > > > > -\tpmbus_clear_fault_page(client, -1);\n> > > > > -\treturn rv >= 0;\n> > > > > +\tdata = i2c_get_clientdata(client);\n> > > +\n> > > > > +\t/*\n> > > > > +\t * pmbus_set_page() guards transactions on the requested page matching\n> > > > > +\t * the current page. This may be done in the execution of func(), but\n> > > > > +\t * at that point a set-page error is conflated with accessing a\n> > > > > +\t * non-existent register.\n> > > > > +\t */\n> > > > > +\trv = pmbus_set_page(client, page);\n> > > > > +\tif (rv < 0)\n> > > > > +\t\treturn rv;\n> > > +\n> > > > > +\tcheck = func(client, page, reg);\n> > > > > +\tif (check >= 0 && !(data->flags & PMBUS_SKIP_STATUS_CHECK))\n> > > > > +\t\tcheck = pmbus_check_status_cml(client);\n> > > +\n> > > > > +\trv = pmbus_clear_fault_page(client, -1);\n> > > > > +\tif (rv < 0)\n> > > > > +\t\treturn rv;\n> > > +\n> > > > > +\treturn check >= 0;\n> > >  }\n> > >  \n> > > -bool pmbus_check_byte_register(struct i2c_client *client, int page, int reg)\n> > > +int pmbus_check_byte_register(struct i2c_client *client, int page, int reg)\n> > >  {\n> > > > >  \treturn pmbus_check_register(client, _pmbus_read_byte_data, page, reg);\n> > >  }\n> > >  EXPORT_SYMBOL_GPL(pmbus_check_byte_register);\n> > >  \n> > > -bool pmbus_check_word_register(struct i2c_client *client, int page, int reg)\n> > > +int pmbus_check_word_register(struct i2c_client *client, int page, int reg)\n> > >  {\n> > > > >  \treturn pmbus_check_register(client, _pmbus_read_word_data, page, reg);\n> > >  }\n> > > @@ -390,7 +413,7 @@ static struct pmbus_data *pmbus_update_device(struct device *dev)\n> > >  \n> > > > >  \tmutex_lock(&data->update_lock);\n> > > > >  \tif (time_after(jiffies, data->last_updated + HZ) || !data->valid) {\n> > > > > -\t\tint i, j;\n> > > > > +\t\tint i, j, ret;\n> > >  \n> > > > >  \t\tfor (i = 0; i < info->pages; i++) {\n> > > > >  \t\t\tdata->status[PB_STATUS_BASE + i]\n> > > @@ -424,7 +447,13 @@ static struct pmbus_data *pmbus_update_device(struct device *dev)\n> > > > >  \t\t\t\t\t\t\t    sensor->page,\n> > > > >  \t\t\t\t\t\t\t    sensor->reg);\n> > > > >  \t\t}\n> > > > > -\t\tpmbus_clear_faults(client);\n> > > +\n> > > > > +\t\tret = pmbus_clear_faults(client);\n> > > > > +\t\tif (ret < 0) {\n> > > > > +\t\t\tmutex_unlock(&data->update_lock);\n> > > > > +\t\t\treturn ERR_PTR(ret);\n> > > > > +\t\t}\n> > > +\n> > > > >  \t\tdata->last_updated = jiffies;\n> > > > >  \t\tdata->valid = 1;\n> > > > >  \t}\n> > > @@ -754,6 +783,9 @@ static ssize_t pmbus_show_boolean(struct device *dev,\n> > > > >  \tstruct pmbus_data *data = pmbus_update_device(dev);\n> > > > >  \tint val;\n> > >  \n> > > > > +\tif (IS_ERR(data))\n> > > > > +\t\treturn PTR_ERR(data);\n> > > +\n> > > > >  \tval = pmbus_get_boolean(data, boolean, attr->index);\n> > > > >  \tif (val < 0)\n> > > > >  \t\treturn val;\n> > > @@ -766,6 +798,9 @@ static ssize_t pmbus_show_sensor(struct device *dev,\n> > > > >  \tstruct pmbus_data *data = pmbus_update_device(dev);\n> > > > >  \tstruct pmbus_sensor *sensor = to_pmbus_sensor(devattr);\n> > >  \n> > > > > +\tif (IS_ERR(data))\n> > > > > +\t\treturn PTR_ERR(data);\n> > > +\n> > > > >  \tif (sensor->data < 0)\n> > > > >  \t\treturn sensor->data;\n> > >  \n> > > @@ -995,7 +1030,11 @@ static int pmbus_add_limit_attrs(struct i2c_client *client,\n> > > > >  \tstruct pmbus_sensor *curr;\n> > >  \n> > > > >  \tfor (i = 0; i < nlimit; i++) {\n> > > > > -\t\tif (pmbus_check_word_register(client, page, l->reg)) {\n> > > > > +\t\tret = pmbus_check_word_register(client, page, l->reg);\n> > > > > +\t\tif (ret < 0)\n> > > > > +\t\t\treturn ret;\n> > > +\n> > > > > +\t\tif (ret) {\n> > > > >  \t\t\tcurr = pmbus_add_sensor(data, name, l->attr, index,\n> > > > >  \t\t\t\t\t\tpage, l->reg, attr->class,\n> > > > >  \t\t\t\t\t\tattr->update || l->update,\n> > > @@ -1041,6 +1080,8 @@ static int pmbus_add_sensor_attrs_one(struct i2c_client *client,\n> > > > >  \tif (!base)\n> > > > >  \t\treturn -ENOMEM;\n> > > > >  \tif (attr->sfunc) {\n> > > > > +\t\tint check;\n> > > +\n> > > > >  \t\tret = pmbus_add_limit_attrs(client, data, info, name,\n> > > > >  \t\t\t\t\t    index, page, base, attr);\n> > > > >  \t\tif (ret < 0)\n> > > @@ -1050,9 +1091,13 @@ static int pmbus_add_sensor_attrs_one(struct i2c_client *client,\n> > > > >  \t\t * alarm attributes, if there is a global alarm bit, and if\n> > > > >  \t\t * the generic status register for this page is accessible.\n> > > > >  \t\t */\n> > > > > -\t\tif (!ret && attr->gbit &&\n> > > > > -\t\t    pmbus_check_byte_register(client, page,\n> > > > > -\t\t\t\t\t      data->status_register)) {\n> > > +\n> > > > > +\t\tcheck = pmbus_check_byte_register(client, page,\n> > > > > +\t\t\t\t\t\t  data->status_register);\n> > > > > +\t\tif (check < 0)\n> > > > > +\t\t\treturn check;\n> > > +\n> > > > > +\t\tif (!ret && attr->gbit && check) {\n> > > > >  \t\t\tret = pmbus_add_boolean(data, name, \"alarm\", index,\n> > > > >  \t\t\t\t\t\tNULL, NULL,\n> > > > >  \t\t\t\t\t\tPB_STATUS_BASE + page,\n> > > @@ -1604,8 +1649,12 @@ static int pmbus_add_fan_attributes(struct i2c_client *client,\n> > > > >  \t\t\tif (!(info->func[page] & pmbus_fan_flags[f]))\n> > > > >  \t\t\t\tbreak;\n> > >  \n> > > > > -\t\t\tif (!pmbus_check_word_register(client, page,\n> > > > > -\t\t\t\t\t\t       pmbus_fan_registers[f]))\n> > > > > +\t\t\tret = pmbus_check_word_register(client, page,\n> > > > > +\t\t\t\t\t\t       pmbus_fan_registers[f]);\n> > > > > +\t\t\tif (ret < 0)\n> > > > > +\t\t\t\treturn ret;\n> > > +\n> > > > > +\t\t\tif (!ret)\n> > > > >  \t\t\t\tbreak;\n> > >  \n> > > > >  \t\t\t/*\n> > > @@ -1628,9 +1677,13 @@ static int pmbus_add_fan_attributes(struct i2c_client *client,\n> > > > >  \t\t\t * Each fan status register covers multiple fans,\n> > > > >  \t\t\t * so we have to do some magic.\n> > > > >  \t\t\t */\n> > > > > -\t\t\tif ((info->func[page] & pmbus_fan_status_flags[f]) &&\n> > > > > -\t\t\t    pmbus_check_byte_register(client,\n> > > > > -\t\t\t\t\tpage, pmbus_fan_status_registers[f])) {\n> > > > > +\t\t\tret =  pmbus_check_byte_register(client, page,\n> > > > > +\t\t\t\t\t\tpmbus_fan_status_registers[f]);\n> > > > > +\t\t\tif (ret < 0)\n> > > > > +\t\t\t\treturn ret;\n> > > +\n> > > > > +\t\t\tif ((info->func[page] & pmbus_fan_status_flags[f])\n> > > > > +\t\t\t\t\t&& ret) {\n> > > > >  \t\t\t\tint base;\n> > >  \n> > > > > > >  \t\t\t\tif (f > 1)\t/* fan 3, 4 */\n> > > @@ -1696,10 +1749,13 @@ static int pmbus_identify_common(struct i2c_client *client,\n> > > > >  \t\t\t\t struct pmbus_data *data, int page)\n> > >  {\n> > > > >  \tint vout_mode = -1;\n> > > > > +\tint rv;\n> > >  \n> > > > > -\tif (pmbus_check_byte_register(client, page, PMBUS_VOUT_MODE))\n> > > > > +\trv = pmbus_check_byte_register(client, page, PMBUS_VOUT_MODE);\n> > > > > +\tif (rv == 1)\n> > > > >  \t\tvout_mode = _pmbus_read_byte_data(client, page,\n> > > > >  \t\t\t\t\t\t  PMBUS_VOUT_MODE);\n> > > +\n> > > > >  \tif (vout_mode >= 0 && vout_mode != 0xff) {\n> > > > >  \t\t/*\n> > > > >  \t\t * Not all chips support the VOUT_MODE command,\n> > > @@ -1725,8 +1781,7 @@ static int pmbus_identify_common(struct i2c_client *client,\n> > > > >  \t\t}\n> > > > >  \t}\n> > >  \n> > > > > -\tpmbus_clear_fault_page(client, page);\n> > > > > -\treturn 0;\n> > > > > +\treturn pmbus_clear_fault_page(client, page);\n> > >  }\n> > >  \n> > >  static int pmbus_init_common(struct i2c_client *client, struct pmbus_data *data,\n> > > @@ -1756,7 +1811,9 @@ static int pmbus_init_common(struct i2c_client *client, struct pmbus_data *data,\n> > > > >  \tif (ret >= 0 && (ret & PB_CAPABILITY_ERROR_CHECK))\n> > > > >  \t\tclient->flags |= I2C_CLIENT_PEC;\n> > >  \n> > > > > -\tpmbus_clear_faults(client);\n> > > > > +\tret = pmbus_clear_faults(client);\n> > > > > +\tif (ret < 0)\n> > > > > +\t\treturn ret;\n> > >  \n> > > > >  \tif (info->identify) {\n> > > > >  \t\tret = (*info->identify)(client, info);\n> > > -- \n> > > 2.11.0\n> > >","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 3xnf495T1wz9s1h\n\tfor ;\n\tThu, 7 Sep 2017 08:52:41 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xnf494LSNzDrKL\n\tfor ;\n\tThu, 7 Sep 2017 08:52:41 +1000 (AEST)","from mail-pf0-x241.google.com (mail-pf0-x241.google.com\n\t[IPv6:2607:f8b0:400e:c00::241])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128\n\tbits)) (No client certificate requested)\n\tby lists.ozlabs.org (Postfix) with ESMTPS id 3xnf2M1F8PzDrVw\n\tfor ; Thu, 7 Sep 2017 08:51:06 +1000 (AEST)","by mail-pf0-x241.google.com with SMTP id q76so3650258pfq.5\n\tfor ; Wed, 06 Sep 2017 15:51:06 -0700 (PDT)","from localhost (108-223-40-66.lightspeed.sntcca.sbcglobal.net.\n\t[108.223.40.66]) by smtp.gmail.com with ESMTPSA id\n\tm184sm994674pfc.82.2017.09.06.15.51.03\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tWed, 06 Sep 2017 15:51:03 -0700 (PDT)"],"Authentication-Results":["ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"LIdUKSeQ\"; dkim-atps=neutral","lists.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"LIdUKSeQ\"; dkim-atps=neutral","ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=gmail.com\n\t(client-ip=2607:f8b0:400e:c00::241; helo=mail-pf0-x241.google.com;\n\tenvelope-from=groeck7@gmail.com; receiver=)","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"LIdUKSeQ\"; dkim-atps=neutral"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=sender:date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=mxx4Klx4SLsnkX3NcYap5sYBEta1RkE8Y+lRDdPzrrI=;\n\tb=LIdUKSeQERD1rnClGO0wRb+7cjDnk6Brw6+OIz60GleJRkiFoCNneCwxJQK9NsRWsw\n\tmRr1KV9H2Inu0eN0gADdJ6otKJUn4/EU5uQH5Q0RXPdh7DK2lWV53nrJXVSOUlIG6TuT\n\t5tdXvrp0Ky5rk7ApN/2uIojM3KXUzFHHaLuTUsrL9fTrI4k12mYwST+qvVC2lrRjzP5U\n\tbb/HFVeCfHZmj2YH58fMWNyFg89baTZSLGneLuW5gg3Kv562C0LLPCc/uz8DnZuATvdh\n\tGdPQwVbGIImRsYmtp7xMTRMkf/ovVHhPvlzdPqKNzH9rW0nT7HFcYRFIAOmE22djI1p+\n\tGeoA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:sender:date:from:to:cc:subject:message-id\n\t:references:mime-version:content-disposition\n\t:content-transfer-encoding:in-reply-to:user-agent;\n\tbh=mxx4Klx4SLsnkX3NcYap5sYBEta1RkE8Y+lRDdPzrrI=;\n\tb=anCfC3H6gPzW68dLs4hTo9C99z17FjiLIHxBN8aJ4A7I1YNYC7VLWHYnATISDK0kdE\n\ty+qWTymiXSqYb3WnThD5WJwYsEcHHkucx1KkbHULFWNtYCDWdkinr3HX9Uh2UHCE/9BV\n\tkJmKpGo7rf4BZFqCzb1q5IoxLbF7vA3wDr5H30GDzDVH0uX2EKk+0eLTnCtQgH4kuhNv\n\tbt65M4nYtH/PYrgeNnEaGX4ZeVI/EgVaHaLO6z2vGgI1q6/vjSnqLLt144OiliR9xqfj\n\tQd9X3oY+1NkBSEQWnRBrj2AtOdjtctitfysv4A/NdAwPVcx9pXIDNPeLhO7z6znwBkre\n\tN6wA==","X-Gm-Message-State":"AHPjjUioGgCY2CoJBKf4b3E/kgECiIFtgs/K8QeEvZmNhyeMsdzXACn2\n\tiTlMFaDrUDP5kA==","X-Google-Smtp-Source":"ADKCNb5sAx9yhbSpcTGpOgNoc9Iq05cUq9hWASEYBgUi1RNfAqO3tUb5qVSEIMNXjru9+kyQ0Pxaqw==","X-Received":"by 10.98.194.84 with SMTP id l81mr670174pfg.303.1504738264194;\n\tWed, 06 Sep 2017 15:51:04 -0700 (PDT)","Date":"Wed, 6 Sep 2017 15:51:02 -0700","From":"Guenter Roeck ","To":"Andrew Jeffery ","Subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","Message-ID":"<20170906225102.GA32210@roeck-us.net>","References":"<20170905070132.17682-1-andrew@aj.id.au>\n\t<20170905170002.GG11478@roeck-us.net>\n\t<1504657417.28363.8.camel@aj.id.au>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<1504657417.28363.8.camel@aj.id.au>","User-Agent":"Mutt/1.5.24 (2015-08-30)","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":"linux-hwmon@vger.kernel.org, jdelvare@suse.com, linux-doc@vger.kernel.org,\n\topenbmc@lists.ozlabs.org, corbet@lwn.net, linux-kernel@vger.kernel.org","Errors-To":"openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"openbmc\"\n\t"}},{"id":1764435,"web_url":"http://patchwork.ozlabs.org/comment/1764435/","msgid":"<1504740749.5042.2.camel@aj.id.au>","list_archive_url":null,"date":"2017-09-06T23:32:29","subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","submitter":{"id":68332,"url":"http://patchwork.ozlabs.org/api/people/68332/","name":"Andrew Jeffery","email":"andrew@aj.id.au"},"content":"On Wed, 2017-09-06 at 15:51 -0700, Guenter Roeck wrote:\n> On Wed, Sep 06, 2017 at 10:23:37AM +1000, Andrew Jeffery wrote:\n> > On Tue, 2017-09-05 at 10:00 -0700, Guenter Roeck wrote:\n> > > On Tue, Sep 05, 2017 at 05:01:32PM +1000, Andrew Jeffery wrote:\n> > > > Some functions exposed by pmbus core conflated errors that occurred when\n> > > > setting the page to access with errors that occurred when accessing\n> > > > registers in a page. In some cases, this caused legitimate errors to be\n> > > > hidden under the guise of the register not being supported.\n> > > > \n> > > \n> > > I'll have to look into this. If I recall correctly, the reason for not returning\n> > > errors from the \"clear faults\" command was that some early chips did not support\n> > > it, or did not support it on all pages. Those chips would now always fail.\n> > \n> > Yes, that is a concern.\n> > \n> > However, shouldn't the lack of support for CLEAR_FAULTS be a described\n> > property of the chip or page?\n> > \n> > Regardless, the issue here is the PAGE command is sometimes failing\n> > (more below). This means we won't have issued a CLEAR_FAULTS against\n> > the page even if the page supports CLEAR_FAULTS. That to me is\n> > something that should be propagated back up the call chain, as faults\n> > that can be cleared will not have been.\n> > \n> \n> We could also consider\n> - retry\n\nI guess that leads to the usual retries problem: How many? Oneshot? More?\n\nMy expectation is that oneshot would be enough, but we'd still need to consider\nwhat to do if that wasn't successful.\n\nDoes the retry policy need to be in the kernel? I guess if it's not we would\nneed all operations in the path to the failure to be idempotent.\n\n> - Add a marker indicating that faults (specifically command errors)\n>   are unreliable after clearing faults failed\n\nWhat kind of marker were you thinking? Something in dmesg? If it's anything\nelse we'd probably want something to respond to the condition, which nothing\nwould do currently.\n\n> \n> If we can't reliably clear faults, all bets are off.\n\nYeah, it's a messy problem. However even if we resolve our issues in hardware\n(i.e. it's discovered that it is a design failure or something), I don't think\nwe should drop a resolution to the problem highlighted by this patch.\n\n> \n> > > \n> > > On a higher level, I am not sure if it is a good idea to return an error\n> > > from a function intended to clear faults (and nothing else). That seems\n> > > counterproductive.\n> > > \n> > > Is this a problem you have actually observed ? \n> > \n> > Unfortunately yes. I was trying to reproduce some issues that we are\n> > seeing in userspace but wasn't having much luck (getting -EIO on\n> > reading a hwmon attribute). I ended up instrumenting our I2C bus\n> > driver, and found that the problem was more prevalent than what the\n> > read failures in userspace were indicating. The paths that were\n> > triggering these unreported conditions all traced through the check\n> > register and clear fault functions, which on analysis were swallowing\n> > the error.\n> > \n> > > If so, what is the chip ?\n> > \n> > Well, the chip that I'm *communicating* with is the Maxim MAX31785\n> > Intelligent Fan Controller. As to whether that's what is *causing* the\n> > PAGE command to fail is still up in the air. I would not be surprised\n> > if we have other issues in the hardware design.\n> > \n> > I haven't sent a follow-up to the series introducing the MAX31785\n> > driver because I've been chasing down communication issues. I felt it\n> > was important to understand whether the device has quirks that need to\n> > be worked around in the driver, or if our hardware design has bigger\n> > problems that should be handled in other ways. I've been in touch with\n> > Maxim who have asserted that some of the problems we're seeing cannot\n> > be caused by their chip.\n> > \n> \n> Guess I need to dig up my eval board and see if I can reproduce the problem.\n> Seems you are saying that the problem is always seen when issuing a sequence\n> of \"clear faults\" commands on multiple pages ?\n\nYeah. We're also seeing bad behaviour under other command sequences as well,\nwhich lead to this hack of a work-around patch[1].\n\nI'd be very interested in the results of testing against the eval board. I\ndon't have access to one and it seems Maxim have discontinued them.\n\n[1] https://patchwork.kernel.org/patch/9876083/\n\n> \n> The MAX31785 is microcode based, so I would not be entirely surprised if\n> it sometimes fails to reply if a sequence of \n> commands is executed.\n\nHave you seen this behaviour in other microcode-based chips?\n\n> \n> If possible, you might try reducing the i2c clock. I have not seen that with\n> Maxim chips, but some other PMBus chips don't operate reliably at 400 KHz.\n\nIt's already running at 100kHz, which is the maximum clock rate the device is\nspecified for. I've even underclocked the bus to 75kHz and still observed\nissues. Again, I wouldn't be surprised if the problem is something other than\nthe Maxim chip, the bus that we have it on has a complex topology. I'm working\nwith hardware people internally to try to isolate the problem.\n\nCheers,\n\nAndrew\n\n> \n> Guenter\n> \n> > > \n> > > > > > Signed-off-by: Andrew Jeffery \n> > > > \n> > > > ---\n> > > >  Documentation/hwmon/pmbus-core   |  12 ++--\n> > > >  drivers/hwmon/pmbus/pmbus.h      |   6 +-\n> > > >  drivers/hwmon/pmbus/pmbus_core.c | 115 +++++++++++++++++++++++++++++----------\n> > > >  3 files changed, 95 insertions(+), 38 deletions(-)\n> > > > \n> > > > diff --git a/Documentation/hwmon/pmbus-core b/Documentation/hwmon/pmbus-core\n> > > > index 8ed10e9ddfb5..3e9f41bb756f 100644\n> > > > --- a/Documentation/hwmon/pmbus-core\n> > > > +++ b/Documentation/hwmon/pmbus-core\n> > > > @@ -218,17 +218,17 @@ Specifically, it provides the following information.\n> > > >    This function calls the device specific write_byte function if defined.\n> > > >    Therefore, it must _not_ be called from that function.\n> > > >  \n> > > > -  bool pmbus_check_byte_register(struct i2c_client *client, int page, int reg);\n> > > > +  int pmbus_check_byte_register(struct i2c_client *client, int page, int reg);\n> > > >  \n> > > > -  Check if byte register exists. Return true if the register exists, false\n> > > > -  otherwise.\n> > > > +  Check if byte register exists. Returns 1 if the register exists, 0 if it does\n> > > > +  not, and less than zero on an unexpected error.\n> > > >    This function calls the device specific write_byte function if defined to\n> > > >    obtain the chip status. Therefore, it must _not_ be called from that function.\n> > > >  \n> > > > -  bool pmbus_check_word_register(struct i2c_client *client, int page, int reg);\n> > > > +  int pmbus_check_word_register(struct i2c_client *client, int page, int reg);\n> > > >  \n> > > > -  Check if word register exists. Return true if the register exists, false\n> > > > -  otherwise.\n> > > > +  Check if word register exists. Returns 1 if the register exists, 0 if it does\n> > > > +  not, and less than zero on an unexpected error.\n> > > >    This function calls the device specific write_byte function if defined to\n> > > >    obtain the chip status. Therefore, it must _not_ be called from that function.\n> > > >  \n> > > > diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h\n> > > > index bfcb13bae34b..c53032a04a6f 100644\n> > > > --- a/drivers/hwmon/pmbus/pmbus.h\n> > > > +++ b/drivers/hwmon/pmbus/pmbus.h\n> > > > @@ -413,9 +413,9 @@ int pmbus_write_byte_data(struct i2c_client *client, int page, u8 reg,\n> > > > > >  \t\t\t  u8 value);\n> > > > \n> > > >  int pmbus_update_byte_data(struct i2c_client *client, int page, u8 reg,\n> > > > > >  \t\t\t   u8 mask, u8 value);\n> > > > \n> > > > -void pmbus_clear_faults(struct i2c_client *client);\n> > > > -bool pmbus_check_byte_register(struct i2c_client *client, int page, int reg);\n> > > > -bool pmbus_check_word_register(struct i2c_client *client, int page, int reg);\n> > > > +int pmbus_clear_faults(struct i2c_client *client);\n> > > > +int pmbus_check_byte_register(struct i2c_client *client, int page, int reg);\n> > > > +int pmbus_check_word_register(struct i2c_client *client, int page, int reg);\n> > > >  int pmbus_do_probe(struct i2c_client *client, const struct i2c_device_id *id,\n> > > > > >  \t\t   struct pmbus_driver_info *info);\n> > > > \n> > > >  int pmbus_do_remove(struct i2c_client *client);\n> > > > diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c\n> > > > index f1eff6b6c798..153700e35431 100644\n> > > > --- a/drivers/hwmon/pmbus/pmbus_core.c\n> > > > +++ b/drivers/hwmon/pmbus/pmbus_core.c\n> > > > @@ -304,18 +304,24 @@ static int _pmbus_read_byte_data(struct i2c_client *client, int page, int reg)\n> > > > > >  \treturn pmbus_read_byte_data(client, page, reg);\n> > > > \n> > > >  }\n> > > >  \n> > > > -static void pmbus_clear_fault_page(struct i2c_client *client, int page)\n> > > > +static int pmbus_clear_fault_page(struct i2c_client *client, int page)\n> > > >  {\n> > > > > > > > > > > > -\t_pmbus_write_byte(client, page, PMBUS_CLEAR_FAULTS);\n> > > > > > +\treturn _pmbus_write_byte(client, page, PMBUS_CLEAR_FAULTS);\n> > > > \n> > > >  }\n> > > >  \n> > > > -void pmbus_clear_faults(struct i2c_client *client)\n> > > > +int pmbus_clear_faults(struct i2c_client *client)\n> > > >  {\n> > > > > > > > > > > >  \tstruct pmbus_data *data = i2c_get_clientdata(client);\n> > > > > > > > > > > > +\tint rv;\n> > > > > >  \tint i;\n> > > > \n> > > >  \n> > > > > > > > > > > > -\tfor (i = 0; i < data->info->pages; i++)\n> > > > > > > > > > > > -\t\tpmbus_clear_fault_page(client, i);\n> > > > > > > > > > > > +\tfor (i = 0; i < data->info->pages; i++) {\n> > > > > > > > > > > > +\t\trv = pmbus_clear_fault_page(client, i);\n> > > > > > > > > > > > +\t\tif (rv)\n> > > > > > > > > > > > +\t\t\treturn rv;\n> > > > > > +\t}\n> > > > \n> > > > +\n> > > > > > +\treturn 0;\n> > > > \n> > > >  }\n> > > >  EXPORT_SYMBOL_GPL(pmbus_clear_faults);\n> > > >  \n> > > > @@ -333,28 +339,45 @@ static int pmbus_check_status_cml(struct i2c_client *client)\n> > > > > >  \treturn 0;\n> > > > \n> > > >  }\n> > > >  \n> > > > -static bool pmbus_check_register(struct i2c_client *client,\n> > > > +static int pmbus_check_register(struct i2c_client *client,\n> > > > > > > > > > > >  \t\t\t\t int (*func)(struct i2c_client *client,\n> > > > > > > > > > > >  \t\t\t\t\t     int page, int reg),\n> > > > > >  \t\t\t\t int page, int reg)\n> > > > \n> > > >  {\n> > > > > > > > > > > > +\tstruct pmbus_data *data;\n> > > > > > > > > > > > +\tint check;\n> > > > > > > > > > > >  \tint rv;\n> > > > > > -\tstruct pmbus_data *data = i2c_get_clientdata(client);\n> > > > \n> > > >  \n> > > > > > > > > > > > -\trv = func(client, page, reg);\n> > > > > > > > > > > > -\tif (rv >= 0 && !(data->flags & PMBUS_SKIP_STATUS_CHECK))\n> > > > > > > > > > > > -\t\trv = pmbus_check_status_cml(client);\n> > > > > > > > > > > > -\tpmbus_clear_fault_page(client, -1);\n> > > > > > > > > > > > -\treturn rv >= 0;\n> > > > > > +\tdata = i2c_get_clientdata(client);\n> > > > \n> > > > +\n> > > > > > > > > > > > +\t/*\n> > > > > > > > > > > > +\t * pmbus_set_page() guards transactions on the requested page matching\n> > > > > > > > > > > > +\t * the current page. This may be done in the execution of func(), but\n> > > > > > > > > > > > +\t * at that point a set-page error is conflated with accessing a\n> > > > > > > > > > > > +\t * non-existent register.\n> > > > > > > > > > > > +\t */\n> > > > > > > > > > > > +\trv = pmbus_set_page(client, page);\n> > > > > > > > > > > > +\tif (rv < 0)\n> > > > > > +\t\treturn rv;\n> > > > \n> > > > +\n> > > > > > > > > > > > +\tcheck = func(client, page, reg);\n> > > > > > > > > > > > +\tif (check >= 0 && !(data->flags & PMBUS_SKIP_STATUS_CHECK))\n> > > > > > +\t\tcheck = pmbus_check_status_cml(client);\n> > > > \n> > > > +\n> > > > > > > > > > > > +\trv = pmbus_clear_fault_page(client, -1);\n> > > > > > > > > > > > +\tif (rv < 0)\n> > > > > > +\t\treturn rv;\n> > > > \n> > > > +\n> > > > > > +\treturn check >= 0;\n> > > > \n> > > >  }\n> > > >  \n> > > > -bool pmbus_check_byte_register(struct i2c_client *client, int page, int reg)\n> > > > +int pmbus_check_byte_register(struct i2c_client *client, int page, int reg)\n> > > >  {\n> > > > > >  \treturn pmbus_check_register(client, _pmbus_read_byte_data, page, reg);\n> > > > \n> > > >  }\n> > > >  EXPORT_SYMBOL_GPL(pmbus_check_byte_register);\n> > > >  \n> > > > -bool pmbus_check_word_register(struct i2c_client *client, int page, int reg)\n> > > > +int pmbus_check_word_register(struct i2c_client *client, int page, int reg)\n> > > >  {\n> > > > > >  \treturn pmbus_check_register(client, _pmbus_read_word_data, page, reg);\n> > > > \n> > > >  }\n> > > > @@ -390,7 +413,7 @@ static struct pmbus_data *pmbus_update_device(struct device *dev)\n> > > >  \n> > > > > > > > > > > >  \tmutex_lock(&data->update_lock);\n> > > > > > > > > > > >  \tif (time_after(jiffies, data->last_updated + HZ) || !data->valid) {\n> > > > > > > > > > > > -\t\tint i, j;\n> > > > > > +\t\tint i, j, ret;\n> > > > \n> > > >  \n> > > > > > > > > > > >  \t\tfor (i = 0; i < info->pages; i++) {\n> > > > > >  \t\t\tdata->status[PB_STATUS_BASE + i]\n> > > > \n> > > > @@ -424,7 +447,13 @@ static struct pmbus_data *pmbus_update_device(struct device *dev)\n> > > > > > > > > > > >  \t\t\t\t\t\t\t    sensor->page,\n> > > > > > > > > > > >  \t\t\t\t\t\t\t    sensor->reg);\n> > > > > > > > > > > >  \t\t}\n> > > > > > -\t\tpmbus_clear_faults(client);\n> > > > \n> > > > +\n> > > > > > > > > > > > +\t\tret = pmbus_clear_faults(client);\n> > > > > > > > > > > > +\t\tif (ret < 0) {\n> > > > > > > > > > > > +\t\t\tmutex_unlock(&data->update_lock);\n> > > > > > > > > > > > +\t\t\treturn ERR_PTR(ret);\n> > > > > > +\t\t}\n> > > > \n> > > > +\n> > > > > > > > > > > >  \t\tdata->last_updated = jiffies;\n> > > > > > > > > > > >  \t\tdata->valid = 1;\n> > > > > >  \t}\n> > > > \n> > > > @@ -754,6 +783,9 @@ static ssize_t pmbus_show_boolean(struct device *dev,\n> > > > > > > > > > > >  \tstruct pmbus_data *data = pmbus_update_device(dev);\n> > > > > >  \tint val;\n> > > > \n> > > >  \n> > > > > > > > > > > > +\tif (IS_ERR(data))\n> > > > > > +\t\treturn PTR_ERR(data);\n> > > > \n> > > > +\n> > > > > > > > > > > >  \tval = pmbus_get_boolean(data, boolean, attr->index);\n> > > > > > > > > > > >  \tif (val < 0)\n> > > > > >  \t\treturn val;\n> > > > \n> > > > @@ -766,6 +798,9 @@ static ssize_t pmbus_show_sensor(struct device *dev,\n> > > > > > > > > > > >  \tstruct pmbus_data *data = pmbus_update_device(dev);\n> > > > > >  \tstruct pmbus_sensor *sensor = to_pmbus_sensor(devattr);\n> > > > \n> > > >  \n> > > > > > > > > > > > +\tif (IS_ERR(data))\n> > > > > > +\t\treturn PTR_ERR(data);\n> > > > \n> > > > +\n> > > > > > > > > > > >  \tif (sensor->data < 0)\n> > > > > >  \t\treturn sensor->data;\n> > > > \n> > > >  \n> > > > @@ -995,7 +1030,11 @@ static int pmbus_add_limit_attrs(struct i2c_client *client,\n> > > > > >  \tstruct pmbus_sensor *curr;\n> > > > \n> > > >  \n> > > > > > > > > > > >  \tfor (i = 0; i < nlimit; i++) {\n> > > > > > > > > > > > -\t\tif (pmbus_check_word_register(client, page, l->reg)) {\n> > > > > > > > > > > > +\t\tret = pmbus_check_word_register(client, page, l->reg);\n> > > > > > > > > > > > +\t\tif (ret < 0)\n> > > > > > +\t\t\treturn ret;\n> > > > \n> > > > +\n> > > > > > > > > > > > +\t\tif (ret) {\n> > > > > > > > > > > >  \t\t\tcurr = pmbus_add_sensor(data, name, l->attr, index,\n> > > > > > > > > > > >  \t\t\t\t\t\tpage, l->reg, attr->class,\n> > > > > >  \t\t\t\t\t\tattr->update || l->update,\n> > > > \n> > > > @@ -1041,6 +1080,8 @@ static int pmbus_add_sensor_attrs_one(struct i2c_client *client,\n> > > > > > > > > > > >  \tif (!base)\n> > > > > > > > > > > >  \t\treturn -ENOMEM;\n> > > > > > > > > > > >  \tif (attr->sfunc) {\n> > > > > > +\t\tint check;\n> > > > \n> > > > +\n> > > > > > > > > > > >  \t\tret = pmbus_add_limit_attrs(client, data, info, name,\n> > > > > > > > > > > >  \t\t\t\t\t    index, page, base, attr);\n> > > > > >  \t\tif (ret < 0)\n> > > > \n> > > > @@ -1050,9 +1091,13 @@ static int pmbus_add_sensor_attrs_one(struct i2c_client *client,\n> > > > > > > > > > > >  \t\t * alarm attributes, if there is a global alarm bit, and if\n> > > > > > > > > > > >  \t\t * the generic status register for this page is accessible.\n> > > > > > > > > > > >  \t\t */\n> > > > > > > > > > > > -\t\tif (!ret && attr->gbit &&\n> > > > > > > > > > > > -\t\t    pmbus_check_byte_register(client, page,\n> > > > > > -\t\t\t\t\t      data->status_register)) {\n> > > > \n> > > > +\n> > > > > > > > > > > > +\t\tcheck = pmbus_check_byte_register(client, page,\n> > > > > > > > > > > > +\t\t\t\t\t\t  data->status_register);\n> > > > > > > > > > > > +\t\tif (check < 0)\n> > > > > > +\t\t\treturn check;\n> > > > \n> > > > +\n> > > > > > > > > > > > +\t\tif (!ret && attr->gbit && check) {\n> > > > > > > > > > > >  \t\t\tret = pmbus_add_boolean(data, name, \"alarm\", index,\n> > > > > > > > > > > >  \t\t\t\t\t\tNULL, NULL,\n> > > > > >  \t\t\t\t\t\tPB_STATUS_BASE + page,\n> > > > \n> > > > @@ -1604,8 +1649,12 @@ static int pmbus_add_fan_attributes(struct i2c_client *client,\n> > > > > > > > > > > >  \t\t\tif (!(info->func[page] & pmbus_fan_flags[f]))\n> > > > > >  \t\t\t\tbreak;\n> > > > \n> > > >  \n> > > > > > > > > > > > -\t\t\tif (!pmbus_check_word_register(client, page,\n> > > > > > > > > > > > -\t\t\t\t\t\t       pmbus_fan_registers[f]))\n> > > > > > > > > > > > +\t\t\tret = pmbus_check_word_register(client, page,\n> > > > > > > > > > > > +\t\t\t\t\t\t       pmbus_fan_registers[f]);\n> > > > > > > > > > > > +\t\t\tif (ret < 0)\n> > > > > > +\t\t\t\treturn ret;\n> > > > \n> > > > +\n> > > > > > > > > > > > +\t\t\tif (!ret)\n> > > > > >  \t\t\t\tbreak;\n> > > > \n> > > >  \n> > > > > >  \t\t\t/*\n> > > > \n> > > > @@ -1628,9 +1677,13 @@ static int pmbus_add_fan_attributes(struct i2c_client *client,\n> > > > > > > > > > > >  \t\t\t * Each fan status register covers multiple fans,\n> > > > > > > > > > > >  \t\t\t * so we have to do some magic.\n> > > > > > > > > > > >  \t\t\t */\n> > > > > > > > > > > > -\t\t\tif ((info->func[page] & pmbus_fan_status_flags[f]) &&\n> > > > > > > > > > > > -\t\t\t    pmbus_check_byte_register(client,\n> > > > > > > > > > > > -\t\t\t\t\tpage, pmbus_fan_status_registers[f])) {\n> > > > > > > > > > > > +\t\t\tret =  pmbus_check_byte_register(client, page,\n> > > > > > > > > > > > +\t\t\t\t\t\tpmbus_fan_status_registers[f]);\n> > > > > > > > > > > > +\t\t\tif (ret < 0)\n> > > > > > +\t\t\t\treturn ret;\n> > > > \n> > > > +\n> > > > > > > > > > > > +\t\t\tif ((info->func[page] & pmbus_fan_status_flags[f])\n> > > > > > > > > > > > +\t\t\t\t\t&& ret) {\n> > > > > >  \t\t\t\tint base;\n> > > > \n> > > >  \n> > > > > > > >  \t\t\t\tif (f > 1)\t/* fan 3, 4 */\n> > > > \n> > > > @@ -1696,10 +1749,13 @@ static int pmbus_identify_common(struct i2c_client *client,\n> > > > > >  \t\t\t\t struct pmbus_data *data, int page)\n> > > > \n> > > >  {\n> > > > > > > > > > > >  \tint vout_mode = -1;\n> > > > > > +\tint rv;\n> > > > \n> > > >  \n> > > > > > > > > > > > -\tif (pmbus_check_byte_register(client, page, PMBUS_VOUT_MODE))\n> > > > > > > > > > > > +\trv = pmbus_check_byte_register(client, page, PMBUS_VOUT_MODE);\n> > > > > > > > > > > > +\tif (rv == 1)\n> > > > > > > > > > > >  \t\tvout_mode = _pmbus_read_byte_data(client, page,\n> > > > > >  \t\t\t\t\t\t  PMBUS_VOUT_MODE);\n> > > > \n> > > > +\n> > > > > > > > > > > >  \tif (vout_mode >= 0 && vout_mode != 0xff) {\n> > > > > > > > > > > >  \t\t/*\n> > > > > >  \t\t * Not all chips support the VOUT_MODE command,\n> > > > \n> > > > @@ -1725,8 +1781,7 @@ static int pmbus_identify_common(struct i2c_client *client,\n> > > > > > > > > > > >  \t\t}\n> > > > > >  \t}\n> > > > \n> > > >  \n> > > > > > > > > > > > -\tpmbus_clear_fault_page(client, page);\n> > > > > > > > > > > > -\treturn 0;\n> > > > > > +\treturn pmbus_clear_fault_page(client, page);\n> > > > \n> > > >  }\n> > > >  \n> > > >  static int pmbus_init_common(struct i2c_client *client, struct pmbus_data *data,\n> > > > @@ -1756,7 +1811,9 @@ static int pmbus_init_common(struct i2c_client *client, struct pmbus_data *data,\n> > > > > > > > > > > >  \tif (ret >= 0 && (ret & PB_CAPABILITY_ERROR_CHECK))\n> > > > > >  \t\tclient->flags |= I2C_CLIENT_PEC;\n> > > > \n> > > >  \n> > > > > > > > > > > > -\tpmbus_clear_faults(client);\n> > > > > > > > > > > > +\tret = pmbus_clear_faults(client);\n> > > > > > > > > > > > +\tif (ret < 0)\n> > > > > > +\t\treturn ret;\n> > > > \n> > > >  \n> > > > > > > > > > > >  \tif (info->identify) {\n> > > > > >  \t\tret = (*info->identify)(client, info);\n> > > > \n> > > > -- \n> > > > 2.11.0\n> > > > \n> \n>","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 3xnfyl6x1Yz9s4s\n\tfor ;\n\tThu, 7 Sep 2017 09:33:03 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xnfyl5VyMzDrWQ\n\tfor ;\n\tThu, 7 Sep 2017 09:33:03 +1000 (AEST)","from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com\n\t[66.111.4.25])\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 3xnfyL3Tj6zDrW1\n\tfor ; Thu, 7 Sep 2017 09:32:41 +1000 (AEST)","from compute4.internal (compute4.nyi.internal [10.202.2.44])\n\tby mailout.nyi.internal (Postfix) with ESMTP id 40793219D9;\n\tWed, 6 Sep 2017 19:32:38 -0400 (EDT)","from frontend2 ([10.202.2.161])\n\tby compute4.internal (MEProxy); Wed, 06 Sep 2017 19:32:38 -0400","from keelia (bh02i525f01.au.ibm.com [202.81.18.30])\n\tby mail.messagingengine.com (Postfix) with ESMTPA id 3182024335;\n\tWed, 6 Sep 2017 19:32:34 -0400 (EDT)"],"Authentication-Results":["ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"YxB+lBrH\";\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"WtXY7e4O\"; \n\tdkim-atps=neutral","lists.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"YxB+lBrH\";\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"WtXY7e4O\"; \n\tdkim-atps=neutral","ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=aj.id.au\n\t(client-ip=66.111.4.25; helo=out1-smtp.messagingengine.com;\n\tenvelope-from=andrew@aj.id.au; receiver=)","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"YxB+lBrH\";\n\tdkim=pass (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com\n\theader.b=\"WtXY7e4O\"; 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=tYX3XD0dXVlZGMYkj/yjx6a8WXa0/SqjdR7wTZ2DJ\n\ta0=; b=YxB+lBrHxOpCYzzh9GTdg3L2N0ygej2N17pzI/DS9lu2rhFjdGIzGfRzf\n\t7cjsN4EtjqzxB5mlwJrdpkWPV6Ol+7oDqVe495X9tANAIR79hzVOBNOCXB6FQRqx\n\t87GA+HoI07wI+s9ZU+VJ17kW9sxYKLMVeBFkCPrg2ITkEsjX9DpF1tPA0Pt4Koxs\n\t6UV7Ahrj+Y3P6GBeX175KC/fCkx79bcx4gFS2/ISQOI0x2NMsAdPYkvqHRfk0l18\n\tWtpWu9HCKIuZcRBCRCJTLdDlHHYtEY8UX4IHDxVOWMfnx/SrtwGJBPydCgI8hkB+\n\tXzMqBmKT1zdpANVyTCzYa5egm337w==","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=tYX3XD0dXVlZGMYkj/\n\tyjx6a8WXa0/SqjdR7wTZ2DJa0=; b=WtXY7e4OvbrlDxlby95kW4XKdM94G/xBpE\n\ti42CHobmLizJbWWTVSeXogwrZll94UFVbuukywDciPQW2PpaCJ6uyKBj1zckdWlv\n\tdAfF5eszrvQa+pobNUAIauLPn5G3UW8AbMKUagl14d1v1M/GylZRhR4sI+KLO1A/\n\tBBRbJyRq8UKv3uabVCj7L7r2rSvuXcFzwe9npLJa417UG/h0BwsZhEFWfGkjiLQ/\n\tZQ7Ebp3ZNUOkO0dbexvbgSB0RMNst158MZonzGGNznAdo8SqMS6K0ghIVob9bi7x\n\t7c4wsAbmfWXOmfP5ueBibgB59LoEthv1vEI5WXQqNJS83braY51w=="],"X-ME-Sender":"","X-Sasl-enc":"gwajT4NDl00x8YpMzT8+IuJwMpg2wOpCIV6e0jOab/kd 1504740757","Message-ID":"<1504740749.5042.2.camel@aj.id.au>","Subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","From":"Andrew Jeffery ","To":"Guenter Roeck ","Date":"Thu, 07 Sep 2017 09:32:29 +1000","In-Reply-To":"<20170906225102.GA32210@roeck-us.net>","References":"<20170905070132.17682-1-andrew@aj.id.au>\n\t<20170905170002.GG11478@roeck-us.net>\n\t<1504657417.28363.8.camel@aj.id.au>\n\t<20170906225102.GA32210@roeck-us.net>","Content-Type":"multipart/signed; micalg=\"pgp-sha512\";\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"=-Tge/ZX86d6Oilq2PelpP\"","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":"linux-hwmon@vger.kernel.org, jdelvare@suse.com, linux-doc@vger.kernel.org,\n\topenbmc@lists.ozlabs.org, corbet@lwn.net, linux-kernel@vger.kernel.org","Errors-To":"openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"openbmc\"\n\t"}},{"id":1764475,"web_url":"http://patchwork.ozlabs.org/comment/1764475/","msgid":"<5089e197-5ee5-8775-501c-201546a0cc1a@roeck-us.net>","list_archive_url":null,"date":"2017-09-07T02:19:36","subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","submitter":{"id":21889,"url":"http://patchwork.ozlabs.org/api/people/21889/","name":"Guenter Roeck","email":"linux@roeck-us.net"},"content":"On 09/06/2017 04:32 PM, Andrew Jeffery wrote:\n> On Wed, 2017-09-06 at 15:51 -0700, Guenter Roeck wrote:\n>> On Wed, Sep 06, 2017 at 10:23:37AM +1000, Andrew Jeffery wrote:\n>>> On Tue, 2017-09-05 at 10:00 -0700, Guenter Roeck wrote:\n>>>> On Tue, Sep 05, 2017 at 05:01:32PM +1000, Andrew Jeffery wrote:\n>>>>> Some functions exposed by pmbus core conflated errors that occurred when\n>>>>> setting the page to access with errors that occurred when accessing\n>>>>> registers in a page. In some cases, this caused legitimate errors to be\n>>>>> hidden under the guise of the register not being supported.\n>>>>> \n>>>> \n>>>> I'll have to look into this. If I recall correctly, the reason for not returning\n>>>> errors from the \"clear faults\" command was that some early chips did not support\n>>>> it, or did not support it on all pages. Those chips would now always fail.\n>>> \n>>> Yes, that is a concern.\n>>> \n>>> However, shouldn't the lack of support for CLEAR_FAULTS be a described\n>>> property of the chip or page?\n>>> \n>>> Regardless, the issue here is the PAGE command is sometimes failing\n>>> (more below). This means we won't have issued a CLEAR_FAULTS against\n>>> the page even if the page supports CLEAR_FAULTS. That to me is\n>>> something that should be propagated back up the call chain, as faults\n>>> that can be cleared will not have been.\n>>> \n>> \n>> We could also consider\n>> - retry\n> \n> I guess that leads to the usual retries problem: How many? Oneshot? More?\n> \n> My expectation is that oneshot would be enough, but we'd still need to consider\n> what to do if that wasn't successful.\n> \n> Does the retry policy need to be in the kernel? I guess if it's not we would\n> need all operations in the path to the failure to be idempotent.\n> \n>> - Add a marker indicating that faults (specifically command errors)\n>> are unreliable after clearing faults failed\n> \n> What kind of marker were you thinking? Something in dmesg? If it's anything\n> else we'd probably want something to respond to the condition, which nothing\n> would do currently.\n> \n>> \n>> If we can't reliably clear faults, all bets are off.\n> \n> Yeah, it's a messy problem. However even if we resolve our issues in hardware\n> (i.e. it's discovered that it is a design failure or something), I don't think\n> we should drop a resolution to the problem highlighted by this patch.\n> \n>> \n>>>> \n>>>> On a higher level, I am not sure if it is a good idea to return an error\n>>>> from a function intended to clear faults (and nothing else). That seems\n>>>> counterproductive.\n>>>> \n>>>> Is this a problem you have actually observed ?\n>>> \n>>> Unfortunately yes. I was trying to reproduce some issues that we are\n>>> seeing in userspace but wasn't having much luck (getting -EIO on\n>>> reading a hwmon attribute). I ended up instrumenting our I2C bus\n>>> driver, and found that the problem was more prevalent than what the\n>>> read failures in userspace were indicating. The paths that were\n>>> triggering these unreported conditions all traced through the check\n>>> register and clear fault functions, which on analysis were swallowing\n>>> the error.\n>>> \n>>>> If so, what is the chip ?\n>>> \n>>> Well, the chip that I'm *communicating* with is the Maxim MAX31785\n>>> Intelligent Fan Controller. As to whether that's what is *causing* the\n>>> PAGE command to fail is still up in the air. I would not be surprised\n>>> if we have other issues in the hardware design.\n>>> \n>>> I haven't sent a follow-up to the series introducing the MAX31785\n>>> driver because I've been chasing down communication issues. I felt it\n>>> was important to understand whether the device has quirks that need to\n>>> be worked around in the driver, or if our hardware design has bigger\n>>> problems that should be handled in other ways. I've been in touch with\n>>> Maxim who have asserted that some of the problems we're seeing cannot\n>>> be caused by their chip.\n>>> \n>> \n>> Guess I need to dig up my eval board and see if I can reproduce the problem.\n>> Seems you are saying that the problem is always seen when issuing a sequence\n>> of \"clear faults\" commands on multiple pages ?\n> \n> Yeah. We're also seeing bad behaviour under other command sequences as well,\n> which lead to this hack of a work-around patch[1].\n> \n> I'd be very interested in the results of testing against the eval board. I\n> don't have access to one and it seems Maxim have discontinued them.\n> \n> [1] https://patchwork.kernel.org/patch/9876083/\n> \n>> \n>> The MAX31785 is microcode based, so I would not be entirely surprised if\n>> it sometimes fails to reply if a sequence of \n>> commands is executed.\n> \n> Have you seen this behaviour in other microcode-based chips?\n> \n\nltc2978 (commit e04d1ce9bbb) and most of the Zilker Labs chips (zl6100.c).\n\nYou could try the approach I used in the zl6100 driver: Add a minimum time\nbetween accesses to the chip. I would probably no longer use udelay(), though,\nbut usleeep_range(), if I were to implement the code today.\n\n>> \n>> If possible, you might try reducing the i2c clock. I have not seen that with\n>> Maxim chips, but some other PMBus chips don't operate reliably at 400 KHz.\n> \n> It's already running at 100kHz, which is the maximum clock rate the device is\n> specified for. I've even underclocked the bus to 75kHz and still observed\n> issues. Again, I wouldn't be surprised if the problem is something other than\n> the Maxim chip, the bus that we have it on has a complex topology. I'm working\n> with hardware people internally to try to isolate the problem.\n> \n\nIf you can, you might try something in between. Early TI chips (ucd9000 variants,\nsome sold as OEM chips to third parties) had a problem with both 100kHz and 400kHz,\nbut worked fine between about 150 kHz and 350 kHz, for whatever reason. Of course,\nmaybe that was a signal problem on our boards at the time (the i2c signal routing\nwas quite complex).\n\nGuenter\n\n> Cheers,\n> \n> Andrew\n> \n>> \n>> Guenter\n>> \n>>>> \n>>>>>>> Signed-off-by: Andrew Jeffery \n>>>>> \n>>>>> ---\n>>>>> Documentation/hwmon/pmbus-core | 12 ++--\n>>>>> drivers/hwmon/pmbus/pmbus.h | 6 +-\n>>>>> drivers/hwmon/pmbus/pmbus_core.c | 115 +++++++++++++++++++++++++++++----------\n>>>>> 3 files changed, 95 insertions(+), 38 deletions(-)\n>>>>> \n>>>>> diff --git a/Documentation/hwmon/pmbus-core b/Documentation/hwmon/pmbus-core\n>>>>> index 8ed10e9ddfb5..3e9f41bb756f 100644\n>>>>> --- a/Documentation/hwmon/pmbus-core\n>>>>> +++ b/Documentation/hwmon/pmbus-core\n>>>>> @@ -218,17 +218,17 @@ Specifically, it provides the following information.\n>>>>> This function calls the device specific write_byte function if defined.\n>>>>> Therefore, it must _not_ be called from that function.\n>>>>> \n>>>>> - bool pmbus_check_byte_register(struct i2c_client *client, int page, int reg);\n>>>>> + int pmbus_check_byte_register(struct i2c_client *client, int page, int reg);\n>>>>> \n>>>>> - Check if byte register exists. Return true if the register exists, false\n>>>>> - otherwise.\n>>>>> + Check if byte register exists. Returns 1 if the register exists, 0 if it does\n>>>>> + not, and less than zero on an unexpected error.\n>>>>> This function calls the device specific write_byte function if defined to\n>>>>> obtain the chip status. Therefore, it must _not_ be called from that function.\n>>>>> \n>>>>> - bool pmbus_check_word_register(struct i2c_client *client, int page, int reg);\n>>>>> + int pmbus_check_word_register(struct i2c_client *client, int page, int reg);\n>>>>> \n>>>>> - Check if word register exists. Return true if the register exists, false\n>>>>> - otherwise.\n>>>>> + Check if word register exists. Returns 1 if the register exists, 0 if it does\n>>>>> + not, and less than zero on an unexpected error.\n>>>>> This function calls the device specific write_byte function if defined to\n>>>>> obtain the chip status. Therefore, it must _not_ be called from that function.\n>>>>> \n>>>>> diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h\n>>>>> index bfcb13bae34b..c53032a04a6f 100644\n>>>>> --- a/drivers/hwmon/pmbus/pmbus.h\n>>>>> +++ b/drivers/hwmon/pmbus/pmbus.h\n>>>>> @@ -413,9 +413,9 @@ int pmbus_write_byte_data(struct i2c_client *client, int page, u8 reg,\n>>>>>>> \t\t\t u8 value);\n>>>>> \n>>>>> int pmbus_update_byte_data(struct i2c_client *client, int page, u8 reg,\n>>>>>>> \t\t\t u8 mask, u8 value);\n>>>>> \n>>>>> -void pmbus_clear_faults(struct i2c_client *client);\n>>>>> -bool pmbus_check_byte_register(struct i2c_client *client, int page, int reg);\n>>>>> -bool pmbus_check_word_register(struct i2c_client *client, int page, int reg);\n>>>>> +int pmbus_clear_faults(struct i2c_client *client);\n>>>>> +int pmbus_check_byte_register(struct i2c_client *client, int page, int reg);\n>>>>> +int pmbus_check_word_register(struct i2c_client *client, int page, int reg);\n>>>>> int pmbus_do_probe(struct i2c_client *client, const struct i2c_device_id *id,\n>>>>>>> \t\t struct pmbus_driver_info *info);\n>>>>> \n>>>>> int pmbus_do_remove(struct i2c_client *client);\n>>>>> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c\n>>>>> index f1eff6b6c798..153700e35431 100644\n>>>>> --- a/drivers/hwmon/pmbus/pmbus_core.c\n>>>>> +++ b/drivers/hwmon/pmbus/pmbus_core.c\n>>>>> @@ -304,18 +304,24 @@ static int _pmbus_read_byte_data(struct i2c_client *client, int page, int reg)\n>>>>>>> \treturn pmbus_read_byte_data(client, page, reg);\n>>>>> \n>>>>> }\n>>>>> \n>>>>> -static void pmbus_clear_fault_page(struct i2c_client *client, int page)\n>>>>> +static int pmbus_clear_fault_page(struct i2c_client *client, int page)\n>>>>> {\n>>>>>>>>>>>>> -\t_pmbus_write_byte(client, page, PMBUS_CLEAR_FAULTS);\n>>>>>>> +\treturn _pmbus_write_byte(client, page, PMBUS_CLEAR_FAULTS);\n>>>>> \n>>>>> }\n>>>>> \n>>>>> -void pmbus_clear_faults(struct i2c_client *client)\n>>>>> +int pmbus_clear_faults(struct i2c_client *client)\n>>>>> {\n>>>>>>>>>>>>> \tstruct pmbus_data *data = i2c_get_clientdata(client);\n>>>>>>>>>>>>> +\tint rv;\n>>>>>>> \tint i;\n>>>>> \n>>>>> \n>>>>>>>>>>>>> -\tfor (i = 0; i < data->info->pages; i++)\n>>>>>>>>>>>>> -\t\tpmbus_clear_fault_page(client, i);\n>>>>>>>>>>>>> +\tfor (i = 0; i < data->info->pages; i++) {\n>>>>>>>>>>>>> +\t\trv = pmbus_clear_fault_page(client, i);\n>>>>>>>>>>>>> +\t\tif (rv)\n>>>>>>>>>>>>> +\t\t\treturn rv;\n>>>>>>> +\t}\n>>>>> \n>>>>> +\n>>>>>>> +\treturn 0;\n>>>>> \n>>>>> }\n>>>>> EXPORT_SYMBOL_GPL(pmbus_clear_faults);\n>>>>> \n>>>>> @@ -333,28 +339,45 @@ static int pmbus_check_status_cml(struct i2c_client *client)\n>>>>>>> \treturn 0;\n>>>>> \n>>>>> }\n>>>>> \n>>>>> -static bool pmbus_check_register(struct i2c_client *client,\n>>>>> +static int pmbus_check_register(struct i2c_client *client,\n>>>>>>>>>>>>> \t\t\t\t int (*func)(struct i2c_client *client,\n>>>>>>>>>>>>> \t\t\t\t\t int page, int reg),\n>>>>>>> \t\t\t\t int page, int reg)\n>>>>> \n>>>>> {\n>>>>>>>>>>>>> +\tstruct pmbus_data *data;\n>>>>>>>>>>>>> +\tint check;\n>>>>>>>>>>>>> \tint rv;\n>>>>>>> -\tstruct pmbus_data *data = i2c_get_clientdata(client);\n>>>>> \n>>>>> \n>>>>>>>>>>>>> -\trv = func(client, page, reg);\n>>>>>>>>>>>>> -\tif (rv >= 0 && !(data->flags & PMBUS_SKIP_STATUS_CHECK))\n>>>>>>>>>>>>> -\t\trv = pmbus_check_status_cml(client);\n>>>>>>>>>>>>> -\tpmbus_clear_fault_page(client, -1);\n>>>>>>>>>>>>> -\treturn rv >= 0;\n>>>>>>> +\tdata = i2c_get_clientdata(client);\n>>>>> \n>>>>> +\n>>>>>>>>>>>>> +\t/*\n>>>>>>>>>>>>> +\t * pmbus_set_page() guards transactions on the requested page matching\n>>>>>>>>>>>>> +\t * the current page. This may be done in the execution of func(), but\n>>>>>>>>>>>>> +\t * at that point a set-page error is conflated with accessing a\n>>>>>>>>>>>>> +\t * non-existent register.\n>>>>>>>>>>>>> +\t */\n>>>>>>>>>>>>> +\trv = pmbus_set_page(client, page);\n>>>>>>>>>>>>> +\tif (rv < 0)\n>>>>>>> +\t\treturn rv;\n>>>>> \n>>>>> +\n>>>>>>>>>>>>> +\tcheck = func(client, page, reg);\n>>>>>>>>>>>>> +\tif (check >= 0 && !(data->flags & PMBUS_SKIP_STATUS_CHECK))\n>>>>>>> +\t\tcheck = pmbus_check_status_cml(client);\n>>>>> \n>>>>> +\n>>>>>>>>>>>>> +\trv = pmbus_clear_fault_page(client, -1);\n>>>>>>>>>>>>> +\tif (rv < 0)\n>>>>>>> +\t\treturn rv;\n>>>>> \n>>>>> +\n>>>>>>> +\treturn check >= 0;\n>>>>> \n>>>>> }\n>>>>> \n>>>>> -bool pmbus_check_byte_register(struct i2c_client *client, int page, int reg)\n>>>>> +int pmbus_check_byte_register(struct i2c_client *client, int page, int reg)\n>>>>> {\n>>>>>>> \treturn pmbus_check_register(client, _pmbus_read_byte_data, page, reg);\n>>>>> \n>>>>> }\n>>>>> EXPORT_SYMBOL_GPL(pmbus_check_byte_register);\n>>>>> \n>>>>> -bool pmbus_check_word_register(struct i2c_client *client, int page, int reg)\n>>>>> +int pmbus_check_word_register(struct i2c_client *client, int page, int reg)\n>>>>> {\n>>>>>>> \treturn pmbus_check_register(client, _pmbus_read_word_data, page, reg);\n>>>>> \n>>>>> }\n>>>>> @@ -390,7 +413,7 @@ static struct pmbus_data *pmbus_update_device(struct device *dev)\n>>>>> \n>>>>>>>>>>>>> \tmutex_lock(&data->update_lock);\n>>>>>>>>>>>>> \tif (time_after(jiffies, data->last_updated + HZ) || !data->valid) {\n>>>>>>>>>>>>> -\t\tint i, j;\n>>>>>>> +\t\tint i, j, ret;\n>>>>> \n>>>>> \n>>>>>>>>>>>>> \t\tfor (i = 0; i < info->pages; i++) {\n>>>>>>> \t\t\tdata->status[PB_STATUS_BASE + i]\n>>>>> \n>>>>> @@ -424,7 +447,13 @@ static struct pmbus_data *pmbus_update_device(struct device *dev)\n>>>>>>>>>>>>> \t\t\t\t\t\t\t sensor->page,\n>>>>>>>>>>>>> \t\t\t\t\t\t\t sensor->reg);\n>>>>>>>>>>>>> \t\t}\n>>>>>>> -\t\tpmbus_clear_faults(client);\n>>>>> \n>>>>> +\n>>>>>>>>>>>>> +\t\tret = pmbus_clear_faults(client);\n>>>>>>>>>>>>> +\t\tif (ret < 0) {\n>>>>>>>>>>>>> +\t\t\tmutex_unlock(&data->update_lock);\n>>>>>>>>>>>>> +\t\t\treturn ERR_PTR(ret);\n>>>>>>> +\t\t}\n>>>>> \n>>>>> +\n>>>>>>>>>>>>> \t\tdata->last_updated = jiffies;\n>>>>>>>>>>>>> \t\tdata->valid = 1;\n>>>>>>> \t}\n>>>>> \n>>>>> @@ -754,6 +783,9 @@ static ssize_t pmbus_show_boolean(struct device *dev,\n>>>>>>>>>>>>> \tstruct pmbus_data *data = pmbus_update_device(dev);\n>>>>>>> \tint val;\n>>>>> \n>>>>> \n>>>>>>>>>>>>> +\tif (IS_ERR(data))\n>>>>>>> +\t\treturn PTR_ERR(data);\n>>>>> \n>>>>> +\n>>>>>>>>>>>>> \tval = pmbus_get_boolean(data, boolean, attr->index);\n>>>>>>>>>>>>> \tif (val < 0)\n>>>>>>> \t\treturn val;\n>>>>> \n>>>>> @@ -766,6 +798,9 @@ static ssize_t pmbus_show_sensor(struct device *dev,\n>>>>>>>>>>>>> \tstruct pmbus_data *data = pmbus_update_device(dev);\n>>>>>>> \tstruct pmbus_sensor *sensor = to_pmbus_sensor(devattr);\n>>>>> \n>>>>> \n>>>>>>>>>>>>> +\tif (IS_ERR(data))\n>>>>>>> +\t\treturn PTR_ERR(data);\n>>>>> \n>>>>> +\n>>>>>>>>>>>>> \tif (sensor->data < 0)\n>>>>>>> \t\treturn sensor->data;\n>>>>> \n>>>>> \n>>>>> @@ -995,7 +1030,11 @@ static int pmbus_add_limit_attrs(struct i2c_client *client,\n>>>>>>> \tstruct pmbus_sensor *curr;\n>>>>> \n>>>>> \n>>>>>>>>>>>>> \tfor (i = 0; i < nlimit; i++) {\n>>>>>>>>>>>>> -\t\tif (pmbus_check_word_register(client, page, l->reg)) {\n>>>>>>>>>>>>> +\t\tret = pmbus_check_word_register(client, page, l->reg);\n>>>>>>>>>>>>> +\t\tif (ret < 0)\n>>>>>>> +\t\t\treturn ret;\n>>>>> \n>>>>> +\n>>>>>>>>>>>>> +\t\tif (ret) {\n>>>>>>>>>>>>> \t\t\tcurr = pmbus_add_sensor(data, name, l->attr, index,\n>>>>>>>>>>>>> \t\t\t\t\t\tpage, l->reg, attr->class,\n>>>>>>> \t\t\t\t\t\tattr->update || l->update,\n>>>>> \n>>>>> @@ -1041,6 +1080,8 @@ static int pmbus_add_sensor_attrs_one(struct i2c_client *client,\n>>>>>>>>>>>>> \tif (!base)\n>>>>>>>>>>>>> \t\treturn -ENOMEM;\n>>>>>>>>>>>>> \tif (attr->sfunc) {\n>>>>>>> +\t\tint check;\n>>>>> \n>>>>> +\n>>>>>>>>>>>>> \t\tret = pmbus_add_limit_attrs(client, data, info, name,\n>>>>>>>>>>>>> \t\t\t\t\t index, page, base, attr);\n>>>>>>> \t\tif (ret < 0)\n>>>>> \n>>>>> @@ -1050,9 +1091,13 @@ static int pmbus_add_sensor_attrs_one(struct i2c_client *client,\n>>>>>>>>>>>>> \t\t * alarm attributes, if there is a global alarm bit, and if\n>>>>>>>>>>>>> \t\t * the generic status register for this page is accessible.\n>>>>>>>>>>>>> \t\t */\n>>>>>>>>>>>>> -\t\tif (!ret && attr->gbit &&\n>>>>>>>>>>>>> -\t\t pmbus_check_byte_register(client, page,\n>>>>>>> -\t\t\t\t\t data->status_register)) {\n>>>>> \n>>>>> +\n>>>>>>>>>>>>> +\t\tcheck = pmbus_check_byte_register(client, page,\n>>>>>>>>>>>>> +\t\t\t\t\t\t data->status_register);\n>>>>>>>>>>>>> +\t\tif (check < 0)\n>>>>>>> +\t\t\treturn check;\n>>>>> \n>>>>> +\n>>>>>>>>>>>>> +\t\tif (!ret && attr->gbit && check) {\n>>>>>>>>>>>>> \t\t\tret = pmbus_add_boolean(data, name, \"alarm\", index,\n>>>>>>>>>>>>> \t\t\t\t\t\tNULL, NULL,\n>>>>>>> \t\t\t\t\t\tPB_STATUS_BASE + page,\n>>>>> \n>>>>> @@ -1604,8 +1649,12 @@ static int pmbus_add_fan_attributes(struct i2c_client *client,\n>>>>>>>>>>>>> \t\t\tif (!(info->func[page] & pmbus_fan_flags[f]))\n>>>>>>> \t\t\t\tbreak;\n>>>>> \n>>>>> \n>>>>>>>>>>>>> -\t\t\tif (!pmbus_check_word_register(client, page,\n>>>>>>>>>>>>> -\t\t\t\t\t\t pmbus_fan_registers[f]))\n>>>>>>>>>>>>> +\t\t\tret = pmbus_check_word_register(client, page,\n>>>>>>>>>>>>> +\t\t\t\t\t\t pmbus_fan_registers[f]);\n>>>>>>>>>>>>> +\t\t\tif (ret < 0)\n>>>>>>> +\t\t\t\treturn ret;\n>>>>> \n>>>>> +\n>>>>>>>>>>>>> +\t\t\tif (!ret)\n>>>>>>> \t\t\t\tbreak;\n>>>>> \n>>>>> \n>>>>>>> \t\t\t/*\n>>>>> \n>>>>> @@ -1628,9 +1677,13 @@ static int pmbus_add_fan_attributes(struct i2c_client *client,\n>>>>>>>>>>>>> \t\t\t * Each fan status register covers multiple fans,\n>>>>>>>>>>>>> \t\t\t * so we have to do some magic.\n>>>>>>>>>>>>> \t\t\t */\n>>>>>>>>>>>>> -\t\t\tif ((info->func[page] & pmbus_fan_status_flags[f]) &&\n>>>>>>>>>>>>> -\t\t\t pmbus_check_byte_register(client,\n>>>>>>>>>>>>> -\t\t\t\t\tpage, pmbus_fan_status_registers[f])) {\n>>>>>>>>>>>>> +\t\t\tret = pmbus_check_byte_register(client, page,\n>>>>>>>>>>>>> +\t\t\t\t\t\tpmbus_fan_status_registers[f]);\n>>>>>>>>>>>>> +\t\t\tif (ret < 0)\n>>>>>>> +\t\t\t\treturn ret;\n>>>>> \n>>>>> +\n>>>>>>>>>>>>> +\t\t\tif ((info->func[page] & pmbus_fan_status_flags[f])\n>>>>>>>>>>>>> +\t\t\t\t\t&& ret) {\n>>>>>>> \t\t\t\tint base;\n>>>>> \n>>>>> \n>>>>>>>>> \t\t\t\tif (f > 1)\t/* fan 3, 4 */\n>>>>> \n>>>>> @@ -1696,10 +1749,13 @@ static int pmbus_identify_common(struct i2c_client *client,\n>>>>>>> \t\t\t\t struct pmbus_data *data, int page)\n>>>>> \n>>>>> {\n>>>>>>>>>>>>> \tint vout_mode = -1;\n>>>>>>> +\tint rv;\n>>>>> \n>>>>> \n>>>>>>>>>>>>> -\tif (pmbus_check_byte_register(client, page, PMBUS_VOUT_MODE))\n>>>>>>>>>>>>> +\trv = pmbus_check_byte_register(client, page, PMBUS_VOUT_MODE);\n>>>>>>>>>>>>> +\tif (rv == 1)\n>>>>>>>>>>>>> \t\tvout_mode = _pmbus_read_byte_data(client, page,\n>>>>>>> \t\t\t\t\t\t PMBUS_VOUT_MODE);\n>>>>> \n>>>>> +\n>>>>>>>>>>>>> \tif (vout_mode >= 0 && vout_mode != 0xff) {\n>>>>>>>>>>>>> \t\t/*\n>>>>>>> \t\t * Not all chips support the VOUT_MODE command,\n>>>>> \n>>>>> @@ -1725,8 +1781,7 @@ static int pmbus_identify_common(struct i2c_client *client,\n>>>>>>>>>>>>> \t\t}\n>>>>>>> \t}\n>>>>> \n>>>>> \n>>>>>>>>>>>>> -\tpmbus_clear_fault_page(client, page);\n>>>>>>>>>>>>> -\treturn 0;\n>>>>>>> +\treturn pmbus_clear_fault_page(client, page);\n>>>>> \n>>>>> }\n>>>>> \n>>>>> static int pmbus_init_common(struct i2c_client *client, struct pmbus_data *data,\n>>>>> @@ -1756,7 +1811,9 @@ static int pmbus_init_common(struct i2c_client *client, struct pmbus_data *data,\n>>>>>>>>>>>>> \tif (ret >= 0 && (ret & PB_CAPABILITY_ERROR_CHECK))\n>>>>>>> \t\tclient->flags |= I2C_CLIENT_PEC;\n>>>>> \n>>>>> \n>>>>>>>>>>>>> -\tpmbus_clear_faults(client);\n>>>>>>>>>>>>> +\tret = pmbus_clear_faults(client);\n>>>>>>>>>>>>> +\tif (ret < 0)\n>>>>>>> +\t\treturn ret;\n>>>>> \n>>>>> \n>>>>>>>>>>>>> \tif (info->identify) {\n>>>>>>> \t\tret = (*info->identify)(client, info);\n>>>>> \n>>>>> -- \n>>>>> 2.11.0\n>>>>> \n>> \n>>","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 [103.22.144.68])\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 3xnkg75zFdz9s7h\n\tfor ;\n\tThu, 7 Sep 2017 12:19:47 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xnkg74Z33zDrWJ\n\tfor ;\n\tThu, 7 Sep 2017 12:19:47 +1000 (AEST)","from mail-pf0-x244.google.com (mail-pf0-x244.google.com\n\t[IPv6:2607:f8b0:400e:c00::244])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128\n\tbits)) (No client certificate requested)\n\tby lists.ozlabs.org (Postfix) with ESMTPS id 3xnkg16LDvzDrVm\n\tfor ; Thu, 7 Sep 2017 12:19:41 +1000 (AEST)","by mail-pf0-x244.google.com with SMTP id h4so477307pfk.0\n\tfor ; Wed, 06 Sep 2017 19:19:41 -0700 (PDT)","from server.roeck-us.net\n\t(108-223-40-66.lightspeed.sntcca.sbcglobal.net. [108.223.40.66])\n\tby smtp.gmail.com with ESMTPSA id\n\tp6sm1311803pfg.168.2017.09.06.19.19.36\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tWed, 06 Sep 2017 19:19:37 -0700 (PDT)"],"Authentication-Results":["ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"KQwGMMVR\"; dkim-atps=neutral","lists.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"KQwGMMVR\"; dkim-atps=neutral","ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=gmail.com\n\t(client-ip=2607:f8b0:400e:c00::244; helo=mail-pf0-x244.google.com;\n\tenvelope-from=groeck7@gmail.com; receiver=)","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"KQwGMMVR\"; dkim-atps=neutral"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=sender:subject:to:cc:references:from:message-id:date:user-agent\n\t:mime-version:in-reply-to:content-language:content-transfer-encoding; \n\tbh=4PM8LOUO2Y8qskKRyJBzatf3HjMO9Dpea/bTA3LiBQQ=;\n\tb=KQwGMMVRAbnftBX2WLGhn3ataFn4JjxlpnGFdmokqnC60UYOfmz00UKOmdi0XzXUnj\n\t14F58FHvL8RPSKyCe++xKCyTKrDmGIGGDaCJ8uj25N+JkOtoTtXVGRA7YoRjQ3yadV/C\n\tpj3DTwh+8yZiws5Dwk9nnUxQ9WRdbD2Vgy7qNgkGeF097wm++Ga3tX0sqWcfWUMdB9Ff\n\t1vR8TFhDqz6Y/85kiasoaboaHXNxJpW/XPSQBkmBIYG0otvWVZj6Kv5f1VT//iruceyG\n\tij/Ov0istgJdv7K5TjTgsBDcfCXUqihQ+JAOqDd8QQeBIcEnC3sdVI30M7Q/F4AhnK4n\n\tGVCA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:sender:subject:to:cc:references:from:message-id\n\t:date:user-agent:mime-version:in-reply-to:content-language\n\t:content-transfer-encoding;\n\tbh=4PM8LOUO2Y8qskKRyJBzatf3HjMO9Dpea/bTA3LiBQQ=;\n\tb=lFVhlNcCw8R8ORJR6fiab3f9PESxHpWsCGYaGJOweMPaC2YXR7YSQMBsgYekiJ0b6p\n\tqeF0mbR/dkvBg1776XymTafR88Ij8YKxwRmlRoDCQQMZPUHz+o+TMqqpW5dnMgDUA1Bq\n\tXUK3rXr/p4M+hQDz/1fXPqhh1y/iMhYF8Z1lXobT1i4cMHpE1STXXg36KW3rS5yp01KL\n\tcBhgVdW1ekuBTu+NGBrF2xV0siDmknYY7j0jvewQkCHBBssVkE85ympRHjh+LDkQNhJ8\n\tz33kN2HHccx7Kg01M04MC3kyb9XgWtkJ/TemllgN1h2/fJFZ91Ktf3pJ1VIZ0M4g9X+b\n\tFvew==","X-Gm-Message-State":"AHPjjUjNq90OsBxBD1hUufhR2jhp0tWu4VCX9Is+w3ir63uyKpljrvw9\n\tYXfnnFBKma6X9w==","X-Google-Smtp-Source":"ADKCNb7/sNhY3J+v33ohbUPBqclM4fREuf2XKswUIP7dzR4ADYIvQxPL2tNOKLrLQWPBMAleQ6srbA==","X-Received":"by 10.98.103.142 with SMTP id t14mr1229164pfj.56.1504750779030; \n\tWed, 06 Sep 2017 19:19:39 -0700 (PDT)","Subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","To":"Andrew Jeffery ","References":"<20170905070132.17682-1-andrew@aj.id.au>\n\t<20170905170002.GG11478@roeck-us.net>\n\t<1504657417.28363.8.camel@aj.id.au>\n\t<20170906225102.GA32210@roeck-us.net>\n\t<1504740749.5042.2.camel@aj.id.au>","From":"Guenter Roeck ","Message-ID":"<5089e197-5ee5-8775-501c-201546a0cc1a@roeck-us.net>","Date":"Wed, 6 Sep 2017 19:19:36 -0700","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<1504740749.5042.2.camel@aj.id.au>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","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":"linux-hwmon@vger.kernel.org, jdelvare@suse.com, linux-doc@vger.kernel.org,\n\topenbmc@lists.ozlabs.org, corbet@lwn.net, linux-kernel@vger.kernel.org","Errors-To":"openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"openbmc\"\n\t"}},{"id":1764728,"web_url":"http://patchwork.ozlabs.org/comment/1764728/","msgid":"","list_archive_url":null,"date":"2017-09-07T13:40:49","subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","submitter":{"id":21889,"url":"http://patchwork.ozlabs.org/api/people/21889/","name":"Guenter Roeck","email":"linux@roeck-us.net"},"content":"On 09/06/2017 04:32 PM, Andrew Jeffery wrote:\n\n>> \n>> Guess I need to dig up my eval board and see if I can reproduce the problem.\n>> Seems you are saying that the problem is always seen when issuing a sequence\n>> of \"clear faults\" commands on multiple pages ?\n> \n> Yeah. We're also seeing bad behaviour under other command sequences as well,\n> which lead to this hack of a work-around patch[1].\n> \n> I'd be very interested in the results of testing against the eval board. I\n> don't have access to one and it seems Maxim have discontinued them.\n> \n\nDo you have a somewhat reliable means to reproduce the problem ?\n\nGuenter","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 3xp1n863PTz9sPm\n\tfor ;\n\tThu, 7 Sep 2017 23:41:00 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xp1n84J4JzDrWQ\n\tfor ;\n\tThu, 7 Sep 2017 23:41:00 +1000 (AEST)","from mail-pf0-x241.google.com (mail-pf0-x241.google.com\n\t[IPv6:2607:f8b0:400e:c00::241])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128\n\tbits)) (No client certificate requested)\n\tby lists.ozlabs.org (Postfix) with ESMTPS id 3xp1n24QSNzDrWN\n\tfor ; Thu, 7 Sep 2017 23:40:53 +1000 (AEST)","by mail-pf0-x241.google.com with SMTP id q76so4525360pfq.5\n\tfor ; Thu, 07 Sep 2017 06:40:53 -0700 (PDT)","from server.roeck-us.net\n\t(108-223-40-66.lightspeed.sntcca.sbcglobal.net. [108.223.40.66])\n\tby smtp.gmail.com with ESMTPSA id\n\tp88sm4453464pfi.164.2017.09.07.06.40.49\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tThu, 07 Sep 2017 06:40:50 -0700 (PDT)"],"Authentication-Results":["ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"oWMj1w7/\"; dkim-atps=neutral","lists.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"oWMj1w7/\"; dkim-atps=neutral","ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=gmail.com\n\t(client-ip=2607:f8b0:400e:c00::241; helo=mail-pf0-x241.google.com;\n\tenvelope-from=groeck7@gmail.com; receiver=)","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"oWMj1w7/\"; dkim-atps=neutral"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=sender:subject:to:cc:references:from:message-id:date:user-agent\n\t:mime-version:in-reply-to:content-language:content-transfer-encoding; \n\tbh=DeaKXJUQur//vNoyow/ffDf5TMiLNOrSbWjaaPYbkyQ=;\n\tb=oWMj1w7/zeg9Bi2KDZKZl4dNXxbHsPnRDDlIuY7H+9rbHmt+kDwXRDS7/ureSMbNr1\n\twyhFm6yEq7AoncxQIg3AaIoDKcc67fclR/ZpcZKhfhRBtM/yyRGfrUPVx81+oP2VAS4j\n\tBez0rfXZ3U8CCOSWRNRWq0iGHZrXB872ZqhSIVysTX5DXYe/f6xK7bue+GdnQRQa68xW\n\tnKzfDvAtTAMbOHfg+8SrZWDiLNbzH6AaYVHMv3KMqf3fj86cq1zRoKiJwaUYxITbjc0G\n\tGtYXU6H5WcocQUtrPDiAkwdCoc04YZj4mlta3BHqiHuW3a4jkmukB28IzDKRsOaecLBU\n\tywaA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:sender:subject:to:cc:references:from:message-id\n\t:date:user-agent:mime-version:in-reply-to:content-language\n\t:content-transfer-encoding;\n\tbh=DeaKXJUQur//vNoyow/ffDf5TMiLNOrSbWjaaPYbkyQ=;\n\tb=D8pcV+KW+HzW+0mkO5E/aFjoKYBU1BQMC4Jc+4MQeSxn0AMhz5RieGKTPNyMvdlQjx\n\t5si0JOro7FmH6tK78Y++Pf9oilWeigddGh4W8LbLkAftD7EvVNaIXirBiEwt+Fz5mS5Z\n\tN1beww6p2Oya1h3IgyHTlJJ4duOeJ19sQCO/wRc6F3hwMID+OTM3wlBFyAujQr3dqG6s\n\tvu0MOYpqx95Zzo6crqlHvJWm11nh+xk7kLyx6LwYTYE0FuPsm+9A6v/mv9gbJVYRbaeK\n\t983zei1S2uuTzfHuEPrcTVzikcTEhPyg/HkMK820jrFjQW5NYS5l59BSBFl0xaZtqmGm\n\t3Hbw==","X-Gm-Message-State":"AHPjjUh4e2VQY7pCKC78ncUwQBHwLe1sNncdvSArrY+AFBfn+xn1cu2L\n\t/8zWZAUudKndUQ==","X-Google-Smtp-Source":"ADKCNb4KaMtsJT3qUdUiY06nggrMkA2w+yEmT/b7WGAyiQrRpDlF0W2PFoA9laaAjKNL9BYCD0t97A==","X-Received":"by 10.101.85.4 with SMTP id f4mr2787041pgr.10.1504791651833;\n\tThu, 07 Sep 2017 06:40:51 -0700 (PDT)","Subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","To":"Andrew Jeffery ","References":"<20170905070132.17682-1-andrew@aj.id.au>\n\t<20170905170002.GG11478@roeck-us.net>\n\t<1504657417.28363.8.camel@aj.id.au>\n\t<20170906225102.GA32210@roeck-us.net>\n\t<1504740749.5042.2.camel@aj.id.au>","From":"Guenter Roeck ","Message-ID":"","Date":"Thu, 7 Sep 2017 06:40:49 -0700","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<1504740749.5042.2.camel@aj.id.au>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","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":"linux-hwmon@vger.kernel.org, jdelvare@suse.com, linux-doc@vger.kernel.org,\n\topenbmc@lists.ozlabs.org, corbet@lwn.net, linux-kernel@vger.kernel.org","Errors-To":"openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"openbmc\"\n\t"}},{"id":1764782,"web_url":"http://patchwork.ozlabs.org/comment/1764782/","msgid":"<1504797770.5105.7.camel@aj.id.au>","list_archive_url":null,"date":"2017-09-07T15:22:50","subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","submitter":{"id":68332,"url":"http://patchwork.ozlabs.org/api/people/68332/","name":"Andrew Jeffery","email":"andrew@aj.id.au"},"content":"On Thu, 2017-09-07 at 06:40 -0700, Guenter Roeck wrote:\n> On 09/06/2017 04:32 PM, Andrew Jeffery wrote:\n> \n> > >   \n> > > Guess I need to dig up my eval board and see if I can reproduce the problem.\n> > > Seems you are saying that the problem is always seen when issuing a sequence\n> > > of \"clear faults\" commands on multiple pages ?\n> > \n> > Yeah. We're also seeing bad behaviour under other command sequences as well,\n> > which lead to this hack of a work-around patch[1].\n> > \n> > I'd be very interested in the results of testing against the eval board. I\n> > don't have access to one and it seems Maxim have discontinued them.\n> > \n> \n> Do you have a somewhat reliable means to reproduce the problem ?\n\nIt seems we hit a bunch of problems by just continually\nbinding/unbinding the driver, if you don't apply that hacky oneshot\nretry patch. We can hit problems (in our design?) with something like:\n\n# cd /sys/bus/i2c/drivers/max31785; \\\n\techo $addr > unbind; \\\n\twhile echo $addr > bind; \\\n\tdo echo $addr > unbind; echo -n .; done;\n\nIt should hit issues covered by this patch, as the register checks are\nused in the operations used by probe.\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 3xp4364HGYz9s8J\n\tfor ;\n\tFri, 8 Sep 2017 01:23:14 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xp4362d8SzDrWN\n\tfor ;\n\tFri, 8 Sep 2017 01:23:14 +1000 (AEST)","from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com\n\t[66.111.4.25])\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 3xp42y0G3YzDrW9\n\tfor ; Fri, 8 Sep 2017 01:23:05 +1000 (AEST)","from compute4.internal (compute4.nyi.internal [10.202.2.44])\n\tby mailout.nyi.internal (Postfix) with ESMTP id F3FC1206F4;\n\tThu, 7 Sep 2017 11:23:02 -0400 (EDT)","from frontend1 ([10.202.2.160])\n\tby compute4.internal (MEProxy); Thu, 07 Sep 2017 11:23:03 -0400","from keelia (unknown [180.200.129.208])\n\tby mail.messagingengine.com (Postfix) with ESMTPA id E083A7E426;\n\tThu, 7 Sep 2017 11:22:59 -0400 (EDT)"],"Authentication-Results":["ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"cjMjNjHs\";\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"N1y6bjKD\"; \n\tdkim-atps=neutral","lists.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"cjMjNjHs\";\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"N1y6bjKD\"; \n\tdkim-atps=neutral","ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=aj.id.au\n\t(client-ip=66.111.4.25; helo=out1-smtp.messagingengine.com;\n\tenvelope-from=andrew@aj.id.au; receiver=)","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"cjMjNjHs\";\n\tdkim=pass (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com\n\theader.b=\"N1y6bjKD\"; 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=7WcXC+oESbJd50tckYwa0+V7giRCWe6m5bimiPDa7\n\tqs=; b=cjMjNjHsxQs9gUbGd1cGVfGwKKBGlaPzIL+sU5oRwMFmRwwWGdDk/T+mn\n\tGq93UOYoccAclLszjCr8V+czNjTIOnVg6KWUOf3tfhPTOwjgEImAV6puk6QbwAss\n\tF6gbF61MBwHazy+ySSaBqzIZ5QR6VhgfiRJmE07pHcFliS6IUSzX+fNPtY50/B+4\n\tDJYXisg4ZJRaSCqt4umndc76bqN6etjk1U9dOJvuE3MrNLIlOLdHSGeB4+SjcenN\n\tMg+Z4FCZliE/XkHWEswPUzjdNfReR8DHwzgpuasgrqT7c2AlVgSwNYOt/GcLfNI+\n\tlrUzTQXnVYxe/gQcugr+KplDCfxLg==","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=7WcXC+oESbJd50tckY\n\twa0+V7giRCWe6m5bimiPDa7qs=; b=N1y6bjKDRK22mX3tq4yFDofcZhd4bbvNi5\n\tdzwYdzUAqXp4vG0xq+UM8Fvs/6UGdeNS25aZ3c++SF/iXpxQuLoHbTvKkspJ/72f\n\t5XGTlt+v+jAes3Z4C2L7/t9hIvpme86KbgCBxLxCCp8AuRWyN9iC9KrYFPjXDiCy\n\taojNa4A7iklGfTm+FgjpwyDG4K23lqexL1eM0pp5gtR3DQmFS1QMsrjJzSNyatCW\n\tgANpDwIrX7tTWeSDZ9ocMqVqX/8WD4ZUA4F8jB+emcsrLrs1ysI815sjv90IPlnq\n\tMqKi6473Ig2GSlBz9BCr6xZaFjoZxK1EYPrYsUcRonVRUOHy0Stw=="],"X-ME-Sender":"","X-Sasl-enc":"SKUs+QiTAB1KIBU2MbwFhdMIS6ll+waYqklyElWcBIQh 1504797782","Message-ID":"<1504797770.5105.7.camel@aj.id.au>","Subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","From":"Andrew Jeffery ","To":"Guenter Roeck ","Date":"Fri, 08 Sep 2017 01:22:50 +1000","In-Reply-To":"","References":"<20170905070132.17682-1-andrew@aj.id.au>\n\t<20170905170002.GG11478@roeck-us.net>\n\t<1504657417.28363.8.camel@aj.id.au>\n\t<20170906225102.GA32210@roeck-us.net>\n\t<1504740749.5042.2.camel@aj.id.au>\n\t","Content-Type":"multipart/signed; micalg=\"pgp-sha512\";\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"=-Ora1ViZxeJ8Db5G7TexF\"","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":"linux-hwmon@vger.kernel.org, jdelvare@suse.com, linux-doc@vger.kernel.org,\n\topenbmc@lists.ozlabs.org, corbet@lwn.net, linux-kernel@vger.kernel.org","Errors-To":"openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"openbmc\"\n\t"}},{"id":1765016,"web_url":"http://patchwork.ozlabs.org/comment/1765016/","msgid":"","list_archive_url":null,"date":"2017-09-08T00:27:25","subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","submitter":{"id":21889,"url":"http://patchwork.ozlabs.org/api/people/21889/","name":"Guenter Roeck","email":"linux@roeck-us.net"},"content":"On 09/07/2017 08:22 AM, Andrew Jeffery wrote:\n> On Thu, 2017-09-07 at 06:40 -0700, Guenter Roeck wrote:\n>> On 09/06/2017 04:32 PM, Andrew Jeffery wrote:\n>>\n>>>> \n>>>> Guess I need to dig up my eval board and see if I can reproduce the problem.\n>>>> Seems you are saying that the problem is always seen when issuing a sequence\n>>>> of \"clear faults\" commands on multiple pages ?\n>>>\n>>> Yeah. We're also seeing bad behaviour under other command sequences as well,\n>>> which lead to this hack of a work-around patch[1].\n>>>\n>>> I'd be very interested in the results of testing against the eval board. I\n>>> don't have access to one and it seems Maxim have discontinued them.\n>>>\n>>\n>> Do you have a somewhat reliable means to reproduce the problem ?\n> \n> It seems we hit a bunch of problems by just continually\n> binding/unbinding the driver, if you don't apply that hacky oneshot\n> retry patch. We can hit problems (in our design?) with something like:\n> \n> # cd /sys/bus/i2c/drivers/max31785; \\\n> \techo $addr > unbind; \\\n> \twhile echo $addr > bind; \\\n> \tdo echo $addr > unbind; echo -n .; done;\n> \n> It should hit issues covered by this patch, as the register checks are\n> used in the operations used by probe.\n> \n\nHmm ... I didn't use your driver but my prototype driver which also supports\ntemperature and voltage attributes, so if anything it should create more\nstress on the chip. No error so far, after running the script for a couple\nof minutes. How long does it take for errors to appear, and how do I see\nthat there is an error ? Does the driver fail to instantiate ?\n\nGuenter","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 [103.22.144.68])\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 3xpJ7C1PDgz9s76\n\tfor ;\n\tFri, 8 Sep 2017 10:27:35 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xpJ7B70g9zDrYg\n\tfor ;\n\tFri, 8 Sep 2017 10:27:34 +1000 (AEST)","from mail-pf0-x242.google.com (mail-pf0-x242.google.com\n\t[IPv6:2607:f8b0:400e:c00::242])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128\n\tbits)) (No client certificate requested)\n\tby lists.ozlabs.org (Postfix) with ESMTPS id 3xpJ766y6RzDrXT\n\tfor ; Fri, 8 Sep 2017 10:27:30 +1000 (AEST)","by mail-pf0-x242.google.com with SMTP id q76so601317pfq.5\n\tfor ; Thu, 07 Sep 2017 17:27:30 -0700 (PDT)","from server.roeck-us.net\n\t(108-223-40-66.lightspeed.sntcca.sbcglobal.net. [108.223.40.66])\n\tby smtp.gmail.com with ESMTPSA id\n\t77sm972441pfz.47.2017.09.07.17.27.26\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tThu, 07 Sep 2017 17:27:27 -0700 (PDT)"],"Authentication-Results":["ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"Vb2HSNhc\"; dkim-atps=neutral","lists.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"Vb2HSNhc\"; dkim-atps=neutral","ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=gmail.com\n\t(client-ip=2607:f8b0:400e:c00::242; helo=mail-pf0-x242.google.com;\n\tenvelope-from=groeck7@gmail.com; receiver=)","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"Vb2HSNhc\"; dkim-atps=neutral"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=sender:subject:to:cc:references:from:message-id:date:user-agent\n\t:mime-version:in-reply-to:content-language:content-transfer-encoding; \n\tbh=BYSUy3GQd6qXLnQfaiTtIzf4kSMPXjyVty8SdUqQdD4=;\n\tb=Vb2HSNhcKLMYK0Tm2EScmMTVqy3Wm6jLPIbcaQ0ViEPbT14ERgaf9xEmeeIlob7vKJ\n\tJM3Ei5AvkDFcF5LrWKDncRcustAdsQciyXVG9S1NYwNEgNACNwKsTjiReFS0FHr86hSG\n\taX2IgU3Q8qNVBlLF6a+uAYrZ951mwdoZSKoVkr2tGJhUuuqDLZNpKANXh+2bGEjOCPjh\n\tM+chOmjbqUnlKrEsc2SfbqgSN7hC9ZApxjkAyL9YA70INarudu2Kg1dwEm1+vS8Wg+AX\n\tUuCXIVLCN/W3SbFZrMbK1yhwMGY0TC5F5QOzR9H59+PxOhyUq0fx1TcTii95ckSQBc/h\n\ta4pg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:sender:subject:to:cc:references:from:message-id\n\t:date:user-agent:mime-version:in-reply-to:content-language\n\t:content-transfer-encoding;\n\tbh=BYSUy3GQd6qXLnQfaiTtIzf4kSMPXjyVty8SdUqQdD4=;\n\tb=mEuqlpZsUtR3ypColD3IoPznf0iiBYnm+DR8dKrZ7W4XxdwcL/hQctgSSSFv6QGO6M\n\tqPgDS6wJyTKj2OLKQCJ+5e9eRb0jgANnG9TngLHd4JqOA6qxtGdWtU5jeDmKFReZXP4l\n\tQJkj9Wm/C98LCUUIeWVYSKW1G3eBtZU2oo8Z9QKGg6Lo+75Zc3opl0ViKJsl4WbCtyxk\n\tvEKdu7AFfV6XWVdQ33BDZEfedUTemb97M4GA/9apnGmIOwj7UriRD7/9ov0SSBeojIkE\n\tFrmlEZjeEyAQpgj0ctPyGGqwSy5HI3lOQkfJ/mFrSMxqcr1lnHwDOiJan61+0Tq8ctjJ\n\tgn9g==","X-Gm-Message-State":"AHPjjUi6Ro563bTzQSh2GHRnAFwBRHtSS4iRsRuBOTa9kTqKuNe/mP3z\n\tUfaNQX3j8b8nfQ==","X-Google-Smtp-Source":"ADKCNb6xjadZIJ/YnKBF71YiboDV2mgs/vEGmfH7c8E7rsXFmiQs/KUeYVsY5wZu4EafyRKdEuORMw==","X-Received":"by 10.99.110.142 with SMTP id j136mr1191651pgc.390.1504830448427;\n\tThu, 07 Sep 2017 17:27:28 -0700 (PDT)","Subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","To":"Andrew Jeffery ","References":"<20170905070132.17682-1-andrew@aj.id.au>\n\t<20170905170002.GG11478@roeck-us.net>\n\t<1504657417.28363.8.camel@aj.id.au>\n\t<20170906225102.GA32210@roeck-us.net>\n\t<1504740749.5042.2.camel@aj.id.au>\n\t\n\t<1504797770.5105.7.camel@aj.id.au>","From":"Guenter Roeck ","Message-ID":"","Date":"Thu, 7 Sep 2017 17:27:25 -0700","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<1504797770.5105.7.camel@aj.id.au>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","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":"linux-hwmon@vger.kernel.org, jdelvare@suse.com, linux-doc@vger.kernel.org,\n\topenbmc@lists.ozlabs.org, corbet@lwn.net, linux-kernel@vger.kernel.org","Errors-To":"openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"openbmc\"\n\t"}},{"id":1765024,"web_url":"http://patchwork.ozlabs.org/comment/1765024/","msgid":"<1504832527.6124.1.camel@aj.id.au>","list_archive_url":null,"date":"2017-09-08T01:02:07","subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","submitter":{"id":68332,"url":"http://patchwork.ozlabs.org/api/people/68332/","name":"Andrew Jeffery","email":"andrew@aj.id.au"},"content":"On Thu, 2017-09-07 at 17:27 -0700, Guenter Roeck wrote:\n> On 09/07/2017 08:22 AM, Andrew Jeffery wrote:\n> > On Thu, 2017-09-07 at 06:40 -0700, Guenter Roeck wrote:\n> > > On 09/06/2017 04:32 PM, Andrew Jeffery wrote:\n> > > \n> > > > >    \n> > > > > Guess I need to dig up my eval board and see if I can reproduce the problem.\n> > > > > Seems you are saying that the problem is always seen when issuing a sequence\n> > > > > of \"clear faults\" commands on multiple pages ?\n> > > > \n> > > > Yeah. We're also seeing bad behaviour under other command sequences as well,\n> > > > which lead to this hack of a work-around patch[1].\n> > > > \n> > > > I'd be very interested in the results of testing against the eval board. I\n> > > > don't have access to one and it seems Maxim have discontinued them.\n> > > > \n> > > \n> > > Do you have a somewhat reliable means to reproduce the problem ?\n> > \n> > It seems we hit a bunch of problems by just continually\n> > binding/unbinding the driver, if you don't apply that hacky oneshot\n> > retry patch. We can hit problems (in our design?) with something like:\n> > \n> > # cd /sys/bus/i2c/drivers/max31785; \\\n> > \techo $addr > unbind; \\\n> > \twhile echo $addr > bind; \\\n> > \tdo echo $addr > unbind; echo -n .; done;\n> > \n> > It should hit issues covered by this patch, as the register checks are\n> > used in the operations used by probe.\n> > \n> \n> Hmm ... I didn't use your driver but my prototype driver which also supports\n> temperature and voltage attributes, so if anything it should create more\n> stress on the chip.\n\nI did add the temp and voltage attributes...\n\nAny chance you can give mine a try? I don't know what I would have done\nto invoke this kind of behaviour, so it would be useful to know whether\nor not it happens with one driver but not the other.\n\n> No error so far, after running the script for a couple\n> of minutes. How long does it take for errors to appear, and how do I see\n> that there is an error ? \n\nI'm seeing failures after anything from a handful of bind/unbinds, to\nhundreds of bind/unbinds. It seems to vary. \n\n> Does the driver fail to instantiate ?\n\nTypically probe fails so the loop exits. It usually gets -EIO and the\nshell spits out \"No such device\".\n\nThanks for testing, it's a useful data point for us hunting down the\nsource of our problems.\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 [103.22.144.68])\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 3xpJvS0ccjz9sBZ\n\tfor ;\n\tFri, 8 Sep 2017 11:02:28 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xpJvR6YPmzDrYn\n\tfor ;\n\tFri, 8 Sep 2017 11:02:27 +1000 (AEST)","from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com\n\t[66.111.4.25])\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 3xpJvG5rpZzDrX8\n\tfor ; Fri, 8 Sep 2017 11:02:18 +1000 (AEST)","from compute4.internal (compute4.nyi.internal [10.202.2.44])\n\tby mailout.nyi.internal (Postfix) with ESMTP id E2FA72102E;\n\tThu, 7 Sep 2017 21:02:14 -0400 (EDT)","from frontend1 ([10.202.2.160])\n\tby compute4.internal (MEProxy); Thu, 07 Sep 2017 21:02:14 -0400","from keelia (unknown [122.99.82.10])\n\tby mail.messagingengine.com (Postfix) with ESMTPA id 61ABA7F96A;\n\tThu, 7 Sep 2017 21:02:12 -0400 (EDT)"],"Authentication-Results":["ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"feMf624r\";\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"M7NgWJXd\"; \n\tdkim-atps=neutral","lists.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"feMf624r\";\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"M7NgWJXd\"; \n\tdkim-atps=neutral","ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=aj.id.au\n\t(client-ip=66.111.4.25; helo=out1-smtp.messagingengine.com;\n\tenvelope-from=andrew@aj.id.au; receiver=)","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"feMf624r\";\n\tdkim=pass (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com\n\theader.b=\"M7NgWJXd\"; 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=DVY0CUq095npf6hWy1KNGCK8hNEgbd+Q3s/6WtTxb\n\t08=; b=feMf624rDGjFmIsWKZ8vf9J4juSZd0ddeN6LutHykLUGHaYqYd1xJfWZa\n\tXRXg6cPkZLwRFHl1nzWXcp7cFdIzMr65fbDN+q95AVhZou1fmDOQaRLDNdD9ZbW9\n\tHIFzmR3f8bYvNGXTU5lHMgW7FTLyEdd3gkCt1bI333nvgpB8x3bp1XpzjfKdjqVN\n\tJio010qolDqRMGHzxf0MT2DGK9nxKruOBdpuKcBcqcIXvXRhyILO6wrm1I8HSqC2\n\tvHNJtBDQTDEPQYASd2DbRn6mZ5zcJ3So26dA4xPr/qU0SIO5350VyCCpLtTcxViZ\n\ty+XRCCaarh8U0i4qNpuEqO1u41Hpw==","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=DVY0CUq095npf6hWy1\n\tKNGCK8hNEgbd+Q3s/6WtTxb08=; b=M7NgWJXdjapYX9uida/HBGNoFqzepBAPz2\n\tGkS//5gtbZiKAb63XeIWFyRbN+Xor6Nn+xlKtGh2QERL4wXcnlBNvFDSDejU6AlI\n\t6gtgshjJSVxngMocwPFOm4QXUxt8EFLXM2Yur1lwXP2cDiQev/P9i3siNuF6L3PS\n\tvpX9OQmD0G20yqC3719yejTaYPbhVVmOR+qE3eZSpYCIUKakwE2YB9Adv3SARYDT\n\tlHwCQCgxlDmYpO+F0xR+k94NXxCAq8ly/3UW0KV8Z8urWTTEpOmNQfG9ySwxQZ9I\n\t2AiY3wA5f9c6xmfSK/V/JtmfB2IU1mjae/cH4z02SYKRvjcnV5FA=="],"X-ME-Sender":"","X-Sasl-enc":"1gaqDpJmzjo0uCZjvXgsf5afSiRwvoBA9/55A/gmkQ3F 1504832534","Message-ID":"<1504832527.6124.1.camel@aj.id.au>","Subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","From":"Andrew Jeffery ","To":"Guenter Roeck ","Date":"Fri, 08 Sep 2017 11:02:07 +1000","In-Reply-To":"","References":"<20170905070132.17682-1-andrew@aj.id.au>\n\t<20170905170002.GG11478@roeck-us.net>\n\t<1504657417.28363.8.camel@aj.id.au>\n\t<20170906225102.GA32210@roeck-us.net>\n\t<1504740749.5042.2.camel@aj.id.au>\n\t\n\t<1504797770.5105.7.camel@aj.id.au>\n\t","Content-Type":"multipart/signed; micalg=\"pgp-sha512\";\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"=-PuSp64QzTKl+lJDFLYDO\"","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":"linux-hwmon@vger.kernel.org, jdelvare@suse.com, linux-doc@vger.kernel.org,\n\topenbmc@lists.ozlabs.org, corbet@lwn.net, linux-kernel@vger.kernel.org","Errors-To":"openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"openbmc\"\n\t"}},{"id":1765030,"web_url":"http://patchwork.ozlabs.org/comment/1765030/","msgid":"","list_archive_url":null,"date":"2017-09-08T01:26:25","subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","submitter":{"id":21889,"url":"http://patchwork.ozlabs.org/api/people/21889/","name":"Guenter Roeck","email":"linux@roeck-us.net"},"content":"On 09/07/2017 06:02 PM, Andrew Jeffery wrote:\n> On Thu, 2017-09-07 at 17:27 -0700, Guenter Roeck wrote:\n>> On 09/07/2017 08:22 AM, Andrew Jeffery wrote:\n>>> On Thu, 2017-09-07 at 06:40 -0700, Guenter Roeck wrote:\n>>>> On 09/06/2017 04:32 PM, Andrew Jeffery wrote:\n>>>>\n>>>>>> \n>>>>>> Guess I need to dig up my eval board and see if I can reproduce the problem.\n>>>>>> Seems you are saying that the problem is always seen when issuing a sequence\n>>>>>> of \"clear faults\" commands on multiple pages ?\n>>>>>\n>>>>> Yeah. We're also seeing bad behaviour under other command sequences as well,\n>>>>> which lead to this hack of a work-around patch[1].\n>>>>>\n>>>>> I'd be very interested in the results of testing against the eval board. I\n>>>>> don't have access to one and it seems Maxim have discontinued them.\n>>>>>\n>>>>\n>>>> Do you have a somewhat reliable means to reproduce the problem ?\n>>>\n>>> It seems we hit a bunch of problems by just continually\n>>> binding/unbinding the driver, if you don't apply that hacky oneshot\n>>> retry patch. We can hit problems (in our design?) with something like:\n>>>\n>>> # cd /sys/bus/i2c/drivers/max31785; \\\n>>> \techo $addr > unbind; \\\n>>> \twhile echo $addr > bind; \\\n>>> \tdo echo $addr > unbind; echo -n .; done;\n>>>\n>>> It should hit issues covered by this patch, as the register checks are\n>>> used in the operations used by probe.\n>>>\n>>\n>> Hmm ... I didn't use your driver but my prototype driver which also supports\n>> temperature and voltage attributes, so if anything it should create more\n>> stress on the chip.\n> \n> I did add the temp and voltage attributes...\n> \n> Any chance you can give mine a try? I don't know what I would have done\n> to invoke this kind of behaviour, so it would be useful to know whether\n> or not it happens with one driver but not the other.\n> \n\nWill do.\n\n>> No error so far, after running the script for a couple\n>> of minutes. How long does it take for errors to appear, and how do I see\n>> that there is an error ?\n> \n> I'm seeing failures after anything from a handful of bind/unbinds, to\n> hundreds of bind/unbinds. It seems to vary.\n> \n>> Does the driver fail to instantiate ?\n> \n> Typically probe fails so the loop exits. It usually gets -EIO and the\n> shell spits out \"No such device\".\n> \n> Thanks for testing, it's a useful data point for us hunting down the\n> source of our problems.\n> \nI aborted the test after ~2,500 loops without error.\n\nGuenter","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 3xpKRL29rzz9sBZ\n\tfor ;\n\tFri, 8 Sep 2017 11:26:38 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xpKRL1226zDrZC\n\tfor ;\n\tFri, 8 Sep 2017 11:26:38 +1000 (AEST)","from mail-pf0-x242.google.com (mail-pf0-x242.google.com\n\t[IPv6:2607:f8b0:400e:c00::242])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128\n\tbits)) (No client certificate requested)\n\tby lists.ozlabs.org (Postfix) with ESMTPS id 3xpKR95bWxzDrYg\n\tfor ; Fri, 8 Sep 2017 11:26:29 +1000 (AEST)","by mail-pf0-x242.google.com with SMTP id g13so669103pfm.2\n\tfor ; Thu, 07 Sep 2017 18:26:29 -0700 (PDT)","from server.roeck-us.net\n\t(108-223-40-66.lightspeed.sntcca.sbcglobal.net. [108.223.40.66])\n\tby smtp.gmail.com with ESMTPSA id\n\tp12sm779182pgn.90.2017.09.07.18.26.25\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tThu, 07 Sep 2017 18:26:26 -0700 (PDT)"],"Authentication-Results":["ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"VYuz6rbU\"; dkim-atps=neutral","lists.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"VYuz6rbU\"; dkim-atps=neutral","ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=gmail.com\n\t(client-ip=2607:f8b0:400e:c00::242; helo=mail-pf0-x242.google.com;\n\tenvelope-from=groeck7@gmail.com; receiver=)","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"VYuz6rbU\"; dkim-atps=neutral"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=sender:subject:to:cc:references:from:message-id:date:user-agent\n\t:mime-version:in-reply-to:content-language:content-transfer-encoding; \n\tbh=COFw40Stp3wyGsznBay+1FgUxxhcLouvrBXGKn0eo5M=;\n\tb=VYuz6rbUL2ATSI0bPiF+sGmb+Jf8tly1vnWJZ+7aQOiAEi2OcM654HowWhKvhlIKRb\n\tJ9XsDLh9rrQ+5sK36Z0qIo3Yq2vkH9/c28PGgwH6dcecOJUjuacMmWGKWTt+3jYpTlDF\n\tCTEdzK+3n2Vq9BLicn9c+QKAe1H+RBeOB9McKrpi5LyMLFxTg3wkNuScCElPe/3C8qC8\n\tIjDDYDM5zvvnXwGX4SahyrxOEqVeszNNNWkFJ0vzpYcLLUASLss3QmdCXDLWbj6bTnn/\n\tqRuErSpwjuh1pieWOlhVa4Dxlaev/k14zzw842/lzilJmBaljPHwj7eUBYZZ8QwKa8KU\n\tBYng==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:sender:subject:to:cc:references:from:message-id\n\t:date:user-agent:mime-version:in-reply-to:content-language\n\t:content-transfer-encoding;\n\tbh=COFw40Stp3wyGsznBay+1FgUxxhcLouvrBXGKn0eo5M=;\n\tb=dk+v4b7CgTdcEvEueg3SM6aSM0o4u4J2ces+Q2MAiu7seJRnt+JL5njm96tpO3i3Us\n\t7X4A60/Gh6/+0kPgqKSImsHS/yZbhmKQzIBM7RljF3A8aDsL0+RFrj7YsOdO3XvLHQ0A\n\t9zgFDKGzE5/xOU96IJkJA4EOk6zQld5SxiQFk1cX6e0rf1xAXaqEOcMJt2u7/8szSySh\n\tUWrhAWcnYIHg7+61TyWb7gPe9W42t+EaxENbpvNCYl3p2AXAz7O/zZRdnMNBxakmRlvY\n\ty+DWg6SlIp5iroLKxvaNcVUAM1E45LQCbx88Zl1B1MIBggyMi0ztEmNpeK4j+vAJOUe/\n\troCA==","X-Gm-Message-State":"AHPjjUhzjAXorVcaVsAtpDXvdzsyy3basO9qaBX0hygh9Jnx6EZAeRz+\n\tGp+JPV0DWykrIw==","X-Google-Smtp-Source":"ADKCNb4th7RjEzuuZHsdX2zOOoH2k5fwohGq4ZZpr2nA3YmDHqnCv+i3xcduIYWInh97ujqoV2wZEQ==","X-Received":"by 10.99.175.14 with SMTP id w14mr1357262pge.365.1504833987568; \n\tThu, 07 Sep 2017 18:26:27 -0700 (PDT)","Subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","To":"Andrew Jeffery ","References":"<20170905070132.17682-1-andrew@aj.id.au>\n\t<20170905170002.GG11478@roeck-us.net>\n\t<1504657417.28363.8.camel@aj.id.au>\n\t<20170906225102.GA32210@roeck-us.net>\n\t<1504740749.5042.2.camel@aj.id.au>\n\t\n\t<1504797770.5105.7.camel@aj.id.au>\n\t\n\t<1504832527.6124.1.camel@aj.id.au>","From":"Guenter Roeck ","Message-ID":"","Date":"Thu, 7 Sep 2017 18:26:25 -0700","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<1504832527.6124.1.camel@aj.id.au>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","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":"linux-hwmon@vger.kernel.org, jdelvare@suse.com, linux-doc@vger.kernel.org,\n\topenbmc@lists.ozlabs.org, corbet@lwn.net, linux-kernel@vger.kernel.org","Errors-To":"openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"openbmc\"\n\t"}},{"id":1765039,"web_url":"http://patchwork.ozlabs.org/comment/1765039/","msgid":"<1504836369.6124.2.camel@aj.id.au>","list_archive_url":null,"date":"2017-09-08T02:06:09","subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","submitter":{"id":68332,"url":"http://patchwork.ozlabs.org/api/people/68332/","name":"Andrew Jeffery","email":"andrew@aj.id.au"},"content":"On Thu, 2017-09-07 at 18:26 -0700, Guenter Roeck wrote:\n> On 09/07/2017 06:02 PM, Andrew Jeffery wrote:\n> > On Thu, 2017-09-07 at 17:27 -0700, Guenter Roeck wrote:\n> > > On 09/07/2017 08:22 AM, Andrew Jeffery wrote:\n> > > > On Thu, 2017-09-07 at 06:40 -0700, Guenter Roeck wrote:\n> > > > > On 09/06/2017 04:32 PM, Andrew Jeffery wrote:\n> > > > > \n> > > > > > >     \n> > > > > > > Guess I need to dig up my eval board and see if I can reproduce the problem.\n> > > > > > > Seems you are saying that the problem is always seen when issuing a sequence\n> > > > > > > of \"clear faults\" commands on multiple pages ?\n> > > > > > \n> > > > > > Yeah. We're also seeing bad behaviour under other command sequences as well,\n> > > > > > which lead to this hack of a work-around patch[1].\n> > > > > > \n> > > > > > I'd be very interested in the results of testing against the eval board. I\n> > > > > > don't have access to one and it seems Maxim have discontinued them.\n> > > > > > \n> > > > > \n> > > > > Do you have a somewhat reliable means to reproduce the problem ?\n> > > > \n> > > > It seems we hit a bunch of problems by just continually\n> > > > binding/unbinding the driver, if you don't apply that hacky oneshot\n> > > > retry patch. We can hit problems (in our design?) with something like:\n> > > > \n> > > > # cd /sys/bus/i2c/drivers/max31785; \\\n> > > > \techo $addr > unbind; \\\n> > > > \twhile echo $addr > bind; \\\n> > > > \tdo echo $addr > unbind; echo -n .; done;\n> > > > \n> > > > It should hit issues covered by this patch, as the register checks are\n> > > > used in the operations used by probe.\n> > > > \n> > > \n> > > Hmm ... I didn't use your driver but my prototype driver which also supports\n> > > temperature and voltage attributes, so if anything it should create more\n> > > stress on the chip.\n> > \n> > I did add the temp and voltage attributes...\n> > \n> > Any chance you can give mine a try? I don't know what I would have done\n> > to invoke this kind of behaviour, so it would be useful to know whether\n> > or not it happens with one driver but not the other.\n> > \n> \n> Will do.\n\nThanks. For reference, here's a devicetree description:\n\nhttps://github.com/openbmc/linux/blob/dev-4.10/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts#L283\n\n\n> \n> > >   No error so far, after running the script for a couple\n> > > of minutes. How long does it take for errors to appear, and how do I see\n> > > that there is an error ?\n> > \n> > I'm seeing failures after anything from a handful of bind/unbinds, to\n> > hundreds of bind/unbinds. It seems to vary.\n> > \n> > > Does the driver fail to instantiate ?\n> > \n> > Typically probe fails so the loop exits. It usually gets -EIO and the\n> > shell spits out \"No such device\".\n> > \n> > Thanks for testing, it's a useful data point for us hunting down the\n> > source of our problems.\n> > \n> \n> I aborted the test after ~2,500 loops without error.\n\nYeah, I'd consider that fairly stable.\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 [103.22.144.68])\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 3xpLKJ42n1z9sQl\n\tfor ;\n\tFri, 8 Sep 2017 12:06:28 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xpLKJ2SFpzDrZQ\n\tfor ;\n\tFri, 8 Sep 2017 12:06:28 +1000 (AEST)","from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com\n\t[66.111.4.25])\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 3xpLK66nDKzDqR8\n\tfor ; Fri, 8 Sep 2017 12:06:18 +1000 (AEST)","from compute4.internal (compute4.nyi.internal [10.202.2.44])\n\tby mailout.nyi.internal (Postfix) with ESMTP id 2B40D20C63;\n\tThu, 7 Sep 2017 22:06:16 -0400 (EDT)","from frontend2 ([10.202.2.161])\n\tby compute4.internal (MEProxy); Thu, 07 Sep 2017 22:06:16 -0400","from keelia (unknown [122.99.82.10])\n\tby mail.messagingengine.com (Postfix) with ESMTPA id 98F19241D9;\n\tThu, 7 Sep 2017 22:06:13 -0400 (EDT)"],"Authentication-Results":["ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"hWpTMDNB\";\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"epTPjH+v\"; \n\tdkim-atps=neutral","lists.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"hWpTMDNB\";\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"epTPjH+v\"; \n\tdkim-atps=neutral","ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=aj.id.au\n\t(client-ip=66.111.4.25; helo=out1-smtp.messagingengine.com;\n\tenvelope-from=andrew@aj.id.au; receiver=)","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"hWpTMDNB\";\n\tdkim=pass (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com\n\theader.b=\"epTPjH+v\"; 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=icCtGkv03aiYp+rUIgj+btEfmKu/I3h5aWuyy8wKE\n\tF4=; b=hWpTMDNBDrg8JUDqQBU7E2dYJRTmKBtgeoon39NB/o9PzPL2gb7wDlPGK\n\t3P4AIXNrH0Sx51pMSYLNVz+qixknmUTDEqTLX3IAz3rnOkwtvScp6/x86p2iouBK\n\t0YDQ1LMd0SidSxiRu/JPGeHPEMJj4AnoP5BPVYWNQTXKI4MQ2DpEK/MOWfEmUSCM\n\tow3NmHSqMz4xljpCYIg5a4C8X4UH8/urQZMSsAG5h9EKmmz6l3WJIiQQg03F9GVt\n\tI4YqlpCtHLYla9k7Mvv+LpboyLLxhQFdLJEwHcZiOHkchgLHkWqiWDbp1tZ7cljE\n\t9M5zSYge6MEQdLfNOw9fzf3p+pvNw==","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=icCtGkv03aiYp+rUIg\n\tj+btEfmKu/I3h5aWuyy8wKEF4=; b=epTPjH+vpMnATDf0aJNdMiXQmZIJtF8xwg\n\t3r4wD2pt/vM8kWGBaM3CkGuBnn/lzKA++UN4QKFyd3v7QwnERHcLIuqGNr3TDIfD\n\tKu4FLHJdtGecXe/8ZxkgsN82VCplEGpDMNGBi0LNSHsuALsa5zWD/YFUXYvUPAQ0\n\tzkr/d+MlVRgdMhq0jdNtWPu6cCndw5vz3zOM+S5y9Fpw/FT5H9Cc9HSZ3iLQ3Zuq\n\t5MAoL3PmHPAMuGN7wgm8tdHcU9NbuUGgUz7ed+3uY4mLxtJZUcL132ip9wZYWwmo\n\thzZQZKA8WVXMmc22dzk/AT/dU2cbFpZ490EdjirYsv+Oaf/Z0aaA=="],"X-ME-Sender":"","X-Sasl-enc":"0Drv8r9JHf1s+UI6EEqG/a8WYrtOabb1fXl1wzX+QjSi 1504836375","Message-ID":"<1504836369.6124.2.camel@aj.id.au>","Subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","From":"Andrew Jeffery ","To":"Guenter Roeck ","Date":"Fri, 08 Sep 2017 12:06:09 +1000","In-Reply-To":"","References":"<20170905070132.17682-1-andrew@aj.id.au>\n\t<20170905170002.GG11478@roeck-us.net>\n\t<1504657417.28363.8.camel@aj.id.au>\n\t<20170906225102.GA32210@roeck-us.net>\n\t<1504740749.5042.2.camel@aj.id.au>\n\t\n\t<1504797770.5105.7.camel@aj.id.au>\n\t\n\t<1504832527.6124.1.camel@aj.id.au>\n\t","Content-Type":"multipart/signed; micalg=\"pgp-sha512\";\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"=-uysjrYDqCPcEeFgRvZbA\"","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":"linux-hwmon@vger.kernel.org, jdelvare@suse.com, linux-doc@vger.kernel.org,\n\topenbmc@lists.ozlabs.org, corbet@lwn.net, linux-kernel@vger.kernel.org","Errors-To":"openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"openbmc\"\n\t"}},{"id":1765042,"web_url":"http://patchwork.ozlabs.org/comment/1765042/","msgid":"<75d9f838-aa4a-5cbd-744a-4d8e8bf1a370@roeck-us.net>","list_archive_url":null,"date":"2017-09-08T02:17:11","subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","submitter":{"id":21889,"url":"http://patchwork.ozlabs.org/api/people/21889/","name":"Guenter Roeck","email":"linux@roeck-us.net"},"content":"On 09/07/2017 07:06 PM, Andrew Jeffery wrote:\n> On Thu, 2017-09-07 at 18:26 -0700, Guenter Roeck wrote:\n>> On 09/07/2017 06:02 PM, Andrew Jeffery wrote:\n>>> On Thu, 2017-09-07 at 17:27 -0700, Guenter Roeck wrote:\n>>>> On 09/07/2017 08:22 AM, Andrew Jeffery wrote:\n>>>>> On Thu, 2017-09-07 at 06:40 -0700, Guenter Roeck wrote:\n>>>>>> On 09/06/2017 04:32 PM, Andrew Jeffery wrote:\n>>>>>>\n>>>>>>>> \n>>>>>>>> Guess I need to dig up my eval board and see if I can reproduce the problem.\n>>>>>>>> Seems you are saying that the problem is always seen when issuing a sequence\n>>>>>>>> of \"clear faults\" commands on multiple pages ?\n>>>>>>>\n>>>>>>> Yeah. We're also seeing bad behaviour under other command sequences as well,\n>>>>>>> which lead to this hack of a work-around patch[1].\n>>>>>>>\n>>>>>>> I'd be very interested in the results of testing against the eval board. I\n>>>>>>> don't have access to one and it seems Maxim have discontinued them.\n>>>>>>>\n>>>>>>\n>>>>>> Do you have a somewhat reliable means to reproduce the problem ?\n>>>>>\n>>>>> It seems we hit a bunch of problems by just continually\n>>>>> binding/unbinding the driver, if you don't apply that hacky oneshot\n>>>>> retry patch. We can hit problems (in our design?) with something like:\n>>>>>\n>>>>> # cd /sys/bus/i2c/drivers/max31785; \\\n>>>>> \techo $addr > unbind; \\\n>>>>> \twhile echo $addr > bind; \\\n>>>>> \tdo echo $addr > unbind; echo -n .; done;\n>>>>>\n>>>>> It should hit issues covered by this patch, as the register checks are\n>>>>> used in the operations used by probe.\n>>>>>\n>>>>\n>>>> Hmm ... I didn't use your driver but my prototype driver which also supports\n>>>> temperature and voltage attributes, so if anything it should create more\n>>>> stress on the chip.\n>>>\n>>> I did add the temp and voltage attributes...\n>>>\n>>> Any chance you can give mine a try? I don't know what I would have done\n>>> to invoke this kind of behaviour, so it would be useful to know whether\n>>> or not it happens with one driver but not the other.\n>>>\n>>\n>> Will do.\n> \n> Thanks. For reference, here's a devicetree description:\n> \n> https://github.com/openbmc/linux/blob/dev-4.10/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts#L283\n> \n\nI can't test with devicetree. x86 system.\n\n2,100+ iterations with your driver, no failures.\n\nEither it is because my chip is a MAX31785 (not A), or the configuration makes a difference,\nor it is your hardware.\n\nI'll try to connect a couple of fans next (so far I did without) and try again.\n\nGuenter","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 [103.22.144.68])\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 3xpLYr664Gz9sQl\n\tfor ;\n\tFri, 8 Sep 2017 12:17:20 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xpLYr4CkRzDrZQ\n\tfor ;\n\tFri, 8 Sep 2017 12:17:20 +1000 (AEST)","from mail-pf0-x243.google.com (mail-pf0-x243.google.com\n\t[IPv6:2607:f8b0:400e:c00::243])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128\n\tbits)) (No client certificate requested)\n\tby lists.ozlabs.org (Postfix) with ESMTPS id 3xpLYl65y3zDrX7\n\tfor ; Fri, 8 Sep 2017 12:17:15 +1000 (AEST)","by mail-pf0-x243.google.com with SMTP id g13so720957pfm.2\n\tfor ; Thu, 07 Sep 2017 19:17:15 -0700 (PDT)","from server.roeck-us.net\n\t(108-223-40-66.lightspeed.sntcca.sbcglobal.net. [108.223.40.66])\n\tby smtp.gmail.com with ESMTPSA id\n\td186sm1106201pfa.97.2017.09.07.19.17.11\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tThu, 07 Sep 2017 19:17:12 -0700 (PDT)"],"Authentication-Results":["ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"qhjv1Gi9\"; dkim-atps=neutral","lists.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"qhjv1Gi9\"; dkim-atps=neutral","ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=gmail.com\n\t(client-ip=2607:f8b0:400e:c00::243; helo=mail-pf0-x243.google.com;\n\tenvelope-from=groeck7@gmail.com; receiver=)","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"qhjv1Gi9\"; dkim-atps=neutral"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=sender:subject:to:cc:references:from:message-id:date:user-agent\n\t:mime-version:in-reply-to:content-language:content-transfer-encoding; \n\tbh=QkFrV6w2TAdCzaZrvh/ObLiqMSl1QZirj1nfPFRFtPw=;\n\tb=qhjv1Gi92tEvH2REG4k/hEr4OtYNupe53pSGjoJ+7RsfWh6yxDcyj1GBmpFsb+hcjj\n\tGcH9lpLaqmM2EbIVbcqShy2Zd5K1en/mPpF0b6sDGauVN0AiyuDoi5850qunnVrxv9FZ\n\tIW3tB6Feal+jVh0J+nMBPEaF9CMWYQjSHSDy/LdRuIjV3+9sADp4nqoanYQwvqS+3Rwx\n\t4QkbW1F/zO1KjEXTzto1P3fBaTTJddrH85O3y5Y7297z7x01S9U4WUcfA6iZSvh1yEJv\n\tPdvOs4Fqlfuj79bvqfmD62Y3wTc/lq+uQ5j+Jz5Svu3OAMaPV3F6aCMMH8ypCAL1b/Ri\n\t0/3Q==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:sender:subject:to:cc:references:from:message-id\n\t:date:user-agent:mime-version:in-reply-to:content-language\n\t:content-transfer-encoding;\n\tbh=QkFrV6w2TAdCzaZrvh/ObLiqMSl1QZirj1nfPFRFtPw=;\n\tb=hnHfQYffVhpgcPxqPXeDHMNSBa5ZlWUED4E5jV3QkC5U9GqMF9WSSGdP7wiMQeh9PI\n\tnoBRK4lnwjAzCcIt35L/c8INZzVZYeq8IRYtxPZ6EYtu6WWkGI48tFVLIj/2Fe2cZLYI\n\trPyYwt2Yo7rWl1tFNe+wk07spZOlp5If39FjSaPCWgluA/iPG3U/IiK49qbqZqB9Wu/3\n\tkljQm0OOmcfMRGWz/JUlmF9Zs/1e2GXGTaPSBPrb2Gnv3XCWOHfROlybxjQuV0OEE9qp\n\tQATY8/qiYNajB+gzMudK/OMfRifJdh6P1CPsoCFoaOo7qTty68r9HospZz/HinL9Npop\n\tnggw==","X-Gm-Message-State":"AHPjjUhw45nt5x/SCBkOKi5TJvUc34MDQ/MOsDN81VR0upUn+MBfun7h\n\tV9VlaYZaYgdHFg==","X-Google-Smtp-Source":"ADKCNb5v+iQ5gVOgyAIfDovJ945uD/j3O0DE5YYLHYv4cHSw6Jw/VB2cMwJH3mdyollmOvygWqpOfg==","X-Received":"by 10.98.159.137 with SMTP id v9mr1448579pfk.49.1504837033395;\n\tThu, 07 Sep 2017 19:17:13 -0700 (PDT)","Subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","To":"Andrew Jeffery ","References":"<20170905070132.17682-1-andrew@aj.id.au>\n\t<20170905170002.GG11478@roeck-us.net>\n\t<1504657417.28363.8.camel@aj.id.au>\n\t<20170906225102.GA32210@roeck-us.net>\n\t<1504740749.5042.2.camel@aj.id.au>\n\t\n\t<1504797770.5105.7.camel@aj.id.au>\n\t\n\t<1504832527.6124.1.camel@aj.id.au>\n\t\n\t<1504836369.6124.2.camel@aj.id.au>","From":"Guenter Roeck ","Message-ID":"<75d9f838-aa4a-5cbd-744a-4d8e8bf1a370@roeck-us.net>","Date":"Thu, 7 Sep 2017 19:17:11 -0700","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<1504836369.6124.2.camel@aj.id.au>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","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":"linux-hwmon@vger.kernel.org, jdelvare@suse.com, linux-doc@vger.kernel.org,\n\topenbmc@lists.ozlabs.org, corbet@lwn.net, linux-kernel@vger.kernel.org","Errors-To":"openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"openbmc\"\n\t"}},{"id":1765048,"web_url":"http://patchwork.ozlabs.org/comment/1765048/","msgid":"<1504839076.6124.8.camel@aj.id.au>","list_archive_url":null,"date":"2017-09-08T02:51:16","subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","submitter":{"id":68332,"url":"http://patchwork.ozlabs.org/api/people/68332/","name":"Andrew Jeffery","email":"andrew@aj.id.au"},"content":"On Thu, 2017-09-07 at 19:17 -0700, Guenter Roeck wrote:\n> On 09/07/2017 07:06 PM, Andrew Jeffery wrote:\n> > On Thu, 2017-09-07 at 18:26 -0700, Guenter Roeck wrote:\n> > > On 09/07/2017 06:02 PM, Andrew Jeffery wrote:\n> > > > On Thu, 2017-09-07 at 17:27 -0700, Guenter Roeck wrote:\n> > > > > On 09/07/2017 08:22 AM, Andrew Jeffery wrote:\n> > > > > > On Thu, 2017-09-07 at 06:40 -0700, Guenter Roeck wrote:\n> > > > > > > On 09/06/2017 04:32 PM, Andrew Jeffery wrote:\n> > > > > > > \n> > > > > > > > >      \n> > > > > > > > > Guess I need to dig up my eval board and see if I can reproduce the problem.\n> > > > > > > > > Seems you are saying that the problem is always seen when issuing a sequence\n> > > > > > > > > of \"clear faults\" commands on multiple pages ?\n> > > > > > > > \n> > > > > > > > Yeah. We're also seeing bad behaviour under other command sequences as well,\n> > > > > > > > which lead to this hack of a work-around patch[1].\n> > > > > > > > \n> > > > > > > > I'd be very interested in the results of testing against the eval board. I\n> > > > > > > > don't have access to one and it seems Maxim have discontinued them.\n> > > > > > > > \n> > > > > > > \n> > > > > > > Do you have a somewhat reliable means to reproduce the problem ?\n> > > > > > \n> > > > > > It seems we hit a bunch of problems by just continually\n> > > > > > binding/unbinding the driver, if you don't apply that hacky oneshot\n> > > > > > retry patch. We can hit problems (in our design?) with something like:\n> > > > > > \n> > > > > > # cd /sys/bus/i2c/drivers/max31785; \\\n> > > > > > \techo $addr > unbind; \\\n> > > > > > \twhile echo $addr > bind; \\\n> > > > > > \tdo echo $addr > unbind; echo -n .; done;\n> > > > > > \n> > > > > > It should hit issues covered by this patch, as the register checks are\n> > > > > > used in the operations used by probe.\n> > > > > > \n> > > > > \n> > > > > Hmm ... I didn't use your driver but my prototype driver which also supports\n> > > > > temperature and voltage attributes, so if anything it should create more\n> > > > > stress on the chip.\n> > > > \n> > > > I did add the temp and voltage attributes...\n> > > > \n> > > > Any chance you can give mine a try? I don't know what I would have done\n> > > > to invoke this kind of behaviour, so it would be useful to know whether\n> > > > or not it happens with one driver but not the other.\n> > > > \n> > > \n> > > Will do.\n> > \n> > Thanks. For reference, here's a devicetree description:\n> > \n> > https://github.com/openbmc/linux/blob/dev-4.10/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts#L283\n> > \n> \n> I can't test with devicetree. x86 system.\n> \n> 2,100+ iterations with your driver, no failures.\n\nGreat. I really appreciate your testing here, so thanks for your\nefforts. I owe you a few drinks if we ever happen to meet.\n\n> \n> Either it is because my chip is a MAX31785 (not A), or the configuration makes a difference,\n> or it is your hardware.\n\nYep. My understanding is the A variant is just a difference of\nmicrocode, but who knows what affect that could have. \n\n> \n> I'll try to connect a couple of fans next (so far I did without) and try again.\n\nKeep me posted if you do.\n\nThanks again.\n\nAndrew\n\n> \n> Guenter","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 3xpMKK2nYCz9sPk\n\tfor ;\n\tFri, 8 Sep 2017 12:51:33 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xpMKJ6Mw2zDrYm\n\tfor ;\n\tFri, 8 Sep 2017 12:51:32 +1000 (AEST)","from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com\n\t[66.111.4.25])\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 3xpMK92VRTzDrYD\n\tfor ; Fri, 8 Sep 2017 12:51:25 +1000 (AEST)","from compute4.internal (compute4.nyi.internal [10.202.2.44])\n\tby mailout.nyi.internal (Postfix) with ESMTP id 90BB220C5F;\n\tThu, 7 Sep 2017 22:51:22 -0400 (EDT)","from frontend2 ([10.202.2.161])\n\tby compute4.internal (MEProxy); Thu, 07 Sep 2017 22:51:22 -0400","from keelia (unknown [122.99.82.10])\n\tby mail.messagingengine.com (Postfix) with ESMTPA id 03406241D9;\n\tThu, 7 Sep 2017 22:51:19 -0400 (EDT)"],"Authentication-Results":["ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"pqYfVsuQ\";\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"jl09Wx22\"; \n\tdkim-atps=neutral","lists.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"pqYfVsuQ\";\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"jl09Wx22\"; \n\tdkim-atps=neutral","ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=aj.id.au\n\t(client-ip=66.111.4.25; helo=out1-smtp.messagingengine.com;\n\tenvelope-from=andrew@aj.id.au; receiver=)","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"pqYfVsuQ\";\n\tdkim=pass (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com\n\theader.b=\"jl09Wx22\"; 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=uiejYW7AxeBaoCMmdbAsC6tjBAD72CL8/E1ttn4+s\n\tPI=; b=pqYfVsuQWsZ0W6J4PV1reUeopI9wJs1R2SabB5oB6JY7WV8eGPojX5Zwi\n\tm+z0ZnQ1A130ACAGxQYzP/DUBkdN6DCVcQouDRBxx20/6iZ67gRcJNsxxqPp0rjd\n\t5HIHXVsuQDfPYbX1PGT9zt8MGj+mZVh+YB9RtfE2CBCGOaD1sunB1JYGwkSNqA9A\n\tNhIx5hy3oQ/XnMbCi8i6eGdGz4nAEU9WSc3bz+SOHhvV7A1FxmIVrWUl9UURvJ+o\n\tBwfqLOwzt6fhWfmrCWsgV1e/FrVa5EpbVqBkHZp1mHPBqUWAJVjzm81d8VDJQq/t\n\t84jQL0/UEJ6+98difvfkrmEWWwd/w==","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=uiejYW7AxeBaoCMmdb\n\tAsC6tjBAD72CL8/E1ttn4+sPI=; b=jl09Wx22lmGATghwVRJLyRiuPco0FVKse2\n\t2lNFET+KXBqL5+1XKJalZI76+PEME/mKteGtgytKOZZsHZLINnJAe/RLavIKqW7h\n\tF63YwBotD6FiZt1a7w1WcI409JAJzZgzZYeRg9WV4S0DXdy9mEQaDVtRLNE9CoWU\n\tqMIaOdEeVopj8t0XLfWBDF5mpGOTtyKm9VFmvVTNHO0rLJNzisrjfJmTgWpENs3H\n\tZixJ4hrYSJ5MRN/xuFHo0+qdhGFgDyAlglsCzxvTcKTGxDiSK/oAe9ZQbgj29YSx\n\tSktQTzjhfEUyOW0URULGI9ki8zWWx32zSlVawlI3vJXFyqwpVAIA=="],"X-ME-Sender":"","X-Sasl-enc":"2ixyeF8gAWQY4o+a6wUXlExNUws2ZsViVXotpY+f6NND 1504839081","Message-ID":"<1504839076.6124.8.camel@aj.id.au>","Subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","From":"Andrew Jeffery ","To":"Guenter Roeck ","Date":"Fri, 08 Sep 2017 12:51:16 +1000","In-Reply-To":"<75d9f838-aa4a-5cbd-744a-4d8e8bf1a370@roeck-us.net>","References":"<20170905070132.17682-1-andrew@aj.id.au>\n\t<20170905170002.GG11478@roeck-us.net>\n\t<1504657417.28363.8.camel@aj.id.au>\n\t<20170906225102.GA32210@roeck-us.net>\n\t<1504740749.5042.2.camel@aj.id.au>\n\t\n\t<1504797770.5105.7.camel@aj.id.au>\n\t\n\t<1504832527.6124.1.camel@aj.id.au>\n\t\n\t<1504836369.6124.2.camel@aj.id.au>\n\t<75d9f838-aa4a-5cbd-744a-4d8e8bf1a370@roeck-us.net>","Content-Type":"multipart/signed; micalg=\"pgp-sha512\";\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"=-gckEVYEcGvBcIqUFY6km\"","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":"linux-hwmon@vger.kernel.org, jdelvare@suse.com, linux-doc@vger.kernel.org,\n\topenbmc@lists.ozlabs.org, corbet@lwn.net, linux-kernel@vger.kernel.org","Errors-To":"openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"openbmc\"\n\t"}},{"id":1765062,"web_url":"http://patchwork.ozlabs.org/comment/1765062/","msgid":"<1504842058.6124.10.camel@aj.id.au>","list_archive_url":null,"date":"2017-09-08T03:40:58","subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","submitter":{"id":68332,"url":"http://patchwork.ozlabs.org/api/people/68332/","name":"Andrew Jeffery","email":"andrew@aj.id.au"},"content":"On Fri, 2017-09-08 at 12:51 +1000, Andrew Jeffery wrote:\n> > I can't test with devicetree. x86 system.\n> > \n> > 2,100+ iterations with your driver, no failures.\n> \n> Great. I really appreciate your testing here, so thanks for your\n> efforts. I owe you a few drinks if we ever happen to meet.\n\nActually, on that, how did you chop out the devicetree parts? Did you\nkeep the configuration writes at the end of max31785_of_fan_config()\nand max31785_of_tmp_config()? Or did you just not call them? These two\nfunctions cause the bulk of the bus traffic with on probe.\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 3xpNR32dYGz9t16\n\tfor ;\n\tFri, 8 Sep 2017 13:41:35 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xpNR12lCwzDrYn\n\tfor ;\n\tFri, 8 Sep 2017 13:41:33 +1000 (AEST)","from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com\n\t[66.111.4.25])\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 3xpNQW16Y0zDrX7\n\tfor ; Fri, 8 Sep 2017 13:41:06 +1000 (AEST)","from compute4.internal (compute4.nyi.internal [10.202.2.44])\n\tby mailout.nyi.internal (Postfix) with ESMTP id 16E7120D84;\n\tThu, 7 Sep 2017 23:41:04 -0400 (EDT)","from frontend2 ([10.202.2.161])\n\tby compute4.internal (MEProxy); Thu, 07 Sep 2017 23:41:04 -0400","from keelia (unknown [122.99.82.10])\n\tby mail.messagingengine.com (Postfix) with ESMTPA id B78D5243C8;\n\tThu, 7 Sep 2017 23:41:01 -0400 (EDT)"],"Authentication-Results":["ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"OaWNdHlF\";\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"jhLnVkqS\"; \n\tdkim-atps=neutral","lists.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"OaWNdHlF\";\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"jhLnVkqS\"; \n\tdkim-atps=neutral","ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=aj.id.au\n\t(client-ip=66.111.4.25; helo=out1-smtp.messagingengine.com;\n\tenvelope-from=andrew@aj.id.au; receiver=)","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"OaWNdHlF\";\n\tdkim=pass (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com\n\theader.b=\"jhLnVkqS\"; 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=XWdDvbnvT4Upe+T2ckmIFKBsjK7sVd0pVp5uVQ63N\n\tC8=; b=OaWNdHlFrGkKGqUxBaYHQKrKOkvJqsjQaoqj2051dZhfs/Y+dF12rujeb\n\t9RXpw0W413QnLgk0wwFvRLXIjrgLnii87gOc+4JAW6IzPFShb/po9av65azYB4dl\n\t8lgoH7cDGUCnqtmLx6YwGWiiG1IXIi2D+BE/W2gEcir1OeQE0FOsesrkKdRP50G6\n\tpJQ0D3WA50yAFzZ1VHLghc3j4+7uh3jUp5ZonbMtxxsK751o3CO+p+PX29ZKas8W\n\tb48bolh/fjc+/NbX2K+Py4ykOoPUOHECC4gJIE7VYyFhwAp8XRU6aWu/7iIGZuwC\n\tqRWWGBf3z7ZvBqXJ5z+ih9lR3ZOBA==","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=XWdDvbnvT4Upe+T2ck\n\tmIFKBsjK7sVd0pVp5uVQ63NC8=; b=jhLnVkqSRPoKK59q9iU1RBYd07gKVB7kGF\n\t5cBCq807oQQv/c1Ykvcaqvz+qAGon3H9NJq7jtwiSOuzGKGl1EGxjPhCuYRF2dHc\n\tfFcM9YCTGa/U+I6UjtAvJYYr3l4IUsxLCaZLVqLNy/I3XMkmXF56Ko3SRMcdCKGK\n\tVHWI/JU0Eajn5m1XlHbhYOz/c7Tio1L1KcO40jxxcpRWNhq0dGn0wsPbntXZ3MM9\n\tzoUlahqc7hW2zFuLIPw0jygHXCUI7ataqy06y23Bkm9DEln8yQVhD1VX1PeGy42o\n\tTZfM+7Scv6bXqu/eb3jKOs7z+G6DAOzuZTtRJEUqgGM+mFJVzEug=="],"X-ME-Sender":"","X-Sasl-enc":"EEaUoAn1Cci2c8mLQPjdFHwOsYIlHkQBMK3cSKxVzzi8 1504842063","Message-ID":"<1504842058.6124.10.camel@aj.id.au>","Subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","From":"Andrew Jeffery ","To":"Guenter Roeck ","Date":"Fri, 08 Sep 2017 13:40:58 +1000","In-Reply-To":"<1504839076.6124.8.camel@aj.id.au>","References":"<20170905070132.17682-1-andrew@aj.id.au>\n\t<20170905170002.GG11478@roeck-us.net>\n\t<1504657417.28363.8.camel@aj.id.au>\n\t<20170906225102.GA32210@roeck-us.net>\n\t<1504740749.5042.2.camel@aj.id.au>\n\t\n\t<1504797770.5105.7.camel@aj.id.au>\n\t\n\t<1504832527.6124.1.camel@aj.id.au>\n\t\n\t<1504836369.6124.2.camel@aj.id.au>\n\t<75d9f838-aa4a-5cbd-744a-4d8e8bf1a370@roeck-us.net>\n\t<1504839076.6124.8.camel@aj.id.au>","Content-Type":"multipart/signed; micalg=\"pgp-sha512\";\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"=-QG6e8yWIzdDght3qgphi\"","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":"linux-hwmon@vger.kernel.org, jdelvare@suse.com, corbet@lwn.net,\n\topenbmc@lists.ozlabs.org, linux-doc@vger.kernel.org,\n\tlinux-kernel@vger.kernel.org","Errors-To":"openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"openbmc\"\n\t"}},{"id":1765075,"web_url":"http://patchwork.ozlabs.org/comment/1765075/","msgid":"<020c1373-6c3e-15e0-340a-e617c3147610@roeck-us.net>","list_archive_url":null,"date":"2017-09-08T04:43:30","subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","submitter":{"id":21889,"url":"http://patchwork.ozlabs.org/api/people/21889/","name":"Guenter Roeck","email":"linux@roeck-us.net"},"content":"On 09/07/2017 08:40 PM, Andrew Jeffery wrote:\n> On Fri, 2017-09-08 at 12:51 +1000, Andrew Jeffery wrote:\n>>> I can't test with devicetree. x86 system.\n>>> \n>>> 2,100+ iterations with your driver, no failures.\n>>\n>> Great. I really appreciate your testing here, so thanks for your\n>> efforts. I owe you a few drinks if we ever happen to meet.\n> \n> Actually, on that, how did you chop out the devicetree parts? Did you\n> keep the configuration writes at the end of max31785_of_fan_config()\n> and max31785_of_tmp_config()? Or did you just not call them? These two\n> functions cause the bulk of the bus traffic with on probe.\n> \n\nI didn't change to code, just compiled and run it. Guess that means\nthis part was skipped.\n\nI'll replaced the fan configuration with some hard-coded values.\nWe'll see if it makes a difference.\n\nGuenter","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 3xpPpl6BM7z9sCZ\n\tfor ;\n\tFri, 8 Sep 2017 14:43:43 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xpPpl3Wk8zDrYl\n\tfor ;\n\tFri, 8 Sep 2017 14:43:43 +1000 (AEST)","from mail-pg0-x244.google.com (mail-pg0-x244.google.com\n\t[IPv6:2607:f8b0:400e:c05::244])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128\n\tbits)) (No client certificate requested)\n\tby lists.ozlabs.org (Postfix) with ESMTPS id 3xpPpd0bg4zDqZ9\n\tfor ; Fri, 8 Sep 2017 14:43:35 +1000 (AEST)","by mail-pg0-x244.google.com with SMTP id m9so845593pgd.0\n\tfor ; Thu, 07 Sep 2017 21:43:35 -0700 (PDT)","from server.roeck-us.net\n\t(108-223-40-66.lightspeed.sntcca.sbcglobal.net. [108.223.40.66])\n\tby smtp.gmail.com with ESMTPSA id\n\tw90sm1559397pfi.80.2017.09.07.21.43.31\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tThu, 07 Sep 2017 21:43:31 -0700 (PDT)"],"Authentication-Results":["ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"UKsIxQuW\"; dkim-atps=neutral","lists.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"UKsIxQuW\"; dkim-atps=neutral","ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=gmail.com\n\t(client-ip=2607:f8b0:400e:c05::244; helo=mail-pg0-x244.google.com;\n\tenvelope-from=groeck7@gmail.com; receiver=)","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"UKsIxQuW\"; dkim-atps=neutral"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=sender:subject:to:cc:references:from:message-id:date:user-agent\n\t:mime-version:in-reply-to:content-language:content-transfer-encoding; \n\tbh=Bpy8WVadnfRV3pFQf249egfIg/r46tXRFF3nNXfYL5g=;\n\tb=UKsIxQuWSxo7JFtV0p1YUm6/CFmhhnykKepUwz5HpesoyCe0qKN6kxmMw29SHfKEsd\n\tD+7/E5teQ9Zmp/AJM2GXVgfnNhbioc0s9zPE9jkhqdfVgSUeccJkfwKFK8SSvuXPAN7K\n\tCLDbz3aj0zV4QyJ/MYZkJ19yAMceKaK6bfRS52NoV3EgiyuriF5OAUt1WasqmdkMUM2o\n\tSDFdz12vidvIhlOw+pmRCMYQuraJuL36z/o/cdrO9aOFXZyoBl8cVIno2OXrj5EuXRBz\n\tkn8tSEPcOrLzwKFjATHJKvEGMPlfaSD0HrwNPRfQAD7nY14SHPWS1+b4aTbqmnG1ngXU\n\twnuQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:sender:subject:to:cc:references:from:message-id\n\t:date:user-agent:mime-version:in-reply-to:content-language\n\t:content-transfer-encoding;\n\tbh=Bpy8WVadnfRV3pFQf249egfIg/r46tXRFF3nNXfYL5g=;\n\tb=L7M1/DGOpUWk6TlzXsNzj11JTdTLWuq9Dec+WGKv4oDG1p92pSblbFFJXuq0Uu9Zko\n\tjb2hIdGasT8XtznpVfyEQyy7StDgCKqi2Jsk6GNRpCdbtGE7RP1h9ls8oNpQ+ExnnlzQ\n\t5399qLQbEjaqcaWECKw37e3L7qYf07//KYifV2rze4CSQMoCjwquSaAISWpDoMwJUlNy\n\tOyZ7WIari19fXTURkhLW/dXBdmDU1pW3qe963ze1QVgRIztlwpwcQPyDP/w4MocR2V+G\n\tGzYHbEmV/GGVIbDWzSvgcTps45mDQ1qWnzRTrnA8EoIql/BeWqCZfu1NVqmMh3B5TIHn\n\t2WtQ==","X-Gm-Message-State":"AHPjjUh7jT4vnGBea3f5ZM5Dh+MufXEaDCZ8K6woQuyCk5STHNet59Yu\n\tW4S/d2A8cujU2A==","X-Google-Smtp-Source":"ADKCNb4VR0ARVXBdjDFC3TRMQbbQgx8i67OXbsIiNmN+XuRW9Exyc8ANR9B4zRzrMvHrlhtOxcL0lg==","X-Received":"by 10.84.216.69 with SMTP id f5mr1965763plj.212.1504845813107;\n\tThu, 07 Sep 2017 21:43:33 -0700 (PDT)","Subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","To":"Andrew Jeffery ","References":"<20170905070132.17682-1-andrew@aj.id.au>\n\t<20170905170002.GG11478@roeck-us.net>\n\t<1504657417.28363.8.camel@aj.id.au>\n\t<20170906225102.GA32210@roeck-us.net>\n\t<1504740749.5042.2.camel@aj.id.au>\n\t\n\t<1504797770.5105.7.camel@aj.id.au>\n\t\n\t<1504832527.6124.1.camel@aj.id.au>\n\t\n\t<1504836369.6124.2.camel@aj.id.au>\n\t<75d9f838-aa4a-5cbd-744a-4d8e8bf1a370@roeck-us.net>\n\t<1504839076.6124.8.camel@aj.id.au>\n\t<1504842058.6124.10.camel@aj.id.au>","From":"Guenter Roeck ","Message-ID":"<020c1373-6c3e-15e0-340a-e617c3147610@roeck-us.net>","Date":"Thu, 7 Sep 2017 21:43:30 -0700","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<1504842058.6124.10.camel@aj.id.au>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","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":"linux-hwmon@vger.kernel.org, jdelvare@suse.com, corbet@lwn.net,\n\topenbmc@lists.ozlabs.org, linux-doc@vger.kernel.org,\n\tlinux-kernel@vger.kernel.org","Errors-To":"openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"openbmc\"\n\t"}},{"id":1765080,"web_url":"http://patchwork.ozlabs.org/comment/1765080/","msgid":"<1504847328.6124.15.camel@aj.id.au>","list_archive_url":null,"date":"2017-09-08T05:08:48","subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","submitter":{"id":68332,"url":"http://patchwork.ozlabs.org/api/people/68332/","name":"Andrew Jeffery","email":"andrew@aj.id.au"},"content":"On Thu, 2017-09-07 at 21:43 -0700, Guenter Roeck wrote:\n> On 09/07/2017 08:40 PM, Andrew Jeffery wrote:\n> > On Fri, 2017-09-08 at 12:51 +1000, Andrew Jeffery wrote:\n> > > > I can't test with devicetree. x86 system.\n> > > >   \n> > > > 2,100+ iterations with your driver, no failures.\n> > > \n> > > Great. I really appreciate your testing here, so thanks for your\n> > > efforts. I owe you a few drinks if we ever happen to meet.\n> > \n> > Actually, on that, how did you chop out the devicetree parts? Did you\n> > keep the configuration writes at the end of max31785_of_fan_config()\n> > and max31785_of_tmp_config()? Or did you just not call them? These two\n> > functions cause the bulk of the bus traffic with on probe.\n> > \n> \n> I didn't change to code, just compiled and run it. Guess that means\n> this part was skipped.\n\nRight, yeah looking at it a bit more, dev->of_node will be NULL for\nfor_each_child_of_node(dev->of_node, child), therefore the loop body\nwon't execute.\n\n> \n> I'll replaced the fan configuration with some hard-coded values.\n> We'll see if it makes a difference.\n\nSounds good.\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 3xpQN20jPvz9sQl\n\tfor ;\n\tFri, 8 Sep 2017 15:09:06 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xpQN13BK8zDrZC\n\tfor ;\n\tFri, 8 Sep 2017 15:09:05 +1000 (AEST)","from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com\n\t[66.111.4.25])\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 3xpQMt6SvTzDrWl\n\tfor ; Fri, 8 Sep 2017 15:08:58 +1000 (AEST)","from compute4.internal (compute4.nyi.internal [10.202.2.44])\n\tby mailout.nyi.internal (Postfix) with ESMTP id AE6FA20C28;\n\tFri, 8 Sep 2017 01:08:55 -0400 (EDT)","from frontend1 ([10.202.2.160])\n\tby compute4.internal (MEProxy); Fri, 08 Sep 2017 01:08:55 -0400","from keelia (unknown [122.99.82.10])\n\tby mail.messagingengine.com (Postfix) with ESMTPA id 5A9387F96A;\n\tFri, 8 Sep 2017 01:08:53 -0400 (EDT)"],"Authentication-Results":["ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"P7r3TFUU\";\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"VVi5HeXR\"; \n\tdkim-atps=neutral","lists.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"P7r3TFUU\";\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"VVi5HeXR\"; \n\tdkim-atps=neutral","ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=aj.id.au\n\t(client-ip=66.111.4.25; helo=out1-smtp.messagingengine.com;\n\tenvelope-from=andrew@aj.id.au; receiver=)","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"P7r3TFUU\";\n\tdkim=pass (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com\n\theader.b=\"VVi5HeXR\"; 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=/cvXfdVpFFLE/8vkKk2cq4Syu7YIpRvdtii2+qTZ0\n\ty0=; b=P7r3TFUU9vM2qAI4y0ArDdA0+Hl1CijHJnXkl8kda0tabe1oyFiXRScGi\n\tfeKGqf+pkderck8WJdpIG1WsOLoWXJ64CIK3Pma9Add3c3K4Vq3mAUn8MND90d/C\n\tjb+RwxHr7n7pjuR4rYANn8vx2ff/cabDZreMSmKV1HTIgm7H7r1hf92uCMYKBULF\n\tt4lpkTheDBJcez4ErJoFc9OQeq2QODPXDSfLRfD23yBxQ0cAJP38jRu/TyLFb1Hb\n\t59I3jrptgOsHC6QTuTgObQtFX+VwkDSvEIWxSPjjHLXH9JpcSFVCLJvzqV817IR1\n\tDiQnt4vJOPssFh0iHl++0VooDyDIw==","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=/cvXfdVpFFLE/8vkKk\n\t2cq4Syu7YIpRvdtii2+qTZ0y0=; b=VVi5HeXRX5iJwBh0lsy1eU6TFp2l2v5llh\n\tDUhPrRNFgnOmniys5Q5ZNF/xC1Y35eC2po72+cCSTQByifjLHcHQ9DhEuiKzuloL\n\tFXHtrIbffatikUKu5QxhxfM57ha2xOKWfm5ZMROxbRI8J40vZSAxajrJiInFA3iP\n\tB00GoqHAsXYyyf+gxVHgkAiUZyFVKrmjLj4M6Fy20WSqixrp/BwITbwKd9S9Gt/x\n\tR56eCJNi0yDs2367IBmqGOaqoFd1jhIdCDjKO+8x3X7/ZNJreZdYKjJ5CmHSSn+C\n\tUNbbPY4GHINFyMCU/SGiAgc/kdX3ODXIgWfceELts4fI5gkQ5S2g=="],"X-ME-Sender":"","X-Sasl-enc":"cFLpJR7JmwuamVgy1EsndEbkwaE5OovlIpLPUSeCwhYu 1504847335","Message-ID":"<1504847328.6124.15.camel@aj.id.au>","Subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","From":"Andrew Jeffery ","To":"Guenter Roeck ","Date":"Fri, 08 Sep 2017 15:08:48 +1000","In-Reply-To":"<020c1373-6c3e-15e0-340a-e617c3147610@roeck-us.net>","References":"<20170905070132.17682-1-andrew@aj.id.au>\n\t<20170905170002.GG11478@roeck-us.net>\n\t<1504657417.28363.8.camel@aj.id.au>\n\t<20170906225102.GA32210@roeck-us.net>\n\t<1504740749.5042.2.camel@aj.id.au>\n\t\n\t<1504797770.5105.7.camel@aj.id.au>\n\t\n\t<1504832527.6124.1.camel@aj.id.au>\n\t\n\t<1504836369.6124.2.camel@aj.id.au>\n\t<75d9f838-aa4a-5cbd-744a-4d8e8bf1a370@roeck-us.net>\n\t<1504839076.6124.8.camel@aj.id.au>\n\t<1504842058.6124.10.camel@aj.id.au>\n\t<020c1373-6c3e-15e0-340a-e617c3147610@roeck-us.net>","Content-Type":"multipart/signed; micalg=\"pgp-sha512\";\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"=-cGhw3ZRf7+/GlwEqq5iR\"","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":"linux-hwmon@vger.kernel.org, jdelvare@suse.com, corbet@lwn.net,\n\topenbmc@lists.ozlabs.org, linux-doc@vger.kernel.org,\n\tlinux-kernel@vger.kernel.org","Errors-To":"openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"openbmc\"\n\t"}},{"id":1765081,"web_url":"http://patchwork.ozlabs.org/comment/1765081/","msgid":"","list_archive_url":null,"date":"2017-09-08T05:14:34","subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","submitter":{"id":21889,"url":"http://patchwork.ozlabs.org/api/people/21889/","name":"Guenter Roeck","email":"linux@roeck-us.net"},"content":"On 09/07/2017 08:40 PM, Andrew Jeffery wrote:\n> On Fri, 2017-09-08 at 12:51 +1000, Andrew Jeffery wrote:\n>>> I can't test with devicetree. x86 system.\n>>> \n>>> 2,100+ iterations with your driver, no failures.\n>>\n>> Great. I really appreciate your testing here, so thanks for your\n>> efforts. I owe you a few drinks if we ever happen to meet.\n> \n> Actually, on that, how did you chop out the devicetree parts? Did you\n> keep the configuration writes at the end of max31785_of_fan_config()\n> and max31785_of_tmp_config()? Or did you just not call them? These two\n> functions cause the bulk of the bus traffic with on probe.\n> \nfan config hardcoded, four fans connected. Still no failure.\n\nGuenter","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 [103.22.144.68])\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 3xpQVW33p8z9sDB\n\tfor ;\n\tFri, 8 Sep 2017 15:14:43 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xpQVW1tTYzDrZC\n\tfor ;\n\tFri, 8 Sep 2017 15:14:43 +1000 (AEST)","from mail-pg0-x241.google.com (mail-pg0-x241.google.com\n\t[IPv6:2607:f8b0:400e:c05::241])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128\n\tbits)) (No client certificate requested)\n\tby lists.ozlabs.org (Postfix) with ESMTPS id 3xpQVQ2jpyzDrX7\n\tfor ; Fri, 8 Sep 2017 15:14:38 +1000 (AEST)","by mail-pg0-x241.google.com with SMTP id v82so873873pgb.1\n\tfor ; Thu, 07 Sep 2017 22:14:38 -0700 (PDT)","from server.roeck-us.net\n\t(108-223-40-66.lightspeed.sntcca.sbcglobal.net. [108.223.40.66])\n\tby smtp.gmail.com with ESMTPSA id\n\ts1sm1772400pfk.27.2017.09.07.22.14.34\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tThu, 07 Sep 2017 22:14:35 -0700 (PDT)"],"Authentication-Results":["ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"MgUZXSXl\"; dkim-atps=neutral","lists.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"MgUZXSXl\"; dkim-atps=neutral","ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=gmail.com\n\t(client-ip=2607:f8b0:400e:c05::241; helo=mail-pg0-x241.google.com;\n\tenvelope-from=groeck7@gmail.com; receiver=)","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"MgUZXSXl\"; dkim-atps=neutral"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=sender:subject:to:cc:references:from:message-id:date:user-agent\n\t:mime-version:in-reply-to:content-language:content-transfer-encoding; \n\tbh=Ai/G3lC6cphYwhcAg8g4JaRY95PtppY8/F/pPNPD5po=;\n\tb=MgUZXSXltfftBuR0IIs/N76c05hF9FwdefB1P5fuLg5SUO4ayubHJ/AR+BxxN+BWsR\n\tB10DOVhwmF3dyiOd3x+oGE3oR40u0PBT21I7B3RxJI0LuLhXiXp2oMmg9DSsMM60f/r+\n\tU2rDJBnA1gza20EMD9SSYV95/GfKGvYG2O0nyzLEdDKfl11zIoxMBPS+e5jbGGHM1+MR\n\t1VjnrQT4EUhac2x+pNZ9IquV9AF3URtZpGl4UruzZ5iV6bG6+1iVUruA/699rYLQjvj7\n\tdXGnrjTj6+7hWFzHtrtW9Nu6Wg2EbBS1yh103BQRHbkHdX7AGgWllANIJGFPfAPIQ4fr\n\tP5Sg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:sender:subject:to:cc:references:from:message-id\n\t:date:user-agent:mime-version:in-reply-to:content-language\n\t:content-transfer-encoding;\n\tbh=Ai/G3lC6cphYwhcAg8g4JaRY95PtppY8/F/pPNPD5po=;\n\tb=GhKOoozO8i1l1jM81Rnpl3AM7TLMScZXkQvr9DpqGyzD0ebUxm9W9n52FuPRKZbw49\n\tYxvTmOFMkNYR+t0605DE+ZsvTY8YBD6DM+kfRMFNjJ3XIJ2DnQNk+t0dkaDtZyshiSBO\n\tLlblOOXu2Hvc83theTvWP9ftgjBOxhwufQWN9FAbyE30t3L3knodn4EqJl71x8sJf11J\n\tq9iRL3tW8fIRYxMAvBvgHwHmOCMHmSPY1kkFxM4XIn3FIj5PIt/gV6H5Qm0v195FJvRw\n\t1iKeyZwIvmqzR4LHFA8IP0pengcyPNNyKRnLGEekkwiIaiuDSKewFARQxOcF1Jo77i/b\n\tVOwA==","X-Gm-Message-State":"AHPjjUjdDuoDYjNyry1i58S0yag+pa0yMgiiIiInPGFEhGXbsTTtRe+u\n\tIjoufA1Xi/ly8Szk","X-Google-Smtp-Source":"ADKCNb6kzdwXtILmPzqqFW7wXoGJ5gZ0kX7QXQyiGJ4u1olzG1/z5uZyrfL6Gm1E0aE2iYFvkjytug==","X-Received":"by 10.98.15.208 with SMTP id 77mr1863274pfp.318.1504847676304;\n\tThu, 07 Sep 2017 22:14:36 -0700 (PDT)","Subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","To":"Andrew Jeffery ","References":"<20170905070132.17682-1-andrew@aj.id.au>\n\t<20170905170002.GG11478@roeck-us.net>\n\t<1504657417.28363.8.camel@aj.id.au>\n\t<20170906225102.GA32210@roeck-us.net>\n\t<1504740749.5042.2.camel@aj.id.au>\n\t\n\t<1504797770.5105.7.camel@aj.id.au>\n\t\n\t<1504832527.6124.1.camel@aj.id.au>\n\t\n\t<1504836369.6124.2.camel@aj.id.au>\n\t<75d9f838-aa4a-5cbd-744a-4d8e8bf1a370@roeck-us.net>\n\t<1504839076.6124.8.camel@aj.id.au>\n\t<1504842058.6124.10.camel@aj.id.au>","From":"Guenter Roeck ","Message-ID":"","Date":"Thu, 7 Sep 2017 22:14:34 -0700","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<1504842058.6124.10.camel@aj.id.au>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","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":"linux-hwmon@vger.kernel.org, jdelvare@suse.com, corbet@lwn.net,\n\topenbmc@lists.ozlabs.org, linux-doc@vger.kernel.org,\n\tlinux-kernel@vger.kernel.org","Errors-To":"openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"openbmc\"\n\t"}},{"id":1765083,"web_url":"http://patchwork.ozlabs.org/comment/1765083/","msgid":"<1504847776.6124.17.camel@aj.id.au>","list_archive_url":null,"date":"2017-09-08T05:16:16","subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","submitter":{"id":68332,"url":"http://patchwork.ozlabs.org/api/people/68332/","name":"Andrew Jeffery","email":"andrew@aj.id.au"},"content":"On Thu, 2017-09-07 at 22:14 -0700, Guenter Roeck wrote:\n> On 09/07/2017 08:40 PM, Andrew Jeffery wrote:\n> > On Fri, 2017-09-08 at 12:51 +1000, Andrew Jeffery wrote:\n> > > > I can't test with devicetree. x86 system.\n> > > >   \n> > > > 2,100+ iterations with your driver, no failures.\n> > > \n> > > Great. I really appreciate your testing here, so thanks for your\n> > > efforts. I owe you a few drinks if we ever happen to meet.\n> > \n> > Actually, on that, how did you chop out the devicetree parts? Did you\n> > keep the configuration writes at the end of max31785_of_fan_config()\n> > and max31785_of_tmp_config()? Or did you just not call them? These two\n> > functions cause the bulk of the bus traffic with on probe.\n> > \n> \n> fan config hardcoded, four fans connected. Still no failure.\n\nGreat. Thanks again for your efforts here.\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 [103.22.144.68])\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 3xpQXj50YXz9sQl\n\tfor ;\n\tFri, 8 Sep 2017 15:16:37 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xpQXj3pJKzDrXQ\n\tfor ;\n\tFri, 8 Sep 2017 15:16:37 +1000 (AEST)","from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com\n\t[66.111.4.25])\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 3xpQXb3hNGzDqF4\n\tfor ; Fri, 8 Sep 2017 15:16:31 +1000 (AEST)","from compute4.internal (compute4.nyi.internal [10.202.2.44])\n\tby mailout.nyi.internal (Postfix) with ESMTP id 5265620DFF;\n\tFri, 8 Sep 2017 01:16:29 -0400 (EDT)","from frontend2 ([10.202.2.161])\n\tby compute4.internal (MEProxy); Fri, 08 Sep 2017 01:16:29 -0400","from keelia (bh02i525f01.au.ibm.com [202.81.18.30])\n\tby mail.messagingengine.com (Postfix) with ESMTPA id 7357D24081;\n\tFri, 8 Sep 2017 01:16:23 -0400 (EDT)"],"Authentication-Results":["ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"VkjHmn7P\";\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"N0DndAqh\"; \n\tdkim-atps=neutral","lists.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"VkjHmn7P\";\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"N0DndAqh\"; \n\tdkim-atps=neutral","ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=aj.id.au\n\t(client-ip=66.111.4.25; helo=out1-smtp.messagingengine.com;\n\tenvelope-from=andrew@aj.id.au; receiver=)","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"VkjHmn7P\";\n\tdkim=pass (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com\n\theader.b=\"N0DndAqh\"; 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=iXo7tmLRiO405hwp3TmDFazbfcY6eVex7FLmI6pbU\n\tm0=; b=VkjHmn7PjngjcAM1y5jUeXBKyrb73r0Pn2FHJiJmiJqw38kBh4bS72M+S\n\tPVLXLP7KaGJO0csKRnUcTg/Y2AE8aM0yjdvokpoJYVONwZA4hj3eA+e1+x5kHq1m\n\tLqKxQl4r8gBg2NDcFie7DaHMy4hYnHdjxBCI0w3B35MLYKbVeg7FsQue5wFhs9wJ\n\tH2lrGEQUblCDlRKgROk7D9yTW29d8SZ7VkE1qY0bdkTzGr11MjqDY6BZ0BFNAojX\n\tV+YymHkXlgpQyHoqtYJZmgH6xnGK5SReuseiT7QS7/5qaD/4nMW2lYdKn+WmtvzG\n\tBKCAMRCuJ22bYi0fDyL7wHF8hNsWg==","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=iXo7tmLRiO405hwp3T\n\tmDFazbfcY6eVex7FLmI6pbUm0=; b=N0DndAqhyA5aarxVI23LRzRZWE+P0x+kpE\n\tdPv9Q9lZOay7qNu04mbl2jN48uinrdmmMPaklMUuivCFcDwbNK1RKjBrJ8ASMrMO\n\tSVoPvLLOJYzXRoCLFSedydMM6vbXC5MCjMcKiPkWPGjviEe2eF8hLimlIfatVr2F\n\tHzIG74+2H6L1QNux5s/imn4vRiDXxZ+GOa8i5e11SsJcDiAT4LdFgSx4ooFmorei\n\tbvawKU59J2LhINTpB8cId8nst6Lj4b6fsSZi1Wg2NojIF8Jc4VxmQY0ydX+IhCcp\n\tEI786y0ve92emIOvByvORevLOMXFiJ21EVGsYCZ2PXZRq5C/+j3w=="],"X-ME-Sender":"","X-Sasl-enc":"F2lW/LW0i4Z8jbzgJTgvNLnggWivBZmZleA7vpSgIt2V 1504847788","Message-ID":"<1504847776.6124.17.camel@aj.id.au>","Subject":"Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions\n\treturn errors","From":"Andrew Jeffery ","To":"Guenter Roeck ","Date":"Fri, 08 Sep 2017 15:16:16 +1000","In-Reply-To":"","References":"<20170905070132.17682-1-andrew@aj.id.au>\n\t<20170905170002.GG11478@roeck-us.net>\n\t<1504657417.28363.8.camel@aj.id.au>\n\t<20170906225102.GA32210@roeck-us.net>\n\t<1504740749.5042.2.camel@aj.id.au>\n\t\n\t<1504797770.5105.7.camel@aj.id.au>\n\t\n\t<1504832527.6124.1.camel@aj.id.au>\n\t\n\t<1504836369.6124.2.camel@aj.id.au>\n\t<75d9f838-aa4a-5cbd-744a-4d8e8bf1a370@roeck-us.net>\n\t<1504839076.6124.8.camel@aj.id.au>\n\t<1504842058.6124.10.camel@aj.id.au>\n\t","Content-Type":"multipart/signed; micalg=\"pgp-sha512\";\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"=-+ucxoPjWpWJMrpdaHBPh\"","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":"linux-hwmon@vger.kernel.org, jdelvare@suse.com, corbet@lwn.net,\n\topenbmc@lists.ozlabs.org, linux-doc@vger.kernel.org,\n\tlinux-kernel@vger.kernel.org","Errors-To":"openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"openbmc\"\n\t"}}]