[PATCH 0/2] Introduce STM32MP1 Reset driver

[PATCH 0/2] Introduce STM32MP1 Reset driver

[gabriel.fernandez at st.com]

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

This patch-set enables the reset of STM32MP1.
It uses the reset simple driver by introducing the clear register offset
parameter.
STM32MP1 reset IP has a register to assert by writing '1' and another
register to de-assert by writing '1'.
The offset between this two registers is '0x4'.

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: simple: Enable stm32mp1 reset driver

 drivers/reset/reset-simple.c                |  27 +++++--
 drivers/reset/reset-simple.h                |   1 +
 include/dt-bindings/reset/stm32mp1-resets.h | 108 ++++++++++++++++++++++++++++
 3 files changed, 130 insertions(+), 6 deletions(-)
 create mode 100644 include/dt-bindings/reset/stm32mp1-resets.h

-- 
1.9.1

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

[gabriel.fernandez at st.com]

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>
---
 include/dt-bindings/reset/stm32mp1-resets.h | 108 ++++++++++++++++++++++++++++
 1 file changed, 108 insertions(+)
 create mode 100644 include/dt-bindings/reset/stm32mp1-resets.h

diff --git a/include/dt-bindings/reset/stm32mp1-resets.h b/include/dt-bindings/reset/stm32mp1-resets.h
new file mode 100644
index 0000000..177e658
--- /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 2/2] reset: simple: Enable stm32mp1 reset driver

[gabriel.fernandez at st.com]

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

The stm32mp1 reset driver is quite similar to simple reset driver.
The difference is that stm32mp1 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

Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
---
 drivers/reset/reset-simple.c | 27 +++++++++++++++++++++------
 drivers/reset/reset-simple.h |  1 +
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c
index f7ce891..57ecb49 100644
--- a/drivers/reset/reset-simple.c
+++ b/drivers/reset/reset-simple.c
@@ -41,15 +41,23 @@ static int reset_simple_update(struct reset_controller_dev *rcdev,
 	int offset = id % (reg_width * BITS_PER_BYTE);
 	unsigned long flags;
 	u32 reg;
+	void __iomem *addr;
 
 	spin_lock_irqsave(&data->lock, flags);
 
-	reg = readl(data->membase + (bank * reg_width));
-	if (assert ^ data->active_low)
-		reg |= BIT(offset);
-	else
-		reg &= ~BIT(offset);
-	writel(reg, data->membase + (bank * reg_width));
+	addr = data->membase + (bank * reg_width);
+	if (data->clr_offset) {
+		reg = BIT(offset);
+		if (!assert)
+			addr += data->clr_offset;
+	} else {
+		reg = readl(addr);
+		if (assert ^ data->active_low)
+			reg |= BIT(offset);
+		else
+			reg &= ~BIT(offset);
+	}
+	writel(reg, addr);
 
 	spin_unlock_irqrestore(&data->lock, flags);
 
@@ -103,6 +111,7 @@ struct reset_simple_devdata {
 	u32 nr_resets;
 	bool active_low;
 	bool status_active_low;
+	u32 clr_offset;
 };
 
 #define SOCFPGA_NR_BANKS	8
@@ -118,9 +127,14 @@ struct reset_simple_devdata {
 	.status_active_low = true,
 };
 
+struct reset_simple_devdata reset_stm32mp1 = {
+	.clr_offset = 0x4,
+};
+
 static const struct of_device_id reset_simple_dt_ids[] = {
 	{ .compatible = "altr,rst-mgr", .data = &reset_simple_socfpga },
 	{ .compatible = "st,stm32-rcc", },
+	{ .compatible = "st,stm32mp1-rcc", .data = &reset_stm32mp1},
 	{ .compatible = "allwinner,sun6i-a31-clock-reset",
 		.data = &reset_simple_active_low },
 	{ .compatible = "zte,zx296718-reset",
@@ -163,6 +177,7 @@ static int reset_simple_probe(struct platform_device *pdev)
 			data->rcdev.nr_resets = devdata->nr_resets;
 		data->active_low = devdata->active_low;
 		data->status_active_low = devdata->status_active_low;
+		data->clr_offset = devdata->clr_offset;
 	}
 
 	if (of_device_is_compatible(dev->of_node, "altr,rst-mgr") &&
diff --git a/drivers/reset/reset-simple.h b/drivers/reset/reset-simple.h
index 8a49602..0bbdd34 100644
--- a/drivers/reset/reset-simple.h
+++ b/drivers/reset/reset-simple.h
@@ -38,6 +38,7 @@ struct reset_simple_data {
 	struct reset_controller_dev	rcdev;
 	bool				active_low;
 	bool				status_active_low;
+	u32				clr_offset;
 };
 
 extern const struct reset_control_ops reset_simple_ops;
-- 
1.9.1

[PATCH 0/2] Introduce STM32MP1 Reset driver

[Philipp Zabel]

Hi Gabriel,

On Tue, 2018-03-13 at 17:34 +0100, gabriel.fernandez at st.com wrote:
> From: Gabriel Fernandez <gabriel.fernandez@st.com>
> 
> This patch-set enables the reset of STM32MP1.
> It uses the reset simple driver by introducing the clear register offset
> parameter.
> STM32MP1 reset IP has a register to assert by writing '1' and another
> register to de-assert by writing '1'.
> The offset between this two registers is '0x4'.

I worry a bit about feature creep in the simple-reset driver.
Your patch on its own is simple enough, and I'm not opposed to add a
SET/CLR feature on principle, but there are a few issues:

The RESET_SIMPLE Kconfig description currently says:
??"This enables a simple reset controller driver for reset lines that
???that can be asserted and deasserted by toggling bits in a contiguous,
???exclusive register space."
That would have to be extended to mention SET/CLR register pairs as an
alternative.

What about status (reset_simple_status)? Can current reset line status
be read back from the SET register, as is currently tried? If not, is
there a way to read current reset line status back at all?

The data->lock spinlock is only needed to protect the read-modify-write
cycle on a toggle register, for separate SET/CLR register access the
locking is not necessary.

At this point, it may or may not be easier to add a custom reset driver.
Either way you go, this is missing binding documentation for the
st,stm32mp1-rcc compatible in Documentation/devicetree/bindings/reset.

> 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: simple: Enable stm32mp1 reset driver
> 
>  drivers/reset/reset-simple.c                |  27 +++++--
>  drivers/reset/reset-simple.h                |   1 +
>  include/dt-bindings/reset/stm32mp1-resets.h | 108 ++++++++++++++++++++++++++++
>  3 files changed, 130 insertions(+), 6 deletions(-)
>  create mode 100644 include/dt-bindings/reset/stm32mp1-resets.h

regards
Philipp

[PATCH 0/2] Introduce STM32MP1 Reset driver

[Gabriel FERNANDEZ]

Hi Philipp,

Okay, i too support the idea to add custom? reset driver.

Many Thanks Philipp.

Best regards

Gabriel


On 03/14/2018 10:12 AM, Philipp Zabel wrote:
> Hi Gabriel,
>
> On Tue, 2018-03-13 at 17:34 +0100, gabriel.fernandez at st.com wrote:
>> From: Gabriel Fernandez <gabriel.fernandez@st.com>
>>
>> This patch-set enables the reset of STM32MP1.
>> It uses the reset simple driver by introducing the clear register offset
>> parameter.
>> STM32MP1 reset IP has a register to assert by writing '1' and another
>> register to de-assert by writing '1'.
>> The offset between this two registers is '0x4'.
> I worry a bit about feature creep in the simple-reset driver.
> Your patch on its own is simple enough, and I'm not opposed to add a
> SET/CLR feature on principle, but there are a few issues:
>
> The RESET_SIMPLE Kconfig description currently says:
>  ??"This enables a simple reset controller driver for reset lines that
>  ???that can be asserted and deasserted by toggling bits in a contiguous,
>  ???exclusive register space."
> That would have to be extended to mention SET/CLR register pairs as an
> alternative.
>
> What about status (reset_simple_status)? Can current reset line status
> be read back from the SET register, as is currently tried? If not, is
> there a way to read current reset line status back at all?
>
> The data->lock spinlock is only needed to protect the read-modify-write
> cycle on a toggle register, for separate SET/CLR register access the
> locking is not necessary.
>
> At this point, it may or may not be easier to add a custom reset driver.
> Either way you go, this is missing binding documentation for the
> st,stm32mp1-rcc compatible in Documentation/devicetree/bindings/reset.
>
>> 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: simple: Enable stm32mp1 reset driver
>>
>>   drivers/reset/reset-simple.c                |  27 +++++--
>>   drivers/reset/reset-simple.h                |   1 +
>>   include/dt-bindings/reset/stm32mp1-resets.h | 108 ++++++++++++++++++++++++++++
>>   3 files changed, 130 insertions(+), 6 deletions(-)
>>   create mode 100644 include/dt-bindings/reset/stm32mp1-resets.h
> regards
> Philipp