From: Frank Li <Frank.li@nxp.com>
To: Guenter Roeck <linux@roeck-us.net>
Cc: alice.guo@nxp.com, festevam@gmail.com, imx@lists.linux.dev,
kernel@pengutronix.de, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-watchdog@vger.kernel.org,
s.hauer@pengutronix.de, shawnguo@kernel.org,
wim@linux-watchdog.org, ye.li@nxp.com
Subject: Re: [PATCH v3 1/1] watchdog: imx7ulp_wdt: move post_rcs_wait into struct imx_wdt_hw_feature
Date: Mon, 15 Jul 2024 17:56:44 -0400 [thread overview]
Message-ID: <ZpWbHLid9du0tdlS@lizhi-Precision-Tower-5810> (raw)
In-Reply-To: <1829e700-6b0f-42cb-8b08-81ca2bca4e24@roeck-us.net>
On Mon, Jul 15, 2024 at 02:42:23PM -0700, Guenter Roeck wrote:
> On 7/15/24 11:34, Frank Li wrote:
> > On Mon, Jul 15, 2024 at 11:01:04AM -0700, Guenter Roeck wrote:
> > > On 7/15/24 10:07, Frank Li wrote:
> > > > Move post_rcs_wait into struct imx_wdt_hw_feature to simplify code logic
> > > > for different compatible strings
> > > >
> > > > i.MX93 and i.MX8ULP watchdog do not need to wait 2.5 clocks after RCS is
> > > > done. Set post_rcs_wait to false explicitly to maintain code consistency.
> > > >
> > >
> > > Why ? That is not necessary and typically frowned upon for static variables.
> >
> > Some maintainer in other subsystem like explicity set to false to read code
> > easily even though not necessary for static variable espcially there are
> > already one which set to false.
> >
> > I am fine for each ways. You are free to pick up v2 instead of v3 if you
> > don't like v3's change.
> >
>
> That is not the point. The point here is that you made an - in my opinion
> unnecessary - change to this patch while at the same time adding my
> Reviewed-by: tag which applied to another version of the patch.
> This is inappropriate. Please refrain from doing that in the future.
According to my previous experience, drop review tag only when there are
'big' change in new version. Of cause, the 'big' is quite subjective. I
think "set false explicitly" is not 'big' enough to drop review tags.
Frank
>
> Guenter
>
next prev parent reply other threads:[~2024-07-15 21:56 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-15 17:07 Frank Li
2024-07-15 17:16 ` Fabio Estevam
2024-07-15 18:01 ` Guenter Roeck
2024-07-15 18:01 ` Guenter Roeck
2024-07-15 18:34 ` Frank Li
2024-07-15 21:42 ` Guenter Roeck
2024-07-15 21:56 ` Frank Li [this message]
2024-07-16 0:25 ` Guenter Roeck
2024-07-15 21:44 ` Guenter Roeck
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=ZpWbHLid9du0tdlS@lizhi-Precision-Tower-5810 \
--to=frank.li@nxp.com \
--cc=alice.guo@nxp.com \
--cc=festevam@gmail.com \
--cc=imx@lists.linux.dev \
--cc=kernel@pengutronix.de \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-watchdog@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=s.hauer@pengutronix.de \
--cc=shawnguo@kernel.org \
--cc=wim@linux-watchdog.org \
--cc=ye.li@nxp.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
all inboxes | Powered by JetHome®