On Tue, Oct 18, 2022 at 02:52:21PM +0200, Julia Lawall wrote: > > > On Tue, 18 Oct 2022, Deepak R Varma wrote: > > > On Tue, Oct 18, 2022 at 09:39:08AM +0200, Julia Lawall wrote: > > > > > > > > > On Tue, 18 Oct 2022, Deepak R Varma wrote: > > > > > > > The done_buffer count is already available in the hdm_channel struct. > > > > Calling dim_get_channel_state function to source this value out of > > > > the same structure is unnecessary. > > > > Further, the second parameter struct dim_ch_state_t to this function > > > > is filled by using the hdm_channel inside the function. This filled in > > > > variable is never used in the caller and can be altogether removed. > > > > So, a call to dim_get_channel_state function in this context also > > > > deems expensive. > > > > > > Thanks for the rewrite. > > > > > > I find "source this value out of" hard to understand. > > > > > > I would have written something like the following: > > > > > > The function dim_get_channel_state only serves to initialize the ready and > > > done_buffers fields of the structure passed as its second argument. In > > > service_done_flag, this structure is never used again and the only purpose > > > of the call is to get the value that is put in the done_buffers field. > > > But that value is just the done_sw_buffers_number field of the call's > > > first argument. So the whole call is useless, and we can just replace it > > > with an access to this field. > > > > > > This change implies that the variable st is no longer used, so drop it as > > > well. > > > > This is really well written. Sounds much structured. Now my own log message > > sounds a little random :) > > > > Is it okay for me to use your verbiage as is in my patch log? > > Yes. Thank you. Can I convert this into a patch set and include the other suggestion from you to correct the dim_ch_state_t struct name? Or should these be separate patches now? ./drv > > julia >
