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 EE643C433EF for ; Thu, 31 Mar 2022 23:44:13 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id A65FC84268; Fri, 1 Apr 2022 01:36:44 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: by phobos.denx.de (Postfix, from userid 109) id CAFD4842C7; Fri, 1 Apr 2022 01:35:37 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by phobos.denx.de (Postfix) with ESMTP id C3666842FB for ; Fri, 1 Apr 2022 01:35:07 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=andre.przywara@arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 3DC48139F; Thu, 31 Mar 2022 16:35:07 -0700 (PDT) Received: from slackpad.lan (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id EA82A3F73B; Thu, 31 Mar 2022 16:35:05 -0700 (PDT) Date: Fri, 1 Apr 2022 00:34:23 +0100 From: Andre Przywara To: Samuel Holland Cc: u-boot@lists.denx.de, Jagan Teki , Sean Anderson , Simon Glass , Heinrich Schuchardt , Heiko Schocher , Joe Hershberger Subject: Re: [PATCH v2 02/23] sunxi: pinctrl: Implement pin muxing functions Message-ID: <20220401003423.6c64b5da@slackpad.lan> In-Reply-To: <20220318035420.15058-3-samuel@sholland.org> References: <20220318035420.15058-1-samuel@sholland.org> <20220318035420.15058-3-samuel@sholland.org> Organization: Arm Ltd. X-Mailer: Claws Mail 4.0.0 (GTK+ 3.24.31; x86_64-slackware-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII 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.5 at phobos.denx.de X-Virus-Status: Clean On Thu, 17 Mar 2022 22:53:59 -0500 Samuel Holland wrote: Hi Samuel, > Implement the operations to get pin and function names, and to set the > mux for a pin. The pin count and pin names are calculated as if each > bank has the maximum number of pins. Function names are simply the index > into a list of { function name, mux value } pairs. Thank you very much for this neat and lean solution, I like that. > We assume all pins associated with a function use the same mux value for > that function. This is generally true within a group of pins on a single > port, but generally false when some peripheral can be muxed to multiple > ports. For example, A64 UART3 uses mux 3 on port D, and mux 2 on port H. > But all of the port D pins use the same mux value, and so do all of the > port H pins. This applies even when the pins for some function are not > contiguous, and when the lower-numbered mux values are unused. A good > example of both of these cases is SPI0 on most SoCs. This is only *almost* universally true, however, but the exceptions are not relevant for U-Boot, as it affects some multimedia functions only. One example I could quickly find is CSI on the H6, for instance. > This strategy saves a lot of space (which is especially important for > SPL), but where the mux value for a certain function differs across > ports, it forces us to choose a single port for that function at build > time. Since almost all boards use the default (i.e. reference design) > pin muxes[1], this is unlikely to be a problem. Yes, I can live with that restriction. Should we come to a point where we need non-consistent muxes across different ports, we can always add a "port" member to struct sunxi_pinctrl_function, and encode 0 as "don't care", so we would just need to explicitly add that to the groups that actually differ. Can you add at least a short summary of your commit message (that it is a simplified mapping, and just noting the restrictions) to the code as a comment, just before the struct sunxi_pinctrl_function declaration? I am not sure this nice explanation will be found easily otherwise. Or you copy the whole explanation in, I don't mind. Cheers, Andre > > [1]: See commit dda9fa734f81 ("sunxi: Simplify MMC pinmux selection") > > Signed-off-by: Samuel Holland > --- > > (no changes since v1) > > drivers/pinctrl/sunxi/Kconfig | 1 + > drivers/pinctrl/sunxi/pinctrl-sunxi.c | 227 ++++++++++++++++++++++++++ > 2 files changed, 228 insertions(+) > > diff --git a/drivers/pinctrl/sunxi/Kconfig b/drivers/pinctrl/sunxi/Kconfig > index 96c2f35f3a..f4949f89e0 100644 > --- a/drivers/pinctrl/sunxi/Kconfig > +++ b/drivers/pinctrl/sunxi/Kconfig > @@ -5,6 +5,7 @@ if ARCH_SUNXI > config PINCTRL_SUNXI > select PINCTRL_FULL > select PINCTRL_GENERIC > + select PINMUX > bool > > config PINCTRL_SUNIV_F1C100S > diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c > index 43bb1ec650..6ea8245c8e 100644 > --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c > +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c > @@ -12,7 +12,14 @@ > > extern U_BOOT_DRIVER(gpio_sunxi); > > +struct sunxi_pinctrl_function { > + const char name[sizeof("gpio_out")]; > + u8 mux; > +}; > + > struct sunxi_pinctrl_desc { > + const struct sunxi_pinctrl_function *functions; > + u8 num_functions; > u8 first_bank; > u8 num_banks; > }; > @@ -21,7 +28,66 @@ struct sunxi_pinctrl_plat { > struct sunxi_gpio __iomem *base; > }; > > +static int sunxi_pinctrl_get_pins_count(struct udevice *dev) > +{ > + const struct sunxi_pinctrl_desc *desc = dev_get_priv(dev); > + > + return desc->num_banks * SUNXI_GPIOS_PER_BANK; > +} > + > +static const char *sunxi_pinctrl_get_pin_name(struct udevice *dev, > + uint pin_selector) > +{ > + const struct sunxi_pinctrl_desc *desc = dev_get_priv(dev); > + static char pin_name[sizeof("PN31")]; > + > + snprintf(pin_name, sizeof(pin_name), "P%c%d", > + pin_selector / SUNXI_GPIOS_PER_BANK + desc->first_bank + 'A', > + pin_selector % SUNXI_GPIOS_PER_BANK); > + > + return pin_name; > +} > + > +static int sunxi_pinctrl_get_functions_count(struct udevice *dev) > +{ > + const struct sunxi_pinctrl_desc *desc = dev_get_priv(dev); > + > + return desc->num_functions; > +} > + > +static const char *sunxi_pinctrl_get_function_name(struct udevice *dev, > + uint func_selector) > +{ > + const struct sunxi_pinctrl_desc *desc = dev_get_priv(dev); > + > + return desc->functions[func_selector].name; > +} > + > +static int sunxi_pinctrl_pinmux_set(struct udevice *dev, uint pin_selector, > + uint func_selector) > +{ > + const struct sunxi_pinctrl_desc *desc = dev_get_priv(dev); > + struct sunxi_pinctrl_plat *plat = dev_get_plat(dev); > + int bank = pin_selector / SUNXI_GPIOS_PER_BANK; > + int pin = pin_selector % SUNXI_GPIOS_PER_BANK; > + > + debug("set mux: %-4s => %s (%d)\n", > + sunxi_pinctrl_get_pin_name(dev, pin_selector), > + sunxi_pinctrl_get_function_name(dev, func_selector), > + desc->functions[func_selector].mux); > + > + sunxi_gpio_set_cfgbank(plat->base + bank, pin, > + desc->functions[func_selector].mux); > + > + return 0; > +} > + > static const struct pinctrl_ops sunxi_pinctrl_ops = { > + .get_pins_count = sunxi_pinctrl_get_pins_count, > + .get_pin_name = sunxi_pinctrl_get_pin_name, > + .get_functions_count = sunxi_pinctrl_get_functions_count, > + .get_function_name = sunxi_pinctrl_get_function_name, > + .pinmux_set = sunxi_pinctrl_pinmux_set, > .set_state = pinctrl_generic_set_state, > }; > > @@ -76,117 +142,278 @@ static int sunxi_pinctrl_probe(struct udevice *dev) > return 0; > } > > +static const struct sunxi_pinctrl_function suniv_f1c100s_pinctrl_functions[] = { > + { "gpio_in", 0 }, > + { "gpio_out", 1 }, > +}; > + > static const struct sunxi_pinctrl_desc __maybe_unused suniv_f1c100s_pinctrl_desc = { > + .functions = suniv_f1c100s_pinctrl_functions, > + .num_functions = ARRAY_SIZE(suniv_f1c100s_pinctrl_functions), > .first_bank = SUNXI_GPIO_A, > .num_banks = 6, > }; > > +static const struct sunxi_pinctrl_function sun4i_a10_pinctrl_functions[] = { > + { "gpio_in", 0 }, > + { "gpio_out", 1 }, > +}; > + > static const struct sunxi_pinctrl_desc __maybe_unused sun4i_a10_pinctrl_desc = { > + .functions = sun4i_a10_pinctrl_functions, > + .num_functions = ARRAY_SIZE(sun4i_a10_pinctrl_functions), > .first_bank = SUNXI_GPIO_A, > .num_banks = 9, > }; > > +static const struct sunxi_pinctrl_function sun5i_a13_pinctrl_functions[] = { > + { "gpio_in", 0 }, > + { "gpio_out", 1 }, > +}; > + > static const struct sunxi_pinctrl_desc __maybe_unused sun5i_a13_pinctrl_desc = { > + .functions = sun5i_a13_pinctrl_functions, > + .num_functions = ARRAY_SIZE(sun5i_a13_pinctrl_functions), > .first_bank = SUNXI_GPIO_A, > .num_banks = 7, > }; > > +static const struct sunxi_pinctrl_function sun6i_a31_pinctrl_functions[] = { > + { "gpio_in", 0 }, > + { "gpio_out", 1 }, > +}; > + > static const struct sunxi_pinctrl_desc __maybe_unused sun6i_a31_pinctrl_desc = { > + .functions = sun6i_a31_pinctrl_functions, > + .num_functions = ARRAY_SIZE(sun6i_a31_pinctrl_functions), > .first_bank = SUNXI_GPIO_A, > .num_banks = 8, > }; > > +static const struct sunxi_pinctrl_function sun6i_a31_r_pinctrl_functions[] = { > + { "gpio_in", 0 }, > + { "gpio_out", 1 }, > +}; > + > static const struct sunxi_pinctrl_desc __maybe_unused sun6i_a31_r_pinctrl_desc = { > + .functions = sun6i_a31_r_pinctrl_functions, > + .num_functions = ARRAY_SIZE(sun6i_a31_r_pinctrl_functions), > .first_bank = SUNXI_GPIO_L, > .num_banks = 2, > }; > > +static const struct sunxi_pinctrl_function sun7i_a20_pinctrl_functions[] = { > + { "gpio_in", 0 }, > + { "gpio_out", 1 }, > +}; > + > static const struct sunxi_pinctrl_desc __maybe_unused sun7i_a20_pinctrl_desc = { > + .functions = sun7i_a20_pinctrl_functions, > + .num_functions = ARRAY_SIZE(sun7i_a20_pinctrl_functions), > .first_bank = SUNXI_GPIO_A, > .num_banks = 9, > }; > > +static const struct sunxi_pinctrl_function sun8i_a23_pinctrl_functions[] = { > + { "gpio_in", 0 }, > + { "gpio_out", 1 }, > +}; > + > static const struct sunxi_pinctrl_desc __maybe_unused sun8i_a23_pinctrl_desc = { > + .functions = sun8i_a23_pinctrl_functions, > + .num_functions = ARRAY_SIZE(sun8i_a23_pinctrl_functions), > .first_bank = SUNXI_GPIO_A, > .num_banks = 8, > }; > > +static const struct sunxi_pinctrl_function sun8i_a23_r_pinctrl_functions[] = { > + { "gpio_in", 0 }, > + { "gpio_out", 1 }, > +}; > + > static const struct sunxi_pinctrl_desc __maybe_unused sun8i_a23_r_pinctrl_desc = { > + .functions = sun8i_a23_r_pinctrl_functions, > + .num_functions = ARRAY_SIZE(sun8i_a23_r_pinctrl_functions), > .first_bank = SUNXI_GPIO_L, > .num_banks = 1, > }; > > +static const struct sunxi_pinctrl_function sun8i_a33_pinctrl_functions[] = { > + { "gpio_in", 0 }, > + { "gpio_out", 1 }, > +}; > + > static const struct sunxi_pinctrl_desc __maybe_unused sun8i_a33_pinctrl_desc = { > + .functions = sun8i_a33_pinctrl_functions, > + .num_functions = ARRAY_SIZE(sun8i_a33_pinctrl_functions), > .first_bank = SUNXI_GPIO_A, > .num_banks = 8, > }; > > +static const struct sunxi_pinctrl_function sun8i_a83t_pinctrl_functions[] = { > + { "gpio_in", 0 }, > + { "gpio_out", 1 }, > +}; > + > static const struct sunxi_pinctrl_desc __maybe_unused sun8i_a83t_pinctrl_desc = { > + .functions = sun8i_a83t_pinctrl_functions, > + .num_functions = ARRAY_SIZE(sun8i_a83t_pinctrl_functions), > .first_bank = SUNXI_GPIO_A, > .num_banks = 8, > }; > > +static const struct sunxi_pinctrl_function sun8i_a83t_r_pinctrl_functions[] = { > + { "gpio_in", 0 }, > + { "gpio_out", 1 }, > +}; > + > static const struct sunxi_pinctrl_desc __maybe_unused sun8i_a83t_r_pinctrl_desc = { > + .functions = sun8i_a83t_r_pinctrl_functions, > + .num_functions = ARRAY_SIZE(sun8i_a83t_r_pinctrl_functions), > .first_bank = SUNXI_GPIO_L, > .num_banks = 1, > }; > > +static const struct sunxi_pinctrl_function sun8i_h3_pinctrl_functions[] = { > + { "gpio_in", 0 }, > + { "gpio_out", 1 }, > +}; > + > static const struct sunxi_pinctrl_desc __maybe_unused sun8i_h3_pinctrl_desc = { > + .functions = sun8i_h3_pinctrl_functions, > + .num_functions = ARRAY_SIZE(sun8i_h3_pinctrl_functions), > .first_bank = SUNXI_GPIO_A, > .num_banks = 7, > }; > > +static const struct sunxi_pinctrl_function sun8i_h3_r_pinctrl_functions[] = { > + { "gpio_in", 0 }, > + { "gpio_out", 1 }, > +}; > + > static const struct sunxi_pinctrl_desc __maybe_unused sun8i_h3_r_pinctrl_desc = { > + .functions = sun8i_h3_r_pinctrl_functions, > + .num_functions = ARRAY_SIZE(sun8i_h3_r_pinctrl_functions), > .first_bank = SUNXI_GPIO_L, > .num_banks = 1, > }; > > +static const struct sunxi_pinctrl_function sun8i_v3s_pinctrl_functions[] = { > + { "gpio_in", 0 }, > + { "gpio_out", 1 }, > +}; > + > static const struct sunxi_pinctrl_desc __maybe_unused sun8i_v3s_pinctrl_desc = { > + .functions = sun8i_v3s_pinctrl_functions, > + .num_functions = ARRAY_SIZE(sun8i_v3s_pinctrl_functions), > .first_bank = SUNXI_GPIO_A, > .num_banks = 7, > }; > > +static const struct sunxi_pinctrl_function sun9i_a80_pinctrl_functions[] = { > + { "gpio_in", 0 }, > + { "gpio_out", 1 }, > +}; > + > static const struct sunxi_pinctrl_desc __maybe_unused sun9i_a80_pinctrl_desc = { > + .functions = sun9i_a80_pinctrl_functions, > + .num_functions = ARRAY_SIZE(sun9i_a80_pinctrl_functions), > .first_bank = SUNXI_GPIO_A, > .num_banks = 8, > }; > > +static const struct sunxi_pinctrl_function sun9i_a80_r_pinctrl_functions[] = { > + { "gpio_in", 0 }, > + { "gpio_out", 1 }, > +}; > + > static const struct sunxi_pinctrl_desc __maybe_unused sun9i_a80_r_pinctrl_desc = { > + .functions = sun9i_a80_r_pinctrl_functions, > + .num_functions = ARRAY_SIZE(sun9i_a80_r_pinctrl_functions), > .first_bank = SUNXI_GPIO_L, > .num_banks = 3, > }; > > +static const struct sunxi_pinctrl_function sun50i_a64_pinctrl_functions[] = { > + { "gpio_in", 0 }, > + { "gpio_out", 1 }, > +}; > + > static const struct sunxi_pinctrl_desc __maybe_unused sun50i_a64_pinctrl_desc = { > + .functions = sun50i_a64_pinctrl_functions, > + .num_functions = ARRAY_SIZE(sun50i_a64_pinctrl_functions), > .first_bank = SUNXI_GPIO_A, > .num_banks = 8, > }; > > +static const struct sunxi_pinctrl_function sun50i_a64_r_pinctrl_functions[] = { > + { "gpio_in", 0 }, > + { "gpio_out", 1 }, > +}; > + > static const struct sunxi_pinctrl_desc __maybe_unused sun50i_a64_r_pinctrl_desc = { > + .functions = sun50i_a64_r_pinctrl_functions, > + .num_functions = ARRAY_SIZE(sun50i_a64_r_pinctrl_functions), > .first_bank = SUNXI_GPIO_L, > .num_banks = 1, > }; > > +static const struct sunxi_pinctrl_function sun50i_h5_pinctrl_functions[] = { > + { "gpio_in", 0 }, > + { "gpio_out", 1 }, > +}; > + > static const struct sunxi_pinctrl_desc __maybe_unused sun50i_h5_pinctrl_desc = { > + .functions = sun50i_h5_pinctrl_functions, > + .num_functions = ARRAY_SIZE(sun50i_h5_pinctrl_functions), > .first_bank = SUNXI_GPIO_A, > .num_banks = 7, > }; > > +static const struct sunxi_pinctrl_function sun50i_h6_pinctrl_functions[] = { > + { "gpio_in", 0 }, > + { "gpio_out", 1 }, > +}; > + > static const struct sunxi_pinctrl_desc __maybe_unused sun50i_h6_pinctrl_desc = { > + .functions = sun50i_h6_pinctrl_functions, > + .num_functions = ARRAY_SIZE(sun50i_h6_pinctrl_functions), > .first_bank = SUNXI_GPIO_A, > .num_banks = 8, > }; > > +static const struct sunxi_pinctrl_function sun50i_h6_r_pinctrl_functions[] = { > + { "gpio_in", 0 }, > + { "gpio_out", 1 }, > +}; > + > static const struct sunxi_pinctrl_desc __maybe_unused sun50i_h6_r_pinctrl_desc = { > + .functions = sun50i_h6_r_pinctrl_functions, > + .num_functions = ARRAY_SIZE(sun50i_h6_r_pinctrl_functions), > .first_bank = SUNXI_GPIO_L, > .num_banks = 2, > }; > > +static const struct sunxi_pinctrl_function sun50i_h616_pinctrl_functions[] = { > + { "gpio_in", 0 }, > + { "gpio_out", 1 }, > +}; > + > static const struct sunxi_pinctrl_desc __maybe_unused sun50i_h616_pinctrl_desc = { > + .functions = sun50i_h616_pinctrl_functions, > + .num_functions = ARRAY_SIZE(sun50i_h616_pinctrl_functions), > .first_bank = SUNXI_GPIO_A, > .num_banks = 9, > }; > > +static const struct sunxi_pinctrl_function sun50i_h616_r_pinctrl_functions[] = { > + { "gpio_in", 0 }, > + { "gpio_out", 1 }, > +}; > + > static const struct sunxi_pinctrl_desc __maybe_unused sun50i_h616_r_pinctrl_desc = { > + .functions = sun50i_h616_r_pinctrl_functions, > + .num_functions = ARRAY_SIZE(sun50i_h616_r_pinctrl_functions), > .first_bank = SUNXI_GPIO_L, > .num_banks = 1, > };