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

[Chee, Tien Fong <tien.fong.chee@intel.com>]

On Fri, 2019-05-24 at 14:00 +0200, Simon Goldschmidt wrote:
> On Fri, May 24, 2019 at 1:57 PM Chee, Tien Fong
> <tien.fong.chee@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@denx.de>
> > > wrote:
> > > > 
> > > > 
> > > > 
> > > > On 5/24/19 1:40 PM, tien.fong.chee at intel.com wrote:
> > > > > 
> > > > > 
> > > > > From: Tien Fong Chee <tien.fong.chee@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@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...

I have no strong opinion on this, but how old the kernel U-Boot want to
support, subject to the maintainer :) .

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