[resend, went to wrong list address]
Manfred, thanks for your comments.
[explicit Cc to jamal becuase of NAPI-related stuff below]
On Sat, Sep 25, 2004 at 10:39:29PM +0200, Manfred Spraul wrote:
> >+config NATSEMI_NAPI
> >+ bool "Use Rx Polling (NAPI) (EXPERIMENTAL)"
> >
> I'm not a big fan on config options for drivers. I'd prefer a runtime
> option.
I see.
> > TODO:
> > * big endian support with CFG:BEM instead of cpu_to_le32
> > * support for an external PHY
> >- * NAPI
> >
> >
> Hmm. Actually all TODO points are done: CFG:BEM is unusable because it
> swaps data words.
> External PHYs are supported. You are closing the last point.
Ok, I'll remove all three with my next patch
> Additional registers. You didn't mention that in the changelog.
> Changelogs should mention every change, please do not piggypack
> unrelated changes.
ack.
> >+ long ioaddr = dev->base_addr;
> >+ int boguscnt = max_interrupt_work;
> >+
> This doesn't compile: you've removed max_interrupt_work.
mh, apparently I've sent an outdated patch then. I'll repost an updated
version once I know what the solution for IntrStatus is
> >+ /* We cannot read IntrStatus since this would acknowledge
> >+ * all interrupt sources. Thus we just blindly assume that
> >+ * the interrupt really was for us -HW! */
> Huge problem - this is not acceptable. It means the driver is unusable
> for shared interrupts. We must find a solution.
yes, I know :( Luckily on my embedded boards, there are no shared
interrupts - but this is obviously a special case.
I think this overall problem can be solved if there was some per-device
variable that saves the IntrStatus until the NAPI callback gets
scheduled. What do you think? This wouldn't even need some locking,
since interrupts would be disabled before the field is updated, and not
re-enabled before the field is read by the NAPI callback?
I was surprised that this solution is not suggested in the NAPI-HOWTO.txt, so I
though there must be an error in my proposal...
By using such a scheme, isn't it also possible to only offload RX into
the NAPI callback with clear-on-read devices?
> IRQ_NONE would be definitively wrong: The kernel disables interrupt
> sources if it can't find a handler. If our handler returns IRQ_NONE and
> the irq is not shared, then the kernel will disable the irq after a
> short while.
ok.
> Also missing in the changelog.
is related to the register definitions above.
> Manfred
--
- Harald Welte <laforge@xxxxxxxxxxxx> http://www.gnumonks.org/
============================================================================
Programming is like sex: One mistake and you have to support it your lifetime
signature.asc
Description: Digital signature
|