Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for i.MX 8M chipsets #8

Merged
merged 12 commits into from
Jan 31, 2024
Merged

Support for i.MX 8M chipsets #8

merged 12 commits into from
Jan 31, 2024

Conversation

passgat
Copy link
Contributor

@passgat passgat commented Sep 27, 2023

The series adds bootcount support for the i.MX8M platform (last patch).
It also includes rework patches to facilitate the addition of a new platform and two patches to handle the
endianness of the bootcount value.
I have tested the series on the STM32MP1, am33xx (BeagleBone), and i.MX8M platforms.

@passgat
Copy link
Contributor Author

passgat commented Dec 14, 2023

Gentle ping.

Thanks,
Dario

@thom-nic thom-nic changed the title Imx8m Support for i.MX 8M chipsets Dec 14, 2023
@thom-nic
Copy link
Contributor

This looks really great, I appreciate the contribution. I'll give a quick manual test on our end and assuming it all works I'll be happy to merge it.

@thom-nic
Copy link
Contributor

@passgat could you provide:

  • reference links (if available online) to the technical manual docs for the i.MX8 registers being used
  • corresponding u-boot config settings?

I realized I am missing the same for am33 and mp157 but I intend to add that... I have it here somewhere...

i2c_eeprom.c:88:45: warning: format ‘%d’ expects argument of type ‘int’, but argument 3 has type ‘ssize_t’ {aka ‘long int’} [-Wformat=]
   88 |         fprintf(stderr, "Incomplete write: %d bytes!\n", written);
      |                                            ~^            ~~~~~~~
      |                                             |            |
      |                                             int          ssize_t {aka long int}
      |                                            %ld

Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
@passgat
Copy link
Contributor Author

passgat commented Dec 15, 2023

@thom-nic
For i.MX8m:
I added documentation, list of reference manuals, in the file header of imx8m.c.
I added U-Boot configurations (the bootcount address) in the commit message

@passgat
Copy link
Contributor Author

passgat commented Jan 15, 2024

A gentle ping.

Thanks,
Dario

@thom-nic
Copy link
Contributor

@passgat thanks for the reminder. I will try to fit this in soon.

Your contribution looks great of course. I don't have much in the way of feedback w/r/t the i.MX 8 code specifically since I don't have that hardware. I mostly need to build your branch and regression test it for our hw platforms that we use.

@passgat
Copy link
Contributor Author

passgat commented Jan 24, 2024

I added the imx8m platform because I needed the bootcounnt support for it. So I run tests on this platform. I run tests on the beaglebome (am33xx) and the stm32mp157f-dk2 too, so I don't expect regressions. I don't remember If I run test on eeprom, but I don't think so.

Thanks,
Dario

@thom-nic
Copy link
Contributor

That's fine, thank you. I'll do some quick testing on my end so I can merge the PR

Copy link
Contributor

@thom-nic thom-nic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@passgat would you mind replacing the \t characters with spaces? I see it in am33xx.c and bootcount.c

Thank you!

Following the software principle 'Don't repeat yourself,' the patch groups
the source code for register access, where reading/writing the bootcount
value, into a single function with parameters to make it usable across
different architectures.

Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Born as a project to read/write the bootcount on the TI am3xx platform,
over time it has introduced bootcount management on EEPROM and for the
stm32mp1 platform. As a result, the project removed the 'davinci' tag
from its name and GitHub link. The patch fixes references to the old
project name that are still present in some project files.

Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
In case of failure for all supported platforms this was the error message:

No DT node /proc/device-tree/compatible
No DT node /proc/device-tree/compatible
No DT node /proc/device-tree/compatible

Now with:

No DT node /proc/device-tree/compatible while searching for "ti,am33xx"
No DT node /proc/device-tree/compatible while searching for "st,stm32mp153"
No DT node /proc/device-tree/compatible while searching for "st,stm32mp157"

the reported info is more meaningful.

Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
This is a preparatory patch for future developments that will increase the
number of platforms on which the bootcount application can operate.

Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
By doing this, the function can be called by platform modules as well as
by the EEPROM module.

This is a preparatory patch for future developments that will expand the
number of platforms on which the bootcount application can operate.

Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
The patch finalizes the separation between the generic bootcount code and
platform-specific code.

Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
The patch hides data that should not be exported in the corresponding .c
file. This prevents them from being improperly used outside of the module.

Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Make the function name consistent with the module name.

Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
In U-Boot, with the CONFIG_SYS_BOOTCOUNT_{BE,LE} configurations, it is
possible to define the endianness of the bootcount value. The patch adds
support for this option while ensuring backward compatibility.

Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
The patch adds support for endianness on the TI am33xx platform.
As highlighted by the output of the following command, some am33xx boards
use big-endian access for the bootcount value, while others adopt
little-endian access (default). The BeagleBone Black, on which I run the
tests, accesses it in big-endian mode.

cd <U-Boot_dir>
git grep CONFIG_SYS_BOOTCOUNT_ | grep am335x
configs/am335x_boneblack_vboot_defconfig:CONFIG_SYS_BOOTCOUNT_BE=y
configs/am335x_evm_defconfig:CONFIG_SYS_BOOTCOUNT_BE=y
configs/am335x_evm_spiboot_defconfig:CONFIG_SYS_BOOTCOUNT_BE=y
configs/am335x_hs_evm_defconfig:CONFIG_SYS_BOOTCOUNT_BE=y
configs/am335x_hs_evm_uart_defconfig:CONFIG_SYS_BOOTCOUNT_BE=y
configs/am335x_sl50_defconfig:CONFIG_SYS_BOOTCOUNT_BE=y

Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Adds bootcount support for the IMX8M platform. Thanks to the previous
commits, the added code is primarily located in the new platform modules.
Few are the changes in the bootcount.c file.
The bootcount address has been taken directly from the U-Boot
configurations:

cd <U-Boot_dir>
git grep BOOTCOUNT_ADDR configs/ | grep imx8m
configs/imx8mm-mx8menlo_defconfig:CONFIG_SYS_BOOTCOUNT_ADDR=0x30370090
configs/imx8mm_data_modul_edm_sbc_defconfig:CONFIG_SYS_BOOTCOUNT_ADDR=0x30370090
configs/imx8mp_data_modul_edm_sbc_defconfig:CONFIG_SYS_BOOTCOUNT_ADDR=0x30370090
configs/imx8mp_dhcom_pdk2_defconfig:CONFIG_SYS_BOOTCOUNT_ADDR=0x30370090
configs/imx8mp_dhcom_pdk3_defconfig:CONFIG_SYS_BOOTCOUNT_ADDR=0x30370090

Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
@passgat
Copy link
Contributor Author

passgat commented Jan 25, 2024

Done,
I hope I haven't forgotten any tabs.

@thom-nic thom-nic merged commit b346043 into VoltServer:master Jan 31, 2024
@thom-nic
Copy link
Contributor

Finally got to testing this on an EEPROM device. Looks great. Thanks for the contribution!

@passgat
Copy link
Contributor Author

passgat commented Jan 31, 2024

@thom-nic Thank you for having merged the PR. Is it possible to add a tag to release a new version so that I can then update the package in Buildroot?

Thanks and regards
Dario

@thom-nic
Copy link
Contributor

@passgat I will do a release. I also really should update the README for IMX8 and STM32MP1 as well..

@thom-nic
Copy link
Contributor

thom-nic commented Feb 1, 2024

@passgat
Copy link
Contributor Author

passgat commented Feb 2, 2024

@thom-nic Many thanks!
Best regards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants