On Thu, Jan 24, 2019 at 10:17 AM Peng Hao <peng.h...@zte.com.cn> 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