Re: [PATCH 2/7] PM / devfreq: exynos-ppmu: Use the regmap interface to handle the registers

[Tobias Jakobi <tjakobi@math.uni-bielefeld.de>]

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.

Also when exactly did __raw_writel() and friends become legacy?

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@kernel.org>
> Cc: Krzysztof Kozlowski <krzk@kernel.org>
> Cc: Javier Martinez Canillas <javier@osg.samsung.com>
> Cc: linux-samsung-soc@vger.kernel.org
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
>  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;
>  }
>