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

From: Guenter Roeck
Date: Fri May 26 2017 - 21:17:58 EST


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