[PATCH 0/1] [x86] BIOS SAR Driver for M.2 Intel Modems

[PATCH 0/1] [x86] BIOS SAR Driver for M.2 Intel Modems

[Shravan S]

SAR (Specific Absorption Rate) driver is a host driver implemented for Linux
or chrome platform to limit the exposure of human body to RF frequency by informing
the Intel M.2 modem to regulate the RF power based on SAR data obtained from the sensors
captured in the BIOS. ACPI interface exposes this data from the BIOS to SAR driver. The
front end application in userspace ( eg: Modem Manager) will interact with SAR driver 
to obtain information like the device mode (Example: tablets, laptops, etx), Antenna index,
baseband index, SAR table index and use available communication like MBIM interface to enable
data communication to modem for RF power regulation.

The BIOS gets notified about device mode changes through Sensor Driver. This information is 
given to a (newly created) WWAN ACPI function driver when there is a device mode change. 
The driver then uses a _DSM method to retrieve the required information from BIOS. 
This information is then forwarded/multicast to the User-space using the NETLINK interface.
A lookup table is maintained inside the BIOS which suggests the SAR Table index and Antenna 
Tuner Table Index values for individual device modes.

The SAR parameters to be used on the Modem differs for each Regulatory Mode like FCC, CE and ISED.
Hence, the SAR parameters are stored separately in the SMBIOS table in the OEM BIOS, 
for each of the Regulatory mode. Regulatory modes will be different based on the region and network
available in that region.

Hence the entire SAR functionality handling is divided into 2 parts:

•	A ACPI function driver implemented that uses a dedicated ACPI node for WWAN device. 
    sends notifications whenever there is change in Device Mode. (each OEM has different mechanism
    of updating this DEVICE Mode info). This is notified to User-space applications using 
    the RT-NETLINK interface.
•	WWAN Device Service listens for RT-NETLINK messages and routes them to Modem using MBIM. 

Shravan S (1):
  [x86]: BIOS Dynamic SAR driver for Intel M.2 Modem

 drivers/platform/x86/Kconfig                |  12 +
 drivers/platform/x86/Makefile               |   1 +
 drivers/platform/x86/intel-sar.c            | 589 ++++++++++++++++++++
 include/linux/platform_data/x86/intel-sar.h | 118 ++++
 4 files changed, 720 insertions(+)
 create mode 100644 drivers/platform/x86/intel-sar.c
 create mode 100644 include/linux/platform_data/x86/intel-sar.h


base-commit: cd1245d75ce93b8fd206f4b34eb58bcfe156d5e9
-- 
2.17.1

[PATCH 1/1] [x86]: BIOS Dynamic SAR driver for Intel M.2 Modem

[Shravan S]

---
 drivers/platform/x86/Kconfig                |  12 +
 drivers/platform/x86/Makefile               |   1 +
 drivers/platform/x86/intel-sar.c            | 589 ++++++++++++++++++++
 include/linux/platform_data/x86/intel-sar.h | 118 ++++
 4 files changed, 720 insertions(+)
 create mode 100644 drivers/platform/x86/intel-sar.c
 create mode 100644 include/linux/platform_data/x86/intel-sar.h

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 60592fb88e7a..6dfa89310677 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -1313,6 +1313,18 @@ config INTEL_TELEMETRY
 	  directly via debugfs files. Various tools may use
 	  this interface for SoC state monitoring.
 
+config INTEL_SAR
+	tristate "Intel Specific Absorption Rate Driver"
+	depends on ACPI
+	help
+	  This driver limit the exposure of human body to RF frequency by informing
+	  the Intel M.2 modem to regulate the RF power based on SAR data obtained
+	  from the sensorscaptured in the BIOS. ACPI interface exposes this data
+	  from the BIOS to SAR driver. The front end application in userspace
+	  will interact with SAR driver to obtain information like the device mode,
+	  Antenna index,baseband index, SAR table index and use available communication
+	  like MBIM interface to enable data communication to modem for RF power regulation.
+
 endif # X86_PLATFORM_DEVICES
 
 config PMC_ATOM
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index dcc8cdb95b4d..548ff663c4af 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -123,6 +123,7 @@ obj-$(CONFIG_INTEL_SMARTCONNECT)		+= intel-smartconnect.o
 obj-$(CONFIG_INTEL_SPEED_SELECT_INTERFACE)	+= intel_speed_select_if/
 obj-$(CONFIG_INTEL_TURBO_MAX_3)			+= intel_turbo_max_3.o
 obj-$(CONFIG_INTEL_UNCORE_FREQ_CONTROL)		+= intel-uncore-frequency.o
+obj-$(CONFIG_INTEL_SAR)				+= intel-sar.o
 
 # Intel PMIC / PMC / P-Unit devices
 obj-$(CONFIG_INTEL_BXTWC_PMIC_TMU)	+= intel_bxtwc_tmu.o
diff --git a/drivers/platform/x86/intel-sar.c b/drivers/platform/x86/intel-sar.c
new file mode 100644
index 000000000000..dd3056b11e53
--- /dev/null
+++ b/drivers/platform/x86/intel-sar.c
@@ -0,0 +1,589 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Intel Corporation - ACPI for Specific Absorption Rate
+ * Copyright (c) 2021, Intel Corporation.
+ */
+
+#include <linux/platform_device.h>
+#include <net/netlink.h>
+#include <net/net_namespace.h>
+#include <linux/acpi.h>
+#include <uapi/linux/errno.h>
+#include <linux/platform_data/x86/intel-sar.h>
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Platform device driver for INTEL MODEM BIOS SAR");
+MODULE_AUTHOR("Shravan S <s.shravan@intel.com>");
+
+static struct WWAN_SAR_CONTEXT *context;
+
+static long sar_dev_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
+
+static const struct file_operations fops = {
+		.owner      = THIS_MODULE,
+		.unlocked_ioctl = sar_dev_ioctl,
+};
+
+static int sar_add(struct platform_device *device);
+static int sar_remove(struct platform_device *device);
+static void sar_shutdown(struct platform_device *device);
+static void sar_notify(acpi_handle handle, u32 event, void *data);
+
+/**
+ * sar_send_notify - Send Device SAR information to userspace
+ *
+ * Send the sar data and device information to the userspace and later sent to modem
+ * by the userspace application. The data sent is collected from the BIOS.
+ * This data is used by userspace for further handling of the modem output.
+ */
+static void sar_send_notify(void)
+{
+		int result = 0;
+		struct sk_buff *skb;
+		struct nlmsghdr *nlh = NULL;
+
+		pr_debug("Entering: %s\n", __func__);
+		if (!context->socket) {
+			pr_err("Socket is not valid\n");
+			return;
+		}
+		skb = nlmsg_new(NLMSG_ALIGN(sizeof(struct WWAN_DEVICE_MODE_INFO)), GFP_KERNEL);
+		if (!skb) {
+			pr_err("Failed to allocate a new skb\n");
+			return;
+		}
+		nlh = nlmsg_put(skb, 0, 1, NLMSG_DONE, sizeof(struct WWAN_DEVICE_MODE_INFO), 0);
+		if (!nlh) {
+			pr_err("Error while nlmsg_put netlink is triggered\n");
+			nlmsg_free(skb);
+			return;
+		}
+		memcpy(nlmsg_data(nlh), (const char *)&context->sar_data,
+		       sizeof(context->sar_data));
+		pr_debug("Device_mode: %d\n", context->sar_data.device_mode);
+		pr_debug("bandtable_index: %d\n", context->sar_data.bandtable_index);
+		pr_debug("antennatable_index : %d\n", context->sar_data.antennatable_index);
+		pr_debug("sartable_index: %d\n", context->sar_data.sartable_index);
+
+		result = nlmsg_multicast(context->socket, skb, 0, NETLINK_SAR_GROUP, GFP_KERNEL);
+		if (result < 0)
+			pr_err("Error while sending bak to user: %d\n", result);
+}
+
+/**
+ * clear_sar_dev_mode - Clear Device Mode by freeing the allocated data
+ *
+ * If the Device Mode Info is present and allocated, clear it. This is for
+ * dynamic allocated memory of device_mode_info
+ */
+static void clear_sar_dev_mode(void)
+{
+		int reg = 0;
+
+		for (reg = 0; reg < MAX_REGULATORY; reg++) {
+			kfree(context->config_data[reg].device_mode_info);
+			context->config_data[reg].device_mode_info = NULL;
+		}
+}
+
+/**
+ * get_int_value - Retrieve Integer values from ACPI Object
+ * @obj: acpi_object pointer which gets the integer value
+ * @out: output pointer for integer
+ *
+ * Value of the integer from the object of ACPI is obtained.
+ * returns 0 on success
+ */
+static int get_int_value(union acpi_object *obj, int *out)
+{
+		if (obj && obj->type == ACPI_TYPE_INTEGER) {
+			*out = (int)obj->integer.value;
+			return 0;
+		} else {
+			return -1;
+		}
+}
+
+/**
+ * update_sar_data - sar data is updated based on reg_value
+ *
+ * sar_data is updated based on regulatory value
+ */
+static void update_sar_data(void)
+{
+		pr_debug("%s: Update SAR data\n", __func__);
+		if (context->config_data[context->reg_value].device_mode_info &&
+		    context->sar_data.device_mode <=
+		    context->config_data[context->reg_value].total_dev_mode) {
+			context->sar_data.antennatable_index =
+			context->config_data[context->reg_value]
+			.device_mode_info[context->sar_data.device_mode].antennatable_index;
+			context->sar_data.bandtable_index =
+			context->config_data[context->reg_value]
+			.device_mode_info[context->sar_data.device_mode].bandtable_index;
+			context->sar_data.sartable_index =
+			context->config_data[context->reg_value]
+			.device_mode_info[context->sar_data.device_mode].sartable_index;
+			pr_debug("bandtable_index: %d\n", context->sar_data.bandtable_index);
+			pr_debug("antennatable_index : %d\n", context->sar_data.antennatable_index);
+			pr_debug("sartable_index: %d\n", context->sar_data.sartable_index);
+		} else {
+			pr_err("%s: sar data not assigned! Dev mode: %d, total_dev_mode: %d\n",
+			       __func__, context->sar_data.device_mode,
+			       context->config_data[context->reg_value].total_dev_mode);
+		}
+}
+
+/**
+ * parse_guid - parse guid based on DSM UUID
+ *
+ * returns if success or error
+ */
+static acpi_status parse_guid(void)
+{
+		if (guid_parse(SAR_DSM_UUID, &context->guid)) {
+			pr_err("%s: UUID error\n", __func__);
+			return AE_ERROR;
+		}
+		context->parse = true;
+		return AE_OK;
+}
+
+/**
+ * parse_package - parse package for SAR
+ *
+ * @item : acpi_object ptr
+ * @reg : integer - regulatory modes
+ *
+ * returns if success or error
+ */
+static acpi_status parse_package(union acpi_object *item, int reg)
+{
+	int value = 0, itr = 0;
+	union acpi_object *num;
+
+	if (context->config_data[reg].total_dev_mode == 0) {
+		pr_err("Dev Mode count is zero, return\n");
+		return AE_ERROR;
+	}
+	context->config_data[reg].device_mode_info =
+	kmalloc_array(context->config_data[reg].total_dev_mode,
+		      sizeof(struct WWAN_DEVICE_MODE_INFO), GFP_KERNEL);
+	if (!context->config_data[reg].device_mode_info) {
+		pr_err("Cannot allocate memory in kernel\n");
+		return AE_ERROR;
+	}
+	num = &item->package.elements[0];
+	if (get_int_value(num, &value) == 0)
+		pr_debug("%s: Regulatory value : %d\n", __func__, value);
+	for (itr = 0; itr < context->config_data[reg].total_dev_mode; itr++) {
+		num = &item->package.elements[1];
+		if (num && num->type == ACPI_TYPE_PACKAGE) {
+			if (get_int_value(&num->package.elements[0], &value) == 0) {
+				pr_debug("%s: Device Mode for mode %d: %d\n", __func__, itr, value);
+				context->config_data[reg].device_mode_info[itr].device_mode = value;
+			}
+			if (get_int_value(&num->package.elements[1], &value) == 0) {
+				pr_debug("%s: band_index mode %d: %d\n", __func__, itr, value);
+				context->config_data[reg].device_mode_info[itr].bandtable_index =
+				value;
+			}
+			if (get_int_value(&num->package.elements[2], &value) == 0) {
+				pr_debug("%s: antenna_index mode %d: %d\n", __func__, itr, value);
+				context->config_data[reg].device_mode_info[itr].antennatable_index =
+				value;
+			}
+			if (get_int_value(&num->package.elements[3], &value) == 0) {
+				pr_debug("%s: sar_index for mode %d: %d\n", __func__, itr, value);
+				context->config_data[reg].device_mode_info[itr].sartable_index =
+				value;
+			}
+		}
+	}
+	return AE_OK;
+}
+
+/**
+ * sar_module_probe - Extraction of information from BIOS via DSM calls
+ * @device: ACPI device for which to retrieve the data
+ *
+ * Retrieve all values related to device mode and SAR Table index,
+ * Antenna Table index, Band Table index
+ * Returns AE_OK on success
+ */
+static acpi_status sar_module_probe(struct platform_device *device)
+{
+		acpi_status status = AE_OK;
+		union acpi_object *out, *item, req;
+		u32 rev = 0, reg = 0;
+		int value = 0;
+
+		pr_alert("%s Triggered\n", __func__);
+		if (!device) {
+			pr_err("%s: platform driver is null\n", __func__);
+			return AE_ERROR;
+		}
+		if (!context) {
+			pr_err("%s: context is null\n", __func__);
+			return AE_ERROR;
+		}
+		pr_debug("ACPI_HANDLE : %p\n", ACPI_HANDLE(&device->dev));
+		status = acpi_install_notify_handler(ACPI_HANDLE(&device->dev), ACPI_DEVICE_NOTIFY,
+						     sar_notify, (void *)device);
+		if (!(context->parse)) {
+			status = parse_guid();
+			if (status != AE_OK)
+				return status;
+		}
+		context->handle = ACPI_HANDLE(&device->dev);
+		out = acpi_evaluate_dsm(context->handle, &context->guid, rev,
+					COMMAND_ID_DEV_MODE, NULL);
+		pr_debug("%s: acpi_evaluate_dsm completed %d\n", __func__, out->type);
+		if (get_int_value(out, &value) == 0) {
+			pr_debug("%s: Device Mode is : %d\n", __func__, value);
+			context->sar_data.device_mode = value;
+		} else {
+			pr_err("%s: Cmd:%d Failed\n", __func__, COMMAND_ID_DEV_MODE);
+			return AE_ERROR;
+		}
+		ACPI_FREE(out);
+		if (!(context->data_read)) {
+			for (reg = 0; reg < MAX_REGULATORY; reg++) {
+				req.type = ACPI_TYPE_INTEGER;
+				req.integer.value = reg;
+				out = acpi_evaluate_dsm(context->handle, &context->guid, rev,
+							COMMAND_ID_CONFIG_TABLE, &req);
+				if (!out) {
+					pr_err("%s: Cmd:%d Failed\n", __func__,
+					       COMMAND_ID_CONFIG_TABLE);
+					continue;
+				}
+				pr_debug("%s: acpi_evaluate_dsm  for regulatory %d completed %d\n",
+					 __func__, reg, out->type);
+				if (out->type == ACPI_TYPE_PACKAGE) {
+					pr_debug("%s: ACPI_TYPE_PACKAGE, count: %d, type: %d\n",
+						 __func__, out->package.count, out->package.type);
+					item = &out->package.elements[0];
+					if (get_int_value(item, &value) == 0) {
+						pr_debug("%s: version : %d\n", __func__, value);
+						context->config_data[reg].version = value;
+					}
+					item = &out->package.elements[1];
+					if (get_int_value(item, &value) == 0) {
+						pr_debug("%s: No of Modes: %d\n", __func__, value);
+						context->config_data[reg].total_dev_mode = value;
+					}
+					if (context->config_data[reg].total_dev_mode <= 0 &&
+					    context->config_data[reg].total_dev_mode >
+					    MAX_DEV_MODES) {
+						pr_err("total_dev_mode is not within range : %d\n",
+						       context->config_data[reg].total_dev_mode);
+						ACPI_FREE(out);
+						return AE_ERROR;
+					}
+					item = &out->package.elements[2];
+					if (item)
+						status = parse_package(item, reg);
+					else
+						status = AE_ERROR;
+					if (status != AE_OK)
+						return status;
+				}
+				ACPI_FREE(out);
+			}
+			update_sar_data();
+			context->data_read = true;
+		}
+		sar_send_notify();
+		return status;
+}
+
+/**
+ * sar_set_device_mode - To set the device mode as BIOS handling test
+ * @device: ACPI device for which to retrieve the data
+ * @mode: Device Mode to be set
+ *
+ * Test Function call to BIOS for device mode handling of data sent via
+ * DSM calls.
+ */
+static acpi_status sar_set_device_mode(struct platform_device *device, int mode)
+{
+		union acpi_object *out, req;
+		u32 rev = 0;
+		int value = 0;
+		acpi_status status = AE_OK;
+
+		pr_alert("%s Triggered : mode : %d\n", __func__, mode);
+		if (!device) {
+			pr_err("%s: Device is null\n", __func__);
+			return AE_ERROR;
+		}
+		if (!context->handle) {
+			pr_err("%s: Handle is null\n", __func__);
+			return AE_ERROR;
+		}
+		if (!(context->parse)) {
+			status = parse_guid();
+			if (status != AE_OK)
+				return status;
+		}
+
+		req.type = ACPI_TYPE_INTEGER;
+		req.integer.value = mode;
+		out = acpi_evaluate_dsm(context->handle, &context->guid,
+					rev, COMMAND_TEST_SET, &req);
+		if (get_int_value(out, &value) != 0) {
+			pr_err("%s: Cmd:%d Failed\n", __func__, COMMAND_ID_DEV_MODE);
+			return AE_ERROR;
+		}
+		pr_debug("%s: return value is : %d\n", __func__, value);
+		ACPI_FREE(out);
+		return status;
+}
+
+/**
+ * sar_dev_ioctl - This function will be called when we perform ioctl calls to driver
+ * @file: file pointer
+ * @cmd: the command passed for action
+ * @arg: argument used in the commands
+ *
+ * Based on command this function
+ * WR_VALUE: writes specific Regulatory value,
+ * MODE_VALUE: changes device modes sent by userspace
+ * SUPPORT_VALUE: Sends the supported data back to userspace
+ * RD_VALUE: Get sar_data value from userspace
+ * Errors if not 0 is obtained from errno.h
+ */
+static long sar_dev_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+		int value = 0;
+		unsigned long ret = 0;
+		acpi_status status = AE_OK;
+
+		if (!context) {
+			pr_err("%s context is null\n", __func__);
+			return -EINVAL;
+		}
+		switch (cmd) {
+		case WR_VALUE:
+				ret = copy_from_user(&value, (void __user *)arg, sizeof(int32_t));
+				if (ret != 0) {
+					pr_err("copy_from_user for %d fails = %ld\n", cmd, ret);
+					return -EFAULT;
+				}
+				pr_debug("Regulatory Value Set by User = %d\n", value);
+				if (value >= 0 && value < MAX_REGULATORY) {
+					context->reg_value = value;
+					update_sar_data();
+					sar_send_notify();
+				}
+				break;
+		case MODE_VALUE:
+				ret = copy_from_user(&value, (void __user *)arg, sizeof(int32_t));
+				if (ret != 0) {
+					pr_err("copy_from_user for %d fails = %ld\n", cmd, ret);
+					return -EFAULT;
+				}
+				pr_debug("Change Device Mode from %d to %d\n",
+					 context->sar_data.device_mode, value);
+				status = sar_set_device_mode(context->sar_device, value);
+				if (status != AE_OK) {
+					pr_err("sar_set_device_mode for %d failed\n", cmd);
+					return -EINVAL;
+				}
+				break;
+		case SUPPORT_VALUE:
+				ret = copy_from_user(&context->supported_data, (void __user *)arg,
+						     sizeof(struct WWAN_SUPPORTED_INFO));
+				if (ret != 0) {
+					pr_err("copy_from_user for %d fails = %ld\n", cmd, ret);
+					return -EFAULT;
+				}
+				pr_debug("reg set= %d\n", context->supported_data.reg_mode_needed);
+				if (context->supported_data.reg_mode_needed < 0 &&
+				    context->supported_data.reg_mode_needed > MAX_REGULATORY) {
+					context->supported_data.reg_mode_needed = 0;
+					return -EINVAL;
+				}
+				context->supported_data.bios_table_revision =
+				context->config_data[context->supported_data.reg_mode_needed]
+				.version;
+				context->supported_data.num_supported_modes =
+				context->config_data[context->supported_data.reg_mode_needed]
+				.total_dev_mode;
+				pr_debug("ver=%d, modes = %d",
+					 context->supported_data.bios_table_revision,
+					 context->supported_data.num_supported_modes);
+				ret = copy_to_user((void __user *)arg, &context->supported_data,
+						   sizeof(context->supported_data));
+				if (ret != 0) {
+					pr_err("copy_to_user for %d fails = %ld\n", cmd, ret);
+					return -EFAULT;
+				}
+				break;
+		case RD_VALUE:
+				ret = copy_to_user((void __user *)arg, &context->sar_data,
+						   sizeof(context->sar_data));
+				if (ret != 0) {
+					pr_err("copy_to_user for %d fails = %ld\n", cmd, ret);
+					return -EFAULT;
+				}
+				break;
+		default:
+				pr_err("%s command not handled = %d\n", __func__, cmd);
+				return -EINVAL;
+		}
+		return ret;
+}
+
+static const struct acpi_device_id sar_device_ids[] = {
+		{ "INTC1092", 0},
+		{ "", 0},
+};
+
+MODULE_DEVICE_TABLE(acpi, sar_device_ids);
+
+static const struct platform_device_id sar_device_table[] = {
+	{"sar", 0},
+	{},
+};
+
+MODULE_DEVICE_TABLE(platform, sar_device_table);
+
+static struct platform_driver sar_driver = {
+	.probe = sar_add,
+	.remove = sar_remove,
+	.shutdown = sar_shutdown,
+	.driver = {
+			.name = DRVNAME,
+			.owner = THIS_MODULE,
+			/* FOR ACPI HANDLING */
+			.acpi_match_table = ACPI_PTR(sar_device_ids),
+			},
+	.id_table = sar_device_table,
+};
+
+static int sar_add(struct platform_device *device)
+{
+		pr_debug("%s Triggered\n", __func__);
+		context->sar_device = device;
+		if (sar_module_probe(device) != AE_OK)
+			return -1;
+		return 0;
+}
+
+static int sar_remove(struct platform_device *device)
+{
+		pr_debug("%s Triggered\n", __func__);
+		acpi_remove_notify_handler(ACPI_HANDLE(&device->dev),
+					   ACPI_DEVICE_NOTIFY, sar_notify);
+		context->sar_device = NULL;
+		memset(&context->sar_data, 0, sizeof(context->sar_data));
+		return 0;
+}
+
+static void sar_shutdown(struct platform_device *device)
+{
+		sar_remove(device);
+		return;
+}
+
+static void sar_notify(acpi_handle handle, u32 event, void *data)
+{
+		struct platform_device *device = data;
+
+		pr_alert("%s Triggered: event: %d\n", __func__, event);
+		if (event == SAR_EVENT) {
+			pr_debug("%s event matched\n", __func__);
+			if (sar_module_probe(device) != AE_OK)
+				pr_err("sar_module_probe error");
+		}
+}
+
+static int sar_init(void)
+{
+		int result = 0;
+
+		pr_alert("SAR Init Triggered\n");
+		context = kmalloc(sizeof(*context), GFP_KERNEL);
+		if (!context) {
+			pr_err("Cannot allocate memory in kernel for WWAN_SAR_CONTEXT\n");
+			return -1;
+		}
+		memset(context, 0, sizeof(struct WWAN_SAR_CONTEXT));
+		if ((alloc_chrdev_region(&context->dev, 0, 1, DRVNAME)) < 0) {
+			pr_err("Cannot allocate major number for device\n");
+			goto r_free;
+		}
+		pr_debug("Major = %d Minor = %d\n", MAJOR(context->dev), MINOR(context->dev));
+
+		/*Creating cdev structure*/
+		cdev_init(&context->dev_cdev, &fops);
+
+		/*Adding character device to the system*/
+		if ((cdev_add(&context->dev_cdev, context->dev, 1)) < 0) {
+			pr_err("Cannot add the device to the system\n");
+			goto r_char;
+		}
+
+		/*Creating struct class*/
+		context->dev_class = class_create(THIS_MODULE, DRVCLASS);
+		if (!context->dev_class) {
+			pr_err("Cannot create the struct class for device\n");
+			goto r_cdev;
+		}
+
+		/*Creating device*/
+		if (!device_create(context->dev_class, NULL, context->dev, NULL, DRVNAME)) {
+			pr_err("Cannot create the Device\n");
+			goto r_dev;
+		}
+
+		pr_debug("created dev entry\n");
+		context->socket = netlink_kernel_create(&init_net, NETLINK_SAR, NULL);
+		if (!context->socket) {
+			pr_err("Error creating socket\n");
+			goto r_dev_class;
+		}
+
+		result = platform_driver_register(&sar_driver);
+		if (result < 0) {
+			pr_err("Error registering driver\n");
+			goto r_sock;
+		}
+		pr_debug("device registered\n");
+
+		return 0;
+
+r_sock:
+		netlink_kernel_release(context->socket);
+r_dev_class:
+		device_destroy(context->dev_class, context->dev);
+r_dev:
+		class_destroy(context->dev_class);
+r_cdev:
+		cdev_del(&context->dev_cdev);
+r_char:
+		unregister_chrdev_region(context->dev, 1);
+r_free:
+		kfree(context);
+		return -1;
+}
+
+static void sar_exit(void)
+{
+		pr_alert("SAR EXIT Triggered\n");
+		if (context->socket)
+			netlink_kernel_release(context->socket);
+		platform_driver_unregister(&sar_driver);
+		device_destroy(context->dev_class, context->dev);
+		class_destroy(context->dev_class);
+		cdev_del(&context->dev_cdev);
+		unregister_chrdev_region(context->dev, 1);
+		clear_sar_dev_mode();
+		kfree(context);
+		pr_debug("Kernel Module Removed Successfully.\n");
+}
+
+module_init(sar_init);
+module_exit(sar_exit);
diff --git a/include/linux/platform_data/x86/intel-sar.h b/include/linux/platform_data/x86/intel-sar.h
new file mode 100644
index 000000000000..437a9774589b
--- /dev/null
+++ b/include/linux/platform_data/x86/intel-sar.h
@@ -0,0 +1,118 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Intel Corporation Header File for Specific Absorption Rate
+ * Copyright (c) 2021, Intel Corporation.
+ */
+#ifndef INTEL_SAR_H
+#define INTEL_SAR_H
+
+#include <linux/cdev.h>
+
+#define DRVNAME   "sar"
+#define DRVCLASS   "sar_class"
+#define SAR_DSM_UUID "82737E72-3A33-4C45-A9C7-57C0411A5F13"
+#define COMMAND_ID_DEV_MODE 1
+#define COMMAND_ID_CONFIG_TABLE 2
+#define COMMAND_TEST_SET 31
+#define MAX_REGULATORY 3
+#define SAR_EVENT 0x80
+#define MAJOR_VER 1
+#define MINOR_VER 0
+
+#define BUFFER_SIZE 256
+#define WR_VALUE _IOW('a', 'a', int32_t*)
+#define RD_VALUE _IOR('a', 'b', int32_t*)
+#define MODE_VALUE _IOW('a', 'd', int32_t*)
+#define SUPPORT_VALUE _IOWR('a', 'e', struct WWAN_SUPPORTED_INFO*)
+#define MAX_DEV_MODES 50
+#define NETLINK_SAR 25
+#define NETLINK_SAR_GROUP 2
+
+/**
+ * Structure WWAN_DEVICE_MODE_INFO
+ *
+ * Holds the data that needs to be passed to userspace.
+ * The data is updated from the BIOS sensor information.
+ *
+ * @device_mode: Specific mode of the device
+ * @bandtable_index: Index of RF band
+ * @antennatable_index: Index of antenna
+ * @sartable_index: Index of SAR
+ */
+struct WWAN_DEVICE_MODE_INFO {
+		int device_mode;
+		int bandtable_index;
+		int antennatable_index;
+		int sartable_index;
+};
+
+/**
+ * Structure WWAN_DEVICE_MODE_CONFIGURATION
+ *
+ * Holds the data that is configured and obtained on probe event.
+ * The data is updated from the BIOS sensor information.
+ *
+ * @version: Mode configuration version
+ * @total_dev_mode: Total number of device modes
+ * @device_mode_info: pointer to structure WWAN_DEVICE_MODE_INFO
+ */
+struct WWAN_DEVICE_MODE_CONFIGURATION {
+		int version;
+		int total_dev_mode;
+		struct WWAN_DEVICE_MODE_INFO *device_mode_info;
+};
+
+/**
+ * Structure WWAN_SUPPORTED_INFO
+ *
+ * Holds the data that is obtained from userspace
+ * The data is updated from the userspace and send value back in the
+ * structure format that is mentioned here.
+ *
+ * @reg_mode_needed: regulatory mode set by user for tests
+ * @bios_table_revision: Version of SAR table
+ * @num_supported_modes: Total supported modes based on reg_mode
+ */
+struct WWAN_SUPPORTED_INFO {
+		int reg_mode_needed;
+		int bios_table_revision;
+		int num_supported_modes;
+};
+
+/**
+ * Structure WWAN_SAR_CONTEXT
+ *
+ * Holds the complete context as long as the driver is in existence
+ * The context holds instance of the data used for different cases.
+ *
+ * @dev: holds device data
+ * @parse: identifies if dsm is parsed
+ * @data_read: identifies if data is already read from BIOS
+ * @guid: Group id
+ * @handle: store acpi handle
+ * @dev_class: stores the dev_class context
+ * @dev_cdev: handle for device cdev type
+ * @socket: sock for netlink
+ * @reg_value: regulatory value
+ * Regulatory 0: FCC, 1: CE, 2: ISED
+ * @sar_device: platform_device type
+ * @supported_data: WWAN_SUPPORTED_INFO struct
+ * @sar_data: WWAN_DEVICE_MODE_INFO struct
+ * @config_data: WWAN_DEVICE_MODE_CONFIGURATION array struct
+ */
+struct WWAN_SAR_CONTEXT {
+		dev_t dev;
+		bool parse;
+		bool data_read;
+		guid_t guid;
+		acpi_handle handle;
+		struct class *dev_class;
+		struct cdev dev_cdev;
+		struct sock *socket;
+		int reg_value;
+		struct platform_device *sar_device;
+		struct WWAN_SUPPORTED_INFO supported_data;
+		struct WWAN_DEVICE_MODE_INFO sar_data;
+		struct WWAN_DEVICE_MODE_CONFIGURATION config_data[MAX_REGULATORY];
+};
+#endif /* INTEL_SAR_H */
-- 
2.17.1

Re: [PATCH 0/1] [x86] BIOS SAR Driver for M.2 Intel Modems

[Andy Shevchenko]

On Fri, Jun 11, 2021 at 12:46 PM Shravan S <s.shravan@intel.com> wrote:
>
> SAR (Specific Absorption Rate) driver is a host driver implemented for Linux
> or chrome platform to limit the exposure of human body to RF frequency by informing
> the Intel M.2 modem to regulate the RF power based on SAR data obtained from the sensors
> captured in the BIOS. ACPI interface exposes this data from the BIOS to SAR driver. The
> front end application in userspace ( eg: Modem Manager) will interact with SAR driver
> to obtain information like the device mode (Example: tablets, laptops, etx), Antenna index,
> baseband index, SAR table index and use available communication like MBIM interface to enable
> data communication to modem for RF power regulation.
>
> The BIOS gets notified about device mode changes through Sensor Driver. This information is
> given to a (newly created) WWAN ACPI function driver when there is a device mode change.
> The driver then uses a _DSM method to retrieve the required information from BIOS.
> This information is then forwarded/multicast to the User-space using the NETLINK interface.
> A lookup table is maintained inside the BIOS which suggests the SAR Table index and Antenna
> Tuner Table Index values for individual device modes.
>
> The SAR parameters to be used on the Modem differs for each Regulatory Mode like FCC, CE and ISED.
> Hence, the SAR parameters are stored separately in the SMBIOS table in the OEM BIOS,
> for each of the Regulatory mode. Regulatory modes will be different based on the region and network
> available in that region.
>
> Hence the entire SAR functionality handling is divided into 2 parts:
>
> •       A ACPI function driver implemented that uses a dedicated ACPI node for WWAN device.
>     sends notifications whenever there is change in Device Mode. (each OEM has different mechanism
>     of updating this DEVICE Mode info). This is notified to User-space applications using
>     the RT-NETLINK interface.
> •       WWAN Device Service listens for RT-NETLINK messages and routes them to Modem using MBIM.

That's a nice feature!
Why is it not a part of some generic subsubsystem under wireless
network subsystem?


-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH 0/1] [x86] BIOS SAR Driver for M.2 Intel Modems

[Shravan, S]

Hi Andy,

Why is it not a part of some generic subsystem under wireless network subsystem?

-- This driver is instantiated only when the BIOS on given host exposes ACPI node corresponding to the BIOS SAR. This depends on support of the BIOS SAR feature by given OEM.
-- It is agnostic of the wireless technology like WWAN, WiFi and BT. Hence, it is not made specific to any given wireless network subsystem.

Please do let me know if you need more information.

Thanks and Regards,
Shravan S

-----Original Message-----
From: Andy Shevchenko <andy.shevchenko@gmail.com> 
Sent: Sunday, June 13, 2021 7:52 PM
To: Shravan, S <s.shravan@intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>; Mark Gross <mgross@linux.intel.com>; Platform Driver <platform-driver-x86@vger.kernel.org>; An, Sudhakar <sudhakar.an@intel.com>
Subject: Re: [PATCH 0/1] [x86] BIOS SAR Driver for M.2 Intel Modems

On Fri, Jun 11, 2021 at 12:46 PM Shravan S <s.shravan@intel.com> wrote:
>
> SAR (Specific Absorption Rate) driver is a host driver implemented for 
> Linux or chrome platform to limit the exposure of human body to RF 
> frequency by informing the Intel M.2 modem to regulate the RF power 
> based on SAR data obtained from the sensors captured in the BIOS. ACPI 
> interface exposes this data from the BIOS to SAR driver. The front end 
> application in userspace ( eg: Modem Manager) will interact with SAR 
> driver to obtain information like the device mode (Example: tablets, 
> laptops, etx), Antenna index, baseband index, SAR table index and use available communication like MBIM interface to enable data communication to modem for RF power regulation.
>
> The BIOS gets notified about device mode changes through Sensor 
> Driver. This information is given to a (newly created) WWAN ACPI function driver when there is a device mode change.
> The driver then uses a _DSM method to retrieve the required information from BIOS.
> This information is then forwarded/multicast to the User-space using the NETLINK interface.
> A lookup table is maintained inside the BIOS which suggests the SAR 
> Table index and Antenna Tuner Table Index values for individual device modes.
>
> The SAR parameters to be used on the Modem differs for each Regulatory Mode like FCC, CE and ISED.
> Hence, the SAR parameters are stored separately in the SMBIOS table in 
> the OEM BIOS, for each of the Regulatory mode. Regulatory modes will 
> be different based on the region and network available in that region.
>
> Hence the entire SAR functionality handling is divided into 2 parts:
>
> •       A ACPI function driver implemented that uses a dedicated ACPI node for WWAN device.
>     sends notifications whenever there is change in Device Mode. (each OEM has different mechanism
>     of updating this DEVICE Mode info). This is notified to User-space applications using
>     the RT-NETLINK interface.
> •       WWAN Device Service listens for RT-NETLINK messages and routes them to Modem using MBIM.

That's a nice feature!
Why is it not a part of some generic subsubsystem under wireless network subsystem?


--
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/1] [x86] BIOS SAR Driver for M.2 Intel Modems

[Enrico Weigelt, metux IT consult]

On 14.06.21 13:48, Shravan, S wrote:

Hi,

> Why is it not a part of some generic subsystem under wireless network subsystem?
> 
> -- This driver is instantiated only when the BIOS on given host exposes ACPI node corresponding to the BIOS SAR. This depends on support of the BIOS SAR feature by given OEM.
> -- It is agnostic of the wireless technology like WWAN, WiFi and BT. Hence, it is not made specific to any given wireless network subsystem.
> 
> Please do let me know if you need more information.

the problems I see here:

1. the device uapi is very vendor specific
2. its unclear for which air interface is the data really retrieved ?
3. unclear how userland this should really handle in a generic way
    --> how does it know which device to tune ?
4. does it really need to be (non-portable) ioctls ?


by the way, who hat that funny idea putting such information into acpi
in such a weird way ?


--mtx

-- 
---
Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert
werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren
GPG/PGP-Schlüssel zu.
---
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

Re: [PATCH 0/1] [x86] BIOS SAR Driver for M.2 Intel Modems

[Andy Shevchenko]

On Tue, Jun 15, 2021 at 9:01 PM Enrico Weigelt, metux IT consult
<lkml@metux.net> wrote:
>
> On 14.06.21 13:48, Shravan, S wrote:
>
> Hi,
>
> > Why is it not a part of some generic subsystem under wireless network subsystem?
> >
> > -- This driver is instantiated only when the BIOS on given host exposes ACPI node corresponding to the BIOS SAR. This depends on support of the BIOS SAR feature by given OEM.
> > -- It is agnostic of the wireless technology like WWAN, WiFi and BT. Hence, it is not made specific to any given wireless network subsystem.
> >
> > Please do let me know if you need more information.
>
> the problems I see here:
>
> 1. the device uapi is very vendor specific

We have a platform profile which is also quite vendor specific,
nevertheless we (as upstream) are trying to have points of
unifications.

I think this driver should be part of the corresponding profile /
network subsystem part and be a one (of the) hardware implementation.
Somebody can add more. Users in Linux should have a common ABI for
that. And I'm not sure it should not be a netlink based one.

> 2. its unclear for which air interface is the data really retrieved ?
> 3. unclear how userland this should really handle in a generic way
>     --> how does it know which device to tune ?
> 4. does it really need to be (non-portable) ioctls ?
>
>
> by the way, who hat that funny idea putting such information into acpi
> in such a weird way ?

I believe its source is a Windows driver and Windows "culture", they
simply don't give a crap about anything else and Windows is a
product-oriented platform (each product is unique even if 99.9% of the
hardware and firmware is the same with twenty more products).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/1] [x86] BIOS SAR Driver for M.2 Intel Modems

[Hans de Goede]

Hi Shravan,

On 4/28/21 5:22 AM, Shravan S wrote:
> SAR (Specific Absorption Rate) driver is a host driver implemented for Linux
> or chrome platform to limit the exposure of human body to RF frequency by informing
> the Intel M.2 modem to regulate the RF power based on SAR data obtained from the sensors
> captured in the BIOS. ACPI interface exposes this data from the BIOS to SAR driver. The
> front end application in userspace ( eg: Modem Manager) will interact with SAR driver 
> to obtain information like the device mode (Example: tablets, laptops, etx), Antenna index,
> baseband index, SAR table index and use available communication like MBIM interface to enable
> data communication to modem for RF power regulation.
> 
> The BIOS gets notified about device mode changes through Sensor Driver. This information is 
> given to a (newly created) WWAN ACPI function driver when there is a device mode change. 
> The driver then uses a _DSM method to retrieve the required information from BIOS. 
> This information is then forwarded/multicast to the User-space using the NETLINK interface.
> A lookup table is maintained inside the BIOS which suggests the SAR Table index and Antenna 
> Tuner Table Index values for individual device modes.
> 
> The SAR parameters to be used on the Modem differs for each Regulatory Mode like FCC, CE and ISED.
> Hence, the SAR parameters are stored separately in the SMBIOS table in the OEM BIOS, 
> for each of the Regulatory mode. Regulatory modes will be different based on the region and network
> available in that region.

If I'm reading the above correct then this code is really doing 2
things in 1 driver:

1. Listening to some sensors, which readings may impact the maximum amount
of tx power which the modem may use. What kind of sensors are these ?
Currently chrome-os based devices are using iio for proximity sensors,
with specific labels added to each sensor to tell userspace that they
indicate a human is close to one of the antennas:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/Documentation/ABI/testing/sysfs-bus-iio?id=6505dfab33c519368e54ae7f3ea1bf4d9916fdc5

Would it be possible to use this standardized userspace interface for
your use case ?

2. Exporting a table with various information from the BIOS tables
to userspace. We do probably need a new userspace API for this,
but I'm not sure netlink is the right answer here.

Maybe just use a binary sysfs attribute under
/sys/bus/platform/devices/INTC1092:00 ?
That will make the interface non-generic, but I assume that the
table contents are going to be Intel specific anyways so that
should be fine. This will also allow simply exporting the table
without the kernel needing to parse it.

Using netlink is highly unusual for a driver living under
platform/drivers/x86; and if you really want to use netlink for
this then you should first define a generic protocol which is
also going to work for other vendors' modems, which is
impossible ATM because we don't know yet what other vendor's
modems will need...

Regards,

Hans







> 
> Hence the entire SAR functionality handling is divided into 2 parts:
> 
> •	A ACPI function driver implemented that uses a dedicated ACPI node for WWAN device. 
>     sends notifications whenever there is change in Device Mode. (each OEM has different mechanism
>     of updating this DEVICE Mode info). This is notified to User-space applications using 
>     the RT-NETLINK interface.
> •	WWAN Device Service listens for RT-NETLINK messages and routes them to Modem using MBIM. 
> 
> Shravan S (1):
>   [x86]: BIOS Dynamic SAR driver for Intel M.2 Modem
> 
>  drivers/platform/x86/Kconfig                |  12 +
>  drivers/platform/x86/Makefile               |   1 +
>  drivers/platform/x86/intel-sar.c            | 589 ++++++++++++++++++++
>  include/linux/platform_data/x86/intel-sar.h | 118 ++++
>  4 files changed, 720 insertions(+)
>  create mode 100644 drivers/platform/x86/intel-sar.c
>  create mode 100644 include/linux/platform_data/x86/intel-sar.h
> 
> 
> base-commit: cd1245d75ce93b8fd206f4b34eb58bcfe156d5e9
> 

Re: [PATCH 0/1] [x86] BIOS SAR Driver for M.2 Intel Modems

[Hans de Goede]

Hi,

On 6/17/21 4:28 PM, Hans de Goede wrote:
> Hi Shravan,
> 
> On 4/28/21 5:22 AM, Shravan S wrote:
>> SAR (Specific Absorption Rate) driver is a host driver implemented for Linux
>> or chrome platform to limit the exposure of human body to RF frequency by informing
>> the Intel M.2 modem to regulate the RF power based on SAR data obtained from the sensors
>> captured in the BIOS. ACPI interface exposes this data from the BIOS to SAR driver. The
>> front end application in userspace ( eg: Modem Manager) will interact with SAR driver 
>> to obtain information like the device mode (Example: tablets, laptops, etx), Antenna index,
>> baseband index, SAR table index and use available communication like MBIM interface to enable
>> data communication to modem for RF power regulation.
>>
>> The BIOS gets notified about device mode changes through Sensor Driver. This information is 
>> given to a (newly created) WWAN ACPI function driver when there is a device mode change. 
>> The driver then uses a _DSM method to retrieve the required information from BIOS. 
>> This information is then forwarded/multicast to the User-space using the NETLINK interface.
>> A lookup table is maintained inside the BIOS which suggests the SAR Table index and Antenna 
>> Tuner Table Index values for individual device modes.
>>
>> The SAR parameters to be used on the Modem differs for each Regulatory Mode like FCC, CE and ISED.
>> Hence, the SAR parameters are stored separately in the SMBIOS table in the OEM BIOS, 
>> for each of the Regulatory mode. Regulatory modes will be different based on the region and network
>> available in that region.
> 
> If I'm reading the above correct then this code is really doing 2
> things in 1 driver:
> 
> 1. Listening to some sensors, which readings may impact the maximum amount
> of tx power which the modem may use. What kind of sensors are these ?
> Currently chrome-os based devices are using iio for proximity sensors,
> with specific labels added to each sensor to tell userspace that they
> indicate a human is close to one of the antennas:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/Documentation/ABI/testing/sysfs-bus-iio?id=6505dfab33c519368e54ae7f3ea1bf4d9916fdc5
> 
> Would it be possible to use this standardized userspace interface for
> your use case ?
> 
> 2. Exporting a table with various information from the BIOS tables
> to userspace. We do probably need a new userspace API for this,
> but I'm not sure netlink is the right answer here.
> 
> Maybe just use a binary sysfs attribute under
> /sys/bus/platform/devices/INTC1092:00 ?
> That will make the interface non-generic, but I assume that the
> table contents are going to be Intel specific anyways so that
> should be fine. This will also allow simply exporting the table
> without the kernel needing to parse it.
> 
> Using netlink is highly unusual for a driver living under
> platform/drivers/x86; and if you really want to use netlink for
> this then you should first define a generic protocol which is
> also going to work for other vendors' modems, which is
> impossible ATM because we don't know yet what other vendor's
> modems will need...

Some more remarks from a quick look at the code, I see that
besides a netlink interface you are also creating a chardev
and doing ioctls on that.

That is really not good, it seems that the userspace API for
this needs to be completely redesigned. Using both netlink
and ioctls at the same time not acceptable.

Also you seem to create the chardev and netlink and
module-load time, rather then from the sar_add() probe
function. Which means that they will also be created on
laptops which don't have an INTC1092 ACPI device at all,
again not good.

Please drop the sar_init() and sar_exit() functions and
use module_platform_driver to have module_init / exit
register / unregister the driver and have them only do that,
everything else should be done from the platform_driver
probe function.

Also you use "sar" for a lot of identifiers in the driver,
but that is very generic where as this is a highly Intel
specific driver; and likely in the future even newer Intel
modems may use a different driver, so please replace the
sar prefix with something more specific, e.g. "intc1092"

Regards,

Hans

RE: [PATCH 0/1] [x86] BIOS SAR Driver for M.2 Intel Modems

[Shravan, S]

Hi,

Response Inline.

-----Original Message-----
> From: Andy Shevchenko <andy.shevchenko@gmail.com> 
> Sent: Wednesday, June 16, 2021 1:59 AM
> To: Enrico Weigelt, metux IT consult <lkml@metux.net>
> Cc: Shravan, S <s.shravan@intel.com>; Hans de Goede <hdegoede@redhat.com>; Mark Gross <mgross@linux.intel.com>; Platform Driver <platform-driver-x86@vger.kernel.org>; An, Sudhakar <sudhakar.an@intel.com>
> Subject: Re: [PATCH 0/1] [x86] BIOS SAR Driver for M.2 Intel Modems
> 
> On Tue, Jun 15, 2021 at 9:01 PM Enrico Weigelt, metux IT consult <lkml@metux.net> wrote:
> >
> > On 14.06.21 13:48, Shravan, S wrote:
> >
> > Hi,
> >
> > > Why is it not a part of some generic subsystem under wireless network subsystem?
> > >
> > > -- This driver is instantiated only when the BIOS on given host exposes ACPI node corresponding to the BIOS SAR. This depends on support of the BIOS SAR feature by given OEM.
> > > -- It is agnostic of the wireless technology like WWAN, WiFi and BT. Hence, it is not made specific to any given wireless network subsystem.
> > >
> > > Please do let me know if you need more information.
> >
> > the problems I see here:
> >
> > 1. the device uapi is very vendor specific
> 
> We have a platform profile which is also quite vendor specific, nevertheless we (as upstream) are trying to have points of unifications.
> 
> I think this driver should be part of the corresponding profile / network subsystem part and be a one (of the) hardware implementation.
> Somebody can add more. Users in Linux should have a common ABI for that. And I'm not sure it should not be a netlink based one.
>

[Shravan]  This driver is exposing the SAR parameters solely influenced by the current status of the host platform and is not specific to any RF device. Hence it is placed at the path (Platform driver).
Also, we have reworked the driver implementation to remove netlink usage. This is replaced with sysfs. Please do have a look and share feedback on the same.

> > 2. its unclear for which air interface is the data really retrieved ?

[Shravan]  The initial implementation supports parameters received for WWAN. Subsequently other RF device types (Wifi, BT) will be supported.

> > 3. unclear how userland this should really handle in a generic way
> >     --> how does it know which device to tune ?

[Shravan] Userland will configure these parameters on the specific RF device. A Table of SAR parameters is already provisioned on the given RF device. 
The information retrieved from this driver specifies which index in this table has to be used by the RF device.

> > 4. does it really need to be (non-portable) ioctls ?
> >

[Shravan] Usage of IOCTLs have been avoided in the latest patch. This has been replaced with sysfs. Please do have a look and suggest if this concern is addressed.

> >
> > by the way, who hat that funny idea putting such information into acpi 
> > in such a weird way ?
> 
> I believe its source is a Windows driver and Windows "culture", they simply don't give a crap about anything else and Windows is a product-oriented platform (each product is unique even if 99.9% of the hardware and firmware is the same with twenty more products).

Thanks and Regards,
Shravan S

RE: [PATCH 0/1] [x86] BIOS SAR Driver for M.2 Intel Modems

[Shravan, S]

Hi,

Thank you for the feedback. Responses Inline.


-----Original Message-----
>From: Hans de Goede <hdegoede@redhat.com> 
>Sent: Thursday, June 17, 2021 8:06 PM
>To: Shravan, S <s.shravan@intel.com>; mgross@linux.intel.com; platform-driver-x86@vger.kernel.org
>Cc: An, Sudhakar <sudhakar.an@intel.com>
>Subject: Re: [PATCH 0/1] [x86] BIOS SAR Driver for M.2 Intel Modems
>
>Hi,
>
>On 6/17/21 4:28 PM, Hans de Goede wrote:
>> Hi Shravan,
>> 
>> On 4/28/21 5:22 AM, Shravan S wrote:
>>> SAR (Specific Absorption Rate) driver is a host driver implemented 
>>> for Linux or chrome platform to limit the exposure of human body to 
>>> RF frequency by informing the Intel M.2 modem to regulate the RF 
>>> power based on SAR data obtained from the sensors captured in the 
>>> BIOS. ACPI interface exposes this data from the BIOS to SAR driver. 
>>> The front end application in userspace ( eg: Modem Manager) will 
>>> interact with SAR driver to obtain information like the device mode 
>>> (Example: tablets, laptops, etx), Antenna index, baseband index, SAR table index and use available communication like MBIM interface to enable data communication to modem for RF power regulation.
>>>
>>> The BIOS gets notified about device mode changes through Sensor 
>>> Driver. This information is given to a (newly created) WWAN ACPI function driver when there is a device mode change.
>>> The driver then uses a _DSM method to retrieve the required information from BIOS. 
>>> This information is then forwarded/multicast to the User-space using the NETLINK interface.
>>> A lookup table is maintained inside the BIOS which suggests the SAR 
>>> Table index and Antenna Tuner Table Index values for individual device modes.
>>>
>>> The SAR parameters to be used on the Modem differs for each Regulatory Mode like FCC, CE and ISED.
>>> Hence, the SAR parameters are stored separately in the SMBIOS table 
>>> in the OEM BIOS, for each of the Regulatory mode. Regulatory modes 
>>> will be different based on the region and network available in that region.
>> 
>> If I'm reading the above correct then this code is really doing 2 
>> things in 1 driver:
>> 
>> 1. Listening to some sensors, which readings may impact the maximum 
>> amount of tx power which the modem may use. What kind of sensors are these ?
>> Currently chrome-os based devices are using iio for proximity sensors, 
>> with specific labels added to each sensor to tell userspace that they 
>> indicate a human is close to one of the antennas:
>> 
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/com
>> mit/Documentation/ABI/testing/sysfs-bus-iio?id=6505dfab33c519368e54ae7
>> f3ea1bf4d9916fdc5
>> 
>> Would it be possible to use this standardized userspace interface for 
>> your use case ?

[Shravan] Proximity sensors can work in scenarios where there is no other sources of information which can alter the sar handling.
OEMs have given feedback that the device mode (tablet mode/laptop mode/clamshell etc) also play a part in SAR handling. Hence this
has to be aggregated with the proximity sensor information. Also such an aggregation is specific to given host platform. As a consequence,
this is best realized within an entity like embedded controller available on the host platform. This new driver exposes such aggregated SAR
parameters that needs to be configured on specific RF device.

>> 
>> 2. Exporting a table with various information from the BIOS tables to 
>> userspace. We do probably need a new userspace API for this, but I'm 
>> not sure netlink is the right answer here.
>> 
>> Maybe just use a binary sysfs attribute under
>> /sys/bus/platform/devices/INTC1092:00 ?
>> That will make the interface non-generic, but I assume that the table 
>> contents are going to be Intel specific anyways so that should be 
>> fine. This will also allow simply exporting the table without the 
>> kernel needing to parse it.
>> 
>> Using netlink is highly unusual for a driver living under 
>> platform/drivers/x86; and if you really want to use netlink for this 
>> then you should first define a generic protocol which is also going to 
>> work for other vendors' modems, which is impossible ATM because we 
>> don't know yet what other vendor's modems will need...

[Shravan] As per your suggestion we are switching to the sysfs interface to expose this information.
We have reworked the driver implemetation with the latest patch to remove the usage of netlink which should address this concern.
Please do have a look at the latest patch to confirm the same.

>Some more remarks from a quick look at the code, I see that besides a netlink interface you are also creating a chardev and doing ioctls on that.
>That is really not good, it seems that the userspace API for this needs to be completely redesigned. Using both netlink and ioctls at the same time not acceptable.

[Shravan] With the latest patch we have removed the IOCTL interface and switched to sysfs mode of communictaion to and from userspace.

>Also you seem to create the chardev and netlink and module-load time, rather then from the sar_add() probe function. Which means that they will also be created on laptops which don't have an INTC1092 ACPI device at all, again not good.
>Please drop the sar_init() and sar_exit() functions and use module_platform_driver to have module_init / exit register / unregister the driver and have them only do that, everything else should be done from the platform_driver probe function.

[Shravan] The latest patch addresses this concern. Please do confirm.

>Also you use "sar" for a lot of identifiers in the driver, but that is very generic where as this is a highly Intel specific driver; and likely in the future even newer Intel modems may use a different driver, so please replace the sar prefix with something more specific, e.g. "intc1092"

[Shravan] Based on the feedback sar_device_table name has been changed from "sar" to "intc1092" on the latest patch V2. Please let us know if these changes addresses this concern.

Thanks and Regards,
Shravan S

Re: [PATCH 0/1] [x86] BIOS SAR Driver for M.2 Intel Modems

[Enrico Weigelt, metux IT consult]

On 23.06.21 16:03, Shravan, S wrote:

Hi,

> [Shravan]  This driver is exposing the SAR parameters solely influenced by 
> the current status of the host platform and is not specific to any RF device. 
> Hence it is placed at the path (Platform driver).

I'm confused, how could that be not specific to any RF device ? It can
only make sense at all, if we know which device certain settings need to
be applied to. And even finding reasonable settings highly depends on
lots of hardware and environmental specific factors, starting with the
frequencies and modulation types, signal routing on the board, shielding
characteristics of the machine's case, etc, etc.

By the way: why does it have to be some proprietary ACPI call ?

Why not just publishing some table where the kernel can do the lookup ?
And do we have some (simple!) identification of the board/machine, so we
can easily override these bios values when necessary ?

Over the last decades I had to learn to *never* trust BIOS vendors with
anything more than just starting the kernel, especially not trusting in
ACPI tables. And we certainly cant expect people doing field bios
upgrades anytime soon, in case some bios vendor actually manages to
clean up his dirt and publish some actual fixes.

Seriously, I'd rather try to keep bios out of the loop as much as
possible. And if it is involved, let it describe the hardware precisely
instead of doing whatever magic logic.

(I need to hold back myself for not starting another rant against ACPI
and bios vendors :p)

>>> 2. its unclear for which air interface is the data really retrieved ?
> 
> [Shravan]  The initial implementation supports parameters received for WWAN. 
> Subsequently other RF device types (Wifi, BT) will be supported.

We still don't know for which individual interface these parameters
apply to. I've got machines with multiple WWAN interfaces out in the
field.

>>> 3. unclear how userland this should really handle in a generic way
>>>      --> how does it know which device to tune ?
> 
> [Shravan] Userland will configure these parameters on the specific RF device.

So the user needs to configure it anyways. Why do we have to have that
acpi stuff in the first place ? If we're already involving a device
specific userland, everything (including the tables) could live entirely
in userland - and we would never ever have to touch bios or kernel
anymore. (remember: bios upgrades are always a total mess).

>>> by the way, who hat that funny idea putting such information into acpi
>>> in such a weird way ?
>>
>> I believe its source is a Windows driver and Windows "culture", they simply 
>> don't give a crap about anything else and Windows is a product-oriented platform 
>> (each product is unique even if 99.9% of the hardware and firmware is the same 
>> with twenty more products).

Okay, and why are you guys (Intel) following such insanity, when this is
meant for Linux-based devices like Chrome ?

Sorry, but doing something just because thousands of programming minions
in Windoze world (which, from my personal expercience, most of them, at
least on driver and firmware side, I have to consider totally
incompetent) are doing it that way, really is a bad excuse and has
nothing to do with decent engineering.

So, please, let's throw away that arbitrary acpi junk and engineer a
technically good solution.

My requirements for such a solution would be:

* the involvement of bios/acpi shall be limited to some exact
   identification, or even better, formal description of the hardware.
   (like we do w/ device tree) [*1]
* beyond that, there shall be no need to ever do any bios upgrade,
   once this data is initially added
* the formal description shall also indentify the which devices are to
   be tuned
* the OS shall be in full control of everything, no hidden firmware
   magic at all.
* the solution shall be generic enough so it can be directly applied to
   *any* similar hardware, no matter what vendor or fw type.


--mtx


[*1] maybe offtopic: I'm actually considering embedding actual DTB into
      the bios and leave acpi just as a legacy fallback - yes, seriously.
--
---
Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert
werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren
GPG/PGP-Schlüssel zu.
---
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

Re: [PATCH 0/1] [x86] BIOS SAR Driver for M.2 Intel Modems

[Andy Shevchenko]

On Mon, Jun 28, 2021 at 5:07 PM Enrico Weigelt, metux IT consult
<lkml@metux.net> wrote:
> On 23.06.21 16:03, Shravan, S wrote:

...

> Over the last decades I had to learn to *never* trust BIOS vendors with
> anything more than just starting the kernel, especially not trusting in
> ACPI tables. And we certainly cant expect people doing field bios
> upgrades anytime soon, in case some bios vendor actually manages to
> clean up his dirt and publish some actual fixes.

But this is not the issue of ACPI, right? Maybe you should stream the
energy to complain and file bugs against vendors who do not know how
to cook ACPI?

> Seriously, I'd rather try to keep bios out of the loop as much as
> possible. And if it is involved, let it describe the hardware precisely
> instead of doing whatever magic logic.

Isn't it applicable to all firmwares? Have you tried to avoid wireless
firmwares (rhetorical question)? My point is that we have to live with
that fortunately or unfortunately.

> (I need to hold back myself for not starting another rant against ACPI
> and bios vendors :p)

As I said, look into the root cause, while I admit that the ACPI spec
is easy to abuse / misinterpret (in some cases).

...

> >>> 3. unclear how userland this should really handle in a generic way
> >>>      --> how does it know which device to tune ?
> >
> > [Shravan] Userland will configure these parameters on the specific RF device.
>
> So the user needs to configure it anyways. Why do we have to have that
> acpi stuff in the first place ? If we're already involving a device
> specific userland, everything (including the tables) could live entirely
> in userland - and we would never ever have to touch bios or kernel
> anymore. (remember: bios upgrades are always a total mess).
>
> >>> by the way, who hat that funny idea putting such information into acpi
> >>> in such a weird way ?
> >>
> >> I believe its source is a Windows driver and Windows "culture", they simply
> >> don't give a crap about anything else and Windows is a product-oriented platform
> >> (each product is unique even if 99.9% of the hardware and firmware is the same
> >> with twenty more products).
>
> Okay, and why are you guys (Intel) following such insanity, when this is
> meant for Linux-based devices like Chrome ?

I haven't got it. How do you deduct that it's solely for Chrome? Even
I'm puzzled with this Yet Another Not So Portable Idea. And above is
my speculation about the roots of it. I can't explain it any other
way.

> Sorry, but doing something just because thousands of programming minions
> in Windoze world (which, from my personal expercience, most of them, at
> least on driver and firmware side, I have to consider totally
> incompetent) are doing it that way, really is a bad excuse and has
> nothing to do with decent engineering.
>
> So, please, let's throw away that arbitrary acpi junk and engineer a
> technically good solution.

ACPI has nothing to do with any solution to be "junk". If one doesn't
know how to cook it, it doesn't prevent them from cooking it in a
better way.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/1] [x86] BIOS SAR Driver for M.2 Intel Modems

[Enrico Weigelt, metux IT consult]

On 23.06.21 16:12, Shravan, S wrote:

Hi,

>>> 1. Listening to some sensors, which readings may impact the maximum
>>> amount of tx power which the modem may use. What kind of sensors are these ?
>>> Currently chrome-os based devices are using iio for proximity sensors,
>>> with specific labels added to each sensor to tell userspace that they
>>> indicate a human is close to one of the antennas:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/com
>>> mit/Documentation/ABI/testing/sysfs-bus-iio?id=6505dfab33c519368e54ae7
>>> f3ea1bf4d9916fdc5
>>>
>>> Would it be possible to use this standardized userspace interface for
>>> your use case ?
> 
> [Shravan] Proximity sensors can work in scenarios where there is no other sources of information which can alter the sar handling.
> OEMs have given feedback that the device mode (tablet mode/laptop mode/clamshell etc) also play a part in SAR handling. Hence this
> has to be aggregated with the proximity sensor information. Also such an aggregation is specific to given host platform. As a consequence,
> this is best realized within an entity like embedded controller available on the host platform. This new driver exposes such aggregated SAR
> parameters that needs to be configured on specific RF device.

Since this is totally hardware specific and doesn't even tell us which
radio interface this is about - why do we have to have this incomplete
stuff in acpi and the kernel in the first place ? As things are right
now, this can be completely done in userspace, including the tables.

It would be different if acpi tables could give us some precise and 
generic hardware description, so we could handle this in a generic
way and userland wouldn't even have to care about it.


--mtx

-- 
---
Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert
werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren
GPG/PGP-Schlüssel zu.
---
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

Re: [PATCH 0/1] [x86] BIOS SAR Driver for M.2 Intel Modems

[Enrico Weigelt, metux IT consult]

On 28.06.21 17:04, Andy Shevchenko wrote:

> But this is not the issue of ACPI, right? Maybe you should stream the
> energy to complain and file bugs against vendors who do not know how
> to cook ACPI?

It's an issue of the acpi ecosystem. Mostly the fault of individual hw
vendors that invent weird ways to abuse it for things it never had been
designed. We're here talking with some (representative of some) hw
vendor.

By the way: this actually isn't an issue of these specific intel modems
at all, but the mainboards where some of these could plugged into. At
that point I wonder why we don't hear a word from these guys, it would
be their duty.

> Isn't it applicable to all firmwares? Have you tried to avoid wireless
> firmwares (rhetorical question)? 

It depends. Firmware for external controllers (which is loaded by the
kernel) is an entirely different area. I wish we wouldn't need to care
about it, but it's better us doing that of board or bios vendors,
and we have reasonabley well methods to cope with it.

> My point is that we have to live with
> that fortunately or unfortunately.

It seems that the whole thing is still under development (even sounds
like for now only one company using it in their closed devices), so we
might still have the chance to prevent another nightmare.

>> (I need to hold back myself for not starting another rant against ACPI
>> and bios vendors :p)
> 
> As I said, look into the root cause, while I admit that the ACPI spec
> is easy to abuse / misinterpret (in some cases).

Yes, and it's used for things it wasn't designed for. Actually, I claim
it was a bad idea in the first place - device tree already has been
there and the superior solution when acpi brought upon us. And even
after that, there way plenty time to introduce a hybrid solution to add
in device tree and leave the old acpi stuff as legacy. This actually
would have been pretty easy to do so.

Yes, ACPI received several useful improvements over the time, but still
very incomplete (anything but well thought), and ... voila ... it's even
embedding pieces of device tree, so it becomes somewhat more useful.
(but still in quite weird ways). At that point I really wonder why we're
not directly using DT ?

One of the major problems at this point is board vendors treating vital
information of device / board composition as some kind of black magic,
instead of just giving us a precise description how things are wired up.

> I haven't got it. How do you deduct that it's solely for Chrome? 

He mentioned in one of the mails. At least it sounded that Chrome
devices are currently the only ones that actually use it.

>> So, please, let's throw away that arbitrary acpi junk and engineer a
>> technically good solution.
> 
> ACPI has nothing to do with any solution to be "junk". If one doesn't
> know how to cook it, it doesn't prevent them from cooking it in a
> better way.

I've been referring about this specific way to put it into acpi. Indeed
acpi could be used to do it in a sane way, but this would look very
differently then. I would declare any way of putting this into acpi
functions as junk - a clean solution would be giving a precise
description of the hardware itself, a purely declarative approach,
that is independent of the actual baseband module.


--mtx

-- 
---
Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert
werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren
GPG/PGP-Schlüssel zu.
---
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287