From ffc10d82ff5df7087a9b737de55a69ac4f89bf56 Mon Sep 17 00:00:00 2001 From: Michal Hocko Date: Mon, 3 Apr 2017 09:40:23 +0200 Subject: [PATCH 01/19] ACPI / scan: Drop support for force_remove /sys/firmware/acpi/hotplug/force_remove was presumably added to support auto offlining in the past. This is, however, inherently dangerous for some hotplugable resources like memory. The memory offlining fails when the memory is still in use and cannot be dropped or migrated. If we ignore the failure we are basically allowing for subtle memory corruption or a crash. We have actually noticed the later while hitting BUG() during the memory hotremove (remove_memory): ret = walk_memory_range(PFN_DOWN(start), PFN_UP(start + size - 1), NULL, check_memblock_offlined_cb); if (ret) BUG(); it took us quite non-trivial time realize that the customer had force_remove enabled. Even if the BUG was removed here and we could propagate the error up the call chain it wouldn't help at all because then we would hit a crash or a memory corruption later and harder to debug. So force_remove is unfixable for the memory hotremove. We haven't checked other hotplugable resources to be prone to a similar problems. Remove the force_remove functionality because it is not fixable currently. Keep the sysfs file and report an error if somebody tries to enable it. Encourage users to report about the missing functionality and work with them with an alternative solution. Reviewed-by: Lee, Chun-Yi Signed-off-by: Michal Hocko Signed-off-by: Rafael J. Wysocki --- Documentation/ABI/obsolete/sysfs-firmware-acpi | 8 ++++++++ Documentation/ABI/testing/sysfs-firmware-acpi | 10 ---------- drivers/acpi/internal.h | 2 -- drivers/acpi/scan.c | 16 +++------------- drivers/acpi/sysfs.c | 9 +++++---- 5 files changed, 16 insertions(+), 29 deletions(-) create mode 100644 Documentation/ABI/obsolete/sysfs-firmware-acpi diff --git a/Documentation/ABI/obsolete/sysfs-firmware-acpi b/Documentation/ABI/obsolete/sysfs-firmware-acpi new file mode 100644 index 000000000000..6715a71bec3d --- /dev/null +++ b/Documentation/ABI/obsolete/sysfs-firmware-acpi @@ -0,0 +1,8 @@ +What: /sys/firmware/acpi/hotplug/force_remove +Date: Mar 2017 +Contact: Rafael J. Wysocki +Description: + Since the force_remove is inherently broken and dangerous to + use for some hotplugable resources like memory (because ignoring + the offline failure might lead to memory corruption and crashes) + enabling this knob is not safe and thus unsupported. diff --git a/Documentation/ABI/testing/sysfs-firmware-acpi b/Documentation/ABI/testing/sysfs-firmware-acpi index c7fc72d4495c..613f42a9d5cd 100644 --- a/Documentation/ABI/testing/sysfs-firmware-acpi +++ b/Documentation/ABI/testing/sysfs-firmware-acpi @@ -44,16 +44,6 @@ Description: or 0 (unset). Attempts to write any other values to it will cause -EINVAL to be returned. -What: /sys/firmware/acpi/hotplug/force_remove -Date: May 2013 -Contact: Rafael J. Wysocki -Description: - The number in this file (0 or 1) determines whether (1) or not - (0) the ACPI subsystem will allow devices to be hot-removed even - if they cannot be put offline gracefully (from the kernel's - viewpoint). That number can be changed by writing a boolean - value to this file. - What: /sys/firmware/acpi/interrupts/ Date: February 2008 Contact: Len Brown diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index f15900132912..66229ffa909b 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -65,8 +65,6 @@ static inline void acpi_cmos_rtc_init(void) {} #endif int acpi_rev_override_setup(char *str); -extern bool acpi_force_hot_remove; - void acpi_sysfs_add_hotplug_profile(struct acpi_hotplug_profile *hotplug, const char *name); int acpi_scan_add_handler_with_hotplug(struct acpi_scan_handler *handler, diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index 192691880d55..e2080b6e54aa 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -30,12 +30,6 @@ extern struct acpi_device *acpi_root; #define INVALID_ACPI_HANDLE ((acpi_handle)empty_zero_page) -/* - * If set, devices will be hot-removed even if they cannot be put offline - * gracefully (from the kernel's standpoint). - */ -bool acpi_force_hot_remove; - static const char *dummy_hid = "device"; static LIST_HEAD(acpi_dep_list); @@ -170,9 +164,6 @@ static acpi_status acpi_bus_offline(acpi_handle handle, u32 lvl, void *data, pn->put_online = false; } ret = device_offline(pn->dev); - if (acpi_force_hot_remove) - continue; - if (ret >= 0) { pn->put_online = !ret; } else { @@ -241,11 +232,11 @@ static int acpi_scan_try_to_offline(struct acpi_device *device) acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX, NULL, acpi_bus_offline, (void *)true, (void **)&errdev); - if (!errdev || acpi_force_hot_remove) + if (!errdev) acpi_bus_offline(handle, 0, (void *)true, (void **)&errdev); - if (errdev && !acpi_force_hot_remove) { + if (errdev) { dev_warn(errdev, "Offline failed.\n"); acpi_bus_online(handle, 0, NULL, NULL); acpi_walk_namespace(ACPI_TYPE_ANY, handle, @@ -263,8 +254,7 @@ static int acpi_scan_hot_remove(struct acpi_device *device) unsigned long long sta; acpi_status status; - if (device->handler && device->handler->hotplug.demand_offline - && !acpi_force_hot_remove) { + if (device->handler && device->handler->hotplug.demand_offline) { if (!acpi_scan_is_offline(device, true)) return -EBUSY; } else { diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c index cf05ae973381..1b5ee1e0e5a3 100644 --- a/drivers/acpi/sysfs.c +++ b/drivers/acpi/sysfs.c @@ -921,7 +921,7 @@ void acpi_sysfs_add_hotplug_profile(struct acpi_hotplug_profile *hotplug, static ssize_t force_remove_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf) { - return sprintf(buf, "%d\n", !!acpi_force_hot_remove); + return sprintf(buf, "%d\n", 0); } static ssize_t force_remove_store(struct kobject *kobj, @@ -935,9 +935,10 @@ static ssize_t force_remove_store(struct kobject *kobj, if (ret < 0) return ret; - lock_device_hotplug(); - acpi_force_hot_remove = val; - unlock_device_hotplug(); + if (val) { + pr_err("Enabling force_remove is not supported anymore. Please report to linux-acpi@vger.kernel.org if you depend on this functionality\n"); + return -EINVAL; + } return size; } From 35b68735988447485074cfa862710e49c082c013 Mon Sep 17 00:00:00 2001 From: "Prakash, Prashanth" Date: Thu, 23 Mar 2017 15:21:53 -0600 Subject: [PATCH 02/19] ACPI / Processor: Drop setup_max_cpus check from acpi_processor_add() When maxcpus=X kernel argument is used, we parse the ACPI tables only for the first X cpus. The per-cpu ACPI data include the _PSD tables which gives information about related cpus. cppc_cpufreq and acpi-cpufreq parses the table once during init to deduce the related cpus. If a user brings a new cpu online after boot the related cpu data becomes incorrect. acpi_get_psd_map() in acpi_cppc.c returns error if it fails to find the parsed ACPI data for possible CPU resulting in cppc_cpufreq initialization failure. With this change we will probe all possible CPUs prior to cpufreq initialization, but will bring only setup_max_cpus online. nr_cpus kernel parameter can be used to restict even parsing per-cpu ACPI tables. Signed-off-by: Prashanth Prakash [ rjw: Subject ] Signed-off-by: Rafael J. Wysocki --- drivers/acpi/acpi_processor.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c index 0143135b3abe..f098e25b6b41 100644 --- a/drivers/acpi/acpi_processor.c +++ b/drivers/acpi/acpi_processor.c @@ -388,11 +388,6 @@ static int acpi_processor_add(struct acpi_device *device, if (result) /* Processor is not physically present or unavailable */ return 0; -#ifdef CONFIG_SMP - if (pr->id >= setup_max_cpus && pr->id != 0) - return 0; -#endif - BUG_ON(pr->id >= nr_cpu_ids); /* From 79f97c3051e16b117b47e0586a368c4f54a82909 Mon Sep 17 00:00:00 2001 From: Shanker Donthineni Date: Wed, 29 Mar 2017 06:39:19 -0500 Subject: [PATCH 03/19] ACPI / platform: Update platform device NUMA node based on _PXM method The optional _PXM method evaluates to an integer that identifies the proximity domain of a device object. On ACPI based kernel boot, the field numa_node in 'struct device' is always set to -1 irrespective of _PXM value that is specified in the ACPI device object. But in case of device-tree based kernel boot the numa_node field is populated and reflects to a DT property that is specified in DTS according to the below document. https://www.kernel.org/doc/Documentation/devicetree/bindings/numa.txt http://lxr.free-electrons.com/source/drivers/of/device.c#L54 Without this patch dev_to_node() always returns -1 for all platform devices. This patch adds support for the ACPI _PXM method and updates the platform device NUMA node id using acpi_get_node() which provides the PXM-to-NUMA mapping information. The individual platform device drivers should be able to use the NUMA-aware memory allocation functions kmalloc_node() and alloc_pages_node() to improve performance. Signed-off-by: Shanker Donthineni Reviewed-by: Hanjun Guo Tested-by: Jiandi An [ rjw: Subject / changelog ] Signed-off-by: Rafael J. Wysocki --- drivers/acpi/acpi_platform.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c index 03250e1f1103..88cd949003f3 100644 --- a/drivers/acpi/acpi_platform.c +++ b/drivers/acpi/acpi_platform.c @@ -121,11 +121,14 @@ struct platform_device *acpi_create_platform_device(struct acpi_device *adev, if (IS_ERR(pdev)) dev_err(&adev->dev, "platform device creation failed: %ld\n", PTR_ERR(pdev)); - else + else { + set_dev_node(&pdev->dev, acpi_get_node(adev->handle)); dev_dbg(&adev->dev, "created platform device %s\n", dev_name(&pdev->dev)); + } kfree(resources); + return pdev; } EXPORT_SYMBOL_GPL(acpi_create_platform_device); From 368520a6b2dd232ea5743a6acd9f056bc30e05b4 Mon Sep 17 00:00:00 2001 From: "Prakash, Prashanth" Date: Wed, 29 Mar 2017 13:49:59 -0600 Subject: [PATCH 04/19] ACPI / CPPC: Read lowest nonlinear perf in cppc_get_perf_caps() Read lowest non linear perf in cppc_get_perf_caps so that it can be exposed via sysfs to the usespace. Lowest non linear perf is the lowest performance level at which nonlinear power savings are achieved. Signed-off-by: Prashanth Prakash Signed-off-by: Rafael J. Wysocki --- drivers/acpi/cppc_acpi.c | 19 +++++++++++-------- include/acpi/cppc_acpi.h | 1 + 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c index 3ca0729f7e0e..ee00a1c94ff6 100644 --- a/drivers/acpi/cppc_acpi.c +++ b/drivers/acpi/cppc_acpi.c @@ -972,9 +972,9 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val) int cppc_get_perf_caps(int cpunum, struct cppc_perf_caps *perf_caps) { struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpunum); - struct cpc_register_resource *highest_reg, *lowest_reg, *ref_perf, - *nom_perf; - u64 high, low, nom; + struct cpc_register_resource *highest_reg, *lowest_reg, + *lowest_non_linear_reg, *nominal_reg; + u64 high, low, nom, min_nonlinear; int ret = 0, regs_in_pcc = 0; if (!cpc_desc) { @@ -984,12 +984,12 @@ int cppc_get_perf_caps(int cpunum, struct cppc_perf_caps *perf_caps) highest_reg = &cpc_desc->cpc_regs[HIGHEST_PERF]; lowest_reg = &cpc_desc->cpc_regs[LOWEST_PERF]; - ref_perf = &cpc_desc->cpc_regs[REFERENCE_PERF]; - nom_perf = &cpc_desc->cpc_regs[NOMINAL_PERF]; + lowest_non_linear_reg = &cpc_desc->cpc_regs[LOW_NON_LINEAR_PERF]; + nominal_reg = &cpc_desc->cpc_regs[NOMINAL_PERF]; /* Are any of the regs PCC ?*/ if (CPC_IN_PCC(highest_reg) || CPC_IN_PCC(lowest_reg) || - CPC_IN_PCC(ref_perf) || CPC_IN_PCC(nom_perf)) { + CPC_IN_PCC(lowest_non_linear_reg) || CPC_IN_PCC(nominal_reg)) { regs_in_pcc = 1; down_write(&pcc_data.pcc_lock); /* Ring doorbell once to update PCC subspace */ @@ -1005,10 +1005,13 @@ int cppc_get_perf_caps(int cpunum, struct cppc_perf_caps *perf_caps) cpc_read(cpunum, lowest_reg, &low); perf_caps->lowest_perf = low; - cpc_read(cpunum, nom_perf, &nom); + cpc_read(cpunum, nominal_reg, &nom); perf_caps->nominal_perf = nom; - if (!high || !low || !nom) + cpc_read(cpunum, lowest_non_linear_reg, &min_nonlinear); + perf_caps->lowest_nonlinear_perf = min_nonlinear; + + if (!high || !low || !nom || !min_nonlinear) ret = -EFAULT; out_err: diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h index 427a7c3e6c75..34e9680c55db 100644 --- a/include/acpi/cppc_acpi.h +++ b/include/acpi/cppc_acpi.h @@ -103,6 +103,7 @@ struct cppc_perf_caps { u32 highest_perf; u32 nominal_perf; u32 lowest_perf; + u32 lowest_nonlinear_perf; }; struct cppc_perf_ctrls { From 2c74d8473d19c159a3c3eabaa4819e110c97e8ec Mon Sep 17 00:00:00 2001 From: "Prakash, Prashanth" Date: Wed, 29 Mar 2017 13:50:00 -0600 Subject: [PATCH 05/19] ACPI / CPPC: add sysfs entries for CPPC perf capabilities Computed delivered performance using CPPC feedback counters are in the CPPC abstract scale, whereas cppc_cpufreq driver operates in KHz scale. Exposing the CPPC performance capabilities (highest,lowest, nominal, lowest non-linear) will allow userspace to figure out the conversion factor from CPPC abstract scale to KHz. Also rename ctr_wrap_time to wraparound_time so that show_cppc_data() macro will work with it. Signed-off-by: Prashanth Prakash Signed-off-by: Rafael J. Wysocki --- drivers/acpi/cppc_acpi.c | 61 ++++++++++++++++++++++------------------ include/acpi/cppc_acpi.h | 2 +- 2 files changed, 34 insertions(+), 29 deletions(-) diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c index ee00a1c94ff6..6cbe6036da99 100644 --- a/drivers/acpi/cppc_acpi.c +++ b/drivers/acpi/cppc_acpi.c @@ -132,49 +132,54 @@ __ATTR(_name, 0444, show_##_name, NULL) #define to_cpc_desc(a) container_of(a, struct cpc_desc, kobj) +#define show_cppc_data(access_fn, struct_name, member_name) \ + static ssize_t show_##member_name(struct kobject *kobj, \ + struct attribute *attr, char *buf) \ + { \ + struct cpc_desc *cpc_ptr = to_cpc_desc(kobj); \ + struct struct_name st_name = {0}; \ + int ret; \ + \ + ret = access_fn(cpc_ptr->cpu_id, &st_name); \ + if (ret) \ + return ret; \ + \ + return scnprintf(buf, PAGE_SIZE, "%llu\n", \ + (u64)st_name.member_name); \ + } \ + define_one_cppc_ro(member_name) + +show_cppc_data(cppc_get_perf_caps, cppc_perf_caps, highest_perf); +show_cppc_data(cppc_get_perf_caps, cppc_perf_caps, lowest_perf); +show_cppc_data(cppc_get_perf_caps, cppc_perf_caps, nominal_perf); +show_cppc_data(cppc_get_perf_caps, cppc_perf_caps, lowest_nonlinear_perf); +show_cppc_data(cppc_get_perf_ctrs, cppc_perf_fb_ctrs, reference_perf); +show_cppc_data(cppc_get_perf_ctrs, cppc_perf_fb_ctrs, wraparound_time); + static ssize_t show_feedback_ctrs(struct kobject *kobj, struct attribute *attr, char *buf) { struct cpc_desc *cpc_ptr = to_cpc_desc(kobj); struct cppc_perf_fb_ctrs fb_ctrs = {0}; + int ret; - cppc_get_perf_ctrs(cpc_ptr->cpu_id, &fb_ctrs); + ret = cppc_get_perf_ctrs(cpc_ptr->cpu_id, &fb_ctrs); + if (ret) + return ret; return scnprintf(buf, PAGE_SIZE, "ref:%llu del:%llu\n", fb_ctrs.reference, fb_ctrs.delivered); } define_one_cppc_ro(feedback_ctrs); -static ssize_t show_reference_perf(struct kobject *kobj, - struct attribute *attr, char *buf) -{ - struct cpc_desc *cpc_ptr = to_cpc_desc(kobj); - struct cppc_perf_fb_ctrs fb_ctrs = {0}; - - cppc_get_perf_ctrs(cpc_ptr->cpu_id, &fb_ctrs); - - return scnprintf(buf, PAGE_SIZE, "%llu\n", - fb_ctrs.reference_perf); -} -define_one_cppc_ro(reference_perf); - -static ssize_t show_wraparound_time(struct kobject *kobj, - struct attribute *attr, char *buf) -{ - struct cpc_desc *cpc_ptr = to_cpc_desc(kobj); - struct cppc_perf_fb_ctrs fb_ctrs = {0}; - - cppc_get_perf_ctrs(cpc_ptr->cpu_id, &fb_ctrs); - - return scnprintf(buf, PAGE_SIZE, "%llu\n", fb_ctrs.ctr_wrap_time); - -} -define_one_cppc_ro(wraparound_time); - static struct attribute *cppc_attrs[] = { &feedback_ctrs.attr, &reference_perf.attr, &wraparound_time.attr, + &highest_perf.attr, + &lowest_perf.attr, + &lowest_nonlinear_perf.attr, + &nominal_perf.attr, NULL }; @@ -1086,7 +1091,7 @@ int cppc_get_perf_ctrs(int cpunum, struct cppc_perf_fb_ctrs *perf_fb_ctrs) perf_fb_ctrs->delivered = delivered; perf_fb_ctrs->reference = reference; perf_fb_ctrs->reference_perf = ref_perf; - perf_fb_ctrs->ctr_wrap_time = ctr_wrap_time; + perf_fb_ctrs->wraparound_time = ctr_wrap_time; out_err: if (regs_in_pcc) up_write(&pcc_data.pcc_lock); diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h index 34e9680c55db..2010c0516f27 100644 --- a/include/acpi/cppc_acpi.h +++ b/include/acpi/cppc_acpi.h @@ -116,7 +116,7 @@ struct cppc_perf_fb_ctrs { u64 reference; u64 delivered; u64 reference_perf; - u64 ctr_wrap_time; + u64 wraparound_time; }; /* Per CPU container for runtime CPPC management. */ From f49c3f90a31f6e19ef3343dcc8809dac1019b59e Mon Sep 17 00:00:00 2001 From: Baoquan He Date: Fri, 7 Apr 2017 16:22:47 +0800 Subject: [PATCH 06/19] ACPI / tables: Drop acpi_parse_entries() which is not used Function acpi_parse_entries() is not used any more and if necessary, acpi_table_parse_entries() can be used instead of it, so drop it. Signed-off-by: Baoquan He [ rjw: Subject / changelog ] Signed-off-by: Rafael J. Wysocki --- drivers/acpi/tables.c | 16 ---------------- include/linux/acpi.h | 4 ---- 2 files changed, 20 deletions(-) diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c index 2604189d6cd1..0dae722ab2ec 100644 --- a/drivers/acpi/tables.c +++ b/drivers/acpi/tables.c @@ -310,22 +310,6 @@ acpi_parse_entries_array(char *id, unsigned long table_size, return errs ? -EINVAL : count; } -int __init -acpi_parse_entries(char *id, - unsigned long table_size, - acpi_tbl_entry_handler handler, - struct acpi_table_header *table_header, - int entry_id, unsigned int max_entries) -{ - struct acpi_subtable_proc proc = { - .id = entry_id, - .handler = handler, - }; - - return acpi_parse_entries_array(id, table_size, table_header, - &proc, 1, max_entries); -} - int __init acpi_table_parse_entries_array(char *id, unsigned long table_size, diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 9b05886f9773..83abbfceabad 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -233,10 +233,6 @@ int acpi_numa_init (void); int acpi_table_init (void); int acpi_table_parse(char *id, acpi_tbl_table_handler handler); -int __init acpi_parse_entries(char *id, unsigned long table_size, - acpi_tbl_entry_handler handler, - struct acpi_table_header *table_header, - int entry_id, unsigned int max_entries); int __init acpi_table_parse_entries(char *id, unsigned long table_size, int entry_id, acpi_tbl_entry_handler handler, From 2cff319e9666f534a7538aa1f4178d1f799b068a Mon Sep 17 00:00:00 2001 From: Kai Heng Feng Date: Wed, 12 Apr 2017 16:12:45 +0800 Subject: [PATCH 07/19] ACPI / blacklist: add _REV quirk for Dell Inspiron 7537 The battery can only be detected after AC power adapter event. Adding the machine to acpi_rev_dmi_table[] can work around this issue. Link: https://bugs.launchpad.net/bugs/1678590 Link: https://bugzilla.kernel.org/show_bug.cgi?id=105721 Signed-off-by: Kai-Heng Feng Signed-off-by: Rafael J. Wysocki --- drivers/acpi/blacklist.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/acpi/blacklist.c b/drivers/acpi/blacklist.c index 4421f7c9981c..bb542acc0574 100644 --- a/drivers/acpi/blacklist.c +++ b/drivers/acpi/blacklist.c @@ -188,6 +188,14 @@ static struct dmi_system_id acpi_rev_dmi_table[] __initdata = { DMI_MATCH(DMI_PRODUCT_NAME, "Latitude 3350"), }, }, + { + .callback = dmi_enable_rev_override, + .ident = "DELL Inspiron 7537", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."), + DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 7537"), + }, + }, #endif {} }; From bf4f5bf131b67ef821e14de4ea92556e4d1fc852 Mon Sep 17 00:00:00 2001 From: Cao jin Date: Wed, 19 Apr 2017 10:07:56 +0800 Subject: [PATCH 08/19] ACPI / doc: linuxized-acpica.txt: fix typos Fix some typos in the linuxized-acpica.txt document. Signed-off-by: Cao jin [ rjw: Subject / changelog ] Signed-off-by: Rafael J. Wysocki --- Documentation/acpi/linuxized-acpica.txt | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Documentation/acpi/linuxized-acpica.txt b/Documentation/acpi/linuxized-acpica.txt index defe2eec5331..3ad7b0dfb083 100644 --- a/Documentation/acpi/linuxized-acpica.txt +++ b/Documentation/acpi/linuxized-acpica.txt @@ -24,7 +24,7 @@ upstream. The homepage of ACPICA project is: www.acpica.org, it is maintained and supported by Intel Corporation. - The following figure depicts the Linux ACPI subystem where the ACPICA + The following figure depicts the Linux ACPI subsystem where the ACPICA adaptation is included: +---------------------------------------------------------+ @@ -110,7 +110,7 @@ upstream. Linux patches. The patches generated by this process are referred to as "linuxized ACPICA patches". The release process is carried out on a local copy the ACPICA git repository. Each commit in the monthly release is - converted into a linuxized ACPICA patch. Together, they form the montly + converted into a linuxized ACPICA patch. Together, they form the monthly ACPICA release patchset for the Linux ACPI community. This process is illustrated in the following figure: @@ -165,7 +165,7 @@ upstream. . Before the linuxized ACPICA patches are sent to the Linux ACPI community - for review, there is a quality ensurance build test process to reduce + for review, there is a quality assurance build test process to reduce porting issues. Currently this build process only takes care of the following kernel configuration options: CONFIG_ACPI/CONFIG_ACPI_DEBUG/CONFIG_ACPI_DEBUGGER @@ -195,12 +195,12 @@ upstream. release utilities (please refer to Section 4 below for the details). 3. Linux specific features - Sometimes it's impossible to use the current ACPICA APIs to implement features required by the Linux kernel, - so Linux developers occasionaly have to change ACPICA code directly. + so Linux developers occasionally have to change ACPICA code directly. Those changes may not be acceptable by ACPICA upstream and in such cases they are left as committed ACPICA divergences unless the ACPICA side can implement new mechanisms as replacements for them. 4. ACPICA release fixups - ACPICA only tests commits using a set of the - user space simulation utilies, thus the linuxized ACPICA patches may + user space simulation utilities, thus the linuxized ACPICA patches may break the Linux kernel, leaving us build/boot failures. In order to avoid breaking Linux bisection, fixes are applied directly to the linuxized ACPICA patches during the release process. When the release From d485ef43072e543e81c08276999987039dfa2755 Mon Sep 17 00:00:00 2001 From: Dmitry Frank Date: Wed, 19 Apr 2017 12:48:07 +0300 Subject: [PATCH 09/19] ACPI / video: get rid of magic numbers and use enum instead The first two items in the _BCL method response are special: - Level when machine has full power - Level when machine is on batteries - .... actual supported levels go there .... So this commits adds an enum and uses its descriptive elements throughout the code, instead of magic numbers. Signed-off-by: Dmitry Frank Signed-off-by: Rafael J. Wysocki --- drivers/acpi/acpi_video.c | 110 +++++++++++++++++++++++--------------- 1 file changed, 66 insertions(+), 44 deletions(-) diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c index d00bc0ef87a6..9a607af971e7 100644 --- a/drivers/acpi/acpi_video.c +++ b/drivers/acpi/acpi_video.c @@ -88,6 +88,18 @@ static int acpi_video_bus_remove(struct acpi_device *device); static void acpi_video_bus_notify(struct acpi_device *device, u32 event); void acpi_video_detect_exit(void); +/* + * Indices in the _BCL method response: the first two items are special, + * the rest are all supported levels. + * + * See page 575 of the ACPI spec 3.0 + */ +enum acpi_video_level_idx { + ACPI_VIDEO_AC_LEVEL, /* level when machine has full power */ + ACPI_VIDEO_BATTERY_LEVEL, /* level when machine is on batteries */ + ACPI_VIDEO_FIRST_LEVEL, /* actual supported levels begin here */ +}; + static const struct acpi_device_id video_device_ids[] = { {ACPI_VIDEO_HID, 0}, {"", 0}, @@ -217,20 +229,16 @@ static int acpi_video_get_brightness(struct backlight_device *bd) if (acpi_video_device_lcd_get_level_current(vd, &cur_level, false)) return -EINVAL; - for (i = 2; i < vd->brightness->count; i++) { + for (i = ACPI_VIDEO_FIRST_LEVEL; i < vd->brightness->count; i++) { if (vd->brightness->levels[i] == cur_level) - /* - * The first two entries are special - see page 575 - * of the ACPI spec 3.0 - */ - return i - 2; + return i - ACPI_VIDEO_FIRST_LEVEL; } return 0; } static int acpi_video_set_brightness(struct backlight_device *bd) { - int request_level = bd->props.brightness + 2; + int request_level = bd->props.brightness + ACPI_VIDEO_FIRST_LEVEL; struct acpi_video_device *vd = bl_get_data(bd); cancel_delayed_work(&vd->switch_brightness_work); @@ -244,18 +252,18 @@ static const struct backlight_ops acpi_backlight_ops = { }; /* thermal cooling device callbacks */ -static int video_get_max_state(struct thermal_cooling_device *cooling_dev, unsigned - long *state) +static int video_get_max_state(struct thermal_cooling_device *cooling_dev, + unsigned long *state) { struct acpi_device *device = cooling_dev->devdata; struct acpi_video_device *video = acpi_driver_data(device); - *state = video->brightness->count - 3; + *state = video->brightness->count - ACPI_VIDEO_FIRST_LEVEL - 1; return 0; } -static int video_get_cur_state(struct thermal_cooling_device *cooling_dev, unsigned - long *state) +static int video_get_cur_state(struct thermal_cooling_device *cooling_dev, + unsigned long *state) { struct acpi_device *device = cooling_dev->devdata; struct acpi_video_device *video = acpi_driver_data(device); @@ -264,7 +272,8 @@ static int video_get_cur_state(struct thermal_cooling_device *cooling_dev, unsig if (acpi_video_device_lcd_get_level_current(video, &level, false)) return -EINVAL; - for (offset = 2; offset < video->brightness->count; offset++) + for (offset = ACPI_VIDEO_FIRST_LEVEL; offset < video->brightness->count; + offset++) if (level == video->brightness->levels[offset]) { *state = video->brightness->count - offset - 1; return 0; @@ -280,7 +289,7 @@ video_set_cur_state(struct thermal_cooling_device *cooling_dev, unsigned long st struct acpi_video_device *video = acpi_driver_data(device); int level; - if (state >= video->brightness->count - 2) + if (state >= video->brightness->count - ACPI_VIDEO_FIRST_LEVEL) return -EINVAL; state = video->brightness->count - state; @@ -345,10 +354,12 @@ acpi_video_device_lcd_set_level(struct acpi_video_device *device, int level) } device->brightness->curr = level; - for (state = 2; state < device->brightness->count; state++) + for (state = ACPI_VIDEO_FIRST_LEVEL; state < device->brightness->count; + state++) if (level == device->brightness->levels[state]) { if (device->backlight) - device->backlight->props.brightness = state - 2; + device->backlight->props.brightness = + state - ACPI_VIDEO_FIRST_LEVEL; return 0; } @@ -530,14 +541,16 @@ acpi_video_bqc_value_to_level(struct acpi_video_device *device, if (device->brightness->flags._BQC_use_index) { /* - * _BQC returns an index that doesn't account for - * the first 2 items with special meaning, so we need - * to compensate for that by offsetting ourselves + * _BQC returns an index that doesn't account for the first 2 + * items with special meaning (see enum acpi_video_level_idx), + * so we need to compensate for that by offsetting ourselves */ if (device->brightness->flags._BCL_reversed) - bqc_value = device->brightness->count - 3 - bqc_value; + bqc_value = device->brightness->count - + ACPI_VIDEO_FIRST_LEVEL - 1 - bqc_value; - level = device->brightness->levels[bqc_value + 2]; + level = device->brightness->levels[bqc_value + + ACPI_VIDEO_FIRST_LEVEL]; } else { level = bqc_value; } @@ -571,7 +584,8 @@ acpi_video_device_lcd_get_level_current(struct acpi_video_device *device, *level = acpi_video_bqc_value_to_level(device, *level); - for (i = 2; i < device->brightness->count; i++) + for (i = ACPI_VIDEO_FIRST_LEVEL; + i < device->brightness->count; i++) if (device->brightness->levels[i] == *level) { device->brightness->curr = *level; return 0; @@ -716,7 +730,9 @@ static int acpi_video_bqc_quirk(struct acpi_video_device *device, * Some systems always report current brightness level as maximum * through _BQC, we need to test another value for them. */ - test_level = current_level == max_level ? br->levels[3] : max_level; + test_level = current_level == max_level + ? br->levels[ACPI_VIDEO_FIRST_LEVEL + 1] + : max_level; result = acpi_video_device_lcd_set_level(device, test_level); if (result) @@ -730,8 +746,8 @@ static int acpi_video_bqc_quirk(struct acpi_video_device *device, /* buggy _BQC found, need to find out if it uses index */ if (level < br->count) { if (br->flags._BCL_reversed) - level = br->count - 3 - level; - if (br->levels[level + 2] == test_level) + level = br->count - ACPI_VIDEO_FIRST_LEVEL - 1 - level; + if (br->levels[level + ACPI_VIDEO_FIRST_LEVEL] == test_level) br->flags._BQC_use_index = 1; } @@ -761,7 +777,7 @@ int acpi_video_get_levels(struct acpi_device *device, goto out; } - if (obj->package.count < 2) { + if (obj->package.count < ACPI_VIDEO_FIRST_LEVEL) { result = -EINVAL; goto out; } @@ -773,8 +789,8 @@ int acpi_video_get_levels(struct acpi_device *device, goto out; } - br->levels = kmalloc((obj->package.count + 2) * sizeof *(br->levels), - GFP_KERNEL); + br->levels = kmalloc((obj->package.count + ACPI_VIDEO_FIRST_LEVEL) * + sizeof(*br->levels), GFP_KERNEL); if (!br->levels) { result = -ENOMEM; goto out_free; @@ -788,7 +804,8 @@ int acpi_video_get_levels(struct acpi_device *device, } value = (u32) o->integer.value; /* Skip duplicate entries */ - if (count > 2 && br->levels[count - 1] == value) + if (count > ACPI_VIDEO_FIRST_LEVEL + && br->levels[count - 1] == value) continue; br->levels[count] = value; @@ -804,27 +821,30 @@ int acpi_video_get_levels(struct acpi_device *device, * In this case, the first two elements in _BCL packages * are also supported brightness levels that OS should take care of. */ - for (i = 2; i < count; i++) { - if (br->levels[i] == br->levels[0]) + for (i = ACPI_VIDEO_FIRST_LEVEL; i < count; i++) { + if (br->levels[i] == br->levels[ACPI_VIDEO_AC_LEVEL]) level_ac_battery++; - if (br->levels[i] == br->levels[1]) + if (br->levels[i] == br->levels[ACPI_VIDEO_BATTERY_LEVEL]) level_ac_battery++; } - if (level_ac_battery < 2) { - level_ac_battery = 2 - level_ac_battery; + if (level_ac_battery < ACPI_VIDEO_FIRST_LEVEL) { + level_ac_battery = ACPI_VIDEO_FIRST_LEVEL - level_ac_battery; br->flags._BCL_no_ac_battery_levels = 1; - for (i = (count - 1 + level_ac_battery); i >= 2; i--) + for (i = (count - 1 + level_ac_battery); + i >= ACPI_VIDEO_FIRST_LEVEL; i--) br->levels[i] = br->levels[i - level_ac_battery]; count += level_ac_battery; - } else if (level_ac_battery > 2) + } else if (level_ac_battery > ACPI_VIDEO_FIRST_LEVEL) ACPI_ERROR((AE_INFO, "Too many duplicates in _BCL package")); /* Check if the _BCL package is in a reversed order */ - if (max_level == br->levels[2]) { + if (max_level == br->levels[ACPI_VIDEO_FIRST_LEVEL]) { br->flags._BCL_reversed = 1; - sort(&br->levels[2], count - 2, sizeof(br->levels[2]), - acpi_video_cmp_level, NULL); + sort(&br->levels[ACPI_VIDEO_FIRST_LEVEL], + count - ACPI_VIDEO_FIRST_LEVEL, + sizeof(br->levels[ACPI_VIDEO_FIRST_LEVEL]), + acpi_video_cmp_level, NULL); } else if (max_level != br->levels[count - 1]) ACPI_ERROR((AE_INFO, "Found unordered _BCL package")); @@ -894,7 +914,7 @@ acpi_video_init_brightness(struct acpi_video_device *device) * level_old is invalid (no matter whether it's a level * or an index). Set the backlight to max_level in this case. */ - for (i = 2; i < br->count; i++) + for (i = ACPI_VIDEO_FIRST_LEVEL; i < br->count; i++) if (level == br->levels[i]) break; if (i == br->count || !level) @@ -906,7 +926,8 @@ set_level: goto out_free_levels; ACPI_DEBUG_PRINT((ACPI_DB_INFO, - "found %d brightness levels\n", br->count - 2)); + "found %d brightness levels\n", + br->count - ACPI_VIDEO_FIRST_LEVEL)); return 0; out_free_levels: @@ -1297,7 +1318,7 @@ acpi_video_get_next_level(struct acpi_video_device *device, max = max_below = 0; min = min_above = 255; /* Find closest level to level_current */ - for (i = 2; i < device->brightness->count; i++) { + for (i = ACPI_VIDEO_FIRST_LEVEL; i < device->brightness->count; i++) { l = device->brightness->levels[i]; if (abs(l - level_current) < abs(delta)) { delta = l - level_current; @@ -1307,7 +1328,7 @@ acpi_video_get_next_level(struct acpi_video_device *device, } /* Ajust level_current to closest available level */ level_current += delta; - for (i = 2; i < device->brightness->count; i++) { + for (i = ACPI_VIDEO_FIRST_LEVEL; i < device->brightness->count; i++) { l = device->brightness->levels[i]; if (l < min) min = l; @@ -1680,7 +1701,8 @@ static void acpi_video_dev_register_backlight(struct acpi_video_device *device) memset(&props, 0, sizeof(struct backlight_properties)); props.type = BACKLIGHT_FIRMWARE; - props.max_brightness = device->brightness->count - 3; + props.max_brightness = + device->brightness->count - ACPI_VIDEO_FIRST_LEVEL - 1; device->backlight = backlight_device_register(name, parent, device, From 592c8095b834c86b0cc780de6501d156e0e92e36 Mon Sep 17 00:00:00 2001 From: Dmitry Frank Date: Wed, 19 Apr 2017 13:36:18 +0300 Subject: [PATCH 10/19] ACPI / video: add comments about subtle cases The comment for acpi_video_bqc_quirk is by Felipe Contreras, taken from the git history. Signed-off-by: Dmitry Frank Signed-off-by: Rafael J. Wysocki --- drivers/acpi/acpi_video.c | 47 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 2 deletions(-) diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c index 9a607af971e7..e88fe3632dd6 100644 --- a/drivers/acpi/acpi_video.c +++ b/drivers/acpi/acpi_video.c @@ -73,6 +73,10 @@ module_param(report_key_events, int, 0644); MODULE_PARM_DESC(report_key_events, "0: none, 1: output changes, 2: brightness changes, 3: all"); +/* + * Whether the struct acpi_video_device_attrib::device_id_scheme bit should be + * assumed even if not actually set. + */ static bool device_id_scheme = false; module_param(device_id_scheme, bool, 0444); @@ -144,7 +148,15 @@ struct acpi_video_device_attrib { the VGA device. */ u32 pipe_id:3; /* For VGA multiple-head devices. */ u32 reserved:10; /* Must be 0 */ - u32 device_id_scheme:1; /* Device ID Scheme */ + + /* + * The device ID might not actually follow the scheme described by this + * struct acpi_video_device_attrib. If it does, then this bit + * device_id_scheme is set; otherwise, other fields should be ignored. + * + * (but also see the global flag device_id_scheme) + */ + u32 device_id_scheme:1; }; struct acpi_video_enumerated_device { @@ -728,7 +740,33 @@ static int acpi_video_bqc_quirk(struct acpi_video_device *device, /* * Some systems always report current brightness level as maximum - * through _BQC, we need to test another value for them. + * through _BQC, we need to test another value for them. However, + * there is a subtlety: + * + * If the _BCL package ordering is descending, the first level + * (br->levels[2]) is likely to be 0, and if the number of levels + * matches the number of steps, we might confuse a returned level to + * mean the index. + * + * For example: + * + * current_level = max_level = 100 + * test_level = 0 + * returned level = 100 + * + * In this case 100 means the level, not the index, and _BCM failed. + * Still, if the _BCL package ordering is descending, the index of + * level 0 is also 100, so we assume _BQC is indexed, when it's not. + * + * This causes all _BQC calls to return bogus values causing weird + * behavior from the user's perspective. For example: + * + * xbacklight -set 10; xbacklight -set 20; + * + * would flash to 90% and then slowly down to the desired level (20). + * + * The solution is simple; test anything other than the first level + * (e.g. 1). */ test_level = current_level == max_level ? br->levels[ACPI_VIDEO_FIRST_LEVEL + 1] @@ -789,6 +827,11 @@ int acpi_video_get_levels(struct acpi_device *device, goto out; } + /* + * Note that we have to reserve 2 extra items (ACPI_VIDEO_FIRST_LEVEL), + * in order to account for buggy BIOS which don't export the first two + * special levels (see below) + */ br->levels = kmalloc((obj->package.count + ACPI_VIDEO_FIRST_LEVEL) * sizeof(*br->levels), GFP_KERNEL); if (!br->levels) { From 8661423eea1a1b58417014716e3f1ba286072379 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 19 Apr 2017 14:02:08 +0200 Subject: [PATCH 11/19] ACPI / utils: Add new acpi_dev_present helper acpi_dev_found just iterates over all ACPI-ids and sees if one matches. This means that it will return true for devices which are in the DSDT but disabled (their _STA method returns 0). For some drivers it is useful to be able to check if a certain HID is not only present in the namespace, but also actually present as in acpi_device_is_present() will return true for the device. For example because if a certain device is present then the driver will want to use an extcon or IIO ADC channel provided by that device. This commit adds a new acpi_dev_present helper which drivers can use to this end. Like acpi_dev_found, acpi_dev_present take a HID as argument, but it also has 2 extra optional arguments to only check for an ACPI device with a specific UID and/or HRV value. This makes it more generic and allows it to replace custom code doing similar checks in several places. Arguably acpi_dev_present is what acpi_dev_found should have been, but there are too many users to just change acpi_dev_found without the risk of breaking something. Signed-off-by: Hans de Goede Reviewed-by: Lukas Wunner Reviewed-by: Mika Westerberg Signed-off-by: Rafael J. Wysocki --- drivers/acpi/utils.c | 66 +++++++++++++++++++++++++++++++++++++++++ include/acpi/acpi_bus.h | 1 + include/linux/acpi.h | 5 ++++ 3 files changed, 72 insertions(+) diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c index 22c09952e177..27d0dcfcf47d 100644 --- a/drivers/acpi/utils.c +++ b/drivers/acpi/utils.c @@ -736,6 +736,72 @@ bool acpi_dev_found(const char *hid) } EXPORT_SYMBOL(acpi_dev_found); +struct acpi_dev_present_info { + struct acpi_device_id hid[2]; + const char *uid; + s64 hrv; +}; + +static int acpi_dev_present_cb(struct device *dev, void *data) +{ + struct acpi_device *adev = to_acpi_device(dev); + struct acpi_dev_present_info *match = data; + unsigned long long hrv; + acpi_status status; + + if (acpi_match_device_ids(adev, match->hid)) + return 0; + + if (match->uid && (!adev->pnp.unique_id || + strcmp(adev->pnp.unique_id, match->uid))) + return 0; + + if (match->hrv == -1) + return 1; + + status = acpi_evaluate_integer(adev->handle, "_HRV", NULL, &hrv); + if (ACPI_FAILURE(status)) + return 0; + + return hrv == match->hrv; +} + +/** + * acpi_dev_present - Detect that a given ACPI device is present + * @hid: Hardware ID of the device. + * @uid: Unique ID of the device, pass NULL to not check _UID + * @hrv: Hardware Revision of the device, pass -1 to not check _HRV + * + * Return %true if a matching device was present at the moment of invocation. + * Note that if the device is pluggable, it may since have disappeared. + * + * Note that unlike acpi_dev_found() this function checks the status + * of the device. So for devices which are present in the dsdt, but + * which are disabled (their _STA callback returns 0) this function + * will return false. + * + * For this function to work, acpi_bus_scan() must have been executed + * which happens in the subsys_initcall() subsection. Hence, do not + * call from a subsys_initcall() or earlier (use acpi_get_devices() + * instead). Calling from module_init() is fine (which is synonymous + * with device_initcall()). + */ +bool acpi_dev_present(const char *hid, const char *uid, s64 hrv) +{ + struct acpi_dev_present_info match = {}; + struct device *dev; + + strlcpy(match.hid[0].id, hid, sizeof(match.hid[0].id)); + match.uid = uid; + match.hrv = hrv; + + dev = bus_find_device(&acpi_bus_type, NULL, &match, + acpi_dev_present_cb); + + return !!dev; +} +EXPORT_SYMBOL(acpi_dev_present); + /* * acpi_backlight= handling, this is done here rather then in video_detect.c * because __setup cannot be used in modules. diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index ef0ae8aaa567..b53c058e7009 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -88,6 +88,7 @@ acpi_evaluate_dsm_typed(acpi_handle handle, const u8 *uuid, u64 rev, u64 func, } bool acpi_dev_found(const char *hid); +bool acpi_dev_present(const char *hid, const char *uid, s64 hrv); #ifdef CONFIG_ACPI diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 9b05886f9773..841a8dc55ade 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -611,6 +611,11 @@ static inline bool acpi_dev_found(const char *hid) return false; } +static inline bool acpi_dev_present(const char *hid, const char *uid, s64 hrv) +{ + return false; +} + static inline bool is_acpi_node(struct fwnode_handle *fwnode) { return false; From bc39fbcf9c782970263bdc5b428e4a755db16efb Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 19 Apr 2017 14:02:09 +0200 Subject: [PATCH 12/19] ACPI / battery: Fix acpi_battery_exit on acpi_battery_init_async errors The acpi_lock_battery_dir() / acpi_bus_register_driver() calls in acpi_battery_init_async() may fail. Check that they succeeded before undoing them. Signed-off-by: Hans de Goede Signed-off-by: Rafael J. Wysocki --- drivers/acpi/battery.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c index 4ef1e4624b2b..b35fca478097 100644 --- a/drivers/acpi/battery.c +++ b/drivers/acpi/battery.c @@ -67,6 +67,7 @@ MODULE_DESCRIPTION("ACPI Battery Driver"); MODULE_LICENSE("GPL"); static async_cookie_t async_cookie; +static bool battery_driver_registered; static int battery_bix_broken_package; static int battery_notification_delay_ms; static unsigned int cache_time = 1000; @@ -1329,6 +1330,7 @@ static void __init acpi_battery_init_async(void *unused, async_cookie_t cookie) if (result < 0) acpi_unlock_battery_dir(acpi_battery_dir); #endif + battery_driver_registered = (result == 0); } static int __init acpi_battery_init(void) @@ -1343,9 +1345,11 @@ static int __init acpi_battery_init(void) static void __exit acpi_battery_exit(void) { async_synchronize_cookie(async_cookie + 1); - acpi_bus_unregister_driver(&acpi_battery_driver); + if (battery_driver_registered) + acpi_bus_unregister_driver(&acpi_battery_driver); #ifdef CONFIG_ACPI_PROCFS_POWER - acpi_unlock_battery_dir(acpi_battery_dir); + if (acpi_battery_dir) + acpi_unlock_battery_dir(acpi_battery_dir); #endif } From dccfae6d4f4c2cfa1fdc3bf55755fcad02184b99 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 19 Apr 2017 14:02:10 +0200 Subject: [PATCH 13/19] ACPI / battery: Add a blacklist with PMIC ACPI HIDs with a native battery driver On some systems we have a native PMIC driver which provides battery monitoring, while the ACPI battery driver is broken on these systems due to bad DSDTs or because we do not support the proprietary and undocumented ACPI opregions these ACPI battery devices rely on (e.g. BMOP opregion). This leads to there being 2 battery power_supply-s registed like this: ~$ acpi Battery 0: Charging, 84%, 00:49:39 until charged Battery 1: Unknown, 0%, rate information unavailable Even if the ACPI battery where to function fine (which on systems where we have a native PMIC driver it often doesn't) we still do not want to export the same battery to userspace twice. This commit adds a blacklist with PMIC ACPI HIDs for which we've a native battery driver and makes the ACPI battery driver not register itself when a PMIC on this list is present. Link: https://bugzilla.kernel.org/show_bug.cgi?id=194811 Signed-off-by: Hans de Goede Signed-off-by: Rafael J. Wysocki --- drivers/acpi/battery.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c index b35fca478097..d42eeef9d928 100644 --- a/drivers/acpi/battery.c +++ b/drivers/acpi/battery.c @@ -94,6 +94,11 @@ static const struct acpi_device_id battery_device_ids[] = { MODULE_DEVICE_TABLE(acpi, battery_device_ids); +/* Lists of PMIC ACPI HIDs with an (often better) native battery driver */ +static const char * const acpi_battery_blacklist[] = { + "INT33F4", /* X-Powers AXP288 PMIC */ +}; + enum { ACPI_BATTERY_ALARM_PRESENT, ACPI_BATTERY_XINFO_PRESENT, @@ -1316,8 +1321,17 @@ static struct acpi_driver acpi_battery_driver = { static void __init acpi_battery_init_async(void *unused, async_cookie_t cookie) { + unsigned int i; int result; + for (i = 0; i < ARRAY_SIZE(acpi_battery_blacklist); i++) + if (acpi_dev_present(acpi_battery_blacklist[i], "1", -1)) { + pr_info(PREFIX ACPI_BATTERY_DEVICE_NAME + ": found native %s PMIC, not loading\n", + acpi_battery_blacklist[i]); + return; + } + dmi_check_system(bat_dmi_table); #ifdef CONFIG_ACPI_PROCFS_POWER From af3ec837b84befedde35a2239f873f6cd7c09e68 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 19 Apr 2017 14:02:11 +0200 Subject: [PATCH 14/19] ACPI / AC: Add a blacklist with PMIC ACPI HIDs with a native charger driver On some systems we have a native PMIC driver which provides Mains monitoring, while the ACPI ac driver is broken on these systems due to bad DSTDs or because we do not support the proprietary and undocumented ACPI opregions these ACPI battery devices rely on (e.g. BMOP opregion). This leads for example to a ADP1 power_supply which reports itself as always online even if no mains are connected. This commit adds a blacklist with PMIC ACPI HIDs for which we've a native charger or extcon driver and makes the ACPI ac driver not register itself when a PMIC on this list is present. Signed-off-by: Hans de Goede Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ac.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c index f71b756b05c4..8f52483219ba 100644 --- a/drivers/acpi/ac.c +++ b/drivers/acpi/ac.c @@ -57,12 +57,23 @@ static int acpi_ac_add(struct acpi_device *device); static int acpi_ac_remove(struct acpi_device *device); static void acpi_ac_notify(struct acpi_device *device, u32 event); +struct acpi_ac_bl { + const char *hid; + int hrv; +}; + static const struct acpi_device_id ac_device_ids[] = { {"ACPI0003", 0}, {"", 0}, }; MODULE_DEVICE_TABLE(acpi, ac_device_ids); +/* Lists of PMIC ACPI HIDs with an (often better) native charger driver */ +static const struct acpi_ac_bl acpi_ac_blacklist[] = { + { "INT33F4", -1 }, /* X-Powers AXP288 PMIC */ + { "INT34D3", 3 }, /* Intel Cherrytrail Whiskey Cove PMIC */ +}; + #ifdef CONFIG_PM_SLEEP static int acpi_ac_resume(struct device *dev); #endif @@ -424,11 +435,20 @@ static int acpi_ac_remove(struct acpi_device *device) static int __init acpi_ac_init(void) { + unsigned int i; int result; if (acpi_disabled) return -ENODEV; + for (i = 0; i < ARRAY_SIZE(acpi_ac_blacklist); i++) + if (acpi_dev_present(acpi_ac_blacklist[i].hid, "1", + acpi_ac_blacklist[i].hrv)) { + pr_info(PREFIX "AC: found native %s PMIC, not loading\n", + acpi_ac_blacklist[i].hid); + return -ENODEV; + } + #ifdef CONFIG_ACPI_PROCFS_POWER acpi_ac_dir = acpi_lock_ac_dir(); if (!acpi_ac_dir) From 6c4c9a9a4a294a2e85784d0eaf6a4f833ee99752 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 19 Apr 2017 14:02:12 +0200 Subject: [PATCH 15/19] power: supply: axp288_charger: Only wait for INT3496 device if present On some devices with an axp288 pmic setting vbus path based on the id-pin is handled by an ACPI _AIE interrupt on the gpio and the INT3496 device is disabled. Instead of returning -EPROBE_DEFER on these devices waiting for the never to show up INT3496 device, check for its presence and only request and monitor the matching extcon if the device is there, otherwise let the firmware handle the vbus path control. Signed-off-by: Hans de Goede Acked-by: Sebastian Reichel Signed-off-by: Rafael J. Wysocki --- drivers/power/supply/axp288_charger.c | 28 +++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/drivers/power/supply/axp288_charger.c b/drivers/power/supply/axp288_charger.c index 6be2fe27bb07..d51ebd1da65e 100644 --- a/drivers/power/supply/axp288_charger.c +++ b/drivers/power/supply/axp288_charger.c @@ -14,6 +14,7 @@ * GNU General Public License for more details. */ +#include #include #include #include @@ -113,7 +114,8 @@ #define ILIM_3000MA 3000 /* 3000mA */ #define AXP288_EXTCON_DEV_NAME "axp288_extcon" -#define USB_HOST_EXTCON_DEV_NAME "INT3496:00" +#define USB_HOST_EXTCON_HID "INT3496" +#define USB_HOST_EXTCON_NAME "INT3496:00" static const unsigned int cable_ids[] = { EXTCON_CHG_USB_SDP, EXTCON_CHG_USB_CDP, EXTCON_CHG_USB_DCP }; @@ -807,10 +809,14 @@ static int axp288_charger_probe(struct platform_device *pdev) return -EPROBE_DEFER; } - info->otg.cable = extcon_get_extcon_dev(USB_HOST_EXTCON_DEV_NAME); - if (info->otg.cable == NULL) { - dev_dbg(dev, "EXTCON_USB_HOST is not ready, probe deferred\n"); - return -EPROBE_DEFER; + if (acpi_dev_present(USB_HOST_EXTCON_HID, NULL, -1)) { + info->otg.cable = extcon_get_extcon_dev(USB_HOST_EXTCON_NAME); + if (info->otg.cable == NULL) { + dev_dbg(dev, "EXTCON_USB_HOST is not ready, probe deferred\n"); + return -EPROBE_DEFER; + } + dev_info(&pdev->dev, + "Using " USB_HOST_EXTCON_HID " extcon for usb-id\n"); } platform_set_drvdata(pdev, info); @@ -849,13 +855,15 @@ static int axp288_charger_probe(struct platform_device *pdev) /* Register for OTG notification */ INIT_WORK(&info->otg.work, axp288_charger_otg_evt_worker); info->otg.id_nb.notifier_call = axp288_charger_handle_otg_evt; - ret = devm_extcon_register_notifier(&pdev->dev, info->otg.cable, + if (info->otg.cable) { + ret = devm_extcon_register_notifier(&pdev->dev, info->otg.cable, EXTCON_USB_HOST, &info->otg.id_nb); - if (ret) { - dev_err(dev, "failed to register EXTCON_USB_HOST notifier\n"); - return ret; + if (ret) { + dev_err(dev, "failed to register EXTCON_USB_HOST notifier\n"); + return ret; + } + schedule_work(&info->otg.work); } - schedule_work(&info->otg.work); /* Register charger interrupts */ for (i = 0; i < CHRG_INTR_END; i++) { From f5beabfe61794d9a9d9549d387cda2bffd81e504 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Mon, 17 Apr 2017 01:19:50 +0200 Subject: [PATCH 16/19] ACPI / scan: Apply default enumeration to devices with ACPI drivers The current code in acpi_bus_attach() is inconsistent with respect to device objects with ACPI drivers bound to them, as it allows ACPI drivers to bind to device objects with existing "physical" device companions, but it doesn't allow "physical" device objects to be created for ACPI device objects with ACPI drivers bound to them. Thus, in some cases, the outcome depends on the ordering of events which is confusing at best. For this reason, modify acpi_bus_attach() to call acpi_default_enumeration() for device objects with the pnp.type.platform_id flag set regardless of whether or not any ACPI drivers are bound to them. Signed-off-by: Rafael J. Wysocki Reviewed-by: Mika Westerberg Reviewed-by: Joey Lee --- drivers/acpi/scan.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index acfa6c0831f2..f74bc0d28692 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -1856,10 +1856,10 @@ static void acpi_bus_attach(struct acpi_device *device) if (ret < 0) return; - if (ret > 0 || !device->pnp.type.platform_id) - acpi_device_set_enumerated(device); - else + if (device->pnp.type.platform_id) acpi_default_enumeration(device); + else + acpi_device_set_enumerated(device); ok: list_for_each_entry(child, &device->children, node) From c381fc3a1bbfc9d780048a0b94afd162e4c3815b Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Mon, 17 Apr 2017 01:20:48 +0200 Subject: [PATCH 17/19] ACPI / scan: Avoid enumerating devices more than once acpi_bus_attach() does not check the visited flag for devices that have been enumerated already and some of them may be enumerated for multiple times as a result, because some callers of acpi_bus_scan() don't check the visited flag either. For this reason, modify acpi_bus_attach() to check the visited flag and avoid enumerating devices that have already been enumerated. Signed-off-by: Rafael J. Wysocki Reviewed-by: Mika Westerberg Reviewed-by: Joey Lee --- drivers/acpi/scan.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index f74bc0d28692..c26931067415 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -1840,6 +1840,8 @@ static void acpi_bus_attach(struct acpi_device *device) device->flags.power_manageable = 0; device->flags.initialized = true; + } else if (device->flags.visited) { + goto ok; } ret = acpi_scan_attach_handler(device); From ac2c4936e9ec76f1d5c4cd2afdc8258769635b7a Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 19 Apr 2017 15:06:59 +0200 Subject: [PATCH 18/19] ACPI / PMIC: Add opregion driver for Intel CHT Whiskey Cove PMIC Add opregion driver for Intel CHT Whiskey Cove PMIC, based on various non upstreamed CHT Whiskey Cove PMIC patches. This does not include support for the Thermal opregion (DPTF) due to lacking documentation. Signed-off-by: Hans de Goede Reviewed-by: Andy Shevchenko Signed-off-by: Rafael J. Wysocki --- drivers/acpi/Kconfig | 6 + drivers/acpi/Makefile | 1 + drivers/acpi/pmic/intel_pmic_chtwc.c | 280 +++++++++++++++++++++++++++ 3 files changed, 287 insertions(+) create mode 100644 drivers/acpi/pmic/intel_pmic_chtwc.c diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig index 83e5f7e1a20d..4f12fe075fad 100644 --- a/drivers/acpi/Kconfig +++ b/drivers/acpi/Kconfig @@ -516,6 +516,12 @@ config BXT_WC_PMIC_OPREGION help This config adds ACPI operation region support for BXT WhiskeyCove PMIC. +config CHT_WC_PMIC_OPREGION + bool "ACPI operation region support for CHT Whiskey Cove PMIC" + depends on INTEL_SOC_PMIC_CHTWC + help + This config adds ACPI operation region support for CHT Whiskey Cove PMIC. + endif config ACPI_CONFIGFS diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile index d94f92f88ca1..d78065cc9324 100644 --- a/drivers/acpi/Makefile +++ b/drivers/acpi/Makefile @@ -101,6 +101,7 @@ obj-$(CONFIG_PMIC_OPREGION) += pmic/intel_pmic.o obj-$(CONFIG_CRC_PMIC_OPREGION) += pmic/intel_pmic_crc.o obj-$(CONFIG_XPOWER_PMIC_OPREGION) += pmic/intel_pmic_xpower.o obj-$(CONFIG_BXT_WC_PMIC_OPREGION) += pmic/intel_pmic_bxtwc.o +obj-$(CONFIG_CHT_WC_PMIC_OPREGION) += pmic/intel_pmic_chtwc.o obj-$(CONFIG_ACPI_CONFIGFS) += acpi_configfs.o diff --git a/drivers/acpi/pmic/intel_pmic_chtwc.c b/drivers/acpi/pmic/intel_pmic_chtwc.c new file mode 100644 index 000000000000..85636d7a9d39 --- /dev/null +++ b/drivers/acpi/pmic/intel_pmic_chtwc.c @@ -0,0 +1,280 @@ +/* + * Intel CHT Whiskey Cove PMIC operation region driver + * Copyright (C) 2017 Hans de Goede + * + * Based on various non upstream patches to support the CHT Whiskey Cove PMIC: + * Copyright (C) 2013-2015 Intel Corporation. All rights reserved. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License version + * 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include +#include +#include +#include +#include +#include "intel_pmic.h" + +#define CHT_WC_V1P05A_CTRL 0x6e3b +#define CHT_WC_V1P15_CTRL 0x6e3c +#define CHT_WC_V1P05A_VSEL 0x6e3d +#define CHT_WC_V1P15_VSEL 0x6e3e +#define CHT_WC_V1P8A_CTRL 0x6e56 +#define CHT_WC_V1P8SX_CTRL 0x6e57 +#define CHT_WC_VDDQ_CTRL 0x6e58 +#define CHT_WC_V1P2A_CTRL 0x6e59 +#define CHT_WC_V1P2SX_CTRL 0x6e5a +#define CHT_WC_V1P8A_VSEL 0x6e5b +#define CHT_WC_VDDQ_VSEL 0x6e5c +#define CHT_WC_V2P8SX_CTRL 0x6e5d +#define CHT_WC_V3P3A_CTRL 0x6e5e +#define CHT_WC_V3P3SD_CTRL 0x6e5f +#define CHT_WC_VSDIO_CTRL 0x6e67 +#define CHT_WC_V3P3A_VSEL 0x6e68 +#define CHT_WC_VPROG1A_CTRL 0x6e90 +#define CHT_WC_VPROG1B_CTRL 0x6e91 +#define CHT_WC_VPROG1F_CTRL 0x6e95 +#define CHT_WC_VPROG2D_CTRL 0x6e99 +#define CHT_WC_VPROG3A_CTRL 0x6e9a +#define CHT_WC_VPROG3B_CTRL 0x6e9b +#define CHT_WC_VPROG4A_CTRL 0x6e9c +#define CHT_WC_VPROG4B_CTRL 0x6e9d +#define CHT_WC_VPROG4C_CTRL 0x6e9e +#define CHT_WC_VPROG4D_CTRL 0x6e9f +#define CHT_WC_VPROG5A_CTRL 0x6ea0 +#define CHT_WC_VPROG5B_CTRL 0x6ea1 +#define CHT_WC_VPROG6A_CTRL 0x6ea2 +#define CHT_WC_VPROG6B_CTRL 0x6ea3 +#define CHT_WC_VPROG1A_VSEL 0x6ec0 +#define CHT_WC_VPROG1B_VSEL 0x6ec1 +#define CHT_WC_V1P8SX_VSEL 0x6ec2 +#define CHT_WC_V1P2SX_VSEL 0x6ec3 +#define CHT_WC_V1P2A_VSEL 0x6ec4 +#define CHT_WC_VPROG1F_VSEL 0x6ec5 +#define CHT_WC_VSDIO_VSEL 0x6ec6 +#define CHT_WC_V2P8SX_VSEL 0x6ec7 +#define CHT_WC_V3P3SD_VSEL 0x6ec8 +#define CHT_WC_VPROG2D_VSEL 0x6ec9 +#define CHT_WC_VPROG3A_VSEL 0x6eca +#define CHT_WC_VPROG3B_VSEL 0x6ecb +#define CHT_WC_VPROG4A_VSEL 0x6ecc +#define CHT_WC_VPROG4B_VSEL 0x6ecd +#define CHT_WC_VPROG4C_VSEL 0x6ece +#define CHT_WC_VPROG4D_VSEL 0x6ecf +#define CHT_WC_VPROG5A_VSEL 0x6ed0 +#define CHT_WC_VPROG5B_VSEL 0x6ed1 +#define CHT_WC_VPROG6A_VSEL 0x6ed2 +#define CHT_WC_VPROG6B_VSEL 0x6ed3 + +/* + * Regulator support is based on the non upstream patch: + * "regulator: whiskey_cove: implements Whiskey Cove pmic VRF support" + * https://github.com/intel-aero/meta-intel-aero/blob/master/recipes-kernel/linux/linux-yocto/0019-regulator-whiskey_cove-implements-WhiskeyCove-pmic-V.patch + */ +static struct pmic_table power_table[] = { + { + .address = 0x0, + .reg = CHT_WC_V1P8A_CTRL, + .bit = 0x01, + }, /* V18A */ + { + .address = 0x04, + .reg = CHT_WC_V1P8SX_CTRL, + .bit = 0x07, + }, /* V18X */ + { + .address = 0x08, + .reg = CHT_WC_VDDQ_CTRL, + .bit = 0x01, + }, /* VDDQ */ + { + .address = 0x0c, + .reg = CHT_WC_V1P2A_CTRL, + .bit = 0x07, + }, /* V12A */ + { + .address = 0x10, + .reg = CHT_WC_V1P2SX_CTRL, + .bit = 0x07, + }, /* V12X */ + { + .address = 0x14, + .reg = CHT_WC_V2P8SX_CTRL, + .bit = 0x07, + }, /* V28X */ + { + .address = 0x18, + .reg = CHT_WC_V3P3A_CTRL, + .bit = 0x01, + }, /* V33A */ + { + .address = 0x1c, + .reg = CHT_WC_V3P3SD_CTRL, + .bit = 0x07, + }, /* V3SD */ + { + .address = 0x20, + .reg = CHT_WC_VSDIO_CTRL, + .bit = 0x07, + }, /* VSD */ +/* { + .address = 0x24, + .reg = ??, + .bit = ??, + }, ** VSW2 */ +/* { + .address = 0x28, + .reg = ??, + .bit = ??, + }, ** VSW1 */ +/* { + .address = 0x2c, + .reg = ??, + .bit = ??, + }, ** VUPY */ +/* { + .address = 0x30, + .reg = ??, + .bit = ??, + }, ** VRSO */ + { + .address = 0x34, + .reg = CHT_WC_VPROG1A_CTRL, + .bit = 0x07, + }, /* VP1A */ + { + .address = 0x38, + .reg = CHT_WC_VPROG1B_CTRL, + .bit = 0x07, + }, /* VP1B */ + { + .address = 0x3c, + .reg = CHT_WC_VPROG1F_CTRL, + .bit = 0x07, + }, /* VP1F */ + { + .address = 0x40, + .reg = CHT_WC_VPROG2D_CTRL, + .bit = 0x07, + }, /* VP2D */ + { + .address = 0x44, + .reg = CHT_WC_VPROG3A_CTRL, + .bit = 0x07, + }, /* VP3A */ + { + .address = 0x48, + .reg = CHT_WC_VPROG3B_CTRL, + .bit = 0x07, + }, /* VP3B */ + { + .address = 0x4c, + .reg = CHT_WC_VPROG4A_CTRL, + .bit = 0x07, + }, /* VP4A */ + { + .address = 0x50, + .reg = CHT_WC_VPROG4B_CTRL, + .bit = 0x07, + }, /* VP4B */ + { + .address = 0x54, + .reg = CHT_WC_VPROG4C_CTRL, + .bit = 0x07, + }, /* VP4C */ + { + .address = 0x58, + .reg = CHT_WC_VPROG4D_CTRL, + .bit = 0x07, + }, /* VP4D */ + { + .address = 0x5c, + .reg = CHT_WC_VPROG5A_CTRL, + .bit = 0x07, + }, /* VP5A */ + { + .address = 0x60, + .reg = CHT_WC_VPROG5B_CTRL, + .bit = 0x07, + }, /* VP5B */ + { + .address = 0x64, + .reg = CHT_WC_VPROG6A_CTRL, + .bit = 0x07, + }, /* VP6A */ + { + .address = 0x68, + .reg = CHT_WC_VPROG6B_CTRL, + .bit = 0x07, + }, /* VP6B */ +/* { + .address = 0x6c, + .reg = ??, + .bit = ??, + } ** VP7A */ +}; + +static int intel_cht_wc_pmic_get_power(struct regmap *regmap, int reg, + int bit, u64 *value) +{ + int data; + + if (regmap_read(regmap, reg, &data)) + return -EIO; + + *value = (data & bit) ? 1 : 0; + return 0; +} + +static int intel_cht_wc_pmic_update_power(struct regmap *regmap, int reg, + int bitmask, bool on) +{ + return regmap_update_bits(regmap, reg, bitmask, on ? 1 : 0); +} + +/* + * The thermal table and ops are empty, we do not support the Thermal opregion + * (DPTF) due to lacking documentation. + */ +static struct intel_pmic_opregion_data intel_cht_wc_pmic_opregion_data = { + .get_power = intel_cht_wc_pmic_get_power, + .update_power = intel_cht_wc_pmic_update_power, + .power_table = power_table, + .power_table_count = ARRAY_SIZE(power_table), +}; + +static int intel_cht_wc_pmic_opregion_probe(struct platform_device *pdev) +{ + struct intel_soc_pmic *pmic = dev_get_drvdata(pdev->dev.parent); + + return intel_pmic_install_opregion_handler(&pdev->dev, + ACPI_HANDLE(pdev->dev.parent), + pmic->regmap, + &intel_cht_wc_pmic_opregion_data); +} + +static struct platform_device_id cht_wc_opregion_id_table[] = { + { .name = "cht_wcove_region" }, + {}, +}; +MODULE_DEVICE_TABLE(platform, cht_wc_opregion_id_table); + +static struct platform_driver intel_cht_wc_pmic_opregion_driver = { + .probe = intel_cht_wc_pmic_opregion_probe, + .driver = { + .name = "cht_whiskey_cove_pmic", + }, + .id_table = cht_wc_opregion_id_table, +}; +module_platform_driver(intel_cht_wc_pmic_opregion_driver); + +MODULE_DESCRIPTION("Intel CHT Whiskey Cove PMIC operation region driver"); +MODULE_AUTHOR("Hans de Goede "); +MODULE_LICENSE("GPL"); From 2e5a7f71095be27064c140faf6cecdab585ff198 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 19 Apr 2017 15:07:00 +0200 Subject: [PATCH 19/19] ACPI / PMIC: Stop xpower OPRegion handler relying on IIO The intel_pmic_xpower code provides an OPRegion handler, which must be available before other drivers using it are loaded, which can only be ensured if both the mfd and opregion drivers are built in, which is why the Kconfig option for intel_pmic_xpower is a bool. The use of IIO is causing trouble for generic distro configs here as distros will typically want to build IIO drivers as modules and there really is no reason to use IIO here. The reading of the ADC value is a single regmap_bulk_read, which is already protected against races by the regmap-lock. This commit removes the use of IIO, allowing distros to enable the driver without needing to built IIO in and also actually simplifies the code. Signed-off-by: Hans de Goede Signed-off-by: Rafael J. Wysocki --- drivers/acpi/Kconfig | 2 +- drivers/acpi/pmic/intel_pmic_xpower.c | 21 ++++----------------- 2 files changed, 5 insertions(+), 18 deletions(-) diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig index 4f12fe075fad..842530fcd41b 100644 --- a/drivers/acpi/Kconfig +++ b/drivers/acpi/Kconfig @@ -506,7 +506,7 @@ config CRC_PMIC_OPREGION config XPOWER_PMIC_OPREGION bool "ACPI operation region support for XPower AXP288 PMIC" - depends on AXP288_ADC = y + depends on MFD_AXP20X_I2C help This config adds ACPI operation region support for XPower AXP288 PMIC. diff --git a/drivers/acpi/pmic/intel_pmic_xpower.c b/drivers/acpi/pmic/intel_pmic_xpower.c index e6e991ac20f3..55f51115f016 100644 --- a/drivers/acpi/pmic/intel_pmic_xpower.c +++ b/drivers/acpi/pmic/intel_pmic_xpower.c @@ -18,7 +18,6 @@ #include #include #include -#include #include "intel_pmic.h" #define XPOWER_GPADC_LOW 0x5b @@ -186,28 +185,16 @@ static int intel_xpower_pmic_update_power(struct regmap *regmap, int reg, * @regmap: regmap of the PMIC device * @reg: register to get the reading * - * We could get the sensor value by manipulating the HW regs here, but since - * the axp288 IIO driver may also access the same regs at the same time, the - * APIs provided by IIO subsystem are used here instead to avoid problems. As - * a result, the two passed in params are of no actual use. - * * Return a positive value on success, errno on failure. */ static int intel_xpower_pmic_get_raw_temp(struct regmap *regmap, int reg) { - struct iio_channel *gpadc_chan; - int ret, val; + u8 buf[2]; - gpadc_chan = iio_channel_get(NULL, "axp288-system-temp"); - if (IS_ERR_OR_NULL(gpadc_chan)) - return -EACCES; + if (regmap_bulk_read(regmap, AXP288_GP_ADC_H, buf, 2)) + return -EIO; - ret = iio_read_channel_raw(gpadc_chan, &val); - if (ret < 0) - val = ret; - - iio_channel_release(gpadc_chan); - return val; + return (buf[0] << 4) + ((buf[1] >> 4) & 0x0F); } static struct intel_pmic_opregion_data intel_xpower_pmic_opregion_data = {