2018-03-14 16:32:12

by Gabriel FERNANDEZ

[permalink] [raw]
Subject: [PATCH v2 0/2] Introduce STM32MP1 Reset driver

From: Gabriel Fernandez <[email protected]>

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



2018-03-14 16:32:29

by Gabriel FERNANDEZ

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

From: Gabriel Fernandez <[email protected]>

This patch adds the reset binding entry for STM32MP1

Signed-off-by: Gabriel Fernandez <[email protected]>
---
.../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 <[email protected]> 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


2018-03-14 16:32:34

by Gabriel FERNANDEZ

[permalink] [raw]
Subject: [PATCH v2 2/2] reset: stm32mp1: Enable stm32mp1 reset driver

From: Gabriel Fernandez <[email protected]>

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 <[email protected]>
---
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 <[email protected]> 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


2018-03-16 12:25:07

by kernel test robot

[permalink] [raw]
Subject: [RFC PATCH] reset: stm32mp1: stm32_reset_ops can be static


Fixes: d05c83341cbc ("reset: stm32mp1: Enable stm32mp1 reset driver")
Signed-off-by: Fengguang Wu <[email protected]>
---
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,

2018-03-16 12:25:28

by kernel test robot

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

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

2018-03-16 13:31:27

by Philipp Zabel

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

Hi Gabriel,

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

On Wed, 2018-03-14 at 17:30 +0100, [email protected] wrote:
> From: Gabriel Fernandez <[email protected]>
>
> 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 <[email protected]>
> ---
> 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 <[email protected]> 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

2018-03-16 14:38:21

by Gabriel FERNANDEZ

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


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, [email protected] wrote:
>> From: Gabriel Fernandez <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>> 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 <[email protected]> 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

2018-03-18 12:52:16

by Rob Herring (Arm)

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

On Wed, Mar 14, 2018 at 05:30:00PM +0100, [email protected] wrote:
> From: Gabriel Fernandez <[email protected]>
>
> This patch adds the reset binding entry for STM32MP1
>
> Signed-off-by: Gabriel Fernandez <[email protected]>
> ---
> .../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 <[email protected]>