From: Greg KH <gregkh@linuxfoundation.org>
To: Deepak R Varma <drv@mailo.com>
Cc: outreachy@lists.linux.dev, linux-staging@lists.linux.dev,
linux-kernel@vger.kernel.org, kumarpraveen@linux.microsoft.com,
saurabh.truth@gmail.com
Subject: Re: [PATCH v4 1/2] staging: most: dim2: read done_buffers count locally from HDM channel
Date: Thu, 20 Oct 2022 17:03:14 +0200 [thread overview]
Message-ID: <Y1FjMrGHc2CdDCPQ@kroah.com> (raw)
In-Reply-To: <83fd237d2ac157d234e9c7cce1206904c2d8773d.1666208065.git.drv@mailo.com>
On Thu, Oct 20, 2022 at 01:24:53AM +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.
Are you sure it is useless?
You have changed the logic here, you are now thinking that this value
can never change, while before you were ensured of getting the "correct"
value as it is under the lock when the function is called.
I can't take this type of change as a "cleanup" patch for outreachy
unless you have the hardware as it is NOT a basic "checkpatch" style
cleanup at all.
If you want to get this change accepted, please work with the maintainer
of the code and get them to agree that the change is correct. And if it
is, odds are more things also would need to be cleaned up at the same
time, right?
thanks,
greg k-h
next prev parent reply other threads:[~2022-10-20 15:03 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-19 19:53 [PATCH v4 0/2] staging: most: dim2: remove unnecessary function call and variable usage Deepak R Varma
2022-10-19 19:54 ` [PATCH v4 1/2] staging: most: dim2: read done_buffers count locally from HDM channel Deepak R Varma
2022-10-20 15:03 ` Greg KH [this message]
2022-10-20 16:41 ` Deepak R Varma
2022-10-19 19:56 ` [PATCH v4 2/2] staging: most: dim2: correct misleading struct type name Deepak R Varma
2022-10-19 20:08 ` Julia Lawall
2022-10-19 20:30 ` Deepak R Varma
2022-10-20 11:48 ` Deepak R Varma
2022-10-20 12:06 ` Julia Lawall
2022-10-20 12:56 ` Deepak R Varma
2022-10-20 15:04 ` Greg KH
2022-10-20 15:05 ` Greg KH
2022-10-20 15:10 ` Julia Lawall
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y1FjMrGHc2CdDCPQ@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=drv@mailo.com \
--cc=kumarpraveen@linux.microsoft.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=outreachy@lists.linux.dev \
--cc=saurabh.truth@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).