* Re: [PATCH v2] ASoC: codecs: add uspport for the TI SRC4392 codec
[not found] <20220808214028.2502801-1-flatmax@flatmax.com>
@ 2022-08-09 1:49 ` kernel test robot
2022-08-16 18:00 ` Nathan Chancellor
2022-08-09 8:25 ` kernel test robot
1 sibling, 1 reply; 10+ messages in thread
From: kernel test robot @ 2022-08-09 1:49 UTC (permalink / raw)
To: Matt Flax, alsa-devel; +Cc: llvm, kbuild-all, broonie, Matt Flax
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] ASoC: codecs: add uspport for the TI SRC4392 codec
[not found] <20220808214028.2502801-1-flatmax@flatmax.com>
2022-08-09 1:49 ` [PATCH v2] ASoC: codecs: add uspport for the TI SRC4392 codec kernel test robot
@ 2022-08-09 8:25 ` kernel test robot
1 sibling, 0 replies; 10+ messages in thread
From: kernel test robot @ 2022-08-09 8:25 UTC (permalink / raw)
To: Matt Flax, alsa-devel; +Cc: llvm, kbuild-all, broonie, Matt Flax
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] ASoC: codecs: add uspport for the TI SRC4392 codec
2022-08-09 1:49 ` [PATCH v2] ASoC: codecs: add uspport for the TI SRC4392 codec kernel test robot
@ 2022-08-16 18:00 ` Nathan Chancellor
2022-08-17 5:44 ` Matt Flax
2022-08-17 16:21 ` Mark Brown
0 siblings, 2 replies; 10+ messages in thread
From: Nathan Chancellor @ 2022-08-16 18:00 UTC (permalink / raw)
To: Matt Flax; +Cc: kernel test robot, alsa-devel, llvm, kbuild-all, broonie
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] ASoC: codecs: add uspport for the TI SRC4392 codec
2022-08-16 18:00 ` Nathan Chancellor
@ 2022-08-17 5:44 ` Matt Flax
2022-08-17 15:45 ` Nathan Chancellor
2022-08-17 16:21 ` Mark Brown
1 sibling, 1 reply; 10+ messages in thread
From: Matt Flax @ 2022-08-17 5:44 UTC (permalink / raw)
To: Nathan Chancellor
Cc: kernel test robot, alsa-devel, llvm, kbuild-all, broonie
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] ASoC: codecs: add uspport for the TI SRC4392 codec
2022-08-17 5:44 ` Matt Flax
@ 2022-08-17 15:45 ` Nathan Chancellor
2022-08-22 18:09 ` Mark Brown
0 siblings, 1 reply; 10+ messages in thread
From: Nathan Chancellor @ 2022-08-17 15:45 UTC (permalink / raw)
To: Matt Flax; +Cc: kernel test robot, alsa-devel, llvm, kbuild-all, broonie
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] ASoC: codecs: add uspport for the TI SRC4392 codec
2022-08-16 18:00 ` Nathan Chancellor
2022-08-17 5:44 ` Matt Flax
@ 2022-08-17 16:21 ` Mark Brown
2022-08-17 17:23 ` Nathan Chancellor
1 sibling, 1 reply; 10+ messages in thread
From: Mark Brown @ 2022-08-17 16:21 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Matt Flax, kernel test robot, alsa-devel, llvm, kbuild-all
[-- 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 --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] ASoC: codecs: add uspport for the TI SRC4392 codec
2022-08-17 16:21 ` Mark Brown
@ 2022-08-17 17:23 ` Nathan Chancellor
2022-08-18 11:44 ` Mark Brown
0 siblings, 1 reply; 10+ messages in thread
From: Nathan Chancellor @ 2022-08-17 17:23 UTC (permalink / raw)
To: Mark Brown; +Cc: Matt Flax, kernel test robot, alsa-devel, llvm, kbuild-all
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] ASoC: codecs: add uspport for the TI SRC4392 codec
2022-08-17 17:23 ` Nathan Chancellor
@ 2022-08-18 11:44 ` Mark Brown
0 siblings, 0 replies; 10+ messages in thread
From: Mark Brown @ 2022-08-18 11:44 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Matt Flax, kernel test robot, alsa-devel, llvm, kbuild-all
[-- 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 --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] ASoC: codecs: add uspport for the TI SRC4392 codec
2022-08-17 15:45 ` Nathan Chancellor
@ 2022-08-22 18:09 ` Mark Brown
2022-08-22 18:32 ` Nathan Chancellor
0 siblings, 1 reply; 10+ messages in thread
From: Mark Brown @ 2022-08-22 18:09 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Matt Flax, kernel test robot, alsa-devel, llvm, kbuild-all
[-- 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 --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] ASoC: codecs: add uspport for the TI SRC4392 codec
2022-08-22 18:09 ` Mark Brown
@ 2022-08-22 18:32 ` Nathan Chancellor
0 siblings, 0 replies; 10+ messages in thread
From: Nathan Chancellor @ 2022-08-22 18:32 UTC (permalink / raw)
To: Mark Brown; +Cc: Matt Flax, kernel test robot, alsa-devel, llvm, kbuild-all
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
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-08-22 18:32 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20220808214028.2502801-1-flatmax@flatmax.com>
2022-08-09 1:49 ` [PATCH v2] ASoC: codecs: add uspport for the TI SRC4392 codec kernel test robot
2022-08-16 18:00 ` Nathan Chancellor
2022-08-17 5:44 ` Matt Flax
2022-08-17 15:45 ` Nathan Chancellor
2022-08-22 18:09 ` Mark Brown
2022-08-22 18:32 ` Nathan Chancellor
2022-08-17 16:21 ` Mark Brown
2022-08-17 17:23 ` Nathan Chancellor
2022-08-18 11:44 ` Mark Brown
2022-08-09 8:25 ` kernel test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox