Re: [PATCH v9 0/6] staging: media: wave5: add wave5 codec driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Nas,

On 28/06/2022 13:08, Nas Chung wrote:
> The Wave5 codec driver is a stateful encoder/decoder.
> It is found on the J721S2 SoC, JH7100 SoC, ssd202d SoC. Etc.
> But current test report is based on J721S2 SoC and pre-silicon FPGA.
> 
> The driver currently supports V4L2_PIX_FMT_HEVC, V4L2_PIX_FMT_H264.
> 
> This driver has so far been tested on J721S2 EVM board and pre-silicon
> FPGA.
> 
> Testing on J721S2 EVM board shows it working fine both decoder and
> encoder.
> The driver is successfully working with gstreamer v4l2 good-plugin
> without any modification.
> 
> Testing on FPGA also shows it working fine, though the FPGA uses polled
> interrupts and copied buffers between the host and it's on board RAM.

I wanted to merge this series, but I got several compile/smatch
errors/warnings:

There are several compile errors under i686 (i.e. a 32 bit architecture):

linux-git-i686: WARNINGS

/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-dec.c: In function 'wave5_vpu_open_dec':
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-dec.c:1358:16: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
 1358 |         return ret;
      |                ^~~
In file included from /home/hans/work/build/media-git/include/linux/device.h:15,
                 from /home/hans/work/build/media-git/include/linux/platform_device.h:13,
                 from /home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu.c:9:
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu.c: In function 'wave5_vpu_probe':
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu.c:232:29: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 3 has type 'dma_addr_t' {aka
'unsigned int'} [-Wformat=]
  232 |         dev_err(&pdev->dev, "sram daddr: 0x%llx, size: 0x%lx\n",
      |                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/hans/work/build/media-git/include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
  110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
      |                              ^~~
/home/hans/work/build/media-git/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__)
      |                                                        ^~~~~~~
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu.c:232:9: note: in expansion of macro 'dev_err'
  232 |         dev_err(&pdev->dev, "sram daddr: 0x%llx, size: 0x%lx\n",
      |         ^~~~~~~
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu.c:232:47: note: format string is defined here
  232 |         dev_err(&pdev->dev, "sram daddr: 0x%llx, size: 0x%lx\n",
      |                                            ~~~^
      |                                               |
      |                                               long long unsigned int
      |                                            %x
In file included from /home/hans/work/build/media-git/include/linux/device.h:15,
                 from /home/hans/work/build/media-git/include/linux/platform_device.h:13,
                 from /home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu.c:9:
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu.c:232:29: warning: format '%lx' expects argument of type 'long unsigned int', but argument 4 has type 'size_t' {aka 'unsigned
int'} [-Wformat=]
  232 |         dev_err(&pdev->dev, "sram daddr: 0x%llx, size: 0x%lx\n",
      |                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/hans/work/build/media-git/include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
  110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
      |                              ^~~
/home/hans/work/build/media-git/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__)
      |                                                        ^~~~~~~
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu.c:232:9: note: in expansion of macro 'dev_err'
  232 |         dev_err(&pdev->dev, "sram daddr: 0x%llx, size: 0x%lx\n",
      |         ^~~~~~~
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu.c:232:60: note: format string is defined here
  232 |         dev_err(&pdev->dev, "sram daddr: 0x%llx, size: 0x%lx\n",
      |                                                          ~~^
      |                                                            |
      |                                                            long unsigned int
      |                                                          %x
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-enc.c: In function 'wave5_vpu_open_enc':
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-enc.c:1479:16: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
 1479 |         return ret;
      |                ^~~

linux-git-x86_64: WARNINGS

/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-dec.c: In function 'wave5_vpu_open_dec':
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-dec.c:1358:16: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
 1358 |         return ret;
      |                ^~~
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-enc.c: In function 'wave5_vpu_open_enc':
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-enc.c:1479:16: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
 1479 |         return ret;
      |                ^~~

And smatch gives this:

/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-dec.c:1143:21: warning: variable 'ret' set but not used [-Wunused-but-set-variable]
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-dec.c:1358:16: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-enc.c:1247:21: warning: variable 'ret' set but not used [-Wunused-but-set-variable]
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-enc.c:1479:16: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpuapi.c: /home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpuapi.c:93 wave5_check_dec_open_param() warn: maybe use
&& instead of &
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpuapi.c: /home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpuapi.c:505 wave5_vpu_dec_get_output_info() warn:
inconsistent indenting
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-dec.c: /home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-dec.c:1357 wave5_vpu_open_dec() warn: '&inst->list'
not removed from list
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-dec.c: /home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-dec.c:1358 wave5_vpu_open_dec() error: uninitialized
symbol 'ret'.
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-enc.c: /home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-enc.c:1478 wave5_vpu_open_enc() warn: '&inst->list'
not removed from list
/home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-enc.c: /home/hans/work/build/media-git/drivers/staging/media/wave5/wave5-vpu-enc.c:1479 wave5_vpu_open_enc() error: uninitialized
symbol 'ret'.

Also two kerneldoc issues:

kerneldoc: WARNINGS
drivers/staging/media/wave5/wave5-vpuapi.h:144: warning: Cannot understand  * \brief parameters of DEC_SET_SEQ_CHANGE_MASK
drivers/staging/media/wave5/wave5-vdi.h:68: warning: Cannot understand  * @brief make clock stable before changing clock frequency

And finally: quite a lot of the new sources have an empty line at the end. Please remove that.

It's all small stuff, but I'd appreciate it if you can fix it and post a v7.

Regards,

	Hans




[Index of Archives]     [Linux Driver Development]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux