From: Kostya Porotchkin <kostap@marvell.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 3/6] arm64: mvebu: pinctrl: Add pin control driver for A8K family
Date: Thu, 24 Nov 2016 08:28:21 +0000 [thread overview]
Message-ID: <1479976201767.85748@marvell.com> (raw)
In-Reply-To: <CAPnjgZ3fm4oSYtNttWxJJVfRu8qbFBRXpECTzXgdSBE8bdLeJA@mail.gmail.com>
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
next prev parent reply other threads:[~2016-11-24 8:28 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-20 15:38 [U-Boot] [PATCH 0/6] arm64: mvebu: Armada-8K family patches kostap at marvell.com
2016-11-20 15:38 ` [U-Boot] [PATCH 1/6] arm64: mvebu: Modify the A8K SPI and I2C config in DTS kostap at marvell.com
2016-11-24 9:02 ` Stefan Roese
2016-11-24 9:22 ` Kostya Porotchkin
2016-11-24 12:05 ` Stefan Roese
2016-11-20 15:38 ` [U-Boot] [PATCH 2/6] arm64: mvebu: Add bubt command for flash image burn kostap at marvell.com
2016-11-24 8:58 ` Stefan Roese
2016-11-20 15:38 ` [U-Boot] [PATCH 3/6] arm64: mvebu: pinctrl: Add pin control driver for A8K family kostap at marvell.com
2016-11-24 2:20 ` Simon Glass
2016-11-24 8:28 ` Kostya Porotchkin [this message]
2016-11-20 15:38 ` [U-Boot] [PATCH 4/6] arm64: mvebu: Add pin control nodes to A8K family DTS files kostap at marvell.com
2016-11-24 9:13 ` Stefan Roese
2016-11-24 14:09 ` Kostya Porotchkin
2016-11-24 16:00 ` Stefan Roese
2016-11-20 15:38 ` [U-Boot] [PATCH 5/6] arm64: mvebu: Enable BUBT command support in A8K default config kostap at marvell.com
2016-11-20 15:38 ` [U-Boot] [PATCH 6/6] arm64: mvebu: Enable pin control " kostap at marvell.com
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1479976201767.85748@marvell.com \
--to=kostap@marvell.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.