Re: [PATCH v2 11/15] ASoC: Intel: avs: Machine board registration

["Amadeusz Sławiński" <amadeuszx.slawinski@linux.intel.com>]

On 5/9/2022 1:58 PM, kernel test robot wrote:
> Hi Cezary,
> 
> I love your patch! Perhaps something to improve:
> 
> [auto build test WARNING on broonie-sound/for-next]
> [also build test WARNING on next-20220506]
> [cannot apply to tiwai-sound/for-next v5.18-rc6]
> [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]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Cezary-Rojewski/ASoC-Intel-avs-Driver-core-and-PCM-operations/20220509-165656
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
> config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20220509/202205091954.7hT2TBhd-lkp@intel.com/config)
> compiler: arceb-elf-gcc (GCC) 11.3.0
> 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/2ef25400a7aee5a8e75c2ccdb5618be31c9f6809
>          git remote add linux-review https://github.com/intel-lab-lkp/linux
>          git fetch --no-tags linux-review Cezary-Rojewski/ASoC-Intel-avs-Driver-core-and-PCM-operations/20220509-165656
>          git checkout 2ef25400a7aee5a8e75c2ccdb5618be31c9f6809
>          # save the config file
>          mkdir build_dir && cp config build_dir/.config
>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash sound/soc/intel/avs/
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All warnings (new ones prefixed by >>):
> 
>     In file included from include/linux/device.h:15,
>                      from include/linux/acpi.h:15,
>                      from sound/soc/intel/avs/board_selection.c:9:
>     sound/soc/intel/avs/board_selection.c: In function 'avs_register_i2s_board':
>>> sound/soc/intel/avs/board_selection.c:328:36: warning: format '%ld' expects argument of type 'long int', but argument 5 has type 'int' [-Wformat=]
>       328 |                 dev_err(adev->dev, "Platform supports %d SSPs but board %s requires SSP%ld\n",
>           |                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
>       110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
>           |                              ^~~
>     include/linux/dev_printk.h:144:56: note: in expansion of macro 'dev_fmt'
>       144 |         dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
>           |                                                        ^~~~~~~
>     sound/soc/intel/avs/board_selection.c:328:17: note: in expansion of macro 'dev_err'
>       328 |                 dev_err(adev->dev, "Platform supports %d SSPs but board %s requires SSP%ld\n",
>           |                 ^~~~~~~
>     sound/soc/intel/avs/board_selection.c:328:90: note: format string is defined here
>       328 |                 dev_err(adev->dev, "Platform supports %d SSPs but board %s requires SSP%ld\n",
>           |                                                                                        ~~^
>           |                                                                                          |
>           |                                                                                          long int
>           |                                                                                        %d
> 
> 
> vim +328 sound/soc/intel/avs/board_selection.c
> 
>     318	
>     319	static int avs_register_i2s_board(struct avs_dev *adev, struct snd_soc_acpi_mach *mach)
>     320	{
>     321		struct platform_device *board;
>     322		int num_ssps;
>     323		char *name;
>     324		int ret;
>     325	
>     326		num_ssps = adev->hw_cfg.i2s_caps.ctrl_count;
>     327		if (fls(mach->mach_params.i2s_link_mask) > num_ssps) {
>   > 328			dev_err(adev->dev, "Platform supports %d SSPs but board %s requires SSP%ld\n",
>     329				num_ssps, mach->drv_name, __fls(mach->mach_params.i2s_link_mask));
>     330			return -ENODEV;
>     331		}
>     332	
>     333		name = devm_kasprintf(adev->dev, GFP_KERNEL, "%s.%d-platform", mach->drv_name,
>     334				      mach->mach_params.i2s_link_mask);
>     335		if (!name)
>     336			return -ENOMEM;
>     337	
>     338		ret = avs_i2s_platform_register(adev, name, mach->mach_params.i2s_link_mask, mach->pdata);
>     339		if (ret < 0)
>     340			return ret;
>     341	
>     342		mach->mach_params.platform = name;
>     343	
>     344		board = platform_device_register_data(NULL, mach->drv_name, mach->mach_params.i2s_link_mask,
>     345						      (const void *)mach, sizeof(*mach));
>     346		if (IS_ERR(board)) {
>     347			dev_err(adev->dev, "ssp board register failed\n");
>     348			return PTR_ERR(board);
>     349		}
>     350	
>     351		ret = devm_add_action(adev->dev, board_pdev_unregister, board);
>     352		if (ret < 0) {
>     353			platform_device_unregister(board);
>     354			return ret;
>     355		}
>     356	
>     357		return 0;
>     358	}
>     359	
> 

Kernel test robot warns us about __fls and it is right, as __fls 
depending on architecture returns either unsigned int or unsigned long.
But I would say that this seems questionable, as I would expect 
consistent return value between arches, especially for functions where 
we operate on bits and probably don't want inconsistent results.

Generic asm header [1] seems to suggest that it should accept unsigned 
long as parameter and return unsigned long. It seems however that arc 
accepts unsigned long as argument and returns int, while m68k uses int 
for argument and return value...

Adding relevant architecture maintainers to CC.

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/asm-generic/bitops/__fls.h