Re: [PATCH] ASoC: wm_hubs: Fix DC Servo readback

[Charles Keepax]

On Wed, Aug 13, 2014 at 12:38:41PM +0100, Mark Brown wrote:
> On Wed, Aug 13, 2014 at 12:08:44PM +0100, Charles Keepax wrote:
> > On Wed, Aug 13, 2014 at 11:47:20AM +0100, Nikesh Oswal wrote:
> 
> > >  	case 2:
> > >  		dcs_reg = WM8994_DC_SERVO_4E;
> > >  		break;
> > > -	case 1:
> > > -		dcs_reg = WM8994_DC_SERVO_READBACK;
> > > -		break;
> > >  	default:
> > >  		dcs_reg = WM8993_DC_SERVO_3;
> > >  		break;
> 
> > This doesn't look right, firstly if it is the same register for
> > all versions then surely we should change the code that sets
> > dcs_readback_mode rather than setting that to different values
> > but treating them the same. Although obviously if nothing still
> > uses this case we could remove it as well.
> 
> > Also I think the situation is more complex for example on version
> > 4.4 of the datasheet for wm8994 the WR_VAL fields appear to be in
> > register 59h. Which is not consistent with this.
> 
> There was a change in the DC servo between revisions of the WM8994 (at
> revision E from the look of the code).  This isn't documented in the
> datasheets as they only document current silicon.

Indeed, but this patch sets all revs of wm8994 to use register 57h.
The lastest version of the datasheet has this register at 59h
although some of the older versions of the datasheet (which likely
match some older revs of the chip although no way to tell which one
just from the datasheet) have it at 57h.

I think basically we need to get some clarity from hardware here
on which revs use which address and update this patch to match,
but either way it looks likely that this patch doesn't address
the whole picture.

Thanks,
Charles
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/