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

cpu: Add PIC32MZ support #6066

Merged
merged 6 commits into from
Mar 29, 2017
Merged

Conversation

neiljay
Copy link
Contributor

@neiljay neiljay commented Nov 7, 2016

This PR is dependant on #6060 - merged

Add support for PIC32 MZ devices which use a MIPS M5150 core.

This PR adds basic support for Microchip PIC32MZ devices, including
bootstrapping the device, the build system produces a hex image loadable via
MPLAB-IPE. It has been tested on a Digilent Wifire board.

NOTE: This port requires the use of the GCC based MIPS Codescape SDK toolchain
available here:

https://community.imgtec.com/developers/mips/tools/codescape-mips-sdk/download-codescape-mips-sdk-essentials/

You cannot use Microchip's MPLAB / Harmony tools to build this port.

The port has been tested with the following examples:
hello-world
ipc_pingpong
timer_periodic_wakeup
riot_and_cpp

Note this is a re-work of #5885 with review comments addressed but now broken down into 3 separate PR's of which this is the second.

@neiljay neiljay added this to the Release 2017.01 milestone Nov 7, 2016
@neiljay neiljay added Platform: MIPS Platform: This PR/issue effects MIPS-based platforms Type: new feature The issue requests / The PR implemements a new feature for RIOT Impact: major The PR changes a significant part of the code base. It should be reviewed carefully labels Nov 7, 2016
@miri64 miri64 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR State: waiting for other PR State: The PR requires another PR to be merged first and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Nov 8, 2016
@neiljay neiljay mentioned this pull request Nov 9, 2016
@OlegHahm
Copy link
Member

I was able to build and flash the ipc_pingpong (at least mdb is saying:

Programming target...
Program succeeded.

but I couldn't get any output.

@OlegHahm
Copy link
Member

Do I assume correctly that UART0 is configured over the mini-USB port?

@neiljay
Copy link
Contributor Author

neiljay commented Nov 21, 2016

Its UART4 on the pic device, but yes the MINI USB header is the one you
want, 9600 8N1.

If you want to power the board off that USB cable you need to set the
'VUSelect' Jumper to 'UART'.

you might need to press enter a couple of times in your console to wake
up the USB<->UART converter.

Neill

On 11/21/16 17:45, Oleg Hahm wrote:

Do I assume correctly that UART0 is configured over the mini-USB port?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#6066 (comment), or
mute the thread
https://github.com/notifications/unsubscribe-auth/AUEU6pKgstiJwJiaPbTIFzZar9stpHe3ks5rAdjPgaJpZM4KrU6C.

@OlegHahm
Copy link
Member

Ah, true. Forgot about the 9600 baud... Now it is working! 👍

@neiljay
Copy link
Contributor Author

neiljay commented Nov 21, 2016

Sweet!

On 11/21/16 17:57, Oleg Hahm wrote:

Ah, true. Forgot about the 9600 baud... Now it is working! 👍


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#6066 (comment), or
mute the thread
https://github.com/notifications/unsubscribe-auth/AUEU6i_uF7-N0n7FaH3gWjyELMNS1j5oks5rAduhgaJpZM4KrU6C.

@PeterKietzmann PeterKietzmann modified the milestones: Release 2017.01, Release 2017.04 Jan 26, 2017
@neiljay neiljay force-pushed the pr/add_mips-pic32_mz branch 5 times, most recently from 4003290 to d273ecd Compare February 13, 2017 15:13
@neiljay
Copy link
Contributor Author

neiljay commented Feb 13, 2017

Hi all,

Why are we running cppcheck on .s and .S files ?

I get this error:
cpu/mips_pic32_common/reset_mod.S:84: error (syntaxError): syntax error
from this line:
beqz s1, init_resources # Branch if this is NOT an NMI exception.
The code assembles fine, I tried an unnamed label '1f' but it didn't like that either...

It looks like this check fails on other (in tree) asm file also (testing in my local docker image):

cpu/lpc2387/asmfunc.s:0: error (syntaxError): syntax error

Hopefully this is now close to a point where we can merge it, The PIC32 ports should be of more interest as the targets are readily available and cheap. @OlegHahm Can you try this on your board.

I ran the unittest suite and that all passed:

main(): This is RIOT! (Version: 2017.04-devel-152-g204aa-a2f4afe6403a-pr/add_mips-pic32_mz)
........................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................
OK (712 tests)

@neiljay
Copy link
Contributor Author

neiljay commented Feb 14, 2017

So cppcheck was failing on the asm style comments (in an asm file) WTF!!

@neiljay neiljay force-pushed the pr/add_mips-pic32_mz branch from fa24898 to 4413a76 Compare February 15, 2017 14:42
@jnohlgard jnohlgard removed the State: waiting for other PR State: The PR requires another PR to be merged first label Mar 1, 2017
@jnohlgard
Copy link
Member

I'm seeing errors with objcopy at the end:

   text	   data	    bss	    dec	    hex	filename
  78616	   2424	   6012	  87052	  1540c	/home/jgn/work/src/riot/examples/default/bin/pic32-clicker/default.elf
objcopy: /home/jgn/work/src/riot/examples/default/bin/pic32-clicker/default.hex: address 0xffffffff1d000000 out of range for Intel Hex file
objcopy:/home/jgn/work/src/riot/examples/default/bin/pic32-clicker/default.hex: Bad value
objcopy: --change-section-lma .gcc_except_table+0xffffffff80000000 never used
make: *** [/home/jgn/work/src/riot/examples/default/../../Makefile.include:278: all] Error 1

Both pic32-wifire and pic32-clicker.

GNU objcopy (Gentoo 2.27 p1.0) 2.27

@neiljay neiljay force-pushed the pr/add_mips-pic32_mz branch 2 times, most recently from f9d717f to 0075137 Compare March 10, 2017 16:04
@neiljay
Copy link
Contributor Author

neiljay commented Mar 13, 2017

@gebart Are you happy for me to squash this now so we can get this merged ?

@@ -0,0 +1,9 @@
# Put defined MCU peripherals here (in alphabetical order)
FEATURES_PROVIDED += periph_timer
FEATURES_PROVIDED += periph_uart
Copy link
Member

Choose a reason for hiding this comment

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

Why is periph_uart in FEATURES_PROVIDED if there is no UART available?
according to comment at https://github.com/RIOT-OS/RIOT/pull/6066/files#diff-8790a8833e3d374cd14889b9f0e69a2cR18

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Uart Tx is implemented but not the full UART API yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tweaked the comment when squashing see e2d7d36

@neiljay neiljay force-pushed the pr/add_mips-pic32_mz branch from 27ed5fc to e2d7d36 Compare March 13, 2017 15:42
@neiljay
Copy link
Contributor Author

neiljay commented Mar 14, 2017

@gebart @OlegHahm @haukepetersen Can we finally get this ACK'd and merged ? :-)

@jnohlgard
Copy link
Member

I don't have any of the boards to test it. I'm fine with the change as long as Murdock is fine.

@neiljay
Copy link
Contributor Author

neiljay commented Mar 14, 2017

@gebart Cheers, can you ACK your change-request then. @OlegHahm has a board we donated.

@jnohlgard jnohlgard added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 14, 2017
Copy link
Member

@jnohlgard jnohlgard left a comment

Choose a reason for hiding this comment

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

Clearing my request for comments since my comments were addressed. I have not reviewed the code or tested this on an actual board though. Leaving the formal ACK to @OlegHahm and colleagues.

@neiljay
Copy link
Contributor Author

neiljay commented Mar 16, 2017

Please can someone merge this @OlegHahm @haukepetersen @miri64 @kaspar030 :-)

@kYc0o
Copy link
Contributor

kYc0o commented Mar 17, 2017

This code is very similar to #6092, so I'll have more or less the same comments. I'd like to have the comments addressed before merging this one too...

@neiljay neiljay force-pushed the pr/add_mips-pic32_mz branch from d70b7ff to d3a0c76 Compare March 21, 2017 14:50
@neiljay
Copy link
Contributor Author

neiljay commented Mar 21, 2017

Can some one re-run the CI Please, I don't believe the error.

@kaspar030
Copy link
Contributor

Can some one re-run the CI Please, I don't believe the error.

Murdock (not Murdock2) is our current last-word, so this PR basically passed CI.

@neiljay
Copy link
Contributor Author

neiljay commented Mar 21, 2017

Cheers, Can we get this merged at last ?



/*
* Without a reference to this function from elsewhere LD throws the whole
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably has to do with missing '--whole-archive'. Could you please check if #5757 removes the need for this?

Copy link
Contributor

Choose a reason for hiding this comment

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

(well, let's not delay the PR because of this. can be checked at a later time.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi,

Yes that has helped cheers, when #5757 gets merged I'll happily remove this.

Neil

Neil Jones added 2 commits March 24, 2017 13:44
Note this is only supported in unvectored mode currently.
specific support for the pic32mz2046efg100 is added along with code common to
all pic32 devices and all pic32mz devices.
@neiljay neiljay force-pushed the pr/add_mips-pic32_mz branch from 4cd9789 to 489e404 Compare March 24, 2017 13:45
Neil Jones added 4 commits March 28, 2017 16:25
This board features a pic32mz2048efg100 PIC32 device with a MIPS core.
Extened the temporary workaround for mips boards to all mips boards
until pr#6639 is merged.
@neiljay neiljay force-pushed the pr/add_mips-pic32_mz branch from 7666577 to 0bafa33 Compare March 28, 2017 15:26
@neiljay
Copy link
Contributor Author

neiljay commented Mar 29, 2017

Now squashed some more, Please Please Please can we finally get this merged.....

Copy link
Contributor

@haukepetersen haukepetersen left a comment

Choose a reason for hiding this comment

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

I think there are still some things that can substantially be simplified und unified, but this can be done later -> so ACK

@haukepetersen
Copy link
Contributor

Murdock is happy, and we got two ACKs -> lets go

@neiljay thanks a lot for all the efforts and your patience!

@haukepetersen haukepetersen merged commit 76bae4b into RIOT-OS:master Mar 29, 2017
@neiljay
Copy link
Contributor Author

neiljay commented Mar 29, 2017

Thanks everyone!!
Now for pic32mx...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: major The PR changes a significant part of the code base. It should be reviewed carefully Platform: MIPS Platform: This PR/issue effects MIPS-based platforms Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants