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.

Sign up for GitHub

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

arch/arm64/imx9 Usb driver cache #289

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
View file
Open in desktop
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 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)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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();
}

/****************************************************************************
Expand Down Expand Up @@ -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,
Copy link

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;
Expand All @@ -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);

Copy link

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);
}

/****************************************************************************
Expand All @@ -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);

Expand Down Expand Up @@ -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,
Expand All @@ -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();
}

/****************************************************************************
Expand Down Expand Up @@ -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();
}

/****************************************************************************
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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 •
edited
Loading

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);

Expand Down
Loading