Re: [PATCH 3/6] misc/pvpanic: add API for pvpanic driver framework

Greg KH
2019-01-22T09:39:03Z

On Tue, Jan 22, 2019 at 03:25:08AM +0800, Peng Hao wrote:
> Add pvpanic_add/remove_device API.

That says what you are doing, but not _why_ you need to do this.

Why?

>
> Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
> ---
> drivers/misc/pvpanic/pvpanic.c | 32 ++++++++++++++++++++++++++++++++
> drivers/misc/pvpanic/pvpanic.h | 14 ++++++++++++++
> 2 files changed, 46 insertions(+)
> create mode 100644 drivers/misc/pvpanic/pvpanic.h
>
> diff --git a/drivers/misc/pvpanic/pvpanic.c b/drivers/misc/pvpanic/pvpanic.c
> index 227ab4e..f842ee4 100644
> --- a/drivers/misc/pvpanic/pvpanic.c
> +++ b/drivers/misc/pvpanic/pvpanic.c
> @@ -48,6 +48,38 @@
> .priority = 1, /* let this called before broken drm_fb_helper */
> };
>
> +int pvpanic_add_device(struct device *dev, struct resource *res)
> +{
> + struct platform_device *pdev;
> + int ret;
> +
> + pdev = platform_device_alloc("pvpanic", -1);
> + if (!pdev)
> + return -ENOMEM;
> +
> + pdev->dev.parent = dev;
> +
> + ret = platform_device_add_resources(pdev, res, 1);
> + if (ret)
> + goto err;
> +
> + ret = platform_device_add(pdev);
> + if (ret)
> + goto err;
> + pvpanic_data.pdev = pdev;
> +
> + return 0;
> +err:
> + platform_device_put(pdev);
> + return -1;
> +}
> +
> +void pvpanic_remove_device(void)
> +{
> + platform_device_unregister(pvpanic_data.pdev);
> + pvpanic_data.pdev = NULL;
> +}
> +
> static int pvpanic_platform_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> diff --git a/drivers/misc/pvpanic/pvpanic.h b/drivers/misc/pvpanic/pvpanic.h
> new file mode 100644
> index 0000000..a72ca59
> --- /dev/null
> +++ b/drivers/misc/pvpanic/pvpanic.h
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later

Do you really mean or later? I have to ask, sorry.

> +/* pvpanic driver framework header file
> + *
> + * Copyright (C) 2019 ZTE Ltd.
> + * Author: Peng Hao <peng.hao2@zte.com.cn>
> + */
> +
> +#ifndef __DRIVERS_MISC_PVPANIC_H
> +#define __DRIVERS_MISC_PVPANIC_H
> +
> +extern int pvpanic_add_device(struct device *dev, struct resource *res);
> +extern void pvpanic_remove_device(void);

No need for "extern", and why have these functions here if no one is
using them?

thanks,

greg k-h