Hi,
On 18.05.25 19:31, Johannes Roith wrote:
> Am Fri, May 16, 2025 at 10:05:49PM +0200 schrieb Ahmad Fatoum:
>>> +static void cora_z7_ps7_init(void)
>>> +{
>>> + /*
>>> + * Read OCM mapping configuration, if only the upper 64 KByte are
>>> + * mapped to the high address, it's very likely that we just got control
>>> + * from the BootROM. If the mapping is changed something other than the
>>> + * BootROM was running before us. Skip PS7 init to avoid cutting the
>>> + * branch we are sitting on in that case.
>>> + */
>>> + if ((readl(0xf8000910) & 0xf) != 0x8)
>>
>> Please use a macro for the address and wrap it in IOMEM().
>> Eventually, we will want to enforce addresses to be pointers, so it would
>> be cool for new code to already use the correct types. If you want type
>> checking, just add
>>
>> #include <linux/io.h> at the top of your includes.
>>
>
> So, it should look like this, right:
>
> void __iomem *ocm_config = IOMEM(OCM_CFG);
Or just #define OCM_CFG IOMEM(0xf8000910).
> Hmm, good question. In theory most of the code is generic or could be
> implemted in functions in mach-zynq and the lowlevel board just calls
> these functions with the correct arguments, e.g. the DDR init. But
> that way it is quite time consuming to implement. This here is basically
> just copy paste from a Vitis generated code adopted to the coding style of
> barebox.
Ok. If the code is generated anyways, then it's fine to duplicate it IMO.
You could also keep the generated code in a separate file and define
some macros to make changes to it minimal, but this is fine too.
>>> +config MACH_CORA_Z7
>>> + bool "Digilent Cora Z7"
>>> + select ARCH_ZYNQ7000
>>
>> Can you enable this in zynq_defconfig?
>
> Yes, it is selectable in the config.
Add it to the defconfigs (arch/arm/configs) that enable CONFIG_MACH_ZEDBOARD.
Cheers,
Ahmad
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
