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

add si32 HAL #29

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

M1cha
Copy link

@M1cha M1cha commented May 13, 2023

Source: precision32-development-environment-offline.exe
sha512: 6bccef30995da803985fcdfa4065c45e00e0eeefc300d61b7215926a97997dc8ce45078ffeb349602340759650f30aeccf3b1aa50086c61edc2426d016063cae
Website: https://www.silabs.com/developers/precision32-32-bit-microcontroller-development-suite?tab=downloads

si32/README Outdated
v1.1.2

Purpose:
Add support for Silican Labs sim3u1xx SoCs

Choose a reason for hiding this comment

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

Suggested change
Add support for Silican Labs sim3u1xx SoCs
Add support for Silican Labs SiM3U1xx SoCs

Sorry, something went wrong.

Copy link
Author

Choose a reason for hiding this comment

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

done

Sorry, something went wrong.

si32/README Outdated
Add support for Silican Labs sim3u1xx SoCs

Description:
This code component is used to add Zephyr support for Silicon Labs sim3u1xx SoCs.

Choose a reason for hiding this comment

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

Suggested change
This code component is used to add Zephyr support for Silicon Labs sim3u1xx SoCs.
This code component is used to add Zephyr support for Silicon Labs SiM3U1xx SoCs.

Sorry, something went wrong.

Copy link
Author

Choose a reason for hiding this comment

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

done

Sorry, something went wrong.

si32/README Outdated

The SDK contains an incompatible, proprietary license in `Documentation/Silabs_License_Agreement.txt`,
but all of the source code links to a newer, permissive license [0].
We have official, public confirmation [1], that the newer license applies.

Choose a reason for hiding this comment

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

Maybe adding a local copy of this statement to this repository?

Sorry, something went wrong.

Copy link
Author

Choose a reason for hiding this comment

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

good idea

Sorry, something went wrong.

si32/README Outdated

* Download precision32-development-environment-offline.exe
* run `./scripts/extract_hal /path/to/downloaded-file`
* The extracted HAL will be in `$CWD/tmp`, Copy the necessary directories

Choose a reason for hiding this comment

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

Suggested change
* The extracted HAL will be in `$CWD/tmp`, Copy the necessary directories
* The extracted HAL will be in `$CWD/tmp`, copy the necessary directories

Sorry, something went wrong.

Copy link
Author

Choose a reason for hiding this comment

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

nope. after a , you don't start with an uppercase character. Also, that sentence is gone now.

Sorry, something went wrong.

+++++++++++++

* Download precision32-development-environment-offline.exe
* run `./scripts/extract_hal /path/to/downloaded-file`

Choose a reason for hiding this comment

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

It does not seem to work for me:

$ sha512sum ~/Downloads/precision32-development-environment-offline.exe
6bccef30995da803985fcdfa4065c45e00e0eeefc300d61b7215926a97997dc8ce45078ffeb349602340759650f30aeccf3b1aa50086c61edc2426d016063cae  /home/reto/Downloads/precision32-development-environment-offline.exe
$ scripts/extract_hal ~/Downloads/precision32-development-environment-offline.exe

7-Zip (a) [64] 16.02 : Copyright (c) 1999-2016 Igor Pavlov : 2016-05-21
p7zip Version 16.02 (locale=en_US.utf8,Utf16=on,HugeFiles=on,64 bits,32 CPUs AMD Ryzen 9 5900X 12-Core Processor             (A20F10),ASM,AES-NI)

Scanning the drive for archives:
1 file, 427228821 bytes (408 MiB)

Extracting archive: /home/username/Downloads/precision32-development-environment-offline.exe

ERRORS:
Headers Error
Unconfirmed start of archive


WARNINGS:
Headers Error
There are data after the end of archive

--
Path = /home/username/Downloads/precision32-development-environment-offline.exe
Type = zip
ERRORS:
Headers Error
Unconfirmed start of archive
WARNINGS:
Headers Error
There are data after the end of archive
Offset = 1815905
Physical Size = 271798
Tail Size = 425141118


No files to process


Archives with Errors: 1

Warnings: 1

Open Errors: 1

Any ideas?

Sorry, something went wrong.

Copy link
Author

Choose a reason for hiding this comment

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

You're using 7zip 16.02 which was released in 2016. I'm using 17.04 which has support for lots of new file formats. (There's also 17.05 which got released recently which I haven't tested, yet).

Sorry, something went wrong.

Copy link
Author

Choose a reason for hiding this comment

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

I documented the version requirement.

Sorry, something went wrong.

si32/README Outdated
@@ -0,0 +1,50 @@
Silicon Labs SI32 HAL

Choose a reason for hiding this comment

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

Suggested change
Silicon Labs SI32 HAL
Silicon Labs Si32 HAL

Sorry, something went wrong.

Copy link
Author

Choose a reason for hiding this comment

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

done

Sorry, something went wrong.

si32/README Outdated
v1.1.2

Purpose:
Add support for Silican Labs sim3u1xx SoCs
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Add support for Silican Labs sim3u1xx SoCs
Add support for Silicon Labs sim3u1xx SoCs

Sorry, something went wrong.

Copy link
Author

Choose a reason for hiding this comment

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

done

Sorry, something went wrong.

* MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE APPLY TO THIS SOFTWARE.
* ARM SHALL NOT, IN ANY CIRCUMSTANCES, BE LIABLE FOR SPECIAL, INCIDENTAL, OR
* CONSEQUENTIAL DAMAGES, FOR ANY REASON WHATSOEVER.
*
Copy link
Member

Choose a reason for hiding this comment

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

Wrong license? Keep in mind that Zephyr accepts only OSI-compliant licenses for HAL source files.

Sorry, something went wrong.

Copy link
Author

Choose a reason for hiding this comment

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

These are actually not needed because zephyr provides them already and we can just use those instead.
But what makes a license OSI-compliant? Is the permissive Silicon Laboratories End User License Agreement still an issue?

Sorry, something went wrong.

Copy link

Choose a reason for hiding this comment

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

See here for a list: https://opensource.org/licenses/

Sorry, something went wrong.

Copy link
Author

Choose a reason for hiding this comment

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

Okay that's unfortunate since it means that this okay is not enough.
@silabs-Joe Is it possible for you to allow us to change the license to z-lib(the same license as the gecko hal)?
We could change it ourselves so you don't have to put in any work, but we do obviously need the okay.

Sorry, something went wrong.

Choose a reason for hiding this comment

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

Hi @M1cha,
I am a Silicon Labs employee. I've checked with our legal team and they approve Gardena applying a ZLIB license to the code in the si32Hal folder for the Zephyr OS project. So you can adapt or modify the files to include the ZLIB license instead of the current license text.

Below is the ZLIB license text to use in the headers for the si32Hal folder:
Copyright 2012 (c) Silicon Laboratories Inc.
 
This siHAL software is provided 'as-is', without any express or implied warranty.  In no event will the authors be held liable for any damages arising from the use of this software.
 
Permission is granted to anyone to use this software for any purpose, including commercial applications, and to alter it and redistribute it freely, subject to the following restrictions:
 
1. The origin of this software must not be misrepresented; you must not claim that you wrote the original software. If you use this software in a product, an acknowledgment in the product documentation would be appreciated but is not required.
 
2. Altered source versions must be plainly marked as such, and must not be misrepresented as being the original software.
 
3. This notice may not be removed or altered from any source distribution.

Please let me know if you have any questions.

Best regards,
Spurthi

Sorry, something went wrong.

Choose a reason for hiding this comment

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

It would good to include the SPDX license identifier as well so its easy to see the license type.

Sorry, something went wrong.

Copy link
Member

Choose a reason for hiding this comment

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

It would good to include the SPDX license identifier as well so its easy to see the license type.

Agreed. I think it would also make sense to include a signoff from @silabs-spurthi in the commit that adds the re-licensed files, and also describe that fact in the commit message. This would ensure that there is a trace of what happened in the commit history, and that this is not lost somewhere in the comment section on GH.

Sorry, something went wrong.

@M1cha M1cha force-pushed the sim3u branch 2 times, most recently from e6e2e11 to b0563af Compare July 13, 2023 04:50
@M1cha
Copy link
Author

M1cha commented Jul 13, 2023

I've done the following:

  • rebased to master
  • addressed all comments
  • added a copy_hal script which copies and patches the SDK
  • added a dump of this site in license-change-permit.html so the relicensing permit will not be lost.
  • I excluded the CMSIS headers since they had a proprietary license and we have a compatible version in modules/hal/cmsis.
  • I excluded startup and linker files since they had a proprietary license and we don't need them.

I did not add a SPDX license identifier since @silabs-spurthi never gave their ok for that.

@silabs-Joe
Copy link

@M1cha Our legal team has approved adding the SPDX license identifier.

Copy link

@rettichschnidi rettichschnidi left a comment

LGTM. Will look even better with SPDX identifiers.

@msierszulski
Copy link
Member

Hi @M1cha, as @silabs-Joe said:

@M1cha Our legal team has approved adding the SPDX license identifier.

Can you please add the SPDX license identifier? Remember the proper description as @fkokosinski said in his previous comment.

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