On 12/23/2014 12:40 PM, Varka Bhadram wrote:
> On Tue, Dec 23, 2014 at 8:48 AM, Chanwoo Choi <cw00.choi@xxxxxxxxxxx> wrote:
>> This patch adds exynos-ppmu devfreq-event driver to get performance data
>> of each IP for Samsung Exynos SoC. These event from Exynos PPMU provide
>> useful information about the behavior of the SoC that you can use when
>> analyzing system performance, and made visible and can be counted using
>> logic in each IP.
>>
>> This patch is based on existing drivers/devfreq/exynos/exynos-ppmu.c
>>
>> Cc: MyungJoo Ham <myungjoo.ham@xxxxxxxxxxx>
>> Cc: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
>> Signed-off-by: Chanwoo Choi <cw00.choi@xxxxxxxxxxx>
>
> (...)
>> +static int exynos_ppmu_parse_dt(struct exynos_ppmu_data *data)
>> +{
>> + struct device *dev = data->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 */
>> + data->ppmu.base = of_iomap(np, 0);
>> + if (IS_ERR_OR_NULL(data->ppmu.base)) {
>> + dev_err(dev, "failed to map memory region\n");
>> + return -EINVAL;
> -ENOMEM or -ENXIO is the proper error return value..?
OK. I'll return -ENOMEM value.
>
>> + }
>> +
>> + data->clk_ppmu = devm_clk_get(dev, "ppmu");
>> + if (IS_ERR(data->clk_ppmu)) {
>> + data->clk_ppmu = NULL;
>> + dev_warn(dev, "failed to get PPMU clock\n");
>> + }
> If PPMU clk get fails..? return PTR_ERR(data->clk_ppmu)
If Exynos SoC don't register the clock to clock tree, the clock remains on state.
So, I print just warning message.
But, It is not proper. I'll fix it.
>> +
>> + ret = of_get_devfreq_events(np, data);
>> + if (ret < 0) {
>> + dev_err(dev, "failed to parse exynos ppmu dt node\n");
>> + goto err;
>> + }
>> +
>> + return 0;
>> +
>> +err:
>> + iounmap(data->ppmu.base);
>> +
>> + return ret;
>> +}
>> +
>> +static int exynos_ppmu_probe(struct platform_device *pdev)
>> +{
>> + struct exynos_ppmu_data *data;
>> + struct devfreq_event_dev **edev;
>> + struct devfreq_event_desc *desc;
>> + int i, ret = 0, size;
>> +
>> + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
>> + if (!data)
>> + return -ENOMEM;
>> +
>> + mutex_init(&data->lock);
>> + data->dev = &pdev->dev;
>> +
>> + /* Parse dt data to get resource */
>> + ret = exynos_ppmu_parse_dt(data);
>> + if (ret < 0) {
>> + dev_err(&pdev->dev,
>> + "failed to parse devicetree for resource\n");
>> + return ret;
>> + }
>> + desc = data->desc;
>> +
>> + size = sizeof(struct devfreq_event_dev *) * data->num_events;
>> + data->edev = devm_kzalloc(&pdev->dev, size, GFP_KERNEL);
>> + if (!data->edev) {
>> + dev_err(&pdev->dev,
>> + "failed to allocate memory devfreq-event devices\n");
>> + return -ENOMEM;
>> + }
>> + edev = data->edev;
>> + platform_set_drvdata(pdev, data);
>> +
>> + for (i = 0; i < data->num_events; i++) {
>> + edev[i] = devfreq_event_add_edev(&pdev->dev, &desc[i]);
>> + if (IS_ERR(edev)) {
>> + ret = PTR_ERR(edev);
>> + dev_err(&pdev->dev,
>> + "failed to add devfreq-event device\n");
>> + goto err;
>> + }
>> + }
>> +
>> + clk_prepare_enable(data->clk_ppmu);
>> +
>> + return 0;
>> +err:
>> + iounmap(data->ppmu.base);
>> +
>> + return ret;
>> +}
>> +
>> +static int exynos_ppmu_remove(struct platform_device *pdev)
>> +{
>> + struct exynos_ppmu_data *data = platform_get_drvdata(pdev);
>> + int i, ret = 0;
>> +
>> + for (i = 0; i < data->num_events; i++) {
>> + ret = devfreq_event_remove_edev(data->edev[i]);
>> + if (ret < 0) {
>> + dev_err(&pdev->dev,
>> + "failed to remove devfreq-event device\n");
>> + goto err;
>> + }
>> + }
>> +
>> +err:
>> + clk_disable_unprepare(data->clk_ppmu);
>> + iounmap(data->ppmu.base);
>> +
>> + return ret;
>> +}
>> +
>> +static struct of_device_id exynos_ppmu_id_match[] = {
>> + { .compatible = "samsung,exynos-ppmu", },
>> + { /* sentinel */ },
>> +};
>> +
>> +static struct platform_driver exynos_ppmu_driver = {
>> + .probe = exynos_ppmu_probe,
>> + .remove = exynos_ppmu_remove,
>> + .driver = {
>> + .name = "exynos-ppmu",
>> + .owner = THIS_MODULE,
> remove owner field. It will be populate by driver core.
OK. I'll remove it.
Thanks for your review.
Best Regards,
Chanwoo Choi
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
