Re: [PATCH 3/8 v2] power: reset: driver for the Versatile syscon reboot

Arnd Bergmann
2014-09-02T12:32:23Z

On Tuesday 02 September 2014 14:02:29 Linus Walleij wrote:
> +enum versatile_reboot {
> + REALVIEW_REBOOT_EB,
> + REALVIEW_REBOOT_PB1176,
> + REALVIEW_REBOOT_PB11MP,
> + REALVIEW_REBOOT_PBA8,
> + REALVIEW_REBOOT_PBX,
> +};
> +static const struct of_device_id versatile_reboot_of_match[] = {
> + {
> + .compatible = "arm,realview-eb-syscon",
> + .data = (void *)REALVIEW_REBOOT_EB,
> + },
...
> +};
> +
> +static void versatile_reboot(enum reboot_mode mode, const char *cmd)
> +{
> + /* Unlock the reset register */
> + regmap_write(syscon_regmap, REALVIEW_SYS_LOCK_OFFSET,
> + REALVIEW_SYS_LOCK_VAL);
> + /* Then hit reset on the different machines */
> + switch (versatile_reboot_type) {
> + case REALVIEW_REBOOT_EB:
> + regmap_write(syscon_regmap,
> + REALVIEW_SYS_RESETCTL_OFFSET, 0x0008);
> + break;
> ...
> + }
> + dsb();

Looks ok to me. Just an idea for further simplification, we could easily encode
the writes into the .data field:

{
.compatible = "arm,realview-eb-syscon",
.data = (void *)0x00080000,
},
...
{
.compatible = "arm,realview-pbx-syscon",
.data = (void *)0x00f000f4,
},

static void versatile_reboot(enum reboot_mode mode, const char *cmd)
{

/* Unlock the reset register */
regmap_write(syscon_regmap, REALVIEW_SYS_LOCK_OFFSET,
REALVIEW_SYS_LOCK_VAL);

regmap_write(syscon_regmap, REALVIEW_SYS_RESETCTL_OFFSET,
versatile_reboot_type >> 16);

if (versatile_reboot_type & 0xffff)
regmap_write(syscon_regmap, REALVIEW_SYS_RESETCTL_OFFSET,
versatile_reboot_type >> 16);

dsb();
}

It's definitely shorter, you can decide if you like it better or not,
and add my 'Acked-by: Arnd Bergmann <arnd@arndb.de>' whichever you choose.

Arnd