[RCFC] LPC4322 SWD-SGPIO. #907

groleo · 2021-12-07T21:02:15Z

Requires #906.

This p-r is replacing the bit-banged SWD implementation with one which uses the lpc4322's SGPIO.
Transfer rate reported by IAR is around 380 kB/sec with DAP.nominal_clock @30mhz and SGPIO_CLK@120MHz.
Maximum I could get was 420 kB/sec with DAP.nominal_clock @40MHz and SGPIO_CLK @204MHz;

testing in progress.

groleo · 2021-12-07T21:23:54Z

v2:

fixed the lpc4322_bl target by defining SWDP_SGPIO per-board (records/board/mimxrt1170_evk_qspi.yaml)
use DAP_Data.nominal_clock instead of infering it from DAP_Data.clock_delay.

groleo · 2021-12-08T09:10:50Z

v3:

fix DAP_Info for debuggers not yet supporting cmsis-dap v2.1.

v2:

fixed the lpc4322_bl target by defining SWDP_SGPIO per-board (records/board/mimxrt1170_evk_qspi.yaml)
use DAP_Data.nominal_clock instead of infering it from DAP_Data.clock_delay.

groleo · 2021-12-08T21:36:31Z

source/hic_hal/nxp/lpc4322/swdp_sgpio.c

+// SHIFT_CLOCK = DAP_Data.nominal_clock
+static uint32_t shift_clk_preset()
+{
+    return SystemCoreClock/DAP_Data.nominal_clock - 1;


This should be using PLL1. This is how I've seen it used in the sgpio_uart NXP's Application Notes.
At the moment I'm trying to make CGU aware of CGU_PERIPHERAL_SGPIO, which is mentioned in the User Manual but
nowhere to be found in lpc43xx_cgu.c.

done. using PLL0AUDIO

groleo · 2021-12-08T21:38:34Z

source/hic_hal/nxp/lpc4322/swdp_sgpio.c

+    ////////////////////////////////////////
+    ////////////////////////////////////////
+    // handle DAP_TRANSFER_FAULT
+    // check parity


Add parity check and return DAP_TRANSFER_ERROR on mismatch.

groleo · 2021-12-08T21:39:34Z

source/hic_hal/nxp/lpc4322/swdp_sgpio.c

+
+// bits IN -> SLICE_H -> SLICE_P
+static void sequence_read(uint32_t bits_sz, uint8_t *data)
+{


Proper implementation is needed on this one.

mathias-arm · 2021-12-08T22:47:31Z

@groleo: Can you put [8774a9e] in a separate PR?

groleo · 2021-12-09T06:30:09Z

@groleo: Can you put [8774a9e] in a separate PR?

Done

groleo · 2022-01-06T13:36:24Z

v4:
add sequence_read().
proper SGPIO_CLK initialization and setup.
add idle_cyles.
explicitly set the P1_17 mode so that it's visible it's a Hard-Drive pin (doesn't support slew-rate programming and it's driven @4ma)

elfmimi

Thank you @groleo this is awesome. I tried your code on LPC-Link2 (LPC4370) and it was fun to tinker around with SGPIO peripheral.
So far I've found 3 concerns. 2 of them are rather trivial. please see my suggested changes.

The real problem I'm experiencing is that I can't run it slowly, which is unexpected.
When SGPIO_CLK is at 120 MHz it runs no problem at SWD clock rate of 30 MHz. like you have explained. And it seems it's fine to lower the clock rate down to 3 MHz. But beyond that it starts to fail. Using non SGPIO version of DAPLink it can go just down to 1 kHz .

elfmimi · 2022-02-19T01:33:22Z

source/hic_hal/nxp/lpc4322/swdp_sgpio.c

+    SLICE_POS(SLICE_SWDIO_TXEN) = POS_POS(period) | POS_PRESET(0);
+    SLICE_REG(SLICE_SWDIO_TXEN) = 0b00000000000000000000000000000000;
+    SLICE_SHD(SLICE_SWDIO_TXEN) = 0;
+    SLICE_CNT(SLICE_SWDIO_TXEN) = 0;
+
+    SLICE_POS(SLICE_SWDIO_TMS_OE) = POS_POS(period) | POS_PRESET(0);
+    SLICE_REG(SLICE_SWDIO_TMS_OE) = 0b00000000000000000000000000000000;
+    SLICE_SHD(SLICE_SWDIO_TMS_OE) = 0;
+    SLICE_CNT(SLICE_SWDIO_TMS_OE) = 0;
+    SLICE_REG(SLICE_SWDIO_TMS_DOUT0) = 0;
+    SLICE_SHD(SLICE_SWDIO_TMS_DOUT0) = 0;


Suggested change

SLICE_POS(SLICE_SWDIO_TXEN) = POS_POS(period) | POS_PRESET(0);

SLICE_REG(SLICE_SWDIO_TXEN) = 0b00000000000000000000000000000000;

SLICE_SHD(SLICE_SWDIO_TXEN) = 0;

SLICE_CNT(SLICE_SWDIO_TXEN) = 0;

SLICE_POS(SLICE_SWDIO_TMS_OE) = POS_POS(period) | POS_PRESET(0);

SLICE_REG(SLICE_SWDIO_TMS_OE) = 0b00000000000000000000000000000000;

SLICE_SHD(SLICE_SWDIO_TMS_OE) = 0;

SLICE_CNT(SLICE_SWDIO_TMS_OE) = 0;

SLICE_REG(SLICE_SWDIO_TMS_DOUT0) = 0;

SLICE_SHD(SLICE_SWDIO_TMS_DOUT0) = 0;

// contents of registers are directly reflected to corresponding signals.

SLICE_REG(SLICE_SWDIO_TXEN) = 0b00000000000000000000000010000000;

SLICE_REG(SLICE_SWDIO_TMS_OE) = 0b00000000000000000000000000000001;

SLICE_REG(SLICE_SWDIO_TMS_DOUT0) = 0;

elfmimi · 2022-02-19T01:33:45Z

source/hic_hal/nxp/lpc4322/swdp_sgpio.c

+    if (info & SWD_SEQUENCE_DIN) {
+        sequence_read(bytes_nb*8, swdi);
+    } else {
+        sequence_send(bytes_nb*8, swdo);
+    }


Suggested change

if (info & SWD_SEQUENCE_DIN) {

sequence_read(bytes_nb*8, swdi);

} else {

sequence_send(bytes_nb*8, swdo);

}

if (info & SWD_SEQUENCE_DIN) {

sequence_read(bytes_nb, swdi);

} else {

sequence_send(bytes_nb, swdo);

}

sequence_send() takes a bits_sz (bits size) as first parameter. I can replace the 8 to BITS_IN_CHAR to clarify the intent.

info & SWD_SEQUENCE_CLK specifies clock cycles , so it is bits , not bytes . I could have renamed bytes_nb to bits_nb . anyway naming is up to you.

Now i get what you mean (thanks for taking a good look at this).
I'll change bytes_nb to bits_nb.

elfmimi · 2022-04-09T02:02:56Z

source/hic_hal/nxp/lpc4322/swdp_sgpio.c

+    SLICE_SHD(SLICE_SWDIO_TXEN) = 0;
+
+    uint32_t bits_per_chunk = bits_sz<=64 ? bits_sz : 64;
+    SLICE_POS(SLICE_SWDIO_TMS_DIN1) = POS_POS(bits_per_chunk) | POS_PRESET(bits_per_chunk);


Suggested change

SLICE_POS(SLICE_SWDIO_TMS_DIN1) = POS_POS(bits_per_chunk) | POS_PRESET(bits_per_chunk);

elfmimi · 2022-04-09T02:03:23Z

source/hic_hal/nxp/lpc4322/swdp_sgpio.c

+
+    if (bits_sz <= BYTES_PER_SLICE*8)
+    {
+        slices_used = SLICE_SWDIO_TMS_DIN0;


Suggested change

slices_used = SLICE_SWDIO_TMS_DIN0;

slices_used = SLICE_SWDIO_TMS_DIN0_MASK;

elfmimi · 2022-04-09T02:03:40Z

source/hic_hal/nxp/lpc4322/swdp_sgpio.c

+    }
+    else
+    {
+        slices_used = SLICE_SWDIO_TMS_DIN0 | SLICE_SWDIO_TMS_DIN1;


Suggested change

slices_used = SLICE_SWDIO_TMS_DIN0 | SLICE_SWDIO_TMS_DIN1;

slices_used = SLICE_SWDIO_TMS_DIN0_MASK | SLICE_SWDIO_TMS_DIN1_MASK;

elfmimi · 2022-04-09T02:04:28Z

source/hic_hal/nxp/lpc4322/swdp_sgpio.c

+        uint32_t bits_remaining = bits_sz - bits_it;
+        uint32_t bytes_remaining = (bits_remaining + 7) / 8;
+        uint32_t slice_it = 0;
+        uint32_t slices_used = 0;


Suggested change

uint32_t slices_used = 0;

elfmimi · 2022-04-09T02:04:56Z

source/hic_hal/nxp/lpc4322/swdp_sgpio.c

+    // Disable TXEN.
+    SLICE_POS(SLICE_SWDIO_TXEN) = POS_POS(0) | POS_PRESET(0);
+    SLICE_CNT(SLICE_SWDIO_TXEN) = 0;
+    SLICE_REG(SLICE_SWDIO_TMS_OE) = 0;


Suggested change

SLICE_REG(SLICE_SWDIO_TMS_OE) = 0;

SLICE_REG(SLICE_SWDIO_TXEN) = 0;

elfmimi · 2022-04-09T02:19:26Z

I'm experimenting reconfiguration of PLL to match required SWD frequency. Based on your code. It can run at SWD clock speed ranging 50MHz down-to 1kHz .
https://github.com/elfmimi/DAPLink/tree/lpc4370_sgpio_pll_sequence_fix_if_wip

Signed-off-by: Adrian Negreanu <adrian.negreanu@nxp.com>

elfmimi · 2022-04-24T06:20:07Z

Current status of this pull-request.

Build the interface firmware from pr's head commit 9f14137
progen generate -p lpc4322_mimxrt1170_evk_qspi_if -t make_gcc_arm -b -o jobs=8
Install the interface firmware using the bootloader firmware lpc4322_bl
Dev board I'm using has different target MCU. But it's OK for testing pyOCD access.
Results
- pyOCD can connect to the onboard target LPC43S67 but only when frequency setting is high enough.
  python -mpyocd cmd -t cortex_m -f 3000000
- pyOCD can connect to external targets. for example RaspberryPi Pico - RP2040 , but with the same frequency constraint.
  python -mpyocd cmd -t rp2040 -f 3000000
- pyOCD can not connect to some external targets. for example nRF52840.

elfmimi

I wasn't sure until now if this change is needed or not.
Now I had a chance to test with and without this change, and I'm convinced that this change fixes stability when connecting with some target , ex nRF52840 .

elfmimi · 2022-04-24T06:24:27Z

source/hic_hal/nxp/lpc4322/swdp_sgpio.c

+#define SGPIO_SWDIO_TXEN                         15
+#define SLICE_SWDIO_TXEN                         SLICE_M


Suggested change

#define SGPIO_SWDIO_TXEN 15

#define SLICE_SWDIO_TXEN SLICE_M

#define SGPIO_SWDIO_TXEN 15

#define SGPIO_SWDIO_TXEN_DOUTM8_BIT 7

#define SLICE_SWDIO_TXEN SLICE_M

elfmimi · 2022-04-24T06:25:58Z

source/hic_hal/nxp/lpc4322/swdp_sgpio.c

+    uint32_t parity;
+
+    SLICE_POS(SLICE_SWDIO_TXEN) = POS_POS(8) | POS_PRESET(0);
+    SLICE_REG(SLICE_SWDIO_TXEN) = 0b00000000000000001111111111111111;


Suggested change

SLICE_REG(SLICE_SWDIO_TXEN) = 0b00000000000000001111111111111111;

SLICE_REG(SLICE_SWDIO_TXEN) = 0b11111111 << SGPIO_SWDIO_TXEN_DOUTM8_BIT;

elfmimi · 2022-04-24T06:26:31Z

source/hic_hal/nxp/lpc4322/swdp_sgpio.c

+    uint32_t pkt[2];
+
+    SLICE_POS(SLICE_SWDIO_TXEN) = POS_POS(8) | POS_PRESET(0);
+    SLICE_REG(SLICE_SWDIO_TXEN) = 0b00000000000000001111111111111111;


Suggested change

SLICE_REG(SLICE_SWDIO_TXEN) = 0b00000000000000001111111111111111;

SLICE_REG(SLICE_SWDIO_TXEN) = 0b11111111 << SGPIO_SWDIO_TXEN_DOUTM8_BIT;

elfmimi · 2022-04-24T06:50:36Z

source/hic_hal/nxp/lpc4322/swdp_sgpio.c

+    slice_pos_disable(SLICE_SWDIO_TMS_DOUT0_MASK | SLICE_SWDIO_TMS_DOUT1_MASK | SLICE_SWCLK_TCK_MASK | SLICE_SWDIO_TXEN_MASK | SLICE_SWDIO_TMS_OE_MASK);
+
+    SLICE_REG(SLICE_SWDIO_TMS_OE) = 1;
+    SLICE_REG(SLICE_SWDIO_TXEN) = 1;


Suggested change

SLICE_REG(SLICE_SWDIO_TXEN) = 1;

SLICE_REG(SLICE_SWDIO_TXEN) = 1 << SGPIO_SWDIO_TXEN_DOUTM8_BIT;

elfmimi · 2022-04-24T06:51:39Z

source/hic_hal/nxp/lpc4322/swdp_sgpio.c

+    slice_pos_disable(SLICE_SWDIO_TMS_DIN0_MASK | SLICE_SWDIO_TMS_DIN1_MASK | SLICE_SWCLK_TCK_MASK);
+
+    SLICE_REG(SLICE_SWDIO_TMS_OE) = 1;
+    SLICE_REG(SLICE_SWDIO_TXEN) = 1;


Suggested change

SLICE_REG(SLICE_SWDIO_TXEN) = 1;

SLICE_REG(SLICE_SWDIO_TXEN) = 1 << SGPIO_SWDIO_TXEN_DOUTM8_BIT;

elfmimi · 2022-08-03T13:39:16Z

I want to see this going forward.

groleo · 2022-08-03T13:57:22Z

Hi @elfmimi ,

what testing infra do you use?

I've more or less used the on-board debug probe (on the rt1170-evk)
and many reboot/flash-daplink cycles but I would benefit from some sort
of logging/output.

I also have a OM13070 (which has a LPC4322)
if this can be used for test setup.

thx

elfmimi · 2022-08-03T14:15:43Z

Oh, I used OM13084 (interface: LPC4322 , target: LPC43S67) .
OM13070 (interface LPC4322 , target: LPC4337) should work just fine as well.

In case of OM13070 , LPCX4337_V3_SCHEMATIC_REVA3 , you can access SWD pins of LPC4322 through these test points.
Z11 = SWCLK
Z12 = GND
Z13 = SWDIO

For logging/output Segger RTT might be the best option.

elfmimi · 2022-08-04T23:27:32Z

Please have a look at this branch for how to do RTT. @groleo
https://github.com/elfmimi/DAPLink/tree/pull907-fix-and-rtt

After executing the above code, attach with pyocd to get the log.
pyocd rtt -t cortex-m -f 12000000 -a 0x10000000 -s 0x8000

elfmimi · 2022-08-06T05:58:04Z

You don't even need soldering to connect to SWD pins.

groleo · 2022-08-06T06:02:27Z

Cool.
And I should stick the SWD wires in a UART-over-USB ?

elfmimi · 2022-08-06T06:16:20Z

Yes, it's cool! No. SWD is SWD. bring in another CMSIS-DAP adapter.
RTT is a software solution which establishes logical pipe over SWD connection.

groleo · 2022-08-06T06:32:18Z

which probe should I get ?
Is the picoprobe ok ?

elfmimi · 2022-08-06T06:39:00Z

picoprobe ~~may be~~ is confirmed okay.
If you have RaspberryPi Pico to spare , try rust-dap .
pre-compiled binaries here. https://github.com/elfmimi/rust-dap/releases/tag/preview-0.1

groleo · 2022-08-06T06:42:57Z

that "may be ok" sounds worrisome (i'd like to avoid debugging the debugger).

Do you have a recommendation that's sure to work (or you use yourself) ?
thx

elfmimi · 2022-08-06T06:47:27Z

I suppose we should move this talk to daplink channel of pyOCD slack.
https://pyocd.slack.com/archives/C02GTJPENEN

groleo force-pushed the lpc4322-sgpio branch from 3456a59 to a84162b Compare December 7, 2021 21:21

mathias-arm requested review from mathias-arm and flit December 7, 2021 21:58

mathias-arm added enhancement hic in progress labels Dec 7, 2021

groleo force-pushed the lpc4322-sgpio branch from a84162b to 8774a9e Compare December 8, 2021 09:09

groleo commented Dec 8, 2021

View reviewed changes

groleo force-pushed the lpc4322-sgpio branch from 8774a9e to bc58485 Compare January 6, 2022 13:33

elfmimi reviewed Feb 19, 2022

View reviewed changes

elfmimi suggested changes Apr 9, 2022

View reviewed changes

[rfc] lpc4322: swd using sgpio

3a00411

Signed-off-by: Adrian Negreanu <adrian.negreanu@nxp.com>

groleo force-pushed the lpc4322-sgpio branch from a1e25e5 to 1b539ac Compare April 9, 2022 04:57

Adrian Negreanu added 3 commits April 9, 2022 08:04

sgpio: a_p_u_r_o suggestions, part 1

50f4295

sgpio: a_p_u_r_o suggestions, part 2

aec0d97

lpc4322(scu): pinmux mode needs more than 8bits

9f14137

groleo force-pushed the lpc4322-sgpio branch from 1b539ac to 9f14137 Compare April 9, 2022 05:04

elfmimi suggested changes Apr 24, 2022

View reviewed changes

elfmimi reviewed Apr 24, 2022

View reviewed changes

	slices_used = SLICE_SWDIO_TMS_DIN0;
	slices_used = SLICE_SWDIO_TMS_DIN0_MASK;

	slices_used = SLICE_SWDIO_TMS_DIN0 \| SLICE_SWDIO_TMS_DIN1;
	slices_used = SLICE_SWDIO_TMS_DIN0_MASK \| SLICE_SWDIO_TMS_DIN1_MASK;

	SLICE_REG(SLICE_SWDIO_TMS_OE) = 0;
	SLICE_REG(SLICE_SWDIO_TXEN) = 0;

	SLICE_REG(SLICE_SWDIO_TXEN) = 0b00000000000000001111111111111111;
	SLICE_REG(SLICE_SWDIO_TXEN) = 0b11111111 << SGPIO_SWDIO_TXEN_DOUTM8_BIT;

	SLICE_REG(SLICE_SWDIO_TXEN) = 1;
	SLICE_REG(SLICE_SWDIO_TXEN) = 1 << SGPIO_SWDIO_TXEN_DOUTM8_BIT;

[RCFC] LPC4322 SWD-SGPIO. #907

Are you sure you want to change the base?

[RCFC] LPC4322 SWD-SGPIO. #907

Conversation

groleo commented Dec 7, 2021 • edited

groleo commented Dec 7, 2021

groleo commented Dec 8, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mathias-arm commented Dec 8, 2021

groleo commented Dec 9, 2021

groleo commented Jan 6, 2022 • edited

elfmimi left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elfmimi commented Apr 9, 2022 • edited

elfmimi commented Apr 24, 2022

elfmimi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elfmimi commented Aug 3, 2022

groleo commented Aug 3, 2022

elfmimi commented Aug 3, 2022 • edited

elfmimi commented Aug 4, 2022 • edited

elfmimi commented Aug 6, 2022 • edited

groleo commented Aug 6, 2022

elfmimi commented Aug 6, 2022

groleo commented Aug 6, 2022 • edited

elfmimi commented Aug 6, 2022 • edited

groleo commented Aug 6, 2022

elfmimi commented Aug 6, 2022

groleo commented Dec 7, 2021 •

edited

groleo commented Jan 6, 2022 •

edited

elfmimi left a comment •

edited

elfmimi commented Apr 9, 2022 •

edited

elfmimi commented Aug 3, 2022 •

edited

elfmimi commented Aug 4, 2022 •

edited

elfmimi commented Aug 6, 2022 •

edited

groleo commented Aug 6, 2022 •

edited

elfmimi commented Aug 6, 2022 •

edited