Le 18/10/2024 à 11:43,
wangweidong.a-tUEr1MkLeujQT0dZR+AlfA@xxxxxxxxxxxxxxxx a écrit :
From: Weidong Wang <wangweidong.a-tUEr1MkLeujQT0dZR+AlfA@xxxxxxxxxxxxxxxx>
The driver is for amplifiers aw88081 of Awinic Technology Corporation.
The awinic AW88081 is an I2S/TDM input, high efficiency digital
Smart K audio amplifier
Signed-off-by: Weidong Wang <wangweidong.a-tUEr1MkLeujQT0dZR+AlfA@xxxxxxxxxxxxxxxx>
---
Hi,
...
+
+#include <linux/i2c.h>
+#include <linux/firmware.h>
Sometimes, alphabecal order is prefered.
+#include <linux/regmap.h>
+#include <sound/soc.h>
+#include "aw88081.h"
+#include "aw88395/aw88395_device.h"
+
+static const struct regmap_config aw88081_regmap_config = {
+ .val_bits = 16,
+ .reg_bits = 8,
+ .max_register = AW88081_REG_MAX,
+ .reg_format_endian = REGMAP_ENDIAN_LITTLE,
+ .val_format_endian = REGMAP_ENDIAN_BIG,
+};
...
+static int aw88081_dev_check_syspll(struct aw_device *aw_dev)
+{
+ int ret;
+
+ ret = aw88081_dev_check_mode1_pll(aw_dev);
+ if (ret) {
+ dev_dbg(aw_dev->dev, "mode1 check iis failed try switch to mode2 check");
+ ret = aw88081_dev_check_mode2_pll(aw_dev);
+ if (ret) {
+ dev_err(aw_dev->dev, "mode2 check iis failed");
+ return ret;
+ }
+ }
+
+ return ret;
Here and in some other places, return 0; could be used to be more explicit.
+}
...
+static int aw88081_reg_update(struct aw88081 *aw88081, bool force)
+{
+ struct aw_device *aw_dev = aw88081->aw_pa;
+ int ret;
+
+ if (force) {
+ ret = regmap_write(aw_dev->regmap,
+ AW88081_ID_REG, AW88081_SOFT_RESET_VALUE);
+ if (ret)
+ return ret;
+
+ ret = aw88081_dev_fw_update(aw88081);
+ if (ret)
+ return ret;
+ } else {
+ if (aw_dev->prof_cur != aw_dev->prof_index) {
+ ret = aw88081_dev_fw_update(aw88081);
+ if (ret)
+ return ret;
+ } else {
+ ret = 0;
This else could be removed, and an explicit return 0; used below at the
end of the function.
+ }
+ }
+
+ aw_dev->prof_cur = aw_dev->prof_index;
+
+ return ret;
+}
...
+static int aw88081_profile_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ struct snd_soc_component *codec = snd_soc_kcontrol_component(kcontrol);
+ struct aw88081 *aw88081 = snd_soc_component_get_drvdata(codec);
+ char *prof_name, *name;
+ int count, ret;
+
+ uinfo->type = SNDRV_CTL_ELEM_TYPE_ENUMERATED;
+ uinfo->count = 1;
+
+ count = aw88081->aw_pa->prof_info.count;
+ if (count <= 0) {
+ uinfo->value.enumerated.items = 0;
+ return 0;
+ }
+
+ uinfo->value.enumerated.items = count;
+
+ if (uinfo->value.enumerated.item >= count)
+ uinfo->value.enumerated.item = count - 1;
+
+ name = uinfo->value.enumerated.name;
+ count = uinfo->value.enumerated.item;
+
+ ret = aw88081_dev_get_prof_name(aw88081->aw_pa, count, &prof_name);
+ if (ret) {
+ strscpy(uinfo->value.enumerated.name, "null",
+ strlen("null") + 1);
Np real use fot this hand computed length. Using the 2 parameters
version of strscpy() should just be fine, I think.
+ return 0;
+ }
+
+ strscpy(name, prof_name, sizeof(uinfo->value.enumerated.name));
If uinfo->value.enumerated.name was used directly as in the if block
above, then 'name' could be removed and the 2 parameters only variant of
strscpy() coulb be used instead, I think.
+
+ return 0;
+}
...
+static int aw88081_init(struct aw88081 *aw88081, struct i2c_client *i2c, struct regmap *regmap)
+{
+ struct aw_device *aw_dev;
+ unsigned int chip_id;
+ int ret;
+
+ /* read chip id */
+ ret = regmap_read(regmap, AW88081_ID_REG, &chip_id);
+ if (ret) {
+ dev_err(&i2c->dev, "%s read chipid error. ret = %d", __func__, ret);
+ return ret;
+ }
+ if (chip_id != AW88081_CHIP_ID) {
+ dev_err(&i2c->dev, "unsupported device");
+ return -ENXIO;
+ }
+
+ dev_dbg(&i2c->dev, "chip id = %x\n", chip_id);
+
+ aw_dev = devm_kzalloc(&i2c->dev, sizeof(*aw_dev), GFP_KERNEL);
+ if (!aw_dev)
+ return -ENOMEM;
+
+ aw88081->aw_pa = aw_dev;
+ aw_dev->i2c = i2c;
+ aw_dev->regmap = regmap;
+ aw_dev->dev = &i2c->dev;
+ aw_dev->chip_id = AW88081_CHIP_ID;
+ aw_dev->acf = NULL;
+ aw_dev->prof_info.prof_desc = NULL;
+ aw_dev->prof_info.count = 0;
No need to init here, and channel below, unless an explicit 0 is more
informative than the kzalloc() above.
+ aw_dev->prof_info.prof_type = AW88395_DEV_NONE_TYPE_ID;
+ aw_dev->channel = 0;
+ aw_dev->fw_status = AW88081_DEV_FW_FAILED;
+ aw_dev->fade_step = AW88081_VOLUME_STEP_DB;
+ aw_dev->volume_desc.ctl_volume = AW88081_VOL_DEFAULT_VALUE;
+ aw_dev->volume_desc.mute_volume = AW88081_MUTE_VOL;
+ aw88081_parse_channel_dt(aw88081);
+
+ return ret;
+}
...
+static int aw88081_request_firmware_file(struct aw88081 *aw88081)
+{
+ const struct firmware *cont = NULL;
+ int ret;
+
+ aw88081->aw_pa->fw_status = AW88081_DEV_FW_FAILED;
+
+ ret = request_firmware(&cont, AW88081_ACF_FILE, aw88081->aw_pa->dev);
+ if (ret)
+ return dev_err_probe(aw88081->aw_pa->dev, ret,
+ "load [%s] failed!", AW88081_ACF_FILE);
+
+ dev_dbg(aw88081->aw_pa->dev, "loaded %s - size: %zu\n",
+ AW88081_ACF_FILE, cont ? cont->size : 0);
+
+ aw88081->aw_cfg = devm_kzalloc(aw88081->aw_pa->dev, cont->size + sizeof(int), GFP_KERNEL);
+ if (!aw88081->aw_cfg) {
+ release_firmware(cont);
+ return -ENOMEM;
+ }
+ aw88081->aw_cfg->len = (int)cont->size;
+ memcpy(aw88081->aw_cfg->data, cont->data, cont->size);
+ release_firmware(cont);
+
+ ret = aw88395_dev_load_acf_check(aw88081->aw_pa, aw88081->aw_cfg);
+ if (ret) {
+ dev_err(aw88081->aw_pa->dev, "load [%s] failed !", AW88081_ACF_FILE);
return dev_err_probe() to be consistent and less verbose.
+ return ret;
+ }
+
+ mutex_lock(&aw88081->lock);
+ /* aw device init */
+ ret = aw88081_dev_init(aw88081, aw88081->aw_cfg);
+ if (ret)
+ dev_err(aw88081->aw_pa->dev, "dev init failed");
return dev_err_probe() to be consistent?
+ mutex_unlock(&aw88081->lock);
+
+ return ret;
+}
...
+static int aw88081_i2c_probe(struct i2c_client *i2c)
+{
+ struct aw88081 *aw88081;
+ int ret;
+
+ ret = i2c_check_functionality(i2c->adapter, I2C_FUNC_I2C);
+ if (!ret)
+ return dev_err_probe(&i2c->dev, -ENXIO, "check_functionality failed");
+
+ aw88081 = devm_kzalloc(&i2c->dev, sizeof(*aw88081), GFP_KERNEL);
+ if (!aw88081)
+ return -ENOMEM;
+
+ mutex_init(&aw88081->lock);
+
+ i2c_set_clientdata(i2c, aw88081);
+
+ aw88081->regmap = devm_regmap_init_i2c(i2c, &aw88081_regmap_config);
+ if (IS_ERR(aw88081->regmap)) {
+ ret = PTR_ERR(aw88081->regmap);
+ return dev_err_probe(&i2c->dev, ret, "failed to init regmap: %d\n", ret);
No need to repeat 'ret' at theend of the message.
+ }
+
+ /* aw pa init */
+ ret = aw88081_init(aw88081, i2c, aw88081->regmap);
+ if (ret)
+ return ret;
+
+ ret = devm_snd_soc_register_component(&i2c->dev,
+ &soc_codec_dev_aw88081,
+ aw88081_dai, ARRAY_SIZE(aw88081_dai));
+ if (ret)
+ dev_err(&i2c->dev, "failed to register aw88081: %d", ret);
dev_err_probe() to be consistent?
+
+ return ret;
+}
...
CJ
