On 17/07/2023 13:58, wangweidong.a@xxxxxxxxxx wrote:
> From: Weidong Wang <wangweidong.a@xxxxxxxxxx>
>
> The AW88261 is an I2S/TDM input, high efficiency
> digital Smart K audio amplifier with an integrated 10.25V
> smart boost convert
It's the same as in patch before. This does not help and does not
justify having one driver split into two.
>
...
> +
> +static void aw_dev_i2s_tx_enable(struct aw_device *aw_dev, bool flag)
> +{
> + int ret;
> +
> + if (flag) {
> + ret = regmap_update_bits(aw_dev->regmap, AW88261_I2SCFG1_REG,
> + ~AW88261_I2STXEN_MASK, AW88261_I2STXEN_ENABLE_VALUE);
> + } else {
> + ret = regmap_update_bits(aw_dev->regmap, AW88261_I2SCFG1_REG,
> + ~AW88261_I2STXEN_MASK, AW88261_I2STXEN_DISABLE_VALUE);
> + }
You should not need {} here and in multiple other places.
> + if (ret)
> + dev_dbg(aw_dev->dev, "%s failed", __func__);
Why you are not handling the errors properly?
> +}
> +
> +static void aw_dev_pwd(struct aw_device *aw_dev, bool pwd)
> +{
> + int ret;
> +
> + if (pwd) {
> + ret = regmap_update_bits(aw_dev->regmap, AW88261_SYSCTRL_REG,
> + ~AW88261_PWDN_MASK, AW88261_PWDN_POWER_DOWN_VALUE);
> + } else {
> + ret = regmap_update_bits(aw_dev->regmap, AW88261_SYSCTRL_REG,
> + ~AW88261_PWDN_MASK, AW88261_PWDN_WORKING_VALUE);
> + }
> + if (ret)
> + dev_dbg(aw_dev->dev, "%s failed", __func__);
> +}
> +
...
> +
> +int aw88261_dev_fw_update(struct aw_device *aw_dev)
> +{
> + struct aw_prof_desc *prof_index_desc;
> + struct aw_sec_data_desc *sec_desc;
> + char *prof_name;
> + int ret;
> +
> + prof_name = aw88261_dev_get_prof_name(aw_dev, aw_dev->prof_index);
> + if (!prof_name) {
> + dev_err(aw_dev->dev, "get prof name failed");
> + return -EINVAL;
> + }
> +
> + dev_dbg(aw_dev->dev, "start update %s", prof_name);
> +
> + ret = aw88261_dev_get_prof_data(aw_dev, aw_dev->prof_index, &prof_index_desc);
> + if (ret)
> + return ret;
> +
> + /* update reg */
> + sec_desc = prof_index_desc->sec_desc;
> + ret = aw_dev_reg_update(aw_dev, sec_desc[AW88261_DATA_TYPE_REG].data,
> + sec_desc[AW88261_DATA_TYPE_REG].len);
> + if (ret) {
> + dev_err(aw_dev->dev, "update reg failed");
> + return ret;
> + }
> +
> + aw_dev->prof_cur = aw_dev->prof_index;
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(aw88261_dev_fw_update);
Why do you need to export this? Where is the user?
> +
> +int aw88261_dev_reg_update(struct aw_device *aw_dev, bool force)
> +{
> + int ret;
> +
> + if (force) {
> + aw88261_dev_soft_reset(aw_dev);
> + ret = aw88261_dev_fw_update(aw_dev);
> + if (ret < 0)
> + return ret;
> + } else {
> + if (aw_dev->prof_cur != aw_dev->prof_index) {
> + ret = aw88261_dev_fw_update(aw_dev);
> + if (ret < 0)
> + return ret;
> + }
> + }
> +
> + aw_dev->prof_cur = aw_dev->prof_index;
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(aw88261_dev_reg_update);
Same question. And in all other places as well.
Best regards,
Krzysztof
