| Message ID | 20240822063205.662-1-shenghao-ding@ti.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] ASoC: tas2781: mark const variables tas2563_dvc_table as __maybe_unused | expand |
Hi Andy Glad to answer your question. > -----Original Message----- > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Sent: Friday, August 23, 2024 9:34 PM > To: Ding, Shenghao <shenghao-ding@ti.com> > Cc: broonie@kernel.org; lgirdwood@gmail.com; perex@perex.cz; pierre- > louis.bossart@linux.intel.com; 13916275206@139.com; zhourui@huaqin.com; > alsa-devel@alsa-project.org; Salazar, Ivan <i-salazar@ti.com>; > liam.r.girdwood@intel.com; Yue, Jaden <jaden-yue@ti.com>; yung- > chuan.liao@linux.intel.com; Rao, Dipa <dipa@ti.com>; yuhsuan@google.com; > Lo, Henry <henry.lo@ti.com>; tiwai@suse.de; Xu, Baojun <baojun.xu@ti.com>; > Baojun.Xu@fpt.com; judyhsiao@google.com; Navada Kanyana, Mukund > <navada@ti.com>; cujomalainey@google.com; Kutty, Aanya <aanya@ti.com>; > Mahmud, Nayeem <nayeem.mahmud@ti.com>; > savyasanchi.shukla@netradyne.com; flaviopr@microsoft.com; Ji, Jesse <jesse- > ji@ti.com>; darren.ye@mediatek.com; antheas.dk@gmail.com; > Jerry2.Huang@lcfuturecenter.com; jim.shil@goertek.com > Subject: Re: [EXTERNAL] Re: [PATCH v1] ASoC: tas2781: mark const variables > tas2563_dvc_table as __maybe_unused > > On Fri, Aug 23, 2024 at 02: 24: 17AM +0000, Ding, Shenghao wrote: > > From: > Andy Shevchenko <andriy. shevchenko@ linux. intel. com> > > Sent: Thursday, > August 22, 2024 9: 37 PM > > To: Ding, Shenghao <shenghao-ding@ ti. com> > 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!uBdnVXyjfyERgskkNHnsf5Dc4Bsz5NHYOuS > CAlM-ECmPvE_-JJDdyUZvDKmnC7E4RAAbEbIHMFCFDpmf6-o$> > Report Suspicious > > ZjQcmQRYFpfptBannerEnd > On Fri, Aug 23, 2024 at 02:24:17AM +0000, Ding, Shenghao wrote: > > > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > Sent: Thursday, August 22, 2024 9:37 PM > > > To: Ding, Shenghao <shenghao-ding@ti.com> On Thu, Aug 22, 2024 at > > > 02:32:02PM +0800, Shenghao Ding wrote: > > ... > > > > > /* pow(10, db/20) * pow(2,30) */ > > > > -static const unsigned char tas2563_dvc_table[][4] = { > > > > +static const __maybe_unused unsigned char tas2563_dvc_table[][4] > > > > += { > > > > { 0X00, 0X00, 0X00, 0X00 }, /* -121.5db */ > > > > { 0X00, 0X00, 0X03, 0XBC }, /* -121.0db */ > > > > { 0X00, 0X00, 0X03, 0XF5 }, /* -120.5db */ > > > > > > As far as I can see in the latest Linux Next the above mentioned > > > table is used solely in sound/soc/codecs/tas2781-i2c.c. Why not > > > moving it to the C file instead? > > We have a big plan to implement the digital gain for tas2563 in the > > side codec driver too, so abstracted this table into a separated header file. > > So, this __maybe_unused is a temporary stub, correct? Just don't forget to > remove it when the mentioned functionality will be ready. > > Thanks for the explanation. To add __maybe_unused is not temporary, because this driver support both tas2563 and tas2781, if the hardware only contains tas2781 chips, tas2563_dvc_table is obviously useless, so marked it as __maybe_unused. > > -- > With Best Regards, > Andy Shevchenko >
On Sat, Aug 24, 2024 at 06:33:41AM +0000, Ding, Shenghao wrote: > > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Sent: Friday, August 23, 2024 9:34 PM > > On Fri, Aug 23, 2024 at 02:24:17AM +0000, Ding, Shenghao wrote: > > > > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > > Sent: Thursday, August 22, 2024 9:37 PM > > > > To: Ding, Shenghao <shenghao-ding@ti.com> On Thu, Aug 22, 2024 at > > > > 02:32:02PM +0800, Shenghao Ding wrote: ... > > > > > /* pow(10, db/20) * pow(2,30) */ > > > > > -static const unsigned char tas2563_dvc_table[][4] = { > > > > > +static const __maybe_unused unsigned char tas2563_dvc_table[][4] > > > > > += { > > > > > { 0X00, 0X00, 0X00, 0X00 }, /* -121.5db */ > > > > > { 0X00, 0X00, 0X03, 0XBC }, /* -121.0db */ > > > > > { 0X00, 0X00, 0X03, 0XF5 }, /* -120.5db */ > > > > > > > > As far as I can see in the latest Linux Next the above mentioned > > > > table is used solely in sound/soc/codecs/tas2781-i2c.c. Why not > > > > moving it to the C file instead? > > > We have a big plan to implement the digital gain for tas2563 in the > > > side codec driver too, so abstracted this table into a separated header file. > > > > So, this __maybe_unused is a temporary stub, correct? Just don't forget to > > remove it when the mentioned functionality will be ready. > > > > Thanks for the explanation. > To add __maybe_unused is not temporary, because this driver support both > tas2563 and tas2781, if the hardware only contains tas2781 chips, > tas2563_dvc_table is obviously useless, so marked it as __maybe_unused. You got me confused. __maybe_unused is a compile-time (okay, link-time) attribute. If what you are telling is true, why do you need it and can't use respective #ifdef? Otherwise, why is it in the _header_ file and not in the C-file?
diff --git a/include/sound/tas2563-tlv.h b/include/sound/tas2563-tlv.h index faa3e194f73b..bb269b21f460 100644 --- a/include/sound/tas2563-tlv.h +++ b/include/sound/tas2563-tlv.h @@ -18,7 +18,7 @@ static const __maybe_unused DECLARE_TLV_DB_SCALE(tas2563_dvc_tlv, -12150, 50, 1); /* pow(10, db/20) * pow(2,30) */ -static const unsigned char tas2563_dvc_table[][4] = { +static const __maybe_unused unsigned char tas2563_dvc_table[][4] = { { 0X00, 0X00, 0X00, 0X00 }, /* -121.5db */ { 0X00, 0X00, 0X03, 0XBC }, /* -121.0db */ { 0X00, 0X00, 0X03, 0XF5 }, /* -120.5db */
In case of tas2781, tas2563_dvc_table will be unused, so mark it as __maybe_unused. Signed-off-by: Shenghao Ding <shenghao-ding@ti.com> --- include/sound/tas2563-tlv.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)