Hi,
In this forth version I improved the driver based on the review from
Linus Walleij and I fixed a configuration issue with uart2. For the
record, this series adds support for the pin and gpio controllers
present on the Armada 37xx SoCs.
Each Armada 37xx SoC comes with 2 pin controllers: one on the south
bridge (managing 28 pins) and one on the north bridge (managing 36 pins).
At the hardware level the controller configure the pins by group and not
pin by pin.
The gpio controller is also capable to handle interrupt from gpio.
Changelog
v3 -> v4
- Some group are configured by several bits in the register:
extend the armada_37xx_pin_group struct to manage it.
- Fix the uart2 and cspi2/3 configuration
- Document the armada_37xx_add_function(), armada_37xx_fill_group()
and armada_37xx_fill_funcs().
- Use devm_gpiochip_add_data()
- Use irq_find_mapping instead of irq_linear_revmap
- Use handle_edge_irq instead of the wrong handle_level_irq
- Add comment about the fact the we have multiple parent interrupt
- Add comment about the mask usage of the irq_data struct
- Use BIT() macro when possible
- Select more CONFIG symbol needed for GPIO and interrupt support
v2 -> v3
- use gpio-ranges (patch 4)
- Document gpio-ranges usage (patch 1)
- do not use anymore a global pin index (patch 3)
v1 -> v2:
- Update binding documentation making clear that mfd and syscon must
be used (patch 1).
- Split the fist patch adding pin controller support for Armada 37xx
in arm64 part (for kconfig) and pinctrl part (patch 2 and 3)
- Add MFD_SYSCON dependency (patch 3)
- Add kerneldoc for the armada_37xx_pin_group struct (patch 3)
- Rename _add_function() to armada_37xx_add_function() (patch 3)
- Use an inline function to update the reg offset (patch 4)
- Rename gpiolib_register to gpiochip_register (patch 4)
- Add a comment about the two registers limit (patch 4)
- Add explicit gpio node in the device tree (patch 4)
- Convert the driver to use GPIOLIB_IRQCHIP (patch 5)
- Add a critical section when accessing the hardware registers (patch 5)
- Use the gpio subnode (patch 5)
Thanks,
Gregory
Gregory CLEMENT (7):
pinctrl: dt-bindings: Add documentation for Armada 37xx pin controllers
arm64: marvell: enable the Armada 37xx pinctrl driver
pinctrl: armada-37xx: Add pin controller support for Armada 37xx
pinctrl: armada-37xx: Add gpio support
pinctrl: aramda-37xx: Add irqchip support
ARM64: dts: marvell: Add pinctrl nodes for Armada 3700
ARM64: dts: marvell: armada37xx: add pinctrl definition
Documentation/devicetree/bindings/clock/armada3700-xtal-clock.txt | 7 +-
Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt | 183 ++++++++++++++-
arch/arm64/Kconfig.platforms | 5 +-
arch/arm64/boot/dts/marvell/armada-3720-db.dts | 8 +-
arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 73 ++++-
drivers/pinctrl/Makefile | 2 +-
drivers/pinctrl/mvebu/Kconfig | 7 +-
drivers/pinctrl/mvebu/Makefile | 3 +-
drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 969 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
9 files changed, 1249 insertions(+), 8 deletions(-)
create mode 100644 Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt
create mode 100644 drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
base-commit: c1ae3cfa0e89fa1a7ecc4c99031f5e9ae99d9201
--
git-series 0.9.1
The Armada 37xx SoCs can handle interrupt through GPIO. However it can
only manage the edge ones.
The way the interrupt are managed are classical so we can use the generic
interrupt chip model.
The only unusual "feature" is that many interrupts are connected to the
parent interrupt controller. But we do not take advantage of this and use
the chained irq with all of them.
Signed-off-by: Gregory CLEMENT <[email protected]>
---
drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 221 +++++++++++++++++++++-
1 file changed, 221 insertions(+)
diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
index 5c96f5558310..7356516e0921 100644
--- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
+++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
@@ -13,7 +13,9 @@
#include <linux/gpio/driver.h>
#include <linux/mfd/syscon.h>
#include <linux/of.h>
+#include <linux/of_address.h>
#include <linux/of_device.h>
+#include <linux/of_irq.h>
#include <linux/pinctrl/pinconf-generic.h>
#include <linux/pinctrl/pinconf.h>
#include <linux/pinctrl/pinctrl.h>
@@ -30,6 +32,11 @@
#define OUTPUT_CTL 0x20
#define SELECTION 0x30
+#define IRQ_EN 0x0
+#define IRQ_POL 0x08
+#define IRQ_STATUS 0x10
+#define IRQ_WKUP 0x18
+
#define NB_FUNCS 2
#define GPIO_PER_REG 32
@@ -75,9 +82,12 @@ struct armada_37xx_pmx_func {
struct armada_37xx_pinctrl {
struct regmap *regmap;
+ void __iomem *base;
const struct armada_37xx_pin_data *data;
struct device *dev;
struct gpio_chip gpio_chip;
+ struct irq_chip irq_chip;
+ spinlock_t irq_lock;
struct pinctrl_desc pctl;
struct pinctrl_dev *pctl_dev;
struct armada_37xx_pin_group *groups;
@@ -346,6 +356,14 @@ static int armada_37xx_pmx_set(struct pinctrl_dev *pctldev,
return armada_37xx_pmx_set_by_name(pctldev, name, grp);
}
+static inline void armada_37xx_irq_update_reg(unsigned int *reg,
+ struct irq_data *d)
+{
+ int offset = irqd_to_hwirq(d);
+
+ armada_37xx_update_reg(reg, offset);
+}
+
static int armada_37xx_gpio_direction_input(struct gpio_chip *chip,
unsigned int offset)
{
@@ -468,6 +486,206 @@ static const struct gpio_chip armada_37xx_gpiolib_chip = {
.owner = THIS_MODULE,
};
+void armada_37xx_irq_ack(struct irq_data *d)
+{
+ struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
+ struct armada_37xx_pinctrl *info = gpiochip_get_data(chip);
+ u32 reg = IRQ_STATUS, mask = d->mask;
+ unsigned long flags;
+
+ armada_37xx_irq_update_reg(®, d);
+ spin_lock_irqsave(&info->irq_lock, flags);
+ writel(mask, info->base + reg);
+ spin_unlock_irqrestore(&info->irq_lock, flags);
+}
+
+void armada_37xx_irq_mask(struct irq_data *d)
+{
+ struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
+ struct armada_37xx_pinctrl *info = gpiochip_get_data(chip);
+ u32 val, reg = IRQ_EN, mask = d->mask;
+ unsigned long flags;
+
+ armada_37xx_irq_update_reg(®, d);
+ spin_lock_irqsave(&info->irq_lock, flags);
+ val = readl(info->base + reg);
+ writel(val & ~mask, info->base + reg);
+ spin_unlock_irqrestore(&info->irq_lock, flags);
+}
+
+void armada_37xx_irq_unmask(struct irq_data *d)
+{
+ struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
+ struct armada_37xx_pinctrl *info = gpiochip_get_data(chip);
+ u32 val, reg = IRQ_EN, mask = d->mask;
+ unsigned long flags;
+
+ armada_37xx_irq_update_reg(®, d);
+ spin_lock_irqsave(&info->irq_lock, flags);
+ val = readl(info->base + reg);
+ writel(val | mask, info->base + reg);
+ spin_unlock_irqrestore(&info->irq_lock, flags);
+}
+
+static int armada_37xx_irq_set_wake(struct irq_data *d, unsigned int on)
+{
+ struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
+ struct armada_37xx_pinctrl *info = gpiochip_get_data(chip);
+ u32 val, reg = IRQ_WKUP, mask = d->mask;
+ unsigned long flags;
+
+ armada_37xx_irq_update_reg(®, d);
+ spin_lock_irqsave(&info->irq_lock, flags);
+ val = readl(info->base + reg);
+ if (on)
+ val |= mask;
+ else
+ val &= ~mask;
+ writel(val, info->base + reg);
+ spin_unlock_irqrestore(&info->irq_lock, flags);
+
+ return 0;
+}
+
+static int armada_37xx_irq_set_type(struct irq_data *d, unsigned int type)
+{
+ struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
+ struct armada_37xx_pinctrl *info = gpiochip_get_data(chip);
+ u32 val, reg = IRQ_POL, mask = d->mask;
+ unsigned long flags;
+
+ spin_lock_irqsave(&info->irq_lock, flags);
+ armada_37xx_irq_update_reg(®, d);
+ val = readl(info->base + reg);
+ switch (type) {
+ case IRQ_TYPE_EDGE_RISING:
+ val &= ~mask;
+ break;
+ case IRQ_TYPE_EDGE_FALLING:
+ val |= mask;
+ break;
+ default:
+ spin_unlock_irqrestore(&info->irq_lock, flags);
+ return -EINVAL;
+ }
+ writel(val, info->base + reg);
+ spin_unlock_irqrestore(&info->irq_lock, flags);
+
+ return 0;
+}
+
+
+static void armada_37xx_irq_handler(struct irq_desc *desc)
+{
+ struct gpio_chip *gc = irq_desc_get_handler_data(desc);
+ struct irq_chip *chip = irq_desc_get_chip(desc);
+ struct armada_37xx_pinctrl *info = gpiochip_get_data(gc);
+ struct irq_domain *d = gc->irqdomain;
+ int i;
+
+ chained_irq_enter(chip, desc);
+ for (i = 0; i <= d->revmap_size / GPIO_PER_REG; i++) {
+ u32 status;
+ unsigned long flags;
+
+ spin_lock_irqsave(&info->irq_lock, flags);
+ status = readl_relaxed(info->base + IRQ_STATUS + 4 * i);
+ /* Manage only the interrupt that was enabled */
+ status &= readl_relaxed(info->base + IRQ_EN + 4 * i);
+ spin_unlock_irqrestore(&info->irq_lock, flags);
+ while (status) {
+ u32 hwirq = ffs(status) - 1;
+ u32 virq = irq_find_mapping(d, hwirq +
+ i * GPIO_PER_REG);
+
+ generic_handle_irq(virq);
+ status &= ~BIT(hwirq);
+ }
+ }
+ chained_irq_exit(chip, desc);
+}
+
+static int armada_37xx_irqchip_register(struct platform_device *pdev,
+ struct armada_37xx_pinctrl *info)
+{
+ struct device_node *np = info->dev->of_node;
+ int nrirqs = info->data->nr_pins;
+ struct gpio_chip *gc = &info->gpio_chip;
+ struct irq_chip *irqchip = &info->irq_chip;
+ struct resource res;
+ int ret = -ENODEV, i, nr_irq_parent;
+
+ for_each_child_of_node(info->dev->of_node, np) {
+ if (of_find_property(np, "gpio-controller", NULL)) {
+ ret = 0;
+ break;
+ }
+ };
+ if (ret)
+ return ret;
+
+ nr_irq_parent = of_irq_count(np);
+ spin_lock_init(&info->irq_lock);
+
+ if (!nr_irq_parent) {
+ dev_err(&pdev->dev, "Invalid or no IRQ\n");
+ return 0;
+ }
+
+ if (of_address_to_resource(info->dev->of_node, 1, &res)) {
+ dev_err(info->dev, "cannot find IO resource\n");
+ return -ENOENT;
+ }
+
+ info->base = devm_ioremap_resource(info->dev, &res);
+ if (IS_ERR(info->base))
+ return PTR_ERR(info->base);
+
+ irqchip->irq_ack = armada_37xx_irq_ack;
+ irqchip->irq_mask = armada_37xx_irq_mask;
+ irqchip->irq_unmask = armada_37xx_irq_unmask;
+ irqchip->irq_set_wake = armada_37xx_irq_set_wake;
+ irqchip->irq_set_type = armada_37xx_irq_set_type;
+ irqchip->name = info->data->name;
+
+ ret = gpiochip_irqchip_add(gc, irqchip, 0,
+ handle_edge_irq, IRQ_TYPE_NONE);
+ if (ret) {
+ dev_info(&pdev->dev, "could not add irqchip\n");
+ return ret;
+ }
+
+ /*
+ * Many interrupts are connected to the parent interrupt
+ * controller. But we do not take advantage of this and use
+ * the chained irq with all of them.
+ */
+ for (i = 0; i < nrirqs; i++) {
+ struct irq_data *d = irq_get_irq_data(gc->irq_base + i);
+
+ /*
+ * The mask field is a "precomputed bitmask for
+ * accessing the chip registers" which was introduced
+ * for the generic irqchip framework. As we don't use
+ * this framework, we can reuse this field for our own
+ * usage.
+ */
+ d->mask = BIT(i % GPIO_PER_REG);
+ }
+
+ for (i = 0; i < nr_irq_parent; i++) {
+ int irq = irq_of_parse_and_map(np, i);
+
+ if (irq < 0)
+ continue;
+
+ gpiochip_set_chained_irqchip(gc, irqchip, irq,
+ armada_37xx_irq_handler);
+ }
+
+ return 0;
+}
+
static int armada_37xx_gpiochip_register(struct platform_device *pdev,
struct armada_37xx_pinctrl *info)
{
@@ -496,6 +714,9 @@ static int armada_37xx_gpiochip_register(struct platform_device *pdev,
ret = devm_gpiochip_add_data(&pdev->dev, gc, info);
if (ret)
return ret;
+ ret = armada_37xx_irqchip_register(pdev, info);
+ if (ret)
+ return ret;
return 0;
}
--
git-series 0.9.1
The Armada 37xx SoC come with 2 pin controllers: one on the south
bridge (managing 28 pins) and one on the north bridge (managing 36 pins).
At the hardware level the controller configure the pins by group and not
pin by pin. This constraint is reflected in the design of the driver:
only the group related functions are implemented.
Signed-off-by: Gregory CLEMENT <[email protected]>
---
drivers/pinctrl/Makefile | 2 +-
drivers/pinctrl/mvebu/Kconfig | 7 +-
drivers/pinctrl/mvebu/Makefile | 3 +-
drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 648 +++++++++++++++++++++-
4 files changed, 658 insertions(+), 2 deletions(-)
create mode 100644 drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index a251f439626f..95080811f36f 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -44,7 +44,7 @@ obj-y += bcm/
obj-$(CONFIG_PINCTRL_BERLIN) += berlin/
obj-y += freescale/
obj-$(CONFIG_X86) += intel/
-obj-$(CONFIG_PINCTRL_MVEBU) += mvebu/
+obj-y += mvebu/
obj-y += nomadik/
obj-$(CONFIG_PINCTRL_PXA) += pxa/
obj-$(CONFIG_ARCH_QCOM) += qcom/
diff --git a/drivers/pinctrl/mvebu/Kconfig b/drivers/pinctrl/mvebu/Kconfig
index 170602407c0d..5bade32d3089 100644
--- a/drivers/pinctrl/mvebu/Kconfig
+++ b/drivers/pinctrl/mvebu/Kconfig
@@ -39,3 +39,10 @@ config PINCTRL_ORION
select PINCTRL_MVEBU
endif
+
+config PINCTRL_ARMADA_37XX
+ bool
+ select GENERIC_PINCONF
+ select MFD_SYSCON
+ select PINCONF
+ select PINMUX
diff --git a/drivers/pinctrl/mvebu/Makefile b/drivers/pinctrl/mvebu/Makefile
index 18270cd5ea43..60c245a60f39 100644
--- a/drivers/pinctrl/mvebu/Makefile
+++ b/drivers/pinctrl/mvebu/Makefile
@@ -1,4 +1,4 @@
-obj-y += pinctrl-mvebu.o
+obj-$(CONFIG_PINCTRL_MVEBU) += pinctrl-mvebu.o
obj-$(CONFIG_PINCTRL_DOVE) += pinctrl-dove.o
obj-$(CONFIG_PINCTRL_KIRKWOOD) += pinctrl-kirkwood.o
obj-$(CONFIG_PINCTRL_ARMADA_370) += pinctrl-armada-370.o
@@ -6,4 +6,5 @@ obj-$(CONFIG_PINCTRL_ARMADA_375) += pinctrl-armada-375.o
obj-$(CONFIG_PINCTRL_ARMADA_38X) += pinctrl-armada-38x.o
obj-$(CONFIG_PINCTRL_ARMADA_39X) += pinctrl-armada-39x.o
obj-$(CONFIG_PINCTRL_ARMADA_XP) += pinctrl-armada-xp.o
+obj-$(CONFIG_PINCTRL_ARMADA_37XX) += pinctrl-armada-37xx.o
obj-$(CONFIG_PINCTRL_ORION) += pinctrl-orion.o
diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
new file mode 100644
index 000000000000..8b769d77db22
--- /dev/null
+++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
@@ -0,0 +1,648 @@
+/*
+ * Marvell 37xx SoC pinctrl driver
+ *
+ * Copyright (C) 2017 Marvell
+ *
+ * Gregory CLEMENT <[email protected]>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2 or later. This program is licensed "as is"
+ * without any warranty of any kind, whether express or implied.
+ */
+
+#include <linux/mfd/syscon.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#include "../pinctrl-utils.h"
+
+#define OUTPUT_EN 0x0
+#define OUTPUT_CTL 0x20
+#define SELECTION 0x30
+
+#define NB_FUNCS 2
+#define GPIO_PER_REG 32
+
+/**
+ * struct armada_37xx_pin_group: represents group of pins of a pinmux function.
+ * The pins of a pinmux groups are composed of one or two groups of contiguous
+ * pins.
+ * @name: Name of the pin group, used to lookup the group.
+ * @start_pins: Index of the first pin of the main range of pins belonging to
+ * the group
+ * @npins: Number of pins included in the first range
+ * @reg_mask: Bit mask matching the group in the selection register
+ * @extra_pins: Index of the first pin of the optional second range of pins
+ * belonging to the group
+ * @npins: Number of pins included in the second optional range
+ * @funcs: A list of pinmux functions that can be selected for this group.
+ * @pins: List of the pins included in the group
+ */
+struct armada_37xx_pin_group {
+ const char *name;
+ unsigned int start_pin;
+ unsigned int npins;
+ u32 reg_mask;
+ u32 val[NB_FUNCS];
+ unsigned int extra_pin;
+ unsigned int extra_npins;
+ const char *funcs[NB_FUNCS];
+ unsigned int *pins;
+};
+
+struct armada_37xx_pin_data {
+ u8 nr_pins;
+ char *name;
+ struct armada_37xx_pin_group *groups;
+ int ngroups;
+};
+
+struct armada_37xx_pmx_func {
+ const char *name;
+ const char **groups;
+ unsigned int ngroups;
+};
+
+struct armada_37xx_pinctrl {
+ struct regmap *regmap;
+ const struct armada_37xx_pin_data *data;
+ struct device *dev;
+ struct pinctrl_desc pctl;
+ struct pinctrl_dev *pctl_dev;
+ struct armada_37xx_pin_group *groups;
+ unsigned int ngroups;
+ struct armada_37xx_pmx_func *funcs;
+ unsigned int nfuncs;
+};
+
+#define PIN_GRP(_name, _start, _nr, _mask, _func1, _func2) \
+ { \
+ .name = _name, \
+ .start_pin = _start, \
+ .npins = _nr, \
+ .reg_mask = _mask, \
+ .val = {0, _mask}, \
+ .funcs = {_func1, _func2} \
+ }
+
+#define PIN_GRP_GPIO(_name, _start, _nr, _mask, _func1) \
+ { \
+ .name = _name, \
+ .start_pin = _start, \
+ .npins = _nr, \
+ .reg_mask = _mask, \
+ .val = {0, _mask}, \
+ .funcs = {_func1, "gpio"} \
+ }
+
+#define PIN_GRP_GPIO_2(_name, _start, _nr, _mask, _val1, _val2, _func1) \
+ { \
+ .name = _name, \
+ .start_pin = _start, \
+ .npins = _nr, \
+ .reg_mask = _mask, \
+ .val = {_val1, _val2}, \
+ .funcs = {_func1, "gpio"} \
+ }
+
+#define PIN_GRP_EXTRA(_name, _start, _nr, _mask, _v1, _v2, _start2, _nr2, \
+ _f1, _f2) \
+ { \
+ .name = _name, \
+ .start_pin = _start, \
+ .npins = _nr, \
+ .reg_mask = _mask, \
+ .val = {_v1, _v2}, \
+ .extra_pin = _start2, \
+ .extra_npins = _nr2, \
+ .funcs = {_f1, _f2} \
+ }
+
+static struct armada_37xx_pin_group armada_37xx_nb_groups[] = {
+ PIN_GRP_GPIO("jtag", 20, 5, BIT(0), "jtag"),
+ PIN_GRP_GPIO("sdio0", 8, 3, BIT(1), "sdio"),
+ PIN_GRP_GPIO("emmc_nb", 27, 9, BIT(2), "emmc"),
+ PIN_GRP_GPIO("pwm0", 11, 1, BIT(3), "pwm"),
+ PIN_GRP_GPIO("pwm1", 12, 1, BIT(4), "pwm"),
+ PIN_GRP_GPIO("pwm2", 13, 1, BIT(5), "pwm"),
+ PIN_GRP_GPIO("pwm3", 14, 1, BIT(6), "pwm"),
+ PIN_GRP_GPIO("pmic1", 17, 1, BIT(7), "pmic"),
+ PIN_GRP_GPIO("pmic0", 16, 1, BIT(8), "pmic"),
+ PIN_GRP_GPIO("i2c2", 2, 2, BIT(9), "i2c"),
+ PIN_GRP_GPIO("i2c1", 0, 2, BIT(10), "i2c"),
+ PIN_GRP_GPIO("spi_cs1", 17, 1, BIT(12), "spi"),
+ PIN_GRP_GPIO_2("spi_cs2", 18, 1, BIT(13) | BIT(19), 0, BIT(13), "spi"),
+ PIN_GRP_GPIO_2("spi_cs3", 19, 1, BIT(14) | BIT(19), 0, BIT(14), "spi"),
+ PIN_GRP_GPIO("onewire", 4, 1, BIT(16), "onewire"),
+ PIN_GRP_GPIO("uart1", 25, 2, BIT(17), "uart"),
+ PIN_GRP_GPIO("spi_quad", 15, 2, BIT(18), "spi"),
+ PIN_GRP_EXTRA("uart2", 9, 2, BIT(13) | BIT(14) | BIT(19),
+ BIT(13) | BIT(14), BIT(19), 18, 2, "gpio", "uart"),
+ PIN_GRP_GPIO("led0_od", 11, 1, BIT(20), "led"),
+ PIN_GRP_GPIO("led1_od", 12, 1, BIT(21), "led"),
+ PIN_GRP_GPIO("led2_od", 13, 1, BIT(22), "led"),
+ PIN_GRP_GPIO("led3_od", 14, 1, BIT(23), "led"),
+
+};
+
+static struct armada_37xx_pin_group armada_37xx_sb_groups[] = {
+ PIN_GRP_GPIO("usb32_drvvbus0", 0, 1, BIT(0), "drvbus"),
+ PIN_GRP_GPIO("usb2_drvvbus1", 1, 1, BIT(1), "drvbus"),
+ PIN_GRP_GPIO("sdio_sb", 24, 5, BIT(2), "sdio"),
+ PIN_GRP_EXTRA("rgmii", 6, 14, BIT(3), 0, BIT(3), 23, 1, "mii", "gpio"),
+ PIN_GRP_GPIO("pcie1", 3, 2, BIT(4), "pcie"),
+ PIN_GRP_GPIO("ptp", 20, 3, BIT(5), "ptp"),
+ PIN_GRP("ptp_clk", 21, 1, BIT(6), "ptp", "mii"),
+ PIN_GRP("ptp_trig", 22, 1, BIT(7), "ptp", "mii"),
+ PIN_GRP("mii_col", 23, 1, BIT(8), "mii", "mii_err"),
+};
+
+const struct armada_37xx_pin_data armada_37xx_pin_nb = {
+ .nr_pins = 36,
+ .name = "GPIO1",
+ .groups = armada_37xx_nb_groups,
+ .ngroups = ARRAY_SIZE(armada_37xx_nb_groups),
+};
+
+const struct armada_37xx_pin_data armada_37xx_pin_sb = {
+ .nr_pins = 29,
+ .name = "GPIO2",
+ .groups = armada_37xx_sb_groups,
+ .ngroups = ARRAY_SIZE(armada_37xx_sb_groups),
+};
+
+static int armada_37xx_get_func_reg(struct armada_37xx_pin_group *grp,
+ const char *func)
+{
+ int f;
+
+ for (f = 0; f < NB_FUNCS; f++)
+ if (!strcmp(grp->funcs[f], func))
+ return f;
+
+ return -ENOTSUPP;
+}
+
+static struct armada_37xx_pin_group *armada_37xx_find_next_grp_by_pin(
+ struct armada_37xx_pinctrl *info, int pin, int *grp)
+{
+ while (*grp < info->ngroups) {
+ struct armada_37xx_pin_group *group = &info->groups[*grp];
+ int j;
+
+ *grp = *grp + 1;
+ for (j = 0; j < (group->npins + group->extra_npins); j++)
+ if (group->pins[j] == pin)
+ return group;
+ }
+ return NULL;
+}
+
+static int armada_37xx_pin_config_group_get(struct pinctrl_dev *pctldev,
+ unsigned int selector, unsigned long *config)
+{
+ return -ENOTSUPP;
+}
+
+static int armada_37xx_pin_config_group_set(struct pinctrl_dev *pctldev,
+ unsigned int selector, unsigned long *configs,
+ unsigned int num_configs)
+{
+ return -ENOTSUPP;
+}
+
+static struct pinconf_ops armada_37xx_pinconf_ops = {
+ .is_generic = true,
+ .pin_config_group_get = armada_37xx_pin_config_group_get,
+ .pin_config_group_set = armada_37xx_pin_config_group_set,
+};
+
+static int armada_37xx_get_groups_count(struct pinctrl_dev *pctldev)
+{
+ struct armada_37xx_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+
+ return info->ngroups;
+}
+
+static const char *armada_37xx_get_group_name(struct pinctrl_dev *pctldev,
+ unsigned int group)
+{
+ struct armada_37xx_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+
+ return info->groups[group].name;
+}
+
+static int armada_37xx_get_group_pins(struct pinctrl_dev *pctldev,
+ unsigned int selector,
+ const unsigned int **pins,
+ unsigned int *npins)
+{
+ struct armada_37xx_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+
+ if (selector >= info->ngroups)
+ return -EINVAL;
+
+ *pins = info->groups[selector].pins;
+ *npins = info->groups[selector].npins +
+ info->groups[selector].extra_npins;
+
+ return 0;
+}
+
+static const struct pinctrl_ops armada_37xx_pctrl_ops = {
+ .get_groups_count = armada_37xx_get_groups_count,
+ .get_group_name = armada_37xx_get_group_name,
+ .get_group_pins = armada_37xx_get_group_pins,
+ .dt_node_to_map = pinconf_generic_dt_node_to_map_group,
+ .dt_free_map = pinctrl_utils_free_map,
+};
+
+/*
+ * Pinmux_ops handling
+ */
+
+static int armada_37xx_pmx_get_funcs_count(struct pinctrl_dev *pctldev)
+{
+ struct armada_37xx_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+
+ return info->nfuncs;
+}
+
+static const char *armada_37xx_pmx_get_func_name(struct pinctrl_dev *pctldev,
+ unsigned int selector)
+{
+ struct armada_37xx_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+
+ return info->funcs[selector].name;
+}
+
+static int armada_37xx_pmx_get_groups(struct pinctrl_dev *pctldev,
+ unsigned int selector,
+ const char * const **groups,
+ unsigned int * const num_groups)
+{
+ struct armada_37xx_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+
+ *groups = info->funcs[selector].groups;
+ *num_groups = info->funcs[selector].ngroups;
+
+ return 0;
+}
+
+static int armada_37xx_pmx_set_by_name(struct pinctrl_dev *pctldev,
+ const char *name,
+ struct armada_37xx_pin_group *grp)
+{
+ struct armada_37xx_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+ unsigned int reg = SELECTION;
+ unsigned int mask = grp->reg_mask;
+ int func, val;
+
+ dev_dbg(info->dev, "enable function %s group %s\n",
+ name, grp->name);
+
+ func = armada_37xx_get_func_reg(grp, name);
+
+ if (func < 0)
+ return func;
+
+ val = grp->val[func];
+
+ regmap_update_bits(info->regmap, reg, mask, val);
+
+ return 0;
+}
+
+static int armada_37xx_pmx_set(struct pinctrl_dev *pctldev,
+ unsigned int selector,
+ unsigned int group)
+{
+
+ struct armada_37xx_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+ struct armada_37xx_pin_group *grp = &info->groups[group];
+ const char *name = info->funcs[selector].name;
+
+ return armada_37xx_pmx_set_by_name(pctldev, name, grp);
+}
+
+static int armada_37xx_pmx_direction_input(struct armada_37xx_pinctrl *info,
+ unsigned int offset)
+{
+ unsigned int reg = OUTPUT_EN;
+ unsigned int mask;
+
+ if (offset >= GPIO_PER_REG) {
+ offset -= GPIO_PER_REG;
+ reg += sizeof(u32);
+ }
+ mask = BIT(offset);
+
+ return regmap_update_bits(info->regmap, reg, mask, 0);
+}
+
+static int armada_37xx_pmx_direction_output(struct armada_37xx_pinctrl *info,
+ unsigned int offset, int value)
+{
+ unsigned int reg = OUTPUT_EN;
+ unsigned int mask;
+
+ if (offset >= GPIO_PER_REG) {
+ offset -= GPIO_PER_REG;
+ reg += sizeof(u32);
+ }
+ mask = BIT(offset);
+
+ return regmap_update_bits(info->regmap, reg, mask, mask);
+}
+
+static int armada_37xx_pmx_gpio_set_direction(struct pinctrl_dev *pctldev,
+ struct pinctrl_gpio_range *range,
+ unsigned int offset, bool input)
+{
+ struct armada_37xx_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+
+ dev_dbg(info->dev, "gpio_direction for pin %u as %s-%d to %s\n",
+ offset, range->name, offset, input ? "input" : "output");
+
+ if (input)
+ armada_37xx_pmx_direction_input(info, offset);
+ else
+ armada_37xx_pmx_direction_output(info, offset, 0);
+
+ return 0;
+}
+
+static int armada_37xx_gpio_request_enable(struct pinctrl_dev *pctldev,
+ struct pinctrl_gpio_range *range,
+ unsigned int offset)
+{
+ struct armada_37xx_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+ struct armada_37xx_pin_group *group;
+ int grp = 0;
+
+ dev_dbg(info->dev, "requesting gpio %d\n", offset);
+
+ while ((group = armada_37xx_find_next_grp_by_pin(info, offset, &grp)))
+ armada_37xx_pmx_set_by_name(pctldev, "gpio", group);
+
+ return 0;
+}
+
+static const struct pinmux_ops armada_37xx_pmx_ops = {
+ .get_functions_count = armada_37xx_pmx_get_funcs_count,
+ .get_function_name = armada_37xx_pmx_get_func_name,
+ .get_function_groups = armada_37xx_pmx_get_groups,
+ .set_mux = armada_37xx_pmx_set,
+ .gpio_request_enable = armada_37xx_gpio_request_enable,
+ .gpio_set_direction = armada_37xx_pmx_gpio_set_direction,
+};
+
+/**
+ * armada_37xx_add_function() - Add a new function to the list
+ * @funcs: array of function to add the new one
+ * @funcsize: size of the remaining space for the function
+ * @name: name of the function to add
+ *
+ * If it is a new function then create it by adding its name else
+ * increment the number of group associated to this function.
+ */
+static int armada_37xx_add_function(struct armada_37xx_pmx_func *funcs,
+ int *funcsize, const char *name)
+{
+ int i = 0;
+
+ if (*funcsize <= 0)
+ return -EOVERFLOW;
+
+ while (funcs->ngroups) {
+ /* function already there */
+ if (strcmp(funcs->name, name) == 0) {
+ funcs->ngroups++;
+
+ return -EEXIST;
+ }
+ funcs++;
+ i++;
+ }
+
+ /* append new unique function */
+ funcs->name = name;
+ funcs->ngroups = 1;
+ (*funcsize)--;
+
+ return 0;
+}
+
+/**
+ * armada_37xx_fill_group() - complete the group array
+ * @info: info driver instance
+ *
+ * Based on the data available from the armada_37xx_pin_group array
+ * completes the last member of the struct for each function: the list
+ * of the groups associated to this function.
+ *
+ */
+static int armada_37xx_fill_group(struct armada_37xx_pinctrl *info)
+{
+ int n, num = 0, funcsize = info->data->nr_pins;
+
+ for (n = 0; n < info->ngroups; n++) {
+ struct armada_37xx_pin_group *grp = &info->groups[n];
+ int i, j, f;
+
+ grp->pins = devm_kzalloc(info->dev,
+ (grp->npins + grp->extra_npins) *
+ sizeof(*grp->pins), GFP_KERNEL);
+ if (!grp->pins)
+ return -ENOMEM;
+
+ for (i = 0; i < grp->npins; i++)
+ grp->pins[i] = grp->start_pin + i;
+
+ for (j = 0; j < grp->extra_npins; j++)
+ grp->pins[i+j] = grp->extra_pin + j;
+
+ for (f = 0; f < NB_FUNCS; f++) {
+ int ret;
+ /* check for unique functions and count groups */
+ ret = armada_37xx_add_function(info->funcs, &funcsize,
+ grp->funcs[f]);
+ if (ret == -EOVERFLOW)
+ dev_err(info->dev,
+ "More functions than pins(%d)\n",
+ info->data->nr_pins);
+ if (ret < 0)
+ continue;
+ num++;
+ }
+ }
+
+ info->nfuncs = num;
+
+ return 0;
+}
+
+/**
+ * armada_37xx_fill_funcs() - complete the funcs array
+ * @info: info driver instance
+ *
+ * Based on the data available from the armada_37xx_pin_group array
+ * completes the last two member of the struct for each group:
+ * - the list of the pins included in the group
+ * - the list of pinmux functions that can be selected for this group
+ *
+ */
+static int armada_37xx_fill_func(struct armada_37xx_pinctrl *info)
+{
+ struct armada_37xx_pmx_func *funcs = info->funcs;
+ int n;
+
+ for (n = 0; n < info->nfuncs; n++) {
+ const char *name = funcs[n].name;
+ const char **groups;
+ int g;
+
+ funcs[n].groups = devm_kzalloc(info->dev, funcs[n].ngroups *
+ sizeof(*(funcs[n].groups)),
+ GFP_KERNEL);
+ if (!funcs[n].groups)
+ return -ENOMEM;
+
+ groups = funcs[n].groups;
+
+ for (g = 0; g < info->ngroups; g++) {
+ struct armada_37xx_pin_group *gp = &info->groups[g];
+ int f;
+
+ for (f = 0; f < NB_FUNCS; f++) {
+ if (strcmp(gp->funcs[f], name) == 0) {
+ *groups = gp->name;
+ groups++;
+ }
+ }
+ }
+ }
+ return 0;
+}
+
+static int armada_37xx_pinctrl_register(struct platform_device *pdev,
+ struct armada_37xx_pinctrl *info)
+{
+ const struct armada_37xx_pin_data *pin_data = info->data;
+ struct pinctrl_desc *ctrldesc = &info->pctl;
+ struct pinctrl_pin_desc *pindesc, *pdesc;
+ int pin, ret;
+
+ info->groups = pin_data->groups;
+ info->ngroups = pin_data->ngroups;
+
+ ctrldesc->name = "armada_37xx-pinctrl";
+ ctrldesc->owner = THIS_MODULE;
+ ctrldesc->pctlops = &armada_37xx_pctrl_ops;
+ ctrldesc->pmxops = &armada_37xx_pmx_ops;
+ ctrldesc->confops = &armada_37xx_pinconf_ops;
+
+ pindesc = devm_kzalloc(&pdev->dev, sizeof(*pindesc) *
+ pin_data->nr_pins, GFP_KERNEL);
+ if (!pindesc)
+ return -ENOMEM;
+
+ ctrldesc->pins = pindesc;
+ ctrldesc->npins = pin_data->nr_pins;
+
+ pdesc = pindesc;
+ for (pin = 0; pin < pin_data->nr_pins; pin++) {
+ pdesc->number = pin;
+ pdesc->name = kasprintf(GFP_KERNEL, "%s-%d",
+ pin_data->name, pin);
+ pdesc++;
+ }
+
+ /*
+ * we allocate functions for number of pins and hope there are
+ * fewer unique functions than pins available
+ */
+ info->funcs = devm_kzalloc(&pdev->dev, pin_data->nr_pins *
+ sizeof(struct armada_37xx_pmx_func), GFP_KERNEL);
+ if (!info->funcs)
+ return -ENOMEM;
+
+
+ ret = armada_37xx_fill_group(info);
+ if (ret)
+ return ret;
+
+ ret = armada_37xx_fill_func(info);
+ if (ret)
+ return ret;
+
+ info->pctl_dev = devm_pinctrl_register(&pdev->dev, ctrldesc, info);
+ if (IS_ERR(info->pctl_dev)) {
+ dev_err(&pdev->dev, "could not register pinctrl driver\n");
+ return PTR_ERR(info->pctl_dev);
+ }
+
+ return 0;
+}
+
+static const struct of_device_id armada_37xx_pinctrl_of_match[] = {
+ {
+ .compatible = "marvell,armada3710-sb-pinctrl",
+ .data = (void *)&armada_37xx_pin_sb,
+ },
+ {
+ .compatible = "marvell,armada3710-nb-pinctrl",
+ .data = (void *)&armada_37xx_pin_nb,
+ },
+ { },
+};
+
+static int __init armada_37xx_pinctrl_probe(struct platform_device *pdev)
+{
+ struct armada_37xx_pinctrl *info;
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
+ struct regmap *regmap;
+ int ret;
+
+ info = devm_kzalloc(dev, sizeof(struct armada_37xx_pinctrl),
+ GFP_KERNEL);
+ if (!info)
+ return -ENOMEM;
+
+ info->dev = dev;
+
+ regmap = syscon_node_to_regmap(np);
+ if (IS_ERR(regmap)) {
+ dev_err(&pdev->dev, "cannot get regmap\n");
+ return PTR_ERR(regmap);
+ }
+ info->regmap = regmap;
+
+ info->data = of_device_get_match_data(dev);
+
+ ret = armada_37xx_pinctrl_register(pdev, info);
+ if (ret)
+ return ret;
+
+ platform_set_drvdata(pdev, info);
+
+ return 0;
+}
+
+static struct platform_driver armada_37xx_pinctrl_driver = {
+ .driver = {
+ .name = "armada-37xx-pinctrl",
+ .of_match_table = armada_37xx_pinctrl_of_match,
+ },
+};
+
+builtin_platform_driver_probe(armada_37xx_pinctrl_driver,
+ armada_37xx_pinctrl_probe);
--
git-series 0.9.1
GPIO management is pretty simple and is part of the same IP than the pin
controller for the Armada 37xx SoCs. This patch adds the GPIO support to
the pinctrl-armada-37xx.c file, it also allows sharing common functions
between the gpiolib and the pinctrl drivers.
Signed-off-by: Gregory CLEMENT <[email protected]>
---
drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 128 ++++++++++++++++++---
1 file changed, 114 insertions(+), 14 deletions(-)
diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
index 8b769d77db22..5c96f5558310 100644
--- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
+++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
@@ -10,6 +10,7 @@
* without any warranty of any kind, whether express or implied.
*/
+#include <linux/gpio/driver.h>
#include <linux/mfd/syscon.h>
#include <linux/of.h>
#include <linux/of_device.h>
@@ -24,6 +25,8 @@
#include "../pinctrl-utils.h"
#define OUTPUT_EN 0x0
+#define INPUT_VAL 0x10
+#define OUTPUT_VAL 0x18
#define OUTPUT_CTL 0x20
#define SELECTION 0x30
@@ -74,6 +77,7 @@ struct armada_37xx_pinctrl {
struct regmap *regmap;
const struct armada_37xx_pin_data *data;
struct device *dev;
+ struct gpio_chip gpio_chip;
struct pinctrl_desc pctl;
struct pinctrl_dev *pctl_dev;
struct armada_37xx_pin_group *groups;
@@ -178,6 +182,16 @@ const struct armada_37xx_pin_data armada_37xx_pin_sb = {
.ngroups = ARRAY_SIZE(armada_37xx_sb_groups),
};
+static inline void armada_37xx_update_reg(unsigned int *reg,
+ unsigned int offset)
+{
+ /* We never have more than 2 registers */
+ if (offset >= GPIO_PER_REG) {
+ offset -= GPIO_PER_REG;
+ *reg += sizeof(u32);
+ }
+}
+
static int armada_37xx_get_func_reg(struct armada_37xx_pin_group *grp,
const char *func)
{
@@ -332,49 +346,88 @@ static int armada_37xx_pmx_set(struct pinctrl_dev *pctldev,
return armada_37xx_pmx_set_by_name(pctldev, name, grp);
}
-static int armada_37xx_pmx_direction_input(struct armada_37xx_pinctrl *info,
- unsigned int offset)
+static int armada_37xx_gpio_direction_input(struct gpio_chip *chip,
+ unsigned int offset)
{
+ struct armada_37xx_pinctrl *info = gpiochip_get_data(chip);
unsigned int reg = OUTPUT_EN;
unsigned int mask;
- if (offset >= GPIO_PER_REG) {
- offset -= GPIO_PER_REG;
- reg += sizeof(u32);
- }
+ armada_37xx_update_reg(®, offset);
mask = BIT(offset);
return regmap_update_bits(info->regmap, reg, mask, 0);
}
-static int armada_37xx_pmx_direction_output(struct armada_37xx_pinctrl *info,
- unsigned int offset, int value)
+static int armada_37xx_gpio_get_direction(struct gpio_chip *chip,
+ unsigned int offset)
{
+ struct armada_37xx_pinctrl *info = gpiochip_get_data(chip);
+ unsigned int reg = OUTPUT_EN;
+ unsigned int val, mask;
+
+ armada_37xx_update_reg(®, offset);
+ mask = BIT(offset);
+ regmap_read(info->regmap, reg, &val);
+
+ return !(val & mask);
+}
+
+static int armada_37xx_gpio_direction_output(struct gpio_chip *chip,
+ unsigned int offset, int value)
+{
+ struct armada_37xx_pinctrl *info = gpiochip_get_data(chip);
unsigned int reg = OUTPUT_EN;
unsigned int mask;
- if (offset >= GPIO_PER_REG) {
- offset -= GPIO_PER_REG;
- reg += sizeof(u32);
- }
+ armada_37xx_update_reg(®, offset);
mask = BIT(offset);
return regmap_update_bits(info->regmap, reg, mask, mask);
}
+static int armada_37xx_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+ struct armada_37xx_pinctrl *info = gpiochip_get_data(chip);
+ unsigned int reg = INPUT_VAL;
+ unsigned int val, mask;
+
+ armada_37xx_update_reg(®, offset);
+ mask = BIT(offset);
+
+ regmap_read(info->regmap, reg, &val);
+
+ return (val & mask) != 0;
+}
+
+static void armada_37xx_gpio_set(struct gpio_chip *chip, unsigned int offset,
+ int value)
+{
+ struct armada_37xx_pinctrl *info = gpiochip_get_data(chip);
+ unsigned int reg = OUTPUT_VAL;
+ unsigned int mask, val;
+
+ armada_37xx_update_reg(®, offset);
+ mask = BIT(offset);
+ val = value ? mask : 0;
+
+ regmap_update_bits(info->regmap, reg, mask, val);
+}
+
static int armada_37xx_pmx_gpio_set_direction(struct pinctrl_dev *pctldev,
struct pinctrl_gpio_range *range,
unsigned int offset, bool input)
{
struct armada_37xx_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+ struct gpio_chip *chip = range->gc;
dev_dbg(info->dev, "gpio_direction for pin %u as %s-%d to %s\n",
offset, range->name, offset, input ? "input" : "output");
if (input)
- armada_37xx_pmx_direction_input(info, offset);
+ armada_37xx_gpio_direction_input(chip, offset);
else
- armada_37xx_pmx_direction_output(info, offset, 0);
+ armada_37xx_gpio_direction_output(chip, offset, 0);
return 0;
}
@@ -404,6 +457,49 @@ static const struct pinmux_ops armada_37xx_pmx_ops = {
.gpio_set_direction = armada_37xx_pmx_gpio_set_direction,
};
+static const struct gpio_chip armada_37xx_gpiolib_chip = {
+ .request = gpiochip_generic_request,
+ .free = gpiochip_generic_free,
+ .set = armada_37xx_gpio_set,
+ .get = armada_37xx_gpio_get,
+ .get_direction = armada_37xx_gpio_get_direction,
+ .direction_input = armada_37xx_gpio_direction_input,
+ .direction_output = armada_37xx_gpio_direction_output,
+ .owner = THIS_MODULE,
+};
+
+static int armada_37xx_gpiochip_register(struct platform_device *pdev,
+ struct armada_37xx_pinctrl *info)
+{
+ struct device_node *np;
+ struct gpio_chip *gc;
+ int ret = -ENODEV;
+
+ for_each_child_of_node(info->dev->of_node, np) {
+ if (of_find_property(np, "gpio-controller", NULL)) {
+ ret = 0;
+ break;
+ }
+ };
+ if (ret)
+ return ret;
+
+ info->gpio_chip = armada_37xx_gpiolib_chip;
+
+ gc = &info->gpio_chip;
+ gc->ngpio = info->data->nr_pins;
+ gc->parent = &pdev->dev;
+ gc->base = -1;
+ gc->of_node = np;
+ gc->label = info->data->name;
+
+ ret = devm_gpiochip_add_data(&pdev->dev, gc, info);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
/**
* armada_37xx_add_function() - Add a new function to the list
* @funcs: array of function to add the new one
@@ -632,6 +728,10 @@ static int __init armada_37xx_pinctrl_probe(struct platform_device *pdev)
if (ret)
return ret;
+ ret = armada_37xx_gpiochip_register(pdev, info);
+ if (ret)
+ return ret;
+
platform_set_drvdata(pdev, info);
return 0;
--
git-series 0.9.1
Start to populate the device tree of the Armada 37xx with the pincontrol
configuration used on the board providing a dts.
Signed-off-by: Gregory CLEMENT <[email protected]>
---
arch/arm64/boot/dts/marvell/armada-3720-db.dts | 8 +++++-
arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 31 +++++++++++++++++++-
2 files changed, 39 insertions(+)
diff --git a/arch/arm64/boot/dts/marvell/armada-3720-db.dts b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
index 86602c907a61..e749c5727490 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-db.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
@@ -63,6 +63,8 @@
};
&i2c0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c1_pins>;
status = "okay";
};
@@ -73,6 +75,8 @@
&spi0 {
status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&spi_quad_pins>;
m25p80@0 {
compatible = "jedec,spi-nor";
@@ -103,6 +107,8 @@
/* Exported on the micro USB connector CON32 through an FTDI */
&uart0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&uart1_pins>;
status = "okay";
};
@@ -128,6 +134,8 @@
};
ð0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&rgmii_pins>;
phy-mode = "rgmii-id";
phy = <&phy0>;
status = "okay";
diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index c02b13479458..2ac25f54d01d 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -186,6 +186,31 @@
clock-output-names = "xtal";
#clock-cells = <0>;
};
+
+ spi_quad_pins: spi-quad-pins {
+ groups = "spi_quad";
+ function = "spi";
+ };
+
+ i2c1_pins: i2c1-pins {
+ groups = "i2c1";
+ function = "i2c";
+ };
+
+ i2c2_pins: i2c2-pins {
+ groups = "i2c2";
+ function = "i2c";
+ };
+
+ uart1_pins: uart1-pins {
+ groups = "uart1";
+ function = "uart";
+ };
+
+ uart2_pins: uart2-pins {
+ groups = "uart2";
+ function = "uart";
+ };
};
pinctrl_sb: pinctrl-sb@18800 {
@@ -203,6 +228,12 @@
<GIC_SPI 157 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 156 IRQ_TYPE_LEVEL_HIGH>;
};
+
+ rgmii_pins: mii-pins {
+ groups = "rgmii";
+ function = "mii";
+ };
+
};
eth0: ethernet@30000 {
--
git-series 0.9.1
Document the device tree binding for the pin controllers found on the
Armada 37xx SoCs.
Update the binding documention of the xtal clk which is a subnode of this
syscon node.
Signed-off-by: Gregory CLEMENT <[email protected]>
---
Documentation/devicetree/bindings/clock/armada3700-xtal-clock.txt | 7 +--
Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt | 183 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 187 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt
diff --git a/Documentation/devicetree/bindings/clock/armada3700-xtal-clock.txt b/Documentation/devicetree/bindings/clock/armada3700-xtal-clock.txt
index a88f1f05fbd6..4c0807f28cfa 100644
--- a/Documentation/devicetree/bindings/clock/armada3700-xtal-clock.txt
+++ b/Documentation/devicetree/bindings/clock/armada3700-xtal-clock.txt
@@ -5,6 +5,7 @@ reading the gpio latch register.
This node must be a subnode of the node exposing the register address
of the GPIO block where the gpio latch is located.
+See Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt
Required properties:
- compatible : shall be one of the following:
@@ -16,9 +17,9 @@ Optional properties:
output names ("xtal")
Example:
-gpio1: gpio@13800 {
- compatible = "marvell,armada-3700-gpio", "syscon", "simple-mfd";
- reg = <0x13800 0x1000>;
+pinctrl_nb: pinctrl-nb@13800 {
+ compatible = "armada3710-nb-pinctrl", "syscon", "simple-mfd";
+ reg = <0x13800 0x100>, <0x13C00 0x20>;
xtalclk: xtal-clk {
compatible = "marvell,armada-3700-xtal-clock";
diff --git a/Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt
new file mode 100644
index 000000000000..2eda81e0bca1
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt
@@ -0,0 +1,183 @@
+* Marvell Armada 37xx SoC pin and gpio controller
+
+Each Armada 37xx SoC come with two pin and gpio controller one for the
+south bridge and the other for the north bridge.
+
+Inside this set of register the gpio latch allows exposing some
+configuration of the SoC and especially the clock frequency of the
+xtal. Hence, this node is a represent as syscon allowing sharing the
+register between multiple hardware block.
+
+GPIO and pin controller:
+------------------------
+
+Main node:
+
+Refer to pinctrl-bindings.txt in this directory for details of the
+common pinctrl bindings used by client devices, including the meaning
+of the phrase "pin configuration node".
+
+Required properties for pinctrl driver:
+
+- compatible: "marvell,armada3710-sb-pinctrl", "syscon, "simple-mfd"
+ for the south bridge
+ "marvell,armada3710-nb-pinctrl", "syscon, "simple-mfd"
+ for the north bridge
+- reg: The first set of register are for pinctrl/gpio and the second
+ set for the interrupt controller
+- interrupts: list of the interrupt use by the gpio
+
+Available groups and functions for the North bridge:
+
+group: jtag
+ - pins 20-24
+ - functions jtag, gpio
+
+group sdio0
+ - pins 8-10
+ - functions sdio, gpio
+
+group emmc_nb
+ - pins 27-35
+ - functions emmc, gpio
+
+group pwm0
+ - pin 11 (GPIO1-11)
+ - functions pwm, gpio
+
+group pwm1
+ - pin 12
+ - functions pwm, gpio
+
+group pwm2
+ - pin 13
+ - functions pwm, gpio
+
+group pwm3
+ - pin 14
+ - functions pwm, gpio
+
+group pmic1
+ - pin 17
+ - functions pmic, gpio
+
+group pmic0
+ - pin 16
+ - functions pmic, gpio
+
+group i2c2
+ - pins 2-3
+ - functions i2c, gpio
+
+group i2c1
+ - pins 0-1
+ - functions i2c, gpio
+
+group spi_cs1
+ - pin 17
+ - functions spi, gpio
+
+group spi_cs2
+ - pin 18
+ - functions spi, gpio
+
+group spi_cs3
+ - pin 19
+ - functions spi, gpio
+
+group onewire
+ - pin 4
+ - functions onewire, gpio
+
+group uart1
+ - pins 25-26
+ - functions uart, gpio
+
+group spi_quad
+ - pins 15-16
+ - functions spi, gpio
+
+group uart_2
+ - pins 9-10
+ - functions uart, gpio
+
+Available groups and functions for the South bridge:
+
+group usb32_drvvbus0
+ - pin 36
+ - functions drvbus, gpio
+
+group usb2_drvvbus1
+ - pin 37
+ - functions drvbus, gpio
+
+group sdio_sb
+ - pins 60-64
+ - functions sdio, gpio
+
+group rgmii
+ - pins 42-55
+ - functions mii, gpio
+
+group pcie1
+ - pins 39-40
+ - functions pcie, gpio
+
+group ptp
+ - pins 56-58
+ - functions ptp, gpio
+
+group ptp_clk
+ - pin 57
+ - functions ptp, mii
+
+group ptp_trig
+ - pin 58
+ - functions ptp, mii
+
+group mii_col
+ - pin 59
+ - functions mii, mii_err
+
+GPIO subnode:
+
+Please refer to gpio.txt in this directory for details of gpio-ranges property
+and the common GPIO bindings used by client devices.
+
+Required properties for gpio driver under the gpio subnode:
+- interrupts: List of interrupt specifier for the controllers interrupt.
+- gpio-controller: Marks the device node as a gpio controller.
+- #gpio-cells: Should be 2. The first cell is the GPIO number and the
+ second cell specifies GPIO flags, as defined in
+ <dt-bindings/gpio/gpio.h>. Only the GPIO_ACTIVE_HIGH and
+ GPIO_ACTIVE_LOW flags are supported.
+- gpio-ranges: Range of pins managed by the GPIO controller.
+
+Xtal Clock bindings for Marvell Armada 37xx SoCs
+------------------------------------------------
+
+see Documentation/devicetree/bindings/clock/armada3700-xtal-clock.txt
+
+
+Example:
+pinctrl_sb: pinctrl-sb@18800 {
+ compatible = "marvell,armada3710-sb-pinctrl", "syscon", "simple-mfd";
+ reg = <0x18800 0x100>, <0x18C00 0x20>;
+ gpiosb: gpiosb {
+ #gpio-cells = <2>;
+ gpio-ranges = <&pinctrl_sb 0 0 29>;
+ gpio-controller;
+ interrupts =
+ <GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 159 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 158 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 157 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 156 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
+ rgmii_pins: mii-pins {
+ groups = "rgmii";
+ function = "mii";
+ };
+
+};
--
git-series 0.9.1
Add the nodes for the two pin controller present in the Armada 37xx SoCs.
Initially the node was named gpio1 using the same name that for the
register range in the datasheet. However renaming it pinctr_nb (nb for
North Bridge) makes more sens.
Signed-off-by: Gregory CLEMENT <[email protected]>
---
arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 42 +++++++++++++++++++--
1 file changed, 39 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index b48d668a6ab6..c02b13479458 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -157,10 +157,29 @@
#clock-cells = <1>;
};
- gpio1: gpio@13800 {
- compatible = "marvell,mvebu-gpio-3700",
+ pinctrl_nb: pinctrl-nb@13800 {
+ compatible = "marvell,armada3710-nb-pinctrl",
"syscon", "simple-mfd";
- reg = <0x13800 0x500>;
+ reg = <0x13800 0x100>, <0x13C00 0x20>;
+ gpionb: gpionb {
+ #gpio-cells = <2>;
+ gpio-ranges = <&pinctrl_nb 0 0 36>;
+ gpio-controller;
+ interrupts =
+ <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 53 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 54 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 152 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
+
+ };
xtalclk: xtal-clk {
compatible = "marvell,armada-3700-xtal-clock";
@@ -169,6 +188,23 @@
};
};
+ pinctrl_sb: pinctrl-sb@18800 {
+ compatible = "marvell,armada3710-sb-pinctrl",
+ "syscon", "simple-mfd";
+ reg = <0x18800 0x100>, <0x18C00 0x20>;
+ gpiosb: gpiosb {
+ #gpio-cells = <2>;
+ gpio-ranges = <&pinctrl_sb 0 0 29>;
+ gpio-controller;
+ interrupts =
+ <GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 159 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 158 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 157 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 156 IRQ_TYPE_LEVEL_HIGH>;
+ };
+ };
+
eth0: ethernet@30000 {
compatible = "marvell,armada-3700-neta";
reg = <0x30000 0x4000>;
--
git-series 0.9.1
This commit makes sure the driver for the Armada 37xx pin controller is
enabled.
Signed-off-by: Gregory CLEMENT <[email protected]>
---
arch/arm64/Kconfig.platforms | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 129cc5ae4091..9aa71a3f3f50 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -103,8 +103,13 @@ config ARCH_MVEBU
select ARMADA_AP806_SYSCON
select ARMADA_CP110_SYSCON
select ARMADA_37XX_CLK
+ select GPIOLIB
+ select GPIOLIB_IRQCHIP
select MVEBU_ODMI
select MVEBU_PIC
+ select OF_GPIO
+ select PINCTRL
+ select PINCTRL_ARMADA_37XX
help
This enables support for Marvell EBU familly, including:
- Armada 3700 SoC Family
--
git-series 0.9.1
Argh, I sill have the typo in the title of this patch! :(
If you are going to apply it could you fix it, else it will be fixed in
the next version.
Sorry,
Gregory
On mer., avril 05 2017, Gregory CLEMENT <[email protected]> wrote:
> The Armada 37xx SoCs can handle interrupt through GPIO. However it can
> only manage the edge ones.
>
> The way the interrupt are managed are classical so we can use the generic
> interrupt chip model.
>
> The only unusual "feature" is that many interrupts are connected to the
> parent interrupt controller. But we do not take advantage of this and use
> the chained irq with all of them.
>
> Signed-off-by: Gregory CLEMENT <[email protected]>
> ---
> drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 221 +++++++++++++++++++++-
> 1 file changed, 221 insertions(+)
>
> diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> index 5c96f5558310..7356516e0921 100644
> --- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> +++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> @@ -13,7 +13,9 @@
> #include <linux/gpio/driver.h>
> #include <linux/mfd/syscon.h>
> #include <linux/of.h>
> +#include <linux/of_address.h>
> #include <linux/of_device.h>
> +#include <linux/of_irq.h>
> #include <linux/pinctrl/pinconf-generic.h>
> #include <linux/pinctrl/pinconf.h>
> #include <linux/pinctrl/pinctrl.h>
> @@ -30,6 +32,11 @@
> #define OUTPUT_CTL 0x20
> #define SELECTION 0x30
>
> +#define IRQ_EN 0x0
> +#define IRQ_POL 0x08
> +#define IRQ_STATUS 0x10
> +#define IRQ_WKUP 0x18
> +
> #define NB_FUNCS 2
> #define GPIO_PER_REG 32
>
> @@ -75,9 +82,12 @@ struct armada_37xx_pmx_func {
>
> struct armada_37xx_pinctrl {
> struct regmap *regmap;
> + void __iomem *base;
> const struct armada_37xx_pin_data *data;
> struct device *dev;
> struct gpio_chip gpio_chip;
> + struct irq_chip irq_chip;
> + spinlock_t irq_lock;
> struct pinctrl_desc pctl;
> struct pinctrl_dev *pctl_dev;
> struct armada_37xx_pin_group *groups;
> @@ -346,6 +356,14 @@ static int armada_37xx_pmx_set(struct pinctrl_dev *pctldev,
> return armada_37xx_pmx_set_by_name(pctldev, name, grp);
> }
>
> +static inline void armada_37xx_irq_update_reg(unsigned int *reg,
> + struct irq_data *d)
> +{
> + int offset = irqd_to_hwirq(d);
> +
> + armada_37xx_update_reg(reg, offset);
> +}
> +
> static int armada_37xx_gpio_direction_input(struct gpio_chip *chip,
> unsigned int offset)
> {
> @@ -468,6 +486,206 @@ static const struct gpio_chip armada_37xx_gpiolib_chip = {
> .owner = THIS_MODULE,
> };
>
> +void armada_37xx_irq_ack(struct irq_data *d)
> +{
> + struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
> + struct armada_37xx_pinctrl *info = gpiochip_get_data(chip);
> + u32 reg = IRQ_STATUS, mask = d->mask;
> + unsigned long flags;
> +
> + armada_37xx_irq_update_reg(®, d);
> + spin_lock_irqsave(&info->irq_lock, flags);
> + writel(mask, info->base + reg);
> + spin_unlock_irqrestore(&info->irq_lock, flags);
> +}
> +
> +void armada_37xx_irq_mask(struct irq_data *d)
> +{
> + struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
> + struct armada_37xx_pinctrl *info = gpiochip_get_data(chip);
> + u32 val, reg = IRQ_EN, mask = d->mask;
> + unsigned long flags;
> +
> + armada_37xx_irq_update_reg(®, d);
> + spin_lock_irqsave(&info->irq_lock, flags);
> + val = readl(info->base + reg);
> + writel(val & ~mask, info->base + reg);
> + spin_unlock_irqrestore(&info->irq_lock, flags);
> +}
> +
> +void armada_37xx_irq_unmask(struct irq_data *d)
> +{
> + struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
> + struct armada_37xx_pinctrl *info = gpiochip_get_data(chip);
> + u32 val, reg = IRQ_EN, mask = d->mask;
> + unsigned long flags;
> +
> + armada_37xx_irq_update_reg(®, d);
> + spin_lock_irqsave(&info->irq_lock, flags);
> + val = readl(info->base + reg);
> + writel(val | mask, info->base + reg);
> + spin_unlock_irqrestore(&info->irq_lock, flags);
> +}
> +
> +static int armada_37xx_irq_set_wake(struct irq_data *d, unsigned int on)
> +{
> + struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
> + struct armada_37xx_pinctrl *info = gpiochip_get_data(chip);
> + u32 val, reg = IRQ_WKUP, mask = d->mask;
> + unsigned long flags;
> +
> + armada_37xx_irq_update_reg(®, d);
> + spin_lock_irqsave(&info->irq_lock, flags);
> + val = readl(info->base + reg);
> + if (on)
> + val |= mask;
> + else
> + val &= ~mask;
> + writel(val, info->base + reg);
> + spin_unlock_irqrestore(&info->irq_lock, flags);
> +
> + return 0;
> +}
> +
> +static int armada_37xx_irq_set_type(struct irq_data *d, unsigned int type)
> +{
> + struct gpio_chip *chip = irq_data_get_irq_chip_data(d);
> + struct armada_37xx_pinctrl *info = gpiochip_get_data(chip);
> + u32 val, reg = IRQ_POL, mask = d->mask;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&info->irq_lock, flags);
> + armada_37xx_irq_update_reg(®, d);
> + val = readl(info->base + reg);
> + switch (type) {
> + case IRQ_TYPE_EDGE_RISING:
> + val &= ~mask;
> + break;
> + case IRQ_TYPE_EDGE_FALLING:
> + val |= mask;
> + break;
> + default:
> + spin_unlock_irqrestore(&info->irq_lock, flags);
> + return -EINVAL;
> + }
> + writel(val, info->base + reg);
> + spin_unlock_irqrestore(&info->irq_lock, flags);
> +
> + return 0;
> +}
> +
> +
> +static void armada_37xx_irq_handler(struct irq_desc *desc)
> +{
> + struct gpio_chip *gc = irq_desc_get_handler_data(desc);
> + struct irq_chip *chip = irq_desc_get_chip(desc);
> + struct armada_37xx_pinctrl *info = gpiochip_get_data(gc);
> + struct irq_domain *d = gc->irqdomain;
> + int i;
> +
> + chained_irq_enter(chip, desc);
> + for (i = 0; i <= d->revmap_size / GPIO_PER_REG; i++) {
> + u32 status;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&info->irq_lock, flags);
> + status = readl_relaxed(info->base + IRQ_STATUS + 4 * i);
> + /* Manage only the interrupt that was enabled */
> + status &= readl_relaxed(info->base + IRQ_EN + 4 * i);
> + spin_unlock_irqrestore(&info->irq_lock, flags);
> + while (status) {
> + u32 hwirq = ffs(status) - 1;
> + u32 virq = irq_find_mapping(d, hwirq +
> + i * GPIO_PER_REG);
> +
> + generic_handle_irq(virq);
> + status &= ~BIT(hwirq);
> + }
> + }
> + chained_irq_exit(chip, desc);
> +}
> +
> +static int armada_37xx_irqchip_register(struct platform_device *pdev,
> + struct armada_37xx_pinctrl *info)
> +{
> + struct device_node *np = info->dev->of_node;
> + int nrirqs = info->data->nr_pins;
> + struct gpio_chip *gc = &info->gpio_chip;
> + struct irq_chip *irqchip = &info->irq_chip;
> + struct resource res;
> + int ret = -ENODEV, i, nr_irq_parent;
> +
> + for_each_child_of_node(info->dev->of_node, np) {
> + if (of_find_property(np, "gpio-controller", NULL)) {
> + ret = 0;
> + break;
> + }
> + };
> + if (ret)
> + return ret;
> +
> + nr_irq_parent = of_irq_count(np);
> + spin_lock_init(&info->irq_lock);
> +
> + if (!nr_irq_parent) {
> + dev_err(&pdev->dev, "Invalid or no IRQ\n");
> + return 0;
> + }
> +
> + if (of_address_to_resource(info->dev->of_node, 1, &res)) {
> + dev_err(info->dev, "cannot find IO resource\n");
> + return -ENOENT;
> + }
> +
> + info->base = devm_ioremap_resource(info->dev, &res);
> + if (IS_ERR(info->base))
> + return PTR_ERR(info->base);
> +
> + irqchip->irq_ack = armada_37xx_irq_ack;
> + irqchip->irq_mask = armada_37xx_irq_mask;
> + irqchip->irq_unmask = armada_37xx_irq_unmask;
> + irqchip->irq_set_wake = armada_37xx_irq_set_wake;
> + irqchip->irq_set_type = armada_37xx_irq_set_type;
> + irqchip->name = info->data->name;
> +
> + ret = gpiochip_irqchip_add(gc, irqchip, 0,
> + handle_edge_irq, IRQ_TYPE_NONE);
> + if (ret) {
> + dev_info(&pdev->dev, "could not add irqchip\n");
> + return ret;
> + }
> +
> + /*
> + * Many interrupts are connected to the parent interrupt
> + * controller. But we do not take advantage of this and use
> + * the chained irq with all of them.
> + */
> + for (i = 0; i < nrirqs; i++) {
> + struct irq_data *d = irq_get_irq_data(gc->irq_base + i);
> +
> + /*
> + * The mask field is a "precomputed bitmask for
> + * accessing the chip registers" which was introduced
> + * for the generic irqchip framework. As we don't use
> + * this framework, we can reuse this field for our own
> + * usage.
> + */
> + d->mask = BIT(i % GPIO_PER_REG);
> + }
> +
> + for (i = 0; i < nr_irq_parent; i++) {
> + int irq = irq_of_parse_and_map(np, i);
> +
> + if (irq < 0)
> + continue;
> +
> + gpiochip_set_chained_irqchip(gc, irqchip, irq,
> + armada_37xx_irq_handler);
> + }
> +
> + return 0;
> +}
> +
> static int armada_37xx_gpiochip_register(struct platform_device *pdev,
> struct armada_37xx_pinctrl *info)
> {
> @@ -496,6 +714,9 @@ static int armada_37xx_gpiochip_register(struct platform_device *pdev,
> ret = devm_gpiochip_add_data(&pdev->dev, gc, info);
> if (ret)
> return ret;
> + ret = armada_37xx_irqchip_register(pdev, info);
> + if (ret)
> + return ret;
>
> return 0;
> }
> --
> git-series 0.9.1
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
On Wed, Apr 05, 2017 at 05:18:02PM +0200, Gregory CLEMENT wrote:
> Document the device tree binding for the pin controllers found on the
> Armada 37xx SoCs.
>
> Update the binding documention of the xtal clk which is a subnode of this
> syscon node.
>
> Signed-off-by: Gregory CLEMENT <[email protected]>
> ---
> Documentation/devicetree/bindings/clock/armada3700-xtal-clock.txt | 7 +--
> Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt | 183 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 187 insertions(+), 3 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt
>
> diff --git a/Documentation/devicetree/bindings/clock/armada3700-xtal-clock.txt b/Documentation/devicetree/bindings/clock/armada3700-xtal-clock.txt
> index a88f1f05fbd6..4c0807f28cfa 100644
> --- a/Documentation/devicetree/bindings/clock/armada3700-xtal-clock.txt
> +++ b/Documentation/devicetree/bindings/clock/armada3700-xtal-clock.txt
> @@ -5,6 +5,7 @@ reading the gpio latch register.
>
> This node must be a subnode of the node exposing the register address
> of the GPIO block where the gpio latch is located.
> +See Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt
>
> Required properties:
> - compatible : shall be one of the following:
> @@ -16,9 +17,9 @@ Optional properties:
> output names ("xtal")
>
> Example:
> -gpio1: gpio@13800 {
> - compatible = "marvell,armada-3700-gpio", "syscon", "simple-mfd";
> - reg = <0x13800 0x1000>;
> +pinctrl_nb: pinctrl-nb@13800 {
> + compatible = "armada3710-nb-pinctrl", "syscon", "simple-mfd";
> + reg = <0x13800 0x100>, <0x13C00 0x20>;
>
> xtalclk: xtal-clk {
> compatible = "marvell,armada-3700-xtal-clock";
> diff --git a/Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt
> new file mode 100644
> index 000000000000..2eda81e0bca1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/marvell,armada-37xx-pinctrl.txt
> @@ -0,0 +1,183 @@
> +* Marvell Armada 37xx SoC pin and gpio controller
> +
> +Each Armada 37xx SoC come with two pin and gpio controller one for the
> +south bridge and the other for the north bridge.
> +
> +Inside this set of register the gpio latch allows exposing some
> +configuration of the SoC and especially the clock frequency of the
> +xtal. Hence, this node is a represent as syscon allowing sharing the
> +register between multiple hardware block.
> +
> +GPIO and pin controller:
> +------------------------
> +
> +Main node:
> +
> +Refer to pinctrl-bindings.txt in this directory for details of the
> +common pinctrl bindings used by client devices, including the meaning
> +of the phrase "pin configuration node".
> +
> +Required properties for pinctrl driver:
> +
> +- compatible: "marvell,armada3710-sb-pinctrl", "syscon, "simple-mfd"
> + for the south bridge
> + "marvell,armada3710-nb-pinctrl", "syscon, "simple-mfd"
> + for the north bridge
> +- reg: The first set of register are for pinctrl/gpio and the second
> + set for the interrupt controller
> +- interrupts: list of the interrupt use by the gpio
> +
> +Available groups and functions for the North bridge:
> +
> +group: jtag
> + - pins 20-24
> + - functions jtag, gpio
> +
> +group sdio0
> + - pins 8-10
> + - functions sdio, gpio
> +
> +group emmc_nb
> + - pins 27-35
> + - functions emmc, gpio
> +
> +group pwm0
> + - pin 11 (GPIO1-11)
> + - functions pwm, gpio
> +
> +group pwm1
> + - pin 12
> + - functions pwm, gpio
> +
> +group pwm2
> + - pin 13
> + - functions pwm, gpio
> +
> +group pwm3
> + - pin 14
> + - functions pwm, gpio
> +
> +group pmic1
> + - pin 17
> + - functions pmic, gpio
> +
> +group pmic0
> + - pin 16
> + - functions pmic, gpio
> +
> +group i2c2
> + - pins 2-3
> + - functions i2c, gpio
> +
> +group i2c1
> + - pins 0-1
> + - functions i2c, gpio
> +
> +group spi_cs1
> + - pin 17
> + - functions spi, gpio
> +
> +group spi_cs2
> + - pin 18
> + - functions spi, gpio
> +
> +group spi_cs3
> + - pin 19
> + - functions spi, gpio
> +
> +group onewire
> + - pin 4
> + - functions onewire, gpio
> +
> +group uart1
> + - pins 25-26
> + - functions uart, gpio
> +
> +group spi_quad
> + - pins 15-16
> + - functions spi, gpio
> +
> +group uart_2
> + - pins 9-10
> + - functions uart, gpio
> +
> +Available groups and functions for the South bridge:
> +
> +group usb32_drvvbus0
> + - pin 36
> + - functions drvbus, gpio
> +
> +group usb2_drvvbus1
> + - pin 37
> + - functions drvbus, gpio
> +
> +group sdio_sb
> + - pins 60-64
> + - functions sdio, gpio
> +
> +group rgmii
> + - pins 42-55
> + - functions mii, gpio
> +
> +group pcie1
> + - pins 39-40
> + - functions pcie, gpio
> +
> +group ptp
> + - pins 56-58
> + - functions ptp, gpio
> +
> +group ptp_clk
> + - pin 57
> + - functions ptp, mii
> +
> +group ptp_trig
> + - pin 58
> + - functions ptp, mii
> +
> +group mii_col
> + - pin 59
> + - functions mii, mii_err
> +
> +GPIO subnode:
> +
> +Please refer to gpio.txt in this directory for details of gpio-ranges property
> +and the common GPIO bindings used by client devices.
> +
> +Required properties for gpio driver under the gpio subnode:
> +- interrupts: List of interrupt specifier for the controllers interrupt.
> +- gpio-controller: Marks the device node as a gpio controller.
> +- #gpio-cells: Should be 2. The first cell is the GPIO number and the
> + second cell specifies GPIO flags, as defined in
> + <dt-bindings/gpio/gpio.h>. Only the GPIO_ACTIVE_HIGH and
> + GPIO_ACTIVE_LOW flags are supported.
> +- gpio-ranges: Range of pins managed by the GPIO controller.
> +
> +Xtal Clock bindings for Marvell Armada 37xx SoCs
> +------------------------------------------------
> +
> +see Documentation/devicetree/bindings/clock/armada3700-xtal-clock.txt
> +
> +
> +Example:
> +pinctrl_sb: pinctrl-sb@18800 {
Just pinctrl@...
> + compatible = "marvell,armada3710-sb-pinctrl", "syscon", "simple-mfd";
> + reg = <0x18800 0x100>, <0x18C00 0x20>;
> + gpiosb: gpiosb {
gpio {
With that,
Acked-by: Rob Herring <[email protected]>
> + #gpio-cells = <2>;
> + gpio-ranges = <&pinctrl_sb 0 0 29>;
> + gpio-controller;
> + interrupts =
> + <GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 159 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 158 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 157 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 156 IRQ_TYPE_LEVEL_HIGH>;
> + };
> +
> + rgmii_pins: mii-pins {
> + groups = "rgmii";
> + function = "mii";
> + };
> +
> +};
> --
> git-series 0.9.1
Hi Linus,
On lun., avril 10 2017, Rob Herring <[email protected]> wrote:
> On Wed, Apr 05, 2017 at 05:18:02PM +0200, Gregory CLEMENT wrote:
>> +Example:
>> +pinctrl_sb: pinctrl-sb@18800 {
>
> Just pinctrl@...
>
>> + compatible = "marvell,armada3710-sb-pinctrl", "syscon", "simple-mfd";
>> + reg = <0x18800 0x100>, <0x18C00 0x20>;
>> + gpiosb: gpiosb {
>
> gpio {
>
> With that,
>
> Acked-by: Rob Herring <[email protected]>
Could you do this change while applying the patches, or do you want a
new series?
Also, if the binding is also OK for you, do you agree that I apply the
last 2 patches on the mvebu trees? (of course with the same changes
asked by Rob)
Thanks,
Gregory
>
>
>> + #gpio-cells = <2>;
>> + gpio-ranges = <&pinctrl_sb 0 0 29>;
>> + gpio-controller;
>> + interrupts =
>> + <GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 159 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 158 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 157 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 156 IRQ_TYPE_LEVEL_HIGH>;
>> + };
>> +
>> + rgmii_pins: mii-pins {
>> + groups = "rgmii";
>> + function = "mii";
>> + };
>> +
>> +};
>> --
>> git-series 0.9.1
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
On Wed, Apr 5, 2017 at 5:18 PM, Gregory CLEMENT
<[email protected]> wrote:
> Document the device tree binding for the pin controllers found on the
> Armada 37xx SoCs.
>
> Update the binding documention of the xtal clk which is a subnode of this
> syscon node.
>
> Signed-off-by: Gregory CLEMENT <[email protected]>
Patch applied, fixed up Rob's comment and added his ACK in the process.
Yours,
Linus Walleij
On Wed, Apr 5, 2017 at 5:18 PM, Gregory CLEMENT
<[email protected]> wrote:
> GPIO management is pretty simple and is part of the same IP than the pin
> controller for the Armada 37xx SoCs. This patch adds the GPIO support to
> the pinctrl-armada-37xx.c file, it also allows sharing common functions
> between the gpiolib and the pinctrl drivers.
>
> Signed-off-by: Gregory CLEMENT <[email protected]>
Patch applied.
Yours,
Linus Walleij
On Wed, Apr 5, 2017 at 5:18 PM, Gregory CLEMENT
<[email protected]> wrote:
> The Armada 37xx SoCs can handle interrupt through GPIO. However it can
> only manage the edge ones.
>
> The way the interrupt are managed are classical so we can use the generic
> interrupt chip model.
>
> The only unusual "feature" is that many interrupts are connected to the
> parent interrupt controller. But we do not take advantage of this and use
> the chained irq with all of them.
>
> Signed-off-by: Gregory CLEMENT <[email protected]>
There are some issues with this patch.
First:
You need to add
select GPIOLIB_IRQCHIP
to the Kconfig entry. It's only working in your setup
because something else is selecting this for you, probably.
At all places like this:
> + u32 mask = d->mask;
(...)
> + if (on)
> + val |= mask;
> + else
> + val &= ~mask;
Isn't it simpler to just use d->mask directly in the code and skip the local
variable?
if (on)
val |= d->mask;
(...)
> +static void armada_37xx_irq_handler(struct irq_desc *desc)
> +{
> + struct gpio_chip *gc = irq_desc_get_handler_data(desc);
> + struct irq_chip *chip = irq_desc_get_chip(desc);
> + struct armada_37xx_pinctrl *info = gpiochip_get_data(gc);
> + struct irq_domain *d = gc->irqdomain;
> + int i;
> +
> + chained_irq_enter(chip, desc);
> + for (i = 0; i <= d->revmap_size / GPIO_PER_REG; i++) {
> + u32 status;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&info->irq_lock, flags);
> + status = readl_relaxed(info->base + IRQ_STATUS + 4 * i);
> + /* Manage only the interrupt that was enabled */
> + status &= readl_relaxed(info->base + IRQ_EN + 4 * i);
> + spin_unlock_irqrestore(&info->irq_lock, flags);
> + while (status) {
> + u32 hwirq = ffs(status) - 1;
> + u32 virq = irq_find_mapping(d, hwirq +
> + i * GPIO_PER_REG);
> +
> + generic_handle_irq(virq);
> + status &= ~BIT(hwirq);
> + }
You hae a problem here is a new IRQ appears while you are inside
of this loop. You need to re-read the status register for each iteration
(and &= with the IRQ_EN I guess).
> +static int armada_37xx_irqchip_register(struct platform_device *pdev,
> + struct armada_37xx_pinctrl *info)
> +{
> + struct device_node *np = info->dev->of_node;
> + int nrirqs = info->data->nr_pins;
> + struct gpio_chip *gc = &info->gpio_chip;
> + struct irq_chip *irqchip = &info->irq_chip;
> + struct resource res;
> + int ret = -ENODEV, i, nr_irq_parent;
> +
This warrants a comment:
/* Check if we have at least one gpio-controller child node */
> + for_each_child_of_node(info->dev->of_node, np) {
> + if (of_find_property(np, "gpio-controller", NULL)) {
> + ret = 0;
> + break;
> + }
Rewrite:
if (of_property_read_bool(np, "gpio-controller"))
> + };
> + if (ret)
> + return ret;
> +
> + nr_irq_parent = of_irq_count(np);
> + spin_lock_init(&info->irq_lock);
> +
> + if (!nr_irq_parent) {
> + dev_err(&pdev->dev, "Invalid or no IRQ\n");
> + return 0;
> + }
> +
> + if (of_address_to_resource(info->dev->of_node, 1, &res)) {
> + dev_err(info->dev, "cannot find IO resource\n");
> + return -ENOENT;
> + }
> +
> + info->base = devm_ioremap_resource(info->dev, &res);
> + if (IS_ERR(info->base))
> + return PTR_ERR(info->base);
> +
> + irqchip->irq_ack = armada_37xx_irq_ack;
> + irqchip->irq_mask = armada_37xx_irq_mask;
> + irqchip->irq_unmask = armada_37xx_irq_unmask;
> + irqchip->irq_set_wake = armada_37xx_irq_set_wake;
> + irqchip->irq_set_type = armada_37xx_irq_set_type;
> + irqchip->name = info->data->name;
> +
> + ret = gpiochip_irqchip_add(gc, irqchip, 0,
> + handle_edge_irq, IRQ_TYPE_NONE);
> + if (ret) {
> + dev_info(&pdev->dev, "could not add irqchip\n");
> + return ret;
> + }
> +
> + /*
> + * Many interrupts are connected to the parent interrupt
> + * controller. But we do not take advantage of this and use
> + * the chained irq with all of them.
> + */
> + for (i = 0; i < nrirqs; i++) {
> + struct irq_data *d = irq_get_irq_data(gc->irq_base + i);
> +
> + /*
> + * The mask field is a "precomputed bitmask for
> + * accessing the chip registers" which was introduced
> + * for the generic irqchip framework. As we don't use
> + * this framework, we can reuse this field for our own
> + * usage.
> + */
> + d->mask = BIT(i % GPIO_PER_REG);
> + }
> +
> + for (i = 0; i < nr_irq_parent; i++) {
> + int irq = irq_of_parse_and_map(np, i);
> +
> + if (irq < 0)
> + continue;
> +
> + gpiochip_set_chained_irqchip(gc, irqchip, irq,
> + armada_37xx_irq_handler);
> + }
> +
> + return 0;
> +}
> +
> static int armada_37xx_gpiochip_register(struct platform_device *pdev,
> struct armada_37xx_pinctrl *info)
> {
> @@ -496,6 +714,9 @@ static int armada_37xx_gpiochip_register(struct platform_device *pdev,
> ret = devm_gpiochip_add_data(&pdev->dev, gc, info);
> if (ret)
> return ret;
> + ret = armada_37xx_irqchip_register(pdev, info);
> + if (ret)
> + return ret;
>
> return 0;
> }
> --
> git-series 0.9.1
So I touched something in gmail and it shot off the mail prematurely.
My apologies.
But my review was done anyways.
Yours,
Linus Walleij
Hi Linus,
On lun., avril 24 2017, Linus Walleij <[email protected]> wrote:
> On Wed, Apr 5, 2017 at 5:18 PM, Gregory CLEMENT
> <[email protected]> wrote:
>
>> The Armada 37xx SoCs can handle interrupt through GPIO. However it can
>> only manage the edge ones.
>>
>> The way the interrupt are managed are classical so we can use the generic
>> interrupt chip model.
>>
>> The only unusual "feature" is that many interrupts are connected to the
>> parent interrupt controller. But we do not take advantage of this and use
>> the chained irq with all of them.
>>
>> Signed-off-by: Gregory CLEMENT <[email protected]>
>
> There are some issues with this patch.
>
> First:
> You need to add
> select GPIOLIB_IRQCHIP
> to the Kconfig entry. It's only working in your setup
> because something else is selecting this for you, probably.
It is done in patch 2 "arm64: marvell: enable the Armada 37xx pinctrl
driver".
>
> At all places like this:
>
>> + u32 mask = d->mask;
> (...)
>> + if (on)
>> + val |= mask;
>> + else
>> + val &= ~mask;
>
> Isn't it simpler to just use d->mask directly in the code and skip the local
> variable?
>
> if (on)
> val |= d->mask;
> (...)
Yes sure I cand do it.
>
>> +static void armada_37xx_irq_handler(struct irq_desc *desc)
>> +{
>> + struct gpio_chip *gc = irq_desc_get_handler_data(desc);
>> + struct irq_chip *chip = irq_desc_get_chip(desc);
>> + struct armada_37xx_pinctrl *info = gpiochip_get_data(gc);
>> + struct irq_domain *d = gc->irqdomain;
>> + int i;
>> +
>> + chained_irq_enter(chip, desc);
>> + for (i = 0; i <= d->revmap_size / GPIO_PER_REG; i++) {
>> + u32 status;
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&info->irq_lock, flags);
>> + status = readl_relaxed(info->base + IRQ_STATUS + 4 * i);
>> + /* Manage only the interrupt that was enabled */
>> + status &= readl_relaxed(info->base + IRQ_EN + 4 * i);
>> + spin_unlock_irqrestore(&info->irq_lock, flags);
>> + while (status) {
>> + u32 hwirq = ffs(status) - 1;
>> + u32 virq = irq_find_mapping(d, hwirq +
>> + i * GPIO_PER_REG);
>> +
>> + generic_handle_irq(virq);
>> + status &= ~BIT(hwirq);
>> + }
>
> You hae a problem here is a new IRQ appears while you are inside
> of this loop. You need to re-read the status register for each iteration
> (and &= with the IRQ_EN I guess).
If a new IRQ appears during the loop, then the irq handler will be
called again because the cause of this new IRQ won't have been acked
yet. So I think we're fine here.
>
>> +static int armada_37xx_irqchip_register(struct platform_device *pdev,
>> + struct armada_37xx_pinctrl *info)
>> +{
>> + struct device_node *np = info->dev->of_node;
>> + int nrirqs = info->data->nr_pins;
>> + struct gpio_chip *gc = &info->gpio_chip;
>> + struct irq_chip *irqchip = &info->irq_chip;
>> + struct resource res;
>> + int ret = -ENODEV, i, nr_irq_parent;
>> +
>
> This warrants a comment:
> /* Check if we have at least one gpio-controller child node */
>
OK
>> + for_each_child_of_node(info->dev->of_node, np) {
>> + if (of_find_property(np, "gpio-controller", NULL)) {
>> + ret = 0;
>> + break;
>> + }
>
> Rewrite:
>
> if (of_property_read_bool(np, "gpio-controller"))
>
OK
Gregory
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
Hi,
On mer., avril 05 2017, Gregory CLEMENT <[email protected]> wrote:
> This commit makes sure the driver for the Armada 37xx pin controller is
> enabled.
>
> Signed-off-by: Gregory CLEMENT <[email protected]>
Applied on mvebu/arm64
Thanks,
Gregory
> ---
> arch/arm64/Kconfig.platforms | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> index 129cc5ae4091..9aa71a3f3f50 100644
> --- a/arch/arm64/Kconfig.platforms
> +++ b/arch/arm64/Kconfig.platforms
> @@ -103,8 +103,13 @@ config ARCH_MVEBU
> select ARMADA_AP806_SYSCON
> select ARMADA_CP110_SYSCON
> select ARMADA_37XX_CLK
> + select GPIOLIB
> + select GPIOLIB_IRQCHIP
> select MVEBU_ODMI
> select MVEBU_PIC
> + select OF_GPIO
> + select PINCTRL
> + select PINCTRL_ARMADA_37XX
> help
> This enables support for Marvell EBU familly, including:
> - Armada 3700 SoC Family
> --
> git-series 0.9.1
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
Hi,
On mer., avril 05 2017, Gregory CLEMENT <[email protected]> wrote:
> Add the nodes for the two pin controller present in the Armada 37xx SoCs.
>
> Initially the node was named gpio1 using the same name that for the
> register range in the datasheet. However renaming it pinctr_nb (nb for
> North Bridge) makes more sens.
>
> Signed-off-by: Gregory CLEMENT <[email protected]>
Applied on mvebu/dt64
Thanks,
Gregory
> ---
> arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 42 +++++++++++++++++++--
> 1 file changed, 39 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> index b48d668a6ab6..c02b13479458 100644
> --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> @@ -157,10 +157,29 @@
> #clock-cells = <1>;
> };
>
> - gpio1: gpio@13800 {
> - compatible = "marvell,mvebu-gpio-3700",
> + pinctrl_nb: pinctrl-nb@13800 {
> + compatible = "marvell,armada3710-nb-pinctrl",
> "syscon", "simple-mfd";
> - reg = <0x13800 0x500>;
> + reg = <0x13800 0x100>, <0x13C00 0x20>;
> + gpionb: gpionb {
> + #gpio-cells = <2>;
> + gpio-ranges = <&pinctrl_nb 0 0 36>;
> + gpio-controller;
> + interrupts =
> + <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 53 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 54 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 152 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 153 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 154 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 155 IRQ_TYPE_LEVEL_HIGH>;
> +
> + };
>
> xtalclk: xtal-clk {
> compatible = "marvell,armada-3700-xtal-clock";
> @@ -169,6 +188,23 @@
> };
> };
>
> + pinctrl_sb: pinctrl-sb@18800 {
> + compatible = "marvell,armada3710-sb-pinctrl",
> + "syscon", "simple-mfd";
> + reg = <0x18800 0x100>, <0x18C00 0x20>;
> + gpiosb: gpiosb {
> + #gpio-cells = <2>;
> + gpio-ranges = <&pinctrl_sb 0 0 29>;
> + gpio-controller;
> + interrupts =
> + <GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 159 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 158 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 157 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 156 IRQ_TYPE_LEVEL_HIGH>;
> + };
> + };
> +
> eth0: ethernet@30000 {
> compatible = "marvell,armada-3700-neta";
> reg = <0x30000 0x4000>;
> --
> git-series 0.9.1
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
Hi,
On mer., avril 05 2017, Gregory CLEMENT <[email protected]> wrote:
> Start to populate the device tree of the Armada 37xx with the pincontrol
> configuration used on the board providing a dts.
>
> Signed-off-by: Gregory CLEMENT <[email protected]>
Applied on mvebu/dt64
Thanks,
Gregory
> ---
> arch/arm64/boot/dts/marvell/armada-3720-db.dts | 8 +++++-
> arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 31 +++++++++++++++++++-
> 2 files changed, 39 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/marvell/armada-3720-db.dts b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
> index 86602c907a61..e749c5727490 100644
> --- a/arch/arm64/boot/dts/marvell/armada-3720-db.dts
> +++ b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
> @@ -63,6 +63,8 @@
> };
>
> &i2c0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c1_pins>;
> status = "okay";
> };
>
> @@ -73,6 +75,8 @@
>
> &spi0 {
> status = "okay";
> + pinctrl-names = "default";
> + pinctrl-0 = <&spi_quad_pins>;
>
> m25p80@0 {
> compatible = "jedec,spi-nor";
> @@ -103,6 +107,8 @@
>
> /* Exported on the micro USB connector CON32 through an FTDI */
> &uart0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&uart1_pins>;
> status = "okay";
> };
>
> @@ -128,6 +134,8 @@
> };
>
> ð0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&rgmii_pins>;
> phy-mode = "rgmii-id";
> phy = <&phy0>;
> status = "okay";
> diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> index c02b13479458..2ac25f54d01d 100644
> --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
> @@ -186,6 +186,31 @@
> clock-output-names = "xtal";
> #clock-cells = <0>;
> };
> +
> + spi_quad_pins: spi-quad-pins {
> + groups = "spi_quad";
> + function = "spi";
> + };
> +
> + i2c1_pins: i2c1-pins {
> + groups = "i2c1";
> + function = "i2c";
> + };
> +
> + i2c2_pins: i2c2-pins {
> + groups = "i2c2";
> + function = "i2c";
> + };
> +
> + uart1_pins: uart1-pins {
> + groups = "uart1";
> + function = "uart";
> + };
> +
> + uart2_pins: uart2-pins {
> + groups = "uart2";
> + function = "uart";
> + };
> };
>
> pinctrl_sb: pinctrl-sb@18800 {
> @@ -203,6 +228,12 @@
> <GIC_SPI 157 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 156 IRQ_TYPE_LEVEL_HIGH>;
> };
> +
> + rgmii_pins: mii-pins {
> + groups = "rgmii";
> + function = "mii";
> + };
> +
> };
>
> eth0: ethernet@30000 {
> --
> git-series 0.9.1
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
On Wed, Apr 26, 2017 at 11:23 AM, Gregory CLEMENT
<[email protected]> wrote:
> On lun., avril 24 2017, Linus Walleij <[email protected]> wrote:
>>> + spin_lock_irqsave(&info->irq_lock, flags);
>>> + status = readl_relaxed(info->base + IRQ_STATUS + 4 * i);
>>> + /* Manage only the interrupt that was enabled */
>>> + status &= readl_relaxed(info->base + IRQ_EN + 4 * i);
>>> + spin_unlock_irqrestore(&info->irq_lock, flags);
>>> + while (status) {
>>> + u32 hwirq = ffs(status) - 1;
>>> + u32 virq = irq_find_mapping(d, hwirq +
>>> + i * GPIO_PER_REG);
>>> +
>>> + generic_handle_irq(virq);
>>> + status &= ~BIT(hwirq);
>>> + }
>>
>> You hae a problem here is a new IRQ appears while you are inside
>> of this loop. You need to re-read the status register for each iteration
>> (and &= with the IRQ_EN I guess).
>
> If a new IRQ appears during the loop, then the irq handler will be
> called again because the cause of this new IRQ won't have been acked
> yet. So I think we're fine here.
That *might* be true. It is true if the CPU gets a level IRQ from the
GPIO controller. But hardware dealing with edge IRQs can be very
quirky here, and just send a pulse on the line to the CPU if the
CPU-bound IRQ is also just edge triggered. And then that
pulse would potentially be missed while dealing with the current
IRQ in this handler. (And exactly this happened to us on other
hardware.)
But anyway: why let the irq handler be called again if you can avoid it?
You would avoid a double context switch by just checking it again
in the loop before exiting the handler. And that can be really nice
for latency-sensitive stuff.
Yours,
Linus Walleij
Hi Linus,
On mer., avril 26 2017, Linus Walleij <[email protected]> wrote:
> On Wed, Apr 26, 2017 at 11:23 AM, Gregory CLEMENT
> <[email protected]> wrote:
>> On lun., avril 24 2017, Linus Walleij <[email protected]> wrote:
>
>>>> + spin_lock_irqsave(&info->irq_lock, flags);
>>>> + status = readl_relaxed(info->base + IRQ_STATUS + 4 * i);
>>>> + /* Manage only the interrupt that was enabled */
>>>> + status &= readl_relaxed(info->base + IRQ_EN + 4 * i);
>>>> + spin_unlock_irqrestore(&info->irq_lock, flags);
>>>> + while (status) {
>>>> + u32 hwirq = ffs(status) - 1;
>>>> + u32 virq = irq_find_mapping(d, hwirq +
>>>> + i * GPIO_PER_REG);
>>>> +
>>>> + generic_handle_irq(virq);
>>>> + status &= ~BIT(hwirq);
>>>> + }
>>>
>>> You hae a problem here is a new IRQ appears while you are inside
>>> of this loop. You need to re-read the status register for each iteration
>>> (and &= with the IRQ_EN I guess).
>>
>> If a new IRQ appears during the loop, then the irq handler will be
>> called again because the cause of this new IRQ won't have been acked
>> yet. So I think we're fine here.
>
> That *might* be true. It is true if the CPU gets a level IRQ from the
> GPIO controller. But hardware dealing with edge IRQs can be very
> quirky here, and just send a pulse on the line to the CPU if the
> CPU-bound IRQ is also just edge triggered. And then that
> pulse would potentially be missed while dealing with the current
> IRQ in this handler. (And exactly this happened to us on other
> hardware.)
OK thanks for sharing your experience, you convinced me, I am going to
send a new version of the patch with this fix.
>
> But anyway: why let the irq handler be called again if you can avoid
> it?
> You would avoid a double context switch by just checking it again
> in the loop before exiting the handler. And that can be really nice
> for latency-sensitive stuff.
I wanted to avoid an uncached access in each loop if it was not
necessary. But as we finally need it, I will do it.
Gregory
>
> Yours,
> Linus Walleij
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com