[{"id":1760636,"web_url":"http://patchwork.ozlabs.org/comment/1760636/","msgid":"<20170831041111.GC26660@roeck-us.net>","list_archive_url":null,"date":"2017-08-31T04:11:11","subject":"Re: [PATCH 2/2] hwmon: (ucd9000) Add sysfs attribute to clear logged\n\tfaults","submitter":{"id":21889,"url":"http://patchwork.ozlabs.org/api/people/21889/","name":"Guenter Roeck","email":"linux@roeck-us.net"},"content":"On Wed, Aug 30, 2017 at 01:55:36PM -0500, Christopher Bostic wrote:\n> Add ability to clear logged faults via sysfs.\n> \nI am not in favor of such chip specific commands, in this case for several\nreasons (besides it being a non-standard attribute).\n\nThe logged faults are not read or used by the driver in the first place.\nClearing them doesn't add any benefit for the driver.\n\nThe command as written only clears non-paged logged faults, but no paged\nfaults.\n\nWhile PMBus \"lingo\" talks of faults, those are really often just status bits,\nnot \"faults\" from hwmon perspective.\n\nThe content of the various fault bits and bites is highly chip specific.\nOn top of that, each chip supported by the driver has different fault bits\nand bytes.\n\nTo read the logged faults you have to access the chip directly, not through\nthe driver. I don't see the benefit of providing a sysfs attribute to clear\nnon-paged logged faults, but not doing anything else with it.\n\nIf you want to do something special with those faults (or status bits/nytes),\nit might be more appropriate to create debugfs entries.\n\nThanks,\nGuenter\n\n> Signed-off-by: Christopher Bostic \n> ---\n> drivers/hwmon/pmbus/ucd9000.c | 45 ++++++++++++++++++++++++++++++++++++++++++-\n> 1 file changed, 44 insertions(+), 1 deletion(-)\n> \n> diff --git a/drivers/hwmon/pmbus/ucd9000.c b/drivers/hwmon/pmbus/ucd9000.c\n> index b74dbec..8bd8cb1 100644\n> --- a/drivers/hwmon/pmbus/ucd9000.c\n> +++ b/drivers/hwmon/pmbus/ucd9000.c\n> @@ -27,6 +27,8 @@\n> #include \n> #include \n> #include \n> +#include \n> +#include \n> #include \"pmbus.h\"\n> \n> enum chips { ucd9000, ucd90120, ucd90124, ucd90160, ucd9090, ucd90910 };\n> @@ -35,6 +37,7 @@\n> #define UCD9000_NUM_PAGES\t\t0xd6\n> #define UCD9000_FAN_CONFIG_INDEX\t0xe7\n> #define UCD9000_FAN_CONFIG\t\t0xe8\n> +#define UCD9000_LOGGED_FAULTS\t\t0xea\n> #define UCD9000_DEVICE_ID\t\t0xfd\n> \n> #define UCD9000_MON_TYPE(x)\t(((x) >> 5) & 0x07)\n> @@ -109,6 +112,36 @@ static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg)\n> \treturn ret;\n> }\n> \n> +static ssize_t ucd9000_clear_logged_faults(struct device *dev,\n> +\t\t\tstruct device_attribute *attr, const char *buf,\n> +\t\t\tsize_t count)\n> +{\n> +\tstruct i2c_client *client = to_i2c_client(dev);\n> +\tint ret;\n> +\n> +\t/* No page set required */\n> +\tret = i2c_smbus_write_byte_data(client, UCD9000_LOGGED_FAULTS, 0);\n> +\tif (ret) {\n> +\t\tdev_err(&client->dev, \"Failed to clear logged faults: %d\\n\",\n> +\t\t\tret);\n> +\t\treturn ret;\n> +\t}\n> +\n> +\treturn count;\n> +}\n> +\n> +static DEVICE_ATTR(clear_logged_faults, 0200, NULL,\n> +\t\t\tucd9000_clear_logged_faults);\n> +\n> +static struct attribute *ucd9000_attributes[] = {\n> +\t&dev_attr_clear_logged_faults.attr,\n> +\tNULL\n> +};\n> +\n> +static const struct attribute_group ucd9000_attr_group = {\n> +\t.attrs = ucd9000_attributes,\n> +};\n> +\n> static const struct i2c_device_id ucd9000_id[] = {\n> \t{\"ucd9000\", ucd9000},\n> \t{\"ucd90120\", ucd90120},\n> @@ -263,9 +296,19 @@ static int ucd9000_probe(struct i2c_client *client,\n> \t\t | PMBUS_HAVE_FAN34 | PMBUS_HAVE_STATUS_FAN34;\n> \t}\n> \n> +\tret = sysfs_create_group(&client->dev.kobj, &ucd9000_attr_group);\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\n> \treturn pmbus_do_probe(client, mid, info);\n> }\n> \n> +static int ucd9000_remove(struct i2c_client *client)\n> +{\n> +\tsysfs_remove_group(&client->dev.kobj, &ucd9000_attr_group);\n> +\treturn pmbus_do_remove(client);\n> +}\n> +\n> /* This is the driver that will be inserted */\n> static struct i2c_driver ucd9000_driver = {\n> \t.driver = {\n> @@ -273,7 +316,7 @@ static int ucd9000_probe(struct i2c_client *client,\n> \t\t.of_match_table = of_match_ptr(ucd9000_of_match),\n> \t},\n> \t.probe = ucd9000_probe,\n> -\t.remove = pmbus_do_remove,\n> +\t.remove = ucd9000_remove,\n> \t.id_table = ucd9000_id,\n> };\n> \n> -- \n> 1.8.2.2\n> \n> --\n> To unsubscribe from this list: send the line \"unsubscribe linux-hwmon\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html","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 3xjTT56Nz3z9s71\n\tfor ;\n\tThu, 31 Aug 2017 14:11:21 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xjTT52tZHzDqVg\n\tfor ;\n\tThu, 31 Aug 2017 14:11:21 +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 3xjTSy0hprzDq5b\n\tfor ; Thu, 31 Aug 2017 14:11:14 +1000 (AEST)","by mail-pf0-x241.google.com with SMTP id r62so2762604pfj.0\n\tfor ; Wed, 30 Aug 2017 21:11:14 -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\tu187sm10938117pfu.140.2017.08.30.21.11.11\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tWed, 30 Aug 2017 21:11:11 -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=\"AamSQdUx\"; 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=\"AamSQdUx\"; dkim-atps=neutral","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"AamSQdUx\"; 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=iDzJYs80a1lTw+1+osTP5Rdh2cht5lAyZei4afmzi58=;\n\tb=AamSQdUxjsrGyj5rXTyo794NSqPqCUx4QxPc+8sMBxni4HnqFKpnAiPv+75i42h+wL\n\tLPbdwJQUhGfsd1asUQD+DvoaQ9JhZwpmCbZlAO++HcEeq6GFgmFKmX0AA7dUV/pgSfdJ\n\th2DDRIZmFRNz/+OPzcZbKEe2uyRZPtjdxEmH/G4jU9gjeYPNyKID6xK8dlXSVVI+53+O\n\t6QLi4uudM3oETKG1pEEAC+3t41q+EE7ZkeruHFrateySoMtV07tSfo3LU1wSOcaTxc+K\n\tqeFZ/0ZQJopepQxmtGeDnRQJuryVmiXZqOXHhX2SIWz3n4W979lVoGylresBUfMyR0Ps\n\tkjBg==","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=iDzJYs80a1lTw+1+osTP5Rdh2cht5lAyZei4afmzi58=;\n\tb=Cctykp+Rf1868ek835L3yrDheCcZCpZLmRM2s27ahuF4kyy2PZT6rVHNgjAWm1m0NY\n\tzCu7FcaPjJ8mhy1l6iRSaUYmH7+mHFcVEEODRGcKryDgmnk7dip+b/z9XJWfxqun5R+M\n\tp1HkpLakztpIrcnY2CH86pItS3+8FcwRqheAT3+gpqXzUTVKyFU4H00SuGHFerDF+IMI\n\t3JtuZBX5G5judarjUxoN7RA0KyH+sGM7OMXoK+m2RRQkfl6E32epU/nzNghhPxsGazyu\n\tXV/psYqbmpygIhnx0GlaBI1Pg44beDbIBceoBuCMPMOpTzfXmyEJlQFYayTd1e71oVSA\n\tz+ag==","X-Gm-Message-State":"AHYfb5i7vITBXkeaGJ0HZkvTvYh5jLK/GN6Ry8L6PA//JrhlJCujkH+I\n\t1b2xoZV97NX6EA==","X-Received":"by 10.98.58.220 with SMTP id v89mr918007pfj.19.1504152672399;\n\tWed, 30 Aug 2017 21:11:12 -0700 (PDT)","Date":"Wed, 30 Aug 2017 21:11:11 -0700","From":"Guenter Roeck ","To":"Christopher Bostic ","Subject":"Re: [PATCH 2/2] hwmon: (ucd9000) Add sysfs attribute to clear logged\n\tfaults","Message-ID":"<20170831041111.GC26660@roeck-us.net>","References":"<20170830185536.71307-1-cbostic@linux.vnet.ibm.com>\n\t<20170830185536.71307-3-cbostic@linux.vnet.ibm.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170830185536.71307-3-cbostic@linux.vnet.ibm.com>","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"}}]