[PATCH v3 5/6] arm: mvebu: Support for 98DX25xx/98DX35xx SoC

[Pali Rohár]

On Thursday 22 September 2022 09:25:37 Chris Packham wrote:
> On Wed, Sep 21, 2022 at 5:58 PM Stefan Roese <sr at denx.de> wrote:
> >
> > On 21.09.22 06:59, Chris Packham wrote:
> > > Add support for the Allecat5/Alleycat5X SoC. These are L3 switches with
> > > an integrated CPU (referred to as the CnM block in Marvell's
> > > documentation). These have dual ARMv8.2 CPUs (Cortex-A55). This support
> > > has been ported from Marvell's SDK which is based on a much older
> > > version of U-Boot.
> > >
> > > Signed-off-by: Chris Packham <judge.packham at gmail.com>
> > > ---
> > >
> 
> <snip>
> 
> > > diff --git a/arch/arm/mach-mvebu/alleycat5/soc.c b/arch/arm/mach-mvebu/alleycat5/soc.c
> > > new file mode 100644
> > > index 0000000000..f388d4ee40
> > > --- /dev/null
> > > +++ b/arch/arm/mach-mvebu/alleycat5/soc.c
> 
> <snip>
> 
> > > +int soc_early_init_f(void)
> > > +{
> > > +#ifdef CONFIG_MVEBU_SAR
> > > +/* Sample at reset register init */
> > > +     mvebu_sar_init();
> > > +#endif
> >
> > Won't CONFIG_MVEBU_SAR always be enabled? Remove the #ifdef in this
> > case.
> >
> > > +     return 0;
> > > +}
> 
> Currently it is possible to turn it off. As I've said I think I do
> need to look at the whole SAR business and see if it can be done
> differently.
> 
> One useful thing it does do is tell me about how the hardware has been
> strapped. U-Boot itself doesn't care about that specifically as the
> separate mv_ddr blob that runs before ATF is the thing that actually
> uses the SAR values. But U-
> Boot is the first place that can give me some friendly output about
> the board and knowing the CPU/DDR clock speed is useful at that level.

I agree that the last thing - print information about CPU/DDR clock
speed is useful [1]. But I do not see reason why to complicate it such
much via new uclass OOP model for other things in DTS, which moreover
use already deprecated structure and requires hack in fdtdec.c file.

I think that the whole SAR code can be simplified and written
straightforward for AC5 to be easier for developer to read... but this
step is not probably such simple as it is required to first understand
what is this "complicated" code doing and it is probably boring job :-(

For 32-bit Armada there is already very simple SAR register handling to
check and detect TCLK speed. So I think AC5 is similar in this area, but
more has more complicated code logic and "very simple 32-bit SAR" is not
suitable to reuse.


[1] - Btw, I added this print in A3720 secure firmware, because it is
really useful and important (!) and secure firmware is who configures it