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.