Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <cover.1559614824.git.han_mao@c-sky.com> <40e0b27c342458360f9f30ef16026cb63e792850.1559614824.git.han_mao@c-sky.com>
In-Reply-To: <40e0b27c342458360f9f30ef16026cb63e792850.1559614824.git.han_mao@c-sky.com>
From:   Guo Ren <guoren@kernel.org>
Date:   Tue, 4 Jun 2019 13:50:03 +0800
Message-ID: <CAJF2gTTV5byJkCU4Z-NMvD+W8K7J56mmYNaXdchb+z-rbQm+1w@mail.gmail.com>
Subject: Re: [PATCH V4 1/6] csky: Init pmu as a device
To:     Mao Han <han_mao@c-sky.com>
Cc:     linux-kernel@vger.kernel.org, linux-csky@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hello Mao,

On Tue, Jun 4, 2019 at 10:25 AM Mao Han <han_mao@c-sky.com> wrote:
>
> This patch change the csky pmu initialization from arch init to
> device init. The pmu can be configued with information from
> device tree(pmu device name, irq number and etc.).
>
> Signed-off-by: Mao Han <han_mao@c-sky.com>
> Cc: Guo Ren <guoren@kernel.org>
> ---
>  arch/csky/kernel/perf_event.c | 58 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 57 insertions(+), 1 deletion(-)
>
> diff --git a/arch/csky/kernel/perf_event.c b/arch/csky/kernel/perf_event.c
> index 376c972..c022acc 100644
> --- a/arch/csky/kernel/perf_event.c
> +++ b/arch/csky/kernel/perf_event.c
> @@ -21,6 +21,8 @@ struct csky_pmu_t {
>         uint32_t        hpcr;
>  } csky_pmu;
>
> +typedef int (*csky_pmu_init)(struct csky_pmu_t *);
Is the type of csky_pmu_init() the same with init_hw_perf_events ?

And I also think you should remove the hook style, because there
is only one init for the driver.

> +
>  #define cprgr(reg)                             \
>  ({                                             \
>         unsigned int tmp;                       \
> @@ -1028,4 +1030,58 @@ int __init init_hw_perf_events(void)
>
>         return perf_pmu_register(&csky_pmu.pmu, "cpu", PERF_TYPE_RAW);
>  }
> -arch_initcall(init_hw_perf_events);
> +
> +int csky_pmu_device_probe(struct platform_device *pdev,
> +                         const struct of_device_id *of_table)
> +{
> +       const struct of_device_id *of_id;
> +       csky_pmu_init init_fn;
> +       struct device_node *node = pdev->dev.of_node;
> +       int ret = -ENODEV;
> +

> +       of_id = of_match_node(of_table, pdev->dev.of_node);
> +       if (node && of_id) {
> +               init_fn = of_id->data;
> +               ret = init_fn(&csky_pmu);
> +       }
Ditto, all 7 lines above should be removed and use directly like:
            ret = init_hw_perf_events();

> +       if (ret) {
> +               pr_notice("[perf] failed to probe PMU!\n");
> +               return ret;
> +       }
> +
> +       return ret;
> +}
> +
> +const static struct of_device_id csky_pmu_of_device_ids[] = {
> +       {.compatible = "csky,csky-pmu", .data = init_hw_perf_events},
Ditto, Nothing for .data.

Best Regards
 Guo Ren