From: Boris Brezillon
> Hi David,
>
> On Mon, 15 Dec 2014 13:34:56 +0000
> David Laight <david.lai...@aculab.com> wrote:
>
> > From: Sergei Shtylyov
> > > Hello.
> > >
> > > On 12/15/2014 4:03 PM, Boris Brezillon wrote:
> > >
> > > > Avoid interpreting useless status flags when we're not waiting for such
> > > > events by masking the status variable with the interrupt enabled
> > > > register
> > > > value.
> > >
> > > > Reported-by: Patrice VILCHEZ <patrice.vilc...@atmel.com>
> > > > Signed-off-by: Boris Brezillon <boris.brezil...@free-electrons.com>
> > > > ---
> > > > drivers/usb/gadget/udc/atmel_usba_udc.c | 6 +++++-
> > > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > > diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c
> > > > b/drivers/usb/gadget/udc/atmel_usba_udc.c
> > > > index 55c8dde..bc3a532 100644
> > > > --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
> > > > +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
> > > > @@ -1612,12 +1612,14 @@ static irqreturn_t usba_udc_irq(int irq, void
> > > > *devid)
> > > >
> > > > spin_lock(&udc->lock);
> > > >
> > > > - status = usba_readl(udc, INT_STA);
> > > > + status = usba_readl(udc, INT_STA) & usba_readl(udc, INT_ENB);
...
> > > Looks like t make sense to read the INT_ENB register into a separate
> > > variable, to save on extra reads?
> >
> >
> > Better still remember the written value in one of the structures so
> > that it doesn't have to be read at all.
>
> Hmm, I'm getting back to this suggestion.
> While I definitely understand why I should use a local variable to
> store INT_ENB value in usba_udc_irq, I don't see the point of mirroring
> INT_EN status in an udc struct field (after all, INT_EN will always
> contain the value we previously set).
This is exactly why it makes sense to mirror it locally.
> Is this a performance concern ?
Absolutely, you really don't want to know how many cpu cycles it is
likely to take to do a read from an io device.
At best it is a uncached read of a fast on-chip peripheral.
If you are reading from a PCIe device then you are looking at hundreds
(if not thousands) of cpu clock cycles.
David
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
