From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 8A2DD2D0C94; Fri, 20 Mar 2026 12:38:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774010345; cv=none; b=bJlUsxgsVCfzSwxg1bHTUtMXsk6tpCKxpLEBrwg+QxMv+d7LOjXtXGdOEOSl/iVT0hPbiQ+n+0jHYIutMVEcmwydk7dMXLG8LV6IdpRuu9CEK+A7ks8MUScnXe2XBaNDqKaagH0FzJf/pl3PJDUU2xfS6QWQFwXmqWBWfTh04Qs= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774010345; c=relaxed/simple; bh=XY65cPfXjEEvgtHrIj0H3/2b97DWd8ZhGD7/a6SJCtc=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=o6obKq6U/XQXkeUzrZ+x6QLhtwO96hr4AWzRdI75pPWxrRw47INYh/7SQjkLvcFvoUT/+6PH1oxp7PEEkUpcYS2gAJNxaTLmry7WoTQiwS3c7J/BJffhImC69dk7WOscLc/8n4nxqidNjdgcmA57MHI1GhpV0+39l26SmoZ6MzE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com 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 82DEA165C; Fri, 20 Mar 2026 05:38:51 -0700 (PDT) Received: from [10.1.29.20] (e122027.cambridge.arm.com [10.1.29.20]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 6AD393F7BD; Fri, 20 Mar 2026 05:38:54 -0700 (PDT) Message-ID: Date: Fri, 20 Mar 2026 12:38:52 +0000 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 3/4] drm/panfrost: Add bus_ace optional clock support for RZ/G2L To: Biju Das , "biju.das.au" , Boris Brezillon , Rob Herring , =?UTF-8?Q?Adri=C3=A1n_Larumbe?= , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter Cc: "dri-devel@lists.freedesktop.org" , "linux-kernel@vger.kernel.org" , Geert Uytterhoeven , Prabhakar Mahadev Lad , "linux-renesas-soc@vger.kernel.org" References: <20260304134845.267030-1-biju.das.jz@bp.renesas.com> <20260304134845.267030-4-biju.das.jz@bp.renesas.com> <5bb58801-2851-4c7b-a8f0-d4b3cc2db474@arm.com> From: Steven Price Content-Language: en-GB In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On 20/03/2026 12:30, Biju Das wrote: > Hi Steven Price, > > Thanks for the feedback. > >> -----Original Message----- >> From: Steven Price >> Sent: 20 March 2026 12:16 >> Subject: Re: [PATCH 3/4] drm/panfrost: Add bus_ace optional clock support for RZ/G2L >> >> On 04/03/2026 13:48, Biju wrote: >>> From: Biju Das >>> >>> On RZ/G2L SoCs, the GPU MMU requires a bus_ace clock to operate correctly. >>> Without it, unbind/bind cycles leave the GPU non-operational, >>> manifesting as an AS_ACTIVE bit stuck and a soft reset timeout falling >>> back to hard reset. Add bus_ace_clock as an optional clock, wiring it >>> into init/fini, and the runtime suspend/resume paths alongside the >>> existing optional bus_clock. >>> >>> Signed-off-by: Biju Das >>> --- >>> drivers/gpu/drm/panfrost/panfrost_device.c | 24 >>> ++++++++++++++++++++++ drivers/gpu/drm/panfrost/panfrost_device.h | >>> 1 + >>> 2 files changed, 25 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c >>> b/drivers/gpu/drm/panfrost/panfrost_device.c >>> index 01e702a0b2f0..87dae0ed748a 100644 >>> --- a/drivers/gpu/drm/panfrost/panfrost_device.c >>> +++ b/drivers/gpu/drm/panfrost/panfrost_device.c >>> @@ -70,8 +70,23 @@ static int panfrost_clk_init(struct panfrost_device *pfdev) >>> goto disable_clock; >>> } >>> >>> + pfdev->bus_ace_clock = devm_clk_get_optional(pfdev->base.dev, "bus_ace"); >>> + if (IS_ERR(pfdev->bus_ace_clock)) { >>> + err = PTR_ERR(pfdev->bus_ace_clock); >>> + dev_err(pfdev->base.dev, "get bus_ace_clock failed %ld\n", >>> + PTR_ERR(pfdev->bus_ace_clock)); >>> + err = PTR_ERR(pfdev->bus_ace_clock); >> >> You've assigned err twice (with the same value), and you can simplify the dev_err() line by using err > > Oops, forgot to take out the bottom assignment. > >> rather than the same PTR_ERR() expression again. > > I get a warning, if I use "err" in dev_err() > > panfrost_device.c:76:42: warning: format ‘%ld’ expects argument of type ‘long int’, but argument 3 has type ‘int’ [-Wformat=] > 76 | dev_err(pfdev->base.dev, "get bus_ace_clock failed %ld\n", You can simply change the format string to "%d". Explanation: PTR_ERR returns a long (which matches the kernel's idea that a long is the same size as a pointer). But the standard return code size is int. So technically the assignment to err is truncating the type. However, the IS_ERR() check uses MAX_ERRNO which is 4095 so all error values will fit in an int. So we know the assignment into 'int' isn't going to truncate. [ Also it's just an error message... ;) ] Thanks, Steve > > Cheers, > Biju > >> >> With that fixed: >> >> Reviewed-by: Steven Price >> >> Thanks, >> Steve >> >>> + goto disable_bus_clock; >>> + } >>> + >>> + err = clk_prepare_enable(pfdev->bus_ace_clock); >>> + if (err) >>> + goto disable_bus_clock; >>> + >>> return 0; >>> >>> +disable_bus_clock: >>> + clk_disable_unprepare(pfdev->bus_clock); >>> disable_clock: >>> clk_disable_unprepare(pfdev->clock); >>> >>> @@ -80,6 +95,7 @@ static int panfrost_clk_init(struct panfrost_device >>> *pfdev) >>> >>> static void panfrost_clk_fini(struct panfrost_device *pfdev) { >>> + clk_disable_unprepare(pfdev->bus_ace_clock); >>> clk_disable_unprepare(pfdev->bus_clock); >>> clk_disable_unprepare(pfdev->clock); >>> } >>> @@ -432,6 +448,10 @@ static int panfrost_device_runtime_resume(struct device *dev) >>> ret = clk_enable(pfdev->bus_clock); >>> if (ret) >>> goto err_bus_clk; >>> + >>> + ret = clk_enable(pfdev->bus_ace_clock); >>> + if (ret) >>> + goto err_bus_ace_clk; >>> } >>> >>> panfrost_device_reset(pfdev, true); >>> @@ -439,6 +459,9 @@ static int panfrost_device_runtime_resume(struct >>> device *dev) >>> >>> return 0; >>> >>> +err_bus_ace_clk: >>> + if (pfdev->comp->pm_features & BIT(GPU_PM_RT)) >>> + clk_disable(pfdev->bus_clock); >>> err_bus_clk: >>> if (pfdev->comp->pm_features & BIT(GPU_PM_RT)) >>> clk_disable(pfdev->clock); >>> @@ -462,6 +485,7 @@ static int panfrost_device_runtime_suspend(struct device *dev) >>> panfrost_gpu_power_off(pfdev); >>> >>> if (pfdev->comp->pm_features & BIT(GPU_PM_RT)) { >>> + clk_disable(pfdev->bus_ace_clock); >>> clk_disable(pfdev->bus_clock); >>> clk_disable(pfdev->clock); >>> reset_control_assert(pfdev->rstc); >>> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h >>> b/drivers/gpu/drm/panfrost/panfrost_device.h >>> index 0f3992412205..ec55c136b1b6 100644 >>> --- a/drivers/gpu/drm/panfrost/panfrost_device.h >>> +++ b/drivers/gpu/drm/panfrost/panfrost_device.h >>> @@ -136,6 +136,7 @@ struct panfrost_device { >>> void __iomem *iomem; >>> struct clk *clock; >>> struct clk *bus_clock; >>> + struct clk *bus_ace_clock; >>> struct regulator_bulk_data *regulators; >>> struct reset_control *rstc; >>> /* pm_domains for devices with more than one. */ >