From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757543AbcLTIcc (ORCPT ); Tue, 20 Dec 2016 03:32:32 -0500 Received: from mailout4.samsung.com ([203.254.224.34]:57956 "EHLO mailout4.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753523AbcLTIc2 (ORCPT ); Tue, 20 Dec 2016 03:32:28 -0500 MIME-version: 1.0 Content-type: text/plain; charset=utf-8 X-AuditID: cbfee61a-f79bd6d000000fc6-06-5858ec9aa4bc Content-transfer-encoding: 8BIT Message-id: <5858EC9A.9090006@samsung.com> Date: Tue, 20 Dec 2016 17:32:26 +0900 From: Chanwoo Choi Organization: Samsung Electronics User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 To: Tobias Jakobi , myungjoo.ham@samsung.com, kyungmin.park@samsung.com Cc: rjw@rjwysocki.net, chanwoo@kernel.org, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, Kukjin Kim , Krzysztof Kozlowski , Javier Martinez Canillas , linux-samsung-soc@vger.kernel.org Subject: Re: [PATCH 2/7] PM / devfreq: exynos-ppmu: Use the regmap interface to handle the registers References: <1481794243-5046-1-git-send-email-cw00.choi@samsung.com> <1481794243-5046-3-git-send-email-cw00.choi@samsung.com> <58583941.1040809@math.uni-bielefeld.de> <585898FB.7050801@samsung.com> <5858E70B.2000902@math.uni-bielefeld.de> <5858EB49.3010609@samsung.com> In-reply-to: <5858EB49.3010609@samsung.com> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrBIsWRmVeSWpSXmKPExsVy+t9jQd1ZbyIiDN7fF7WYeOMKi8Wbt2uY LPofv2a2OH9+A7vF2aY37BaXd81hs/jce4TRYsb5fUwWtxtXsFmcOX2J1aJt9QdWB26PTas6 2Tz+HWP32NJ/F0hcbWfx6NuyitHj8ya5ALYoN5uM1MSU1CKF1Lzk/JTMvHRbpdAQN10LJYW8 xNxUW6UIXd+QICWFssScUiDPyAANODgHuAcr6dsluGXsmnSZpeDBVsaK7StnsTQwvpjM2MXI ySEhYCKxpXc5M4QtJnHh3nq2LkYuDiGBWYwS//tnsoEkeAUEJX5MvsfSxcjBwSwgL3HkUjaE qS4xZUouRPkDRond+89ClWtJvDl9nR3EZhFQlbh48wLYfDag+P4XN8Bq+AUUJa7+eMwIMkdU IEKi+0QlSFhEIEOia8YrdpCZzAKtTBIrGp6C1QsLpElMbpnLDrHsOZPE6/0QD3AKaEvsWNPN PoFRcBaSU2chnDoL4dQFjMyrGCVSC5ILipPScw3zUsv1ihNzi0vz0vWS83M3MYJj9ZnUDsaD u9wPMQpwMCrx8HZUR0QIsSaWFVfmHmKU4GBWEuE1fg0U4k1JrKxKLcqPLyrNSS0+xGgK9OtE ZinR5HxgGskriTc0MTcxNzawMLe0NDFSEudtnP0sXEggPbEkNTs1tSC1CKaPiYNTqoFR4lR3 4JFjb/4LMPzXYD9fe9/o8Oe+YEGvqbt2X3+/RCt5d0Ha5taLIRW7Xt//Fiu7Uvuq8exS6+ZV N/K+S/j0ytsYm3zdaXW0ZnlopEPnZOPQGvfJj2qXuk6S4J3yddPNnUp279vXRaj7HrnL0Jam 3G9fnPLvovfm+6feaosa5x5Ls5447dk+JZbijERDLeai4kQAS3cmq+sCAAA= X-MTR: 20000000000000000@CPGS Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2016년 12월 20일 17:26, Chanwoo Choi wrote: > On 2016년 12월 20일 17:08, Tobias Jakobi wrote: >> Hello Chanwoo, >> >> >> Chanwoo Choi wrote: >>> Hi, >>> >>> On 2016년 12월 20일 04:47, Tobias Jakobi wrote: >>>> Hello, >>>> >>>> I was just wondering what is improved by moving to regmap. For me this >>>> looks like it only complicates the code. Lots of regmap_{read,write}() >>>> and for each one of these we need to check the return code. >>> >>> It is correct to check the return value. It cover all of exception. >> that doesn't really answer my question. Which 'exceptions' are we >> talking about? What can go wrong with __raw_{writel,readl}(), that > > When using __raw_readl/writel() don't check the any return value, it it not correct. > When calling the function, basically we should check whether return value is error or success. > What is problem to check the return value? I use 'exception' word means the error situations. When handling the register of PPMU, the error might happen. So, we need to check the return value by using the regmap interface. > >> makes it necessary to put another layer on top of it? AFAIK regmap was >> introduced to handle read/writes to slow busses like I2C and SPI. I >> don't see that this applies here. > > The regmap support the MMIO on following driver. > - drivers/base/regmap/regmap-mmio.c > >> >> >>>> Also when exactly did __raw_writel() and friends become legacy? >>> >>> Also, I want to use the regmap interface (regmap-io) instead of __raw_readl/__raw_writel using the base address >>> directly. >> I see, but why are __raw_{writel,readl}() legacy as you say? I don't see >> this anywhere else in the kernel. > > If you just don't like the 'legacy' expression. I'll remove it. > It is not any important. The key point of this patch uses the regmap interface. > Usually, when adding new device driver, I use the regmap mmio interface > instead of __raw_readl/writel. So, this patch changes it. > > Regards, > Chanwoo Choi > >> >> >> With best wishes, >> Tobias >> >> >>> >>> Regards, >>> Chanwoo Choi >>> >>>> >>>> With best wishes, >>>> Tobias >>>> >>>> >>>> Chanwoo Choi wrote: >>>>> This patch uses the regmap interface to read and write the registers for exynos >>>>> PPMU device instead of the legacy memory map functions. >>>>> >>>>> Cc: Kukjin Kim >>>>> Cc: Krzysztof Kozlowski >>>>> Cc: Javier Martinez Canillas >>>>> Cc: linux-samsung-soc@vger.kernel.org >>>>> Signed-off-by: Chanwoo Choi >>>>> --- >>>>> drivers/devfreq/event/exynos-ppmu.c | 326 ++++++++++++++++++++++++++---------- >>>>> 1 file changed, 237 insertions(+), 89 deletions(-) >>>>> >>>>> diff --git a/drivers/devfreq/event/exynos-ppmu.c b/drivers/devfreq/event/exynos-ppmu.c >>>>> index 107eb91a9415..fb3706faf5bd 100644 >>>>> --- a/drivers/devfreq/event/exynos-ppmu.c >>>>> +++ b/drivers/devfreq/event/exynos-ppmu.c >>>>> @@ -17,13 +17,13 @@ >>>>> #include >>>>> #include >>>>> #include >>>>> +#include >>>>> #include >>>>> #include >>>>> >>>>> #include "exynos-ppmu.h" >>>>> >>>>> struct exynos_ppmu_data { >>>>> - void __iomem *base; >>>>> struct clk *clk; >>>>> }; >>>>> >>>>> @@ -33,6 +33,7 @@ struct exynos_ppmu { >>>>> unsigned int num_events; >>>>> >>>>> struct device *dev; >>>>> + struct regmap *regmap; >>>>> >>>>> struct exynos_ppmu_data ppmu; >>>>> }; >>>>> @@ -107,20 +108,28 @@ static int exynos_ppmu_find_ppmu_id(struct devfreq_event_dev *edev) >>>>> static int exynos_ppmu_disable(struct devfreq_event_dev *edev) >>>>> { >>>>> struct exynos_ppmu *info = devfreq_event_get_drvdata(edev); >>>>> + int ret; >>>>> u32 pmnc; >>>>> >>>>> /* Disable all counters */ >>>>> - __raw_writel(PPMU_CCNT_MASK | >>>>> - PPMU_PMCNT0_MASK | >>>>> - PPMU_PMCNT1_MASK | >>>>> - PPMU_PMCNT2_MASK | >>>>> - PPMU_PMCNT3_MASK, >>>>> - info->ppmu.base + PPMU_CNTENC); >>>>> + ret = regmap_write(info->regmap, PPMU_CNTENC, >>>>> + PPMU_CCNT_MASK | >>>>> + PPMU_PMCNT0_MASK | >>>>> + PPMU_PMCNT1_MASK | >>>>> + PPMU_PMCNT2_MASK | >>>>> + PPMU_PMCNT3_MASK); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> >>>>> /* Disable PPMU */ >>>>> - pmnc = __raw_readl(info->ppmu.base + PPMU_PMNC); >>>>> + ret = regmap_read(info->regmap, PPMU_PMNC, &pmnc); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> pmnc &= ~PPMU_PMNC_ENABLE_MASK; >>>>> - __raw_writel(pmnc, info->ppmu.base + PPMU_PMNC); >>>>> + ret = regmap_write(info->regmap, PPMU_PMNC, pmnc); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> >>>>> return 0; >>>>> } >>>>> @@ -129,29 +138,42 @@ static int exynos_ppmu_set_event(struct devfreq_event_dev *edev) >>>>> { >>>>> struct exynos_ppmu *info = devfreq_event_get_drvdata(edev); >>>>> int id = exynos_ppmu_find_ppmu_id(edev); >>>>> + int ret; >>>>> u32 pmnc, cntens; >>>>> >>>>> if (id < 0) >>>>> return id; >>>>> >>>>> /* Enable specific counter */ >>>>> - cntens = __raw_readl(info->ppmu.base + PPMU_CNTENS); >>>>> + ret = regmap_read(info->regmap, PPMU_CNTENS, &cntens); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> cntens |= (PPMU_CCNT_MASK | (PPMU_ENABLE << id)); >>>>> - __raw_writel(cntens, info->ppmu.base + PPMU_CNTENS); >>>>> + ret = regmap_write(info->regmap, PPMU_CNTENS, cntens); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> >>>>> /* Set the event of Read/Write data count */ >>>>> - __raw_writel(PPMU_RO_DATA_CNT | PPMU_WO_DATA_CNT, >>>>> - info->ppmu.base + PPMU_BEVTxSEL(id)); >>>>> + ret = regmap_write(info->regmap, PPMU_BEVTxSEL(id), >>>>> + PPMU_RO_DATA_CNT | PPMU_WO_DATA_CNT); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> >>>>> /* Reset cycle counter/performance counter and enable PPMU */ >>>>> - pmnc = __raw_readl(info->ppmu.base + PPMU_PMNC); >>>>> + ret = regmap_read(info->regmap, PPMU_PMNC, &pmnc); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> pmnc &= ~(PPMU_PMNC_ENABLE_MASK >>>>> | PPMU_PMNC_COUNTER_RESET_MASK >>>>> | PPMU_PMNC_CC_RESET_MASK); >>>>> pmnc |= (PPMU_ENABLE << PPMU_PMNC_ENABLE_SHIFT); >>>>> pmnc |= (PPMU_ENABLE << PPMU_PMNC_COUNTER_RESET_SHIFT); >>>>> pmnc |= (PPMU_ENABLE << PPMU_PMNC_CC_RESET_SHIFT); >>>>> - __raw_writel(pmnc, info->ppmu.base + PPMU_PMNC); >>>>> + ret = regmap_write(info->regmap, PPMU_PMNC, pmnc); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> >>>>> return 0; >>>>> } >>>>> @@ -161,40 +183,64 @@ static int exynos_ppmu_get_event(struct devfreq_event_dev *edev, >>>>> { >>>>> struct exynos_ppmu *info = devfreq_event_get_drvdata(edev); >>>>> int id = exynos_ppmu_find_ppmu_id(edev); >>>>> - u32 pmnc, cntenc; >>>>> + unsigned int total_count, load_count; >>>>> + unsigned int pmcnt3_high, pmcnt3_low; >>>>> + unsigned int pmnc, cntenc; >>>>> + int ret; >>>>> >>>>> if (id < 0) >>>>> return -EINVAL; >>>>> >>>>> /* Disable PPMU */ >>>>> - pmnc = __raw_readl(info->ppmu.base + PPMU_PMNC); >>>>> + ret = regmap_read(info->regmap, PPMU_PMNC, &pmnc); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> pmnc &= ~PPMU_PMNC_ENABLE_MASK; >>>>> - __raw_writel(pmnc, info->ppmu.base + PPMU_PMNC); >>>>> + ret = regmap_write(info->regmap, PPMU_PMNC, pmnc); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> >>>>> /* Read cycle count */ >>>>> - edata->total_count = __raw_readl(info->ppmu.base + PPMU_CCNT); >>>>> + ret = regmap_read(info->regmap, PPMU_CCNT, &total_count); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + edata->total_count = total_count; >>>>> >>>>> /* Read performance count */ >>>>> switch (id) { >>>>> case PPMU_PMNCNT0: >>>>> case PPMU_PMNCNT1: >>>>> case PPMU_PMNCNT2: >>>>> - edata->load_count >>>>> - = __raw_readl(info->ppmu.base + PPMU_PMNCT(id)); >>>>> + ret = regmap_read(info->regmap, PPMU_PMNCT(id), &load_count); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + edata->load_count = load_count; >>>>> break; >>>>> case PPMU_PMNCNT3: >>>>> - edata->load_count = >>>>> - ((__raw_readl(info->ppmu.base + PPMU_PMCNT3_HIGH) << 8) >>>>> - | __raw_readl(info->ppmu.base + PPMU_PMCNT3_LOW)); >>>>> + ret = regmap_read(info->regmap, PPMU_PMCNT3_HIGH, &pmcnt3_high); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> + ret = regmap_read(info->regmap, PPMU_PMCNT3_LOW, &pmcnt3_low); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> + edata->load_count = ((pmcnt3_high << 8) | pmcnt3_low); >>>>> break; >>>>> default: >>>>> return -EINVAL; >>>>> } >>>>> >>>>> /* Disable specific counter */ >>>>> - cntenc = __raw_readl(info->ppmu.base + PPMU_CNTENC); >>>>> + ret = regmap_read(info->regmap, PPMU_CNTENC, &cntenc); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> cntenc |= (PPMU_CCNT_MASK | (PPMU_ENABLE << id)); >>>>> - __raw_writel(cntenc, info->ppmu.base + PPMU_CNTENC); >>>>> + ret = regmap_write(info->regmap, PPMU_CNTENC, cntenc); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> >>>>> dev_dbg(&edev->dev, "%s (event: %ld/%ld)\n", edev->desc->name, >>>>> edata->load_count, edata->total_count); >>>>> @@ -214,36 +260,93 @@ static int exynos_ppmu_get_event(struct devfreq_event_dev *edev, >>>>> static int exynos_ppmu_v2_disable(struct devfreq_event_dev *edev) >>>>> { >>>>> struct exynos_ppmu *info = devfreq_event_get_drvdata(edev); >>>>> + int ret; >>>>> u32 pmnc, clear; >>>>> >>>>> /* Disable all counters */ >>>>> clear = (PPMU_CCNT_MASK | PPMU_PMCNT0_MASK | PPMU_PMCNT1_MASK >>>>> | PPMU_PMCNT2_MASK | PPMU_PMCNT3_MASK); >>>>> + ret = regmap_write(info->regmap, PPMU_V2_FLAG, clear); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> + ret = regmap_write(info->regmap, PPMU_V2_INTENC, clear); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> >>>>> - __raw_writel(clear, info->ppmu.base + PPMU_V2_FLAG); >>>>> - __raw_writel(clear, info->ppmu.base + PPMU_V2_INTENC); >>>>> - __raw_writel(clear, info->ppmu.base + PPMU_V2_CNTENC); >>>>> - __raw_writel(clear, info->ppmu.base + PPMU_V2_CNT_RESET); >>>>> - >>>>> - __raw_writel(0x0, info->ppmu.base + PPMU_V2_CIG_CFG0); >>>>> - __raw_writel(0x0, info->ppmu.base + PPMU_V2_CIG_CFG1); >>>>> - __raw_writel(0x0, info->ppmu.base + PPMU_V2_CIG_CFG2); >>>>> - __raw_writel(0x0, info->ppmu.base + PPMU_V2_CIG_RESULT); >>>>> - __raw_writel(0x0, info->ppmu.base + PPMU_V2_CNT_AUTO); >>>>> - __raw_writel(0x0, info->ppmu.base + PPMU_V2_CH_EV0_TYPE); >>>>> - __raw_writel(0x0, info->ppmu.base + PPMU_V2_CH_EV1_TYPE); >>>>> - __raw_writel(0x0, info->ppmu.base + PPMU_V2_CH_EV2_TYPE); >>>>> - __raw_writel(0x0, info->ppmu.base + PPMU_V2_CH_EV3_TYPE); >>>>> - __raw_writel(0x0, info->ppmu.base + PPMU_V2_SM_ID_V); >>>>> - __raw_writel(0x0, info->ppmu.base + PPMU_V2_SM_ID_A); >>>>> - __raw_writel(0x0, info->ppmu.base + PPMU_V2_SM_OTHERS_V); >>>>> - __raw_writel(0x0, info->ppmu.base + PPMU_V2_SM_OTHERS_A); >>>>> - __raw_writel(0x0, info->ppmu.base + PPMU_V2_INTERRUPT_RESET); >>>>> + ret = regmap_write(info->regmap, PPMU_V2_CNTENC, clear); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> + ret = regmap_write(info->regmap, PPMU_V2_CNT_RESET, clear); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> + ret = regmap_write(info->regmap, PPMU_V2_CIG_CFG0, 0x0); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> + ret = regmap_write(info->regmap, PPMU_V2_CIG_CFG1, 0x0); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> + ret = regmap_write(info->regmap, PPMU_V2_CIG_CFG2, 0x0); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> + ret = regmap_write(info->regmap, PPMU_V2_CIG_RESULT, 0x0); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> + ret = regmap_write(info->regmap, PPMU_V2_CNT_AUTO, 0x0); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> + ret = regmap_write(info->regmap, PPMU_V2_CH_EV0_TYPE, 0x0); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> + ret = regmap_write(info->regmap, PPMU_V2_CH_EV1_TYPE, 0x0); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> + ret = regmap_write(info->regmap, PPMU_V2_CH_EV2_TYPE, 0x0); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> + ret = regmap_write(info->regmap, PPMU_V2_CH_EV3_TYPE, 0x0); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> + ret = regmap_write(info->regmap, PPMU_V2_SM_ID_V, 0x0); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> + ret = regmap_write(info->regmap, PPMU_V2_SM_ID_A, 0x0); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> + ret = regmap_write(info->regmap, PPMU_V2_SM_OTHERS_V, 0x0); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> + ret = regmap_write(info->regmap, PPMU_V2_SM_OTHERS_A, 0x0); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> + ret = regmap_write(info->regmap, PPMU_V2_INTERRUPT_RESET, 0x0); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> >>>>> /* Disable PPMU */ >>>>> - pmnc = __raw_readl(info->ppmu.base + PPMU_V2_PMNC); >>>>> + ret = regmap_read(info->regmap, PPMU_V2_PMNC, &pmnc); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> pmnc &= ~PPMU_PMNC_ENABLE_MASK; >>>>> - __raw_writel(pmnc, info->ppmu.base + PPMU_V2_PMNC); >>>>> + ret = regmap_write(info->regmap, PPMU_V2_PMNC, pmnc); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> >>>>> return 0; >>>>> } >>>>> @@ -251,30 +354,43 @@ static int exynos_ppmu_v2_disable(struct devfreq_event_dev *edev) >>>>> static int exynos_ppmu_v2_set_event(struct devfreq_event_dev *edev) >>>>> { >>>>> struct exynos_ppmu *info = devfreq_event_get_drvdata(edev); >>>>> + unsigned int pmnc, cntens; >>>>> int id = exynos_ppmu_find_ppmu_id(edev); >>>>> - u32 pmnc, cntens; >>>>> + int ret; >>>>> >>>>> /* Enable all counters */ >>>>> - cntens = __raw_readl(info->ppmu.base + PPMU_V2_CNTENS); >>>>> + ret = regmap_read(info->regmap, PPMU_V2_CNTENS, &cntens); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> cntens |= (PPMU_CCNT_MASK | (PPMU_ENABLE << id)); >>>>> - __raw_writel(cntens, info->ppmu.base + PPMU_V2_CNTENS); >>>>> + ret = regmap_write(info->regmap, PPMU_V2_CNTENS, cntens); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> >>>>> /* Set the event of Read/Write data count */ >>>>> switch (id) { >>>>> case PPMU_PMNCNT0: >>>>> case PPMU_PMNCNT1: >>>>> case PPMU_PMNCNT2: >>>>> - __raw_writel(PPMU_V2_RO_DATA_CNT | PPMU_V2_WO_DATA_CNT, >>>>> - info->ppmu.base + PPMU_V2_CH_EVx_TYPE(id)); >>>>> + ret = regmap_write(info->regmap, PPMU_V2_CH_EVx_TYPE(id), >>>>> + PPMU_V2_RO_DATA_CNT | PPMU_V2_WO_DATA_CNT); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> break; >>>>> case PPMU_PMNCNT3: >>>>> - __raw_writel(PPMU_V2_EVT3_RW_DATA_CNT, >>>>> - info->ppmu.base + PPMU_V2_CH_EVx_TYPE(id)); >>>>> + ret = regmap_write(info->regmap, PPMU_V2_CH_EVx_TYPE(id), >>>>> + PPMU_V2_EVT3_RW_DATA_CNT); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> break; >>>>> } >>>>> >>>>> /* Reset cycle counter/performance counter and enable PPMU */ >>>>> - pmnc = __raw_readl(info->ppmu.base + PPMU_V2_PMNC); >>>>> + ret = regmap_read(info->regmap, PPMU_V2_PMNC, &pmnc); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> pmnc &= ~(PPMU_PMNC_ENABLE_MASK >>>>> | PPMU_PMNC_COUNTER_RESET_MASK >>>>> | PPMU_PMNC_CC_RESET_MASK >>>>> @@ -284,7 +400,10 @@ static int exynos_ppmu_v2_set_event(struct devfreq_event_dev *edev) >>>>> pmnc |= (PPMU_ENABLE << PPMU_PMNC_COUNTER_RESET_SHIFT); >>>>> pmnc |= (PPMU_ENABLE << PPMU_PMNC_CC_RESET_SHIFT); >>>>> pmnc |= (PPMU_V2_MODE_MANUAL << PPMU_V2_PMNC_START_MODE_SHIFT); >>>>> - __raw_writel(pmnc, info->ppmu.base + PPMU_V2_PMNC); >>>>> + >>>>> + ret = regmap_write(info->regmap, PPMU_V2_PMNC, pmnc); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> >>>>> return 0; >>>>> } >>>>> @@ -294,37 +413,61 @@ static int exynos_ppmu_v2_get_event(struct devfreq_event_dev *edev, >>>>> { >>>>> struct exynos_ppmu *info = devfreq_event_get_drvdata(edev); >>>>> int id = exynos_ppmu_find_ppmu_id(edev); >>>>> - u32 pmnc, cntenc; >>>>> - u32 pmcnt_high, pmcnt_low; >>>>> - u64 load_count = 0; >>>>> + int ret; >>>>> + unsigned int pmnc, cntenc; >>>>> + unsigned int pmcnt_high, pmcnt_low; >>>>> + unsigned int total_count, count; >>>>> + unsigned long load_count = 0; >>>>> >>>>> /* Disable PPMU */ >>>>> - pmnc = __raw_readl(info->ppmu.base + PPMU_V2_PMNC); >>>>> + ret = regmap_read(info->regmap, PPMU_V2_PMNC, &pmnc); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> pmnc &= ~PPMU_PMNC_ENABLE_MASK; >>>>> - __raw_writel(pmnc, info->ppmu.base + PPMU_V2_PMNC); >>>>> + ret = regmap_write(info->regmap, PPMU_V2_PMNC, pmnc); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> >>>>> /* Read cycle count and performance count */ >>>>> - edata->total_count = __raw_readl(info->ppmu.base + PPMU_V2_CCNT); >>>>> + ret = regmap_read(info->regmap, PPMU_V2_CCNT, &total_count); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + edata->total_count = total_count; >>>>> >>>>> switch (id) { >>>>> case PPMU_PMNCNT0: >>>>> case PPMU_PMNCNT1: >>>>> case PPMU_PMNCNT2: >>>>> - load_count = __raw_readl(info->ppmu.base + PPMU_V2_PMNCT(id)); >>>>> + ret = regmap_read(info->regmap, PPMU_V2_PMNCT(id), &count); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + load_count = count; >>>>> break; >>>>> case PPMU_PMNCNT3: >>>>> - pmcnt_high = __raw_readl(info->ppmu.base + PPMU_V2_PMCNT3_HIGH); >>>>> - pmcnt_low = __raw_readl(info->ppmu.base + PPMU_V2_PMCNT3_LOW); >>>>> - load_count = ((u64)((pmcnt_high & 0xff)) << 32) >>>>> - + (u64)pmcnt_low; >>>>> + ret = regmap_read(info->regmap, PPMU_V2_PMCNT3_HIGH, >>>>> + &pmcnt_high); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> + ret = regmap_read(info->regmap, PPMU_V2_PMCNT3_LOW, &pmcnt_low); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> + load_count = ((u64)((pmcnt_high & 0xff)) << 32)+ (u64)pmcnt_low; >>>>> break; >>>>> } >>>>> edata->load_count = load_count; >>>>> >>>>> /* Disable all counters */ >>>>> - cntenc = __raw_readl(info->ppmu.base + PPMU_V2_CNTENC); >>>>> + ret = regmap_read(info->regmap, PPMU_V2_CNTENC, &cntenc); >>>>> + if (ret < 0) >>>>> + return 0; >>>>> + >>>>> cntenc |= (PPMU_CCNT_MASK | (PPMU_ENABLE << id)); >>>>> - __raw_writel(cntenc, info->ppmu.base + PPMU_V2_CNTENC); >>>>> + ret = regmap_write(info->regmap, PPMU_V2_CNTENC, cntenc); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> >>>>> dev_dbg(&edev->dev, "%25s (load: %ld / %ld)\n", edev->desc->name, >>>>> edata->load_count, edata->total_count); >>>>> @@ -411,10 +554,19 @@ static int of_get_devfreq_events(struct device_node *np, >>>>> return 0; >>>>> } >>>>> >>>>> -static int exynos_ppmu_parse_dt(struct exynos_ppmu *info) >>>>> +static struct regmap_config exynos_ppmu_regmap_config = { >>>>> + .reg_bits = 32, >>>>> + .val_bits = 32, >>>>> + .reg_stride = 4, >>>>> +}; >>>>> + >>>>> +static int exynos_ppmu_parse_dt(struct platform_device *pdev, >>>>> + struct exynos_ppmu *info) >>>>> { >>>>> struct device *dev = info->dev; >>>>> struct device_node *np = dev->of_node; >>>>> + struct resource *res; >>>>> + void __iomem *base; >>>>> int ret = 0; >>>>> >>>>> if (!np) { >>>>> @@ -423,10 +575,17 @@ static int exynos_ppmu_parse_dt(struct exynos_ppmu *info) >>>>> } >>>>> >>>>> /* Maps the memory mapped IO to control PPMU register */ >>>>> - info->ppmu.base = of_iomap(np, 0); >>>>> - if (IS_ERR_OR_NULL(info->ppmu.base)) { >>>>> - dev_err(dev, "failed to map memory region\n"); >>>>> - return -ENOMEM; >>>>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>>>> + base = devm_ioremap_resource(dev, res); >>>>> + if (IS_ERR(base)) >>>>> + return PTR_ERR(base); >>>>> + >>>>> + exynos_ppmu_regmap_config.max_register = resource_size(res) - 4; >>>>> + info->regmap = devm_regmap_init_mmio(dev, base, >>>>> + &exynos_ppmu_regmap_config); >>>>> + if (IS_ERR(info->regmap)) { >>>>> + dev_err(dev, "failed to initialize regmap\n"); >>>>> + return PTR_ERR(info->regmap); >>>>> } >>>>> >>>>> info->ppmu.clk = devm_clk_get(dev, "ppmu"); >>>>> @@ -438,15 +597,10 @@ static int exynos_ppmu_parse_dt(struct exynos_ppmu *info) >>>>> ret = of_get_devfreq_events(np, info); >>>>> if (ret < 0) { >>>>> dev_err(dev, "failed to parse exynos ppmu dt node\n"); >>>>> - goto err; >>>>> + return ret; >>>>> } >>>>> >>>>> return 0; >>>>> - >>>>> -err: >>>>> - iounmap(info->ppmu.base); >>>>> - >>>>> - return ret; >>>>> } >>>>> >>>>> static int exynos_ppmu_probe(struct platform_device *pdev) >>>>> @@ -463,7 +617,7 @@ static int exynos_ppmu_probe(struct platform_device *pdev) >>>>> info->dev = &pdev->dev; >>>>> >>>>> /* Parse dt data to get resource */ >>>>> - ret = exynos_ppmu_parse_dt(info); >>>>> + ret = exynos_ppmu_parse_dt(pdev, info); >>>>> if (ret < 0) { >>>>> dev_err(&pdev->dev, >>>>> "failed to parse devicetree for resource\n"); >>>>> @@ -476,8 +630,7 @@ static int exynos_ppmu_probe(struct platform_device *pdev) >>>>> if (!info->edev) { >>>>> dev_err(&pdev->dev, >>>>> "failed to allocate memory devfreq-event devices\n"); >>>>> - ret = -ENOMEM; >>>>> - goto err; >>>>> + return -ENOMEM; >>>>> } >>>>> edev = info->edev; >>>>> platform_set_drvdata(pdev, info); >>>>> @@ -488,17 +641,13 @@ static int exynos_ppmu_probe(struct platform_device *pdev) >>>>> ret = PTR_ERR(edev[i]); >>>>> dev_err(&pdev->dev, >>>>> "failed to add devfreq-event device\n"); >>>>> - goto err; >>>>> + return PTR_ERR(edev[i]); >>>>> } >>>>> } >>>>> >>>>> clk_prepare_enable(info->ppmu.clk); >>>>> >>>>> return 0; >>>>> -err: >>>>> - iounmap(info->ppmu.base); >>>>> - >>>>> - return ret; >>>>> } >>>>> >>>>> static int exynos_ppmu_remove(struct platform_device *pdev) >>>>> @@ -506,7 +655,6 @@ static int exynos_ppmu_remove(struct platform_device *pdev) >>>>> struct exynos_ppmu *info = platform_get_drvdata(pdev); >>>>> >>>>> clk_disable_unprepare(info->ppmu.clk); >>>>> - iounmap(info->ppmu.base); >>>>> >>>>> return 0; >>>>> } >>>>> >>>> >>>> -- >>>> To unsubscribe from this list: send the line "unsubscribe linux-pm" in >>>> the body of a message to majordomo@vger.kernel.org >>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>> >>>> >>>> >>> >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-pm" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> >> > > -- Regards, Chanwoo Choi