Message ID | 20230913-gpll_cleanup-v2-1-c8ceb1a37680@quicinc.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:612c:172:b0:3f2:4152:657d with SMTP id h50csp189431vqi; Thu, 14 Sep 2023 01:12:33 -0700 (PDT) X-Google-Smtp-Source: AGHT+IE1d1Bew0yP2xAZYwN21EJl+1hX/KKyMFWIaH/fcNiDKiHPG+Z5KhBWsG3dhySsMwWoJHCa X-Received: by 2002:a05:6358:5920:b0:142:d032:1bc6 with SMTP id g32-20020a056358592000b00142d0321bc6mr5537227rwf.0.1694679152749; Thu, 14 Sep 2023 01:12:32 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1694679152; cv=none; d=google.com; s=arc-20160816; b=f58MWdCVRnDp6ptHWZBCOJ5BnZ15oHM+A9y7Mha7NJWQYme40jasLUZLLfzBufX7Qg ZBhF+MkLGsX6E/ytuja2Xr/txYZAfPAwh+Ji/cDyME8X2NRejqOpyqnl54qkytqqphl+ 17Dwst0GXMsamJQ5XuhNqf+WrfGmzYPintECYok/UEFGSXXYjZoPAZHLBcS0MwuKnb1y heGA5/lcbgugxc0KBKoycwSknLd2p0Pb7hYtAVPGDoOWhBKgcUs0BzwezsfCNP+RZ4vS 22rBCPAl/Sm9yQXbvSxnLK5coF+lZp9N4nsY0JQP/pHtogag8bj04Cdck9XxeF1w0t8A VWDQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:in-reply-to:references:message-id :content-transfer-encoding:mime-version:subject:date:from :dkim-signature; bh=CFJMCiR1Bo0Zs0u0v0vhwfmpU2tjmHZnbZry9lOc/ak=; fh=El3CbEE15ZsesgBWsikxymCJesZUZu1vPQj2Zepn3Uw=; b=n9+GbA7pr5K4XgmTr0b8ZdpUQOEdx357UuaPmk4EBtEbOiN0fPQU2JAkmg5N1AOd0A fhy72G74CCdSO8i23Ow5xgo+dGu9rqeS04wJ1yEDNn40gkYUoPMDJFg9KQ8lZc/QwMNE P2knsn+VEMKjLPtyVxs+GS1DdalM1GnXf6dlQNJwlJY9YsFxdq5fqlmGD9P5IbtpY+xS CkgtbTu9e+H9IqXjTWeR0EqBsbAbjvaUH/X+8WAJKlo5fE08ABX8kI3VxMisSzm5gCXm ZwgPgSYvzmVnMOxNVKUR4pQFy5q6GRe0inW/KwmQwpmjFC3VOWjdNNzZU9nNmUdpMdFX hSXQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=now9rawA; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=quicinc.com Received: from morse.vger.email (morse.vger.email. [23.128.96.31]) by mx.google.com with ESMTPS id y21-20020a634b15000000b00565f20a03f3si986009pga.774.2023.09.14.01.12.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 Sep 2023 01:12:32 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31; Authentication-Results: mx.google.com; dkim=pass header.i=@quicinc.com header.s=qcppdkim1 header.b=now9rawA; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=quicinc.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by morse.vger.email (Postfix) with ESMTP id 5A5A9826EB4B; Thu, 14 Sep 2023 00:00:51 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at morse.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232593AbjINHAf (ORCPT <rfc822;chrisfriedt@gmail.com> + 35 others); Thu, 14 Sep 2023 03:00:35 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42944 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229460AbjINHAe (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 14 Sep 2023 03:00:34 -0400 Received: from mx0b-0031df01.pphosted.com (mx0b-0031df01.pphosted.com [205.220.180.131]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 48678CD8; Thu, 14 Sep 2023 00:00:30 -0700 (PDT) Received: from pps.filterd (m0279870.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 38E6SS2M006211; Thu, 14 Sep 2023 07:00:24 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; h=from : date : subject : mime-version : content-type : content-transfer-encoding : message-id : references : in-reply-to : to : cc; s=qcppdkim1; bh=CFJMCiR1Bo0Zs0u0v0vhwfmpU2tjmHZnbZry9lOc/ak=; b=now9rawAAN+sLZgAW9v8PvRkCgPje0hdNiBlWHwkaC1PVwHm1gJmaXrNpF+ZwF/FX9HP mWVCCfy4ppIYXWjw6Bqk3O+1rvv/lLPZUZ0FyBIXhPBGQ/GZul/LqSqd0HXATM4gUt4r BMWfc9mizPUfGla62bonGC8YyG6IFwE5Wyv9CRNtEppe8cGcwNgLDjDBTc6L5fFR9tsT zs6ayPPMRNPTPvK83fHI37pQ2jjzpb03aXTzwadofThrbFJ3zqKAJMcOmlwQNroFsPs/ G2SNAR/q9opxRharGWffIQFbrMW5q5D6hP9rIoseFC2/5R4nnknQwLHBsflpUvk+k4AB SQ== Received: from nalasppmta05.qualcomm.com (Global_NAT1.qualcomm.com [129.46.96.20]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 3t3r15rnb4-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 14 Sep 2023 07:00:24 +0000 Received: from nalasex01a.na.qualcomm.com (nalasex01a.na.qualcomm.com [10.47.209.196]) by NALASPPMTA05.qualcomm.com (8.17.1.5/8.17.1.5) with ESMTPS id 38E70MNE016955 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 14 Sep 2023 07:00:22 GMT Received: from hu-kathirav-blr.qualcomm.com (10.80.80.8) by nalasex01a.na.qualcomm.com (10.47.209.196) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1118.36; Thu, 14 Sep 2023 00:00:17 -0700 From: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com> Date: Thu, 14 Sep 2023 12:29:51 +0530 Subject: [PATCH v2 01/11] clk: qcom: ipq8074: drop the CLK_SET_RATE_PARENT flag from PLL clocks MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit Message-ID: <20230913-gpll_cleanup-v2-1-c8ceb1a37680@quicinc.com> References: <20230913-gpll_cleanup-v2-0-c8ceb1a37680@quicinc.com> In-Reply-To: <20230913-gpll_cleanup-v2-0-c8ceb1a37680@quicinc.com> To: Andy Gross <agross@kernel.org>, Bjorn Andersson <andersson@kernel.org>, Konrad Dybcio <konrad.dybcio@linaro.org>, Michael Turquette <mturquette@baylibre.com>, Stephen Boyd <sboyd@kernel.org>, "Sricharan Ramabadhran" <quic_srichara@quicinc.com>, Gokul Sriram Palanisamy <quic_gokulsri@quicinc.com>, Varadarajan Narayanan <quic_varada@quicinc.com>, Anusha Rao <quic_anusha@quicinc.com>, Devi Priya <quic_devipriy@quicinc.com>, Jassi Brar <jassisinghbrar@gmail.com>, Rob Herring <robh+dt@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley <conor+dt@kernel.org> CC: <linux-arm-msm@vger.kernel.org>, <linux-clk@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <devicetree@vger.kernel.org>, "Kathiravan Thirumoorthy" <quic_kathirav@quicinc.com>, <stable@vger.kernel.org> X-Mailer: b4 0.12.2 X-Developer-Signature: v=1; a=ed25519-sha256; t=1694674810; l=2123; i=quic_kathirav@quicinc.com; s=20230906; h=from:subject:message-id; bh=et3sbNNIo4FjZBdgJ3tqzZdx4KTgZ9SvcoeqRTD+t+M=; b=a4zoKlRUweg4vBAcCp/aBCXpZJFNyRz1BS3djGaLExd3N4Np87yZLEWPHI5WqLNZyciyajzV5 rkDUDkLQd1tAl3UiTGxDm+AntDiN9wpTOba7X+OoBnqHL047CzUXFle X-Developer-Key: i=quic_kathirav@quicinc.com; a=ed25519; pk=xWsR7pL6ch+vdZ9MoFGEaP61JUaRf0XaZYWztbQsIiM= X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01b.na.qualcomm.com (10.46.141.250) To nalasex01a.na.qualcomm.com (10.47.209.196) X-QCInternal: smtphost X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-GUID: 1bF7F7q9bYES3-dm8AHcVlagZ83aWq3A X-Proofpoint-ORIG-GUID: 1bF7F7q9bYES3-dm8AHcVlagZ83aWq3A X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.267,Aquarius:18.0.980,Hydra:6.0.601,FMLib:17.11.176.26 definitions=2023-09-14_03,2023-09-13_01,2023-05-22_02 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 clxscore=1015 impostorscore=0 malwarescore=0 adultscore=0 spamscore=0 lowpriorityscore=0 suspectscore=0 phishscore=0 priorityscore=1501 mlxlogscore=920 bulkscore=0 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2308100000 definitions=main-2309140061 Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (morse.vger.email [0.0.0.0]); Thu, 14 Sep 2023 00:00:51 -0700 (PDT) X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.vger.email X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1776999887289628539 X-GMAIL-MSGID: 1776999887289628539 |
Series |
Add GPLL0 as clock provider for the Qualcomm's IPQ mailbox controller
|
|
Commit Message
Kathiravan Thirumoorthy
Sept. 14, 2023, 6:59 a.m. UTC
GPLL, NSS crypto PLL clock rates are fixed and shouldn't be scaled based
on the request from dependent clocks. Doing so will result in the
unexpected behaviour. So drop the CLK_SET_RATE_PARENT flag from the PLL
clocks.
Cc: stable@vger.kernel.org
Fixes: b8e7e519625f ("clk: qcom: ipq8074: add remaining PLL’s")
Signed-off-by: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com>
---
Changes in V2:
- Include the stable mailing list
- Keep the CLK_SET_RATE_PARENT in UBI32 PLL, looks like these
PLL rates can be changed. So don't drop the flag.
---
drivers/clk/qcom/gcc-ipq8074.c | 6 ------
1 file changed, 6 deletions(-)
Comments
On 14.09.2023 08:59, Kathiravan Thirumoorthy wrote: > GPLL, NSS crypto PLL clock rates are fixed and shouldn't be scaled based > on the request from dependent clocks. Doing so will result in the > unexpected behaviour. So drop the CLK_SET_RATE_PARENT flag from the PLL > clocks. > > Cc: stable@vger.kernel.org > Fixes: b8e7e519625f ("clk: qcom: ipq8074: add remaining PLL’s") > Signed-off-by: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com> > --- Stephen, do you think there should be some sort of error or at least warning thrown when SET_RATE_PARENT is used with RO ops? Konrad
Quoting Konrad Dybcio (2023-09-15 05:19:56) > On 14.09.2023 08:59, Kathiravan Thirumoorthy wrote: > > GPLL, NSS crypto PLL clock rates are fixed and shouldn't be scaled based > > on the request from dependent clocks. Doing so will result in the > > unexpected behaviour. So drop the CLK_SET_RATE_PARENT flag from the PLL > > clocks. > > > > Cc: stable@vger.kernel.org > > Fixes: b8e7e519625f ("clk: qcom: ipq8074: add remaining PLL’s") > > Signed-off-by: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com> > > --- > Stephen, do you think there should be some sort of error > or at least warning thrown when SET_RATE_PARENT is used with > RO ops? > Sure? How would that be implemented?
On 10/19/23 02:16, Stephen Boyd wrote: > Quoting Konrad Dybcio (2023-09-15 05:19:56) >> On 14.09.2023 08:59, Kathiravan Thirumoorthy wrote: >>> GPLL, NSS crypto PLL clock rates are fixed and shouldn't be scaled based >>> on the request from dependent clocks. Doing so will result in the >>> unexpected behaviour. So drop the CLK_SET_RATE_PARENT flag from the PLL >>> clocks. >>> >>> Cc: stable@vger.kernel.org >>> Fixes: b8e7e519625f ("clk: qcom: ipq8074: add remaining PLL’s") >>> Signed-off-by: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com> >>> --- >> Stephen, do you think there should be some sort of error >> or at least warning thrown when SET_RATE_PARENT is used with >> RO ops? >> > > Sure? How would that be implemented? drivers/clk/clk.c : static void clk_change_rate() if (!skip_set_rate && core->ops->set_rate) core->ops->set_rate(core->hw, core->new_rate, best_parent_rate); -> if (!skip_set_rate) { if (core->ops->set_rate) core->ops->set_rate(core->hw, core->new_rate, best_parent_rate); else pr_err("bad idea"); } Konrad
Quoting Konrad Dybcio (2023-10-19 04:22:33) > > > On 10/19/23 02:16, Stephen Boyd wrote: > > Quoting Konrad Dybcio (2023-09-15 05:19:56) > >> On 14.09.2023 08:59, Kathiravan Thirumoorthy wrote: > >>> GPLL, NSS crypto PLL clock rates are fixed and shouldn't be scaled based > >>> on the request from dependent clocks. Doing so will result in the > >>> unexpected behaviour. So drop the CLK_SET_RATE_PARENT flag from the PLL > >>> clocks. > >>> > >>> Cc: stable@vger.kernel.org > >>> Fixes: b8e7e519625f ("clk: qcom: ipq8074: add remaining PLL’s") > >>> Signed-off-by: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com> > >>> --- > >> Stephen, do you think there should be some sort of error > >> or at least warning thrown when SET_RATE_PARENT is used with > >> RO ops? > >> > > > > Sure? How would that be implemented? > drivers/clk/clk.c : static void clk_change_rate() > > if (!skip_set_rate && core->ops->set_rate) > core->ops->set_rate(core->hw, core->new_rate, best_parent_rate); > > -> > > if (!skip_set_rate) { > if (core->ops->set_rate) > core->ops->set_rate(core->hw, core->new_rate, > best_parent_rate); > else > pr_err("bad idea"); > } > CLK_SET_RATE_PARENT means that "calling clk_set_rate() on this clk will propagate up to the parent". Changing the rate of the parent could change the rate of this clk to be the same frequency as the parent if this clk doesn't have a set_rate clk op, or it could be that this clk has a fixed divider so during the determine_rate() callback it calculated what rate the parent should be to achieve the requested rate in clk_set_rate(). It really matters what determine_rate() returns for a clk and if after changing rates that rate is actually achieved. I suppose if the determine_rate() callback returns some rate, and then we recalc rates and notice that the rate determined earlier doesn't match we're concerned. So far in the last decade we've never cared about this though and I'm hesitant to start adding that check. I believe some qcom clk drivers take a shortcut and round the rate in frequency tables so whatever is returned in determine_rate() doesn't match what recalc_rate() calculates. It would be interesting to get rid of the CLK_SET_RATE_PARENT check in clk_calc_new_rates() and simply always call clk_calc_new_rates() on the parent if the parent->rate doesn't match what determine_rate thought it should be. The framework currently calls the rounding clk op for a clk and gets back the parent rate that the clk requires to achieve that rate and then it blindly trusts that the parent rate is going to be achieved. If the CLK_SET_RATE_PARENT flag is set it calls clk_calc_new_rates() recursively on the parent, but then it doesn't check that the parent rate is what was requested. That's mostly there to figure out if the parent also needs to change rate, i.e. calculating the 'top' clk in a rate change. Note that this also calls determine_rate again on the parent, once from the child clk's determine_rate clk op and once from the framework. I wouldn't be surprised if some driver is relying on this behavior where the rate isn't checked after being set. Maybe when we extend struct clk_rate_request to have a linked list that allows a clk to build up a chain of rate requests we can also enforce more things like matching rates on recalc. Then any drivers that are relying on this behavior will have to opt in to a different method of changing rates and notice that things aren't working.
diff --git a/drivers/clk/qcom/gcc-ipq8074.c b/drivers/clk/qcom/gcc-ipq8074.c index 63ac2ced76bb..b7faf12a511a 100644 --- a/drivers/clk/qcom/gcc-ipq8074.c +++ b/drivers/clk/qcom/gcc-ipq8074.c @@ -75,7 +75,6 @@ static struct clk_fixed_factor gpll0_out_main_div2 = { &gpll0_main.clkr.hw }, .num_parents = 1, .ops = &clk_fixed_factor_ops, - .flags = CLK_SET_RATE_PARENT, }, }; @@ -121,7 +120,6 @@ static struct clk_alpha_pll_postdiv gpll2 = { &gpll2_main.clkr.hw }, .num_parents = 1, .ops = &clk_alpha_pll_postdiv_ro_ops, - .flags = CLK_SET_RATE_PARENT, }, }; @@ -154,7 +152,6 @@ static struct clk_alpha_pll_postdiv gpll4 = { &gpll4_main.clkr.hw }, .num_parents = 1, .ops = &clk_alpha_pll_postdiv_ro_ops, - .flags = CLK_SET_RATE_PARENT, }, }; @@ -188,7 +185,6 @@ static struct clk_alpha_pll_postdiv gpll6 = { &gpll6_main.clkr.hw }, .num_parents = 1, .ops = &clk_alpha_pll_postdiv_ro_ops, - .flags = CLK_SET_RATE_PARENT, }, }; @@ -201,7 +197,6 @@ static struct clk_fixed_factor gpll6_out_main_div2 = { &gpll6_main.clkr.hw }, .num_parents = 1, .ops = &clk_fixed_factor_ops, - .flags = CLK_SET_RATE_PARENT, }, }; @@ -266,7 +261,6 @@ static struct clk_alpha_pll_postdiv nss_crypto_pll = { &nss_crypto_pll_main.clkr.hw }, .num_parents = 1, .ops = &clk_alpha_pll_postdiv_ro_ops, - .flags = CLK_SET_RATE_PARENT, }, };