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 functionvalue 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@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 00 3 0 0 0 0 0 0 0 3>;};};};cp110-master {config-space {cpm_pinctl: pinctl@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 0xff0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff0xff 0 7 0 7 0 0 2 2 00 0 8 8 1 1 1 1 1 11 1 1 1 1 1 0xE 0xE 0xE 0xE0xE 0xE 0xE>;};};};cp110-slave {config-space {cps_pinctl: pinctl@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 0x30x3 0x3 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff0xff 0xff 0xff 0xff 0xff 0xff 0xff 0x8 0xff 0xff0xff 0x8 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff0xff 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_MVEBUbooldefault yhelpSupport pin multiplexing and pin configuration control onMarvell'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