mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
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.


      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®