Linux Hardware Monitor development
 help / color / mirror / Atom feed
From: "Thomas Weißschuh" <thomas@weissschuh.net>
To: Guenter Roeck <linux@roeck-us.net>
Cc: Sung-Chi Li <lschyi@chromium.org>,
	Jean Delvare <jdelvare@suse.com>,
	 Benson Leung <bleung@chromium.org>,
	Guenter Roeck <groeck@chromium.org>,
	 chrome-platform@lists.linux.dev, linux-hwmon@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/3] hwmon: (cros_ec) Add reading target fan RPM function
Date: Fri, 14 Mar 2025 09:52:11 +0100	[thread overview]
Message-ID: <5cb816c0-c445-4a23-82de-e3d9cbce54ee@t-8ch.de> (raw)
In-Reply-To: <523fe47a-73da-4eec-80b7-61408e0ba27b@roeck-us.net>

On 2025-03-13 16:36:54-0700, Guenter Roeck wrote:
> On 3/13/25 09:24, Thomas Weißschuh wrote:
> > On 2025-03-13 12:47:43+0800, Sung-Chi Li wrote:
> > > Implement the functionality of reading the target fan RPM setting from
> > > ChromeOS embedded controller under framework.
> > > 
> > > Signed-off-by: Sung-Chi Li <lschyi@chromium.org>
> > > ---
> > >   drivers/hwmon/cros_ec_hwmon.c | 18 ++++++++++++++++++
> > >   1 file changed, 18 insertions(+)
> > > 
> > > diff --git a/drivers/hwmon/cros_ec_hwmon.c b/drivers/hwmon/cros_ec_hwmon.c
> > > index b2fec0768301f116f49c57b8dbfb042b98a573e1..73bfcbbaf9531be6b753cfef8045fd5dab5b2ab3 100644
> > > --- a/drivers/hwmon/cros_ec_hwmon.c
> > > +++ b/drivers/hwmon/cros_ec_hwmon.c
> > > @@ -36,6 +36,19 @@ static int cros_ec_hwmon_read_fan_speed(struct cros_ec_device *cros_ec, u8 index
> > >   	return 0;
> > >   }
> > > +static int cros_ec_hwmon_read_fan_target(struct cros_ec_device *cros_ec, u8 index, int32_t *speed)
> > 
> > int32_t is a userspace type. In the kernel use i32, or even better u32.
> > 
> 
> Seems to be pretty widely used to complain about.

There is even a checkpatch.pl test for it, which should have triggered:
PREFER_KERNEL_TYPES.

> $ git grep int32_t drivers/ | wc
>   43662  192381 3555402

33k of those are in generated amdgpu headers.
This search also happens to include the more frequently used uint32_t.

> Also, in comparison:
> 
> $ git grep i32 drivers/ | wc
>     820    4009   68486

The numbers for u32 look a bit different:

$ git grep u32 drivers/ | wc
    234768 1137059 17410570


Also this specific driver already consistently uses uNN.
This does look wrong:

	int32_t target_rpm;
 	u16 speed;
 	u8 temp;


Thomas

  reply	other threads:[~2025-03-14  8:52 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-13  4:47 [PATCH 0/3] Export the target RPM fan control by ChromeOS EC under hwmon Sung-Chi Li
2025-03-13  4:47 ` [PATCH 1/3] hwmon: (cros_ec) Add setting target fan RPM function Sung-Chi Li
2025-03-13 16:21   ` Thomas Weißschuh
2025-03-17  3:55     ` Sung-Chi, Li
2025-03-13  4:47 ` [PATCH 2/3] hwmon: (cros_ec) Add reading " Sung-Chi Li
2025-03-13 16:24   ` Thomas Weißschuh
2025-03-13 23:36     ` Guenter Roeck
2025-03-14  8:52       ` Thomas Weißschuh [this message]
2025-03-14 14:23         ` Guenter Roeck
2025-03-17  3:51     ` Sung-Chi, Li
2025-03-17  6:40       ` Thomas Weißschuh
2025-03-14 11:25   ` kernel test robot
2025-03-13  4:47 ` [PATCH 3/3] hwmon: (cros_ec) Register fan target attribute Sung-Chi Li
2025-03-13 16:16 ` [PATCH 0/3] Export the target RPM fan control by ChromeOS EC under hwmon Thomas Weißschuh

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=5cb816c0-c445-4a23-82de-e3d9cbce54ee@t-8ch.de \
    --to=thomas@weissschuh.net \
    --cc=bleung@chromium.org \
    --cc=chrome-platform@lists.linux.dev \
    --cc=groeck@chromium.org \
    --cc=jdelvare@suse.com \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=lschyi@chromium.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