[PATCH v2] pinctrl: nuvoton: Add NPCM8xx pinctrl driver

[Simon Glass]

Hi Stanley,

On Thu, 17 Mar 2022 at 22:33, Stanley Chu <stanley.chuys at gmail.com> wrote:
>
> Add Nuvoton BMC NPCM845 Pinmux and Pinconf support.
>
> Signed-off-by: Stanley Chu <yschu at nuvoton.com>
> ---
> v2:
>  - drop the WDnRCRB/CORSTCB register access, it is not for
>    GPIO modules reset control
> ---
>  drivers/pinctrl/Kconfig                   |    1 +
>  drivers/pinctrl/Makefile                  |    1 +
>  drivers/pinctrl/nuvoton/Kconfig           |    6 +
>  drivers/pinctrl/nuvoton/Makefile          |    1 +
>  drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c | 1163 +++++++++++++++++++++
>  5 files changed, 1172 insertions(+)
>  create mode 100644 drivers/pinctrl/nuvoton/Kconfig
>  create mode 100644 drivers/pinctrl/nuvoton/Makefile
>  create mode 100644 drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c
>

Reviewed-by: Simon Glass <sjg at chromium.org>

A few thoughts below

> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index 03946245c7..076aff1a8d 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -329,6 +329,7 @@ source "drivers/pinctrl/mscc/Kconfig"
>  source "drivers/pinctrl/mtmips/Kconfig"
>  source "drivers/pinctrl/mvebu/Kconfig"
>  source "drivers/pinctrl/nexell/Kconfig"
> +source "drivers/pinctrl/nuvoton/Kconfig"
>  source "drivers/pinctrl/nxp/Kconfig"
>  source "drivers/pinctrl/renesas/Kconfig"
>  source "drivers/pinctrl/rockchip/Kconfig"
> diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
> index df37c32033..de84f8912b 100644
> --- a/drivers/pinctrl/Makefile
> +++ b/drivers/pinctrl/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_ARCH_ASPEED) += aspeed/
>  obj-$(CONFIG_ARCH_ATH79) += ath79/
>  obj-$(CONFIG_PINCTRL_INTEL) += intel/
>  obj-$(CONFIG_ARCH_MTMIPS) += mtmips/
> +obj-$(CONFIG_ARCH_NPCM) += nuvoton/
>  obj-$(CONFIG_ARCH_RMOBILE) += renesas/
>  obj-$(CONFIG_PINCTRL_SANDBOX)  += pinctrl-sandbox.o
>
> diff --git a/drivers/pinctrl/nuvoton/Kconfig b/drivers/pinctrl/nuvoton/Kconfig
> new file mode 100644
> index 0000000000..b490b41a60
> --- /dev/null
> +++ b/drivers/pinctrl/nuvoton/Kconfig
> @@ -0,0 +1,6 @@
> +config PINCTRL_NPCM8XX
> +       bool "Pinctrl driver for Nuvoton NPCM8XX"
> +       depends on DM && PINCTRL_GENERIC && ARCH_NPCM8XX
> +       help
> +         Support pin muxing and pin configuration on
> +         Nuvoton NPCM8XX SoC.

This should have more text

> diff --git a/drivers/pinctrl/nuvoton/Makefile b/drivers/pinctrl/nuvoton/Makefile
> new file mode 100644
> index 0000000000..a6dfdf3672
> --- /dev/null
> +++ b/drivers/pinctrl/nuvoton/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_PINCTRL_NPCM8XX)  += pinctrl-npcm8xx.o
> diff --git a/drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c b/drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c
> new file mode 100644
> index 0000000000..4531923cff
> --- /dev/null
> +++ b/drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c
> @@ -0,0 +1,1163 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2022 Nuvoton Technology Corp.
> + */
> +
> +#include <dm.h>
> +#include <errno.h>
> +#include <regmap.h>
> +#include <syscon.h>
> +#include <asm/io.h>
> +#include <dm/device_compat.h>
> +#include <dm/pinctrl.h>
> +#include <linux/bitfield.h>
> +#include <asm/arch/rst.h>
> +
> +/* GCR register offsets */
> +#define WD0RCR         0x38
> +#define WD1RCR         0x3C

lower-case hex consistently

> +#define WD2RCR         0x40
> +#define SWRSTC1                0x44
> +#define SWRSTC2                0x48
> +#define SWRSTC3                0x4C
> +#define SWRSTC4                0x50
> +#define CORSTC         0x5C
> +#define FLOCKR1                0x74
> +#define INTCR4         0xC0
> +#define I2CSEGSEL      0xE0
> +#define MFSEL1         0x260
> +#define MFSEL2         0x264
> +#define MFSEL3         0x268
> +#define MFSEL4         0x26C
> +#define MFSEL5         0x270
> +#define MFSEL6         0x274
> +#define MFSEL7         0x278
> +

[..]

> +
> +struct group_config {
> +       char *name;
> +       const unsigned int *pins;

uint

> +       unsigned int npins;
> +       u32 reg; /* Register of setting func */
> +       u32 bit;
> +};
> +
> +#define GRP(x, _reg, _bit) {                   \
> +       .name = #x,                             \
> +       .pins = x## _pins,                      \
> +       .npins = ARRAY_SIZE(x## _pins),         \
> +       .reg = _reg,                            \
> +       .bit = _bit,                            \
> +       }

[..]

> +static int npcm8xx_pinmux_group_set(struct udevice *dev,
> +                                   unsigned int group_selector,
> +                                   unsigned int func_selector)
> +{
> +       const struct group_config *group;
> +       int pin_selector;
> +       int i;
> +
> +       dev_dbg(dev, "set_mux [grp %s][func %s]\n",
> +               npcm8xx_groups[group_selector].name,
> +               npcm8xx_groups[func_selector].name);
> +       group = &npcm8xx_groups[group_selector];
> +
> +       if (group->npins == 0) {

!group->npins

[..]

Regards,
Simon