Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

arch/arm64/imx9 Usb driver cache #289

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 32 additions & 22 deletions arch/arm64/src/imx9/imx9_usbdev.c
Original file line number Diff line number Diff line change
@@ -60,7 +60,7 @@
#endif

#define DCACHE_LINEMASK (ARMV8A_DCACHE_LINESIZE - 1)

#define CACHE_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)
@@ -541,7 +541,7 @@ static struct imx9_dtd_s g_usb1_td[IMX9_NPHYSENDPOINTS];

static struct imx9_usb_s g_usbdev[] =
{
#ifdef CONFIG_IMX9_USBDEV_USBC1
#ifdef CONFIG_IMX9_USBDEV_USBC1
{
.id = 0,
.base = IMX9_USB_OTG1_BASE,
@@ -696,6 +696,7 @@ static inline void imx9_modifyreg(struct imx9_usb_s *priv, off_t offset,
reg &= ~clear;
reg |= set;
imx9_putreg(priv, offset, reg);
ARM64_DSB();
}

/****************************************************************************
@@ -764,6 +765,9 @@ static inline void imx9_writedtd(struct imx9_dtd_s *dtd,
const uint8_t *data,
uint32_t nbytes)
{
up_invalidate_dcache((uintptr_t)data,

Choose a reason for hiding this comment

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

This shouldn't be needed, the same is done below

(uintptr_t)data + CACHE_ALIGN_UP(nbytes));

dtd->nextdesc = DTD_NEXTDESC_INVALID;
dtd->config = DTD_CONFIG_LENGTH(nbytes) | DTD_CONFIG_IOC |
DTD_CONFIG_ACTIVE;
@@ -776,8 +780,9 @@ 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);

Choose a reason for hiding this comment

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

I believe the original line is correct, except for CACHE_ALIGN_UP.

The *3 should not be needed

Perhaps the whole buffer should be invalidated (or cleaned) when it is allocated in imx9_epallocbuffer.

up_invalidate_dcache((uintptr_t)dtd->buffer0,
(uintptr_t)dtd->buffer0 + CACHE_ALIGN_UP(nbytes) * 3);
}

/****************************************************************************
@@ -803,8 +808,8 @@ 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,
(uintptr_t)dqh + sizeof(struct imx9_dqh_s));
up_clean_dcache((uintptr_t)dqh,
(uintptr_t)dqh + sizeof(struct imx9_dqh_s));

uint32_t bit = IMX9_ENDPTMASK(epphy);

@@ -845,21 +850,19 @@ 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));
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,
@@ -869,13 +872,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();
}

/****************************************************************************
@@ -1303,6 +1304,8 @@ static inline void imx9_ep0state(struct imx9_usb_s *priv,
imx9_putreg(priv, IMX9_USBDEV_ENDPTNAKEN_OFFSET, 0);
break;
}

ARM64_DSB();
}

/****************************************************************************
@@ -1346,7 +1349,6 @@ static inline void imx9_ep0setup(struct imx9_usb_s *priv)
value = GETUINT16(ctrl->value);
index = GETUINT16(ctrl->index);
len = GETUINT16(ctrl->len);

priv->ep0.buf_len = len;

uinfo("type=%02x req=%02x value=%04x index=%04x len=%04x\n",
@@ -1870,8 +1872,16 @@ bool imx9_epcomplete(struct imx9_usb_s *priv, uint8_t epphy)

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);
(uintptr_t)dtd->buffer0 + CACHE_ALIGN_UP(dtd->xfer_len));
Copy link

@jlaitine jlaitine Sep 25, 2024

Choose a reason for hiding this comment

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

This part is fishy at best :) Just add the CACHE_ALIGN_UP(), and remove the additional lines below.

buffer1,2,3,4 are adjacent, so if xfer_len is longer than 1 first buffer, then next ones are just invalidated


up_invalidate_dcache((uintptr_t)dtd->buffer1,
(uintptr_t)dtd->buffer1 + CACHE_ALIGN_UP(dtd->xfer_len));
up_invalidate_dcache((uintptr_t)dtd->buffer2,
(uintptr_t)dtd->buffer2 + CACHE_ALIGN_UP(dtd->xfer_len));
up_invalidate_dcache((uintptr_t)dtd->buffer3,
(uintptr_t)dtd->buffer3 + CACHE_ALIGN_UP(dtd->xfer_len));

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

Loading