On 02/08/2019 12:10, Mark Brown wrote:
On Fri, Aug 02, 2019 at 09:32:17AM +0100, Thomas Preston wrote:
On 02/08/2019 00:42, Mark Brown wrote:
Yes, that's definitely doable - we've got some other drivers with similar things like calibration triggers exposed that way.
One problem with using a kcontrol as a trigger for the turn-on diagnostic is that the diagnostic routine has a "return value".
You can use a read only control for the readback, or just have it be triggered by overwriting the readback value. You can cache the result.
Keeping the trigger and result together like that would be better I think, although the routine isn't supposed to run mid way through playback. If we're mid playback the debugfs routine has to turn off AMP_ON, take the device back to a known state, run diagnostics, then restore. Which causes a gap in the audible sound.
Hm, maybe a better idea is to have the turn on diagnostic only run on device probe (as its name suggests!), and print something to dmesg:
modprobe tda7802 turn_on_diagnostic=1
tda7802-codec i2c-TDA7802:00: Turn on diagnostic 04 04 04 04
Kirill Marinushkin mentioned this in the first review [0], it just didn't really sink in until now!
You could do that too, yeah. Depends on what this is diagnosing and if that'd be useful.
The diagnostic status bits describe situations such as: - open load (no speaker connected) - short to GND - short to VCC - etc
The intention is to test if all the speakers are connected. So, one might have a self test which runs the diagnostic and verifies it outputs:
00 00 00 00
For example, on my test rig there is only one speaker connected. So it reads:
04 04 00 04
Where the second bit is "open load". So this would fail the test.
So in the kcontrol case the test would be something like:
amixer sset "AMP1 turn on diagnostic" on amixer sget "AMP1 diagnostic"
And the module parameter case:
rmmod tda7802 modprobe tda7802 turn_on_diagnostic=1 dmesg | grep "Turn on diagnostic 04 04 04 04" rmmod tda7802 modprobe tda7802
I think the module parameter method is more appropriate for a "Turn-on diagnostic", even though I don't really like grepping dmesg for the result. I'll go ahead and implement that unless anyone has a particular preference for the kcontrol-trigger.
Thanks