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

[PATCH v5] 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.

Convert datatpe of read_timeout from 'int' to 'long int' because
implicit conversion of 'long int' to 'int' in statement 'read_timeout =
MAX_SCHEDULE_TIMEOUT' results in an overflow warning.

Perform same steps formodule parameter, write_timeout.

Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Khadija Kamran <kamrankhadijadj@gmail.com>
---

Changes in v5:
 - Convert timeout's datatype from int to long.
Changes in v4:
 - Initialize timeouts once as suggested by Greg; this automatically
   fixes the indentation problems.
 - Change the subject and description.
Changes in v3:
 - Fix grammatical mistakes
 - Do not change the second argument's indentation in split lines 
Changes in v2:
 - Instead of matching alignment to open parenthesis, align second and
   the last argument instead.
 - Change the subject to 'remove tabs to align arguments'.
 - Use imperative language in subject and description

 drivers/staging/axis-fifo/axis-fifo.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/axis-fifo/axis-fifo.c b/drivers/staging/axis-fifo/axis-fifo.c
index dfd2b357f484..d667dc80df47 100644
--- a/drivers/staging/axis-fifo/axis-fifo.c
+++ b/drivers/staging/axis-fifo/axis-fifo.c
@@ -103,17 +103,17 @@
  *           globals
  * ----------------------------
  */
-static int read_timeout = 1000; /* ms to wait before read() times out */
-static int write_timeout = 1000; /* ms to wait before write() times out */
+static long read_timeout = 1000; /* ms to wait before read() times out */
+static long write_timeout = 1000; /* ms to wait before write() times out */

 /* ----------------------------
  * module command-line arguments
  * ----------------------------
  */

-module_param(read_timeout, int, 0444);
+module_param(read_timeout, long, 0444);
 MODULE_PARM_DESC(read_timeout, "ms to wait before blocking read() timing out; set to -1 for no timeout");
-module_param(write_timeout, int, 0444);
+module_param(write_timeout, long, 0444);
 MODULE_PARM_DESC(write_timeout, "ms to wait before blocking write() timing out; set to -1 for no timeout");

 /* ----------------------------
@@ -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 v5] staging: axis-fifo: initialize timeouts in probe only

[Greg Kroah-Hartman]

On Thu, Mar 16, 2023 at 05:56:02PM +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.
> 
> Convert datatpe of read_timeout from 'int' to 'long int' because
> implicit conversion of 'long int' to 'int' in statement 'read_timeout =
> MAX_SCHEDULE_TIMEOUT' results in an overflow warning.
> 
> Perform same steps formodule parameter, write_timeout.
> 
> Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Khadija Kamran <kamrankhadijadj@gmail.com>
> ---

I do not think you built this version as I get the following build
errors when I apply it and build:

In file included from ./include/linux/kernel.h:29,
                 from drivers/staging/axis-fifo/axis-fifo.c:17:
drivers/staging/axis-fifo/axis-fifo.c: In function ‘axis_fifo_init’:
./include/linux/kern_levels.h:5:25: error: format ‘%i’ expects argument of type ‘int’, but argument 2 has type ‘long int’ [-Werror=format=]
    5 | #define KERN_SOH        "\001"          /* ASCII Start Of Header */
      |                         ^~~~~~
./include/linux/printk.h:427:25: note: in definition of macro ‘printk_index_wrap’
  427 |                 _p_func(_fmt, ##__VA_ARGS__);                           \
      |                         ^~~~
./include/linux/printk.h:528:9: note: in expansion of macro ‘printk’
  528 |         printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
      |         ^~~~~~
./include/linux/kern_levels.h:14:25: note: in expansion of macro ‘KERN_SOH’
   14 | #define KERN_INFO       KERN_SOH "6"    /* informational */
      |                         ^~~~~~~~
./include/linux/printk.h:528:16: note: in expansion of macro ‘KERN_INFO’
  528 |         printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
      |                ^~~~~~~~~
drivers/staging/axis-fifo/axis-fifo.c:957:9: note: in expansion of macro ‘pr_info’
  957 |         pr_info("axis-fifo driver loaded with parameters read_timeout = %i, write_timeout = %i\n",
      |         ^~~~~~~
./include/linux/kern_levels.h:5:25: error: format ‘%i’ expects argument of type ‘int’, but argument 3 has type ‘long int’ [-Werror=format=]
    5 | #define KERN_SOH        "\001"          /* ASCII Start Of Header */
      |                         ^~~~~~
./include/linux/printk.h:427:25: note: in definition of macro ‘printk_index_wrap’
  427 |                 _p_func(_fmt, ##__VA_ARGS__);                           \
      |                         ^~~~
./include/linux/printk.h:528:9: note: in expansion of macro ‘printk’
  528 |         printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
      |         ^~~~~~
./include/linux/kern_levels.h:14:25: note: in expansion of macro ‘KERN_SOH’
   14 | #define KERN_INFO       KERN_SOH "6"    /* informational */
      |                         ^~~~~~~~
./include/linux/printk.h:528:16: note: in expansion of macro ‘KERN_INFO’
  528 |         printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
      |                ^~~~~~~~~
drivers/staging/axis-fifo/axis-fifo.c:957:9: note: in expansion of macro ‘pr_info’
  957 |         pr_info("axis-fifo driver loaded with parameters read_timeout = %i, write_timeout = %i\n",
      |         ^~~~~~~
cc1: all warnings being treated as errors


Please always test-build your patches.

thanks,

greg k-h

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

[Greg Kroah-Hartman]

On Thu, Mar 16, 2023 at 05:56:02PM +0500, Khadija Kamran wrote:
> @@ -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;

Also, this change needs to go into the axis_fifo_init() function, not
the driver probe as now the print message in there is not correct.

thanks,

greg k-h

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

[Khadija Kamran]

Hey Greg,
Thank you for the feedback, I am working on it.
I have some compilation problems currently. I'll make sure to fix them
before sending the next patch.

Regards,
Khadija

Re: [PATCH v5] 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/20230316-205814
patch link:    https://lore.kernel.org/r/ZBMR4s8xyHGqMm72%40khadija-virtual-machine
patch subject: [PATCH v5] staging: axis-fifo: initialize timeouts in probe only
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230316/202303162137.a6Y0CjU8-lkp@intel.com/config)
compiler: m68k-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/7e359baa2318b55e12a0c099b75fbd0d799907cf
        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/20230316-205814
        git checkout 7e359baa2318b55e12a0c099b75fbd0d799907cf
        # 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=m68k olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k 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/202303162137.a6Y0CjU8-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from include/linux/kernel.h:29,
                    from drivers/staging/axis-fifo/axis-fifo.c:17:
   drivers/staging/axis-fifo/axis-fifo.c: In function 'axis_fifo_init':
>> include/linux/kern_levels.h:5:25: warning: format '%i' expects argument of type 'int', but argument 2 has type 'long int' [-Wformat=]
       5 | #define KERN_SOH        "\001"          /* ASCII Start Of Header */
         |                         ^~~~~~
   include/linux/printk.h:427:25: note: in definition of macro 'printk_index_wrap'
     427 |                 _p_func(_fmt, ##__VA_ARGS__);                           \
         |                         ^~~~
   include/linux/printk.h:528:9: note: in expansion of macro 'printk'
     528 |         printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
         |         ^~~~~~
   include/linux/kern_levels.h:14:25: note: in expansion of macro 'KERN_SOH'
      14 | #define KERN_INFO       KERN_SOH "6"    /* informational */
         |                         ^~~~~~~~
   include/linux/printk.h:528:16: note: in expansion of macro 'KERN_INFO'
     528 |         printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
         |                ^~~~~~~~~
   drivers/staging/axis-fifo/axis-fifo.c:957:9: note: in expansion of macro 'pr_info'
     957 |         pr_info("axis-fifo driver loaded with parameters read_timeout = %i, write_timeout = %i\n",
         |         ^~~~~~~
   include/linux/kern_levels.h:5:25: warning: format '%i' expects argument of type 'int', but argument 3 has type 'long int' [-Wformat=]
       5 | #define KERN_SOH        "\001"          /* ASCII Start Of Header */
         |                         ^~~~~~
   include/linux/printk.h:427:25: note: in definition of macro 'printk_index_wrap'
     427 |                 _p_func(_fmt, ##__VA_ARGS__);                           \
         |                         ^~~~
   include/linux/printk.h:528:9: note: in expansion of macro 'printk'
     528 |         printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
         |         ^~~~~~
   include/linux/kern_levels.h:14:25: note: in expansion of macro 'KERN_SOH'
      14 | #define KERN_INFO       KERN_SOH "6"    /* informational */
         |                         ^~~~~~~~
   include/linux/printk.h:528:16: note: in expansion of macro 'KERN_INFO'
     528 |         printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
         |                ^~~~~~~~~
   drivers/staging/axis-fifo/axis-fifo.c:957:9: note: in expansion of macro 'pr_info'
     957 |         pr_info("axis-fifo driver loaded with parameters read_timeout = %i, write_timeout = %i\n",
         |         ^~~~~~~


vim +5 include/linux/kern_levels.h

314ba3520e513a Joe Perches 2012-07-30  4  
04d2c8c83d0e3a Joe Perches 2012-07-30 @5  #define KERN_SOH	"\001"		/* ASCII Start Of Header */
04d2c8c83d0e3a Joe Perches 2012-07-30  6  #define KERN_SOH_ASCII	'\001'
04d2c8c83d0e3a Joe Perches 2012-07-30  7  

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

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

[Fabio M. De Francesco]

Khadija,

Just saw your v5 patch and Greg's two replies.

For v6 you will need to change the subject to "[PATCH v6] staging: axis-fifo: 
initialize timeouts in init only" to indicate that you are doing assignments 
in axis_fifo_init().

Don't forget to extend the version log with "Changes in v6:" and clarify that 
v5 had a different "Object" (you should probably also add a link to the v5 
patch in lore: https://lore.kernel.org/lkml /ZBMR4s8xyHGqMm72@khadija-virtual-
machine/). When the "Subject" changes, readers may not find the previous 
versions easily.    

On giovedì 16 marzo 2023 13:56:02 CET 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

Substitute "probe" with "init".

> function.
> 
> As a result, only use read_timeout as the last argument in
> wait_event_interruptible_timeout() call.

This two sentences are not much clear. I'd merge and rework:

"Initialize the module parameters read_timeout and write_timeout once in 
init().

Module parameters can only be set once and cannot be modified later, so we 
don't need to evaluate them again when passing the parameters to  
wait_event_interruptible_timeout()."   

> 
> Convert datatpe

s/datatpe/type/

> of read_timeout

of {read,write}_timeout

> from 'int' to 'long int' because
> implicit conversion of 'long int' to 'int' in statement 'read_timeout =
> MAX_SCHEDULE_TIMEOUT' results in an overflow warning.

We don't care too much about the warning themselves: I mean, it overflows and 
you must avoid it to happen (as you are doing with the changes of types), not 
merely be interested in avoiding the warning. "[] results in an overflow." is 
all we care about.

Add also the previous paragraph in the last part of the commit message.
 
> Perform same steps formodule parameter, write_timeout.

And instead delete the this last phrase.

> Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Khadija Kamran <kamrankhadijadj@gmail.com>
> ---
> 
> Changes in v5:
>  - Convert timeout's datatype from int to long.
> Changes in v4:
>  - Initialize timeouts once as suggested by Greg; this automatically
>    fixes the indentation problems.
>  - Change the subject and description.
> Changes in v3:
>  - Fix grammatical mistakes
>  - Do not change the second argument's indentation in split lines
> Changes in v2:
>  - Instead of matching alignment to open parenthesis, align second and
>    the last argument instead.
>  - Change the subject to 'remove tabs to align arguments'.
>  - Use imperative language in subject and description
> 
>  drivers/staging/axis-fifo/axis-fifo.c | 26 ++++++++++++++++----------
>  1 file changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/staging/axis-fifo/axis-fifo.c
> b/drivers/staging/axis-fifo/axis-fifo.c index dfd2b357f484..d667dc80df47
> 100644
> --- a/drivers/staging/axis-fifo/axis-fifo.c
> +++ b/drivers/staging/axis-fifo/axis-fifo.c
> @@ -103,17 +103,17 @@
>   *           globals
>   * ----------------------------
>   */
> -static int read_timeout = 1000; /* ms to wait before read() times out */
> -static int write_timeout = 1000; /* ms to wait before write() times out */
> +static long read_timeout = 1000; /* ms to wait before read() times out */
> +static long write_timeout = 1000; /* ms to wait before write() times out */
> 
>  /* ----------------------------
>   * module command-line arguments
>   * ----------------------------
>   */
> 
> -module_param(read_timeout, int, 0444);
> +module_param(read_timeout, long, 0444);
>  MODULE_PARM_DESC(read_timeout, "ms to wait before blocking read() timing 
out;
> set to -1 for no timeout"); -module_param(write_timeout, int, 0444);
> +module_param(write_timeout, long, 0444);
>  MODULE_PARM_DESC(write_timeout, "ms to wait before blocking write() timing
> out; set to -1 for no timeout");
> 
>  /* ----------------------------
> @@ -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,

What is this? You haven't yet compiled your patch.
Any further problems with enabling axis-fifo as a module?

Fabio

> 
> -				 (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 v5] staging: axis-fifo: initialize timeouts in probe only

[Khadija Kamran]

On Thu, Mar 16, 2023 at 03:38:03PM +0100, Fabio M. De Francesco wrote:
> Khadija,
>
> Just saw your v5 patch and Greg's two replies.
> 
> For v6 you will need to change the subject to "[PATCH v6] staging: axis-fifo: 
> initialize timeouts in init only" to indicate that you are doing assignments 
> in axis_fifo_init().
> 
> Don't forget to extend the version log with "Changes in v6:" and clarify that 
> v5 had a different "Object" (you should probably also add a link to the v5 
> patch in lore: https://lore.kernel.org/lkml /ZBMR4s8xyHGqMm72@khadija-virtual-
> machine/). When the "Subject" changes, readers may not find the previous 
> versions easily.    
>
> On giovedì 16 marzo 2023 13:56:02 CET 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
> 
> Substitute "probe" with "init".
> 
> > function.
> > 
> > As a result, only use read_timeout as the last argument in
> > wait_event_interruptible_timeout() call.
> 
> This two sentences are not much clear. I'd merge and rework:
> 
> "Initialize the module parameters read_timeout and write_timeout once in 
> init().
> 
> Module parameters can only be set once and cannot be modified later, so we 
> don't need to evaluate them again when passing the parameters to  
> wait_event_interruptible_timeout()."   
> 
> > 
> > Convert datatpe
> 
> s/datatpe/type/
> 
> > of read_timeout
> 
> of {read,write}_timeout
> 
> > from 'int' to 'long int' because
> > implicit conversion of 'long int' to 'int' in statement 'read_timeout =
> > MAX_SCHEDULE_TIMEOUT' results in an overflow warning.
> 
> We don't care too much about the warning themselves: I mean, it overflows and 
> you must avoid it to happen (as you are doing with the changes of types), not 
> merely be interested in avoiding the warning. "[] results in an overflow." is 
> all we care about.
>

Hey Fabio!
Thank you for your feedback. I have undertood it and will make sure to
send them in the next PATCH v6.

> Add also the previous paragraph in the last part of the commit message.
>  
> > Perform same steps formodule parameter, write_timeout.
> 
> And instead delete the this last phrase.
> 

Can you please explain the above feedback. I am confused. What should I
use instead of this last phrase?

> > Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Signed-off-by: Khadija Kamran <kamrankhadijadj@gmail.com>
> > ---
> > 
> > Changes in v5:
> >  - Convert timeout's datatype from int to long.
> > Changes in v4:
> >  - Initialize timeouts once as suggested by Greg; this automatically
> >    fixes the indentation problems.
> >  - Change the subject and description.
> > Changes in v3:
> >  - Fix grammatical mistakes
> >  - Do not change the second argument's indentation in split lines
> > Changes in v2:
> >  - Instead of matching alignment to open parenthesis, align second and
> >    the last argument instead.
> >  - Change the subject to 'remove tabs to align arguments'.
> >  - Use imperative language in subject and description
> > 
> >  drivers/staging/axis-fifo/axis-fifo.c | 26 ++++++++++++++++----------
> >  1 file changed, 16 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/staging/axis-fifo/axis-fifo.c
> > b/drivers/staging/axis-fifo/axis-fifo.c index dfd2b357f484..d667dc80df47
> > 100644
> > --- a/drivers/staging/axis-fifo/axis-fifo.c
> > +++ b/drivers/staging/axis-fifo/axis-fifo.c
> > @@ -103,17 +103,17 @@
> >   *           globals
> >   * ----------------------------
> >   */
> > -static int read_timeout = 1000; /* ms to wait before read() times out */
> > -static int write_timeout = 1000; /* ms to wait before write() times out */
> > +static long read_timeout = 1000; /* ms to wait before read() times out */
> > +static long write_timeout = 1000; /* ms to wait before write() times out */
> > 
> >  /* ----------------------------
> >   * module command-line arguments
> >   * ----------------------------
> >   */
> > 
> > -module_param(read_timeout, int, 0444);
> > +module_param(read_timeout, long, 0444);
> >  MODULE_PARM_DESC(read_timeout, "ms to wait before blocking read() timing 
> out;
> > set to -1 for no timeout"); -module_param(write_timeout, int, 0444);
> > +module_param(write_timeout, long, 0444);
> >  MODULE_PARM_DESC(write_timeout, "ms to wait before blocking write() timing
> > out; set to -1 for no timeout");
> > 
> >  /* ----------------------------
> > @@ -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,
> 
> What is this? You haven't yet compiled your patch.
> Any further problems with enabling axis-fifo as a module?
>


Sorry, my bad.  Instead of fixing the menuconfig I used this command to
remove the warnings: 
make -j"$(nproc)" ARCH=arm64 LLVM=1 drivers/staging/axis-fifo/
I thought it is compiling my module correctly.
But I am working on your feedback. And before sending my next patch I
will make sure to compile it properly.


> Fabio
> 
> > 
> > -				 (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 v5] staging: axis-fifo: initialize timeouts in probe only

[Fabio M. De Francesco]

On giovedì 16 marzo 2023 16:09:08 CET Khadija Kamran wrote:
> On Thu, Mar 16, 2023 at 03:38:03PM +0100, Fabio M. De Francesco wrote:
> > Khadija,
> > 
> > Just saw your v5 patch and Greg's two replies.
> > 
> > For v6 you will need to change the subject to "[PATCH v6] staging:
> > axis-fifo:
> > initialize timeouts in init only" to indicate that you are doing 
assignments
> > in axis_fifo_init().
> > 
> > Don't forget to extend the version log with "Changes in v6:" and clarify
> > that
> > v5 had a different "Object" (you should probably also add a link to the v5
> > patch in lore: https://lore.kernel.org/lkml
> > /ZBMR4s8xyHGqMm72@khadija-virtual- machine/). When the "Subject" changes,
> > readers may not find the previous versions easily.
> > 
> > On giovedì 16 marzo 2023 13:56:02 CET 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
> > 
> > Substitute "probe" with "init".
> > 
> > > function.
> > > 
> > > As a result, only use read_timeout as the last argument in
> > > wait_event_interruptible_timeout() call.
> > 
> > This two sentences are not much clear. I'd merge and rework:
> > 
> > "Initialize the module parameters read_timeout and write_timeout once in
> > init().
> > 
> > Module parameters can only be set once and cannot be modified later, so we
> > don't need to evaluate them again when passing the parameters to
> > wait_event_interruptible_timeout()."
> > 
> > > Convert datatpe
> > 
> > s/datatpe/type/
> > 
> > > of read_timeout
> > 
> > of {read,write}_timeout
> > 
> > > from 'int' to 'long int' because
> > > implicit conversion of 'long int' to 'int' in statement 'read_timeout =
> > > MAX_SCHEDULE_TIMEOUT' results in an overflow warning.
> > 
> > We don't care too much about the warning themselves: I mean, it overflows
> > and
> > you must avoid it to happen (as you are doing with the changes of types),
> > not
> > merely be interested in avoiding the warning. "[] results in an overflow."
> > is
> > all we care about.
> 
> Hey Fabio!
> Thank you for your feedback. I have understood it and will make sure to
> send them in the next PATCH v6.

Great to hear it!

> > Add also the previous paragraph in the last part of the commit message.
> > 
> > > Perform same steps formodule parameter, write_timeout.
> > 
> > And instead delete the this last phrase.
> 
> Can you please explain the above feedback. I am confused. What should I
> use instead of this last phrase?

Sorry, I made a typo in the sentence above and that may confuse you :-(

I just intended to suggest to delete "Perform same steps formodule parameter, 
write_timeout.".

In the previous lines I suggested you to merge and rework your entire commit 
message. If you like it you are left with the following text (that I put for 
you between two '"'):

"Initialize the module parameters read_timeout and write_timeout once in
init().

Module parameters can only be set once and cannot be modified later, so we
don't need to evaluate them again when passing the parameters to
wait_event_interruptible_timeout().

Convert the type of {read,write}_timeout from 'int' to 'long int' because 
implicit conversion of 'long int' to 'int' in statement 'read_timeout = 
MAX_SCHEDULE_TIMEOUT' results in an overflow.".

Just three small sentences are all you need (and don't forget to change the 
Subject - "probe()" -> "init()".

I hope I have been clearer this time.
If not, please ask for further clarification.

Thanks,

Fabio 

> > > Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Signed-off-by: Khadija Kamran <kamrankhadijadj@gmail.com>
> > > ---
> > > 
> > > Changes in v5:
> > >  - Convert timeout's datatype from int to long.
> > > 
> > > Changes in v4:
> > >  - Initialize timeouts once as suggested by Greg; this automatically
> > >  
> > >    fixes the indentation problems.
> > >  
> > >  - Change the subject and description.
> > > 
> > > Changes in v3:
> > >  - Fix grammatical mistakes
> > >  - Do not change the second argument's indentation in split lines
> > > 
> > > Changes in v2:
> > >  - Instead of matching alignment to open parenthesis, align second and
> > >  
> > >    the last argument instead.
> > >  
> > >  - Change the subject to 'remove tabs to align arguments'.
> > >  - Use imperative language in subject and description
> > >  
> > >  drivers/staging/axis-fifo/axis-fifo.c | 26 ++++++++++++++++----------
> > >  1 file changed, 16 insertions(+), 10 deletions(-)

[snip]

> > >  			
> > >  				 >= words_to_write,
> > 
> > What is this? You haven't yet compiled your patch.
> > Any further problems with enabling axis-fifo as a module?
> 
> Sorry, my bad.  Instead of fixing the menuconfig I used this command to
> remove the warnings:
> make -j"$(nproc)" ARCH=arm64 LLVM=1 drivers/staging/axis-fifo/
> I thought it is compiling my module correctly.
> But I am working on your feedback. And before sending my next patch I
> will make sure to compile it properly.

When you are done with build, install, and final reboot to test if your module 
can "modprobe" or "insmod" (i.e. link with the running custom kernel you 
built, installed and boot), try to compare the output of the following 
commands:

# uname -a
Linux suse 6.2.2-1-default #1 SMP PREEMPT_DYNAMIC Thu Mar  9 06:06:13 UTC 2023 
(44ca817) x86_64 x86_64 x86_64 GNU/Linux

AND

# modinfo <name of the module you are testing here>

I'm running "modinfo kvm" (but showing only two of many lines):

# modinfo kvm 
filename:       /lib/modules/6.2.2-1-default/kernel/arch/x86/kvm/kvm.ko.zst
vermagic:       6.2.2-1-default SMP preempt mod_unload modversions 

Can you see that the kernel in "uname -a" and the filename and vermagic have 
the same "6.2.2-1-default"? Well, so I'm sure I'm running the right Kernel and 
inserted the appropriate "kvm" module. 

Furthermore, before rebooting your custom kernel, you may also look at the 
directory in the Kernel where you compiled your module and search for "*.o" 
"*mod*" and "*.ko" files. If you have them, you built your module properly.

Thanks,

Fabio

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

[Fabio M. De Francesco]

On giovedì 16 marzo 2023 17:17:47 CET Fabio M. De Francesco wrote:
> On giovedì 16 marzo 2023 16:09:08 CET Khadija Kamran wrote:
> > On Thu, Mar 16, 2023 at 03:38:03PM +0100, Fabio M. De Francesco wrote:
> > > Khadija,
> > > 
> > > Just saw your v5 patch and Greg's two replies.
> > > 
> > > For v6 you will need to change the subject to "[PATCH v6] staging:
> > > axis-fifo:
> > > initialize timeouts in init only" to indicate that you are doing
> 
> assignments
> 
> > > in axis_fifo_init().
> > > 
> > > Don't forget to extend the version log with "Changes in v6:" and clarify
> > > that
> > > v5 had a different "Object" (you should probably also add a link to the 
v5
> > > patch in lore: https://lore.kernel.org/lkml
> > > /ZBMR4s8xyHGqMm72@khadija-virtual- machine/). When the "Subject" 
changes,
> > > readers may not find the previous versions easily.
> > > 
> > > On giovedì 16 marzo 2023 13:56:02 CET 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
> > > 
> > > Substitute "probe" with "init".
> > > 
> > > > function.
> > > > 
> > > > As a result, only use read_timeout as the last argument in
> > > > wait_event_interruptible_timeout() call.
> > > 
> > > This two sentences are not much clear. I'd merge and rework:
> > > 
> > > "Initialize the module parameters read_timeout and write_timeout once in
> > > init().
> > > 
> > > Module parameters can only be set once and cannot be modified later, so 
we
> > > don't need to evaluate them again when passing the parameters to
> > > wait_event_interruptible_timeout()."
> > > 
> > > > Convert datatpe
> > > 
> > > s/datatpe/type/
> > > 
> > > > of read_timeout
> > > 
> > > of {read,write}_timeout
> > > 
> > > > from 'int' to 'long int' because
> > > > implicit conversion of 'long int' to 'int' in statement 'read_timeout 
=
> > > > MAX_SCHEDULE_TIMEOUT' results in an overflow warning.
> > > 
> > > We don't care too much about the warning themselves: I mean, it 
overflows
> > > and
> > > you must avoid it to happen (as you are doing with the changes of 
types),
> > > not
> > > merely be interested in avoiding the warning. "[] results in an 
overflow."
> > > is
> > > all we care about.
> > 
> > Hey Fabio!
> > Thank you for your feedback. I have understood it and will make sure to
> > send them in the next PATCH v6.
> 
> Great to hear it!
> 
> > > Add also the previous paragraph in the last part of the commit message.
> > > 
> > > > Perform same steps formodule parameter, write_timeout.
> > > 
> > > And instead delete the this last phrase.
> > 
> > Can you please explain the above feedback. I am confused. What should I
> > use instead of this last phrase?
> 
> Sorry, I made a typo in the sentence above and that may confuse you :-(
> 
> I just intended to suggest to delete "Perform same steps formodule 
parameter,
> write_timeout.".
> 
> In the previous lines I suggested you to merge and rework your entire commit
> message. If you like it you are left with the following text (that I put for
> you between two '"'):
> 
> "Initialize the module parameters read_timeout and write_timeout once in
> init().
> 
> Module parameters can only be set once and cannot be modified later, so we
> don't need to evaluate them again when passing the parameters to
> wait_event_interruptible_timeout().
> 
> Convert the type of {read,write}_timeout from 'int' to 'long int' because
> implicit conversion of 'long int' to 'int' in statement 'read_timeout =
> MAX_SCHEDULE_TIMEOUT' results in an overflow.".
> 
> Just three small sentences are all you need (and don't forget to change the
> Subject - "probe()" -> "init()".
> 
> I hope I have been clearer this time.
> If not, please ask for further clarification.
> 
> Thanks,
> 
> Fabio

I put my signature here, but it was a mistake.
Did you see the rest of the message below?

> > > > Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > Signed-off-by: Khadija Kamran <kamrankhadijadj@gmail.com>
> > > > ---
> > > > 
> > > > Changes in v5:
> > > >  - Convert timeout's datatype from int to long.
> > > > 
> > > > Changes in v4:
> > > >  - Initialize timeouts once as suggested by Greg; this automatically
> > > >  
> > > >    fixes the indentation problems.
> > > >  
> > > >  - Change the subject and description.
> > > > 
> > > > Changes in v3:
> > > >  - Fix grammatical mistakes
> > > >  - Do not change the second argument's indentation in split lines
> > > > 
> > > > Changes in v2:
> > > >  - Instead of matching alignment to open parenthesis, align second and
> > > >  
> > > >    the last argument instead.
> > > >  
> > > >  - Change the subject to 'remove tabs to align arguments'.
> > > >  - Use imperative language in subject and description
> > > >  
> > > >  drivers/staging/axis-fifo/axis-fifo.c | 26 ++++++++++++++++----------
> > > >  1 file changed, 16 insertions(+), 10 deletions(-)
> 
> [snip]
> 
> > > >  				 >= words_to_write,
> > > 
> > > What is this? You haven't yet compiled your patch.

This unnecessary objection was due to that split I was talking about in the v6 
of your patch. At first glance it looked to me like a remnant of some mistake 
with copy-pasting. Sorry, I should have looked at it more carefully. 

> > > Any further problems with enabling axis-fifo as a module?
> > Sorry, my bad.  Instead of fixing the menuconfig I used this command to
> > remove the warnings:
> > make -j"$(nproc)" ARCH=arm64 LLVM=1 drivers/staging/axis-fifo/
> > I thought it is compiling my module correctly.
> > But I am working on your feedback. And before sending my next patch I
> > will make sure to compile it properly.
> 
> When you are done with build, install, and final reboot to test if your 
module
> can "modprobe" or "insmod" (i.e. link with the running custom kernel you
> built, installed and boot), try to compare the output of the following
> commands:
> 
> # uname -a
> Linux suse 6.2.2-1-default #1 SMP PREEMPT_DYNAMIC Thu Mar  9 06:06:13 UTC 
2023
> (44ca817) x86_64 x86_64 x86_64 GNU/Linux
> 
> AND
> 
> # modinfo <name of the module you are testing here>
> 
> I'm running "modinfo kvm" (but showing only two of many lines):
> 
> # modinfo kvm
> filename:       /lib/modules/6.2.2-1-default/kernel/arch/x86/kvm/kvm.ko.zst
> vermagic:       6.2.2-1-default SMP preempt mod_unload modversions
> 
> Can you see that the kernel in "uname -a" and the filename and vermagic have
> the same "6.2.2-1-default"? Well, so I'm sure I'm running the right Kernel 
and
> inserted the appropriate "kvm" module.
> 
> Furthermore, before rebooting your custom kernel, you may also look at the
> directory in the Kernel where you compiled your module and search for "*.o"
> "*mod*" and "*.ko" files. If you have them, you built your module properly.
> 
> Thanks,
> 
> Fabio

Fabio

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

[Khadija Kamran]

On Thu, Mar 16, 2023 at 05:17:47PM +0100, Fabio M. De Francesco wrote:
> On giovedì 16 marzo 2023 16:09:08 CET Khadija Kamran wrote:
> > On Thu, Mar 16, 2023 at 03:38:03PM +0100, Fabio M. De Francesco wrote:
> > > Khadija,
> > > 
> > > Just saw your v5 patch and Greg's two replies.
> > > 
> > > For v6 you will need to change the subject to "[PATCH v6] staging:
> > > axis-fifo:
> > > initialize timeouts in init only" to indicate that you are doing 
> assignments
> > > in axis_fifo_init().
> > > 
> > > Don't forget to extend the version log with "Changes in v6:" and clarify
> > > that
> > > v5 had a different "Object" (you should probably also add a link to the v5
> > > patch in lore: https://lore.kernel.org/lkml
> > > /ZBMR4s8xyHGqMm72@khadija-virtual- machine/). When the "Subject" changes,
> > > readers may not find the previous versions easily.
> > > 
> > > On giovedì 16 marzo 2023 13:56:02 CET 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
> > > 
> > > Substitute "probe" with "init".
> > > 
> > > > function.
> > > > 
> > > > As a result, only use read_timeout as the last argument in
> > > > wait_event_interruptible_timeout() call.
> > > 
> > > This two sentences are not much clear. I'd merge and rework:
> > > 
> > > "Initialize the module parameters read_timeout and write_timeout once in
> > > init().
> > > 
> > > Module parameters can only be set once and cannot be modified later, so we
> > > don't need to evaluate them again when passing the parameters to
> > > wait_event_interruptible_timeout()."
> > > 
> > > > Convert datatpe
> > > 
> > > s/datatpe/type/
> > > 
> > > > of read_timeout
> > > 
> > > of {read,write}_timeout
> > > 
> > > > from 'int' to 'long int' because
> > > > implicit conversion of 'long int' to 'int' in statement 'read_timeout =
> > > > MAX_SCHEDULE_TIMEOUT' results in an overflow warning.
> > > 
> > > We don't care too much about the warning themselves: I mean, it overflows
> > > and
> > > you must avoid it to happen (as you are doing with the changes of types),
> > > not
> > > merely be interested in avoiding the warning. "[] results in an overflow."
> > > is
> > > all we care about.
> > 
> > Hey Fabio!
> > Thank you for your feedback. I have understood it and will make sure to
> > send them in the next PATCH v6.
> 
> Great to hear it!
> 
> > > Add also the previous paragraph in the last part of the commit message.
> > > 
> > > > Perform same steps formodule parameter, write_timeout.
> > > 
> > > And instead delete the this last phrase.
> > 
> > Can you please explain the above feedback. I am confused. What should I
> > use instead of this last phrase?
> 
> Sorry, I made a typo in the sentence above and that may confuse you :-(
> 
> I just intended to suggest to delete "Perform same steps formodule parameter, 
> write_timeout.".
> 
> In the previous lines I suggested you to merge and rework your entire commit 
> message. If you like it you are left with the following text (that I put for 
> you between two '"'):
> 
> "Initialize the module parameters read_timeout and write_timeout once in
> init().
> 
> Module parameters can only be set once and cannot be modified later, so we
> don't need to evaluate them again when passing the parameters to
> wait_event_interruptible_timeout().
> 
> Convert the type of {read,write}_timeout from 'int' to 'long int' because 
> implicit conversion of 'long int' to 'int' in statement 'read_timeout = 
> MAX_SCHEDULE_TIMEOUT' results in an overflow.".
> 
> Just three small sentences are all you need (and don't forget to change the 
> Subject - "probe()" -> "init()".
> 
> I hope I have been clearer this time.
> If not, please ask for further clarification.
> 
> Thanks,
> 
> Fabio 
> 
> > > > Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > Signed-off-by: Khadija Kamran <kamrankhadijadj@gmail.com>
> > > > ---
> > > > 
> > > > Changes in v5:
> > > >  - Convert timeout's datatype from int to long.
> > > > 
> > > > Changes in v4:
> > > >  - Initialize timeouts once as suggested by Greg; this automatically
> > > >  
> > > >    fixes the indentation problems.
> > > >  
> > > >  - Change the subject and description.
> > > > 
> > > > Changes in v3:
> > > >  - Fix grammatical mistakes
> > > >  - Do not change the second argument's indentation in split lines
> > > > 
> > > > Changes in v2:
> > > >  - Instead of matching alignment to open parenthesis, align second and
> > > >  
> > > >    the last argument instead.
> > > >  
> > > >  - Change the subject to 'remove tabs to align arguments'.
> > > >  - Use imperative language in subject and description
> > > >  
> > > >  drivers/staging/axis-fifo/axis-fifo.c | 26 ++++++++++++++++----------
> > > >  1 file changed, 16 insertions(+), 10 deletions(-)
> 
> [snip]
> 
> > > >  			
> > > >  				 >= words_to_write,
> > > 
> > > What is this? You haven't yet compiled your patch.
> > > Any further problems with enabling axis-fifo as a module?
> > 
> > Sorry, my bad.  Instead of fixing the menuconfig I used this command to
> > remove the warnings:
> > make -j"$(nproc)" ARCH=arm64 LLVM=1 drivers/staging/axis-fifo/
> > I thought it is compiling my module correctly.
> > But I am working on your feedback. And before sending my next patch I
> > will make sure to compile it properly.
>

Hey Fabio!

Hope you are doing well. After  spending a lot of time on this I am
stuck now. Kindly help me resolve this issueor understand it better.

Following your instructions I deleted my config file and copied one from
the /boot/ directory. After that I enabled the dependencies(CONFIG_OF=y
and CONFIG_HAS_IOMEM=y). I was successfully able to enable axis-fifo as
a module.

I then ran the following commands:
 - make drivers/staging/axis-fifo/
 - sudo make modules_install install(this command took hours) :'(

> When you are done with build, install, and final reboot to test if your module 
> can "modprobe" or "insmod" (i.e. link with the running custom kernel you 
> built, installed and boot), try to compare the output of the following 
> commands:
> 
> # uname -a
> Linux suse 6.2.2-1-default #1 SMP PREEMPT_DYNAMIC Thu Mar  9 06:06:13 UTC 2023 
> (44ca817) x86_64 x86_64 x86_64 GNU/Linux
> 

The above command works

> AND
> 
> # modinfo <name of the module you are testing here>
>

On running 'modinfo axis-fifo' I get error saying module axis-fifo not
found.

> I'm running "modinfo kvm" (but showing only two of many lines):
> 
> # modinfo kvm 
> filename:       /lib/modules/6.2.2-1-default/kernel/arch/x86/kvm/kvm.ko.zst
> vermagic:       6.2.2-1-default SMP preempt mod_unload modversions 
> 
> Can you see that the kernel in "uname -a" and the filename and vermagic have 
> the same "6.2.2-1-default"? Well, so I'm sure I'm running the right Kernel and 
> inserted the appropriate "kvm" module. 
> 
> Furthermore, before rebooting your custom kernel, you may also look at the 
> directory in the Kernel where you compiled your module and search for "*.o" 
> "*mod*" and "*.ko" files. If you have them, you built your module properly.
>

There is a "*.o" file and "*.mod" file but there is no "*.ko" file in
the axis-fifo directory.

Kindly help me with this.

Regards,
Khadija


> Thanks,
> 
> Fabio
> 
> 
> 

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

[Fabio M. De Francesco]

On giovedì 16 marzo 2023 19:35:09 CET Khadija Kamran wrote:
> On Thu, Mar 16, 2023 at 05:17:47PM +0100, Fabio M. De Francesco wrote:
> > On giovedì 16 marzo 2023 16:09:08 CET Khadija Kamran wrote:

[snip]

> Hey Fabio!
> 
> Hope you are doing well. After  spending a lot of time on this I am
> stuck now. Kindly help me resolve this issue or understand it better.
> 
> Following your instructions I deleted my config file and copied one from
> the /boot/ directory. After that I enabled the dependencies(CONFIG_OF=y
> and CONFIG_HAS_IOMEM=y). I was successfully able to enable axis-fifo as
> a module.
> 
> I then ran the following commands:
>  - make drivers/staging/axis-fifo/

No, this is not the right command... you are not invoking the linker to make 
the .ko object.

Use "make M=drivers/staging/axis-fifo/"
or "make M=drivers/staging/axis-fifo/ W=1 -j8" (the latter to enable level '1' 
warning and run on your 2 * 4 logical cores).

>  - sudo make modules_install install(this command took hours) :'(

This is odd, it shouldn't :-/

As I said in another message, I'll set aside some time to help you check if 
you need to fine tune your VM and Hypervisor configuration. 

I'm returning on the same subject we have been talked about because you said 
at least twice that your builds and install are too slow. We'll try to 
diagnose it in an IRC session on #kernel-outreachy (I'm pretty sure for the 
first days of next week - I'll send an invite).

> > When you are done with build, install, and final reboot to test if your
> > module can "modprobe" or "insmod" (i.e. link with the running custom 
kernel
> > you built, installed and boot), try to compare the output of the following
> > commands:
> > 
> > # uname -a
> > Linux suse 6.2.2-1-default #1 SMP PREEMPT_DYNAMIC Thu Mar  9 06:06:13 UTC
> > 2023 (44ca817) x86_64 x86_64 x86_64 GNU/Linux
> 
> The above command works
> 
> > AND
> > 
> > # modinfo <name of the module you are testing here>
> 
> On running 'modinfo axis-fifo' I get error saying module axis-fifo not
> found.

Try again after building with "M=drivers/staging" (as said above). Don't 
forget to run "make modules_install install" and then reboot into your custom 
built Kernel, not the distribution's kernel.

While you are there, run "lsmod" to see all loaded modules. Pick one randomly 
from the output list and run "modinfo name_of_the_module_you_want_info_about".
 
> > I'm running "modinfo kvm" (but showing only two of many lines):
> > 
> > # modinfo kvm
> > filename:       /lib/modules/6.2.2-1-default/kernel/arch/x86/kvm/
kvm.ko.zst
> > vermagic:       6.2.2-1-default SMP preempt mod_unload modversions
> > 
> > Can you see that the kernel in "uname -a" and the filename and vermagic 
have
> > the same "6.2.2-1-default"? Well, so I'm sure I'm running the right Kernel
> > and inserted the appropriate "kvm" module.
> > 
> > Furthermore, before rebooting your custom kernel, you may also look at the
> > directory in the Kernel where you compiled your module and search for 
"*.o"
> > "*mod*" and "*.ko" files. If you have them, you built your module 
properly.
> 
> There is a "*.o" file and "*.mod" file but there is no "*.ko" file in
> the axis-fifo directory.
> 
> Kindly help me with this.
> 
> Regards,
> Khadija
> 
> > Thanks,
> > 
> > Fabio

Let me know if this time it works.

Fabio

P.S.: Have you had time to read that "Linux Kernel Module Programming" guide I 
sent you the link of? You can find a lot of information about modules there. 
I'd strongly recommend you to read it. 

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

[Khadija Kamran]

On Thu, Mar 16, 2023 at 09:07:09PM +0100, Fabio M. De Francesco wrote:
> On giovedì 16 marzo 2023 19:35:09 CET Khadija Kamran wrote:
> > On Thu, Mar 16, 2023 at 05:17:47PM +0100, Fabio M. De Francesco wrote:
> > > On giovedì 16 marzo 2023 16:09:08 CET Khadija Kamran wrote:
> 
> [snip]
> 
> > Hey Fabio!
> > 
> > Hope you are doing well. After  spending a lot of time on this I am
> > stuck now. Kindly help me resolve this issue or understand it better.
> > 
> > Following your instructions I deleted my config file and copied one from
> > the /boot/ directory. After that I enabled the dependencies(CONFIG_OF=y
> > and CONFIG_HAS_IOMEM=y). I was successfully able to enable axis-fifo as
> > a module.
> > 
> > I then ran the following commands:
> >  - make drivers/staging/axis-fifo/
>

Hey Fabio!
Sorry for reaching out to you very late. For the past two days I have
had problems with my Virtual Machine. It is stuck at boot, and this
happened after it accidentally shut down. I have tried to resolve this
problem but nothing is working. Currently I have boot into an older
kernel version from the GRUB menu. 

I am also sharing the error here:
Gave up waiting for root device. Common problems:
-Boot args (cat /proc/cmdline)
  -Check rootdelay= (did the system wait long enough?)
-Missing modules (cat /proc/modules; ls /dev)
ALERT! UUID=718ed077-947d-4018-80ad-59825678e81d does not exist.
Dropping to a shell!
BusyBox v1.27.2 (Ubuntu 1:1.27.2-2ubuntu3.2) built-in shell (ash)
Enter 'help' for a list of built-in commands.

I have Windows10 installed and I have created Ubunutu 22.04.1 VM on VMWare with
13GB RAM and 2 processors(4 cores each).

(initramfs)_
> No, this is not the right command... you are not invoking the linker to make 
> the .ko object.
> 
> Use "make M=drivers/staging/axis-fifo/"
> or "make M=drivers/staging/axis-fifo/ W=1 -j8" (the latter to enable level '1' 
> warning and run on your 2 * 4 logical cores).
>

This command gives error saying: 
scripts/Makefile.build:41: /drivers/staging/axis-fifo/Makefile: 
No such file or directory
make[1]: *** No rule to make target '/drivers/staging/axis-fifo/Makefile'.  Stop.
make: *** [Makefile:2028: /drivers/staging/axis-fifo] Error 2

> >  - sudo make modules_install install(this command took hours) :'(
> 
> This is odd, it shouldn't :-/
> 
> As I said in another message, I'll set aside some time to help you check if 
> you need to fine tune your VM and Hypervisor configuration. 
> 
> I'm returning on the same subject we have been talked about because you said 
> at least twice that your builds and install are too slow. We'll try to 
> diagnose it in an IRC session on #kernel-outreachy (I'm pretty sure for the 
> first days of next week - I'll send an invite).
>

Yes I would love that. Kindly help me with this.
Thank you!

Regards,
Khadija Kamran

> > > When you are done with build, install, and final reboot to test if your
> > > module can "modprobe" or "insmod" (i.e. link with the running custom 
> kernel
> > > you built, installed and boot), try to compare the output of the following
> > > commands:
> > > 
> > > # uname -a
> > > Linux suse 6.2.2-1-default #1 SMP PREEMPT_DYNAMIC Thu Mar  9 06:06:13 UTC
> > > 2023 (44ca817) x86_64 x86_64 x86_64 GNU/Linux
> > 
> > The above command works
> > 
> > > AND
> > > 
> > > # modinfo <name of the module you are testing here>
> > 
> > On running 'modinfo axis-fifo' I get error saying module axis-fifo not
> > found.
> 
> Try again after building with "M=drivers/staging" (as said above). Don't 
> forget to run "make modules_install install" and then reboot into your custom 
> built Kernel, not the distribution's kernel.
> 
> While you are there, run "lsmod" to see all loaded modules. Pick one randomly 
> from the output list and run "modinfo name_of_the_module_you_want_info_about".
>  
> > > I'm running "modinfo kvm" (but showing only two of many lines):
> > > 
> > > # modinfo kvm
> > > filename:       /lib/modules/6.2.2-1-default/kernel/arch/x86/kvm/
> kvm.ko.zst
> > > vermagic:       6.2.2-1-default SMP preempt mod_unload modversions
> > > 
> > > Can you see that the kernel in "uname -a" and the filename and vermagic 
> have
> > > the same "6.2.2-1-default"? Well, so I'm sure I'm running the right Kernel
> > > and inserted the appropriate "kvm" module.
> > > 
> > > Furthermore, before rebooting your custom kernel, you may also look at the
> > > directory in the Kernel where you compiled your module and search for 
> "*.o"
> > > "*mod*" and "*.ko" files. If you have them, you built your module 
> properly.
> > 
> > There is a "*.o" file and "*.mod" file but there is no "*.ko" file in
> > the axis-fifo directory.
> > 
> > Kindly help me with this.
> > 
> > Regards,
> > Khadija
> > 
> > > Thanks,
> > > 
> > > Fabio
> 
> Let me know if this time it works.
> 
> Fabio
> 
> P.S.: Have you had time to read that "Linux Kernel Module Programming" guide I 
> sent you the link of? You can find a lot of information about modules there. 
> I'd strongly recommend you to read it. 
> 
> 

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

[Fabio M. De Francesco]

On lunedì 20 marzo 2023 07:00:28 CET Khadija Kamran wrote:
> On Thu, Mar 16, 2023 at 09:07:09PM +0100, Fabio M. De Francesco wrote:
> > On giovedì 16 marzo 2023 19:35:09 CET Khadija Kamran wrote:
> > > On Thu, Mar 16, 2023 at 05:17:47PM +0100, Fabio M. De Francesco wrote:
> > > > On giovedì 16 marzo 2023 16:09:08 CET Khadija Kamran wrote:
> > [snip]
> > 
> > > Hey Fabio!
> > > 
> > > Hope you are doing well. After  spending a lot of time on this I am
> > > stuck now. Kindly help me resolve this issue or understand it better.
> > > 
> > > Following your instructions I deleted my config file and copied one from
> > > the /boot/ directory. After that I enabled the dependencies(CONFIG_OF=y
> > > and CONFIG_HAS_IOMEM=y). I was successfully able to enable axis-fifo as
> > > a module.
> > > 
> > > I then ran the following commands:
> > >  - make drivers/staging/axis-fifo/
> 
> Hey Fabio!
> Sorry for reaching out to you very late. For the past two days I have
> had problems with my Virtual Machine. It is stuck at boot, and this
> happened after it accidentally shut down. I have tried to resolve this
> problem but nothing is working. Currently I have boot into an older
> kernel version from the GRUB menu.
> 
> I am also sharing the error here:
> Gave up waiting for root device. Common problems:
> -Boot args (cat /proc/cmdline)
>   -Check rootdelay= (did the system wait long enough?)
> -Missing modules (cat /proc/modules; ls /dev)
> ALERT! UUID=718ed077-947d-4018-80ad-59825678e81d does not exist.
> Dropping to a shell!
> BusyBox v1.27.2 (Ubuntu 1:1.27.2-2ubuntu3.2) built-in shell (ash)
> Enter 'help' for a list of built-in commands.
> 
> I have Windows10 installed and I have created Ubunutu 22.04.1 VM on VMWare
> with 13GB RAM and 2 processors(4 cores each).
> 
> (initramfs)_
> 
> > No, this is not the right command... you are not invoking the linker to 
make
> > the .ko object.
> > 
> > Use "make M=drivers/staging/axis-fifo/"
> > or "make M=drivers/staging/axis-fifo/ W=1 -j8" (the latter to enable level
> > '1' warning and run on your 2 * 4 logical cores).
> 
> This command gives error saying:
> scripts/Makefile.build:41: /drivers/staging/axis-fifo/Makefile:
> No such file or directory
> make[1]: *** No rule to make target '/drivers/staging/axis-fifo/Makefile'. 
> Stop. make: *** [Makefile:2028: /drivers/staging/axis-fifo] Error 2
> 
> > >  - sudo make modules_install install(this command took hours) :'(
> > 
> > This is odd, it shouldn't :-/
> > 
> > As I said in another message, I'll set aside some time to help you check 
if
> > you need to fine tune your VM and Hypervisor configuration.
> > 
> > I'm returning on the same subject we have been talked about because you 
said
> > at least twice that your builds and install are too slow. We'll try to
> > diagnose it in an IRC session on #kernel-outreachy (I'm pretty sure for 
the
> > first days of next week - I'll send an invite).
> 
> Yes I would love that. Kindly help me with this.
> Thank you!
> 
> Regards,
> Khadija Kamran
> 
> > > > When you are done with build, install, and final reboot to test if 
your
> > > > module can "modprobe" or "insmod" (i.e. link with the running custom
> > 
> > kernel
> > 
> > > > you built, installed and boot), try to compare the output of the
> > > > following
> > > > commands:
> > > > 
> > > > # uname -a
> > > > Linux suse 6.2.2-1-default #1 SMP PREEMPT_DYNAMIC Thu Mar  9 06:06:13
> > > > UTC
> > > > 2023 (44ca817) x86_64 x86_64 x86_64 GNU/Linux
> > > 
> > > The above command works
> > > 
> > > > AND
> > > > 
> > > > # modinfo <name of the module you are testing here>
> > > 
> > > On running 'modinfo axis-fifo' I get error saying module axis-fifo not
> > > found.
> > 
> > Try again after building with "M=drivers/staging" (as said above). Don't
> > forget to run "make modules_install install" and then reboot into your
> > custom
> > built Kernel, not the distribution's kernel.
> > 
> > While you are there, run "lsmod" to see all loaded modules. Pick one
> > randomly
> > from the output list and run "modinfo
> > name_of_the_module_you_want_info_about".> 
> > > > I'm running "modinfo kvm" (but showing only two of many lines):
> > > > 
> > > > # modinfo kvm
> > > > filename:       /lib/modules/6.2.2-1-default/kernel/arch/x86/kvm/
> > 
> > kvm.ko.zst
> > 
> > > > vermagic:       6.2.2-1-default SMP preempt mod_unload modversions
> > > > 
> > > > Can you see that the kernel in "uname -a" and the filename and 
vermagic
> > 
> > have
> > 
> > > > the same "6.2.2-1-default"? Well, so I'm sure I'm running the right
> > > > Kernel
> > > > and inserted the appropriate "kvm" module.
> > > > 
> > > > Furthermore, before rebooting your custom kernel, you may also look at
> > > > the
> > > > directory in the Kernel where you compiled your module and search for
> > 
> > "*.o"
> > 
> > > > "*mod*" and "*.ko" files. If you have them, you built your module
> > 
> > properly.
> > 
> > > There is a "*.o" file and "*.mod" file but there is no "*.ko" file in
> > > the axis-fifo directory.
> > > 
> > > Kindly help me with this.
> > > 
> > > Regards,
> > > Khadija
> > > 
> > > > Thanks,
> > > > 
> > > > Fabio
> > 
> > Let me know if this time it works.
> > 
> > Fabio
> > 
> > P.S.: Have you had time to read that "Linux Kernel Module Programming" 
guide
> > I sent you the link of? You can find a lot of information about modules
> > there. I'd strongly recommend you to read it.

Khadija,

I think that you are trying to skip some passages. Let me explain better...

Soon after copying from /boot your new .config you should run "make 
menuconfig" or "make oldconfig" to re-create the necessary dependence and 
other files which are required to build properly.

I'd suggest "make menuconfig" so that you can check if you enabled correctly 
the module(s) you are interested to build and other built-in options you may 
optionally need.

Then you must rebuild and install. So...

1) "make menuconfig" (don't forget to save before exiting),
2) "make W=1 -j8" (because you have 8 logical cores reserved to your VMWare 
VM, correct?).
3) "make modules_install install".

At this point check that you have a fresh kernel image in /boot, that /boot/
grub/grub.conf contains a new entry with your freshly built custom kernel so 
that you can boot it from selecting the proper entry in the menu that you'll 
see after reboot. Also check that you have axis-fifo.ko under /lib/modules/
<the new kernel version>/build/drivers/staging/axis-fifo/.

For example in my VM for test I see axis-fifo with...

ls -l /lib/modules/6.3.0-rc2-x86-32-debug+/build/drivers/staging/axis-fifo/
total 564
-rw-r--r-- 1 1001 fsgqa    196 Mar 16 20:57 .Module.symvers.cmd
-rw-r--r-- 1 1001 fsgqa    275 Mar 17 15:39 .axis-fifo.ko.cmd
-rw-r--r-- 1 1001 fsgqa    183 Mar 12 20:29 .axis-fifo.mod.cmd
-rw-r--r-- 1 1001 fsgqa  50093 Mar 17 15:39 .axis-fifo.mod.o.cmd
-rw-r--r-- 1 1001 fsgqa  61110 Mar 17 15:37 .axis-fifo.o.cmd
-rw-r--r-- 1 1001 fsgqa    147 Mar 17 15:37 .modules.order.cmd
-rw-r--r-- 1 1001 fsgqa      0 Mar 16 20:57 Module.symvers
-rw-r--r-- 1 1001 fsgqa 255748 Mar 17 15:39 axis-fifo.ko
-rw-r--r-- 1 1001 fsgqa     38 Mar 12 20:29 axis-fifo.mod
-rw-r--r-- 1 1001 fsgqa   2354 Mar 17 15:38 axis-fifo.mod.c
-rw-r--r-- 1 1001 fsgqa  44604 Mar 17 15:39 axis-fifo.mod.o
-rw-r--r-- 1 1001 fsgqa 128156 Mar 17 15:37 axis-fifo.o
-rw-r--r-- 1 1001 fsgqa     38 Mar 17 15:37 modules.order

(as you can see, the last time I built and installed the kernel and the axis-
fifo driver's module was March 17, 15:39).

Please notice that my kernel has that "-x86-32-debug+" suffix after the kernel 
version but this depends on a special option I need to set to differentiate 
"debug" kernels from "production" kernels. You may have something else that 
your distribution appends.

If you can't find the module run again "ls -l" one level up, like in /lib/
modules/6.3.0-rc2-x86-32-debug+/build/drivers/staging/ to see if at least you 
have any of the drivers/staging/ modules. Go one more layer up if you don't 
find them.

If everything I cited above is where it is supposed to be and grub.conf is 
properly configured with the new entry, reboot and enjoy your module (try 
modprobe, modinfo, lsmod, and the likes).

Fabio 

Fabio

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

[Khadija Kamran]

On Thu, Mar 16, 2023 at 09:07:09PM +0100, Fabio M. De Francesco wrote:
> On giovedì 16 marzo 2023 19:35:09 CET Khadija Kamran wrote:
> > On Thu, Mar 16, 2023 at 05:17:47PM +0100, Fabio M. De Francesco wrote:
> > > On giovedì 16 marzo 2023 16:09:08 CET Khadija Kamran wrote:
> 
> [snip]
> 
> > Hey Fabio!
> > 
> > Hope you are doing well. After  spending a lot of time on this I am
> > stuck now. Kindly help me resolve this issue or understand it better.
> > 
> > Following your instructions I deleted my config file and copied one from
> > the /boot/ directory. After that I enabled the dependencies(CONFIG_OF=y
> > and CONFIG_HAS_IOMEM=y). I was successfully able to enable axis-fifo as
> > a module.
> > 
> > I then ran the following commands:
> >  - make drivers/staging/axis-fifo/
> 
> No, this is not the right command... you are not invoking the linker to make 
> the .ko object.
> 
> Use "make M=drivers/staging/axis-fifo/"
> or "make M=drivers/staging/axis-fifo/ W=1 -j8" (the latter to enable level '1' 
> warning and run on your 2 * 4 logical cores).
> 
> >  - sudo make modules_install install(this command took hours) :'(
> 
> This is odd, it shouldn't :-/
> 
> As I said in another message, I'll set aside some time to help you check if 
> you need to fine tune your VM and Hypervisor configuration. 
> 
> I'm returning on the same subject we have been talked about because you said 
> at least twice that your builds and install are too slow. We'll try to 
> diagnose it in an IRC session on #kernel-outreachy (I'm pretty sure for the 
> first days of next week - I'll send an invite).
> 
> > > When you are done with build, install, and final reboot to test if your
> > > module can "modprobe" or "insmod" (i.e. link with the running custom 
> kernel
> > > you built, installed and boot), try to compare the output of the following
> > > commands:
> > > 
> > > # uname -a
> > > Linux suse 6.2.2-1-default #1 SMP PREEMPT_DYNAMIC Thu Mar  9 06:06:13 UTC
> > > 2023 (44ca817) x86_64 x86_64 x86_64 GNU/Linux
> > 
> > The above command works
> > 
> > > AND
> > > 
> > > # modinfo <name of the module you are testing here>
> > 
> > On running 'modinfo axis-fifo' I get error saying module axis-fifo not
> > found.
> 
> Try again after building with "M=drivers/staging" (as said above). Don't 
> forget to run "make modules_install install" and then reboot into your custom 
> built Kernel, not the distribution's kernel.
> 
> While you are there, run "lsmod" to see all loaded modules. Pick one randomly 
> from the output list and run "modinfo name_of_the_module_you_want_info_about".
>  
> > > I'm running "modinfo kvm" (but showing only two of many lines):
> > > 
> > > # modinfo kvm
> > > filename:       /lib/modules/6.2.2-1-default/kernel/arch/x86/kvm/
> kvm.ko.zst
> > > vermagic:       6.2.2-1-default SMP preempt mod_unload modversions
> > > 
> > > Can you see that the kernel in "uname -a" and the filename and vermagic 
> have
> > > the same "6.2.2-1-default"? Well, so I'm sure I'm running the right Kernel
> > > and inserted the appropriate "kvm" module.
> > > 
> > > Furthermore, before rebooting your custom kernel, you may also look at the
> > > directory in the Kernel where you compiled your module and search for 
> "*.o"
> > > "*mod*" and "*.ko" files. If you have them, you built your module 
> properly.
> > 
> > There is a "*.o" file and "*.mod" file but there is no "*.ko" file in
> > the axis-fifo directory.
> > 
> > Kindly help me with this.
> > 
> > Regards,
> > Khadija
> > 
> > > Thanks,
> > > 
> > > Fabio
> 
> Let me know if this time it works.
> 
> Fabio
> 
> P.S.: Have you had time to read that "Linux Kernel Module Programming" guide I 
> sent you the link of? You can find a lot of information about modules there. 
> I'd strongly recommend you to read it. 
>

Fabio,

I have not read it yet. But it is in my mind and as soon as I get some
free time I will start reading it :)

Thank you!

> 

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

[Fabio M. De Francesco]

On giovedì 16 marzo 2023 19:35:09 CET Khadija Kamran wrote:
> On Thu, Mar 16, 2023 at 05:17:47PM +0100, Fabio M. De Francesco wrote:

[snip]

> > When you are done with build, install, and final reboot to test if your
> > module can "modprobe" or "insmod" (i.e. link with the running custom 
kernel
> > you built, installed and boot), try to compare the output of the following
> > commands:
> > 
> > # uname -a
> > Linux suse 6.2.2-1-default #1 SMP PREEMPT_DYNAMIC Thu Mar  9 06:06:13 UTC
> > 2023 (44ca817) x86_64 x86_64 x86_64 GNU/Linux
> 
> The above command works
> 
> > AND
> > 
> > # modinfo <name of the module you are testing here>
> 
> On running 'modinfo axis-fifo' I get error saying module axis-fifo not
> found.

I built axis-fifo with your changes and then I ran "make install 
modules_install" in a QEMU/KVM x86_32 VM that I use on a Linux host for 
testing my patches (Linux on Linux).

tweed32:~ # uname -a
Linux tweed32 6.3.0-rc2-x86-32-debug+ #32 SMP PREEMPT_DYNAMIC Thu Mar 16 
18:09:49 CET 2023 i686 athlon i386 GNU/Linux

tweed32:~ # modinfo axis-fifo 
filename:       /lib/modules/6.3.0-rc2-x86-32-debug+/kernel/drivers/staging/
axis-fifo/axis-fifo.ko
description:    Xilinx AXI-Stream FIFO v4.1 IP core driver
author:         Jacob Feder <jacobsfeder@gmail.com>
license:        GPL
srcversion:     EBF46AD6851EAAE67D1000B
alias:          of:N*T*Cxlnx,axi-fifo-mm-s-4.1C*
alias:          of:N*T*Cxlnx,axi-fifo-mm-s-4.1
depends:        
staging:        Y
retpoline:      Y
intree:         Y
name:           axis_fifo
vermagic:       6.3.0-rc2-x86-32-debug+ SMP preempt mod_unload modversions K7 
parm:           read_timeout:ms to wait before blocking read() timing out; set 
to -1 for no timeout (long)
parm:           write_timeout:ms to wait before blocking write() timing out; 
set to -1 for no timeout (long)

Do you see the "parm" lines? What's the type of read and write_timeout?

Fabio

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

[Khadija Kamran]

On Thu, Mar 16, 2023 at 09:17:52PM +0100, Fabio M. De Francesco wrote:
> On giovedì 16 marzo 2023 19:35:09 CET Khadija Kamran wrote:
> > On Thu, Mar 16, 2023 at 05:17:47PM +0100, Fabio M. De Francesco wrote:
> 
> [snip]
> 
> > > When you are done with build, install, and final reboot to test if your
> > > module can "modprobe" or "insmod" (i.e. link with the running custom 
> kernel
> > > you built, installed and boot), try to compare the output of the following
> > > commands:
> > > 
> > > # uname -a
> > > Linux suse 6.2.2-1-default #1 SMP PREEMPT_DYNAMIC Thu Mar  9 06:06:13 UTC
> > > 2023 (44ca817) x86_64 x86_64 x86_64 GNU/Linux
> > 
> > The above command works
> > 
> > > AND
> > > 
> > > # modinfo <name of the module you are testing here>
> > 
> > On running 'modinfo axis-fifo' I get error saying module axis-fifo not
> > found.
> 
> I built axis-fifo with your changes and then I ran "make install 
> modules_install" in a QEMU/KVM x86_32 VM that I use on a Linux host for 
> testing my patches (Linux on Linux).
> 
> tweed32:~ # uname -a
> Linux tweed32 6.3.0-rc2-x86-32-debug+ #32 SMP PREEMPT_DYNAMIC Thu Mar 16 
> 18:09:49 CET 2023 i686 athlon i386 GNU/Linux
>

Hey Fabio!

The following command is not working for me. I think I have mentioned
this before, it says, module axis-fifo not found.

Thank you!

Regards,
Khadija

> tweed32:~ # modinfo axis-fifo 
> filename:       /lib/modules/6.3.0-rc2-x86-32-debug+/kernel/drivers/staging/
> axis-fifo/axis-fifo.ko
> description:    Xilinx AXI-Stream FIFO v4.1 IP core driver
> author:         Jacob Feder <jacobsfeder@gmail.com>
> license:        GPL
> srcversion:     EBF46AD6851EAAE67D1000B
> alias:          of:N*T*Cxlnx,axi-fifo-mm-s-4.1C*
> alias:          of:N*T*Cxlnx,axi-fifo-mm-s-4.1
> depends:        
> staging:        Y
> retpoline:      Y
> intree:         Y
> name:           axis_fifo
> vermagic:       6.3.0-rc2-x86-32-debug+ SMP preempt mod_unload modversions K7 
> parm:           read_timeout:ms to wait before blocking read() timing out; set 
> to -1 for no timeout (long)
> parm:           write_timeout:ms to wait before blocking write() timing out; 
> set to -1 for no timeout (long)
> 
> Do you see the "parm" lines? What's the type of read and write_timeout?
> 
> Fabio
> 
> 

Re: [PATCH v5] 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/20230316-205814
patch link:    https://lore.kernel.org/r/ZBMR4s8xyHGqMm72%40khadija-virtual-machine
patch subject: [PATCH v5] staging: axis-fifo: initialize timeouts in probe only
config: hexagon-randconfig-r033-20230312 (https://download.01.org/0day-ci/archive/20230317/202303171454.sScrVVOh-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
        # https://github.com/intel-lab-lkp/linux/commit/7e359baa2318b55e12a0c099b75fbd0d799907cf
        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/20230316-205814
        git checkout 7e359baa2318b55e12a0c099b75fbd0d799907cf
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon 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/202303171454.sScrVVOh-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/staging/axis-fifo/axis-fifo.c:25:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
                                                     ^
   In file included from drivers/staging/axis-fifo/axis-fifo.c:25:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
                                                     ^
   In file included from drivers/staging/axis-fifo/axis-fifo.c:25:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
>> drivers/staging/axis-fifo/axis-fifo.c:958:3: warning: format specifies type 'int' but the argument has type 'long' [-Wformat]
                   read_timeout, write_timeout);
                   ^~~~~~~~~~~~
   include/linux/printk.h:528:34: note: expanded from macro 'pr_info'
           printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
                                   ~~~     ^~~~~~~~~~~
   include/linux/printk.h:455:60: note: expanded from macro 'printk'
   #define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
                                                       ~~~    ^~~~~~~~~~~
   include/linux/printk.h:427:19: note: expanded from macro 'printk_index_wrap'
                   _p_func(_fmt, ##__VA_ARGS__);                           \
                           ~~~~    ^~~~~~~~~~~
   drivers/staging/axis-fifo/axis-fifo.c:958:17: warning: format specifies type 'int' but the argument has type 'long' [-Wformat]
                   read_timeout, write_timeout);
                                 ^~~~~~~~~~~~~
   include/linux/printk.h:528:34: note: expanded from macro 'pr_info'
           printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
                                   ~~~     ^~~~~~~~~~~
   include/linux/printk.h:455:60: note: expanded from macro 'printk'
   #define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
                                                       ~~~    ^~~~~~~~~~~
   include/linux/printk.h:427:19: note: expanded from macro 'printk_index_wrap'
                   _p_func(_fmt, ##__VA_ARGS__);                           \
                           ~~~~    ^~~~~~~~~~~
   8 warnings generated.


vim +958 drivers/staging/axis-fifo/axis-fifo.c

4a965c5f89decd Jacob Feder 2018-07-22  954  
4a965c5f89decd Jacob Feder 2018-07-22  955  static int __init axis_fifo_init(void)
4a965c5f89decd Jacob Feder 2018-07-22  956  {
4a965c5f89decd Jacob Feder 2018-07-22  957  	pr_info("axis-fifo driver loaded with parameters read_timeout = %i, write_timeout = %i\n",
4a965c5f89decd Jacob Feder 2018-07-22 @958  		read_timeout, write_timeout);
4a965c5f89decd Jacob Feder 2018-07-22  959  	return platform_driver_register(&axis_fifo_driver);
4a965c5f89decd Jacob Feder 2018-07-22  960  }
4a965c5f89decd Jacob Feder 2018-07-22  961  

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