[PATCH rtems 00/12] bsp/imxrt: Update SDK and prepare for new variant

[oss at c-mauderer.de]

Hello Chris,

Am 15.05.23 um 05:18 schrieb Chris Johns:
> 
> 
> On 6/5/2023 2:02 am, Gedare Bloom wrote:
>> On Fri, May 5, 2023 at 9:02 AM <oss at c-mauderer.de> wrote:
>>>
>>> Hello Gedare,
>>>
>>> thanks for taking a look at the patch set.
>>>
>>> Am 05.05.23 um 15:56 schrieb Gedare Bloom:
>>>> On Thu, May 4, 2023 at 9:01 AM Christian Mauderer
>>>> <christian.mauderer at embedded-brains.de> wrote:
>>>>>
>>>>> Hello,
>>>>>
>>>>> this patch set for the arm/imxrt BSP family updates the SDK files to the
>>>>> latest version of the mcux-sdk from NXP and prepares the BSP for further
>>>>> chip variants. I plan to add a BSP that uses the IMXRT1166 soon.
>>>>>
>>>>> As a base for the mcux-sdk files, I now use the NXP git repository
>>>>> instead of zip files that can be downloaded from NXP. I kept the exact
>>>>> file system structure to make future updates simpler.
>>>>>
>>>>> To import the files, I used a script. It is not a clean script and it
>>>>> was only tested on a Linux machine. Despite that, I added that script to
>>>>> the BSP directory in case someone else ever wants to update the
>>>>> mcux-sdk. Updating the SDK is also possible without the script. It's
>>>>> just a lot more manual work. So if we don't want a script in that state
>>>>> in the repository, I can also just keep it on a private branch.
>>>>>
>>>> I would recommend that you document the import/update process for the
>>>> SDK in this BSP (or family) documentation. You could provide a stable
>>>> external link to the script there instead of including it in the
>>>> rtems.git tree. I'm not convinced that including scripts etc in the
>>>> same tree as the source they manipulate is a great practice. It seems
>>>> to violate separation of concerns.
>>>
>>> Disadvantage is that the script (and documentation) is a bit less simple
>>> to find. But that's not a problem for me (I know where to search) so I
>>> will adapt that.
>>>
>>> De we have an official preferred stable location? Otherwise, I think
>>> either a small repo in my https://git.rtems.org/christianm or a repo on
>>> github should be a good place (most likely the later one).
>>>
>> This should be fine. ftp.rtems.org should also be usable (in theory)
>>
>>>>
>>>>> The patches that import the new SDK files (patch 0002) and remove the
>>>>> old ones (patch 0004) are too big for the list. I'll only send the
>>>>> summary. You can find the full patches here:
>>>>>
>>>>>     0002: https://gitlab.com/c-mauderer/rtems/-/commit/2a871672767a95598e5af42373bfebd3eb9440d3
>>>>>     0004: https://gitlab.com/c-mauderer/rtems/-/commit/2a3e104fa808d7f34a1930344d7b39d11cf39f3d
>>>>>
>>>>> The complete patch set is on this branch:
>>>>>
>>>>>     https://gitlab.com/c-mauderer/rtems/-/commits/cm/20230504_imxrt/
>>>>>
>>>>> At the moment, I import the support files for all currently available
>>>>> i.MXRT* variants. The headers for the CPU registers are really big (a
>>>>> few megabytes per header) which makes the complete source tree of the
>>>>> mcux-sdk bigger than 90MB. If preferred, I can remove most variants and
>>>>> only keep the ones that are currently used or will be used soon
>>>>> (IMXRT1052 and IMXRT1166) to reduce the size.
>>>>>
>>>> Thanks, I think this is fine. The increase in the number of HAL/SDK
>>>> that we have to import is starting to concern me in general. I wonder
>>>> if there is a better way to manage these external sources.
>>>>
>>>
>>> I think I have a language problem here: Is it fine that I push 96MB or
>>> is it fine that I reduce the size of the patch before pushing?
>>>
>> My apologies, I missed this also. If it's not that much more work, it
>> might be best to remove the unused variants. We don't like to carry
>> around dead code in the repo as another general rule.
>>
>>> In this case I only updated the existing HAL with more chips, but I
>>> agree that having a better solution to manage HALs than the
>>> clone-and-own approach would be great.
>>>
>>> One possible solutions could be to use subrepos. We can clone the lib to
>>> the git.rtems.org, add necessary patches and add that as a subrepo. If
>>> an update is necessary, patches can just be rebased on the latest
>>> version of the upstream lib. Another one could be something like the
>>> west tool that is used in Zephyr. Maybe can also make it simpler to
>>> include libs with vendor specific licenses.
>>>
>>> But approaches like that will have a big impact on how to handle a lot
>>> of tasks in RTEMS. For example you would have to init submodules before
>>> building BSPs. The submodules have to be handled during release. And a
>>> lot more. So we shouldn't discuss that as a side note to a patch set but
>>> rather as a new mail thread.
>>>
>> Agreed.
> 
> Do these HAL systems provide stable API interfaces? If so is all RTEMS needs to
> interface to is the API headers?
> 
> Chris
> 

I think that depends a lot on the vendor. In my experience, the better 
HALs are mostly stable. For example, I don't think that the ST HAL 
changed the API in the versions that I know. And during the update of 
the NXP SDK I didn't note a breaking API change either. Only addition of 
new functions.

The HAL provides basic drivers. For example in the i.MXRT BSP I use it 
for stuff like setting up clocks or adding drivers like serial, I2C or 
SPI. I expect that at least some drivers are necessary to link every 
program. So without the HAL, the test suite wouldn't build. As far as I 
know that would be something new in RTEMS that the tests can't be built 
without external dependencies.

I don't have a problem with a change like that, but it is something that 
would have a quite big influence on workflows and how we plan to handle 
similar cases in the future.

Like said to Gedare: If we want to discuss possible solutions in detail: 
Please let's start a separate thread that doesn't target NXP HAL only 
but HALs in general (ST HAL, ARM CMSIS, NXP HAL, ...). Should I write a 
mail starting a discussion?

I don't expect that we find a solution for that before the next release. 
Therefore, I still would like to apply the v2 of the patch set, which is 
just an update of the currently existing HAL in the BSP. Of course if 
someone suggests a better solution that can be included in the release, 
I'm happy to update the patch set to that solution.

Best regards

Christian