[PATCH 0/3] reset: add socfpga reset-controller

[PATCH 0/3] reset: add socfpga reset-controller

[Steffen Trumtrar]

Hi!

This series converts the current rst-mgr entry in the socfpga.dtsi to
a driver that registers with the reset-controller framework.

1/3 adds the driver,
2/3 updates the binding to reflect this
3/3 updates the socfpga.dtsi and adds the DT-header file

Regards,
Steffen

Steffen Trumtrar (3):
  reset: add driver for socfpga
  Documentation: dt: socfpga: add reset-cells property
  ARM: socfpga: dts: add reset-controller

 .../bindings/arm/altera/socfpga-reset.txt          |   2 +
 arch/arm/boot/dts/socfpga.dtsi                     |   8 +-
 drivers/reset/Makefile                             |   1 +
 drivers/reset/reset-socfpga.c                      | 149 +++++++++++++++++++++
 include/dt-bindings/reset/altr,rst-mgr.h           |  90 +++++++++++++
 5 files changed, 249 insertions(+), 1 deletion(-)
 create mode 100644 drivers/reset/reset-socfpga.c
 create mode 100644 include/dt-bindings/reset/altr,rst-mgr.h

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] reset: add driver for socfpga

[Steffen Trumtrar]

Add a reset-controller driver for the socfpga platform.
The reset-controller has four banks with up to 32 entries all encapsulated in
one module block.

Signed-off-by: Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/reset/Makefile        |   1 +
 drivers/reset/reset-socfpga.c | 149 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 150 insertions(+)
 create mode 100644 drivers/reset/reset-socfpga.c

diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index cc29832..ab64077 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_RESET_CONTROLLER) += core.o
 obj-$(CONFIG_ARCH_SUNXI) += reset-sunxi.o
+obj-$(CONFIG_ARCH_SOCFPGA) += reset-socfpga.o
diff --git a/drivers/reset/reset-socfpga.c b/drivers/reset/reset-socfpga.c
new file mode 100644
index 0000000..b21e4e4
--- /dev/null
+++ b/drivers/reset/reset-socfpga.c
@@ -0,0 +1,149 @@
+/*
+ * Copyright 2014 Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
+ *
+ * based on
+ * Allwinner SoCs Reset Controller driver
+ *
+ * Copyright 2013 Maxime Ripard
+ *
+ * Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+
+#define NR_BANKS		4
+#define MAX_BANK_WIDTH		32
+#define OFFSET_MODRST		0x10
+
+struct socfpga_reset_data {
+	spinlock_t			lock;
+	void __iomem			*membase;
+	struct reset_controller_dev	rcdev;
+};
+
+static int socfpga_reset_assert(struct reset_controller_dev *rcdev,
+				unsigned long id)
+{
+	struct socfpga_reset_data *data = container_of(rcdev,
+						     struct socfpga_reset_data,
+						     rcdev);
+	int bank = id / BITS_PER_LONG;
+	int offset = id % BITS_PER_LONG;
+	unsigned long flags;
+	u32 reg;
+
+	spin_lock_irqsave(&data->lock, flags);
+
+	reg = readl(data->membase + OFFSET_MODRST + (bank * NR_BANKS));
+	writel(reg | BIT(offset), data->membase + OFFSET_MODRST +
+				 (bank * NR_BANKS));
+	spin_unlock_irqrestore(&data->lock, flags);
+
+	return 0;
+}
+
+static int socfpga_reset_deassert(struct reset_controller_dev *rcdev,
+				  unsigned long id)
+{
+	struct socfpga_reset_data *data = container_of(rcdev,
+						     struct socfpga_reset_data,
+						     rcdev);
+
+	int bank = id / BITS_PER_LONG;
+	int offset = id % BITS_PER_LONG;
+	unsigned long flags;
+	u32 reg;
+
+	spin_lock_irqsave(&data->lock, flags);
+
+	reg = readl(data->membase + OFFSET_MODRST + (bank * NR_BANKS));
+	writel(reg & ~BIT(offset), data->membase + OFFSET_MODRST +
+				  (bank * NR_BANKS));
+
+	spin_unlock_irqrestore(&data->lock, flags);
+
+	return 0;
+}
+
+static struct reset_control_ops socfpga_reset_ops = {
+	.assert		= socfpga_reset_assert,
+	.deassert	= socfpga_reset_deassert,
+};
+
+static int socfpga_reset_probe(struct platform_device *pdev)
+{
+	struct socfpga_reset_data *data;
+	struct resource *res;
+	int ret;
+
+	/*
+	 * The binding was mainlined without the required property.
+	 * Do not continue, when we encounter an old DT.
+	 */
+	if (!of_find_property(pdev->dev.of_node, "#reset-cells", NULL)) {
+		dev_err(&pdev->dev, "dt missing #reset-cells property\n");
+		return -EINVAL;
+	}
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	data->membase = devm_ioremap_resource(&pdev->dev, res);
+	if (!data->membase) {
+		ret = -ENOMEM;
+		return ret;
+	}
+
+	spin_lock_init(&data->lock);
+
+	data->rcdev.owner = THIS_MODULE;
+	data->rcdev.nr_resets = NR_BANKS * MAX_BANK_WIDTH;
+	data->rcdev.ops = &socfpga_reset_ops;
+	data->rcdev.of_node = pdev->dev.of_node;
+	reset_controller_register(&data->rcdev);
+
+	return 0;
+}
+
+static int socfpga_reset_remove(struct platform_device *pdev)
+{
+	struct socfpga_reset_data *data = platform_get_drvdata(pdev);
+
+	reset_controller_unregister(&data->rcdev);
+
+	return 0;
+}
+
+static const struct of_device_id socfpga_reset_dt_ids[] = {
+	{ .compatible = "altr,rst-mgr", },
+	{ /* sentinel */ },
+};
+
+static struct platform_driver socfpga_reset_driver = {
+	.probe	= socfpga_reset_probe,
+	.remove	= socfpga_reset_remove,
+	.driver = {
+		.name		= "socfpga-reset",
+		.owner		= THIS_MODULE,
+		.of_match_table	= socfpga_reset_dt_ids,
+	},
+};
+module_platform_driver(socfpga_reset_driver);
+
+MODULE_AUTHOR("Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org");
+MODULE_DESCRIPTION("Socfpga Reset Controller Driver");
+MODULE_LICENSE("GPL");
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] Documentation: dt: socfpga: add reset-cells property

[Steffen Trumtrar]

To be able to use the reset-controller framework, the property
	#reset-cells
is mandatory.

Signed-off-by: Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 Documentation/devicetree/bindings/arm/altera/socfpga-reset.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/altera/socfpga-reset.txt b/Documentation/devicetree/bindings/arm/altera/socfpga-reset.txt
index ecdb57d..32c1c8b 100644
--- a/Documentation/devicetree/bindings/arm/altera/socfpga-reset.txt
+++ b/Documentation/devicetree/bindings/arm/altera/socfpga-reset.txt
@@ -3,9 +3,11 @@ Altera SOCFPGA Reset Manager
 Required properties:
 - compatible : "altr,rst-mgr"
 - reg : Should contain 1 register ranges(address and length)
+- #reset-cells: 1
 
 Example:
 	 rstmgr@ffd05000 {
+		#reset-cells = <1>;
 		compatible = "altr,rst-mgr";
 		reg = <0xffd05000 0x1000>;
 	};
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] ARM: socfpga: dts: add reset-controller

[Steffen Trumtrar]

Add the necessary #reset-cells property to the rst-mgr node and
provide a header-file with all possible resets specified.

Signed-off-by: Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 arch/arm/boot/dts/socfpga.dtsi           |  8 ++-
 include/dt-bindings/reset/altr,rst-mgr.h | 90 ++++++++++++++++++++++++++++++++
 2 files changed, 97 insertions(+), 1 deletion(-)
 create mode 100644 include/dt-bindings/reset/altr,rst-mgr.h

diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
index 1f71e96..027d2c6 100644
--- a/arch/arm/boot/dts/socfpga.dtsi
+++ b/arch/arm/boot/dts/socfpga.dtsi
@@ -16,6 +16,7 @@
  */
 
 #include "skeleton.dtsi"
+#include <dt-bindings/reset/altr,rst-mgr.h>
 
 / {
 	#address-cells = <1>;
@@ -456,6 +457,8 @@
 			mac-address = [00 00 00 00 00 00];/* Filled in by U-Boot */
 			clocks = <&emac0_clk>;
 			clock-names = "stmmaceth";
+			resets = <&rst EMAC0_RESET>;
+			reset-names = "stmmaceth";
 			status = "disabled";
 		};
 
@@ -467,6 +470,8 @@
 			mac-address = [00 00 00 00 00 00];/* Filled in by U-Boot */
 			clocks = <&emac1_clk>;
 			clock-names = "stmmaceth";
+			resets = <&rst EMAC1_RESET>;
+			reset-names = "stmmaceth";
 			status = "disabled";
 		};
 
@@ -584,7 +589,8 @@
 			reg-io-width = <4>;
 		};
 
-		rstmgr@ffd05000 {
+		rst: rstmgr@ffd05000 {
+			#reset-cells = <1>;
 			compatible = "altr,rst-mgr";
 			reg = <0xffd05000 0x1000>;
 		};
diff --git a/include/dt-bindings/reset/altr,rst-mgr.h b/include/dt-bindings/reset/altr,rst-mgr.h
new file mode 100644
index 0000000..3f04908
--- /dev/null
+++ b/include/dt-bindings/reset/altr,rst-mgr.h
@@ -0,0 +1,90 @@
+/*
+ * Copyright (c) 2014, Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _DT_BINDINGS_RESET_ALTR_RST_MGR_H
+#define _DT_BINDINGS_RESET_ALTR_RST_MGR_H
+
+/* MPUMODRST */
+#define CPU0_RESET		0
+#define CPU1_RESET		1
+#define WDS_RESET		2
+#define SCUPER_RESET		3
+#define L2_RESET		4
+
+/* PERMODRST */
+#define EMAC0_RESET		32
+#define EMAC1_RESET		33
+#define USB0_RESET		34
+#define USB1_RESET		35
+#define NAND_RESET		36
+#define QSPI_RESET		37
+#define L4WD0_RESET		38
+#define L4WD1_RESET		39
+#define OSC1TIMER0_RESET	40
+#define OSC1TIMER1_RESET	41
+#define SPTIMER0_RESET		42
+#define SPTIMER1_RESET		43
+#define I2C0_RESET		44
+#define I2C1_RESET		45
+#define I2C2_RESET		46
+#define I2C3_RESET		47
+#define UART0_RESET		48
+#define UART1_RESET		49
+#define SPIM0_RESET		50
+#define SPIM1_RESET		51
+#define SPIS0_RESET		52
+#define SPIS1_RESET		53
+#define SDMMC_RESET		54
+#define CAN0_RESET		55
+#define CAN1_RESET		56
+#define GPIO0_RESET		57
+#define GPIO1_RESET		58
+#define GPIO2_RESET		59
+#define DMA_RESET		60
+#define SDR_RESET		61
+
+/* PER2MODRST */
+#define DMAIF0_RESET		64
+#define DMAIF1_RESET		65
+#define DMAIF2_RESET		66
+#define DMAIF3_RESET		67
+#define DMAIF4_RESET		68
+#define DMAIF5_RESET		69
+#define DMAIF6_RESET		70
+#define DMAIF7_RESET		71
+
+/* BRGMODRST */
+#define HPS2FPGA_RESET		96
+#define LWHPS2FPGA_RESET	97
+#define FPGA2HPS_RESET		98
+
+/* MISCMODRST*/
+#define ROM_RESET		128
+#define OCRAM_RESET		129
+#define SYSMGR_RESET		130
+#define SYSMGRCOLD_RESET	131
+#define FPGAMGR_RESET		132
+#define ACPIDMAP_RESET		133
+#define S2F_RESET		134
+#define S2FCOLD_RESET		135
+#define NRSTPIN_RESET		136
+#define TIMESTAMPCOLD_RESET	137
+#define CLKMGRCOLD_RESET	138
+#define SCANMGR_RESET		139
+#define FRZCTRLCOLD_RESET	140
+#define SYSDBG_RESET		141
+#define DBG_RESET		142
+#define TAPCOLD_RESET		143
+#define SDRCOLD_RESET		144
+
+#endif
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] reset: add driver for socfpga

[Philipp Zabel]

Hi Steffen,

a few minor issues below:

Am Freitag, den 04.04.2014, 18:31 +0200 schrieb Steffen Trumtrar:
> Add a reset-controller driver for the socfpga platform.
> The reset-controller has four banks with up to 32 entries all encapsulated in
> one module block.
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
>  drivers/reset/Makefile        |   1 +
>  drivers/reset/reset-socfpga.c | 149 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 150 insertions(+)
>  create mode 100644 drivers/reset/reset-socfpga.c
> 
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index cc29832..ab64077 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -1,2 +1,3 @@
>  obj-$(CONFIG_RESET_CONTROLLER) += core.o
>  obj-$(CONFIG_ARCH_SUNXI) += reset-sunxi.o
> +obj-$(CONFIG_ARCH_SOCFPGA) += reset-socfpga.o
> diff --git a/drivers/reset/reset-socfpga.c b/drivers/reset/reset-socfpga.c
> new file mode 100644
> index 0000000..b21e4e4
> --- /dev/null
> +++ b/drivers/reset/reset-socfpga.c
> @@ -0,0 +1,149 @@
> +/*
> + * Copyright 2014 Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> + *
> + * based on
> + * Allwinner SoCs Reset Controller driver
> + *
> + * Copyright 2013 Maxime Ripard
> + *
> + * Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> +#include <linux/spinlock.h>
> +#include <linux/types.h>
> +
> +#define NR_BANKS		4
> +#define MAX_BANK_WIDTH		32

Since you are using BITS_PER_LONG for bank and offset calculation, why
not just drop MAX_BANK_WIDTH and use BITS_PER_LONG directly? (Or use
MAX_BANK_WIDTH everywhere.)

> +#define OFFSET_MODRST		0x10
> +
> +struct socfpga_reset_data {
> +	spinlock_t			lock;
> +	void __iomem			*membase;
> +	struct reset_controller_dev	rcdev;
> +};
> +
> +static int socfpga_reset_assert(struct reset_controller_dev *rcdev,
> +				unsigned long id)
> +{
> +	struct socfpga_reset_data *data = container_of(rcdev,
> +						     struct socfpga_reset_data,
> +						     rcdev);
> +	int bank = id / BITS_PER_LONG;
> +	int offset = id % BITS_PER_LONG;
> +	unsigned long flags;
> +	u32 reg;
> +
> +	spin_lock_irqsave(&data->lock, flags);
> +
> +	reg = readl(data->membase + OFFSET_MODRST + (bank * NR_BANKS));
> +	writel(reg | BIT(offset), data->membase + OFFSET_MODRST +
> +				 (bank * NR_BANKS));
> +	spin_unlock_irqrestore(&data->lock, flags);
> +
> +	return 0;
> +}
> +
> +static int socfpga_reset_deassert(struct reset_controller_dev *rcdev,
> +				  unsigned long id)
> +{
> +	struct socfpga_reset_data *data = container_of(rcdev,
> +						     struct socfpga_reset_data,
> +						     rcdev);
> +
> +	int bank = id / BITS_PER_LONG;
> +	int offset = id % BITS_PER_LONG;
> +	unsigned long flags;
> +	u32 reg;
> +
> +	spin_lock_irqsave(&data->lock, flags);
> +
> +	reg = readl(data->membase + OFFSET_MODRST + (bank * NR_BANKS));
> +	writel(reg & ~BIT(offset), data->membase + OFFSET_MODRST +
> +				  (bank * NR_BANKS));
> +
> +	spin_unlock_irqrestore(&data->lock, flags);
> +
> +	return 0;
> +}
> +
> +static struct reset_control_ops socfpga_reset_ops = {
> +	.assert		= socfpga_reset_assert,
> +	.deassert	= socfpga_reset_deassert,
> +};
> +
> +static int socfpga_reset_probe(struct platform_device *pdev)
> +{
> +	struct socfpga_reset_data *data;
> +	struct resource *res;
> +	int ret;
> +
> +	/*
> +	 * The binding was mainlined without the required property.
> +	 * Do not continue, when we encounter an old DT.
> +	 */
> +	if (!of_find_property(pdev->dev.of_node, "#reset-cells", NULL)) {
> +		dev_err(&pdev->dev, "dt missing #reset-cells property\n");

Maybe also print pdev->dev.of_node->full_name here?

> +		return -EINVAL;
> +	}
> +
> +	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	data->membase = devm_ioremap_resource(&pdev->dev, res);
> +	if (!data->membase) {
> +		ret = -ENOMEM;
> +		return ret;

This should be:
	if (IS_ERR(data->membase))
		return PTR_ERR(data->membase);

> +	}
> +
> +	spin_lock_init(&data->lock);
> +
> +	data->rcdev.owner = THIS_MODULE;
> +	data->rcdev.nr_resets = NR_BANKS * MAX_BANK_WIDTH;

See above.

> +	data->rcdev.ops = &socfpga_reset_ops;
> +	data->rcdev.of_node = pdev->dev.of_node;
> +	reset_controller_register(&data->rcdev);
> +
> +	return 0;
> +}
> +
> +static int socfpga_reset_remove(struct platform_device *pdev)
> +{
> +	struct socfpga_reset_data *data = platform_get_drvdata(pdev);
> +
> +	reset_controller_unregister(&data->rcdev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id socfpga_reset_dt_ids[] = {
> +	{ .compatible = "altr,rst-mgr", },
> +	{ /* sentinel */ },
> +};
> +
> +static struct platform_driver socfpga_reset_driver = {
> +	.probe	= socfpga_reset_probe,
> +	.remove	= socfpga_reset_remove,
> +	.driver = {
> +		.name		= "socfpga-reset",
> +		.owner		= THIS_MODULE,
> +		.of_match_table	= socfpga_reset_dt_ids,
> +	},
> +};
> +module_platform_driver(socfpga_reset_driver);
> +
> +MODULE_AUTHOR("Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org");
> +MODULE_DESCRIPTION("Socfpga Reset Controller Driver");
> +MODULE_LICENSE("GPL");

regards
Philipp

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] reset: add driver for socfpga

[Steffen Trumtrar]

Hi!

On Mon, Apr 07, 2014 at 10:46:48AM +0200, Philipp Zabel wrote:
> Hi Steffen,
> 
> a few minor issues below:
> 
> Am Freitag, den 04.04.2014, 18:31 +0200 schrieb Steffen Trumtrar:
> > Add a reset-controller driver for the socfpga platform.
> > The reset-controller has four banks with up to 32 entries all encapsulated in
> > one module block.
> > 
> > Signed-off-by: Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > ---
> >  drivers/reset/Makefile        |   1 +
> >  drivers/reset/reset-socfpga.c | 149 ++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 150 insertions(+)
> >  create mode 100644 drivers/reset/reset-socfpga.c
> > 
> > diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> > index cc29832..ab64077 100644
> > --- a/drivers/reset/Makefile
> > +++ b/drivers/reset/Makefile
> > @@ -1,2 +1,3 @@
> >  obj-$(CONFIG_RESET_CONTROLLER) += core.o
> >  obj-$(CONFIG_ARCH_SUNXI) += reset-sunxi.o
> > +obj-$(CONFIG_ARCH_SOCFPGA) += reset-socfpga.o
> > diff --git a/drivers/reset/reset-socfpga.c b/drivers/reset/reset-socfpga.c
> > new file mode 100644
> > index 0000000..b21e4e4
> > --- /dev/null
> > +++ b/drivers/reset/reset-socfpga.c
> > @@ -0,0 +1,149 @@
> > +/*
> > + * Copyright 2014 Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > + *
> > + * based on
> > + * Allwinner SoCs Reset Controller driver
> > + *
> > + * Copyright 2013 Maxime Ripard
> > + *
> > + * Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + */
> > +
> > +#include <linux/err.h>
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/reset-controller.h>
> > +#include <linux/spinlock.h>
> > +#include <linux/types.h>
> > +
> > +#define NR_BANKS		4
> > +#define MAX_BANK_WIDTH		32
> 
> Since you are using BITS_PER_LONG for bank and offset calculation, why
> not just drop MAX_BANK_WIDTH and use BITS_PER_LONG directly? (Or use
> MAX_BANK_WIDTH everywhere.)
>

Hm, yeah. I will change that to BITS_PER_LONG everywhere.

> > +#define OFFSET_MODRST		0x10
> > +
> > +struct socfpga_reset_data {
> > +	spinlock_t			lock;
> > +	void __iomem			*membase;
> > +	struct reset_controller_dev	rcdev;
> > +};
> > +
> > +static int socfpga_reset_assert(struct reset_controller_dev *rcdev,
> > +				unsigned long id)
> > +{
> > +	struct socfpga_reset_data *data = container_of(rcdev,
> > +						     struct socfpga_reset_data,
> > +						     rcdev);
> > +	int bank = id / BITS_PER_LONG;
> > +	int offset = id % BITS_PER_LONG;
> > +	unsigned long flags;
> > +	u32 reg;
> > +
> > +	spin_lock_irqsave(&data->lock, flags);
> > +
> > +	reg = readl(data->membase + OFFSET_MODRST + (bank * NR_BANKS));
> > +	writel(reg | BIT(offset), data->membase + OFFSET_MODRST +
> > +				 (bank * NR_BANKS));
> > +	spin_unlock_irqrestore(&data->lock, flags);
> > +
> > +	return 0;
> > +}
> > +
> > +static int socfpga_reset_deassert(struct reset_controller_dev *rcdev,
> > +				  unsigned long id)
> > +{
> > +	struct socfpga_reset_data *data = container_of(rcdev,
> > +						     struct socfpga_reset_data,
> > +						     rcdev);
> > +
> > +	int bank = id / BITS_PER_LONG;
> > +	int offset = id % BITS_PER_LONG;
> > +	unsigned long flags;
> > +	u32 reg;
> > +
> > +	spin_lock_irqsave(&data->lock, flags);
> > +
> > +	reg = readl(data->membase + OFFSET_MODRST + (bank * NR_BANKS));
> > +	writel(reg & ~BIT(offset), data->membase + OFFSET_MODRST +
> > +				  (bank * NR_BANKS));
> > +
> > +	spin_unlock_irqrestore(&data->lock, flags);
> > +
> > +	return 0;
> > +}
> > +
> > +static struct reset_control_ops socfpga_reset_ops = {
> > +	.assert		= socfpga_reset_assert,
> > +	.deassert	= socfpga_reset_deassert,
> > +};
> > +
> > +static int socfpga_reset_probe(struct platform_device *pdev)
> > +{
> > +	struct socfpga_reset_data *data;
> > +	struct resource *res;
> > +	int ret;
> > +
> > +	/*
> > +	 * The binding was mainlined without the required property.
> > +	 * Do not continue, when we encounter an old DT.
> > +	 */
> > +	if (!of_find_property(pdev->dev.of_node, "#reset-cells", NULL)) {
> > +		dev_err(&pdev->dev, "dt missing #reset-cells property\n");
> 
> Maybe also print pdev->dev.of_node->full_name here?
> 

Okay.

> > +		return -EINVAL;
> > +	}
> > +
> > +	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> > +	if (!data)
> > +		return -ENOMEM;
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	data->membase = devm_ioremap_resource(&pdev->dev, res);
> > +	if (!data->membase) {
> > +		ret = -ENOMEM;
> > +		return ret;
> 
> This should be:
> 	if (IS_ERR(data->membase))
> 		return PTR_ERR(data->membase);
> 

Ah, of course. Somehow didn't think about that here.

> > +	}
> > +
> > +	spin_lock_init(&data->lock);
> > +
> > +	data->rcdev.owner = THIS_MODULE;
> > +	data->rcdev.nr_resets = NR_BANKS * MAX_BANK_WIDTH;
> 
> See above.
> 
> > +	data->rcdev.ops = &socfpga_reset_ops;
> > +	data->rcdev.of_node = pdev->dev.of_node;
> > +	reset_controller_register(&data->rcdev);
> > +
> > +	return 0;
> > +}
> > +
> > +static int socfpga_reset_remove(struct platform_device *pdev)
> > +{
> > +	struct socfpga_reset_data *data = platform_get_drvdata(pdev);
> > +
> > +	reset_controller_unregister(&data->rcdev);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id socfpga_reset_dt_ids[] = {
> > +	{ .compatible = "altr,rst-mgr", },
> > +	{ /* sentinel */ },
> > +};
> > +
> > +static struct platform_driver socfpga_reset_driver = {
> > +	.probe	= socfpga_reset_probe,
> > +	.remove	= socfpga_reset_remove,
> > +	.driver = {
> > +		.name		= "socfpga-reset",
> > +		.owner		= THIS_MODULE,
> > +		.of_match_table	= socfpga_reset_dt_ids,
> > +	},
> > +};
> > +module_platform_driver(socfpga_reset_driver);
> > +
> > +MODULE_AUTHOR("Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org");
> > +MODULE_DESCRIPTION("Socfpga Reset Controller Driver");
> > +MODULE_LICENSE("GPL");
> 

Thanks,
Steffen

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] ARM: socfpga: dts: add reset-controller

[Dinh Nguyen]

On Fri, 2014-04-04 at 18:31 +0200, Steffen Trumtrar wrote:
> Add the necessary #reset-cells property to the rst-mgr node and
> provide a header-file with all possible resets specified.
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
>  arch/arm/boot/dts/socfpga.dtsi           |  8 ++-
>  include/dt-bindings/reset/altr,rst-mgr.h | 90 ++++++++++++++++++++++++++++++++

Does this file belong under dt-bindings/reset/ or
dt-bindings/reset-controller/?

Dinh

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] ARM: socfpga: dts: add reset-controller

[Steffen Trumtrar]

On Mon, Apr 07, 2014 at 04:12:01PM -0500, Dinh Nguyen wrote:
> On Fri, 2014-04-04 at 18:31 +0200, Steffen Trumtrar wrote:
> > Add the necessary #reset-cells property to the rst-mgr node and
> > provide a header-file with all possible resets specified.
> > 
> > Signed-off-by: Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > ---
> >  arch/arm/boot/dts/socfpga.dtsi           |  8 ++-
> >  include/dt-bindings/reset/altr,rst-mgr.h | 90 ++++++++++++++++++++++++++++++++
> 
> Does this file belong under dt-bindings/reset/ or
> dt-bindings/reset-controller/?
> 

ATM there is no such directory, or is there ?
So, reset seems appropriate I think.

But this reminds me, that I wanted to move the binding documentation from
Documentation/devicetree/bindings/arm/altera/socfpga-reset.txt ->
Documentation/devicetree/bindings/reset/socfpga-reset.txt

I will have to add a patch for that to the series in v2.

Thanks,
Steffen

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] ARM: socfpga: dts: add reset-controller

[Dinh Nguyen]

On 4/7/14 5:41 PM, Steffen Trumtrar wrote:
> On Mon, Apr 07, 2014 at 04:12:01PM -0500, Dinh Nguyen wrote:
>> On Fri, 2014-04-04 at 18:31 +0200, Steffen Trumtrar wrote:
>>> Add the necessary #reset-cells property to the rst-mgr node and
>>> provide a header-file with all possible resets specified.
>>>
>>> Signed-off-by: Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>>> ---
>>>  arch/arm/boot/dts/socfpga.dtsi           |  8 ++-
>>>  include/dt-bindings/reset/altr,rst-mgr.h | 90 ++++++++++++++++++++++++++++++++
>> Does this file belong under dt-bindings/reset/ or
>> dt-bindings/reset-controller/?
>>
> ATM there is no such directory, or is there ?
> So, reset seems appropriate I think.
commit [6b7f06 ARM: STi: STiH415: Add reset controller support.]
Seems more appropriate for this to go into reset-controller.

Dinh
>
> But this reminds me, that I wanted to move the binding documentation from
> Documentation/devicetree/bindings/arm/altera/socfpga-reset.txt ->
> Documentation/devicetree/bindings/reset/socfpga-reset.txt
>
> I will have to add a patch for that to the series in v2.
>
> Thanks,
> Steffen
>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] ARM: socfpga: dts: add reset-controller

[Steffen Trumtrar]

Dinh Nguyen <dinh.linux@gmail.com> writes:

> On 4/7/14 5:41 PM, Steffen Trumtrar wrote:
>> On Mon, Apr 07, 2014 at 04:12:01PM -0500, Dinh Nguyen wrote:
>>> On Fri, 2014-04-04 at 18:31 +0200, Steffen Trumtrar wrote:
>>>> Add the necessary #reset-cells property to the rst-mgr node and
>>>> provide a header-file with all possible resets specified.
>>>>
>>>> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
>>>> ---
>>>>  arch/arm/boot/dts/socfpga.dtsi           |  8 ++-
>>>>  include/dt-bindings/reset/altr,rst-mgr.h | 90 ++++++++++++++++++++++++++++++++
>>> Does this file belong under dt-bindings/reset/ or
>>> dt-bindings/reset-controller/?
>>>
>> ATM there is no such directory, or is there ?
>> So, reset seems appropriate I think.
> commit [6b7f06 ARM: STi: STiH415: Add reset controller support.]
> Seems more appropriate for this to go into reset-controller.
>

Okay, the current mainline master now has include/dt-bindings/reset and
include/dt-bindings/reset-controller.

I think one of those directories shouldn't be there, but be merged in
the other instead. But which one? Where shall I put my precious code?


Regards,
Steffen

-- 
Pengutronix e.K.                           | Steffen Trumtrar            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 3/3] ARM: socfpga: dts: add reset-controller

[Philipp Zabel]

Am Mittwoch, den 09.04.2014, 10:37 +0200 schrieb Steffen Trumtrar:
> Dinh Nguyen <dinh.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
> 
> > On 4/7/14 5:41 PM, Steffen Trumtrar wrote:
> >> On Mon, Apr 07, 2014 at 04:12:01PM -0500, Dinh Nguyen wrote:
> >>> On Fri, 2014-04-04 at 18:31 +0200, Steffen Trumtrar wrote:
> >>>> Add the necessary #reset-cells property to the rst-mgr node and
> >>>> provide a header-file with all possible resets specified.
> >>>>
> >>>> Signed-off-by: Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> >>>> ---
> >>>>  arch/arm/boot/dts/socfpga.dtsi           |  8 ++-
> >>>>  include/dt-bindings/reset/altr,rst-mgr.h | 90 ++++++++++++++++++++++++++++++++
> >>> Does this file belong under dt-bindings/reset/ or
> >>> dt-bindings/reset-controller/?
> >>>
> >> ATM there is no such directory, or is there ?
> >> So, reset seems appropriate I think.
> > commit [6b7f06 ARM: STi: STiH415: Add reset controller support.]
> > Seems more appropriate for this to go into reset-controller.
> >
> 
> Okay, the current mainline master now has include/dt-bindings/reset and
> include/dt-bindings/reset-controller.
> 
> I think one of those directories shouldn't be there, but be merged in
> the other instead. But which one? Where shall I put my precious code?

My vote would be for dt-bindings/reset/, since that's shorter and
2ec941304df9e1cd5e2f2404303a5fab0929969a came before
6b7f06cc805bb4755e69cd916b3f565947e0a77a.

Maybe something also should be done about dt-bindings/clk/ and
dt-bindings/clock/.

regards
Philipp

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] ARM: socfpga: dts: add reset-controller

[Steffen Trumtrar]

Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> writes:

> Am Mittwoch, den 09.04.2014, 10:37 +0200 schrieb Steffen Trumtrar:
>> Dinh Nguyen <dinh.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
>> 
>> > On 4/7/14 5:41 PM, Steffen Trumtrar wrote:
>> >> On Mon, Apr 07, 2014 at 04:12:01PM -0500, Dinh Nguyen wrote:
>> >>> On Fri, 2014-04-04 at 18:31 +0200, Steffen Trumtrar wrote:
>> >>>> Add the necessary #reset-cells property to the rst-mgr node and
>> >>>> provide a header-file with all possible resets specified.
>> >>>>
>> >>>> Signed-off-by: Steffen Trumtrar <s.trumtrar-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>> >>>> ---
>> >>>>  arch/arm/boot/dts/socfpga.dtsi           |  8 ++-
>> >>>>  include/dt-bindings/reset/altr,rst-mgr.h | 90 ++++++++++++++++++++++++++++++++
>> >>> Does this file belong under dt-bindings/reset/ or
>> >>> dt-bindings/reset-controller/?
>> >>>
>> >> ATM there is no such directory, or is there ?
>> >> So, reset seems appropriate I think.
>> > commit [6b7f06 ARM: STi: STiH415: Add reset controller support.]
>> > Seems more appropriate for this to go into reset-controller.
>> >
>> 
>> Okay, the current mainline master now has include/dt-bindings/reset and
>> include/dt-bindings/reset-controller.
>> 
>> I think one of those directories shouldn't be there, but be merged in
>> the other instead. But which one? Where shall I put my precious code?
>
> My vote would be for dt-bindings/reset/, since that's shorter and
> 2ec941304df9e1cd5e2f2404303a5fab0929969a came before
> 6b7f06cc805bb4755e69cd916b3f565947e0a77a.
>

Agreed.

Regards,
Steffen

-- 
Pengutronix e.K.                           | Steffen Trumtrar            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html