[PATCH v2 0/2] Introduce STM32MP1 Reset driver

[PATCH v2 0/2] Introduce STM32MP1 Reset driver

[gabriel.fernandez]

From: Gabriel Fernandez <gabriel.fernandez@st.com>

v2:
Don't use reset-simple driver but a custom reset driver.
add dt-binding documentation.

This patch-set enables the reset of STM32MP1.
STM32MP1 reset IP has a register to assert by writing '1' and another
register to de-assert by writing '1'.

The patch 'dt-bindings: reset: add STM32MP1 resets' could be squashed
with the patch:
'dt-bindings: Document STM32MP1 Reset Clock Controller (RCC) bindings'
commit 3830681d354f

Gabriel Fernandez (2):
  dt-bindings: reset: add STM32MP1 resets
  reset: stm32mp1: Enable stm32mp1 reset driver

 .../devicetree/bindings/reset/st,stm32mp1-rcc.txt  |   6 +
 drivers/reset/Kconfig                              |   6 +
 drivers/reset/Makefile                             |   1 +
 drivers/reset/reset-stm32mp1.c                     | 122 +++++++++++++++++++++
 include/dt-bindings/reset/stm32mp1-resets.h        | 108 ++++++++++++++++++
 5 files changed, 243 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/st,stm32mp1-rcc.txt
 create mode 100644 drivers/reset/reset-stm32mp1.c
 create mode 100644 include/dt-bindings/reset/stm32mp1-resets.h

-- 
1.9.1

[PATCH v2 1/2] dt-bindings: reset: add STM32MP1 resets

[gabriel.fernandez]

From: Gabriel Fernandez <gabriel.fernandez@st.com>

This patch adds the reset binding entry for STM32MP1

Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
---
 .../devicetree/bindings/reset/st,stm32mp1-rcc.txt  |   6 ++
 include/dt-bindings/reset/stm32mp1-resets.h        | 108 +++++++++++++++++++++
 2 files changed, 114 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/st,stm32mp1-rcc.txt
 create mode 100644 include/dt-bindings/reset/stm32mp1-resets.h

diff --git a/Documentation/devicetree/bindings/reset/st,stm32mp1-rcc.txt b/Documentation/devicetree/bindings/reset/st,stm32mp1-rcc.txt
new file mode 100644
index 0000000..b4edaf7
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/st,stm32mp1-rcc.txt
@@ -0,0 +1,6 @@
+STMicroelectronics STM32MP1 Peripheral Reset Controller
+=======================================================
+
+The RCC IP is both a reset and a clock controller.
+
+Please see Documentation/devicetree/bindings/clock/st,stm32mp1-rcc.txt
diff --git a/include/dt-bindings/reset/stm32mp1-resets.h b/include/dt-bindings/reset/stm32mp1-resets.h
new file mode 100644
index 0000000..f0c3aae
--- /dev/null
+++ b/include/dt-bindings/reset/stm32mp1-resets.h
@@ -0,0 +1,108 @@
+/* SPDX-License-Identifier: GPL-2.0 or BSD-3-Clause */
+/*
+ * Copyright (C) STMicroelectronics 2018 - All Rights Reserved
+ * Author: Gabriel Fernandez <gabriel.fernandez@st.com> for STMicroelectronics.
+ */
+
+#ifndef _DT_BINDINGS_STM32MP1_RESET_H_
+#define _DT_BINDINGS_STM32MP1_RESET_H_
+
+#define LTDC_R		3072
+#define DSI_R		3076
+#define DDRPERFM_R	3080
+#define USBPHY_R	3088
+#define SPI6_R		3136
+#define I2C4_R		3138
+#define I2C6_R		3139
+#define USART1_R	3140
+#define STGEN_R		3156
+#define GPIOZ_R		3200
+#define CRYP1_R		3204
+#define HASH1_R		3205
+#define RNG1_R		3206
+#define AXIM_R		3216
+#define GPU_R		3269
+#define ETHMAC_R	3274
+#define FMC_R		3276
+#define QSPI_R		3278
+#define SDMMC1_R	3280
+#define SDMMC2_R	3281
+#define CRC1_R		3284
+#define USBH_R		3288
+#define MDMA_R		3328
+#define MCU_R		8225
+#define TIM2_R		19456
+#define TIM3_R		19457
+#define TIM4_R		19458
+#define TIM5_R		19459
+#define TIM6_R		19460
+#define TIM7_R		19461
+#define TIM12_R		16462
+#define TIM13_R		16463
+#define TIM14_R		16464
+#define LPTIM1_R	19465
+#define SPI2_R		19467
+#define SPI3_R		19468
+#define USART2_R	19470
+#define USART3_R	19471
+#define UART4_R		19472
+#define UART5_R		19473
+#define UART7_R		19474
+#define UART8_R		19475
+#define I2C1_R		19477
+#define I2C2_R		19478
+#define I2C3_R		19479
+#define I2C5_R		19480
+#define SPDIF_R		19482
+#define CEC_R		19483
+#define DAC12_R		19485
+#define MDIO_R		19847
+#define TIM1_R		19520
+#define TIM8_R		19521
+#define TIM15_R		19522
+#define TIM16_R		19523
+#define TIM17_R		19524
+#define SPI1_R		19528
+#define SPI4_R		19529
+#define SPI5_R		19530
+#define USART6_R	19533
+#define SAI1_R		19536
+#define SAI2_R		19537
+#define SAI3_R		19538
+#define DFSDM_R		19540
+#define FDCAN_R		19544
+#define LPTIM2_R	19584
+#define LPTIM3_R	19585
+#define LPTIM4_R	19586
+#define LPTIM5_R	19587
+#define SAI4_R		19592
+#define SYSCFG_R	19595
+#define VREF_R		19597
+#define TMPSENS_R	19600
+#define PMBCTRL_R	19601
+#define DMA1_R		19648
+#define DMA2_R		19649
+#define DMAMUX_R	19650
+#define ADC12_R		19653
+#define USBO_R		19656
+#define SDMMC3_R	19664
+#define CAMITF_R	19712
+#define CRYP2_R		19716
+#define HASH2_R		19717
+#define RNG2_R		19718
+#define CRC2_R		19719
+#define HSEM_R		19723
+#define MBOX_R		19724
+#define GPIOA_R		19776
+#define GPIOB_R		19777
+#define GPIOC_R		19778
+#define GPIOD_R		19779
+#define GPIOE_R		19780
+#define GPIOF_R		19781
+#define GPIOG_R		19782
+#define GPIOH_R		19783
+#define GPIOI_R		19784
+#define GPIOJ_R		19785
+#define GPIOK_R		19786
+
+#endif /* _DT_BINDINGS_STM32MP1_RESET_H_ */
-- 
1.9.1

[PATCH v2 2/2] reset: stm32mp1: Enable stm32mp1 reset driver

[gabriel.fernandez]

From: Gabriel Fernandez <gabriel.fernandez@st.com>

stm32mp1 RCC IP 1 has a reset SET register and a reset CLEAR register.

Writing '0' on reset SET register has no effect
Writing '1' on reset SET register
	activates the reset of the corresponding peripheral

Writing '0' on reset CLEAR register	has no effect
Writing '1' on reset CLEAR register
	releases the reset of the corresponding peripheral

See Documentation/devicetree/bindings/clock/st,stm32mp1-rcc.txt

Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
---
 drivers/reset/Kconfig          |   6 ++
 drivers/reset/Makefile         |   1 +
 drivers/reset/reset-stm32mp1.c | 122 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 129 insertions(+)
 create mode 100644 drivers/reset/reset-stm32mp1.c

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 1efbc6c..c0b292b 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -97,6 +97,12 @@ config RESET_SIMPLE
 	   - Allwinner SoCs
 	   - ZTE's zx2967 family
 
+config RESET_STM32MP157
+	bool "STM32MP157 Reset Driver" if COMPILE_TEST
+	default MACH_STM32MP157
+	help
+	  This enables the RCC reset controller driver for STM32 MPUs.
+
 config RESET_SUNXI
 	bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
 	default ARCH_SUNXI
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 132c24f..c1261dc 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_RESET_MESON) += reset-meson.o
 obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
 obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
 obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
+obj-$(CONFIG_RESET_STM32MP157) += reset-stm32mp1.o
 obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
 obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
 obj-$(CONFIG_RESET_TI_SYSCON) += reset-ti-syscon.o
diff --git a/drivers/reset/reset-stm32mp1.c b/drivers/reset/reset-stm32mp1.c
new file mode 100644
index 0000000..5e25388
--- /dev/null
+++ b/drivers/reset/reset-stm32mp1.c
@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) STMicroelectronics 2018 - All Rights Reserved
+ * Author: Gabriel Fernandez <gabriel.fernandez@st.com> for STMicroelectronics.
+ */
+
+#include <linux/arm-smccc.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+
+#define CLR_OFFSET 0x4
+
+struct stm32_reset_data {
+	struct reset_controller_dev	rcdev;
+	void __iomem			*membase;
+};
+
+static inline struct stm32_reset_data *
+to_stm32_reset_data(struct reset_controller_dev *rcdev)
+{
+	return container_of(rcdev, struct stm32_reset_data, rcdev);
+}
+
+static int stm32_reset_update(struct reset_controller_dev *rcdev,
+			      unsigned long id, bool assert)
+{
+	struct stm32_reset_data *data = to_stm32_reset_data(rcdev);
+	int reg_width = sizeof(u32);
+	int bank = id / (reg_width * BITS_PER_BYTE);
+	int offset = id % (reg_width * BITS_PER_BYTE);
+	void __iomem *addr;
+
+	addr = data->membase + (bank * reg_width);
+	if (!assert)
+		addr += CLR_OFFSET;
+
+	writel(BIT(offset), addr);
+
+	return 0;
+}
+
+static int stm32_reset_assert(struct reset_controller_dev *rcdev,
+			      unsigned long id)
+{
+	return stm32_reset_update(rcdev, id, true);
+}
+
+static int stm32_reset_deassert(struct reset_controller_dev *rcdev,
+				unsigned long id)
+{
+	return stm32_reset_update(rcdev, id, false);
+}
+
+static int stm32_reset_status(struct reset_controller_dev *rcdev,
+			      unsigned long id)
+{
+	struct stm32_reset_data *data = to_stm32_reset_data(rcdev);
+	int reg_width = sizeof(u32);
+	int bank = id / (reg_width * BITS_PER_BYTE);
+	int offset = id % (reg_width * BITS_PER_BYTE);
+	u32 reg;
+
+	reg = readl(data->membase + (bank * reg_width));
+
+	return !(reg & BIT(offset));
+}
+
+const struct reset_control_ops stm32_reset_ops = {
+	.assert		= stm32_reset_assert,
+	.deassert	= stm32_reset_deassert,
+	.status		= stm32_reset_status,
+};
+
+static const struct of_device_id stm32_reset_dt_ids[] = {
+	{ .compatible = "st,stm32mp1-rcc"},
+	{ /* sentinel */ },
+};
+
+static int stm32_reset_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct stm32_reset_data *data;
+	void __iomem *membase;
+	struct resource *res;
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	membase = devm_ioremap_resource(dev, res);
+	if (IS_ERR(membase))
+		return PTR_ERR(membase);
+
+	data->membase = membase;
+	data->rcdev.owner = THIS_MODULE;
+	data->rcdev.nr_resets = resource_size(res) * BITS_PER_BYTE;
+	data->rcdev.ops = &stm32_reset_ops;
+	data->rcdev.of_node = dev->of_node;
+
+	return devm_reset_controller_register(dev, &data->rcdev);
+}
+
+static struct platform_driver stm32_reset_driver = {
+	.probe	= stm32_reset_probe,
+	.driver = {
+		.name		= "stm32mp1-reset",
+		.of_match_table	= stm32_reset_dt_ids,
+	},
+};
+
+static int __init stm32_reset_init(void)
+{
+	return platform_driver_register(&stm32_reset_driver);
+}
+
+postcore_initcall(stm32_reset_init);
-- 
1.9.1

[RFC PATCH] reset: stm32mp1: stm32_reset_ops can be static

[kbuild test robot]

Fixes: d05c83341cbc ("reset: stm32mp1: Enable stm32mp1 reset driver")
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---
 reset-stm32mp1.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/reset/reset-stm32mp1.c b/drivers/reset/reset-stm32mp1.c
index 5e25388..ed44604 100644
--- a/drivers/reset/reset-stm32mp1.c
+++ b/drivers/reset/reset-stm32mp1.c
@@ -70,7 +70,7 @@ static int stm32_reset_status(struct reset_controller_dev *rcdev,
 	return !(reg & BIT(offset));
 }
 
-const struct reset_control_ops stm32_reset_ops = {
+static const struct reset_control_ops stm32_reset_ops = {
 	.assert		= stm32_reset_assert,
 	.deassert	= stm32_reset_deassert,
 	.status		= stm32_reset_status,

Re: [PATCH v2 2/2] reset: stm32mp1: Enable stm32mp1 reset driver

[kbuild test robot]

Hi Gabriel,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on next-20180309]
[also build test WARNING on v4.16-rc5]
[cannot apply to linus/master v4.16-rc4 v4.16-rc3 v4.16-rc2]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/gabriel-fernandez-st-com/Introduce-STM32MP1-Reset-driver/20180316-082607
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/reset/reset-stm32mp1.c:73:32: sparse: symbol 'stm32_reset_ops' was not declared. Should it be static?

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Re: [PATCH v2 2/2] reset: stm32mp1: Enable stm32mp1 reset driver

[Philipp Zabel]

Hi Gabriel,

this looks mostly good to me, a few questions and comments below:

On Wed, 2018-03-14 at 17:30 +0100, gabriel.fernandez@st.com wrote:
> From: Gabriel Fernandez <gabriel.fernandez@st.com>
> 
> stm32mp1 RCC IP 1 has a reset SET register and a reset CLEAR register.
> 
> Writing '0' on reset SET register has no effect
> Writing '1' on reset SET register
> 	activates the reset of the corresponding peripheral
> 
> Writing '0' on reset CLEAR register	has no effect
> Writing '1' on reset CLEAR register
> 	releases the reset of the corresponding peripheral
> 
> See Documentation/devicetree/bindings/clock/st,stm32mp1-rcc.txt
> 
> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> ---
>  drivers/reset/Kconfig          |   6 ++
>  drivers/reset/Makefile         |   1 +
>  drivers/reset/reset-stm32mp1.c | 122 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 129 insertions(+)
>  create mode 100644 drivers/reset/reset-stm32mp1.c
> 
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 1efbc6c..c0b292b 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -97,6 +97,12 @@ config RESET_SIMPLE
>  	   - Allwinner SoCs
>  	   - ZTE's zx2967 family
>  
> +config RESET_STM32MP157
> +	bool "STM32MP157 Reset Driver" if COMPILE_TEST
> +	default MACH_STM32MP157
> +	help
> +	  This enables the RCC reset controller driver for STM32 MPUs.
> +
>  config RESET_SUNXI
>  	bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
>  	default ARCH_SUNXI
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 132c24f..c1261dc 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_RESET_MESON) += reset-meson.o
>  obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
>  obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
>  obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
> +obj-$(CONFIG_RESET_STM32MP157) += reset-stm32mp1.o
>  obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
>  obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
>  obj-$(CONFIG_RESET_TI_SYSCON) += reset-ti-syscon.o
> diff --git a/drivers/reset/reset-stm32mp1.c b/drivers/reset/reset-stm32mp1.c
> new file mode 100644
> index 0000000..5e25388
> --- /dev/null
> +++ b/drivers/reset/reset-stm32mp1.c
> @@ -0,0 +1,122 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) STMicroelectronics 2018 - All Rights Reserved
> + * Author: Gabriel Fernandez <gabriel.fernandez@st.com> for STMicroelectronics.
> + */
> +
> +#include <linux/arm-smccc.h>

This does not seem to be necessary.

> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/of.h>

> +#include <linux/of_device.h>

This does not seem to be necessary either.

> +#include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> +
> +#define CLR_OFFSET 0x4
> +
> +struct stm32_reset_data {
> +	struct reset_controller_dev	rcdev;
> +	void __iomem			*membase;
> +};
> +
> +static inline struct stm32_reset_data *
> +to_stm32_reset_data(struct reset_controller_dev *rcdev)
> +{
> +	return container_of(rcdev, struct stm32_reset_data, rcdev);
> +}
> +
> +static int stm32_reset_update(struct reset_controller_dev *rcdev,
> +			      unsigned long id, bool assert)
> +{
> +	struct stm32_reset_data *data = to_stm32_reset_data(rcdev);
> +	int reg_width = sizeof(u32);
> +	int bank = id / (reg_width * BITS_PER_BYTE);
> +	int offset = id % (reg_width * BITS_PER_BYTE);
> +	void __iomem *addr;
> +
> +	addr = data->membase + (bank * reg_width);
> +	if (!assert)
> +		addr += CLR_OFFSET;
> +
> +	writel(BIT(offset), addr);
> +
> +	return 0;
> +}
> +
> +static int stm32_reset_assert(struct reset_controller_dev *rcdev,
> +			      unsigned long id)
> +{
> +	return stm32_reset_update(rcdev, id, true);
> +}
> +
> +static int stm32_reset_deassert(struct reset_controller_dev *rcdev,
> +				unsigned long id)
> +{
> +	return stm32_reset_update(rcdev, id, false);
> +}
> +
> +static int stm32_reset_status(struct reset_controller_dev *rcdev,
> +			      unsigned long id)
> +{
> +	struct stm32_reset_data *data = to_stm32_reset_data(rcdev);
> +	int reg_width = sizeof(u32);
> +	int bank = id / (reg_width * BITS_PER_BYTE);
> +	int offset = id % (reg_width * BITS_PER_BYTE);
> +	u32 reg;
> +
> +	reg = readl(data->membase + (bank * reg_width));
> +
> +	return !(reg & BIT(offset));
> +}

So the SET register can be read back and returns 0 for reset lines that
are currently asserted and returns 1 for reset lines that are currently
deasserted?

> +
> +const struct reset_control_ops stm32_reset_ops = {
> +	.assert		= stm32_reset_assert,
> +	.deassert	= stm32_reset_deassert,
> +	.status		= stm32_reset_status,
> +};
> +
> +static const struct of_device_id stm32_reset_dt_ids[] = {
> +	{ .compatible = "st,stm32mp1-rcc"},
> +	{ /* sentinel */ },
> +};

>From the DT bindings it looks like the clock and reset drivers are
sharing the same node. Is there just no clock platform_driver at all?

> +
> +static int stm32_reset_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct stm32_reset_data *data;
> +	void __iomem *membase;
> +	struct resource *res;
> +
> +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	membase = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(membase))
> +		return PTR_ERR(membase);
> +
> +	data->membase = membase;
> +	data->rcdev.owner = THIS_MODULE;
> +	data->rcdev.nr_resets = resource_size(res) * BITS_PER_BYTE;
> +	data->rcdev.ops = &stm32_reset_ops;
> +	data->rcdev.of_node = dev->of_node;
> +
> +	return devm_reset_controller_register(dev, &data->rcdev);
> +}
> +
> +static struct platform_driver stm32_reset_driver = {
> +	.probe	= stm32_reset_probe,
> +	.driver = {
> +		.name		= "stm32mp1-reset",
> +		.of_match_table	= stm32_reset_dt_ids,
> +	},
> +};
> +
> +static int __init stm32_reset_init(void)
> +{
> +	return platform_driver_register(&stm32_reset_driver);
> +}
> +
> +postcore_initcall(stm32_reset_init);

Isn't builtin_platform_driver early enough?

regards
Philipp

Re: [PATCH v2 2/2] reset: stm32mp1: Enable stm32mp1 reset driver

[Gabriel FERNANDEZ]

Hi Philipp,

Thanks for reviewing.

On 03/16/2018 02:29 PM, Philipp Zabel wrote:
> Hi Gabriel,
>
> this looks mostly good to me, a few questions and comments below:
>
> On Wed, 2018-03-14 at 17:30 +0100, gabriel.fernandez@st.com wrote:
>> From: Gabriel Fernandez <gabriel.fernandez@st.com>
>>
>> stm32mp1 RCC IP 1 has a reset SET register and a reset CLEAR register.
>>
>> Writing '0' on reset SET register has no effect
>> Writing '1' on reset SET register
>> 	activates the reset of the corresponding peripheral
>>
>> Writing '0' on reset CLEAR register	has no effect
>> Writing '1' on reset CLEAR register
>> 	releases the reset of the corresponding peripheral
>>
>> See Documentation/devicetree/bindings/clock/st,stm32mp1-rcc.txt
>>
>> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
>> ---
>>   drivers/reset/Kconfig          |   6 ++
>>   drivers/reset/Makefile         |   1 +
>>   drivers/reset/reset-stm32mp1.c | 122 +++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 129 insertions(+)
>>   create mode 100644 drivers/reset/reset-stm32mp1.c
>>
>> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
>> index 1efbc6c..c0b292b 100644
>> --- a/drivers/reset/Kconfig
>> +++ b/drivers/reset/Kconfig
>> @@ -97,6 +97,12 @@ config RESET_SIMPLE
>>   	   - Allwinner SoCs
>>   	   - ZTE's zx2967 family
>>   
>> +config RESET_STM32MP157
>> +	bool "STM32MP157 Reset Driver" if COMPILE_TEST
>> +	default MACH_STM32MP157
>> +	help
>> +	  This enables the RCC reset controller driver for STM32 MPUs.
>> +
>>   config RESET_SUNXI
>>   	bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
>>   	default ARCH_SUNXI
>> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
>> index 132c24f..c1261dc 100644
>> --- a/drivers/reset/Makefile
>> +++ b/drivers/reset/Makefile
>> @@ -15,6 +15,7 @@ obj-$(CONFIG_RESET_MESON) += reset-meson.o
>>   obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
>>   obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
>>   obj-$(CONFIG_RESET_SIMPLE) += reset-simple.o
>> +obj-$(CONFIG_RESET_STM32MP157) += reset-stm32mp1.o
>>   obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
>>   obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
>>   obj-$(CONFIG_RESET_TI_SYSCON) += reset-ti-syscon.o
>> diff --git a/drivers/reset/reset-stm32mp1.c b/drivers/reset/reset-stm32mp1.c
>> new file mode 100644
>> index 0000000..5e25388
>> --- /dev/null
>> +++ b/drivers/reset/reset-stm32mp1.c
>> @@ -0,0 +1,122 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) STMicroelectronics 2018 - All Rights Reserved
>> + * Author: Gabriel Fernandez <gabriel.fernandez@st.com> for STMicroelectronics.
>> + */
>> +
>> +#include <linux/arm-smccc.h>
> This does not seem to be necessary.
right
>> +#include <linux/device.h>
>> +#include <linux/err.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
> This does not seem to be necessary either.
ok
>> +#include <linux/platform_device.h>
>> +#include <linux/reset-controller.h>
>> +
>> +#define CLR_OFFSET 0x4
>> +
>> +struct stm32_reset_data {
>> +	struct reset_controller_dev	rcdev;
>> +	void __iomem			*membase;
>> +};
>> +
>> +static inline struct stm32_reset_data *
>> +to_stm32_reset_data(struct reset_controller_dev *rcdev)
>> +{
>> +	return container_of(rcdev, struct stm32_reset_data, rcdev);
>> +}
>> +
>> +static int stm32_reset_update(struct reset_controller_dev *rcdev,
>> +			      unsigned long id, bool assert)
>> +{
>> +	struct stm32_reset_data *data = to_stm32_reset_data(rcdev);
>> +	int reg_width = sizeof(u32);
>> +	int bank = id / (reg_width * BITS_PER_BYTE);
>> +	int offset = id % (reg_width * BITS_PER_BYTE);
>> +	void __iomem *addr;
>> +
>> +	addr = data->membase + (bank * reg_width);
>> +	if (!assert)
>> +		addr += CLR_OFFSET;
>> +
>> +	writel(BIT(offset), addr);
>> +
>> +	return 0;
>> +}
>> +
>> +static int stm32_reset_assert(struct reset_controller_dev *rcdev,
>> +			      unsigned long id)
>> +{
>> +	return stm32_reset_update(rcdev, id, true);
>> +}
>> +
>> +static int stm32_reset_deassert(struct reset_controller_dev *rcdev,
>> +				unsigned long id)
>> +{
>> +	return stm32_reset_update(rcdev, id, false);
>> +}
>> +
>> +static int stm32_reset_status(struct reset_controller_dev *rcdev,
>> +			      unsigned long id)
>> +{
>> +	struct stm32_reset_data *data = to_stm32_reset_data(rcdev);
>> +	int reg_width = sizeof(u32);
>> +	int bank = id / (reg_width * BITS_PER_BYTE);
>> +	int offset = id % (reg_width * BITS_PER_BYTE);
>> +	u32 reg;
>> +
>> +	reg = readl(data->membase + (bank * reg_width));
>> +
>> +	return !(reg & BIT(offset));
>> +}
> So the SET register can be read back and returns 0 for reset lines that
> are currently asserted and returns 1 for reset lines that are currently
> deasserted?
yes you have spotted a error, i will replace by'return !!(reg & 
BIT(offset));'
>> +
>> +const struct reset_control_ops stm32_reset_ops = {
>> +	.assert		= stm32_reset_assert,
>> +	.deassert	= stm32_reset_deassert,
>> +	.status		= stm32_reset_status,
>> +};
>> +
>> +static const struct of_device_id stm32_reset_dt_ids[] = {
>> +	{ .compatible = "st,stm32mp1-rcc"},
>> +	{ /* sentinel */ },
>> +};
>  From the DT bindings it looks like the clock and reset drivers are
> sharing the same node. Is there just no clock platform_driver at all?
there is one hardware block that is used for clock, reset and power, 
they share the same node but each have it own platform_driver

>> +
>> +static int stm32_reset_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct stm32_reset_data *data;
>> +	void __iomem *membase;
>> +	struct resource *res;
>> +
>> +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>> +	if (!data)
>> +		return -ENOMEM;
>> +
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	membase = devm_ioremap_resource(dev, res);
>> +	if (IS_ERR(membase))
>> +		return PTR_ERR(membase);
>> +
>> +	data->membase = membase;
>> +	data->rcdev.owner = THIS_MODULE;
>> +	data->rcdev.nr_resets = resource_size(res) * BITS_PER_BYTE;
>> +	data->rcdev.ops = &stm32_reset_ops;
>> +	data->rcdev.of_node = dev->of_node;
>> +
>> +	return devm_reset_controller_register(dev, &data->rcdev);
>> +}
>> +
>> +static struct platform_driver stm32_reset_driver = {
>> +	.probe	= stm32_reset_probe,
>> +	.driver = {
>> +		.name		= "stm32mp1-reset",
>> +		.of_match_table	= stm32_reset_dt_ids,
>> +	},
>> +};
>> +
>> +static int __init stm32_reset_init(void)
>> +{
>> +	return platform_driver_register(&stm32_reset_driver);
>> +}
>> +
>> +postcore_initcall(stm32_reset_init);
> Isn't builtin_platform_driver early enough?
ok

Best Regards

Gabriel

>
> regards
> Philipp

Re: [PATCH v2 1/2] dt-bindings: reset: add STM32MP1 resets

[Rob Herring]

On Wed, Mar 14, 2018 at 05:30:00PM +0100, gabriel.fernandez@st.com wrote:
> From: Gabriel Fernandez <gabriel.fernandez@st.com>
> 
> This patch adds the reset binding entry for STM32MP1
> 
> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> ---
>  .../devicetree/bindings/reset/st,stm32mp1-rcc.txt  |   6 ++
>  include/dt-bindings/reset/stm32mp1-resets.h        | 108 +++++++++++++++++++++
>  2 files changed, 114 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/st,stm32mp1-rcc.txt
>  create mode 100644 include/dt-bindings/reset/stm32mp1-resets.h

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