* [PATCH v4 1/6] firmware: coreboot: Let OF core populate platform device
2018-08-15 20:37 [PATCH v4 0/6] firmware: coreboot: Fix probe and simplify code Stephen Boyd
@ 2018-08-15 20:37 ` Stephen Boyd
2018-08-15 20:37 ` [PATCH v4 2/6] firmware: coreboot: Unmap ioregion after device population Stephen Boyd
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Stephen Boyd @ 2018-08-15 20:37 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-kernel, Wei-Ning Huang, Julius Werner, Samuel Holland
Now that the /firmware/coreboot node in DT is populated by the core DT
platform code with commit 3aa0582fdb82 ("of: platform: populate
/firmware/ node from of_platform_default_populate_init()") we should and
can remove the platform device creation here. Otherwise, the
of_platform_device_create() call will fail, the coreboot of driver won't
be registered, and this driver will never bind. At the same time, we
should move this driver to use MODULE_DEVICE_TABLE so that module
auto-load works properly when the coreboot device is auto-populated and
we should drop the of_node handling that was presumably placed here to
hold a reference to the DT node created during module init that no
longer happens.
Cc: Wei-Ning Huang <wnhuang@chromium.org>
Cc: Julius Werner <jwerner@chromium.org>
Reviewed-by: Brian Norris <briannorris@chromium.org>
Cc: Samuel Holland <samuel@sholland.org>
Reviewed-by: Sudeep Holla <Sudeep.Holla@arm.com>
Fixes: 3aa0582fdb82 ("of: platform: populate /firmware/ node from of_platform_default_populate_init()")
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
drivers/firmware/google/coreboot_table-of.c | 28 +++------------------
1 file changed, 3 insertions(+), 25 deletions(-)
diff --git a/drivers/firmware/google/coreboot_table-of.c b/drivers/firmware/google/coreboot_table-of.c
index f15bf404c579..9b90c0fa4a0b 100644
--- a/drivers/firmware/google/coreboot_table-of.c
+++ b/drivers/firmware/google/coreboot_table-of.c
@@ -19,7 +19,6 @@
#include <linux/io.h>
#include <linux/module.h>
#include <linux/of_address.h>
-#include <linux/of_platform.h>
#include <linux/platform_device.h>
#include "coreboot_table.h"
@@ -30,7 +29,6 @@ static int coreboot_table_of_probe(struct platform_device *pdev)
void __iomem *ptr;
ptr = of_iomap(fw_dn, 0);
- of_node_put(fw_dn);
if (!ptr)
return -ENOMEM;
@@ -44,8 +42,9 @@ static int coreboot_table_of_remove(struct platform_device *pdev)
static const struct of_device_id coreboot_of_match[] = {
{ .compatible = "coreboot" },
- {},
+ {}
};
+MODULE_DEVICE_TABLE(of, coreboot_of_match);
static struct platform_driver coreboot_table_of_driver = {
.probe = coreboot_table_of_probe,
@@ -55,28 +54,7 @@ static struct platform_driver coreboot_table_of_driver = {
.of_match_table = coreboot_of_match,
},
};
-
-static int __init platform_coreboot_table_of_init(void)
-{
- struct platform_device *pdev;
- struct device_node *of_node;
-
- /* Limit device creation to the presence of /firmware/coreboot node */
- of_node = of_find_node_by_path("/firmware/coreboot");
- if (!of_node)
- return -ENODEV;
-
- if (!of_match_node(coreboot_of_match, of_node))
- return -ENODEV;
-
- pdev = of_platform_device_create(of_node, "coreboot_table_of", NULL);
- if (!pdev)
- return -ENODEV;
-
- return platform_driver_register(&coreboot_table_of_driver);
-}
-
-module_init(platform_coreboot_table_of_init);
+module_platform_driver(coreboot_table_of_driver);
MODULE_AUTHOR("Google, Inc.");
MODULE_LICENSE("GPL");
--
Sent by a computer through tubes
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v4 2/6] firmware: coreboot: Unmap ioregion after device population
2018-08-15 20:37 [PATCH v4 0/6] firmware: coreboot: Fix probe and simplify code Stephen Boyd
2018-08-15 20:37 ` [PATCH v4 1/6] firmware: coreboot: Let OF core populate platform device Stephen Boyd
@ 2018-08-15 20:37 ` Stephen Boyd
2018-08-15 20:37 ` [PATCH v4 3/6] firmware: coreboot: Make bus registration symmetric Stephen Boyd
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Stephen Boyd @ 2018-08-15 20:37 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-kernel, Wei-Ning Huang, Julius Werner, Brian Norris,
Samuel Holland
Both callers of coreboot_table_init() ioremap the pointer that comes in
but they don't unmap the memory on failure. Both of them also fail probe
immediately with the return value of coreboot_table_init(), leaking a
mapping when it fails. The mapping isn't necessary at all after devices
are populated either, so we can just drop the mapping here when we exit
the function. Let's do that to simplify the code a bit and plug the leak.
Cc: Wei-Ning Huang <wnhuang@chromium.org>
Cc: Julius Werner <jwerner@chromium.org>
Cc: Brian Norris <briannorris@chromium.org>
Cc: Samuel Holland <samuel@sholland.org>
Fixes: 570d30c2823f ("firmware: coreboot: Expose the coreboot table as a bus")
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
drivers/firmware/google/coreboot_table.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/firmware/google/coreboot_table.c b/drivers/firmware/google/coreboot_table.c
index 19db5709ae28..898bb9abc41f 100644
--- a/drivers/firmware/google/coreboot_table.c
+++ b/drivers/firmware/google/coreboot_table.c
@@ -110,7 +110,8 @@ int coreboot_table_init(struct device *dev, void __iomem *ptr)
if (strncmp(header.signature, "LBIO", sizeof(header.signature))) {
pr_warn("coreboot_table: coreboot table missing or corrupt!\n");
- return -ENODEV;
+ ret = -ENODEV;
+ goto out;
}
ptr_entry = (void *)ptr_header + header.header_bytes;
@@ -137,7 +138,8 @@ int coreboot_table_init(struct device *dev, void __iomem *ptr)
ptr_entry += entry.size;
}
-
+out:
+ iounmap(ptr);
return ret;
}
EXPORT_SYMBOL(coreboot_table_init);
@@ -146,7 +148,6 @@ int coreboot_table_exit(void)
{
if (ptr_header) {
bus_unregister(&coreboot_bus_type);
- iounmap(ptr_header);
ptr_header = NULL;
}
--
Sent by a computer through tubes
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v4 3/6] firmware: coreboot: Make bus registration symmetric
2018-08-15 20:37 [PATCH v4 0/6] firmware: coreboot: Fix probe and simplify code Stephen Boyd
2018-08-15 20:37 ` [PATCH v4 1/6] firmware: coreboot: Let OF core populate platform device Stephen Boyd
2018-08-15 20:37 ` [PATCH v4 2/6] firmware: coreboot: Unmap ioregion after device population Stephen Boyd
@ 2018-08-15 20:37 ` Stephen Boyd
2018-08-15 20:37 ` [PATCH v4 4/6] firmware: coreboot: Collapse platform drivers into bus core Stephen Boyd
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Stephen Boyd @ 2018-08-15 20:37 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-kernel, Wei-Ning Huang, Julius Werner, Brian Norris,
Samuel Holland
The bus is registered in module_init() but is unregistered when the
platform driver remove() function calls coreboot_table_exit(). That
isn't symmetric and it causes the bus to appear on systems that compile
this code in, even when there isn't any coreboot firmware on the device.
Let's move the registration to the coreboot_table_init() function so
that it matches the exit path.
Cc: Wei-Ning Huang <wnhuang@chromium.org>
Cc: Julius Werner <jwerner@chromium.org>
Cc: Brian Norris <briannorris@chromium.org>
Cc: Samuel Holland <samuel@sholland.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
drivers/firmware/google/coreboot_table.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/firmware/google/coreboot_table.c b/drivers/firmware/google/coreboot_table.c
index 898bb9abc41f..f417170f83ea 100644
--- a/drivers/firmware/google/coreboot_table.c
+++ b/drivers/firmware/google/coreboot_table.c
@@ -70,12 +70,6 @@ static struct bus_type coreboot_bus_type = {
.remove = coreboot_bus_remove,
};
-static int __init coreboot_bus_init(void)
-{
- return bus_register(&coreboot_bus_type);
-}
-module_init(coreboot_bus_init);
-
static void coreboot_device_release(struct device *dev)
{
struct coreboot_device *device = CB_DEV(dev);
@@ -114,6 +108,10 @@ int coreboot_table_init(struct device *dev, void __iomem *ptr)
goto out;
}
+ ret = bus_register(&coreboot_bus_type);
+ if (ret)
+ goto out;
+
ptr_entry = (void *)ptr_header + header.header_bytes;
for (i = 0; i < header.table_entries; i++) {
memcpy_fromio(&entry, ptr_entry, sizeof(entry));
@@ -138,6 +136,10 @@ int coreboot_table_init(struct device *dev, void __iomem *ptr)
ptr_entry += entry.size;
}
+
+ if (ret)
+ bus_unregister(&coreboot_bus_type);
+
out:
iounmap(ptr);
return ret;
--
Sent by a computer through tubes
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v4 4/6] firmware: coreboot: Collapse platform drivers into bus core
2018-08-15 20:37 [PATCH v4 0/6] firmware: coreboot: Fix probe and simplify code Stephen Boyd
` (2 preceding siblings ...)
2018-08-15 20:37 ` [PATCH v4 3/6] firmware: coreboot: Make bus registration symmetric Stephen Boyd
@ 2018-08-15 20:37 ` Stephen Boyd
2018-08-15 20:37 ` [PATCH v4 5/6] firmware: coreboot: Remap RAM with memremap() instead of ioremap() Stephen Boyd
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Stephen Boyd @ 2018-08-15 20:37 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-kernel, Wei-Ning Huang, Julius Werner, Brian Norris,
Samuel Holland
The DT based and ACPI based platform drivers here do the same thing; map
some memory and hand it over to the coreboot bus to populate devices.
The only major difference is that the DT based driver doesn't map the
coreboot table header to figure out how large of a region to map for the
whole coreboot table and it uses of_iomap() instead of ioremap_cache().
A cached or non-cached mapping shouldn't matter here and mapping some
smaller region first before mapping the whole table is just more work
but should be OK. In the end, we can remove two files and combine the
code all in one place making it easier to reason about things.
We leave the old Kconfigs in place for a little while longer but make
them hidden and select the previously hidden config option. This way
users can upgrade without having to know to reselect this config in the
future. Later on we can remove the old hidden configs.
Cc: Wei-Ning Huang <wnhuang@chromium.org>
Cc: Julius Werner <jwerner@chromium.org>
Cc: Brian Norris <briannorris@chromium.org>
Cc: Samuel Holland <samuel@sholland.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
drivers/firmware/google/Kconfig | 26 +++---
drivers/firmware/google/Makefile | 2 -
drivers/firmware/google/coreboot_table-acpi.c | 88 -------------------
drivers/firmware/google/coreboot_table-of.c | 60 -------------
drivers/firmware/google/coreboot_table.c | 66 +++++++++++++-
drivers/firmware/google/coreboot_table.h | 6 --
6 files changed, 72 insertions(+), 176 deletions(-)
delete mode 100644 drivers/firmware/google/coreboot_table-acpi.c
delete mode 100644 drivers/firmware/google/coreboot_table-of.c
diff --git a/drivers/firmware/google/Kconfig b/drivers/firmware/google/Kconfig
index a456a000048b..16564cfa47ef 100644
--- a/drivers/firmware/google/Kconfig
+++ b/drivers/firmware/google/Kconfig
@@ -19,28 +19,22 @@ config GOOGLE_SMI
variables.
config GOOGLE_COREBOOT_TABLE
- tristate
- depends on GOOGLE_COREBOOT_TABLE_ACPI || GOOGLE_COREBOOT_TABLE_OF
-
-config GOOGLE_COREBOOT_TABLE_ACPI
- tristate "Coreboot Table Access - ACPI"
- depends on ACPI
- select GOOGLE_COREBOOT_TABLE
+ tristate "Coreboot Table Access"
+ depends on ACPI || OF
help
This option enables the coreboot_table module, which provides other
- firmware modules to access to the coreboot table. The coreboot table
- pointer is accessed through the ACPI "GOOGCB00" object.
+ firmware modules access to the coreboot table. The coreboot table
+ pointer is accessed through the ACPI "GOOGCB00" object or the
+ device tree node /firmware/coreboot.
If unsure say N.
+config GOOGLE_COREBOOT_TABLE_ACPI
+ tristate
+ select GOOGLE_COREBOOT_TABLE
+
config GOOGLE_COREBOOT_TABLE_OF
- tristate "Coreboot Table Access - Device Tree"
- depends on OF
+ tristate
select GOOGLE_COREBOOT_TABLE
- help
- This option enable the coreboot_table module, which provide other
- firmware modules to access coreboot table. The coreboot table pointer
- is accessed through the device tree node /firmware/coreboot.
- If unsure say N.
config GOOGLE_MEMCONSOLE
tristate
diff --git a/drivers/firmware/google/Makefile b/drivers/firmware/google/Makefile
index d0b3fba96194..d17caded5d88 100644
--- a/drivers/firmware/google/Makefile
+++ b/drivers/firmware/google/Makefile
@@ -2,8 +2,6 @@
obj-$(CONFIG_GOOGLE_SMI) += gsmi.o
obj-$(CONFIG_GOOGLE_COREBOOT_TABLE) += coreboot_table.o
-obj-$(CONFIG_GOOGLE_COREBOOT_TABLE_ACPI) += coreboot_table-acpi.o
-obj-$(CONFIG_GOOGLE_COREBOOT_TABLE_OF) += coreboot_table-of.o
obj-$(CONFIG_GOOGLE_FRAMEBUFFER_COREBOOT) += framebuffer-coreboot.o
obj-$(CONFIG_GOOGLE_MEMCONSOLE) += memconsole.o
obj-$(CONFIG_GOOGLE_MEMCONSOLE_COREBOOT) += memconsole-coreboot.o
diff --git a/drivers/firmware/google/coreboot_table-acpi.c b/drivers/firmware/google/coreboot_table-acpi.c
deleted file mode 100644
index 77197fe3d42f..000000000000
--- a/drivers/firmware/google/coreboot_table-acpi.c
+++ /dev/null
@@ -1,88 +0,0 @@
-/*
- * coreboot_table-acpi.c
- *
- * Using ACPI to locate Coreboot table and provide coreboot table access.
- *
- * Copyright 2017 Google Inc.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License v2.0 as published by
- * the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- */
-
-#include <linux/acpi.h>
-#include <linux/device.h>
-#include <linux/err.h>
-#include <linux/init.h>
-#include <linux/io.h>
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/platform_device.h>
-
-#include "coreboot_table.h"
-
-static int coreboot_table_acpi_probe(struct platform_device *pdev)
-{
- phys_addr_t phyaddr;
- resource_size_t len;
- struct coreboot_table_header __iomem *header = NULL;
- struct resource *res;
- void __iomem *ptr = NULL;
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res)
- return -EINVAL;
-
- len = resource_size(res);
- if (!res->start || !len)
- return -EINVAL;
-
- phyaddr = res->start;
- header = ioremap_cache(phyaddr, sizeof(*header));
- if (header == NULL)
- return -ENOMEM;
-
- ptr = ioremap_cache(phyaddr,
- header->header_bytes + header->table_bytes);
- iounmap(header);
- if (!ptr)
- return -ENOMEM;
-
- return coreboot_table_init(&pdev->dev, ptr);
-}
-
-static int coreboot_table_acpi_remove(struct platform_device *pdev)
-{
- return coreboot_table_exit();
-}
-
-static const struct acpi_device_id cros_coreboot_acpi_match[] = {
- { "GOOGCB00", 0 },
- { "BOOT0000", 0 },
- { }
-};
-MODULE_DEVICE_TABLE(acpi, cros_coreboot_acpi_match);
-
-static struct platform_driver coreboot_table_acpi_driver = {
- .probe = coreboot_table_acpi_probe,
- .remove = coreboot_table_acpi_remove,
- .driver = {
- .name = "coreboot_table_acpi",
- .acpi_match_table = ACPI_PTR(cros_coreboot_acpi_match),
- },
-};
-
-static int __init coreboot_table_acpi_init(void)
-{
- return platform_driver_register(&coreboot_table_acpi_driver);
-}
-
-module_init(coreboot_table_acpi_init);
-
-MODULE_AUTHOR("Google, Inc.");
-MODULE_LICENSE("GPL");
diff --git a/drivers/firmware/google/coreboot_table-of.c b/drivers/firmware/google/coreboot_table-of.c
deleted file mode 100644
index 9b90c0fa4a0b..000000000000
--- a/drivers/firmware/google/coreboot_table-of.c
+++ /dev/null
@@ -1,60 +0,0 @@
-/*
- * coreboot_table-of.c
- *
- * Coreboot table access through open firmware.
- *
- * Copyright 2017 Google Inc.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License v2.0 as published by
- * the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- */
-
-#include <linux/device.h>
-#include <linux/io.h>
-#include <linux/module.h>
-#include <linux/of_address.h>
-#include <linux/platform_device.h>
-
-#include "coreboot_table.h"
-
-static int coreboot_table_of_probe(struct platform_device *pdev)
-{
- struct device_node *fw_dn = pdev->dev.of_node;
- void __iomem *ptr;
-
- ptr = of_iomap(fw_dn, 0);
- if (!ptr)
- return -ENOMEM;
-
- return coreboot_table_init(&pdev->dev, ptr);
-}
-
-static int coreboot_table_of_remove(struct platform_device *pdev)
-{
- return coreboot_table_exit();
-}
-
-static const struct of_device_id coreboot_of_match[] = {
- { .compatible = "coreboot" },
- {}
-};
-MODULE_DEVICE_TABLE(of, coreboot_of_match);
-
-static struct platform_driver coreboot_table_of_driver = {
- .probe = coreboot_table_of_probe,
- .remove = coreboot_table_of_remove,
- .driver = {
- .name = "coreboot_table_of",
- .of_match_table = coreboot_of_match,
- },
-};
-module_platform_driver(coreboot_table_of_driver);
-
-MODULE_AUTHOR("Google, Inc.");
-MODULE_LICENSE("GPL");
diff --git a/drivers/firmware/google/coreboot_table.c b/drivers/firmware/google/coreboot_table.c
index f417170f83ea..e9cd4da61404 100644
--- a/drivers/firmware/google/coreboot_table.c
+++ b/drivers/firmware/google/coreboot_table.c
@@ -16,12 +16,15 @@
* GNU General Public License for more details.
*/
+#include <linux/acpi.h>
#include <linux/device.h>
#include <linux/err.h>
#include <linux/init.h>
#include <linux/io.h>
#include <linux/kernel.h>
#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
#include <linux/slab.h>
#include "coreboot_table.h"
@@ -91,7 +94,7 @@ void coreboot_driver_unregister(struct coreboot_driver *driver)
}
EXPORT_SYMBOL(coreboot_driver_unregister);
-int coreboot_table_init(struct device *dev, void __iomem *ptr)
+static int coreboot_table_init(struct device *dev, void __iomem *ptr)
{
int i, ret;
void *ptr_entry;
@@ -144,9 +147,38 @@ int coreboot_table_init(struct device *dev, void __iomem *ptr)
iounmap(ptr);
return ret;
}
-EXPORT_SYMBOL(coreboot_table_init);
-int coreboot_table_exit(void)
+static int coreboot_table_probe(struct platform_device *pdev)
+{
+ phys_addr_t phyaddr;
+ resource_size_t len;
+ struct coreboot_table_header __iomem *header = NULL;
+ struct resource *res;
+ void __iomem *ptr = NULL;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -EINVAL;
+
+ len = resource_size(res);
+ if (!res->start || !len)
+ return -EINVAL;
+
+ phyaddr = res->start;
+ header = ioremap_cache(phyaddr, sizeof(*header));
+ if (header == NULL)
+ return -ENOMEM;
+
+ ptr = ioremap_cache(phyaddr,
+ header->header_bytes + header->table_bytes);
+ iounmap(header);
+ if (!ptr)
+ return -ENOMEM;
+
+ return coreboot_table_init(&pdev->dev, ptr);
+}
+
+static int coreboot_table_remove(struct platform_device *pdev)
{
if (ptr_header) {
bus_unregister(&coreboot_bus_type);
@@ -155,7 +187,33 @@ int coreboot_table_exit(void)
return 0;
}
-EXPORT_SYMBOL(coreboot_table_exit);
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id cros_coreboot_acpi_match[] = {
+ { "GOOGCB00", 0 },
+ { "BOOT0000", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(acpi, cros_coreboot_acpi_match);
+#endif
+
+#ifdef CONFIG_OF
+static const struct of_device_id coreboot_of_match[] = {
+ { .compatible = "coreboot" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, coreboot_of_match);
+#endif
+
+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),
+ },
+};
+module_platform_driver(coreboot_table_driver);
MODULE_AUTHOR("Google, Inc.");
MODULE_LICENSE("GPL");
diff --git a/drivers/firmware/google/coreboot_table.h b/drivers/firmware/google/coreboot_table.h
index 8ad95a94481b..71a9de6b15fa 100644
--- a/drivers/firmware/google/coreboot_table.h
+++ b/drivers/firmware/google/coreboot_table.h
@@ -91,10 +91,4 @@ int coreboot_driver_register(struct coreboot_driver *driver);
/* Unregister a driver that uses the data from a coreboot table. */
void coreboot_driver_unregister(struct coreboot_driver *driver);
-/* Initialize coreboot table module given a pointer to iomem */
-int coreboot_table_init(struct device *dev, void __iomem *ptr);
-
-/* Cleanup coreboot table module */
-int coreboot_table_exit(void);
-
#endif /* __COREBOOT_TABLE_H */
--
Sent by a computer through tubes
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v4 5/6] firmware: coreboot: Remap RAM with memremap() instead of ioremap()
2018-08-15 20:37 [PATCH v4 0/6] firmware: coreboot: Fix probe and simplify code Stephen Boyd
` (3 preceding siblings ...)
2018-08-15 20:37 ` [PATCH v4 4/6] firmware: coreboot: Collapse platform drivers into bus core Stephen Boyd
@ 2018-08-15 20:37 ` Stephen Boyd
2018-08-15 20:37 ` [PATCH v4 6/6] firmware: coreboot: Only populate devices in coreboot_table_init() Stephen Boyd
2018-08-15 21:19 ` [PATCH v4 0/6] firmware: coreboot: Fix probe and simplify code Julius Werner
6 siblings, 0 replies; 8+ messages in thread
From: Stephen Boyd @ 2018-08-15 20:37 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-kernel, Wei-Ning Huang, Julius Werner, Brian Norris,
Samuel Holland
This is all system memory, so we shouldn't be mapping this all with
ioremap() as these aren't I/O regions. Instead, they're memory regions
so we should use memremap(). Pick MEMREMAP_WB so we can map memory from
RAM directly if that's possible, otherwise it falls back to
ioremap_cache() like is being done here already. This also nicely
silences the sparse warnings in this code and reduces the need to copy
anything around anymore.
Cc: Wei-Ning Huang <wnhuang@chromium.org>
Cc: Julius Werner <jwerner@chromium.org>
Cc: Brian Norris <briannorris@chromium.org>
Cc: Samuel Holland <samuel@sholland.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
drivers/firmware/google/coreboot_table.c | 40 +++++++++++-------------
1 file changed, 19 insertions(+), 21 deletions(-)
diff --git a/drivers/firmware/google/coreboot_table.c b/drivers/firmware/google/coreboot_table.c
index e9cd4da61404..2c6c35f0da32 100644
--- a/drivers/firmware/google/coreboot_table.c
+++ b/drivers/firmware/google/coreboot_table.c
@@ -32,7 +32,7 @@
#define CB_DEV(d) container_of(d, struct coreboot_device, dev)
#define CB_DRV(d) container_of(d, struct coreboot_driver, drv)
-static struct coreboot_table_header __iomem *ptr_header;
+static struct coreboot_table_header *ptr_header;
static int coreboot_bus_match(struct device *dev, struct device_driver *drv)
{
@@ -94,18 +94,18 @@ void coreboot_driver_unregister(struct coreboot_driver *driver)
}
EXPORT_SYMBOL(coreboot_driver_unregister);
-static int coreboot_table_init(struct device *dev, void __iomem *ptr)
+static int coreboot_table_init(struct device *dev, void *ptr)
{
int i, ret;
void *ptr_entry;
struct coreboot_device *device;
- struct coreboot_table_entry entry;
- struct coreboot_table_header header;
+ struct coreboot_table_entry *entry;
+ struct coreboot_table_header *header;
ptr_header = ptr;
- memcpy_fromio(&header, ptr_header, sizeof(header));
+ header = ptr;
- if (strncmp(header.signature, "LBIO", sizeof(header.signature))) {
+ if (strncmp(header->signature, "LBIO", sizeof(header->signature))) {
pr_warn("coreboot_table: coreboot table missing or corrupt!\n");
ret = -ENODEV;
goto out;
@@ -115,11 +115,11 @@ static int coreboot_table_init(struct device *dev, void __iomem *ptr)
if (ret)
goto out;
- ptr_entry = (void *)ptr_header + header.header_bytes;
- for (i = 0; i < header.table_entries; i++) {
- memcpy_fromio(&entry, ptr_entry, sizeof(entry));
+ ptr_entry = ptr_header + header->header_bytes;
+ for (i = 0; i < header->table_entries; i++) {
+ entry = ptr_entry;
- device = kzalloc(sizeof(struct device) + entry.size, GFP_KERNEL);
+ device = kzalloc(sizeof(struct device) + entry->size, GFP_KERNEL);
if (!device) {
ret = -ENOMEM;
break;
@@ -129,7 +129,7 @@ static int coreboot_table_init(struct device *dev, void __iomem *ptr)
device->dev.parent = dev;
device->dev.bus = &coreboot_bus_type;
device->dev.release = coreboot_device_release;
- memcpy_fromio(&device->entry, ptr_entry, entry.size);
+ memcpy(&device->entry, ptr_entry, entry->size);
ret = device_register(&device->dev);
if (ret) {
@@ -137,24 +137,23 @@ static int coreboot_table_init(struct device *dev, void __iomem *ptr)
break;
}
- ptr_entry += entry.size;
+ ptr_entry += entry->size;
}
if (ret)
bus_unregister(&coreboot_bus_type);
out:
- iounmap(ptr);
+ memunmap(ptr);
return ret;
}
static int coreboot_table_probe(struct platform_device *pdev)
{
- phys_addr_t phyaddr;
resource_size_t len;
- struct coreboot_table_header __iomem *header = NULL;
+ struct coreboot_table_header *header;
struct resource *res;
- void __iomem *ptr = NULL;
+ void *ptr;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res)
@@ -164,14 +163,13 @@ static int coreboot_table_probe(struct platform_device *pdev)
if (!res->start || !len)
return -EINVAL;
- phyaddr = res->start;
- header = ioremap_cache(phyaddr, sizeof(*header));
+ header = memremap(res->start, sizeof(*header), MEMREMAP_WB);
if (header == NULL)
return -ENOMEM;
- ptr = ioremap_cache(phyaddr,
- header->header_bytes + header->table_bytes);
- iounmap(header);
+ ptr = memremap(res->start, header->header_bytes + header->table_bytes,
+ MEMREMAP_WB);
+ memunmap(header);
if (!ptr)
return -ENOMEM;
--
Sent by a computer through tubes
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v4 6/6] firmware: coreboot: Only populate devices in coreboot_table_init()
2018-08-15 20:37 [PATCH v4 0/6] firmware: coreboot: Fix probe and simplify code Stephen Boyd
` (4 preceding siblings ...)
2018-08-15 20:37 ` [PATCH v4 5/6] firmware: coreboot: Remap RAM with memremap() instead of ioremap() Stephen Boyd
@ 2018-08-15 20:37 ` Stephen Boyd
2018-08-15 21:19 ` [PATCH v4 0/6] firmware: coreboot: Fix probe and simplify code Julius Werner
6 siblings, 0 replies; 8+ messages in thread
From: Stephen Boyd @ 2018-08-15 20:37 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-kernel, Wei-Ning Huang, Julius Werner, Brian Norris,
Samuel Holland
This function checks the header for sanity, registers a bus, and
populates devices for each coreboot table entry. Let's just populate
devices here and pull the other bits up into the caller so that this
function can be repurposed for pure device creation and registration.
Cc: Wei-Ning Huang <wnhuang@chromium.org>
Cc: Julius Werner <jwerner@chromium.org>
Cc: Brian Norris <briannorris@chromium.org>
Cc: Samuel Holland <samuel@sholland.org>
Suggested-by: Julius Werner <jwerner@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
drivers/firmware/google/coreboot_table.c | 67 ++++++++++--------------
1 file changed, 29 insertions(+), 38 deletions(-)
diff --git a/drivers/firmware/google/coreboot_table.c b/drivers/firmware/google/coreboot_table.c
index 2c6c35f0da32..078d3bbe632f 100644
--- a/drivers/firmware/google/coreboot_table.c
+++ b/drivers/firmware/google/coreboot_table.c
@@ -32,8 +32,6 @@
#define CB_DEV(d) container_of(d, struct coreboot_device, dev)
#define CB_DRV(d) container_of(d, struct coreboot_driver, drv)
-static struct coreboot_table_header *ptr_header;
-
static int coreboot_bus_match(struct device *dev, struct device_driver *drv)
{
struct coreboot_device *device = CB_DEV(dev);
@@ -94,36 +92,21 @@ void coreboot_driver_unregister(struct coreboot_driver *driver)
}
EXPORT_SYMBOL(coreboot_driver_unregister);
-static int coreboot_table_init(struct device *dev, void *ptr)
+static int coreboot_table_populate(struct device *dev, void *ptr)
{
int i, ret;
void *ptr_entry;
struct coreboot_device *device;
struct coreboot_table_entry *entry;
- struct coreboot_table_header *header;
-
- ptr_header = ptr;
- header = ptr;
+ struct coreboot_table_header *header = ptr;
- if (strncmp(header->signature, "LBIO", sizeof(header->signature))) {
- pr_warn("coreboot_table: coreboot table missing or corrupt!\n");
- ret = -ENODEV;
- goto out;
- }
-
- ret = bus_register(&coreboot_bus_type);
- if (ret)
- goto out;
-
- ptr_entry = ptr_header + header->header_bytes;
+ ptr_entry = ptr + header->header_bytes;
for (i = 0; i < header->table_entries; i++) {
entry = ptr_entry;
device = kzalloc(sizeof(struct device) + entry->size, GFP_KERNEL);
- if (!device) {
- ret = -ENOMEM;
- break;
- }
+ if (!device)
+ return -ENOMEM;
dev_set_name(&device->dev, "coreboot%d", i);
device->dev.parent = dev;
@@ -134,18 +117,13 @@ static int coreboot_table_init(struct device *dev, void *ptr)
ret = device_register(&device->dev);
if (ret) {
put_device(&device->dev);
- break;
+ return ret;
}
ptr_entry += entry->size;
}
- if (ret)
- bus_unregister(&coreboot_bus_type);
-
-out:
- memunmap(ptr);
- return ret;
+ return 0;
}
static int coreboot_table_probe(struct platform_device *pdev)
@@ -153,7 +131,9 @@ static int coreboot_table_probe(struct platform_device *pdev)
resource_size_t len;
struct coreboot_table_header *header;
struct resource *res;
+ struct device *dev = &pdev->dev;
void *ptr;
+ int ret;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res)
@@ -163,26 +143,37 @@ static int coreboot_table_probe(struct platform_device *pdev)
if (!res->start || !len)
return -EINVAL;
+ /* Check just the header first to make sure things are sane */
header = memremap(res->start, sizeof(*header), MEMREMAP_WB);
- if (header == NULL)
+ if (!header)
return -ENOMEM;
- ptr = memremap(res->start, header->header_bytes + header->table_bytes,
- MEMREMAP_WB);
+ len = header->header_bytes + header->table_bytes;
+ ret = strncmp(header->signature, "LBIO", sizeof(header->signature));
memunmap(header);
+ if (ret) {
+ dev_warn(dev, "coreboot table missing or corrupt!\n");
+ return -ENODEV;
+ }
+
+ ptr = memremap(res->start, len, MEMREMAP_WB);
if (!ptr)
return -ENOMEM;
- return coreboot_table_init(&pdev->dev, ptr);
+ ret = bus_register(&coreboot_bus_type);
+ if (!ret) {
+ ret = coreboot_table_populate(dev, ptr);
+ if (ret)
+ bus_unregister(&coreboot_bus_type);
+ }
+ memunmap(ptr);
+
+ return ret;
}
static int coreboot_table_remove(struct platform_device *pdev)
{
- if (ptr_header) {
- bus_unregister(&coreboot_bus_type);
- ptr_header = NULL;
- }
-
+ bus_unregister(&coreboot_bus_type);
return 0;
}
--
Sent by a computer through tubes
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v4 0/6] firmware: coreboot: Fix probe and simplify code
2018-08-15 20:37 [PATCH v4 0/6] firmware: coreboot: Fix probe and simplify code Stephen Boyd
` (5 preceding siblings ...)
2018-08-15 20:37 ` [PATCH v4 6/6] firmware: coreboot: Only populate devices in coreboot_table_init() Stephen Boyd
@ 2018-08-15 21:19 ` Julius Werner
6 siblings, 0 replies; 8+ messages in thread
From: Julius Werner @ 2018-08-15 21:19 UTC (permalink / raw)
To: swboyd
Cc: Greg Kroah-Hartman, LKML, Wei-Ning Huang, Julius Werner,
Brian Norris, samuel
Thanks for all the clean-up, looks great now!
For the whole series:
Reviewed-by: Julius Werner <jwerner@chromium.org>
^ permalink raw reply [flat|nested] 8+ messages in thread