Messages in this thread | | | Date | Wed, 24 May 2017 19:38:33 -0700 | From | Dmitry Torokhov <> | Subject | Re: [PATCH 7/8] firmware: vpd: remove platform driver |
| |
On Wed, May 24, 2017 at 05:04:41PM -0700, Julius Werner wrote: > I'm not a kernel expert so maybe I don't understand this right, but... > I think this might have been done this way to ensure that the driver > can get initialized correctly regardless of probe ordering. > coreboot_table_find() may fail with -EPROBE_DEFER if the > coreboot_table driver and its dependent (coreboot_table-acpi or > coreboot_table-of) haven't been probed yet. In that case we want the > VPD driver to wait and try again later, after they've been probed. I > believe with the way this used to be written the driver core did that > automatically for us, but with your change I'm not sure if it still > does.
Hmm, I missed that. OK, I think we have a maze of platform drivers there, I'll see what can be done.
> > On Wed, May 24, 2017 at 10:22 AM, Guenter Roeck <groeck@google.com> wrote: > > On Tue, May 23, 2017 at 5:07 PM, Dmitry Torokhov > > <dmitry.torokhov@gmail.com> 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@gmail.com> > > > > 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@chromium.org> > > > >> --- > >> 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 > >>
-- Dmitry
| |