From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753960AbcD2NzT (ORCPT ); Fri, 29 Apr 2016 09:55:19 -0400 Received: from mail-bn1on0119.outbound.protection.outlook.com ([157.56.110.119]:26208 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753609AbcD2NzP convert rfc822-to-8bit (ORCPT ); Fri, 29 Apr 2016 09:55:15 -0400 X-Greylist: delayed 164382 seconds by postgrey-1.27 at vger.kernel.org; Fri, 29 Apr 2016 09:55:15 EDT From: "Dall, Betty" To: "Rafael J. Wysocki" CC: "lenb@kernel.org" , "linux-acpi@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH] ACPI/device_sysfs: Add sysfs support for _HRV hardware revision Thread-Topic: [PATCH] ACPI/device_sysfs: Add sysfs support for _HRV hardware revision Thread-Index: AQHRoKCcgScEqgDbwUOsm6w7O7yHbA== Date: Fri, 29 Apr 2016 13:55:12 +0000 Message-ID: References: <1460558894-11971-1-git-send-email-betty.dall@hpe.com> <2388310.jGjaKS2NDe@vostro.rjw.lan> <1818106.auxzDWcdFV@vostro.rjw.lan> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: rjwysocki.net; dkim=none (message not signed) header.d=none;rjwysocki.net; dmarc=none action=none header.from=hpe.com; x-originating-ip: [15.65.252.15] x-ms-office365-filtering-correlation-id: d4a7e751-da58-4363-e878-08d37035e265 x-microsoft-exchange-diagnostics: 1;CS1PR84MB0085;5:JWx6oMloD4JJWw2Gbr2fkv9OtBK24gAR1MNt3vUvzqNTY08eY/+F7TpIXizl5t+iOuaPxkwlQ972C1m6BdVZQ/fb4cxwGh3gDpJhRVC7gvGoSMf/INp5vUGfNXfO2TjS6PX3dbWZM6KeJtTTxO43Vw==;24:/OY2PGXf2MKbyhrvBgrbOfhGjV5ieP625sQW2Z+I9k025YJ8Qfl/pvkN6cilV3EhX8KAotuJ4rUgFVVgYy4jIKb2GB1/kB1lrh53kReS5ME=;7:sqvKd1M2ZYhqiZjDKIY058RgmHh7E9bcmd/W/t7lPhT7VHP5pNAPfF25I4zeWfWtO8+TwKvUkheGPSIwGCLiCX4YHy+JZmLa4UOJ7p7cGyNvRS8W+i/+p0wjL20hVAz0uw3AUlE9DlBUwWwf7jMJGjCG5yZg24yAkpSwDmWrfppG0RqGgtFJnKNOeE7S8Rbx x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:CS1PR84MB0085; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:; x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(9101521072)(601004)(2401047)(8121501046)(5005006)(3002001)(10201501046);SRVR:CS1PR84MB0085;BCL:0;PCL:0;RULEID:;SRVR:CS1PR84MB0085; x-forefront-prvs: 0927AA37C7 x-forefront-antispam-report: SFV:NSPM;SFS:(10019020)(6009001)(377454003)(24454002)(4326007)(10400500002)(5002640100001)(92566002)(2900100001)(77096005)(50986999)(66066001)(6116002)(102836003)(87936001)(5003600100002)(3846002)(1220700001)(1096002)(586003)(33656002)(5004730100002)(3660700001)(3280700002)(110136002)(106116001)(189998001)(99286002)(11100500001)(86362001)(122556002)(54356999)(76176999)(2906002)(5008740100001)(93886004)(9686002)(19580395003)(81166005)(19580405001);DIR:OUT;SFP:1102;SCL:1;SRVR:CS1PR84MB0085;H:CS1PR84MB0085.NAMPRD84.PROD.OUTLOOK.COM;FPR:;SPF:None;MLV:sfv;LANG:en; spamdiagnosticoutput: 1:23 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 X-OriginatorOrg: hpe.com X-MS-Exchange-CrossTenant-originalarrivaltime: 29 Apr 2016 13:55:12.2185 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 105b2061-b669-4b31-92ac-24d304d195dc X-MS-Exchange-Transport-CrossTenantHeadersStamped: CS1PR84MB0085 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 04/27/2016 03:19 PM, Rafael J. Wysocki wrote: > On Wednesday, April 27, 2016 04:19:45 PM Dall, Betty wrote: >> On 04/26/2016 02:39 PM, Rafael J. Wysocki wrote: >>> On Wednesday, April 13, 2016 08:48:14 AM Betty Dall wrote: >>>> The ACPI _HRV object on the device is used to supply Linux with >>>> the device's hardware revision. This is an optional object. Add >>>> sysfs support for the _HRV object if it exists on the device. >>>> >>>> Signed-off-by: Betty Dall >>>> --- >>>> drivers/acpi/device_sysfs.c | 24 ++++++++++++++++++++++++ >>>> 1 file changed, 24 insertions(+) >>>> >>>> diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c >>>> index b9afb47..bf12dbe 100644 >>>> --- a/drivers/acpi/device_sysfs.c >>>> +++ b/drivers/acpi/device_sysfs.c >>>> @@ -473,6 +473,21 @@ acpi_device_sun_show(struct device *dev, struct device_attribute *attr, >>>> } >>>> static DEVICE_ATTR(sun, 0444, acpi_device_sun_show, NULL); >>>> >>>> +static ssize_t >>>> +acpi_device_hrv_show(struct device *dev, struct device_attribute *attr, >>>> + char *buf) { >>>> + struct acpi_device *acpi_dev = to_acpi_device(dev); >>>> + acpi_status status; >>>> + unsigned long long hrv; >>>> + >>>> + status = acpi_evaluate_integer(acpi_dev->handle, "_HRV", NULL, &hrv); >>>> + if (ACPI_FAILURE(status)) >>>> + return -ENODEV; >>> >>> Actually, this should be -EIO I think. >>> >>> Thanks, >>> Rafael >> >> Hi Rafael, >> >> I picked -ENODEV because the _SUN and _STA show functions use -ENODEV >> for a return value when the acpi_evaluate_integer() fails. > > Which isn't the best choice. > >> I checked in the sysfs code what the return value is used for and any >> negative value is treated the same, that is, the sysfs code is not >> looking specifically for -EIO. > > But doesn't it pass the return value up the call chain? Yes, it passes the return up the call chain. I see ENODEV returned from the read system call by using strace with a hard coded an error return in my show function. The kernel call chain is: acpi_device_hrv_show+0x1c/0x48 dev_attr_show+0x20/0x58 sysfs_kf_seq_show+0xc0/0x158 kernfs_seq_show+0x28/0x30 seq_read+0x19c/0x418 kernfs_fop_read+0x104/0x198 __vfs_read+0x1c/0xd8 vfs_read+0x78/0x160 SyS_read+0x44/0xa0 >> Do you still want me to change it to -EIO? > > I may, depending on the answer to the question above. I will change it to EIO. ENODEV makes less sense since the "device" exists or there wouldn't be a sysfs file. I can do the same for _SUN and _STA. Thanks, -Betty