Re: [PATCH] staging: axis-fifo: initialize timeouts in probe only

From: Nathan Chancellor <nathan@kernel.org>
To: Khadija Kamran <kamrankhadijadj@gmail.com>
Cc: outreachy@lists.linux.dev, llvm@lists.linux.dev,
	oe-kbuild-all@lists.linux.dev,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: axis-fifo: initialize timeouts in probe only
Date: Tue, 14 Mar 2023 07:42:07 -0700	[thread overview]
Message-ID: <20230314144207.GA4106922@dev-arch.thelio-3990X> (raw)
In-Reply-To: <ZBB/30ZBW9EU1QfK@khadija-virtual-machine>

Hi Khadija,

On Tue, Mar 14, 2023 at 07:08:31PM +0500, Khadija Kamran wrote:
> On Tue, Mar 14, 2023 at 11:45:51AM +0800, kernel test robot wrote:
> > Hi Khadija,
> > 
> > Thank you for the patch! Perhaps something to improve:
> > 
> > [auto build test WARNING on staging/staging-testing]
> > 
> > url:    https://github.com/intel-lab-lkp/linux/commits/Khadija-Kamran/staging-axis-fifo-initialize-timeouts-in-probe-only/20230314-020827
> > patch link:    https://lore.kernel.org/r/ZA9mThZ7NyRrQAMX%40khadija-virtual-machine
> > patch subject: [PATCH] staging: axis-fifo: initialize timeouts in probe only
> > config: arm64-randconfig-r012-20230313 (https://download.01.org/0day-ci/archive/20230314/202303141159.6wN9HNP9-lkp@intel.com/config)
> > compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 67409911353323ca5edf2049ef0df54132fa1ca7)
> > 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
> >         # install arm64 cross compiling tool for clang build
> >         # apt-get install binutils-aarch64-linux-gnu
> >         # https://github.com/intel-lab-lkp/linux/commit/9d186f6c9f9bf467b48da3e28b0e9aa31fc3faf3
> >         git remote add linux-review https://github.com/intel-lab-lkp/linux
> >         git fetch --no-tags linux-review Khadija-Kamran/staging-axis-fifo-initialize-timeouts-in-probe-only/20230314-020827
> >         git checkout 9d186f6c9f9bf467b48da3e28b0e9aa31fc3faf3
> >         # 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=arm64 olddefconfig
> >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/staging/axis-fifo/
> > 
> > If you fix the issue, kindly add following tag where applicable
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Link: https://lore.kernel.org/oe-kbuild-all/202303141159.6wN9HNP9-lkp@intel.com/
> > 
> > All warnings (new ones prefixed by >>):
> > 
> > >> drivers/staging/axis-fifo/axis-fifo.c:817:18: warning: implicit conversion from 'long' to 'int' changes value from 9223372036854775807 to -1 [-Wconstant-conversion]
> >                    read_timeout = MAX_SCHEDULE_TIMEOUT;
> >                                 ~ ^~~~~~~~~~~~~~~~~~~~
> >    include/linux/sched.h:296:31: note: expanded from macro 'MAX_SCHEDULE_TIMEOUT'
> >    #define MAX_SCHEDULE_TIMEOUT            LONG_MAX
> >                                            ^~~~~~~~
> >    include/vdso/limits.h:11:19: note: expanded from macro 'LONG_MAX'
> >    #define LONG_MAX        ((long)(~0UL >> 1))
> >                             ^~~~~~~~~~~~~~~~~
> >    drivers/staging/axis-fifo/axis-fifo.c:822:19: warning: implicit conversion from 'long' to 'int' changes value from 9223372036854775807 to -1 [-Wconstant-conversion]
> >                    write_timeout = MAX_SCHEDULE_TIMEOUT;
> >                                  ~ ^~~~~~~~~~~~~~~~~~~~
> >    include/linux/sched.h:296:31: note: expanded from macro 'MAX_SCHEDULE_TIMEOUT'
> >    #define MAX_SCHEDULE_TIMEOUT            LONG_MAX
> >                                            ^~~~~~~~
> >    include/vdso/limits.h:11:19: note: expanded from macro 'LONG_MAX'
> >    #define LONG_MAX        ((long)(~0UL >> 1))
> >                             ^~~~~~~~~~~~~~~~~
> >    2 warnings generated.
> >
> 
> Hi everyone!
> Kindly let me know if I should look into these warnings.
> Thank you!

You should always avoid introducing new warnings whenever possible. In
this case, it appears that read_timeout and write_timeout should be
changed from 'int' to 'long' to account for the fact that
MAX_SCHEDULE_TIMEOUT is being assigned to it directly now, versus being
passed as a parameter to wait_event_interruptible_timeout(), which
assigned it to 'long' anyways.

If you have any other questions or need further help, let me know :)

Cheers,
Nathan

> > vim +817 drivers/staging/axis-fifo/axis-fifo.c
> > 
> >    805	
> >    806	static int axis_fifo_probe(struct platform_device *pdev)
> >    807	{
> >    808		struct resource *r_mem; /* IO mem resources */
> >    809		struct device *dev = &pdev->dev; /* OS device (from device tree) */
> >    810		struct axis_fifo *fifo = NULL;
> >    811		char *device_name;
> >    812		int rc = 0; /* error return value */
> >    813	
> >    814		if (read_timeout >= 0)
> >    815			read_timeout = msecs_to_jiffies(read_timeout);
> >    816		else
> >  > 817			read_timeout = MAX_SCHEDULE_TIMEOUT;
> >    818	
> >    819		if (write_timeout >= 0)
> >    820			write_timeout = msecs_to_jiffies(write_timeout);
> >    821		else
> >    822			write_timeout = MAX_SCHEDULE_TIMEOUT;
> >    823	
> >    824		/* ----------------------------
> >    825		 *     init wrapper device
> >    826		 * ----------------------------
> >    827		 */
> >    828	
> >    829		device_name = devm_kzalloc(dev, 32, GFP_KERNEL);
> >    830		if (!device_name)
> >    831			return -ENOMEM;
> >    832	
> >    833		/* allocate device wrapper memory */
> >    834		fifo = devm_kzalloc(dev, sizeof(*fifo), GFP_KERNEL);
> >    835		if (!fifo)
> >    836			return -ENOMEM;
> >    837	
> >    838		dev_set_drvdata(dev, fifo);
> >    839		fifo->dt_device = dev;
> >    840	
> >    841		init_waitqueue_head(&fifo->read_queue);
> >    842		init_waitqueue_head(&fifo->write_queue);
> >    843	
> >    844		mutex_init(&fifo->read_lock);
> >    845		mutex_init(&fifo->write_lock);
> >    846	
> >    847		/* ----------------------------
> >    848		 *   init device memory space
> >    849		 * ----------------------------
> >    850		 */
> >    851	
> >    852		/* get iospace for the device */
> >    853		r_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >    854		if (!r_mem) {
> >    855			dev_err(fifo->dt_device, "invalid address\n");
> >    856			rc = -ENODEV;
> >    857			goto err_initial;
> >    858		}
> >    859	
> >    860		/* request physical memory */
> >    861		fifo->base_addr = devm_ioremap_resource(fifo->dt_device, r_mem);
> >    862		if (IS_ERR(fifo->base_addr)) {
> >    863			rc = PTR_ERR(fifo->base_addr);
> >    864			goto err_initial;
> >    865		}
> >    866	
> >    867		dev_dbg(fifo->dt_device, "remapped memory to 0x%p\n", fifo->base_addr);
> >    868	
> >    869		/* create unique device name */
> >    870		snprintf(device_name, 32, "%s_%pa", DRIVER_NAME, &r_mem->start);
> >    871		dev_dbg(fifo->dt_device, "device name [%s]\n", device_name);
> >    872	
> >    873		/* ----------------------------
> >    874		 *          init IP
> >    875		 * ----------------------------
> >    876		 */
> >    877	
> >    878		rc = axis_fifo_parse_dt(fifo);
> >    879		if (rc)
> >    880			goto err_initial;
> >    881	
> >    882		reset_ip_core(fifo);
> >    883	
> >    884		/* ----------------------------
> >    885		 *    init device interrupts
> >    886		 * ----------------------------
> >    887		 */
> >    888	
> >    889		/* get IRQ resource */
> >    890		rc = platform_get_irq(pdev, 0);
> >    891		if (rc < 0)
> >    892			goto err_initial;
> >    893	
> >    894		/* request IRQ */
> >    895		fifo->irq = rc;
> >    896		rc = devm_request_irq(fifo->dt_device, fifo->irq, &axis_fifo_irq, 0,
> >    897				      DRIVER_NAME, fifo);
> >    898		if (rc) {
> >    899			dev_err(fifo->dt_device, "couldn't allocate interrupt %i\n",
> >    900				fifo->irq);
> >    901			goto err_initial;
> >    902		}
> >    903	
> >    904		/* ----------------------------
> >    905		 *      init char device
> >    906		 * ----------------------------
> >    907		 */
> >    908	
> >    909		/* create character device */
> >    910		fifo->miscdev.fops = &fops;
> >    911		fifo->miscdev.minor = MISC_DYNAMIC_MINOR;
> >    912		fifo->miscdev.name = device_name;
> >    913		fifo->miscdev.groups = axis_fifo_attrs_groups;
> >    914		fifo->miscdev.parent = dev;
> >    915		rc = misc_register(&fifo->miscdev);
> >    916		if (rc < 0)
> >    917			goto err_initial;
> >    918	
> >    919		dev_info(fifo->dt_device, "axis-fifo created at %pa mapped to 0x%pa, irq=%i\n",
> >    920			 &r_mem->start, &fifo->base_addr, fifo->irq);
> >    921	
> >    922		return 0;
> >    923	
> >    924	err_initial:
> >    925		dev_set_drvdata(dev, NULL);
> >    926		return rc;
> >    927	}
> >    928	
> > 
> > -- 
> > 0-DAY CI Kernel Test Service
> > https://github.com/intel/lkp-tests
> 