From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 4DA63C43334 for ; Tue, 21 Jun 2022 04:14:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date: Message-ID:From:References:Cc:To:Subject:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Owner; bh=8pFyW54bpDGyYcW2P/AHcIn/sXmmIdZeb2HfcW47CH0=; b=jCNXThGAVNlmIxCd4JarqlULEm DFgNUA/O9gAqENO58VzvutfOo9ueGAPrFzFemb0nDMNwwClUZ9gT0TJa6tLmzJT9eKKzjJLCRX1rM /em/BeYqMwbl+SWi9liykQzIEnBsuFvGYKL/CJih4KMmNKC9yqn2sLZ9vUuE0qrvJ21tQ5kdbG81i zGDhDtF2LChUZZiE//701jUHziCFjlQV/gTZ6zro+t9ytO/kqpgy+HOrnQ2SN2Hkc23XeSt+rnjI4 5XXMIxi/tvFXEzxOXWJPelpNT+M3VJai6jDfZU0JE1OWxYRSlPelkKo+zFSaQStRk6VFAR7RMz3f2 DEPiLEUA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1o3VG0-003YOY-RA; Tue, 21 Jun 2022 04:12:52 +0000 Received: from wout4-smtp.messagingengine.com ([64.147.123.20]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1o3VFy-003YNr-EG for linux-arm-kernel@lists.infradead.org; Tue, 21 Jun 2022 04:12:51 +0000 Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.west.internal (Postfix) with ESMTP id CB03632004F8; Tue, 21 Jun 2022 00:12:48 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute3.internal (MEProxy); Tue, 21 Jun 2022 00:12:49 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sholland.org; h= cc:cc:content-transfer-encoding:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to; s=fm3; t=1655784768; x= 1655871168; bh=YFC9enACBq9N/iDDvL+ELyNOFTG6bJu/nZay/ScaEg4=; b=C 1M85zd3eSzPbPbrqJLqJf9XGPjCp5qspTW0Pb2G5upYU2zzPTmqd1H/GkSDF4R2h jTVkPgJvlPCbUT6D66cr5MOagfZp8/qJ7gT7m6CH/IPz1QgwP5kmuhn2pcMJ+Bjq wF3m6DwBEx8ZkjB9ljR1S07Y371FLj2Rk4NejXkBpjEyR1qdIpmSmNNV/St6FPaX Axq3ihYYd9b+xHhWwlk4BM9Ze4n+fNrYaOeJYCXxsgDruW8N2Niew1cwoZ7I6oOe ZmRbDc5CSGn+GXqrjbz/U4POJ0yx9yl+FXAEafjH5TgkODpPrvBLIzxVO3B5T9mK mnzQnlTfPmlOth79Drw5A== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:date:date:feedback-id:feedback-id:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t=1655784768; x= 1655871168; bh=YFC9enACBq9N/iDDvL+ELyNOFTG6bJu/nZay/ScaEg4=; b=O eb/c7KAsWiuQnepLqcYmFDJjhF1toXYwtArej8zdAYFOE4m6yXPYTKUgH8DFUAXT CBCYQPUEnqhlY9h1iOeG4lFU8VqoZAYJxlU7Jwkwkangi5r3GhyuXcvc3bhvgydU mEodnq47L0sIIQoNhDYj9+sLqkfddMBdepKvDioqFEr/DwquVHV9bQr55sUgy3YQ d7U/L0jmpEC4IUz0LR/iiWEp+ZahM5L7hGyFEk1uB6mC4kkJPZ0fC4MUanPnu48s TROyoixs26PBdxrFpylGlkzEiwvXMr0HmCLvWfVor5suPMHuhfpg/NFBxmvb0SUu hnuE4vdC8TYCbD8YSqzXA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvfedrudefvddgjeeiucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepuffvvehfhffkffgfgggjtgfgsehtjeertddtfeejnecuhfhrohhmpefurghm uhgvlhcujfholhhlrghnugcuoehsrghmuhgvlhesshhhohhllhgrnhgurdhorhhgqeenuc ggtffrrghtthgvrhhnpefftdevkedvgeekueeutefgteffieelvedukeeuhfehledvhfei tdehudfhudehhfenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfh hrohhmpehsrghmuhgvlhesshhhohhllhgrnhgurdhorhhg X-ME-Proxy: Feedback-ID: i0ad843c9:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 21 Jun 2022 00:12:47 -0400 (EDT) Subject: Re: [PATCH v4 2/4] Input: pinephone-keyboard - Add PinePhone keyboard driver To: Andy Shevchenko Cc: Dmitry Torokhov , linux-input , Linux Kernel Mailing List , Ondrej Jirman , Andy Shevchenko , Chen-Yu Tsai , Jernej Skrabec , linux-arm Mailing List , linux-sunxi@lists.linux.dev References: <20220618165747.55709-1-samuel@sholland.org> <20220618165747.55709-3-samuel@sholland.org> From: Samuel Holland Message-ID: <2e6499b8-d8f4-b63a-689b-1e93aca7c210@sholland.org> Date: Mon, 20 Jun 2022 23:12:46 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220620_211250_566056_D6A04245 X-CRM114-Status: GOOD ( 22.05 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 6/19/22 6:43 AM, Andy Shevchenko wrote: > On Sat, Jun 18, 2022 at 7:10 PM Samuel Holland wrote: >> >> The official Pine64 PinePhone keyboard case contains a matrix keypad and >> a MCU which runs a libre firmware. Add support for its I2C interface. > > ... > >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include > > Missed > types.h > > ... > >> +#define PPKB_ROWS 6 >> +#define PPKB_COLS 12 > > ... > >> + for (col = 0; col < PPKB_COLS; ++col) { >> + u8 old = old_buf[1 + col]; >> + u8 new = new_buf[1 + col]; >> + u8 changed = old ^ new; >> + >> + if (!changed) >> + continue; >> + >> + for (row = 0; row < PPKB_ROWS; ++row) { >> + u8 mask = BIT(row); >> + u8 value = new & mask; >> + unsigned short code; >> + bool fn_state; >> + >> + if (!(changed & mask)) >> + continue; >> + >> + /* >> + * Save off the FN key state when the key was pressed, >> + * and use that to determine the code during a release. >> + */ >> + fn_state = value ? ppkb->fn_pressed : ppkb->fn_state[col] & mask; >> + if (fn_state) >> + ppkb->fn_state[col] ^= mask; > > Can't it be converted to use bitmap APIs? This is a 2D matrix, with one byte per column, and one bit per row. There are only 6 rows, so two bits per byte are unused. Converting this to the bitmap API would unnecessarily complicate things. >> + } >> + } > > ... > >> +static int ppkb_set_scan(struct i2c_client *client, bool enable) >> +{ >> + struct device *dev = &client->dev; >> + int ret, val; >> + >> + ret = i2c_smbus_read_byte_data(client, PPKB_SYS_CONFIG); >> + if (ret < 0) { >> + dev_err(dev, "Failed to read config: %d\n", ret); >> + return ret; >> + } >> + >> + if (enable) >> + val = ret & ~PPKB_SYS_CONFIG_DISABLE_SCAN; >> + else >> + val = ret | PPKB_SYS_CONFIG_DISABLE_SCAN; >> + ret = i2c_smbus_write_byte_data(client, PPKB_SYS_CONFIG, val); >> + if (ret) { >> + dev_err(dev, "Failed to write config: %d\n", ret); > >> + return ret; >> + } >> + >> + return 0; > > return ret; The "return 0" pattern is idiomatic, and more diff-friendly when adding error handling or more operations. But I don't have that strong of an opinion on it. Regards, Samuel _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel