On Thu, 17 Mar 2022 22:53:59 -0500
Samuel Holland <sam...@sholland.org> 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 <sam...@sholland.org>
> ---
> 
> (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,
>  };

Reply via email to