On 28.05.21 20:53 Pali Rohár wrote:
On Friday 28 May 2021 19:37:11 W_Armin@xxxxxx wrote:
From: Armin Wolf <W_Armin@xxxxxx>
Register a platform device for usage with
devm_hwmon_device_register_with_groups.
Also the platform device is necessary for
future changes.
This patch changes output in 'sensors' utility:
Without this patch it prints:
dell_smm-virtual-0
Adapter: Virtual device
And with patch it prints:
dell_smm-isa-0000
Adapter: ISA adapter
Interesting is that in this patch there is no reference to ISA and
neither to Virtual.
I think it needs to be related to symlinks in /sys/class/hwmon hierarchy
as this patch changes it:
(prior)
$ ll /sys/class/hwmon/hwmon1
lrwxrwxrwx 1 root root 0 máj 28 20:42 /sys/class/hwmon/hwmon1 -> ../../devices/virtual/hwmon/hwmon1/
(after)
$ ll /sys/class/hwmon/hwmon1
lrwxrwxrwx 1 root root 0 máj 28 20:42 /sys/class/hwmon/hwmon1 -> ../../devices/platform/dell_smm_hwmon/hwmon/hwmon1/
But I'm not sure what is correct to print in 'Adapter' section. Both
Virtual and ISA looks like bogus values for which I do not care.
Guenter, I will this part up to you, what you want to have in output.
Other comments are below.
Signed-off-by: Armin Wolf <W_Armin@xxxxxx>
---
drivers/hwmon/dell-smm-hwmon.c | 115 ++++++++++++++++++---------------
1 file changed, 63 insertions(+), 52 deletions(-)
diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index f2221ca0aa7b..2038f2a50e11 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -14,7 +14,9 @@
#include <linux/cpu.h>
#include <linux/delay.h>
+#include <linux/err.h>
#include <linux/module.h>
+#include <linux/platform_device.h>
#include <linux/types.h>
#include <linux/init.h>
#include <linux/proc_fs.h>
@@ -896,7 +898,7 @@ static const struct attribute_group i8k_group = {
};
__ATTRIBUTE_GROUPS(i8k);
-static int __init i8k_init_hwmon(void)
+static int __init dell_smm_init_hwmon(struct device *dev)
{
int err;
@@ -956,15 +958,9 @@ static int __init i8k_init_hwmon(void)
if (err >= 0)
i8k_hwmon_flags |= I8K_HWMON_HAVE_FAN3;
- i8k_hwmon_dev = hwmon_device_register_with_groups(NULL, "dell_smm",
- NULL, i8k_groups);
- if (IS_ERR(i8k_hwmon_dev)) {
- err = PTR_ERR(i8k_hwmon_dev);
- i8k_hwmon_dev = NULL;
- pr_err("hwmon registration failed (%d)\n", err);
- return err;
- }
- return 0;
+ i8k_hwmon_dev = devm_hwmon_device_register_with_groups(dev, "dell_smm", NULL, i8k_groups);
+
+ return PTR_ERR_OR_ZERO(i8k_hwmon_dev);
}
struct i8k_config_data {
@@ -1221,28 +1217,11 @@ static struct dmi_system_id i8k_whitelist_fan_control[] __initdata = {
{ }
};
-/*
- * Probe for the presence of a supported laptop.
- */
-static int __init i8k_probe(void)
+static int __init dell_smm_probe(struct platform_device *pdev)
{
const struct dmi_system_id *id, *fan_control;
int fan, ret;
- /*
- * Get DMI information
- */
- if (!dmi_check_system(i8k_dmi_table)) {
- if (!ignore_dmi && !force)
- return -ENODEV;
-
- pr_info("not running on a supported Dell system.\n");
- pr_info("vendor=%s, model=%s, version=%s\n",
- i8k_get_dmi_data(DMI_SYS_VENDOR),
- i8k_get_dmi_data(DMI_PRODUCT_NAME),
- i8k_get_dmi_data(DMI_BIOS_VERSION));
- }
-
if (dmi_check_system(i8k_blacklist_fan_support_dmi_table)) {
pr_warn("broken Dell BIOS detected, disallow fan support\n");
if (!force)
@@ -1255,21 +1234,11 @@ static int __init i8k_probe(void)
disallow_fan_type_call = true;
}
- strlcpy(bios_version, i8k_get_dmi_data(DMI_BIOS_VERSION),
+ strscpy(bios_version, i8k_get_dmi_data(DMI_BIOS_VERSION),
sizeof(bios_version));
- strlcpy(bios_machineid, i8k_get_dmi_data(DMI_PRODUCT_SERIAL),
+ strscpy(bios_machineid, i8k_get_dmi_data(DMI_PRODUCT_SERIAL),
sizeof(bios_machineid));
It is intentional to replace strlcpy by strscpy? If yes then I think it
should be part of other change as this replacement is not related nor
not needed by platform device registration.
It was because of a checkpatch warning, i will add that to the commit description
in the next version.
- /*
- * Get SMM Dell signature
- */
- if (i8k_get_dell_signature(I8K_SMM_GET_DELL_SIG1) &&
- i8k_get_dell_signature(I8K_SMM_GET_DELL_SIG2)) {
- pr_err("unable to get SMM Dell signature\n");
- if (!force)
- return -ENODEV;
- }
-
/*
* Set fan multiplier and maximal fan speed from dmi config
* Values specified in module parameters override values from dmi
@@ -1277,13 +1246,14 @@ static int __init i8k_probe(void)
id = dmi_first_match(i8k_dmi_table);
if (id && id->driver_data) {
const struct i8k_config_data *conf = id->driver_data;
+
if (!fan_mult && conf->fan_mult)
fan_mult = conf->fan_mult;
if (!fan_max && conf->fan_max)
fan_max = conf->fan_max;
}
- i8k_fan_max = fan_max ? : I8K_FAN_HIGH; /* Must not be 0 */
+ i8k_fan_max = fan_max ? : I8K_FAN_HIGH; /* Must not be 0 */
This is empty change?
Hm... now I see tab with 1 char size is replaced by space. Quite hard to
detect this change as it was not mentioned in commit message and such
tab is visible in most editors in the same way as as space.
My bad, i removed that empty change.
i8k_pwm_mult = DIV_ROUND_UP(255, i8k_fan_max);
fan_control = dmi_first_match(i8k_whitelist_fan_control);
@@ -1313,29 +1283,70 @@ static int __init i8k_probe(void)
i8k_fan_mult = fan_mult;
}
+ ret = dell_smm_init_hwmon(&pdev->dev);
+ if (ret)
+ return ret;
+
+ i8k_init_procfs();
+
return 0;
}
+static int dell_smm_remove(struct platform_device *pdev)
+{
+ i8k_exit_procfs();
+
+ return 0;
+}
+
+static struct platform_driver dell_smm_driver = {
+ .driver = {
+ .name = KBUILD_MODNAME,
+ },
+ .remove = dell_smm_remove,
+};
+
+static struct platform_device *dell_smm_device;
+
+/*
+ * Probe for the presence of a supported laptop.
+ */
static int __init i8k_init(void)
{
- int err;
+ /*
+ * Get DMI information
+ */
+ if (!dmi_check_system(i8k_dmi_table)) {
+ if (!ignore_dmi && !force)
+ return -ENODEV;
- /* Are we running on an supported laptop? */
- if (i8k_probe())
- return -ENODEV;
+ pr_info("not running on a supported Dell system.\n");
+ pr_info("vendor=%s, model=%s, version=%s\n",
+ i8k_get_dmi_data(DMI_SYS_VENDOR),
+ i8k_get_dmi_data(DMI_PRODUCT_NAME),
+ i8k_get_dmi_data(DMI_BIOS_VERSION));
+ }
- err = i8k_init_hwmon();
- if (err)
- return err;
+ /*
+ * Get SMM Dell signature
+ */
+ if (i8k_get_dell_signature(I8K_SMM_GET_DELL_SIG1) &&
+ i8k_get_dell_signature(I8K_SMM_GET_DELL_SIG2)) {
+ pr_err("unable to get SMM Dell signature\n");
+ if (!force)
+ return -ENODEV;
+ }
- i8k_init_procfs();
- return 0;
+ dell_smm_device = platform_create_bundle(&dell_smm_driver, dell_smm_probe, NULL, 0, NULL,
+ 0);
+
+ return PTR_ERR_OR_ZERO(dell_smm_device);
}
static void __exit i8k_exit(void)
{
- hwmon_device_unregister(i8k_hwmon_dev);
- i8k_exit_procfs();
+ platform_device_unregister(dell_smm_device);
+ platform_driver_unregister(&dell_smm_driver);
}
module_init(i8k_init);
--
2.20.1
