[U-Boot,v3,4/8] reset: socfpga: add reset handling for old kernels

This adds code to take peripherals out of reset based on an environment
variable. This is in preparation for removing the code that does this from
SPL.

However, some drivers even in current Linux cannot handle peripheral reset,
so until this works, we need a compatibility workaround.

This workaround is implemented in the 'assert' and 'remove' callbacks of
this reset driver: the 'assert' callback does not disable peripherals that
were already taken out of reset, while the 'remove' callback, which is
called on OS_PREPARE, deasserts all peripheral resets if the environment
variable "socfpga_legacy_reset_compat" is set to 1, which is what the gen5
SPL did up to now.

This is in preparation to clean up the SPL and implementing proper reset
handling for U-Boot.

Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---

Changes in v3:
- fix falcon mode in SPL should work, too
- change env var name "socfpga_permodrst_ungate" to
  "socfpga_legacy_reset_compat"
- in compat mode, don't reset peripherals once they are enabled

Changes in v2:
- moved from Kernel option "OLD_SOCFPGA_KERNEL_COMPAT" to environment
  variable "socfpga_permodrst_ungate"

 drivers/reset/reset-socfpga.c | 44 +++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

On 2/26/19 9:31 PM, Simon Goldschmidt wrote:
> This adds code to take peripherals out of reset based on an environment
> variable. This is in preparation for removing the code that does this from
> SPL.
> 
> However, some drivers even in current Linux cannot handle peripheral reset,
> so until this works, we need a compatibility workaround.
> 
> This workaround is implemented in the 'assert' and 'remove' callbacks of
> this reset driver: the 'assert' callback does not disable peripherals that
> were already taken out of reset, while the 'remove' callback, which is
> called on OS_PREPARE, deasserts all peripheral resets if the environment
> variable "socfpga_legacy_reset_compat" is set to 1, which is what the gen5
> SPL did up to now.
> 
> This is in preparation to clean up the SPL and implementing proper reset
> handling for U-Boot.
> 
> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> ---
> 
> Changes in v3:
> - fix falcon mode in SPL should work, too
> - change env var name "socfpga_permodrst_ungate" to
>   "socfpga_legacy_reset_compat"
> - in compat mode, don't reset peripherals once they are enabled
> 
> Changes in v2:
> - moved from Kernel option "OLD_SOCFPGA_KERNEL_COMPAT" to environment
>   variable "socfpga_permodrst_ungate"
> 
>  drivers/reset/reset-socfpga.c | 44 +++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/drivers/reset/reset-socfpga.c b/drivers/reset/reset-socfpga.c
> index b2acfcd2ec..39d0b9e8f2 100644
> --- a/drivers/reset/reset-socfpga.c
> +++ b/drivers/reset/reset-socfpga.c
> @@ -27,6 +27,36 @@ struct socfpga_reset_data {
>  	void __iomem *membase;
>  };
>  
> +/*
> + * For compatibility with Kernels that don't support peripheral reset, this
> + * driver can keep the old behaviour of not asserting peripheral reset before
> + * starting the OS and deasserting all peripheral resets (enabling all
> + * peripherals).
> + *
> + * For that, the reset driver checks the environment variable
> + * "socfpga_legacy_reset_compat". If this variable is '1', perihperals are not
> + * reset again once taken out of reset and all peripherals in 'permodrst' are
> + * taken out of reset before booting into the OS.
> + * Note that this should be required for gen5 systems only that are running
> + * Linux kernels without proper peripheral reset support for all drivers used.
> + */
> +static bool socfpga_reset_keep_enabled(void)
> +{
> +#if !defined(CONFIG_SPL_BUILD) || CONFIG_IS_ENABLED(ENV_SUPPORT)
> +	const char *env_str;
> +	long val;
> +
> +	env_str = env_get("socfpga_legacy_reset_compat");
> +	if (env_str) {
> +		val = simple_strtol(env_str, NULL, 0);
> +		if (val == 1)
> +			return true;
> +	}
> +#endif
> +
> +	return false;
> +}
> +
>  static int socfpga_reset_assert(struct reset_ctl *reset_ctl)
>  {
>  	struct socfpga_reset_data *data = dev_get_priv(reset_ctl->dev);
> @@ -89,6 +119,18 @@ static int socfpga_reset_probe(struct udevice *dev)
>  	return 0;
>  }
>  
> +static int socfpga_reset_remove(struct udevice *dev)
> +{
> +	struct socfpga_reset_data *data = dev_get_priv(dev);
> +
> +	if (socfpga_reset_keep_enabled()) {
> +		puts("Deasserting all peripheral resets\n");
> +		writel(0, data->membase + 4);

Isn't permodreset at +0x14 ?

> +	}
> +
> +	return 0;
> +}
> +
>  static const struct udevice_id socfpga_reset_match[] = {
>  	{ .compatible = "altr,rst-mgr" },
>  	{ /* sentinel */ },
> @@ -101,4 +143,6 @@ U_BOOT_DRIVER(socfpga_reset) = {
>  	.probe = socfpga_reset_probe,
>  	.priv_auto_alloc_size = sizeof(struct socfpga_reset_data),
>  	.ops = &socfpga_reset_ops,
> +	.remove = socfpga_reset_remove,
> +	.flags	= DM_FLAG_OS_PREPARE,
>  };
>

On Fri, Mar 1, 2019 at 1:00 PM Marek Vasut <marex@denx.de> wrote:
>
> On 2/26/19 9:31 PM, Simon Goldschmidt wrote:
> > This adds code to take peripherals out of reset based on an environment
> > variable. This is in preparation for removing the code that does this from
> > SPL.
> >
> > However, some drivers even in current Linux cannot handle peripheral reset,
> > so until this works, we need a compatibility workaround.
> >
> > This workaround is implemented in the 'assert' and 'remove' callbacks of
> > this reset driver: the 'assert' callback does not disable peripherals that
> > were already taken out of reset, while the 'remove' callback, which is
> > called on OS_PREPARE, deasserts all peripheral resets if the environment
> > variable "socfpga_legacy_reset_compat" is set to 1, which is what the gen5
> > SPL did up to now.
> >
> > This is in preparation to clean up the SPL and implementing proper reset
> > handling for U-Boot.
> >
> > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> > ---
> >
> > Changes in v3:
> > - fix falcon mode in SPL should work, too
> > - change env var name "socfpga_permodrst_ungate" to
> >   "socfpga_legacy_reset_compat"
> > - in compat mode, don't reset peripherals once they are enabled
> >
> > Changes in v2:
> > - moved from Kernel option "OLD_SOCFPGA_KERNEL_COMPAT" to environment
> >   variable "socfpga_permodrst_ungate"
> >
> >  drivers/reset/reset-socfpga.c | 44 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 44 insertions(+)
> >
> > diff --git a/drivers/reset/reset-socfpga.c b/drivers/reset/reset-socfpga.c
> > index b2acfcd2ec..39d0b9e8f2 100644
> > --- a/drivers/reset/reset-socfpga.c
> > +++ b/drivers/reset/reset-socfpga.c
> > @@ -27,6 +27,36 @@ struct socfpga_reset_data {
> >       void __iomem *membase;
> >  };
> >
> > +/*
> > + * For compatibility with Kernels that don't support peripheral reset, this
> > + * driver can keep the old behaviour of not asserting peripheral reset before
> > + * starting the OS and deasserting all peripheral resets (enabling all
> > + * peripherals).
> > + *
> > + * For that, the reset driver checks the environment variable
> > + * "socfpga_legacy_reset_compat". If this variable is '1', perihperals are not
> > + * reset again once taken out of reset and all peripherals in 'permodrst' are
> > + * taken out of reset before booting into the OS.
> > + * Note that this should be required for gen5 systems only that are running
> > + * Linux kernels without proper peripheral reset support for all drivers used.
> > + */
> > +static bool socfpga_reset_keep_enabled(void)
> > +{
> > +#if !defined(CONFIG_SPL_BUILD) || CONFIG_IS_ENABLED(ENV_SUPPORT)
> > +     const char *env_str;
> > +     long val;
> > +
> > +     env_str = env_get("socfpga_legacy_reset_compat");
> > +     if (env_str) {
> > +             val = simple_strtol(env_str, NULL, 0);
> > +             if (val == 1)
> > +                     return true;
> > +     }
> > +#endif
> > +
> > +     return false;
> > +}
> > +
> >  static int socfpga_reset_assert(struct reset_ctl *reset_ctl)
> >  {
> >       struct socfpga_reset_data *data = dev_get_priv(reset_ctl->dev);
> > @@ -89,6 +119,18 @@ static int socfpga_reset_probe(struct udevice *dev)
> >       return 0;
> >  }
> >
> > +static int socfpga_reset_remove(struct udevice *dev)
> > +{
> > +     struct socfpga_reset_data *data = dev_get_priv(dev);
> > +
> > +     if (socfpga_reset_keep_enabled()) {
> > +             puts("Deasserting all peripheral resets\n");
> > +             writel(0, data->membase + 4);
>
> Isn't permodreset at +0x14 ?

Yes it is. However, data->membase does not point to the actual base
address of the rstmgr but to the start of the "modrst" register group.

And permodreset is at offset 4 in this view.

While this looks a bit odd, it ensures the driver can be used for gen5
and a10 (and proably for s10 as well).

Regards,
Simon

>
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> >  static const struct udevice_id socfpga_reset_match[] = {
> >       { .compatible = "altr,rst-mgr" },
> >       { /* sentinel */ },
> > @@ -101,4 +143,6 @@ U_BOOT_DRIVER(socfpga_reset) = {
> >       .probe = socfpga_reset_probe,
> >       .priv_auto_alloc_size = sizeof(struct socfpga_reset_data),
> >       .ops = &socfpga_reset_ops,
> > +     .remove = socfpga_reset_remove,
> > +     .flags  = DM_FLAG_OS_PREPARE,
> >  };
> >
>
>
> --
> Best regards,
> Marek Vasut

On 3/1/19 1:14 PM, Simon Goldschmidt wrote:
> On Fri, Mar 1, 2019 at 1:00 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 2/26/19 9:31 PM, Simon Goldschmidt wrote:
>>> This adds code to take peripherals out of reset based on an environment
>>> variable. This is in preparation for removing the code that does this from
>>> SPL.
>>>
>>> However, some drivers even in current Linux cannot handle peripheral reset,
>>> so until this works, we need a compatibility workaround.
>>>
>>> This workaround is implemented in the 'assert' and 'remove' callbacks of
>>> this reset driver: the 'assert' callback does not disable peripherals that
>>> were already taken out of reset, while the 'remove' callback, which is
>>> called on OS_PREPARE, deasserts all peripheral resets if the environment
>>> variable "socfpga_legacy_reset_compat" is set to 1, which is what the gen5
>>> SPL did up to now.
>>>
>>> This is in preparation to clean up the SPL and implementing proper reset
>>> handling for U-Boot.
>>>
>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>>> ---
>>>
>>> Changes in v3:
>>> - fix falcon mode in SPL should work, too
>>> - change env var name "socfpga_permodrst_ungate" to
>>>   "socfpga_legacy_reset_compat"
>>> - in compat mode, don't reset peripherals once they are enabled
>>>
>>> Changes in v2:
>>> - moved from Kernel option "OLD_SOCFPGA_KERNEL_COMPAT" to environment
>>>   variable "socfpga_permodrst_ungate"
>>>
>>>  drivers/reset/reset-socfpga.c | 44 +++++++++++++++++++++++++++++++++++
>>>  1 file changed, 44 insertions(+)
>>>
>>> diff --git a/drivers/reset/reset-socfpga.c b/drivers/reset/reset-socfpga.c
>>> index b2acfcd2ec..39d0b9e8f2 100644
>>> --- a/drivers/reset/reset-socfpga.c
>>> +++ b/drivers/reset/reset-socfpga.c
>>> @@ -27,6 +27,36 @@ struct socfpga_reset_data {
>>>       void __iomem *membase;
>>>  };
>>>
>>> +/*
>>> + * For compatibility with Kernels that don't support peripheral reset, this
>>> + * driver can keep the old behaviour of not asserting peripheral reset before
>>> + * starting the OS and deasserting all peripheral resets (enabling all
>>> + * peripherals).
>>> + *
>>> + * For that, the reset driver checks the environment variable
>>> + * "socfpga_legacy_reset_compat". If this variable is '1', perihperals are not
>>> + * reset again once taken out of reset and all peripherals in 'permodrst' are
>>> + * taken out of reset before booting into the OS.
>>> + * Note that this should be required for gen5 systems only that are running
>>> + * Linux kernels without proper peripheral reset support for all drivers used.
>>> + */
>>> +static bool socfpga_reset_keep_enabled(void)
>>> +{
>>> +#if !defined(CONFIG_SPL_BUILD) || CONFIG_IS_ENABLED(ENV_SUPPORT)
>>> +     const char *env_str;
>>> +     long val;
>>> +
>>> +     env_str = env_get("socfpga_legacy_reset_compat");
>>> +     if (env_str) {
>>> +             val = simple_strtol(env_str, NULL, 0);
>>> +             if (val == 1)
>>> +                     return true;
>>> +     }
>>> +#endif
>>> +
>>> +     return false;
>>> +}
>>> +
>>>  static int socfpga_reset_assert(struct reset_ctl *reset_ctl)
>>>  {
>>>       struct socfpga_reset_data *data = dev_get_priv(reset_ctl->dev);
>>> @@ -89,6 +119,18 @@ static int socfpga_reset_probe(struct udevice *dev)
>>>       return 0;
>>>  }
>>>
>>> +static int socfpga_reset_remove(struct udevice *dev)
>>> +{
>>> +     struct socfpga_reset_data *data = dev_get_priv(dev);
>>> +
>>> +     if (socfpga_reset_keep_enabled()) {
>>> +             puts("Deasserting all peripheral resets\n");
>>> +             writel(0, data->membase + 4);
>>
>> Isn't permodreset at +0x14 ?
> 
> Yes it is. However, data->membase does not point to the actual base
> address of the rstmgr but to the start of the "modrst" register group.
> 
> And permodreset is at offset 4 in this view.
> 
> While this looks a bit odd, it ensures the driver can be used for gen5
> and a10 (and proably for s10 as well).
> 

Some comment explaining this would be nice :)

On Fri, Mar 1, 2019 at 1:15 PM Marek Vasut <marex@denx.de> wrote:
>
> On 3/1/19 1:14 PM, Simon Goldschmidt wrote:
> > On Fri, Mar 1, 2019 at 1:00 PM Marek Vasut <marex@denx.de> wrote:
> >>
> >> On 2/26/19 9:31 PM, Simon Goldschmidt wrote:
> >>> This adds code to take peripherals out of reset based on an environment
> >>> variable. This is in preparation for removing the code that does this from
> >>> SPL.
> >>>
> >>> However, some drivers even in current Linux cannot handle peripheral reset,
> >>> so until this works, we need a compatibility workaround.
> >>>
> >>> This workaround is implemented in the 'assert' and 'remove' callbacks of
> >>> this reset driver: the 'assert' callback does not disable peripherals that
> >>> were already taken out of reset, while the 'remove' callback, which is
> >>> called on OS_PREPARE, deasserts all peripheral resets if the environment
> >>> variable "socfpga_legacy_reset_compat" is set to 1, which is what the gen5
> >>> SPL did up to now.
> >>>
> >>> This is in preparation to clean up the SPL and implementing proper reset
> >>> handling for U-Boot.
> >>>
> >>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> >>> ---
> >>>
> >>> Changes in v3:
> >>> - fix falcon mode in SPL should work, too
> >>> - change env var name "socfpga_permodrst_ungate" to
> >>>   "socfpga_legacy_reset_compat"
> >>> - in compat mode, don't reset peripherals once they are enabled
> >>>
> >>> Changes in v2:
> >>> - moved from Kernel option "OLD_SOCFPGA_KERNEL_COMPAT" to environment
> >>>   variable "socfpga_permodrst_ungate"
> >>>
> >>>  drivers/reset/reset-socfpga.c | 44 +++++++++++++++++++++++++++++++++++
> >>>  1 file changed, 44 insertions(+)
> >>>
> >>> diff --git a/drivers/reset/reset-socfpga.c b/drivers/reset/reset-socfpga.c
> >>> index b2acfcd2ec..39d0b9e8f2 100644
> >>> --- a/drivers/reset/reset-socfpga.c
> >>> +++ b/drivers/reset/reset-socfpga.c
> >>> @@ -27,6 +27,36 @@ struct socfpga_reset_data {
> >>>       void __iomem *membase;
> >>>  };
> >>>
> >>> +/*
> >>> + * For compatibility with Kernels that don't support peripheral reset, this
> >>> + * driver can keep the old behaviour of not asserting peripheral reset before
> >>> + * starting the OS and deasserting all peripheral resets (enabling all
> >>> + * peripherals).
> >>> + *
> >>> + * For that, the reset driver checks the environment variable
> >>> + * "socfpga_legacy_reset_compat". If this variable is '1', perihperals are not
> >>> + * reset again once taken out of reset and all peripherals in 'permodrst' are
> >>> + * taken out of reset before booting into the OS.
> >>> + * Note that this should be required for gen5 systems only that are running
> >>> + * Linux kernels without proper peripheral reset support for all drivers used.
> >>> + */
> >>> +static bool socfpga_reset_keep_enabled(void)
> >>> +{
> >>> +#if !defined(CONFIG_SPL_BUILD) || CONFIG_IS_ENABLED(ENV_SUPPORT)
> >>> +     const char *env_str;
> >>> +     long val;
> >>> +
> >>> +     env_str = env_get("socfpga_legacy_reset_compat");
> >>> +     if (env_str) {
> >>> +             val = simple_strtol(env_str, NULL, 0);
> >>> +             if (val == 1)
> >>> +                     return true;
> >>> +     }
> >>> +#endif
> >>> +
> >>> +     return false;
> >>> +}
> >>> +
> >>>  static int socfpga_reset_assert(struct reset_ctl *reset_ctl)
> >>>  {
> >>>       struct socfpga_reset_data *data = dev_get_priv(reset_ctl->dev);
> >>> @@ -89,6 +119,18 @@ static int socfpga_reset_probe(struct udevice *dev)
> >>>       return 0;
> >>>  }
> >>>
> >>> +static int socfpga_reset_remove(struct udevice *dev)
> >>> +{
> >>> +     struct socfpga_reset_data *data = dev_get_priv(dev);
> >>> +
> >>> +     if (socfpga_reset_keep_enabled()) {
> >>> +             puts("Deasserting all peripheral resets\n");
> >>> +             writel(0, data->membase + 4);
> >>
> >> Isn't permodreset at +0x14 ?
> >
> > Yes it is. However, data->membase does not point to the actual base
> > address of the rstmgr but to the start of the "modrst" register group.
> >
> > And permodreset is at offset 4 in this view.
> >
> > While this looks a bit odd, it ensures the driver can be used for gen5
> > and a10 (and proably for s10 as well).
> >
>
> Some comment explaining this would be nice :)

Well, I guess you're right there. I found that odd myself when starting
to read the driver. But then I just ended copying the code from the other
callbacks.

It would probably be a good idea to rename 'membase' to something
more appropriate given its usage...

So I'll send v4 soon ;-)

Regards,
Simon

On 3/1/19 1:26 PM, Simon Goldschmidt wrote:
> On Fri, Mar 1, 2019 at 1:15 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 3/1/19 1:14 PM, Simon Goldschmidt wrote:
>>> On Fri, Mar 1, 2019 at 1:00 PM Marek Vasut <marex@denx.de> wrote:
>>>>
>>>> On 2/26/19 9:31 PM, Simon Goldschmidt wrote:
>>>>> This adds code to take peripherals out of reset based on an environment
>>>>> variable. This is in preparation for removing the code that does this from
>>>>> SPL.
>>>>>
>>>>> However, some drivers even in current Linux cannot handle peripheral reset,
>>>>> so until this works, we need a compatibility workaround.
>>>>>
>>>>> This workaround is implemented in the 'assert' and 'remove' callbacks of
>>>>> this reset driver: the 'assert' callback does not disable peripherals that
>>>>> were already taken out of reset, while the 'remove' callback, which is
>>>>> called on OS_PREPARE, deasserts all peripheral resets if the environment
>>>>> variable "socfpga_legacy_reset_compat" is set to 1, which is what the gen5
>>>>> SPL did up to now.
>>>>>
>>>>> This is in preparation to clean up the SPL and implementing proper reset
>>>>> handling for U-Boot.
>>>>>
>>>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>>>>> ---
>>>>>
>>>>> Changes in v3:
>>>>> - fix falcon mode in SPL should work, too
>>>>> - change env var name "socfpga_permodrst_ungate" to
>>>>>   "socfpga_legacy_reset_compat"
>>>>> - in compat mode, don't reset peripherals once they are enabled
>>>>>
>>>>> Changes in v2:
>>>>> - moved from Kernel option "OLD_SOCFPGA_KERNEL_COMPAT" to environment
>>>>>   variable "socfpga_permodrst_ungate"
>>>>>
>>>>>  drivers/reset/reset-socfpga.c | 44 +++++++++++++++++++++++++++++++++++
>>>>>  1 file changed, 44 insertions(+)
>>>>>
>>>>> diff --git a/drivers/reset/reset-socfpga.c b/drivers/reset/reset-socfpga.c
>>>>> index b2acfcd2ec..39d0b9e8f2 100644
>>>>> --- a/drivers/reset/reset-socfpga.c
>>>>> +++ b/drivers/reset/reset-socfpga.c
>>>>> @@ -27,6 +27,36 @@ struct socfpga_reset_data {
>>>>>       void __iomem *membase;
>>>>>  };
>>>>>
>>>>> +/*
>>>>> + * For compatibility with Kernels that don't support peripheral reset, this
>>>>> + * driver can keep the old behaviour of not asserting peripheral reset before
>>>>> + * starting the OS and deasserting all peripheral resets (enabling all
>>>>> + * peripherals).
>>>>> + *
>>>>> + * For that, the reset driver checks the environment variable
>>>>> + * "socfpga_legacy_reset_compat". If this variable is '1', perihperals are not
>>>>> + * reset again once taken out of reset and all peripherals in 'permodrst' are
>>>>> + * taken out of reset before booting into the OS.
>>>>> + * Note that this should be required for gen5 systems only that are running
>>>>> + * Linux kernels without proper peripheral reset support for all drivers used.
>>>>> + */
>>>>> +static bool socfpga_reset_keep_enabled(void)
>>>>> +{
>>>>> +#if !defined(CONFIG_SPL_BUILD) || CONFIG_IS_ENABLED(ENV_SUPPORT)
>>>>> +     const char *env_str;
>>>>> +     long val;
>>>>> +
>>>>> +     env_str = env_get("socfpga_legacy_reset_compat");
>>>>> +     if (env_str) {
>>>>> +             val = simple_strtol(env_str, NULL, 0);
>>>>> +             if (val == 1)
>>>>> +                     return true;
>>>>> +     }
>>>>> +#endif
>>>>> +
>>>>> +     return false;
>>>>> +}
>>>>> +
>>>>>  static int socfpga_reset_assert(struct reset_ctl *reset_ctl)
>>>>>  {
>>>>>       struct socfpga_reset_data *data = dev_get_priv(reset_ctl->dev);
>>>>> @@ -89,6 +119,18 @@ static int socfpga_reset_probe(struct udevice *dev)
>>>>>       return 0;
>>>>>  }
>>>>>
>>>>> +static int socfpga_reset_remove(struct udevice *dev)
>>>>> +{
>>>>> +     struct socfpga_reset_data *data = dev_get_priv(dev);
>>>>> +
>>>>> +     if (socfpga_reset_keep_enabled()) {
>>>>> +             puts("Deasserting all peripheral resets\n");
>>>>> +             writel(0, data->membase + 4);
>>>>
>>>> Isn't permodreset at +0x14 ?
>>>
>>> Yes it is. However, data->membase does not point to the actual base
>>> address of the rstmgr but to the start of the "modrst" register group.
>>>
>>> And permodreset is at offset 4 in this view.
>>>
>>> While this looks a bit odd, it ensures the driver can be used for gen5
>>> and a10 (and proably for s10 as well).
>>>
>>
>> Some comment explaining this would be nice :)
> 
> Well, I guess you're right there. I found that odd myself when starting
> to read the driver. But then I just ended copying the code from the other
> callbacks.
> 
> It would probably be a good idea to rename 'membase' to something
> more appropriate given its usage...
> 
> So I'll send v4 soon ;-)

Please do, otherwise the patchset is good :)