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 1BEBABA2D; Wed, 13 Mar 2024 04:33:13 +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=1710304397; cv=none; b=m680Jf3rv8CCOGTxpJgQm7rHfquBGaZTeLho7wfSF8988eJd+I3h8OxoeGt3SHFrYDwBNYFvB5oeVU1bToHqqgzVZupuMytrbUfTPuLNVZ/EJvA8IitKu2bih+0RrdFZra0YPSivq+X9OAiT2ha+oWQS0nc9Yqt7+6h8lKkCAPg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710304397; c=relaxed/simple; bh=nsV1JfBWq/A84vQXi0hgqmhpryA7fQMeUCoGSii3KnI=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=PlT9fhKi57gAT++5j8vHNrbSgl9R5Z8ecOkKbLmjpeSdRpSSPJnA2jrA1oX+1M3/TYIhANw9ADWSBhsQVEd+LZvVgNhkq02qruOyoAVN6xQjAA2w9xBVfeWhk/S6Otpx14vOyw5LGxdmLciKaEtMDBrpGbMW5SfMICtBNv+zRmE= 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 F2A681007; Tue, 12 Mar 2024 21:33:49 -0700 (PDT) Received: from [10.162.43.8] (a077893.blr.arm.com [10.162.43.8]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 93EF43F64C; Tue, 12 Mar 2024 21:33:09 -0700 (PDT) Message-ID: Date: Wed, 13 Mar 2024 10:03:06 +0530 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 V6 06/11] coresight: funnel: Move ACPI support from AMBA driver to platform driver Content-Language: en-US To: Suzuki K Poulose , linux-arm-kernel@lists.infradead.org Cc: Lorenzo Pieralisi , Sudeep Holla , Mike Leach , James Clark , Maxime Coquelin , Alexandre Torgue , linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org, coresight@lists.linaro.org, linux-stm32@st-md-mailman.stormreply.com References: <20240312102318.2285165-1-anshuman.khandual@arm.com> <20240312102318.2285165-7-anshuman.khandual@arm.com> <2dbef82c-96a0-419f-9950-8ee4169fb634@arm.com> From: Anshuman Khandual In-Reply-To: <2dbef82c-96a0-419f-9950-8ee4169fb634@arm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On 3/12/24 20:11, Suzuki K Poulose wrote: > On 12/03/2024 10:23, Anshuman Khandual wrote: >> Add support for the dynamic funnel device in the platform driver, which can >> then be used on ACPI based platforms. This change would allow runtime power >> management for ACPI based systems. >> >> The driver would try to enable the APB clock if available. Also, rename the >> code to reflect the fact that it now handles both static and dynamic >> funnels. But first this refactors funnel_probe() making sure it can be used >> both for platform and AMBA drivers, by moving the pm_runtime_put() to the >> callers. >> >> Cc: Lorenzo Pieralisi >> Cc: Sudeep Holla >> Cc: Suzuki K Poulose >> Cc: Mike Leach >> Cc: James Clark >> Cc: linux-acpi@vger.kernel.org >> Cc: linux-arm-kernel@lists.infradead.org >> Cc: linux-kernel@vger.kernel.org >> Cc: coresight@lists.linaro.org >> Tested-by: Sudeep Holla # Boot and driver probe only >> Acked-by: Sudeep Holla # For ACPI related changes >> Reviewed-by: James Clark >> Signed-off-by: Anshuman Khandual >> --- >> Changes in V6: >> >> - Added clk_disable_unprepare() for pclk in funnel_probe() error path >> - Added WARN_ON(!drvdata) check in funnel_platform_remove() >> - Added additional elements for acpi_device_id[] >> >>   drivers/acpi/arm64/amba.c                     |  1 - >>   .../hwtracing/coresight/coresight-funnel.c    | 72 ++++++++++++------- >>   2 files changed, 48 insertions(+), 25 deletions(-) >> >> diff --git a/drivers/acpi/arm64/amba.c b/drivers/acpi/arm64/amba.c >> index 270f4e3819a2..afb6afb66967 100644 >> --- a/drivers/acpi/arm64/amba.c >> +++ b/drivers/acpi/arm64/amba.c >> @@ -28,7 +28,6 @@ static const struct acpi_device_id amba_id_list[] = { >>       {"ARMHC979", 0}, /* ARM CoreSight TPIU */ >>       {"ARMHC97C", 0}, /* ARM CoreSight SoC-400 TMC, SoC-600 ETF/ETB */ >>       {"ARMHC9CA", 0}, /* ARM CoreSight CATU */ >> -    {"ARMHC9FF", 0}, /* ARM CoreSight Dynamic Funnel */ >>       {"", 0}, >>   }; >>   diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c >> index ff3ea0670a5b..3b4be10a0f0c 100644 >> --- a/drivers/hwtracing/coresight/coresight-funnel.c >> +++ b/drivers/hwtracing/coresight/coresight-funnel.c >> @@ -36,6 +36,7 @@ DEFINE_CORESIGHT_DEVLIST(funnel_devs, "funnel"); >>    * struct funnel_drvdata - specifics associated to a funnel component >>    * @base:    memory mapped base address for this component. >>    * @atclk:    optional clock for the core parts of the funnel. >> + * @pclk:    APB clock if present, otherwise NULL >>    * @csdev:    component vitals needed by the framework. >>    * @priority:    port selection order. >>    * @spinlock:    serialize enable/disable operations. >> @@ -43,6 +44,7 @@ DEFINE_CORESIGHT_DEVLIST(funnel_devs, "funnel"); >>   struct funnel_drvdata { >>       void __iomem        *base; >>       struct clk        *atclk; >> +    struct clk        *pclk; >>       struct coresight_device    *csdev; >>       unsigned long        priority; >>       spinlock_t        spinlock; >> @@ -236,6 +238,10 @@ static int funnel_probe(struct device *dev, struct resource *res) >>               return ret; >>       } >>   +    drvdata->pclk = coresight_get_enable_apb_pclk(dev); >> +    if (IS_ERR(drvdata->pclk)) >> +        return -ENODEV; >> + >>       /* >>        * Map the device base for dynamic-funnel, which has been >>        * validated by AMBA core. >> @@ -272,12 +278,13 @@ static int funnel_probe(struct device *dev, struct resource *res) >>           goto out_disable_clk; >>       } >>   -    pm_runtime_put(dev); >>       ret = 0; >>     out_disable_clk: >>       if (ret && !IS_ERR_OR_NULL(drvdata->atclk)) >>           clk_disable_unprepare(drvdata->atclk); >> +    if (ret && !IS_ERR_OR_NULL(drvdata->pclk)) >> +        clk_disable_unprepare(drvdata->pclk); >>       return ret; >>   } >>   @@ -298,6 +305,9 @@ static int funnel_runtime_suspend(struct device *dev) >>       if (drvdata && !IS_ERR(drvdata->atclk)) >>           clk_disable_unprepare(drvdata->atclk); >>   +    if (drvdata && !IS_ERR_OR_NULL(drvdata->pclk)) >> +        clk_disable_unprepare(drvdata->pclk); >> + >>       return 0; >>   } >>   @@ -308,6 +318,8 @@ static int funnel_runtime_resume(struct device *dev) >>       if (drvdata && !IS_ERR(drvdata->atclk)) >>           clk_prepare_enable(drvdata->atclk); >>   +    if (drvdata && !IS_ERR_OR_NULL(drvdata->pclk)) >> +        clk_prepare_enable(drvdata->pclk); >>       return 0; >>   } >>   #endif >> @@ -316,55 +328,61 @@ static const struct dev_pm_ops funnel_dev_pm_ops = { >>       SET_RUNTIME_PM_OPS(funnel_runtime_suspend, funnel_runtime_resume, NULL) >>   }; >>   -static int static_funnel_probe(struct platform_device *pdev) >> +static int funnel_platform_probe(struct platform_device *pdev) >>   { >> +    struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>       int ret; >>         pm_runtime_get_noresume(&pdev->dev); >>       pm_runtime_set_active(&pdev->dev); >>       pm_runtime_enable(&pdev->dev); >>   -    /* Static funnel do not have programming base */ >> -    ret = funnel_probe(&pdev->dev, NULL); >> - >> -    if (ret) { >> -        pm_runtime_put_noidle(&pdev->dev); >> +    ret = funnel_probe(&pdev->dev, res); >> +    pm_runtime_put(&pdev->dev); >> +    if (ret) >>           pm_runtime_disable(&pdev->dev); >> -    } >>         return ret; >>   } >>   -static void static_funnel_remove(struct platform_device *pdev) >> +static void funnel_platform_remove(struct platform_device *pdev) >>   { >> +    struct funnel_drvdata *drvdata = dev_get_drvdata(&pdev->dev); >> + >> +    if (WARN_ON(!drvdata)) >> +        return; >> + >>       funnel_remove(&pdev->dev); >>       pm_runtime_disable(&pdev->dev); >> +    if (drvdata && !IS_ERR_OR_NULL(drvdata->pclk)) > > Same as the previous patch Right, unlike in xxx_runtime_suspend/resume() functions, xxx_platform_remove() functions do have a (!drvdata) check right at the beginning. Hence a redundant check in such places can be dropped.