From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eigenstate.org ([206.124.132.107]) by ewsd; Sun Nov 17 16:19:55 EST 2019
Received: from eigenstate.org (localhost [127.0.0.1])
	by eigenstate.org (OpenSMTPD) with ESMTP id a2d35e7f;
	Sun, 17 Nov 2019 13:19:53 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=eigenstate.org; h=
	message-id:to:subject:date:from:in-reply-to:mime-version
	:content-type:content-transfer-encoding; s=mail; bh=X2LWU50r5K6+
	Hj4gs8xK6mvpUsQ=; b=QZfis4C36LK/9e2fRPysAGJmK53js5mFerbqo1Be7Wro
	+bxnatMO2MkeU09ZHSGkxNc6kjyf2tEYdsfXnSu+Sq/BoHz7ycKGQXlaz+SbS2HH
	yjM76xHlymRAkqWy4lROnyUr9/RKJ6zhhdLEdCBOZTMepBfwCK5mJKmkXsfydKU=
DomainKey-Signature: a=rsa-sha1; c=nofws; d=eigenstate.org; h=message-id
	:to:subject:date:from:in-reply-to:mime-version:content-type
	:content-transfer-encoding; q=dns; s=mail; b=PTW8Q+A6/ItxDto4L+n
	DwuQ9FKMXj8BEpVWUJllzUivx2dBDaSjv4BV71P0fHzaZAmIRmKfolQssDr1GjAi
	Kq4lVfZDN02L5j02CdXTLDYN/FDnSNzH6pBQ12ql6YNgipcI2rrt+Xn4+PhqeScU
	KcplR859XY6lOKB7aQZFnoW0=
Received: from abbatoir.hsd1.ca.comcast.net (c-76-21-119-139.hsd1.ca.comcast.net [76.21.119.139])
	by eigenstate.org (OpenSMTPD) with ESMTPSA id 771467ca (TLSv1.2:ECDHE-RSA-AES256-SHA:256:NO);
	Sun, 17 Nov 2019 13:19:53 -0800 (PST)
Message-ID: <AB758A26ECC378A906EE299B0210F78E@eigenstate.org>
To: cinap_lenrek@felloff.net, 9front@9front.org
Subject: Re: [9front] ahci led: reset spins.
Date: Sun, 17 Nov 2019 13:19:52 -0800
From: ori@eigenstate.org
In-Reply-To: 6630B066158B3C1C022B5438CA4F5BAC@felloff.net
MIME-Version: 1.0
Content-Type: text/plain; charset="US-ASCII"
Content-Transfer-Encoding: 7bit
List-ID: <9front.9front.org>
List-Help: <http://lists.9front.org>
X-Glyph: ➈
X-Bullshit: overflow-preventing mobile standard HTML CSS self-signing framework 

> the whole code in ledkproc() looks rather suspect to me.
> 
> for example, the map[] is NCtlr sized, so i would
> assume that the index is the controller number,
> but later, we index into it by the driver number.

Yes. What was going on there seemed odd.  especially
the 'j += c->enctype' bit. I was planning on asking
wtf was going on with it on IRC before trying to
make changes, since it wasn't part of my current
problem.

> theres also a microdelay() loop in blink() that
> could be spinning forever. theres no reason to use
> delay() or microdelay() as well as we run on behalf
> of a kproc so we can properly sleep and not burn
> cpu cycles.

This diff definitely looks better, and works on my
machines -- thanks!

> 
> diff -r 874554781a81 sys/src/9/pc/sdiahci.c
> --- a/sys/src/9/pc/sdiahci.c	Sun Nov 17 19:04:38 2019 +0100
> +++ b/sys/src/9/pc/sdiahci.c	Sun Nov 17 21:04:51 2019 +0100
> @@ -1215,14 +1215,19 @@
>  ahciencreset(Ctlr *c)
>  {
>  	Ahba *h;
> +	int i;
>  
>  	if(c->enctype == Eesb)
>  		return 0;
>  	h = c->hba;
>  	h->emctl |= Emrst;
> -	while(h->emctl & Emrst)
> -		delay(1);
> -	return 0;
> +	for(i = 0; i < 1000; i++){
> +		if((h->emctl & Emrst) == 0)
> +			return 0;
> +		esleep(1);
> +	}
> +	print("%s: ahciencreset: hung ctlr\n", Tname(c));
> +	return -1;
>  }
>  
>  /*
> @@ -1301,9 +1306,11 @@
>  		return 0;
>  	c = d->ctlr;
>  	h = c->hba;
> +
>  	/* ensure last message has been transmitted */
> -	while(h->emctl & Tmsg)
> -		microdelay(1);
> +	if(h->emctl & Tmsg)
> +		return -1;
> +
>  	switch(c->enctype){
>  	default:
>  		panic("%s: bad led type %d", dnam(d), c->enctype);
> @@ -1403,30 +1410,34 @@
>  	memset(map, 0, sizeof map);
>  	for(i = 0; i < niactlr; i++)
>  		if(iactlr[i].enctype != 0){
> -			ahciencreset(iactlr + i);
> +			if(ahciencreset(iactlr + i) == -1)
> +				continue;
>  			map[i] = 1;
>  			j++;
>  		}
>  	if(j == 0)
>  		pexit("no work", 1);
>  	for(i = 0; i < niadrive; i++){
> -		iadrive[i]->nled = 3;		/* hardcoded */
> -		if(iadrive[i]->ctlr->enctype == Eesb)
> -			iadrive[i]->nled = 3;
> -		iadrive[i]->ledbits = -1;
> +		d = iadrive[i];
> +		d->nled = 3;		/* hardcoded */
> +		if(d->ctlr->enctype == Eesb)
> +			d->nled = 3;
> +		d->ledbits = -1;
>  	}
>  	for(i = 0; ; i++){
>  		t0 = Ticks;
>  		for(j = 0; j < niadrive; ){
> -			c = iadrive[j]->ctlr;
> -			if(map[j] == 0)
> -				j += c->enctype;
> -			else if(c->enctype == Eesb){
> +			d = iadrive[j];
> +			c = d->ctlr;
> +			if(map[c - iactlr] == 0)
> +				j++;
> +			else
> +			if(c->enctype == Eesb){
>  				blinkesb(c, i);
>  				j += c->ndrive;
>  			}else{
> -				d = iadrive[j++];
>  				blink(d, i);
> +				j++;
>  			}
>  		}
>  		t1 = Ticks;
> 
> --
> cinap