TAS speaker amp temperature is bogus on Mac Minis

[hoshinolina]

This is just cosmetic but I noticed it in dashboards:

j274:

tas2770-i2c-0-31
Adapter: PA Semi SMBus adapter (235014000.i2c)
temp1:        -92.3°C  

j473:

tas2764-i2c-1-38
Adapter: PA Semi SMBus adapter (235014000.i2c)
temp1:        -93.0°C  

j474:

tas2764-i2c-1-38
Adapter: PA Semi SMBus adapter (39b044000.i2c)
temp1:        -93.0°C  

This does seem to work fine on the laptops (on the same chip), so I don't know what's up.

[hoshinolina]

Ah, right:

linux/sound/soc/codecs/tas2770.c

Line 555 in 7457799

/*

But I tried playing something back and it still reads -93... so maybe something is broken and it's not updating?

[jannau]

See ff73e27
This should be the reset value. Should have sensible values when codec is used. The -92.3°C for j274 is slightly off so maybe tas2770 and tas2764 are not completely identical here.

[hoshinolina]

After opening the PCM on the tas2770 machine, it started returning a valid temperature. However, on the tas2764 machines, it does not.

[chadmed]

All of my 2764 machines end up returning sane values after being used. Did you play any audio back or just open the PCM?

2770 and 2764 are not identical, but neither is the code. The drivers should both calculate temperature correctly according to the codecs' datasheets.

[hoshinolina]

Yeah, I did a speaker-test (and it was making sound) on all 3 machines, but only the 2770 one updated the temperature.

[hoshinolina]

@chadmed You forgot to mark the temperature registers as volatile on 2764, so after the first read they never update. Might be other forgotten registers too, compare with 2770 (PAGE is missing, seems sus).

[hoshinolina]

Also the math is wrong:

*result = (reading - (93 * 16)) * (1000 / 16);

1000 / 16 is not an integer. You want (val * 1000) / 16 instead. This is where the error comes from:

>>> (-93 * 16) * (1000 // 16)
-92256

Also the comment is wrong for 2764. It says temp SAR is initalized to 2.6ºC, but that doesn't seem to actually appear at the register while it is shut down. The actual reset value of the register is 0, and that is what is seen.

I suggest detecting the 0 values and returning hwmon_temp_fault as true in that case, and possibly replacing the temperature with 0ºC or something like that so it's a bit less crazy on graphs if the app ignores the fault flag.

[chadmed]

I'm going to assume that the 2764 datasheet is actually just wrong, because it is not possible to even derive a fractional temperature... Not sure why I didn't twig to that earlier.

I've fixed 2764 locally. Reading 0 from the register will return -ENODATA, which will also trigger hwmon_temp_fault. I think this is cleaner than pretending -93 is 0 and should make tools do the right thing even if they don't support fault attributes. I don't have a 2770 machine on hand at the moment (or any more time), so I'll sort that out on the weekend and then submit the patches to the ML. We should probably pull those down until they get merged too.