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

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

[Dmitry Torokhov]

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@gmail.com>
---

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

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

[Guenter Roeck]

On Fri, May 26, 2017 at 1:57 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> 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@gmail.com>

For process' sake:

Reviewed-by: Guenter Roeck <groeck@chromium.org>

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

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

[Greg Kroah-Hartman]

On Fri, May 26, 2017 at 01:57:49PM -0700, Dmitry Torokhov 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@gmail.com>
> Reviewed-by: Guenter Roeck <groeck@chromium.org>
> ---
> 
> 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.

If they aren't platform devices, then why just make them "virtual"
devices, using a simple device_create() call with no parent?

thanks,

greg k-h