linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] Amlogic: Meson: Add reset controller
@ 2016-05-25  9:49 Neil Armstrong
  2016-05-25  9:49 ` [PATCH v2 1/4] reset: Add support for the Amlogic Meson SoC Reset Controller Neil Armstrong
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Neil Armstrong @ 2016-05-25  9:49 UTC (permalink / raw)
  To: linux-kernel, p.zabel, linux-arm-kernel, linux-amlogic
  Cc: Neil Armstrong, xing.xu, victor.wan, jerry.cao

Patchset to add and enable the reset controller driver on Meson SoCs platforms.

This reset controller has up to 256 reset lines with reset pulse generation only,
so the assert and deassert calls are not available.

Depends on the patch :
 - "reset: add devm_reset_controller_register API" [1]

Changes since v1 at http://lkml.kernel.org/r/1463732875-23141-1-git-send-email-narmstrong@baylibre.com :
- Remove _gxbb_ in names
- Depends build on ARCH_MESON only
- Add meson8b compatible and bindings
- Re-indent dt-bindings headers
- Switch to devm_ reset controller register
- Remove platform "remove" callback
- Update Dual BSD/GPL file headers

Changes since the RFC at http://lkml.kernel.org/r/1463148012-25988-1-git-send-email-narmstrong@baylibre.com :
- Fix register mapping and bit defines in bindings header
- Remove assert and unassert calls
- Fix missing __iomem

[1] https://patchwork.kernel.org/patch/8988471/

Neil Armstrong (4):
  reset: Add support for the Amlogic Meson SoC Reset Controller
  dt-bindings: reset: Add bindings for the Meson SoC Reset Controller
  ARM64: dts: amlogic: Enable Reset Controller on GXBB-based platforms
  ARM: dts: amlogic: Enable Reset Controller on Meson8b platforms

 .../bindings/reset/amlogic,meson-reset.txt         |  18 ++
 arch/arm/boot/dts/meson8b.dtsi                     |   7 +
 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi        |   7 +
 drivers/reset/Makefile                             |   1 +
 drivers/reset/reset-meson.c                        | 137 ++++++++++++++
 .../dt-bindings/reset/amlogic,meson-gxbb-reset.h   | 210 +++++++++++++++++++++
 include/dt-bindings/reset/amlogic,meson8b-reset.h  | 175 +++++++++++++++++
 7 files changed, 555 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/amlogic,meson-reset.txt
 create mode 100644 drivers/reset/reset-meson.c
 create mode 100644 include/dt-bindings/reset/amlogic,meson-gxbb-reset.h
 create mode 100644 include/dt-bindings/reset/amlogic,meson8b-reset.h

-- 
2.7.0

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/4] reset: Add support for the Amlogic Meson SoC Reset Controller
  2016-05-25  9:49 [PATCH v2 0/4] Amlogic: Meson: Add reset controller Neil Armstrong
@ 2016-05-25  9:49 ` Neil Armstrong
  2016-05-25 10:14   ` Philipp Zabel
       [not found]   ` <m2iny1h74c.fsf@baylibre.com>
  2016-05-25  9:49 ` [PATCH v2 2/4] dt-bindings: reset: Add bindings for the " Neil Armstrong
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 11+ messages in thread
From: Neil Armstrong @ 2016-05-25  9:49 UTC (permalink / raw)
  To: linux-kernel, p.zabel, linux-arm-kernel, linux-amlogic
  Cc: Neil Armstrong, xing.xu, victor.wan, jerry.cao

This patch adds the platform driver for the Amlogic Meson SoC Reset
Controller.

The Meson8b and GXBB SoCs are supported.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/reset/Makefile      |   1 +
 drivers/reset/reset-meson.c | 137 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 138 insertions(+)
 create mode 100644 drivers/reset/reset-meson.c

diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index f173fc3..03dc1bb 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_ARCH_LPC18XX) += reset-lpc18xx.o
 obj-$(CONFIG_ARCH_SOCFPGA) += reset-socfpga.o
 obj-$(CONFIG_ARCH_BERLIN) += reset-berlin.o
 obj-$(CONFIG_MACH_PISTACHIO) += reset-pistachio.o
+obj-$(CONFIG_ARCH_MESON) += reset-meson.o
 obj-$(CONFIG_ARCH_SUNXI) += reset-sunxi.o
 obj-$(CONFIG_ARCH_STI) += sti/
 obj-$(CONFIG_ARCH_HISI) += hisilicon/
diff --git a/drivers/reset/reset-meson.c b/drivers/reset/reset-meson.c
new file mode 100644
index 0000000..98087ae
--- /dev/null
+++ b/drivers/reset/reset-meson.c
@@ -0,0 +1,137 @@
+/*
+ * This file is provided under a dual BSD/GPLv2 license.  When using or
+ * redistributing this file, you may do so under either license.
+ *
+ * GPL LICENSE SUMMARY
+ *
+ * Copyright (c) 2016 BayLibre, SAS.
+ * Author: Neil Armstrong <narmstrong@baylibre.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ * The full GNU General Public License is included in this distribution
+ * in the file called COPYING.
+ *
+ * BSD LICENSE
+ *
+ * Copyright (c) 2016 BayLibre, SAS.
+ * Author: Neil Armstrong <narmstrong@baylibre.com>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ *   * Redistributions of source code must retain the above copyright
+ *     notice, this list of conditions and the following disclaimer.
+ *   * Redistributions in binary form must reproduce the above copyright
+ *     notice, this list of conditions and the following disclaimer in
+ *     the documentation and/or other materials provided with the
+ *     distribution.
+ *   * Neither the name of Intel Corporation nor the names of its
+ *     contributors may be used to endorse or promote products derived
+ *     from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+#define REG_COUNT	8
+#define BITS_PER_REG	32
+
+struct meson_reset {
+	void __iomem *reg_base;
+	struct reset_controller_dev rcdev;
+};
+
+static int meson_reset_reset(struct reset_controller_dev *rcdev,
+			      unsigned long id)
+{
+	struct meson_reset *data =
+		container_of(rcdev, struct meson_reset, rcdev);
+	unsigned int bank = id / BITS_PER_REG;
+	unsigned int offset = id % BITS_PER_REG;
+	void *reg_addr = data->reg_base + (bank << 2);
+
+	if (bank >= REG_COUNT)
+		return -EINVAL;
+
+	writel(BIT(offset), reg_addr);
+
+	return 0;
+}
+
+static const struct reset_control_ops meson_reset_ops = {
+	.reset		= meson_reset_reset,
+};
+
+static const struct of_device_id meson_reset_dt_ids[] = {
+	 { .compatible = "amlogic,meson8b-reset", },
+	 { .compatible = "amlogic,meson-gxbb-reset", },
+	 { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, meson_reset_dt_ids);
+
+static int meson_reset_probe(struct platform_device *pdev)
+{
+	struct meson_reset *data;
+	struct resource *res;
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	data->reg_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(data->reg_base))
+		return PTR_ERR(data->reg_base);
+
+	platform_set_drvdata(pdev, data);
+
+	data->rcdev.owner = THIS_MODULE;
+	data->rcdev.nr_resets = REG_COUNT * BITS_PER_REG;
+	data->rcdev.ops = &meson_reset_ops;
+	data->rcdev.of_node = pdev->dev.of_node;
+
+	return devm_reset_controller_register(&pdev->dev, &data->rcdev);
+}
+
+static struct platform_driver meson_reset_driver = {
+	.probe	= meson_reset_probe,
+	.driver = {
+		.name		= "meson_reset",
+		.of_match_table	= meson_reset_dt_ids,
+	},
+};
+
+module_platform_driver(meson_reset_driver);
+
+MODULE_ALIAS("platform:meson_reset");
+MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
+MODULE_DESCRIPTION("Amlogic Meson Reset Controller driver");
+MODULE_LICENSE("Dual BSD/GPL");
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/4] dt-bindings: reset: Add bindings for the Meson SoC Reset Controller
  2016-05-25  9:49 [PATCH v2 0/4] Amlogic: Meson: Add reset controller Neil Armstrong
  2016-05-25  9:49 ` [PATCH v2 1/4] reset: Add support for the Amlogic Meson SoC Reset Controller Neil Armstrong
@ 2016-05-25  9:49 ` Neil Armstrong
  2016-05-25 19:10   ` Rob Herring
  2016-05-25  9:49 ` [PATCH v2 3/4] ARM64: dts: amlogic: Enable Reset Controller on GXBB-based platforms Neil Armstrong
  2016-05-25  9:49 ` [PATCH v2 4/4] ARM: dts: amlogic: Enable Reset Controller on Meson8b platforms Neil Armstrong
  3 siblings, 1 reply; 11+ messages in thread
From: Neil Armstrong @ 2016-05-25  9:49 UTC (permalink / raw)
  To: linux-kernel, p.zabel, linux-arm-kernel, linux-amlogic
  Cc: Neil Armstrong, xing.xu, victor.wan, jerry.cao, devicetree

Add DT bindings for the Meson SoC Reset Controller documentation and the
associated include file.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 .../bindings/reset/amlogic,meson-reset.txt         |  18 ++
 .../dt-bindings/reset/amlogic,meson-gxbb-reset.h   | 210 +++++++++++++++++++++
 include/dt-bindings/reset/amlogic,meson8b-reset.h  | 175 +++++++++++++++++
 3 files changed, 403 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/amlogic,meson-reset.txt
 create mode 100644 include/dt-bindings/reset/amlogic,meson-gxbb-reset.h
 create mode 100644 include/dt-bindings/reset/amlogic,meson8b-reset.h

diff --git a/Documentation/devicetree/bindings/reset/amlogic,meson-reset.txt b/Documentation/devicetree/bindings/reset/amlogic,meson-reset.txt
new file mode 100644
index 0000000..e746b63
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/amlogic,meson-reset.txt
@@ -0,0 +1,18 @@
+Amlogic Meson SoC Reset Controller
+=======================================
+
+Please also refer to reset.txt in this directory for common reset
+controller binding usage.
+
+Required properties:
+- compatible: Should be "amlogic,meson8b-reset" or "amlogic,meson-gxbb-reset"
+- reg: should contain the register address base
+- #reset-cells: 1, see below
+
+example:
+
+reset: reset-controller {
+	compatible = "amlogic,meson-gxbb-reset";
+	reg = <0x0 0x04404 0x0 0x20>;
+	#reset-cells = <1>;
+};
diff --git a/include/dt-bindings/reset/amlogic,meson-gxbb-reset.h b/include/dt-bindings/reset/amlogic,meson-gxbb-reset.h
new file mode 100644
index 0000000..524d607
--- /dev/null
+++ b/include/dt-bindings/reset/amlogic,meson-gxbb-reset.h
@@ -0,0 +1,210 @@
+/*
+ * This file is provided under a dual BSD/GPLv2 license.  When using or
+ * redistributing this file, you may do so under either license.
+ *
+ * GPL LICENSE SUMMARY
+ *
+ * Copyright (c) 2016 BayLibre, SAS.
+ * Author: Neil Armstrong <narmstrong@baylibre.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ * The full GNU General Public License is included in this distribution
+ * in the file called COPYING.
+ *
+ * BSD LICENSE
+ *
+ * Copyright (c) 2016 BayLibre, SAS.
+ * Author: Neil Armstrong <narmstrong@baylibre.com>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ *   * Redistributions of source code must retain the above copyright
+ *     notice, this list of conditions and the following disclaimer.
+ *   * Redistributions in binary form must reproduce the above copyright
+ *     notice, this list of conditions and the following disclaimer in
+ *     the documentation and/or other materials provided with the
+ *     distribution.
+ *   * Neither the name of Intel Corporation nor the names of its
+ *     contributors may be used to endorse or promote products derived
+ *     from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+#ifndef _DT_BINDINGS_AMLOGIC_MESON_GXBB_RESET_H
+#define _DT_BINDINGS_AMLOGIC_MESON_GXBB_RESET_H
+
+/*	RESET0					*/
+#define RESET_HIU			0
+/*					1	*/
+#define RESET_DOS_RESET			2
+#define RESET_DDR_TOP			3
+#define RESET_DCU_RESET			4
+#define RESET_VIU			5
+#define RESET_AIU			6
+#define RESET_VID_PLL_DIV		7
+/*					8	*/
+#define RESET_PMUX			9
+#define RESET_VENC			10
+#define RESET_ASSIST			11
+#define RESET_AFIFO2			12
+#define RESET_VCBUS			13
+/*					14	*/
+/*					15	*/
+#define RESET_GIC			16
+#define RESET_CAPB3_DECODE		17
+#define RESET_NAND_CAPB3		18
+#define RESET_HDMITX_CAPB3		19
+#define RESET_MALI_CAPB3		20
+#define RESET_DOS_CAPB3			21
+#define RESET_SYS_CPU_CAPB3		22
+#define RESET_CBUS_CAPB3		23
+#define RESET_AHB_CNTL			24
+#define RESET_AHB_DATA			25
+#define RESET_VCBUS_CLK81		26
+#define RESET_MMC			27
+#define RESET_MIPI_0			28
+#define RESET_MIPI_1			29
+#define RESET_MIPI_2			30
+#define RESET_MIPI_3			31
+/*	RESET1					*/
+#define RESET_CPPM			32
+#define RESET_DEMUX			33
+#define RESET_USB_OTG			34
+#define RESET_DDR			35
+#define RESET_AO_RESET			36
+#define RESET_BT656			37
+#define RESET_AHB_SRAM			38
+/*					39	*/
+#define RESET_PARSER			40
+#define RESET_BLKMV			41
+#define RESET_ISA			42
+#define RESET_ETHERNET			43
+#define RESET_SD_EMMC_A			44
+#define RESET_SD_EMMC_B			45
+#define RESET_SD_EMMC_C			46
+#define RESET_ROM_BOOT			47
+#define RESET_SYS_CPU_0			48
+#define RESET_SYS_CPU_1			49
+#define RESET_SYS_CPU_2			50
+#define RESET_SYS_CPU_3			51
+#define RESET_SYS_CPU_CORE_0		52
+#define RESET_SYS_CPU_CORE_1		53
+#define RESET_SYS_CPU_CORE_2		54
+#define RESET_SYS_CPU_CORE_3		55
+#define RESET_SYS_PLL_DIV		56
+#define RESET_SYS_CPU_AXI		57
+#define RESET_SYS_CPU_L2		58
+#define RESET_SYS_CPU_P			59
+#define RESET_SYS_CPU_MBIST		60
+/*					61	*/
+/*					62	*/
+/*					63	*/
+/*	RESET2					*/
+#define RESET_VD_RMEM			64
+#define RESET_AUDIN			65
+#define RESET_HDMI_TX			66
+/*					67	*/
+/*					68	*/
+/*					69	*/
+#define RESET_GE2D			70
+#define RESET_PARSER_REG		71
+#define RESET_PARSER_FETCH		72
+#define RESET_PARSER_CTL		73
+#define RESET_PARSER_TOP		74
+/*					75	*/
+/*					76	*/
+#define RESET_AO_CPU_RESET		77
+#define RESET_MALI			78
+#define RESET_HDMI_SYSTEM_RESET		79
+/*					80-95	*/
+/*	RESET3					*/
+#define RESET_RING_OSCILLATOR		96
+#define RESET_SYS_CPU			97
+#define RESET_EFUSE			98
+#define RESET_SYS_CPU_BVCI		99
+#define RESET_AIFIFO			100
+#define RESET_TVFE			101
+#define RESET_AHB_BRIDGE_CNTL		102
+/*					103	*/
+#define RESET_AUDIO_DAC			104
+#define RESET_DEMUX_TOP			105
+#define RESET_DEMUX_DES			106
+#define RESET_DEMUX_S2P_0		107
+#define RESET_DEMUX_S2P_1		108
+#define RESET_DEMUX_RESET_0		109
+#define RESET_DEMUX_RESET_1		110
+#define RESET_DEMUX_RESET_2		111
+/*					112-127	*/
+/*	RESET4					*/
+/*					128	*/
+/*					129	*/
+/*					130	*/
+/*					131	*/
+#define RESET_DVIN_RESET		132
+#define RESET_RDMA			133
+#define RESET_VENCI			134
+#define RESET_VENCP			135
+/*					136	*/
+#define RESET_VDAC			137
+#define RESET_RTC			138
+/*					139	*/
+#define RESET_VDI6			140
+#define RESET_VENCL			141
+#define RESET_I2C_MASTER_2		142
+#define RESET_I2C_MASTER_1		143
+/*					144-159	*/
+/*	RESET5					*/
+/*					160-191	*/
+/*	RESET6					*/
+#define RESET_PERIPHS_GENERAL		192
+#define RESET_PERIPHS_SPICC		193
+#define RESET_PERIPHS_SMART_CARD	194
+#define RESET_PERIPHS_SAR_ADC		195
+#define RESET_PERIPHS_I2C_MASTER_0	196
+#define RESET_SANA			197
+/*					198	*/
+#define RESET_PERIPHS_STREAM_INTERFACE	199
+#define RESET_PERIPHS_SDIO		200
+#define RESET_PERIPHS_UART_0		201
+#define RESET_PERIPHS_UART_1_2		202
+#define RESET_PERIPHS_ASYNC_0		203
+#define RESET_PERIPHS_ASYNC_1		204
+#define RESET_PERIPHS_SPI_0		205
+#define RESET_PERIPHS_SDHC		206
+#define RESET_UART_SLIP			207
+/*					208-223	*/
+/*	RESET7					*/
+#define RESET_USB_DDR_0			224
+#define RESET_USB_DDR_1			225
+#define RESET_USB_DDR_2			226
+#define RESET_USB_DDR_3			227
+/*					228	*/
+#define RESET_DEVICE_MMC_ARB		229
+/*					230	*/
+#define RESET_VID_LOCK			231
+#define RESET_A9_DMC_PIPEL		232
+/*					233-255	*/
+
+#endif
diff --git a/include/dt-bindings/reset/amlogic,meson8b-reset.h b/include/dt-bindings/reset/amlogic,meson8b-reset.h
new file mode 100644
index 0000000..614aff2
--- /dev/null
+++ b/include/dt-bindings/reset/amlogic,meson8b-reset.h
@@ -0,0 +1,175 @@
+/*
+ * This file is provided under a dual BSD/GPLv2 license.  When using or
+ * redistributing this file, you may do so under either license.
+ *
+ * GPL LICENSE SUMMARY
+ *
+ * Copyright (c) 2016 BayLibre, SAS.
+ * Author: Neil Armstrong <narmstrong@baylibre.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ * The full GNU General Public License is included in this distribution
+ * in the file called COPYING.
+ *
+ * BSD LICENSE
+ *
+ * Copyright (c) 2016 BayLibre, SAS.
+ * Author: Neil Armstrong <narmstrong@baylibre.com>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ *   * Redistributions of source code must retain the above copyright
+ *     notice, this list of conditions and the following disclaimer.
+ *   * Redistributions in binary form must reproduce the above copyright
+ *     notice, this list of conditions and the following disclaimer in
+ *     the documentation and/or other materials provided with the
+ *     distribution.
+ *   * Neither the name of Intel Corporation nor the names of its
+ *     contributors may be used to endorse or promote products derived
+ *     from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+#ifndef _DT_BINDINGS_AMLOGIC_MESON8B_RESET_H
+#define _DT_BINDINGS_AMLOGIC_MESON8B_RESET_H
+
+/*	RESET0					*/
+#define RESET_HIU			0
+#define RESET_VLD			1
+#define RESET_IQIDCT			2
+#define RESET_MC			3
+/*					8	*/
+#define RESET_VIU			5
+#define RESET_AIU			6
+#define RESET_MCPU			7
+#define RESET_CCPU			8
+#define RESET_PMUX			9
+#define RESET_VENC			10
+#define RESET_ASSIST			11
+#define RESET_AFIFO2			12
+#define RESET_MDEC			13
+#define RESET_VLD_PART			14
+#define RESET_VIFIFO			15
+/*					16-31	*/
+/*	RESET1					*/
+/*					32	*/
+#define RESET_DEMUX			33
+#define RESET_USB_OTG			34
+#define RESET_DDR			35
+#define RESET_VDAC_1			36
+#define RESET_BT656			37
+#define RESET_AHB_SRAM			38
+#define RESET_AHB_BRIDGE		39
+#define RESET_PARSER			40
+#define RESET_BLKMV			41
+#define RESET_ISA			42
+#define RESET_ETHERNET			43
+#define RESET_ABUF			44
+#define RESET_AHB_DATA			45
+#define RESET_AHB_CNTL			46
+#define RESET_ROM_BOOT			47
+/*					48-63	*/
+/*	RESET2					*/
+#define RESET_VD_RMEM			64
+#define RESET_AUDIN			65
+#define RESET_DBLK			66
+#define RESET_PIC_DC			66
+#define RESET_PSC			66
+#define RESET_NAND			66
+#define RESET_GE2D			70
+#define RESET_PARSER_REG		71
+#define RESET_PARSER_FETCH		72
+#define RESET_PARSER_CTL		73
+#define RESET_PARSER_TOP		74
+#define RESET_HDMI_APB			75
+#define RESET_AUDIO_APB			76
+#define RESET_MEDIA_CPU			77
+#define RESET_MALI			78
+#define RESET_HDMI_SYSTEM_RESET		79
+/*					80-95	*/
+/*	RESET3					*/
+#define RESET_RING_OSCILLATOR		96
+#define RESET_SYS_CPU_0			97
+#define RESET_EFUSE			98
+#define RESET_SYS_CPU_BVCI		99
+#define RESET_AIFIFO			100
+#define RESET_AUDIO_PLL_MODULATOR	101
+#define RESET_AHB_BRIDGE_CNTL		102
+#define RESET_SYS_CPU_1			103
+#define RESET_AUDIO_DAC			104
+#define RESET_DEMUX_TOP			105
+#define RESET_DEMUX_DES			106
+#define RESET_DEMUX_S2P_0		107
+#define RESET_DEMUX_S2P_1		108
+#define RESET_DEMUX_RESET_0		109
+#define RESET_DEMUX_RESET_1		110
+#define RESET_DEMUX_RESET_2		111
+/*					112-127	*/
+/*	RESET4					*/
+#define RESET_PL310			128
+#define RESET_A5_APB			129
+#define RESET_A5_AXI			130
+#define RESET_A5			131
+#define RESET_DVIN			132
+#define RESET_RDMA			133
+#define RESET_VENCI			134
+#define RESET_VENCP			135
+#define RESET_VENCT			136
+#define RESET_VDAC_4			137
+#define RESET_RTC			138
+#define RESET_A5_DEBUG			139
+#define RESET_VDI6			140
+#define RESET_VENCL			141
+/*					142-159	*/
+/*	RESET5					*/
+#define RESET_DDR_PLL			160
+#define RESET_MISC_PLL			161
+#define RESET_SYS_PLL			162
+#define RESET_HPLL_PLL			163
+#define RESET_AUDIO_PLL			164
+#define RESET_VID2_PLL			165
+/*					166-191	*/
+/*	RESET6					*/
+#define RESET_PERIPHS_GENERAL		192
+#define RESET_PERIPHS_IR_REMOTE		193
+#define RESET_PERIPHS_SMART_CARD	194
+#define RESET_PERIPHS_SAR_ADC		195
+#define RESET_PERIPHS_I2C_MASTER_0	196
+#define RESET_PERIPHS_I2C_MASTER_1	197
+#define RESET_PERIPHS_I2C_SLAVE		198
+#define RESET_PERIPHS_STREAM_INTERFACE	199
+#define RESET_PERIPHS_SDIO		200
+#define RESET_PERIPHS_UART_0		201
+#define RESET_PERIPHS_UART_1		202
+#define RESET_PERIPHS_ASYNC_0		203
+#define RESET_PERIPHS_ASYNC_1		204
+#define RESET_PERIPHS_SPI_0		205
+#define RESET_PERIPHS_SPI_1		206
+#define RESET_PERIPHS_LED_PWM		207
+/*					208-223	*/
+/*	RESET7					*/
+/*					224-255	*/
+
+#endif
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 3/4] ARM64: dts: amlogic: Enable Reset Controller on GXBB-based platforms
  2016-05-25  9:49 [PATCH v2 0/4] Amlogic: Meson: Add reset controller Neil Armstrong
  2016-05-25  9:49 ` [PATCH v2 1/4] reset: Add support for the Amlogic Meson SoC Reset Controller Neil Armstrong
  2016-05-25  9:49 ` [PATCH v2 2/4] dt-bindings: reset: Add bindings for the " Neil Armstrong
@ 2016-05-25  9:49 ` Neil Armstrong
  2016-05-25  9:49 ` [PATCH v2 4/4] ARM: dts: amlogic: Enable Reset Controller on Meson8b platforms Neil Armstrong
  3 siblings, 0 replies; 11+ messages in thread
From: Neil Armstrong @ 2016-05-25  9:49 UTC (permalink / raw)
  To: linux-kernel, p.zabel, linux-arm-kernel, linux-amlogic
  Cc: Neil Armstrong, xing.xu, victor.wan, jerry.cao, devicetree

Update DTSI file to add the reset controller node.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index 832815d..45e9c55 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -43,6 +43,7 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/reset/amlogic,meson-gxbb-reset.h>
 
 / {
 	compatible = "amlogic,meson-gxbb";
@@ -129,6 +130,12 @@
 			#size-cells = <2>;
 			ranges = <0x0 0x0 0x0 0xc1100000 0x0 0x100000>;
 
+			reset: reset-controller@4404 {
+				compatible = "amlogic,meson-gxbb-reset";
+				reg = <0x0 0x04404 0x0 0x20>;
+				#reset-cells = <1>;
+			};
+
 			uart_A: serial@84c0 {
 				compatible = "amlogic,meson-uart";
 				reg = <0x0 0x084c0 0x0 0x14>;
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 4/4] ARM: dts: amlogic: Enable Reset Controller on Meson8b platforms
  2016-05-25  9:49 [PATCH v2 0/4] Amlogic: Meson: Add reset controller Neil Armstrong
                   ` (2 preceding siblings ...)
  2016-05-25  9:49 ` [PATCH v2 3/4] ARM64: dts: amlogic: Enable Reset Controller on GXBB-based platforms Neil Armstrong
@ 2016-05-25  9:49 ` Neil Armstrong
  3 siblings, 0 replies; 11+ messages in thread
From: Neil Armstrong @ 2016-05-25  9:49 UTC (permalink / raw)
  To: linux-kernel, p.zabel, linux-arm-kernel, linux-amlogic
  Cc: Neil Armstrong, xing.xu, victor.wan, jerry.cao, devicetree

Update DTSI file to add the reset controller node.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 arch/arm/boot/dts/meson8b.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi
index 2bfe401..fc4080d 100644
--- a/arch/arm/boot/dts/meson8b.dtsi
+++ b/arch/arm/boot/dts/meson8b.dtsi
@@ -46,6 +46,7 @@
 
 #include <dt-bindings/clock/meson8b-clkc.h>
 #include <dt-bindings/gpio/meson8b-gpio.h>
+#include <dt-bindings/reset/amlogic,meson8b-reset.h>
 #include "skeleton.dtsi"
 
 / {
@@ -105,6 +106,12 @@
 			#interrupt-cells = <3>;
 		};
 
+		reset: reset-controller@c1104404 {
+			compatible = "amlogic,meson8b-reset";
+			reg = <0xc1104404 0x20>;
+			#reset-cells = <1>;
+		};
+
 		wdt: watchdog@c1109900 {
 			compatible = "amlogic,meson8b-wdt";
 			reg = <0xc1109900 0x8>;
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/4] reset: Add support for the Amlogic Meson SoC Reset Controller
  2016-05-25  9:49 ` [PATCH v2 1/4] reset: Add support for the Amlogic Meson SoC Reset Controller Neil Armstrong
@ 2016-05-25 10:14   ` Philipp Zabel
       [not found]   ` <m2iny1h74c.fsf@baylibre.com>
  1 sibling, 0 replies; 11+ messages in thread
From: Philipp Zabel @ 2016-05-25 10:14 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: linux-kernel, linux-arm-kernel, linux-amlogic, xing.xu,
	victor.wan, jerry.cao

Hi Neil,

looks fine, just two comments below.

Am Mittwoch, den 25.05.2016, 11:49 +0200 schrieb Neil Armstrong:
> This patch adds the platform driver for the Amlogic Meson SoC Reset
> Controller.
> 
> The Meson8b and GXBB SoCs are supported.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/reset/Makefile      |   1 +
>  drivers/reset/reset-meson.c | 137 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 138 insertions(+)
>  create mode 100644 drivers/reset/reset-meson.c
> 
[...]
> diff --git a/drivers/reset/reset-meson.c b/drivers/reset/reset-meson.c
> new file mode 100644
> index 0000000..98087ae
> --- /dev/null
> +++ b/drivers/reset/reset-meson.c
[...]
> +static int meson_reset_reset(struct reset_controller_dev *rcdev,
> +			      unsigned long id)
> +{
> +	struct meson_reset *data =
> +		container_of(rcdev, struct meson_reset, rcdev);
> +	unsigned int bank = id / BITS_PER_REG;
> +	unsigned int offset = id % BITS_PER_REG;
> +	void *reg_addr = data->reg_base + (bank << 2);

We lost the __iomem here.

> +
> +	if (bank >= REG_COUNT)
> +		return -EINVAL;
> +
> +	writel(BIT(offset), reg_addr);
> +
> +	return 0;
> +}
> +
> +static const struct reset_control_ops meson_reset_ops = {
> +	.reset		= meson_reset_reset,
> +};
> +
> +static const struct of_device_id meson_reset_dt_ids[] = {
> +	 { .compatible = "amlogic,meson8b-reset", },
> +	 { .compatible = "amlogic,meson-gxbb-reset", },
> +	 { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, meson_reset_dt_ids);
[...]
> +MODULE_ALIAS("platform:meson_reset");

And I think you can drop the MODULE_ALIAS since of: modaliases will be
generated from the MODULE_DEVICE_TABLE above. This driver is only ever
going to be probed from device tree, isn't it?

> +MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
> +MODULE_DESCRIPTION("Amlogic Meson Reset Controller driver");
> +MODULE_LICENSE("Dual BSD/GPL");

regards
Philipp

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/4] dt-bindings: reset: Add bindings for the Meson SoC Reset Controller
  2016-05-25  9:49 ` [PATCH v2 2/4] dt-bindings: reset: Add bindings for the " Neil Armstrong
@ 2016-05-25 19:10   ` Rob Herring
  2016-05-26  7:30     ` Neil Armstrong
  0 siblings, 1 reply; 11+ messages in thread
From: Rob Herring @ 2016-05-25 19:10 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: linux-kernel, p.zabel, linux-arm-kernel, linux-amlogic,
	jerry.cao, devicetree, xing.xu, victor.wan

On Wed, May 25, 2016 at 11:49:16AM +0200, Neil Armstrong wrote:
> Add DT bindings for the Meson SoC Reset Controller documentation and the
> associated include file.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  .../bindings/reset/amlogic,meson-reset.txt         |  18 ++
>  .../dt-bindings/reset/amlogic,meson-gxbb-reset.h   | 210 +++++++++++++++++++++
>  include/dt-bindings/reset/amlogic,meson8b-reset.h  | 175 +++++++++++++++++
>  3 files changed, 403 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/amlogic,meson-reset.txt
>  create mode 100644 include/dt-bindings/reset/amlogic,meson-gxbb-reset.h
>  create mode 100644 include/dt-bindings/reset/amlogic,meson8b-reset.h

Please add acks when posting new versions.

Rob

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/4] dt-bindings: reset: Add bindings for the Meson SoC Reset Controller
  2016-05-25 19:10   ` Rob Herring
@ 2016-05-26  7:30     ` Neil Armstrong
  0 siblings, 0 replies; 11+ messages in thread
From: Neil Armstrong @ 2016-05-26  7:30 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-kernel, p.zabel, linux-arm-kernel, linux-amlogic,
	jerry.cao, devicetree, xing.xu, victor.wan

On 05/25/2016 09:10 PM, Rob Herring wrote:
> On Wed, May 25, 2016 at 11:49:16AM +0200, Neil Armstrong wrote:
>> Add DT bindings for the Meson SoC Reset Controller documentation and the
>> associated include file.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>>  .../bindings/reset/amlogic,meson-reset.txt         |  18 ++
>>  .../dt-bindings/reset/amlogic,meson-gxbb-reset.h   | 210 +++++++++++++++++++++
>>  include/dt-bindings/reset/amlogic,meson8b-reset.h  | 175 +++++++++++++++++
>>  3 files changed, 403 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/reset/amlogic,meson-reset.txt
>>  create mode 100644 include/dt-bindings/reset/amlogic,meson-gxbb-reset.h
>>  create mode 100644 include/dt-bindings/reset/amlogic,meson8b-reset.h
> 
> Please add acks when posting new versions.
> 
> Rob
> 

Hi Rob,

Actually I made a lot of changes here since I added support for the meson8b.
I asked myself if I could carry your ack actually...

If the ack is still valid, I'll won't forget it for v4.

Thanks,
Neil

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/4] reset: Add support for the Amlogic Meson SoC Reset Controller
       [not found]   ` <m2iny1h74c.fsf@baylibre.com>
@ 2016-05-26 11:02     ` Philipp Zabel
  2016-05-26 18:42       ` Kevin Hilman
  0 siblings, 1 reply; 11+ messages in thread
From: Philipp Zabel @ 2016-05-26 11:02 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Neil Armstrong, linux-kernel, linux-arm-kernel, linux-amlogic,
	jerry.cao, xing.xu, victor.wan

Hi Kevin,

Am Mittwoch, den 25.05.2016, 19:42 -0700 schrieb Kevin Hilman:
> Neil Armstrong <narmstrong@baylibre.com> writes:
> 
> > This patch adds the platform driver for the Amlogic Meson SoC Reset
> > Controller.
> >
> > The Meson8b and GXBB SoCs are supported.
> >
> > Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> 
> Maybe a question for Philipp, but when testog this version with the
> stmmac ethernet driver on the Amlogic boards, I noticed that ->reset was
> never getting called.
> 
> Turns out, the stmmac driver only uses reset_control_assert() and
> reset_control_deassert(), both of which return -ENOTSUPP since this
> driver doesn't provide ->assert or ->deassert, so the driver's ->reset()
> never gets called.
> 
> I haven't looked into the reset framework in detail, but if there's only
> a ->reset hook, I kind of expected that reset_control_deassert() would
> use that instead of returning -ENOTSUPP.
> 
> If not, what's the proper way of handling hardware that only supports a
> write-only reset pulse?  Should users of reset_control_* be adapted to
> check if ->deassert returns -ENOTSUPP and call ->reset?

I initially came up with this for i.MX6, which has a reset controller
that just like the Meson ones doesn't support manual assertion and
deassertion of the reset line.
My assumption then was that reset() is the default for all devices that
just need their internal state to be reset at some point. I didn't
expect the clock-like usage of manual deassert()/assert() for power
management to become so common.
So assert() and deassert() return -ENOTSUPP if the reset controller
doesn't support manually asserting or deasserting the reset line.

Of course you can argue that after a reset() pulse the reset line is
deasserted, and if you instead interpret the API in terms of expected
outcome, that would be similar to the state after call to deassert()
(iff the line was asserted before).
That would blur the line between reset() and deassert() somewhat, and in
my opinion having a call to deassert() first doing the exact opposite
isn't good nomenclature.
Personally, I'd prefer if the driver could switch to only using
	reset_control_reset(rstc);
or, if the reset line needs to stay asserted during powerdown where the
reset controller supports it, but doesn't matter whert not, use:
	ret = reset_control_deassert(rstc);
	if (ret == -ENOTSUPP)
		ret = reset_control_reset(rstc);

We could add a helper reset_control_deassert_or_reset() for that.

regards
Philipp

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/4] reset: Add support for the Amlogic Meson SoC Reset Controller
  2016-05-26 11:02     ` Philipp Zabel
@ 2016-05-26 18:42       ` Kevin Hilman
  2016-05-30 16:14         ` Philipp Zabel
  0 siblings, 1 reply; 11+ messages in thread
From: Kevin Hilman @ 2016-05-26 18:42 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: Neil Armstrong, linux-kernel, linux-arm-kernel, linux-amlogic,
	jerry.cao, xing.xu, victor.wan

Philipp Zabel <p.zabel@pengutronix.de> writes:

> Am Mittwoch, den 25.05.2016, 19:42 -0700 schrieb Kevin Hilman:
>> Neil Armstrong <narmstrong@baylibre.com> writes:
>> 
>> > This patch adds the platform driver for the Amlogic Meson SoC Reset
>> > Controller.
>> >
>> > The Meson8b and GXBB SoCs are supported.
>> >
>> > Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> 
>> Maybe a question for Philipp, but when testog this version with the
>> stmmac ethernet driver on the Amlogic boards, I noticed that ->reset was
>> never getting called.
>> 
>> Turns out, the stmmac driver only uses reset_control_assert() and
>> reset_control_deassert(), both of which return -ENOTSUPP since this
>> driver doesn't provide ->assert or ->deassert, so the driver's ->reset()
>> never gets called.
>> 
>> I haven't looked into the reset framework in detail, but if there's only
>> a ->reset hook, I kind of expected that reset_control_deassert() would
>> use that instead of returning -ENOTSUPP.
>> 
>> If not, what's the proper way of handling hardware that only supports a
>> write-only reset pulse?  Should users of reset_control_* be adapted to
>> check if ->deassert returns -ENOTSUPP and call ->reset?
>
> I initially came up with this for i.MX6, which has a reset controller
> that just like the Meson ones doesn't support manual assertion and
> deassertion of the reset line.
> My assumption then was that reset() is the default for all devices that
> just need their internal state to be reset at some point. I didn't
> expect the clock-like usage of manual deassert()/assert() for power
> management to become so common.
> So assert() and deassert() return -ENOTSUPP if the reset controller
> doesn't support manually asserting or deasserting the reset line.
>
> Of course you can argue that after a reset() pulse the reset line is
> deasserted, and if you instead interpret the API in terms of expected
> outcome, that would be similar to the state after call to deassert()
> (iff the line was asserted before).
> That would blur the line between reset() and deassert() somewhat, and in
> my opinion having a call to deassert() first doing the exact opposite
> isn't good nomenclature.
> Personally, I'd prefer if the driver could switch to only using
> 	reset_control_reset(rstc);

Then what would happen if that same driver is used on platforms that
have ->assert and ->deassert but no -reset?

The bind I'm in is that I'm using an exsting network driver
(stmicro/stmmac) which is used on a bunch of platforms and I dont' know
what all those reset controllers are actually doing, so going and
changing the driver seems problematic.

> or, if the reset line needs to stay asserted during powerdown where the
> reset controller supports it, but doesn't matter whert not, use:
> 	ret = reset_control_deassert(rstc);
> 	if (ret == -ENOTSUPP)
> 		ret = reset_control_reset(rstc);
>
> We could add a helper reset_control_deassert_or_reset() for that.

IMO, that would be a useful helper function, but I still think that
should be the default behavior of _deassert, otherwise it will still
require changing all the drivers.

Kevin

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/4] reset: Add support for the Amlogic Meson SoC Reset Controller
  2016-05-26 18:42       ` Kevin Hilman
@ 2016-05-30 16:14         ` Philipp Zabel
  0 siblings, 0 replies; 11+ messages in thread
From: Philipp Zabel @ 2016-05-30 16:14 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Neil Armstrong, linux-kernel, linux-arm-kernel, linux-amlogic,
	jerry.cao, xing.xu, victor.wan

Hi Kevin,

Am Donnerstag, den 26.05.2016, 11:42 -0700 schrieb Kevin Hilman:
[...]
> > Personally, I'd prefer if the driver could switch to only using
> > 	reset_control_reset(rstc);
> 
> Then what would happen if that same driver is used on platforms that
> have ->assert and ->deassert but no -reset?

If the implementer of the reset controller driver doesn't have enough
information about all connected devices to implement ->reset, then
_reset() has to return -ENOTSUPP, so this is not an option in the
general case.

> The bind I'm in is that I'm using an exsting network driver
> (stmicro/stmmac) which is used on a bunch of platforms and I dont' know
> what all those reset controllers are actually doing, so going and
> changing the driver seems problematic.

I see. It's all the more important that the API clearly states the
intended effects.

> > or, if the reset line needs to stay asserted during powerdown where the
> > reset controller supports it, but doesn't matter whert not, use:
> > 	ret = reset_control_deassert(rstc);
> > 	if (ret == -ENOTSUPP)
> > 		ret = reset_control_reset(rstc);
> >
> > We could add a helper reset_control_deassert_or_reset() for that.
> 
> IMO, that would be a useful helper function, but I still think that
> should be the default behavior of _deassert, otherwise it will still
> require changing all the drivers.

"All the drivers" makes me reconsider. If all drivers eventually,
reasonably could be changed to use reset_control_deassert_or_reset()
instead of reset_control_deassert(), there would be no reason not to
make _deassert() behave like you expect (well, except for the naming
issue).

One thing that has to be considered is that in the case of synchronous
resets there may be a significant difference in the behaviour of ->reset
and ->deassert: With ->deassert you could prime the reset line before
enabling clocks and letting the reset signal propagate. This is not
possible with ->reset, which could hang or have no effect at all with
the relevant clocks disabled. So it should be clearly documented that
anybody intending to make use of this must call reset_control_assert()
first and back off if that returns -ENOTSUPP.

regards
Philipp

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2016-05-30 16:14 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-25  9:49 [PATCH v2 0/4] Amlogic: Meson: Add reset controller Neil Armstrong
2016-05-25  9:49 ` [PATCH v2 1/4] reset: Add support for the Amlogic Meson SoC Reset Controller Neil Armstrong
2016-05-25 10:14   ` Philipp Zabel
     [not found]   ` <m2iny1h74c.fsf@baylibre.com>
2016-05-26 11:02     ` Philipp Zabel
2016-05-26 18:42       ` Kevin Hilman
2016-05-30 16:14         ` Philipp Zabel
2016-05-25  9:49 ` [PATCH v2 2/4] dt-bindings: reset: Add bindings for the " Neil Armstrong
2016-05-25 19:10   ` Rob Herring
2016-05-26  7:30     ` Neil Armstrong
2016-05-25  9:49 ` [PATCH v2 3/4] ARM64: dts: amlogic: Enable Reset Controller on GXBB-based platforms Neil Armstrong
2016-05-25  9:49 ` [PATCH v2 4/4] ARM: dts: amlogic: Enable Reset Controller on Meson8b platforms Neil Armstrong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).