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

[Andy Shevchenko]

On Thu, Jan 24, 2019 at 10:17 AM Peng Hao <peng.hao2@xxxxxxxxxx> wrote:
>
> Add pvpanic_add/remove_device API. Follow-up patches will use them to
> add/remove specific drivers into framework.

I'm not sure this is the best way to instantiate the drivers.

Code with platfrom_device_add*() is related to platform which
instantiates the necessary set of the drivers.
In case of OF it should be done in Device Tree, in ACPI case you would
have some device description for that in DSDT table.

> +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;

It should return proper %-ERRNO code. I think to achieve this the
following can be used

return ret;

> +}

>  static int pvpanic_platform_probe (struct platform_device *pdev)
>  {
>         struct device *dev = &pdev->dev;
>         struct resource *res;
> +       void __iomem *base;
>
>         res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>         if (res) {
> @@ -59,8 +96,10 @@ static int pvpanic_platform_probe (struct platform_device *pdev)
>                 base = ioport_map(res->start, resource_size(res));
>                 if (!base)
>                         return -ENODEV;

> +               pvpanic_data.is_ioport = true;

You better allocate this on a heap and put as a pointer to the device
driver private data.

>          }
>
> +       pvpanic_data.base = base;

Ditto for entire pvpanic_data instance.

-- 
With Best Regards,
Andy Shevchenko