benmccrea commented on a change in pull request #2392:
URL: https://github.com/apache/mynewt-core/pull/2392#discussion_r506617502



##########
File path: hw/drivers/crypto/crypto_da1469x/src/crypto_da1469x.c
##########
@@ -73,7 +74,10 @@ do_dma_key_tx(const void *key, uint16_t keylen)
     da1469x_otp_set_mode(OTPC_MODE_READ);
 
     /* securely DMA hardware key from secret storage to QSPI decrypt engine */
-    dma_regs->DMA_REQ_MUX_REG |= 0xf000;
+    OS_ENTER_CRITICAL(sr);
+    /* Use the secure channel #7 */
+    MCU_DMA_SET_MUX(7, MCU_DMA_PERIPH_NONE);

Review comment:
       @nkaje I don't believe this will work properly. I think the 
MCU_DMA_SET_MUX macro expects to work on channel-pair requests, where the lower 
channel index of the pair is passed as cidx. 
   
   DMA_REQ_MUX_REG is a 16-bit register. If you pass cidx=7 to this macro, the 
resulting bit mask and bit set values will be shifted beyond bit 15 in the 
register:
   
   Example: cidx=7, periph=MCU_DMA_PERIPH_NONE=0xf
   (_periph) << ((_cidx) * 2)  --> left-shift 0xf by 14 bits = 
b111100000000000000 = 0x3c000
   
   I think the `MCU_DMA_SET_MUX` macro was intended to be used privately in 
da1469x_dma.c by `da1469x_dma_acquire_single()` and 
`da1469x_dma_acquire_periph()`. 
   
   Take a look at this code in `da1469x_dma_acquire_single()`:
   ```
       /*
        * DMA_REQ_MUX_REG applies only to channels < 8
        */
       if (cidx < 8) {
           MCU_DMA_SET_MUX(cidx, MCU_DMA_PERIPH_NONE);
       }
   ```
   Note that this code will not work properly for odd-numbered channels. Even 
though it is a single channel request, the lower channel # of a channel pair 
must be passed for it to work properly. 
   
   I think we need to do both of the following to really fix this issue:
   1. Fix `da1469x_dma_acquire_single()` so it properly works for both even and 
odd single-channel requests. 
   2. Use `da1469x_dma_acquire_single()` and `da1469x_dma_release_channel()` in 
the crypto code for channel 7, so that the DMA driver can function properly 
when channel 7 is in use. 
   
   
   
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to