On Mon, Mar 13, 2023 at 03:13:01PM +0100, Fabio M. De Francesco wrote: > On domenica 12 marzo 2023 18:33:19 CET Khadija Kamran wrote: > > Module parameter, read_timeout, can only be set at 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. > > > > Nice idea... But it's not yours :-) > > Therefore, you should give credit to Greg with the following tag: > > Suggested-by: Greg Kroah-Hartman <...> > > Place the above-mentioned tag a line before the "Signed-off-by:" (which is > always the last line, whatever other tags you might need to add). > Hey Fabio! Thank you for letting me know. I was confused as to where should I mention that this change was recommended by Greg. > > Signed-off-by: Khadija Kamran <kamrankhadijadj@xxxxxxxxx> > > --- > > If this patch was a v4 you should have put a log right here, after the three > dashes, explaining what changed from one release to another, release after > release. Please read some other well formatted and accepted patches for real > world examples of how to write version logs. > Okay, got it! I shouldn't have missed it. > However, this patch is _not_ a v4 (so no version log is needed after the three > dashes). This is your _first_ patch that addresses Greg's suggested > refactoring. Therefore, just put [PATCH] in the subject line. > > That inappropriate "v4" seems to explain the second error showed by the patch- > bot. Thus, read carefully its message and ask for further explanations if > something is still unclear. > Thank you! It is clear. I will send this again as first_patch. > Thanks, > > Fabio > > P.S.: The code looks good but I could not apply it in mainline tree. I don't > know whether this patch is somehow broken or the driver's files differ between > the most recent staging tree and mainline. > > However, does it work for you on the most recent staging tree? Did you run > checkpatch on your own patch? (I'm also asking this question because of the > first error showed by the patch-bot). Can you git-reset to a previous state > and reapply your own patches to your local work branch? > Yes, I did run checkpatch on my patch as suggested by Dan before. It showed errors regarding trailing white spaces. Sorry, I ignored them thinking that they were present before in the code. I will correct them in the next patch submission. Also, I had one question. Is it okay to write a long subject as I have used in this patch? Regards, Khadija > > drivers/staging/axis-fifo/axis-fifo.c | 18 ++++++++++++------ > > 1 file changed, 12 insertions(+), 6 deletions(-) > > >
