Hi Benjamin,
On Sat, Jan 7, 2023 at 8:22 AM Szőke Kálmán Benjamin
<egyszer...@freemail.hu> wrote:
>
> Original code was a nested if statement for checking 1g() and 2g().
> https://github.com/TechNexion/u-boot-tn-imx/blob/tn-imx_v2021.04_5.10.72_2.2.0-stable/board/technexion/pico-imx7d/pico-imx7d_spl.c#L106
Yes, I saw that, but it doesn't look good. We can't blindly take the
vendor code as-is and throw it to mainline.
When I added the is_1g() function there was only the 512MB or 1GB
variants, so the function name was meaningful.
After adding the 2GB variant, the is_1g() function no longer means
that the board has 1GB of RAM.
If you want to keep TechNexion logic, at least rename is_1g() to
is_gpio1_12_low()
and is_2g() to is_gpio1_13_high().
> Do you have any information about the PCB layout of the different i.MX7 pico
> SoM revisions?Question is, what is the electrical states of GPIO1_12 and
> GPIO1_13 in real for these variants? My fear is that maybe one of them is not
> connected to any VDD (high) or GND (low). Question can be, what is the
> default logical state for it high or low if one of them is not connected
> neither to GND or VDD?
>
> I totaly do not understand why you need to completly refactoring the code if
> you have no posibbilites to test it with all revision of SoMs. I rather
> prefere to keep the original if statement code from Technexion, because that
> should works 100% and tested. I have no any 2GB varians SoMs yet, also i can
> not test it yet.
The code below really bothers me:
static void ddr_init(void)
{
if (is_1g()) {
if (is_2g()) {
"If the board has 1GB of RAM, then check if it has 2GB of RAM"
If you want to replace it with is_gpio1_12_low() and
is_gpio1_13_high(), as suggested above, I would accept it.
Also, you submitted the patch without even testing it. That's not good.
The pico-imx7d board was not even booting at all. I fixed it recently by:
https://source.denx.de/u-boot/u-boot/-/commit/f8548ce0e09385926574283b17af6c2cd4e32af2
and now you say that you also don't have the 2GB variant.
Please make sure to test your patches against U-Boot mainline.
