Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1658330 > unrolled thread

[PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded Binary WMI MOF metadata

Started byAndy Lutomirski <luto@kernel.org>
First post2017-06-06 05:20 +0200
Last post2017-06-07 00:40 +0200
Articles 9 — 5 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded  Binary WMI MOF metadata Andy Lutomirski <luto@kernel.org> - 2017-06-06 05:20 +0200
    Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded  Binary WMI MOF metadata Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-06-06 11:40 +0200
      Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded  Binary WMI MOF metadata Darren Hart <dvhart@infradead.org> - 2017-06-06 18:40 +0200
      Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded  Binary WMI MOF metadata Darren Hart <dvhart@infradead.org> - 2017-06-06 19:00 +0200
        Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded  Binary WMI MOF metadata Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-06-06 20:50 +0200
    Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded  Binary WMI MOF metadata Pali Rohár <pali.rohar@gmail.com> - 2017-06-06 12:10 +0200
      Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded  Binary WMI MOF metadata Darren Hart <dvhart@infradead.org> - 2017-06-06 19:10 +0200
        Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded Binary WMI MOF metadata Pali Rohár <pali.rohar@gmail.com> - 2017-06-06 23:00 +0200
    Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded  Binary WMI MOF metadata Andy Lutomirski <luto@amacapital.net> - 2017-06-07 00:40 +0200

#1658330 — [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded Binary WMI MOF metadata

FromAndy Lutomirski <luto@kernel.org>
Date2017-06-06 05:20 +0200
Subject[PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded Binary WMI MOF metadata
Message-ID<tPdcd-3gP-5@gated-at.bofh.it>
Many laptops (and maybe servers?) have embedded WMI Binary MOF metadata.
We do not yet have open-source tools for processing the data, although
one is in the works thanks to Pali:

	https://github.com/pali/bmfdec

There is currently no interface to get the data in the first place. By
exposing it, we facilitate the development of new tools.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Mario Limonciello <mario_limonciello@dell.com>
Cc: Pali Rohár <pali.rohar@gmail.com>
Cc: linux-kernel@vger.kernel.org
Cc: platform-driver-x86@vger.kernel.org
Cc: linux-acpi@vger.kernel.org
[dvhart: make sysfs mof binary read only, fixup comment block format]
[dvhart: use bmof terminology and dev_err instead of dev_warn]
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Darren Hart (VMware) <dvhart@infradead.org>
---
since-v1:
 * address Pali's comments:
   * update the cover letter for clarity and accuracy
   * update mof->bmof and MOF to Binary MOF throughout the patch
   * use dev_err instead of dev_warn in wmi_bmof_probe


 drivers/platform/x86/Kconfig    |  12 ++++
 drivers/platform/x86/Makefile   |   1 +
 drivers/platform/x86/wmi-bmof.c | 125 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 138 insertions(+)
 create mode 100644 drivers/platform/x86/wmi-bmof.c

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 49a1d01..6ebe393 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -656,6 +656,18 @@ config ACPI_WMI
 	  It is safe to enable this driver even if your DSDT doesn't define
 	  any ACPI-WMI devices.
 
+config WMI_BMOF
+	tristate "WMI embedded Binary MOF driver"
+	depends on ACPI_WMI
+	default y
+	---help---
+	  Say Y here if you want to be able to read a firmware-embedded
+	  WMI Binary MOF data. Using this requires userspace tools and may be
+	  rather tedious.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called wmi-bmof.
+
 config MSI_WMI
 	tristate "MSI WMI extras"
 	depends on ACPI_WMI
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 652d7c8..6a1063e 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -38,6 +38,7 @@ obj-$(CONFIG_MSI_WMI)		+= msi-wmi.o
 obj-$(CONFIG_PEAQ_WMI)		+= peaq-wmi.o
 obj-$(CONFIG_SURFACE3_WMI)	+= surface3-wmi.o
 obj-$(CONFIG_TOPSTAR_LAPTOP)	+= topstar-laptop.o
+obj-$(CONFIG_WMI_BMOF)		+= wmi-bmof.o
 
 # toshiba_acpi must link after wmi to ensure that wmi devices are found
 # before toshiba_acpi initializes
diff --git a/drivers/platform/x86/wmi-bmof.c b/drivers/platform/x86/wmi-bmof.c
new file mode 100644
index 0000000..e1c0963
--- /dev/null
+++ b/drivers/platform/x86/wmi-bmof.c
@@ -0,0 +1,125 @@
+/*
+ * WMI embedded Binary MOF driver
+ *
+ * Copyright (c) 2015 Andrew Lutomirski
+ *
+ *  This program is free software; you can redistribute it and/or modify it
+ *  under the terms of the GNU General Public License version 2 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.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/input.h>
+#include <linux/input/sparse-keymap.h>
+#include <linux/acpi.h>
+#include <linux/string.h>
+#include <linux/dmi.h>
+#include <linux/wmi.h>
+#include <acpi/video.h>
+
+#define WMI_BMOF_GUID "05901221-D566-11D1-B2F0-00A0C9062910"
+MODULE_ALIAS("wmi:" WMI_BMOF_GUID);
+
+struct bmof_priv {
+	union acpi_object *bmofdata;
+	struct bin_attribute bmof_bin_attr;
+};
+
+static ssize_t
+read_bmof(struct file *filp, struct kobject *kobj,
+	 struct bin_attribute *attr,
+	 char *buf, loff_t off, size_t count)
+{
+	struct bmof_priv *priv =
+		container_of(attr, struct bmof_priv, bmof_bin_attr);
+
+	if (off >= priv->bmofdata->buffer.length)
+		return 0;
+
+	if (count > priv->bmofdata->buffer.length - off)
+		count = priv->bmofdata->buffer.length - off;
+
+	memcpy(buf, priv->bmofdata->buffer.pointer + off, count);
+	return count;
+}
+
+static int wmi_bmof_probe(struct wmi_device *wdev)
+{
+	int ret;
+
+	struct bmof_priv *priv =
+		devm_kzalloc(&wdev->dev, sizeof(struct bmof_priv), GFP_KERNEL);
+
+	if (!priv)
+		return -ENOMEM;
+
+	dev_set_drvdata(&wdev->dev, priv);
+
+	priv->bmofdata = wmidev_block_query(wdev, 0);
+	if (!priv->bmofdata) {
+		dev_err(&wdev->dev, "failed to read Binary MOF\n");
+		return -EIO;
+	}
+
+	if (priv->bmofdata->type != ACPI_TYPE_BUFFER) {
+		dev_err(&wdev->dev, "Binary MOF is not a buffer\n");
+		ret = -EIO;
+		goto err_free;
+	}
+
+	sysfs_bin_attr_init(&priv->bmof_bin_attr);
+	priv->bmof_bin_attr.attr.name = "bmof";
+	priv->bmof_bin_attr.attr.mode = 0400;
+	priv->bmof_bin_attr.read = read_bmof;
+	priv->bmof_bin_attr.size = priv->bmofdata->buffer.length;
+
+	ret = sysfs_create_bin_file(&wdev->dev.kobj, &priv->bmof_bin_attr);
+	if (ret)
+		goto err_free;
+
+	return 0;
+
+ err_free:
+	kfree(priv->bmofdata);
+	return ret;
+}
+
+static int wmi_bmof_remove(struct wmi_device *wdev)
+{
+	struct bmof_priv *priv = dev_get_drvdata(&wdev->dev);
+
+	sysfs_remove_bin_file(&wdev->dev.kobj, &priv->bmof_bin_attr);
+	kfree(priv->bmofdata);
+	return 0;
+}
+
+static const struct wmi_device_id wmi_bmof_id_table[] = {
+	{ .guid_string = WMI_BMOF_GUID },
+	{ },
+};
+
+static struct wmi_driver wmi_bmof_driver = {
+	.driver = {
+		.name = "wmi-bmof",
+	},
+	.probe = wmi_bmof_probe,
+	.remove = wmi_bmof_remove,
+	.id_table = wmi_bmof_id_table,
+};
+
+module_wmi_driver(wmi_bmof_driver);
+
+MODULE_AUTHOR("Andrew Lutomirski <luto@kernel.org>");
+MODULE_DESCRIPTION("WMI embedded Binary MOF driver");
+MODULE_LICENSE("GPL");
-- 
2.9.4


-- 
Darren Hart
VMware Open Source Technology Center

[toc] | [next] | [standalone]


#1658541

FromAndy Shevchenko <andy.shevchenko@gmail.com>
Date2017-06-06 11:40 +0200
Message-ID<tPj7Z-6SP-39@gated-at.bofh.it>
In reply to#1658330
On Tue, Jun 6, 2017 at 6:16 AM, Andy Lutomirski <luto@kernel.org> wrote:
> Many laptops (and maybe servers?) have embedded WMI Binary MOF metadata.
> We do not yet have open-source tools for processing the data, although
> one is in the works thanks to Pali:
>
>         https://github.com/pali/bmfdec
>
> There is currently no interface to get the data in the first place. By
> exposing it, we facilitate the development of new tools.

My comments below.
Overall, FWIW,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>


> +config WMI_BMOF
> +       tristate "WMI embedded Binary MOF driver"
> +       depends on ACPI_WMI

> +       default y

Since it can be module it would be better to have more sane default
(distros usually prefers modules over built-in).
Thus, I would go, for example, with

default ACPI_WMI

> +       ---help---
> +         Say Y here if you want to be able to read a firmware-embedded
> +         WMI Binary MOF data. Using this requires userspace tools and may be
> +         rather tedious.
> +
> +         To compile this driver as a module, choose M here: the module will
> +         be called wmi-bmof.

> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +#include <linux/input.h>
> +#include <linux/input/sparse-keymap.h>
> +#include <linux/acpi.h>
> +#include <linux/string.h>
> +#include <linux/dmi.h>
> +#include <linux/wmi.h>
> +#include <acpi/video.h>

Alphabetical order? Up to you.

> +#define WMI_BMOF_GUID "05901221-D566-11D1-B2F0-00A0C9062910"

> +MODULE_ALIAS("wmi:" WMI_BMOF_GUID);

I would gather all MODULE_* together, but it's also matter of taste.

> +static ssize_t
> +read_bmof(struct file *filp, struct kobject *kobj,
> +        struct bin_attribute *attr,
> +        char *buf, loff_t off, size_t count)
> +{
> +       struct bmof_priv *priv =
> +               container_of(attr, struct bmof_priv, bmof_bin_attr);
> +
> +       if (off >= priv->bmofdata->buffer.length)
> +               return 0;

Shouldn't we return an error code here? -ERANGE or alike?

> +static int wmi_bmof_probe(struct wmi_device *wdev)
> +{

> +       int ret;
> +
> +       struct bmof_priv *priv =
> +               devm_kzalloc(&wdev->dev, sizeof(struct bmof_priv), GFP_KERNEL);

I'm not a fan of memory allocation in definition block, so, I would rewrite this

      struct bmof_priv *priv;
      int ret;

      priv = devm_kzalloc(&wdev->dev, sizeof(struct bmof_priv), GFP_KERNEL);

(sizeof(*priv) by your choice)

> +
> +       if (!priv)
> +               return -ENOMEM;

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]


#1658926

FromDarren Hart <dvhart@infradead.org>
Date2017-06-06 18:40 +0200
Message-ID<tPpGp-2N6-1@gated-at.bofh.it>
In reply to#1658541
On Tue, Jun 06, 2017 at 12:30:38PM +0300, Andy Shevchenko wrote:
> On Tue, Jun 6, 2017 at 6:16 AM, Andy Lutomirski <luto@kernel.org> wrote:
> > Many laptops (and maybe servers?) have embedded WMI Binary MOF metadata.
> > We do not yet have open-source tools for processing the data, although
> > one is in the works thanks to Pali:
> >
> >         https://github.com/pali/bmfdec
> >
> > There is currently no interface to get the data in the first place. By
> > exposing it, we facilitate the development of new tools.
> 
> My comments below.
> Overall, FWIW,
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> 
> > +config WMI_BMOF
> > +       tristate "WMI embedded Binary MOF driver"
> > +       depends on ACPI_WMI
> 
> > +       default y
> 
> Since it can be module it would be better to have more sane default
> (distros usually prefers modules over built-in).
> Thus, I would go, for example, with
> 
> default ACPI_WMI

Good point, done.

> 
> > +       ---help---
> > +         Say Y here if you want to be able to read a firmware-embedded
> > +         WMI Binary MOF data. Using this requires userspace tools and may be
> > +         rather tedious.
> > +
> > +         To compile this driver as a module, choose M here: the module will
> > +         be called wmi-bmof.
> 
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/init.h>
> > +#include <linux/slab.h>
> > +#include <linux/types.h>
> > +#include <linux/input.h>
> > +#include <linux/input/sparse-keymap.h>
> > +#include <linux/acpi.h>
> > +#include <linux/string.h>
> > +#include <linux/dmi.h>
> > +#include <linux/wmi.h>
> > +#include <acpi/video.h>
> 
> Alphabetical order? Up to you.

Hrm. There seems to be plenty of similar suggestions on the mailing lists, but
nothing documented in coding-style.rst. If this is a thing we are going to ask
of our contributors, it should be documented. I'm happy to reorder, would you
consider sending the coding-style patch?

> 
> > +#define WMI_BMOF_GUID "05901221-D566-11D1-B2F0-00A0C9062910"
> 
> > +MODULE_ALIAS("wmi:" WMI_BMOF_GUID);
> 
> I would gather all MODULE_* together, but it's also matter of taste.
> 

Sure, done.

> > +static ssize_t
> > +read_bmof(struct file *filp, struct kobject *kobj,
> > +        struct bin_attribute *attr,
> > +        char *buf, loff_t off, size_t count)
> > +{
> > +       struct bmof_priv *priv =
> > +               container_of(attr, struct bmof_priv, bmof_bin_attr);
> > +
> > +       if (off >= priv->bmofdata->buffer.length)
> > +               return 0;
> 
> Shouldn't we return an error code here? -ERANGE or alike?
> 

I took some time and compared this with:

read(2)
lseek(2)
fseek(3)
memory_read_from_buffer()

If offset is <0, we should return EINVAL
If offset is >end_of_buffer.... it's not so cut and dry. It is simpler to just
return 0, and as far as how it affects usage... returning 0 seems perfectly
acceptable for typical read loop usage.

As loff_t is a long long, it could conceivably be < 0, so I've added a check for
that and return -EINVAL in that case.

> > +static int wmi_bmof_probe(struct wmi_device *wdev)
> > +{
> 
> > +       int ret;
> > +
> > +       struct bmof_priv *priv =
> > +               devm_kzalloc(&wdev->dev, sizeof(struct bmof_priv), GFP_KERNEL);
> 
> I'm not a fan of memory allocation in definition block, so, I would rewrite this
> 
>       struct bmof_priv *priv;
>       int ret;
> 
>       priv = devm_kzalloc(&wdev->dev, sizeof(struct bmof_priv), GFP_KERNEL);
> 

Agreed, changed.

Thanks for the review Andy.

-- 
Darren Hart
VMware Open Source Technology Center

[toc] | [prev] | [next] | [standalone]


#1658945

FromDarren Hart <dvhart@infradead.org>
Date2017-06-06 19:00 +0200
Message-ID<tPpZL-2W5-7@gated-at.bofh.it>
In reply to#1658541
On Tue, Jun 06, 2017 at 12:30:38PM +0300, Andy Shevchenko wrote:
> On Tue, Jun 6, 2017 at 6:16 AM, Andy Lutomirski <luto@kernel.org> wrote:
> > Many laptops (and maybe servers?) have embedded WMI Binary MOF metadata.
> > We do not yet have open-source tools for processing the data, although
> > one is in the works thanks to Pali:
> >
> >         https://github.com/pali/bmfdec
> >
> > There is currently no interface to get the data in the first place. By
> > exposing it, we facilitate the development of new tools.
> 
> My comments below.
> Overall, FWIW,
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> 
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/init.h>
> > +#include <linux/slab.h>
> > +#include <linux/types.h>
> > +#include <linux/input.h>
> > +#include <linux/input/sparse-keymap.h>
> > +#include <linux/acpi.h>
> > +#include <linux/string.h>
> > +#include <linux/dmi.h>
> > +#include <linux/wmi.h>
> > +#include <acpi/video.h>
> 
> Alphabetical order? Up to you.

OK, I failed to audit this... lots we don't need in here.

The minimum to build is:

#include <linux/wmi.h>

So assuming this was copy/pasted from another file.

Again, no guidance in coding-style.rst on includes. Seems to me we should
include what we specifically require, regardless of whether or not another
header also happens to include it. We need acpi for example, even though wmi
also includes it.

We should include modules, even though acpi includes it.

We use several other things we aren't including for, like

memcpy
dev_kzalloc
sysfs_create_bin_file

So I suggest:

#include <linux/acpi.h>
#include <linux/device.h>
#include <linux/fs.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/string.h>
#include <linux/sysfs.h>
#include <linux/types.h>
#include <linux/wmi.h>

Which removes:
#include <acpi/video.h>
#include <linux/dmi.h>
#include <linux/init.h>
#include <linux/input.h>
#include <linux/input/sparse-keymap.h>
#include <linux/slab.h>

And adds:
#include <linux/device.h>
#include <linux/fs.h>
#include <linux/sysfs.h>

-- 
Darren Hart
VMware Open Source Technology Center

[toc] | [prev] | [next] | [standalone]


#1659049

FromAndy Shevchenko <andy.shevchenko@gmail.com>
Date2017-06-06 20:50 +0200
Message-ID<tPrId-45h-5@gated-at.bofh.it>
In reply to#1658945
On Tue, Jun 6, 2017 at 7:54 PM, Darren Hart <dvhart@infradead.org> wrote:
> On Tue, Jun 06, 2017 at 12:30:38PM +0300, Andy Shevchenko wrote:
>> On Tue, Jun 6, 2017 at 6:16 AM, Andy Lutomirski <luto@kernel.org> wrote:

>> > +#include <linux/kernel.h>
>> > +#include <linux/module.h>
>> > +#include <linux/init.h>
>> > +#include <linux/slab.h>
>> > +#include <linux/types.h>
>> > +#include <linux/input.h>
>> > +#include <linux/input/sparse-keymap.h>
>> > +#include <linux/acpi.h>
>> > +#include <linux/string.h>
>> > +#include <linux/dmi.h>
>> > +#include <linux/wmi.h>
>> > +#include <acpi/video.h>
>>
>> Alphabetical order? Up to you.
>
> OK, I failed to audit this... lots we don't need in here.
>
> The minimum to build is:
>
> #include <linux/wmi.h>
>
> So assuming this was copy/pasted from another file.

> Again, no guidance in coding-style.rst on includes. Seems to me we should
> include what we specifically require, regardless of whether or not another
> header also happens to include it.

Usually it's a sane choice.
Regarding to order the rationale I see there is easiest way to detect
(on the glance) what headers are already there and there is no
duplication. I saw in the past few patches to remove header
duplication since the original list wasn't in order in the first
place.

Of course there might be exceptions.

> We need acpi for example, even though wmi
> also includes it.
>
> We should include modules, even though acpi includes it.
>
> We use several other things we aren't including for, like
>
> memcpy
> dev_kzalloc
> sysfs_create_bin_file
>
> So I suggest:
>
> #include <linux/acpi.h>
> #include <linux/device.h>
> #include <linux/fs.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/string.h>
> #include <linux/sysfs.h>
> #include <linux/types.h>
> #include <linux/wmi.h>
>
> Which removes:
> #include <acpi/video.h>
> #include <linux/dmi.h>
> #include <linux/init.h>
> #include <linux/input.h>
> #include <linux/input/sparse-keymap.h>
> #include <linux/slab.h>
>
> And adds:
> #include <linux/device.h>
> #include <linux/fs.h>
> #include <linux/sysfs.h>

Works for me!

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]


#1658573

FromPali Rohár <pali.rohar@gmail.com>
Date2017-06-06 12:10 +0200
Message-ID<tPjB0-7ik-19@gated-at.bofh.it>
In reply to#1658330
On Monday 05 June 2017 20:16:44 Andy Lutomirski wrote:
> +#define WMI_BMOF_GUID "05901221-D566-11D1-B2F0-00A0C9062910"
> +MODULE_ALIAS("wmi:" WMI_BMOF_GUID);

Cannot we generate MODULE_ALIAS from module_wmi_driver()? IIRC it is
working for i2c drivers.

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]


#1658950

FromDarren Hart <dvhart@infradead.org>
Date2017-06-06 19:10 +0200
Message-ID<tPq9s-3eA-11@gated-at.bofh.it>
In reply to#1658573
On Tue, Jun 06, 2017 at 12:04:40PM +0200, Pali Rohár wrote:
> On Monday 05 June 2017 20:16:44 Andy Lutomirski wrote:
> > +#define WMI_BMOF_GUID "05901221-D566-11D1-B2F0-00A0C9062910"
> > +MODULE_ALIAS("wmi:" WMI_BMOF_GUID);
> 
> Cannot we generate MODULE_ALIAS from module_wmi_driver()? IIRC it is
> working for i2c drivers.

I could see this being automated since we always use wmi:GUID, but it isn't
currently. Happy to consider it as a follow on.

Do you have a specific i2c example you think we should consider following?

-- 
Darren Hart
VMware Open Source Technology Center

[toc] | [prev] | [next] | [standalone]


#1659199 — Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded Binary WMI MOF metadata

FromPali Rohár <pali.rohar@gmail.com>
Date2017-06-06 23:00 +0200
SubjectRe: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded Binary WMI MOF metadata
Message-ID<tPtK2-5mC-11@gated-at.bofh.it>
In reply to#1658950

[Multipart message — attachments visible in raw view] — view raw

On Tuesday 06 June 2017 19:02:01 Darren Hart wrote:
> On Tue, Jun 06, 2017 at 12:04:40PM +0200, Pali Rohár wrote:
> > On Monday 05 June 2017 20:16:44 Andy Lutomirski wrote:
> > > +#define WMI_BMOF_GUID "05901221-D566-11D1-B2F0-00A0C9062910"
> > > +MODULE_ALIAS("wmi:" WMI_BMOF_GUID);
> > 
> > Cannot we generate MODULE_ALIAS from module_wmi_driver()? IIRC it
> > is working for i2c drivers.
> 
> I could see this being automated since we always use wmi:GUID, but it
> isn't currently. Happy to consider it as a follow on.
> 
> Do you have a specific i2c example you think we should consider
> following?

For i2c you can specify in driver code:

MODULE_DEVICE_TABLE(i2c, id_table);

And it automatically provides (via file.mod.c) all needed MODULE_ALIAS.

So when we have wmi_bmof_id_table in driver, cannot we use this?

MODULE_DEVICE_TABLE(wmi, wmi_bmof_id_table);

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]


#1659263

FromAndy Lutomirski <luto@amacapital.net>
Date2017-06-07 00:40 +0200
Message-ID<tPviO-6ua-13@gated-at.bofh.it>
In reply to#1658330
On Mon, Jun 5, 2017 at 8:16 PM, Andy Lutomirski <luto@kernel.org> wrote:
> +static ssize_t
> +read_bmof(struct file *filp, struct kobject *kobj,
> +        struct bin_attribute *attr,
> +        char *buf, loff_t off, size_t count)
> +{
> +       struct bmof_priv *priv =
> +               container_of(attr, struct bmof_priv, bmof_bin_attr);
> +
> +       if (off >= priv->bmofdata->buffer.length)
> +               return 0;
> +
> +       if (count > priv->bmofdata->buffer.length - off)
> +               count = priv->bmofdata->buffer.length - off;
> +
> +       memcpy(buf, priv->bmofdata->buffer.pointer + off, count);
> +       return count;
> +}

I just discovered simple_read_from_buffer().  I think this whole
function could be:

struct bmof_priv *priv = ...;
return simple_read_from_buffer(buf, count, &off,
priv->bmofdata->buffer.pointer, priv->bmofdata->buffer.length);

--Andy

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web