[U-Boot] [PATCH 3/6] arm64: mvebu: pinctrl: Add pin control driver for A8K family

Kostya Porotchkin
2016-11-24T08:28:21Z

Hello, Simon,

Thank you very much for your review.
I am preparing a second patch release, which will include the requested fixes.
For the license I have to check with the company legal department first.
I am also going to fix some functions values in DTS files following internal review.

Regards
Konstantin
________________________________________
From: sjg@google.com <sjg@google.com> on behalf of Simon Glass <sjg@chromium.org>
Sent: Thursday, November 24, 2016 04:20
To: Kostya Porotchkin
Cc: U-Boot Mailing List; Stefan Roese; Nadav Haklai; Neta Zur Hershkovits; Omri Itach; Igal Liberman; Haim Boot; Hanna Hawa
Subject: Re: [PATCH 3/6] arm64: mvebu: pinctrl: Add pin control driver for A8K family

Hi,

On 20 November 2016 at 08:38, <kostap@marvell.com> wrote:
> From: Konstantin Porotchkin <kostap@marvell.com>
>
> Add a port of Marvell pin control driver.
> The A8K SoC family contains several silicone dies interconnected
> in a single package. Every die is normally equuipped with its own
> pin controller unit.
> Since the UCLASS_PINCTRL device only calls the probe method for
> the first detected pin controller, a trick similar to used with
> comphy driver is required.
> In order to bring up all pin controllers available in A8K SoC,
> the arch_early_init_r() function sequentially calls the
> uclass_get_device() function for each UCLASS_PINCTRL device.
>
> Change-Id: Iff143827e8f1558a554d77173562c4b52ce179d7
> Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Stefan Roese <sr@denx.de>
> Cc: Nadav Haklai <nadavh@marvell.com>
> Cc: Neta Zur Hershkovits <neta@marvell.com>
> Cc: Omri Itach <omrii@marvell.com>
> Cc: Igal Liberman <igall@marvell.com>
> Cc: Haim Boot <hayim@marvell.com>
> Cc: Hanna Hawa <hannah@marvell.com>
> ---
> arch/arm/include/asm/arch-armada8k/soc-info.h | 45 +++++
> arch/arm/mach-mvebu/arm64-common.c | 1 -
> .../pinctrl/marvell,mvebu-pinctrl.txt | 113 ++++++++++++
> drivers/pinctrl/Kconfig | 1 +
> drivers/pinctrl/Makefile | 1 +
> drivers/pinctrl/mvebu/Kconfig | 7 +
> drivers/pinctrl/mvebu/Makefile | 17 ++
> drivers/pinctrl/mvebu/pinctrl-mvebu.c | 195 +++++++++++++++++++++
> drivers/pinctrl/mvebu/pinctrl-mvebu.h | 44 +++++
> 9 files changed, 423 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm/include/asm/arch-armada8k/soc-info.h
> create mode 100644 doc/device-tree-bindings/pinctrl/marvell,mvebu-pinctrl.txt
> create mode 100644 drivers/pinctrl/mvebu/Kconfig
> create mode 100644 drivers/pinctrl/mvebu/Makefile
> create mode 100644 drivers/pinctrl/mvebu/pinctrl-mvebu.c
> create mode 100644 drivers/pinctrl/mvebu/pinctrl-mvebu.h
>

Generally looks good but I have a load of nits sorry.

> diff --git a/arch/arm/include/asm/arch-armada8k/soc-info.h b/arch/arm/include/asm/arch-armada8k/soc-info.h
> new file mode 100644
> index 0000000..4640deb
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-armada8k/soc-info.h
> @@ -0,0 +1,45 @@
> +/*
> + * ***************************************************************************
> + * Copyright (C) 2015 Marvell International Ltd.
> + * ***************************************************************************
> + * This program is free software: you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation, either version 2 of the License, or any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + * ***************************************************************************

Can you please use SPDX?

> + */
> +
> +#ifndef _SOC_INFO_H_
> +#define _SOC_INFO_H_
> +
> +/* General MPP definitions */
> +#define MAX_MPP_OPTS 7
> +#define MAX_MPP_ID 15
> +
> +#define MPP_BIT_CNT 4
> +#define MPP_FIELD_MASK 0x7
> +#define MPP_FIELD_BITS 3
> +#define MPP_VAL_MASK 0xF
> +
> +#define MPPS_PER_REG (32 / MPP_BIT_CNT)
> +#define MAX_MPP_REGS ((MAX_MPP_ID + MPPS_PER_REG) / MPPS_PER_REG)
> +
> +/* MPP pins and functions correcsponding to UART RX connections
> + This information is used for detection of recovery boot mode (boot from UART) */

/*
* MPP pins
* ...
* /

> +#define MPP_UART_RX_PINS { 3, 5 }
> +#define MPP_UART_RX_FUNCTIONS { 1, 2 }
> +
> +/* Pin Ctrl driver definitions */
> +#define BITS_PER_PIN 4
> +#define PIN_FUNC_MASK ((1 << BITS_PER_PIN) - 1)
> +#define PIN_REG_SHIFT 3
> +#define PIN_FIELD_MASK ((1 << PIN_REG_SHIFT) - 1)
> +
> +#endif /* _SOC_INFO_H_ */
> diff --git a/arch/arm/mach-mvebu/arm64-common.c b/arch/arm/mach-mvebu/arm64-common.c
> index 1fc2ff2..78fe7a7 100644
> --- a/arch/arm/mach-mvebu/arm64-common.c
> +++ b/arch/arm/mach-mvebu/arm64-common.c
> @@ -124,7 +124,6 @@ int arch_early_init_r(void)
> if (ret)
> break;
> }
> -

Unrelated change?

> /* Cause the SATA device to do its early init */
> uclass_first_device(UCLASS_AHCI, &dev);
>
> diff --git a/doc/device-tree-bindings/pinctrl/marvell,mvebu-pinctrl.txt b/doc/device-tree-bindings/pinctrl/marvell,mvebu-pinctrl.txt
> new file mode 100644
> index 0000000..0973db8
> --- /dev/null
> +++ b/doc/device-tree-bindings/pinctrl/marvell,mvebu-pinctrl.txt
> @@ -0,0 +1,113 @@
> +The pinctrl driver enables Marvell Armada 8K SoCs to configure the multi-purpose
> +pins (mpp) to a specific function.
> +A Marvell SoC pin configuration node is a node of a group of pins which can
> +be used for a specific device or function. Each node requires one or more
> +mpp pins or group of pins and a mpp function common to all pins.
> +
> +Required properties for the pinctrl driver:
> +- compatible: "marvell,mvebu-pinctrl",
> + "marvell,armada-ap806-pinctrl",
> + "marvell,a70x0-pinctrl",
> + "marvell,a80x0-cp0-pinctrl",
> + "marvell,a80x0-cp1-pinctrl"
> +- bank-name: A string defining the pinc controller bank name
> +- reg: A pair of values defining the pin controller base address
> + and the address space
> +- pin-count: Numeric value defining the amount of multi purpose pins
> + included in this bank
> +- max-func: Numeric value defining the maximum function value for
> + pins in this bank
> +- pin-func: Array of pin function values for every pin in the bank.
> + When the function value for a specific pin equal 0xFF,
> + the pin configuration is skipped and a default function
> + value is used for this pin.
> +
> +The A8K is a hybrid SoC that contains several silicon dies interconnected in
> +a single package. Each such die may have a separate pin controller.
> +
> +Example:
> +/ {
> + ap806 {
> + config-space {
> + pinctl: pinctl at 6F4000 {
> + compatible = "marvell,mvebu-pinctrl",
> + "marvell,armada-ap806-pinctrl";
> + bank-name ="apn-806";
> + reg = <0x6F4000 0x10>;
> + pin-count = <20>;
> + max-func = <3>;
> + /* MPP Bus:
> + SPI0 [0-3]
> + I2C0 [4-5]
> + UART0 [11,19]
> + */
> + /* 0 1 2 3 4 5 6 7 8 9 */
> + pin-func = < 3 3 3 3 3 3 0 0 0 0
> + 0 3 0 0 0 0 0 0 0 3>;
> + };
> + };
> + };
> +
> + cp110-master {
> + config-space {
> + cpm_pinctl: pinctl at 44000 {
> + compatible = "marvell,mvebu-pinctrl",
> + "marvell,a70x0-pinctrl",
> + "marvell,a80x0-cp0-pinctrl";
> + bank-name ="cp0-110";
> + reg = <0x440000 0x20>;
> + pin-count = <63>;
> + max-func = <0xf>;
> + /* MPP Bus:
> + [0-31] = 0xff: Keep default CP0_shared_pins:
> + [11] CLKOUT_MPP_11 (out)
> + [23] LINK_RD_IN_CP2CP (in)
> + [25] CLKOUT_MPP_25 (out)
> + [29] AVS_FB_IN_CP2CP (in)
> + [32,34] SMI
> + [31] GPIO: push button/Wake
> + [35-36] GPIO
> + [37-38] I2C
> + [40-41] SATA[0/1]_PRESENT_ACTIVEn
> + [42-43] XSMI
> + [44-55] RGMII1
> + [56-62] SD
> + */
> + /* 0 1 2 3 4 5 6 7 8 9 */
> + pin-func = < 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0 7 0 7 0 0 2 2 0
> + 0 0 8 8 1 1 1 1 1 1
> + 1 1 1 1 1 1 0xE 0xE 0xE 0xE
> + 0xE 0xE 0xE>;
> + };
> + };
> + };
> +
> + cp110-slave {
> + config-space {
> + cps_pinctl: pinctl at 44000 {
> + compatible = "marvell,mvebu-pinctrl",
> + "marvell,a80x0-cp1-pinctrl";
> + bank-name ="cp1-110";
> + reg = <0x440000 0x20>;
> + pin-count = <63>;
> + max-func = <0xf>;
> + /* MPP Bus:
> + [0-11] RGMII0
> + [27,31] GE_MDIO/MDC
> + [32-62] = 0xff: Keep default CP1_shared_pins:
> + */
> + /* 0 1 2 3 4 5 6 7 8 9 */
> + pin-func = < 0x3 0x3 0x3 0x3 0x3 0x3 0x3 0x3 0x3 0x3
> + 0x3 0x3 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0x8 0xff 0xff
> + 0xff 0x8 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff
> + 0xff 0xff 0xff>;
> + };
> + };
> + };
> +}
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index 12be3cf..efcb4c0 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -181,5 +181,6 @@ source "drivers/pinctrl/meson/Kconfig"
> source "drivers/pinctrl/nxp/Kconfig"
> source "drivers/pinctrl/uniphier/Kconfig"
> source "drivers/pinctrl/exynos/Kconfig"
> +source "drivers/pinctrl/mvebu/Kconfig"
>
> endmenu
> diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
> index f28b5c1..512112a 100644
> --- a/drivers/pinctrl/Makefile
> +++ b/drivers/pinctrl/Makefile
> @@ -15,3 +15,4 @@ obj-$(CONFIG_PINCTRL_UNIPHIER) += uniphier/
> obj-$(CONFIG_PIC32_PINCTRL) += pinctrl_pic32.o
> obj-$(CONFIG_PINCTRL_EXYNOS) += exynos/
> obj-$(CONFIG_PINCTRL_MESON) += meson/
> +obj-$(CONFIG_PINCTRL_MVEBU) += mvebu/
> diff --git a/drivers/pinctrl/mvebu/Kconfig b/drivers/pinctrl/mvebu/Kconfig
> new file mode 100644
> index 0000000..cf9c299
> --- /dev/null
> +++ b/drivers/pinctrl/mvebu/Kconfig
> @@ -0,0 +1,7 @@
> +config PINCTRL_MVEBU
> + depends on ARCH_MVEBU
> + bool
> + default y
> + help
> + Support pin multiplexing and pin configuration control on
> + Marvell's Armada-8K SoC.
> diff --git a/drivers/pinctrl/mvebu/Makefile b/drivers/pinctrl/mvebu/Makefile
> new file mode 100644
> index 0000000..7db2b97
> --- /dev/null
> +++ b/drivers/pinctrl/mvebu/Makefile
> @@ -0,0 +1,17 @@
> +#* ***************************************************************************
> +#* Copyright (C) 2016 Marvell International Ltd.
> +#* ***************************************************************************
> +#* This program is free software: you can redistribute it and/or modify it
> +#* under the terms of the GNU General Public License as published by the Free
> +#* Software Foundation, either version 2 of the License, or any later version.
> +#*
> +#* This program is distributed in the hope that it will be useful,
> +#* but WITHOUT ANY WARRANTY; without even the implied warranty of
> +#* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +#* GNU General Public License for more details.
> +#*
> +#* You should have received a copy of the GNU General Public License
> +#* along with this program. If not, see <http://www.gnu.org/licenses/>.
> +#* ***************************************************************************
> +
> +obj-$(CONFIG_PINCTRL_MVEBU) += pinctrl-mvebu.o
> diff --git a/drivers/pinctrl/mvebu/pinctrl-mvebu.c b/drivers/pinctrl/mvebu/pinctrl-mvebu.c
> new file mode 100644
> index 0000000..02fcd2f
> --- /dev/null
> +++ b/drivers/pinctrl/mvebu/pinctrl-mvebu.c
> @@ -0,0 +1,195 @@
> +/*
> + * ***************************************************************************
> + * Copyright (C) 2016 Marvell International Ltd.
> + * ***************************************************************************
> + * This program is free software: you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation, either version 2 of the License, or any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + * ***************************************************************************
> + */
> +
> +#include <config.h>
> +#include <common.h>
> +#include <dm.h>
> +#include <asm/system.h>
> +#include <asm/io.h>
> +#include <dm/pinctrl.h>
> +#include <dm/root.h>
> +#include <fdtdec.h>
> +#include <errno.h>
> +#include <asm/arch/fdt.h>
> +#include <asm/arch-armada8k/soc-info.h>
> +#include "pinctrl-mvebu.h"

Please check include-file ordering here:

http://www.denx.de/wiki/U-Boot/CodingStyle

It is close.

> +
> +/*
> + * mvebu_pinctrl_set_state: configure pin functions.
> + * dev: the pinctrl device to be configured.
> + * config: the state to be configured.

/*
* mvebu_pinctrl_set_state() - configure pin functions
* @dev: the pinctrl device to be configured.
* @config: the state to be configured.
* @return ...
*/

Please use that consistently.

> + */
> +int mvebu_pinctrl_set_state(struct udevice *dev, struct udevice *config)
> +{
> + const void *blob = gd->fdt_blob;
> + int node = config->of_offset;
> + struct mvebu_pinctrl_priv *priv;
> + u32 pin_arr[CONFIG_MAX_PINS_PER_BANK];
> + u32 function;
> + int i, pin_count;
> +
> + priv = dev_get_priv(dev);
> + if (!priv) {
> + printf("%s: Failed to get private\n", __func__);

debug()

> + return -EINVAL;
> + }
> +
> + pin_count = fdtdec_get_int_array_count(blob, node, "marvell,pins", pin_arr, CONFIG_MAX_PINS_PER_BANK);

Are you sure this passes checkpatch? That line seems to long.

> + if (pin_count <= 0) {
> + error("Failed reading pins array for pinconfig %s (%d)\n", config->name, pin_count);

debug() to avoid bloating the code? (globally)

> + return -EINVAL;
> + }
> +
> + function = fdtdec_get_int(blob, node, "marvell,function", 0xFF);

nit: Lower case hex throughout

> +
> + for (i = 0; i < pin_count; i++) {
> + int reg_offset;
> + int field_offset;
> + u32 reg, mask;
> + int pin = pin_arr[i];
> +
> + if (function > priv->max_func) {
> + error("Illegal function %d for pinconfig %s\n", function, config->name);
> + return -EINVAL;
> + }
> +
> + /* Calculate register address and bit in register */
> + reg_offset = priv->reg_direction * 4 * (pin >> (PIN_REG_SHIFT));
> + field_offset = (BITS_PER_PIN) * (pin & PIN_FIELD_MASK);
> + mask = ~(PIN_FUNC_MASK << field_offset);
> +
> + /* Clip value to field resolution */
> + function &= PIN_FUNC_MASK;
> +
> + reg = readl(priv->base_reg + reg_offset);
> + reg = (reg & mask) | (function << field_offset);
> + writel(reg, priv->base_reg + reg_offset);

You can use clrsetbits_le32()

> + }
> +
> + return 0;
> +}
> +
> +/*
> + * mvebu_pinctrl_set_state_all: configure the entire bank pin functions.
> + * dev: the pinctrl device to be configured.
> + * config: the state to be configured.
> + */
> +static int mvebu_pinctrl_set_state_all(struct udevice *dev, struct udevice *config)
> +{
> + const void *blob = gd->fdt_blob;
> + int node = config->of_offset;
> + struct mvebu_pinctrl_priv *priv;
> + u32 func_arr[CONFIG_MAX_PINS_PER_BANK];
> + int pin, err;
> +
> + priv = dev_get_priv(dev);
> + if (!priv) {
> + printf("%s: Failed to get private\n", __func__);

This cannot happen if the device is probed. Add an assert() if you are
paranoid, but it has not benefit.

> + return -EINVAL;
> + }
> +
> + err = fdtdec_get_int_array(blob, node, "pin-func", func_arr, priv->pin_cnt);
> + if (err) {
> + error("Failed reading pin functions for bank %s\n", priv->bank_name);
> + return -EINVAL;
> + }
> +
> + for (pin = 0; pin < priv->pin_cnt; pin++) {
> + int reg_offset;
> + int field_offset;
> + u32 reg, mask;
> + u32 func = func_arr[pin];
> +
> + /* Bypass pins with function 0xFF */
> + if (func == 0xFF) {
> + debug("Warning: pin %d value is not modified (kept as default\n", pin);
> + continue;
> + } else if (func > priv->max_func) {
> + error("Illegal function %d for pin %d\n", func, pin);
> + return -EINVAL;
> + }
> +
> + /* Calculate register address and bit in register */
> + reg_offset = priv->reg_direction * 4 * (pin >> (PIN_REG_SHIFT));
> + field_offset = (BITS_PER_PIN) * (pin & PIN_FIELD_MASK);
> + mask = ~(PIN_FUNC_MASK << field_offset);
> +
> + /* Clip value to field resolution */
> + func &= PIN_FUNC_MASK;
> +
> + reg = readl(priv->base_reg + reg_offset);
> + reg = (reg & mask) | (func << field_offset);
> + writel(reg, priv->base_reg + reg_offset);

clrsetbits_le32()

> + }
> +
> + return 0;
> +}
> +
> +int mvebu_pinctl_probe(struct udevice *dev)
> +{
> + const void *blob = gd->fdt_blob;
> + int node = dev->of_offset;
> + struct mvebu_pinctrl_priv *priv;
> + fdt_addr_t base;
> +
> + priv = dev_get_priv(dev);
> + if (!priv) {
> + printf("%s: Failed to get private\n", __func__);

debug() - please fix globally - drivers should not print messages.

> + return -EINVAL;
> + }
> +
> + base = dev_get_addr(dev);
> + if (base == FDT_ADDR_T_NONE) {
> + printf("%s: Failed to get base address\n", __func__);
> + return -EINVAL;
> + }
> +
> + priv->base_reg = (u8 *)base;

Or use dev_get_addr_ptr() ?

> + priv->pin_cnt = fdtdec_get_int(blob, node, "pin-count", CONFIG_MAX_PINS_PER_BANK);
> + priv->max_func = fdtdec_get_int(blob, node, "max-func", CONFIG_MAX_FUNC);
> + priv->bank_name = fdt_getprop(blob, node, "bank-name", NULL);
> +
> + priv->reg_direction = 1;
> + if (fdtdec_get_bool(blob, node, "reverse-reg"))
> + priv->reg_direction = -1;
> +
> + return mvebu_pinctrl_set_state_all(dev, dev);
> +}
> +
> +
> +static struct pinctrl_ops mvebu_pinctrl_ops = {
> + .set_state = mvebu_pinctrl_set_state
> +};
> +
> +static const struct udevice_id mvebu_pinctrl_ids[] = {
> + { .compatible = "marvell,mvebu-pinctrl" },
> + { .compatible = "marvell,armada-ap806-pinctrl" },
> + { .compatible = "marvell,a70x0-pinctrl" },
> + { .compatible = "marvell,a80x0-cp0-pinctrl" },
> + { .compatible = "marvell,a80x0-cp1-pinctrl" },
> + { }
> +};
> +
> +U_BOOT_DRIVER(pinctrl_mvebu) = {
> + .name = "mvebu_pinctrl",
> + .id = UCLASS_PINCTRL,
> + .of_match = mvebu_pinctrl_ids,
> + .priv_auto_alloc_size = sizeof(struct mvebu_pinctrl_priv),
> + .ops = &mvebu_pinctrl_ops,
> + .probe = mvebu_pinctl_probe
> +};
> diff --git a/drivers/pinctrl/mvebu/pinctrl-mvebu.h b/drivers/pinctrl/mvebu/pinctrl-mvebu.h
> new file mode 100644
> index 0000000..61c84ce
> --- /dev/null
> +++ b/drivers/pinctrl/mvebu/pinctrl-mvebu.h
> @@ -0,0 +1,44 @@
> +/*
> + * ***************************************************************************
> + * Copyright (C) 2016 Marvell International Ltd.
> + * ***************************************************************************
> + * This program is free software: you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation, either version 2 of the License, or any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + * ***************************************************************************
> + */
> +
> + #ifndef __PINCTRL_MVEBU_H_
> + #define __PINCTRL_MVEBU_H_
> +
> + #define CONFIG_MAX_PINCTL_BANKS 4
> + #define CONFIG_MAX_PINS_PER_BANK 100
> + #define CONFIG_MAX_FUNC 0xF

Avoid using CONFIG_ prefixes since this looks like a new global CONFIG
option. Just dropping the CONFIG_ will do.

> +
> +DECLARE_GLOBAL_DATA_PTR;

Please put that in the C file that needs it.

> +
> +/*
> + * struct mvebu_pin_bank_data: mvebu-pinctrl bank data

* struct mvebu_pin_bank_data - mvebu-pinctrl bank data

> + * @base_reg: controller base address for this bank
> + * @pin_cnt: number of ping included in this bank

ping?

> + * @max_func: maximum configurable function value for pins in this bank
> + * @reg_direction:

?

> + * @bank_name: the pins bank name

pin's

> + */
> +struct mvebu_pinctrl_priv {
> + u8 *base_reg;
> + u32 pin_cnt;
> + u32 max_func;

Why are these u32? Can they be uint?

> + int reg_direction;
> + const char *bank_name;
> +};
> +
> +#endif /* __PINCTRL_MVEBU_H_ */
> --
> 2.7.4
>

Regards,
Simon