On Mon, Jun 12, 2023 at 09:25:44AM +0200, Krzysztof Kozlowski wrote:
> On 11/06/2023 13:57, Mark Brown wrote:
> > On Sun, Jun 11, 2023 at 12:26:57PM +0200, Krzysztof Kozlowski wrote:
> >> + { WSA884X_OTP_REG_40, 0x00 },
> >> + { WSA884X_OTP_REG_41, 0x00 },
> >> + { WSA884X_OTP_REG_63, 0x40 },
> > These appear to be OTP data which suggests that they shouldn't have
> > defaults either since they can be programmed.
> Just to be clear - I don't have access to datasheet so I don't know what
> are these. The downstream driver actually initializes (writes to) two
> OTP registers - 38 and 40.
That's... counterintuitive given the naming.
> >> +static bool wsa884x_readonly_register(struct device *dev, unsigned int reg)
> >> +{
> >> + switch (reg) {
> > In general the read only registers probably shouldn't have defaults...
> For the case when regmap is being read before device enumerates (thus
> synced)?
If you're reading read only registers from the cache defaults like that
you may as well cut out the middle man and not bother parsing the
register at all - the value is just hard coded. Either power up the
device to find out what the values are or use the values directly.
> >> + /* Speaker mode by default */
> >> + { WSA884X_DRE_CTL_0, 0x7 << WSA884X_DRE_CTL_0_PROG_DELAY_SHIFT },
> >> + { WSA884X_CLSH_CTL_0, (0x37 & ~WSA884X_CLSH_CTL_0_DLY_CODE_MASK) |
> >> + (0x6 << WSA884X_CLSH_CTL_0_DLY_CODE_SHIFT) },
> >> + { WSA884X_CLSH_SOFT_MAX, 0xff },
> > Why not just leave as the chip default?
> Sincerely, I don't know. Without any documentation, I am relying
> entirely on downstream driver which has some code like this. I don't
> know whether some parts here make sense only for downstream case or
> shall be applicable also for upstream.
This sounds like someone hard coding their use case TBH.
Attachment:
signature.asc
Description: PGP signature
