On SAMA5D2 SoC the PIOBU pins do not lose their
voltage during Backup/Self-refresh mode. This can
be useful, for example, when the voltage must remain
positive for a peripheral during Backup/Self-refresh
mode (suspend-to ram is the Linux equivalent state).
v2:
- make driver be a subnode of the syscon node
- change Kconfig to depend on MFD_SYSCON and select GPIO_SYSCON
- change include header from linux/gpio.h to linux/gpio/driver.h
- include linux/bits.h header
- change intrusion in comment to tamper
- fix kerneldoc of functions
- replace GPIOF_DIR_* flags with 0/1
- replace ?: statement with if-else
- remove the use of sama5d2_piobu_template_chip
- retrieve syscon via syscon_node_to_regmap(pdev->dev.parent->of_node);
Note that PIOBU_REG_SIZE is used to determine the register to write to
with regmap:
reg = PIOBU_BASE + pin * PIOBU_REG_SIZE;
Also, no irq capability implemented.
Andrei Stefanescu (2):
dt-bindings: gpio: add SAMA5D2 PIOBU support
gpio: add driver for SAMA5D2 PIOBU pins
.../bindings/gpio/gpio-sama5d2-piobu.txt | 31 +++
MAINTAINERS | 7 +
drivers/gpio/Kconfig | 11 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-sama5d2-piobu.c | 253 +++++++++++++++++++++
5 files changed, 303 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-sama5d2-piobu.txt
create mode 100644 drivers/gpio/gpio-sama5d2-piobu.c
--
2.7.4
PIOBU pins do not lose their voltage during Backup/Self-refresh.
This patch adds a simple GPIO controller for them and a
maintainer for the driver.
This driver adds support for using the pins as GPIO
offering the possibility to read/set the voltage.
Signed-off-by: Andrei Stefanescu <[email protected]>
---
MAINTAINERS | 7 ++
drivers/gpio/Kconfig | 11 ++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-sama5d2-piobu.c | 253 ++++++++++++++++++++++++++++++++++++++
4 files changed, 272 insertions(+)
create mode 100644 drivers/gpio/gpio-sama5d2-piobu.c
diff --git a/MAINTAINERS b/MAINTAINERS
index f485597..88369f1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9760,6 +9760,13 @@ M: Nicolas Ferre <[email protected]>
S: Supported
F: drivers/power/reset/at91-sama5d2_shdwc.c
+MICROCHIP SAMA5D2-COMPATIBLE PIOBU GPIO
+M: Andrei Stefanescu <[email protected]>
+L: [email protected] (moderated for non-subscribers)
+L: [email protected]
+F: drivers/gpio/gpio-sama5d2-piobu.c
+F: Documentation/devicetree/bindings/gpio/gpio-sama5d2-piobu.txt
+
MICROCHIP SPI DRIVER
M: Nicolas Ferre <[email protected]>
S: Supported
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 833a1b5..1c41fac 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -429,6 +429,17 @@ config GPIO_REG
A 32-bit single register GPIO fixed in/out implementation. This
can be used to represent any register as a set of GPIO signals.
+config GPIO_SAMA5D2_PIOBU
+ tristate "SAMA5D2 PIOBU GPIO support"
+ depends on MFD_SYSCON
+ select GPIO_SYSCON
+ help
+ Say yes here to use the PIOBU pins as GPIOs.
+
+ PIOBU pins on the SAMA5D2 can be used as GPIOs.
+ The difference from regular GPIOs is that they
+ maintain their value during backup/self-refresh.
+
config GPIO_SIOX
tristate "SIOX GPIO support"
depends on SIOX
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 671c447..f18d345 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -108,6 +108,7 @@ obj-$(CONFIG_GPIO_RDC321X) += gpio-rdc321x.o
obj-$(CONFIG_GPIO_RCAR) += gpio-rcar.o
obj-$(CONFIG_GPIO_REG) += gpio-reg.o
obj-$(CONFIG_ARCH_SA1100) += gpio-sa1100.o
+obj-$(CONFIG_GPIO_SAMA5D2_PIOBU) += gpio-sama5d2-piobu.o
obj-$(CONFIG_GPIO_SCH) += gpio-sch.o
obj-$(CONFIG_GPIO_SCH311X) += gpio-sch311x.o
obj-$(CONFIG_GPIO_SNPS_CREG) += gpio-creg-snps.o
diff --git a/drivers/gpio/gpio-sama5d2-piobu.c b/drivers/gpio/gpio-sama5d2-piobu.c
new file mode 100644
index 0000000..db0cb5d
--- /dev/null
+++ b/drivers/gpio/gpio-sama5d2-piobu.c
@@ -0,0 +1,253 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * SAMA5D2 PIOBU GPIO controller
+ *
+ * Copyright (C) 2018 Microchip Technology Inc. and its subsidiaries
+ *
+ * Author: Andrei Stefanescu <[email protected]>
+ *
+ */
+#include <linux/bits.h>
+#include <linux/gpio/driver.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define PIOBU_NUM 8
+#define PIOBU_REG_SIZE 4
+
+/*
+ * backup mode protection register for tamper detection
+ * normal mode protection register for tamper detection
+ * wakeup signal generation
+ */
+#define PIOBU_BMPR 0x7C
+#define PIOBU_NMPR 0x80
+#define PIOBU_WKPR 0x90
+
+#define PIOBU_BASE 0x18 /* PIOBU offset from SECUMOD base register address. */
+
+#define PIOBU_DET_OFFSET 16
+
+/* In the datasheet this bit is called OUTPUT */
+#define PIOBU_DIRECTION BIT(8)
+#define PIOBU_OUT BIT(8)
+#define PIOBU_IN 0
+
+#define PIOBU_SOD BIT(9)
+#define PIOBU_PDS BIT(10)
+
+#define PIOBU_HIGH BIT(9)
+#define PIOBU_LOW 0
+
+struct sama5d2_piobu {
+ struct gpio_chip chip;
+ struct regmap *regmap;
+};
+
+/**
+ * sama5d2_piobu_setup_pin() - prepares a pin for set_direction call
+ *
+ * Do not consider pin for tamper detection (normal and backup modes)
+ * Do not consider pin as tamper wakeup interrupt source
+ */
+static int sama5d2_piobu_setup_pin(struct gpio_chip *chip, unsigned int pin)
+{
+ int ret;
+ struct sama5d2_piobu *piobu = container_of(chip, struct sama5d2_piobu,
+ chip);
+ unsigned int mask = BIT(PIOBU_DET_OFFSET + pin);
+
+ ret = regmap_update_bits(piobu->regmap, PIOBU_BMPR, mask, 0);
+ if (ret)
+ return ret;
+
+ ret = regmap_update_bits(piobu->regmap, PIOBU_NMPR, mask, 0);
+ if (ret)
+ return ret;
+
+ return regmap_update_bits(piobu->regmap, PIOBU_WKPR, mask, 0);
+}
+
+/**
+ * sama5d2_piobu_write_value() - writes value & mask at the pin's PIOBU register
+ */
+static int sama5d2_piobu_write_value(struct gpio_chip *chip, unsigned int pin,
+ unsigned int mask, unsigned int value)
+{
+ int reg;
+ struct sama5d2_piobu *piobu = container_of(chip, struct sama5d2_piobu,
+ chip);
+
+ reg = PIOBU_BASE + pin * PIOBU_REG_SIZE;
+
+ return regmap_update_bits(piobu->regmap, reg, mask, value);
+}
+
+/**
+ * sama5d2_piobu_read_value() - read the value with masking from the pin's PIOBU
+ * register
+ */
+static int sama5d2_piobu_read_value(struct gpio_chip *chip, unsigned int pin,
+ unsigned int mask)
+{
+ struct sama5d2_piobu *piobu = container_of(chip, struct sama5d2_piobu,
+ chip);
+ unsigned int val, reg;
+ int ret;
+
+ reg = PIOBU_BASE + pin * PIOBU_REG_SIZE;
+ ret = regmap_read(piobu->regmap, reg, &val);
+ if (ret < 0)
+ return ret;
+
+ return val & mask;
+}
+
+/**
+ * sama5d2_piobu_set_direction() - mark pin as input or output
+ */
+static int sama5d2_piobu_set_direction(struct gpio_chip *chip,
+ unsigned int direction,
+ unsigned int pin)
+{
+ return sama5d2_piobu_write_value(chip, pin, PIOBU_DIRECTION, direction);
+}
+
+/**
+ * sama5d2_piobu_get_direction() - gpiochip get_direction
+ */
+static int sama5d2_piobu_get_direction(struct gpio_chip *chip,
+ unsigned int pin)
+{
+ int ret = sama5d2_piobu_read_value(chip, pin, PIOBU_DIRECTION);
+
+ if (ret < 0)
+ return ret;
+
+ return (ret == PIOBU_IN) ? 1 : 0;
+}
+
+/**
+ * sama5d2_piobu_direction_input() - gpiochip direction_input
+ */
+static int sama5d2_piobu_direction_input(struct gpio_chip *chip,
+ unsigned int pin)
+{
+ return sama5d2_piobu_set_direction(chip, PIOBU_IN, pin);
+}
+
+/**
+ * sama5d2_piobu_direction_output() - gpiochip direction_output
+ */
+static int sama5d2_piobu_direction_output(struct gpio_chip *chip,
+ unsigned int pin, int value)
+{
+ return sama5d2_piobu_set_direction(chip, PIOBU_OUT, pin);
+}
+
+/**
+ * sama5d2_piobu_get() - gpiochip get
+ */
+static int sama5d2_piobu_get(struct gpio_chip *chip, unsigned int pin)
+{
+ /* if pin is input, read value from PDS else read from SOD */
+ int ret = sama5d2_piobu_get_direction(chip, pin);
+
+ if (ret == 1)
+ ret = sama5d2_piobu_read_value(chip, pin, PIOBU_PDS);
+ else if (!ret)
+ ret = sama5d2_piobu_read_value(chip, pin, PIOBU_SOD);
+
+ if (ret < 0)
+ return ret;
+
+ return !!ret;
+}
+
+/**
+ * sama5d2_piobu_set() - gpiochip set
+ */
+static void sama5d2_piobu_set(struct gpio_chip *chip, unsigned int pin,
+ int value)
+{
+ if (!value)
+ value = PIOBU_LOW;
+ else
+ value = PIOBU_HIGH;
+
+ sama5d2_piobu_write_value(chip, pin, PIOBU_SOD, value);
+}
+
+static int sama5d2_piobu_probe(struct platform_device *pdev)
+{
+ struct sama5d2_piobu *piobu;
+ int ret, i;
+
+ piobu = devm_kzalloc(&pdev->dev, sizeof(*piobu), GFP_KERNEL);
+ if (!piobu)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, piobu);
+ piobu->chip.label = pdev->name;
+ piobu->chip.parent = &pdev->dev;
+ piobu->chip.of_node = pdev->dev.of_node;
+ piobu->chip.owner = THIS_MODULE,
+ piobu->chip.get_direction = sama5d2_piobu_get_direction,
+ piobu->chip.direction_input = sama5d2_piobu_direction_input,
+ piobu->chip.direction_output = sama5d2_piobu_direction_output,
+ piobu->chip.get = sama5d2_piobu_get,
+ piobu->chip.set = sama5d2_piobu_set,
+ piobu->chip.base = -1,
+ piobu->chip.ngpio = PIOBU_NUM,
+ piobu->chip.can_sleep = 0,
+
+ piobu->regmap = syscon_node_to_regmap(pdev->dev.parent->of_node);
+ if (IS_ERR(piobu->regmap)) {
+ dev_err(&pdev->dev, "Failed to get syscon regmap %ld\n",
+ PTR_ERR(piobu->regmap));
+ return PTR_ERR(piobu->regmap);
+ }
+
+ ret = devm_gpiochip_add_data(&pdev->dev, &piobu->chip, piobu);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to add gpiochip %d\n", ret);
+ return ret;
+ }
+
+ for (i = 0; i < PIOBU_NUM; ++i) {
+ ret = sama5d2_piobu_setup_pin(&piobu->chip, i);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to setup pin: %d %d\n",
+ i, ret);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static const struct of_device_id sama5d2_piobu_ids[] = {
+ { .compatible = "microchip,sama5d2-piobu" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, sama5d2_piobu_ids);
+
+static struct platform_driver sama5d2_piobu_driver = {
+ .driver = {
+ .name = "sama5d2-piobu",
+ .of_match_table = of_match_ptr(sama5d2_piobu_ids)
+ },
+ .probe = sama5d2_piobu_probe,
+};
+
+module_platform_driver(sama5d2_piobu_driver);
+
+MODULE_VERSION("1.0");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("SAMA5D2 PIOBU controller driver");
+MODULE_AUTHOR("Andrei Stefanescu <[email protected]>");
--
2.7.4
This patch describes the compatible and the device tree
bindings necessary for the SAMA5D2 PIOBU GPIO.
Signed-off-by: Andrei Stefanescu <[email protected]>
---
.../bindings/gpio/gpio-sama5d2-piobu.txt | 31 ++++++++++++++++++++++
1 file changed, 31 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-sama5d2-piobu.txt
diff --git a/Documentation/devicetree/bindings/gpio/gpio-sama5d2-piobu.txt b/Documentation/devicetree/bindings/gpio/gpio-sama5d2-piobu.txt
new file mode 100644
index 0000000..2e260e1
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-sama5d2-piobu.txt
@@ -0,0 +1,31 @@
+GPIO controller for SAMA5D2 PIOBU pins.
+
+These pins have the property of not losing their voltage
+during Backup/Self-refresh mode.
+
+These bindings should be set to a node in the dtsi file.
+
+Required properties:
+- compatible: "syscon", "microchip,sama5d2-piobu"
+- #gpio-cells: There are 2. The pin number is the
+ first, the second represents additional
+ parameters such as GPIO_ACTIVE_HIGH/LOW.
+- gpio-controller: Marks the port as GPIO controller.
+
+Note that the driver uses syscon and should be the child of
+the syscon node.
+
+Example:
+
+ secumod@fc040000 {
+ compatible = "atmel,sama5d2-secumod", "syscon", "simple-mfd";
+ status = "okay";
+ reg = <0xfc040000 0x100>;
+
+ pioBU: piobu {
+ status = "okay";
+ compatible = "microchip,sama5d2-piobu";
+ gpio-controller;
+ #gpio-cells = <2>;
+ };
+ };
--
2.7.4
Hi Andrei,
On Tue, Nov 20, 2018 at 9:08 AM <[email protected]> wrote:
> On SAMA5D2 SoC the PIOBU pins do not lose their
> voltage during Backup/Self-refresh mode. This can
> be useful, for example, when the voltage must remain
> positive for a peripheral during Backup/Self-refresh
> mode (suspend-to ram is the Linux equivalent state).
>
> v2:
This version looks good to me, I'm just waiting for a DT
binding review.
Yours,
Linus Walleij
Hello Rob,
Could you please tell me if there are any improvements
to be made to the patch?
Best regards,
Andrei
On 20.11.2018 10:08, Andrei Stefanescu - M50506 wrote:
> This patch describes the compatible and the device tree
> bindings necessary for the SAMA5D2 PIOBU GPIO.
>
> Signed-off-by: Andrei Stefanescu <[email protected]>
> ---
> .../bindings/gpio/gpio-sama5d2-piobu.txt | 31 ++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/gpio/gpio-sama5d2-piobu.txt
>
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-sama5d2-piobu.txt b/Documentation/devicetree/bindings/gpio/gpio-sama5d2-piobu.txt
> new file mode 100644
> index 0000000..2e260e1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-sama5d2-piobu.txt
> @@ -0,0 +1,31 @@
> +GPIO controller for SAMA5D2 PIOBU pins.
> +
> +These pins have the property of not losing their voltage
> +during Backup/Self-refresh mode.
> +
> +These bindings should be set to a node in the dtsi file.
> +
> +Required properties:
> +- compatible: "syscon", "microchip,sama5d2-piobu"
> +- #gpio-cells: There are 2. The pin number is the
> + first, the second represents additional
> + parameters such as GPIO_ACTIVE_HIGH/LOW.
> +- gpio-controller: Marks the port as GPIO controller.
> +
> +Note that the driver uses syscon and should be the child of
> +the syscon node.
> +
> +Example:
> +
> + secumod@fc040000 {
> + compatible = "atmel,sama5d2-secumod", "syscon", "simple-mfd";
> + status = "okay";
> + reg = <0xfc040000 0x100>;
> +
> + pioBU: piobu {
> + status = "okay";
> + compatible = "microchip,sama5d2-piobu";
> + gpio-controller;
> + #gpio-cells = <2>;
> + };
> + };
On Tue, Nov 20, 2018 at 08:08:36AM +0000, [email protected] wrote:
> This patch describes the compatible and the device tree
> bindings necessary for the SAMA5D2 PIOBU GPIO.
>
> Signed-off-by: Andrei Stefanescu <[email protected]>
> ---
> .../bindings/gpio/gpio-sama5d2-piobu.txt | 31 ++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/gpio/gpio-sama5d2-piobu.txt
>
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-sama5d2-piobu.txt b/Documentation/devicetree/bindings/gpio/gpio-sama5d2-piobu.txt
> new file mode 100644
> index 0000000..2e260e1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-sama5d2-piobu.txt
microchip,sama5d2-piobu.txt for the file name.
> @@ -0,0 +1,31 @@
> +GPIO controller for SAMA5D2 PIOBU pins.
> +
> +These pins have the property of not losing their voltage
> +during Backup/Self-refresh mode.
> +
> +These bindings should be set to a node in the dtsi file.
> +
> +Required properties:
> +- compatible: "syscon", "microchip,sama5d2-piobu"
syscon should be removed.
> +- #gpio-cells: There are 2. The pin number is the
> + first, the second represents additional
> + parameters such as GPIO_ACTIVE_HIGH/LOW.
> +- gpio-controller: Marks the port as GPIO controller.
> +
> +Note that the driver uses syscon and should be the child of
> +the syscon node.
child of the "atmel,sama5d2-secumod" node to be more specific.
But why do you need a child node? The parent can be a gpio provider.
What other nodes does this need?
> +
> +Example:
> +
> + secumod@fc040000 {
> + compatible = "atmel,sama5d2-secumod", "syscon", "simple-mfd";
This is not documented as being a simple-mfd.
> + status = "okay";
> + reg = <0xfc040000 0x100>;
> +
> + pioBU: piobu {
gpio {
Is there not a register range you can put here?
> + status = "okay";
> + compatible = "microchip,sama5d2-piobu";
> + gpio-controller;
> + #gpio-cells = <2>;
> + };
> + };
> --
> 2.7.4
>
Hello Rob,
Thank you for your feedback.
I will add a bit of context regarding the secumod. The
"atmel,sama5d2-secumod"
compatible string is not used for loading a driver. It is used by atmel
securam
driver (drivers/misc/sram.c) which has access to secumod's registers via:
syscon_regmap_lookup_by_compatible("atmel,sama5d2-secumod")
So the secumod exports multiple hardware functions, eg: the securam, the
PIOBU
pins which can be used as GPIO pins.
My initial patch had the "microchip,sama5d2-piobu" compatible appended
to the
secumod's compatible e.g.:
secumod@fc040000 {
compatible = "syscon", "microchip,sama5d2-piobu";
...
Taking into consideration Linus Walleij's advice I arrived to the current
version. I thought this was a good idea because it separates the secumod
node
from the GPIO controller node. Please notice that securam node is already
separated from secumod node (it is a separate node in the device tree).
I have a few questions because I am not sure of the best approach:
1. Is it ok to have the GPIO controller as a child node?
2. I used simple-mfd because it was the only way I could think of in
order to
get the driver probed.
3. Should I add a register range? I thought that because the driver uses
syscon
it is not necessary to add the register range. Also, the register
range would
have been a subset of the secumod's register range.
On Wed, Dec 5, 2018 at 5:06 AM <[email protected]> wrote:
>
> Hello Rob,
>
> Thank you for your feedback.
>
> I will add a bit of context regarding the secumod. The
> "atmel,sama5d2-secumod"
> compatible string is not used for loading a driver. It is used by atmel
> securam
> driver (drivers/misc/sram.c) which has access to secumod's registers via:
>
> syscon_regmap_lookup_by_compatible("atmel,sama5d2-secumod")
>
> So the secumod exports multiple hardware functions, eg: the securam, the
> PIOBU
> pins which can be used as GPIO pins.
Yes, I understand why you want to design it this way, but don't design
your bindings around how 1 OS happens to currently work.
>
> My initial patch had the "microchip,sama5d2-piobu" compatible appended
> to the
> secumod's compatible e.g.:
>
> secumod@fc040000 {
> compatible = "syscon", "microchip,sama5d2-piobu";
That doesn't look appended to me. The documentation says it should
look like this:
compatible = "atmel,sama5d2-secumod", "syscon";
> ...
>
> Taking into consideration Linus Walleij's advice I arrived to the current
> version. I thought this was a good idea because it separates the secumod
> node
> from the GPIO controller node. Please notice that securam node is already
> separated from secumod node (it is a separate node in the device tree).
>
> I have a few questions because I am not sure of the best approach:
>
> 1. Is it ok to have the GPIO controller as a child node?
Is it a separate h/w block and do you have other child nodes that need
their own resources in DT (interrupts, clocks, etc.)? If these aren't
the case, then no you shouldn't. As it stands, you don't have any
other child nodes, so no you shouldn't have a child node here. Just
add to properties to the existing binding:
secumod@fc040000 {
compatible = "atmel,sama5d2-secumod", "syscon";
reg = <0xfc040000 0x100>;
#gpio-cells = <2>;
gpio-controller;
};
Now perhaps you have 10 other functions in this block like pinctrl,
clocks, phys, power domains, etc. Then child nodes would be warranted.
But if that's the case, then please write a complete binding for the
secumod block.
> 2. I used simple-mfd because it was the only way I could think of in
> order to
> get the driver probed.
Sounds like an OS problem that has little to do with the binding. In
any case, if you change a binding which you have, then it needs to be
documented.
> 3. Should I add a register range? I thought that because the driver uses
> syscon
> it is not necessary to add the register range. Also, the register
> range would
> have been a subset of the secumod's register range.
If it has one, then yes you should. If it doesn't then it is probably
not a separate h/w block and shouldn't have a child node to begin
with.
Rob