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.