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 DC11EC433F5 for ; Mon, 11 Apr 2022 11:51:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S244464AbiDKLxo (ORCPT ); Mon, 11 Apr 2022 07:53:44 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43440 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232463AbiDKLxl (ORCPT ); Mon, 11 Apr 2022 07:53:41 -0400 Received: from mailgw02.mediatek.com (unknown [210.61.82.184]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 00D4E3150B; Mon, 11 Apr 2022 04:51:26 -0700 (PDT) X-UUID: d071f3d7ed7e49e1a12796f81c8bc2e4-20220411 X-UUID: d071f3d7ed7e49e1a12796f81c8bc2e4-20220411 Received: from mtkmbs10n1.mediatek.inc [(172.21.101.34)] by mailgw02.mediatek.com (envelope-from ) (Generic MTA with TLSv1.2 ECDHE-RSA-AES256-GCM-SHA384 256/256) with ESMTP id 1001954099; Mon, 11 Apr 2022 19:51:19 +0800 Received: from mtkcas10.mediatek.inc (172.21.101.39) by mtkmbs10n1.mediatek.inc (172.21.101.34) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384) id 15.2.792.15; Mon, 11 Apr 2022 19:51:18 +0800 Received: from mtksdccf07 (172.21.84.99) by mtkcas10.mediatek.inc (172.21.101.73) with Microsoft SMTP Server id 15.0.1497.2 via Frontend Transport; Mon, 11 Apr 2022 19:51:18 +0800 Message-ID: Subject: Re: [PATCH V2 13/15] cpufreq: mediatek: Link CCI device to CPU From: Rex-BC Chen To: Kevin Hilman , , , , CC: , , , , , , , , , Date: Mon, 11 Apr 2022 19:51:18 +0800 In-Reply-To: <7hfsmn5m9f.fsf@baylibre.com> References: <20220408045908.21671-1-rex-bc.chen@mediatek.com> <20220408045908.21671-14-rex-bc.chen@mediatek.com> <7hfsmn5m9f.fsf@baylibre.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.28.5-0ubuntu0.18.04.2 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-MTK: N Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 2022-04-08 at 13:54 -0700, Kevin Hilman wrote: > Rex-BC Chen writes: > > > From: Jia-Wei Chang > > > > In some MediaTek SoCs, like MT8183, CPU and CCI share the same > > power > > supplies. Cpufreq needs to check if CCI devfreq exists and wait > > until > > CCI devfreq ready before scaling frequency. > > > > - Add is_ccifreq_ready() to link CCI device to CPI, and CPU will > > start > > DVFS when CCI is ready. > > - Add platform data for MT8183. > > > > Signed-off-by: Jia-Wei Chang > > The checks here are not enough, and will lead to unexpected behavior. > IIUC, before doing DVFS, you're checking: > > 1) if the "cci" DT node is present and > 2) if the driver for that device is bound > > If both those conditions are not met, you don't actually fail, you > just > silently do nothing in ->set_target(). As Angelo pointed out also, > this > is not a good idea, and will be rather confusing to users. > > The same thing would happen if the cci DT node was present, but the > CCI > devfreq driver was disabled. Silent failure would also be quite > unexpected behavior. Similarily, if the cci DT node is not present > at > all (like it is in current upstream DT), this CPUfreq driver will > silently do nothing. Not good. > > So, this patch needs to handle several scenarios: > > 1) CCI DT node not present > > In this case, the driver should still operate normally. With no CCI > node, or driver there's no conflict. > > 2) CCI DT present/enabled but not yet bound > > In this case, you could return -EAGAIN as suggested by Angelo, or > maybe > better, it should do a deferred probe. > > 3) CCI DT present, but driver disabled > > This case is similar to (1), this driver should continue to work. > > Kevin Hello Kevin, Thanks for your review. I will review these three situations and do some modification. BRs, Rex