From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qv1-f49.google.com (mail-qv1-f49.google.com [209.85.219.49]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8EEC17B for ; Fri, 29 Apr 2022 18:52:44 +0000 (UTC) Received: by mail-qv1-f49.google.com with SMTP id r8so5961379qvx.10 for ; Fri, 29 Apr 2022 11:52:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=message-id:date:mime-version:user-agent:subject:content-language:to :cc:references:from:in-reply-to:content-transfer-encoding; bh=kMONR8uP8o1V9wDmONNEpi44JYnbSaTBErCeX+Ef8mM=; b=MUwX5h8GHYTDN93S5LQ8HgipQCFvTgIJKkyQsoDuwUIJtqQlnvDOsWHlV05eaC8Ri/ aj2P8a4Y7arfynDH3NX+WbPF34XjQ5ExG7rNPjVnk7NT0K1wQZG/W0mGYL1zbl88pmsU ojCPyRS//omI5vTAAO4qjW/XkOqvRD3PZzryTMb9Z7TL7+TxD2tFXUuhy+MlH5JeLgPp NILsPP4yFMrqjNKkp6zOegLylz9rgNharn+C6dUtPOMv/q2zNzyoODPWGprdgL1EbxMi zfz6WkTYuvk+Cpj1IWITDaZyqfbYl9YNoDgv6evqUEbZh2fhePEm38ELdY/mfd4WV6Uk Abqw== 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:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=kMONR8uP8o1V9wDmONNEpi44JYnbSaTBErCeX+Ef8mM=; b=MiLW6GQGX+P1iRNYqtHv6UNHsjnsGOBZzwDrEXH9AIUN0pYUVNA82sSCCluFNSFncB 63THBdhQy6ceUqxg30kUBwmEBJ1TM3JrH1CRV6+VmU7AhfLgmAVubBlmeHUKoLwqJN1s D86AUXf427ThdXp05/AANLDHcVTrohIUOH4HAc8qZiA7lexeW29xvSIzZV/YMa4aq7xA rJKt2+IjH1Rdqe52qsqwGqUvdfiNttwRrgITC1qsvtQ9WdFEjbt1r37G7r1tbvfXMXFE OykQ6bk6VMR8kMw2yTrBf82H1S4pUYZNn/OA849RxSCCm6Vddd+gfZ3MeU7O7FNbv9Wa DzWw== X-Gm-Message-State: AOAM532K7jn2OnGX/OZNgxOdYoptJe5becMkvBl0HAqw+lmeAYW6mYCY eA+4VaxJ8Lgh0de6MS5s2TjqiQ== X-Google-Smtp-Source: ABdhPJzb5oXknS64kuiz02rv4HWvkZbDRu3ubeC0w7hDduzsE6tvZcGKS9f3Lgfwig0zajCWBQ93Gg== X-Received: by 2002:a05:6214:e4a:b0:446:6567:6554 with SMTP id o10-20020a0562140e4a00b0044665676554mr305731qvc.25.1651258363472; Fri, 29 Apr 2022 11:52:43 -0700 (PDT) Received: from [172.22.22.4] (c-73-185-129-58.hsd1.mn.comcast.net. [73.185.129.58]) by smtp.googlemail.com with ESMTPSA id az14-20020a05620a170e00b0069fb6140d2fsm1374694qkb.45.2022.04.29.11.52.42 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 29 Apr 2022 11:52:43 -0700 (PDT) Message-ID: Date: Fri, 29 Apr 2022 13:52:41 -0500 Precedence: bulk X-Mailing-List: patches@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 Subject: Re: [PATCH] interconnect: Restore sync state by ignoring ipa-virt in provider count Content-Language: en-US To: Georgi Djakov , Stephen Boyd Cc: linux-kernel@vger.kernel.org, patches@lists.linux.dev, linux-pm@vger.kernel.org, linux-arm-msm@vger.kernel.org, Bjorn Andersson , Doug Anderson , Taniya Das , Mike Tipton References: <20220427013226.341209-1-swboyd@chromium.org> <38149635-26ee-ab02-7c69-c5dd5f64fab5@kernel.org> From: Alex Elder In-Reply-To: <38149635-26ee-ab02-7c69-c5dd5f64fab5@kernel.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 4/27/22 3:52 PM, Georgi Djakov wrote: > On 27.04.22 15:00, Alex Elder wrote: >> On 4/26/22 8:32 PM, Stephen Boyd wrote: >>> Ignore compatible strings for the IPA virt drivers that were removed in >>> commits 2fb251c26560 ("interconnect: qcom: sdx55: Drop IP0 >>> interconnects") and 2f3724930eb4 ("interconnect: qcom: sc7180: Drop IP0 >>> interconnects") so that the sync state logic can kick in again. >>> Otherwise all the interconnects in the system will stay pegged at max >>> speeds because 'providers_count' is always going to be one larger than >>> the number of drivers that will ever probe on sc7180 or sdx55. This >>> fixes suspend on sc7180 and sdx55 devices when you don't have a >>> devicetree patch to remove the ipa-virt compatible node. >>> >>> Cc: Bjorn Andersson >>> Cc: Doug Anderson >>> Cc: Alex Elder >>> Cc: Taniya Das >>> Cc: Mike Tipton >>> Fixes: 2fb251c26560 ("interconnect: qcom: sdx55: Drop IP0 >>> interconnects") >>> Fixes: 2f3724930eb4 ("interconnect: qcom: sc7180: Drop IP0 >>> interconnects") >>> Signed-off-by: Stephen Boyd >> >> So of_count_icc_providers() counts the number of >> interconnect providers defined in the DTB, regardless >> of whether anything in the code supports it. > > Yes, that's the case currently. There could be multiple provider drivers > in different modules, and the modules may be loaded even not during boot, > but later. So we rely on DT. Georgi, do you have any other ideas about how to improve this? What you say makes sense--you figure out the total number of providers at init time based on what's defined in DT. But when all is said and done, what if even *one* of those has no provider driver? Isn't that what causes the problem here, that icc_sync_state assumes all providers specified in DT will eventually have a driver that calls icc_sync_state()? (I don't claim to know the interconnect code in detail, so I might not be quite understanding how this works.) -Alex > Thanks, > Georgi > >> This seems to be a more general problem, but I >> suppose in practice it's not likely to occur. >> >> I think your solution looks fine, but I'm interested >> in what Georgi has to say. >> >> Reviewed-by: Alex Elder >> >> >>> --- >>>   drivers/interconnect/core.c | 8 +++++++- >>>   1 file changed, 7 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c >>> index 9050ca1f4285..c52915a58b22 100644 >>> --- a/drivers/interconnect/core.c >>> +++ b/drivers/interconnect/core.c >>> @@ -1087,9 +1087,15 @@ static int of_count_icc_providers(struct >>> device_node *np) >>>   { >>>       struct device_node *child; >>>       int count = 0; >>> +    const struct of_device_id ignore_list[] = { >>> +        { .compatible = "qcom,sc7180-ipa-virt" }, >>> +        { .compatible = "qcom,sdx55-ipa-virt" }, >>> +        {} >>> +    }; >>>       for_each_available_child_of_node(np, child) { >>> -        if (of_property_read_bool(child, "#interconnect-cells")) >>> +        if (of_property_read_bool(child, "#interconnect-cells") && >>> +            likely(!of_match_node(ignore_list, child))) >>>               count++; >>>           count += of_count_icc_providers(child); >>>       } >>> >>> base-commit: 2fb251c265608636fc961b7d38e1a03937e57371 >> >