diff mbox series

[1/2] staging: most: dim2: read done_buffers count locally from HDM channel

Message ID 83fd237d2ac157d234e9c7cce1206904c2d8773d.1666105876.git.drv@mailo.com
State New
Headers
Series staging: most: dim2: remove unnecessary function call and variable usage |

Commit Message

Deepak R Varma Oct. 18, 2022, 4:49 p.m. UTC 
  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.

Signed-off-by: Deepak R Varma <drv@mailo.com>
---

PLEASE NOTE:
   1. I have only built the module on my machine, but have not tested it.
      I am not sure how to test this change. I am willing to test it with
      appropriate guidance provided I have the necessary hardware.
   2. This was a standalone patch earlier. It is now combined into a patch set
      with another patch for the same driver. Hence I am carry forwarding the
      change log for this patch here:

Changes in v3:
   1. The patch log message is further improved. This revised verbiage is as
      thankfully provided by julia.lawall@inria.fr

Changes in v2:
   1. Update patch log message to be more descriptive about the reason for change.
      Feedback provided by julia.lawall@inria.fr



 drivers/staging/most/dim2/dim2.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

--
2.30.2

Comments

Greg KH Oct. 18, 2022, 5:12 p.m. UTC | #1 
On Tue, Oct 18, 2022 at 10:19:21PM +0530, Deepak R Varma wrote:
> 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.
> 
> Signed-off-by: Deepak R Varma <drv@mailo.com>
> ---
> 
> PLEASE NOTE:
>    1. I have only built the module on my machine, but have not tested it.
>       I am not sure how to test this change. I am willing to test it with
>       appropriate guidance provided I have the necessary hardware.
>    2. This was a standalone patch earlier. It is now combined into a patch set
>       with another patch for the same driver. Hence I am carry forwarding the
>       change log for this patch here:
> 
> Changes in v3:
>    1. The patch log message is further improved. This revised verbiage is as
>       thankfully provided by julia.lawall@inria.fr
> 
> Changes in v2:
>    1. Update patch log message to be more descriptive about the reason for change.
>       Feedback provided by julia.lawall@inria.fr
> 

You need to say this is a v3 in the subject line as documented :(
Deepak R Varma Oct. 18, 2022, 5:40 p.m. UTC | #2 
On Tue, Oct 18, 2022 at 07:12:45PM +0200, Greg KH wrote:
> On Tue, Oct 18, 2022 at 10:19:21PM +0530, Deepak R Varma wrote:
> > 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.
> >
> > Signed-off-by: Deepak R Varma <drv@mailo.com>
> > ---
> >
> > PLEASE NOTE:
> >    1. I have only built the module on my machine, but have not tested it.
> >       I am not sure how to test this change. I am willing to test it with
> >       appropriate guidance provided I have the necessary hardware.
> >    2. This was a standalone patch earlier. It is now combined into a patch set
> >       with another patch for the same driver. Hence I am carry forwarding the
> >       change log for this patch here:
> >
> > Changes in v3:
> >    1. The patch log message is further improved. This revised verbiage is as
> >       thankfully provided by julia.lawall@inria.fr
> >
> > Changes in v2:
> >    1. Update patch log message to be more descriptive about the reason for change.
> >       Feedback provided by julia.lawall@inria.fr
> >
>
> You need to say this is a v3 in the subject line as documented :(
>
Hi Greg,
This was a standalone patch earlier and first time included in the patch set. I
was not aware I am allowed to version a patch individually, different from the
cover letter. I have posted a query to Outreachy mailing list for additional
clarification. I will send in a v4 once I have clarity.

Thank you,
./drv
Greg KH Oct. 18, 2022, 6:07 p.m. UTC | #3 
On Tue, Oct 18, 2022 at 11:10:34PM +0530, Deepak R Varma wrote:
> On Tue, Oct 18, 2022 at 07:12:45PM +0200, Greg KH wrote:
> > On Tue, Oct 18, 2022 at 10:19:21PM +0530, Deepak R Varma wrote:
> > > 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.
> > >
> > > Signed-off-by: Deepak R Varma <drv@mailo.com>
> > > ---
> > >
> > > PLEASE NOTE:
> > >    1. I have only built the module on my machine, but have not tested it.
> > >       I am not sure how to test this change. I am willing to test it with
> > >       appropriate guidance provided I have the necessary hardware.
> > >    2. This was a standalone patch earlier. It is now combined into a patch set
> > >       with another patch for the same driver. Hence I am carry forwarding the
> > >       change log for this patch here:
> > >
> > > Changes in v3:
> > >    1. The patch log message is further improved. This revised verbiage is as
> > >       thankfully provided by julia.lawall@inria.fr
> > >
> > > Changes in v2:
> > >    1. Update patch log message to be more descriptive about the reason for change.
> > >       Feedback provided by julia.lawall@inria.fr
> > >
> >
> > You need to say this is a v3 in the subject line as documented :(
> >
> Hi Greg,
> This was a standalone patch earlier and first time included in the patch set. I
> was not aware I am allowed to version a patch individually, different from the
> cover letter. I have posted a query to Outreachy mailing list for additional
> clarification. I will send in a v4 once I have clarity.

The cover letter will have v4 and so will each individual patch.
Turning 1 patch into a series or adding new patches to a series does not
mean you start over with the numbering.  Otherwise that would be
impossible for us to track, right?

thanks,

greg k-h
diff mbox series

Patch

diff --git a/drivers/staging/most/dim2/dim2.c b/drivers/staging/most/dim2/dim2.c
index ab72e11ac5ab..4c1f27898a29 100644
--- a/drivers/staging/most/dim2/dim2.c
+++ b/drivers/staging/most/dim2/dim2.c
@@ -259,7 +259,6 @@  static void retrieve_netinfo(struct dim2_hdm *dev, struct mbo *mbo)
 static void service_done_flag(struct dim2_hdm *dev, int ch_idx)
 {
 	struct hdm_channel *hdm_ch = dev->hch + ch_idx;
-	struct dim_ch_state_t st;
 	struct list_head *head;
 	struct mbo *mbo;
 	int done_buffers;
@@ -271,7 +270,7 @@  static void service_done_flag(struct dim2_hdm *dev, int ch_idx)

 	spin_lock_irqsave(&dim_lock, flags);

-	done_buffers = dim_get_channel_state(&hdm_ch->ch, &st)->done_buffers;
+	done_buffers = hdm_ch->ch.done_sw_buffers_number;
 	if (!done_buffers) {
 		spin_unlock_irqrestore(&dim_lock, flags);
 		return;