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

[Khadija Kamran]

On Mon, Mar 20, 2023 at 02:38:24PM +0100, Fabio M. De Francesco wrote:
> On lunedì 20 marzo 2023 07:34:04 CET Khadija Kamran wrote:
> > On Fri, Mar 17, 2023 at 11:29:25AM +0100, Fabio M. De Francesco wrote:
> > > Khadija,
> > > 
> > > Congratulations for having your first patch in Linux, via Greg's staging
> > > tree.
> > > 
> > > It will take some time before it reaches mainline, although it is already 
> on
> > > its way to get upstream.
> > 
> > Thank you! :)
> > 
> > > On giovedì 16 marzo 2023 21:09:00 CET Khadija Kamran wrote:
> > > > 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().
> > >


Hey Fabio,
I am talking about this below:


> > > Greg made you (and indirectly me notice) that the statement above is a 
> kind
> > > of short-circuit because it misses to make the readers notice that you are
> > > dealing with specific permissions granted to these two module's 
> parameters.
> 
> Only "read" permissions for owner, group, others.
> Obviously, when the module is initialized, "insmod" can pass actual values to 
> the arguments. The point is that from that moment onward nobody is allowed to 
> change the initial values associated with this variables, but they can still 
> be read at will.
> 
> > I am trying to make sense of this. As the permissions do not allow
> > write, so the value cannot be configured afterwards.
> 
> Yes, if with "afterwards" you are intending after they are set at insmod runs. 
> 
> > Instead of saying 'cannot be modified later', we should talk more about
> > permissions here too.
> 
> I'm confused by this statement. Can you please rephrase?
> 
> > Am I getting it right?
> 
> Not sure, it depends on what you meant with the previous phrase.

As you said above that the commit message makes the reader miss the
permission details, so should we write more about permissions in the
description?


>  
> > Thank you!
> > 
> > Regards,
> > Khadija
> 
> You're welcome!
> 
> So, thanks for working on this patch as long as it takes to get it done. 
> 
> I think the lesson to be learned is that in our community there are barriers 
> to the entry of substandard products and therefore people have to do their 
> best if they really want to see their work applied.


Yes, you are right. Due to this reason, the whole process is making me
learn a lot. I am really glad to be a part of it ^-^ 


> These tasks are not for the "faints of heart" :-)
> 
> Fabio
>

Regards,
Khadija :)


> 
>