Re: [PATCH/RFC v3 02/22] soc: renesas: Add R-Car RST driver
On Friday 10 Jun 2016 09:58:28 Geert Uytterhoeven wrote: > Hi Dirk, > > On Thu, Jun 2, 2016 at 7:42 AM, Dirk Behme wrote: > >> +int __init rcar_rst_read_mode_pins(u32 *mode) > > > > Just a style issue: Is the string 'pins' in the function name still > > relevant? I.e. what's about just 'rcar_rst_read_mode()'? > > I feel "mode" is a too generic word for a public API. > It's used a several contexts inside the RST module (secure mode, 64-bit > addressing mode, free-running mode, step-up mode). If it's "pins" that bothers Dirk, how about rcar_rst_read_boot_mode() ? Or maybe rcar_rst_boot_mode(), given that the function caches the value, it doesn't read it every time. -- Regards, Laurent Pinchart
Re: [PATCH/RFC v3 02/22] soc: renesas: Add R-Car RST driver
On 10.06.2016 09:58, Geert Uytterhoeven wrote: Hi Dirk, On Thu, Jun 2, 2016 at 7:42 AM, Dirk Behme wrote: +int __init rcar_rst_read_mode_pins(u32 *mode) Just a style issue: Is the string 'pins' in the function name still relevant? I.e. what's about just 'rcar_rst_read_mode()'? I feel "mode" is a too generic word for a public API. It's used a several contexts inside the RST module (secure mode, 64-bit addressing mode, free-running mode, step-up mode). What's about rcar_rst_read_mode_monitor() then? Taken from the manual ;) Best regards Dirk
Re: [PATCH/RFC v3 02/22] soc: renesas: Add R-Car RST driver
Hi Dirk,
On Thu, Jun 2, 2016 at 7:42 AM, Dirk Behme wrote:
>> +int __init rcar_rst_read_mode_pins(u32 *mode)
>
> Just a style issue: Is the string 'pins' in the function name still
> relevant? I.e. what's about just 'rcar_rst_read_mode()'?
I feel "mode" is a too generic word for a public API.
It's used a several contexts inside the RST module (secure mode, 64-bit
addressing mode, free-running mode, step-up mode).
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH/RFC v3 02/22] soc: renesas: Add R-Car RST driver
Hi Laurent,
On Thu, Jun 2, 2016 at 11:58 PM, Laurent Pinchart
wrote:
>> --- /dev/null
>> +++ b/drivers/soc/renesas/rcar-rst.c
>> +static int __init rcar_rst_init(void)
>> +{
[...]
>> +}
>> +arch_initcall(rcar_rst_init);
>
> Given that rcar_rst_init() is only used as a support function for
> rcar_rst_read_mode_pins(), how about removing the initcall ?
Thanks, good idea!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH/RFC v3 02/22] soc: renesas: Add R-Car RST driver
Hi Geert,
Thank you for the patch.
On Wednesday 01 Jun 2016 21:21:00 Geert Uytterhoeven wrote:
> Add a driver for the Renesas R-Car Gen1 RESET/WDT and R-Car Gen2/Gen3
> RST module.
>
> For now this driver just provides an API to obtain the state of the mode
> pins, as latched at reset time. As this is typically called from the
> probe function of a clock driver, which can run much earlier than any
> initcall, calling rcar_rst_read_mode_pins() may force an early
> initialization of the driver.
>
> Despite the current simple and almost identical handling for all
> supported SoCs, the driver matches against SoC-specific compatible
> values only, as the features provided by the hardware module differ a
> lot across the various SoC families and members.
>
> Signed-off-by: Geert Uytterhoeven
> ---
> v3:
> - New.
> ---
> drivers/soc/renesas/Makefile | 5 ++
> drivers/soc/renesas/rcar-rst.c | 94 +
> include/linux/soc/renesas/rcar-rst.h | 6 +++
> 3 files changed, 105 insertions(+)
> create mode 100644 drivers/soc/renesas/rcar-rst.c
> create mode 100644 include/linux/soc/renesas/rcar-rst.h
>
> diff --git a/drivers/soc/renesas/Makefile b/drivers/soc/renesas/Makefile
> index f32a001c65c3ab44..0d732ff893dd8fba 100644
> --- a/drivers/soc/renesas/Makefile
> +++ b/drivers/soc/renesas/Makefile
> @@ -1,3 +1,8 @@
> +obj-$(CONFIG_ARCH_RCAR_GEN1) += rcar-rst.o
> +obj-$(CONFIG_ARCH_RCAR_GEN2) += rcar-rst.o
> +obj-$(CONFIG_ARCH_R8A7795) += rcar-rst.o
> +obj-$(CONFIG_ARCH_R8A7796) += rcar-rst.o
> +
> obj-$(CONFIG_ARCH_R8A7779) += rcar-sysc.o r8a7779-sysc.o
> obj-$(CONFIG_ARCH_R8A7790) += rcar-sysc.o r8a7790-sysc.o
> obj-$(CONFIG_ARCH_R8A7791) += rcar-sysc.o r8a7791-sysc.o
> diff --git a/drivers/soc/renesas/rcar-rst.c b/drivers/soc/renesas/rcar-rst.c
> new file mode 100644
> index ..1997c348c0853254
> --- /dev/null
> +++ b/drivers/soc/renesas/rcar-rst.c
> @@ -0,0 +1,94 @@
> +/*
> + * R-Car Gen1 RESET/WDT and R-Car Gen2/Gen3 RST Driver
> + *
> + * Copyright (C) 2016 Glider bvba
> + *
> + * This file is subject to the terms and conditions of the GNU General
> Public
> + * License. See the file "COPYING" in the main directory of this archive
> + * for more details.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +
> +struct rst_config {
> + unsigned int modemr;/* Mode Monitoring Register Offset */
> +};
> +
> +static const struct rst_config rcar_rst_gen1 __initconst = {
> + .modemr = 0x20,
> +};
> +
> +static const struct rst_config rcar_rst_gen2 __initconst = {
> + .modemr = 0x60,
> +};
> +
> +static const struct rst_config rcar_rst_gen3 __initconst = {
> + .modemr = 0x60,
> +};
> +
> +static const struct of_device_id rcar_rst_matches[] __initconst = {
> + { .compatible = "renesas,r8a7778-reset-wdt", .data = &rcar_rst_gen1 },
> + { .compatible = "renesas,r8a7779-reset-wdt", .data = &rcar_rst_gen1 },
> + { .compatible = "renesas,r8a7790-rst", .data = &rcar_rst_gen2 },
> + { .compatible = "renesas,r8a7791-rst", .data = &rcar_rst_gen2 },
> + { .compatible = "renesas,r8a7792-rst", .data = &rcar_rst_gen2 },
> + { .compatible = "renesas,r8a7793-rst", .data = &rcar_rst_gen2 },
> + { .compatible = "renesas,r8a7794-rst", .data = &rcar_rst_gen2 },
> + { .compatible = "renesas,r8a7795-rst", .data = &rcar_rst_gen3 },
> + { .compatible = "renesas,r8a7796-rst", .data = &rcar_rst_gen3 },
> + { /* sentinel */ }
> +};
> +
> +static void __iomem *rcar_rst_base __initdata;
> +static u32 saved_mode __initdata;
> +
> +static int __init rcar_rst_init(void)
> +{
> + const struct of_device_id *match;
> + const struct rst_config *cfg;
> + struct device_node *np;
> + void __iomem *base;
> + int error = 0;
> +
> + if (rcar_rst_base)
> + return 0;
> +
> + np = of_find_matching_node_and_match(NULL, rcar_rst_matches, &match);
> + if (!np)
> + return -ENODEV;
> +
> + base = of_iomap(np, 0);
> + if (!base) {
> + pr_warn("%s: Cannot map regs\n", np->full_name);
> + error = -ENOMEM;
> + goto out_put;
> + }
> +
> + rcar_rst_base = base;
> + cfg = match->data;
> + saved_mode = ioread32(base + cfg->modemr);
> +
> + pr_debug("%s: MODE = 0x%08x\n", np->full_name, saved_mode);
> +
> +out_put:
> + of_node_put(np);
> + return error;
> +}
> +arch_initcall(rcar_rst_init);
Given that rcar_rst_init() is only used as a support function for
rcar_rst_read_mode_pins(), how about removing the initcall ?
> +int __init rcar_rst_read_mode_pins(u32 *mode)
> +{
> + int error;
> +
> + if (!rcar_rst_base) {
> + error = rcar_rst_init();
> + if (error)
> + return error;
> + }
> +
> + *mode = saved_mode;
> + return 0;
> +}
> diff --git a/include/linux/soc/renesas/rcar-rst.h
> b/include/linux/soc/renesas/rcar-rst.h new file mode 100644
Re: [PATCH/RFC v3 02/22] soc: renesas: Add R-Car RST driver
On 01.06.2016 21:21, Geert Uytterhoeven wrote:
Add a driver for the Renesas R-Car Gen1 RESET/WDT and R-Car Gen2/Gen3
RST module.
For now this driver just provides an API to obtain the state of the mode
pins, as latched at reset time. As this is typically called from the
probe function of a clock driver, which can run much earlier than any
initcall, calling rcar_rst_read_mode_pins() may force an early
initialization of the driver.
Despite the current simple and almost identical handling for all
supported SoCs, the driver matches against SoC-specific compatible
values only, as the features provided by the hardware module differ a
lot across the various SoC families and members.
Signed-off-by: Geert Uytterhoeven
---
v3:
- New.
---
drivers/soc/renesas/Makefile | 5 ++
drivers/soc/renesas/rcar-rst.c | 94
include/linux/soc/renesas/rcar-rst.h | 6 +++
3 files changed, 105 insertions(+)
create mode 100644 drivers/soc/renesas/rcar-rst.c
create mode 100644 include/linux/soc/renesas/rcar-rst.h
diff --git a/drivers/soc/renesas/Makefile b/drivers/soc/renesas/Makefile
index f32a001c65c3ab44..0d732ff893dd8fba 100644
--- a/drivers/soc/renesas/Makefile
+++ b/drivers/soc/renesas/Makefile
@@ -1,3 +1,8 @@
+obj-$(CONFIG_ARCH_RCAR_GEN1) += rcar-rst.o
+obj-$(CONFIG_ARCH_RCAR_GEN2) += rcar-rst.o
+obj-$(CONFIG_ARCH_R8A7795) += rcar-rst.o
+obj-$(CONFIG_ARCH_R8A7796) += rcar-rst.o
+
obj-$(CONFIG_ARCH_R8A7779) += rcar-sysc.o r8a7779-sysc.o
obj-$(CONFIG_ARCH_R8A7790) += rcar-sysc.o r8a7790-sysc.o
obj-$(CONFIG_ARCH_R8A7791) += rcar-sysc.o r8a7791-sysc.o
diff --git a/drivers/soc/renesas/rcar-rst.c b/drivers/soc/renesas/rcar-rst.c
new file mode 100644
index ..1997c348c0853254
--- /dev/null
+++ b/drivers/soc/renesas/rcar-rst.c
@@ -0,0 +1,94 @@
+/*
+ * R-Car Gen1 RESET/WDT and R-Car Gen2/Gen3 RST Driver
+ *
+ * Copyright (C) 2016 Glider bvba
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+
+#include
+#include
+#include
+#include
+
+struct rst_config {
+ unsigned int modemr;/* Mode Monitoring Register Offset */
+};
+
+static const struct rst_config rcar_rst_gen1 __initconst = {
+ .modemr = 0x20,
+};
+
+static const struct rst_config rcar_rst_gen2 __initconst = {
+ .modemr = 0x60,
+};
+
+static const struct rst_config rcar_rst_gen3 __initconst = {
+ .modemr = 0x60,
+};
+
+static const struct of_device_id rcar_rst_matches[] __initconst = {
+ { .compatible = "renesas,r8a7778-reset-wdt", .data = &rcar_rst_gen1 },
+ { .compatible = "renesas,r8a7779-reset-wdt", .data = &rcar_rst_gen1 },
+ { .compatible = "renesas,r8a7790-rst", .data = &rcar_rst_gen2 },
+ { .compatible = "renesas,r8a7791-rst", .data = &rcar_rst_gen2 },
+ { .compatible = "renesas,r8a7792-rst", .data = &rcar_rst_gen2 },
+ { .compatible = "renesas,r8a7793-rst", .data = &rcar_rst_gen2 },
+ { .compatible = "renesas,r8a7794-rst", .data = &rcar_rst_gen2 },
+ { .compatible = "renesas,r8a7795-rst", .data = &rcar_rst_gen3 },
+ { .compatible = "renesas,r8a7796-rst", .data = &rcar_rst_gen3 },
+ { /* sentinel */ }
+};
+
+static void __iomem *rcar_rst_base __initdata;
+static u32 saved_mode __initdata;
+
+static int __init rcar_rst_init(void)
+{
+ const struct of_device_id *match;
+ const struct rst_config *cfg;
+ struct device_node *np;
+ void __iomem *base;
+ int error = 0;
+
+ if (rcar_rst_base)
+ return 0;
+
+ np = of_find_matching_node_and_match(NULL, rcar_rst_matches, &match);
+ if (!np)
+ return -ENODEV;
+
+ base = of_iomap(np, 0);
+ if (!base) {
+ pr_warn("%s: Cannot map regs\n", np->full_name);
+ error = -ENOMEM;
+ goto out_put;
+ }
+
+ rcar_rst_base = base;
+ cfg = match->data;
+ saved_mode = ioread32(base + cfg->modemr);
+
+ pr_debug("%s: MODE = 0x%08x\n", np->full_name, saved_mode);
+
+out_put:
+ of_node_put(np);
+ return error;
+}
+arch_initcall(rcar_rst_init);
+
+int __init rcar_rst_read_mode_pins(u32 *mode)
Just a style issue: Is the string 'pins' in the function name still
relevant? I.e. what's about just 'rcar_rst_read_mode()'?
Best regards
Dirk
[PATCH/RFC v3 02/22] soc: renesas: Add R-Car RST driver
Add a driver for the Renesas R-Car Gen1 RESET/WDT and R-Car Gen2/Gen3
RST module.
For now this driver just provides an API to obtain the state of the mode
pins, as latched at reset time. As this is typically called from the
probe function of a clock driver, which can run much earlier than any
initcall, calling rcar_rst_read_mode_pins() may force an early
initialization of the driver.
Despite the current simple and almost identical handling for all
supported SoCs, the driver matches against SoC-specific compatible
values only, as the features provided by the hardware module differ a
lot across the various SoC families and members.
Signed-off-by: Geert Uytterhoeven
---
v3:
- New.
---
drivers/soc/renesas/Makefile | 5 ++
drivers/soc/renesas/rcar-rst.c | 94
include/linux/soc/renesas/rcar-rst.h | 6 +++
3 files changed, 105 insertions(+)
create mode 100644 drivers/soc/renesas/rcar-rst.c
create mode 100644 include/linux/soc/renesas/rcar-rst.h
diff --git a/drivers/soc/renesas/Makefile b/drivers/soc/renesas/Makefile
index f32a001c65c3ab44..0d732ff893dd8fba 100644
--- a/drivers/soc/renesas/Makefile
+++ b/drivers/soc/renesas/Makefile
@@ -1,3 +1,8 @@
+obj-$(CONFIG_ARCH_RCAR_GEN1) += rcar-rst.o
+obj-$(CONFIG_ARCH_RCAR_GEN2) += rcar-rst.o
+obj-$(CONFIG_ARCH_R8A7795) += rcar-rst.o
+obj-$(CONFIG_ARCH_R8A7796) += rcar-rst.o
+
obj-$(CONFIG_ARCH_R8A7779) += rcar-sysc.o r8a7779-sysc.o
obj-$(CONFIG_ARCH_R8A7790) += rcar-sysc.o r8a7790-sysc.o
obj-$(CONFIG_ARCH_R8A7791) += rcar-sysc.o r8a7791-sysc.o
diff --git a/drivers/soc/renesas/rcar-rst.c b/drivers/soc/renesas/rcar-rst.c
new file mode 100644
index ..1997c348c0853254
--- /dev/null
+++ b/drivers/soc/renesas/rcar-rst.c
@@ -0,0 +1,94 @@
+/*
+ * R-Car Gen1 RESET/WDT and R-Car Gen2/Gen3 RST Driver
+ *
+ * Copyright (C) 2016 Glider bvba
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+
+#include
+#include
+#include
+#include
+
+struct rst_config {
+ unsigned int modemr;/* Mode Monitoring Register Offset */
+};
+
+static const struct rst_config rcar_rst_gen1 __initconst = {
+ .modemr = 0x20,
+};
+
+static const struct rst_config rcar_rst_gen2 __initconst = {
+ .modemr = 0x60,
+};
+
+static const struct rst_config rcar_rst_gen3 __initconst = {
+ .modemr = 0x60,
+};
+
+static const struct of_device_id rcar_rst_matches[] __initconst = {
+ { .compatible = "renesas,r8a7778-reset-wdt", .data = &rcar_rst_gen1 },
+ { .compatible = "renesas,r8a7779-reset-wdt", .data = &rcar_rst_gen1 },
+ { .compatible = "renesas,r8a7790-rst", .data = &rcar_rst_gen2 },
+ { .compatible = "renesas,r8a7791-rst", .data = &rcar_rst_gen2 },
+ { .compatible = "renesas,r8a7792-rst", .data = &rcar_rst_gen2 },
+ { .compatible = "renesas,r8a7793-rst", .data = &rcar_rst_gen2 },
+ { .compatible = "renesas,r8a7794-rst", .data = &rcar_rst_gen2 },
+ { .compatible = "renesas,r8a7795-rst", .data = &rcar_rst_gen3 },
+ { .compatible = "renesas,r8a7796-rst", .data = &rcar_rst_gen3 },
+ { /* sentinel */ }
+};
+
+static void __iomem *rcar_rst_base __initdata;
+static u32 saved_mode __initdata;
+
+static int __init rcar_rst_init(void)
+{
+ const struct of_device_id *match;
+ const struct rst_config *cfg;
+ struct device_node *np;
+ void __iomem *base;
+ int error = 0;
+
+ if (rcar_rst_base)
+ return 0;
+
+ np = of_find_matching_node_and_match(NULL, rcar_rst_matches, &match);
+ if (!np)
+ return -ENODEV;
+
+ base = of_iomap(np, 0);
+ if (!base) {
+ pr_warn("%s: Cannot map regs\n", np->full_name);
+ error = -ENOMEM;
+ goto out_put;
+ }
+
+ rcar_rst_base = base;
+ cfg = match->data;
+ saved_mode = ioread32(base + cfg->modemr);
+
+ pr_debug("%s: MODE = 0x%08x\n", np->full_name, saved_mode);
+
+out_put:
+ of_node_put(np);
+ return error;
+}
+arch_initcall(rcar_rst_init);
+
+int __init rcar_rst_read_mode_pins(u32 *mode)
+{
+ int error;
+
+ if (!rcar_rst_base) {
+ error = rcar_rst_init();
+ if (error)
+ return error;
+ }
+
+ *mode = saved_mode;
+ return 0;
+}
diff --git a/include/linux/soc/renesas/rcar-rst.h
b/include/linux/soc/renesas/rcar-rst.h
new file mode 100644
index ..a18e0783946b66ec
--- /dev/null
+++ b/include/linux/soc/renesas/rcar-rst.h
@@ -0,0 +1,6 @@
+#ifndef __LINUX_SOC_RENESAS_RCAR_RST_H__
+#define __LINUX_SOC_RENESAS_RCAR_RST_H__
+
+int rcar_rst_read_mode_pins(u32 *mode);
+
+#endif /* __LINUX_SOC_RENESAS_RCAR_RST_H__ */
--
1.9.1
