platform: surface: Introduce Surface XBL Driver

Introduce yaml for surface xbl driver.

Signed-off-by Jarrett Schultz <[email protected]>

---
 .../platform/microsoft/surface-xbl.yaml       | 37 +++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml

Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml new file +0 -0

new file mode 100644
index 000000000000..3d2771322e72

Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml new file +10 -0

+
+properties:
+  compatible:
+    const: microsoft,sm8150-surface-duo-xbl
+
+  reg:
+    maxItems: 1
+
+unevaluatedProperties: false
+

Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml new file +1 -1

+    };
--

2.25.1

Introduce yaml for surface xbl driver.

Signed-off-by Jarrett Schultz <[email protected]>

---
 .../platform/microsoft/surface-xbl.yaml       | 37 +++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml

Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml new file +26 -0

new file mode 100644
index 000000000000..3d2771322e72
@@ -0,0 +1,37 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/platform/microsoft/surface-xbl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Surface Extensible Bootloader for Microsoft Surface Duo
+
+maintainers:
+  - Jarrett Schultz <[email protected]>
+
+description: |
+  Exposes device information to user space.
+
+allOf:
+  - $ref: /schemas/platform/microsoft/surface-xbl.c#
+
+properties:
+  compatible:
+    const: microsoft,sm8150-surface-duo-xbl
+
+  reg:
+    maxItems: 1
+
+unevaluatedProperties: false
+

Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml new file +1 -0

+    };

Signature · 1 line

2.25.1

drivers/platform/surface/Kconfig +0 -0

index 3105f651614f..ca0546397414 100644
@@ -180,6 +180,16 @@ config SURFACE_PRO3_BUTTON
  	help
  	  This driver handles the power/home/volume buttons on the Microsoft Surface Pro 3/4 tablet.

Introduce support for the Extensible Boot Loader driver found on the Surface Duo. Makes device information available to users via sysfs.

Signed-off-by Jarrett Schultz <[email protected]>

---
  .../ABI/testing/sysfs-platform-surface-xbl    |  78 ++++++
  MAINTAINERS                                   |   9 +
  drivers/platform/surface/Kconfig              |  10 +
  drivers/platform/surface/Makefile             |   1 +
  drivers/platform/surface/surface-xbl.c        | 223 ++++++++++++++++++
  5 files changed, 321 insertions(+)

create mode 100644 Documentation/ABI/testing/sysfs-platform-surface-xbl create mode 100644 drivers/platform/surface/surface-xbl.c

Documentation/ABI/testing/sysfs-platform-surface-xbl new file +78 -0

new file mode 100644
index 000000000000..d3104dbbc6c1
@@ -0,0 +1,78 @@
+What:		/sys/devices/platform/146bfa94.xbl/battery_present
+Date:		October 2021
+KernelVersion:	5.15
+Contact:	[email protected]
+Description:
+		Read only. It returns whether the battery is present. Valid
+		values are:
+			0 - battery absent
+			1 - battery present
+
+What:		/sys/devices/platform/146bfa94.xbl/board_id
+Date:		October 2021
+KernelVersion:	5.15
+Contact:	[email protected]
+Description:
+		Read only. It returns the board id.
+
+What:		/sys/devices/platform/146bfa94.xbl/hw_init_retries
+Date:		October 2021
+KernelVersion:	5.15
+Contact:	[email protected]
+Description:
+		Read only. It returns retries attempted to initialize the
+		discrete hardware circuit.
+
+What:		/sys/devices/platform/146bfa94.xbl/is_act_mode
+Date:		October 2021
+KernelVersion:	5.15
+Contact:	[email protected]
+Description:
+		Read only. It returns whether ACT mode is enabled. Valid values
+		are:
+			0 - ACT disabled
+			1 - ACT enabled
+
+		ACT mode is used to run checks and put the device to shipmode
+		at factory.
+
+What:		/sys/devices/platform/146bfa94.xbl/is_customer_mode
+Date:		October 2021
+KernelVersion:	5.15
+Contact:	[email protected]
+Description:
+		Read only. It returns whether the device is in manufacturing
+		mode. Valid values are:
+			0 - Not in manufacturing mode
+			1 - In manufacturing mode
+
+What:		/sys/devices/platform/146bfa94.xbl/ocp_error_location
+Date:		October 2021
+KernelVersion:	5.15
+Contact:	[email protected]
+Description:
+		Read only. It returns 0 or which power rail has the OCP error.
+		Valid values are:
+			Bit(s)		Meaning
+			15		More than one OCP error occurred
+			14-12		PMIC
+			11-7		SMPS
+			6-2		LDO
+			1-0		BOB
+
+What:		/sys/devices/platform/146bfa94.xbl/pmic_reset_reason
+Date:		October 2021
+KernelVersion:	5.15
+Contact:	[email protected]
+Description:
+		Read only. It returns the reason for the reset. Valid values
+		are:
+			0 - no reason lol
+			9 - Battery driver triggered
+
+What:		/sys/devices/platform/146bfa94.xbl/touch_fw_version
+Date:		October 2021
+KernelVersion:	5.15
+Contact:	[email protected]
+Description:
+		Read only. It returns the version of the firmware.

MAINTAINERS +9 -0

index eeb4c70b3d5b..d08b68d626f6 100644
@@ -12423,6 +12423,15 @@ F:	Documentation/driver-api/surface_aggregator/clients/dtx.rst
  F:	drivers/platform/surface/surface_dtx.c
  F:	include/uapi/linux/surface_aggregator/dtx.h
 
+MICROSOFT SURFACE DUO XBL DRIVER
+M:	Jarrett Schultz <[email protected]>
+L:	[email protected]
+L:	[email protected]
+S:	Supported
+F:	Documentation/ABI/testing/sysfs-platform-surface-xbl
+F:	Documentation/devicetree/bindings/platform/microsoft/surface-xbl.yaml
+F:	drivers/platform/surface/surface-xbl.c
+
  MICROSOFT SURFACE GPE LID SUPPORT DRIVER
  M:	Maximilian Luz <[email protected]>
  L:	[email protected]

drivers/platform/surface/Kconfig +10 -0

index 3105f651614f..ca0546397414 100644
@@ -180,6 +180,16 @@ config SURFACE_PRO3_BUTTON
  	help
  	  This driver handles the power/home/volume buttons on the Microsoft Surface Pro 3/4 tablet.
 
+config SURFACE_XBL
+	tristate "Surface XBL Driver"
+	depends on ARM64 || COMPILE_TEST
+	help
+	  If you say 'Y' to this option, support will be included for the
+	  Surface XBL Driver.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called surface-xbl.
+
  source "drivers/platform/surface/aggregator/Kconfig"
 
  endif # SURFACE_PLATFORMS

drivers/platform/surface/Makefile +1 -0

index 32889482de55..1ed5808301e9 100644
@@ -16,3 +16,4 @@ obj-$(CONFIG_SURFACE_GPE)		+= surface_gpe.o
  obj-$(CONFIG_SURFACE_HOTPLUG)		+= surface_hotplug.o
  obj-$(CONFIG_SURFACE_PLATFORM_PROFILE)	+= surface_platform_profile.o
  obj-$(CONFIG_SURFACE_PRO3_BUTTON)	+= surfacepro3_button.o
+obj-$(CONFIG_SURFACE_XBL)		+= surface-xbl.o

drivers/platform/surface/surface-xbl.c new file +223 -0

new file mode 100644
index 000000000000..910287f0c987
@@ -0,0 +1,223 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * surface-xbl.c - Surface E(x)tensible (B)oot(l)oader
+ *
+ * Copyright (C) 2021 Microsoft Corporation
+ * Author: Jarrett Schultz <[email protected]>
+ */
+
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/kstrtox.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#define SURFACE_XBL_MAX_VERSION_LEN	16
+#define SURFACE_XBL_BOARD_ID		0
+#define SURFACE_XBL_BATTERY_PRESENT	1
+#define SURFACE_XBL_HW_INIT_RETRIES	2
+#define SURFACE_XBL_IS_CUSTOMER_MODE	3
+#define SURFACE_XBL_IS_ACT_MODE		4
+#define SURFACE_XBL_PMIC_RESET_REASON	5
+#define SURFACE_XBL_TOUCH_FW_VERSION	6
+#define SURFACE_XBL_OCP_ERROR_LOCATION	((SURFACE_XBL_TOUCH_FW_VERSION) +\
+					(SURFACE_XBL_MAX_VERSION_LEN))
+
+struct surface_xbl {
+	struct device	*dev;
+	void __iomem	*regs;
+
+	u8		board_id;
+	u8		battery_present;
+	u8		hw_init_retries;
+	u8		is_customer_mode;
+	u8		is_act_mode;
+	u8		pmic_reset_reason;
+	char		touch_fw_version[SURFACE_XBL_MAX_VERSION_LEN];
+	u16		ocp_error_location;
+} __packed;
+
+static ssize_t
+board_id_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct surface_xbl	*sxbl = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%d\n", sxbl->board_id);
+}
+static DEVICE_ATTR_RO(board_id);
+
+static ssize_t
+battery_present_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct surface_xbl	*sxbl = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%d\n", sxbl->battery_present);
+}
+static DEVICE_ATTR_RO(battery_present);
+
+static ssize_t
+hw_init_retries_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct surface_xbl	*sxbl = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%d\n", sxbl->hw_init_retries);
+}
+static DEVICE_ATTR_RO(hw_init_retries);
+
+static ssize_t
+is_customer_mode_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct surface_xbl	*sxbl = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%d\n", sxbl->is_customer_mode);
+}
+static DEVICE_ATTR_RO(is_customer_mode);
+
+static ssize_t
+is_act_mode_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct surface_xbl	*sxbl = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%d\n", sxbl->is_act_mode);
+}
+static DEVICE_ATTR_RO(is_act_mode);
+
+static ssize_t
+pmic_reset_reason_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct surface_xbl	*sxbl = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%d\n", sxbl->pmic_reset_reason);
+}
+static DEVICE_ATTR_RO(pmic_reset_reason);
+
+static ssize_t
+touch_fw_version_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct surface_xbl	*sxbl = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "0x%s\n", sxbl->touch_fw_version);
+}
+static DEVICE_ATTR_RO(touch_fw_version);
+
+static ssize_t
+ocp_error_location_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct surface_xbl	*sxbl = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%d\n", sxbl->ocp_error_location);
+}
+static DEVICE_ATTR_RO(ocp_error_location);
+
+static struct attribute *inputs_attrs[] = {
+	&dev_attr_board_id.attr,
+	&dev_attr_battery_present.attr,
+	&dev_attr_hw_init_retries.attr,
+	&dev_attr_is_customer_mode.attr,
+	&dev_attr_is_act_mode.attr,
+	&dev_attr_pmic_reset_reason.attr,
+	&dev_attr_touch_fw_version.attr,
+	&dev_attr_ocp_error_location.attr,
+	NULL,
+};
+
+static const struct attribute_group inputs_attr_group = {
+	.attrs = inputs_attrs,
+};
+
+static u8 surface_xbl_readb(void __iomem *base, u32 offset)
+{
+	return readb(base + offset);
+}
+
+static u16 surface_xbl_readw(void __iomem *base, u32 offset)
+{
+	return readw(base + offset);
+}
+
+static int surface_xbl_probe(struct platform_device *pdev)
+{
+	struct surface_xbl	*sxbl;
+	struct device		*dev;
+	void __iomem		*regs;
+	int					index;
+	int					retval;
+
+	dev = &pdev->dev;
+	sxbl = devm_kzalloc(dev, sizeof(*sxbl), GFP_KERNEL);
+	if (!sxbl)
+		return -ENOMEM;
+
+	sxbl->dev = dev;
+
+	regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(regs))
+		return PTR_ERR(regs);
+
+	sxbl->regs = regs;
+
+	platform_set_drvdata(pdev, sxbl);
+
+	sxbl->board_id = surface_xbl_readb(sxbl->regs,
+					   SURFACE_XBL_BOARD_ID);
+	sxbl->battery_present = surface_xbl_readb(sxbl->regs,
+						  SURFACE_XBL_BATTERY_PRESENT);
+	sxbl->hw_init_retries = surface_xbl_readb(sxbl->regs,
+						  SURFACE_XBL_HW_INIT_RETRIES);
+	sxbl->is_customer_mode = surface_xbl_readb(sxbl->regs,
+						   SURFACE_XBL_IS_CUSTOMER_MODE);
+	sxbl->is_act_mode = surface_xbl_readb(sxbl->regs,
+					      SURFACE_XBL_IS_ACT_MODE);
+	sxbl->pmic_reset_reason = surface_xbl_readb(sxbl->regs,
+						    SURFACE_XBL_PMIC_RESET_REASON);
+
+	for (index = 0; index < SURFACE_XBL_MAX_VERSION_LEN; index++)
+		sxbl->touch_fw_version[index] = surface_xbl_readb(sxbl->regs,
+							SURFACE_XBL_TOUCH_FW_VERSION + index);
+
+	sxbl->ocp_error_location = surface_xbl_readw(sxbl->regs,
+						     SURFACE_XBL_OCP_ERROR_LOCATION);
+
+	retval = sysfs_create_group(&sxbl->dev->kobj, &inputs_attr_group);
+	if (retval < 0) {
+		dev_dbg(sxbl->dev,
+			"Can't register sysfs attr group: %d\n", retval);
+		return retval;
+	}
+
+	return 0;
+}
+
+static int surface_xbl_remove(struct platform_device *pdev)
+{
+	sysfs_remove_group(&pdev->dev.kobj, &inputs_attr_group);
+
+	return 0;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id surface_xbl_of_match[] = {
+	{
+		.compatible = "microsoft,sm8150-surface-duo-xbl"
+	},
+	{  }, /* Terminating Entry */
+};
+MODULE_DEVICE_TABLE(of, surface_xbl_of_match);
+#endif
+
+static struct platform_driver surface_xbl_driver = {
+	.probe		= surface_xbl_probe,
+	.remove		= surface_xbl_remove,
+	.driver		= {
+		.name	= "surface-xbl",
+		.of_match_table = of_match_ptr(surface_xbl_of_match),
+	},
+};
+
+module_platform_driver(surface_xbl_driver);
+
+MODULE_AUTHOR("Jarrett Schultz <[email protected]>");
+MODULE_DESCRIPTION("Surface Extensible Bootloader");
+MODULE_LICENSE("GPL");

diff --git a/drivers/platform/surface/surface-xbl.c b/drivers/platform/surface/surface-xbl.c new file mode 100644
index 000000000000..910287f0c987
--- /dev/null
+++ b/drivers/platform/surface/surface-xbl.c
@@ -0,0 +1,223 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * surface-xbl.c - Surface E(x)tensible (B)oot(l)oader
+ *

First of all, do not include filename in the file.

Capital L will be better to read and understand the abbreviation. Actually usually we do something like this:

Extensible Boot Loader (EBL)

+static const struct attribute_group inputs_attr_group = {
+ .attrs = inputs_attrs,
+};
+
+static u8 surface_xbl_readb(void __iomem *base, u32 offset) +{
+ return readb(base + offset);
+}
+
+static u16 surface_xbl_readw(void __iomem *base, u32 offset) +{
+ return readw(base + offset);
+}

Either use corresponding io accessors in-line, or make first parameter to be sirface_xbl pointer. Otherwise these helpers useless.

Andy Shevchenko <[email protected]> writes:

Quoted 4 lines

Capital L will be better to read and understand the abbreviation. Actually usually we do something like this:

Extensible Boot Loader (EBL)

nah, this is silly Andy. It's just capitalized as eXtensible Boot Loader, very much akin to eXtensible Host Controller Interface.

Quoted 14 lines

+static const struct attribute_group inputs_attr_group = {
+ .attrs = inputs_attrs,
+};
+
+static u8 surface_xbl_readb(void __iomem *base, u32 offset) +{
+ return readb(base + offset);
+}
+
+static u16 surface_xbl_readw(void __iomem *base, u32 offset) +{
+ return readw(base + offset);
+}

Quoted 2 lines

Either use corresponding io accessors in-line, or make first parameter to be sirface_xbl pointer. Otherwise these helpers useless.

I agree with passing surface_xbl point as first parameter, but calling the accessors pointless is a bit much. At a minimum, they make it easier to ftrace the entire driver by simply ftracing surface_xbl_*

Quoted 1 line · Felipe Balbi

Andy Shevchenko <[email protected]> writes:

...

Quoted 3 lines

Quoted 3 lines

Capital L will be better to read and understand the abbreviation. Actually usually we do something like this:

Quoted 4 lines

Quoted 1 line

Extensible Boot Loader (EBL)

nah, this is silly Andy. It's just capitalized as eXtensible Boot Loader, very much akin to eXtensible Host Controller Interface.

My point here is to have a full name followed by the abbreviation. and n(O)t in (F)ancy st(Y)le.

Quoted 17 lines

Quoted 16 lines

+static const struct attribute_group inputs_attr_group = {
+ .attrs = inputs_attrs,
+};
+
+static u8 surface_xbl_readb(void __iomem *base, u32 offset) +{
+ return readb(base + offset);
+}
+
+static u16 surface_xbl_readw(void __iomem *base, u32 offset) +{
+ return readw(base + offset);
+}

Either use corresponding io accessors in-line, or make first parameter to be sirface_xbl pointer. Otherwise these helpers useless.

Quoted 3 lines

I agree with passing surface_xbl point as first parameter, but calling the accessors pointless is a bit much. At a minimum, they make it easier to ftrace the entire driver by simply ftracing surface_xbl_*

My point is that the above seems half-baked. It's pointless to have a func(a,b) { return readl(a + b); }. It doesn't add value.

Quoted 2 lines · Andy Shevchenko

Quoted 1 line · Felipe Balbi

Andy Shevchenko <[email protected]> writes:

Quoted 10 lines

Quoted 7 lines

Quoted 4 lines

Capital L will be better to read and understand the abbreviation. Actually usually we do something like this:

Extensible Boot Loader (EBL)

nah, this is silly Andy. It's just capitalized as eXtensible Boot Loader, very much akin to eXtensible Host Controller Interface.

My point here is to have a full name followed by the abbreviation. and n(O)t in (F)ancy st(Y)le.

too bad my patch removing acronyms from the kernel got rejects :-p

Seriously, this is pretty pointless. You're vouching for something that will just cause confusion. Every piece of internal documentation refers to xbl and you want this to be renamed to ebl because it looks nicer for you. Thanks, but no thanks.

Quoted 21 lines

Quoted 20 lines

Quoted 14 lines

+static const struct attribute_group inputs_attr_group = {
+ .attrs = inputs_attrs,
+};
+
+static u8 surface_xbl_readb(void __iomem *base, u32 offset) +{
+ return readb(base + offset);
+}
+
+static u16 surface_xbl_readw(void __iomem *base, u32 offset) +{
+ return readw(base + offset);
+}

Quoted 2 lines

Either use corresponding io accessors in-line, or make first parameter to be sirface_xbl pointer. Otherwise these helpers useless.

I agree with passing surface_xbl point as first parameter, but calling the accessors pointless is a bit much. At a minimum, they make it easier to ftrace the entire driver by simply ftracing surface_xbl_*

Quoted 2 lines

My point is that the above seems half-baked. It's pointless to have a func(a,b) { return readl(a + b); }. It doesn't add value.

sure it does. echo surface_xbl_* > ftrace_filter_function (or whatever the filename was) it reason enough IMHO. Not to mention that these little accessors will likely be optimized by the compiler.

Quoted 11 lines

Quoted 10 lines

Quoted 7 lines

Quoted 4 lines

Capital L will be better to read and understand the abbreviation. Actually usually we do something like this:

Extensible Boot Loader (EBL)

nah, this is silly Andy. It's just capitalized as eXtensible Boot Loader, very much akin to eXtensible Host Controller Interface.

My point here is to have a full name followed by the abbreviation. and n(O)t in (F)ancy st(Y)le.

Quoted 6 lines

too bad my patch removing acronyms from the kernel got rejects :-p

Seriously, this is pretty pointless. You're vouching for something that will just cause confusion. Every piece of internal documentation refers to xbl and you want this to be renamed to ebl because it looks nicer for you. Thanks, but no thanks.

Maybe I was too unclear. I'm not pushing for EBL, I'm pushing for the form os

"Foo bAr BullSh*t (FABS)" vs. "(F)oo b(a)r (B)ull(s)h*t".

If you have x there to be capitalized, do it like "eXtensible Boot Loader (XBL)". Is it too hard?

Quoted 24 lines

Quoted 23 lines

Quoted 14 lines

Quoted 14 lines

+static const struct attribute_group inputs_attr_group = {
+ .attrs = inputs_attrs,
+};
+
+static u8 surface_xbl_readb(void __iomem *base, u32 offset) +{
+ return readb(base + offset);
+}
+
+static u16 surface_xbl_readw(void __iomem *base, u32 offset) +{
+ return readw(base + offset);
+}

Quoted 6 lines

Quoted 2 lines

Either use corresponding io accessors in-line, or make first parameter to be sirface_xbl pointer. Otherwise these helpers useless.

I agree with passing surface_xbl point as first parameter, but calling the accessors pointless is a bit much. At a minimum, they make it easier to ftrace the entire driver by simply ftracing surface_xbl_*

My point is that the above seems half-baked. It's pointless to have a func(a,b) { return readl(a + b); }. It doesn't add value.

Quoted 3 lines

sure it does. echo surface_xbl_* > ftrace_filter_function (or whatever the filename was) it reason enough IMHO. Not to mention that these little accessors will likely be optimized by the compiler.

readl() will appear in the traces, no? But yeah I also was thinking about the weakness in your argument that the compiler can silently inline them anyway.

Andy Shevchenko <[email protected]> writes:

Take a breather Andy, you need it. Winter sure is coming