* [PATCH 0/2] Add support for Microsoft Surface 3 power
@ 2016-07-29 15:49 Benjamin Tissoires
2016-07-29 15:49 ` [PATCH 1/2] ACPICA: adapt buffer length for Field Attrib Raw Process in Ops Region Benjamin Tissoires
2016-07-29 15:49 ` [PATCH 2/2] power: surface3_power: MSHW0011 rev-eng implementation Benjamin Tissoires
0 siblings, 2 replies; 11+ messages in thread
From: Benjamin Tissoires @ 2016-07-29 15:49 UTC (permalink / raw)
To: Stephen Just, Robert Moore, Lv Zheng, Rafael J. Wysocki,
Len Brown, Sebastian Reichel, Dmitry Eremin-Solenikov,
David Woodhouse
Cc: linux-acpi, devel, linux-pm, linux-kernel
Hi,
The MS Surface 3 is a reduced platform which doesn't use a regular chip
to report the battery information. Instead, they use ACPI Operation Region
to do the processing in the MSHW0011 driver.
This series aims at adding a reverse-engineered driver.
I am not sure whether the ACPICA patch is required because of a bug in the
ACPICA parsing or if MS just screwed up the DSDT. Anyway, it's there
and was judged as acceptable while we find out the exact issue (in
https://bugzilla.kernel.org/show_bug.cgi?id=106231 )
Cheers,
Benjamin
Benjamin Tissoires (2):
ACPICA: adapt buffer length for Field Attrib Raw Process in Ops Region
power: surface3_power: MSHW0011 rev-eng implementation
drivers/acpi/acpica/exfield.c | 11 +
drivers/power/Kconfig | 8 +
drivers/power/Makefile | 1 +
drivers/power/surface3_power.c | 724 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 744 insertions(+)
create mode 100644 drivers/power/surface3_power.c
--
2.5.5
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] ACPICA: adapt buffer length for Field Attrib Raw Process in Ops Region
2016-07-29 15:49 [PATCH 0/2] Add support for Microsoft Surface 3 power Benjamin Tissoires
@ 2016-07-29 15:49 ` Benjamin Tissoires
2016-08-01 5:58 ` Zheng, Lv
2016-07-29 15:49 ` [PATCH 2/2] power: surface3_power: MSHW0011 rev-eng implementation Benjamin Tissoires
1 sibling, 1 reply; 11+ messages in thread
From: Benjamin Tissoires @ 2016-07-29 15:49 UTC (permalink / raw)
To: Stephen Just, Robert Moore, Lv Zheng, Rafael J. Wysocki,
Len Brown, Sebastian Reichel, Dmitry Eremin-Solenikov,
David Woodhouse
Cc: linux-acpi, devel, linux-pm, linux-kernel
Detected on the Surface 3:
The MSHW0011 driver uses Field Attrib Raw Process to return information
for the ACPI Battery. The DSDT declares a parameter of 2 though
functions like _BST or _BIX require a much bigger out buffer.
It looks like the incoming buffer has the requested size and we can
work around the issue by using this input size as the output and
parameters size.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=106231
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/acpi/acpica/exfield.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/acpi/acpica/exfield.c b/drivers/acpi/acpica/exfield.c
index d7d3ee3..a1bd041 100644
--- a/drivers/acpi/acpica/exfield.c
+++ b/drivers/acpi/acpica/exfield.c
@@ -413,6 +413,17 @@ acpi_ex_write_data_to_field(union acpi_operand_object *source_desc,
* Data[x-1]: (Bytes 2-x of the arbitrary length data buffer)
*/
length += 2;
+
+ /*
+ * When using Field Attrib Raw Process, it looks like
+ * the parameter access_length can be wrong and the
+ * required output buffer can be much bigger.
+ * So just take the incoming buffer length as the
+ * reference.
+ */
+ if (accessor_type == AML_FIELD_ATTRIB_RAW_PROCESS)
+ length = source_desc->buffer.length;
+
function = ACPI_WRITE | (accessor_type << 16);
} else { /* IPMI */
--
2.5.5
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] power: surface3_power: MSHW0011 rev-eng implementation
2016-07-29 15:49 [PATCH 0/2] Add support for Microsoft Surface 3 power Benjamin Tissoires
2016-07-29 15:49 ` [PATCH 1/2] ACPICA: adapt buffer length for Field Attrib Raw Process in Ops Region Benjamin Tissoires
@ 2016-07-29 15:49 ` Benjamin Tissoires
2016-07-29 19:51 ` kbuild test robot
` (5 more replies)
1 sibling, 6 replies; 11+ messages in thread
From: Benjamin Tissoires @ 2016-07-29 15:49 UTC (permalink / raw)
To: Stephen Just, Robert Moore, Lv Zheng, Rafael J. Wysocki,
Len Brown, Sebastian Reichel, Dmitry Eremin-Solenikov,
David Woodhouse
Cc: linux-acpi, devel, linux-pm, linux-kernel
MSHW0011 replaces the battery firmware by using ACPI operation regions.
The values have been obtained by reverse engineering, and are subject to
errors. Looks like it works on overall pretty well.
I couldn't manage to get the IRQ correctly triggered, so I am using a
good old polling thread to check for changes.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=106231
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Stephen Just <stephenjust@gmail.com>
---
drivers/power/Kconfig | 8 +
drivers/power/Makefile | 1 +
drivers/power/surface3_power.c | 724 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 733 insertions(+)
create mode 100644 drivers/power/surface3_power.c
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 421770d..fb54d8b 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -509,6 +509,14 @@ config AXP20X_POWER
This driver provides support for the power supply features of
AXP20x PMIC.
+config SURFACE3_POWER
+ tristate "Surface 3 power management driver"
+ depends on I2C
+ depends on GPIOLIB
+ help
+ This driver provides support for the power supply features of
+ the Surface 3.
+
endif # POWER_SUPPLY
source "drivers/power/reset/Kconfig"
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index e46b75d..a147a0e 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -74,3 +74,4 @@ obj-$(CONFIG_CHARGER_TPS65217) += tps65217_charger.o
obj-$(CONFIG_POWER_RESET) += reset/
obj-$(CONFIG_AXP288_FUEL_GAUGE) += axp288_fuel_gauge.o
obj-$(CONFIG_AXP288_CHARGER) += axp288_charger.o
+obj-$(CONFIG_SURFACE3_POWER) += surface3_power.o
diff --git a/drivers/power/surface3_power.c b/drivers/power/surface3_power.c
new file mode 100644
index 0000000..bbfaa25
--- /dev/null
+++ b/drivers/power/surface3_power.c
@@ -0,0 +1,724 @@
+/*
+ * Supports for the power IC on the Surface 3 tablet.
+ *
+ * (C) Copyright 2016 Red Hat, Inc
+ * (C) Copyright 2016 Benjamin Tissoires <benjamin.tissoires@gmail.com>
+ * (C) Copyright 2016 Stephen Just <stephenjust@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2
+ * of the License.
+ */
+
+/*
+ * This driver has been reverse-engineered by parsing the DSDT of the Surface 3
+ * and looking at the registers of the chips.
+ *
+ * The DSDT allowed to find out that:
+ * - the driver is required for the ACPI BAT0 device to communicate to the chip
+ * through an operation region.
+ * - the various defines for the operation region functions to communicate with
+ * this driver
+ * - the DSM 3f99e367-6220-4955-8b0f-06ef2ae79412 allows to trigger ACPI
+ * events to BAT0 (the code is all available in the DSDT).
+ *
+ * Further findings regarding the 2 chips declared in the MSHW0011 are:
+ * - there are 2 chips declared:
+ * . 0x22 seems to control the ADP1 line status (and probably the charger)
+ * . 0x55 controls the battery directly
+ * - the battery chip uses a SMBus protocol (using plain SMBus allows non
+ * destructive commands):
+ * . the commands/registers used are in the range 0x00..0x7F
+ * . if bit 8 (0x80) is set in the SMBus command, the returned value is the
+ * same as when it is not set. There is a high chance this bit is the
+ * read/write
+ * . the various registers semantic as been deduced by observing the register
+ * dumps.
+ */
+
+#include <linux/kernel.h>
+#include <linux/i2c.h>
+#include <linux/slab.h>
+#include <linux/acpi.h>
+#include <linux/kthread.h>
+#include <linux/freezer.h>
+
+#include <asm/unaligned.h>
+
+#define POLL_INTERVAL (HZ * 2)
+
+static bool dump_registers;
+module_param_named(dump_registers, dump_registers, bool, 0644);
+MODULE_PARM_DESC(dump_registers,
+ "Dump the SMBus register at probe (debugging only).");
+
+struct mshw0011_data {
+ struct i2c_client *adp1;
+ struct i2c_client *bat0;
+ unsigned short notify_version;
+ struct task_struct *poll_task;
+ bool kthread_running;
+
+ bool charging;
+ bool bat_charging;
+ u8 trip_point;
+ s32 full_capacity;
+};
+
+struct mshw0011_lookup {
+ struct mshw0011_data *cdata;
+ unsigned int n;
+ unsigned int index;
+ int addr;
+};
+
+struct mshw0011_handler_data {
+ struct acpi_connection_info info;
+ struct i2c_client *client;
+};
+
+struct bix {
+ u32 revision;
+ u32 power_unit;
+ u32 design_capacity;
+ u32 last_full_charg_capacity;
+ u32 battery_technology;
+ u32 design_voltage;
+ u32 design_capacity_of_warning;
+ u32 design_capacity_of_low;
+ u32 cycle_count;
+ u32 measurement_accuracy;
+ u32 max_sampling_time;
+ u32 min_sampling_time;
+ u32 max_average_interval;
+ u32 min_average_interval;
+ u32 battery_capacity_granularity_1;
+ u32 battery_capacity_granularity_2;
+ char model[10];
+ char serial[10];
+ char type[10];
+ char OEM[10];
+} __packed;
+
+struct bst {
+ u32 battery_state;
+ s32 battery_present_rate;
+ u32 battery_remaining_capacity;
+ u32 battery_present_voltage;
+} __packed;
+
+struct gsb_command {
+ u8 arg0;
+ u8 arg1;
+ u8 arg2;
+} __packed;
+
+struct gsb_buffer {
+ u8 status;
+ u8 len;
+ u8 ret;
+ union {
+ struct gsb_command cmd;
+ struct bst bst;
+ struct bix bix;
+ } __packed;
+} __packed;
+
+#define ACPI_BATTERY_STATE_DISCHARGING 0x1
+#define ACPI_BATTERY_STATE_CHARGING 0x2
+#define ACPI_BATTERY_STATE_CRITICAL 0x4
+
+#define MSHW0011_CMD_DEST_BAT0 0x01
+#define MSHW0011_CMD_DEST_ADP1 0x03
+
+#define MSHW0011_CMD_BAT0_STA 0x01
+#define MSHW0011_CMD_BAT0_BIX 0x02
+#define MSHW0011_CMD_BAT0_BCT 0x03
+#define MSHW0011_CMD_BAT0_BTM 0x04
+#define MSHW0011_CMD_BAT0_BST 0x05
+#define MSHW0011_CMD_BAT0_BTP 0x06
+#define MSHW0011_CMD_ADP1_PSR 0x07
+#define MSHW0011_CMD_BAT0_PSOC 0x09
+#define MSHW0011_CMD_BAT0_PMAX 0x0A
+#define MSHW0011_CMD_BAT0_PSRC 0x0B
+#define MSHW0011_CMD_BAT0_CHGI 0x0C
+#define MSHW0011_CMD_BAT0_ARTG 0x0D
+
+#define MSHW0011_NOTIFY_GET_VERSION 0x00
+#define MSHW0011_NOTIFY_ADP1 0x01
+#define MSHW0011_NOTIFY_BAT0_BST 0x02
+#define MSHW0011_NOTIFY_BAT0_BIX 0x05
+
+#define MSHW0011_ADP1_REG_PSR 0x04
+
+#define MSHW0011_BAT0_REG_CAPACITY 0x0c
+#define MSHW0011_BAT0_REG_FULL_CHG_CAPACITY 0x0e
+#define MSHW0011_BAT0_REG_DESIGN_CAPACITY 0x40
+#define MSHW0011_BAT0_REG_VOLTAGE 0x08
+#define MSHW0011_BAT0_REG_RATE 0x14
+#define MSHW0011_BAT0_REG_OEM 0x45
+#define MSHW0011_BAT0_REG_TYPE 0x4e
+#define MSHW0011_BAT0_REG_SERIAL_NO 0x56
+#define MSHW0011_BAT0_REG_CYCLE_CNT 0x6e
+
+#define MSHW0011_EV_2_5 0x1ff
+
+static int mshw0011_i2c_read_block(struct i2c_client *client, u8 reg, u8 *buf,
+ int len)
+{
+ int status, i;
+
+ for (i = 0; i < len; i++) {
+ status = i2c_smbus_read_byte_data(client, reg + i);
+ if (status < 0) {
+ buf[i] = 0xff;
+ continue;
+ }
+
+ buf[i] = (u8)status;
+ }
+
+ return 0;
+}
+
+static int
+mshw0011_notify(struct mshw0011_data *cdata, u8 arg1, u8 arg2,
+ unsigned int *ret_value)
+{
+ static const u8 mshw0011_guid[] = {
+ 0x67, 0xE3, 0x99, 0x3F, 0x20, 0x62, 0x55, 0x49,
+ 0x8b, 0x0f, 0x06, 0xef, 0x2a, 0xe7, 0x94, 0x12,
+ };
+ union acpi_object *obj;
+ struct acpi_device *adev;
+ acpi_handle handle;
+ unsigned int i;
+
+ handle = ACPI_HANDLE(&cdata->adp1->dev);
+ if (!handle || acpi_bus_get_device(handle, &adev))
+ return -ENODEV;
+
+ obj = acpi_evaluate_dsm_typed(handle, mshw0011_guid, arg1, arg2, NULL,
+ ACPI_TYPE_BUFFER);
+ if (!obj) {
+ dev_err(&cdata->adp1->dev, "device _DSM execution failed\n");
+ return -ENODEV;
+ }
+
+ *ret_value = 0;
+ for (i = 0; i < obj->buffer.length; i++)
+ *ret_value |= obj->buffer.pointer[i] << (i * 8);
+
+ ACPI_FREE(obj);
+ return 0;
+}
+
+static const struct bix default_bix = {
+ .revision = 0x00,
+ .power_unit = 0x01,
+ .design_capacity = 0x1dca,
+ .last_full_charg_capacity = 0x1dca,
+ .battery_technology = 0x01,
+ .design_voltage = 0x10df,
+ .design_capacity_of_warning = 0x8f,
+ .design_capacity_of_low = 0x47,
+ .cycle_count = 0xffffffff,
+ .measurement_accuracy = 0x00015F90,
+ .max_sampling_time = 0x03E8,
+ .min_sampling_time = 0x03E8,
+ .max_average_interval = 0x03E8,
+ .min_average_interval = 0x03E8,
+ .battery_capacity_granularity_1 = 0x45,
+ .battery_capacity_granularity_2 = 0x11,
+ .model = "P11G8M",
+ .serial = "",
+ .type = "LION",
+ .OEM = "",
+};
+
+static int mshw0011_bix(struct mshw0011_data *cdata, struct bix *bix)
+{
+ struct i2c_client *client = cdata->bat0;
+ int ret;
+ char buf[10];
+
+ *bix = default_bix;
+
+ /* get design capacity */
+ ret = i2c_smbus_read_word_data(client,
+ MSHW0011_BAT0_REG_DESIGN_CAPACITY);
+ if (ret < 0) {
+ dev_err(&client->dev, "Error reading design capacity: %d\n",
+ ret);
+ return ret;
+ }
+ bix->design_capacity = le16_to_cpu(ret);
+
+ /* get last full charge capacity */
+ ret = i2c_smbus_read_word_data(client,
+ MSHW0011_BAT0_REG_FULL_CHG_CAPACITY);
+ if (ret < 0) {
+ dev_err(&client->dev,
+ "Error reading last full charge capacity: %d\n", ret);
+ return ret;
+ }
+ bix->last_full_charg_capacity = le16_to_cpu(ret);
+
+ /* get serial number */
+ ret = mshw0011_i2c_read_block(client, MSHW0011_BAT0_REG_SERIAL_NO,
+ buf, 10);
+ if (ret) {
+ dev_err(&client->dev, "Error reading serial no: %d\n", ret);
+ return ret;
+ }
+ memcpy(bix->serial, buf + 7, 3);
+ memcpy(bix->serial + 3, buf, 6);
+ bix->serial[9] = '\0';
+
+ /* get cycle count */
+ ret = i2c_smbus_read_word_data(client, MSHW0011_BAT0_REG_CYCLE_CNT);
+ if (ret < 0) {
+ dev_err(&client->dev, "Error reading cycle count: %d\n", ret);
+ return ret;
+ }
+ bix->cycle_count = le16_to_cpu(ret);
+
+ /* get OEM name */
+ ret = mshw0011_i2c_read_block(client, MSHW0011_BAT0_REG_OEM, buf, 4);
+ if (ret) {
+ dev_err(&client->dev, "Error reading cycle count: %d\n", ret);
+ return ret;
+ }
+ memcpy(bix->OEM, buf, 3);
+ bix->OEM[4] = '\0';
+
+ return 0;
+}
+
+static int mshw0011_bst(struct mshw0011_data *cdata, struct bst *bst)
+{
+ struct i2c_client *client = cdata->bat0;
+ int rate, capacity, voltage, state;
+ s16 tmp;
+
+ rate = i2c_smbus_read_word_data(client, MSHW0011_BAT0_REG_RATE);
+ if (rate < 0)
+ return rate;
+
+ capacity = i2c_smbus_read_word_data(client, MSHW0011_BAT0_REG_CAPACITY);
+ if (capacity < 0)
+ return capacity;
+
+ voltage = i2c_smbus_read_word_data(client, MSHW0011_BAT0_REG_VOLTAGE);
+ if (voltage < 0)
+ return voltage;
+
+ tmp = le16_to_cpu(rate);
+ bst->battery_present_rate = abs((s32)tmp);
+
+ state = 0;
+ if ((s32) tmp > 0)
+ state |= ACPI_BATTERY_STATE_CHARGING;
+ else if ((s32) tmp < 0)
+ state |= ACPI_BATTERY_STATE_DISCHARGING;
+ bst->battery_state = state;
+
+ bst->battery_remaining_capacity = le16_to_cpu(capacity);
+ bst->battery_present_voltage = le16_to_cpu(voltage);
+
+ return 0;
+}
+
+static int mshw0011_adp_psr(struct mshw0011_data *cdata)
+{
+ struct i2c_client *client = cdata->adp1;
+ int ret;
+
+ ret = i2c_smbus_read_byte_data(client, MSHW0011_ADP1_REG_PSR);
+ if (ret < 0)
+ return ret;
+
+ return ret;
+}
+
+static int mshw0011_isr(struct mshw0011_data *cdata)
+{
+ struct bst bst;
+ struct bix bix;
+ int ret;
+ bool status, bat_status;
+
+ ret = mshw0011_adp_psr(cdata);
+ if (ret < 0)
+ return ret;
+
+ status = ret;
+
+ if (status != cdata->charging)
+ mshw0011_notify(cdata, cdata->notify_version,
+ MSHW0011_NOTIFY_ADP1, &ret);
+
+ cdata->charging = status;
+
+ ret = mshw0011_bst(cdata, &bst);
+ if (ret < 0)
+ return ret;
+
+ bat_status = bst.battery_state;
+
+ if (bat_status != cdata->bat_charging)
+ mshw0011_notify(cdata, cdata->notify_version,
+ MSHW0011_NOTIFY_BAT0_BST, &ret);
+
+ cdata->bat_charging = bat_status;
+
+ ret = mshw0011_bix(cdata, &bix);
+ if (ret < 0)
+ return ret;
+ if (bix.last_full_charg_capacity != cdata->full_capacity)
+ mshw0011_notify(cdata, cdata->notify_version,
+ MSHW0011_NOTIFY_BAT0_BIX, &ret);
+
+ cdata->full_capacity = bix.last_full_charg_capacity;
+
+ return 0;
+}
+
+static int mshw0011_poll_task(void *data)
+{
+ struct mshw0011_data *cdata = data;
+ int ret = 0;
+
+ cdata->kthread_running = true;
+
+ set_freezable();
+
+ while (!kthread_should_stop()) {
+ schedule_timeout_interruptible(POLL_INTERVAL);
+ try_to_freeze();
+ ret = mshw0011_isr(data);
+ if (ret)
+ goto out;
+ }
+
+out:
+ cdata->kthread_running = false;
+ return ret;
+}
+
+static acpi_status
+mshw0011_space_handler(u32 function, acpi_physical_address command,
+ u32 bits, u64 *value64,
+ void *handler_context, void *region_context)
+{
+ struct gsb_buffer *gsb = (struct gsb_buffer *)value64;
+ struct mshw0011_handler_data *data = handler_context;
+ struct acpi_connection_info *info = &data->info;
+ struct acpi_resource_i2c_serialbus *sb;
+ struct i2c_client *client = data->client;
+ struct mshw0011_data *cdata = i2c_get_clientdata(client);
+ struct acpi_resource *ares;
+ u32 accessor_type = function >> 16;
+ acpi_status ret;
+ int status = 1;
+
+ ret = acpi_buffer_to_resource(info->connection, info->length, &ares);
+ if (ACPI_FAILURE(ret))
+ return ret;
+
+ if (!value64 || ares->type != ACPI_RESOURCE_TYPE_SERIAL_BUS) {
+ ret = AE_BAD_PARAMETER;
+ goto err;
+ }
+
+ sb = &ares->data.i2c_serial_bus;
+ if (sb->type != ACPI_RESOURCE_SERIAL_TYPE_I2C) {
+ ret = AE_BAD_PARAMETER;
+ goto err;
+ }
+
+ if (accessor_type != ACPI_GSB_ACCESS_ATTRIB_RAW_PROCESS) {
+ ret = AE_BAD_PARAMETER;
+ goto err;
+ }
+
+ if (gsb->cmd.arg0 == MSHW0011_CMD_DEST_ADP1 &&
+ gsb->cmd.arg1 == MSHW0011_CMD_ADP1_PSR) {
+ ret = mshw0011_adp_psr(cdata);
+ if (ret >= 0) {
+ status = ret;
+ ret = 0;
+ }
+ goto out;
+ }
+
+ if (gsb->cmd.arg0 != MSHW0011_CMD_DEST_BAT0) {
+ ret = AE_BAD_PARAMETER;
+ goto err;
+ }
+
+ switch (gsb->cmd.arg1) {
+ case MSHW0011_CMD_BAT0_STA:
+ status = 1;
+ ret = 0;
+ break;
+ case MSHW0011_CMD_BAT0_BIX:
+ status = 1;
+ ret = mshw0011_bix(cdata, &gsb->bix);
+ break;
+ case MSHW0011_CMD_BAT0_BTP:
+ status = 1;
+ ret = 0;
+ cdata->trip_point = gsb->cmd.arg2;
+ break;
+ case MSHW0011_CMD_BAT0_BST:
+ status = 1;
+ ret = mshw0011_bst(cdata, &gsb->bst);
+ break;
+ default:
+ pr_info("command(0x%02x) is not supported.\n", gsb->cmd.arg1);
+ ret = AE_BAD_PARAMETER;
+ goto err;
+ }
+
+ out:
+ gsb->ret = status;
+ gsb->status = 0;
+
+ err:
+ ACPI_FREE(ares);
+ return ret;
+}
+
+static int mshw0011_install_space_handler(struct i2c_client *client)
+{
+ acpi_handle handle;
+ struct mshw0011_handler_data *data;
+ acpi_status status;
+
+ handle = ACPI_HANDLE(&client->dev);
+
+ if (!handle)
+ return -ENODEV;
+
+ data = kzalloc(sizeof(struct mshw0011_handler_data),
+ GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->client = client;
+ status = acpi_bus_attach_private_data(handle, (void *)data);
+ if (ACPI_FAILURE(status)) {
+ kfree(data);
+ return -ENOMEM;
+ }
+
+ status = acpi_install_address_space_handler(handle,
+ ACPI_ADR_SPACE_GSBUS,
+ &mshw0011_space_handler,
+ NULL,
+ data);
+ if (ACPI_FAILURE(status)) {
+ dev_err(&client->dev, "Error installing i2c space handler\n");
+ acpi_bus_detach_private_data(handle);
+ kfree(data);
+ return -ENOMEM;
+ }
+
+ acpi_walk_dep_device_list(handle);
+ return 0;
+}
+
+static void mshw0011_remove_space_handler(struct i2c_client *client)
+{
+ acpi_handle handle;
+ struct mshw0011_handler_data *data;
+ acpi_status status;
+
+ handle = ACPI_HANDLE(&client->dev);
+
+ if (!handle)
+ return;
+
+ acpi_remove_address_space_handler(handle,
+ ACPI_ADR_SPACE_GSBUS,
+ &mshw0011_space_handler);
+
+ status = acpi_bus_get_private_data(handle, (void **)&data);
+ if (ACPI_SUCCESS(status))
+ kfree(data);
+
+ acpi_bus_detach_private_data(handle);
+}
+
+static int acpi_find_i2c(struct acpi_resource *ares, void *data)
+{
+ struct mshw0011_lookup *lookup = data;
+
+ if (ares->type != ACPI_RESOURCE_TYPE_SERIAL_BUS)
+ return 1;
+
+ if (lookup->n++ == lookup->index && !lookup->addr)
+ lookup->addr = ares->data.i2c_serial_bus.slave_address;
+
+ return 1;
+}
+
+static int mshw0011_i2c_resource_lookup(struct mshw0011_data *cdata,
+ unsigned int index)
+{
+ struct i2c_client *client = cdata->adp1;
+ struct acpi_device *adev = ACPI_COMPANION(&client->dev);
+ struct mshw0011_lookup lookup = {
+ .cdata = cdata,
+ .index = index,
+ };
+ struct list_head res_list;
+ int ret;
+
+ INIT_LIST_HEAD(&res_list);
+
+ ret = acpi_dev_get_resources(adev, &res_list, acpi_find_i2c, &lookup);
+ if (ret < 0)
+ return ret;
+
+ acpi_dev_free_resource_list(&res_list);
+
+ if (!lookup.addr)
+ return -ENOENT;
+
+ return lookup.addr;
+}
+
+static void mshw0011_dump_registers(struct i2c_client *client,
+ struct i2c_client *bat0, u8 max)
+{
+ char rd_buf[60];
+ int error, i, c;
+ char buff[17 * 3 * 2] = {0};
+
+ dev_info(&client->dev, "dumping registers 0x00 to 0x%02x:\n",
+ (max - 1) / 0x20 * 0x20 + 0x1f);
+
+ for (i = 0; i < max; i += 0x20) {
+ memset(rd_buf, 0, sizeof(rd_buf));
+ error = mshw0011_i2c_read_block(bat0, i, rd_buf, 0x20);
+ dev_info(&client->dev, " read 0x%02x: %*ph|%*ph\n",
+ i,
+ 0x10, rd_buf,
+ 0x10, rd_buf + 0x10);
+ for (c = 0; c < 0x20; c++) {
+ if (rd_buf[c] >= 0x20 && rd_buf[c] <= 0x7e) {
+ buff[c * 3 + 0] = ' ';
+ buff[c * 3 + 1] = rd_buf[c];
+ } else {
+ buff[c * 3 + 0] = '-';
+ buff[c * 3 + 1] = '-';
+ }
+ buff[c * 3 + 2] = (c + 1) % 0x10 ? ' ' : '|';
+ }
+ buff[0x1f * 3 + 2] = '\0';
+ dev_info(&client->dev, "ascii 0x%02x: %s\n", i, buff);
+ }
+}
+
+static int mshw0011_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct device *dev = &client->dev;
+ struct i2c_client *bat0;
+ struct mshw0011_data *data;
+ int error, version, addr;
+
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->adp1 = client;
+ i2c_set_clientdata(client, data);
+
+ addr = mshw0011_i2c_resource_lookup(data, 1);
+ if (addr < 0)
+ return addr;
+
+ bat0 = i2c_new_dummy(client->adapter, addr);
+ if (!bat0)
+ return -ENOMEM;
+
+ data->bat0 = bat0;
+ i2c_set_clientdata(bat0, data);
+
+ if (dump_registers) {
+ mshw0011_dump_registers(client, client, 0xFF);
+ mshw0011_dump_registers(client, bat0, 0x80);
+ }
+
+ error = mshw0011_notify(data, 1, MSHW0011_NOTIFY_GET_VERSION, &version);
+ if (error)
+ goto out_err;
+
+ data->notify_version = version == MSHW0011_EV_2_5;
+
+ data->poll_task = kthread_run(mshw0011_poll_task, data, "mshw0011_adp");
+ if (IS_ERR(data->poll_task)) {
+ error = PTR_ERR(data->poll_task);
+ dev_err(&client->dev, "Unable to run kthread err %d\n", error);
+ goto out_err;
+ }
+
+ error = mshw0011_install_space_handler(client);
+ if (error)
+ goto out_err;
+
+ return 0;
+
+out_err:
+ if (data->kthread_running)
+ kthread_stop(data->poll_task);
+ i2c_unregister_device(data->bat0);
+ return error;
+}
+
+static int mshw0011_remove(struct i2c_client *client)
+{
+ struct mshw0011_data *cdata = i2c_get_clientdata(client);
+
+ mshw0011_remove_space_handler(client);
+
+ if (cdata->kthread_running)
+ kthread_stop(cdata->poll_task);
+
+ i2c_unregister_device(cdata->bat0);
+
+ return 0;
+}
+
+static const struct i2c_device_id mshw0011_id[] = {
+ { "MSHW0011:00", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, mshw0011_id);
+
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id mshw0011_acpi_match[] = {
+ { "MSHW0011", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(acpi, mshw0011_acpi_match);
+#endif
+
+static struct i2c_driver mshw0011_driver = {
+ .probe = mshw0011_probe,
+ .remove = mshw0011_remove,
+ .id_table = mshw0011_id,
+ .driver = {
+ .name = "mshw0011",
+ .acpi_match_table = ACPI_PTR(mshw0011_acpi_match),
+ },
+};
+module_i2c_driver(mshw0011_driver);
+
+MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@gmail.com>");
+MODULE_DESCRIPTION("mshw0011 driver");
+MODULE_LICENSE("GPL v2");
--
2.5.5
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] power: surface3_power: MSHW0011 rev-eng implementation
2016-07-29 15:49 ` [PATCH 2/2] power: surface3_power: MSHW0011 rev-eng implementation Benjamin Tissoires
@ 2016-07-29 19:51 ` kbuild test robot
2016-08-01 9:09 ` Benjamin Tissoires
2016-07-29 20:01 ` kbuild test robot
` (4 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: kbuild test robot @ 2016-07-29 19:51 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: kbuild-all, Stephen Just, Robert Moore, Lv Zheng,
Rafael J. Wysocki, Len Brown, Sebastian Reichel,
Dmitry Eremin-Solenikov, David Woodhouse, linux-acpi, devel,
linux-pm, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 9131 bytes --]
Hi,
[auto build test ERROR on battery/master]
[also build test ERROR on v4.7 next-20160729]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Benjamin-Tissoires/Add-support-for-Microsoft-Surface-3-power/20160729-235348
base: git://git.infradead.org/battery-2.6.git master
config: tile-allyesconfig (attached as .config)
compiler: tilegx-linux-gcc (GCC) 4.6.2
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=tile
All errors (new ones prefixed by >>):
drivers/power/surface3_power.c:52:52: error: expected ')' before 'bool'
drivers/power/surface3_power.c:54:4: error: expected ')' before string constant
drivers/power/surface3_power.c: In function 'mshw0011_notify':
drivers/power/surface3_power.c:199:2: error: implicit declaration of function 'acpi_bus_get_device'
drivers/power/surface3_power.c:202:2: error: implicit declaration of function 'acpi_evaluate_dsm_typed'
drivers/power/surface3_power.c:202:6: warning: assignment makes pointer from integer without a cast [enabled by default]
drivers/power/surface3_power.c: In function 'mshw0011_space_handler':
drivers/power/surface3_power.c:441:23: error: 'ACPI_GSB_ACCESS_ATTRIB_RAW_PROCESS' undeclared (first use in this function)
drivers/power/surface3_power.c:441:23: note: each undeclared identifier is reported only once for each function it appears in
drivers/power/surface3_power.c: In function 'mshw0011_install_space_handler':
drivers/power/surface3_power.c:511:2: error: implicit declaration of function 'acpi_bus_attach_private_data'
drivers/power/surface3_power.c:524:3: error: implicit declaration of function 'acpi_bus_detach_private_data'
drivers/power/surface3_power.c:529:2: error: implicit declaration of function 'acpi_walk_dep_device_list'
drivers/power/surface3_power.c: In function 'mshw0011_remove_space_handler':
drivers/power/surface3_power.c:548:2: error: implicit declaration of function 'acpi_bus_get_private_data'
drivers/power/surface3_power.c: In function 'mshw0011_i2c_resource_lookup':
drivers/power/surface3_power.c:582:2: error: implicit declaration of function 'acpi_dev_get_resources'
drivers/power/surface3_power.c:586:2: error: implicit declaration of function 'acpi_dev_free_resource_list'
drivers/power/surface3_power.c:571:21: warning: unused variable 'client'
drivers/power/surface3_power.c: At top level:
drivers/power/surface3_power.c:701:1: warning: data definition has no type or storage class [enabled by default]
drivers/power/surface3_power.c:701:1: error: type defaults to 'int' in declaration of 'MODULE_DEVICE_TABLE'
drivers/power/surface3_power.c:701:1: warning: parameter names (without types) in function declaration [enabled by default]
drivers/power/surface3_power.c:720:1: warning: data definition has no type or storage class [enabled by default]
>> drivers/power/surface3_power.c:720:1: error: type defaults to 'int' in declaration of 'module_init'
drivers/power/surface3_power.c:720:1: warning: parameter names (without types) in function declaration [enabled by default]
drivers/power/surface3_power.c:720:1: warning: data definition has no type or storage class [enabled by default]
>> drivers/power/surface3_power.c:720:1: error: type defaults to 'int' in declaration of 'module_exit'
drivers/power/surface3_power.c:720:1: warning: parameter names (without types) in function declaration [enabled by default]
drivers/power/surface3_power.c:722:15: error: expected declaration specifiers or '...' before string constant
drivers/power/surface3_power.c:723:20: error: expected declaration specifiers or '...' before string constant
drivers/power/surface3_power.c:724:16: error: expected declaration specifiers or '...' before string constant
drivers/power/surface3_power.c:720:1: warning: 'mshw0011_driver_init' defined but not used
cc1: some warnings being treated as errors
vim +720 drivers/power/surface3_power.c
565 return 1;
566 }
567
568 static int mshw0011_i2c_resource_lookup(struct mshw0011_data *cdata,
569 unsigned int index)
570 {
> 571 struct i2c_client *client = cdata->adp1;
572 struct acpi_device *adev = ACPI_COMPANION(&client->dev);
573 struct mshw0011_lookup lookup = {
574 .cdata = cdata,
575 .index = index,
576 };
577 struct list_head res_list;
578 int ret;
579
580 INIT_LIST_HEAD(&res_list);
581
> 582 ret = acpi_dev_get_resources(adev, &res_list, acpi_find_i2c, &lookup);
583 if (ret < 0)
584 return ret;
585
586 acpi_dev_free_resource_list(&res_list);
587
588 if (!lookup.addr)
589 return -ENOENT;
590
591 return lookup.addr;
592 }
593
594 static void mshw0011_dump_registers(struct i2c_client *client,
595 struct i2c_client *bat0, u8 max)
596 {
597 char rd_buf[60];
598 int error, i, c;
599 char buff[17 * 3 * 2] = {0};
600
601 dev_info(&client->dev, "dumping registers 0x00 to 0x%02x:\n",
602 (max - 1) / 0x20 * 0x20 + 0x1f);
603
604 for (i = 0; i < max; i += 0x20) {
605 memset(rd_buf, 0, sizeof(rd_buf));
606 error = mshw0011_i2c_read_block(bat0, i, rd_buf, 0x20);
607 dev_info(&client->dev, " read 0x%02x: %*ph|%*ph\n",
608 i,
609 0x10, rd_buf,
610 0x10, rd_buf + 0x10);
611 for (c = 0; c < 0x20; c++) {
612 if (rd_buf[c] >= 0x20 && rd_buf[c] <= 0x7e) {
613 buff[c * 3 + 0] = ' ';
614 buff[c * 3 + 1] = rd_buf[c];
615 } else {
616 buff[c * 3 + 0] = '-';
617 buff[c * 3 + 1] = '-';
618 }
619 buff[c * 3 + 2] = (c + 1) % 0x10 ? ' ' : '|';
620 }
621 buff[0x1f * 3 + 2] = '\0';
622 dev_info(&client->dev, "ascii 0x%02x: %s\n", i, buff);
623 }
624 }
625
626 static int mshw0011_probe(struct i2c_client *client,
627 const struct i2c_device_id *id)
628 {
629 struct device *dev = &client->dev;
630 struct i2c_client *bat0;
631 struct mshw0011_data *data;
632 int error, version, addr;
633
634 data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
635 if (!data)
636 return -ENOMEM;
637
638 data->adp1 = client;
639 i2c_set_clientdata(client, data);
640
641 addr = mshw0011_i2c_resource_lookup(data, 1);
642 if (addr < 0)
643 return addr;
644
645 bat0 = i2c_new_dummy(client->adapter, addr);
646 if (!bat0)
647 return -ENOMEM;
648
649 data->bat0 = bat0;
650 i2c_set_clientdata(bat0, data);
651
652 if (dump_registers) {
653 mshw0011_dump_registers(client, client, 0xFF);
654 mshw0011_dump_registers(client, bat0, 0x80);
655 }
656
657 error = mshw0011_notify(data, 1, MSHW0011_NOTIFY_GET_VERSION, &version);
658 if (error)
659 goto out_err;
660
661 data->notify_version = version == MSHW0011_EV_2_5;
662
663 data->poll_task = kthread_run(mshw0011_poll_task, data, "mshw0011_adp");
664 if (IS_ERR(data->poll_task)) {
665 error = PTR_ERR(data->poll_task);
666 dev_err(&client->dev, "Unable to run kthread err %d\n", error);
667 goto out_err;
668 }
669
670 error = mshw0011_install_space_handler(client);
671 if (error)
672 goto out_err;
673
674 return 0;
675
676 out_err:
677 if (data->kthread_running)
678 kthread_stop(data->poll_task);
679 i2c_unregister_device(data->bat0);
680 return error;
681 }
682
683 static int mshw0011_remove(struct i2c_client *client)
684 {
685 struct mshw0011_data *cdata = i2c_get_clientdata(client);
686
687 mshw0011_remove_space_handler(client);
688
689 if (cdata->kthread_running)
690 kthread_stop(cdata->poll_task);
691
692 i2c_unregister_device(cdata->bat0);
693
694 return 0;
695 }
696
697 static const struct i2c_device_id mshw0011_id[] = {
698 { "MSHW0011:00", 0 },
699 { }
700 };
701 MODULE_DEVICE_TABLE(i2c, mshw0011_id);
702
703 #ifdef CONFIG_ACPI
704 static const struct acpi_device_id mshw0011_acpi_match[] = {
705 { "MSHW0011", 0 },
706 { }
707 };
708 MODULE_DEVICE_TABLE(acpi, mshw0011_acpi_match);
709 #endif
710
711 static struct i2c_driver mshw0011_driver = {
712 .probe = mshw0011_probe,
713 .remove = mshw0011_remove,
714 .id_table = mshw0011_id,
715 .driver = {
716 .name = "mshw0011",
717 .acpi_match_table = ACPI_PTR(mshw0011_acpi_match),
718 },
719 };
> 720 module_i2c_driver(mshw0011_driver);
721
722 MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@gmail.com>");
723 MODULE_DESCRIPTION("mshw0011 driver");
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 44923 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] power: surface3_power: MSHW0011 rev-eng implementation
2016-07-29 15:49 ` [PATCH 2/2] power: surface3_power: MSHW0011 rev-eng implementation Benjamin Tissoires
2016-07-29 19:51 ` kbuild test robot
@ 2016-07-29 20:01 ` kbuild test robot
2016-07-29 20:09 ` kbuild test robot
` (3 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: kbuild test robot @ 2016-07-29 20:01 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: kbuild-all, Stephen Just, Robert Moore, Lv Zheng,
Rafael J. Wysocki, Len Brown, Sebastian Reichel,
Dmitry Eremin-Solenikov, David Woodhouse, linux-acpi, devel,
linux-pm, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 11759 bytes --]
Hi,
[auto build test WARNING on battery/master]
[also build test WARNING on v4.7 next-20160729]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Benjamin-Tissoires/Add-support-for-Microsoft-Surface-3-power/20160729-235348
base: git://git.infradead.org/battery-2.6.git master
config: mips-allyesconfig (attached as .config)
compiler: mips-linux-gnu-gcc (Debian 5.4.0-6) 5.4.0 20160609
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=mips
All warnings (new ones prefixed by >>):
drivers/power/surface3_power.c:52:52: error: expected ')' before 'bool'
module_param_named(dump_registers, dump_registers, bool, 0644);
^
drivers/power/surface3_power.c:54:4: error: expected ')' before string constant
"Dump the SMBus register at probe (debugging only).");
^
drivers/power/surface3_power.c: In function 'mshw0011_notify':
drivers/power/surface3_power.c:199:17: error: implicit declaration of function 'acpi_bus_get_device' [-Werror=implicit-function-declaration]
if (!handle || acpi_bus_get_device(handle, &adev))
^
drivers/power/surface3_power.c:202:8: error: implicit declaration of function 'acpi_evaluate_dsm_typed' [-Werror=implicit-function-declaration]
obj = acpi_evaluate_dsm_typed(handle, mshw0011_guid, arg1, arg2, NULL,
^
drivers/power/surface3_power.c:202:6: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
obj = acpi_evaluate_dsm_typed(handle, mshw0011_guid, arg1, arg2, NULL,
^
drivers/power/surface3_power.c: In function 'mshw0011_space_handler':
drivers/power/surface3_power.c:441:23: error: 'ACPI_GSB_ACCESS_ATTRIB_RAW_PROCESS' undeclared (first use in this function)
if (accessor_type != ACPI_GSB_ACCESS_ATTRIB_RAW_PROCESS) {
^
drivers/power/surface3_power.c:441:23: note: each undeclared identifier is reported only once for each function it appears in
drivers/power/surface3_power.c: In function 'mshw0011_install_space_handler':
drivers/power/surface3_power.c:511:11: error: implicit declaration of function 'acpi_bus_attach_private_data' [-Werror=implicit-function-declaration]
status = acpi_bus_attach_private_data(handle, (void *)data);
^
drivers/power/surface3_power.c:524:3: error: implicit declaration of function 'acpi_bus_detach_private_data' [-Werror=implicit-function-declaration]
acpi_bus_detach_private_data(handle);
^
drivers/power/surface3_power.c:529:2: error: implicit declaration of function 'acpi_walk_dep_device_list' [-Werror=implicit-function-declaration]
acpi_walk_dep_device_list(handle);
^
drivers/power/surface3_power.c: In function 'mshw0011_remove_space_handler':
drivers/power/surface3_power.c:548:11: error: implicit declaration of function 'acpi_bus_get_private_data' [-Werror=implicit-function-declaration]
status = acpi_bus_get_private_data(handle, (void **)&data);
^
drivers/power/surface3_power.c: In function 'mshw0011_i2c_resource_lookup':
drivers/power/surface3_power.c:582:8: error: implicit declaration of function 'acpi_dev_get_resources' [-Werror=implicit-function-declaration]
ret = acpi_dev_get_resources(adev, &res_list, acpi_find_i2c, &lookup);
^
drivers/power/surface3_power.c:586:2: error: implicit declaration of function 'acpi_dev_free_resource_list' [-Werror=implicit-function-declaration]
acpi_dev_free_resource_list(&res_list);
^
drivers/power/surface3_power.c:571:21: warning: unused variable 'client' [-Wunused-variable]
struct i2c_client *client = cdata->adp1;
^
drivers/power/surface3_power.c: At top level:
drivers/power/surface3_power.c:701:1: warning: data definition has no type or storage class
MODULE_DEVICE_TABLE(i2c, mshw0011_id);
^
drivers/power/surface3_power.c:701:1: error: type defaults to 'int' in declaration of 'MODULE_DEVICE_TABLE' [-Werror=implicit-int]
drivers/power/surface3_power.c:701:1: warning: parameter names (without types) in function declaration
In file included from include/linux/i2c.h:30:0,
from drivers/power/surface3_power.c:41:
include/linux/device.h:1350:1: warning: data definition has no type or storage class
module_init(__driver##_init); \
^
include/linux/i2c.h:712:2: note: in expansion of macro 'module_driver'
module_driver(__i2c_driver, i2c_add_driver, \
^
drivers/power/surface3_power.c:720:1: note: in expansion of macro 'module_i2c_driver'
module_i2c_driver(mshw0011_driver);
^
include/linux/device.h:1350:1: error: type defaults to 'int' in declaration of 'module_init' [-Werror=implicit-int]
module_init(__driver##_init); \
^
include/linux/i2c.h:712:2: note: in expansion of macro 'module_driver'
module_driver(__i2c_driver, i2c_add_driver, \
^
drivers/power/surface3_power.c:720:1: note: in expansion of macro 'module_i2c_driver'
module_i2c_driver(mshw0011_driver);
^
In file included from include/linux/linkage.h:6:0,
from include/linux/kernel.h:6,
from drivers/power/surface3_power.c:40:
>> include/linux/export.h:36:30: warning: parameter names (without types) in function declaration
#define THIS_MODULE ((struct module *)0)
^
>> include/linux/i2c.h:651:22: note: in expansion of macro 'THIS_MODULE'
i2c_register_driver(THIS_MODULE, driver)
^
>> include/linux/device.h:1348:9: note: in expansion of macro 'i2c_add_driver'
return __register(&(__driver) , ##__VA_ARGS__); \
^
include/linux/i2c.h:712:2: note: in expansion of macro 'module_driver'
module_driver(__i2c_driver, i2c_add_driver, \
^
drivers/power/surface3_power.c:720:1: note: in expansion of macro 'module_i2c_driver'
module_i2c_driver(mshw0011_driver);
^
In file included from include/linux/i2c.h:30:0,
from drivers/power/surface3_power.c:41:
include/linux/device.h:1355:1: warning: data definition has no type or storage class
module_exit(__driver##_exit);
^
include/linux/i2c.h:712:2: note: in expansion of macro 'module_driver'
module_driver(__i2c_driver, i2c_add_driver, \
^
drivers/power/surface3_power.c:720:1: note: in expansion of macro 'module_i2c_driver'
module_i2c_driver(mshw0011_driver);
^
include/linux/device.h:1355:1: error: type defaults to 'int' in declaration of 'module_exit' [-Werror=implicit-int]
module_exit(__driver##_exit);
^
include/linux/i2c.h:712:2: note: in expansion of macro 'module_driver'
module_driver(__i2c_driver, i2c_add_driver, \
^
drivers/power/surface3_power.c:720:1: note: in expansion of macro 'module_i2c_driver'
module_i2c_driver(mshw0011_driver);
^
In file included from include/linux/linkage.h:6:0,
from include/linux/kernel.h:6,
from drivers/power/surface3_power.c:40:
>> include/linux/export.h:36:30: warning: parameter names (without types) in function declaration
#define THIS_MODULE ((struct module *)0)
^
>> include/linux/i2c.h:651:22: note: in expansion of macro 'THIS_MODULE'
i2c_register_driver(THIS_MODULE, driver)
^
>> include/linux/device.h:1348:9: note: in expansion of macro 'i2c_add_driver'
return __register(&(__driver) , ##__VA_ARGS__); \
^
include/linux/i2c.h:712:2: note: in expansion of macro 'module_driver'
module_driver(__i2c_driver, i2c_add_driver, \
^
drivers/power/surface3_power.c:720:1: note: in expansion of macro 'module_i2c_driver'
module_i2c_driver(mshw0011_driver);
^
drivers/power/surface3_power.c:722:15: error: expected declaration specifiers or '...' before string constant
MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@gmail.com>");
^
drivers/power/surface3_power.c:723:20: error: expected declaration specifiers or '...' before string constant
MODULE_DESCRIPTION("mshw0011 driver");
^
drivers/power/surface3_power.c:724:16: error: expected declaration specifiers or '...' before string constant
MODULE_LICENSE("GPL v2");
^
In file included from include/linux/i2c.h:30:0,
from drivers/power/surface3_power.c:41:
drivers/power/surface3_power.c:720:19: warning: 'mshw0011_driver_init' defined but not used [-Wunused-function]
module_i2c_driver(mshw0011_driver);
^
include/linux/device.h:1346:19: note: in definition of macro 'module_driver'
static int __init __driver##_init(void) \
^
drivers/power/surface3_power.c:720:1: note: in expansion of macro 'module_i2c_driver'
module_i2c_driver(mshw0011_driver);
^
cc1: some warnings being treated as errors
vim +/THIS_MODULE +651 include/linux/i2c.h
c7036673 Hans Verkuil 2009-06-19 635
^1da177e Linus Torvalds 2005-04-16 636
^1da177e Linus Torvalds 2005-04-16 637 /* ----- functions exported by i2c.o */
^1da177e Linus Torvalds 2005-04-16 638
^1da177e Linus Torvalds 2005-04-16 639 /* administration...
^1da177e Linus Torvalds 2005-04-16 640 */
23af8400 Jean Delvare 2009-06-19 641 #if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)
^1da177e Linus Torvalds 2005-04-16 642 extern int i2c_add_adapter(struct i2c_adapter *);
71546300 Lars-Peter Clausen 2013-03-09 643 extern void i2c_del_adapter(struct i2c_adapter *);
6e13e641 David Brownell 2007-05-01 644 extern int i2c_add_numbered_adapter(struct i2c_adapter *);
^1da177e Linus Torvalds 2005-04-16 645
de59cf9e Greg Kroah-Hartman 2005-12-06 646 extern int i2c_register_driver(struct module *, struct i2c_driver *);
b3e82096 Jean Delvare 2007-05-01 647 extern void i2c_del_driver(struct i2c_driver *);
^1da177e Linus Torvalds 2005-04-16 648
eb5589a8 Paul Gortmaker 2011-05-27 649 /* use a define to avoid include chaining to get THIS_MODULE */
eb5589a8 Paul Gortmaker 2011-05-27 650 #define i2c_add_driver(driver) \
eb5589a8 Paul Gortmaker 2011-05-27 @651 i2c_register_driver(THIS_MODULE, driver)
de59cf9e Greg Kroah-Hartman 2005-12-06 652
e48d3319 Jean Delvare 2008-01-27 653 extern struct i2c_client *i2c_use_client(struct i2c_client *client);
e48d3319 Jean Delvare 2008-01-27 654 extern void i2c_release_client(struct i2c_client *client);
^1da177e Linus Torvalds 2005-04-16 655
^1da177e Linus Torvalds 2005-04-16 656 /* call the i2c_client->command() of all attached clients with
^1da177e Linus Torvalds 2005-04-16 657 * the given arguments */
^1da177e Linus Torvalds 2005-04-16 658 extern void i2c_clients_command(struct i2c_adapter *adap,
^1da177e Linus Torvalds 2005-04-16 659 unsigned int cmd, void *arg);
:::::: The code at line 651 was first introduced by commit
:::::: eb5589a8f0dab7e29021344228856339e6a1249c include: convert various register fcns to macros to avoid include chaining
:::::: TO: Paul Gortmaker <paul.gortmaker@windriver.com>
:::::: CC: Paul Gortmaker <paul.gortmaker@windriver.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 42414 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] power: surface3_power: MSHW0011 rev-eng implementation
2016-07-29 15:49 ` [PATCH 2/2] power: surface3_power: MSHW0011 rev-eng implementation Benjamin Tissoires
2016-07-29 19:51 ` kbuild test robot
2016-07-29 20:01 ` kbuild test robot
@ 2016-07-29 20:09 ` kbuild test robot
2016-08-15 21:58 ` Sebastian Reichel
` (2 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: kbuild test robot @ 2016-07-29 20:09 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: kbuild-all, Stephen Just, Robert Moore, Lv Zheng,
Rafael J. Wysocki, Len Brown, Sebastian Reichel,
Dmitry Eremin-Solenikov, David Woodhouse, linux-acpi, devel,
linux-pm, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 5973 bytes --]
Hi,
[auto build test ERROR on battery/master]
[also build test ERROR on v4.7 next-20160729]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Benjamin-Tissoires/Add-support-for-Microsoft-Surface-3-power/20160729-235348
base: git://git.infradead.org/battery-2.6.git master
config: openrisc-allyesconfig (attached as .config)
compiler: or32-linux-gcc (GCC) 4.5.1-or32-1.0rc1
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=openrisc
All errors (new ones prefixed by >>):
drivers/power/surface3_power.c:52:52: error: expected ')' before 'bool'
drivers/power/surface3_power.c:54:4: error: expected ')' before string constant
drivers/power/surface3_power.c: In function 'mshw0011_notify':
drivers/power/surface3_power.c:199:2: error: implicit declaration of function 'acpi_bus_get_device'
drivers/power/surface3_power.c:202:2: error: implicit declaration of function 'acpi_evaluate_dsm_typed'
drivers/power/surface3_power.c:202:6: warning: assignment makes pointer from integer without a cast
drivers/power/surface3_power.c: In function 'mshw0011_space_handler':
drivers/power/surface3_power.c:441:23: error: 'ACPI_GSB_ACCESS_ATTRIB_RAW_PROCESS' undeclared (first use in this function)
drivers/power/surface3_power.c:441:23: note: each undeclared identifier is reported only once for each function it appears in
drivers/power/surface3_power.c: In function 'mshw0011_install_space_handler':
drivers/power/surface3_power.c:511:2: error: implicit declaration of function 'acpi_bus_attach_private_data'
drivers/power/surface3_power.c:524:3: error: implicit declaration of function 'acpi_bus_detach_private_data'
drivers/power/surface3_power.c:529:2: error: implicit declaration of function 'acpi_walk_dep_device_list'
drivers/power/surface3_power.c: In function 'mshw0011_remove_space_handler':
drivers/power/surface3_power.c:548:2: error: implicit declaration of function 'acpi_bus_get_private_data'
drivers/power/surface3_power.c: In function 'mshw0011_i2c_resource_lookup':
drivers/power/surface3_power.c:582:2: error: implicit declaration of function 'acpi_dev_get_resources'
drivers/power/surface3_power.c:586:2: error: implicit declaration of function 'acpi_dev_free_resource_list'
drivers/power/surface3_power.c:571:21: warning: unused variable 'client'
drivers/power/surface3_power.c: At top level:
drivers/power/surface3_power.c:701:1: warning: data definition has no type or storage class
drivers/power/surface3_power.c:701:1: error: type defaults to 'int' in declaration of 'MODULE_DEVICE_TABLE'
drivers/power/surface3_power.c:701:1: warning: parameter names (without types) in function declaration
drivers/power/surface3_power.c:720:1: warning: data definition has no type or storage class
drivers/power/surface3_power.c:720:1: error: type defaults to 'int' in declaration of 'module_init'
drivers/power/surface3_power.c:720:1: warning: parameter names (without types) in function declaration
drivers/power/surface3_power.c:720:1: warning: data definition has no type or storage class
drivers/power/surface3_power.c:720:1: error: type defaults to 'int' in declaration of 'module_exit'
drivers/power/surface3_power.c:720:1: warning: parameter names (without types) in function declaration
drivers/power/surface3_power.c:722:15: error: expected declaration specifiers or '...' before string constant
drivers/power/surface3_power.c:722:1: warning: data definition has no type or storage class
>> drivers/power/surface3_power.c:722:1: error: type defaults to 'int' in declaration of 'MODULE_AUTHOR'
>> drivers/power/surface3_power.c:722:15: error: function declaration isn't a prototype
drivers/power/surface3_power.c:723:20: error: expected declaration specifiers or '...' before string constant
drivers/power/surface3_power.c:723:1: warning: data definition has no type or storage class
>> drivers/power/surface3_power.c:723:1: error: type defaults to 'int' in declaration of 'MODULE_DESCRIPTION'
drivers/power/surface3_power.c:723:20: error: function declaration isn't a prototype
drivers/power/surface3_power.c:724:16: error: expected declaration specifiers or '...' before string constant
drivers/power/surface3_power.c:724:1: warning: data definition has no type or storage class
>> drivers/power/surface3_power.c:724:1: error: type defaults to 'int' in declaration of 'MODULE_LICENSE'
drivers/power/surface3_power.c:724:16: error: function declaration isn't a prototype
drivers/power/surface3_power.c:720:1: warning: 'mshw0011_driver_init' defined but not used
vim +722 drivers/power/surface3_power.c
695 }
696
697 static const struct i2c_device_id mshw0011_id[] = {
698 { "MSHW0011:00", 0 },
699 { }
700 };
> 701 MODULE_DEVICE_TABLE(i2c, mshw0011_id);
702
703 #ifdef CONFIG_ACPI
704 static const struct acpi_device_id mshw0011_acpi_match[] = {
705 { "MSHW0011", 0 },
706 { }
707 };
708 MODULE_DEVICE_TABLE(acpi, mshw0011_acpi_match);
709 #endif
710
711 static struct i2c_driver mshw0011_driver = {
712 .probe = mshw0011_probe,
713 .remove = mshw0011_remove,
714 .id_table = mshw0011_id,
715 .driver = {
716 .name = "mshw0011",
717 .acpi_match_table = ACPI_PTR(mshw0011_acpi_match),
718 },
719 };
> 720 module_i2c_driver(mshw0011_driver);
721
> 722 MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@gmail.com>");
> 723 MODULE_DESCRIPTION("mshw0011 driver");
> 724 MODULE_LICENSE("GPL v2");
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 38004 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH 1/2] ACPICA: adapt buffer length for Field Attrib Raw Process in Ops Region
2016-07-29 15:49 ` [PATCH 1/2] ACPICA: adapt buffer length for Field Attrib Raw Process in Ops Region Benjamin Tissoires
@ 2016-08-01 5:58 ` Zheng, Lv
0 siblings, 0 replies; 11+ messages in thread
From: Zheng, Lv @ 2016-08-01 5:58 UTC (permalink / raw)
To: Benjamin Tissoires, Stephen Just, Moore, Robert, Wysocki,
Rafael J, Len Brown, Sebastian Reichel, Dmitry Eremin-Solenikov,
David Woodhouse
Cc: linux-acpi, devel, linux-pm, linux-kernel
Hi,
> From: Benjamin Tissoires [mailto:benjamin.tissoires@redhat.com]
> Subject: [PATCH 1/2] ACPICA: adapt buffer length for Field Attrib Raw
> Process in Ops Region
>
> Detected on the Surface 3:
> The MSHW0011 driver uses Field Attrib Raw Process to return information
> for the ACPI Battery. The DSDT declares a parameter of 2 though
> functions like _BST or _BIX require a much bigger out buffer.
>
> It looks like the incoming buffer has the requested size and we can
> work around the issue by using this input size as the output and
> parameters size.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=106231
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
> drivers/acpi/acpica/exfield.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/acpi/acpica/exfield.c b/drivers/acpi/acpica/exfield.c
> index d7d3ee3..a1bd041 100644
> --- a/drivers/acpi/acpica/exfield.c
> +++ b/drivers/acpi/acpica/exfield.c
> @@ -413,6 +413,17 @@ acpi_ex_write_data_to_field(union
> acpi_operand_object *source_desc,
> * Data[x-1]: (Bytes 2-x of the arbitrary length
> data buffer)
> */
> length += 2;
> +
> + /*
> + * When using Field Attrib Raw Process, it looks like
> + * the parameter access_length can be wrong and
> the
> + * required output buffer can be much bigger.
> + * So just take the incoming buffer length as the
> + * reference.
> + */
> + if (accessor_type ==
> AML_FIELD_ATTRIB_RAW_PROCESS)
> + length = source_desc->buffer.length;
> +
[Lv Zheng]
I think the related code is:
OperationRegion (OR02, GenericSerialBus, Zero, 0x0100)
Field (OR02, BufferAcc, NoLock, Preserve)
{
Connection (BX00),
AccessAs (BufferAcc, AttribRawProcessBytes (0x02)),
BB00, 8
}
Name (SPB0, Buffer (0x80) {})
CreateByteField (SPB0, Zero, STA2)
CreateByteField (SPB0, One, LEN0)
CreateByteField (SPB0, 0x02, CMD0)
CreateByteField (SPB0, 0x03, DAT0)
CreateByteField (SPB0, 0x04, DAT1)
CreateDWordField (SPB0, 0x05, DAT2)
CreateField (SPB0, 0x10, 0x0320, DAT3)
Method (RSPB, 3, NotSerialized)
{
CMD0 = One
LEN0 = 0x08
DAT0 = Arg0
DAT1 = Arg1
DAT2 = Arg2
SPB0 = ^^I2Z1.BB00 = SPB0 /* \_SB_.BAT0.SPB0 */
...
}
And we have confirmed that the iASL disassembly result is correct.
Then this issue is not a parser issue, but looks like an issue either in interpreter or in BIOS as:
1. The following line triggers i2c operation region handlers:
SPB0 = ^^I2Z1.BB00 = SPB0
Where:
SPB0 is sized as 0x80, BB00 is sized as 0x02.
And in spec 19.6.1 AccessAs (Change Field Unit Access), it is said that AttribRawProcessBytes(AccessLength) requires an AccessLength parameter.
However in this case, the length of the output buffer is not correct, much smaller than the container buffer.
So this firstly looks like a BIOS issue. And then needn't be fixed in OS.
2. If this is not a BIOS bug but an interpreter bug, then SPB0 should be a buffer passing to the i2c region handler.
The bug should be something related to "data type conversion".
Please refer to:
19.2.5.4 Implicit Source Operand Conversion
19.2.5.5 Implicit Result Object Conversion
It looks like that we have troubles in either 1st Store or 2nd Store.
If so, it seems we shouldn't fix this issue here. And thus this fix also looks invalid.
IMO (may be wrong), this bug finally may cause dissension among different parties (BIOS, Windows, ACPICA, spec).
However, for the time being, is that possible to work this issue around in the i2c operation region driver?
Thanks and best regards
-Lv
> function = ACPI_WRITE | (accessor_type << 16);
> } else { /* IPMI */
>
> --
> 2.5.5
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] power: surface3_power: MSHW0011 rev-eng implementation
2016-07-29 19:51 ` kbuild test robot
@ 2016-08-01 9:09 ` Benjamin Tissoires
0 siblings, 0 replies; 11+ messages in thread
From: Benjamin Tissoires @ 2016-08-01 9:09 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, Stephen Just, Robert Moore, Lv Zheng,
Rafael J. Wysocki, Len Brown, Sebastian Reichel,
Dmitry Eremin-Solenikov, David Woodhouse, linux-acpi, devel,
linux-pm, linux-kernel
On Jul 30 2016 or thereabouts, kbuild test robot wrote:
> Hi,
>
> [auto build test ERROR on battery/master]
> [also build test ERROR on v4.7 next-20160729]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Benjamin-Tissoires/Add-support-for-Microsoft-Surface-3-power/20160729-235348
> base: git://git.infradead.org/battery-2.6.git master
> config: tile-allyesconfig (attached as .config)
> compiler: tilegx-linux-gcc (GCC) 4.6.2
> reproduce:
> wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=tile
I guess adding depends on ACPI in the Kconfig should prevent those
architectures to be compiled as a target.
Will submit a v2 when I get around the issues raised by Lv in 1/2
regarding the ACPICA bits.
Cheers,
Benjamin
>
> All errors (new ones prefixed by >>):
>
> drivers/power/surface3_power.c:52:52: error: expected ')' before 'bool'
> drivers/power/surface3_power.c:54:4: error: expected ')' before string constant
> drivers/power/surface3_power.c: In function 'mshw0011_notify':
> drivers/power/surface3_power.c:199:2: error: implicit declaration of function 'acpi_bus_get_device'
> drivers/power/surface3_power.c:202:2: error: implicit declaration of function 'acpi_evaluate_dsm_typed'
> drivers/power/surface3_power.c:202:6: warning: assignment makes pointer from integer without a cast [enabled by default]
> drivers/power/surface3_power.c: In function 'mshw0011_space_handler':
> drivers/power/surface3_power.c:441:23: error: 'ACPI_GSB_ACCESS_ATTRIB_RAW_PROCESS' undeclared (first use in this function)
> drivers/power/surface3_power.c:441:23: note: each undeclared identifier is reported only once for each function it appears in
> drivers/power/surface3_power.c: In function 'mshw0011_install_space_handler':
> drivers/power/surface3_power.c:511:2: error: implicit declaration of function 'acpi_bus_attach_private_data'
> drivers/power/surface3_power.c:524:3: error: implicit declaration of function 'acpi_bus_detach_private_data'
> drivers/power/surface3_power.c:529:2: error: implicit declaration of function 'acpi_walk_dep_device_list'
> drivers/power/surface3_power.c: In function 'mshw0011_remove_space_handler':
> drivers/power/surface3_power.c:548:2: error: implicit declaration of function 'acpi_bus_get_private_data'
> drivers/power/surface3_power.c: In function 'mshw0011_i2c_resource_lookup':
> drivers/power/surface3_power.c:582:2: error: implicit declaration of function 'acpi_dev_get_resources'
> drivers/power/surface3_power.c:586:2: error: implicit declaration of function 'acpi_dev_free_resource_list'
> drivers/power/surface3_power.c:571:21: warning: unused variable 'client'
> drivers/power/surface3_power.c: At top level:
> drivers/power/surface3_power.c:701:1: warning: data definition has no type or storage class [enabled by default]
> drivers/power/surface3_power.c:701:1: error: type defaults to 'int' in declaration of 'MODULE_DEVICE_TABLE'
> drivers/power/surface3_power.c:701:1: warning: parameter names (without types) in function declaration [enabled by default]
> drivers/power/surface3_power.c:720:1: warning: data definition has no type or storage class [enabled by default]
> >> drivers/power/surface3_power.c:720:1: error: type defaults to 'int' in declaration of 'module_init'
> drivers/power/surface3_power.c:720:1: warning: parameter names (without types) in function declaration [enabled by default]
> drivers/power/surface3_power.c:720:1: warning: data definition has no type or storage class [enabled by default]
> >> drivers/power/surface3_power.c:720:1: error: type defaults to 'int' in declaration of 'module_exit'
> drivers/power/surface3_power.c:720:1: warning: parameter names (without types) in function declaration [enabled by default]
> drivers/power/surface3_power.c:722:15: error: expected declaration specifiers or '...' before string constant
> drivers/power/surface3_power.c:723:20: error: expected declaration specifiers or '...' before string constant
> drivers/power/surface3_power.c:724:16: error: expected declaration specifiers or '...' before string constant
> drivers/power/surface3_power.c:720:1: warning: 'mshw0011_driver_init' defined but not used
> cc1: some warnings being treated as errors
>
> vim +720 drivers/power/surface3_power.c
>
> 565 return 1;
> 566 }
> 567
> 568 static int mshw0011_i2c_resource_lookup(struct mshw0011_data *cdata,
> 569 unsigned int index)
> 570 {
> > 571 struct i2c_client *client = cdata->adp1;
> 572 struct acpi_device *adev = ACPI_COMPANION(&client->dev);
> 573 struct mshw0011_lookup lookup = {
> 574 .cdata = cdata,
> 575 .index = index,
> 576 };
> 577 struct list_head res_list;
> 578 int ret;
> 579
> 580 INIT_LIST_HEAD(&res_list);
> 581
> > 582 ret = acpi_dev_get_resources(adev, &res_list, acpi_find_i2c, &lookup);
> 583 if (ret < 0)
> 584 return ret;
> 585
> 586 acpi_dev_free_resource_list(&res_list);
> 587
> 588 if (!lookup.addr)
> 589 return -ENOENT;
> 590
> 591 return lookup.addr;
> 592 }
> 593
> 594 static void mshw0011_dump_registers(struct i2c_client *client,
> 595 struct i2c_client *bat0, u8 max)
> 596 {
> 597 char rd_buf[60];
> 598 int error, i, c;
> 599 char buff[17 * 3 * 2] = {0};
> 600
> 601 dev_info(&client->dev, "dumping registers 0x00 to 0x%02x:\n",
> 602 (max - 1) / 0x20 * 0x20 + 0x1f);
> 603
> 604 for (i = 0; i < max; i += 0x20) {
> 605 memset(rd_buf, 0, sizeof(rd_buf));
> 606 error = mshw0011_i2c_read_block(bat0, i, rd_buf, 0x20);
> 607 dev_info(&client->dev, " read 0x%02x: %*ph|%*ph\n",
> 608 i,
> 609 0x10, rd_buf,
> 610 0x10, rd_buf + 0x10);
> 611 for (c = 0; c < 0x20; c++) {
> 612 if (rd_buf[c] >= 0x20 && rd_buf[c] <= 0x7e) {
> 613 buff[c * 3 + 0] = ' ';
> 614 buff[c * 3 + 1] = rd_buf[c];
> 615 } else {
> 616 buff[c * 3 + 0] = '-';
> 617 buff[c * 3 + 1] = '-';
> 618 }
> 619 buff[c * 3 + 2] = (c + 1) % 0x10 ? ' ' : '|';
> 620 }
> 621 buff[0x1f * 3 + 2] = '\0';
> 622 dev_info(&client->dev, "ascii 0x%02x: %s\n", i, buff);
> 623 }
> 624 }
> 625
> 626 static int mshw0011_probe(struct i2c_client *client,
> 627 const struct i2c_device_id *id)
> 628 {
> 629 struct device *dev = &client->dev;
> 630 struct i2c_client *bat0;
> 631 struct mshw0011_data *data;
> 632 int error, version, addr;
> 633
> 634 data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> 635 if (!data)
> 636 return -ENOMEM;
> 637
> 638 data->adp1 = client;
> 639 i2c_set_clientdata(client, data);
> 640
> 641 addr = mshw0011_i2c_resource_lookup(data, 1);
> 642 if (addr < 0)
> 643 return addr;
> 644
> 645 bat0 = i2c_new_dummy(client->adapter, addr);
> 646 if (!bat0)
> 647 return -ENOMEM;
> 648
> 649 data->bat0 = bat0;
> 650 i2c_set_clientdata(bat0, data);
> 651
> 652 if (dump_registers) {
> 653 mshw0011_dump_registers(client, client, 0xFF);
> 654 mshw0011_dump_registers(client, bat0, 0x80);
> 655 }
> 656
> 657 error = mshw0011_notify(data, 1, MSHW0011_NOTIFY_GET_VERSION, &version);
> 658 if (error)
> 659 goto out_err;
> 660
> 661 data->notify_version = version == MSHW0011_EV_2_5;
> 662
> 663 data->poll_task = kthread_run(mshw0011_poll_task, data, "mshw0011_adp");
> 664 if (IS_ERR(data->poll_task)) {
> 665 error = PTR_ERR(data->poll_task);
> 666 dev_err(&client->dev, "Unable to run kthread err %d\n", error);
> 667 goto out_err;
> 668 }
> 669
> 670 error = mshw0011_install_space_handler(client);
> 671 if (error)
> 672 goto out_err;
> 673
> 674 return 0;
> 675
> 676 out_err:
> 677 if (data->kthread_running)
> 678 kthread_stop(data->poll_task);
> 679 i2c_unregister_device(data->bat0);
> 680 return error;
> 681 }
> 682
> 683 static int mshw0011_remove(struct i2c_client *client)
> 684 {
> 685 struct mshw0011_data *cdata = i2c_get_clientdata(client);
> 686
> 687 mshw0011_remove_space_handler(client);
> 688
> 689 if (cdata->kthread_running)
> 690 kthread_stop(cdata->poll_task);
> 691
> 692 i2c_unregister_device(cdata->bat0);
> 693
> 694 return 0;
> 695 }
> 696
> 697 static const struct i2c_device_id mshw0011_id[] = {
> 698 { "MSHW0011:00", 0 },
> 699 { }
> 700 };
> 701 MODULE_DEVICE_TABLE(i2c, mshw0011_id);
> 702
> 703 #ifdef CONFIG_ACPI
> 704 static const struct acpi_device_id mshw0011_acpi_match[] = {
> 705 { "MSHW0011", 0 },
> 706 { }
> 707 };
> 708 MODULE_DEVICE_TABLE(acpi, mshw0011_acpi_match);
> 709 #endif
> 710
> 711 static struct i2c_driver mshw0011_driver = {
> 712 .probe = mshw0011_probe,
> 713 .remove = mshw0011_remove,
> 714 .id_table = mshw0011_id,
> 715 .driver = {
> 716 .name = "mshw0011",
> 717 .acpi_match_table = ACPI_PTR(mshw0011_acpi_match),
> 718 },
> 719 };
> > 720 module_i2c_driver(mshw0011_driver);
> 721
> 722 MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@gmail.com>");
> 723 MODULE_DESCRIPTION("mshw0011 driver");
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] power: surface3_power: MSHW0011 rev-eng implementation
2016-07-29 15:49 ` [PATCH 2/2] power: surface3_power: MSHW0011 rev-eng implementation Benjamin Tissoires
` (2 preceding siblings ...)
2016-07-29 20:09 ` kbuild test robot
@ 2016-08-15 21:58 ` Sebastian Reichel
2016-08-16 10:02 ` Mika Westerberg
2016-10-07 18:44 ` Andy Shevchenko
5 siblings, 0 replies; 11+ messages in thread
From: Sebastian Reichel @ 2016-08-15 21:58 UTC (permalink / raw)
To: Benjamin Tissoires, Rafael J. Wysocki, Len Brown
Cc: Stephen Just, Robert Moore, Lv Zheng, linux-acpi, devel,
linux-pm, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 600 bytes --]
Hi,
On Fri, Jul 29, 2016 at 05:49:26PM +0200, Benjamin Tissoires wrote:
> MSHW0011 replaces the battery firmware by using ACPI operation regions.
> The values have been obtained by reverse engineering, and are subject to
> errors. Looks like it works on overall pretty well.
>
> I couldn't manage to get the IRQ correctly triggered, so I am using a
> good old polling thread to check for changes.
With CONFIG_ACPI_BATTERY being in drivers/acpi/ and this not
using the power-supply subsystem's framework at all, I wonder
if this driver should also go to drivers/acpi/.
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] power: surface3_power: MSHW0011 rev-eng implementation
2016-07-29 15:49 ` [PATCH 2/2] power: surface3_power: MSHW0011 rev-eng implementation Benjamin Tissoires
` (3 preceding siblings ...)
2016-08-15 21:58 ` Sebastian Reichel
@ 2016-08-16 10:02 ` Mika Westerberg
2016-10-07 18:44 ` Andy Shevchenko
5 siblings, 0 replies; 11+ messages in thread
From: Mika Westerberg @ 2016-08-16 10:02 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Stephen Just, Robert Moore, Lv Zheng, Rafael J. Wysocki,
Len Brown, Sebastian Reichel, Dmitry Eremin-Solenikov,
David Woodhouse, linux-acpi, devel, linux-pm, linux-kernel
On Fri, Jul 29, 2016 at 05:49:26PM +0200, Benjamin Tissoires wrote:
> +static const struct i2c_device_id mshw0011_id[] = {
> + { "MSHW0011:00", 0 },
You should not use ACPI device names in non-ACPI matching table. I think
you can just leave this table empty.
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, mshw0011_id);
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] power: surface3_power: MSHW0011 rev-eng implementation
2016-07-29 15:49 ` [PATCH 2/2] power: surface3_power: MSHW0011 rev-eng implementation Benjamin Tissoires
` (4 preceding siblings ...)
2016-08-16 10:02 ` Mika Westerberg
@ 2016-10-07 18:44 ` Andy Shevchenko
5 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2016-10-07 18:44 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Stephen Just, Robert Moore, Lv Zheng, Rafael J. Wysocki,
Len Brown, Sebastian Reichel, Dmitry Eremin-Solenikov,
David Woodhouse, linux-acpi, devel, linux-pm, linux-kernel
On Fri, Jul 29, 2016 at 6:49 PM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> MSHW0011 replaces the battery firmware by using ACPI operation regions.
> The values have been obtained by reverse engineering, and are subject to
> errors. Looks like it works on overall pretty well.
>
> I couldn't manage to get the IRQ correctly triggered, so I am using a
> good old polling thread to check for changes.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=106231
> +/*
> + * This driver has been reverse-engineered by parsing the DSDT of the Surface 3
> + * and looking at the registers of the chips.
> + *
> + * The DSDT allowed to find out that:
> + * - the driver is required for the ACPI BAT0 device to communicate to the chip
> + * through an operation region.
> + * - the various defines for the operation region functions to communicate with
> + * this driver
> + * - the DSM 3f99e367-6220-4955-8b0f-06ef2ae79412 allows to trigger ACPI
> + * events to BAT0 (the code is all available in the DSDT).
> + *
> + * Further findings regarding the 2 chips declared in the MSHW0011 are:
> + * - there are 2 chips declared:
> + * . 0x22 seems to control the ADP1 line status (and probably the charger)
> + * . 0x55 controls the battery directly
> + * - the battery chip uses a SMBus protocol (using plain SMBus allows non
> + * destructive commands):
> + * . the commands/registers used are in the range 0x00..0x7F
> + * . if bit 8 (0x80) is set in the SMBus command, the returned value is the
> + * same as when it is not set. There is a high chance this bit is the
> + * read/write
> + * . the various registers semantic as been deduced by observing the register
> + * dumps.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/i2c.h>
> +#include <linux/slab.h>
> +#include <linux/acpi.h>
> +#include <linux/kthread.h>
> +#include <linux/freezer.h>
Alphabetical order?
> +
> +#include <asm/unaligned.h>
> +
> +#define POLL_INTERVAL (HZ * 2)
(2 * HZ)
> +static int
> +mshw0011_notify(struct mshw0011_data *cdata, u8 arg1, u8 arg2,
> + unsigned int *ret_value)
> +{
> + static const u8 mshw0011_guid[] = {
> + 0x67, 0xE3, 0x99, 0x3F, 0x20, 0x62, 0x55, 0x49,
> + 0x8b, 0x0f, 0x06, 0xef, 0x2a, 0xe7, 0x94, 0x12,
> + };
What about uuid_le?
> + union acpi_object *obj;
> + struct acpi_device *adev;
> + acpi_handle handle;
> + unsigned int i;
> +
> + handle = ACPI_HANDLE(&cdata->adp1->dev);
> + if (!handle || acpi_bus_get_device(handle, &adev))
> + return -ENODEV;
> +
> + obj = acpi_evaluate_dsm_typed(handle, mshw0011_guid, arg1, arg2, NULL,
> + ACPI_TYPE_BUFFER);
> + if (!obj) {
> + dev_err(&cdata->adp1->dev, "device _DSM execution failed\n");
> + return -ENODEV;
> + }
> +
> + *ret_value = 0;
> + for (i = 0; i < obj->buffer.length; i++)
> + *ret_value |= obj->buffer.pointer[i] << (i * 8);
> +
> + ACPI_FREE(obj);
> + return 0;
> +}
> +
> +static const struct bix default_bix = {
> + .revision = 0x00,
> + .power_unit = 0x01,
> + .design_capacity = 0x1dca,
> + .last_full_charg_capacity = 0x1dca,
> + .battery_technology = 0x01,
> + .design_voltage = 0x10df,
> + .design_capacity_of_warning = 0x8f,
> + .design_capacity_of_low = 0x47,
> + .cycle_count = 0xffffffff,
> + .measurement_accuracy = 0x00015F90,
> + .max_sampling_time = 0x03E8,
> + .min_sampling_time = 0x03E8,
> + .max_average_interval = 0x03E8,
> + .min_average_interval = 0x03E8,
Capital vs. small letters for hex?
> + .battery_capacity_granularity_1 = 0x45,
> + .battery_capacity_granularity_2 = 0x11,
> + .model = "P11G8M",
> + .serial = "",
> + .type = "LION",
> + .OEM = "",
> +};
> +static int mshw0011_bix(struct mshw0011_data *cdata, struct bix *bix)
> +{
> + struct i2c_client *client = cdata->bat0;
> + int ret;
> + char buf[10];
Exchange order?
> +static int mshw0011_poll_task(void *data)
> +{
> + struct mshw0011_data *cdata = data;
> + int ret = 0;
> +
> + cdata->kthread_running = true;
> +
> + set_freezable();
> +
> + while (!kthread_should_stop()) {
> + schedule_timeout_interruptible(POLL_INTERVAL);
> + try_to_freeze();
> + ret = mshw0011_isr(data);
> + if (ret)
> + goto out;
break; ?
> + }
> +
> +out:
> + cdata->kthread_running = false;
> + return ret;
> +}
> +
> +static acpi_status
> +mshw0011_space_handler(u32 function, acpi_physical_address command,
> + u32 bits, u64 *value64,
> + void *handler_context, void *region_context)
> +{
> + int status = 1;
> + switch (gsb->cmd.arg1) {
> + case MSHW0011_CMD_BAT0_STA:
> + status = 1;
Already 1?
> + ret = 0;
> + break;
> + case MSHW0011_CMD_BAT0_BIX:
> + status = 1;
Ditto.
> + ret = mshw0011_bix(cdata, &gsb->bix);
> + break;
> + case MSHW0011_CMD_BAT0_BTP:
> + status = 1;
Ditto.
> + ret = 0;
> + cdata->trip_point = gsb->cmd.arg2;
> + break;
> + case MSHW0011_CMD_BAT0_BST:
> + status = 1;
Ditto.
> + ret = mshw0011_bst(cdata, &gsb->bst);
> + break;
> + default:
> + pr_info("command(0x%02x) is not supported.\n", gsb->cmd.arg1);
> + ret = AE_BAD_PARAMETER;
> + goto err;
> + }
> +
> + out:
> + gsb->ret = status;
> + gsb->status = 0;
> +
> + err:
> + ACPI_FREE(ares);
> + return ret;
> +}
> +static void mshw0011_remove_space_handler(struct i2c_client *client)
> +{
> + acpi_handle handle;
> + struct mshw0011_handler_data *data;
> + acpi_status status;
> +
> + handle = ACPI_HANDLE(&client->dev);
> +
Redundant.
> + if (!handle)
> + return;
> +static void mshw0011_dump_registers(struct i2c_client *client,
> + struct i2c_client *bat0, u8 max)
> +{
> + char rd_buf[60];
> + int error, i, c;
> + char buff[17 * 3 * 2] = {0};
> +
> + dev_info(&client->dev, "dumping registers 0x00 to 0x%02x:\n",
> + (max - 1) / 0x20 * 0x20 + 0x1f);
DIV_ROUND_UP()?
> +
> + for (i = 0; i < max; i += 0x20) {
> + memset(rd_buf, 0, sizeof(rd_buf));
> + error = mshw0011_i2c_read_block(bat0, i, rd_buf, 0x20);
> + dev_info(&client->dev, " read 0x%02x: %*ph|%*ph\n",
> + i,
> + 0x10, rd_buf,
> + 0x10, rd_buf + 0x10);
> + for (c = 0; c < 0x20; c++) {
> + if (rd_buf[c] >= 0x20 && rd_buf[c] <= 0x7e) {
> + buff[c * 3 + 0] = ' ';
> + buff[c * 3 + 1] = rd_buf[c];
> + } else {
> + buff[c * 3 + 0] = '-';
> + buff[c * 3 + 1] = '-';
> + }
> + buff[c * 3 + 2] = (c + 1) % 0x10 ? ' ' : '|';
> + }
> + buff[0x1f * 3 + 2] = '\0';
> + dev_info(&client->dev, "ascii 0x%02x: %s\n", i, buff);
hex_dump_to_buffer() / print_hex_dump() ?
> + }
> +}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-10-07 18:44 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-29 15:49 [PATCH 0/2] Add support for Microsoft Surface 3 power Benjamin Tissoires
2016-07-29 15:49 ` [PATCH 1/2] ACPICA: adapt buffer length for Field Attrib Raw Process in Ops Region Benjamin Tissoires
2016-08-01 5:58 ` Zheng, Lv
2016-07-29 15:49 ` [PATCH 2/2] power: surface3_power: MSHW0011 rev-eng implementation Benjamin Tissoires
2016-07-29 19:51 ` kbuild test robot
2016-08-01 9:09 ` Benjamin Tissoires
2016-07-29 20:01 ` kbuild test robot
2016-07-29 20:09 ` kbuild test robot
2016-08-15 21:58 ` Sebastian Reichel
2016-08-16 10:02 ` Mika Westerberg
2016-10-07 18:44 ` Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).