Re: [PATCH] platform/x86: samsung-galaxybook: Add samsung-galaxybook driver

From: Guenter Roeck
Date: Mon Dec 09 2024 - 12:02:29 EST


On 12/9/24 08:37, Joshua Grisham wrote:
This patch will add a new driver for Samsung Galaxy Book series notebook
devices. This should hopefully include all suggestions from my original
mailing list feedback thread [1], as well as a first version for
associated updates to the documentation, Kconfig, Makefile, and the
MAINTAINERS file related to this new driver.

I have tested the driver both using m and y in the config, as well as
with various other options mentioned in the patch checklist of the
documentation.

Other users with other device IDs have also tested successfully using
a version of these same driver updates which I have maintained in a
separate branch of my GitHub repository [2].

I have made an attempt with the coding style to find a balance between what
is written in the kernel documentation and what actually exists in current
x86 platform drivers, but any feedback on this (or anything else) is
certainly welcome!

[1]: https://lore.kernel.org/platform-driver-x86/CAMF+KeYus9dW00WNJMLVxLLHdG9JgCfrGJ491fu7NM8GAEqqCg@xxxxxxxxxxxxxx/
[2]: https://github.com/joshuagrisham/samsung-galaxybook-extras/pull/44

Signed-off-by: Joshua Grisham <josh@xxxxxxxxxxxxxxxxx>
---
...
+/*
+ * Hwmon device
+ */
+
+#if IS_ENABLED(CONFIG_HWMON)
+static umode_t galaxybook_hwmon_is_visible(const void *drvdata, enum hwmon_sensor_types type,
+ u32 attr, int channel)
+{
+ switch (type) {
+ case hwmon_fan:
+ if (channel < galaxybook_ptr->fans_count &&
+ (attr == hwmon_fan_input || attr == hwmon_fan_label))
+ return 0444;
+ return 0;
+ default:
+ return 0;
+ }
+}
+
+static int galaxybook_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
+ u32 attr, int channel, long *val)
+{
+ unsigned int speed;
+
+ switch (type) {
+ case hwmon_fan:
+ if (channel < galaxybook_ptr->fans_count && attr == hwmon_fan_input) {

Why is this check necesary ? The is_visible function should have masked it out.

+ if (fan_speed_get(&galaxybook_ptr->fans[channel], &speed))
+ return -EIO;
+ *val = speed;
+ return 0;
+ }
+ return -EOPNOTSUPP;
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static int galaxybook_hwmon_read_string(struct device *dev, enum hwmon_sensor_types type,
+ u32 attr, int channel, const char **str)
+{
+ switch (type) {
+ case hwmon_fan:
+ if (channel < galaxybook_ptr->fans_count && attr == hwmon_fan_label) {
+ *str = galaxybook_ptr->fans[channel].description;
+ return 0;
+ }

Same as above.

Guenter