Re: [PATCH v2] ASoC: codecs: add uspport for the TI SRC4392 codec

Re: [PATCH v2] ASoC: codecs: add uspport for the TI SRC4392 codec

[kernel test robot]

Hi Matt,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on broonie-sound/for-next]
[also build test WARNING on tiwai-sound/for-next linus/master v5.19 next-20220808]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Matt-Flax/ASoC-codecs-add-uspport-for-the-TI-SRC4392-codec/20220809-054524
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: hexagon-randconfig-r002-20220808 (https://download.01.org/0day-ci/archive/20220809/202208090909.Pg0BZGie-lkp@intel.com/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 5f1c7e2cc5a3c07cbc2412e851a7283c1841f520)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/7a9219a8431d7740c0958e53078820cbfef4f3f7
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Matt-Flax/ASoC-codecs-add-uspport-for-the-TI-SRC4392-codec/20220809-054524
        git checkout 7a9219a8431d7740c0958e53078820cbfef4f3f7
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash sound/soc/codecs/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> sound/soc/codecs/src4xxx.c:277:3: warning: variable 'd' is used uninitialized whenever switch default is taken [-Wsometimes-uninitialized]
                   default:
                   ^~~~~~~
   sound/soc/codecs/src4xxx.c:294:59: note: uninitialized use occurs here
                   ret = regmap_write(src4xxx->regmap, SRC4XXX_RCV_PLL_11, d);
                                                                           ^
   sound/soc/codecs/src4xxx.c:221:20: note: initialize the variable 'd' to silence this warning
           int val, pj, jd, d;
                             ^
                              = 0
>> sound/soc/codecs/src4xxx.c:277:3: warning: variable 'jd' is used uninitialized whenever switch default is taken [-Wsometimes-uninitialized]
                   default:
                   ^~~~~~~
   sound/soc/codecs/src4xxx.c:289:59: note: uninitialized use occurs here
                   ret = regmap_write(src4xxx->regmap, SRC4XXX_RCV_PLL_10, jd);
                                                                           ^~
   sound/soc/codecs/src4xxx.c:221:17: note: initialize the variable 'jd' to silence this warning
           int val, pj, jd, d;
                          ^
                           = 0
>> sound/soc/codecs/src4xxx.c:277:3: warning: variable 'pj' is used uninitialized whenever switch default is taken [-Wsometimes-uninitialized]
                   default:
                   ^~~~~~~
   sound/soc/codecs/src4xxx.c:284:59: note: uninitialized use occurs here
                   ret = regmap_write(src4xxx->regmap, SRC4XXX_RCV_PLL_0F, pj);
                                                                           ^~
   sound/soc/codecs/src4xxx.c:221:13: note: initialize the variable 'pj' to silence this warning
           int val, pj, jd, d;
                      ^
                       = 0
   3 warnings generated.
--
>> sound/soc/codecs/src4xxx-i2c.c:35:34: warning: unused variable 'src4xxx_of_match' [-Wunused-const-variable]
   static const struct of_device_id src4xxx_of_match[] = {
                                    ^
   1 warning generated.


vim +/d +277 sound/soc/codecs/src4xxx.c

   213	
   214	static int src4xxx_hw_params(struct snd_pcm_substream *substream,
   215				struct snd_pcm_hw_params *params,
   216				struct snd_soc_dai *dai)
   217	{
   218		struct snd_soc_component *component = dai->component;
   219		struct src4xxx *src4xxx = snd_soc_component_get_drvdata(component);
   220		unsigned int mclk_div;
   221		int val, pj, jd, d;
   222		int reg;
   223		int ret;
   224	
   225		switch (dai->id) {
   226		case SRC4XXX_PORTB:
   227			reg = SRC4XXX_PORTB_CTL_06;
   228			break;
   229		default:
   230			reg = SRC4XXX_PORTA_CTL_04;
   231		}
   232	
   233		if (src4xxx->master[dai->id]) {
   234			mclk_div = src4xxx->mclk_hz/params_rate(params);
   235			if (src4xxx->mclk_hz != mclk_div*params_rate(params)) {
   236				dev_err(component->dev,
   237					"mclk %d / rate %d has a remainder.\n",
   238					src4xxx->mclk_hz, params_rate(params));
   239				return -EINVAL;
   240			}
   241	
   242			val = ((int)mclk_div - 128) / 128;
   243			if ((val < 0) | (val > 3)) {
   244				dev_err(component->dev,
   245					"div register setting %d is out of range\n",
   246					val);
   247				dev_err(component->dev,
   248					"unsupported sample rate %d Hz for the master clock of %d Hz\n",
   249					params_rate(params), src4xxx->mclk_hz);
   250				return -EINVAL;
   251			}
   252	
   253			/* set the TX DIV */
   254			ret = regmap_update_bits(src4xxx->regmap,
   255				SRC4XXX_TX_CTL_07, SRC4XXX_TX_MCLK_DIV_MASK,
   256				val<<SRC4XXX_TX_MCLK_DIV_SHIFT);
   257			if (ret) {
   258				dev_err(component->dev,
   259					"Couldn't set the TX's div register to %d << %d = 0x%x\n",
   260					val, SRC4XXX_TX_MCLK_DIV_SHIFT,
   261					val<<SRC4XXX_TX_MCLK_DIV_SHIFT);
   262				return ret;
   263			}
   264	
   265			/* set the PLL for the digital receiver */
   266			switch (src4xxx->mclk_hz) {
   267			case 24576000:
   268				pj = 0x22;
   269				jd = 0x00;
   270				d = 0x00;
   271				break;
   272			case 22579200:
   273				pj = 0x22;
   274				jd = 0x1b;
   275				d = 0xa3;
   276				break;
 > 277			default:
   278				/* don't error out here,
   279				 * other parts of the chip are still functional
   280				 */
   281				dev_info(component->dev,
   282					"Couldn't set the RCV PLL as this master clock rate is unknown\n");
   283			}
   284			ret = regmap_write(src4xxx->regmap, SRC4XXX_RCV_PLL_0F, pj);
   285			if (ret < 0)
   286				dev_err(component->dev,
   287					"Failed to update PLL register 0x%x\n",
   288					SRC4XXX_RCV_PLL_0F);
 > 289			ret = regmap_write(src4xxx->regmap, SRC4XXX_RCV_PLL_10, jd);
   290			if (ret < 0)
   291				dev_err(component->dev,
   292					"Failed to update PLL register 0x%x\n",
   293					SRC4XXX_RCV_PLL_10);
 > 294			ret = regmap_write(src4xxx->regmap, SRC4XXX_RCV_PLL_11, d);
   295			if (ret < 0)
   296				dev_err(component->dev,
   297					"Failed to update PLL register 0x%x\n",
   298					SRC4XXX_RCV_PLL_11);
   299	
   300			ret = regmap_update_bits(src4xxx->regmap,
   301				SRC4XXX_TX_CTL_07, SRC4XXX_TX_MCLK_DIV_MASK,
   302				val<<SRC4XXX_TX_MCLK_DIV_SHIFT);
   303			if (ret < 0) {
   304				dev_err(component->dev,
   305					"Couldn't set the TX's div register to %d << %d = 0x%x\n",
   306					val, SRC4XXX_TX_MCLK_DIV_SHIFT,
   307					val<<SRC4XXX_TX_MCLK_DIV_SHIFT);
   308				return ret;
   309			}
   310	
   311			return regmap_update_bits(src4xxx->regmap, reg,
   312						SRC4XXX_MCLK_DIV_MASK, val);
   313		} else
   314			dev_info(dai->dev, "not setting up MCLK as not master\n");
   315	
   316		return 0;
   317	};
   318	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH v2] ASoC: codecs: add uspport for the TI SRC4392 codec

[Nathan Chancellor]

On Tue, Aug 09, 2022 at 09:49:38AM +0800, kernel test robot wrote:
> Hi Matt,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> [auto build test WARNING on broonie-sound/for-next]
> [also build test WARNING on tiwai-sound/for-next linus/master v5.19 next-20220808]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Matt-Flax/ASoC-codecs-add-uspport-for-the-TI-SRC4392-codec/20220809-054524
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
> config: hexagon-randconfig-r002-20220808 (https://download.01.org/0day-ci/archive/20220809/202208090909.Pg0BZGie-lkp@intel.com/config)
> compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 5f1c7e2cc5a3c07cbc2412e851a7283c1841f520)
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # https://github.com/intel-lab-lkp/linux/commit/7a9219a8431d7740c0958e53078820cbfef4f3f7
>         git remote add linux-review https://github.com/intel-lab-lkp/linux
>         git fetch --no-tags linux-review Matt-Flax/ASoC-codecs-add-uspport-for-the-TI-SRC4392-codec/20220809-054524
>         git checkout 7a9219a8431d7740c0958e53078820cbfef4f3f7
>         # save the config file
>         mkdir build_dir && cp config build_dir/.config
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash sound/soc/codecs/
> 
> If you fix the issue, kindly add following tag where applicable
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All warnings (new ones prefixed by >>):

It doesn't look like these warnings were addressed before the change was
applied to -next as commit 4e6bedd3c396 ("ASoC: codecs: add support for
the TI SRC4392 codec"). I now see them in next-20220816.

> >> sound/soc/codecs/src4xxx.c:277:3: warning: variable 'd' is used uninitialized whenever switch default is taken [-Wsometimes-uninitialized]
>                    default:
>                    ^~~~~~~
>    sound/soc/codecs/src4xxx.c:294:59: note: uninitialized use occurs here
>                    ret = regmap_write(src4xxx->regmap, SRC4XXX_RCV_PLL_11, d);
>                                                                            ^
>    sound/soc/codecs/src4xxx.c:221:20: note: initialize the variable 'd' to silence this warning
>            int val, pj, jd, d;
>                              ^
>                               = 0
> >> sound/soc/codecs/src4xxx.c:277:3: warning: variable 'jd' is used uninitialized whenever switch default is taken [-Wsometimes-uninitialized]
>                    default:
>                    ^~~~~~~
>    sound/soc/codecs/src4xxx.c:289:59: note: uninitialized use occurs here
>                    ret = regmap_write(src4xxx->regmap, SRC4XXX_RCV_PLL_10, jd);
>                                                                            ^~
>    sound/soc/codecs/src4xxx.c:221:17: note: initialize the variable 'jd' to silence this warning
>            int val, pj, jd, d;
>                           ^
>                            = 0
> >> sound/soc/codecs/src4xxx.c:277:3: warning: variable 'pj' is used uninitialized whenever switch default is taken [-Wsometimes-uninitialized]
>                    default:
>                    ^~~~~~~
>    sound/soc/codecs/src4xxx.c:284:59: note: uninitialized use occurs here
>                    ret = regmap_write(src4xxx->regmap, SRC4XXX_RCV_PLL_0F, pj);
>                                                                            ^~
>    sound/soc/codecs/src4xxx.c:221:13: note: initialize the variable 'pj' to silence this warning
>            int val, pj, jd, d;
>                       ^
>                        = 0
>    3 warnings generated.
> --
> >> sound/soc/codecs/src4xxx-i2c.c:35:34: warning: unused variable 'src4xxx_of_match' [-Wunused-const-variable]
>    static const struct of_device_id src4xxx_of_match[] = {
>                                     ^
>    1 warning generated.
> 
> 
> vim +/d +277 sound/soc/codecs/src4xxx.c
> 
>    213	
>    214	static int src4xxx_hw_params(struct snd_pcm_substream *substream,
>    215				struct snd_pcm_hw_params *params,
>    216				struct snd_soc_dai *dai)
>    217	{
>    218		struct snd_soc_component *component = dai->component;
>    219		struct src4xxx *src4xxx = snd_soc_component_get_drvdata(component);
>    220		unsigned int mclk_div;
>    221		int val, pj, jd, d;
>    222		int reg;
>    223		int ret;
>    224	
>    225		switch (dai->id) {
>    226		case SRC4XXX_PORTB:
>    227			reg = SRC4XXX_PORTB_CTL_06;
>    228			break;
>    229		default:
>    230			reg = SRC4XXX_PORTA_CTL_04;
>    231		}
>    232	
>    233		if (src4xxx->master[dai->id]) {
>    234			mclk_div = src4xxx->mclk_hz/params_rate(params);
>    235			if (src4xxx->mclk_hz != mclk_div*params_rate(params)) {
>    236				dev_err(component->dev,
>    237					"mclk %d / rate %d has a remainder.\n",
>    238					src4xxx->mclk_hz, params_rate(params));
>    239				return -EINVAL;
>    240			}
>    241	
>    242			val = ((int)mclk_div - 128) / 128;
>    243			if ((val < 0) | (val > 3)) {
>    244				dev_err(component->dev,
>    245					"div register setting %d is out of range\n",
>    246					val);
>    247				dev_err(component->dev,
>    248					"unsupported sample rate %d Hz for the master clock of %d Hz\n",
>    249					params_rate(params), src4xxx->mclk_hz);
>    250				return -EINVAL;
>    251			}
>    252	
>    253			/* set the TX DIV */
>    254			ret = regmap_update_bits(src4xxx->regmap,
>    255				SRC4XXX_TX_CTL_07, SRC4XXX_TX_MCLK_DIV_MASK,
>    256				val<<SRC4XXX_TX_MCLK_DIV_SHIFT);
>    257			if (ret) {
>    258				dev_err(component->dev,
>    259					"Couldn't set the TX's div register to %d << %d = 0x%x\n",
>    260					val, SRC4XXX_TX_MCLK_DIV_SHIFT,
>    261					val<<SRC4XXX_TX_MCLK_DIV_SHIFT);
>    262				return ret;
>    263			}
>    264	
>    265			/* set the PLL for the digital receiver */
>    266			switch (src4xxx->mclk_hz) {
>    267			case 24576000:
>    268				pj = 0x22;
>    269				jd = 0x00;
>    270				d = 0x00;
>    271				break;
>    272			case 22579200:
>    273				pj = 0x22;
>    274				jd = 0x1b;
>    275				d = 0xa3;
>    276				break;
>  > 277			default:
>    278				/* don't error out here,
>    279				 * other parts of the chip are still functional
>    280				 */
>    281				dev_info(component->dev,
>    282					"Couldn't set the RCV PLL as this master clock rate is unknown\n");

In the final commit, there is a 'break' here. Should it be a 'return 0'
instead? Or should there be a different fix for these warnings?

>    283			}
>    284			ret = regmap_write(src4xxx->regmap, SRC4XXX_RCV_PLL_0F, pj);
>    285			if (ret < 0)
>    286				dev_err(component->dev,
>    287					"Failed to update PLL register 0x%x\n",
>    288					SRC4XXX_RCV_PLL_0F);
>  > 289			ret = regmap_write(src4xxx->regmap, SRC4XXX_RCV_PLL_10, jd);
>    290			if (ret < 0)
>    291				dev_err(component->dev,
>    292					"Failed to update PLL register 0x%x\n",
>    293					SRC4XXX_RCV_PLL_10);
>  > 294			ret = regmap_write(src4xxx->regmap, SRC4XXX_RCV_PLL_11, d);
>    295			if (ret < 0)
>    296				dev_err(component->dev,
>    297					"Failed to update PLL register 0x%x\n",
>    298					SRC4XXX_RCV_PLL_11);
>    299	
>    300			ret = regmap_update_bits(src4xxx->regmap,
>    301				SRC4XXX_TX_CTL_07, SRC4XXX_TX_MCLK_DIV_MASK,
>    302				val<<SRC4XXX_TX_MCLK_DIV_SHIFT);
>    303			if (ret < 0) {
>    304				dev_err(component->dev,
>    305					"Couldn't set the TX's div register to %d << %d = 0x%x\n",
>    306					val, SRC4XXX_TX_MCLK_DIV_SHIFT,
>    307					val<<SRC4XXX_TX_MCLK_DIV_SHIFT);
>    308				return ret;
>    309			}
>    310	
>    311			return regmap_update_bits(src4xxx->regmap, reg,
>    312						SRC4XXX_MCLK_DIV_MASK, val);
>    313		} else
>    314			dev_info(dai->dev, "not setting up MCLK as not master\n");
>    315	
>    316		return 0;
>    317	};
>    318	
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://01.org/lkp
> 

Cheers,
Nathan

Re: [PATCH v2] ASoC: codecs: add uspport for the TI SRC4392 codec

[Matt Flax]

On 17/8/22 04:00, Nathan Chancellor wrote:

>>     265			/* set the PLL for the digital receiver */
>>     266			switch (src4xxx->mclk_hz) {
>>     267			case 24576000:
>>     268				pj = 0x22;
>>     269				jd = 0x00;
>>     270				d = 0x00;
>>     271				break;
>>     272			case 22579200:
>>     273				pj = 0x22;
>>     274				jd = 0x1b;
>>     275				d = 0xa3;
>>     276				break;
>>   > 277			default:
>>     278				/* don't error out here,
>>     279				 * other parts of the chip are still functional
>>     280				 */
>>     281				dev_info(component->dev,
>>     282					"Couldn't set the RCV PLL as this master clock rate is unknown\n");
> In the final commit, there is a 'break' here. Should it be a 'return 0'
> instead? Or should there be a different fix for these warnings?

The data sheet for the src4392 doesn't list defaults for these registers 
(loaded with pj, jd and d). The actual state of these registers is not 
known until we load them with something, arbitrary or not.

	{  SRC4XXX_RCV_PLL_0F,		0x00  },  /* not spec. in the datasheet */
	{  SRC4XXX_RCV_PLL_10,		0xff  },  /* not spec. in the datasheet */
	{  SRC4XXX_RCV_PLL_11,		0xff  },  /* not spec. in the datasheet */

The state of DIR PLL registers aren't important if the user doesn't 
specify a known mclk frequency.  The implication is that the DIR will 
not function, however that is already implied by the user lacking to 
specify a known mclk frequency.

The other functions on the chip (port A/B I2S, SRC, DIT, etc) will 
behave as per usual, only the DIR will be dysfunctional.


>>     283			}
>>     284			ret = regmap_write(src4xxx->regmap, SRC4XXX_RCV_PLL_0F, pj);
>>     285			if (ret < 0)
>>     286				dev_err(component->dev,
>>     287					"Failed to update PLL register 0x%x\n",
>>     288					SRC4XXX_RCV_PLL_0F);
>>   > 289			ret = regmap_write(src4xxx->regmap, SRC4XXX_RCV_PLL_10, jd);
>>     290			if (ret < 0)
>>     291				dev_err(component->dev,
>>     292					"Failed to update PLL register 0x%x\n",
>>     293					SRC4XXX_RCV_PLL_10);
>>   > 294			ret = regmap_write(src4xxx->regmap, SRC4XXX_RCV_PLL_11, d);
>>     295			if (ret < 0)
>>     296				dev_err(component->dev,
>>     297					"Failed to update PLL register 0x%x\n",
>>     298					SRC4XXX_RCV_PLL_11);
>>     
> Cheers,
> Nathan

Re: [PATCH v2] ASoC: codecs: add uspport for the TI SRC4392 codec

[Nathan Chancellor]

On Wed, Aug 17, 2022 at 03:44:33PM +1000, Matt Flax wrote:
> 
> On 17/8/22 04:00, Nathan Chancellor wrote:
> 
> > >     265			/* set the PLL for the digital receiver */
> > >     266			switch (src4xxx->mclk_hz) {
> > >     267			case 24576000:
> > >     268				pj = 0x22;
> > >     269				jd = 0x00;
> > >     270				d = 0x00;
> > >     271				break;
> > >     272			case 22579200:
> > >     273				pj = 0x22;
> > >     274				jd = 0x1b;
> > >     275				d = 0xa3;
> > >     276				break;
> > >   > 277			default:
> > >     278				/* don't error out here,
> > >     279				 * other parts of the chip are still functional
> > >     280				 */
> > >     281				dev_info(component->dev,
> > >     282					"Couldn't set the RCV PLL as this master clock rate is unknown\n");
> > In the final commit, there is a 'break' here. Should it be a 'return 0'
> > instead? Or should there be a different fix for these warnings?
> 
> The data sheet for the src4392 doesn't list defaults for these registers
> (loaded with pj, jd and d). The actual state of these registers is not known
> until we load them with something, arbitrary or not.
> 
> 	{  SRC4XXX_RCV_PLL_0F,		0x00  },  /* not spec. in the datasheet */
> 	{  SRC4XXX_RCV_PLL_10,		0xff  },  /* not spec. in the datasheet */
> 	{  SRC4XXX_RCV_PLL_11,		0xff  },  /* not spec. in the datasheet */
> 
> The state of DIR PLL registers aren't important if the user doesn't specify
> a known mclk frequency.  The implication is that the DIR will not function,
> however that is already implied by the user lacking to specify a known mclk
> frequency.
> 
> The other functions on the chip (port A/B I2S, SRC, DIT, etc) will behave as
> per usual, only the DIR will be dysfunctional.

So I suppose there is little point to all of the calls to regmap_write()
and regmap_update_bits() in the default case then, meaning a 'return 0'
would be appropriate? Sorry, I am having a hard time parsing what should
be done about the warnings, which are fatal for allmodconfig due to
CONFIG_WERROR.

Cheers,
Nathan

> > >     283			}
> > >     284			ret = regmap_write(src4xxx->regmap, SRC4XXX_RCV_PLL_0F, pj);
> > >     285			if (ret < 0)
> > >     286				dev_err(component->dev,
> > >     287					"Failed to update PLL register 0x%x\n",
> > >     288					SRC4XXX_RCV_PLL_0F);
> > >   > 289			ret = regmap_write(src4xxx->regmap, SRC4XXX_RCV_PLL_10, jd);
> > >     290			if (ret < 0)
> > >     291				dev_err(component->dev,
> > >     292					"Failed to update PLL register 0x%x\n",
> > >     293					SRC4XXX_RCV_PLL_10);
> > >   > 294			ret = regmap_write(src4xxx->regmap, SRC4XXX_RCV_PLL_11, d);
> > >     295			if (ret < 0)
> > >     296				dev_err(component->dev,
> > >     297					"Failed to update PLL register 0x%x\n",
> > >     298					SRC4XXX_RCV_PLL_11);
> > Cheers,
> > Nathan

Re: [PATCH v2] ASoC: codecs: add uspport for the TI SRC4392 codec

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1096 bytes --]

On Wed, Aug 17, 2022 at 08:45:51AM -0700, Nathan Chancellor wrote:
> On Wed, Aug 17, 2022 at 03:44:33PM +1000, Matt Flax wrote:
> > On 17/8/22 04:00, Nathan Chancellor wrote:

> > > In the final commit, there is a 'break' here. Should it be a 'return 0'
> > > instead? Or should there be a different fix for these warnings?

> > The state of DIR PLL registers aren't important if the user doesn't specify
> > a known mclk frequency.  The implication is that the DIR will not function,
> > however that is already implied by the user lacking to specify a known mclk
> > frequency.

> > The other functions on the chip (port A/B I2S, SRC, DIT, etc) will behave as
> > per usual, only the DIR will be dysfunctional.

> So I suppose there is little point to all of the calls to regmap_write()
> and regmap_update_bits() in the default case then, meaning a 'return 0'
> would be appropriate? Sorry, I am having a hard time parsing what should
> be done about the warnings, which are fatal for allmodconfig due to
> CONFIG_WERROR.

Are either of you still looking at fixing this?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2] ASoC: codecs: add uspport for the TI SRC4392 codec

[Nathan Chancellor]

On Mon, Aug 22, 2022 at 07:09:05PM +0100, Mark Brown wrote:
> On Wed, Aug 17, 2022 at 08:45:51AM -0700, Nathan Chancellor wrote:
> > On Wed, Aug 17, 2022 at 03:44:33PM +1000, Matt Flax wrote:
> > > On 17/8/22 04:00, Nathan Chancellor wrote:
> 
> > > > In the final commit, there is a 'break' here. Should it be a 'return 0'
> > > > instead? Or should there be a different fix for these warnings?
> 
> > > The state of DIR PLL registers aren't important if the user doesn't specify
> > > a known mclk frequency.  The implication is that the DIR will not function,
> > > however that is already implied by the user lacking to specify a known mclk
> > > frequency.
> 
> > > The other functions on the chip (port A/B I2S, SRC, DIT, etc) will behave as
> > > per usual, only the DIR will be dysfunctional.
> 
> > So I suppose there is little point to all of the calls to regmap_write()
> > and regmap_update_bits() in the default case then, meaning a 'return 0'
> > would be appropriate? Sorry, I am having a hard time parsing what should
> > be done about the warnings, which are fatal for allmodconfig due to
> > CONFIG_WERROR.
> 
> Are either of you still looking at fixing this?

I sent https://lore.kernel.org/20220822183101.1115095-1-nathan@kernel.org/.

If that is not the right fix, I need some guidance on what it should be.

Cheers,
Nathan

Re: [PATCH v2] ASoC: codecs: add uspport for the TI SRC4392 codec

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 883 bytes --]

On Tue, Aug 16, 2022 at 11:00:10AM -0700, Nathan Chancellor wrote:
> On Tue, Aug 09, 2022 at 09:49:38AM +0800, kernel test robot wrote:

> > config: hexagon-randconfig-r002-20220808 (https://download.01.org/0day-ci/archive/20220809/202208090909.Pg0BZGie-lkp@intel.com/config)
> > compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 5f1c7e2cc5a3c07cbc2412e851a7283c1841f520)

> It doesn't look like these warnings were addressed before the change was
> applied to -next as commit 4e6bedd3c396 ("ASoC: codecs: add support for
> the TI SRC4392 codec"). I now see them in next-20220816.

It's probably worth talking to the 0day people about prioritising what
they're reporting against, especially given that the reports have
evolved into a bit of an eye chart - this was reported against a Hexagon
randconfig with an unreleased compiler which is underselling it rather.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2] ASoC: codecs: add uspport for the TI SRC4392 codec

[Nathan Chancellor]

On Wed, Aug 17, 2022 at 05:21:23PM +0100, Mark Brown wrote:
> On Tue, Aug 16, 2022 at 11:00:10AM -0700, Nathan Chancellor wrote:
> > On Tue, Aug 09, 2022 at 09:49:38AM +0800, kernel test robot wrote:
> 
> > > config: hexagon-randconfig-r002-20220808 (https://download.01.org/0day-ci/archive/20220809/202208090909.Pg0BZGie-lkp@intel.com/config)
> > > compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 5f1c7e2cc5a3c07cbc2412e851a7283c1841f520)
> 
> > It doesn't look like these warnings were addressed before the change was
> > applied to -next as commit 4e6bedd3c396 ("ASoC: codecs: add support for
> > the TI SRC4392 codec"). I now see them in next-20220816.
> 
> It's probably worth talking to the 0day people about prioritising what
> they're reporting against, especially given that the reports have
> evolved into a bit of an eye chart - this was reported against a Hexagon
> randconfig with an unreleased compiler which is underselling it rather.

Sure, that might be interesting to have certain architectures and
in-tree configurations prioritized over others (like arm64/x86_64 +
allmodconfig).

At the same time, I would expect developers and maintainers to focus on
the warning text first and foremost, not what architecture,
configuration, or compiler is being used. This issue is very clearly not
architecture or configuration specific, there is no #ifdef in this
function that changes the nature of the warning. While it is compiler
specific (because possible uninitialized variable warnings are disabled
with GCC), it is not dependent on the version (although I guess that
isn't apparent). I suppose I can just comment on future randconfig
reports to point out how they will affect allmodconfig and such.

Cheers,
Nathan

Re: [PATCH v2] ASoC: codecs: add uspport for the TI SRC4392 codec

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1836 bytes --]

On Wed, Aug 17, 2022 at 10:23:23AM -0700, Nathan Chancellor wrote:
> On Wed, Aug 17, 2022 at 05:21:23PM +0100, Mark Brown wrote:

> > It's probably worth talking to the 0day people about prioritising what
> > they're reporting against, especially given that the reports have
> > evolved into a bit of an eye chart - this was reported against a Hexagon
> > randconfig with an unreleased compiler which is underselling it rather.

> At the same time, I would expect developers and maintainers to focus on
> the warning text first and foremost, not what architecture,
> configuration, or compiler is being used. This issue is very clearly not
> architecture or configuration specific, there is no #ifdef in this

That's the eye chart bit of it - part of the problem with 0day
specificially is that a lot of their reports have become quite hard to
read, they've been putting in something that looks a lot like git
annotate output which makes things very wide which causes wrapping
issues (this one is actually a bit better than most now I go look at it
again since it doesn't have that indentation).  Picking obscure
configurations makes it more likely that people won't get round to
figuring out what the issue being reported is since it seems less urgent
and therefore gets pushed further to the back of the queue.  For
something that's cropping up on a wide range of configurations it'd be
good to priorirtize the more prominent ones to mitigate against this.

> function that changes the nature of the warning. While it is compiler
> specific (because possible uninitialized variable warnings are disabled
> with GCC), it is not dependent on the version (although I guess that
> isn't apparent). I suppose I can just comment on future randconfig
> reports to point out how they will affect allmodconfig and such.

That'd probably help.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2] ASoC: codecs: add uspport for the TI SRC4392 codec

[kernel test robot]

Hi Matt,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on broonie-sound/for-next]
[also build test ERROR on tiwai-sound/for-next linus/master v5.19 next-20220809]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Matt-Flax/ASoC-codecs-add-uspport-for-the-TI-SRC4392-codec/20220809-054524
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: hexagon-randconfig-r002-20220808 (https://download.01.org/0day-ci/archive/20220809/202208091637.XWbySJt5-lkp@intel.com/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 5f1c7e2cc5a3c07cbc2412e851a7283c1841f520)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/7a9219a8431d7740c0958e53078820cbfef4f3f7
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Matt-Flax/ASoC-codecs-add-uspport-for-the-TI-SRC4392-codec/20220809-054524
        git checkout 7a9219a8431d7740c0958e53078820cbfef4f3f7
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> ld.lld: error: undefined symbol: src4xxx_remove
   >>> referenced by src4xxx-i2c.c:25 (sound/soc/codecs/src4xxx-i2c.c:25)
   >>>               soc/codecs/src4xxx-i2c.o:(src4xxx_i2c_remove) in archive sound/built-in.a
   >>> referenced by src4xxx-i2c.c:25 (sound/soc/codecs/src4xxx-i2c.c:25)
   >>>               soc/codecs/src4xxx-i2c.o:(src4xxx_i2c_remove) in archive sound/built-in.a

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp