Hi Andy Answer inline > -----Original Message----- > From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > Sent: Friday, August 9, 2024 11:18 PM > To: Ding, Shenghao <shenghao-ding@xxxxxx> > Cc: broonie@xxxxxxxxxx; lgirdwood@xxxxxxxxx; perex@xxxxxxxx; pierre- > louis.bossart@xxxxxxxxxxxxxxx; 13916275206@xxxxxxx; zhourui@xxxxxxxxxx; > alsa-devel@xxxxxxxxxxxxxxxx; Salazar, Ivan <i-salazar@xxxxxx>; linux- > kernel@xxxxxxxxxxxxxxx; Chadha, Jasjot Singh <j-chadha@xxxxxx>; > liam.r.girdwood@xxxxxxxxx; Yue, Jaden <jaden-yue@xxxxxx>; yung- > chuan.liao@xxxxxxxxxxxxxxx; Rao, Dipa <dipa@xxxxxx>; yuhsuan@xxxxxxxxxx; > Lo, Henry <henry.lo@xxxxxx>; tiwai@xxxxxxx; Xu, Baojun <baojun.xu@xxxxxx>; > soyer@xxxxxx; Baojun.Xu@xxxxxxx; judyhsiao@xxxxxxxxxx; Navada Kanyana, > Mukund <navada@xxxxxx>; cujomalainey@xxxxxxxxxx; Kutty, Aanya > <aanya@xxxxxx>; Mahmud, Nayeem <nayeem.mahmud@xxxxxx>; > savyasanchi.shukla@xxxxxxxxxxxxx; flaviopr@xxxxxxxxxxxxx; Ji, Jesse <jesse- > ji@xxxxxx>; darren.ye@xxxxxxxxxxxx > Subject: [EXTERNAL] Re: [PATCH v1] ASoc: tas2781: Add new Kontrol to set > tas2563 digital gain > > 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. ZjQcmQRYFpfptBannerStart This message was sent from > outside of Texas Instruments. > Do not click links or open attachments unless you recognize the source of this > email and know the content is safe. > <https://us-phishalarm- > ewt.proofpoint.com/EWT/v1/G3vK!uBdnVVqmOIH3BkzsUFgO9QgWJ- > u9qFEVKhhfo2Ubp9J6d6r4srvhwGiHZoYkKiQ5od83XAJWq9sCybuN7u4$> > Report Suspicious > > ZjQcmQRYFpfptBannerEnd > 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. [ding]Apply to the new patch. > > ... > > > + 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. [ding] target save the vol register value, and the code will calculate the closest value in the tas2563_dvc_table. Sometimes, the target value is not same as the value in the table. It is wise to find the closest one. /* pow(10, db/20) * pow(2,30) */ static const unsigned char tas2563_dvc_table[][4] = { { 0X00, 0X00, 0X00, 0X00 }, /* -121.5db */ { 0X00, 0X00, 0X03, 0XBC }, /* -121.0db */ { 0X00, 0X00, 0X03, 0XF5 }, /* -120.5db */ ... > > ... > > > +out: > > + mutex_unlock(&tas_dev->codec_lock); > > Why not using cleanup.h? [ding] Accept. > > > + return 0; > > ... > > This all reminds me that I already gave same/similar comments in the past... > > > -- > With Best Regards, > Andy Shevchenko >
