Re: [PATCH v3 04/12] reset: Add generic reset driver
On 2/4/20 6:06 AM, Bin Meng wrote: > Hi Sean, > > On Mon, Feb 3, 2020 at 4:01 AM Sean Anderson wrote: >> +Required properties: >> +- compatible: should contain "syscon-reset" > > Shouldn't we follow the same generic "syscon-reboot" device bindings > defined in the Linux kernel? > See Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml This is a different driver. That driver is already in u-boot as UCLASS_SYSRESET, but this driver is for UCLASS_RESET. I modeled this one off syscon-reboot. > nits: this line should not be deleted. Ok, will restore for v4. --Sean
Re: [PATCH v3 04/12] reset: Add generic reset driver
Hi Sean,
On Mon, Feb 3, 2020 at 4:01 AM Sean Anderson wrote:
>
> This patch adds a generic reset driver. It is designed to be useful when one
> has
> a register in a regmap which contains bits that reset other devices. I thought
> this seemed like a very generic use, so here is a generic driver. The overall
> structure has been modeled on the syscon-reboot driver.
>
> Signed-off-by: Sean Anderson
> ---
> Changes for v3:
> - New
>
> .../reset/syscon-reset.txt| 36 +
> drivers/reset/Kconfig | 6 +-
> drivers/reset/Makefile| 1 +
> drivers/reset/reset-syscon.c | 79 +++
> 4 files changed, 121 insertions(+), 1 deletion(-)
> create mode 100644 doc/device-tree-bindings/reset/syscon-reset.txt
> create mode 100644 drivers/reset/reset-syscon.c
>
> diff --git a/doc/device-tree-bindings/reset/syscon-reset.txt
> b/doc/device-tree-bindings/reset/syscon-reset.txt
> new file mode 100644
> index 00..47c1226567
> --- /dev/null
> +++ b/doc/device-tree-bindings/reset/syscon-reset.txt
> @@ -0,0 +1,36 @@
> +Generic SYSCON mapped register reset driver
> +
> +This is a generic reset driver using syscon to map the reset register.
> +The reset is generally performed with a write to the reset register
> +defined by the register map pointed by syscon reference plus the offset and
> +shifted by the reset specifier/
> +
> +To assert a reset on some device, the equivalent of the following operation
> is
> +performed, where reset_id is the reset specifier from the device's resets
> +property.
> +
> + if (BIT(reset_id) & mask)
> + regmap[offset][reset_id] = assert-high;
> +
> +Required properties:
> +- compatible: should contain "syscon-reset"
Shouldn't we follow the same generic "syscon-reboot" device bindings
defined in the Linux kernel?
See Documentation/devicetree/bindings/power/reset/syscon-reboot.yaml
> +- #reset-cells: must be 1
> +- regmap: this is phandle to the register map node
> +- offset: offset in the register map for the reboot register (in bytes)
> +
> +Optional properties:
> +- mask: accept only the reset specifiers defined by the mask (32 bit)
> +- assert-high: Bit to write when asserting a reset. Defaults to 1.
> +
> +Default will be little endian mode, 32 bit access only.
> +
> +Example:
> +
> + reset-controller {
> + compatible = "syscon-reset";
> + #reset-cells = <1>;
> + regmap = <>;
> + offset = <0x20>;
> + mask = <0x27FF>;
> + assert-high = <0>;
> + };
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 75ccd65799..759d659c82 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -147,5 +147,9 @@ config RESET_IMX7
> default y
> help
> Support for reset controller on i.MX7/8 SoCs.
> -
nits: this line should not be deleted.
> +config RESET_SYSCON
> + bool "Enable generic syscon reset driver support"
> + depends on DM_RESET
> + help
> + Support generic syscon mapped register reset devices.
> endmenu
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 0a044d5d8c..433f1eca54 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -23,3 +23,4 @@ obj-$(CONFIG_RESET_MTMIPS) += reset-mtmips.o
> obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
> obj-$(CONFIG_RESET_HISILICON) += reset-hisilicon.o
> obj-$(CONFIG_RESET_IMX7) += reset-imx7.o
> +obj-$(CONFIG_RESET_SYSCON) += reset-syscon.o
[snip]
Regards,
Bin
Re: [PATCH v3 04/12] reset: Add generic reset driver
> Is there a sandbox test for this driver somewhere in your series? There is not. Presumably if such a test were to exist, it would try asserting different resets, and then check to see if the associated register got changed. However, I am confused as to where such a test would be located. There doesn't seem to be a directory for driver tests under tests/. tests/dm seems to be for uclass testing, and tests/py seems to be for tests which can work via the console. --Sean
Re: [PATCH v3 04/12] reset: Add generic reset driver
HI Sean, On Sun, 2 Feb 2020 at 13:01, Sean Anderson wrote: > > This patch adds a generic reset driver. It is designed to be useful when one > has > a register in a regmap which contains bits that reset other devices. I thought > this seemed like a very generic use, so here is a generic driver. The overall > structure has been modeled on the syscon-reboot driver. > > Signed-off-by: Sean Anderson > --- > Changes for v3: > - New > > .../reset/syscon-reset.txt| 36 + > drivers/reset/Kconfig | 6 +- > drivers/reset/Makefile| 1 + > drivers/reset/reset-syscon.c | 79 +++ > 4 files changed, 121 insertions(+), 1 deletion(-) > create mode 100644 doc/device-tree-bindings/reset/syscon-reset.txt > create mode 100644 drivers/reset/reset-syscon.c Is there a sandbox test for this driver somewhere in your series? Regards, Simon
[PATCH v3 04/12] reset: Add generic reset driver
This patch adds a generic reset driver. It is designed to be useful when one has
a register in a regmap which contains bits that reset other devices. I thought
this seemed like a very generic use, so here is a generic driver. The overall
structure has been modeled on the syscon-reboot driver.
Signed-off-by: Sean Anderson
---
Changes for v3:
- New
.../reset/syscon-reset.txt| 36 +
drivers/reset/Kconfig | 6 +-
drivers/reset/Makefile| 1 +
drivers/reset/reset-syscon.c | 79 +++
4 files changed, 121 insertions(+), 1 deletion(-)
create mode 100644 doc/device-tree-bindings/reset/syscon-reset.txt
create mode 100644 drivers/reset/reset-syscon.c
diff --git a/doc/device-tree-bindings/reset/syscon-reset.txt
b/doc/device-tree-bindings/reset/syscon-reset.txt
new file mode 100644
index 00..47c1226567
--- /dev/null
+++ b/doc/device-tree-bindings/reset/syscon-reset.txt
@@ -0,0 +1,36 @@
+Generic SYSCON mapped register reset driver
+
+This is a generic reset driver using syscon to map the reset register.
+The reset is generally performed with a write to the reset register
+defined by the register map pointed by syscon reference plus the offset and
+shifted by the reset specifier/
+
+To assert a reset on some device, the equivalent of the following operation is
+performed, where reset_id is the reset specifier from the device's resets
+property.
+
+ if (BIT(reset_id) & mask)
+ regmap[offset][reset_id] = assert-high;
+
+Required properties:
+- compatible: should contain "syscon-reset"
+- #reset-cells: must be 1
+- regmap: this is phandle to the register map node
+- offset: offset in the register map for the reboot register (in bytes)
+
+Optional properties:
+- mask: accept only the reset specifiers defined by the mask (32 bit)
+- assert-high: Bit to write when asserting a reset. Defaults to 1.
+
+Default will be little endian mode, 32 bit access only.
+
+Example:
+
+ reset-controller {
+ compatible = "syscon-reset";
+ #reset-cells = <1>;
+ regmap = <>;
+ offset = <0x20>;
+ mask = <0x27FF>;
+ assert-high = <0>;
+ };
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 75ccd65799..759d659c82 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -147,5 +147,9 @@ config RESET_IMX7
default y
help
Support for reset controller on i.MX7/8 SoCs.
-
+config RESET_SYSCON
+ bool "Enable generic syscon reset driver support"
+ depends on DM_RESET
+ help
+ Support generic syscon mapped register reset devices.
endmenu
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 0a044d5d8c..433f1eca54 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -23,3 +23,4 @@ obj-$(CONFIG_RESET_MTMIPS) += reset-mtmips.o
obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
obj-$(CONFIG_RESET_HISILICON) += reset-hisilicon.o
obj-$(CONFIG_RESET_IMX7) += reset-imx7.o
+obj-$(CONFIG_RESET_SYSCON) += reset-syscon.o
diff --git a/drivers/reset/reset-syscon.c b/drivers/reset/reset-syscon.c
new file mode 100644
index 00..db58b7705a
--- /dev/null
+++ b/drivers/reset/reset-syscon.c
@@ -0,0 +1,79 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 Sean Anderson
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct syscon_reset_priv {
+ struct regmap *regmap;
+ uint offset;
+ uint mask;
+ bool assert_high;
+};
+
+static int syscon_reset_request(struct reset_ctl *rst)
+{
+ struct syscon_reset_priv *priv = dev_get_priv(rst->dev);
+
+ if (BIT(rst->id) & priv->mask)
+ return 0;
+ else
+ return -EINVAL;
+}
+
+static int syscon_reset_assert(struct reset_ctl *rst)
+{
+ struct syscon_reset_priv *priv = dev_get_priv(rst->dev);
+
+ return regmap_update_bits(priv->regmap, priv->offset, BIT(rst->id),
+ priv->assert_high);
+}
+
+static int syscon_reset_deassert(struct reset_ctl *rst)
+{
+ struct syscon_reset_priv *priv = dev_get_priv(rst->dev);
+
+ return regmap_update_bits(priv->regmap, priv->offset, BIT(rst->id),
+ !priv->assert_high);
+}
+
+static const struct reset_ops syscon_reset_ops = {
+ .request = syscon_reset_request,
+ .rst_assert = syscon_reset_assert,
+ .rst_deassert = syscon_reset_deassert,
+};
+
+int syscon_reset_probe(struct udevice *dev)
+{
+ struct syscon_reset_priv *priv = dev_get_priv(dev);
+
+ priv->regmap = syscon_regmap_lookup_by_phandle(dev, "regmap");
+ if (IS_ERR(priv->regmap))
+ return -ENODEV;
+
+ priv->offset = dev_read_u32_default(dev, "offset", 0);
+ priv->mask = dev_read_u32_default(dev, "mask", 0);
+ priv->assert_high =
