From patchwork Wed Jan 31 16:46:58 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "Limonciello, Mario" X-Patchwork-Id: 10194221 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id E9D7C603EE for ; Wed, 31 Jan 2018 16:48:06 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id BF2BF28488 for ; Wed, 31 Jan 2018 16:48:06 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B3624286CC; Wed, 31 Jan 2018 16:48:06 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5BD2628488 for ; Wed, 31 Jan 2018 16:48:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753995AbeAaQrW (ORCPT ); Wed, 31 Jan 2018 11:47:22 -0500 Received: from esa4.dell-outbound.iphmx.com ([68.232.149.214]:34726 "EHLO esa4.dell-outbound.iphmx.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753782AbeAaQrU (ORCPT ); Wed, 31 Jan 2018 11:47:20 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=dell.com; i=@dell.com; q=dns/txt; s=smtpout; t=1517417240; x=1548953240; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=9geE4Vn5Y6Vp7j+iQewyCegK1A8vf5fdJghhYtPNx44=; b=C2dKPpRjUF0Sr+5UtHZYGCiwJCM6K7+x4V0oNiEfmY+P4VbLfdVmt1FL EMMif7qVR+hU18giibLV/2njgs0YS9qaRCn6mNZGRp/dJLjUI9WGb+fiY LNJzCxcwl9DEk2HLxMHcsH5Oo9DSXPjxz1/6uDVME/Whj4VJ6B10V/CFg 4=; X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: =?us-ascii?q?A2HvAAC28nFahz+a6ERcGQEBAQEBAQEBA?= =?us-ascii?q?QEBAQcBAQEBAYMVgQOBBSgKg1aYT4ICggGHEZBLCoU7AhqCNVgUAQEBAQEBAQE?= =?us-ascii?q?CAQIQAQEBCgsJCCgvgjgigkoBAQEEIxFFDAQCAQgRBAEBAQICIwMCAgIfJQEIC?= =?us-ascii?q?AIEAQ0FCIoVAxWnIIInhzgNgx0BAQEBAQEBAQEBAQEBAQEBAQEBAQEdgQ+DTII?= =?us-ascii?q?Vgz+DLoJrRASBdhAGHYJdgmUFkkORHT6QZoR9lDaOLok7gTw2gXNwgnyCVRAMG?= =?us-ascii?q?XEBAnl4iw+BFwEBAQ?= X-IPAS-Result: =?us-ascii?q?A2HvAAC28nFahz+a6ERcGQEBAQEBAQEBAQEBAQcBAQEBAYM?= =?us-ascii?q?VgQOBBSgKg1aYT4ICggGHEZBLCoU7AhqCNVgUAQEBAQEBAQECAQIQAQEBCgsJC?= =?us-ascii?q?CgvgjgigkoBAQEEIxFFDAQCAQgRBAEBAQICIwMCAgIfJQEICAIEAQ0FCIoVAxW?= =?us-ascii?q?nIIInhzgNgx0BAQEBAQEBAQEBAQEBAQEBAQEBAQEdgQ+DTIIVgz+DLoJrRASBd?= =?us-ascii?q?hAGHYJdgmUFkkORHT6QZoR9lDaOLok7gTw2gXNwgnyCVRAMGXEBAnl4iw+BFwE?= =?us-ascii?q?BAQ?= Received: from esa3.dell-outbound2.iphmx.com ([68.232.154.63]) by esa4.dell-outbound.iphmx.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 31 Jan 2018 10:47:05 -0600 From: Received: from ausxipps301.us.dell.com ([143.166.148.223]) by esa3.dell-outbound2.iphmx.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 31 Jan 2018 22:43:43 +0600 X-LoopCount0: from 10.166.132.169 X-IronPort-AV: E=Sophos;i="5.46,440,1511848800"; d="scan'208";a="122627404" X-DLP: DLP_GlobalPCIDSS To: , CC: , , Subject: RE: [PATCH] platform/x86: dell-laptop: Allocate buffer on heap rather than globally Thread-Topic: [PATCH] platform/x86: dell-laptop: Allocate buffer on heap rather than globally Thread-Index: AQHTme5ImxMrC69/skeqoMXZgynY36ONHQsAgAD5pgCAABshQA== Date: Wed, 31 Jan 2018 16:46:58 +0000 Message-ID: References: <1517331540-13266-1-git-send-email-mario.limonciello@dell.com> <20180130171745.i5wwcwr4qdbcoxhm@pali> <20180131090758.v6eyj35gymlfyzhk@pali> In-Reply-To: <20180131090758.v6eyj35gymlfyzhk@pali> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.143.242.75] MIME-Version: 1.0 Sender: platform-driver-x86-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: platform-driver-x86@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP > -----Original Message----- > From: Pali Rohár [mailto:pali.rohar@gmail.com] > Sent: Wednesday, January 31, 2018 3:08 AM > To: Andy Shevchenko > Cc: Limonciello, Mario ; Darren Hart > ; LKML ; Platform Driver > > Subject: Re: [PATCH] platform/x86: dell-laptop: Allocate buffer on heap rather than > globally > > On Tuesday 30 January 2018 20:14:26 Andy Shevchenko wrote: > > >> -static void dell_set_arguments(u32 arg0, u32 arg1, u32 arg2, u32 arg3) > > >> +static void dell_set_arguments(struct calling_interface_buffer *buffer, > > >> + u32 arg0, u32 arg1, u32 arg2, u32 arg3) > > > > > > Hm... this function has too many parameters :-( > > > > > > What about following API? > > > > > > static struct calling_interface_buffer dell_set_arguments(u32 arg0, u32 arg1, > u32 arg2, u32 arg3); > > > > > > It would return filled structure (not a pointer !) > > > > I do not think it's a good idea. Either we allocate request on a heap > > and return a pointer, or we fill the struct with some data on spot. > > > > To naming: > > > > for allocation: ..._alloc_request() > > for filling: _fill_request() / _prepare_request() > > > > or alike. > > > > _set_arguments() not good enough to me. > > Ok. Then we need to stick with 5 arguments... What about name > dell_fill_request()? E.g. > > struct calling_interface_buffer buffer; > dell_fill_request(&buffer, 0x2, 0, 0, 0); > ret = dell_send_request(&buffer, CLASS_INFO, SELECT_RFKILL); > The other alternative is to just define the input of the structure immediately with an initializer, no multi argument filler function. Like this: diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c index ab58373..e7a971b 100644 --- a/drivers/platform/x86/dell-laptop.c +++ b/drivers/platform/x86/dell-laptop.c @@ -469,13 +469,15 @@ static int dell_rfkill_set(void *data, bool blocked) int disable = blocked ? 1 : 0; unsigned long radio = (unsigned long)data; int hwswitch_bit = (unsigned long)data - 1; - struct calling_interface_buffer buffer; + struct calling_interface_buffer buffer = {CLASS_INFO, + SELECT_RFKILL, + {0, 0, 0, 0}, + {0, 0, 0, 0}}; int hwswitch; int status; int ret; - dell_set_arguments(&buffer, 0, 0, 0, 0); - ret = dell_send_request(&buffer, CLASS_INFO, SELECT_RFKILL); + ret = dell_send_request(&buffer); if (ret) return ret; status = buffer.output[1];