Messages in this thread |  | | From | Julius Werner <> | Date | Wed, 8 Aug 2018 12:07:30 -0700 | Subject | Re: [PATCH v2 2/2] firmware: coreboot: Collapse platform drivers into bus core |
| |
> +config GOOGLE_COREBOOT_TABLE_ACPI > + tristate > + default GOOGLE_COREBOOT_TABLE
I don't think this helps in upgrading (as your commit message says) unless you also keep the 'select GOOGLE_COREBOOT_TABLE' here, right?
> -int coreboot_table_init(struct device *dev, void __iomem *ptr) > +static int coreboot_table_init(struct device *dev, void __iomem *ptr)
nit: There's little reason to keep coreboot_table_init() a separate function now. Could maybe compact the code a little more if you merge it into probe()? (Also could then do the signature sanity check before trusting the length values to map the whole thing, which is probably a good idea.)
> if (ptr_header) { > bus_unregister(&coreboot_bus_type); > iounmap(ptr_header);
Could ptr_header be handled by devm now, somehow? Also, don't you have two bus_unregister() now (here and in coreboot_exit())? Or is that intentional?
> +static struct platform_driver coreboot_table_driver = { > + .probe = coreboot_table_probe, > + .remove = coreboot_table_remove, > + .driver = { > + .name = "coreboot_table", > + .acpi_match_table = ACPI_PTR(cros_coreboot_acpi_match), > + .of_match_table = of_match_ptr(coreboot_of_match),
Who takes precedence if they both exist? Will we have two coreboot_table busses? (That would probably not be so good...)
|  |