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 phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id DD69BC4332F for ; Thu, 15 Dec 2022 05:59:42 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id CDC8685497; Thu, 15 Dec 2022 06:59:40 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=sholland.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=sholland.org header.i=@sholland.org header.b="uv1wvkxu"; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=messagingengine.com header.i=@messagingengine.com header.b="nYCSDNna"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 26B7785489; Thu, 15 Dec 2022 06:59:39 +0100 (CET) Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 14EE98549C for ; Thu, 15 Dec 2022 06:59:34 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=sholland.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=samuel@sholland.org Received: from compute6.internal (compute6.nyi.internal [10.202.2.47]) by mailout.nyi.internal (Postfix) with ESMTP id E53B05C0095; Thu, 15 Dec 2022 00:59:31 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute6.internal (MEProxy); Thu, 15 Dec 2022 00:59:31 -0500 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=1671083971; x= 1671170371; bh=lfxHbHBpLQSrsSArplArXeaiUDoXMi88m8bsseBQrnQ=; b=u v1wvkxu0GylosuGNq1DlIFEcWCvIjcGkjDNrK1b9SqOa3GA1IgFaNIQYgk2L8CYh 6sYgCJ6JNfvlXqM1tf2nvEI+VkA6s7cneVjfhdBJutGCpsRigZzRyFWe/mdjTK+t lQ9WEPX+Jtq72fuQlpP6V77HEk5RqvCqD5HwS4RQLG+dGSLALqeeXk2j4RP1uvZB mtes8Qn4Ea6ot4if3wYGfqV3GZVdBJFbGwbRwSbyiqdT7ums9o86kGdTvG+FT9Vb 2PeeMYh6UyDum2khEaQ5/WiAUmja1R2kWlE1vFbjhsx8aB+CV6sHZ70i2Vl5B16n XngURpomDVpP4Bj71svkg== 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=1671083971; x= 1671170371; bh=lfxHbHBpLQSrsSArplArXeaiUDoXMi88m8bsseBQrnQ=; b=n YCSDNna4wlj83/nns+uu7bXmEHdsYiQrKYEuXATRPWRVVOyQZzS00dfCQtgiuZQ4 Lg63OreSpO2FDtSH5t4qjX/GwRhTFyfXHOWYUILo3XArxowgnjcwC/d+TcN4334B 4SY8DOR/ySjqpphMSeUJsCi9JCLDCin0XiiSi5ZRRmHE4aO3KcLpefqlLV9vIjwA jCwH+azSsTzCQDMfL8nXtCuwGAP01UA+kF7GgDSTrmtuquNHtvuoRNPljdwdGq40 ezGNe9YrD80tpTNgOPYEEMCf8YqIrBWs38rFnTxLkDwUYGXtazMOtraJzLLBJn+Y pp0dp9qKj/vq/yDP34y9w== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvhedrfeeggdekjecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefkffggfgfvvehfhffujggtgfesthejredttdefjeenucfhrhhomhepufgrmhhu vghlucfjohhllhgrnhguuceoshgrmhhuvghlsehshhholhhlrghnugdrohhrgheqnecugg ftrfgrthhtvghrnhepjefgfffhudejfedtuedugeeutdetgfeiteffffehjeeugfeuvdeh jeetfedtffdtnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrh homhepshgrmhhuvghlsehshhholhhlrghnugdrohhrgh X-ME-Proxy: Feedback-ID: i0ad843c9:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 15 Dec 2022 00:59:31 -0500 (EST) Message-ID: <048c7ce0-23b7-74ab-0ab6-c9720ba21160@sholland.org> Date: Wed, 14 Dec 2022 23:59:30 -0600 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux ppc64le; rv:102.0) Gecko/20100101 Thunderbird/102.3.3 Content-Language: en-US To: Andre Przywara , Jagan Teki Cc: u-boot@lists.denx.de, Icenowy Zheng , Jernej Skrabec References: <20221206004549.29015-1-andre.przywara@arm.com> <20221206004549.29015-5-andre.przywara@arm.com> From: Samuel Holland Subject: Re: [RFC PATCH 04/17] pinctrl: sunxi: add GPIO in/out wrappers In-Reply-To: <20221206004549.29015-5-andre.przywara@arm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean On 12/5/22 18:45, Andre Przywara wrote: > So far we were open-coding the pincontroller's GPIO output/input access > in each function using that. > > Provide two functions that wrap that nicely, so users don't need to know > about the internals, and we can abstract the new D1 pinctrl more easily. > > Signed-off-by: Andre Przywara > --- > arch/arm/include/asm/arch-sunxi/gpio.h | 2 ++ > arch/arm/mach-sunxi/pinmux.c | 10 ++++++++++ > drivers/gpio/sunxi_gpio.c | 26 +++++--------------------- > 3 files changed, 17 insertions(+), 21 deletions(-) > > diff --git a/arch/arm/include/asm/arch-sunxi/gpio.h b/arch/arm/include/asm/arch-sunxi/gpio.h > index 8333810a69f..42ca03d8c18 100644 > --- a/arch/arm/include/asm/arch-sunxi/gpio.h > +++ b/arch/arm/include/asm/arch-sunxi/gpio.h > @@ -211,6 +211,8 @@ void sunxi_gpio_set_cfgbank(void *bank_base, int pin_offset, u32 val); > void sunxi_gpio_set_cfgpin(u32 pin, u32 val); > int sunxi_gpio_get_cfgbank(void *bank_base, int pin_offset); > int sunxi_gpio_get_cfgpin(u32 pin); > +void sunxi_gpio_set_output_bank(void *bank_base, u32 clear_mask, u32 set_mask); > +u32 sunxi_gpio_get_output_bank(void *bank_base); > void sunxi_gpio_set_drv(u32 pin, u32 val); > void sunxi_gpio_set_drv_bank(void *bank_base, u32 pin_offset, u32 val); > void sunxi_gpio_set_pull(u32 pin, u32 val); > diff --git a/arch/arm/mach-sunxi/pinmux.c b/arch/arm/mach-sunxi/pinmux.c > index b650f6b1aea..91acbf9269f 100644 > --- a/arch/arm/mach-sunxi/pinmux.c > +++ b/arch/arm/mach-sunxi/pinmux.c > @@ -46,6 +46,16 @@ int sunxi_gpio_get_cfgpin(u32 pin) > return sunxi_gpio_get_cfgbank(bank_base, pin % 32); > } > > +void sunxi_gpio_set_output_bank(void *bank_base, u32 clear_mask, u32 set_mask) > +{ > + clrsetbits_le32(bank_base + GPIO_DAT_REG_OFFSET, clear_mask, set_mask); > +} > + > +u32 sunxi_gpio_get_output_bank(void *bank_base) > +{ > + return readl(bank_base + GPIO_DAT_REG_OFFSET); > +} > + > void sunxi_gpio_set_drv(u32 pin, u32 val) > { > u32 bank = GPIO_BANK(pin); > diff --git a/drivers/gpio/sunxi_gpio.c b/drivers/gpio/sunxi_gpio.c > index 1bf691a204a..767996c10fc 100644 > --- a/drivers/gpio/sunxi_gpio.c > +++ b/drivers/gpio/sunxi_gpio.c > @@ -21,33 +21,22 @@ > #if !CONFIG_IS_ENABLED(DM_GPIO) > static int sunxi_gpio_output(u32 pin, u32 val) > { > - u32 dat; > u32 bank = GPIO_BANK(pin); > u32 num = GPIO_NUM(pin); > void *pio = BANK_TO_GPIO(bank); > > - dat = readl(pio + 0x10); > - if (val) > - dat |= 0x1 << num; > - else > - dat &= ~(0x1 << num); > - > - writel(dat, pio + 0x10); > - > + sunxi_gpio_set_output_bank(pio, val ? 0 : 1U << num, > + val ? 1U << num : 0); > return 0; > } > > static int sunxi_gpio_input(u32 pin) > { > - u32 dat; > u32 bank = GPIO_BANK(pin); > u32 num = GPIO_NUM(pin); > void *pio = BANK_TO_GPIO(bank); > > - dat = readl(pio + 0x10); > - dat >>= num; > - > - return dat & 0x1; > + return (sunxi_gpio_get_output_bank(pio) >> num) & 0x1; > } I would suggest putting this change before patch 3. And I would suggest following the existing pattern of functions, with an inner one taking (bank pointer, pin offset, value), and a wrapper calling BANK_TO_GPIO. This would consolidate the shifting/masking as well. If you move these two functions to pinmux.c, then all of the BANK_TO_GPIO callers are in that file, and you can move BANK_TO_GPIO to pinmux.c as well when you remove the struct and touch all of the call sites anyway. Regards, Samuel > > int gpio_request(unsigned gpio, const char *label) > @@ -136,12 +125,8 @@ static int sunxi_gpio_get_value(struct udevice *dev, unsigned offset) > { > struct sunxi_gpio_plat *plat = dev_get_plat(dev); > u32 num = GPIO_NUM(offset); > - unsigned dat; > - > - dat = readl(plat->regs + GPIO_DAT_REG_OFFSET); > - dat >>= num; > > - return dat & 0x1; > + return (sunxi_gpio_get_output_bank(plat->regs) >> num) & 0x1; > } > > static int sunxi_gpio_get_function(struct udevice *dev, unsigned offset) > @@ -181,8 +166,7 @@ static int sunxi_gpio_set_flags(struct udevice *dev, unsigned int offset, > u32 value = !!(flags & GPIOD_IS_OUT_ACTIVE); > u32 num = GPIO_NUM(offset); > > - clrsetbits_le32(plat->regs + GPIO_DAT_REG_OFFSET, > - 1 << num, value << num); > + sunxi_gpio_set_output_bank(plat->regs, 1U << num, value << num); > sunxi_gpio_set_cfgbank(plat->regs, offset, SUNXI_GPIO_OUTPUT); > } else if (flags & GPIOD_IS_IN) { > u32 pull = 0;