Re: [PATCH] Revert "firmware: vpd: remove platform driver"

[Guenter Roeck]

On Fri, May 26, 2017 at 1:57 PM, Dmitry Torokhov
<dmitry.torokhov@xxxxxxxxx> wrote:
> This reverts commit 7975bd4cca05a99aa14964cfa22366ee64da50ad, because
> VPD relies on driver core to handle deferrals returned by
> coreboot_table_find().
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>

For process' sake:

Reviewed-by: Guenter Roeck <groeck@xxxxxxxxxxxx>

> ---
>
> Greg, here is the revert we discussed.
>
> I will look into whether we could make coreboot table not use deferrals,
> but some mother mechanism to notify users that the data is available.
> The entities we are dealing here with are not really hardware devices
> and I consider using platform devices/drivers for them misuse of the
> driver model, but for now it needs to stay as it was.
>
>  drivers/firmware/google/vpd.c | 44 +++++++++++++++++++++++++++----------------
>  1 file changed, 28 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
> index d28f62fed50f..4f8f99edbbfa 100644
> --- a/drivers/firmware/google/vpd.c
> +++ b/drivers/firmware/google/vpd.c
> @@ -22,6 +22,8 @@
>  #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>
>
> @@ -277,37 +279,47 @@ 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) {
> -                       vpd_section_destroy(&ro_vpd);
> +               if (ret)
>                         return ret;
> -               }
>         }
>
>         return 0;
>  }
>
> -static int __init vpd_platform_init(void)
> +static int vpd_probe(struct platform_device *pdev)
>  {
> +       int ret;
>         struct lb_cbmem_ref entry;
> -       int err;
> +
> +       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);
>
>         vpd_kobj = kobject_create_and_add("vpd", firmware_kobj);
>         if (!vpd_kobj)
>                 return -ENOMEM;
>
> -       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;
> +       platform_driver_register(&vpd_driver);
>
>         return 0;
> -
> -err_kobject_put:
> -       kobject_put(vpd_kobj);
> -       return err;
>  }
>
>  static void __exit vpd_platform_exit(void)
> --
> 2.13.0.219.gdb65acc882-goog
>
>
> --
> Dmitry