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 562B3C7EE2C for ; Tue, 16 May 2023 16:17:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Subject:CC:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=AOMekVvitH6+jjYT085uRfrPuQym0ZlGnkBa1KQBvxY=; b=e92yR+Hw3cIORM Fimxq9fIxmyfTe1HMqGO5fhJQNHs8uvEE31rpXx3hoYLJgGxOdZhA2EO4yI9S5TFjDf8SENM2HoUD vZ3N1eV2UzRpxz9BdDZQsPstdvsyGBCgQQHY2aIdWPyCWjxDaae55/4/AJtDnw92oKitSG8TaHFfr OZ2WUYcoiUQl2tj1H031bjPRPlAbux2g8ISk5E+ekXSolf3C8+ksd11LHFgmXl3vQU7UetDa1OdoN kR6fQHZSaSTki+ZwGmfBdUZbCBXLhjUr2GCDZhGz/REdRBARIQwIf8BlrM92ew5NM5cs6PPxspFMg hB5rM0h1DPOsUUkyxTsg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1pyxMK-006QxH-39; Tue, 16 May 2023 16:17:08 +0000 Received: from frasgout.his.huawei.com ([185.176.79.56]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1pyxMG-006QvZ-0i; Tue, 16 May 2023 16:17:06 +0000 Received: from lhrpeml500005.china.huawei.com (unknown [172.18.147.206]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4QLLtR52cmz67Xkv; Wed, 17 May 2023 00:15:11 +0800 (CST) Received: from localhost (10.202.227.76) by lhrpeml500005.china.huawei.com (7.191.163.240) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Tue, 16 May 2023 17:16:58 +0100 Date: Tue, 16 May 2023 17:16:57 +0100 From: Jonathan Cameron To: Sascha Hauer CC: , , Heiko Stuebner , Kyungmin Park , MyungJoo Ham , Will Deacon , Mark Rutland , , Michael Riesch Subject: Re: [PATCH v4 14/21] PM / devfreq: rockchip-dfi: Prepare for multiple users Message-ID: <20230516171657.00000c6c@Huawei.com> In-Reply-To: <20230505113856.463650-15-s.hauer@pengutronix.de> References: <20230505113856.463650-1-s.hauer@pengutronix.de> <20230505113856.463650-15-s.hauer@pengutronix.de> Organization: Huawei Technologies Research and Development (UK) Ltd. X-Mailer: Claws Mail 4.1.0 (GTK 3.24.33; x86_64-w64-mingw32) MIME-Version: 1.0 X-Originating-IP: [10.202.227.76] X-ClientProxiedBy: lhrpeml500006.china.huawei.com (7.191.161.198) To lhrpeml500005.china.huawei.com (7.191.163.240) X-CFilter-Loop: Reflected X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230516_091704_564271_894437B5 X-CRM114-Status: GOOD ( 25.55 ) X-BeenThere: linux-rockchip@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Upstream kernel work for Rockchip platforms List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+linux-rockchip=archiver.kernel.org@lists.infradead.org On Fri, 5 May 2023 13:38:49 +0200 Sascha Hauer wrote: > When adding perf support later the DFI must be enabled when > either of devfreq-event or perf is active. Prepare for that > by adding a usage counter for the DFI. Also move enabling > and disabling of the clock away from the devfreq-event specific > functions to which the perf specific part won't have access. > > Signed-off-by: Sascha Hauer The clock code is IIRC refcounted anyway and I think has appropriate locks, so you code just enable / disable that in both paths. Probably not that helpful though given you still need to refcount to protect the register writes. So this looks fine to me. Reviewed-by: Jonathan Cameron > --- > drivers/devfreq/event/rockchip-dfi.c | 57 +++++++++++++++++++--------- > 1 file changed, 40 insertions(+), 17 deletions(-) > > diff --git a/drivers/devfreq/event/rockchip-dfi.c b/drivers/devfreq/event/rockchip-dfi.c > index c0b7b1e9805e9..eae010644935a 100644 > --- a/drivers/devfreq/event/rockchip-dfi.c > +++ b/drivers/devfreq/event/rockchip-dfi.c > @@ -69,13 +69,28 @@ struct rockchip_dfi { > void __iomem *regs; > struct regmap *regmap_pmu; > struct clk *clk; > + int usecount; > + struct mutex mutex; > u32 ddr_type; > unsigned int channel_mask; > }; > > -static void rockchip_dfi_start_hardware_counter(struct rockchip_dfi *dfi) > +static int rockchip_dfi_enable(struct rockchip_dfi *dfi) > { > void __iomem *dfi_regs = dfi->regs; > + int ret = 0; > + > + mutex_lock(&dfi->mutex); > + > + dfi->usecount++; > + if (dfi->usecount > 1) > + goto out; > + > + ret = clk_prepare_enable(dfi->clk); > + if (ret) { > + dev_err(&dfi->edev->dev, "failed to enable dfi clk: %d\n", ret); > + goto out; > + } > > /* clear DDRMON_CTRL setting */ > writel_relaxed(HIWORD_UPDATE(0, 0xffff), dfi_regs + DDRMON_CTRL); > @@ -99,14 +114,30 @@ static void rockchip_dfi_start_hardware_counter(struct rockchip_dfi *dfi) > /* enable count, use software mode */ > writel_relaxed(HIWORD_UPDATE(DDRMON_CTRL_SOFTWARE_EN, DDRMON_CTRL_SOFTWARE_EN), > dfi_regs + DDRMON_CTRL); > +out: > + mutex_unlock(&dfi->mutex); > + > + return ret; > } > > -static void rockchip_dfi_stop_hardware_counter(struct rockchip_dfi *dfi) > +static void rockchip_dfi_disable(struct rockchip_dfi *dfi) > { > void __iomem *dfi_regs = dfi->regs; > > + mutex_lock(&dfi->mutex); > + > + dfi->usecount--; > + > + WARN_ON_ONCE(dfi->usecount < 0); > + > + if (dfi->usecount > 0) > + goto out; > + > writel_relaxed(HIWORD_UPDATE(0, DDRMON_CTRL_SOFTWARE_EN), > dfi_regs + DDRMON_CTRL); > + clk_disable_unprepare(dfi->clk); > +out: > + mutex_unlock(&dfi->mutex); > } > > static void rockchip_dfi_read_counters(struct rockchip_dfi *dfi, struct dmc_count *count) > @@ -124,29 +155,20 @@ static void rockchip_dfi_read_counters(struct rockchip_dfi *dfi, struct dmc_coun > } > } > > -static int rockchip_dfi_disable(struct devfreq_event_dev *edev) > +static int rockchip_dfi_event_disable(struct devfreq_event_dev *edev) > { > struct rockchip_dfi *dfi = devfreq_event_get_drvdata(edev); > > - rockchip_dfi_stop_hardware_counter(dfi); > - clk_disable_unprepare(dfi->clk); > + rockchip_dfi_disable(dfi); Similar to below. Up to you on whether you prefer shorter code vs having the type of the parameter explicit in here. > > return 0; > } > > -static int rockchip_dfi_enable(struct devfreq_event_dev *edev) > +static int rockchip_dfi_event_enable(struct devfreq_event_dev *edev) > { > struct rockchip_dfi *dfi = devfreq_event_get_drvdata(edev); > - int ret; > - > - ret = clk_prepare_enable(dfi->clk); > - if (ret) { > - dev_err(&edev->dev, "failed to enable dfi clk: %d\n", ret); > - return ret; > - } > > - rockchip_dfi_start_hardware_counter(dfi); > - return 0; > + return rockchip_dfi_enable(dfi); No real point in local variable any more, so maybe return rockchip_dfi_enable(devfreq_event_get_drvdata(edev)); > } > > static int rockchip_dfi_set_event(struct devfreq_event_dev *edev) > @@ -190,8 +212,8 @@ static int rockchip_dfi_get_event(struct devfreq_event_dev *edev, > } > > static const struct devfreq_event_ops rockchip_dfi_ops = { > - .disable = rockchip_dfi_disable, > - .enable = rockchip_dfi_enable, > + .disable = rockchip_dfi_event_disable, > + .enable = rockchip_dfi_event_enable, > .get_event = rockchip_dfi_get_event, > .set_event = rockchip_dfi_set_event, > }; > @@ -285,6 +307,7 @@ static int rockchip_dfi_probe(struct platform_device *pdev) > return PTR_ERR(dfi->regmap_pmu); > > dfi->dev = dev; > + mutex_init(&dfi->mutex); > > desc = &dfi->desc; > desc->ops = &rockchip_dfi_ops; _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip 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 553A8C77B7A for ; Tue, 16 May 2023 16:17:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Subject:CC:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=UHcQWMVNRwN3oMkS1w4BHy+m4nmHm+y7tw661qoV5uw=; b=JtNmKLh1rge5LA B1rYMYv4F4Ul/ybbJFvrTcbFA+kd1lDYNn9iqK/Hg7Gbu8pijoR7cLaOm9LTQK6Uj9LqpoHM0omRT LdWdh8ak8xVoBSxnnCD37zXKdJ4psnqW2YZ8Gmnwq2eO3q6Wwo22tUiOGlsgp+9zO8qKwegRVfoFe y4ysWuT3WAvDV//WGfUI5Jwl5EeQdg8bPI8sIWv0WoiSl5C73gc+rRqvgoSWA9ObHyyM7J9OmPuOW tarDKcDRKdyQT3mwLOB9DIfdmmxU4Y4HCmzfeaiWPvR4susNK0ZfwfNvTnz8ogDeBxf9SE2tixPh+ 4J6aL6a3s+2ihI41wQxg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1pyxMK-006Qx7-1b; Tue, 16 May 2023 16:17:08 +0000 Received: from frasgout.his.huawei.com ([185.176.79.56]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1pyxMG-006QvZ-0i; Tue, 16 May 2023 16:17:06 +0000 Received: from lhrpeml500005.china.huawei.com (unknown [172.18.147.206]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4QLLtR52cmz67Xkv; Wed, 17 May 2023 00:15:11 +0800 (CST) Received: from localhost (10.202.227.76) by lhrpeml500005.china.huawei.com (7.191.163.240) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Tue, 16 May 2023 17:16:58 +0100 Date: Tue, 16 May 2023 17:16:57 +0100 From: Jonathan Cameron To: Sascha Hauer CC: , , Heiko Stuebner , Kyungmin Park , MyungJoo Ham , Will Deacon , Mark Rutland , , Michael Riesch Subject: Re: [PATCH v4 14/21] PM / devfreq: rockchip-dfi: Prepare for multiple users Message-ID: <20230516171657.00000c6c@Huawei.com> In-Reply-To: <20230505113856.463650-15-s.hauer@pengutronix.de> References: <20230505113856.463650-1-s.hauer@pengutronix.de> <20230505113856.463650-15-s.hauer@pengutronix.de> Organization: Huawei Technologies Research and Development (UK) Ltd. X-Mailer: Claws Mail 4.1.0 (GTK 3.24.33; x86_64-w64-mingw32) MIME-Version: 1.0 X-Originating-IP: [10.202.227.76] X-ClientProxiedBy: lhrpeml500006.china.huawei.com (7.191.161.198) To lhrpeml500005.china.huawei.com (7.191.163.240) X-CFilter-Loop: Reflected X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230516_091704_564271_894437B5 X-CRM114-Status: GOOD ( 25.55 ) 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: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, 5 May 2023 13:38:49 +0200 Sascha Hauer wrote: > When adding perf support later the DFI must be enabled when > either of devfreq-event or perf is active. Prepare for that > by adding a usage counter for the DFI. Also move enabling > and disabling of the clock away from the devfreq-event specific > functions to which the perf specific part won't have access. > > Signed-off-by: Sascha Hauer The clock code is IIRC refcounted anyway and I think has appropriate locks, so you code just enable / disable that in both paths. Probably not that helpful though given you still need to refcount to protect the register writes. So this looks fine to me. Reviewed-by: Jonathan Cameron > --- > drivers/devfreq/event/rockchip-dfi.c | 57 +++++++++++++++++++--------- > 1 file changed, 40 insertions(+), 17 deletions(-) > > diff --git a/drivers/devfreq/event/rockchip-dfi.c b/drivers/devfreq/event/rockchip-dfi.c > index c0b7b1e9805e9..eae010644935a 100644 > --- a/drivers/devfreq/event/rockchip-dfi.c > +++ b/drivers/devfreq/event/rockchip-dfi.c > @@ -69,13 +69,28 @@ struct rockchip_dfi { > void __iomem *regs; > struct regmap *regmap_pmu; > struct clk *clk; > + int usecount; > + struct mutex mutex; > u32 ddr_type; > unsigned int channel_mask; > }; > > -static void rockchip_dfi_start_hardware_counter(struct rockchip_dfi *dfi) > +static int rockchip_dfi_enable(struct rockchip_dfi *dfi) > { > void __iomem *dfi_regs = dfi->regs; > + int ret = 0; > + > + mutex_lock(&dfi->mutex); > + > + dfi->usecount++; > + if (dfi->usecount > 1) > + goto out; > + > + ret = clk_prepare_enable(dfi->clk); > + if (ret) { > + dev_err(&dfi->edev->dev, "failed to enable dfi clk: %d\n", ret); > + goto out; > + } > > /* clear DDRMON_CTRL setting */ > writel_relaxed(HIWORD_UPDATE(0, 0xffff), dfi_regs + DDRMON_CTRL); > @@ -99,14 +114,30 @@ static void rockchip_dfi_start_hardware_counter(struct rockchip_dfi *dfi) > /* enable count, use software mode */ > writel_relaxed(HIWORD_UPDATE(DDRMON_CTRL_SOFTWARE_EN, DDRMON_CTRL_SOFTWARE_EN), > dfi_regs + DDRMON_CTRL); > +out: > + mutex_unlock(&dfi->mutex); > + > + return ret; > } > > -static void rockchip_dfi_stop_hardware_counter(struct rockchip_dfi *dfi) > +static void rockchip_dfi_disable(struct rockchip_dfi *dfi) > { > void __iomem *dfi_regs = dfi->regs; > > + mutex_lock(&dfi->mutex); > + > + dfi->usecount--; > + > + WARN_ON_ONCE(dfi->usecount < 0); > + > + if (dfi->usecount > 0) > + goto out; > + > writel_relaxed(HIWORD_UPDATE(0, DDRMON_CTRL_SOFTWARE_EN), > dfi_regs + DDRMON_CTRL); > + clk_disable_unprepare(dfi->clk); > +out: > + mutex_unlock(&dfi->mutex); > } > > static void rockchip_dfi_read_counters(struct rockchip_dfi *dfi, struct dmc_count *count) > @@ -124,29 +155,20 @@ static void rockchip_dfi_read_counters(struct rockchip_dfi *dfi, struct dmc_coun > } > } > > -static int rockchip_dfi_disable(struct devfreq_event_dev *edev) > +static int rockchip_dfi_event_disable(struct devfreq_event_dev *edev) > { > struct rockchip_dfi *dfi = devfreq_event_get_drvdata(edev); > > - rockchip_dfi_stop_hardware_counter(dfi); > - clk_disable_unprepare(dfi->clk); > + rockchip_dfi_disable(dfi); Similar to below. Up to you on whether you prefer shorter code vs having the type of the parameter explicit in here. > > return 0; > } > > -static int rockchip_dfi_enable(struct devfreq_event_dev *edev) > +static int rockchip_dfi_event_enable(struct devfreq_event_dev *edev) > { > struct rockchip_dfi *dfi = devfreq_event_get_drvdata(edev); > - int ret; > - > - ret = clk_prepare_enable(dfi->clk); > - if (ret) { > - dev_err(&edev->dev, "failed to enable dfi clk: %d\n", ret); > - return ret; > - } > > - rockchip_dfi_start_hardware_counter(dfi); > - return 0; > + return rockchip_dfi_enable(dfi); No real point in local variable any more, so maybe return rockchip_dfi_enable(devfreq_event_get_drvdata(edev)); > } > > static int rockchip_dfi_set_event(struct devfreq_event_dev *edev) > @@ -190,8 +212,8 @@ static int rockchip_dfi_get_event(struct devfreq_event_dev *edev, > } > > static const struct devfreq_event_ops rockchip_dfi_ops = { > - .disable = rockchip_dfi_disable, > - .enable = rockchip_dfi_enable, > + .disable = rockchip_dfi_event_disable, > + .enable = rockchip_dfi_event_enable, > .get_event = rockchip_dfi_get_event, > .set_event = rockchip_dfi_set_event, > }; > @@ -285,6 +307,7 @@ static int rockchip_dfi_probe(struct platform_device *pdev) > return PTR_ERR(dfi->regmap_pmu); > > dfi->dev = dev; > + mutex_init(&dfi->mutex); > > desc = &dfi->desc; > desc->ops = &rockchip_dfi_ops; _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel