| Message ID | 1417616227-6715-2-git-send-email-geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | Deferred |
| Delegated to: | Simon Horman |
| Headers | show |
On Wednesday 03 December 2014 15:17:04 Geert Uytterhoeven wrote: > Extend the PM domain platform driver for the R-Mobile System > Controller (SYSC) to register a restart handler, to be used on various > Renesas ARM SoCs (e.g. R-Mobile A1 (r8a7740) and APE6 (r8a73a4), and > SH-Mobile AP4 (sh7372) and AG5 (sh73a0)). > > Note that this supports DT-based platforms only. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> Is this one of the typical system controllers that contains lots of random registers? Maybe you could mark it as "syscon" in DT and just use the existing drivers/power/reset/syscon-reboot.c driver? > +static void __iomem *sysc_base2; > + > +static void rmobile_sysc_restart(enum reboot_mode reboot_mode, const char *cmd) > +{ > + pr_debug("%s %u/%s\n", __func__, reboot_mode, cmd); > + > + writel(RESCNT2_PRES, sysc_base2 + RESCNT2); > +} > + > +static int rmobile_sysc_probe(struct platform_device *pdev) > +{ > + dev_dbg(&pdev->dev, "%s\n", __func__); > + > + sysc_base2 = of_iomap(pdev->dev.of_node, 1); > + if (!sysc_base2) > + return -ENODEV; > + > + arm_pm_restart = rmobile_sysc_restart; > + > + return 0; > +} > + > +static const struct of_device_id rmobile_sysc_of_match[] = { > + { .compatible = "renesas,sysc-rmobile", }, > + { /* sentinel */ } > +}; > + > +static struct platform_driver rmobile_sysc_driver = { > + .probe = rmobile_sysc_probe, > + .driver = { > + .name = "rmobile_sysc", > + .of_match_table = rmobile_sysc_of_match, > + }, > +}; > + > +module_platform_driver(rmobile_sysc_driver); This clearly looks much better than the previous implementation, so that is great. If you can decouple it from the rest of this file, either through syscon or by asserting that sysc_base2 is not going to be needed for anything else, how about moving the new code to a standalon driver in drivers/power/reset? Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Arnd, On Thu, Dec 4, 2014 at 11:08 AM, Arnd Bergmann <arnd@arndb.de> wrote: > On Wednesday 03 December 2014 15:17:04 Geert Uytterhoeven wrote: >> Extend the PM domain platform driver for the R-Mobile System >> Controller (SYSC) to register a restart handler, to be used on various >> Renesas ARM SoCs (e.g. R-Mobile A1 (r8a7740) and APE6 (r8a73a4), and >> SH-Mobile AP4 (sh7372) and AG5 (sh73a0)). >> >> Note that this supports DT-based platforms only. >> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > > Is this one of the typical system controllers that contains lots of > random registers? Maybe you could mark it as "syscon" in DT and just > use the existing drivers/power/reset/syscon-reboot.c driver? The hardware block has two register ranges. The first one handles boot mode and power management, the second handles reset control and allows to read the mode strap pins. The PM domain driver uses the first range, the reset handler uses the second range. I tried using syscon and syscon-reboot, but that needs some changes to the syscon framework, as it currently provides access to the first register range only. Providing access to the second region requires either: 1. Extending the regmap to cover all ranges. Does regmap support multiple ranges? It seems regmap_range_cfg handles only virtual ranges (switchable by writing to a register), not always-mapped discontiguous ranges? 2. Providing multiple regmaps, one for each register range. This needs modifications to look up ranges using a phandle and an (optional for backwards compatibility) index, and updates to drivers and bindings. Am I missing an option? >> +static void __iomem *sysc_base2; >> + >> +static void rmobile_sysc_restart(enum reboot_mode reboot_mode, const char *cmd) >> +{ >> + pr_debug("%s %u/%s\n", __func__, reboot_mode, cmd); >> + >> + writel(RESCNT2_PRES, sysc_base2 + RESCNT2); >> +} >> + >> +static int rmobile_sysc_probe(struct platform_device *pdev) >> +{ >> + dev_dbg(&pdev->dev, "%s\n", __func__); >> + >> + sysc_base2 = of_iomap(pdev->dev.of_node, 1); >> + if (!sysc_base2) >> + return -ENODEV; >> + >> + arm_pm_restart = rmobile_sysc_restart; >> + >> + return 0; >> +} >> + >> +static const struct of_device_id rmobile_sysc_of_match[] = { >> + { .compatible = "renesas,sysc-rmobile", }, >> + { /* sentinel */ } >> +}; >> + >> +static struct platform_driver rmobile_sysc_driver = { >> + .probe = rmobile_sysc_probe, >> + .driver = { >> + .name = "rmobile_sysc", >> + .of_match_table = rmobile_sysc_of_match, >> + }, >> +}; >> + >> +module_platform_driver(rmobile_sysc_driver); > > This clearly looks much better than the previous implementation, so that > is great. If you can decouple it from the rest of this file, either through > syscon or by asserting that sysc_base2 is not going to be needed for > anything else, how about moving the new code to a standalon driver in > drivers/power/reset? Yes, sysc_base2 is used for reset control only (I don't think we need to read the mode strap pins). So unless there's a simple solution to the regmap problem, I think moving this driver part to drivers/power/reset is the most viable option. Anyway, thanks for the syscon hint! I think it'll be useful for reset on R-Car Gen2. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Arnd, On Thu, Dec 4, 2014 at 12:47 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote: >> This clearly looks much better than the previous implementation, so that >> is great. If you can decouple it from the rest of this file, either through >> syscon or by asserting that sysc_base2 is not going to be needed for >> anything else, how about moving the new code to a standalon driver in >> drivers/power/reset? > > Yes, sysc_base2 is used for reset control only (I don't think we need to read > the mode strap pins). > > So unless there's a simple solution to the regmap problem, I think moving > this driver part to drivers/power/reset is the most viable option. There's another reasone for having a separate driver: if both the SH core and the ARM core are in use, the second register bank can only be accessed after acquiring the HPB (Peripheral Bus Bridge) semaphore. While we don't actively support running anything on the SH core, we don't want to prevent users from doing that. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/mach-shmobile/pm-rmobile.c b/arch/arm/mach-shmobile/pm-rmobile.c index f540096c05f6eabd..b82da04c7210a98d 100644 --- a/arch/arm/mach-shmobile/pm-rmobile.c +++ b/arch/arm/mach-shmobile/pm-rmobile.c @@ -23,14 +23,22 @@ #include <linux/slab.h> #include <asm/io.h> +#include <asm/system_misc.h> #include "pm-rmobile.h" -/* SYSC */ +/* SYSC Register Bank 1 */ #define SPDCR 0x08 /* SYS Power Down Control Register */ #define SWUCR 0x14 /* SYS Wakeup Control Register */ #define PSTR 0x80 /* Power Status Register */ +/* SYSC Register Bank 2 */ +#define RESCNT2 0x20 /* Reset Control Register 2 */ + +/* Reset Control Register 2 */ +#define RESCNT2_PRES 0x80000000 /* Soft power-on reset */ + + #define PSTR_RETRIES 100 #define PSTR_DELAY_US 10 @@ -400,3 +408,41 @@ static int __init rmobile_init_pm_domains(void) core_initcall(rmobile_init_pm_domains); #endif /* !CONFIG_ARCH_SHMOBILE_LEGACY */ + + +static void __iomem *sysc_base2; + +static void rmobile_sysc_restart(enum reboot_mode reboot_mode, const char *cmd) +{ + pr_debug("%s %u/%s\n", __func__, reboot_mode, cmd); + + writel(RESCNT2_PRES, sysc_base2 + RESCNT2); +} + +static int rmobile_sysc_probe(struct platform_device *pdev) +{ + dev_dbg(&pdev->dev, "%s\n", __func__); + + sysc_base2 = of_iomap(pdev->dev.of_node, 1); + if (!sysc_base2) + return -ENODEV; + + arm_pm_restart = rmobile_sysc_restart; + + return 0; +} + +static const struct of_device_id rmobile_sysc_of_match[] = { + { .compatible = "renesas,sysc-rmobile", }, + { /* sentinel */ } +}; + +static struct platform_driver rmobile_sysc_driver = { + .probe = rmobile_sysc_probe, + .driver = { + .name = "rmobile_sysc", + .of_match_table = rmobile_sysc_of_match, + }, +}; + +module_platform_driver(rmobile_sysc_driver);
Extend the PM domain platform driver for the R-Mobile System Controller (SYSC) to register a restart handler, to be used on various Renesas ARM SoCs (e.g. R-Mobile A1 (r8a7740) and APE6 (r8a73a4), and SH-Mobile AP4 (sh7372) and AG5 (sh73a0)). Note that this supports DT-based platforms only. Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- arch/arm/mach-shmobile/pm-rmobile.c | 48 ++++++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-)