Hey Chanwoo, Chanwoo Choi wrote: > 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. Again, what error do you want to handle here? For which reason could __raw_{writel,readl}() fail here, to make it necessary to go through regmap for this? With best wishes, Tobias >>> 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 <kgene@xxxxxxxxxx> >>>>>> Cc: Krzysztof Kozlowski <krzk@xxxxxxxxxx> >>>>>> Cc: Javier Martinez Canillas <javier@xxxxxxxxxxxxxxx> >>>>>> Cc: linux-samsung-soc@xxxxxxxxxxxxxxx >>>>>> Signed-off-by: Chanwoo Choi <cw00.choi@xxxxxxxxxxx> >>>>>> --- >>>>>> 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 <linux/module.h> >>>>>> #include <linux/of_address.h> >>>>>> #include <linux/platform_device.h> >>>>>> +#include <linux/regmap.h> >>>>>> #include <linux/suspend.h> >>>>>> #include <linux/devfreq-event.h> >>>>>> >>>>>> #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@xxxxxxxxxxxxxxx >>>>> 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@xxxxxxxxxxxxxxx >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> >>> >>> >> >> > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html