Re: [PATCH 7/8] firmware: vpd: remove platform driver

[Guenter Roeck]

On Tue, May 23, 2017 at 5:07 PM, Dmitry Torokhov
<dmitry.torokhov@xxxxxxxxx> wrote:
> There is no reason why VPD should register platform device and driver,
> given that we do not use their respective kobjects to attach attributes,
> nor do we need suspend/resume hooks, or any other features of device
> core.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>

One reason might be to use devm_ functions for memory allocations to
simplify error handling and cleanup. Without that, I agree.

Reviewed-by: Guenter Roeck <groeck@xxxxxxxxxxxx>

> ---
>  drivers/firmware/google/vpd.c | 44 ++++++++++++++++---------------------------
>  1 file changed, 16 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
> index 78945729388e..f4766bf6785a 100644
> --- a/drivers/firmware/google/vpd.c
> +++ b/drivers/firmware/google/vpd.c
> @@ -22,8 +22,6 @@
>  #include <linux/kobject.h>
>  #include <linux/list.h>
>  #include <linux/module.h>
> -#include <linux/of_address.h>
> -#include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/sysfs.h>
>
> @@ -279,47 +277,37 @@ static int vpd_sections_init(phys_addr_t physaddr)
>                 ret = vpd_section_init("rw", &rw_vpd,
>                                        physaddr + sizeof(struct vpd_cbmem) +
>                                        header.ro_size, header.rw_size);
> -               if (ret)
> +               if (ret) {
> +                       vpd_section_destroy(&ro_vpd);
>                         return ret;
> +               }
>         }
>
>         return 0;
>  }
>
> -static int vpd_probe(struct platform_device *pdev)
> -{
> -       int ret;
> -       struct lb_cbmem_ref entry;
> -
> -       ret = coreboot_table_find(CB_TAG_VPD, &entry, sizeof(entry));
> -       if (ret)
> -               return ret;
> -
> -       return vpd_sections_init(entry.cbmem_addr);
> -}
> -
> -static struct platform_driver vpd_driver = {
> -       .probe = vpd_probe,
> -       .driver = {
> -               .name = "vpd",
> -       },
> -};
> -
>  static int __init vpd_platform_init(void)
>  {
> -       struct platform_device *pdev;
> -
> -       pdev = platform_device_register_simple("vpd", -1, NULL, 0);
> -       if (IS_ERR(pdev))
> -               return PTR_ERR(pdev);
> +       struct lb_cbmem_ref entry;
> +       int err;
>
>         vpd_kobj = kobject_create_and_add("vpd", firmware_kobj);
>         if (!vpd_kobj)
>                 return -ENOMEM;
>
> -       platform_driver_register(&vpd_driver);
> +       err = coreboot_table_find(CB_TAG_VPD, &entry, sizeof(entry));
> +       if (err)
> +               goto err_kobject_put;
> +
> +       err = vpd_sections_init(entry.cbmem_addr);
> +       if (err)
> +               goto err_kobject_put;
>
>         return 0;
> +
> +err_kobject_put:
> +       kobject_put(vpd_kobj);
> +       return err;
>  }
>
>  static void __exit vpd_platform_exit(void)
> --
> 2.13.0.219.gdb65acc882-goog
>