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

Microchip addition of new devices PIC32CX_SG41, PIC32CX_SG60, PIC32CX_SG61 #21

Open
wants to merge 27 commits into
base: master
Choose a base branch
from

Conversation

MyGh64605
Copy link

These devices are compatible with ATSAMD/E XX series of devices that are currently incorporated within Zephyr.

These modifications are for the HAL layer only.
Boards will be added later.

MyGh64605 and others added 17 commits May 6, 2024 10:02
commit 71eba05
Author: Scott Worley <[email protected]>
Date:   Tue May 21 17:32:20 2024 -0400

    hal: microchip: mec5: Version 0.2 MEC5 HAL consistent naming

    Change symbols name to use prefix "mec_hal_" to prevent name
    clashes with symbols from other components (application, Zephyr,
    etc).

    Signed-off-by: Scott Worley <[email protected]>

commit 1279561
Author: Scott Worley <[email protected]>
Date:   Mon May 13 10:05:22 2024 -0400

    hal: microchip: mec5: Add initial I2C Network HAL support

    Add initial HAL API for starting an I2C transfer using the
    network layer in the I2C controller. Add an API to enable
    and disable network layer interrupts.

    Signed-off-by: Scott Worley <[email protected]>

commit f95f0f5
Author: Scott Worley <[email protected]>
Date:   Mon May 13 10:00:58 2024 -0400

    hal: microchip: mec5: Fix bug in central DMA configuration

    Fix a bug in central DMA channel configuration. The bug was not
    found by Zephyr DMA driver tests because these tests only exercise
    memory to memory transfers. The bug was in code paths for peripheral
    DMA to/from memory. Bug was found during I2C network layer driver
    development since this driver uses central DMA.

    Signed-off-by: Scott Worley <[email protected]>

commit 6985aff
Author: Scott Worley <[email protected]>
Date:   Mon May 6 15:41:12 2024 -0400

    hal: microchip: mec5: Add HAL for MEC174x, MEC175x, and MECH172x.

    Add a full HAL (header and peripheral C code) for Microchip "MEC5"
    family SoC's: MEC174x and MEC175x. Also support the older MEC172x
    whose HAL name is MECH172x. The HAL also include device tree PINCTRL
    files. NOTE: This is version 0.1 of the HAL and is not feature
    complete.

    Signed-off-by: Scott Worley <[email protected]>
hal: microchip: pic32cx:Adding support for Microchip PIC32CX SG devices
This is based on the hal atmel libraries as a starting point.

Signed-off-by: Michael Sherwood <[email protected]>
@MyGh64605
Copy link
Author

Question, should I create a pull request for the main Zephyr project or wait until this request is approved.

@MyGh64605 MyGh64605 marked this pull request as ready for review July 9, 2024 17:51
@MyGh64605
Copy link
Author

@nandojve This is first pull request to incorporate the PIC32CX SG devices, that are compatible with the ATDAMD/E XX devices.

commit 1281fa5866a0f9dcb78c5902ba1633bd163318da
Author: Zephyr DeveloperMichael <[email protected]>
Date:   Mon Jul 8 08:04:22 2024 -0700

    Squashed commit of the following:

    commit 8bcb80b
    Author: Zephyr DeveloperMichael <[email protected]>
    Date:   Thu Jun 27 10:02:26 2024 -0700

        Squashed commit of the following:

        commit 71eba05
        Author: Scott Worley <[email protected]>
        Date:   Tue May 21 17:32:20 2024 -0400

            hal: microchip: mec5: Version 0.2 MEC5 HAL consistent naming

            Change symbols name to use prefix "mec_hal_" to prevent name
            clashes with symbols from other components (application, Zephyr,
            etc).

            Signed-off-by: Scott Worley <[email protected]>

        commit 1279561
        Author: Scott Worley <[email protected]>
        Date:   Mon May 13 10:05:22 2024 -0400

            hal: microchip: mec5: Add initial I2C Network HAL support

            Add initial HAL API for starting an I2C transfer using the
            network layer in the I2C controller. Add an API to enable
            and disable network layer interrupts.

            Signed-off-by: Scott Worley <[email protected]>

        commit f95f0f5
        Author: Scott Worley <[email protected]>
        Date:   Mon May 13 10:00:58 2024 -0400

            hal: microchip: mec5: Fix bug in central DMA configuration

            Fix a bug in central DMA channel configuration. The bug was not
            found by Zephyr DMA driver tests because these tests only exercise
            memory to memory transfers. The bug was in code paths for peripheral
            DMA to/from memory. Bug was found during I2C network layer driver
            development since this driver uses central DMA.

            Signed-off-by: Scott Worley <[email protected]>

        commit 6985aff
        Author: Scott Worley <[email protected]>
        Date:   Mon May 6 15:41:12 2024 -0400

            hal: microchip: mec5: Add HAL for MEC174x, MEC175x, and MECH172x.

            Add a full HAL (header and peripheral C code) for Microchip "MEC5"
            family SoC's: MEC174x and MEC175x. Also support the older MEC172x
            whose HAL name is MECH172x. The HAL also include device tree PINCTRL
            files. NOTE: This is version 0.1 of the HAL and is not feature
            complete.

            Signed-off-by: Scott Worley <[email protected]>

    commit 81f02ec
    Author: Zephyr DeveloperMichael <[email protected]>
    Date:   Tue Jun 25 07:37:16 2024 -0700

        Adding support for Microchip PIC32CX SG devices

    commit e25427c
    Author: Zephyr DeveloperMichael <[email protected]>
    Date:   Mon Jun 24 09:27:09 2024 -0700

        Adding support for Microchip PIC32CX SG devices

    commit 22f2a0d
    Author: Zephyr DeveloperMichael <[email protected]>
    Date:   Tue Jun 18 12:20:35 2024 -0700

        Adding support for Microchip PIC32CX SG devices

    commit 1bf0f21
    Author: Zephyr DeveloperMichael <[email protected]>
    Date:   Tue Jun 18 11:51:34 2024 -0700

        Adding support for Microchip PIC32CX SG devices

    commit c26836d
    Author: Zephyr DeveloperMichael <[email protected]>
    Date:   Fri Jun 14 11:06:25 2024 -0700

        Adding support for Microchip PIC32CX SG devices

    commit 3002c9f
    Author: Zephyr DeveloperMichael <[email protected]>
    Date:   Mon Jun 3 12:38:17 2024 -0700

        Adding support for Microchip PIC32CX SG devices

    commit e2b7978
    Author: Zephyr DeveloperMichael <[email protected]>
    Date:   Fri May 31 11:08:36 2024 -0700

        Adding support for Microchip PIC32CX SG devices

    commit f026a5b
    Author: Zephyr DeveloperMichael <[email protected]>
    Date:   Fri May 31 09:52:56 2024 -0700

        Adding support for Microchip PIC32CX SG devices

    commit d12239d
    Author: Zephyr DeveloperMichael <[email protected]>
    Date:   Fri May 31 09:47:00 2024 -0700

        Adding support for Microchip PIC32CX SG devices

    commit 7c3252a
    Author: Zephyr DeveloperMichael <[email protected]>
    Date:   Thu May 30 06:11:19 2024 -0700

        Adding support for Microchip PIC32CX SG devices

    commit 29156b3
    Author: Zephyr DeveloperMichael <[email protected]>
    Date:   Mon May 13 12:32:30 2024 -0700

        Adding support for Microchip PIC32CX SG devices

    commit 678ee3b
    Author: Zephyr DeveloperMichael <[email protected]>
    Date:   Mon May 13 08:59:00 2024 -0700

        Adding support for Microchip PIC32CX SG devices

    commit 7902150
    Author: Zephyr DeveloperMichael <[email protected]>
    Date:   Tue May 7 08:02:14 2024 -0700

        Adding support for Microchip PIC32CX SG devices

    commit 82d8d1b
    Author: Zephyr DeveloperMichael <[email protected]>
    Date:   Mon May 6 11:32:54 2024 -0700

        Adding support for Microchip PIC32CX SG devices

    commit 0229627
    Author: Zephyr DeveloperMichael <[email protected]>
    Date:   Mon May 6 10:02:09 2024 -0700

        Adding support for Microchip PIC32CX SG devices

    commit 68575aa
    Author: Fabio Baltieri <[email protected]>
    Date:   Tue Feb 27 14:05:51 2024 +0000

        CMakeLists: use MEC15XX for the series option

        Rename the SoC series option to MEC15XX to align with the new hwmv2
        config name, drop the 1701x option which had no references anyway.

        Signed-off-by: Fabio Baltieri <[email protected]>

commit 9fc0e92
Author: Fabio Baltieri <[email protected]>
Date:   Tue Feb 27 14:05:51 2024 +0000

    CMakeLists: use MEC15XX for the series option

    Rename the SoC series option to MEC15XX to align with the new hwmv2
    config name, drop the 1701x option which had no references anyway.

    Signed-off-by: Fabio Baltieri <[email protected]>
Merge branch 'master' of https://github.com/MyGh64605/hal_microchip
@kartben
Copy link

kartben commented Jul 9, 2024

Question, should I create a pull request for the main Zephyr project or wait until this request is approved.

https://docs.zephyrproject.org/latest/develop/modules.html#process-for-submitting-changes-to-existing-modules

Copy link
Collaborator

@scottwcpg scottwcpg left a comment

Choose a reason for hiding this comment

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

Please move into subfolder. DTS can be at the top level because it has subfolders for vendor/chip_family.

CMakeLists.txt Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

add_subdirectory(mplabx) add this path to MEC_HAL (CPG old stuff), MPFS_HAL (microsemi RISC-V), and MEC5_HAL (CPU new stuff). Should be add_subdirectory_ifdef

Copy link
Member

Choose a reason for hiding this comment

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

I think this is a good opportunity to start organize the content. I believe that having names like ASF or MPLABX do not help. The content that are placed inside those folders are platform specific, The platform directory should have a tree that follows an specific family. Inside each family the variants will be placed. The below tree presents 2 well know families sam and sam0. The new pic32cxsg should be organized in the same manner. I let to you propose the names but I believe the content will be well organized as in the below tree.

Notes:

1- Add the .github rules that are available on hal_atmel to check if the licenses are compliance with Zephyr rules.
2- The mec/mec5/mpfs are candidates to be moved inside platform [Improvement - not part of this PR]

.
├── .github
│   ├── license_config.yml
│   └── workflows
│       ├── license_check.yml
│       └── test.yml
├── platform
│   ├── CMakeLists.txt
│   ├── common
│   │   ├── CMakeLists.txt
│   │   └── components
│   ├── ? family name
│   │   ├── CMakeLists.txt
│   │   └── include
│   │       ├── ? part variant name
│   │       └── next gen ?
│   ├── sam
│   │   ├── CMakeLists.txt
│   │   └── include
│   │       ├── sam3x
│   │       ├── sam4e
│   │       ├── sam4l
│   │       ├── sam4s
│   │       ├── same70
│   │       ├── same70b
│   │       ├── samv71
│   │       └── samv71b
│   └── sam0
│       ├── CMakeLists.txt
│       └── include
│           ├── CMakeLists.txt
│           ├── samc20
│           ├── samc20n
│           ├── samc21
│           ├── samc21n
│           ├── samd20
│           ├── samd21
│           ├── samd51
│           ├── same51
│           ├── same53
│           ├── same54
│           ├── saml21
│           ├── samr21
│           ├── samr34
│           └── samr35
├── CMakeLists.txt
├── include
│   └── dt-bindings
│       └── pinctrl
├── pinconfigs
│   ├── README.md
│   ├── sam-3x.yml
│   ├── sam-4e.yml
│   ├── sam-4l.yml
│   ├── sam-4s-4sa-4sd.yml
│   ├── sam-c2x.yml
│   ├── sam-d2x-da1-abcd.yml
│   ├── sam-d5x-e5x.yml
│   ├── sam-l21.yml
│   ├── sam-r21.yml
│   ├── sam-r34-r35.yml
│   └── sam-s70-e70-v7x.yml
├── README.md
├── scripts
│   ├── README.md
│   ├── requirements-dev.txt
│   ├── requirements-test.txt
│   ├── requirements.txt
│   ├── sampinctrl.py
│   └── tests
│       └── sampinctrl
└── zephyr

Copy link
Contributor

Choose a reason for hiding this comment

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

I think @nandojve suggestion for organization is a good one. have you thought about it @scottwcpg?

Copy link
Member

Choose a reason for hiding this comment

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

I think this can make more sense when you think about integration of hal_mchp/atmel in the future. So, everything goes in the right place and future generations will be side by side. This makes zephyr soc configuration far more easy to handle since hal will be same for MCUs independent of the core ARM/RISC-V etc.

Copy link
Collaborator

Choose a reason for hiding this comment

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

All the original content is gone. Many of your instructions do not apply to the the other HAL's. These HAL's use the zephyr SDK toolchain.

Copy link
Member

Choose a reason for hiding this comment

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

I still think to have a platform folder name instead pic32c seems better for the long run. Then, when moving sam, sam0 they will be side by side with pic32cxsg instead creating 2 new folders on the root folder.

The above are general guidelines and exceptions could happen. In this case, the
exception should be addressed at review phase.

## The standard API
Copy link
Collaborator

Choose a reason for hiding this comment

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

This applies to our HAL only. The others use their own naming convention.

@@ -0,0 +1,179 @@
/*
Copy link
Collaborator

Choose a reason for hiding this comment

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

This folder is at the top level. Most hal_vendor folders have subdirectories containing the whole chip family. mec, mec5, mpfs contain all the specific stuff for those chip families. Is it possible for you to put your stuff in a subdirectory also, so we are all consistent and isolated?

@nandojve
Copy link
Member

nandojve commented Jul 10, 2024

Hi @MyGh64605,

@nandojve This is first pull request to incorporate the PIC32CX SG devices, that are compatible with the ATDAMD/E XX devices.

Ok, I'll start to review soon.

@nandojve nandojve self-assigned this Jul 10, 2024
Copy link
Member

@nandojve nandojve left a comment

Choose a reason for hiding this comment

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

Hi @MyGh64605 ,

Question, should I create a pull request for the main Zephyr project or wait until this request is approved.

It is very important to follow the how-to-submit-code. I also recommend this old side that help to understand how I usually expect a commit message https://cbea.ms/git-commit. Not a must - desirable.

It is important organize the commit brief message to allow easy filtering. Usually I start this way:

1- More than one device, try group by family
2- functionality: family or device. Try get some inspiration from hal_atmel:

* c1c4410 - docs: Improve documentation (1 year, 4 months ago) <Gerson Fernando Budke>
* d45adfb - samc20: samc21: Add ADC_INPUTCTRL internal ground for ADC Driver (1 year, 9 months ago) <Kamil Serwus>
* ffe9191 - pinctrl: sam-c2x: Add pinconfigs and include files (1 year, 9 months ago) <Kamil Serwus>
* 3eaaa0b - ext: Import Atmel SAMC20 header files from ASF library (1 year, 9 months ago) <Kamil Serwus>
* a269f63 - ext: Import Atmel SAMC21 header files from ASF library (1 year, 9 months ago) <Kamil Serwus>
* 840b6ac - sam0: Add revisions to soc includes (1 year, 9 months ago) <Kamil Serwus>
* 43c73d8 - sam4l: Add US_MR_CHRL_{n}_BIT Register Aliases for USART Driver (1 year, 9 months ago) <Nick Kraus>
* 1d237f2 - samv71b: Fix CIDR for V71 Revision B Silicon (1 year, 11 months ago) <Nick Kraus>
* bb4e710 - CMakeLists.txt: Add include directory only for atmel devices (2 years ago) <Yonatan Schachter>
* a486271 - winc1500: nmspi: Fix compiler warning (2 years, 1 month ago) <Kumar Gala>
* 7458978 - ext: winc1500: Rename function to avoid name clash (2 years, 1 month ago) <Ole Morten Haaland>
* 7518d0b - pinctrl: sam-r34-r35: Add pinconfigs and include files (2 years, 3 months ago) <Attie Grande>
* 8989ec4 - pinctrl: sam-l21: Add pinconfigs and include files (2 years, 3 months ago) <Attie Grande>
* 7e9d3fc - ext: Import Atmel SAMR35 header files from ASF library (2 years, 3 months ago) <Attie Grande>
* 4749709 - ext: Import Atmel SAMR34 header files from ASF library (2 years, 3 months ago) <Attie Grande>
* ceacbcb - ext: Import Atmel SAML21 header files from ASF library (2 years, 3 months ago) <Attie Grande>
* 2c2ce9f - ci: license_check: Update to scancode action v4 (2 years, 4 months ago) <Stephanos Ioannidis>
* 78c5567 - samv71: patch: Fix XDMAC_CHID register name (2 years, 4 months ago) <Gerson Fernando Budke>
* 94d8d86 - pinctrl: pinconfigs: Add README file (2 years, 4 months ago) <Gerson Fernando Budke>
* 5efcf60 - pinctrl: sam-d51-d53-d54: Add pinconfigs and include files (2 years, 4 months ago) <Gerson Fernando Budke>
* 5a4f715 - pinctrl: sam-r21: Add pinconfigs and include files (2 years, 4 months ago) <Gerson Fernando Budke>
* 3d10116 - pinctrl: sam-d20-d21-da1: Add pinconfigs and include files (2 years, 4 months ago) <Gerson Fernando Budke>
* 1876356 - pinctrl: sam-s70-e70-v7x: Add pinconfigs and include files (2 years, 4 months ago) <Gerson Fernando Budke>
* b33b97a - pinctrl: sam-4s-4sa-4sd: Add pinconfigs and include files (2 years, 4 months ago) <Gerson Fernando Budke>
* 845b8f2 - pinctrl: sam-4l: Add pinconfigs and include files (2 years, 4 months ago) <Gerson Fernando Budke>
* 674b12d - pinctrl: sam-4e: Add pinconfigs and include files (2 years, 4 months ago) <Gerson Fernando Budke>
* ac2bac9 - pinctrl: sam-3x: Add pinconfigs and include files (2 years, 4 months ago) <Gerson Fernando Budke>
* 527de70 - pinctrl: Add pinctrl tests (2 years, 4 months ago) <Gerson Fernando Budke>
* 9597144 - pinctrl: Add pinctrl script (2 years, 4 months ago) <Gerson Fernando Budke>
* 45cb8f8 - patch: samd21/r21: Normalize underscore defines (2 years, 4 months ago) <Gerson Fernando Budke>
* 7c24242 - patch: samd21: Correct missing underscore in defines for variants (2 years, 5 months ago) <rocky134>

It is mandatory when add a new platform to have a new board at same time. The board only needs a serial port on pooling mode that runs the hello world application. This means that we need to align hal / soc / devicetree / board. On the remaining features will be add incrementally. This allows us to do a good review process and make sure that both tests are Ok and code is on a usable stage.

CMakeLists.txt Outdated
Copy link
Member

Choose a reason for hiding this comment

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

I think this is a good opportunity to start organize the content. I believe that having names like ASF or MPLABX do not help. The content that are placed inside those folders are platform specific, The platform directory should have a tree that follows an specific family. Inside each family the variants will be placed. The below tree presents 2 well know families sam and sam0. The new pic32cxsg should be organized in the same manner. I let to you propose the names but I believe the content will be well organized as in the below tree.

Notes:

1- Add the .github rules that are available on hal_atmel to check if the licenses are compliance with Zephyr rules.
2- The mec/mec5/mpfs are candidates to be moved inside platform [Improvement - not part of this PR]

.
├── .github
│   ├── license_config.yml
│   └── workflows
│       ├── license_check.yml
│       └── test.yml
├── platform
│   ├── CMakeLists.txt
│   ├── common
│   │   ├── CMakeLists.txt
│   │   └── components
│   ├── ? family name
│   │   ├── CMakeLists.txt
│   │   └── include
│   │       ├── ? part variant name
│   │       └── next gen ?
│   ├── sam
│   │   ├── CMakeLists.txt
│   │   └── include
│   │       ├── sam3x
│   │       ├── sam4e
│   │       ├── sam4l
│   │       ├── sam4s
│   │       ├── same70
│   │       ├── same70b
│   │       ├── samv71
│   │       └── samv71b
│   └── sam0
│       ├── CMakeLists.txt
│       └── include
│           ├── CMakeLists.txt
│           ├── samc20
│           ├── samc20n
│           ├── samc21
│           ├── samc21n
│           ├── samd20
│           ├── samd21
│           ├── samd51
│           ├── same51
│           ├── same53
│           ├── same54
│           ├── saml21
│           ├── samr21
│           ├── samr34
│           └── samr35
├── CMakeLists.txt
├── include
│   └── dt-bindings
│       └── pinctrl
├── pinconfigs
│   ├── README.md
│   ├── sam-3x.yml
│   ├── sam-4e.yml
│   ├── sam-4l.yml
│   ├── sam-4s-4sa-4sd.yml
│   ├── sam-c2x.yml
│   ├── sam-d2x-da1-abcd.yml
│   ├── sam-d5x-e5x.yml
│   ├── sam-l21.yml
│   ├── sam-r21.yml
│   ├── sam-r34-r35.yml
│   └── sam-s70-e70-v7x.yml
├── README.md
├── scripts
│   ├── README.md
│   ├── requirements-dev.txt
│   ├── requirements-test.txt
│   ├── requirements.txt
│   ├── sampinctrl.py
│   └── tests
│       └── sampinctrl
└── zephyr

* SPDX-License-Identifier: Apache-2.0
*/

#ifndef DT_BINDINGS_PINCTRL_MICROCHIP_PIC32CXSG_PINCTRL_H_
Copy link
Member

Choose a reason for hiding this comment

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

I'm glad to see that MCHP appreciate the work that was made on the hal_atmel. This means that all the files that have copyright that are base for some work in this repository and were captured from hal_atmel or even are inspirations for some derived work in this repository must have the original copyright.

# SPDX-License-Identifier: Apache-2.0
#

add_subdirectory(include)
Copy link
Member

Choose a reason for hiding this comment

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

File should have a newline at end.
Fix all the files that apply.

Copy link
Author

Choose a reason for hiding this comment

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

Good afternoon,
I will make the suggested modifications.

Will multiple pull requests need to be made ?
This one for hal_microchip and
One to add the board files ?
One to modify 'west.yml' ?
One to add the soc files etc ...... ?

FYI,
I have created or modified all the pertinent files within the main Zephyr directory. Created a 'Hello World' project for both of the evaluation boards to be added.
The 'Hello World' project compiles without error.

@@ -0,0 +1,56 @@
/*
Copy link
Member

Choose a reason for hiding this comment

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

run find * type f -print0 | xargs -0 dos2unix -k on all the files imported and make sure they are using utf-8 format.

Copy link
Author

Choose a reason for hiding this comment

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

Good afternoon, will this command actually 'fixup' the files and provide a list of modified files ?

Copy link
Author

Choose a reason for hiding this comment

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

Good afternoon,

error after running this command

"'xargs' is not recognized as an internal or external command,
operable program or batch file."

…CXSG61 boards build

After running command "find * -type f -print0 | xargs -0 dos2unix -k"
…CXSG61 boards build

After running command "find * -type f -print0 | xargs -0 dos2unix -k"

Signed-off-by: Michael D Sherwood <[email protected]>
MyGh64605 and others added 2 commits July 24, 2024 11:25
Addressing PR comments:
Renamed 'mplax' folder to 'pic32c', tested that PIC32CXSG41 and PIC32CXSG61 boards build

After running command "find * -type f -print0 | xargs -0 dos2unix -k"

Signed-off-by: Michael Sherwood <[email protected]>
@MyGh64605
Copy link
Author

Have updated the hal_microchip PIC32CX SG files.

Associated pull request for the addition of the PIC32CX_SG devices to zephyr main:
zephyrproject-rtos/zephyr#76503

Copy link
Member

@nandojve nandojve left a comment

Choose a reason for hiding this comment

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

Overall LGTM. I'll suggest to change the title to pic32c: Introduce hal platform

@@ -0,0 +1,287 @@
# Copyright (c) 2024 Microchip
Copy link
Member

Choose a reason for hiding this comment

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

You should keep original copyright with the new one, even when you rename a file.

@@ -0,0 +1,283 @@
# Copyright (c) 2024 Microchip
Copy link
Member

Choose a reason for hiding this comment

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

You should keep original copyright with the new one, even when you rename a file.

Copy link
Member

Choose a reason for hiding this comment

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

I still think to have a platform folder name instead pic32c seems better for the long run. Then, when moving sam, sam0 they will be side by side with pic32cxsg instead creating 2 new folders on the root folder.

@nandojve
Copy link
Member

nandojve commented Aug 8, 2024

Please, do not execute merge branch. Always rebase your commits (Fast Forward Only) on top of the main. The goal is to have a clean history

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.

6 participants