svn commit: r228785 - in head/sys/dev/ath/ath_hal: ar5210 ar5211

[John Baldwin]

On Wednesday, December 21, 2011 6:27:54 pm Alexander Best wrote:
> On Wed Dec 21 11, John Baldwin wrote:
> > On Wednesday, December 21, 2011 4:52:04 pm Adrian Chadd wrote:
> > > Erm, why did you do this without first getting clearance from someone
> > > who has the hardware to test it?
> > > 
> > > Just because it looks obviously wrong to you, doesn't at all mean that
> > > it's "wrong". It's quite possible that the driver _requires_ those
> > > bits to be written to the hardware as 0.
> > > 
> > > 
> > > I'd appreciate it if would please revert this and other ath/hal
> > > changes until I've had time to research them and test them out.
> > 
> > I agree it should be reviewed, but if you are seriously depending on
> > the fact that the shifted values are beyond the edge of the word boundary
> > and so the result "wraps" to zero, then I'd question the sanity of your code.
> 
> i disagree.

You don't think changes should be reviewed (that's what I said above, I did
not necessarily say it should be reverted)?  That's way out in left field if
that is what you really think.

As for reverting the changes, I think they are small enough that is probably
a bit overboard unless someone else reports an actual problem with them.
(Specifically, I don't think the bar is high enough in this case to warrant a
reversion.)  However, I think that for future changes, Dimitry should get
these sort of changes reviewed before committing them.

-- 
John Baldwin