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

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

[Khadija Kamran]

Module parameter, read_timeout, can only be set at the loading time. As
it can only be modified once, initialize read_timeout once in the probe
function.
As a result, only use read_timeout as the last argument in
wait_event_interruptible_timeout() call.

Same goes for write_timeout.

Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Khadija Kamran <kamrankhadijadj@gmail.com>
---
 drivers/staging/axis-fifo/axis-fifo.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/axis-fifo/axis-fifo.c b/drivers/staging/axis-fifo/axis-fifo.c
index dfd2b357f484..563caf155713 100644
--- a/drivers/staging/axis-fifo/axis-fifo.c
+++ b/drivers/staging/axis-fifo/axis-fifo.c
@@ -384,9 +384,7 @@ static ssize_t axis_fifo_read(struct file *f, char __user *buf,
 		mutex_lock(&fifo->read_lock);
 		ret = wait_event_interruptible_timeout(fifo->read_queue,
 			ioread32(fifo->base_addr + XLLF_RDFO_OFFSET),
-				 (read_timeout >= 0) ?
-				  msecs_to_jiffies(read_timeout) :
-				  MAX_SCHEDULE_TIMEOUT);
+			read_timeout);
 
 		if (ret <= 0) {
 			if (ret == 0) {
@@ -528,9 +526,7 @@ static ssize_t axis_fifo_write(struct file *f, const char __user *buf,
 		ret = wait_event_interruptible_timeout(fifo->write_queue,
 			ioread32(fifo->base_addr + XLLF_TDFV_OFFSET)
 				 >= words_to_write,
-				 (write_timeout >= 0) ?
-				  msecs_to_jiffies(write_timeout) :
-				  MAX_SCHEDULE_TIMEOUT);
+			write_timeout);
 
 		if (ret <= 0) {
 			if (ret == 0) {
@@ -815,6 +811,16 @@ static int axis_fifo_probe(struct platform_device *pdev)
 	char *device_name;
 	int rc = 0; /* error return value */
 
+	if (read_timeout >= 0)
+		read_timeout = msecs_to_jiffies(read_timeout);
+	else
+		read_timeout = MAX_SCHEDULE_TIMEOUT;
+
+	if (write_timeout >= 0)
+		write_timeout = msecs_to_jiffies(write_timeout);
+	else
+		write_timeout = MAX_SCHEDULE_TIMEOUT;
+
 	/* ----------------------------
 	 *     init wrapper device
 	 * ----------------------------
-- 
2.34.1

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

[kernel test robot]

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: ia64-allyesconfig (https://download.01.org/0day-ci/archive/20230314/202303140222.Ku4vFjgO-lkp@intel.com/config)
compiler: ia64-linux-gcc (GCC) 12.1.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/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=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/staging/

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/202303140222.Ku4vFjgO-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from include/linux/limits.h:7,
                    from include/linux/kernel.h:16,
                    from drivers/staging/axis-fifo/axis-fifo.c:17:
   drivers/staging/axis-fifo/axis-fifo.c: In function 'axis_fifo_probe':
>> include/vdso/limits.h:11:25: warning: overflow in conversion from 'long int' to 'int' changes value from '9223372036854775807' to '-1' [-Woverflow]
      11 | #define LONG_MAX        ((long)(~0UL >> 1))
         |                         ^
   include/linux/sched.h:296:41: note: in expansion of macro 'LONG_MAX'
     296 | #define MAX_SCHEDULE_TIMEOUT            LONG_MAX
         |                                         ^~~~~~~~
   drivers/staging/axis-fifo/axis-fifo.c:817:32: note: in expansion of macro 'MAX_SCHEDULE_TIMEOUT'
     817 |                 read_timeout = MAX_SCHEDULE_TIMEOUT;
         |                                ^~~~~~~~~~~~~~~~~~~~
>> include/vdso/limits.h:11:25: warning: overflow in conversion from 'long int' to 'int' changes value from '9223372036854775807' to '-1' [-Woverflow]
      11 | #define LONG_MAX        ((long)(~0UL >> 1))
         |                         ^
   include/linux/sched.h:296:41: note: in expansion of macro 'LONG_MAX'
     296 | #define MAX_SCHEDULE_TIMEOUT            LONG_MAX
         |                                         ^~~~~~~~
   drivers/staging/axis-fifo/axis-fifo.c:822:33: note: in expansion of macro 'MAX_SCHEDULE_TIMEOUT'
     822 |                 write_timeout = MAX_SCHEDULE_TIMEOUT;
         |                                 ^~~~~~~~~~~~~~~~~~~~


vim +11 include/vdso/limits.h

3e0e9f8c6e3ca9 Vincenzo Frascino 2020-03-20   4  
3e0e9f8c6e3ca9 Vincenzo Frascino 2020-03-20   5  #define USHRT_MAX	((unsigned short)~0U)
3e0e9f8c6e3ca9 Vincenzo Frascino 2020-03-20   6  #define SHRT_MAX	((short)(USHRT_MAX >> 1))
3e0e9f8c6e3ca9 Vincenzo Frascino 2020-03-20   7  #define SHRT_MIN	((short)(-SHRT_MAX - 1))
3e0e9f8c6e3ca9 Vincenzo Frascino 2020-03-20   8  #define INT_MAX		((int)(~0U >> 1))
3e0e9f8c6e3ca9 Vincenzo Frascino 2020-03-20   9  #define INT_MIN		(-INT_MAX - 1)
3e0e9f8c6e3ca9 Vincenzo Frascino 2020-03-20  10  #define UINT_MAX	(~0U)
3e0e9f8c6e3ca9 Vincenzo Frascino 2020-03-20 @11  #define LONG_MAX	((long)(~0UL >> 1))
3e0e9f8c6e3ca9 Vincenzo Frascino 2020-03-20  12  #define LONG_MIN	(-LONG_MAX - 1)
3e0e9f8c6e3ca9 Vincenzo Frascino 2020-03-20  13  #define ULONG_MAX	(~0UL)
3e0e9f8c6e3ca9 Vincenzo Frascino 2020-03-20  14  #define LLONG_MAX	((long long)(~0ULL >> 1))
3e0e9f8c6e3ca9 Vincenzo Frascino 2020-03-20  15  #define LLONG_MIN	(-LLONG_MAX - 1)
3e0e9f8c6e3ca9 Vincenzo Frascino 2020-03-20  16  #define ULLONG_MAX	(~0ULL)
3e0e9f8c6e3ca9 Vincenzo Frascino 2020-03-20  17  #define UINTPTR_MAX	ULONG_MAX
3e0e9f8c6e3ca9 Vincenzo Frascino 2020-03-20  18  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

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

[kernel test robot]

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.


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

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

[Khadija Kamran]

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!

Regards,
Khadija


> 
> 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

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

[Nathan Chancellor]

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
> 

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

[Alison Schofield]

On Tue, Mar 14, 2023 at 07:42:07AM -0700, Nathan Chancellor wrote:
> 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

Hi Khadaji,

Echoing Nathan's feedback - yes, you need to correct these.

Do you see these warnings when you compiled?  If yes, then your path to
fixing them is easier. Make sure it recompiles with no warnings.

If you don't see these warnings, then you need to follow the steps to
reproduce in the lkp email. You have to be able to 'see the warning'
make a change and 'see it go away'.

Alison

> 
> > > 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
> > 
> 

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

[Fabio M. De Francesco]

On martedì 14 marzo 2023 16:13:53 CET Alison Schofield wrote:
> On Tue, Mar 14, 2023 at 07:42:07AM -0700, Nathan Chancellor wrote:
> > 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 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
> 
> Hi Khadija,
> 
> Echoing Nathan's feedback - yes, you need to correct these.
> 
> Do you see these warnings when you compiled?  If yes, then your path to
> fixing them is easier. Make sure it recompiles with no warnings.
> 
> If you don't see these warnings, then you need to follow the steps to
> reproduce in the lkp email. You have to be able to 'see the warning'
> make a change and 'see it go away'.
> 
> Alison
> 

Hi Khadija,

I would suggest trying to recompile using "make W=1 -j...". Not sure if you'll 
see the warnings (depends on your compiler, version and target architecture). 
However, it's worth a try as it may save you the time it takes to go through 
the steps for reproduction.

Anyway, as Nathan said, that warning is triggered by assigning 'long' to 'int' 
(i.e., "MAX_SCHEDULE_TIMEOUT" is cast to 'long', while the types of 
"read_timeout" and "write_timeout " are both 'int'). Obviously, these kinds of  
assignments overflow wherever the representation of type 'long' uses more bits 
than the type 'int'.

That implies that you should try to understand where the module's parameters 
are defined and how to work with the related macros.

I hope it helps.

Fabio

P.S.: If you don't know yet what to do, take a look at the following document:
https://sysprog21.github.io/lkmpg/ (The Linux Kernel Module Programming 
Guide).
   

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

[Alison Schofield]

On Mon, Mar 13, 2023 at 11:07:10PM +0500, Khadija Kamran wrote:
> Module parameter, read_timeout, can only be set at the loading time. As
> it can only be modified once, initialize read_timeout once in the probe
> function.
> As a result, only use read_timeout as the last argument in
> wait_event_interruptible_timeout() call.
> 
> Same goes for write_timeout.
> 
> Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Khadija Kamran <kamrankhadijadj@gmail.com>
> ---

Looks like this is [PATCH v5] and needs a changelog.


>  drivers/staging/axis-fifo/axis-fifo.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/axis-fifo/axis-fifo.c b/drivers/staging/axis-fifo/axis-fifo.c
> index dfd2b357f484..563caf155713 100644
> --- a/drivers/staging/axis-fifo/axis-fifo.c
> +++ b/drivers/staging/axis-fifo/axis-fifo.c
> @@ -384,9 +384,7 @@ static ssize_t axis_fifo_read(struct file *f, char __user *buf,
>  		mutex_lock(&fifo->read_lock);
>  		ret = wait_event_interruptible_timeout(fifo->read_queue,
>  			ioread32(fifo->base_addr + XLLF_RDFO_OFFSET),
> -				 (read_timeout >= 0) ?
> -				  msecs_to_jiffies(read_timeout) :
> -				  MAX_SCHEDULE_TIMEOUT);
> +			read_timeout);
>  
>  		if (ret <= 0) {
>  			if (ret == 0) {
> @@ -528,9 +526,7 @@ static ssize_t axis_fifo_write(struct file *f, const char __user *buf,
>  		ret = wait_event_interruptible_timeout(fifo->write_queue,
>  			ioread32(fifo->base_addr + XLLF_TDFV_OFFSET)
>  				 >= words_to_write,
> -				 (write_timeout >= 0) ?
> -				  msecs_to_jiffies(write_timeout) :
> -				  MAX_SCHEDULE_TIMEOUT);
> +			write_timeout);
>  
>  		if (ret <= 0) {
>  			if (ret == 0) {
> @@ -815,6 +811,16 @@ static int axis_fifo_probe(struct platform_device *pdev)
>  	char *device_name;
>  	int rc = 0; /* error return value */
>  
> +	if (read_timeout >= 0)
> +		read_timeout = msecs_to_jiffies(read_timeout);
> +	else
> +		read_timeout = MAX_SCHEDULE_TIMEOUT;
> +
> +	if (write_timeout >= 0)
> +		write_timeout = msecs_to_jiffies(write_timeout);
> +	else
> +		write_timeout = MAX_SCHEDULE_TIMEOUT;
> +
>  	/* ----------------------------
>  	 *     init wrapper device
>  	 * ----------------------------
> -- 
> 2.34.1
> 
> 

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

[Fabio M. De Francesco]

On martedì 14 marzo 2023 21:43:40 CET Alison Schofield wrote:
> On Mon, Mar 13, 2023 at 11:07:10PM +0500, Khadija Kamran wrote:
> > Module parameter, read_timeout, can only be set at the loading time. As
> > it can only be modified once, initialize read_timeout once in the probe
> > function.
> > As a result, only use read_timeout as the last argument in
> > wait_event_interruptible_timeout() call.
> > 
> > Same goes for write_timeout.
> > 
> > Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Signed-off-by: Khadija Kamran <kamrankhadijadj@gmail.com>
> > ---
> 
> Looks like this is [PATCH v5] and needs a changelog.
>
Alison,

In fact, this is only the second patch that addresses Greg's suggested 
refactoring. 

Khadija moved from v3 of "staging: axis-fifo: remove tabs to align arguments" 
to v4 of this completely independent patch. And then back to v1, because (at 
the time of v4) I pointed out to her that she had started working on a project 
that has a completely different purpose than the previous one.

The best course of action would have been to ask Greg to drop the previous 
patches and then reset the numbering of the new job to v1. Unfortunately I did 
not pay attention to how she then managed the numbering following my 
observation.

What would be the best course of action at this point?

Fabio

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

[Alison Schofield]

On Tue, Mar 14, 2023 at 10:31:21PM +0100, Fabio wrote:
> On martedì 14 marzo 2023 21:43:40 CET Alison Schofield wrote:
> > On Mon, Mar 13, 2023 at 11:07:10PM +0500, Khadija Kamran wrote:
> > > Module parameter, read_timeout, can only be set at the loading time. As
> > > it can only be modified once, initialize read_timeout once in the probe
> > > function.
> > > As a result, only use read_timeout as the last argument in
> > > wait_event_interruptible_timeout() call.
> > > 
> > > Same goes for write_timeout.
> > > 
> > > Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Signed-off-by: Khadija Kamran <kamrankhadijadj@gmail.com>
> > > ---
> > 
> > Looks like this is [PATCH v5] and needs a changelog.
> >
> Alison,
> 
> In fact, this is only the second patch that addresses Greg's suggested 
> refactoring. 
> 
> Khadija moved from v3 of "staging: axis-fifo: remove tabs to align arguments" 
> to v4 of this completely independent patch. And then back to v1, because (at 
> the time of v4) I pointed out to her that she had started working on a project 
> that has a completely different purpose than the previous one.
> 
> The best course of action would have been to ask Greg to drop the previous 
> patches and then reset the numbering of the new job to v1. Unfortunately I did 
> not pay attention to how she then managed the numbering following my 
> observation.
> 
> What would be the best course of action at this point?

My guess is that this patch gets ignored because it has a lower version
number than a previous patch.

Take the feedback given here, and rev to
[PATCH v5] staging: axis-fifo: initialize timeouts in probe only

Be sure the Changelog, below the --- explains the journey.

Changes in v5:

Changes in v4:

Changes in v3:

Changes in v2:


> 
> Fabio
> 
> 

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

[Khadija Kamran]

On Tue, Mar 14, 2023 at 04:57:47PM -0700, Alison Schofield wrote:
> On Tue, Mar 14, 2023 at 10:31:21PM +0100, Fabio wrote:
> > On martedì 14 marzo 2023 21:43:40 CET Alison Schofield wrote:
> > > On Mon, Mar 13, 2023 at 11:07:10PM +0500, Khadija Kamran wrote:
> > > > Module parameter, read_timeout, can only be set at the loading time. As
> > > > it can only be modified once, initialize read_timeout once in the probe
> > > > function.
> > > > As a result, only use read_timeout as the last argument in
> > > > wait_event_interruptible_timeout() call.
> > > > 
> > > > Same goes for write_timeout.
> > > > 
> > > > Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > Signed-off-by: Khadija Kamran <kamrankhadijadj@gmail.com>
> > > > ---
> > > 
> > > Looks like this is [PATCH v5] and needs a changelog.
> > >
> > Alison,
> > 
> > In fact, this is only the second patch that addresses Greg's suggested 
> > refactoring. 
> > 
> > Khadija moved from v3 of "staging: axis-fifo: remove tabs to align arguments" 
> > to v4 of this completely independent patch. And then back to v1, because (at 
> > the time of v4) I pointed out to her that she had started working on a project 
> > that has a completely different purpose than the previous one.
> > 
> > The best course of action would have been to ask Greg to drop the previous 
> > patches and then reset the numbering of the new job to v1. Unfortunately I did 
> > not pay attention to how she then managed the numbering following my 
> > observation.
> > 
> > What would be the best course of action at this point?
> 
> My guess is that this patch gets ignored because it has a lower version
> number than a previous patch.
> 
> Take the feedback given here, and rev to
> [PATCH v5] staging: axis-fifo: initialize timeouts in probe only
> 
> Be sure the Changelog, below the --- explains the journey.
> 
> Changes in v5:
> 
> Changes in v4:
> 
> Changes in v3:
> 
> Changes in v2:
> 

Hey Alison!
Based on Nathan's feedback I am trying to recompile and send a patch
without any warnings.
As suggested by Fabio, I am running "make w=1 -jX" command to see if I
get any warnings. But it is taking a lot of time, is there any way of
speeding it up?
If this doesn't work then I have to follow the steps to reproduce in lkp
mail as you said before. 
After dealing with these warnings I will send a [PATCH v5], following
your instructions above.
Kindly, let me know if I am on the wrong track.
Thank you!

Regards,
Khadija

> 
> > 
> > Fabio
> > 
> > 

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

[Fabio M. De Francesco]

On mercoledì 15 marzo 2023 13:32:55 CET Khadija Kamran wrote:
> On Tue, Mar 14, 2023 at 04:57:47PM -0700, Alison Schofield wrote:
> > On Tue, Mar 14, 2023 at 10:31:21PM +0100, Fabio wrote:
> > > On martedì 14 marzo 2023 21:43:40 CET Alison Schofield wrote:
> > > > On Mon, Mar 13, 2023 at 11:07:10PM +0500, Khadija Kamran wrote:
> > > > > Module parameter, read_timeout, can only be set at the loading time.
> > > > > As
> > > > > it can only be modified once, initialize read_timeout once in the
> > > > > probe
> > > > > function.
> > > > > As a result, only use read_timeout as the last argument in
> > > > > wait_event_interruptible_timeout() call.
> > > > > 
> > > > > Same goes for write_timeout.
> > > > > 
> > > > > Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > > Signed-off-by: Khadija Kamran <kamrankhadijadj@gmail.com>
> > > > > ---
> > > > 
> > > > Looks like this is [PATCH v5] and needs a changelog.
> > > 
> > > Alison,
> > > 
> > > In fact, this is only the second patch that addresses Greg's suggested
> > > refactoring.
> > > 
> > > Khadija moved from v3 of "staging: axis-fifo: remove tabs to align
> > > arguments"
> > > to v4 of this completely independent patch. And then back to v1, because
> > > (at
> > > the time of v4) I pointed out to her that she had started working on a
> > > project that has a completely different purpose than the previous one.
> > > 
> > > The best course of action would have been to ask Greg to drop the 
previous
> > > patches and then reset the numbering of the new job to v1. Unfortunately 
I
> > > did not pay attention to how she then managed the numbering following my
> > > observation.
> > > 
> > > What would be the best course of action at this point?
> > 
> > My guess is that this patch gets ignored because it has a lower version
> > number than a previous patch.
> > 
> > Take the feedback given here, and rev to
> > [PATCH v5] staging: axis-fifo: initialize timeouts in probe only
> > 
> > Be sure the Changelog, below the --- explains the journey.
> > 
> > Changes in v5:
> > 
> > Changes in v4:
> > 
> > Changes in v3:
> 
> > Changes in v2:
> Hey Alison!

Hi Khadija,

Please put one or two blank lines between the last message you are replying 
and the new you are writing (exactly as I'm doing here between "Hey Alison!" 
and "Hi Khadija").

> Based on Nathan's feedback I am trying to recompile and send a patch
> without any warnings.

Great!

> As suggested by Fabio, I am running "make w=1 -jX" command to see if I
> get any warnings.

I suppose that "w=1" is a typo. The option is enabled with "W=1" (capital 
case, Linux and all UNIX-like are case-sensitive). 

> But it is taking a lot of time, is there any way of
> speeding it up?

What is you choice for 'X' in "-jX"?
Did you try with the exact number of logical cores?
Are you building into a VM with enough logical cores? 
If you are building into a VM, did you reserve enough RAM?

Please read carefully my questions above and try to understand your 
environment and reply, so that I can help you more effectively.

> If this doesn't work then I have to follow the steps to reproduce in lkp
> mail as you said before.

The steps to reproduce will take your precious time and use more resources. 
Again, try to respond my questions.

> After dealing with these warnings I will send a [PATCH v5], following
> your instructions above.

Sorry for inadvertently overlooking to warn you about to send a message to 
Greg and ask him to drop your first 3 + 1 patches. Now you are doing good by 
following what Alison suggested: send v5 and write the log of revisions under 
the three dashes (exactly how Alison explained).

> Kindly, let me know if I am on the wrong track.
> Thank you!
> 

I think you are in the right track.
Let's try to speed up your builds because you'll need to build again your 
kernel many, many times for future works.  

Thanks,

Fabio

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

[Khadija Kamran]

On Wed, Mar 15, 2023 at 02:13:51PM +0100, Fabio M. De Francesco wrote:
> On mercoledì 15 marzo 2023 13:32:55 CET Khadija Kamran wrote:
> > On Tue, Mar 14, 2023 at 04:57:47PM -0700, Alison Schofield wrote:
> > > My guess is that this patch gets ignored because it has a lower version
> > > number than a previous patch.
> > > 
> > > Take the feedback given here, and rev to
> > > [PATCH v5] staging: axis-fifo: initialize timeouts in probe only
> > > 
> > > Be sure the Changelog, below the --- explains the journey.
> > > 
> > > Changes in v5:
> > > 
> > > Changes in v4:
> > > 
> > > Changes in v3:
> > 
> > > Changes in v2:
> > Hey Alison!
> 
> Hi Khadija,
> 
> Please put one or two blank lines between the last message you are replying 
> and the new you are writing (exactly as I'm doing here between "Hey Alison!" 
> and "Hi Khadija").
>

Hey Fabio!

Sorry about that. This was pointed by Alison before and I have been
adding spaces since then. Hopefully I am doing it right this time.

> > Based on Nathan's feedback I am trying to recompile and send a patch
> > without any warnings.
> 
> Great!
> 
> > As suggested by Fabio, I am running "make w=1 -jX" command to see if I
> > get any warnings.
> 
> I suppose that "w=1" is a typo. The option is enabled with "W=1" (capital 
> case, Linux and all UNIX-like are case-sensitive). 
>

Okay. I should re-run it with "W=1".

> > But it is taking a lot of time, is there any way of
> > speeding it up?
> 
> What is you choice for 'X' in "-jX"?

I used "-j4".

> Did you try with the exact number of logical cores?
> Are you building into a VM with enough logical cores? 
> If you are building into a VM, did you reserve enough RAM?

I am using Ubuntu 22.04.01 with the help of VM on VMware.
My machine has 13GB RAM and 2 processors(4 cores each).


> Please read carefully my questions above and try to understand your 
> environment and reply, so that I can help you more effectively.
> 
> > If this doesn't work then I have to follow the steps to reproduce in lkp
> > mail as you said before.
> 
> The steps to reproduce will take your precious time and use more resources. 
> Again, try to respond my questions.
> 
> > After dealing with these warnings I will send a [PATCH v5], following
> > your instructions above.
> 
> Sorry for inadvertently overlooking to warn you about to send a message to 
> Greg and ask him to drop your first 3 + 1 patches. Now you are doing good by 
> following what Alison suggested: send v5 and write the log of revisions under 
> the three dashes (exactly how Alison explained).
> 
> > Kindly, let me know if I am on the wrong track.
> > Thank you!
> > 
> 
> I think you are in the right track.
> Let's try to speed up your builds because you'll need to build again your 
> kernel many, many times for future works.  
>

Okay great! Thank you.

Regards,
Khadija

> Thanks,
> 
> Fabio
> 
> 

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

[Fabio M. De Francesco]

On mercoledì 15 marzo 2023 14:56:27 CET Khadija Kamran wrote:
> On Wed, Mar 15, 2023 at 02:13:51PM +0100, Fabio M. De Francesco wrote:
> > On mercoledì 15 marzo 2023 13:32:55 CET Khadija Kamran wrote:
> > > On Tue, Mar 14, 2023 at 04:57:47PM -0700, Alison Schofield wrote:
> > > > My guess is that this patch gets ignored because it has a lower 
version
> > > > number than a previous patch.
> > > > 
> > > > Take the feedback given here, and rev to
> > > > [PATCH v5] staging: axis-fifo: initialize timeouts in probe only
> > > > 
> > > > Be sure the Changelog, below the --- explains the journey.
> > > > 
> > > > Changes in v5:
> > > > 
> > > > Changes in v4:
> > > > 
> > > > Changes in v3:
> > > 
> > > > Changes in v2:
> > > Hey Alison!
> > 
> > Hi Khadija,
> > 
> > Please put one or two blank lines between the last message you are 
replying
> > and the new you are writing (exactly as I'm doing here between "Hey 
Alison!"
> > and "Hi Khadija").
> 
> Hey Fabio!
> 
> Sorry about that. This was pointed by Alison before and I have been
> adding spaces since then. Hopefully I am doing it right this time.
> 
You are doing right this time :-)
>
> > > Based on Nathan's feedback I am trying to recompile and send a patch
> > > without any warnings.
> > 
> > Great!
> > 
> > > As suggested by Fabio, I am running "make w=1 -jX" command to see if I
> > > get any warnings.
> > 
> > I suppose that "w=1" is a typo. The option is enabled with "W=1" (capital
> > case, Linux and all UNIX-like are case-sensitive).
> 
> Okay. I should re-run it with "W=1".
> 
> > > But it is taking a lot of time, is there any way of
> > > speeding it up?
> > 
> > What is you choice for 'X' in "-jX"?
> 
> I used "-j4".
> 
> > Did you try with the exact number of logical cores?
> > Are you building into a VM with enough logical cores?
> > If you are building into a VM, did you reserve enough RAM?
> 
> I am using Ubuntu 22.04.01 with the help of VM on VMware.
> My machine has 13GB RAM and 2 processors(4 cores each).

Therefore, you are using a Linux guest on a Linux host. This is a wise choice. 
However, you didn't say where you are running your builds...

I mean, the better things to do are the following steps:

1) Your workspace with the staging tree should stay in the host.
2) Shut down your guest in order to have all RAM and all logical processors 
available for the build.
3) Run "make -j8" in the host. Since you shutdown your guest VM you can use 
all 8 logical cores and the maximum available RAM (without the VM draining 
resources while building)
4) When the build is done, switch on your VM on VMware with at least 4 logical 
cores and 6GB of reserved RAM.
5) Mount your cloned base directory as a shared folder between host and guest.
6) In the guest, 'cd' to the shared folder and then run "make modules_install 
install" (in the guest, attention). This will install and configure the 
kernel, the modules, GRUB2 and everything else in your guest VM.
7) Reboot the VM and test your patches.

This procedure will speed up your next builds.
The fundamental point is that you don't need to partition precious resources 
while building, Do everything without running the VM and switch it on only for 
install and tests. Since you only build in the host but never install and boot 
in it, you don't risk any system's damage.

This is what I do for Kernel development purposes.

I hope it helps to answer your question about how to run fast recompilation.

Fabio  


> 
> > Please read carefully my questions above and try to understand your
> > environment and reply, so that I can help you more effectively.
> > 
> > > If this doesn't work then I have to follow the steps to reproduce in lkp
> > > mail as you said before.
> > 
> > The steps to reproduce will take your precious time and use more 
resources.
> > Again, try to respond my questions.
> > 
> > > After dealing with these warnings I will send a [PATCH v5], following
> > > your instructions above.
> > 
> > Sorry for inadvertently overlooking to warn you about to send a message to
> > Greg and ask him to drop your first 3 + 1 patches. Now you are doing good 
by
> > following what Alison suggested: send v5 and write the log of revisions
> > under
> > the three dashes (exactly how Alison explained).
> > 
> > > Kindly, let me know if I am on the wrong track.
> > > Thank you!
> > 
> > I think you are in the right track.
> > Let's try to speed up your builds because you'll need to build again your
> > kernel many, many times for future works.
> 
> Okay great! Thank you.
> 
> Regards,
> Khadija
> 
> > Thanks,
> > 
> > Fabio

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

[Khadija Kamran]

On Wed, Mar 15, 2023 at 05:44:47PM +0100, Fabio M. De Francesco wrote:
> On mercoledì 15 marzo 2023 14:56:27 CET Khadija Kamran wrote:
> > On Wed, Mar 15, 2023 at 02:13:51PM +0100, Fabio M. De Francesco wrote:
> > > On mercoledì 15 marzo 2023 13:32:55 CET Khadija Kamran wrote:
> > > > On Tue, Mar 14, 2023 at 04:57:47PM -0700, Alison Schofield wrote:
> > > > > My guess is that this patch gets ignored because it has a lower 
> version
> > > > > number than a previous patch.
> > > > > 
> > > > > Take the feedback given here, and rev to
> > > > > [PATCH v5] staging: axis-fifo: initialize timeouts in probe only
> > > > > 
> > > > > Be sure the Changelog, below the --- explains the journey.
> > > > > 
> > > > > Changes in v5:
> > > > > 
> > > > > Changes in v4:
> > > > > 
> > > > > Changes in v3:
> > > > 
> > > > > Changes in v2:
> > > > Hey Alison!
> > > 
> > > Hi Khadija,
> > > 
> > > Please put one or two blank lines between the last message you are 
> replying
> > > and the new you are writing (exactly as I'm doing here between "Hey 
> Alison!"
> > > and "Hi Khadija").
> > 
> > Hey Fabio!
> > 
> > Sorry about that. This was pointed by Alison before and I have been
> > adding spaces since then. Hopefully I am doing it right this time.
> > 
> You are doing right this time :-)
> >
> > > > Based on Nathan's feedback I am trying to recompile and send a patch
> > > > without any warnings.
> > > 
> > > Great!
> > > 
> > > > As suggested by Fabio, I am running "make w=1 -jX" command to see if I
> > > > get any warnings.
> > > 
> > > I suppose that "w=1" is a typo. The option is enabled with "W=1" (capital
> > > case, Linux and all UNIX-like are case-sensitive).
> > 
> > Okay. I should re-run it with "W=1".
> > 
> > > > But it is taking a lot of time, is there any way of
> > > > speeding it up?
> > > 
> > > What is you choice for 'X' in "-jX"?
> > 
> > I used "-j4".
> > 
> > > Did you try with the exact number of logical cores?
> > > Are you building into a VM with enough logical cores?
> > > If you are building into a VM, did you reserve enough RAM?
> > 
> > I am using Ubuntu 22.04.01 with the help of VM on VMware.
> > My machine has 13GB RAM and 2 processors(4 cores each).
> 
> Therefore, you are using a Linux guest on a Linux host. This is a wise choice. 
> However, you didn't say where you are running your builds...

Hey Fabio!

I am not using a Linux guest on Linux host. Sorry if I did not explain
it right. I am using Windows 10 and in order to run Ubuntu, I have
created a VM(on VMWare). This VM has 13GB RAM and 2 processors(4 cores
each).
Thank you!

Regards,
Khadija
> 
> I mean, the better things to do are the following steps:
> 
> 1) Your workspace with the staging tree should stay in the host.
> 2) Shut down your guest in order to have all RAM and all logical processors 
> available for the build.
> 3) Run "make -j8" in the host. Since you shutdown your guest VM you can use 
> all 8 logical cores and the maximum available RAM (without the VM draining 
> resources while building)
> 4) When the build is done, switch on your VM on VMware with at least 4 logical 
> cores and 6GB of reserved RAM.
> 5) Mount your cloned base directory as a shared folder between host and guest.
> 6) In the guest, 'cd' to the shared folder and then run "make modules_install 
> install" (in the guest, attention). This will install and configure the 
> kernel, the modules, GRUB2 and everything else in your guest VM.
> 7) Reboot the VM and test your patches.
> 
> This procedure will speed up your next builds.
> The fundamental point is that you don't need to partition precious resources 
> while building, Do everything without running the VM and switch it on only for 
> install and tests. Since you only build in the host but never install and boot 
> in it, you don't risk any system's damage.
> 
> This is what I do for Kernel development purposes.
> 
> I hope it helps to answer your question about how to run fast recompilation.
> 
> Fabio  
> 
> 
> > 
> > > Please read carefully my questions above and try to understand your
> > > environment and reply, so that I can help you more effectively.
> > > 
> > > > If this doesn't work then I have to follow the steps to reproduce in lkp
> > > > mail as you said before.
> > > 
> > > The steps to reproduce will take your precious time and use more 
> resources.
> > > Again, try to respond my questions.
> > > 
> > > > After dealing with these warnings I will send a [PATCH v5], following
> > > > your instructions above.
> > > 
> > > Sorry for inadvertently overlooking to warn you about to send a message to
> > > Greg and ask him to drop your first 3 + 1 patches. Now you are doing good 
> by
> > > following what Alison suggested: send v5 and write the log of revisions
> > > under
> > > the three dashes (exactly how Alison explained).
> > > 
> > > > Kindly, let me know if I am on the wrong track.
> > > > Thank you!
> > > 
> > > I think you are in the right track.
> > > Let's try to speed up your builds because you'll need to build again your
> > > kernel many, many times for future works.
> > 
> > Okay great! Thank you.
> > 
> > Regards,
> > Khadija
> > 
> > > Thanks,
> > > 
> > > Fabio
> 
> 
> 
> 

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

[Fabio M. De Francesco]

On giovedì 16 marzo 2023 10:50:50 CET Khadija Kamran wrote:
> On Wed, Mar 15, 2023 at 05:44:47PM +0100, Fabio M. De Francesco wrote:
> > On mercoledì 15 marzo 2023 14:56:27 CET Khadija Kamran wrote:
> > > On Wed, Mar 15, 2023 at 02:13:51PM +0100, Fabio M. De Francesco wrote:
> > > > On mercoledì 15 marzo 2023 13:32:55 CET Khadija Kamran wrote:

[snip]

> > > > I suppose that "w=1" is a typo. The option is enabled with "W=1"
> > > > (capital
> > > > case, Linux and all UNIX-like are case-sensitive).
> > > 
> > > Okay. I should re-run it with "W=1".
> > > 
> > > > > But it is taking a lot of time, is there any way of
> > > > > speeding it up?
> > > > 
> > > > What is you choice for 'X' in "-jX"?
> > > 
> > > I used "-j4".
> > > 
> > > > Did you try with the exact number of logical cores?
> > > > Are you building into a VM with enough logical cores?
> > > > If you are building into a VM, did you reserve enough RAM?
> > > 
> > > I am using Ubuntu 22.04.01 with the help of VM on VMware.
> > > My machine has 13GB RAM and 2 processors(4 cores each).
> > 
> > Therefore, you are using a Linux guest on a Linux host. This is a wise
> > choice. However, you didn't say where you are running your builds...
> 
> Hey Fabio!
> 
> I am not using a Linux guest on Linux host. Sorry if I did not explain
> it right. I am using Windows 10 and in order to run Ubuntu, I have
> created a VM(on VMWare). This VM has 13GB RAM and 2 processors(4 cores
> each).
> Thank you!
> 
> Regards,
> Khadija

Khadija,

I'm not yet sure whether or not you are doing well with reserving 13GB to a 
VM. First of all you should better use multiples of 2GB. Furthermore it looks 
too much memory unless you have a total "real" RAM in your host exceeding 
18GB. You must be sure you are leaving enough room for the Windows host to run 
smoothly and avoid too much swapping to/from disk.

At the same time I doubt that you can reserve 8 logical cores (2x4) for 
running the VM on VMWare.

However, we have time to investigate and adjust your configuration (if it 
really needs to be fine tuned). I'll try to reach you on IRC, open a private 
window, and assist with this task ASAP.

At the moment I'd prefer to see you focusing on getting your first patch 
accepted by Greg.

Thanks,

Fabio 
 
> > I mean, the better things to do are the following steps:
> > 
> > 1) Your workspace with the staging tree should stay in the host.
> > 2) Shut down your guest in order to have all RAM and all logical 
processors
> > available for the build.
> > 3) Run "make -j8" in the host. Since you shutdown your guest VM you can 
use
> > all 8 logical cores and the maximum available RAM (without the VM draining
> > resources while building)
> > 4) When the build is done, switch on your VM on VMware with at least 4
> > logical cores and 6GB of reserved RAM.
> > 5) Mount your cloned base directory as a shared folder between host and
> > guest. 6) In the guest, 'cd' to the shared folder and then run "make
> > modules_install install" (in the guest, attention). This will install and
> > configure the kernel, the modules, GRUB2 and everything else in your guest
> > VM.
> > 7) Reboot the VM and test your patches.
> > 
> > This procedure will speed up your next builds.
> > The fundamental point is that you don't need to partition precious 
resources
> > while building, Do everything without running the VM and switch it on only
> > for install and tests. Since you only build in the host but never install
> > and boot in it, you don't risk any system's damage.
> > 
> > This is what I do for Kernel development purposes.
> > 
> > I hope it helps to answer your question about how to run fast 
recompilation.
> > 
> > Fabio
> > 

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

[Khadija Kamran]

On Thu, Mar 16, 2023 at 12:13:23PM +0100, Fabio M. De Francesco wrote:
> Khadija,
> 
> I'm not yet sure whether or not you are doing well with reserving 13GB to a 
> VM. First of all you should better use multiples of 2GB. Furthermore it looks 
> too much memory unless you have a total "real" RAM in your host exceeding 
> 18GB. You must be sure you are leaving enough room for the Windows host to run 
> smoothly and avoid too much swapping to/from disk.
>

Hey Fabio,

I have 16GB RAM. I just chose the highest option that VMware
recommended. My Windows is running smoothly. 

> At the same time I doubt that you can reserve 8 logical cores (2x4) for 
> running the VM on VMWare.
> 
> However, we have time to investigate and adjust your configuration (if it 
> really needs to be fine tuned). I'll try to reach you on IRC, open a private 
> window, and assist with this task ASAP.
> 
> At the moment I'd prefer to see you focusing on getting your first patch 
> accepted by Greg.
>

I am working on the new [PATCH v5] as I was able to remove the warnings.
However, If you still think that we should work on the configurations
then we can start working on it. :)

Thank you for all the help.

Regards,
Khadija



> Thanks,
> 
> Fabio 
>  
> 
> 

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

[Fabio M. De Francesco]

On mercoledì 15 marzo 2023 13:32:55 CET Khadija Kamran wrote:
> On Tue, Mar 14, 2023 at 04:57:47PM -0700, Alison Schofield wrote:
> > On Tue, Mar 14, 2023 at 10:31:21PM +0100, Fabio wrote:
> > > On martedì 14 marzo 2023 21:43:40 CET Alison Schofield wrote:
> > > > On Mon, Mar 13, 2023 at 11:07:10PM +0500, Khadija Kamran wrote:
> > > > > Module parameter, read_timeout, can only be set at the loading time.
> > > > > As
> > > > > it can only be modified once, initialize read_timeout once in the
> > > > > probe
> > > > > function.
> > > > > As a result, only use read_timeout as the last argument in
> > > > > wait_event_interruptible_timeout() call.
> > > > > 
> > > > > Same goes for write_timeout.
> > > > > 
> > > > > Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > > Signed-off-by: Khadija Kamran <kamrankhadijadj@gmail.com>
> > > > > ---
> > > > 
> > > > Looks like this is [PATCH v5] and needs a changelog.
> > > 
> > > Alison,
> > > 
> > > In fact, this is only the second patch that addresses Greg's suggested
> > > refactoring.
> > > 
> > > Khadija moved from v3 of "staging: axis-fifo: remove tabs to align
> > > arguments"
> > > to v4 of this completely independent patch. And then back to v1, because
> > > (at
> > > the time of v4) I pointed out to her that she had started working on a
> > > project that has a completely different purpose than the previous one.
> > > 
> > > The best course of action would have been to ask Greg to drop the 
previous
> > > patches and then reset the numbering of the new job to v1. Unfortunately 
I
> > > did not pay attention to how she then managed the numbering following my
> > > observation.
> > > 
> > > What would be the best course of action at this point?
> > 
> > My guess is that this patch gets ignored because it has a lower version
> > number than a previous patch.
> > 
> > Take the feedback given here, and rev to
> > [PATCH v5] staging: axis-fifo: initialize timeouts in probe only
> > 
> > Be sure the Changelog, below the --- explains the journey.
> > 
> > Changes in v5:
> > 
> > Changes in v4:
> > 
> > Changes in v3:
> 
> > Changes in v2:
> Hey Alison!
> Based on Nathan's feedback I am trying to recompile and send a patch
> without any warnings.
> As suggested by Fabio, I am running "make w=1 -jX" command to see if I
> get any warnings. But it is taking a lot of time, is there any way of
> speeding it up?
> If this doesn't work then I have to follow the steps to reproduce in lkp
> mail as you said before.
> After dealing with these warnings I will send a [PATCH v5], following
> your instructions above.
> Kindly, let me know if I am on the wrong track.
> Thank you!
> 
> Regards,
> Khadija
> 
> > > Fabio

Aside from what I said and asked for with the other message of this same 
thread, please take note that you can build a specific module if you prefer 
not to re-build the whole kernel and other modules at the same time. 

I'm pretty sure that the instructions to do so are in the OutreachyFirstPatch 
tutorial.

If they are not there, please let us know.

Fabio

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

[Khadija Kamran]

On Wed, Mar 15, 2023 at 02:34:31PM +0100, Fabio M. De Francesco wrote:
> Aside from what I said and asked for with the other message of this same 
> thread, please take note that you can build a specific module if you prefer 
> not to re-build the whole kernel and other modules at the same time. 
> 
> I'm pretty sure that the instructions to do so are in the OutreachyFirstPatch 
> tutorial.
> 
> If they are not there, please let us know.
> 
> Fabio

Hey Fabio!

In the Outreachy FirstPatchTutorial under the 'Compiling only part of
the kernel' section there are ways to compile only some part of the
kernel.

I have tried using "make W=1 drivers/staging/axis-fifo/" and it says
'nothing to be done for'. 

Should I start with the steps to reproduce? :'(

Thank you!

Regards,
Khadija


> 
> 
> 

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

[Nathan Chancellor]

Hi Khadija,

On Wed, Mar 15, 2023 at 07:22:39PM +0500, Khadija Kamran wrote:
> On Wed, Mar 15, 2023 at 02:34:31PM +0100, Fabio M. De Francesco wrote:
> > Aside from what I said and asked for with the other message of this same 
> > thread, please take note that you can build a specific module if you prefer 
> > not to re-build the whole kernel and other modules at the same time. 
> > 
> > I'm pretty sure that the instructions to do so are in the OutreachyFirstPatch 
> > tutorial.
> > 
> > If they are not there, please let us know.
> > 
> > Fabio
> 
> Hey Fabio!
> 
> In the Outreachy FirstPatchTutorial under the 'Compiling only part of
> the kernel' section there are ways to compile only some part of the
> kernel.
> 
> I have tried using "make W=1 drivers/staging/axis-fifo/" and it says
> 'nothing to be done for'. 

Is CONFIG_XIL_AXIS_FIFO enabled in your configuration?

> Should I start with the steps to reproduce? :'(

I did see a report of this same warning occurring with GCC but the
report I commented on cane from clang/LLVM; using that toolchain may
make it easier for you to reproduce this issue. The robot's reproduction
instructions are fine but I think doing it manually is actually simpler.

Ubuntu 22.04 should have a pretty modern version of clang/LLVM, which
you can install via 'sudo apt install clang lld llvm'.
https://apt.llvm.org is another resource.

First, we will grab the configuration that was provided in the report:

$ wget -O .config https://download.01.org/0day-ci/archive/20230314/202303141159.6wN9HNP9-lkp@intel.com/config

Next, we want to make sure the configuration is synced, since we are
technically changing compilers:

$ make -j"$(nproc)" ARCH=arm64 LLVM=1 olddefconfig

Finally, you should be able to build that object file and see the
warning:

$ make -j"$(nproc)" ARCH=arm64 LLVM=1 drivers/staging/axis-fifo/
...
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.

Just repeat the last step as you investigate. If you have any further
issues or questions, please let me know. For the record, I am not
associated with Outreachy (I am one of the maintainers of clang/LLVM
support in the kernel), so if I have messed something up or overstepped
some boundary, I do apologize.

Cheers,
Nathan

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

[Fabio M. De Francesco]

On mercoledì 15 marzo 2023 16:06:56 CET Nathan Chancellor wrote:
> Hi Khadija,
> 
> On Wed, Mar 15, 2023 at 07:22:39PM +0500, Khadija Kamran wrote:
> > On Wed, Mar 15, 2023 at 02:34:31PM +0100, Fabio M. De Francesco wrote:
> > > Aside from what I said and asked for with the other message of this same
> > > thread, please take note that you can build a specific module if you
> > > prefer
> > > not to re-build the whole kernel and other modules at the same time.
> > > 
> > > I'm pretty sure that the instructions to do so are in the
> > > OutreachyFirstPatch
> > > tutorial.
> > > 
> > > If they are not there, please let us know.
> > > 
> > > Fabio
> > 
> > Hey Fabio!
> > 
> > In the Outreachy FirstPatchTutorial under the 'Compiling only part of
> > the kernel' section there are ways to compile only some part of the
> > kernel.
> > 
> > I have tried using "make W=1 drivers/staging/axis-fifo/" and it says
> > 'nothing to be done for'.
> 
> Is CONFIG_XIL_AXIS_FIFO enabled in your configuration?

@Khadija:

Maybe that you won't find CONFIG_XIL_AXIS_FIFO among the staging drivers 
entries while using "make menuconfig" (or the other similar commands for 
working with the configuration). 

In that case, first enable the CONFIG_OF option if disabled. 
CONFIG_XIL_AXIS_FIFO depends on the former. 

For "CONFIG_XIL_AXIS_FIFO depends on CONFIG_OF" we mean that you can the 
former if and only if the latter is already enabled. 

Actually, CONFIG_XIL_AXIS_FIFO depends also on two other options. However you 
shouldn't worry about those others because they are commonly enabled in more 
than 99% of PC distributions.

> > Should I start with the steps to reproduce? :'(
> 
> I did see a report of this same warning occurring with GCC but the
> report I commented on cane from clang/LLVM; using that toolchain may
> make it easier for you to reproduce this issue. The robot's reproduction
> instructions are fine but I think doing it manually is actually simpler.
> 
> Ubuntu 22.04 should have a pretty modern version of clang/LLVM, which
> you can install via 'sudo apt install clang lld llvm'.
> https://apt.llvm.org is another resource.

@Nathan: Congratulations, you now have another clang/LLVM adept!
(Obviously, I'm just kidding :-))

Fabio

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

[Khadija Kamran]

On Wed, Mar 15, 2023 at 05:24:23PM +0100, Fabio M. De Francesco wrote:
> On mercoledì 15 marzo 2023 16:06:56 CET Nathan Chancellor wrote:
> > Hi Khadija,
> > 
> > On Wed, Mar 15, 2023 at 07:22:39PM +0500, Khadija Kamran wrote:
> > > On Wed, Mar 15, 2023 at 02:34:31PM +0100, Fabio M. De Francesco wrote:
> > > > Aside from what I said and asked for with the other message of this same
> > > > thread, please take note that you can build a specific module if you
> > > > prefer
> > > > not to re-build the whole kernel and other modules at the same time.
> > > > 
> > > > I'm pretty sure that the instructions to do so are in the
> > > > OutreachyFirstPatch
> > > > tutorial.
> > > > 
> > > > If they are not there, please let us know.
> > > > 
> > > > Fabio
> > > 
> > > Hey Fabio!
> > > 
> > > In the Outreachy FirstPatchTutorial under the 'Compiling only part of
> > > the kernel' section there are ways to compile only some part of the
> > > kernel.
> > > 
> > > I have tried using "make W=1 drivers/staging/axis-fifo/" and it says
> > > 'nothing to be done for'.
> > 
> > Is CONFIG_XIL_AXIS_FIFO enabled in your configuration?
> 
> @Khadija:
> 
> Maybe that you won't find CONFIG_XIL_AXIS_FIFO among the staging drivers 
> entries while using "make menuconfig" (or the other similar commands for 
> working with the configuration). 
> 
> In that case, first enable the CONFIG_OF option if disabled. 
> CONFIG_XIL_AXIS_FIFO depends on the former. 
> 
> For "CONFIG_XIL_AXIS_FIFO depends on CONFIG_OF" we mean that you can the 
> former if and only if the latter is already enabled. 
> 
> Actually, CONFIG_XIL_AXIS_FIFO depends also on two other options. However you 
> shouldn't worry about those others because they are commonly enabled in more 
> than 99% of PC distributions.
>

Hey Fabio!

I have checked in my .config file and both options are enabled. The file
says,
CONFIG_OF=y
CONFIG_HAS_IOMEM=y

The "make menuconfig" shows 'Xilinx AXI-Stream FIFO IP core driver' with
built-in selection [*].
The problem I am having here is that in the OutreachyFIrstPatch
tutorial, it asks you to change [*] -> [m], but I cannot change it.

Kindly help me with this!

Thank you!
Regards,
Khadija



> > > Should I start with the steps to reproduce? :'(
> > 
> > I did see a report of this same warning occurring with GCC but the
> > report I commented on cane from clang/LLVM; using that toolchain may
> > make it easier for you to reproduce this issue. The robot's reproduction
> > instructions are fine but I think doing it manually is actually simpler.
> > 
> > Ubuntu 22.04 should have a pretty modern version of clang/LLVM, which
> > you can install via 'sudo apt install clang lld llvm'.
> > https://apt.llvm.org is another resource.
> 
> @Nathan: Congratulations, you now have another clang/LLVM adept!
> (Obviously, I'm just kidding :-))
> 
> Fabio
> 
> 
> 

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

[Fabio M. De Francesco]

On giovedì 16 marzo 2023 11:17:05 CET Khadija Kamran wrote:
> On Wed, Mar 15, 2023 at 05:24:23PM +0100, Fabio M. De Francesco wrote:
> > On mercoledì 15 marzo 2023 16:06:56 CET Nathan Chancellor wrote:
> > > Hi Khadija,
> > > 
> > > On Wed, Mar 15, 2023 at 07:22:39PM +0500, Khadija Kamran wrote:
> > > > On Wed, Mar 15, 2023 at 02:34:31PM +0100, Fabio M. De Francesco wrote:
> > > > > Aside from what I said and asked for with the other message of this
> > > > > same
> > > > > thread, please take note that you can build a specific module if you
> > > > > prefer
> > > > > not to re-build the whole kernel and other modules at the same time.
> > > > > 
> > > > > I'm pretty sure that the instructions to do so are in the
> > > > > OutreachyFirstPatch
> > > > > tutorial.
> > > > > 
> > > > > If they are not there, please let us know.
> > > > > 
> > > > > Fabio
> > > > 
> > > > Hey Fabio!
> > > > 
> > > > In the Outreachy FirstPatchTutorial under the 'Compiling only part of
> > > > the kernel' section there are ways to compile only some part of the
> > > > kernel.
> > > > 
> > > > I have tried using "make W=1 drivers/staging/axis-fifo/" and it says
> > > > 'nothing to be done for'.
> > > 
> > > Is CONFIG_XIL_AXIS_FIFO enabled in your configuration?
> > 
> > @Khadija:
> > 
> > Maybe that you won't find CONFIG_XIL_AXIS_FIFO among the staging drivers
> > entries while using "make menuconfig" (or the other similar commands for
> > working with the configuration).
> > 
> > In that case, first enable the CONFIG_OF option if disabled.
> > CONFIG_XIL_AXIS_FIFO depends on the former.
> > 
> > For "CONFIG_XIL_AXIS_FIFO depends on CONFIG_OF" we mean that you can the
> > former if and only if the latter is already enabled.
> > 
> > Actually, CONFIG_XIL_AXIS_FIFO depends also on two other options. However
> > you
> > shouldn't worry about those others because they are commonly enabled in 
more
> > than 99% of PC distributions.
> 
> Hey Fabio!
> 
> I have checked in my .config file and both options are enabled. The file
> says,
> CONFIG_OF=y
> CONFIG_HAS_IOMEM=y
> 
> The "make menuconfig" shows 'Xilinx AXI-Stream FIFO IP core driver' with
> built-in selection [*].
> The problem I am having here is that in the OutreachyFirstPatch
> tutorial, it asks you to change [*] -> [m], but I cannot change it.
> 
> Kindly help me with this!

Hi Khadija,

Sorry for being here so late.

I just saw also your other messages with further help requests.
Please do other work while waiting for help on a specific issue. 

If I understand you correctly, you are saying that you can only build axis-
fifo in-kernel because you cannot change the '*' to 'm'. Correct?

If so, you have probably touched other configuration options in a way that 
forces that driver to work only if compiled in-kernel). Therefore, I'd restart 
from scratch.

Please delete your current .config and make a copy of the one from your 
running kernel. Then check the three options Alison and I were talking about. 

Remember that you cannot see the axis-fifo module entry (CONFIG_XIL_AXIS_FIFO) 
in "Device Drivers" -> "Staging Drivers" section if you have not yet enabled  
(either with 'm' or 'y') CONFIG_OF and CONFIG_HAS_IOMEM. 

Now you should end up with a visible CONFIG_XIL_AXIS_FIFO that can be enabled 
as a module with 'm'. 

Let me know if this procedure works.

Fabio

> Thank you!
> Regards,
> Khadija
> 
> > > > Should I start with the steps to reproduce? :'(
> > > 
> > > I did see a report of this same warning occurring with GCC but the
> > > report I commented on cane from clang/LLVM; using that toolchain may
> > > make it easier for you to reproduce this issue. The robot's reproduction
> > > instructions are fine but I think doing it manually is actually simpler.
> > > 
> > > Ubuntu 22.04 should have a pretty modern version of clang/LLVM, which
> > > you can install via 'sudo apt install clang lld llvm'.
> > > https://apt.llvm.org is another resource.
> > 
> > @Nathan: Congratulations, you now have another clang/LLVM adept!
> > (Obviously, I'm just kidding :-))
> > 
> > Fabio

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

[Khadija Kamran]

On Thu, Mar 16, 2023 at 12:39:31PM +0100, Fabio M. De Francesco wrote:
> Hi Khadija,
> 
> Sorry for being here so late.
> 
> I just saw also your other messages with further help requests.
> Please do other work while waiting for help on a specific issue. 
> 
> If I understand you correctly, you are saying that you can only build axis-
> fifo in-kernel because you cannot change the '*' to 'm'. Correct?
>

Yes, corect.

> If so, you have probably touched other configuration options in a way that 
> forces that driver to work only if compiled in-kernel). Therefore, I'd restart 
> from scratch.
> 
> Please delete your current .config and make a copy of the one from your 
> running kernel. Then check the three options Alison and I were talking about. 
>
> Remember that you cannot see the axis-fifo module entry (CONFIG_XIL_AXIS_FIFO) 
> in "Device Drivers" -> "Staging Drivers" section if you have not yet enabled  
> (either with 'm' or 'y') CONFIG_OF and CONFIG_HAS_IOMEM. 
> 
> Now you should end up with a visible CONFIG_XIL_AXIS_FIFO that can be enabled 
> as a module with 'm'. 
>

Hey Fabio!

I am on it. I have undertood the steps. I will let you know if this
works.

Also, with the help of Nathan's feedback I was able to reproduce the
warnings. I have changed the files to remove the warnings and it works. 

I will submit another [PATCH v5].

Thank you!
Regards,
Khadija Kamran

> Let me know if this procedure works.
> 
> Fabio
> 
> 

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

[Julia Lawall]

> Just repeat the last step as you investigate. If you have any further
> issues or questions, please let me know. For the record, I am not
> associated with Outreachy (I am one of the maintainers of clang/LLVM
> support in the kernel), so if I have messed something up or overstepped
> some boundary, I do apologize.

Definitely not.  Thanks for helping out!

julia

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

[Khadija Kamran]

On Wed, Mar 15, 2023 at 08:06:56AM -0700, Nathan Chancellor wrote:
> Hi Khadija,
> 
> On Wed, Mar 15, 2023 at 07:22:39PM +0500, Khadija Kamran wrote:
> > On Wed, Mar 15, 2023 at 02:34:31PM +0100, Fabio M. De Francesco wrote:
> > > Aside from what I said and asked for with the other message of this same 
> > > thread, please take note that you can build a specific module if you prefer 
> > > not to re-build the whole kernel and other modules at the same time. 
> > > 
> > > I'm pretty sure that the instructions to do so are in the OutreachyFirstPatch 
> > > tutorial.
> > > 
> > > If they are not there, please let us know.
> > > 
> > > Fabio
> > 
> > Hey Fabio!
> > 
> > In the Outreachy FirstPatchTutorial under the 'Compiling only part of
> > the kernel' section there are ways to compile only some part of the
> > kernel.
> > 
> > I have tried using "make W=1 drivers/staging/axis-fifo/" and it says
> > 'nothing to be done for'. 
> 
> Is CONFIG_XIL_AXIS_FIFO enabled in your configuration?
>

Hey Nathan!
Thank you for all the help. 
'Xilinx AXI-Stream FIFO IP core driver' option in my menufig shows [*]
and I cannot change it to ['m'] following all the intrustcions in
OutreachyFIrstPatch step by step.

> > Should I start with the steps to reproduce? :'(
> 
> I did see a report of this same warning occurring with GCC but the
> report I commented on cane from clang/LLVM; using that toolchain may
> make it easier for you to reproduce this issue. The robot's reproduction
> instructions are fine but I think doing it manually is actually simpler.
> 
> Ubuntu 22.04 should have a pretty modern version of clang/LLVM, which
> you can install via 'sudo apt install clang lld llvm'.
> https://apt.llvm.org is another resource.
> 
> First, we will grab the configuration that was provided in the report:
> 
> $ wget -O .config https://download.01.org/0day-ci/archive/20230314/202303141159.6wN9HNP9-lkp@intel.com/config
> 
> Next, we want to make sure the configuration is synced, since we are
> technically changing compilers:
> 
> $ make -j"$(nproc)" ARCH=arm64 LLVM=1 olddefconfig
> 
> Finally, you should be able to build that object file and see the
> warning:
> 
> $ make -j"$(nproc)" ARCH=arm64 LLVM=1 drivers/staging/axis-fifo/
>

It did not show any warnings. A followed all your instructions step by
step.

Thanks again!

Regards,
Khadija

> 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.
> 
> Just repeat the last step as you investigate. If you have any further
> issues or questions, please let me know. For the record, I am not
> associated with Outreachy (I am one of the maintainers of clang/LLVM
> support in the kernel), so if I have messed something up or overstepped
> some boundary, I do apologize.
> 
> Cheers,
> Nathan

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

[Khadija Kamran]

On Wed, Mar 15, 2023 at 08:06:56AM -0700, Nathan Chancellor wrote:
> Hi Khadija,
> 
> On Wed, Mar 15, 2023 at 07:22:39PM +0500, Khadija Kamran wrote:
> > On Wed, Mar 15, 2023 at 02:34:31PM +0100, Fabio M. De Francesco wrote:
> > > Aside from what I said and asked for with the other message of this same 
> > > thread, please take note that you can build a specific module if you prefer 
> > > not to re-build the whole kernel and other modules at the same time. 
> > > 
> > > I'm pretty sure that the instructions to do so are in the OutreachyFirstPatch 
> > > tutorial.
> > > 
> > > If they are not there, please let us know.
> > > 
> > > Fabio
> > 
> > Hey Fabio!
> > 
> > In the Outreachy FirstPatchTutorial under the 'Compiling only part of
> > the kernel' section there are ways to compile only some part of the
> > kernel.
> > 
> > I have tried using "make W=1 drivers/staging/axis-fifo/" and it says
> > 'nothing to be done for'. 
> 
> Is CONFIG_XIL_AXIS_FIFO enabled in your configuration?
> 
> > Should I start with the steps to reproduce? :'(
> 
> I did see a report of this same warning occurring with GCC but the
> report I commented on cane from clang/LLVM; using that toolchain may
> make it easier for you to reproduce this issue. The robot's reproduction
> instructions are fine but I think doing it manually is actually simpler.
> 
> Ubuntu 22.04 should have a pretty modern version of clang/LLVM, which
> you can install via 'sudo apt install clang lld llvm'.
> https://apt.llvm.org is another resource.
> 
> First, we will grab the configuration that was provided in the report:
> 
> $ wget -O .config https://download.01.org/0day-ci/archive/20230314/202303141159.6wN9HNP9-lkp@intel.com/config
> 
> Next, we want to make sure the configuration is synced, since we are
> technically changing compilers:
> 
> $ make -j"$(nproc)" ARCH=arm64 LLVM=1 olddefconfig
> 
> Finally, you should be able to build that object file and see the
> warning:
> 
> $ make -j"$(nproc)" ARCH=arm64 LLVM=1 drivers/staging/axis-fifo/
> ...
> 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.
> 
> Just repeat the last step as you investigate. If you have any further
> issues or questions, please let me know. For the record, I am not
> associated with Outreachy (I am one of the maintainers of clang/LLVM
> support in the kernel), so if I have messed something up or overstepped
> some boundary, I do apologize.
> 
> Cheers,
> Nathan

Hi Nathan! 

Sorry about the last email where I said that this is not working. I was
working in the wrong branch, my bad. 

This works! And it has helped me remove the warnings too. I have
replaced int datatype with long and the two warnings that were showing
are no longer there. 

I am working on [PATCH v5] and I will submit it in no time.

This was great help. :)

Thank you!
Regards,
Khadija

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

[Alison Schofield]

On Wed, Mar 15, 2023 at 07:22:39PM +0500, Khadija Kamran wrote:
> On Wed, Mar 15, 2023 at 02:34:31PM +0100, Fabio M. De Francesco wrote:
> > Aside from what I said and asked for with the other message of this same 
> > thread, please take note that you can build a specific module if you prefer 
> > not to re-build the whole kernel and other modules at the same time. 
> > 
> > I'm pretty sure that the instructions to do so are in the OutreachyFirstPatch 
> > tutorial.
> > 
> > If they are not there, please let us know.
> > 
> > Fabio
> 
> Hey Fabio!
> 
> In the Outreachy FirstPatchTutorial under the 'Compiling only part of
> the kernel' section there are ways to compile only some part of the
> kernel.
> 
> I have tried using "make W=1 drivers/staging/axis-fifo/" and it says
> 'nothing to be done for'. 
> 
> Should I start with the steps to reproduce? :'(

Khadija,

I've applied your patch and it fails to compile with the warnings
that LKP reports.

If you are doing: $ make drivers/staging/axis-fifo/

and it is saying 'nothing to be done...'

You either have not changed anything since the last compile, or you
do not have the module configured.

I suspect it has never compiled for you and you need to look at
the direction in the tutorial for 'Recompiling the driver' section
and learn how to use make menuconfig.

This driver has a couple of dependencies described in the Kconfig
file. You will not even see the 'XIL_AXIS_FIFO' option until you
turn on 'OF' and HAS_'IOMEM'.

See the drivers/staging/axis-fifo/Kconfig

Please confirm that you have compiled the driver before chasing
after the build env that lkp reports.

Thanks,
Alison


> 
> Thank you!
> 
> Regards,
> Khadija
> 
> 
> > 
> > 
> > 

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

[Khadija Kamran]

On Wed, Mar 15, 2023 at 09:09:16AM -0700, Alison Schofield wrote:
> On Wed, Mar 15, 2023 at 07:22:39PM +0500, Khadija Kamran wrote:
> > On Wed, Mar 15, 2023 at 02:34:31PM +0100, Fabio M. De Francesco wrote:
> > > Aside from what I said and asked for with the other message of this same 
> > > thread, please take note that you can build a specific module if you prefer 
> > > not to re-build the whole kernel and other modules at the same time. 
> > > 
> > > I'm pretty sure that the instructions to do so are in the OutreachyFirstPatch 
> > > tutorial.
> > > 
> > > If they are not there, please let us know.
> > > 
> > > Fabio
> > 
> > Hey Fabio!
> > 
> > In the Outreachy FirstPatchTutorial under the 'Compiling only part of
> > the kernel' section there are ways to compile only some part of the
> > kernel.
> > 
> > I have tried using "make W=1 drivers/staging/axis-fifo/" and it says
> > 'nothing to be done for'. 
> > 
> > Should I start with the steps to reproduce? :'(
> 
> Khadija,
> 
> I've applied your patch and it fails to compile with the warnings
> that LKP reports.
> 
> If you are doing: $ make drivers/staging/axis-fifo/
> 
> and it is saying 'nothing to be done...'
> 
> You either have not changed anything since the last compile, or you
> do not have the module configured.


Hey Alison!
I might've written the statement wrong. Actually, here is the output of
make drivers/staging/axis-fifo

YACC    scripts/genksyms/parse.tab.[ch]
  HOSTCC  scripts/genksyms/parse.tab.o
  HOSTCC  scripts/genksyms/lex.lex.o
  HOSTLD  scripts/genksyms/genksyms
  CC      scripts/mod/empty.o
  MKELF   scripts/mod/elfconfig.h
  HOSTCC  scripts/mod/modpost.o
  CC      scripts/mod/devicetable-offsets.s
  HOSTCC  scripts/mod/file2alias.o
  HOSTCC  scripts/mod/sumversion.o
  HOSTLD  scripts/mod/modpost
  CC      kernel/bounds.s
  CC      arch/x86/kernel/asm-offsets.s
  CALL    scripts/checksyscalls.sh
  DESCEND objtool
  INSTALL libsubcmd_headers
  DESCEND bpf/resolve_btfids
  INSTALL libsubcmd_headers
make[3]: Nothing to be done for 'drivers/staging/axis-fifo/'.


> 
> I suspect it has never compiled for you and you need to look at
> the direction in the tutorial for 'Recompiling the driver' section
> and learn how to use make menuconfig.
>
> This driver has a couple of dependencies described in the Kconfig
> file. You will not even see the 'XIL_AXIS_FIFO' option until you
> turn on 'OF' and HAS_'IOMEM'.
> 
> See the drivers/staging/axis-fifo/Kconfig
> 
> Please confirm that you have compiled the driver before chasing
> after the build env that lkp reports.
>

Sorry, I made a mistake here.

I did not change the driver 'Xilinx AXI-Stream FIFO IP core driver' to
being compiled as a module by typing 'm'. 
Is that the problem here?

Also, now when I try to change it by pressing 'm', it is not working.
And I have tried many times but I cannot change the driver from '*' to
'm'.

Thank you!
Regards,
Khadija


> Thanks,
> Alison
> 
> 
> > 
> > Thank you!
> > 
> > Regards,
> > Khadija
> > 
> > 
> > > 
> > > 
> > > 

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

[Khadija Kamran]

On Wed, Mar 15, 2023 at 09:09:16AM -0700, Alison Schofield wrote:
> On Wed, Mar 15, 2023 at 07:22:39PM +0500, Khadija Kamran wrote:
> > On Wed, Mar 15, 2023 at 02:34:31PM +0100, Fabio M. De Francesco wrote:
> > > Aside from what I said and asked for with the other message of this same 
> > > thread, please take note that you can build a specific module if you prefer 
> > > not to re-build the whole kernel and other modules at the same time. 
> > > 
> > > I'm pretty sure that the instructions to do so are in the OutreachyFirstPatch 
> > > tutorial.
> > > 
> > > If they are not there, please let us know.
> > > 
> > > Fabio
> > 
> > Hey Fabio!
> > 
> > In the Outreachy FirstPatchTutorial under the 'Compiling only part of
> > the kernel' section there are ways to compile only some part of the
> > kernel.
> > 
> > I have tried using "make W=1 drivers/staging/axis-fifo/" and it says
> > 'nothing to be done for'. 
> > 
> > Should I start with the steps to reproduce? :'(
> 
> Khadija,
> 
> I've applied your patch and it fails to compile with the warnings
> that LKP reports.
> 
> If you are doing: $ make drivers/staging/axis-fifo/
> 
> and it is saying 'nothing to be done...'
> 
> You either have not changed anything since the last compile, or you
> do not have the module configured.
>

Hey Alison!
I might've written the statement wrong. Actually, here is the output of
make drivers/staging/axis-fifo

YACC    scripts/genksyms/parse.tab.[ch]
  HOSTCC  scripts/genksyms/parse.tab.o
  HOSTCC  scripts/genksyms/lex.lex.o
  HOSTLD  scripts/genksyms/genksyms
  CC      scripts/mod/empty.o
  MKELF   scripts/mod/elfconfig.h
  HOSTCC  scripts/mod/modpost.o
  CC      scripts/mod/devicetable-offsets.s
  HOSTCC  scripts/mod/file2alias.o
  HOSTCC  scripts/mod/sumversion.o
  HOSTLD  scripts/mod/modpost
  CC      kernel/bounds.s
  CC      arch/x86/kernel/asm-offsets.s
  CALL    scripts/checksyscalls.sh
  DESCEND objtool
  INSTALL libsubcmd_headers
  DESCEND bpf/resolve_btfids
  INSTALL libsubcmd_headers
make[3]: Nothing to be done for 'drivers/staging/axis-fifo/'.


> I suspect it has never compiled for you and you need to look at
> the direction in the tutorial for 'Recompiling the driver' section
> and learn how to use make menuconfig.
> 
> This driver has a couple of dependencies described in the Kconfig
> file. You will not even see the 'XIL_AXIS_FIFO' option until you
> turn on 'OF' and HAS_'IOMEM'.
> 
> See the drivers/staging/axis-fifo/Kconfig
> 
> Please confirm that you have compiled the driver before chasing
> after the build env that lkp reports.
> 
> Thanks,
> Alison

Sorry, I made a mistake here.

I did not change the driver 'Xilinx AXI-Stream FIFO IP core driver' to
being compiled as a module by typing 'm'.
Is that the problem here?

Also, now when I try to change it by pressing 'm', it is not working.
And I have tried many times but I cannot change the driver from '*' to
'm'.

Kindly help me with this.

Regards,
Khadija

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

[Julia Lawall]

On Thu, 16 Mar 2023, Khadija Kamran wrote:

> On Wed, Mar 15, 2023 at 09:09:16AM -0700, Alison Schofield wrote:
> > On Wed, Mar 15, 2023 at 07:22:39PM +0500, Khadija Kamran wrote:
> > > On Wed, Mar 15, 2023 at 02:34:31PM +0100, Fabio M. De Francesco wrote:
> > > > Aside from what I said and asked for with the other message of this same
> > > > thread, please take note that you can build a specific module if you prefer
> > > > not to re-build the whole kernel and other modules at the same time.
> > > >
> > > > I'm pretty sure that the instructions to do so are in the OutreachyFirstPatch
> > > > tutorial.
> > > >
> > > > If they are not there, please let us know.
> > > >
> > > > Fabio
> > >
> > > Hey Fabio!
> > >
> > > In the Outreachy FirstPatchTutorial under the 'Compiling only part of
> > > the kernel' section there are ways to compile only some part of the
> > > kernel.
> > >
> > > I have tried using "make W=1 drivers/staging/axis-fifo/" and it says
> > > 'nothing to be done for'.
> > >
> > > Should I start with the steps to reproduce? :'(
> >
> > Khadija,
> >
> > I've applied your patch and it fails to compile with the warnings
> > that LKP reports.
> >
> > If you are doing: $ make drivers/staging/axis-fifo/
> >
> > and it is saying 'nothing to be done...'
> >
> > You either have not changed anything since the last compile, or you
> > do not have the module configured.
> >
>
> Hey Alison!
> I might've written the statement wrong. Actually, here is the output of
> make drivers/staging/axis-fifo
>
> YACC    scripts/genksyms/parse.tab.[ch]
>   HOSTCC  scripts/genksyms/parse.tab.o
>   HOSTCC  scripts/genksyms/lex.lex.o
>   HOSTLD  scripts/genksyms/genksyms
>   CC      scripts/mod/empty.o
>   MKELF   scripts/mod/elfconfig.h
>   HOSTCC  scripts/mod/modpost.o
>   CC      scripts/mod/devicetable-offsets.s
>   HOSTCC  scripts/mod/file2alias.o
>   HOSTCC  scripts/mod/sumversion.o
>   HOSTLD  scripts/mod/modpost
>   CC      kernel/bounds.s
>   CC      arch/x86/kernel/asm-offsets.s
>   CALL    scripts/checksyscalls.sh
>   DESCEND objtool
>   INSTALL libsubcmd_headers
>   DESCEND bpf/resolve_btfids
>   INSTALL libsubcmd_headers
> make[3]: Nothing to be done for 'drivers/staging/axis-fifo/'.

Did you try make allyesconfig and make drivers/staging/axis-fifo/axis-fifo.o

I get no warnings with W=1, but I do get a .o file.  I also tried

make drivers/staging/axis-fifo/axis-fifo.i

which just expands macros and simplifies ifdefs, and I see the various
code from the file.  So I don't have the impression that it is getting
ifdef'd away in some obscure way.  Perhaps I am missing something.  Or
perhaps the warning in question only comres from LLVM?

julia

>
>
> > I suspect it has never compiled for you and you need to look at
> > the direction in the tutorial for 'Recompiling the driver' section
> > and learn how to use make menuconfig.
> >
> > This driver has a couple of dependencies described in the Kconfig
> > file. You will not even see the 'XIL_AXIS_FIFO' option until you
> > turn on 'OF' and HAS_'IOMEM'.
> >
> > See the drivers/staging/axis-fifo/Kconfig
> >
> > Please confirm that you have compiled the driver before chasing
> > after the build env that lkp reports.
> >
> > Thanks,
> > Alison
>
> Sorry, I made a mistake here.
>
> I did not change the driver 'Xilinx AXI-Stream FIFO IP core driver' to
> being compiled as a module by typing 'm'.
> Is that the problem here?
>
> Also, now when I try to change it by pressing 'm', it is not working.
> And I have tried many times but I cannot change the driver from '*' to
> 'm'.
>
> Kindly help me with this.
>
> Regards,
> Khadija
>
>
>
>