From: Ivor Wanders <ivor@iwanders.net>
To: w_armin@gmx.de
Cc: corbet@lwn.net, hdegoede@redhat.com, ivor@iwanders.net,
jdelvare@suse.com, linux-doc@vger.kernel.org,
linux-hwmon@vger.kernel.org, linux-kernel@vger.kernel.org,
linux@roeck-us.net, luzmaximilian@gmail.com,
markgross@kernel.org, platform-driver-x86@vger.kernel.org
Subject: Re: [PATCH v2 1/2] hwmon: add fan speed monitoring driver for Surface devices
Date: Thu, 28 Dec 2023 18:50:30 -0500 [thread overview]
Message-ID: <20231228235030.6526-1-ivor@iwanders.net> (raw)
In-Reply-To: <764c0645-674a-4a25-9bdb-2707e9532af7@gmx.de>
Hi Armin, thank you for your time and comments.
> You can drop those checks, the hwmon core.
> Maybe you could use a more unique name for the hwmon chip, like "surface_fan".
Sure, incorporated both suggestions.
> Please propagate the error from __ssam_fan_rpm_get().
Done. Currently that function can only return -EIO and 0, but indeed best to
propagate that in case more error codes are returned in the future.
> What exactly is the purpose of calling ssam_device_set_drvdata() here?
That line was necessary when this driver also hooked into the thermal subsystem.
Integration with the thermal subsystem was removed in downstream iterations when
I discovered we can't directly control the fan speed, see [1] for more info.
I've removed this line, as it no longer serves any purpose here.
~Ivor
[1]: https://github.com/linux-surface/kernel/pull/144#issuecomment-1853123261
next prev parent reply other threads:[~2023-12-28 23:50 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-28 0:34 [PATCH v2 0/2] Surface fan monitoring driver Ivor Wanders
2023-12-28 0:34 ` [PATCH v2 1/2] hwmon: add fan speed monitoring driver for Surface devices Ivor Wanders
2023-12-28 15:28 ` Armin Wolf
2023-12-28 23:50 ` Ivor Wanders [this message]
2023-12-30 16:32 ` Maximilian Luz
2023-12-30 22:46 ` Ivor Wanders
2023-12-28 0:34 ` [PATCH v2 2/2] platform/surface: aggregator_registry: add entry for fan speed Ivor Wanders
2023-12-29 22:42 ` Maximilian Luz
2023-12-30 17:19 ` Maximilian Luz
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=20231228235030.6526-1-ivor@iwanders.net \
--to=ivor@iwanders.net \
--cc=corbet@lwn.net \
--cc=hdegoede@redhat.com \
--cc=jdelvare@suse.com \
--cc=linux-doc@vger.kernel.org \
--cc=linux-hwmon@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=luzmaximilian@gmail.com \
--cc=markgross@kernel.org \
--cc=platform-driver-x86@vger.kernel.org \
--cc=w_armin@gmx.de \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.