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 A6267C5AD49 for ; Mon, 26 May 2025 10:10: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:Content-Type:Cc:To:Subject: Message-ID:Date:From:In-Reply-To:References:MIME-Version: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=NpPv+jwx+sCOwe5Xg7tTvOuLFtJsEPCNaLZVy2I7eGY=; b=h5opHiHwWoi7IIY5Ffsi49ciK/ cjnhNUKeIj8PaHDXWCKIeSW+mctDVJyCWqpuFeSBW5RkwdTu8Y/RCJnxbKU6etXmk+pmAehVej3q3 xWlLJZLiP9FYbRu2o5dHjwrgaWP4nEJOCa8WEePKiXJn+sx716ZTumQfMhhBzswcYJfEpFs8xRNyF lSO/vSZ0pG3WWDnWISHGwmCGqMjWJJvZ1pnZNeVJFlskJwNGeQ03a/d4xYgl3GzEy6C1mK98fufSP dEdzZlOjes6wKrXAWdg6yh5664XUL8OG+liZO+f2oNaI9RBqP/3u1PfGwwMmglKRvU9cBFTUD8ypR iihvPtYA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1uJUn2-00000008b0c-2tB5; Mon, 26 May 2025 10:10:40 +0000 Received: from mail-yb1-xb36.google.com ([2607:f8b0:4864:20::b36]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1uJUkt-00000008asP-1LEf for linux-arm-kernel@lists.infradead.org; Mon, 26 May 2025 10:08:28 +0000 Received: by mail-yb1-xb36.google.com with SMTP id 3f1490d57ef6-e7d969146abso1233397276.1 for ; Mon, 26 May 2025 03:08:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1748254106; x=1748858906; darn=lists.infradead.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=NpPv+jwx+sCOwe5Xg7tTvOuLFtJsEPCNaLZVy2I7eGY=; b=yE1ny8C5wkpakKQhdMCd2VpyvAlF+WfghwLWT/wjgCJhC+t4S6Xjtk6mtLd4hml9rQ fplg8To8kO9rWqBkUGCIC5n6QTdLnoo4nojws+R5KJkLdc4zsq7rLk2RKoXwhs9Afpdt EIew1hERjR5purF80KFao8EGAmIY5iAUQqHt/gbVUbE1SQqjOu+x+NZz6cFCmeDMcP5+ syFHzxws8avARdMUCd0+fgsTxFv84FVnl3yREibvKlluqULosNQWAMUvtHUJt6TqoUvc saeoroYXif4HREvcAZKMGcZsN3hoItBmjStK55XFPwwkubYy/2EetEIpWuMf8E8uwCe1 cvcA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1748254106; x=1748858906; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=NpPv+jwx+sCOwe5Xg7tTvOuLFtJsEPCNaLZVy2I7eGY=; b=spcdRT9YNlL4LncQdOxO6y0Eut/It7DTU6PuNP6Mj2pJl6OWiFbH+rN/mk64Z0keWr YjdzFdIxI1dRx6MoDkO5Nb7eKU/pMcvnkhiEA0trfDLUR377sWI8cfyp6GvQQBpUSmVe pUkVqUtPJ0mvZptV65CmrTDDdPWpyHiqHVE3SwRrl0jUaZeVVGZwd2VS7sMEZt/HtkPe 6Tq6kuOQRtH3pwbXwh3Yu7V3TOScztjMiH2CEV4EaBkhoj8/0wcF1C/JJtycfcRUr0CH EUP+p2f9LMO29YBcJ98IS4COcE6sc6tdnB9Z3ilNf/UsBTOyoBDZkgPY5czXgi4vwpQl AqIg== X-Forwarded-Encrypted: i=1; AJvYcCVfAStrxJEFUGR9sdJn6cvNCWxN6VS07/TOtgHdK9Tw7JOvPIyBE6p99FlBWf7LVrXXu7M+sBXoENSEZ12oAhXu@lists.infradead.org X-Gm-Message-State: AOJu0YwCNYJPWAR3iufsYKiWhiZLvSrpQvJNc9Np+PdkfwGGL9V885ON CjuotlWkR4WlwQE/dbMa/V3UJ6/sKcviNgT/uyO8ok+ozW/xI2DzwwpaC2hh2SbRcZ8sOgcjnMt N/4mY2QvAgxE2+mXwkaPr0cwOjzA8+0yMsX45/Jox9A== X-Gm-Gg: ASbGncsML3B0eUtb9SqsC0a6f283/UHJ9F1CSYG617T6iCei6s8GVu2GIIagpwCIQxK YX0CMJh6SSXqw0Xe/WfWn7Eoy9R+PS/xpfuRfbZGL8PbKXSBk7sYL4rx52hIwu8VDdViCp/h6m+ 6auD+J3EM/9hJs+uNERDR8Le3pl78yoPYXOA== X-Google-Smtp-Source: AGHT+IFdBGTGYZOotDK/y3Q59f0r8dFms0TkmjC4C0sHaJmgkuM3Am4LonkHZ4KeqelTVat8o4uyIxhJmZyFRyDPL7U= X-Received: by 2002:a05:6902:160d:b0:e7d:b8ce:cb91 with SMTP id 3f1490d57ef6-e7db8cecd3dmr1219626276.5.1748254105723; Mon, 26 May 2025 03:08:25 -0700 (PDT) MIME-Version: 1.0 References: <20250508202826.33bke6atcvqdkfa4@hiago-nb> <20250509191308.6i3ydftzork3sv5c@hiago-nb> <20250519172357.vfnwehrbkk24vkge@hiago-nb> <20250521041306.GA28017@nxa18884-linux> <20250521041840.GB28017@nxa18884-linux> <20250523191713.nylhi74jq6z4hqmr@hiago-nb> In-Reply-To: <20250523191713.nylhi74jq6z4hqmr@hiago-nb> From: Ulf Hansson Date: Mon, 26 May 2025 12:07:49 +0200 X-Gm-Features: AX0GCFtV9JrsWnRtXpr1h2X2HDFYAFvpBaxszSIspVyj2HxlBm4_wnbvi_OW3II Message-ID: Subject: Re: [PATCH v2 3/3] remoteproc: imx_rproc: add power mode check for remote core attachment To: Hiago De Franco Cc: Peng Fan , Mathieu Poirier , linux-pm@vger.kernel.org, linux-remoteproc@vger.kernel.org, Shawn Guo , Sascha Hauer , Bjorn Andersson , Hiago De Franco , imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, daniel.baluta@nxp.com, iuliana.prodan@oss.nxp.com, Fabio Estevam , Pengutronix Kernel Team , Peng Fan Content-Type: text/plain; charset="UTF-8" X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250526_030827_362055_06C345A4 X-CRM114-Status: GOOD ( 44.41 ) 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 Fri, 23 May 2025 at 21:17, Hiago De Franco wrote: > > Hi Ulf, > > On Wed, May 21, 2025 at 02:11:02PM +0200, Ulf Hansson wrote: > > You should not provide any flag (or attach_data to > > dev_pm_domain_attach_list()) at all. In other words just call > > dev_pm_domain_attach_list(dev, NULL, &priv->pd_list), similar to how > > drivers/remoteproc/imx_dsp_rproc.c does it. > > > > In this way, the device_link is created by making the platform->dev > > the consumer and by keeping the supplier-devices (corresponding to the > > genpds) in RPM_SUSPENDED state. > > > > The PM domains (genpds) are then left in their current state, which > > should allow us to call dev_pm_genpd_is_on() for the corresponding > > supplier-devices, to figure out whether the bootloader turned them on > > or not, I think. > > > > Moreover, to make sure the genpds are turned on when needed, we also > > need to call pm_runtime_enable(platform->dev) and > > pm_runtime_get_sync(platform->dev). The easiest approach is probably > > to do that during ->probe() - and then as an improvement on top you > > may want to implement more fine-grained support for runtime PM. > > > > [...] > > > > Kind regards > > Uffe > > I did some tests here and I might be missing something. I used the > dev_pm_genpd_is_on() inside imx_rproc.c with the following changes: > > @@ -902,7 +902,12 @@ static int imx_rproc_attach_pd(struct imx_rproc *priv) > if (dev->pm_domain) > return 0; > > ret = dev_pm_domain_attach_list(dev, &pd_data, &priv->pd_list); > + printk("hfranco: returned pd devs is %d", ret); > + for (int i = 0; i < ret; i++) { > + test = dev_pm_genpd_is_on(priv->pd_list->pd_devs[i]); > + printk("hfranco: returned value is %d", test); > + } > return ret < 0 ? ret : 0; > } > > This was a quick test to check the returned value, and it always return > 1 for both pds, even if I did not boot the remote core. > > So I was wondering if it was because of PD_FLAG_DEV_LINK_ON, I removed > it and passed NULL to dev_pm_domain_attach_list(). Right, that's exactly what we should be doing. > Booting the kernel > now it correctly reports 0 for both pds, however when I start the > remote core with a hello world firmware and boot the kernel, the CPU > resets with a fault reset ("Reset cause: SCFW fault reset"). > > I added both pm functions to probe, just to test: > > @@ -1152,6 +1158,9 @@ static int imx_rproc_probe(struct platform_device *pdev) > goto err_put_clk; > } > > + pm_runtime_enable(dev); > + pm_runtime_get_sync(dev); > + Indeed, calling pm_runtime_enable() and then pm_runtime_get_sync() should turn on the PM domains for the device, which I assume is needed at some point. Although, I wonder if this may be a bit too late, I would expect that you at least need to call these *before* the call to rproc_add(), as I assume the rproc-core may start using the device/driver beyond that point. > return 0 > > Now the kernel boot with the remote core running, but it still returns > 0 from dev_pm_genpd_is_on(). So basically now it always returns 0, with > or without the remote core running. dev_pm_genpd_is_on() is returning the current status of the PM domain (genpd) for the device. Could it be that the genpd provider doesn't register its PM domains with the state that the HW is really in? pm_genpd_init() is the call that allows the genpd provider to specify the initial state. I think we need Peng's help here to understand what goes on. > > I tried to move pm_runtime_get_sync() to .prepare function but it make > the kernel not boot anymore (with the SCU fault reset). Try move pm_runtime_enable() before rproc_add(). > > Do you have any suggestions? Am I doing something wrong with these PDs? > > Best regards, > Hiago. Kind regards Uffe