DW3_QM33_SDK, 'bug' and some code optimization questions

Wireless Connectivity Ultra-Wideband
1

I am working on some extensions to the UCI support, and while debugging I see a couple of odd code optimization problems..

I’m just trying to call
uwbmac_helper_init_fira();
in task_uci.c, original code

and this in
DW3_QM33_SDK_1.0.2/SDK/Firmware/Libs/uwb-stack/libs/qhal/src/nrfx/qspi.c

	if (spi->is_master) {
#if NRFX_SPIM_ENABLED
		nrf_spim_enable(spi->type.spim.p_reg);

		const nrfx_spim_xfer_desc_t xfer_desc = { xfer->tx_buf, xfer->tx_size, xfer->rx_buf,
							  xfer->rx_size };
		result = nrfx_spim_xfer(&spi->type.spim, &xfer_desc, xfer->flags);
#else
		nrf_spi_enable(spi->type.spi.p_reg);

		if (spi->ss_active_high) {
			qgpio_pin_write(spi->cs_pin, 1);
		} else {
			qgpio_pin_write(spi->cs_pin, 0);
		}

		const nrfx_spi_xfer_desc_t xfer_desc = { xfer->tx_buf, xfer->tx_size, xfer->rx_buf,
							 xfer->rx_size };
		result = nrfx_spi_xfer(&spi->type.spi, &xfer_desc, 0);
;		
#endif /* NRFX_SPIM_ENABLED */
		if (result != NRFX_SUCCESS)  // new code
			return QERR_EIO;            // new code

		if (!spi->handler) {
			while (spi->txf_is_finished == false)
				;

either path sets result.. but result is never tested before going into the while loop
which I would guess will cause a hang..

in qplatform, there were two compilation optimization problems

static int32_t qplatform_uwb_spi_read(uint16_t header_length, uint8_t *header_buffer,
              uint16_t read_length, uint8_t *read_buffer)
{
  /* Read always bring header length on return. */
  uint32_t rxtx_size = read_length + header_length;
  uint8_t *temp_buf = temp_tx_buf;
  enum qerr r;

  if (rxtx_size > DATA_BUFFER_SIZE)
    return QERR_ENOMEM;

// old code 
	memcpy(temp_buf, header_buffer, header_length); // temp buff pointer good. 
	temp_buf += header_length;                                     
	memset(temp_buf, 0x00, read_length);                    // temp_buf == 1

// new code 
  // clear the buffer
  memset(temp_buf, 0x00, sizeof(temp_tx_buf));  

  memcpy(temp_buf, header_buffer, header_length);  // data will be 0 terminated, unless full buffer size.. same problem with prior code

// yes,  more memset cycles.

  // increment past copied data
  // temp_buf += header_length;

on entry, header_length=1, read_length=4
at the if (rxtx_size , rxtx_size = 4 ????? say what , should be 5???
at the memcpy(temp_buf, header_buffer,     temp_buf points to the variable temp_tx_buf , found in the link map at

.bss.temp_tx_buf
0x00000000200271e4 0xc8 qplatform/libqplatform.a(qplatform.c.obj)


at the memset(temp_buf, 0x00, temp_buf is 0x00000001!!!!!! <--------===========
and causes a memory access fault. 

then in nfrx_spim.c

rfx_err_t nrfx_spim_xfer(nrfx_spim_t const * const p_instance,
nrfx_spim_xfer_desc_t const * p_xfer_desc,
uint32_t flags)
{
spim_control_block_t * p_cb = &m_cb[p_instance->drv_inst_idx]; <---- this sets the p_cb pointer to 2!!

change to 

spim_control_block_t * p_cb = m_cb+p_instance->drv_inst_idx;

gives correct results. 

using 
-- The C compiler identification is GNU 10.3.1
-- The CXX compiler identification is GNU 10.3.1
(using 14.1 doesn't help)
using -o3 as provided -O0 or 1 no change

but still getting fault in

nrfx_err_t nrfx_spim_xfer(nrfx_spim_t const * const p_instance,
nrfx_spim_xfer_desc_t const * p_xfer_desc,
uint32_t flags)
{
spim_control_block_t * p_cb = m_cb+p_instance->drv_inst_idx;
NRFX_ASSERT(p_cb->state != NRFX_DRV_STATE_UNINITIALIZED);
NRFX_ASSERT(p_xfer_desc->p_tx_buffer != NULL || p_xfer_desc->tx_length == 0);
NRFX_ASSERT(p_xfer_desc->p_rx_buffer != NULL || p_xfer_desc->rx_length == 0);
NRFX_ASSERT(SPIM_LENGTH_VALIDATE(p_instance->drv_inst_idx,
p_xfer_desc->rx_length,
p_xfer_desc->tx_length));

nrfx_err_t err_code = NRFX_SUCCESS;

if (p_cb->transfer_in_progress)   <-------

0007A300 LDRB.W R3, [R12, #29]
0007A304 CMP R3, #0
0007A306 BNE.W 0x0007A46C ; <nrfx_spim_xfer>+0x184


fault code, id 1001 (access) , pc 7A304, 
R3 = 00048A8D (in flash space) 
R12 = 20035E8C that is on the stack

not in the heap

.bss.m_cb 0x0000000020035e44 0x6c Nordic/libSDK.a(nrfx_spim.c.obj)
.bss.FSb 0x0000000020035eb0 0x100 Nordic/libSDK.a(aes.c.obj)
.bss.FT0 0x0000000020035fb0 0x400 Nordic/libSDK.a(aes.c.obj)



this is on the Murata Type2AB board.

I don't see this fault if I step thru the code in ozone..

and get it in the irq_handler , 79EF0, read access is weird..  same 00048a8D address in R3

source

if (nrf_spim_event_check(p_spim, NRF_SPIM_EVENT_END))
{

#if NRFX_CHECK(NRFX_SPIM3_NRF52840_ANOMALY_198_WORKAROUND_ENABLED)

disassembly

irq_handler
$Thumb
{
if (nrf_spim_event_check(p_spim, NRF_SPIM_EVENT_END))
__STATIC_INLINE bool nrf_spim_event_check(NRF_SPIM_Type * p_reg,
return (bool)*(volatile uint32_t *)((uint8_t *)p_reg + (uint32_t)event);
00079EEC LDR.W R3, [R0, #0x0118]
00079EF0 CMP R3, #0

3

Hi @rexxdad

I’m looking at the points you listed, but in the meantime, could you download the newer SDK?

There are some code improvements, like the first sequence you mentioned from Libs/uwb-stack/libs/qhal/src/nrfx/qspi.c, the “result” check is at the end of the function, like:

	if (result != NRFX_SUCCESS)
		return QERR_EIO;

	return QERR_SUCCESS;

Maybe SDK 1.1.0 or later is not suitable for you to swap from 1.0.2 (considering you already have much work done in this version), but at least you can backport some code improvements.

Kind regards!

4

working on porting to 1.1.1

5

yes, the check is at the end, but it makes actions earlier without checking the return code on the prior calls.

the result was important enough to report an error to the caller, but still ok to do work in the mean time…

                result might be non-zero here
		if (!spi->handler) {
			while (!spi->txf_is_finished)
				;

anyhow, imho, thats bad practice for supportability..

6

this is due to
sdk_config.h
#define NRFX_SPIM3_NRF52840_ANOMALY_198_WORKAROUND_ENABLED 1

when SPIM3 is used this defaults to 1.. in the QANI package this is set to 0

7

I see,
I was checking Nordic implementations in their SDK, and they use “APP_ERROR_CHECK” straight in the function call. I’m checking the other points, and I will bring these to the development team.

Kind regards!

8

I have been working with Nordic on the SPIM3 problem, and it seems there is a known problem with SPIM3 on the 52840

if you enable the problem, bypass, then it conflicts with SD
if you disable the problem bypass, it requires special coding to use SPIM3
from my nordic conversation

Flag NRFX_SPIM3_NRF52840_ANOMALY_198_WORKAROUND_ENABLED must be set to zero.
The workaround based on a dedicated RAM block for SPIM3 must be used instead.

If SPIM3 is a must, then you will have to use the solution to use dedicated RAM block for SPIM3.

One my coworkers have made an example a few years back. I attached the project (SDK v17.1.0) and the instruction here:

devzone.nordicsemi.com/…/ble_5F00_app_5F00_beacon_5F00_spim.zip

Look at the flash placement file:
[image]

To see how this reserves one RAM block for the SPI, and then in main.c how it allocates the spim buffer to be within this block.

Then you should use this buffer for your SPI when you initialize it. The softdevice will not touch the allocated ram block.

Give it a go, and let me know how it works in your application. Please note that you can see where this RAM block ends up (this is dynamic). If the application grows too big, then it will go into ram block 8, and things may start to get messy with the callstack and so on. You should see this after compiling:

is there a reason the SDK uses spim3 (vs 2…) which would require the fixed ram buffer space (not coded in either QANI or the UCI/CLI builds)