Re: I/O bus reset to fix CMD MSCP controllers (and probably others)

[Johnny Billquist <bqt%softjar.se@localhost>]

Hi.

On 2025-03-30 19:22, Hans Rosenfeld wrote:
> On Sat, Mar 29, 2025 at 01:20:40PM +0100, Johnny Billquist wrote:
> > On 2025-03-29 10:05, Hans Rosenfeld wrote:
> > > On Sat, Mar 29, 2025 at 09:11:25AM +0100, Anders Magnusson wrote:
> > > > The MP_STEP1 parts you quote below is just that.  It resets theuda and sees
> > > > if it gets any answer.
> > > > If it succeeds then STEP1MASK etc is checked later on to walk through the
> > > > initialization process.
> > > >
> > > > I do not know where Robert's CMD controller fails,
> > >
> > > After the writing IP, repeated reads of SA read 0. I've put in a loop
> > > after writing IP which reads SA once a second up to 60s and sometimes
> > > there was no reaction at all until the retry.
> >
> > You mean in that code, or some other test code?
>
> In udamatch(), sys/dev/qbus/uda.c. I didn't test with any other code.

Ok. Good. Then I know where we're looking and how.

> > mscp_waitstep is basically waiting 10 seconds for any kind of reply that
> > match what is expected. The problem is that as long as SA and MASK is not
> > equal to RESULT, it will continue waiting. So if you have an error
> > condition, it will wait those 10 seconds.
>
> I know. That's why I fixed this early on. It just turned out this wasn't
> in any way relevant to the odd behaviour of the CMD controller.

Ok. That diff looks good anyway.

> > The same if something else picked up the completion of step 1.
> > And since step 1 is enabling interrupts, it might get an interrupt in before
> > you poll, at which point you're dependent on what the interrupt handler is
> > doing. Or is this running with interrupts generally disabled?
>
> Interrupts aren't enabled yet in any meaningful sense. As far as I
> understand, the uba driver and/or the autoconf framework enabled
> interrupts to catch any interrupts during autoconfiguration to detect
> which devices use which interrupt vectors.
>
> The match/probe functions of drivers are supposed to poke the device to
> see whether it's there and whether it's actually what they expect it to
> be. In general, they also try to cause an interrupt to discover the
> vector being used, but in case of udamatch() it just programs the next
> free vector it got passed from the autoconf framework, and that's where
> the interrupt caused by udamatch() ultimately goes.

Right. So in essence, not sure what the need for generating an interrupt 
is, and it could potentially be a strange/bad thing. But based on more 
information below, this is currently not relevant at all...

> But none of that happens in udamatch() on a CMD controller because we
> don't even get to step 1. Even if we fix early return and add a retry,
> that's usually not enough to get it to step 1.

Oh. So we just write to IP, and nothing happens? That would be seriously 
broken.

> > Hmm, and by the way, when we say "step 1", are we talking about the
> > controller even indicating that it is at step 1, or the response afterwe
> > write the data in step 1?
>
> I'm talking about SA indicating it's at step 1, following the write to IP:
>
> 	tries = 0;
> again:
>
> 	bus_space_write_2(mi.mi_iot, mi.mi_iph, 0, 0); /* Start init */
> 	if (mscp_waitstep(&mi, MP_STEP1, MP_STEP1) == 0)
> 		return 0; /* Nothing here... */

And detecting errors are not even triggering? So it just sits and waits 
until timeout?

And one or two additional writes eventually gets it to a reset state?

> > I think mscp_waitstep really should be modified to detect error states.
>
> Sure, I have a patch attached to do that. It just isn't enough to get
> the CMD controller going after boot. But it shouldn't hurt either, so
> I'll probably just go and commit that, and the fix to udamatch() to try
> again if we don't get to step 1 as well.

Yeah. That fix should go in there. It might also be a good idea to 
somehow report if we get an error, I think.

> > Small question, why are we enabling interrupts in the initialization phase?
> > Since we have code which is basically polling all through initialization, it
> > would seem more natural to just not have interrupts involved here.
>
> Thats really just the interrupt vector detection mechanism of autoconf,
> at least as far as I have understood it. That goes back all the way to
> 4BSD if I'm not mistaken.

I don't think it actually makes sense here. But this is clearly a 
different question as well. Setting the IE just means it would be doing 
interrupts for each initialization step. It have nothing to do with 
interrupts during normal operation.

> The real initialization of the controller happens in udaattach(),
> including the real interrupt setup and all that. But for the CMD
> controller, we don't even get there because udamatch() can't detect it.

Really strange. Are you booting from that controller? So it's currently 
operational when we get to udamatch() ?

  Johnny

--
Johnny Billquist                  || "I'm on a bus
                                  ||  on a psychedelic trip
email: bqt%softjar.se@localhost             ||  Reading murder books
pdp is alive!                     ||  tryin' to stay hip" - B. Idol