[u-boot][master][PATCH 1/3] pico-imx7d: add support for 2GB memory SoMs

[Szőke Kálmán Benjamin]

My last three patches were tested in my company's custom i.MX7D pico SoMs carrier board, it was workd well with 512MB and 1 GB SoM variants in U-boot v2022.10. I have no any 2GB variants but i can trust in Technexion that it was tested in their house, bedore.But it can be better if you take contact with them and request a confirmation about your swtich case logic, is it will be fine or not for them.https://github.com/richard-huhttps://github.com/JoeZhang-tn@Tom RiniIs there any plan to improve the patches, pull-requests, technical review/threads processes to use a better web-technnolgies instead of to use infinity long-long ping-pong mailing in a legacy mailing lists? In 2023, It's time to use something better, for example to use a normal pull request features on GitHub/GitLab website for U-boot as every modern open-source project use it. It could be more traceabla and faster in ask a question for a developer, moreover the code reviewing is much much better in this modern way.https://github.com/u-boot/u-boot/pulls-------- Eredeti levél --------Feladó: Fabio Estevam <festevam at gmail.com>Dátum: 2023 január 7 14:37:47Tárgy: Re: [u-boot][master][PATCH 1/3] pico-imx7d: add support for 2GB memory SoMsCímzett: Szőke Kálmán Benjamin <egyszeregy at freemail.hu>Hi Benjamin, On Sat, Jan 7, 2023 at 8:22 AM Szőke Kálmán Benjamin <egyszeregy at 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.