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