From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: Daniel <dany97@live.ca>
Cc: LKML <linux-kernel@vger.kernel.org>,
matan@svgalib.org, platform-driver-x86@vger.kernel.org
Subject: Re: [PATCH] platform/x86: lg-laptop: Fix WMAB call in fan_mode_store
Date: Mon, 8 Sep 2025 13:37:22 +0300 (EEST) [thread overview]
Message-ID: <45f04f10-a003-fc42-2551-a2e180358ca8@linux.intel.com> (raw)
In-Reply-To: <MN2PR06MB55980FFA32BF5DABCE34333EDC0DA@MN2PR06MB5598.namprd06.prod.outlook.com>
On Sat, 6 Sep 2025, Daniel wrote:
> > Is it okay to remove preserving the other bits?
>
> The WMAB call reads only the lower nibble or the upper nibble from the
> lowest byte. Similarly when the fan status is read from WMAB, something
> of the form ((x << 4) | x) is always returned.
So the upper portion of the mask in the old code was overkill? Please note
that down into the changelog so it's recorded into the git history. This
avoids somebody having to wonder why a seemingly unrelated change like
that was made if somebody looks at this commit after a few years from the
git history.
> > Please name these field with defined GENMASK() and then use FIELD_PREP()
> > here for both fields.
>
> I don't see why something like that should be necessary, seems overkill,
> but if you insist then I'll do it.
Yes, I insist. When we touch older code, the expectation is to use modern
style and helpers. In here, naming the fields will improve readability
and document the fields more explicitly.
> I'll try to send another patch sometime soon, I've been very busy.
Don't worry, there's time. :-)
--
i.
prev parent reply other threads:[~2025-09-08 10:37 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-22 20:17 Daniel
2025-08-28 11:10 ` Ilpo Järvinen
2025-08-28 11:12 ` Ilpo Järvinen
2025-09-07 2:28 ` Daniel
2025-09-08 10:37 ` Ilpo Järvinen [this message]
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=45f04f10-a003-fc42-2551-a2e180358ca8@linux.intel.com \
--to=ilpo.jarvinen@linux.intel.com \
--cc=dany97@live.ca \
--cc=linux-kernel@vger.kernel.org \
--cc=matan@svgalib.org \
--cc=platform-driver-x86@vger.kernel.org \
/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®