isp(4) - kernel panic on initialization of driver

[Alexander Sack]

On Tue, Sep 2, 2008 at 12:52 PM, Ross <westr at connection.ca> wrote:
>
> AS> I think your doing some great work but I don't think this is the
> AS> *right* direction to take.  The bottom line is the ISP should have
> AS> interrupts disabled until it completes a full reset and loads the
> AS> firmware, period.  You shouldn't have to ignore ASYNC events during a
> AS> reset - that doesn't make sense to me....yet....!
>
> I've created a small patch that so far seems to be working (survived
> ~10+ reboots). From what I can tell, using the ISP_ENABLE_INTS &
> ISP_DISABLE_INTS functions won't do anything to stop the problem.

Yes Ross, I just discovered this myself that AYNC events are let
through to the host regardless of ISP_ENABLE_INTS.

> Basically my hypotheses is that the ASYNC command is already sitting
> around in the mailbox of the card waiting to be read, so no interrupt
> is actually being generated during the time the driver is starting up
> - so you can't disable it.

That's right but the ISP should be reset.  YWe actually issue mailbox
commands BEFORE we do the RISC reset.  I think that maybe ultimately
the bug.

> (The card is active with a valid running rom before freebsd gets it's
> paws on it, so it's probably already cleanly read it, but hasn't acted
> upon it)

Yes I believe your right.

> The crash is located with the very first mailbox read, where if it
> doesn't recognize the mailbox response, it parses it anyways
> (in case it's something that needs to be done), and exit out if
> there's an error.
>
> But the isp_parse_async() function makes the assumption that isp_state
> == ISP_RUNSTATE, so it's safe to do anything. Where in fact is
> actually gets called when not in ISP_RUNSTATE (with the assumption
> that no problematic async command could be generated at this point).
>
> I'm guessing the true fix would be upon startup to somehow make sure
> the mailbox queue is emptied before attempting to query the card
> (for the firmware version).  But how to do that is beyond me.

Are you ok with trying another patch to maybe do exactly that?  Let me
take a look at your patch as well.

Thanks!

-aps