Re: [RFC PATCHv2 4/7] devfreq: event: Add exynos-ppmu devfreq event driver

[Krzysztof Kozlowski <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>]

On wto, 2014-12-09 at 23:13 +0900, Chanwoo Choi wrote:
> This patch add exynos-ppmu devfreq event driver to provider raw data about
> the utilization of each IP in Exynos SoC series.
> 
> Cc: MyungJoo Ham <myungjoo.ham-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Cc: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Chanwoo Choi <cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/devfreq/Kconfig             |   9 +
>  drivers/devfreq/event/Makefile      |   1 +
>  drivers/devfreq/event/exynos-ppmu.c | 409 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 419 insertions(+)
>  create mode 100644 drivers/devfreq/event/exynos-ppmu.c

I would rather see this as an incremental change for existing
exynos_ppmu.c file. However I do not insists on that.

> 
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> index 4d15b62..d4559f7 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -89,4 +89,13 @@ config ARM_EXYNOS5_BUS_DEVFREQ
>  
>  comment "DEVFREQ Event Drivers"
>  
> +config DEVFREQ_EVENT_EXYNOS_PPMU
> +	bool "EXYNOS PPMU (Performance Profiling Monitoring Unit) DEVFREQ event Driver"
> +	depends on ARCH_EXYNOS
> +	select PM_OPP
> +	help
> +	 This add the DEVFREQ event driver for Exynos SoC. It provides PPMU
> +	 (Performance Profiling Monitoring Unit) counters to estimate the
> +	 utilization of each module.
> +
>  endif # PM_DEVFREQ
> diff --git a/drivers/devfreq/event/Makefile b/drivers/devfreq/event/Makefile
> index dc56005..be146ea 100644
> --- a/drivers/devfreq/event/Makefile
> +++ b/drivers/devfreq/event/Makefile
> @@ -1 +1,2 @@
>  # Exynos DEVFREQ Event Drivers
> +obj-$(CONFIG_DEVFREQ_EVENT_EXYNOS_PPMU) += exynos-ppmu.o
> diff --git a/drivers/devfreq/event/exynos-ppmu.c b/drivers/devfreq/event/exynos-ppmu.c
> new file mode 100644
> index 0000000..2706f23
> --- /dev/null
> +++ b/drivers/devfreq/event/exynos-ppmu.c
> @@ -0,0 +1,409 @@
> +/*
> + * exynos_ppmu.c - EXYNOS PPMU (Performance Profiling Monitoring Units) support
> + *
> + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
> + * Author : Chanwoo Choi <cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This driver is based on drivers/devfreq/exynos/exynos_ppmu.c
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/suspend.h>
> +#include <linux/devfreq.h>
> +
> +#define PPMU_ENABLE             BIT(0)
> +#define PPMU_DISABLE            0x0
> +#define PPMU_CYCLE_RESET        BIT(1)
> +#define PPMU_COUNTER_RESET      BIT(2)
> +
> +#define PPMU_ENABLE_COUNT0      BIT(0)
> +#define PPMU_ENABLE_COUNT1      BIT(1)
> +#define PPMU_ENABLE_COUNT2      BIT(2)
> +#define PPMU_ENABLE_COUNT3      BIT(3)
> +#define PPMU_ENABLE_CYCLE       BIT(31)
> +
> +#define PPMU_CNTENS		0x10
> +#define PPMU_FLAG		0x50
> +#define PPMU_CCNT_OVERFLOW	BIT(31)
> +#define PPMU_CCNT		0x100
> +
> +#define PPMU_PMCNT0		0x110
> +#define PPMU_PMCNT_OFFSET	0x10
> +#define PMCNT_OFFSET(x)		(PPMU_PMCNT0 + (PPMU_PMCNT_OFFSET * x))
> +
> +#define PPMU_BEVT0SEL		0x1000
> +#define PPMU_BEVTSEL_OFFSET	0x100
> +#define PPMU_BEVTSEL(x)		(PPMU_BEVT0SEL + (x * PPMU_BEVTSEL_OFFSET))
> +
> +#define RD_DATA_COUNT		0x5
> +#define WR_DATA_COUNT		0x6
> +#define RDWR_DATA_COUNT		0x7
> +
> +enum ppmu_counter {
> +	PPMU_PMNCNT0,
> +	PPMU_PMNCNT1,
> +	PPMU_PMNCNT2,
> +	PPMU_PMNCNT3,
> +	PPMU_PMNCNT_MAX,
> +};
> +
> +/* Platform data */
> +struct exynos_ppmu_data {
> +	struct devfreq *devfreq;

Looks like not used here.

> +	struct devfreq_event_dev **event_dev;
> +	struct devfreq_event_desc *desc;
> +	unsigned int num_events;
> +
> +	struct device *dev;
> +	struct clk *clk_ppmu;
> +	struct mutex lock;
> +
> +	struct __exynos_ppmu {
> +		void __iomem *hw_base;
> +		unsigned int ccnt;
> +		unsigned int event[PPMU_PMNCNT_MAX];
> +		unsigned int count[PPMU_PMNCNT_MAX];
> +		unsigned long long ns;
> +		ktime_t reset_time;
> +		bool ccnt_overflow;
> +		bool count_overflow[PPMU_PMNCNT_MAX];
> +	} ppmu;
> +};
> +
> +struct __exynos_ppmu_events {
> +	char *name;
> +	int id;
> +} ppmu_events[] = {
> +	{ "ppmu-dmc0-pmcnt0", PPMU_PMNCNT0 },
> +	{ "ppmu-dmc0-pmcnt1", PPMU_PMNCNT1 },
> +	{ "ppmu-dmc0-pmcnt2", PPMU_PMNCNT2 },
> +	{ "ppmu-dmc0-pmcnt3", PPMU_PMNCNT3 },
> +
> +	{ "ppmu-dmc1-pmcnt0", PPMU_PMNCNT0 },
> +	{ "ppmu-dmc1-pmcnt1", PPMU_PMNCNT1 },
> +	{ "ppmu-dmc1-pmcnt2", PPMU_PMNCNT2 },
> +	{ "ppmu-dmc1-pmcnt3", PPMU_PMNCNT3 },
> +
> +	{ "ppmu-cpu-pmcnt0", PPMU_PMNCNT0 },
> +	{ "ppmu-cpu-pmcnt1", PPMU_PMNCNT1 },
> +	{ "ppmu-cpu-pmcnt2", PPMU_PMNCNT2 },
> +	{ "ppmu-cpu-pmcnt3", PPMU_PMNCNT3 },
> +
> +	{ "ppmu-rightbus-pmcnt0", PPMU_PMNCNT0 },
> +	{ "ppmu-rightbus-pmcnt1", PPMU_PMNCNT1 },
> +	{ "ppmu-rightbus-pmcnt2", PPMU_PMNCNT2 },
> +	{ "ppmu-rightbus-pmcnt3", PPMU_PMNCNT3 },
> +
> +	{ "ppmu-leftbus-pmcnt0", PPMU_PMNCNT0 },
> +	{ "ppmu-leftbus-pmcnt1", PPMU_PMNCNT1 },
> +	{ "ppmu-leftbus-pmcnt2", PPMU_PMNCNT2 },
> +	{ "ppmu-leftbus-pmcnt3", PPMU_PMNCNT3 },
> +
> +	{ "ppmu-camif-pmcnt0", PPMU_PMNCNT0 },
> +	{ "ppmu-camif-pmcnt1", PPMU_PMNCNT1 },
> +	{ "ppmu-camif-pmcnt2", PPMU_PMNCNT2 },
> +	{ "ppmu-camif-pmcnt3", PPMU_PMNCNT3 },
> +
> +	{ "ppmu-lcd0-pmcnt0", PPMU_PMNCNT0 },
> +	{ "ppmu-lcd0-pmcnt1", PPMU_PMNCNT1 },
> +	{ "ppmu-lcd0-pmcnt2", PPMU_PMNCNT2 },
> +	{ "ppmu-lcd0-pmcnt3", PPMU_PMNCNT3 },
> +
> +	{ "ppmu-g3d-pmcnt0", PPMU_PMNCNT0 },
> +	{ "ppmu-g3d-pmcnt1", PPMU_PMNCNT1 },
> +	{ "ppmu-g3d-pmcnt2", PPMU_PMNCNT2 },
> +	{ "ppmu-g3d-pmcnt3", PPMU_PMNCNT3 },
> +
> +	{ "ppmu-mfc-pmcnt0", PPMU_PMNCNT0 },
> +	{ "ppmu-mfc-pmcnt1", PPMU_PMNCNT1 },
> +	{ "ppmu-mfc-pmcnt2", PPMU_PMNCNT2 },
> +	{ "ppmu-mfc-pmcnt3", PPMU_PMNCNT3 },
> +
> +	{ "ppmu-fsys-pmcnt0", PPMU_PMNCNT0 },
> +	{ "ppmu-fsys-pmcnt1", PPMU_PMNCNT1 },
> +	{ "ppmu-fsys-pmcnt2", PPMU_PMNCNT2 },
> +	{ "ppmu-fsys-pmcnt3", PPMU_PMNCNT3 },
> +	{ /* sentinel */ },
> +};
> +
> +static int exynos_ppmu_enable(struct devfreq_event_dev *event_dev)
> +{
> +	struct exynos_ppmu_data *exynos_ppmu = event_dev_get_drvdata(event_dev);
> +
> +	__raw_writel(PPMU_ENABLE, exynos_ppmu->ppmu.hw_base);
> +
> +	return 0;
> +}
> +
> +static int exynos_ppmu_disable(struct devfreq_event_dev *event_dev)
> +{
> +	struct exynos_ppmu_data *exynos_ppmu = event_dev_get_drvdata(event_dev);
> +
> +	__raw_writel(PPMU_DISABLE, exynos_ppmu->ppmu.hw_base);
> +
> +	return 0;
> +}
> +
> +static bool exynos_ppmu_is_enabled(struct devfreq_event_dev *event_dev)
> +{
> +	struct exynos_ppmu_data *exynos_ppmu = event_dev_get_drvdata(event_dev);
> +	int val;
> +
> +	val = __raw_readl(exynos_ppmu->ppmu.hw_base);
> +	if (val & PPMU_ENABLE)
> +		return true;
> +
> +	return false;
> +}
> +
> +static int exynos_ppmu_find_ppmu_id(struct devfreq_event_dev *event_dev)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(ppmu_events); i++)
> +		if (!strcmp(event_dev->desc->name, ppmu_events[i].name))
> +			return ppmu_events[i].id;
> +
> +	return -EINVAL;
> +}
> +
> +static int exynos_ppmu_set_event(struct devfreq_event_dev *event_dev,
> +				enum devfreq_event_type event_type)
> +{
> +	struct exynos_ppmu_data *exynos_ppmu = event_dev_get_drvdata(event_dev);
> +	void __iomem *ppmu_base = exynos_ppmu->ppmu.hw_base;
> +	int id = exynos_ppmu_find_ppmu_id(event_dev);
> +
> +	__raw_writel(RDWR_DATA_COUNT, ppmu_base + PPMU_BEVTSEL(id));
> +
> +	return 0;
> +}
> +
> +static int exynos_ppmu_get_event(struct devfreq_event_dev *event_dev,
> +				enum devfreq_event_type event_type,
> +				int *total_event)
> +{
> +	struct exynos_ppmu_data *exynos_ppmu = event_dev_get_drvdata(event_dev);
> +	void __iomem *ppmu_base = exynos_ppmu->ppmu.hw_base;
> +	int id = exynos_ppmu_find_ppmu_id(event_dev);
> +	int cnt, total_cnt;
> +
> +	total_cnt = __raw_readl(ppmu_base + PPMU_CCNT);
> +
> +	if (id == PPMU_PMNCNT3)
> +		cnt = ((__raw_readl(ppmu_base + PMCNT_OFFSET(id)) << 8) |
> +			  __raw_readl(ppmu_base + PMCNT_OFFSET(id + 1)));
> +	else
> +		cnt = __raw_readl(ppmu_base + PMCNT_OFFSET(id));
> +
> +	*total_event = total_cnt;
> +
> +	return cnt;
> +}
> +
> +static int exynos_ppmu_reset(struct devfreq_event_dev *event_dev)
> +{
> +	struct exynos_ppmu_data *exynos_ppmu = event_dev_get_drvdata(event_dev);
> +	void __iomem *ppmu_base = exynos_ppmu->ppmu.hw_base;
> +
> +	__raw_writel(PPMU_CYCLE_RESET | PPMU_COUNTER_RESET, ppmu_base);
> +	__raw_writel(PPMU_ENABLE_CYCLE  |
> +		     PPMU_ENABLE_COUNT0 |
> +		     PPMU_ENABLE_COUNT1 |
> +		     PPMU_ENABLE_COUNT2 |
> +		     PPMU_ENABLE_COUNT3,
> +		     ppmu_base + PPMU_CNTENS);
> +	__raw_writel(PPMU_ENABLE, exynos_ppmu->ppmu.hw_base);
> +
> +	return 0;
> +}
> +
> +static struct devfreq_event_ops exynos_ppmu_ops = {
> +	.enable		= exynos_ppmu_enable,
> +	.disable	= exynos_ppmu_disable,
> +	.is_enabled	= exynos_ppmu_is_enabled,
> +	.set_event	= exynos_ppmu_set_event,
> +	.get_event	= exynos_ppmu_get_event,
> +	.reset		= exynos_ppmu_reset,
> +};
> +
> +static int of_get_devfreq_events(struct device_node *np,
> +				struct exynos_ppmu_data *exynos_ppmu)
> +{
> +	struct devfreq_event_desc *desc;
> +	struct device *dev = exynos_ppmu->dev;
> +	struct device_node *events_np, *node;
> +	int i, j, count;
> +
> +	events_np = of_find_node_by_name(np, "events");

of_get_child_by_name()

> +	if (!events_np) {
> +		dev_err(dev, "Failed to find ppmus sub-node\n");
> +		return -EINVAL;
> +	}
> +
> +	count = of_get_child_count(events_np);
> +	desc = devm_kzalloc(dev, sizeof(struct devfreq_event_desc) * count,
> +			      GFP_KERNEL);
> +	if (!desc)
> +		return -ENOMEM;
> +	exynos_ppmu->num_events = count;
> +
> +	j = 0;
> +	for_each_child_of_node(events_np, node) {
> +		for (i = 0; i < ARRAY_SIZE(ppmu_events); i++) {
> +			if (!of_node_cmp(node->name, ppmu_events[i].name))
> +				break;
> +		}
> +
> +		if (i == ARRAY_SIZE(ppmu_events)) {
> +			dev_warn(dev,
> +				"don't know how to configure events : %s\n",
> +				node->name);
> +			continue;
> +		}
> +		desc[j].ops = &exynos_ppmu_ops;
> +		desc[j].driver_data = exynos_ppmu;
> +		desc[j].event_type |= DEVFREQ_EVENT_TYPE_RAW_DATA;
> +		of_property_read_string(node, "event-name", &desc[j].name);
> +		j++;
> +	}
> +	exynos_ppmu->desc = desc;

of_node_put() for 'events_np' and 'node' in loop.

> +
> +	return 0;
> +}
> +
> +static int exynos_ppmu_parse_dt(struct exynos_ppmu_data *exynos_ppmu)
> +{
> +	struct device *dev = exynos_ppmu->dev;
> +	struct device_node *np = dev->of_node;
> +	int ret = 0;
> +
> +	if (!np) {
> +		dev_err(dev, "Failed to find devicetree node\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Maps the memory mapped IO to control PPMU register */
> +	exynos_ppmu->ppmu.hw_base = of_iomap(np, 0);
> +	if (IS_ERR_OR_NULL(exynos_ppmu->ppmu.hw_base)) {
> +		dev_err(dev, "Failed to map memory region\n");
> +		ret = -EINVAL;
> +		goto err_iomap;
> +	}
> +
> +	/* FIXME: Get the clock of ppmu and enable this clock */
> +	exynos_ppmu->clk_ppmu = devm_clk_get(dev, "ppmu");
> +	if (IS_ERR(exynos_ppmu->clk_ppmu))
> +		dev_warn(dev, "Failed to get PPMU clock\n");
> +
> +	ret = of_get_devfreq_events(np, exynos_ppmu);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to parse exynos ppmu dt node\n");
> +		goto err_clock;
> +	}
> +
> +	return 0;
> +
> +err_clock:
> +	clk_disable_unprepare(exynos_ppmu->clk_ppmu);

Not needed. Clock is not enabled here.

> +err_iomap:
> +	iounmap(exynos_ppmu->ppmu.hw_base);
> +
> +	return ret;
> +}
> +
> +static int exynos_ppmu_probe(struct platform_device *pdev)
> +{
> +	struct exynos_ppmu_data *exynos_ppmu;
> +	struct devfreq_event_dev **event_dev;
> +	struct devfreq_event_desc *desc;
> +	int i, ret = 0, size;
> +
> +	/* Allocate the memory of exynos_ppmu_data and initialize it */
> +	exynos_ppmu = devm_kzalloc(&pdev->dev, sizeof(struct exynos_ppmu_data),
> +				   GFP_KERNEL);
> +	if (!exynos_ppmu)
> +		return -ENOMEM;
> +
> +	mutex_init(&exynos_ppmu->lock);
> +	exynos_ppmu->dev = &pdev->dev;
> +
> +	/* Parse dt data to get resource */
> +	ret = exynos_ppmu_parse_dt(exynos_ppmu);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "Failed to parse DT node for resource\n");
> +		return ret;
> +	}
> +	desc = exynos_ppmu->desc;
> +
> +	size = sizeof(struct devfreq_event_dev *) * exynos_ppmu->num_events;
> +	exynos_ppmu->event_dev = devm_kzalloc(&pdev->dev, size, GFP_KERNEL);
> +	if (!exynos_ppmu->event_dev) {
> +		dev_err(&pdev->dev,
> +			"Failed to allocate memory devfreq_event_dev list\n");
> +		return -ENOMEM;
> +	}
> +	event_dev = exynos_ppmu->event_dev;
> +	platform_set_drvdata(pdev, exynos_ppmu);

Missing clk_prepare_enable().

> +
> +	for (i = 0; i < exynos_ppmu->num_events; i++) {
> +		event_dev[i] = devfreq_add_event_device(&pdev->dev, &desc[i]);
> +		if (IS_ERR(event_dev)) {
> +			ret = PTR_ERR(event_dev);
> +			dev_err(&pdev->dev, "Failed to add devfreq evt dev\n");
> +			goto err;
> +		}
> +	}
> +
> +	return 0;
> +err:
> +	clk_disable_unprepare(exynos_ppmu->clk_ppmu);
> +	iounmap(exynos_ppmu->ppmu.hw_base);
> +
> +	return ret;
> +}
> +
> +static int exynos_ppmu_remove(struct platform_device *pdev)
> +{
> +	struct exynos_ppmu_data *exynos_ppmu = platform_get_drvdata(pdev);
> +
> +	clk_disable_unprepare(exynos_ppmu->clk_ppmu);
> +	iounmap(exynos_ppmu->ppmu.hw_base);
> +
> +	/* Remove devfreq instance */
> +	devfreq_remove_device(exynos_ppmu->devfreq);

Shouldn't this be devfreq_remove_event_dev()?

Best regards,
Krzysztof


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html