Here's some minor fixes and cleanups for the Google coreboot drivers
that I've had lying around in my tree for a little bit. They
tighten up the code a bit and get rid of some boiler plate.
Stephen Boyd (5):
firmware: google: Add a module_coreboot_driver() macro and use it
firmware: google: memconsole: Use devm_memremap()
firmware: google: memconsole: Drop __iomem on memremap memory
firmware: google: memconsole: Drop global func pointer
firmware: google: coreboot: Drop unnecessary headers
Cc: Wei-Ning Huang <[email protected]>
Cc: Julius Werner <[email protected]>
Cc: Brian Norris <[email protected]>
Cc: Samuel Holland <[email protected]>
Cc: Guenter Roeck <[email protected]>
drivers/firmware/google/coreboot_table.h | 11 ++++++++-
.../firmware/google/framebuffer-coreboot.c | 14 +----------
drivers/firmware/google/memconsole-coreboot.c | 24 ++++---------------
drivers/firmware/google/memconsole.c | 9 +++----
drivers/firmware/google/vpd.c | 14 +----------
drivers/firmware/google/vpd_decode.c | 2 --
6 files changed, 22 insertions(+), 52 deletions(-)
base-commit: e93c9c99a629c61837d5a7fc2120cd2b6c70dbdd
--
Sent by a computer through tubes
memremap() doesn't return __iomem marked memory, so drop the marking
here. This makes static analysis tools like sparse happy again.
Cc: Wei-Ning Huang <[email protected]>
Cc: Julius Werner <[email protected]>
Cc: Brian Norris <[email protected]>
Cc: Samuel Holland <[email protected]>
Cc: Guenter Roeck <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/firmware/google/memconsole-coreboot.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/google/memconsole-coreboot.c b/drivers/firmware/google/memconsole-coreboot.c
index 0b29b27b86f5..ab0fe93b88ad 100644
--- a/drivers/firmware/google/memconsole-coreboot.c
+++ b/drivers/firmware/google/memconsole-coreboot.c
@@ -34,7 +34,7 @@ struct cbmem_cons {
#define CURSOR_MASK ((1 << 28) - 1)
#define OVERFLOW (1 << 31)
-static struct cbmem_cons __iomem *cbmem_console;
+static struct cbmem_cons *cbmem_console;
static u32 cbmem_console_size;
/*
@@ -75,7 +75,7 @@ static ssize_t memconsole_coreboot_read(char *buf, loff_t pos, size_t count)
static int memconsole_probe(struct coreboot_device *dev)
{
- struct cbmem_cons __iomem *tmp_cbmc;
+ struct cbmem_cons *tmp_cbmc;
tmp_cbmc = memremap(dev->cbmem_ref.cbmem_addr,
sizeof(*tmp_cbmc), MEMREMAP_WB);
--
Sent by a computer through tubes
Remove some boiler plate code we have in three drivers with a single
line each time. This also gets us a free assignment of the driver .owner
field, making these drivers work better as modules.
Cc: Wei-Ning Huang <[email protected]>
Cc: Julius Werner <[email protected]>
Cc: Brian Norris <[email protected]>
Cc: Samuel Holland <[email protected]>
Cc: Guenter Roeck <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/firmware/google/coreboot_table.h | 10 ++++++++++
drivers/firmware/google/framebuffer-coreboot.c | 14 +-------------
drivers/firmware/google/memconsole-coreboot.c | 14 +-------------
drivers/firmware/google/vpd.c | 14 +-------------
4 files changed, 13 insertions(+), 39 deletions(-)
diff --git a/drivers/firmware/google/coreboot_table.h b/drivers/firmware/google/coreboot_table.h
index 71a9de6b15fa..054fa9374c59 100644
--- a/drivers/firmware/google/coreboot_table.h
+++ b/drivers/firmware/google/coreboot_table.h
@@ -20,6 +20,7 @@
#ifndef __COREBOOT_TABLE_H
#define __COREBOOT_TABLE_H
+#include <linux/device.h>
#include <linux/io.h>
/* Coreboot table header structure */
@@ -91,4 +92,13 @@ 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);
+/* module_coreboot_driver() - Helper macro for drivers that don't do
+ * anything special in module init/exit. This eliminates a lot of
+ * boilerplate. Each module may only use this macro once, and
+ * calling it replaces module_init() and module_exit()
+ */
+#define module_coreboot_driver(__coreboot_driver) \
+ module_driver(__coreboot_driver, coreboot_driver_register, \
+ coreboot_driver_unregister)
+
#endif /* __COREBOOT_TABLE_H */
diff --git a/drivers/firmware/google/framebuffer-coreboot.c b/drivers/firmware/google/framebuffer-coreboot.c
index b8b49c067157..69a43116211c 100644
--- a/drivers/firmware/google/framebuffer-coreboot.c
+++ b/drivers/firmware/google/framebuffer-coreboot.c
@@ -97,19 +97,7 @@ static struct coreboot_driver framebuffer_driver = {
},
.tag = CB_TAG_FRAMEBUFFER,
};
-
-static int __init coreboot_framebuffer_init(void)
-{
- return coreboot_driver_register(&framebuffer_driver);
-}
-
-static void coreboot_framebuffer_exit(void)
-{
- coreboot_driver_unregister(&framebuffer_driver);
-}
-
-module_init(coreboot_framebuffer_init);
-module_exit(coreboot_framebuffer_exit);
+module_coreboot_driver(framebuffer_driver);
MODULE_AUTHOR("Samuel Holland <[email protected]>");
MODULE_LICENSE("GPL");
diff --git a/drivers/firmware/google/memconsole-coreboot.c b/drivers/firmware/google/memconsole-coreboot.c
index b29e10757bfb..86331807f1d5 100644
--- a/drivers/firmware/google/memconsole-coreboot.c
+++ b/drivers/firmware/google/memconsole-coreboot.c
@@ -116,19 +116,7 @@ static struct coreboot_driver memconsole_driver = {
},
.tag = CB_TAG_CBMEM_CONSOLE,
};
-
-static void coreboot_memconsole_exit(void)
-{
- coreboot_driver_unregister(&memconsole_driver);
-}
-
-static int __init coreboot_memconsole_init(void)
-{
- return coreboot_driver_register(&memconsole_driver);
-}
-
-module_exit(coreboot_memconsole_exit);
-module_init(coreboot_memconsole_init);
+module_coreboot_driver(memconsole_driver);
MODULE_AUTHOR("Google, Inc.");
MODULE_LICENSE("GPL");
diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
index c0c0b4e4e281..4e978f4914ee 100644
--- a/drivers/firmware/google/vpd.c
+++ b/drivers/firmware/google/vpd.c
@@ -324,19 +324,7 @@ static struct coreboot_driver vpd_driver = {
},
.tag = CB_TAG_VPD,
};
-
-static int __init coreboot_vpd_init(void)
-{
- return coreboot_driver_register(&vpd_driver);
-}
-
-static void __exit coreboot_vpd_exit(void)
-{
- coreboot_driver_unregister(&vpd_driver);
-}
-
-module_init(coreboot_vpd_init);
-module_exit(coreboot_vpd_exit);
+module_coreboot_driver(vpd_driver);
MODULE_AUTHOR("Google, Inc.");
MODULE_LICENSE("GPL");
--
Sent by a computer through tubes
These headers aren't used by the files they're included in, so drop
them. The memconsole file uses memremap() though, so include io.h there
so that the include is explicit.
Cc: Wei-Ning Huang <[email protected]>
Cc: Julius Werner <[email protected]>
Cc: Brian Norris <[email protected]>
Cc: Samuel Holland <[email protected]>
Cc: Guenter Roeck <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/firmware/google/coreboot_table.h | 1 -
drivers/firmware/google/memconsole-coreboot.c | 1 +
drivers/firmware/google/memconsole.c | 1 -
drivers/firmware/google/vpd_decode.c | 2 --
4 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/firmware/google/coreboot_table.h b/drivers/firmware/google/coreboot_table.h
index 054fa9374c59..d33c139447f6 100644
--- a/drivers/firmware/google/coreboot_table.h
+++ b/drivers/firmware/google/coreboot_table.h
@@ -21,7 +21,6 @@
#define __COREBOOT_TABLE_H
#include <linux/device.h>
-#include <linux/io.h>
/* Coreboot table header structure */
struct coreboot_table_header {
diff --git a/drivers/firmware/google/memconsole-coreboot.c b/drivers/firmware/google/memconsole-coreboot.c
index ab0fe93b88ad..f8a66354e5a4 100644
--- a/drivers/firmware/google/memconsole-coreboot.c
+++ b/drivers/firmware/google/memconsole-coreboot.c
@@ -16,6 +16,7 @@
*/
#include <linux/device.h>
+#include <linux/io.h>
#include <linux/kernel.h>
#include <linux/module.h>
diff --git a/drivers/firmware/google/memconsole.c b/drivers/firmware/google/memconsole.c
index 968135025e4f..c8156db0e3a0 100644
--- a/drivers/firmware/google/memconsole.c
+++ b/drivers/firmware/google/memconsole.c
@@ -15,7 +15,6 @@
* GNU General Public License for more details.
*/
-#include <linux/init.h>
#include <linux/sysfs.h>
#include <linux/kobject.h>
#include <linux/module.h>
diff --git a/drivers/firmware/google/vpd_decode.c b/drivers/firmware/google/vpd_decode.c
index 943acaa8aa76..f8c9143472df 100644
--- a/drivers/firmware/google/vpd_decode.c
+++ b/drivers/firmware/google/vpd_decode.c
@@ -15,8 +15,6 @@
* GNU General Public License for more details.
*/
-#include <linux/export.h>
-
#include "vpd_decode.h"
static int vpd_decode_len(const s32 max_len, const u8 *in,
--
Sent by a computer through tubes
Use the devm version of memremap so that we can delete the unmapping
code in driver remove, but more importantly so that we can unmap this
memory region if memconsole_sysfs_init() errors out for some reason.
Cc: Wei-Ning Huang <[email protected]>
Cc: Julius Werner <[email protected]>
Cc: Brian Norris <[email protected]>
Cc: Samuel Holland <[email protected]>
Cc: Guenter Roeck <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/firmware/google/memconsole-coreboot.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/firmware/google/memconsole-coreboot.c b/drivers/firmware/google/memconsole-coreboot.c
index 86331807f1d5..0b29b27b86f5 100644
--- a/drivers/firmware/google/memconsole-coreboot.c
+++ b/drivers/firmware/google/memconsole-coreboot.c
@@ -85,7 +85,7 @@ static int memconsole_probe(struct coreboot_device *dev)
/* Read size only once to prevent overrun attack through /dev/mem. */
cbmem_console_size = tmp_cbmc->size_dont_access_after_boot;
- cbmem_console = memremap(dev->cbmem_ref.cbmem_addr,
+ cbmem_console = devm_memremap(&dev->dev, dev->cbmem_ref.cbmem_addr,
cbmem_console_size + sizeof(*cbmem_console),
MEMREMAP_WB);
memunmap(tmp_cbmc);
@@ -102,9 +102,6 @@ static int memconsole_remove(struct coreboot_device *dev)
{
memconsole_exit();
- if (cbmem_console)
- memunmap(cbmem_console);
-
return 0;
}
--
Sent by a computer through tubes
> Here's some minor fixes and cleanups for the Google coreboot drivers
> that I've had lying around in my tree for a little bit. They
> tighten up the code a bit and get rid of some boiler plate.
>
> Stephen Boyd (5):
> firmware: google: Add a module_coreboot_driver() macro and use it
> firmware: google: memconsole: Use devm_memremap()
> firmware: google: memconsole: Drop __iomem on memremap memory
> firmware: google: memconsole: Drop global func pointer
> firmware: google: coreboot: Drop unnecessary headers
Thanks, these all look good to me.
Reviewed-by: Julius Werner <[email protected]>
Use the devm version of memremap so that we can delete the unmapping
code in driver remove, but more importantly so that we can unmap this
memory region if memconsole_sysfs_init() errors out for some reason.
Cc: Wei-Ning Huang <[email protected]>
Cc: Julius Werner <[email protected]>
Cc: Brian Norris <[email protected]>
Cc: Samuel Holland <[email protected]>
Cc: Guenter Roeck <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
Quoting Stephen Boyd (2019-05-10 11:01:48)
> diff --git a/drivers/firmware/google/memconsole-coreboot.c b/drivers/firmware/google/memconsole-coreboot.c
> index 86331807f1d5..0b29b27b86f5 100644
> --- a/drivers/firmware/google/memconsole-coreboot.c
> +++ b/drivers/firmware/google/memconsole-coreboot.c
> @@ -85,7 +85,7 @@ static int memconsole_probe(struct coreboot_device *dev)
>
> /* Read size only once to prevent overrun attack through /dev/mem. */
> cbmem_console_size = tmp_cbmc->size_dont_access_after_boot;
> - cbmem_console = memremap(dev->cbmem_ref.cbmem_addr,
> + cbmem_console = devm_memremap(&dev->dev, dev->cbmem_ref.cbmem_addr,
Whoops, this returns an error pointer now. Here's an updated patch.
drivers/firmware/google/memconsole-coreboot.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/firmware/google/memconsole-coreboot.c b/drivers/firmware/google/memconsole-coreboot.c
index 86331807f1d5..cc3797f1ba85 100644
--- a/drivers/firmware/google/memconsole-coreboot.c
+++ b/drivers/firmware/google/memconsole-coreboot.c
@@ -85,13 +85,13 @@ static int memconsole_probe(struct coreboot_device *dev)
/* Read size only once to prevent overrun attack through /dev/mem. */
cbmem_console_size = tmp_cbmc->size_dont_access_after_boot;
- cbmem_console = memremap(dev->cbmem_ref.cbmem_addr,
+ cbmem_console = devm_memremap(&dev->dev, dev->cbmem_ref.cbmem_addr,
cbmem_console_size + sizeof(*cbmem_console),
MEMREMAP_WB);
memunmap(tmp_cbmc);
- if (!cbmem_console)
- return -ENOMEM;
+ if (IS_ERR(cbmem_console))
+ return PTR_ERR(cbmem_console);
memconsole_setup(memconsole_coreboot_read);
@@ -102,9 +102,6 @@ static int memconsole_remove(struct coreboot_device *dev)
{
memconsole_exit();
- if (cbmem_console)
- memunmap(cbmem_console);
-
return 0;
}
base-commit: e93c9c99a629c61837d5a7fc2120cd2b6c70dbdd
prerequisite-patch-id: b7c2a1e21fb108364f0e8cfaf1970cbc7903c750
--
Sent by a computer through tubes
On 5/10/19 1:01 PM, Stephen Boyd wrote:
> Here's some minor fixes and cleanups for the Google coreboot drivers
> that I've had lying around in my tree for a little bit. They
> tighten up the code a bit and get rid of some boiler plate.
>
> Stephen Boyd (5):
> firmware: google: Add a module_coreboot_driver() macro and use it
> firmware: google: memconsole: Use devm_memremap()
> firmware: google: memconsole: Drop __iomem on memremap memory
> firmware: google: memconsole: Drop global func pointer
> firmware: google: coreboot: Drop unnecessary headers
With v2 of patch 2:
Reviewed-by: Samuel Holland <[email protected]>
On Fri, May 10, 2019 at 11:01:47AM -0700, Stephen Boyd wrote:
> --- a/drivers/firmware/google/coreboot_table.h
> +++ b/drivers/firmware/google/coreboot_table.h
> @@ -91,4 +92,13 @@ 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);
>
> +/* module_coreboot_driver() - Helper macro for drivers that don't do
Have you been writing too much net/ code recently? :) Or just copying
from platform_device.h I guess. Oh well.
Series looks fine to me.
Brian
> + * anything special in module init/exit. This eliminates a lot of
> + * boilerplate. Each module may only use this macro once, and
> + * calling it replaces module_init() and module_exit()
> + */
> +#define module_coreboot_driver(__coreboot_driver) \
> + module_driver(__coreboot_driver, coreboot_driver_register, \
> + coreboot_driver_unregister)
> +
> #endif /* __COREBOOT_TABLE_H */