Re: [PATCH v1] ASoc: tas2781: Add new Kontrol to set tas2563 digital gain

From: Andy Shevchenko
Date: Fri Aug 09 2024 - 11:18:36 EST


On Fri, Jun 28, 2024 at 12:18:43PM +0800, Shenghao Ding wrote:
> Requriment from customer to add new kcontrol to set tas2563 digital gain
> and set "Speaker Force Firmware Load" as the common kcontrol for both
> tas27871 and tas2563.

...

> #include <sound/tas2781.h>
> #include <sound/tlv.h>
> #include <sound/tas2781-tlv.h>

> +#include <asm/unaligned.h>

Before sound would be better, but I'm not insisting.

...

> + ret = tasdevice_dev_bulk_read(tas_dev, 0, reg, data, 4);

Too many spaces.

...

> + /* find out the member same as or closer to the current volume */
> + ucontrol->value.integer.value[0] =
> + abs(target - ar_l) <= abs(target - ar_r) ? l : r;

Why do you need to have target to be applied here? IIUC arithmetics correctly
it makes no value to use target in this equation.

...

> +out:
> + mutex_unlock(&tas_dev->codec_lock);

Why not using cleanup.h?

> + return 0;

...

This all reminds me that I already gave same/similar comments in the past...


--
With Best Regards,
Andy Shevchenko