On 19/07/2023 05:09, wangweidong.a@awinic.com wrote:
Hi, Krzysztof, Thank you very much for your advice, but I have a few questions I'd like to discuss with you
On 18/07/2023 16:58, krzysztof.kozlowski@linaro.org wrote:
On 17/07/2023 13:58, wangweidong.a@awinic.com wrote:
From: Weidong Wang wangweidong.a@awinic.com
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.
I will modify the commit information and differentiate the commit information for each file
...
+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.
I will delete {} as suggested
- if (ret)
dev_dbg(aw_dev->dev, "%s failed", __func__);Why you are not handling the errors properly?
Do you need to use dev_err instead?
No, return err and handle it somehow (if it is reasonable).
..
- } 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.
This function will be called in aw88261.c, can I keep it?
Not really. Being used in other unit does not mean you need export.
Best regards, Krzysztof