linux-kernel - RE: [PATCH v1 2/2] usb:cdns3 Add Cadence USB3 DRD Driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 12 Dec 2018 07:38:11 +0000
From:   Peter Chen <peter.chen@....com>
To:     Felipe Balbi <balbi@...nel.org>,
        Peter Chen <hzpeterchen@...il.com>,
        "pawell@...ence.com" <pawell@...ence.com>
CC:     "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "rogerq@...com" <rogerq@...com>,
        lkml <linux-kernel@...r.kernel.org>,
        "adouglas@...ence.com" <adouglas@...ence.com>,
        "jbergsagel@...com" <jbergsagel@...com>,
        "nsekhar@...com" <nsekhar@...com>, "nm@...com" <nm@...com>,
        "sureshp@...ence.com" <sureshp@...ence.com>,
        "pjez@...ence.com" <pjez@...ence.com>,
        "kurahul@...ence.com" <kurahul@...ence.com>
Subject: RE: [PATCH v1 2/2] usb:cdns3 Add Cadence USB3 DRD Driver

 
> >> >> +                                           USB_CMD_STMODE |
> >> >> +
> >> >> + USB_STS_TMODE_SEL(tmode - 1));
> >> >
> >> >I'm 90% sure this won't work. There's a reason why we only enter the
> >> >requested test mode from status stage. How have you tested this?
> >>
> >
> > What's the reason?
> > It can work although the code is a little different with above, I
> > tested it using USBxHSETT tool at Windows.
> 
> put a sniffer. Status stage won't complete
> 

Ok, you mean sending test packet during the status completion handler?
I will see whether the status stage can complete next time when doing test.

> >> >> +    irqreturn_t ret = IRQ_NONE;
> >> >> +    unsigned long flags;
> >> >> +    u32 reg;
> >> >> +
> >> >> +    priv_dev = cdns->gadget_dev;
> >> >> +    spin_lock_irqsave(&priv_dev->lock, flags);
> >> >
> >> >you're already running in hardirq context. Why do you need this lock
> >> >at all? I would be better to use the hardirq handler to mask your
> >> >interrupts, so they don't fire again, then used the top-half
> >> >(softirq) handler to actually handle the interrupts.
> >>
> >
> > This controller may be ran at SMP environment, register and flag
> > access needs to be protected among CPUs running.
> 
> in hardirq context? When interrupts are already disabled?

Interrupt handler (hardirq context) at CPU0, and process at CPU1, eg role switch, unload module, etc.

Peter