[U-Boot] [PATCH] reset: socfpga: release more A10 peripherals out of reset

[Simon Goldschmidt]

On Fri, May 24, 2019 at 1:57 PM Chee, Tien Fong
<tien.fong.chee at intel.com> wrote:
>
> On Fri, 2019-05-24 at 13:53 +0200, Simon Goldschmidt wrote:
> > On Fri, May 24, 2019 at 1:44 PM Marek Vasut <marex at denx.de> wrote:
> > >
> > >
> > > On 5/24/19 1:40 PM, tien.fong.chee at intel.com wrote:
> > > >
> > > > From: Tien Fong Chee <tien.fong.chee at intel.com>
> > > >
> > > > Current implementation almost release all peripherals out of
> > > > reset for
> > > > gen5, but A10 has more peripherals than gen5, hence this patch is
> > > > required
> > > > to release the rest of peripherals to support old kernels.
> > > >
> > > > Signed-off-by: Tien Fong Chee <tien.fong.chee at intel.com>
> > > > ---
> > > >  drivers/reset/reset-socfpga.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/drivers/reset/reset-socfpga.c b/drivers/reset/reset-
> > > > socfpga.c
> > > > index cb8312619f..d8b8b25fc3 100644
> > > > --- a/drivers/reset/reset-socfpga.c
> > > > +++ b/drivers/reset/reset-socfpga.c
> > > > @@ -127,6 +127,9 @@ static int socfpga_reset_remove(struct
> > > > udevice *dev)
> > > >       if (socfpga_reset_keep_enabled()) {
> > > >               puts("Deasserting all peripheral resets\n");
> > > >               writel(0, data->modrst_base + 4);
> > > > +#if defined(CONFIG_TARGET_SOCFPGA_ARRIA10)
> > > > +             writel(0, data->modrst_base + 8);
> > > This should be using match on compatible string.
> > > Which register is this modrst_base + 8 ?
> > Well, even modrst_base + 4 is not really ideal. After all,
> > modrst_base is a
> > dts-given offset into the rstmgr's registers, so there is not really
> > a
> > comptile time known constant to do this.
> >
> > Maybe it would be better to use an offset to the device's membase.
> > However, as this is a backwards-compatibility workaround, there's no
> > use to
> > add this into the devicetree.
> >
> > And unfortunately, both gen5 and a10 use the same compatible string,
> > which
> > probably would have to be changed.
> >
> > Regards,
> > Simon
>
> This register is per1modrst, here is the link https://www.intel.com/con
> tent/www/us/en/programmable/hps/arria-
> 10/hps.html#reg_soc_top/sfo1429890576520.html
> This whole mechanism should be removed once Dinh has fixed this issue
> at Linux v4.14-lts or moving to Linux v5.0 onwards.

I agree it can be removed in the future, but is it really enough to have this
in 4.14-lts? I'm not so sure that we should prevent people from using
mainline U-Boot with older kernels...

Regards,
Simon

>
> >
> > >
> > >
> > > >
> > > > +#endif
> > > >       }
> > > >
> > > >       return 0;
> > > >
> > >
> > > --
> > > Best regards,
> > > Marek Vasut