[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux1394-devel
Subject:    Re: [PATCH v2 2/3] firewire: Improve bus reset error messages
From:       Peter Hurley <peter () hurleysoftware ! com>
Date:       2013-04-29 20:04:41
Message-ID: 1367265881.3494.39.camel () thor ! lan
[Download RAW message or body]

On Sun, 2013-04-28 at 14:36 +0200, Stefan Richter wrote:
> On Mar 27 Peter Hurley wrote:
> > @@ -1895,7 +1895,9 @@ static void bus_reset_work(struct work_struct *work)
> >  	rmb();
> >  
> >  	for (i = 1, j = 0; j < self_id_count; i += 2, j++) {
> > -		if (ohci->self_id_cpu[i] != ~ohci->self_id_cpu[i + 1]) {
> > +		u32 id = ohci->self_id_cpu[i];
> > +
> > +		if (id != ~ohci->self_id_cpu[i + 1]) {
> >  			/*
> >  			 * If the invalid data looks like a cycle start packet,
> >  			 * it's likely to be the result of the cycle master
> > @@ -1903,19 +1905,19 @@ static void bus_reset_work(struct work_struct *work)
> >  			 * so far are valid and should be processed so that the
> >  			 * bus manager can then correct the gap count.
> >  			 */
> > -			if (cond_le32_to_cpu(ohci->self_id_cpu[i])
> > -							== 0xffff008f) {
> > +			if (cond_le32_to_cpu(id) == 0xffff008f) {
> >  				ohci_notice(ohci,
> >  					    "ignoring spurious self IDs\n");
> >  				self_id_count = j;
> >  				break;
> > -			} else {
> > -				ohci_notice(ohci, "inconsistent self IDs\n");
> > -				return;
> >  			}
> > +
> > +			ohci_notice(ohci, "bad self ID %d/%d (%08x != ~%08x)\n",
> > +				    j, self_id_count, id,
> > +				    ohci->self_id_cpu[i+1]);
> > +			return;
> >  		}
> > -		ohci->self_id_buffer[j] =
> > -				cond_le32_to_cpu(ohci->self_id_cpu[i]);
> > +		ohci->self_id_buffer[j] = cond_le32_to_cpu(id);
> >  	}
> >  
> >  	if (ohci->quirks & QUIRK_TI_SLLZ059) {
> 
> 'make C=1 CF="-D__CHECK_ENDIAN__"' says:
> drivers/firewire/ohci.c:1929:43: warning: incorrect type in initializer (different base types)
> drivers/firewire/ohci.c:1929:43:    expected unsigned int [unsigned] [usertype] id
> drivers/firewire/ohci.c:1929:43:    got restricted __le32 [usertype] <noident>
> drivers/firewire/ohci.c:1931:27: warning: restricted __le32 degrades to integer
> drivers/firewire/ohci.c:1939:29: warning: cast to restricted __le32
> drivers/firewire/ohci.c:1951:43: warning: cast to restricted __le32
> 
> (Line numbers differ from the patch posting due to rebasing onto other patches.)
> 
> I am rewriting it this way:
> 	for (i = 1, j = 0; j < self_id_count; i += 2, j++) {
> 		u32 id  = cond_le32_to_cpu(ohci->self_id_cpu[i]);
> 		u32 id2 = cond_le32_to_cpu(ohci->self_id_cpu[i + 1]);
> 
> 		if (id != ~id2) {
> 			/*
> 			 * If the invalid data looks like a cycle start packet,
> 			 * it's likely to be the result of the cycle master
> 			 * having a wrong gap count.  In this case, the self IDs
> 			 * so far are valid and should be processed so that the
> 			 * bus manager can then correct the gap count.
> 			 */
> 			if (id == 0xffff008f) {
> 				ohci_notice(ohci, "ignoring spurious self IDs\n");
> 				self_id_count = j;
> 				break;
> 			}
> 
> 			ohci_notice(ohci, "bad self ID %d/%d (%08x != ~%08x)\n",
> 				    j, self_id_count, id, id2);
> 			return;
> 		}
> 		ohci->self_id_buffer[j] = id;
> 	}

Sure, this looks ok to me.

Regards,
Peter Hurley



------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service 
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_apr
_______________________________________________
mailing list linux1394-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux1394-devel
[prev in list] [next in list] [prev in thread] [next in thread]