[PATCH 0/3] Amlogic: GXBB: Add reset controller

[PATCH 0/3] Amlogic: GXBB: Add reset controller

[Neil Armstrong]

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

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

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

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

 .../bindings/reset/amlogic,meson-gxbb-reset.txt    |  18 ++
 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi        |   6 +
 drivers/reset/Kconfig                              |   6 +
 drivers/reset/Makefile                             |   1 +
 drivers/reset/reset-meson-gxbb.c                   | 129 +++++++++++++
 .../dt-bindings/reset/amlogic,meson-gxbb-reset.h   | 199 +++++++++++++++++++++
 6 files changed, 359 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/amlogic,meson-gxbb-reset.txt
 create mode 100644 drivers/reset/reset-meson-gxbb.c
 create mode 100644 include/dt-bindings/reset/amlogic,meson-gxbb-reset.h

-- 
2.7.0

[PATCH 1/3] reset: Add support for the Amlogic Meson GXBB Reset Controller

[Neil Armstrong]

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

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/reset/Kconfig            |   6 ++
 drivers/reset/Makefile           |   1 +
 drivers/reset/reset-meson-gxbb.c | 129 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 136 insertions(+)
 create mode 100644 drivers/reset/reset-meson-gxbb.c

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index df37212..4ac5c4d 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -12,5 +12,11 @@ menuconfig RESET_CONTROLLER
 
 	  If unsure, say no.
 
+config MESON_GXBB_RESET
+	tristate "Amlogic Meson GXBB Reset Driver"
+	depends on (ARCH_MESON && RESET_CONTROLLER)
+	help
+	  Build the Amlogic Meson GxBB reset driver.
+
 source "drivers/reset/sti/Kconfig"
 source "drivers/reset/hisilicon/Kconfig"
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index a1fc8ed..5ff83a1 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_MESON_GXBB_RESET) += reset-meson-gxbb.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-gxbb.c b/drivers/reset/reset-meson-gxbb.c
new file mode 100644
index 0000000..e96691b
--- /dev/null
+++ b/drivers/reset/reset-meson-gxbb.c
@@ -0,0 +1,129 @@
+/*
+ * Copyright (c) 2016 BayLibre, SAS.
+ * Author: Neil Armstrong <narmstrong@baylibre.com>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This library 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.
+ *
+ *     This library 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.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use,
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+#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_gxbb_reset {
+	void __iomem *reg_base;
+	struct reset_controller_dev rcdev;
+};
+
+static int meson_gxbb_reset_reset(struct reset_controller_dev *rcdev,
+			      unsigned long id)
+{
+	struct meson_gxbb_reset *data =
+		container_of(rcdev, struct meson_gxbb_reset, rcdev);
+	unsigned int bank = id / BITS_PER_REG;
+	unsigned int offset = id % BITS_PER_REG;
+	void __iomem *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_gxbb_reset_ops = {
+	.reset		= meson_gxbb_reset_reset,
+};
+
+static const struct of_device_id meson_gxbb_reset_dt_ids[] = {
+	 { .compatible = "amlogic,meson-gxbb-reset", },
+	 { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, meson_gxbb_reset_dt_ids);
+
+static int meson_gxbb_reset_probe(struct platform_device *pdev)
+{
+	struct meson_gxbb_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_gxbb_reset_ops;
+	data->rcdev.of_node = pdev->dev.of_node;
+
+	return reset_controller_register(&data->rcdev);
+}
+
+static int meson_gxbb_reset_remove(struct platform_device *pdev)
+{
+	struct meson_gxbb_reset *data = platform_get_drvdata(pdev);
+
+	reset_controller_unregister(&data->rcdev);
+
+	return 0;
+}
+
+static struct platform_driver meson_gxbb_reset_driver = {
+	.probe	= meson_gxbb_reset_probe,
+	.remove	= meson_gxbb_reset_remove,
+	.driver = {
+		.name		= "meson_gxbb_reset",
+		.of_match_table	= meson_gxbb_reset_dt_ids,
+	},
+};
+
+module_platform_driver(meson_gxbb_reset_driver);
-- 
2.7.0

[PATCH 2/3] dt-bindings: reset: Add bindings for the Meson GXBB Reset Controller

[Neil Armstrong]

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

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

diff --git a/Documentation/devicetree/bindings/reset/amlogic,meson-gxbb-reset.txt b/Documentation/devicetree/bindings/reset/amlogic,meson-gxbb-reset.txt
new file mode 100644
index 0000000..0758259
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/amlogic,meson-gxbb-reset.txt
@@ -0,0 +1,18 @@
+Amlogic Meson GXBB SoC Reset Controller
+=======================================
+
+Please also refer to reset.txt in this directory for common reset
+controller binding usage.
+
+Required properties:
+- compatible: Should be "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..ff4e804
--- /dev/null
+++ b/include/dt-bindings/reset/amlogic,meson-gxbb-reset.h
@@ -0,0 +1,199 @@
+/*
+ * Copyright (c) 2016 BayLibre, SAS.
+ * Author: Neil Armstrong <narmstrong@baylibre.com>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This library 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.
+ *
+ *     This library 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.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use,
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#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
-- 
2.7.0

[PATCH 3/3] ARM64: dts: amlogic: Enable Reset Controller on GXBB-based platforms

[Neil Armstrong]

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 | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index 832815d..2463e04 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -129,6 +129,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

Re: [PATCH 2/3] dt-bindings: reset: Add bindings for the Meson GXBB Reset Controller

[Carlo Caione]

On 20/05/16 10:27, Neil Armstrong wrote:
> Add DT bindings for the Meson GXBB SoC Reset Controller documentation and the
> associated include file.

[...]

> +#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

Indentation looks really messy. Can you just shift the numbers to the
right so that they are on the same column?

-- 
Carlo Caione

Re: [PATCH 3/3] ARM64: dts: amlogic: Enable Reset Controller on GXBB-based platforms

[Carlo Caione]

On 20/05/16 10:27, Neil Armstrong wrote:
> 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 | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> index 832815d..2463e04 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> @@ -129,6 +129,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>;

Missing #include <dt-bindings/reset/amlogic,meson-gxbb-reset.h> ?

Thanks,

-- 
Carlo Caione

Re: [PATCH 2/3] dt-bindings: reset: Add bindings for the Meson GXBB Reset Controller

[Neil Armstrong]

On 05/20/2016 10:46 AM, Carlo Caione wrote:
> On 20/05/16 10:27, Neil Armstrong wrote:
>> Add DT bindings for the Meson GXBB SoC Reset Controller documentation and the
>> associated include file.
> 
> [...]
> 
>> +#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
> 
> Indentation looks really messy. Can you just shift the numbers to the
> right so that they are on the same column?
> 
Hi Carlo,

The patch format makes it very messy, not as in the original format :
https://github.com/superna9999/linux/blob/6163f8742454bb7ff962956b4e286d110ec0fb79/include/dt-bindings/reset/amlogic%2Cmeson-gxbb-reset.h

Neil

Re: [PATCH 3/3] ARM64: dts: amlogic: Enable Reset Controller on GXBB-based platforms

[Neil Armstrong]

On 05/20/2016 10:47 AM, Carlo Caione wrote:
> On 20/05/16 10:27, Neil Armstrong wrote:
>> 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 | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>> index 832815d..2463e04 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>> @@ -129,6 +129,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>;
> 
> Missing #include <dt-bindings/reset/amlogic,meson-gxbb-reset.h> ?
> 
> Thanks,
> 

Shouldn't we wait until the reset bindings are actually used in the dtsi ?
I'm quite sure kevin will post it with the Ethernet nodes.

Neil

Re: [PATCH 1/3] reset: Add support for the Amlogic Meson GXBB Reset Controller

[Carlo Caione]

On 20/05/16 10:27, Neil Armstrong wrote:
> This patch adds the platform driver for the Amlogic Meson GXBB Reset
> Controller.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/reset/Kconfig            |   6 ++
>  drivers/reset/Makefile           |   1 +
>  drivers/reset/reset-meson-gxbb.c | 129 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 136 insertions(+)
>  create mode 100644 drivers/reset/reset-meson-gxbb.c

Do we really need to be that specific (-gxbb)? This driver looks generic
and simple enough to be used for several Amlogic families. You are
already differentiating between them with the include file defining the
reset indexes for the SoC.

> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index df37212..4ac5c4d 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -12,5 +12,11 @@ menuconfig RESET_CONTROLLER
>  
>  	  If unsure, say no.
>  
> +config MESON_GXBB_RESET
> +	tristate "Amlogic Meson GXBB Reset Driver"
> +	depends on (ARCH_MESON && RESET_CONTROLLER)
> +	help
> +	  Build the Amlogic Meson GxBB reset driver.
> +
>  source "drivers/reset/sti/Kconfig"
>  source "drivers/reset/hisilicon/Kconfig"
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index a1fc8ed..5ff83a1 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_MESON_GXBB_RESET) += reset-meson-gxbb.o

obj-$(CONFIG_ARCH_MESON) ?

[...]
> +static const struct reset_control_ops meson_gxbb_reset_ops = {
> +	.reset		= meson_gxbb_reset_reset,
> +};

nit: unfortunate name :)

Any reason to not have also .assert / .deassert?

[...]
> +static struct platform_driver meson_gxbb_reset_driver = {
> +	.probe	= meson_gxbb_reset_probe,
> +	.remove	= meson_gxbb_reset_remove,
> +	.driver = {
> +		.name		= "meson_gxbb_reset",
> +		.of_match_table	= meson_gxbb_reset_dt_ids,
> +	},
> +};
> +
> +module_platform_driver(meson_gxbb_reset_driver);

No MODULE_AUTHOR, MODULE_LICENSE, etc... ?

-- 
Carlo Caione

Re: [PATCH 2/3] dt-bindings: reset: Add bindings for the Meson GXBB Reset Controller

[Carlo Caione]

On 20/05/16 10:51, Neil Armstrong wrote:
> On 05/20/2016 10:46 AM, Carlo Caione wrote:
> > On 20/05/16 10:27, Neil Armstrong wrote:
> >> Add DT bindings for the Meson GXBB SoC Reset Controller documentation and the
> >> associated include file.
> > 
> > [...]
> > 
> >> +#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
> > 
> > Indentation looks really messy. Can you just shift the numbers to the
> > right so that they are on the same column?
> > 
> Hi Carlo,
> 
> The patch format makes it very messy, not as in the original format :
> https://github.com/superna9999/linux/blob/6163f8742454bb7ff962956b4e286d110ec0fb79/include/dt-bindings/reset/amlogic%2Cmeson-gxbb-reset.h

https://raw.githubusercontent.com/superna9999/linux/6163f8742454bb7ff962956b4e286d110ec0fb79/include/dt-bindings/reset/amlogic%2Cmeson-gxbb-reset.h

yes, but 192 -> 200 are not nicely indented to me :)

-- 
Carlo Caione

Re: [PATCH 1/3] reset: Add support for the Amlogic Meson GXBB Reset Controller

[Neil Armstrong]

On 05/20/2016 11:04 AM, Carlo Caione wrote:
> On 20/05/16 10:27, Neil Armstrong wrote:
>> This patch adds the platform driver for the Amlogic Meson GXBB Reset
>> Controller.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>>  drivers/reset/Kconfig            |   6 ++
>>  drivers/reset/Makefile           |   1 +
>>  drivers/reset/reset-meson-gxbb.c | 129 +++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 136 insertions(+)
>>  create mode 100644 drivers/reset/reset-meson-gxbb.c
> 
> Do we really need to be that specific (-gxbb)? This driver looks generic
> and simple enough to be used for several Amlogic families. You are
> already differentiating between them with the include file defining the
> reset indexes for the SoC.

This is a good question, do the S805 have similar registers ? Same count and width ?
I no, it should need a rework to add a data structure per-SoC.

> 
>> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
>> index df37212..4ac5c4d 100644
>> --- a/drivers/reset/Kconfig
>> +++ b/drivers/reset/Kconfig
>> @@ -12,5 +12,11 @@ menuconfig RESET_CONTROLLER
>>  
>>  	  If unsure, say no.
>>  
>> +config MESON_GXBB_RESET
>> +	tristate "Amlogic Meson GXBB Reset Driver"
>> +	depends on (ARCH_MESON && RESET_CONTROLLER)
>> +	help
>> +	  Build the Amlogic Meson GxBB reset driver.
>> +
>>  source "drivers/reset/sti/Kconfig"
>>  source "drivers/reset/hisilicon/Kconfig"
>> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
>> index a1fc8ed..5ff83a1 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_MESON_GXBB_RESET) += reset-meson-gxbb.o
> 
> obj-$(CONFIG_ARCH_MESON) ?
> 
> [...]
>> +static const struct reset_control_ops meson_gxbb_reset_ops = {
>> +	.reset		= meson_gxbb_reset_reset,
>> +};
> 
> nit: unfortunate name :)
> 
> Any reason to not have also .assert / .deassert?

The registers seems write-only and does no retain the bit written, assert and deassert makes no sense in this case.

> 
> [...]
>> +static struct platform_driver meson_gxbb_reset_driver = {
>> +	.probe	= meson_gxbb_reset_probe,
>> +	.remove	= meson_gxbb_reset_remove,
>> +	.driver = {
>> +		.name		= "meson_gxbb_reset",
>> +		.of_match_table	= meson_gxbb_reset_dt_ids,
>> +	},
>> +};
>> +
>> +module_platform_driver(meson_gxbb_reset_driver);
> 
> No MODULE_AUTHOR, MODULE_LICENSE, etc... ?

What is the MODULE_LICENCE format for dual licensing ?

Thanks,
Neil

Re: [PATCH 3/3] ARM64: dts: amlogic: Enable Reset Controller on GXBB-based platforms

[Carlo Caione]

On 20/05/16 10:53, Neil Armstrong wrote:
> On 05/20/2016 10:47 AM, Carlo Caione wrote:
> > On 20/05/16 10:27, Neil Armstrong wrote:
> >> 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 | 6 ++++++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> >> index 832815d..2463e04 100644
> >> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> >> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
> >> @@ -129,6 +129,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>;
> > 
> > Missing #include <dt-bindings/reset/amlogic,meson-gxbb-reset.h> ?
> > 
> > Thanks,
> > 
> 
> Shouldn't we wait until the reset bindings are actually used in the dtsi ?
> I'm quite sure kevin will post it with the Ethernet nodes.

Why? The header file is related to the reset controller. Better add it
now with this patchset.

Cheers,

-- 
Carlo Caione

Re: [PATCH 1/3] reset: Add support for the Amlogic Meson GXBB Reset Controller

[Philipp Zabel]

Hi Neil,

Am Freitag, den 20.05.2016, 10:27 +0200 schrieb Neil Armstrong:
> This patch adds the platform driver for the Amlogic Meson GXBB Reset
> Controller.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/reset/Kconfig            |   6 ++
>  drivers/reset/Makefile           |   1 +
>  drivers/reset/reset-meson-gxbb.c | 129 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 136 insertions(+)
>  create mode 100644 drivers/reset/reset-meson-gxbb.c
> 
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index df37212..4ac5c4d 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -12,5 +12,11 @@ menuconfig RESET_CONTROLLER
>  
>  	  If unsure, say no.
>  
> +config MESON_GXBB_RESET
> +	tristate "Amlogic Meson GXBB Reset Driver"
> +	depends on (ARCH_MESON && RESET_CONTROLLER)

With the "reset: fix Kconfig menu to include reset drivers in sub-menu"
patch [1] applied this is wrapped in "if RESET_CONTROLLER ... endif", so
no need to depend on RESET_CONTROLLER.
Is there a reason to have this configurable at all, though?

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

> +	help
> +	  Build the Amlogic Meson GxBB reset driver.
> +
>  source "drivers/reset/sti/Kconfig"
>  source "drivers/reset/hisilicon/Kconfig"
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index a1fc8ed..5ff83a1 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_MESON_GXBB_RESET) += reset-meson-gxbb.o

Or drop CONFIG_MESON_GXBB_RESET and use CONFIG_ARCH_MESON directly.

[...]
> +static int meson_gxbb_reset_probe(struct platform_device *pdev)
> +{
> +	struct meson_gxbb_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_gxbb_reset_ops;
> +	data->rcdev.of_node = pdev->dev.of_node;
> +
> +	return reset_controller_register(&data->rcdev);

With the "reset: add devm_reset_controller_register API" patch [2]
applied you can use devm_reset_controller_register() here and remove the
meson_gxbb_reset_remove() function below.

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

> +}
> +
> +static int meson_gxbb_reset_remove(struct platform_device *pdev)
> +{
> +	struct meson_gxbb_reset *data = platform_get_drvdata(pdev);
> +
> +	reset_controller_unregister(&data->rcdev);
> +
> +	return 0;
> +}

Could be removed, then.

regards
Philipp

Re: [PATCH 1/3] reset: Add support for the Amlogic Meson GXBB Reset Controller

[Philipp Zabel]

Am Freitag, den 20.05.2016, 11:10 +0200 schrieb Neil Armstrong:
[...]
> >> +module_platform_driver(meson_gxbb_reset_driver);
> > 
> > No MODULE_AUTHOR, MODULE_LICENSE, etc... ?
> 
> What is the MODULE_LICENCE format for dual licensing ?

"Dual BSD/GPL"

regards
Philipp

Re: [PATCH 1/3] reset: Add support for the Amlogic Meson GXBB Reset Controller

[Carlo Caione]

On 20/05/16 11:10, Neil Armstrong wrote:
> On 05/20/2016 11:04 AM, Carlo Caione wrote:
> > On 20/05/16 10:27, Neil Armstrong wrote:
> >> This patch adds the platform driver for the Amlogic Meson GXBB Reset
> >> Controller.
> >>
> >> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> >> ---
> >>  drivers/reset/Kconfig            |   6 ++
> >>  drivers/reset/Makefile           |   1 +
> >>  drivers/reset/reset-meson-gxbb.c | 129 +++++++++++++++++++++++++++++++++++++++
> >>  3 files changed, 136 insertions(+)
> >>  create mode 100644 drivers/reset/reset-meson-gxbb.c
> > 
> > Do we really need to be that specific (-gxbb)? This driver looks generic
> > and simple enough to be used for several Amlogic families. You are
> > already differentiating between them with the include file defining the
> > reset indexes for the SoC.
> 
> This is a good question, do the S805 have similar registers ? Same count and width ?
> I no, it should need a rework to add a data structure per-SoC.

According to the datasheet on S805 we have 7 registers with 16 reset
bits per register.

[...]
> > 
> > [...]
> >> +static struct platform_driver meson_gxbb_reset_driver = {
> >> +	.probe	= meson_gxbb_reset_probe,
> >> +	.remove	= meson_gxbb_reset_remove,
> >> +	.driver = {
> >> +		.name		= "meson_gxbb_reset",
> >> +		.of_match_table	= meson_gxbb_reset_dt_ids,
> >> +	},
> >> +};
> >> +
> >> +module_platform_driver(meson_gxbb_reset_driver);
> > 
> > No MODULE_AUTHOR, MODULE_LICENSE, etc... ?
> 
> What is the MODULE_LICENCE format for dual licensing ?

Dual BSD/GPL ?

Cheers,

-- 
Carlo Caione

Re: [PATCH 1/3] reset: Add support for the Amlogic Meson GXBB Reset Controller

[Neil Armstrong]

Hi Philipp,

On 05/20/2016 11:27 AM, Philipp Zabel wrote:
> Hi Neil,
> 
> Am Freitag, den 20.05.2016, 10:27 +0200 schrieb Neil Armstrong:
>> +config MESON_GXBB_RESET
>> +	tristate "Amlogic Meson GXBB Reset Driver"
>> +	depends on (ARCH_MESON && RESET_CONTROLLER)
> 
> With the "reset: fix Kconfig menu to include reset drivers in sub-menu"
> patch [1] applied this is wrapped in "if RESET_CONTROLLER ... endif", so
> no need to depend on RESET_CONTROLLER.
> Is there a reason to have this configurable at all, though?
> 
> [1] https://patchwork.kernel.org/patch/9000591/
> 
> Or drop CONFIG_MESON_GXBB_RESET and use CONFIG_ARCH_MESON directly.

No strong reason, I will switch to CONFIG_ARCH_MESON.

> [...]
> 
> With the "reset: add devm_reset_controller_register API" patch [2]
> applied you can use devm_reset_controller_register() here and remove the
> meson_gxbb_reset_remove() function below.
> 
> [2] https://patchwork.kernel.org/patch/8988471/
> 
>> +}
>> +
>> +static int meson_gxbb_reset_remove(struct platform_device *pdev)
>> +{
>> +	struct meson_gxbb_reset *data = platform_get_drvdata(pdev);
>> +
>> +	reset_controller_unregister(&data->rcdev);
>> +
>> +	return 0;
>> +}
> 
> Could be removed, then.
> 
> regards
> Philipp

It will be for sure !

Neil

Re: [PATCH 2/3] dt-bindings: reset: Add bindings for the Meson GXBB Reset Controller

[Neil Armstrong]

On 05/20/2016 11:08 AM, Carlo Caione wrote:
> On 20/05/16 10:51, Neil Armstrong wrote:
>> On 05/20/2016 10:46 AM, Carlo Caione wrote:
>>> On 20/05/16 10:27, Neil Armstrong wrote:
>>>> Add DT bindings for the Meson GXBB SoC Reset Controller documentation and the
>>>> associated include file.
>>>
>>> [...]
>>>
>>>> +#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
>>>
>>> Indentation looks really messy. Can you just shift the numbers to the
>>> right so that they are on the same column?
>>>
>> Hi Carlo,
>>
>> The patch format makes it very messy, not as in the original format :
>> https://github.com/superna9999/linux/blob/6163f8742454bb7ff962956b4e286d110ec0fb79/include/dt-bindings/reset/amlogic%2Cmeson-gxbb-reset.h
> 
> https://raw.githubusercontent.com/superna9999/linux/6163f8742454bb7ff962956b4e286d110ec0fb79/include/dt-bindings/reset/amlogic%2Cmeson-gxbb-reset.h
> 
> yes, but 192 -> 200 are not nicely indented to me :)
> 

Aw, you got me !

I will add a shift and add a separate meson8b version of this file.

Neil

Re: [PATCH 3/3] ARM64: dts: amlogic: Enable Reset Controller on GXBB-based platforms

[Neil Armstrong]

On 05/20/2016 11:10 AM, Carlo Caione wrote:
> On 20/05/16 10:53, Neil Armstrong wrote:
>> On 05/20/2016 10:47 AM, Carlo Caione wrote:
>>> On 20/05/16 10:27, Neil Armstrong wrote:
>>>> 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 | 6 ++++++
>>>>  1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>>>> index 832815d..2463e04 100644
>>>> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>>>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>>>> @@ -129,6 +129,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>;
>>>
>>> Missing #include <dt-bindings/reset/amlogic,meson-gxbb-reset.h> ?
>>>
>>> Thanks,
>>>
>>
>> Shouldn't we wait until the reset bindings are actually used in the dtsi ?
>> I'm quite sure kevin will post it with the Ethernet nodes.
> 
> Why? The header file is related to the reset controller. Better add it
> now with this patchset.
> 
> Cheers,
> 

OK, I will add it in gxbb and meson8b dtsi files.

Neil

Re: [PATCH 1/3] reset: Add support for the Amlogic Meson GXBB Reset Controller

[Neil Armstrong]

On 05/20/2016 12:04 PM, Carlo Caione wrote:
> On 20/05/16 11:10, Neil Armstrong wrote:
>> On 05/20/2016 11:04 AM, Carlo Caione wrote:
>>> On 20/05/16 10:27, Neil Armstrong wrote:
>>>> This patch adds the platform driver for the Amlogic Meson GXBB Reset
>>>> Controller.
>>>>
>>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>>> ---
>>>>  drivers/reset/Kconfig            |   6 ++
>>>>  drivers/reset/Makefile           |   1 +
>>>>  drivers/reset/reset-meson-gxbb.c | 129 +++++++++++++++++++++++++++++++++++++++
>>>>  3 files changed, 136 insertions(+)
>>>>  create mode 100644 drivers/reset/reset-meson-gxbb.c
>>>
>>> Do we really need to be that specific (-gxbb)? This driver looks generic
>>> and simple enough to be used for several Amlogic families. You are
>>> already differentiating between them with the include file defining the
>>> reset indexes for the SoC.
>>
>> This is a good question, do the S805 have similar registers ? Same count and width ?
>> I no, it should need a rework to add a data structure per-SoC.
> 
> According to the datasheet on S805 we have 7 registers with 16 reset
> bits per register.

It will fit, I made the change to be generic. Do you know if the meson8 and previous meson6 has these registers ?

Neil

Re: [PATCH 3/3] ARM64: dts: amlogic: Enable Reset Controller on GXBB-based platforms

[Carlo Caione]

On 20/05/16 14:19, Neil Armstrong wrote:

[...]
> >>> Missing #include <dt-bindings/reset/amlogic,meson-gxbb-reset.h> ?
> >>>
> >>> Thanks,
> >>>
> >>
> >> Shouldn't we wait until the reset bindings are actually used in the dtsi ?
> >> I'm quite sure kevin will post it with the Ethernet nodes.
> > 
> > Why? The header file is related to the reset controller. Better add it
> > now with this patchset.
> > 
> > Cheers,
> > 
> 
> OK, I will add it in gxbb and meson8b dtsi files.

Why also meson8b DTSI?

-- 
Carlo Caione

Re: [PATCH 3/3] ARM64: dts: amlogic: Enable Reset Controller on GXBB-based platforms

[Neil Armstrong]

On 05/20/2016 02:22 PM, Carlo Caione wrote:
> On 20/05/16 14:19, Neil Armstrong wrote:
> 
> [...]
>>>>> Missing #include <dt-bindings/reset/amlogic,meson-gxbb-reset.h> ?
>>>>>
>>>>> Thanks,
>>>>>
>>>>
>>>> Shouldn't we wait until the reset bindings are actually used in the dtsi ?
>>>> I'm quite sure kevin will post it with the Ethernet nodes.
>>>
>>> Why? The header file is related to the reset controller. Better add it
>>> now with this patchset.
>>>
>>> Cheers,
>>>
>>
>> OK, I will add it in gxbb and meson8b dtsi files.
> 
> Why also meson8b DTSI?
> 

Sorry, I'll add the meson8b specific header of course.

Neil

Re: [PATCH 1/3] reset: Add support for the Amlogic Meson GXBB Reset Controller

[Carlo Caione]

On 20/05/16 14:20, Neil Armstrong wrote:
> On 05/20/2016 12:04 PM, Carlo Caione wrote:
> > On 20/05/16 11:10, Neil Armstrong wrote:
> >> On 05/20/2016 11:04 AM, Carlo Caione wrote:
> >>> On 20/05/16 10:27, Neil Armstrong wrote:
> >>>> This patch adds the platform driver for the Amlogic Meson GXBB Reset
> >>>> Controller.
> >>>>
> >>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> >>>> ---
> >>>>  drivers/reset/Kconfig            |   6 ++
> >>>>  drivers/reset/Makefile           |   1 +
> >>>>  drivers/reset/reset-meson-gxbb.c | 129 +++++++++++++++++++++++++++++++++++++++
> >>>>  3 files changed, 136 insertions(+)
> >>>>  create mode 100644 drivers/reset/reset-meson-gxbb.c
> >>>
> >>> Do we really need to be that specific (-gxbb)? This driver looks generic
> >>> and simple enough to be used for several Amlogic families. You are
> >>> already differentiating between them with the include file defining the
> >>> reset indexes for the SoC.
> >>
> >> This is a good question, do the S805 have similar registers ? Same count and width ?
> >> I no, it should need a rework to add a data structure per-SoC.
> > 
> > According to the datasheet on S805 we have 7 registers with 16 reset
> > bits per register.
> 
> It will fit, I made the change to be generic. Do you know if the meson8 and previous meson6 has these registers ?

No idea. No datasheet for those.

Cheers,

-- 
Carlo Caione

Re: [PATCH 2/3] dt-bindings: reset: Add bindings for the Meson GXBB Reset Controller

[Rob Herring]

On Fri, May 20, 2016 at 10:27:54AM +0200, Neil Armstrong wrote:
> Add DT bindings for the Meson GXBB SoC Reset Controller documentation and the
> associated include file.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  .../bindings/reset/amlogic,meson-gxbb-reset.txt    |  18 ++
>  .../dt-bindings/reset/amlogic,meson-gxbb-reset.h   | 199 +++++++++++++++++++++
>  2 files changed, 217 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/amlogic,meson-gxbb-reset.txt
>  create mode 100644 include/dt-bindings/reset/amlogic,meson-gxbb-reset.h

Other than the whitespace comments,

Acked-by: Rob Herring <robh@kernel.org>