| Message ID | 20201130133129.1024662-15-djrscally@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add functionality to ipu3-cio2 driver allowing software_node connections to sensors on platforms designed for Windows | expand |
On Mon, Nov 30, 2020 at 01:31:25PM +0000, Daniel Scally wrote: > ACPI devices declare themselves dependent on other devices via the _DEP > buffer. Fetching the dependee from dependent is a matter of parsing > _DEP, but currently there's no method to fetch dependent from dependee. > Add one, so we can parse sensors dependent on a PMIC from the PMIC's > acpi_driver. Do I understand correctly that it's an existing table provided by firmware that (ab)uses _DEP in such way? Note, the specification doesn't tell we may use it in this way, OTOH I don't remember if it strictly forbids such use. So, please elaborate in the commit message why you need this and pint out to the 6.5.8 "_DEP (Operation Region Dependencies)" which clearly says about OpRegions and that part already supported by ACPI in the Linux, if I'm not mistaken, need to refresh my memory. ... > + handle = adev->handle; > + > + if (!acpi_has_method(handle, "_DEP")) > + return 0; > + > + status = acpi_evaluate_reference(handle, "_DEP", NULL, &dep_handles); > + if (ACPI_FAILURE(status)) > + return 0; > + > + for (i = 0; i < dep_handles.count; i++) { > + struct acpi_device_info *info; > + > + status = acpi_get_object_info(dep_handles.handles[i], &info); > + if (ACPI_FAILURE(status)) > + continue; > + > + if (info->valid & ACPI_VALID_HID) { > + ret = acpi_bus_get_device(dep_handles.handles[i], &candidate); > + if (ret || !candidate) { > + kfree(info); > + continue; > + } > + > + if (candidate == dependee) { > + acpi_dev_put(candidate); > + kfree(info); > + return 1; > + } > + > + kfree(info); > + } > + } Can you utilize (by moving to here and export for ACPI layer the acpi_lpss_dep()?
On Mon, Nov 30, 2020 at 11:54:44PM +0000, Dan Scally wrote: > Hi Andy > > On 30/11/2020 18:23, Andy Shevchenko wrote: > > On Mon, Nov 30, 2020 at 01:31:25PM +0000, Daniel Scally wrote: > >> ACPI devices declare themselves dependent on other devices via the _DEP > >> buffer. Fetching the dependee from dependent is a matter of parsing > >> _DEP, but currently there's no method to fetch dependent from dependee. > >> Add one, so we can parse sensors dependent on a PMIC from the PMIC's > >> acpi_driver. > > Do I understand correctly that it's an existing table provided by firmware that > > (ab)uses _DEP in such way? Note, the specification doesn't tell we may use it > > in this way, OTOH I don't remember if it strictly forbids such use. > > > > So, please elaborate in the commit message why you need this and pint out to > > the 6.5.8 "_DEP (Operation Region Dependencies)" which clearly says about > > OpRegions and that part already supported by ACPI in the Linux, if I'm not > > mistaken, need to refresh my memory. > > > Laurent's reply is good explanation, but for example see my Lenovo Miix > 510's DSDT: > > > https://gist.githubusercontent.com/djrscally/e64d112180517352fa3392878b0f4a7d/raw/88b90b3ea4204fd7845257b6666fdade47cc2981/dsdt.dsl > > > Search OVTI2680 and OVTI5648 for the cameras. Both are dependent on > IN3472 devices (PMI0 and PMI1) which are the discrete type that we're > attempting to handle here. Yes, it seems since PMIC is kinda "power resource" (don't mix with real power resource as by ACPI specifications) and that's why they decided to include it into _DEP. So, it seems a de facto common practice. Thus, it would be nice to have the above in the commit message in some form. Can you do it?
On 01/12/2020 15:10, Andy Shevchenko wrote: > On Mon, Nov 30, 2020 at 11:54:44PM +0000, Dan Scally wrote: >> Hi Andy >> >> On 30/11/2020 18:23, Andy Shevchenko wrote: >>> On Mon, Nov 30, 2020 at 01:31:25PM +0000, Daniel Scally wrote: >>>> ACPI devices declare themselves dependent on other devices via the _DEP >>>> buffer. Fetching the dependee from dependent is a matter of parsing >>>> _DEP, but currently there's no method to fetch dependent from dependee. >>>> Add one, so we can parse sensors dependent on a PMIC from the PMIC's >>>> acpi_driver. >>> Do I understand correctly that it's an existing table provided by firmware that >>> (ab)uses _DEP in such way? Note, the specification doesn't tell we may use it >>> in this way, OTOH I don't remember if it strictly forbids such use. >>> >>> So, please elaborate in the commit message why you need this and pint out to >>> the 6.5.8 "_DEP (Operation Region Dependencies)" which clearly says about >>> OpRegions and that part already supported by ACPI in the Linux, if I'm not >>> mistaken, need to refresh my memory. >> >> Laurent's reply is good explanation, but for example see my Lenovo Miix >> 510's DSDT: >> >> >> https://gist.githubusercontent.com/djrscally/e64d112180517352fa3392878b0f4a7d/raw/88b90b3ea4204fd7845257b6666fdade47cc2981/dsdt.dsl >> >> >> Search OVTI2680 and OVTI5648 for the cameras. Both are dependent on >> IN3472 devices (PMI0 and PMI1) which are the discrete type that we're >> attempting to handle here. > Yes, it seems since PMIC is kinda "power resource" (don't mix with real power > resource as by ACPI specifications) and that's why they decided to include it > into _DEP. So, it seems a de facto common practice. Thus, it would be nice to > have the above in the commit message in some form. Can you do it? > Sure, no problem. I'll include that in the next version
diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c index c177165c8db2..7099529121db 100644 --- a/drivers/acpi/utils.c +++ b/drivers/acpi/utils.c @@ -807,6 +807,52 @@ static int acpi_dev_match_cb(struct device *dev, const void *data) return hrv == match->hrv; } +static int acpi_dev_match_by_dep(struct device *dev, const void *data) +{ + struct acpi_device *adev = to_acpi_device(dev); + const struct acpi_device *dependee = data; + struct acpi_handle_list dep_handles; + struct acpi_device *candidate; + acpi_handle handle; + acpi_status status; + unsigned int i; + int ret; + + handle = adev->handle; + + if (!acpi_has_method(handle, "_DEP")) + return 0; + + status = acpi_evaluate_reference(handle, "_DEP", NULL, &dep_handles); + if (ACPI_FAILURE(status)) + return 0; + + for (i = 0; i < dep_handles.count; i++) { + struct acpi_device_info *info; + + status = acpi_get_object_info(dep_handles.handles[i], &info); + if (ACPI_FAILURE(status)) + continue; + + if (info->valid & ACPI_VALID_HID) { + ret = acpi_bus_get_device(dep_handles.handles[i], &candidate); + if (ret || !candidate) { + kfree(info); + continue; + } + + if (candidate == dependee) { + acpi_dev_put(candidate); + kfree(info); + return 1; + } + + kfree(info); + } + } + return 0; +} + /** * acpi_dev_present - Detect that a given ACPI device is present * @hid: Hardware ID of the device. @@ -842,6 +888,28 @@ bool acpi_dev_present(const char *hid, const char *uid, s64 hrv) } EXPORT_SYMBOL(acpi_dev_present); +/** + * acpi_dev_get_next_dep_dev - Return next ACPI device dependent on input dev + * @adev: Pointer to the dependee device + * @prev: Pointer to the previous dependent device (or NULL for first match) + * + * Return the next ACPI device which declares itself dependent on @adev in + * the _DEP buffer. + * + * The caller is responsible to call put_device() on the returned device. + */ +struct acpi_device *acpi_dev_get_next_dep_dev(struct acpi_device *adev, + struct acpi_device *prev) +{ + struct device *start = prev ? &prev->dev : NULL; + struct device *dev; + + dev = bus_find_device(&acpi_bus_type, start, adev, acpi_dev_match_by_dep); + + return dev ? to_acpi_device(dev) : NULL; +} +EXPORT_SYMBOL(acpi_dev_get_next_dep_dev); + /** * acpi_dev_get_next_match_dev - Return the next match of ACPI device * @adev: Pointer to the previous acpi_device matching this hid, uid and hrv diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index 0a028ba967d3..f5dfeb030b9c 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -688,6 +688,8 @@ static inline bool acpi_device_can_poweroff(struct acpi_device *adev) bool acpi_dev_hid_uid_match(struct acpi_device *adev, const char *hid2, const char *uid2); +struct acpi_device * +acpi_dev_get_next_dep_dev(struct acpi_device *adev, struct acpi_device *prev); struct acpi_device * acpi_dev_get_next_match_dev(struct acpi_device *adev, const char *hid, const char *uid, s64 hrv); struct acpi_device *
ACPI devices declare themselves dependent on other devices via the _DEP buffer. Fetching the dependee from dependent is a matter of parsing _DEP, but currently there's no method to fetch dependent from dependee. Add one, so we can parse sensors dependent on a PMIC from the PMIC's acpi_driver. Signed-off-by: Daniel Scally <djrscally@gmail.com> --- Changes since RFC v3: - Patch introduced drivers/acpi/utils.c | 68 +++++++++++++++++++++++++++++++++++++++++ include/acpi/acpi_bus.h | 2 ++ 2 files changed, 70 insertions(+)