Re: [PATCH v2 05/13] pinctrl: sunxi: Prepare for alternative bias voltage setting methods

Maxime Ripard
2019-04-09T08:44:05Z

[-- Attachment #1: Type: text/plain, Size: 3719 bytes --]

Hi,

On Tue, Apr 09, 2019 at 02:24:44AM +0200, megous@megous.com wrote:
> From: Ondrej Jirman <megous@megous.com>
>
> H6 has a different I/O voltage bias setting method than A80. Prepare
> existing code for using alternative bias voltage setting methods.
>
> Signed-off-by: Ondrej Jirman <megous@megous.com>
> ---
> drivers/pinctrl/sunxi/pinctrl-sun9i-a80.c | 2 +-
> drivers/pinctrl/sunxi/pinctrl-sunxi.c | 41 +++++++++++++----------
> drivers/pinctrl/sunxi/pinctrl-sunxi.h | 5 ++-
> 3 files changed, 28 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sun9i-a80.c b/drivers/pinctrl/sunxi/pinctrl-sun9i-a80.c
> index da37d594a13d..3aa210079b18 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sun9i-a80.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sun9i-a80.c
> @@ -722,7 +722,7 @@ static const struct sunxi_pinctrl_desc sun9i_a80_pinctrl_data = {
> .npins = ARRAY_SIZE(sun9i_a80_pins),
> .irq_banks = 5,
> .disable_strict_mode = true,
> - .has_io_bias_cfg = true,
> + .io_bias_cfg_variant = IO_BIAS_CFG_V1,
> };
>
> static int sun9i_a80_pinctrl_probe(struct platform_device *pdev)
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> index 8dd25caea2cf..b8dd58ef33b7 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
> @@ -610,7 +610,7 @@ static int sunxi_pinctrl_set_io_bias_cfg(struct sunxi_pinctrl *pctl,
> u32 val, reg;
> int uV;
>
> - if (!pctl->desc->has_io_bias_cfg)
> + if (!pctl->desc->io_bias_cfg_variant)
> return 0;
>
> uV = regulator_get_voltage(supply);
> @@ -621,23 +621,28 @@ static int sunxi_pinctrl_set_io_bias_cfg(struct sunxi_pinctrl *pctl,
> if (uV == 0)
> return 0;
>
> - /* Configured value must be equal or greater to actual voltage */
> - if (uV <= 1800000)
> - val = 0x0; /* 1.8V */
> - else if (uV <= 2500000)
> - val = 0x6; /* 2.5V */
> - else if (uV <= 2800000)
> - val = 0x9; /* 2.8V */
> - else if (uV <= 3000000)
> - val = 0xA; /* 3.0V */
> - else
> - val = 0xD; /* 3.3V */
> -
> - pin -= pctl->desc->pin_base;
> -
> - reg = readl(pctl->membase + sunxi_grp_config_reg(pin));
> - reg &= ~IO_BIAS_MASK;
> - writel(reg | val, pctl->membase + sunxi_grp_config_reg(pin));
> + if (pctl->desc->io_bias_cfg_variant == IO_BIAS_CFG_V1) {
> + /*
> + * Configured value must be equal or greater to actual
> + * voltage.
> + */
> + if (uV <= 1800000)
> + val = 0x0; /* 1.8V */
> + else if (uV <= 2500000)
> + val = 0x6; /* 2.5V */
> + else if (uV <= 2800000)
> + val = 0x9; /* 2.8V */
> + else if (uV <= 3000000)
> + val = 0xA; /* 3.0V */
> + else
> + val = 0xD; /* 3.3V */
> +
> + pin -= pctl->desc->pin_base;
> +
> + reg = readl(pctl->membase + sunxi_grp_config_reg(pin));
> + reg &= ~IO_BIAS_MASK;
> + writel(reg | val, pctl->membase + sunxi_grp_config_reg(pin));
> + }
>
> return 0;
> }
> diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.h b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> index ee15ab067b5f..642f667e99d2 100644
> --- a/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> +++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.h
> @@ -95,6 +95,9 @@
> #define PINCTRL_SUN7I_A20 BIT(7)
> #define PINCTRL_SUN8I_R40 BIT(8)
>
> +/* Bias voltage configuration done via Pn_GRP_CONFIG registers. */
> +#define IO_BIAS_CFG_V1 1
> +

Can we turn this into an enum, and give them proper name? Mentionning
an example in the commit would be great too.

Something like:

enum sunxi_desc_bias_voltage {
/* Bias Voltage configuration is done through Pn_GRP_CONFIG registers, as seen on the A83t */
BIAS_VOLTAGE_GRP_CONFIG,
};

etc.

Thanks!
Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]