[PATCH 0/3] Add Reset Controller for MediaTek SoC

[PATCH 0/3] Add Reset Controller for MediaTek SoC

[flora.fu at mediatek.com]

This driver is based on 3.18-rc1. . 
This series adds support generic reset controller for MediaTek SoC.
Reset registers contain several bytes and each bit is able to reset individual module in SoC.
 - Patch 1/3: Add a driver in reset controller
 - Patch 2/3: Add device tree bindings
 - Patch 3/3: Add reset controller to MT8135 board dts
   
 Flora Fu (3):
  ARM: mediatek: Add Reset Controller for MediaTek SoC
  dt-bindings: Add Reset Controller for MediaTek SoC
  ARM: dts: mt8135: Add Reset Controller for MediaTek SoC

 .../devicetree/bindings/reset/mediatek,reset.txt   |  37 +++++
 arch/arm/boot/dts/mt8135.dtsi                      |  22 +++
 drivers/reset/Makefile                             |   1 +
 drivers/reset/reset-mtk.c                          | 149 +++++++++++++++++++++
 4 files changed, 209 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/mediatek,reset.txt
 create mode 100644 drivers/reset/reset-mtk.c

--
1.8.1.1.dirty

[PATCH 1/3] ARM: mediatek: Add Reset Controller for MediaTek SoC

[flora.fu at mediatek.com]

From: Flora Fu <flora.fu@mediatek.com>

Add a driver in reset controller.

Signed-off-by: Flora Fu <flora.fu@mediatek.com>
---
 drivers/reset/Makefile    |   1 +
 drivers/reset/reset-mtk.c | 149 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 150 insertions(+)
 create mode 100644 drivers/reset/reset-mtk.c

diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 60fed3d..adcebdf 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -2,3 +2,4 @@ obj-$(CONFIG_RESET_CONTROLLER) += core.o
 obj-$(CONFIG_ARCH_SOCFPGA) += reset-socfpga.o
 obj-$(CONFIG_ARCH_SUNXI) += reset-sunxi.o
 obj-$(CONFIG_ARCH_STI) += sti/
+obj-$(CONFIG_ARCH_MEDIATEK) += reset-mtk.o
diff --git a/drivers/reset/reset-mtk.c b/drivers/reset/reset-mtk.c
new file mode 100644
index 0000000..e2211f7
--- /dev/null
+++ b/drivers/reset/reset-mtk.c
@@ -0,0 +1,149 @@
+/*
+ * Copyright (c) 2014 MediaTek Inc.
+ * Author: Flora.Fu <flora.fu@mediatek.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * 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.
+ */
+
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/reset-controller.h>
+#include <linux/slab.h>
+
+struct mt_reset_data {
+	struct regmap *regmap;
+	unsigned int resetbase;
+	unsigned int size;
+	struct reset_controller_dev rcdev;
+};
+
+static int mt_reset_assert(struct reset_controller_dev *rcdev,
+			      unsigned long id)
+{
+	struct regmap *regmap;
+	unsigned addr;
+	unsigned mask;
+	struct mt_reset_data *data = container_of(rcdev,
+						     struct mt_reset_data,
+						     rcdev);
+	regmap = data->regmap;
+	addr = data->resetbase + ((id / 32) << 2);
+	mask = BIT(id % 32);
+
+	return regmap_update_bits(regmap, addr, mask, mask);
+}
+
+static int mt_reset_deassert(struct reset_controller_dev *rcdev,
+				unsigned long id)
+{
+	struct regmap *regmap;
+	unsigned addr;
+	unsigned mask;
+	struct mt_reset_data *data = container_of(rcdev,
+						     struct mt_reset_data,
+						     rcdev);
+
+	regmap = data->regmap;
+	addr = data->resetbase + ((id / 32) << 2);
+	mask = BIT(id % 32);
+
+	return regmap_update_bits(regmap, addr, mask, ~mask);
+}
+
+static struct reset_control_ops mt_reset_ops = {
+	.assert = mt_reset_assert,
+	.deassert = mt_reset_deassert,
+};
+
+static int mt_reset_probe(struct platform_device *pdev)
+{
+	struct mt_reset_data *data;
+	struct device_node *np = pdev->dev.of_node;
+	unsigned int resetbase;
+	unsigned int width;
+	int ret;
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->regmap = syscon_regmap_lookup_by_phandle(np,
+		"mediatek,syscon-reset");
+	if (IS_ERR(data->regmap)) {
+		dev_err(&pdev->dev, "couldn't get syscon-reset regmap\n");
+		return PTR_ERR(data->regmap);
+	}
+
+	ret = of_property_read_u32_index(np, "mediatek,syscon-reset", 1,
+		&resetbase);
+	if (ret) {
+		dev_err(&pdev->dev, "couldn't read reset base from syscon!\n");
+		return -EINVAL;
+	}
+
+	ret = of_property_read_u32_index(np, "mediatek,syscon-reset", 2,
+		&width);
+	if (ret) {
+		dev_err(&pdev->dev, "couldn't read reset bytes from syscon!\n");
+		return -EINVAL;
+	}
+
+	data->resetbase = resetbase;
+	data->size = width >> 2;
+	data->rcdev.owner = THIS_MODULE;
+	data->rcdev.nr_resets = data->size * 32;
+	data->rcdev.ops = &mt_reset_ops;
+	data->rcdev.of_node = pdev->dev.of_node;
+
+	return reset_controller_register(&data->rcdev);
+}
+
+static int mt_reset_remove(struct platform_device *pdev)
+{
+	struct mt_reset_data *data = platform_get_drvdata(pdev);
+
+	reset_controller_unregister(&data->rcdev);
+	return 0;
+}
+
+static const struct of_device_id mt_reset_dt_ids[] = {
+	{ .compatible = "mediatek,reset", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, mt_reset_dt_ids);
+
+static struct platform_driver mt_reset_driver = {
+	.probe = mt_reset_probe,
+	.remove = mt_reset_remove,
+	.driver = {
+		.name = "mtk-reset",
+		.owner = THIS_MODULE,
+		.of_match_table = mt_reset_dt_ids,
+	},
+};
+
+static int __init mt_reset_init(void)
+{
+	return platform_driver_register(&mt_reset_driver);
+}
+module_init(mt_reset_init);
+
+static void __exit mt_reset_exit(void)
+{
+	platform_driver_unregister(&mt_reset_driver);
+}
+module_exit(mt_reset_exit);
+
+MODULE_AUTHOR("Flora Fu <flora.fu@mediatek.com>");
+MODULE_DESCRIPTION("MediaTek SoC Generic Reset Controller");
+MODULE_LICENSE("GPL");
-- 
1.8.1.1.dirty

[PATCH 1/3] ARM: mediatek: Add Reset Controller for MediaTek SoC

[Philipp Zabel]

Hi Flora,

apart from the question about whether the regmap should be looked up by
phandle or from the parent device if we can put reset nodes as children
of the syscon nodes, I have two small nitpicks below.

Am Donnerstag, den 30.10.2014, 11:12 +0800 schrieb
flora.fu at mediatek.com:
> From: Flora Fu <flora.fu@mediatek.com>
> 
> Add a driver in reset controller.
> 
> Signed-off-by: Flora Fu <flora.fu@mediatek.com>
> ---
>  drivers/reset/Makefile    |   1 +
>  drivers/reset/reset-mtk.c | 149 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 150 insertions(+)
>  create mode 100644 drivers/reset/reset-mtk.c
> 
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 60fed3d..adcebdf 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -2,3 +2,4 @@ obj-$(CONFIG_RESET_CONTROLLER) += core.o
>  obj-$(CONFIG_ARCH_SOCFPGA) += reset-socfpga.o
>  obj-$(CONFIG_ARCH_SUNXI) += reset-sunxi.o
>  obj-$(CONFIG_ARCH_STI) += sti/
> +obj-$(CONFIG_ARCH_MEDIATEK) += reset-mtk.o
> diff --git a/drivers/reset/reset-mtk.c b/drivers/reset/reset-mtk.c
> new file mode 100644
> index 0000000..e2211f7
> --- /dev/null
> +++ b/drivers/reset/reset-mtk.c
> @@ -0,0 +1,149 @@
> +/*
> + * Copyright (c) 2014 MediaTek Inc.
> + * Author: Flora.Fu <flora.fu@mediatek.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * 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.
> + */
> +
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/reset-controller.h>
> +#include <linux/slab.h>

Do you need to include <linux/slab.h>?

> +struct mt_reset_data {
> +	struct regmap *regmap;
> +	unsigned int resetbase;
> +	unsigned int size;
> +	struct reset_controller_dev rcdev;
> +};
> +
> +static int mt_reset_assert(struct reset_controller_dev *rcdev,
> +			      unsigned long id)
> +{
> +	struct regmap *regmap;
> +	unsigned addr;
> +	unsigned mask;
> +	struct mt_reset_data *data = container_of(rcdev,
> +						     struct mt_reset_data,
> +						     rcdev);
> +	regmap = data->regmap;
> +	addr = data->resetbase + ((id / 32) << 2);
> +	mask = BIT(id % 32);
> +
> +	return regmap_update_bits(regmap, addr, mask, mask);
> +}
> +
> +static int mt_reset_deassert(struct reset_controller_dev *rcdev,
> +				unsigned long id)
> +{
> +	struct regmap *regmap;
> +	unsigned addr;
> +	unsigned mask;
> +	struct mt_reset_data *data = container_of(rcdev,
> +						     struct mt_reset_data,
> +						     rcdev);
> +
> +	regmap = data->regmap;
> +	addr = data->resetbase + ((id / 32) << 2);
> +	mask = BIT(id % 32);
> +
> +	return regmap_update_bits(regmap, addr, mask, ~mask);
> +}
> +
> +static struct reset_control_ops mt_reset_ops = {
> +	.assert = mt_reset_assert,
> +	.deassert = mt_reset_deassert,
> +};
> +
> +static int mt_reset_probe(struct platform_device *pdev)
> +{
> +	struct mt_reset_data *data;
> +	struct device_node *np = pdev->dev.of_node;
> +	unsigned int resetbase;
> +	unsigned int width;
> +	int ret;
> +
> +	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	data->regmap = syscon_regmap_lookup_by_phandle(np,
> +		"mediatek,syscon-reset");
> +	if (IS_ERR(data->regmap)) {
> +		dev_err(&pdev->dev, "couldn't get syscon-reset regmap\n");
> +		return PTR_ERR(data->regmap);
> +	}
> +
> +	ret = of_property_read_u32_index(np, "mediatek,syscon-reset", 1,
> +		&resetbase);
> +	if (ret) {
> +		dev_err(&pdev->dev, "couldn't read reset base from syscon!\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = of_property_read_u32_index(np, "mediatek,syscon-reset", 2,
> +		&width);
> +	if (ret) {
> +		dev_err(&pdev->dev, "couldn't read reset bytes from syscon!\n");
> +		return -EINVAL;
> +	}
> +
> +	data->resetbase = resetbase;
> +	data->size = width >> 2;

It is common to measure size of a resource in bytes, so maybe it would
be easier to understand if you renamed width to size and renamed
data->size to something like data->num_regs.

> +	data->rcdev.owner = THIS_MODULE;
> +	data->rcdev.nr_resets = data->size * 32;
> +	data->rcdev.ops = &mt_reset_ops;
> +	data->rcdev.of_node = pdev->dev.of_node;
> +
> +	return reset_controller_register(&data->rcdev);
> +}
> +
> +static int mt_reset_remove(struct platform_device *pdev)
> +{
> +	struct mt_reset_data *data = platform_get_drvdata(pdev);
> +
> +	reset_controller_unregister(&data->rcdev);
> +	return 0;
> +}
> +
> +static const struct of_device_id mt_reset_dt_ids[] = {
> +	{ .compatible = "mediatek,reset", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, mt_reset_dt_ids);
> +
> +static struct platform_driver mt_reset_driver = {
> +	.probe = mt_reset_probe,
> +	.remove = mt_reset_remove,
> +	.driver = {
> +		.name = "mtk-reset",
> +		.owner = THIS_MODULE,
> +		.of_match_table = mt_reset_dt_ids,
> +	},
> +};
> +
> +static int __init mt_reset_init(void)
> +{
> +	return platform_driver_register(&mt_reset_driver);
> +}
> +module_init(mt_reset_init);
> +
> +static void __exit mt_reset_exit(void)
> +{
> +	platform_driver_unregister(&mt_reset_driver);
> +}
> +module_exit(mt_reset_exit);

You can use the module_platform_driver(mt_reset_driver); macro here.

> +MODULE_AUTHOR("Flora Fu <flora.fu@mediatek.com>");
> +MODULE_DESCRIPTION("MediaTek SoC Generic Reset Controller");
> +MODULE_LICENSE("GPL");

regards
Philipp

[PATCH 1/3] ARM: mediatek: Add Reset Controller for MediaTek SoC

[Matthias Brugger]

2014-10-30 4:12 GMT+01:00  <flora.fu@mediatek.com>:
> From: Flora Fu <flora.fu@mediatek.com>
>
> Add a driver in reset controller.
>
> Signed-off-by: Flora Fu <flora.fu@mediatek.com>
> ---
>  drivers/reset/Makefile    |   1 +
>  drivers/reset/reset-mtk.c | 149 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 150 insertions(+)
>  create mode 100644 drivers/reset/reset-mtk.c
>
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 60fed3d..adcebdf 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -2,3 +2,4 @@ obj-$(CONFIG_RESET_CONTROLLER) += core.o
>  obj-$(CONFIG_ARCH_SOCFPGA) += reset-socfpga.o
>  obj-$(CONFIG_ARCH_SUNXI) += reset-sunxi.o
>  obj-$(CONFIG_ARCH_STI) += sti/
> +obj-$(CONFIG_ARCH_MEDIATEK) += reset-mtk.o
> diff --git a/drivers/reset/reset-mtk.c b/drivers/reset/reset-mtk.c
> new file mode 100644
> index 0000000..e2211f7
> --- /dev/null
> +++ b/drivers/reset/reset-mtk.c
> @@ -0,0 +1,149 @@
> +/*
> + * Copyright (c) 2014 MediaTek Inc.
> + * Author: Flora.Fu <flora.fu@mediatek.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * 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.
> + */
> +
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/reset-controller.h>
> +#include <linux/slab.h>
> +
> +struct mt_reset_data {

Please rename mt_ prefixes to the prefix mtk_ to keep consistent
naming in the kernel.

> +       struct regmap *regmap;
> +       unsigned int resetbase;
> +       unsigned int size;
> +       struct reset_controller_dev rcdev;
> +};
> +
> +static int mt_reset_assert(struct reset_controller_dev *rcdev,
> +                             unsigned long id)
> +{
> +       struct regmap *regmap;
> +       unsigned addr;
> +       unsigned mask;
> +       struct mt_reset_data *data = container_of(rcdev,
> +                                                    struct mt_reset_data,
> +                                                    rcdev);
> +       regmap = data->regmap;
> +       addr = data->resetbase + ((id / 32) << 2);
> +       mask = BIT(id % 32);
> +
> +       return regmap_update_bits(regmap, addr, mask, mask);
> +}
> +
> +static int mt_reset_deassert(struct reset_controller_dev *rcdev,
> +                               unsigned long id)
> +{
> +       struct regmap *regmap;
> +       unsigned addr;
> +       unsigned mask;
> +       struct mt_reset_data *data = container_of(rcdev,
> +                                                    struct mt_reset_data,
> +                                                    rcdev);
> +
> +       regmap = data->regmap;
> +       addr = data->resetbase + ((id / 32) << 2);
> +       mask = BIT(id % 32);
> +
> +       return regmap_update_bits(regmap, addr, mask, ~mask);
> +}
> +
> +static struct reset_control_ops mt_reset_ops = {
> +       .assert = mt_reset_assert,
> +       .deassert = mt_reset_deassert,
> +};
> +
> +static int mt_reset_probe(struct platform_device *pdev)
> +{
> +       struct mt_reset_data *data;
> +       struct device_node *np = pdev->dev.of_node;
> +       unsigned int resetbase;
> +       unsigned int width;
> +       int ret;
> +
> +       data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> +       if (!data)
> +               return -ENOMEM;
> +
> +       data->regmap = syscon_regmap_lookup_by_phandle(np,
> +               "mediatek,syscon-reset");
> +       if (IS_ERR(data->regmap)) {
> +               dev_err(&pdev->dev, "couldn't get syscon-reset regmap\n");
> +               return PTR_ERR(data->regmap);
> +       }
> +
> +       ret = of_property_read_u32_index(np, "mediatek,syscon-reset", 1,
> +               &resetbase);
> +       if (ret) {
> +               dev_err(&pdev->dev, "couldn't read reset base from syscon!\n");
> +               return -EINVAL;
> +       }
> +
> +       ret = of_property_read_u32_index(np, "mediatek,syscon-reset", 2,
> +               &width);
> +       if (ret) {
> +               dev_err(&pdev->dev, "couldn't read reset bytes from syscon!\n");
> +               return -EINVAL;
> +       }
> +
> +       data->resetbase = resetbase;
> +       data->size = width >> 2;
> +       data->rcdev.owner = THIS_MODULE;
> +       data->rcdev.nr_resets = data->size * 32;
> +       data->rcdev.ops = &mt_reset_ops;
> +       data->rcdev.of_node = pdev->dev.of_node;
> +
> +       return reset_controller_register(&data->rcdev);
> +}
> +
> +static int mt_reset_remove(struct platform_device *pdev)
> +{
> +       struct mt_reset_data *data = platform_get_drvdata(pdev);
> +
> +       reset_controller_unregister(&data->rcdev);
> +       return 0;
> +}
> +
> +static const struct of_device_id mt_reset_dt_ids[] = {
> +       { .compatible = "mediatek,reset", },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(of, mt_reset_dt_ids);
> +
> +static struct platform_driver mt_reset_driver = {
> +       .probe = mt_reset_probe,
> +       .remove = mt_reset_remove,
> +       .driver = {
> +               .name = "mtk-reset",
> +               .owner = THIS_MODULE,
> +               .of_match_table = mt_reset_dt_ids,
> +       },
> +};
> +
> +static int __init mt_reset_init(void)
> +{
> +       return platform_driver_register(&mt_reset_driver);
> +}
> +module_init(mt_reset_init);
> +
> +static void __exit mt_reset_exit(void)
> +{
> +       platform_driver_unregister(&mt_reset_driver);
> +}
> +module_exit(mt_reset_exit);
> +
> +MODULE_AUTHOR("Flora Fu <flora.fu@mediatek.com>");
> +MODULE_DESCRIPTION("MediaTek SoC Generic Reset Controller");
> +MODULE_LICENSE("GPL");
> --
> 1.8.1.1.dirty
>



-- 
motzblog.wordpress.com

[PATCH 2/3] dt-bindings: Add Reset Controller for MediaTek SoC

[flora.fu at mediatek.com]

From: Flora Fu <flora.fu@mediatek.com>

Add device tree bindings.

Signed-off-by: Flora Fu <flora.fu@mediatek.com>
---
 .../devicetree/bindings/reset/mediatek,reset.txt   | 37 ++++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/mediatek,reset.txt

diff --git a/Documentation/devicetree/bindings/reset/mediatek,reset.txt b/Documentation/devicetree/bindings/reset/mediatek,reset.txt
new file mode 100644
index 0000000..0dd23e4
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/mediatek,reset.txt
@@ -0,0 +1,37 @@
+MediaTek SoC Reset Controller
+======================================
+
+Required properties:
+- compatible : "mediatek,reset"
+- #reset-cells: 1
+- mediatek,syscon-reset: The first parameter is refer to the syscon registers base. 
+  Follows are reset base address offset and byte width.
+
+example:
+	infrarst: reset-controller at 10001030 {
+		#reset-cells = <1>;
+		compatible = "mediatek,mt8135-infracfg-reset", "mediatek,reset";
+		mediatek,syscon-reset = <&infracfg 0x30 0x8>;
+	};
+
+Specifying reset lines connected to IP modules
+==============================================
+
+The reset controller(mtk-reset) manages various reset sources. Those device nodes should
+specify the reset line on the rstc in their resets property, containing a phandle to the
+rstc device node and a RESET_INDEX specifying which module to reset, as described in 
+reset.txt.
+
+For MediaTek SoC, RESET_INDEX is reset bit defined in INFRACFG or PERICFG registers.
+
+example:
+pwrap: pwrap at 1000f000 {
+	compatible = "mediatek,mt8135-pwrap";
+	reg = <0 0x1000f000 0 0x1000>,
+		<0 0x11017000 0 0x1000>;
+	reg-names = "pwrap-base",
+		"pwrap-bridge-base";
+	resets = <&infrarst 7>, <&perirst 34>;
+	reset-names = "infrarst", "perirst";
+	};
+};
\ No newline at end of file
-- 
1.8.1.1.dirty

[PATCH 2/3] dt-bindings: Add Reset Controller for MediaTek SoC

[Arnd Bergmann]

On Thursday 30 October 2014 11:12:12 flora.fu at mediatek.com wrote:
> +
> +example:
> +       infrarst: reset-controller at 10001030 {
> +               #reset-cells = <1>;
> +               compatible = "mediatek,mt8135-infracfg-reset", "mediatek,reset";
> +               mediatek,syscon-reset = <&infracfg 0x30 0x8>;
> +       };

I think you need to change the 'unit-address' now, i.e. the @10001030
value above no longer matches any reg property. You could just
remove that part.

> +Specifying reset lines connected to IP modules
> +==============================================
> +
> +The reset controller(mtk-reset) manages various reset sources. Those device nodes should
> +specify the reset line on the rstc in their resets property, containing a phandle to the
> +rstc device node and a RESET_INDEX specifying which module to reset, as described in 
> +reset.txt.
> +
> +For MediaTek SoC, RESET_INDEX is reset bit defined in INFRACFG or PERICFG registers.
> +
> +example:
> +pwrap: pwrap at 1000f000 {
> +       compatible = "mediatek,mt8135-pwrap";
> +       reg = <0 0x1000f000 0 0x1000>,
> +               <0 0x11017000 0 0x1000>;
> +       reg-names = "pwrap-base",
> +               "pwrap-bridge-base";
> +       resets = <&infrarst 7>, <&perirst 34>;
> +       reset-names = "infrarst", "perirst";
> +       };
> +};
> 

You have an extraneous '};' here.

	Arnd

[PATCH 3/3] ARM: dts: mt8135: Add Reset Controller for MediaTek SoC

[flora.fu at mediatek.com]

From: Flora Fu <flora.fu@mediatek.com>

Add reset controller to MT8135 board dts.

Signed-off-by: Flora Fu <flora.fu@mediatek.com>
---
 arch/arm/boot/dts/mt8135.dtsi | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/arm/boot/dts/mt8135.dtsi b/arch/arm/boot/dts/mt8135.dtsi
index 90a56ad..0b1ddc9 100644
--- a/arch/arm/boot/dts/mt8135.dtsi
+++ b/arch/arm/boot/dts/mt8135.dtsi
@@ -102,6 +102,28 @@
 			clock-names = "system-clk", "rtc-clk";
 		};
 
+		infracfg: syscon at 10001000 {
+			compatible = "mediatek,mt8135-infracfg", "syscon";
+			reg = <0 0x10001000 0 0x1000>;
+		};
+
+		pericfg: syscon at 10003000 {
+			compatible = "mediatek,mt8135-pericfg", "syscon";
+			reg = <0 0x10003000 0 0x1000>;
+		};
+
+		infrarst: reset-controller at 10001030 {
+			#reset-cells = <1>;
+			compatible = "mediatek,mt8135-infracfg-reset", "mediatek,reset";
+			mediatek,syscon-reset = <&infracfg 0x30 0x8>;
+		};
+
+		perirst: reset-controller at 10003000 {
+			#reset-cells = <1>;
+			compatible = "mediatek,mt8135-pericfg-reset", "mediatek,reset";
+			mediatek,syscon-reset = <&pericfg 0x00 0x8>;
+		};
+
 		gic: interrupt-controller at 10211000 {
 			compatible = "arm,cortex-a15-gic";
 			interrupt-controller;
-- 
1.8.1.1.dirty

[PATCH 3/3] ARM: dts: mt8135: Add Reset Controller for MediaTek SoC

[Philipp Zabel]

Hi,

Am Donnerstag, den 30.10.2014, 11:12 +0800 schrieb
flora.fu at mediatek.com:
> From: Flora Fu <flora.fu@mediatek.com>
> 
> Add reset controller to MT8135 board dts.
> 
> Signed-off-by: Flora Fu <flora.fu@mediatek.com>
> ---
>  arch/arm/boot/dts/mt8135.dtsi | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/mt8135.dtsi b/arch/arm/boot/dts/mt8135.dtsi
> index 90a56ad..0b1ddc9 100644
> --- a/arch/arm/boot/dts/mt8135.dtsi
> +++ b/arch/arm/boot/dts/mt8135.dtsi
> @@ -102,6 +102,28 @@
>  			clock-names = "system-clk", "rtc-clk";
>  		};
>  
> +		infracfg: syscon at 10001000 {
> +			compatible = "mediatek,mt8135-infracfg", "syscon";
> +			reg = <0 0x10001000 0 0x1000>;
> +		};
> +
> +		pericfg: syscon at 10003000 {
> +			compatible = "mediatek,mt8135-pericfg", "syscon";
> +			reg = <0 0x10003000 0 0x1000>;
> +		};
> +
> +		infrarst: reset-controller at 10001030 {
> +			#reset-cells = <1>;
> +			compatible = "mediatek,mt8135-infracfg-reset", "mediatek,reset";
> +			mediatek,syscon-reset = <&infracfg 0x30 0x8>;
> +		};
> +
> +		perirst: reset-controller at 10003000 {
> +			#reset-cells = <1>;
> +			compatible = "mediatek,mt8135-pericfg-reset", "mediatek,reset";
> +			mediatek,syscon-reset = <&pericfg 0x00 0x8>;
> +		};
> +

Since the reset controller driver accesses registers solely through the
syscon regmap, I'd prefer to keep with the device tree control graph
concept and make the reset-controller nodes children of the syscon
nodes. I've brought this up before: https://lkml.org/lkml/2014/5/27/422,
and I think this is another case where child node support for syscon
makes sense:

        infracfg: syscon at 10001000 {
                compatible = "mediatek,mt8135-infracfg", "syscon";
                reg = <0 0x10001000 0 0x1000>;
                
                infrarst: reset-controller at 30 {
                        #reset-cells = <1>;
                        compatible = "mediatek,mt8135-infracfg-reset", "mediatek,reset";
                        reg = <0x30 0x8>;
                };
        };

        pericfg: syscon at 10003000 {
                compatible = "mediatek,mt8135-pericfg", "syscon";
                reg = <0 0x10003000 0 0x1000>;

                perirst: reset-controller at 00 {                                                                                               
                        #reset-cells = <1>;
                        compatible = "mediatek,mt8135-pericfg-reset", "mediatek,reset";
                        reg = <0x00 0x8>;
                };
        };

regards
Philipp

[PATCH 3/3] ARM: dts: mt8135: Add Reset Controller for MediaTek SoC

[Arnd Bergmann]

On Thursday 30 October 2014 10:02:25 Philipp Zabel wrote:
> Since the reset controller driver accesses registers solely through the
> syscon regmap, I'd prefer to keep with the device tree control graph
> concept and make the reset-controller nodes children of the syscon
> nodes. I've brought this up before: https://lkml.org/lkml/2014/5/27/422,
> and I think this is another case where child node support for syscon
> makes sense:
> 
>         infracfg: syscon at 10001000 {
>                 compatible = "mediatek,mt8135-infracfg", "syscon";
>                 reg = <0 0x10001000 0 0x1000>;
>                 
>                 infrarst: reset-controller at 30 {
>                         #reset-cells = <1>;
>                         compatible = "mediatek,mt8135-infracfg-reset", "mediatek,reset";
>                         reg = <0x30 0x8>;
>                 };
>         };
> 

Yes, this looks much better to me too.

	Arnd

[PATCH 3/3] ARM: dts: mt8135: Add Reset Controller for MediaTek SoC

[Flora Fu]

Hi, Philipp,

On Thu, 2014-10-30 at 10:02 +0100, Philipp Zabel wrote:
> Since the reset controller driver accesses registers solely through the
> syscon regmap, I'd prefer to keep with the device tree control graph
> concept and make the reset-controller nodes children of the syscon
> nodes. I've brought this up before: https://lkml.org/lkml/2014/5/27/422,
> and I think this is another case where child node support for syscon
> makes sense:
> 
>         infracfg: syscon at 10001000 {
>                 compatible = "mediatek,mt8135-infracfg", "syscon";
>                 reg = <0 0x10001000 0 0x1000>;
>                 
>                 infrarst: reset-controller at 30 {
>                         #reset-cells = <1>;
>                         compatible = "mediatek,mt8135-infracfg-reset", "mediatek,reset";
>                         reg = <0x30 0x8>;
>                 };
>         };
> 
>         pericfg: syscon at 10003000 {
>                 compatible = "mediatek,mt8135-pericfg", "syscon";
>                 reg = <0 0x10003000 0 0x1000>;
> 
>                 perirst: reset-controller at 00 {                                                                                               
>                         #reset-cells = <1>;
>                         compatible = "mediatek,mt8135-pericfg-reset", "mediatek,reset";
>                         reg = <0x00 0x8>;
>                 };
>         };
> 
> regards
> Philipp
> 

Yes, such dts arrangement looks better to me. Implementation in this
version is trying to doing the same thing as your proposal. The new
property "mediatek,syscon-reset = <&infracfg 0x30 0x8>;" specifies base
address of reset and byte width for controlling resets.

If https://lkml.org/lkml/2014/5/27/422 is adopt into kernel release, it
will be well organized to configure reset controller as child of regmap
which is compatible to syscon.

In reset driver, it is able to get syscon regmap from parent node and
retrieve the address offset and byte with for controlling resets.
--- 
syscon_np = of_get_parent(np);
data->regmap = syscon_node_to_regmap(syscon_np);
if (IS_ERR(data->regmap)) {
	dev_err(&pdev->dev, "couldn't get syscon-reset regmap\n");
	return PTR_ERR(data->regmap);
}
ret = of_property_read_u32_array(np, "reg", reg, 2);
if (ret) {
	dev_err(&pdev->dev, "couldn't read reset base from syscon!\n");
	return -EINVAL;
}

---


Thanks,
Flora