From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1CB06C433F5 for ; Mon, 21 Feb 2022 10:31:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1354683AbiBUKbh (ORCPT ); Mon, 21 Feb 2022 05:31:37 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:47804 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1345191AbiBUKam (ORCPT ); Mon, 21 Feb 2022 05:30:42 -0500 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 11B9769CC6 for ; Mon, 21 Feb 2022 01:52:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1645437075; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=hB9b8zcCF73wPaCqrb6w4xSegPlVd9cxYTgtjhyicfw=; b=McEW6Q1WXf/kpgWZFcryfOuHoDm14Iec/uXv8x9s1RvhiRwmzfe23lPiKLlvNy0OaOw94z M5PiDnfqCBryIO85xQNOAkp6ipyVqfuuCfLnVSQRIpMbyb9csob5S652eXg3DE+B3iL87P f1YRi7V5HPSN79FjQwotd41UmBrR4g8= Received: from mail-ed1-f71.google.com (mail-ed1-f71.google.com [209.85.208.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-517-lHTgmysXNZuPiAJkHA9SiQ-1; Mon, 21 Feb 2022 04:51:14 -0500 X-MC-Unique: lHTgmysXNZuPiAJkHA9SiQ-1 Received: by mail-ed1-f71.google.com with SMTP id j9-20020a056402238900b004128085d906so8938736eda.19 for ; Mon, 21 Feb 2022 01:51:14 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:from:to:cc:references:in-reply-to :content-transfer-encoding; bh=hB9b8zcCF73wPaCqrb6w4xSegPlVd9cxYTgtjhyicfw=; b=7WGlOiVWjOIcrng4GQlNIgAE4UITJAve6nkl9DFw9XA1jtfAtD+8Mr5+1MRrVkatKl b2EZDiXCjPrWEcSjFt62wfqRkgpYiBBiQZz9rcrt2mRETZ4IOlmAPq9oiJYM7hJ7P3kp JwCKvlPhX4KAnocWWSiFgzYstj06HlBLqN5GHRoOoLxmtQrO2to4DfS+HLqx5hN+h9q8 mx6VU/HtJcZAtsYqs2SSoathw5vhBhnOZjeIdMDFG0HtC33sA5Yb0pSFPwsUOdGF3j7n 3c/uP7PiEeeNqGFucduk6I8aXNcG2tZfVTjCnGELaaVLIKL53+keSMN5VpV2nHru9V6g 8XXQ== X-Gm-Message-State: AOAM531uyOal7VM4ias7WfDdIA6kIFJ0TsZJ+wpTCIt0qZgR8MR2jjYs bWXQAul1paxV80VlMHwxligQqym+7flDB7MlKnyv5wyG14DoavbVb2VFc8klm8xaEU7c0X3Un4l RMGxVJ6dsePrnlOrqf9L1 X-Received: by 2002:a05:6402:11cd:b0:410:d432:2e30 with SMTP id j13-20020a05640211cd00b00410d4322e30mr20320051edw.119.1645437073039; Mon, 21 Feb 2022 01:51:13 -0800 (PST) X-Google-Smtp-Source: ABdhPJye1adEJ8Kmx47qBL6X7KG+mjDUvnk5CjNgYTII8w0TE68ywE4uMzzzWMSbEL0amjS7NvTVDg== X-Received: by 2002:a05:6402:11cd:b0:410:d432:2e30 with SMTP id j13-20020a05640211cd00b00410d4322e30mr20320028edw.119.1645437072792; Mon, 21 Feb 2022 01:51:12 -0800 (PST) Received: from [10.40.98.142] ([78.108.130.194]) by smtp.gmail.com with ESMTPSA id c29sm4971862ejj.117.2022.02.21.01.51.12 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 21 Feb 2022 01:51:12 -0800 (PST) Message-ID: <26dfe4f3-a9b3-60dc-19a7-7707afea552b@redhat.com> Date: Mon, 21 Feb 2022 10:51:11 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.4.0 Subject: Re: [PATCH 1/6] ACPI: scan: Add acpi_dev_get_next_consumer_dev() Content-Language: en-US From: Hans de Goede To: Daniel Scally , linux-acpi@vger.kernel.org, linux-clk@vger.kernel.org, platform-driver-x86@vger.kernel.org Cc: rafael@kernel.org, lenb@kernel.org, mturquette@baylibre.com, sboyd@kernel.org, markgross@kernel.org, robert.moore@intel.com References: <20220216225304.53911-1-djrscally@gmail.com> <20220216225304.53911-2-djrscally@gmail.com> <74f89182-1699-f4a7-85e0-66976021913d@redhat.com> In-Reply-To: <74f89182-1699-f4a7-85e0-66976021913d@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-clk@vger.kernel.org Hi, On 2/21/22 10:50, Hans de Goede wrote: > Hi, > > On 2/16/22 23:52, Daniel Scally wrote: >> In commit b83e2b306736 ("ACPI: scan: Add function to fetch dependent of ACPI >> device") we added a means of fetching the first device to declare itself >> dependent on another ACPI device in the _DEP method. One assumption >> in that patch was that there would only be a single consuming device, >> but this has not held. >> >> Extend the functionality by adding a new function that fetches the >> next consumer of a supplier device. We can then simplify the original >> function by simply calling the new one. >> >> Signed-off-by: Daniel Scally >> --- >> drivers/acpi/scan.c | 47 ++++++++++++++++++++++++++++++++++------- >> include/acpi/acpi_bus.h | 2 ++ >> 2 files changed, 41 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c >> index 4463c2eda61e..b3ed664ee1cb 100644 >> --- a/drivers/acpi/scan.c >> +++ b/drivers/acpi/scan.c >> @@ -2256,9 +2256,21 @@ static void acpi_bus_attach(struct acpi_device *device, bool first_pass) >> device->handler->hotplug.notify_online(device); >> } >> >> -static int acpi_dev_get_first_consumer_dev_cb(struct acpi_dep_data *dep, void *data) >> +static int acpi_dev_get_next_consumer_dev_cb(struct acpi_dep_data *dep, void *data) >> { >> - struct acpi_device *adev; >> + struct acpi_device *adev = *(struct acpi_device **)data; >> + >> + /* >> + * If we're passed a 'previous' consumer device then we need to skip >> + * any consumers until we meet the previous one, and then NULL @data >> + * so the next one can be returned. >> + */ >> + if (adev) { >> + if (dep->consumer == adev->handle) >> + *(struct acpi_device **)data = NULL; >> + >> + return 0; >> + } >> >> adev = acpi_bus_get_acpi_device(dep->consumer); >> if (adev) { >> @@ -2389,23 +2401,42 @@ bool acpi_dev_ready_for_enumeration(const struct acpi_device *device) >> EXPORT_SYMBOL_GPL(acpi_dev_ready_for_enumeration); >> >> /** >> - * acpi_dev_get_first_consumer_dev - Return ACPI device dependent on @supplier >> + * acpi_dev_get_next_consumer_dev - Return the next adev dependent on @supplier >> * @supplier: Pointer to the dependee device >> + * @start: Pointer to the current dependent device >> * >> - * Returns the first &struct acpi_device which declares itself dependent on >> + * Returns the next &struct acpi_device which declares itself dependent on >> * @supplier via the _DEP buffer, parsed from the acpi_dep_list. >> * >> * The caller is responsible for putting the reference to adev when it is no >> * longer needed. > > This bit of the help text seems to not be entirely correct, since the reference to > start gets consumed by this, so the caller only needs to put the device when it > does NOT pass it as start to another acpi_dev_get_next_consumer_dev call. > > > >> */ >> -struct acpi_device *acpi_dev_get_first_consumer_dev(struct acpi_device *supplier) >> +struct acpi_device *acpi_dev_get_next_consumer_dev(struct acpi_device *supplier, >> + struct acpi_device *start) >> { >> - struct acpi_device *adev = NULL; >> + struct acpi_device *adev = start; >> >> acpi_walk_dep_device_list(supplier->handle, >> - acpi_dev_get_first_consumer_dev_cb, &adev); >> + acpi_dev_get_next_consumer_dev_cb, &adev); >> >> - return adev; >> + acpi_dev_put(start); >> + return adev == start ? NULL : adev; >> +} >> +EXPORT_SYMBOL_GPL(acpi_dev_get_next_consumer_dev); >> + >> +/** >> + * acpi_dev_get_first_consumer_dev - Return ACPI device dependent on @supplier >> + * @supplier: Pointer to the dependee device >> + * >> + * Returns the first &struct acpi_device which declares itself dependent on >> + * @supplier via the _DEP buffer, parsed from the acpi_dep_list. >> + * >> + * The caller is responsible for putting the reference to adev when it is no >> + * longer needed. >> + */ >> +struct acpi_device *acpi_dev_get_first_consumer_dev(struct acpi_device *supplier) >> +{ >> + return acpi_dev_get_next_consumer_dev(supplier, NULL); >> } >> EXPORT_SYMBOL_GPL(acpi_dev_get_first_consumer_dev); > > The only caller of this is skl_int3472_get_sensor_adev_and_name() IMHO it would > be better to just move that over to acpi_dev_get_next_consumer_dev(..., NULL); > in this same patch and just drop acpi_dev_get_first_consumer_dev() all together. > > I expect this entire series to get merged through the pdx86 tree, so from > that pov doing this should be fine.. I forgot to add: that otherwise this looks good to me, so with the above addressed you may add my: Reviewed-by: Hans de Goede Regards, Hans >> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h >> index ca88c4706f2b..8b06fef04722 100644 >> --- a/include/acpi/acpi_bus.h >> +++ b/include/acpi/acpi_bus.h >> @@ -720,6 +720,8 @@ bool acpi_dev_hid_uid_match(struct acpi_device *adev, const char *hid2, const ch >> >> void acpi_dev_clear_dependencies(struct acpi_device *supplier); >> bool acpi_dev_ready_for_enumeration(const struct acpi_device *device); >> +struct acpi_device *acpi_dev_get_next_consumer_dev(struct acpi_device *supplier, >> + struct acpi_device *start); >> struct acpi_device *acpi_dev_get_first_consumer_dev(struct acpi_device *supplier); >> struct acpi_device * >> acpi_dev_get_next_match_dev(struct acpi_device *adev, const char *hid, const char *uid, s64 hrv);