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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 00192C52D7C for ; Tue, 13 Aug 2024 13:39:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=IUssDIrqQJO90DKgwcqMYBIZvecxXxEsj4Nbx88FgFA=; b=IMiVCw2MtVvGeLqioGfS0AnmsT KSjc6kiLbC6pugODihr7tSTXYM3AlAbzR6sjEoufoc3G0iGtBksh//SSM35CxYHbUKagmohSWhqhj xtXwHD6NNKpFhCTo4MvEzbJd64s4T3v6THPhrkOcqDdptb399hL0cIAeCLn5wtIFmoAU4aEYjOISA Qjgx93nF7eq5ql/hAjFQ+bvsKVinqV+OlJ/dgkhxoL0P/Z94QNkzOXmY9uy0qBUCGSlR3JkJnDdh5 ku2jzWV6pOzIDvmv3fWEadxjD+jVMY+KspJNI0hxjumcL3k4MxKZrIjmMjftrzNXdKGFdd8kjoLjv oTnxeQBQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sdrkR-00000003uZq-24LA; Tue, 13 Aug 2024 13:39:39 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sdrjK-00000003uMn-0t2X for linux-arm-kernel@lists.infradead.org; Tue, 13 Aug 2024 13:38:31 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 77EDF12FC; Tue, 13 Aug 2024 06:38:55 -0700 (PDT) Received: from pluto (usa-sjc-mx-foss1.foss.arm.com [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 101FA3F6A8; Tue, 13 Aug 2024 06:38:27 -0700 (PDT) Date: Tue, 13 Aug 2024 14:38:25 +0100 From: Cristian Marussi To: Peng Fan Cc: Dhruva Gole , "Peng Fan (OSS)" , "saravanak@google.com" , "sudeep.holla@arm.com" , "cristian.marussi@arm.com" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "arm-scmi@vger.kernel.org" Subject: Re: [PATCH V2] firmware: arm_scmi: bus: bypass set fwnode for scmi cpufreq Message-ID: References: <20240729070325.2065286-1-peng.fan@oss.nxp.com> <20240813085703.zz6ltcxmrrbdgt77@lcpd911> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240813_063830_362753_29417307 X-CRM114-Status: GOOD ( 39.21 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Aug 13, 2024 at 10:25:31AM +0000, Peng Fan wrote: > > Subject: Re: [PATCH V2] firmware: arm_scmi: bus: bypass set fwnode > > for scmi cpufreq > > > > On Jul 29, 2024 at 15:03:25 +0800, Peng Fan (OSS) wrote: > > > From: Peng Fan > > > > > > Two drivers scmi_cpufreq.c and scmi_perf_domain.c both use > > > SCMI_PROTCOL_PERF protocol, but with different name, so two scmi > > > devices will be created. But the fwnode->dev could only point to one > > device. > > > > > > If scmi cpufreq device created earlier, the fwnode->dev will point to > > > the scmi cpufreq device. Then the fw_devlink will link performance > > > domain user device(consumer) to the scmi cpufreq device(supplier). > > > But actually the performance domain user device, such as GPU, > > should > > > use the scmi perf device as supplier. Also if 'cpufreq.off=1' in > > > bootargs, the GPU driver will defer probe always, because of the scmi > > > cpufreq > > > > The commit message itself seems very specific to some platform to me. > > What about platforms that don't atall have a GPU? Why would they > > care about this? > > It is a generic issue if a platform has performance domain to serve > scmi cpufreq and device performance level. > > > > > > device not ready. > > > > > > Because for cpufreq, no need use fw_devlink. So bypass setting > > fwnode > > > for scmi cpufreq device. > > > > > > Fixes: 96da4a99ce50 ("firmware: arm_scmi: Set fwnode for the > > > scmi_device") > > > Signed-off-by: Peng Fan > > > --- > > > > > > V2: > > > Use A!=B to replace !(A == B) > > > Add fixes tag > > > This might be a workaround, but since this is a fix, it is simple for > > > backporting. > > > > More than a workaround, it feels like a HACK to me. > > > > > > > > V1: > > > > > > > > > > > > drivers/firmware/arm_scmi/bus.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/firmware/arm_scmi/bus.c > > > b/drivers/firmware/arm_scmi/bus.c index > > 96b2e5f9a8ef..be91a82e0cda > > > 100644 > > > --- a/drivers/firmware/arm_scmi/bus.c > > > +++ b/drivers/firmware/arm_scmi/bus.c > > > @@ -395,7 +395,8 @@ __scmi_device_create(struct device_node > > *np, struct device *parent, > > > scmi_dev->id = id; > > > scmi_dev->protocol_id = protocol; > > > scmi_dev->dev.parent = parent; > > > - device_set_node(&scmi_dev->dev, of_fwnode_handle(np)); > > > + if ((protocol != SCMI_PROTOCOL_PERF) || strcmp(name, > > "cpufreq")) > > > + device_set_node(&scmi_dev->dev, > > of_fwnode_handle(np)); > > > > I kind of disagree with the idea here to be specific about the > > PROTOCOL_PERF or cpufreq. This is a generic arm scmi bus driver right? > > Why bring in specific code into a bus driver? We will never fix the > > actual root cause of the issue this way. > > The root cause is fwnode devlink only supports one fwnode, one device. > 1:1 match. But current arm scmi driver use one fwnode for two devices. > > If your platform has scmi cpufreq and scmi device performance domain, > you might see that some devices are consumer of scmi cpufreq, but actually > they should be consumer of scmi device performance domain. > > I not have a good idea that this is fw devlink design that only allows > 1 fwnode has 1 device or not. If yes, that arm scmi should be fixed. > If not, fw devlink should be updated. > > The current patch is the simplest method for stable tree fixes as I > could work out. So this is the same root cause at the end of the issues you had with IMX pinctrl coexistence...i.e. the SCMI stack creates scmi devices that embeds the protocol node, BUT since you can have multiple device/drivers doing different things on different resources within the same protocol you can end with 2 devices having the same embedded device_node, since we dont really have anything else to use as device_node, I rigth ? Thanks, Cristian