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 X-Spam-Level: X-Spam-Status: No, score=-5.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0F28BC00454 for ; Thu, 12 Dec 2019 15:15:07 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id E24BC2067C for ; Thu, 12 Dec 2019 15:15:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728822AbfLLPPD (ORCPT ); Thu, 12 Dec 2019 10:15:03 -0500 Received: from mx2.suse.de ([195.135.220.15]:34634 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1728654AbfLLPPD (ORCPT ); Thu, 12 Dec 2019 10:15:03 -0500 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id A95F9ADDF; Thu, 12 Dec 2019 15:15:00 +0000 (UTC) Subject: Re: [RFC 04/25] spi: gpio: Implement LSB First bitbang support To: Geert Uytterhoeven Cc: linux-realtek-soc@lists.infradead.org, linux-leds@vger.kernel.org, Linux Kernel Mailing List , linux-spi , Mark Brown , Jacek Anaszewski , Pavel Machek , Linux ARM , Dan Murphy References: <20191212033952.5967-1-afaerber@suse.de> <20191212033952.5967-5-afaerber@suse.de> From: =?UTF-8?Q?Andreas_F=c3=a4rber?= Organization: SUSE Software Solutions Germany GmbH Message-ID: <9b4b6287-c1d9-1b41-88a8-7ac9fe222642@suse.de> Date: Thu, 12 Dec 2019 16:14:59 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-leds-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-leds@vger.kernel.org Hi Geert, Am 12.12.19 um 09:40 schrieb Geert Uytterhoeven: > On Thu, Dec 12, 2019 at 4:41 AM Andreas Färber wrote: >> Add support for slave DT property spi-lsb-first, i.e., SPI_LSB_FIRST mode. >> >> Duplicate the inline helpers bitbang_txrx_be_cpha{0,1} as LE versions. >> Make checkpatch.pl happy by changing "unsigned" to "unsigned int". >> >> Conditionally call them from all the spi-gpio txrx_word callbacks. >> >> Signed-off-by: Andreas Färber > > Thanks for your patch! NP. I prefer fixing issues at the source over awkward workarounds. :) >> --- a/drivers/spi/spi-gpio.c >> +++ b/drivers/spi/spi-gpio.c >> @@ -135,25 +135,37 @@ static inline int getmiso(const struct spi_device *spi) >> static u32 spi_gpio_txrx_word_mode0(struct spi_device *spi, >> unsigned nsecs, u32 word, u8 bits, unsigned flags) >> { >> - return bitbang_txrx_be_cpha0(spi, nsecs, 0, flags, word, bits); >> + if (unlikely(spi->mode & SPI_LSB_FIRST)) >> + return bitbang_txrx_le_cpha0(spi, nsecs, 0, flags, word, bits); >> + else >> + return bitbang_txrx_be_cpha0(spi, nsecs, 0, flags, word, bits); >> } > > Duplicating all functions sounds a bit wasteful to me. Two functions duplicated, eight function calls duplicated. > What about reverting the word first, and calling the normal functions? > > if (unlikely(spi->mode & SPI_LSB_FIRST)) { > if (bits <= 8) > word = bitrev8(word) >> (bits - 8); > else if (bits <= 16) > word = bitrev16(word) >> (bits - 16); > else > word = bitrev32(word) >> (bits - 32); > } > return bitbang_txrx_be_cpha0(spi, nsecs, 0, flags, word, bits); Hm, wasn't aware of those helpers, so I opted not to loop over the bits for reversing myself, as Thomas Gleixner disliked bit loops in irqchip. Performance appeared to be a concern here, too. Eight functions duplicated then. I don't like that - at least we should pack that into an inline helper or macro, to not copy&paste even more lines around. Who knows, maybe we'll get 64-bit support at one point? Do you think it would be acceptable to instead encapsulate this inside the _be inline helpers? That would lead the name ad absurdum, of course, but we would then need to do it only twice, not eight times. However, either way would seem to make the LSB code path slower than MSB due to the prepended reversal. Delays are already stubbed out, with open TODOs for further inlining functions that are being dispatched today. So from that angle I don't see a better way than either duplicating the functions or using some macro magic to #include the header twice. If we wanted to go down that path, we could probably de-duplicate the existing two functions, too, but I was trying to err on the cautious side, since I don't have setups to test all four code paths myself (and a ton of more relevant but less fun patches to flush out ;)). Regards, Andreas -- SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer HRB 36809 (AG Nürnberg) 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 X-Spam-Level: X-Spam-Status: No, score=-5.3 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3943AC43603 for ; Thu, 12 Dec 2019 15:15:11 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 0AB042067C for ; Thu, 12 Dec 2019 15:15:10 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="QlbXfKHK" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0AB042067C Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date: Message-ID:From:References:To:Subject:Reply-To:Content-ID:Content-Description :Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=A9CWil3eqjQqoNA3RVPNmJ/ZY+mdD+goRimHHMBrGSM=; b=QlbXfKHK5Te6QU K8lqsvyUaTihuecD/Dww7nHR5xFEPv9krls6iVbzRv/ZC/yLLjID6KDG2Dcb0HsHilMeffHumX+8f 7bPb+Izdy6hhQ7MNZFtTx7Oe6A4U/OrWm5jTLJIYuPhXeNlQPL0diOlQvPo4ZpHsgj4MDz+lIltXR pYaGeQLOoZ+vZoqon+BHO4OGYU+CExu13U542wiZczAscIg5fnyX7lxoXwgeJw1h5GOJTt000qUCZ R2vkjq2p5eLEafYlGYKujG+LS5mD/RMfXduYFLiHY5Ev2lUAZuOwrzolgw0UloaUHg65gib2UIHPG LNm6VhLdt/RJt7/jna0w==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1ifQBG-0005wW-TI; Thu, 12 Dec 2019 15:15:06 +0000 Received: from mx2.suse.de ([195.135.220.15] helo=mx1.suse.de) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1ifQBD-0005L7-1t; Thu, 12 Dec 2019 15:15:04 +0000 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id A95F9ADDF; Thu, 12 Dec 2019 15:15:00 +0000 (UTC) Subject: Re: [RFC 04/25] spi: gpio: Implement LSB First bitbang support To: Geert Uytterhoeven References: <20191212033952.5967-1-afaerber@suse.de> <20191212033952.5967-5-afaerber@suse.de> From: =?UTF-8?Q?Andreas_F=c3=a4rber?= Organization: SUSE Software Solutions Germany GmbH Message-ID: <9b4b6287-c1d9-1b41-88a8-7ac9fe222642@suse.de> Date: Thu, 12 Dec 2019 16:14:59 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.1 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-20191212_071503_392259_5B514BD4 X-CRM114-Status: GOOD ( 17.94 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Linux ARM , linux-realtek-soc@lists.infradead.org, Linux Kernel Mailing List , linux-spi , Mark Brown , Jacek Anaszewski , Pavel Machek , linux-leds@vger.kernel.org, Dan Murphy Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org SGkgR2VlcnQsCgpBbSAxMi4xMi4xOSB1bSAwOTo0MCBzY2hyaWViIEdlZXJ0IFV5dHRlcmhvZXZl bjoKPiBPbiBUaHUsIERlYyAxMiwgMjAxOSBhdCA0OjQxIEFNIEFuZHJlYXMgRsOkcmJlciA8YWZh ZXJiZXJAc3VzZS5kZT4gd3JvdGU6Cj4+IEFkZCBzdXBwb3J0IGZvciBzbGF2ZSBEVCBwcm9wZXJ0 eSBzcGktbHNiLWZpcnN0LCBpLmUuLCBTUElfTFNCX0ZJUlNUIG1vZGUuCj4+Cj4+IER1cGxpY2F0 ZSB0aGUgaW5saW5lIGhlbHBlcnMgYml0YmFuZ190eHJ4X2JlX2NwaGF7MCwxfSBhcyBMRSB2ZXJz aW9ucy4KPj4gTWFrZSBjaGVja3BhdGNoLnBsIGhhcHB5IGJ5IGNoYW5naW5nICJ1bnNpZ25lZCIg dG8gInVuc2lnbmVkIGludCIuCj4+Cj4+IENvbmRpdGlvbmFsbHkgY2FsbCB0aGVtIGZyb20gYWxs IHRoZSBzcGktZ3BpbyB0eHJ4X3dvcmQgY2FsbGJhY2tzLgo+Pgo+PiBTaWduZWQtb2ZmLWJ5OiBB bmRyZWFzIEbDpHJiZXIgPGFmYWVyYmVyQHN1c2UuZGU+Cj4gCj4gVGhhbmtzIGZvciB5b3VyIHBh dGNoIQoKTlAuIEkgcHJlZmVyIGZpeGluZyBpc3N1ZXMgYXQgdGhlIHNvdXJjZSBvdmVyIGF3a3dh cmQgd29ya2Fyb3VuZHMuIDopCgo+PiAtLS0gYS9kcml2ZXJzL3NwaS9zcGktZ3Bpby5jCj4+ICsr KyBiL2RyaXZlcnMvc3BpL3NwaS1ncGlvLmMKPj4gQEAgLTEzNSwyNSArMTM1LDM3IEBAIHN0YXRp YyBpbmxpbmUgaW50IGdldG1pc28oY29uc3Qgc3RydWN0IHNwaV9kZXZpY2UgKnNwaSkKPj4gIHN0 YXRpYyB1MzIgc3BpX2dwaW9fdHhyeF93b3JkX21vZGUwKHN0cnVjdCBzcGlfZGV2aWNlICpzcGks Cj4+ICAgICAgICAgICAgICAgICB1bnNpZ25lZCBuc2VjcywgdTMyIHdvcmQsIHU4IGJpdHMsIHVu c2lnbmVkIGZsYWdzKQo+PiAgewo+PiAtICAgICAgIHJldHVybiBiaXRiYW5nX3R4cnhfYmVfY3Bo YTAoc3BpLCBuc2VjcywgMCwgZmxhZ3MsIHdvcmQsIGJpdHMpOwo+PiArICAgICAgIGlmICh1bmxp a2VseShzcGktPm1vZGUgJiBTUElfTFNCX0ZJUlNUKSkKPj4gKyAgICAgICAgICAgICAgIHJldHVy biBiaXRiYW5nX3R4cnhfbGVfY3BoYTAoc3BpLCBuc2VjcywgMCwgZmxhZ3MsIHdvcmQsIGJpdHMp Owo+PiArICAgICAgIGVsc2UKPj4gKyAgICAgICAgICAgICAgIHJldHVybiBiaXRiYW5nX3R4cnhf YmVfY3BoYTAoc3BpLCBuc2VjcywgMCwgZmxhZ3MsIHdvcmQsIGJpdHMpOwo+PiAgfQo+IAo+IER1 cGxpY2F0aW5nIGFsbCBmdW5jdGlvbnMgc291bmRzIGEgYml0IHdhc3RlZnVsIHRvIG1lLgoKVHdv IGZ1bmN0aW9ucyBkdXBsaWNhdGVkLCBlaWdodCBmdW5jdGlvbiBjYWxscyBkdXBsaWNhdGVkLgoK PiBXaGF0IGFib3V0IHJldmVydGluZyB0aGUgd29yZCBmaXJzdCwgYW5kIGNhbGxpbmcgdGhlIG5v cm1hbCBmdW5jdGlvbnM/Cj4gCj4gICAgIGlmICh1bmxpa2VseShzcGktPm1vZGUgJiBTUElfTFNC X0ZJUlNUKSkgewo+ICAgICAgICAgICAgIGlmIChiaXRzIDw9IDgpCj4gICAgICAgICAgICAgICAg ICAgICB3b3JkID0gYml0cmV2OCh3b3JkKSA+PiAoYml0cyAtIDgpOwo+ICAgICAgICAgICAgIGVs c2UgaWYgKGJpdHMgPD0gMTYpCj4gICAgICAgICAgICAgICAgICAgICB3b3JkID0gYml0cmV2MTYo d29yZCkgPj4gKGJpdHMgLSAxNik7Cj4gICAgICAgICAgICAgZWxzZQo+ICAgICAgICAgICAgICAg ICAgICAgd29yZCA9IGJpdHJldjMyKHdvcmQpID4+IChiaXRzIC0gMzIpOwo+ICAgICB9Cj4gICAg IHJldHVybiBiaXRiYW5nX3R4cnhfYmVfY3BoYTAoc3BpLCBuc2VjcywgMCwgZmxhZ3MsIHdvcmQs IGJpdHMpOwoKSG0sIHdhc24ndCBhd2FyZSBvZiB0aG9zZSBoZWxwZXJzLCBzbyBJIG9wdGVkIG5v dCB0byBsb29wIG92ZXIgdGhlIGJpdHMKZm9yIHJldmVyc2luZyBteXNlbGYsIGFzIFRob21hcyBH bGVpeG5lciBkaXNsaWtlZCBiaXQgbG9vcHMgaW4gaXJxY2hpcC4KUGVyZm9ybWFuY2UgYXBwZWFy ZWQgdG8gYmUgYSBjb25jZXJuIGhlcmUsIHRvby4KCkVpZ2h0IGZ1bmN0aW9ucyBkdXBsaWNhdGVk IHRoZW4uIEkgZG9uJ3QgbGlrZSB0aGF0IC0gYXQgbGVhc3Qgd2Ugc2hvdWxkCnBhY2sgdGhhdCBp bnRvIGFuIGlubGluZSBoZWxwZXIgb3IgbWFjcm8sIHRvIG5vdCBjb3B5JnBhc3RlIGV2ZW4gbW9y ZQpsaW5lcyBhcm91bmQuIFdobyBrbm93cywgbWF5YmUgd2UnbGwgZ2V0IDY0LWJpdCBzdXBwb3J0 IGF0IG9uZSBwb2ludD8KCkRvIHlvdSB0aGluayBpdCB3b3VsZCBiZSBhY2NlcHRhYmxlIHRvIGlu c3RlYWQgZW5jYXBzdWxhdGUgdGhpcyBpbnNpZGUKdGhlIF9iZSBpbmxpbmUgaGVscGVycz8gVGhh dCB3b3VsZCBsZWFkIHRoZSBuYW1lIGFkIGFic3VyZHVtLCBvZiBjb3Vyc2UsCmJ1dCB3ZSB3b3Vs ZCB0aGVuIG5lZWQgdG8gZG8gaXQgb25seSB0d2ljZSwgbm90IGVpZ2h0IHRpbWVzLgoKSG93ZXZl ciwgZWl0aGVyIHdheSB3b3VsZCBzZWVtIHRvIG1ha2UgdGhlIExTQiBjb2RlIHBhdGggc2xvd2Vy IHRoYW4gTVNCCmR1ZSB0byB0aGUgcHJlcGVuZGVkIHJldmVyc2FsLgoKRGVsYXlzIGFyZSBhbHJl YWR5IHN0dWJiZWQgb3V0LCB3aXRoIG9wZW4gVE9ET3MgZm9yIGZ1cnRoZXIgaW5saW5pbmcKZnVu Y3Rpb25zIHRoYXQgYXJlIGJlaW5nIGRpc3BhdGNoZWQgdG9kYXkuCgpTbyBmcm9tIHRoYXQgYW5n bGUgSSBkb24ndCBzZWUgYSBiZXR0ZXIgd2F5IHRoYW4gZWl0aGVyIGR1cGxpY2F0aW5nIHRoZQpm dW5jdGlvbnMgb3IgdXNpbmcgc29tZSBtYWNybyBtYWdpYyB0byAjaW5jbHVkZSB0aGUgaGVhZGVy IHR3aWNlLiBJZiB3ZQp3YW50ZWQgdG8gZ28gZG93biB0aGF0IHBhdGgsIHdlIGNvdWxkIHByb2Jh Ymx5IGRlLWR1cGxpY2F0ZSB0aGUgZXhpc3RpbmcKdHdvIGZ1bmN0aW9ucywgdG9vLCBidXQgSSB3 YXMgdHJ5aW5nIHRvIGVyciBvbiB0aGUgY2F1dGlvdXMgc2lkZSwgc2luY2UKSSBkb24ndCBoYXZl IHNldHVwcyB0byB0ZXN0IGFsbCBmb3VyIGNvZGUgcGF0aHMgbXlzZWxmIChhbmQgYSB0b24gb2YK bW9yZSByZWxldmFudCBidXQgbGVzcyBmdW4gcGF0Y2hlcyB0byBmbHVzaCBvdXQgOykpLgoKUmVn YXJkcywKQW5kcmVhcwoKLS0gClNVU0UgU29mdHdhcmUgU29sdXRpb25zIEdlcm1hbnkgR21iSApN YXhmZWxkc3RyLiA1LCA5MDQwOSBOw7xybmJlcmcsIEdlcm1hbnkKR0Y6IEZlbGl4IEltZW5kw7Zy ZmZlcgpIUkIgMzY4MDkgKEFHIE7DvHJuYmVyZykKCl9fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fCmxpbnV4LWFybS1rZXJuZWwgbWFpbGluZyBsaXN0CmxpbnV4 LWFybS1rZXJuZWxAbGlzdHMuaW5mcmFkZWFkLm9yZwpodHRwOi8vbGlzdHMuaW5mcmFkZWFkLm9y Zy9tYWlsbWFuL2xpc3RpbmZvL2xpbnV4LWFybS1rZXJuZWwK