On Thu, Sep 22, 2022 at 9:40 AM Pali Rohár pali@kernel.org wrote:
On Thursday 22 September 2022 09:25:37 Chris Packham wrote:
On Wed, Sep 21, 2022 at 5:58 PM Stefan Roese sr@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@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.
Agreed. The fact that I can turn the driver off and (after fixing some code that calls into it directly) everything still works (except for the reporting of the clock speeds) it does indicate that I don't really need this.
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 :-(
Not boring but it doesn't help that Marvell don't document the SAR registers so I can only go by following their overly complicated code.
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.
I think I can come up with something that replaces the sar-uclass business with code that just accesses the relevant register(s) directly in the alleycat5/soc.c code.
[1] - Btw, I added this print in A3720 secure firmware, because it is really useful and important (!) and secure firmware is who configures it