[linux dev-6.6 v1] usb: chipidea: udc: enforce write to the memory.

[Tomer Maimon]

Hi Paul,

Thanks a lot for your comments, it will be addressed in the next version.

Tomer

On Wed, 10 Jul 2024 at 16:52, Paul Menzel <pmenzel at molgen.mpg.de> wrote:
>
> Dear Tomer,
>
>
> Thank you for your patch.
>
> Am 10.07.24 um 14:41 schrieb Tomer Maimon:
> > In the prime endpoint function, we need to read from qh.ptr->td.token
> > to ensure that the previous write to it has indeed been committed
> > to memory.
>
> Please document the datasheet name, revision and section. If this
> actually caused user visible problems, please also document the test case.
>
> > Signed-off-by: Tomer Maimon <tmaimon77 at gmail.com>
> > ---
> >   drivers/usb/chipidea/udc.c | 15 ++++++++++++---
> >   1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> > index 0b7bd3c643c3..0b14a1d54d59 100644
> > --- a/drivers/usb/chipidea/udc.c
> > +++ b/drivers/usb/chipidea/udc.c
> > @@ -190,12 +190,21 @@ static int hw_ep_get_halt(struct ci_hdrc *ci, int num, int dir)
> >    *
> >    * This function returns an error code
> >    */
> > -static int hw_ep_prime(struct ci_hdrc *ci, int num, int dir, int is_ctrl)
> > +static int hw_ep_prime(struct ci_hdrc *ci, struct ci_hw_ep *hwep, int num,
> > +                    int dir, int is_ctrl)
> >   {
> >       int n = hw_ep_bit(num, dir);
> >
> >       /* Synchronize before ep prime */
> >       wmb();
> > +
> > +     /*
> > +      * We add the read from qh.ptr->td.token to make sure the previous
> > +      * write to it indeed got into the mamory so when we prime the DMA
>
> m*e*mory
>
> > +      * will read the updated data
> > +      */
> > +     if (hwep->qh.ptr->td.token & 0x80000000)
> > +             pr_info("%s(): hwep->qh.ptr->td.token=%08x\n", __func__, hwep->qh.ptr->td.token);
> >
> >       if (is_ctrl && dir == RX && hw_read(ci, OP_ENDPTSETUPSTAT, BIT(num)))
> >               return -EAGAIN;
> > @@ -632,7 +641,7 @@ static int _hardware_enqueue(struct ci_hw_ep *hwep, struct ci_hw_req *hwreq)
> >               hwep->qh.ptr->cap |= cpu_to_le32(mul << __ffs(QH_MULT));
> >       }
> >
> > -     ret = hw_ep_prime(ci, hwep->num, hwep->dir,
> > +     ret = hw_ep_prime(ci, hwep, hwep->num, hwep->dir,
> >                          hwep->type == USB_ENDPOINT_XFER_CONTROL);
> >   done:
> >       return ret;
> > @@ -658,7 +667,7 @@ static int reprime_dtd(struct ci_hdrc *ci, struct ci_hw_ep *hwep,
> >       hwep->qh.ptr->td.token &=
> >               cpu_to_le32(~(TD_STATUS_HALTED | TD_STATUS_ACTIVE));
> >
> > -     return hw_ep_prime(ci, hwep->num, hwep->dir,
> > +     return hw_ep_prime(ci, hwep, hwep->num, hwep->dir,
> >                               hwep->type == USB_ENDPOINT_XFER_CONTROL);
> >   }
> >
>
>
> Kind regards,
>
> Paul