On 2019-06-11 19:49, Thomas Preston wrote:
Add speaker_test device attribute. When the speaker-test node is read the hardware speaker test is started.
$ cat /sys/devices/.../device:00/speaker_test 04 04 04 04
Signed-off-by: Thomas Preston thomas.preston@codethink.co.uk Cc: Patrick Glaser pglaser@tesla.com Cc: Rob Duncan rduncan@tesla.com Cc: Nate Case ncase@tesla.com
sound/soc/codecs/tda7802.c | 172 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 172 insertions(+)
diff --git a/sound/soc/codecs/tda7802.c b/sound/soc/codecs/tda7802.c index 62aae011d9f1..edfa752c0c9f 100644 --- a/sound/soc/codecs/tda7802.c +++ b/sound/soc/codecs/tda7802.c @@ -157,6 +157,7 @@ static const u8 IB3_FORMAT[4][4] = { #define DUMP_NUM_REGS (DUMP_NUM_BLOCK * 2)
struct tda7802_priv {
- struct mutex mutex; struct i2c_client *i2c; struct regmap *regmap; struct regulator *enable_reg;
@@ -458,6 +459,159 @@ static struct snd_soc_dai_driver tda7802_dai_driver = { .ops = &tda7802_dai_ops, };
+/* The speaker test is triggered by reading a sysfs attribute file attached to
- the I2C device. The user space thread that's doing the reading is blocked
- until the test completes (or times out). We only permit one test to be in
- progress at a time.
- */
+static int speaker_test_start(struct regmap *regmap) +{
- int err;
- err = regmap_update_bits(regmap, TDA7802_IB5, IB5_AMPLIFIER_ON, 0);
- if (err < 0) {
dev_err(regmap_get_device(regmap),"Cannot disable amp for speaker test (%d)\n", err);return err;- }
- err = regmap_update_bits(regmap, TDA7802_IB4, IB4_DIAG_MODE_ENABLE, 0);
- if (err < 0) {
dev_err(regmap_get_device(regmap),"Cannot disable diag mode before speaker test (%d)\n",err);return err;- }
- /* Seem to need at least a 15 ms delay here before the rising
* edge. Otherwise the diagnostics never complete (perhaps* they never start).*/- msleep(20);
- err = regmap_update_bits(regmap, TDA7802_IB4,
IB4_DIAG_MODE_ENABLE, IB4_DIAG_MODE_ENABLE);- if (err < 0) {
dev_err(regmap_get_device(regmap),"Cannot enable diag mode for speaker test (%d)\n", err);return err;- }
- return 0;
You may want to follow path set by speaker_test_stop: replace "return 0" with "return err" and have only error msg found within preceding if-statement.
+}
+static int speaker_test_stop(struct regmap *regmap) +{
- int err;
- err = regmap_update_bits(regmap, TDA7802_IB4, IB4_DIAG_MODE_ENABLE, 0);
- if (err < 0)
dev_err(regmap_get_device(regmap),"Cannot disable diag mode after speaker test (%d)\n",err);- return err;
+}
+/* We poll the test completion bit, letting the current thread sleep
- until we're done. These values are not critical.
- */
+#define SPEAKER_TEST_DONE_POLL_PERIOD_US 5000 +#define SPEAKER_TEST_DONE_TIMEOUT_US 5000000
+static int speaker_test_check(struct tda7802_priv *tda7802,
unsigned int *status)+{
- struct regmap *regmap = tda7802->regmap;
- struct device *dev = &tda7802->i2c->dev;
- int reg_err = 0, err = 0, i, amp_on;
Both reg_err and err are initialized unnecessarily.
- unsigned int val;
- reg_err = regulator_enable(tda7802->enable_reg);
- if (reg_err < 0) {
dev_err(dev, "Could not enable (speaker-test).\n");return reg_err;- }
- msleep(ENABLE_DELAY_MS);
- /* we should return amp on state when speaker-test is done */
- err = regmap_read(regmap, TDA7802_IB5, &_on);
- if (err < 0) {
dev_err(dev, "Could not read amp on state (speaker-test)\n");goto speaker_test_disable;- }
- for (i = 0; i < CHANNELS_PER_AMP; ++i)
status[i] = 0xff;- err = speaker_test_start(regmap);
- if (err < 0)
goto speaker_test_restore;- /* Wait until DB0_STARTUP_DIAG_STATUS is set */
- err = regmap_read_poll_timeout(regmap, TDA7802_DB0, val,
val & DB0_STARTUP_DIAG_STATUS,SPEAKER_TEST_DONE_POLL_PERIOD_US,SPEAKER_TEST_DONE_TIMEOUT_US);- if (err < 0) {
if (err == -ENODATA)dev_err(dev,"Speaker diagnostic test did not complete\n");speaker_test_stop(regmap);goto speaker_test_restore;- }
- err = speaker_test_stop(regmap);
- if (err < 0)
goto speaker_test_restore;
This sequence looks weird and as if it could be simplified but I might be just plain wrong. speaker_test_stop gets invoked whether or not regmap_read_poll_timeout fails, though only in "happy" path its return is account for. If it's not too much to ask, could you clarify?
- for (i = 0; i < CHANNELS_PER_AMP; ++i) {
err = regmap_read(regmap, TDA7802_DB1 + i, status + i);if (err < 0) {dev_err(dev,"Cannot get speaker %d status (%d)\n", i, err);goto speaker_test_restore;}- }
+speaker_test_restore:
- err = regmap_update_bits(regmap, TDA7802_IB5, IB5_AMPLIFIER_ON,
amp_on);- if (err < 0)
dev_err(dev, "Could not restore amp on state (speaker-test)\n");+speaker_test_disable:
- reg_err = regulator_disable(tda7802->enable_reg);
- if (reg_err < 0) {
dev_err(dev, "Could not disable (speaker-test).\n");return reg_err;- }
- return err;
+}
+static ssize_t speaker_test_show(struct device *dev,
struct device_attribute *attr, char *buf)+{
- struct tda7802_priv *tda7802 = dev_get_drvdata(dev);
- unsigned int channel_status[CHANNELS_PER_AMP];
- char *str = buf;
- int ret, i;
- mutex_lock(&tda7802->mutex);
- ret = speaker_test_check(tda7802, channel_status);
- mutex_unlock(&tda7802->mutex);
- if (ret < 0)
return ret;- for (i = 0; i < CHANNELS_PER_AMP; i++)
str += sprintf(str, "%02x ", channel_status[i]);- str += sprintf(str, "\n");
- return str - buf;
+}
+static DEVICE_ATTR_RO(speaker_test);
- static ssize_t enable_show(struct device *dev, struct device_attribute *attr, char *buf) {
@@ -537,12 +691,28 @@ static int tda7802_probe(struct snd_soc_component *component)
dev_dbg(dev, "%s\n", __func__);
/* Device is ready, now we should create sysfs attributes.
* Rememer to cleanup*/err = device_create_file(dev, &dev_attr_enable); if (err < 0) { dev_err(dev, "Could not create enable attr\n"); return err; }
mutex_init(&tda7802->mutex);
err = device_create_file(dev, &dev_attr_speaker_test);
if (err < 0) {
dev_err(dev, "Could not create speaker_test attr\n");goto cleanup_speaker_test;}
return 0;
+cleanup_speaker_test:
- mutex_destroy(&tda7802->mutex);
- device_remove_file(&tda7802->i2c->dev, &dev_attr_enable);
- return err; }
@@ -550,6 +720,8 @@ static void tda7802_remove(struct snd_soc_component *component) { struct tda7802_priv *tda7802 = snd_soc_component_get_drvdata(component);
- device_remove_file(&tda7802->i2c->dev, &dev_attr_speaker_test);
- mutex_destroy(&tda7802->mutex); device_remove_file(&tda7802->i2c->dev, &dev_attr_enable); }