From: Guenter Roeck <linux@roeck-us.net>
To: "Joaquín Ignacio Aramendía" <samsagax@gmail.com>
Cc: pobrn@protonmail.com, hdegoede@redhat.com, jdelvare@suse.com,
linux-hwmon@vger.kernel.org, markgross@kernel.org,
platform-driver-x86@vger.kernel.org
Subject: Re: [PATCH v3] Add OneXPlayer mini AMD sensors driver
Date: Mon, 31 Oct 2022 08:34:50 -0700 [thread overview]
Message-ID: <20221031153450.GA2462187@roeck-us.net> (raw)
In-Reply-To: <20221031145308.341776-1-samsagax@gmail.com>
On Mon, Oct 31, 2022 at 11:53:08AM -0300, Joaquín Ignacio Aramendía wrote:
> Sensors driver for OXP Handhelds from One-Netbook that expose fan reading
> and control via hwmon sysfs.
>
> As far as I could gather all OXP boards have the same DMI strings and
> they are told appart by the boot cpu vendor (Intel/AMD).
> Currently only AMD boards are supported.
>
> Fan control is provided via pwm interface in the range [0-255]. AMD
> boards have [0-100] as range in the EC, the written value is scaled to
> accommodate for that.
>
> Signed-off-by: Joaquín Ignacio Aramendía <samsagax@gmail.com>
I didn't have time to look at your patch, but _pease_ stop sending new
versions of your patches in response to previous versions.
Guenter
> ---
> Removed fan_control reference in comment.
> Bugfix MIX/MIN reporting not available
> Bugfix pwm_enable register set wrong
> ---
> drivers/hwmon/Kconfig | 13 +-
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/oxp-sensors.c | 277 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 290 insertions(+), 1 deletion(-)
> create mode 100644 drivers/hwmon/oxp-sensors.c
>
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 7ac3daaf59ce..a1cdb03b4d13 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1607,6 +1607,17 @@ config SENSORS_NZXT_SMART2
>
> source "drivers/hwmon/occ/Kconfig"
>
> +config SENSORS_OXP
> + tristate "OneXPlayer EC fan control"
> + depends on ACPI
> + depends on X86
> + help
> + If you say yes here you get support for fan readings and control over
> + OneXPlayer handheld devices. Only OneXPlayer mini AMD handheld variant
> + boards are supported.
> +
> + Can also be built as a module. In that case it will be called oxp-sensors.
> +
> config SENSORS_PCF8591
> tristate "Philips PCF8591 ADC/DAC"
> depends on I2C
> @@ -1957,7 +1968,7 @@ config SENSORS_ADS7871
>
> config SENSORS_AMC6821
> tristate "Texas Instruments AMC6821"
> - depends on I2C
> + depends on I2C
> help
> If you say yes here you get support for the Texas Instruments
> AMC6821 hardware monitoring chips.
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 11d076cad8a2..35824f8be455 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -167,6 +167,7 @@ obj-$(CONFIG_SENSORS_NSA320) += nsa320-hwmon.o
> obj-$(CONFIG_SENSORS_NTC_THERMISTOR) += ntc_thermistor.o
> obj-$(CONFIG_SENSORS_NZXT_KRAKEN2) += nzxt-kraken2.o
> obj-$(CONFIG_SENSORS_NZXT_SMART2) += nzxt-smart2.o
> +obj-$(CONFIG_SENSORS_OXP) += oxp-sensors.o
> obj-$(CONFIG_SENSORS_PC87360) += pc87360.o
> obj-$(CONFIG_SENSORS_PC87427) += pc87427.o
> obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o
> diff --git a/drivers/hwmon/oxp-sensors.c b/drivers/hwmon/oxp-sensors.c
> new file mode 100644
> index 000000000000..f5895dc11094
> --- /dev/null
> +++ b/drivers/hwmon/oxp-sensors.c
> @@ -0,0 +1,277 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Platform driver for OXP Handhelds that expose fan reading and control
> + * via hwmon sysfs.
> + *
> + * All boards have the same DMI strings and they are told appart by the
> + * boot cpu vendor (Intel/AMD). Currently only AMD boards are supported
> + * but the code is made to be simple to add other handheld boards in the
> + * future.
> + * Fan control is provided via pwm interface in the range [0-255]. AMD
> + * boards use [0-100] as range in the EC, the written value is scaled to
> + * accommodate for that.
> + *
> + * Copyright (C) 2022 Joaquín I. Aramendía <samsagax@gmail.com>
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/dev_printk.h>
> +#include <linux/dmi.h>
> +#include <linux/hwmon.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/processor.h>
> +
> +#define ACPI_LOCK_DELAY_MS 500
> +
> +/* Handle ACPI lock mechanism */
> +struct lock_data {
> + u32 mutex;
> + bool (*lock)(struct lock_data *data);
> + bool (*unlock)(struct lock_data *data);
> +};
> +
> +static bool lock_global_acpi_lock(struct lock_data *data)
> +{
> + return ACPI_SUCCESS(acpi_acquire_global_lock(ACPI_LOCK_DELAY_MS,
> + &data->mutex));
> +}
> +
> +static bool unlock_global_acpi_lock(struct lock_data *data)
> +{
> + return ACPI_SUCCESS(acpi_release_global_lock(data->mutex));
> +}
> +
> +#define OXP_SENSOR_FAN_REG 0x76 /* Fan reading is 2 registers long */
> +#define OXP_SENSOR_PWM_ENABLE_REG 0x4A /* PWM enable is 1 register long */
> +#define OXP_SENSOR_PWM_REG 0x4B /* PWM reading is 1 register long */
> +
> +static const struct dmi_system_id dmi_table[] = {
> + {
> + .matches = {
> + DMI_EXACT_MATCH(DMI_BOARD_VENDOR,
> + "ONE-NETBOOK TECHNOLOGY CO., LTD."),
> + },
> + },
> + {
> + .matches = {
> + DMI_EXACT_MATCH(DMI_BOARD_VENDOR,
> + "ONE-NETBOOK"),
> + },
> + },
> + {},
> +};
> +
> +struct oxp_status {
> + struct lock_data lock_data;
> +};
> +
> +/* Helper functions to handle EC read/write */
> +static int read_from_ec(u8 reg, int size, long *val)
> +{
> + int i;
> + int ret;
> + u8 buffer;
> +
> + *val = 0;
> + for (i = 0; i < size; i++) {
> + ret = ec_read(reg + i, &buffer);
> + if (ret)
> + return ret;
> + (*val) <<= i * 8;
> + *val += buffer;
> + }
> + return ret;
> +}
> +
> +static int write_to_ec(const struct device *dev, u8 reg, u8 value)
> +{
> + struct oxp_status *state = dev_get_drvdata(dev);
> + int ret;
> +
> + if (!state->lock_data.lock(&state->lock_data)) {
> + dev_warn(dev, "Failed to acquire mutex");
> + return -EBUSY;
> + }
> +
> + ret = ec_write(reg, value);
> +
> + if (!state->lock_data.unlock(&state->lock_data))
> + dev_err(dev, "Failed to release mutex");
> +
> + return ret;
> +}
> +
> +static int oxp_pwm_enable(const struct device *dev)
> +{
> + return write_to_ec(dev, OXP_SENSOR_PWM_ENABLE_REG, 0x01);
> +}
> +
> +static int oxp_pwm_disable(const struct device *dev)
> +{
> + return write_to_ec(dev, OXP_SENSOR_PWM_ENABLE_REG, 0x00);
> +}
> +
> +/* Callbacks for hwmon interface */
> +static umode_t oxp_ec_hwmon_is_visible(const void *drvdata,
> + enum hwmon_sensor_types type, u32 attr, int channel)
> +{
> + switch (type) {
> + case hwmon_fan:
> + return 0444;
> + case hwmon_pwm:
> + return 0644;
> + default:
> + return 0;
> + }
> + return 0;
> +}
> +
> +static int oxp_platform_read(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long *val)
> +{
> + int ret;
> +
> + switch (type) {
> + case hwmon_fan:
> + switch (attr) {
> + case hwmon_fan_input:
> + return read_from_ec(OXP_SENSOR_FAN_REG,
> + 2,
> + val);
> + default:
> + dev_dbg(dev, "Unknown attribute for type %d: %d\n", type, attr);
> + return -EOPNOTSUPP;
> + }
> + case hwmon_pwm:
> + switch (attr) {
> + case hwmon_pwm_input:
> + ret = read_from_ec(OXP_SENSOR_PWM_REG,
> + 2, val);
> + *val = (*val * 255) / 100;
> + return ret;
> + case hwmon_pwm_enable:
> + return read_from_ec(OXP_SENSOR_PWM_ENABLE_REG, 1, val);
> + default:
> + dev_dbg(dev, "Unknown attribute for type %d: %d\n", type, attr);
> + return -EOPNOTSUPP;
> + }
> + default:
> + dev_dbg(dev, "Unknown sensor type %d.\n", type);
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static int oxp_platform_write(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long val)
> +{
> + switch (type) {
> + case hwmon_pwm:
> + switch (attr) {
> + case hwmon_pwm_enable:
> + if (val == 1)
> + return oxp_pwm_enable(dev);
> + else if (val == 0)
> + return oxp_pwm_disable(dev);
> + else
> + return -EINVAL;
> + case hwmon_pwm_input:
> + if (val < 0 || val > 255)
> + return -EINVAL;
> + val = (val * 100) / 255;
> + return write_to_ec(dev, OXP_SENSOR_PWM_REG, val);
> + default:
> + dev_dbg(dev, "Unknown attribute for type %d: %d", type, attr);
> + return -EOPNOTSUPP;
> + }
> + default:
> + dev_dbg(dev, "Unknown sensor type: %d", type);
> + return -EOPNOTSUPP;
> + }
> + return -EINVAL;
> +}
> +
> +/* Known sensors in the OXP EC controllers */
> +static const struct hwmon_channel_info *oxp_platform_sensors[] = {
> + HWMON_CHANNEL_INFO(fan,
> + HWMON_F_INPUT),
> + HWMON_CHANNEL_INFO(pwm,
> + HWMON_PWM_INPUT | HWMON_PWM_ENABLE),
> + NULL,
> +};
> +
> +static const struct hwmon_ops oxp_ec_hwmon_ops = {
> + .is_visible = oxp_ec_hwmon_is_visible,
> + .read = oxp_platform_read,
> + .write = oxp_platform_write,
> +};
> +
> +static const struct hwmon_chip_info oxp_ec_chip_info = {
> + .ops = &oxp_ec_hwmon_ops,
> + .info = oxp_platform_sensors,
> +};
> +
> +/* Initialization logic */
> +static int oxp_platform_probe(struct platform_device *pdev)
> +{
> + const struct dmi_system_id *dmi_entry;
> + struct device *dev = &pdev->dev;
> + struct device *hwdev;
> + struct oxp_status *state;
> +
> + /* Have to check for AMD processor here */
> + dmi_entry = dmi_first_match(dmi_table);
> + if (!dmi_entry || boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
> + return -ENODEV;
> +
> + state = devm_kzalloc(dev, sizeof(struct oxp_status), GFP_KERNEL);
> + if (!state)
> + return -ENOMEM;
> +
> + state->lock_data.mutex = 0;
> + state->lock_data.lock = lock_global_acpi_lock;
> + state->lock_data.unlock = unlock_global_acpi_lock;
> +
> + hwdev = devm_hwmon_device_register_with_info(dev, "oxpec", state,
> + &oxp_ec_chip_info, NULL);
> +
> + return PTR_ERR_OR_ZERO(hwdev);
> +}
> +
> +static struct platform_driver oxp_platform_driver = {
> + .driver = {
> + .name = "oxp-platform",
> + },
> + .probe = oxp_platform_probe,
> +};
> +
> +static struct platform_device *oxp_platform_device;
> +
> +static int __init oxp_platform_init(void)
> +{
> + oxp_platform_device =
> + platform_create_bundle(&oxp_platform_driver,
> + oxp_platform_probe, NULL, 0, NULL, 0);
> +
> + if (IS_ERR(oxp_platform_device))
> + return PTR_ERR(oxp_platform_device);
> +
> + return 0;
> +}
> +
> +static void __exit oxp_platform_exit(void)
> +{
> + platform_device_unregister(oxp_platform_device);
> + platform_driver_unregister(&oxp_platform_driver);
> +}
> +
> +MODULE_DEVICE_TABLE(dmi, dmi_table);
> +module_init(oxp_platform_init);
> +module_exit(oxp_platform_exit);
> +
> +MODULE_AUTHOR("Joaquín Ignacio Aramendía <samsagax@gmail.com>");
> +MODULE_DESCRIPTION(
> + "Platform driver that handles ACPI EC of OneXPlayer devices");
> +MODULE_LICENSE("GPL");
> --
> 2.38.1
>
next prev parent reply other threads:[~2022-10-31 15:34 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-29 22:50 [PATCH] Add OneXPlayer mini AMD board driver Joaquín Ignacio Aramendía
2022-10-29 23:30 ` Guenter Roeck
2022-10-30 0:29 ` Joaquin Aramendia
2022-10-30 3:24 ` Guenter Roeck
2022-10-30 20:32 ` [PATCH v2] Add OneXPlayer mini AMD sensors driver Joaquín Ignacio Aramendía
2022-10-31 0:03 ` Barnabás Pőcze
2022-10-31 14:49 ` Joaquin Aramendia
2022-10-31 14:53 ` [PATCH v3] " Joaquín Ignacio Aramendía
2022-10-31 15:34 ` Guenter Roeck [this message]
2022-10-31 15:57 ` Joaquin Aramendia
2022-10-31 16:43 ` Limonciello, Mario
2022-10-31 18:57 ` Joaquin Aramendia
2022-10-31 19:21 ` Limonciello, Mario
2022-10-31 19:33 ` Joaquin Aramendia
2022-10-31 19:56 ` Guenter Roeck
2022-10-31 20:53 ` Joaquin Aramendia
2022-10-31 21:30 ` Guenter Roeck
2022-10-31 11:45 ` [PATCH] Add OneXPlayer mini AMD board driver Joaquin Aramendia
2022-10-31 13:07 ` Guenter Roeck
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=20221031153450.GA2462187@roeck-us.net \
--to=linux@roeck-us.net \
--cc=hdegoede@redhat.com \
--cc=jdelvare@suse.com \
--cc=linux-hwmon@vger.kernel.org \
--cc=markgross@kernel.org \
--cc=platform-driver-x86@vger.kernel.org \
--cc=pobrn@protonmail.com \
--cc=samsagax@gmail.com \
/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;
as well as URLs for NNTP newsgroup(s).