Skip to content

Commit

Permalink
arch/arm64/src/imx9/imx9_usbdev.c: Clean up cache operations, add DEB…
Browse files Browse the repository at this point in the history 
…UGASSERTS

Correct some of the cache operations:

- EP0 request length was handled incorrectly
- Received data cache invalidate was exceeding the received buffer
- writedtd is also called with no data (EP0 ACK/NACK). Don't touch cache in that case.

Fix trip wire handling to conform with the IMX93 reference manual

Also add DEBUGASSERTS for future to check the validity of pointers and sizes

Co-authored-by: Jukka Laitinen <[email protected]>
  • Loading branch information
@joukkone @jlaitine
joukkone and jlaitine committed Oct 3, 2024
1 parent b3bdd9e commit d82c104
Showing 1 changed file with 57 additions and 30 deletions.
87 changes: 57 additions & 30 deletions arch/arm64/src/imx9/imx9_usbdev.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
#endif

#define DCACHE_LINEMASK (ARMV8A_DCACHE_LINESIZE - 1)

#define DCACHE_ALIGN_UP(a) (((a) + DCACHE_LINEMASK) & ~DCACHE_LINEMASK)
#if !defined(CONFIG_ARM64_DCACHE_DISABLE)
# define cache_aligned_alloc(s) kmm_memalign(ARMV8A_DCACHE_LINESIZE,(s))
# define CACHE_ALIGNED_DATA aligned_data(ARMV8A_DCACHE_LINESIZE)
Expand All @@ -69,6 +69,11 @@
# define CACHE_ALIGNED_DATA
#endif

/* Check that both start and length are cache-aligned */

#define IS_CACHE_ALIGNED(x,y) (((uintptr_t)(x) & DCACHE_LINEMASK) == 0 && \
((y) & DCACHE_LINEMASK) == 0)

/* Configuration ************************************************************/

#ifndef CONFIG_USBDEV_EP0_MAXSIZE
Expand Down Expand Up @@ -218,6 +223,10 @@ const struct trace_msg_t g_usb_trace_strings_intdecode[] =
};
#endif

/* Length of allocated EP0 buffer (DCACHE aligned) */

#define EP0_MAXBUFLEN 64

/* Hardware interface *******************************************************/

/* This represents a Endpoint Transfer Descriptor dQH overlay (32 bytes) */
Expand Down Expand Up @@ -528,20 +537,20 @@ static int imx9_pullup(struct usbdev_s *dev, bool enable);
*/

#ifdef CONFIG_IMX9_USBDEV_USBC1
static uint8_t g_usb0_ep0buf[64] CACHE_ALIGNED_DATA;
static uint8_t g_usb0_ep0buf[EP0_MAXBUFLEN] CACHE_ALIGNED_DATA;
static struct imx9_dqh_s g_usb0_qh[IMX9_NPHYSENDPOINTS] aligned_data(2048);
static struct imx9_dtd_s g_usb0_td[IMX9_NPHYSENDPOINTS];
#endif

#ifdef CONFIG_IMX9_USBDEV_USBC2
static uint8_t g_usb1_ep0buf[64] CACHE_ALIGNED_DATA;
static uint8_t g_usb1_ep0buf[EP0_MAXBUFLEN] CACHE_ALIGNED_DATA;
static struct imx9_dqh_s g_usb1_qh[IMX9_NPHYSENDPOINTS] aligned_data(2048);
static struct imx9_dtd_s g_usb1_td[IMX9_NPHYSENDPOINTS];
#endif

static struct imx9_usb_s g_usbdev[] =
{
#ifdef CONFIG_IMX9_USBDEV_USBC1
#ifdef CONFIG_IMX9_USBDEV_USBC1
{
.id = 0,
.base = IMX9_USB_OTG1_BASE,
Expand Down Expand Up @@ -692,10 +701,7 @@ static void imx9_putreg(struct imx9_usb_s *priv, off_t offset, uint32_t val)
static inline void imx9_modifyreg(struct imx9_usb_s *priv, off_t offset,
uint32_t clear, uint32_t set)
{
uint32_t reg = imx9_getreg(priv, offset);
reg &= ~clear;
reg |= set;
imx9_putreg(priv, offset, reg);
modifyreg32(priv->base + offset, clear, set);
}

/****************************************************************************
Expand Down Expand Up @@ -764,6 +770,8 @@ static inline void imx9_writedtd(struct imx9_dtd_s *dtd,
const uint8_t *data,
uint32_t nbytes)
{
DEBUGASSERT(IS_CACHE_ALIGNED(dtd, sizeof(*dtd)));

dtd->nextdesc = DTD_NEXTDESC_INVALID;
dtd->config = DTD_CONFIG_LENGTH(nbytes) | DTD_CONFIG_IOC |
DTD_CONFIG_ACTIVE;
Expand All @@ -776,8 +784,18 @@ static inline void imx9_writedtd(struct imx9_dtd_s *dtd,

up_clean_dcache((uintptr_t)dtd,
(uintptr_t)dtd + sizeof(struct imx9_dtd_s));
up_clean_dcache((uintptr_t)data,
(uintptr_t)data + nbytes);

/* the pointer is NULL for EP0 NAK IN/OUT */

if (data != NULL)
{
DEBUGASSERT(IS_CACHE_ALIGNED(data, DCACHE_ALIGN_UP(nbytes)));
DEBUGASSERT(nbytes <= 0x5000);

/* Data needs to be clean before TX, clean or invalid before RX */

up_clean_dcache((uintptr_t)data, (uintptr_t)data + nbytes);
}
}

/****************************************************************************
Expand All @@ -803,7 +821,7 @@ static void imx9_queuedtd(struct imx9_usb_s *priv, uint8_t epphy,
dqh->overlay.nextdesc = (uint32_t)(uintptr_t)dtd;
dqh->overlay.config &= ~(DTD_CONFIG_ACTIVE | DTD_CONFIG_HALTED);

up_flush_dcache((uintptr_t)dqh,
up_clean_dcache((uintptr_t)dqh,
(uintptr_t)dqh + sizeof(struct imx9_dqh_s));

uint32_t bit = IMX9_ENDPTMASK(epphy);
Expand Down Expand Up @@ -845,21 +863,20 @@ static void imx9_readsetup(struct imx9_usb_s *priv, uint8_t epphy,
struct imx9_dqh_s *dqh = &priv->qh[epphy];
int i;

do
{
/* Set the trip wire */
/* Set the trip wire */

imx9_modifyreg(priv, IMX9_USBDEV_USBCMD_OFFSET, 0, USBDEV_USBCMD_SUTW);
imx9_modifyreg(priv, IMX9_USBDEV_USBCMD_OFFSET, 0, USBDEV_USBCMD_SUTW);
ARM64_DSB();

up_invalidate_dcache((uintptr_t)dqh,
(uintptr_t)dqh + sizeof(struct imx9_dqh_s));
DEBUGASSERT(IS_CACHE_ALIGNED(dqh, sizeof(struct imx9_dqh_s)));
up_invalidate_dcache((uintptr_t)dqh,
(uintptr_t)dqh + sizeof(struct imx9_dqh_s));

/* Copy the request... */
/* Copy the request... */

for (i = 0; i < 8; i++)
{
((uint8_t *) ctrl)[i] = ((uint8_t *) dqh->setup)[i];
}
for (i = 0; i < 8; i++)
{
((uint8_t *) ctrl)[i] = ((uint8_t *) dqh->setup)[i];
}

while (!(imx9_getreg(priv,
Expand All @@ -869,13 +886,11 @@ static void imx9_readsetup(struct imx9_usb_s *priv, uint8_t epphy,

imx9_modifyreg(priv, IMX9_USBDEV_USBCMD_OFFSET, USBDEV_USBCMD_SUTW, 0);

up_clean_dcache((uintptr_t)dqh,
(uintptr_t)dqh + sizeof(struct imx9_dqh_s));

/* Clear the Setup Interrupt */

imx9_putreg(priv, IMX9_USBDEV_ENDPTSETUPSTAT_OFFSET,
IMX9_ENDPTMASK(IMX9_EP0_OUT));
ARM64_DSB();
}

/****************************************************************************
Expand Down Expand Up @@ -1120,8 +1135,9 @@ static void imx9_dispatchrequest(struct imx9_usb_s *priv,
{
/* Invalidate buffer data cache */

DEBUGASSERT(IS_CACHE_ALIGNED(priv->ep0.buf, EP0_MAXBUFLEN));
up_invalidate_dcache((uintptr_t)priv->ep0.buf,
(uintptr_t)priv->ep0.buf + sizeof(priv->ep0.buf));
(uintptr_t)priv->ep0.buf + EP0_MAXBUFLEN);

/* Forward to the control request to the class driver implementation */

Expand Down Expand Up @@ -1162,7 +1178,7 @@ static void imx9_ep0configure(struct imx9_usb_s *priv)
priv->qh[IMX9_EP0_IN].currdesc = DTD_NEXTDESC_INVALID;

up_clean_dcache((uintptr_t)priv->qh,
(uintptr_t)priv->qh + (sizeof(struct imx9_dqh_s) * 2));
(uintptr_t)priv->qh + (sizeof(struct imx9_dqh_s)));

/* Enable EP0 */

Expand Down Expand Up @@ -1303,6 +1319,8 @@ static inline void imx9_ep0state(struct imx9_usb_s *priv,
imx9_putreg(priv, IMX9_USBDEV_ENDPTNAKEN_OFFSET, 0);
break;
}

ARM64_DSB();
}

/****************************************************************************
Expand Down Expand Up @@ -1347,7 +1365,9 @@ static inline void imx9_ep0setup(struct imx9_usb_s *priv)
index = GETUINT16(ctrl->index);
len = GETUINT16(ctrl->len);

priv->ep0.buf_len = len;
/* Limit the ep0 length to maximum */

priv->ep0.buf_len = len <= EP0_MAXBUFLEN ? len : EP0_MAXBUFLEN;

uinfo("type=%02x req=%02x value=%04x index=%04x len=%04x\n",
ctrl->type, ctrl->req, value, index, len);
Expand Down Expand Up @@ -1868,10 +1888,9 @@ bool imx9_epcomplete(struct imx9_usb_s *priv, uint8_t epphy)

/* Make sure we have updated data after the DMA transfer. */

DEBUGASSERT(IS_CACHE_ALIGNED(dtd, sizeof(struct imx9_dtd_s)));
up_invalidate_dcache((uintptr_t)dtd,
(uintptr_t)dtd + sizeof(struct imx9_dtd_s));
up_invalidate_dcache((uintptr_t)dtd->buffer0,
(uintptr_t)dtd->buffer0 + dtd->xfer_len);

int xfrd = dtd->xfer_len - (dtd->config >> 16);

Expand All @@ -1885,6 +1904,14 @@ bool imx9_epcomplete(struct imx9_usb_s *priv, uint8_t epphy)
*/

usbtrace(TRACE_INTDECODE(IMX9_TRACEINTID_EPIN), complete);

/* Invalidate rx buffer cache */

DEBUGASSERT(IS_CACHE_ALIGNED(privreq->req.buf,
DCACHE_ALIGN_UP(privreq->req.xfrd)));
up_invalidate_dcache((uintptr_t)privreq->req.buf,
(uintptr_t)privreq->req.buf +
DCACHE_ALIGN_UP(privreq->req.xfrd));
}
else
{
Expand Down

0 comments on commit d82c104

Please sign in to comment.