>
>On Mon, Sep 16, 2002 at 01:28:44PM -0400, James Chacon wrote:
>
> > After talking with Jason a few months ago it seemed like #2 is the cleaner
> > way to do this and makes the most sense.
>
>Cool.
>
> > I'd like any comments/concerns on these before I commit them as it affects
> > a fairly critical area of code for systems. I've been running with them in
> > a 2 scsibus configured box for months now and every reboot probes in the
> > correct order. (and firewire doesn't deadlock anymore).
>
>The actual changes themselves look good to me, but I have a couple of
>tiny nits to pick :-)
>
> > +	printf(": %d %s, %d luns per target\n",
> > +	    chan->chan_ntargets, chan->chan_ntargets == 1 ? "target":"targets",
> > +	    chan->chan_nluns);
> > +
>
>Construct that as:
>
>	printf(": %s target%s, %d lun%s per target\n",
>	    chan->chan_ntargets,
>	    chan->chan_ntargets == 1 ? "" : "s",
>	    chan->chan_nluns,
>	    chan->chan_nluns == 1 ? "" : "s");

Hehe..The printf in my code was the original code :-)

>
> >  	/* Initialize the channel structure first */
> > +	chan->chan_init_cb = scsibus_config;
> > +	chan->chan_init_cb_arg = sc;
> > +	
> > +	scsi_initq = malloc(sizeof(struct scsi_initq), M_DEVBUF, M_WAITOK);
> > +	if (!scsi_initq_inited) {
> > +		TAILQ_INIT(&scsi_initq_head);
> > +		scsi_initq_inited = 1;
> > +	}
>
>Instead of using "scsi_initq_inited", just use TAILQ_INITIALIZER when
>the scsi_initq_head is declard.

I realized that as I sent this off. I'll fix it before I commit.

>
> > -	if ((sc->sc_channel->chan_flags & SCSIPI_CHAN_NOSETTLE) == 0 &&
> > +	config_pending_incr();
> > +
> > +	/* Make sure the devices probe in scsibus order to avoid jitter. */
> > +	simple_lock(&scsibus_interlock);
> > +	while (1) {
>
>Style guide says use "for (;;) {" for forever loops.

Oops. Ok


> >  
> > +	if (chan->chan_init_cb)
> > +		chan->chan_init_cb(chan, chan->chan_init_cb_arg);
> > +
>
>Just another style nit: (*chan->chan_init_cb)(...);
>

Umm..actually there's nothing in the style guide about these :-)

James