[v4] Add OneXPlayer mini AMD sensors driver

Commit Message

Joaquín Ignacio Aramendía Nov. 2, 2022, 3:04 p.m. UTC

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 <[e-mail]>
---
Added Documentation entry
Added MAINTAINERS entry
Removed unnecessary code complexity
Fixed all checkpatch --strict checks
---
 Documentation/hwmon/index.rst       |   1 +
 Documentation/hwmon/oxp-sensors.rst |  24 +++
 MAINTAINERS                         |   6 +
 drivers/hwmon/Kconfig               |  11 ++
 drivers/hwmon/Makefile              |   1 +
 drivers/hwmon/oxp-sensors.c         | 254 ++++++++++++++++++++++++++++
 6 files changed, 297 insertions(+)
 create mode 100644 Documentation/hwmon/oxp-sensors.rst
 create mode 100644 drivers/hwmon/oxp-sensors.c

--
2.38.1

Comments

Guenter Roeck Nov. 2, 2022, 6:04 p.m. UTC | #1

On Wed, Nov 02, 2022 at 12:04:40PM -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 <[e-mail]>
> ---
> Added Documentation entry
> Added MAINTAINERS entry
> Removed unnecessary code complexity
> Fixed all checkpatch --strict checks
> ---
>  Documentation/hwmon/index.rst       |   1 +
>  Documentation/hwmon/oxp-sensors.rst |  24 +++
>  MAINTAINERS                         |   6 +
>  drivers/hwmon/Kconfig               |  11 ++
>  drivers/hwmon/Makefile              |   1 +
>  drivers/hwmon/oxp-sensors.c         | 254 ++++++++++++++++++++++++++++
>  6 files changed, 297 insertions(+)
>  create mode 100644 Documentation/hwmon/oxp-sensors.rst
>  create mode 100644 drivers/hwmon/oxp-sensors.c
> 
> diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
> index c1d11cf13eef..098986bfbfdd 100644
> --- a/Documentation/hwmon/index.rst
> +++ b/Documentation/hwmon/index.rst
> @@ -160,6 +160,7 @@ Hardware Monitoring Kernel Drivers
>     nzxt-kraken2
>     nzxt-smart2
>     occ
> +   oxp-sensors
>     pc87360
>     pc87427
>     pcf8591
> diff --git a/Documentation/hwmon/oxp-sensors.rst b/Documentation/hwmon/oxp-sensors.rst
> new file mode 100644
> index 000000000000..023441d17a45
> --- /dev/null
> +++ b/Documentation/hwmon/oxp-sensors.rst
> @@ -0,0 +1,24 @@
> +.. SPDX-License-Identifier: GPL-2.0-or-later
> +
> +Kernel driver oxp-sensors
> +=========================
> +
> +Authors:

s/Authors/Author/

> +    - Joaquín Ignacio Aramendía <[e-mail]>
> +
> +Description:
> +------------
> +
> +One X Player devices from One Netbook provide fan readings and fan control
> +through its Embedded Controller.
> +
> +Currently only supports AMD boards from the One X Player lineup. Intel boards
> +could be supported if we could figure out the EC registers and values to write
> +to since the EC layout and model is different.
> +
> +Sensor values are read and written from EC registers, and to avoid race with
> +the board firmware the driver acquires ACPI mutex.
> +
> +Fan control is provided via pwm sysfs attribute in the range [0-255]. AMD
> +boards use [0-100] as range values in the EC, the value is scaled to
> +accommodate for that.

Drop the last two paragraphs; those are implementation details
which belong into the driver.

List the supported sysfs attributes instead (without the scaling detail).
See other driver documentation for examples.

> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2034e7d26684..c2e24a830875 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15337,6 +15337,12 @@ S:	Maintained
>  F:	drivers/mtd/nand/onenand/
>  F:	include/linux/mtd/onenand*.h
> 
> +ONEXPLAYER FAN DRIVER
> +M:	Joaquín Ignacio Aramendía <[e-mail]>
> +L:	[e-mail]
> +S:	Maintained
> +F:	drivers/hwmon/oxp-sensors.c
> +
>  ONION OMEGA2+ BOARD
>  M:	Harvey Hunt <[e-mail]>
>  L:	[e-mail]
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 7ac3daaf59ce..993ffa26e44f 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
> 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..00b3aacfb017
> --- /dev/null
> +++ b/drivers/hwmon/oxp-sensors.c
> @@ -0,0 +1,254 @@
> +// 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 <[e-mail]>
> + */
> +
> +#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>
> +
> +/* Handle ACPI lock mechanism */
> +static u32 oxp_mutex;
> +
> +#define ACPI_LOCK_DELAY_MS	500
> +
> +static bool lock_global_acpi_lock(void)
> +{
> +	return ACPI_SUCCESS(acpi_acquire_global_lock(ACPI_LOCK_DELAY_MS, &oxp_mutex));
> +}
> +
> +static bool unlock_global_acpi_lock(void)
> +{
> +	return ACPI_SUCCESS(acpi_release_global_lock(oxp_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_MATCH(DMI_BOARD_VENDOR, "ONE-NETBOOK"),
> +			DMI_EXACT_MATCH(DMI_BOARD_NAME, "ONE XPLAYER"),
> +		},
> +	},
> +	{},
> +};
> +
> +/* Helper functions to handle EC read/write */
> +static int read_from_ec(u8 reg, int size, long *val)
> +{
> +	int i;
> +	int ret;
> +	u8 buffer;
> +
> +	if (!lock_global_acpi_lock())
> +		return -EBUSY;
> +
> +	*val = 0;
> +	for (i = 0; i < size; i++) {
> +		ret = ec_read(reg + i, &buffer);
> +		if (ret)
> +			return ret;
> +		*val <<= i * 8;
> +		*val += buffer;
> +	}
> +
> +	if (!unlock_global_acpi_lock())
> +		return -EBUSY;
> +
> +	return 0;
> +}
> +
> +static int write_to_ec(const struct device *dev, u8 reg, u8 value)
> +{
> +	int ret;
> +
> +	if (!lock_global_acpi_lock())
> +		return -EBUSY;
> +
> +	ret = ec_write(reg, value);
> +
> +	if (!unlock_global_acpi_lock())
> +		return -EBUSY;
> +
> +	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;

I think you can drop the above since all branches of the switch statement
return.

> +}
> +
> +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:

Missing break;

> +		}
> +		break;
> +	case hwmon_pwm:
> +		switch (attr) {
> +		case hwmon_pwm_input:
> +			ret = read_from_ec(OXP_SENSOR_PWM_REG, 2, val);
> +			*val = (*val * 255) / 100;

That isn't really clean since it will overwrite *val after an error.

  			if (ret)
				return ret;
			*val = (*val * 255) / 100;
			return 0;

would be more appropriate.

> +			return ret;
> +		case hwmon_pwm_enable:
> +			return read_from_ec(OXP_SENSOR_PWM_ENABLE_REG, 1, val);
> +		default:


Missing break;

> +		}
> +		break;
> +	default:

Missing break;

> +	}
> +	dev_dbg(dev, "Unknown attribute for type %d: %d\n", type, attr);

That isn't really useful since it will never happen.

> +	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;

else after return is unnecessary.

> +		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:
> +			break;
> +		}

		break;

> +	default:

		break;

> +	}
> +	dev_dbg(dev, "Unknown sensor type: %d", type);

Same as above. It just increases code size for no good reason.

> +	return -EOPNOTSUPP;
> +}
> +
> +/* 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;
> +
> +	/* Have to check for AMD processor here because DMI strings are the
> +	 * same between Intel and AMD boards, the only way to tell them appart
> +	 * is the CPU.
> +	 * Intel boards seem to have different EC registers and values to
> +	 * read/write.
> +	 */

/*
 * Please use standard multi-line comments
 */

> +	dmi_entry = dmi_first_match(dmi_table);
> +	if (!dmi_entry || boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
> +		return -ENODEV;
> +
> +	hwdev = devm_hwmon_device_register_with_info(dev, "oxpec", NULL,
> +						     &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);

Why not PTR_ERR_OR_ZERO() here ?

> +
> +	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);

Can you use module_platform_driver() instead ?

> +
> +MODULE_AUTHOR("Joaquín Ignacio Aramendía <[e-mail]>");
> +MODULE_DESCRIPTION("Platform driver that handles EC sensors of OneXPlayer devices");
> +MODULE_LICENSE("GPL");
> --
> 2.38.1
>

Joaquín Ignacio Aramendía Nov. 2, 2022, 8:11 p.m. UTC | #2

El mié, 2 nov 2022 a la(s) 15:04, Guenter Roeck ([e-mail]) escribió:
>
> On Wed, Nov 02, 2022 at 12:04:40PM -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 <[e-mail]>
> > ---
> > Added Documentation entry
> > Added MAINTAINERS entry
> > Removed unnecessary code complexity
> > Fixed all checkpatch --strict checks
> > ---
> >  Documentation/hwmon/index.rst       |   1 +
> >  Documentation/hwmon/oxp-sensors.rst |  24 +++
> >  MAINTAINERS                         |   6 +
> >  drivers/hwmon/Kconfig               |  11 ++
> >  drivers/hwmon/Makefile              |   1 +
> >  drivers/hwmon/oxp-sensors.c         | 254 ++++++++++++++++++++++++++++
> >  6 files changed, 297 insertions(+)
> >  create mode 100644 Documentation/hwmon/oxp-sensors.rst
> >  create mode 100644 drivers/hwmon/oxp-sensors.c
> >
> > diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
> > index c1d11cf13eef..098986bfbfdd 100644
> > --- a/Documentation/hwmon/index.rst
> > +++ b/Documentation/hwmon/index.rst
> > @@ -160,6 +160,7 @@ Hardware Monitoring Kernel Drivers
> >     nzxt-kraken2
> >     nzxt-smart2
> >     occ
> > +   oxp-sensors
> >     pc87360
> >     pc87427
> >     pcf8591
> > diff --git a/Documentation/hwmon/oxp-sensors.rst b/Documentation/hwmon/oxp-sensors.rst
> > new file mode 100644
> > index 000000000000..023441d17a45
> > --- /dev/null
> > +++ b/Documentation/hwmon/oxp-sensors.rst
> > @@ -0,0 +1,24 @@
> > +.. SPDX-License-Identifier: GPL-2.0-or-later
> > +
> > +Kernel driver oxp-sensors
> > +=========================
> > +
> > +Authors:
>
> s/Authors/Author/

Done

> > +    - Joaquín Ignacio Aramendía <[e-mail]>
> > +
> > +Description:
> > +------------
> > +
> > +One X Player devices from One Netbook provide fan readings and fan control
> > +through its Embedded Controller.
> > +
> > +Currently only supports AMD boards from the One X Player lineup. Intel boards
> > +could be supported if we could figure out the EC registers and values to write
> > +to since the EC layout and model is different.
> > +
> > +Sensor values are read and written from EC registers, and to avoid race with
> > +the board firmware the driver acquires ACPI mutex.
> > +
> > +Fan control is provided via pwm sysfs attribute in the range [0-255]. AMD
> > +boards use [0-100] as range values in the EC, the value is scaled to
> > +accommodate for that.
>
> Drop the last two paragraphs; those are implementation details
> which belong into the driver.
>
> List the supported sysfs attributes instead (without the scaling detail).
> See other driver documentation for examples.

Done

> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 2034e7d26684..c2e24a830875 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -15337,6 +15337,12 @@ S:   Maintained
> >  F:   drivers/mtd/nand/onenand/
> >  F:   include/linux/mtd/onenand*.h
> >
> > +ONEXPLAYER FAN DRIVER
> > +M:   Joaquín Ignacio Aramendía <[e-mail]>
> > +L:   [e-mail]
> > +S:   Maintained
> > +F:   drivers/hwmon/oxp-sensors.c
> > +
> >  ONION OMEGA2+ BOARD
> >  M:   Harvey Hunt <[e-mail]>
> >  L:   [e-mail]
> > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > index 7ac3daaf59ce..993ffa26e44f 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
> > 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..00b3aacfb017
> > --- /dev/null
> > +++ b/drivers/hwmon/oxp-sensors.c
> > @@ -0,0 +1,254 @@
> > +// 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 <[e-mail]>
> > + */
> > +
> > +#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>
> > +
> > +/* Handle ACPI lock mechanism */
> > +static u32 oxp_mutex;
> > +
> > +#define ACPI_LOCK_DELAY_MS   500
> > +
> > +static bool lock_global_acpi_lock(void)
> > +{
> > +     return ACPI_SUCCESS(acpi_acquire_global_lock(ACPI_LOCK_DELAY_MS, &oxp_mutex));
> > +}
> > +
> > +static bool unlock_global_acpi_lock(void)
> > +{
> > +     return ACPI_SUCCESS(acpi_release_global_lock(oxp_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_MATCH(DMI_BOARD_VENDOR, "ONE-NETBOOK"),
> > +                     DMI_EXACT_MATCH(DMI_BOARD_NAME, "ONE XPLAYER"),
> > +             },
> > +     },
> > +     {},
> > +};
> > +
> > +/* Helper functions to handle EC read/write */
> > +static int read_from_ec(u8 reg, int size, long *val)
> > +{
> > +     int i;
> > +     int ret;
> > +     u8 buffer;
> > +
> > +     if (!lock_global_acpi_lock())
> > +             return -EBUSY;
> > +
> > +     *val = 0;
> > +     for (i = 0; i < size; i++) {
> > +             ret = ec_read(reg + i, &buffer);
> > +             if (ret)
> > +                     return ret;
> > +             *val <<= i * 8;
> > +             *val += buffer;
> > +     }
> > +
> > +     if (!unlock_global_acpi_lock())
> > +             return -EBUSY;
> > +
> > +     return 0;
> > +}
> > +
> > +static int write_to_ec(const struct device *dev, u8 reg, u8 value)
> > +{
> > +     int ret;
> > +
> > +     if (!lock_global_acpi_lock())
> > +             return -EBUSY;
> > +
> > +     ret = ec_write(reg, value);
> > +
> > +     if (!unlock_global_acpi_lock())
> > +             return -EBUSY;
> > +
> > +     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;
>
> I think you can drop the above since all branches of the switch statement
> return.

Ok. Done.

> > +}
> > +
> > +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:
>
> Missing break;

Oops. Is not really needed but I'll add them.

> > +             }
> > +             break;
> > +     case hwmon_pwm:
> > +             switch (attr) {
> > +             case hwmon_pwm_input:
> > +                     ret = read_from_ec(OXP_SENSOR_PWM_REG, 2, val);
> > +                     *val = (*val * 255) / 100;
>
> That isn't really clean since it will overwrite *val after an error.
>
>                         if (ret)
>                                 return ret;
>                         *val = (*val * 255) / 100;
>                         return 0;
>
> would be more appropriate.

Much better, thanks for suggestion.

> > +                     return ret;
> > +             case hwmon_pwm_enable:
> > +                     return read_from_ec(OXP_SENSOR_PWM_ENABLE_REG, 1, val);
> > +             default:
>
>
> Missing break;

Added

> > +             }
> > +             break;
> > +     default:
>
> Missing break;
>
> > +     }
> > +     dev_dbg(dev, "Unknown attribute for type %d: %d\n", type, attr);
>
> That isn't really useful since it will never happen.

Dropped the message.

> > +     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;
>
> else after return is unnecessary.

Removed

> > +             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:
> > +                     break;
> > +             }
>
>                 break;

Added

> > +     default:
>
>                 break;

Added

> > +     }
> > +     dev_dbg(dev, "Unknown sensor type: %d", type);
>
> Same as above. It just increases code size for no good reason.

Removed the message entirely.

> > +     return -EOPNOTSUPP;
> > +}
> > +
> > +/* 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;
> > +
> > +     /* Have to check for AMD processor here because DMI strings are the
> > +      * same between Intel and AMD boards, the only way to tell them appart
> > +      * is the CPU.
> > +      * Intel boards seem to have different EC registers and values to
> > +      * read/write.
> > +      */
>
> /*
>  * Please use standard multi-line comments
>  */

Fixed

> > +     dmi_entry = dmi_first_match(dmi_table);
> > +     if (!dmi_entry || boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
> > +             return -ENODEV;
> > +
> > +     hwdev = devm_hwmon_device_register_with_info(dev, "oxpec", NULL,
> > +                                                  &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);
>
> Why not PTR_ERR_OR_ZERO() here ?

This function can be dropped if using module_platform_driver()
Removed entirely.

> > +
> > +     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);
>
> Can you use module_platform_driver() instead ?

The original module had this. I'll bring it back and drop init/exit functions.

> > +
> > +MODULE_AUTHOR("Joaquín Ignacio Aramendía <[e-mail]>");
> > +MODULE_DESCRIPTION("Platform driver that handles EC sensors of OneXPlayer devices");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.38.1
> >

Thanks for your time and feedback!

Guenter Roeck Nov. 2, 2022, 8:44 p.m. UTC | #3

On Wed, Nov 02, 2022 at 05:11:00PM -0300, Joaquin Aramendia wrote:
[ ... ]
> > > +
> > > +     switch (type) {
> > > +     case hwmon_fan:
> > > +             switch (attr) {
> > > +             case hwmon_fan_input:
> > > +                     return read_from_ec(OXP_SENSOR_FAN_REG, 2, val);
> > > +             default:
> >
> > Missing break;
> 
> Oops. Is not really needed but I'll add them.
> 
Technically you are correct, but we would have static analyzers scream at
us, and it is against kernel coding style. There is a practical reason
for that: Missing break statements are often the result of coding errors.

Guenter

Joaquín Ignacio Aramendía Nov. 2, 2022, 9:10 p.m. UTC | #4

> >
> > Oops. Is not really needed but I'll add them.
> >
> Technically you are correct, but we would have static analyzers scream at
> us, and it is against kernel coding style. There is a practical reason
> for that: Missing break statements are often the result of coding errors.
>
> Guenter

Great, thanks for the clarification.

As for my last statement, module_platform_driver() usage broke the
module, don't know why, but seems like the probe function is not run?
If you are ok with it, I'll revert to module_init/module_exit macros.

Guenter Roeck Nov. 3, 2022, 1:15 a.m. UTC | #5

On Wed, Nov 02, 2022 at 06:10:37PM -0300, Joaquin Aramendia wrote:
> > >
> > > Oops. Is not really needed but I'll add them.
> > >
> > Technically you are correct, but we would have static analyzers scream at
> > us, and it is against kernel coding style. There is a practical reason
> > for that: Missing break statements are often the result of coding errors.
> >
> > Guenter
> 
> Great, thanks for the clarification.
> 
> As for my last statement, module_platform_driver() usage broke the
> module, don't know why, but seems like the probe function is not run?
> If you are ok with it, I'll revert to module_init/module_exit macros.
> 
Go ahead, just add a comment to the driver. Mabe someone manages
to sort it out.

Thanks,
Guenter

Joaquín Ignacio Aramendía Nov. 4, 2022, 12:48 p.m. UTC | #6

El mié, 2 nov 2022 a la(s) 22:15, Guenter Roeck ([e-mail]) escribió:
>
> On Wed, Nov 02, 2022 at 06:10:37PM -0300, Joaquin Aramendia wrote:
> > > >
> > > > Oops. Is not really needed but I'll add them.
> > > >
> > > Technically you are correct, but we would have static analyzers scream at
> > > us, and it is against kernel coding style. There is a practical reason
> > > for that: Missing break statements are often the result of coding errors.
> > >
> > > Guenter
> >
> > Great, thanks for the clarification.
> >
> > As for my last statement, module_platform_driver() usage broke the
> > module, don't know why, but seems like the probe function is not run?
> > If you are ok with it, I'll revert to module_init/module_exit macros.
> >
> Go ahead, just add a comment to the driver. Mabe someone manages
> to sort it out.
>
> Thanks,
> Guenter

Will add that comment, rebase and submit again the patch.
Thanks for your time, Guenter. And sorry If I got on your nerves :)

Joaquín Ignacio Aramendía Nov. 7, 2022, 2:24 p.m. UTC | #7

Guenter, thanks so much for your feedback. Just posted the final v6
with your suggestions. Let me know If anything needs to be changed or
clarified.

Cheers

Patch

diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index c1d11cf13eef..098986bfbfdd 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -160,6 +160,7 @@  Hardware Monitoring Kernel Drivers
    nzxt-kraken2
    nzxt-smart2
    occ
+   oxp-sensors
    pc87360
    pc87427
    pcf8591
diff --git a/Documentation/hwmon/oxp-sensors.rst b/Documentation/hwmon/oxp-sensors.rst
new file mode 100644
index 000000000000..023441d17a45
--- /dev/null
+++ b/Documentation/hwmon/oxp-sensors.rst
@@ -0,0 +1,24 @@ 
+.. SPDX-License-Identifier: GPL-2.0-or-later
+
+Kernel driver oxp-sensors
+=========================
+
+Authors:
+    - Joaquín Ignacio Aramendía <[e-mail]>
+
+Description:
+------------
+
+One X Player devices from One Netbook provide fan readings and fan control
+through its Embedded Controller.
+
+Currently only supports AMD boards from the One X Player lineup. Intel boards
+could be supported if we could figure out the EC registers and values to write
+to since the EC layout and model is different.
+
+Sensor values are read and written from EC registers, and to avoid race with
+the board firmware the driver acquires ACPI mutex.
+
+Fan control is provided via pwm sysfs attribute in the range [0-255]. AMD
+boards use [0-100] as range values in the EC, the value is scaled to
+accommodate for that.
diff --git a/MAINTAINERS b/MAINTAINERS
index 2034e7d26684..c2e24a830875 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15337,6 +15337,12 @@  S:	Maintained
 F:	drivers/mtd/nand/onenand/
 F:	include/linux/mtd/onenand*.h

+ONEXPLAYER FAN DRIVER
+M:	Joaquín Ignacio Aramendía <[e-mail]>
+L:	[e-mail]
+S:	Maintained
+F:	drivers/hwmon/oxp-sensors.c
+
 ONION OMEGA2+ BOARD
 M:	Harvey Hunt <[e-mail]>
 L:	[e-mail]
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 7ac3daaf59ce..993ffa26e44f 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
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..00b3aacfb017
--- /dev/null
+++ b/drivers/hwmon/oxp-sensors.c
@@ -0,0 +1,254 @@ 
+// 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 <[e-mail]>
+ */
+
+#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>
+
+/* Handle ACPI lock mechanism */
+static u32 oxp_mutex;
+
+#define ACPI_LOCK_DELAY_MS	500
+
+static bool lock_global_acpi_lock(void)
+{
+	return ACPI_SUCCESS(acpi_acquire_global_lock(ACPI_LOCK_DELAY_MS, &oxp_mutex));
+}
+
+static bool unlock_global_acpi_lock(void)
+{
+	return ACPI_SUCCESS(acpi_release_global_lock(oxp_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_MATCH(DMI_BOARD_VENDOR, "ONE-NETBOOK"),
+			DMI_EXACT_MATCH(DMI_BOARD_NAME, "ONE XPLAYER"),
+		},
+	},
+	{},
+};
+
+/* Helper functions to handle EC read/write */
+static int read_from_ec(u8 reg, int size, long *val)
+{
+	int i;
+	int ret;
+	u8 buffer;
+
+	if (!lock_global_acpi_lock())
+		return -EBUSY;
+
+	*val = 0;
+	for (i = 0; i < size; i++) {
+		ret = ec_read(reg + i, &buffer);
+		if (ret)
+			return ret;
+		*val <<= i * 8;
+		*val += buffer;
+	}
+
+	if (!unlock_global_acpi_lock())
+		return -EBUSY;
+
+	return 0;
+}
+
+static int write_to_ec(const struct device *dev, u8 reg, u8 value)
+{
+	int ret;
+
+	if (!lock_global_acpi_lock())
+		return -EBUSY;
+
+	ret = ec_write(reg, value);
+
+	if (!unlock_global_acpi_lock())
+		return -EBUSY;
+
+	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:
+		}
+		break;
+	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:
+		}
+		break;
+	default:
+	}
+	dev_dbg(dev, "Unknown attribute for type %d: %d\n", type, attr);
+	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:
+			break;
+		}
+	default:
+	}
+	dev_dbg(dev, "Unknown sensor type: %d", type);
+	return -EOPNOTSUPP;
+}
+
+/* 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;
+
+	/* Have to check for AMD processor here because DMI strings are the
+	 * same between Intel and AMD boards, the only way to tell them appart
+	 * is the CPU.
+	 * Intel boards seem to have different EC registers and values to
+	 * read/write.
+	 */
+	dmi_entry = dmi_first_match(dmi_table);
+	if (!dmi_entry || boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
+		return -ENODEV;
+
+	hwdev = devm_hwmon_device_register_with_info(dev, "oxpec", NULL,
+						     &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 <[e-mail]>");
+MODULE_DESCRIPTION("Platform driver that handles EC sensors of OneXPlayer devices");
+MODULE_LICENSE("GPL");