[U-Boot] [PATCH v1 2/2] pinctrl: pinctrl-at91: Add pinctrl driver
Simon Glass
sjg at chromium.org
Thu Oct 13 02:03:25 CEST 2016
Hi Wenyou,
On 11 October 2016 at 23:22, Wenyou Yang <wenyou.yang at atmel.com> wrote:
> AT91 PIO controller is a combined gpio-controller, pin-mux and
> pin-config module. The peripheral's pins are assigned through
> per-pin based muxing logic.
>
> Each soc will have to describe the SoC limitation and pin
> configuration via DT. This will allow to do not need to touch
> the C code when adding new SoC if the IP version is supported.
>
> Signed-off-by: Wenyou Yang <wenyou.yang at atmel.com>
> ---
>
> arch/arm/mach-at91/include/mach/at91_pio.h | 6 +-
> drivers/pinctrl/Kconfig | 7 +
> drivers/pinctrl/Makefile | 1 +
> drivers/pinctrl/pinctrl-at91.c | 451 +++++++++++++++++++++++++++++
> 4 files changed, 464 insertions(+), 1 deletion(-)
> create mode 100644 drivers/pinctrl/pinctrl-at91.c
Reviewed-by: Simon Glass <sjg at chromium.org>
Some comments below.
>
> diff --git a/arch/arm/mach-at91/include/mach/at91_pio.h b/arch/arm/mach-at91/include/mach/at91_pio.h
> index 393a163..f195a7d 100644
> --- a/arch/arm/mach-at91/include/mach/at91_pio.h
> +++ b/arch/arm/mach-at91/include/mach/at91_pio.h
> @@ -107,7 +107,11 @@ typedef struct at91_port {
> u32 wpsr; /* 0xE8 Write Protect Status Register */
> u32 reserved11[5]; /* */
> u32 schmitt; /* 0x100 Schmitt Trigger Register */
> - u32 reserved12[63];
> + u32 reserved12[4]; /* 0x104 ~ 0x110 */
> + u32 driver1; /* 0x114 I/O Driver Register1(AT91SAM9x5's driver1) */
> + u32 driver12; /* 0x118 I/O Driver Register12(AT91SAM9x5's driver2 or SAMA5D3x's driver1 ) */
> + u32 driver2; /* 0x11C I/O Driver Register2(SAMA5D3x's driver2) */
> + u32 reserved13[12]; /* 0x120 ~ 0x14C */
> } at91_port_t;
>
> typedef union at91_pio {
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index 12be3cf..87a7ff0 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -141,6 +141,13 @@ config ROCKCHIP_RK3288_PINCTRL
> definitions and pin control functions for each available multiplex
> function.
>
> +config PINCTRL_AT91
> + bool "AT91 pinctrl driver"
> + depends on DM
> + help
> + This option is to enable the AT91 pinctrl driver for AT91 PIO
> + controller.
Can you add a bit more info? What features does it support? How does
it relate to the pio4 driver?
> +
> config PINCTRL_AT91PIO4
> bool "AT91 PIO4 pinctrl driver"
> depends on DM
> diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
> index f28b5c1..a9535fa 100644
> --- a/drivers/pinctrl/Makefile
> +++ b/drivers/pinctrl/Makefile
> @@ -5,6 +5,7 @@
> obj-y += pinctrl-uclass.o
> obj-$(CONFIG_$(SPL_)PINCTRL_GENERIC) += pinctrl-generic.o
>
> +obj-$(CONFIG_PINCTRL_AT91) += pinctrl-at91.o
> obj-$(CONFIG_PINCTRL_AT91PIO4) += pinctrl-at91-pio4.o
> obj-y += nxp/
> obj-$(CONFIG_ARCH_ATH79) += ath79/
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> new file mode 100644
> index 0000000..c7f75cb
> --- /dev/null
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -0,0 +1,451 @@
> +/*
> + * Atmel PIO pinctrl driver
> + *
> + * Copyright (C) 2016 Atmel Corporation
> + * Wenyou.Yang <wenyou.yang at atmel.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <dm/device.h>
> +#include <dm/pinctrl.h>
> +#include <linux/io.h>
> +#include <linux/err.h>
> +#include <mach/at91_pio.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#define MAX_GPIO_BANKS 5
> +#define MAX_NB_GPIO_PER_BANK 32
> +
> +#define MAX_PINMUX_ENTRIES 200
> +
> +struct at91_pinctrl_priv {
> + struct at91_port *reg_base[MAX_GPIO_BANKS];
> + u32 nbanks;
> +};
> +
> +#define PULL_UP BIT(0)
> +#define MULTI_DRIVE BIT(1)
> +#define DEGLITCH BIT(2)
> +#define PULL_DOWN BIT(3)
> +#define DIS_SCHMIT BIT(4)
> +#define DRIVE_STRENGTH_SHIFT 5
> +#define DRIVE_STRENGTH_MASK 0x3
> +#define DRIVE_STRENGTH (DRIVE_STRENGTH_MASK << DRIVE_STRENGTH_SHIFT)
> +#define DEBOUNCE BIT(16)
> +#define DEBOUNCE_VAL_SHIFT 17
> +#define DEBOUNCE_VAL (0x3fff << DEBOUNCE_VAL_SHIFT)
> +
> +/**
> + * These defines will translated the dt binding settings to our internal
> + * settings. They are not necessarily the same value as the register setting.
> + * The actual drive strength current of low, medium and high must be looked up
> + * from the corresponding device datasheet. This value is different for pins
> + * that are even in the same banks. It is also dependent on VCC.
> + * DRIVE_STRENGTH_DEFAULT is just a placeholder to avoid changing the drive
> + * strength when there is no dt config for it.
> + */
> +#define DRIVE_STRENGTH_DEFAULT (0 << DRIVE_STRENGTH_SHIFT)
> +#define DRIVE_STRENGTH_LOW (1 << DRIVE_STRENGTH_SHIFT)
> +#define DRIVE_STRENGTH_MED (2 << DRIVE_STRENGTH_SHIFT)
> +#define DRIVE_STRENGTH_HI (3 << DRIVE_STRENGTH_SHIFT)
> +
> +enum at91_mux {
> + AT91_MUX_GPIO = 0,
> + AT91_MUX_PERIPH_A = 1,
> + AT91_MUX_PERIPH_B = 2,
> + AT91_MUX_PERIPH_C = 3,
> + AT91_MUX_PERIPH_D = 4,
> +};
> +
> +/**
> + * struct at91_pinctrl_mux_ops - describes an AT91 mux ops group
> + * on new IP with support for periph C and D the way to mux in
> + * periph A and B has changed
> + * So provide the right callbacks
> + * if not present means the IP does not support it
> + * @mux_A_periph: mux as periph A
> + * @mux_B_periph: mux as periph B
> + * @mux_C_periph: mux as periph C
> + * @mux_D_periph: mux as periph D
> + * @set_deglitch: enable/disable deglitch
> + * @set_debounce: enable/disable debounce
> + * @set_pulldown: enable/disable pulldown
> + * @disable_schmitt_trig: disable schmitt trigger
> + */
> +struct at91_pinctrl_mux_ops {
> + void (*mux_A_periph)(struct at91_port *pio, u32 mask);
> + void (*mux_B_periph)(struct at91_port *pio, u32 mask);
> + void (*mux_C_periph)(struct at91_port *pio, u32 mask);
> + void (*mux_D_periph)(struct at91_port *pio, u32 mask);
> + void (*set_deglitch)(struct at91_port *pio, u32 mask, bool is_on);
> + void (*set_debounce)(struct at91_port *pio, u32 mask, bool is_on,
> + u32 div);
> + void (*set_pulldown)(struct at91_port *pio, u32 mask, bool is_on);
> + void (*disable_schmitt_trig)(struct at91_port *pio, u32 mask);
> + void (*set_drivestrength)(struct at91_port *pio, u32 pin,
> + u32 strength);
Can you add comments for these functions? It isn't clear why they do
or what the arguments mean.
[..]
> +static void set_drive_strength(void *reg, u32 pin, u32 strength)
> +{
> + u32 tmp = readl(reg);
> + u32 shift = two_bit_pin_value_shift_amount(pin);
> +
> + tmp &= ~(DRIVE_STRENGTH_MASK << shift);
> + tmp |= strength << shift;
> +
> + writel(tmp, reg);
Can you use clrsetbits_le32() here?
[..]
> +static int at91_pinconf_set(struct at91_pinctrl_mux_ops *ops,
> + struct at91_port *pio, u32 pin, u32 config)
> +{
> + u32 mask = BIT(pin);
> +
> + if (config & PULL_UP && config & PULL_DOWN)
I'm not sure what that means but I think it needs brackets for readability.
> + return -EINVAL;
> +
> + at91_mux_set_pullup(pio, mask, config & PULL_UP);
> + at91_mux_set_multidrive(pio, mask, config & MULTI_DRIVE);
> + if (ops->set_deglitch)
> + ops->set_deglitch(pio, mask, config & DEGLITCH);
> + if (ops->set_debounce)
> + ops->set_debounce(pio, mask, config & DEBOUNCE,
> + (config & DEBOUNCE_VAL) >> DEBOUNCE_VAL_SHIFT);
> + if (ops->set_pulldown)
> + ops->set_pulldown(pio, mask, config & PULL_DOWN);
> + if (ops->disable_schmitt_trig && config & DIS_SCHMIT)
> + ops->disable_schmitt_trig(pio, mask);
> + if (ops->set_drivestrength)
> + ops->set_drivestrength(pio, pin,
> + (config & DRIVE_STRENGTH) >> DRIVE_STRENGTH_SHIFT);
> +
> + return 0;
> +}
> +
> +static int at91_pin_check_config(struct udevice *dev, u32 bank, u32 pin)
> +{
> + struct at91_pinctrl_priv *priv = dev_get_priv(dev);
> +
> + if (bank >= priv->nbanks) {
> + printf("pin conf bank %d >= nbanks %d\n", bank, priv->nbanks);
debug()
> + return -EINVAL;
> + }
> +
> + if (pin >= MAX_NB_GPIO_PER_BANK) {
> + printf("pin conf pin %d >= %d\n", pin, MAX_NB_GPIO_PER_BANK);
debug()
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static struct at91_port *at91_bank_base(struct udevice *dev, u32 bank)
This function doesn't seem very useful - remove it?
Regards,
Simon
More information about the U-Boot
mailing list