Skip to content

Add SWO support for MIMXRT1170-EVK#712

Closed
groleo wants to merge 16 commits intoARMmbed:mainfrom
groleo:master
Closed

Add SWO support for MIMXRT1170-EVK#712
groleo wants to merge 16 commits intoARMmbed:mainfrom
groleo:master

Conversation

@groleo
Copy link

@groleo groleo commented May 1, 2020

This p-r is adding support for NXP's i.MXRT1170-EVK board
and enables SWO for it.

@groleo
Copy link
Author

groleo commented May 1, 2020

@groleo groleo changed the title Add MIMXRT1170-EVK board support Add SWO support for MIMXRT1170-EVK May 5, 2020
@0xc0170
Copy link
Contributor

0xc0170 commented Jun 2, 2020

/morph test

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 2, 2020

I'll review this PR today or tomorrow

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 3, 2020

/morph test

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 3, 2020

There's build error:

04:41:15 Building project daplink (lpc4322_mimxrt1170_evk_qspi_if, ARM)
04:41:15 Scan: daplink
04:41:15 Compile [  1.3%]: mimxrt1170_evk.c
04:41:15 Compile [  2.6%]: HardFault_Handler.c
04:41:15 Compile [  3.8%]: circ_buf.c
04:41:15 Compile [  5.1%]: DAP_queue.c
04:41:15 Compile [  6.4%]: JTAG_DP.c
04:41:15 Compile [  7.7%]: SWO.c
04:41:15 [Error] SWO.c@31,27:  #5: cannot open source input file "Driver_USART.h": No such file or directory
04:41:16 [ERROR] ".\source\daplink\cmsis-dap\SWO.c", line 31: Error:  #5: cannot open source input file "Driver_USART.h": No such file or directory
04:41:16   #include "Driver_USART.h"

This issue is related (the last comment): #651 (comment)

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 3, 2020

Can we reuse the files from KL26 or move them as suggested in the issue?

@groleo
Copy link
Author

groleo commented Jun 3, 2020

There's build error:

fixed. I forgot to add the RTE_Driver files.

@groleo
Copy link
Author

groleo commented Jun 3, 2020

Can we reuse the files from KL26 or move them as suggested in the issue?

Sure. Let me know where you want to move the Driver_Common.h and Driver_USART.h headers from K26F HIC folder.

I don't know if the RTE_Driver files I've added should also be common.

@askyuan
Copy link

askyuan commented Jun 3, 2020

study

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 3, 2020

/morph test


/// Indicate that UART Serial Wire Output (SWO) trace is available.
/// This information is returned by the command \ref DAP_Info as part of <b>Capabilities</b>.
#define SWO_UART 1 ///< SWO UART: 1 = available, 0 = not available
Copy link
Collaborator

Choose a reason for hiding this comment

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

The DAP_config.h file should be shared between all uses of the LPC4322, not specific to a target. If you want to customize per target, you can wrap this line with #if !defined(SWO_UART), then define that macro in the target project yaml.

Comment on lines +6 to +12
- source/hic_hal/nxp/lpc4322/RTE_Driver
- source/hic_hal/nxp/lpc4322/RTE_Driver/Config
sources:
lpc4322:
- source/hic_hal/nxp/lpc4322/RTE_Driver/USART_LPC43xx.c
- source/hic_hal/nxp/lpc4322/RTE_Driver/GPDMA_LPC43xx.c
- source/hic_hal/nxp/lpc4322/RTE_Driver/SCU_LPC43xx.c
Copy link
Collaborator

@flit flit Jun 11, 2020

Choose a reason for hiding this comment

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

Please add these sources and includes to the lpc4322.yaml so they are common to all LPC4322 builds. Specifically, we want the generic lpc4322_if project to support SWO.

@flit
Copy link
Collaborator

flit commented Jun 11, 2020

Hi @groleo, would you mind removing the RTE_Driver sources that aren't used? To answer your question, these files are specific to the LPC4322, so should stay under that folder.

And yes, please move the Driver_Common.h and Driver_USART.h headers. For now, please place them directly under source/hic_hal/. Or do you think source/cmsis-driver/ would be better?

@flit
Copy link
Collaborator

flit commented Jun 11, 2020

cc @mmahadevan108

@groleo
Copy link
Author

groleo commented Jun 11, 2020 via email

@flit
Copy link
Collaborator

flit commented Jun 14, 2020

You don't need to create a new PR (please don't). Just force push (assuming you even need to) from the same branch, and it will update this PR.

Let's put the CMSIS-Driver headers under source/cmsis-driver as you prefer.

@0xc0170

This comment has been minimized.

1 similar comment
@0xc0170
Copy link
Contributor

0xc0170 commented Jun 29, 2020

/morph test

@groleo
Copy link
Author

groleo commented Jul 3, 2020

The ci-morph "Details" link is pointing to an internal IP:

 http://10.118.80.30:8080/job/daplink_build_mbedcli_pr/257/

Is there another way I can see what went wrong ?
thx

@groleo
Copy link
Author

groleo commented Aug 10, 2020

v2: CMSIS-Driver headers under source/cmsis-driver as you prefer.
v3: reorder commits (first reorder the headers, then add platform specific changes)

@0xc0170
Copy link
Contributor

0xc0170 commented Aug 11, 2020

/morph test

@groleo
Copy link
Author

groleo commented Aug 12, 2020

/morph test

Is there a way to see the CI failures ?

@0xc0170
Copy link
Contributor

0xc0170 commented Aug 12, 2020

It's internal server, I'll find out now

@0xc0170
Copy link
Contributor

0xc0170 commented Aug 12, 2020

10:56:38 Compile [ 50.0%]: SCU_LPC43xx.c
10:56:38 [Error] SCU_LPC43xx.c@33,26:  #5: cannot open source input file "SCU_LPC43xx.h": No such file or directory
10:56:39 [ERROR] ".\source\hic_hal\nxp\lpc4322\RTE_Driver\SCU_LPC43xx.c", line 33: Error:  #5: cannot open source input file "SCU_LPC43xx.h": No such file or directory
10:56:39   #include "SCU_LPC43xx.h"
10:56:39                           ^
10:56:39 .\source\hic_hal\nxp\lpc4322\RTE_Driver\SCU_LPC43xx.c: 0 warnings, 1 error

This is the building error.

Comment on lines +4 to +5
- SWO_UART=1
- SWO_USART_PORT=1
Copy link
Collaborator

@flit flit Jan 6, 2021

Choose a reason for hiding this comment

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

Is there a reason to restrict enabling SWO to only the RT1170-EVK and not enable on all LPC4322 boards? I know the LPCXpresso55S69 has SWO connected to the LPC4322.

Copy link
Author

Choose a reason for hiding this comment

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

the only reason would be that I'm only testing on the RT1170-EVK board.
If you say I can enable SWO for other boards, it's ok with me, I can add a patch for those too.

* @brief board ID for the NXP MIMXRT1170-EVK board.
*
* DAPLink Interface Firmware
* Copyright (c) 2009-2020, ARM Limited, All Rights Reserved
Copy link
Collaborator

@flit flit Jan 6, 2021

Choose a reason for hiding this comment

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

Please change the copyright to your own (or NXP's, as appropriate) for 2020-2021. The same applies to all other new files you added.

Copy link
Author

Choose a reason for hiding this comment

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

fixed as suggested.

static bool buf_empty;
static bool current_sector_valid;
static bool page_erase_enabled = false;
static bool page_erase_enabled = true;
Copy link
Collaborator

@flit flit Jan 6, 2021

Choose a reason for hiding this comment

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

I'm slightly concerned about making this change globally, only because most of the flash algos have not been tested in page erase mode. You can use set the kEnablePageErase flag in the board_info_t struct to set it per board (see frdmk64f.c for an example). It's probably safe to change it globally, but that would better be done in a separate PR.

Copy link
Author

Choose a reason for hiding this comment

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

added kEnablePageErase to source/board/mimxrt1170_evk.c.

Comment on lines +50 to +64
#include "target_config.h"
extern target_cfg_t target_device ;

if(buf[0] == 'F' && buf[1] == 'C' && buf[2] == 'F' && buf[3] == 'B')
{
target_device.flash_regions[0].start = 0x30000400;
return 1;
}
else
else if(buf[0] == 0xFF && buf[1] == 0xFF && buf[2] == 0xFF && buf[3] == 0xFF)
{
return 0;
target_device.flash_regions[0].start = 0x30000000;
return 1;
}

return 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice hack! Would you mind adding some comments describing what this code does so we don't forget in the future? 😄

Copy link
Author

Choose a reason for hiding this comment

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

From what I understand, if buf is a FlexSPI Config block (the "FCFB"/0x42464346 test), it moves the flash start address;
but I have no idea what the test against 0xFFFFFFFF part does; I see some other platforms have a "cookieList"

Copy link
Collaborator

@flit flit left a comment

Choose a reason for hiding this comment

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

The only changes requested are updating the copyrights, page erase mode default, and possibly enabling SWO on all LPC4322 based boards.

@flit
Copy link
Collaborator

flit commented Jan 6, 2021

@groleo My apologies this PR has been outstanding for so long! Honestly I haven't been paying much attention since I've been primarily focused on pyocd.

nxpyandld and others added 16 commits May 20, 2021 21:21
Signed-off-by: Alex Yang <alex.yang@nxp.com>
Signed-off-by: Adrian Negreanu <adrian.negreanu@nxp.com>
Signed-off-by: Adrian Negreanu <adrian.negreanu@nxp.com>
It provides GetClockFreq() which is needed by USART_LPC43xx.
this provides definitions needed by GetClockFreq()
Signed-off-by: Adrian Negreanu <adrian.negreanu@nxp.com>
Signed-off-by: Adrian Negreanu <adrian.negreanu@nxp.com>
This way, one can set SWO_UART from the target's yaml.

Signed-off-by: Adrian Negreanu <adrian.negreanu@nxp.com>
Signed-off-by: Adrian Negreanu <adrian.negreanu@nxp.com>
Signed-off-by: Adrian Negreanu <adrian.negreanu@nxp.com>
Signed-off-by: Adrian Negreanu <adrian.negreanu@nxp.com>
Signed-off-by: Adrian Negreanu <adrian.negreanu@nxp.com>
Signed-off-by: Adrian Negreanu <adrian.negreanu@nxp.com>
Signed-off-by: Adrian Negreanu <adrian.negreanu@nxp.com>
@mbrossard
Copy link
Contributor

Superseded by #906.

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