From: Oliver Schinagl <[email protected]>
Hopefully it is polished enough for inclusion now :)
Oliver
Changes from v5:
* Added dts for A10s
* Move ABI documentation to testing
* Improve documentation overal
* Expand comments about future changes for when Greg fixes sysfs bin attrs
* Rename sun7i-sid to sun7i-a20-sid for the compatible field
Changes from v4:
* Added sun7i (A20) support, also tested on Cubieboard 2.0 and OlinuXino A20
* prepare source for move to binary attribute groups
* Removed useless internal braces
* Dropped DRV_NAME from dvb_dvb
* Add and pass struct using platform_[gs]et_drvdata()
* Add Documentation!
* Reviewed-by: Tomasz Figa <[email protected]>
Changes from v3:
* Cleanup comments
* Remove last byte masking and useless casting, the C standard guarntees
we are ok
* Removed some complexity from sid_read, thanks to Russel
* Replace dev_info with dev_dbg reducing the verbosity
* Removed driver version
* Reorderd variable declrations based on usage, return value always last
* Removed all goto in exchange for return, due to popular request
* Reduced line count by removing extra lines
Changes from v2:
* Removed the global pointer, we can change that when the need for external
access arises
* Fixed header inclusions
* Corrected if guards. There where some crude mistakes there
* Changed offset to an unsigned int so we don't have to worry about negatives
* Cleaned up variable declarations
* Changed ret value, ENXIO (No device/io) as that better matches a missing dt
* Made the loading informercial print version so it is somewhat usefull
Changes from v1:
* Rename the sys-fs exported key to eeprom, since it really a read-only eeprom
* Removed mention of sun[67]i since we haven't tested those
* Fixed up mistakes in comments
* Removed PAGE_SIZE references, since this is a binary only driver
* Removed lookup table and calculate offsets better
* Use proper endianess
* Add the SID to seed the kernel entropy pool
* Rewrite probe to use platform_get_resource/devm_ioremap_resource instead
The Allwinner A-series of SoC's have efuses exposed via registers to read the
factory programmed e-fuses. These should in theory be programmable but this is
still to be confirmed. It does appear that these fuses are unique enough to be
used as serial numbers, RSA keys, generate MAC addresses from etc. If it turns
out to be user programmable, the use obviously increases. Allwinner did use the
fuses initially to determine the chip-type.
This driver supports all currently known chips based on datasheets and 'dumped'
drivers that we have so far, the dts is only implemented for known chips.
It has been tested on a Cubieboard 1
This is my very first driver so please try to be gentle
Oliver Schinagl (2):
ARM: sunxi: Initial support for Allwinner's Security ID fuses
ARM: sunxi: dt: Add sunxi-sid to dts for sun4i, sun5i and sun7i
Documentation/ABI/testing/sysfs-driver-sunxi-sid | 22 +++
.../bindings/misc/allwinner,sunxi-sid.txt | 17 ++
arch/arm/boot/dts/sun4i-a10.dtsi | 5 +
arch/arm/boot/dts/sun5i-a10s.dtsi | 5 +
arch/arm/boot/dts/sun5i-a13.dtsi | 5 +
arch/arm/boot/dts/sun7i-a20.dtsi | 5 +
drivers/misc/eeprom/Kconfig | 13 ++
drivers/misc/eeprom/Makefile | 1 +
drivers/misc/eeprom/sunxi_sid.c | 177 +++++++++++++++++++++
9 files changed, 250 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-driver-sunxi-sid
create mode 100644 Documentation/devicetree/bindings/misc/allwinner,sunxi-sid.txt
create mode 100644 drivers/misc/eeprom/sunxi_sid.c
--
1.8.1.5
From: Oliver Schinagl <[email protected]>
Allwinner has electric fuses (efuse) on their line of chips. This driver
reads those fuses, seeds the kernel entropy and exports them as a sysfs
node.
These fuses are most likely to be programmed at the factory, encoding
things like Chip ID, some sort of serial number etc and appear to be
reasonable unique.
While in theory, these should be writeable by the user, it will probably
be inconvinient to do so. Allwinner recommends that a certain input pin,
labeled 'efuse_vddq', be connected to GND. To write these fuses however,
a 2.5 V programming voltage needs to be applied to this pin.
Even so, they can still be used to generate a board-unique mac from,
board unique RSA key and seed the kernel RNG.
On sun7i additional storage is available, this is initially used for an
UEFI BOOT key, Secure JTAG key, HDMI-HDCP key and vendor specific keys.
Currently supported are the following known chips:
Allwinner sun4i (A10)
Allwinner sun5i (A10s, A13)
Allwinner sun7i (A20)
Signed-off-by: Oliver Schinagl <[email protected]>
---
Documentation/ABI/testing/sysfs-driver-sunxi-sid | 22 +++
.../bindings/misc/allwinner,sunxi-sid.txt | 17 ++
drivers/misc/eeprom/Kconfig | 13 ++
drivers/misc/eeprom/Makefile | 1 +
drivers/misc/eeprom/sunxi_sid.c | 177 +++++++++++++++++++++
5 files changed, 230 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-driver-sunxi-sid
create mode 100644 Documentation/devicetree/bindings/misc/allwinner,sunxi-sid.txt
create mode 100644 drivers/misc/eeprom/sunxi_sid.c
diff --git a/Documentation/ABI/testing/sysfs-driver-sunxi-sid b/Documentation/ABI/testing/sysfs-driver-sunxi-sid
new file mode 100644
index 0000000..ffb9536
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-sunxi-sid
@@ -0,0 +1,22 @@
+What: /sys/devices/*/<our-device>/eeprom
+Date: August 2013
+Contact: Oliver Schinagl <[email protected]>
+Description: read-only access to the SID (Security-ID) on current
+ A-series SoC's from Allwinner. Currently supports A10, A10s, A13
+ and A20 CPU's. The earlier A1x series of SoCs exports 16 bytes,
+ whereas the newer A20 SoC exposes 512 bytes split into sections.
+ Besides the 16 bytes of SID, there's also an SJTAG area,
+ HDMI-HDCP key and some custom keys. Below a quick overview, for
+ details see the user manual:
+ 0x000 128 bit root-key (sun[457]i)
+ 0x010 128 bit boot-key (sun7i)
+ 0x020 64 bit security-jtag-key (sun7i)
+ 0x028 16 bit key configuration (sun7i)
+ 0x02b 16 bit custom-vendor-key (sun7i)
+ 0x02c 320 bit low general key (sun7i)
+ 0x040 32 bit read-control access (sun7i)
+ 0x064 224 bit low general key (sun7i)
+ 0x080 2304 bit HDCP-key (sun7i)
+ 0x1a0 768 bit high general key (sun7i)
+Users: any user space application which wants to read the SID on
+ Allwinner's A-series of CPU's.
diff --git a/Documentation/devicetree/bindings/misc/allwinner,sunxi-sid.txt b/Documentation/devicetree/bindings/misc/allwinner,sunxi-sid.txt
new file mode 100644
index 0000000..68ba372
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/allwinner,sunxi-sid.txt
@@ -0,0 +1,17 @@
+Allwinner sunxi-sid
+
+Required properties:
+- compatible: "allwinner,sun4i-sid" or "allwinner,sun7i-a20-sid".
+- reg: Should contain registers location and length
+
+Example for sun4i:
+ sid@01c23800 {
+ compatible = "allwinner,sun4i-sid";
+ reg = <0x01c23800 0x10>
+ };
+
+Example for sun7i:
+ sid@01c23800 {
+ compatible = "allwinner,sun7i-a20-sid";
+ reg = <0x01c23800 0x200>
+ };
diff --git a/drivers/misc/eeprom/Kconfig b/drivers/misc/eeprom/Kconfig
index 04f2e1f..9536852f 100644
--- a/drivers/misc/eeprom/Kconfig
+++ b/drivers/misc/eeprom/Kconfig
@@ -96,4 +96,17 @@ config EEPROM_DIGSY_MTC_CFG
If unsure, say N.
+config EEPROM_SUNXI_SID
+ tristate "Allwinner sunxi security ID support"
+ depends on ARCH_SUNXI && SYSFS
+ help
+ This is a driver for the 'security ID' available on various Allwinner
+ devices.
+
+ Due to the potential risks involved with changing e-fuses,
+ this driver is read-only.
+
+ This driver can also be built as a module. If so, the module
+ will be called sunxi_sid.
+
endmenu
diff --git a/drivers/misc/eeprom/Makefile b/drivers/misc/eeprom/Makefile
index fc1e81d..9507aec 100644
--- a/drivers/misc/eeprom/Makefile
+++ b/drivers/misc/eeprom/Makefile
@@ -4,4 +4,5 @@ obj-$(CONFIG_EEPROM_LEGACY) += eeprom.o
obj-$(CONFIG_EEPROM_MAX6875) += max6875.o
obj-$(CONFIG_EEPROM_93CX6) += eeprom_93cx6.o
obj-$(CONFIG_EEPROM_93XX46) += eeprom_93xx46.o
+obj-$(CONFIG_EEPROM_SUNXI_SID) += sunxi_sid.o
obj-$(CONFIG_EEPROM_DIGSY_MTC_CFG) += digsy_mtc_eeprom.o
diff --git a/drivers/misc/eeprom/sunxi_sid.c b/drivers/misc/eeprom/sunxi_sid.c
new file mode 100644
index 0000000..39a9a62
--- /dev/null
+++ b/drivers/misc/eeprom/sunxi_sid.c
@@ -0,0 +1,177 @@
+/*
+ * Copyright (c) 2013 Oliver Schinagl <[email protected]>
+ * http://www.linux-sunxi.org
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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.
+ *
+ * This driver exposes the Allwinner security ID, efuses exported in byte-
+ * sized chunks.
+ */
+
+#include <linux/compiler.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/export.h>
+#include <linux/fs.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/kobject.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/random.h>
+#include <linux/slab.h>
+#include <linux/stat.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
+
+#define DRV_NAME "sunxi-sid"
+
+struct sunxi_sid_data {
+ void __iomem *reg_base;
+ unsigned int keysize;
+};
+
+/* We read the entire key, due to a 32 bit read alignment requirement. Since we
+ * want to return the requested byte, this resuls in somewhat slower code and
+ * uses 4 times more reads as needed but keeps code simpler. Since the SID is
+ * only very rarly probed, this is not really an issue.
+ */
+static u8 sunxi_sid_read_byte(const struct sunxi_sid_data *sid_data,
+ const unsigned int offset)
+{
+ u32 sid_key;
+
+ if (offset >= sid_data->keysize)
+ return 0;
+
+ sid_key = ioread32be(sid_data->reg_base + round_down(offset, 4));
+ sid_key >>= (offset % 4) * 8;
+
+ return sid_key; /* Only return the last byte */
+}
+
+static ssize_t sid_read(struct file *fd, struct kobject *kobj,
+ struct bin_attribute *attr, char *buf,
+ loff_t pos, size_t size)
+{
+ struct platform_device *pdev;
+ struct sunxi_sid_data *sid_data;
+ int i;
+
+ pdev = to_platform_device(kobj_to_dev(kobj));
+ sid_data = platform_get_drvdata(pdev);
+
+ if (pos < 0 || pos >= sid_data->keysize)
+ return 0;
+ if (size > sid_data->keysize - pos)
+ size = sid_data->keysize - pos;
+
+ for (i = 0; i < size; i++)
+ buf[i] = sunxi_sid_read_byte(sid_data, pos + i);
+
+ return i;
+}
+
+static struct bin_attribute sid_bin_attr = {
+ .attr = { .name = "eeprom", .mode = S_IRUGO, },
+ .read = sid_read,
+};
+
+static struct bin_attribute *sunxi_sid_bin_attrs[] = {
+ &sid_bin_attr,
+ NULL,
+};
+
+static const struct attribute_group sunxi_sid_group = {
+ .bin_attrs = sunxi_sid_bin_attrs,
+};
+
+/* fixme: Unused struct to be used with proper sysfs bin_attr groups */
+static const struct attribute_group *sunxi_sid_groups[] = {
+ &sunxi_sid_group,
+ NULL,
+};
+
+static int sunxi_sid_remove(struct platform_device *pdev)
+{
+ struct sunxi_sid_data *sid_data;
+
+ device_remove_bin_file(&pdev->dev, &sid_bin_attr); /* fixme: remove when using sysfs bin attrs */
+ sid_data = platform_get_drvdata(pdev);
+ dev_dbg(&pdev->dev, "driver unloaded\n");
+
+ return 0;
+}
+
+static const struct of_device_id sunxi_sid_of_match[] = {
+ { .compatible = "allwinner,sun4i-sid", .data = (void *)16},
+ { .compatible = "allwinner,sun7i-a20-sid", .data = (void *)512},
+ {/* sentinel */},
+};
+MODULE_DEVICE_TABLE(of, sunxi_sid_of_match);
+
+static int __init sunxi_sid_probe(struct platform_device *pdev)
+{
+ struct sunxi_sid_data *sid_data;
+ struct resource *res;
+ const struct of_device_id *of_dev_id;
+ u8 *entropy;
+ unsigned int i;
+
+ sid_data = devm_kzalloc(&pdev->dev, sizeof(struct sunxi_sid_data),
+ GFP_KERNEL);
+ if (!sid_data)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ sid_data->reg_base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(sid_data->reg_base))
+ return PTR_ERR(sid_data->reg_base);
+
+ of_dev_id = of_match_device(sunxi_sid_of_match, &pdev->dev);
+ if (!of_dev_id)
+ return -ENODEV;
+ sid_data->keysize = (int)of_dev_id->data;
+
+ platform_set_drvdata(pdev, sid_data);
+
+ sid_bin_attr.size = sid_data->keysize; /* fixme: this should be properly set by the sysfs bin attr groups later */
+ if (device_create_bin_file(&pdev->dev, &sid_bin_attr)) /* fixme: this will need to be removed when using sysfs bin attr groups */
+ return -ENODEV;
+
+ entropy = kzalloc(sizeof(u8) * sid_data->keysize, GFP_KERNEL);
+ for (i = 0; i < sid_data->keysize; i++)
+ entropy[i] = sunxi_sid_read_byte(sid_data, i);
+ add_device_randomness(entropy, sid_data->keysize);
+ kfree(entropy);
+
+ dev_dbg(&pdev->dev, "loaded\n");
+
+ return 0;
+}
+
+static struct platform_driver sunxi_sid_driver = {
+ .probe = sunxi_sid_probe,
+ .remove = sunxi_sid_remove,
+ .driver = {
+ .name = DRV_NAME,
+ .owner = THIS_MODULE,
+ .of_match_table = sunxi_sid_of_match,
+ /* .groups = sunxi_sid_groups, fixme: this is the proper way using bin attr groups */
+ },
+};
+module_platform_driver(sunxi_sid_driver);
+
+MODULE_AUTHOR("Oliver Schinagl <[email protected]>");
+MODULE_DESCRIPTION("Allwinner sunxi security id driver");
+MODULE_LICENSE("GPL");
--
1.8.1.5
From: Oliver Schinagl <[email protected]>
This patch shall add support for the sunxi-sid driver to the device
tree for A10, A10s, A13 and A20.
Signed-off-by: Oliver Schinagl <[email protected]>
---
arch/arm/boot/dts/sun4i-a10.dtsi | 5 +++++
arch/arm/boot/dts/sun5i-a10s.dtsi | 5 +++++
arch/arm/boot/dts/sun5i-a13.dtsi | 5 +++++
arch/arm/boot/dts/sun7i-a20.dtsi | 5 +++++
4 files changed, 20 insertions(+)
diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index b2bd6e1..179e024 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -267,6 +267,11 @@
reg = <0x01c20c90 0x10>;
};
+ sid: eeprom@01c23800 {
+ compatible = "allwinner,sun4i-sid";
+ reg = <0x01c23800 0x10>;
+ };
+
uart0: serial@01c28000 {
compatible = "snps,dw-apb-uart";
reg = <0x01c28000 0x400>;
diff --git a/arch/arm/boot/dts/sun5i-a10s.dtsi b/arch/arm/boot/dts/sun5i-a10s.dtsi
index b8fc1c2..724285cc 100644
--- a/arch/arm/boot/dts/sun5i-a10s.dtsi
+++ b/arch/arm/boot/dts/sun5i-a10s.dtsi
@@ -256,6 +256,11 @@
reg = <0x01c20c90 0x10>;
};
+ sid: eeprom@01c23800 {
+ compatible = "allwinner,sun4i-sid";
+ reg = <0x01c23800 0x10>;
+ };
+
uart0: serial@01c28000 {
compatible = "snps,dw-apb-uart";
reg = <0x01c28000 0x400>;
diff --git a/arch/arm/boot/dts/sun5i-a13.dtsi b/arch/arm/boot/dts/sun5i-a13.dtsi
index 7363211..999fdb3 100644
--- a/arch/arm/boot/dts/sun5i-a13.dtsi
+++ b/arch/arm/boot/dts/sun5i-a13.dtsi
@@ -223,6 +223,11 @@
reg = <0x01c20c90 0x10>;
};
+ sid: eeprom@01c23800 {
+ compatible = "allwinner,sun4i-sid";
+ reg = <0x01c23800 0x10>;
+ };
+
uart1: serial@01c28400 {
compatible = "snps,dw-apb-uart";
reg = <0x01c28400 0x400>;
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index fb81e78..ff3cc90 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -217,6 +217,11 @@
reg = <0x01c20c90 0x10>;
};
+ sid: eeprom@01c23800 {
+ compatible = "allwinner,sun7i-a20-sid";
+ reg = <0x01c23800 0x200>;
+ };
+
uart0: serial@01c28000 {
compatible = "snps,dw-apb-uart";
reg = <0x01c28000 0x400>;
--
1.8.1.5
Hi Oliver,
Most of my comments on this are nitpicks, overall it looks good to me
El 01/09/13 08:30, [email protected] escribi?:
> From: Oliver Schinagl <[email protected]>
>
> Allwinner has electric fuses (efuse) on their line of chips. This driver
> reads those fuses, seeds the kernel entropy and exports them as a sysfs
> node.
>
> These fuses are most likely to be programmed at the factory, encoding
> things like Chip ID, some sort of serial number etc and appear to be
"..number, etc. and appear.."
> reasonable unique.
reasonably
> While in theory, these should be writeable by the user, it will probably
> be inconvinient to do so. Allwinner recommends that a certain input pin,
inconvenient
> labeled 'efuse_vddq', be connected to GND. To write these fuses however,
> a 2.5 V programming voltage needs to be applied to this pin.
>
> Even so, they can still be used to generate a board-unique mac from,
> board unique RSA key and seed the kernel RNG.
>
> On sun7i additional storage is available, this is initially used for an
> UEFI BOOT key, Secure JTAG key, HDMI-HDCP key and vendor specific keys.
>
> Currently supported are the following known chips:
> Allwinner sun4i (A10)
> Allwinner sun5i (A10s, A13)
> Allwinner sun7i (A20)
>
> Signed-off-by: Oliver Schinagl <[email protected]>
> ---
> Documentation/ABI/testing/sysfs-driver-sunxi-sid | 22 +++
> .../bindings/misc/allwinner,sunxi-sid.txt | 17 ++
> drivers/misc/eeprom/Kconfig | 13 ++
> drivers/misc/eeprom/Makefile | 1 +
> drivers/misc/eeprom/sunxi_sid.c | 177 +++++++++++++++++++++
> 5 files changed, 230 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-driver-sunxi-sid
> create mode 100644 Documentation/devicetree/bindings/misc/allwinner,sunxi-sid.txt
> create mode 100644 drivers/misc/eeprom/sunxi_sid.c
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-sunxi-sid b/Documentation/ABI/testing/sysfs-driver-sunxi-sid
> new file mode 100644
> index 0000000..ffb9536
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-sunxi-sid
> @@ -0,0 +1,22 @@
> +What: /sys/devices/*/<our-device>/eeprom
> +Date: August 2013
> +Contact: Oliver Schinagl <[email protected]>
> +Description: read-only access to the SID (Security-ID) on current
> + A-series SoC's from Allwinner. Currently supports A10, A10s, A13
> + and A20 CPU's. The earlier A1x series of SoCs exports 16 bytes,
> + whereas the newer A20 SoC exposes 512 bytes split into sections.
> + Besides the 16 bytes of SID, there's also an SJTAG area,
> + HDMI-HDCP key and some custom keys. Below a quick overview, for
> + details see the user manual:
> + 0x000 128 bit root-key (sun[457]i)
> + 0x010 128 bit boot-key (sun7i)
> + 0x020 64 bit security-jtag-key (sun7i)
> + 0x028 16 bit key configuration (sun7i)
> + 0x02b 16 bit custom-vendor-key (sun7i)
> + 0x02c 320 bit low general key (sun7i)
> + 0x040 32 bit read-control access (sun7i)
> + 0x064 224 bit low general key (sun7i)
> + 0x080 2304 bit HDCP-key (sun7i)
> + 0x1a0 768 bit high general key (sun7i)
> +Users: any user space application which wants to read the SID on
> + Allwinner's A-series of CPU's.
> diff --git a/Documentation/devicetree/bindings/misc/allwinner,sunxi-sid.txt b/Documentation/devicetree/bindings/misc/allwinner,sunxi-sid.txt
> new file mode 100644
> index 0000000..68ba372
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/allwinner,sunxi-sid.txt
> @@ -0,0 +1,17 @@
> +Allwinner sunxi-sid
> +
> +Required properties:
> +- compatible: "allwinner,sun4i-sid" or "allwinner,sun7i-a20-sid".
> +- reg: Should contain registers location and length
> +
> +Example for sun4i:
> + sid@01c23800 {
> + compatible = "allwinner,sun4i-sid";
> + reg = <0x01c23800 0x10>
> + };
> +
> +Example for sun7i:
> + sid@01c23800 {
> + compatible = "allwinner,sun7i-a20-sid";
> + reg = <0x01c23800 0x200>
> + };
> diff --git a/drivers/misc/eeprom/Kconfig b/drivers/misc/eeprom/Kconfig
> index 04f2e1f..9536852f 100644
> --- a/drivers/misc/eeprom/Kconfig
> +++ b/drivers/misc/eeprom/Kconfig
> @@ -96,4 +96,17 @@ config EEPROM_DIGSY_MTC_CFG
>
> If unsure, say N.
>
> +config EEPROM_SUNXI_SID
> + tristate "Allwinner sunxi security ID support"
> + depends on ARCH_SUNXI && SYSFS
> + help
> + This is a driver for the 'security ID' available on various Allwinner
> + devices.
> +
> + Due to the potential risks involved with changing e-fuses,
> + this driver is read-only.
> +
> + This driver can also be built as a module. If so, the module
> + will be called sunxi_sid.
> +
> endmenu
> diff --git a/drivers/misc/eeprom/Makefile b/drivers/misc/eeprom/Makefile
> index fc1e81d..9507aec 100644
> --- a/drivers/misc/eeprom/Makefile
> +++ b/drivers/misc/eeprom/Makefile
> @@ -4,4 +4,5 @@ obj-$(CONFIG_EEPROM_LEGACY) += eeprom.o
> obj-$(CONFIG_EEPROM_MAX6875) += max6875.o
> obj-$(CONFIG_EEPROM_93CX6) += eeprom_93cx6.o
> obj-$(CONFIG_EEPROM_93XX46) += eeprom_93xx46.o
> +obj-$(CONFIG_EEPROM_SUNXI_SID) += sunxi_sid.o
> obj-$(CONFIG_EEPROM_DIGSY_MTC_CFG) += digsy_mtc_eeprom.o
> diff --git a/drivers/misc/eeprom/sunxi_sid.c b/drivers/misc/eeprom/sunxi_sid.c
> new file mode 100644
> index 0000000..39a9a62
> --- /dev/null
> +++ b/drivers/misc/eeprom/sunxi_sid.c
> @@ -0,0 +1,177 @@
> +/*
> + * Copyright (c) 2013 Oliver Schinagl <[email protected]>
> + * http://www.linux-sunxi.org
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * 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.
> + *
> + * This driver exposes the Allwinner security ID, efuses exported in byte-
> + * sized chunks.
> + */
> +
> +#include <linux/compiler.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/export.h>
> +#include <linux/fs.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/kobject.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/random.h>
> +#include <linux/slab.h>
> +#include <linux/stat.h>
> +#include <linux/sysfs.h>
> +#include <linux/types.h>
> +
> +#define DRV_NAME "sunxi-sid"
> +
> +struct sunxi_sid_data {
> + void __iomem *reg_base;
> + unsigned int keysize;
> +};
> +
> +/* We read the entire key, due to a 32 bit read alignment requirement. Since we
> + * want to return the requested byte, this resuls in somewhat slower code and
> + * uses 4 times more reads as needed but keeps code simpler. Since the SID is
> + * only very rarly probed, this is not really an issue.
rarely
> + */
> +static u8 sunxi_sid_read_byte(const struct sunxi_sid_data *sid_data,
> + const unsigned int offset)
> +{
> + u32 sid_key;
> +
> + if (offset >= sid_data->keysize)
> + return 0;
> +
> + sid_key = ioread32be(sid_data->reg_base + round_down(offset, 4));
> + sid_key >>= (offset % 4) * 8;
> +
> + return sid_key; /* Only return the last byte */
> +}
> +
> +static ssize_t sid_read(struct file *fd, struct kobject *kobj,
> + struct bin_attribute *attr, char *buf,
> + loff_t pos, size_t size)
> +{
> + struct platform_device *pdev;
> + struct sunxi_sid_data *sid_data;
> + int i;
> +
> + pdev = to_platform_device(kobj_to_dev(kobj));
> + sid_data = platform_get_drvdata(pdev);
> +
> + if (pos < 0 || pos >= sid_data->keysize)
> + return 0;
> + if (size > sid_data->keysize - pos)
> + size = sid_data->keysize - pos;
> +
> + for (i = 0; i < size; i++)
> + buf[i] = sunxi_sid_read_byte(sid_data, pos + i);
> +
> + return i;
> +}
> +
> +static struct bin_attribute sid_bin_attr = {
> + .attr = { .name = "eeprom", .mode = S_IRUGO, },
> + .read = sid_read,
> +};
> +
> +static struct bin_attribute *sunxi_sid_bin_attrs[] = {
> + &sid_bin_attr,
> + NULL,
> +};
> +
> +static const struct attribute_group sunxi_sid_group = {
> + .bin_attrs = sunxi_sid_bin_attrs,
> +};
> +
> +/* fixme: Unused struct to be used with proper sysfs bin_attr groups */
> +static const struct attribute_group *sunxi_sid_groups[] = {
> + &sunxi_sid_group,
> + NULL,
> +};
> +
> +static int sunxi_sid_remove(struct platform_device *pdev)
> +{
> + struct sunxi_sid_data *sid_data;
> +
> + device_remove_bin_file(&pdev->dev, &sid_bin_attr); /* fixme: remove when using sysfs bin attrs */
> + sid_data = platform_get_drvdata(pdev);
> + dev_dbg(&pdev->dev, "driver unloaded\n");
> +
> + return 0;
> +}
> +
> +static const struct of_device_id sunxi_sid_of_match[] = {
> + { .compatible = "allwinner,sun4i-sid", .data = (void *)16},
> + { .compatible = "allwinner,sun7i-a20-sid", .data = (void *)512},
> + {/* sentinel */},
> +};
> +MODULE_DEVICE_TABLE(of, sunxi_sid_of_match);
> +
> +static int __init sunxi_sid_probe(struct platform_device *pdev)
> +{
> + struct sunxi_sid_data *sid_data;
> + struct resource *res;
> + const struct of_device_id *of_dev_id;
> + u8 *entropy;
> + unsigned int i;
> +
> + sid_data = devm_kzalloc(&pdev->dev, sizeof(struct sunxi_sid_data),
> + GFP_KERNEL);
> + if (!sid_data)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + sid_data->reg_base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(sid_data->reg_base))
> + return PTR_ERR(sid_data->reg_base);
> +
> + of_dev_id = of_match_device(sunxi_sid_of_match, &pdev->dev);
> + if (!of_dev_id)
> + return -ENODEV;
> + sid_data->keysize = (int)of_dev_id->data;
> +
> + platform_set_drvdata(pdev, sid_data);
> +
> + sid_bin_attr.size = sid_data->keysize; /* fixme: this should be properly set by the sysfs bin attr groups later */
> + if (device_create_bin_file(&pdev->dev, &sid_bin_attr)) /* fixme: this will need to be removed when using sysfs bin attr groups */
> + return -ENODEV;
> +
> + entropy = kzalloc(sizeof(u8) * sid_data->keysize, GFP_KERNEL);
> + for (i = 0; i < sid_data->keysize; i++)
> + entropy[i] = sunxi_sid_read_byte(sid_data, i);
> + add_device_randomness(entropy, sid_data->keysize);
> + kfree(entropy);
> +
> + dev_dbg(&pdev->dev, "loaded\n");
> +
> + return 0;
> +}
> +
> +static struct platform_driver sunxi_sid_driver = {
> + .probe = sunxi_sid_probe,
> + .remove = sunxi_sid_remove,
> + .driver = {
> + .name = DRV_NAME,
> + .owner = THIS_MODULE,
> + .of_match_table = sunxi_sid_of_match,
> + /* .groups = sunxi_sid_groups, fixme: this is the proper way using bin attr groups */
> + },
> +};
> +module_platform_driver(sunxi_sid_driver);
As the probe function is marked __init, and this is a non-hotpluggable
device, I think you should use module_platform_driver_probe(...) instead
of module_platform_driver(...)
> +
> +MODULE_AUTHOR("Oliver Schinagl <[email protected]>");
> +MODULE_DESCRIPTION("Allwinner sunxi security id driver");
> +MODULE_LICENSE("GPL");
>
Thanks for working on this!
Emilio
Hi Oliver,
On Sun, Sep 01, 2013 at 01:30:18PM +0200, [email protected] wrote:
> From: Oliver Schinagl <[email protected]>
>
> Allwinner has electric fuses (efuse) on their line of chips. This driver
> reads those fuses, seeds the kernel entropy and exports them as a sysfs
> node.
>
> These fuses are most likely to be programmed at the factory, encoding
> things like Chip ID, some sort of serial number etc and appear to be
> reasonable unique.
> While in theory, these should be writeable by the user, it will probably
> be inconvinient to do so. Allwinner recommends that a certain input pin,
> labeled 'efuse_vddq', be connected to GND. To write these fuses however,
> a 2.5 V programming voltage needs to be applied to this pin.
>
> Even so, they can still be used to generate a board-unique mac from,
> board unique RSA key and seed the kernel RNG.
>
> On sun7i additional storage is available, this is initially used for an
> UEFI BOOT key, Secure JTAG key, HDMI-HDCP key and vendor specific keys.
>
> Currently supported are the following known chips:
> Allwinner sun4i (A10)
> Allwinner sun5i (A10s, A13)
> Allwinner sun7i (A20)
>
> Signed-off-by: Oliver Schinagl <[email protected]>
> ---
> Documentation/ABI/testing/sysfs-driver-sunxi-sid | 22 +++
> .../bindings/misc/allwinner,sunxi-sid.txt | 17 ++
> drivers/misc/eeprom/Kconfig | 13 ++
> drivers/misc/eeprom/Makefile | 1 +
> drivers/misc/eeprom/sunxi_sid.c | 177 +++++++++++++++++++++
> 5 files changed, 230 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-driver-sunxi-sid
> create mode 100644 Documentation/devicetree/bindings/misc/allwinner,sunxi-sid.txt
> create mode 100644 drivers/misc/eeprom/sunxi_sid.c
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-sunxi-sid b/Documentation/ABI/testing/sysfs-driver-sunxi-sid
> new file mode 100644
> index 0000000..ffb9536
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-sunxi-sid
> @@ -0,0 +1,22 @@
> +What: /sys/devices/*/<our-device>/eeprom
> +Date: August 2013
> +Contact: Oliver Schinagl <[email protected]>
> +Description: read-only access to the SID (Security-ID) on current
> + A-series SoC's from Allwinner. Currently supports A10, A10s, A13
> + and A20 CPU's. The earlier A1x series of SoCs exports 16 bytes,
> + whereas the newer A20 SoC exposes 512 bytes split into sections.
> + Besides the 16 bytes of SID, there's also an SJTAG area,
> + HDMI-HDCP key and some custom keys. Below a quick overview, for
> + details see the user manual:
> + 0x000 128 bit root-key (sun[457]i)
> + 0x010 128 bit boot-key (sun7i)
> + 0x020 64 bit security-jtag-key (sun7i)
> + 0x028 16 bit key configuration (sun7i)
> + 0x02b 16 bit custom-vendor-key (sun7i)
> + 0x02c 320 bit low general key (sun7i)
> + 0x040 32 bit read-control access (sun7i)
> + 0x064 224 bit low general key (sun7i)
> + 0x080 2304 bit HDCP-key (sun7i)
> + 0x1a0 768 bit high general key (sun7i)
> +Users: any user space application which wants to read the SID on
> + Allwinner's A-series of CPU's.
> diff --git a/Documentation/devicetree/bindings/misc/allwinner,sunxi-sid.txt b/Documentation/devicetree/bindings/misc/allwinner,sunxi-sid.txt
> new file mode 100644
> index 0000000..68ba372
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/allwinner,sunxi-sid.txt
> @@ -0,0 +1,17 @@
> +Allwinner sunxi-sid
> +
> +Required properties:
> +- compatible: "allwinner,sun4i-sid" or "allwinner,sun7i-a20-sid".
> +- reg: Should contain registers location and length
> +
> +Example for sun4i:
> + sid@01c23800 {
> + compatible = "allwinner,sun4i-sid";
> + reg = <0x01c23800 0x10>
> + };
> +
> +Example for sun7i:
> + sid@01c23800 {
> + compatible = "allwinner,sun7i-a20-sid";
> + reg = <0x01c23800 0x200>
> + };
> diff --git a/drivers/misc/eeprom/Kconfig b/drivers/misc/eeprom/Kconfig
> index 04f2e1f..9536852f 100644
> --- a/drivers/misc/eeprom/Kconfig
> +++ b/drivers/misc/eeprom/Kconfig
> @@ -96,4 +96,17 @@ config EEPROM_DIGSY_MTC_CFG
>
> If unsure, say N.
>
> +config EEPROM_SUNXI_SID
> + tristate "Allwinner sunxi security ID support"
> + depends on ARCH_SUNXI && SYSFS
> + help
> + This is a driver for the 'security ID' available on various Allwinner
> + devices.
> +
> + Due to the potential risks involved with changing e-fuses,
> + this driver is read-only.
> +
> + This driver can also be built as a module. If so, the module
> + will be called sunxi_sid.
> +
> endmenu
> diff --git a/drivers/misc/eeprom/Makefile b/drivers/misc/eeprom/Makefile
> index fc1e81d..9507aec 100644
> --- a/drivers/misc/eeprom/Makefile
> +++ b/drivers/misc/eeprom/Makefile
> @@ -4,4 +4,5 @@ obj-$(CONFIG_EEPROM_LEGACY) += eeprom.o
> obj-$(CONFIG_EEPROM_MAX6875) += max6875.o
> obj-$(CONFIG_EEPROM_93CX6) += eeprom_93cx6.o
> obj-$(CONFIG_EEPROM_93XX46) += eeprom_93xx46.o
> +obj-$(CONFIG_EEPROM_SUNXI_SID) += sunxi_sid.o
> obj-$(CONFIG_EEPROM_DIGSY_MTC_CFG) += digsy_mtc_eeprom.o
> diff --git a/drivers/misc/eeprom/sunxi_sid.c b/drivers/misc/eeprom/sunxi_sid.c
> new file mode 100644
> index 0000000..39a9a62
> --- /dev/null
> +++ b/drivers/misc/eeprom/sunxi_sid.c
> @@ -0,0 +1,177 @@
> +/*
> + * Copyright (c) 2013 Oliver Schinagl <[email protected]>
> + * http://www.linux-sunxi.org
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * 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.
> + *
> + * This driver exposes the Allwinner security ID, efuses exported in byte-
> + * sized chunks.
> + */
> +
> +#include <linux/compiler.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/export.h>
> +#include <linux/fs.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/kobject.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/random.h>
> +#include <linux/slab.h>
> +#include <linux/stat.h>
> +#include <linux/sysfs.h>
> +#include <linux/types.h>
> +
> +#define DRV_NAME "sunxi-sid"
> +
> +struct sunxi_sid_data {
> + void __iomem *reg_base;
> + unsigned int keysize;
> +};
> +
> +/* We read the entire key, due to a 32 bit read alignment requirement. Since we
> + * want to return the requested byte, this resuls in somewhat slower code and
> + * uses 4 times more reads as needed but keeps code simpler. Since the SID is
> + * only very rarly probed, this is not really an issue.
> + */
> +static u8 sunxi_sid_read_byte(const struct sunxi_sid_data *sid_data,
> + const unsigned int offset)
> +{
> + u32 sid_key;
> +
> + if (offset >= sid_data->keysize)
> + return 0;
> +
> + sid_key = ioread32be(sid_data->reg_base + round_down(offset, 4));
> + sid_key >>= (offset % 4) * 8;
> +
> + return sid_key; /* Only return the last byte */
> +}
> +
> +static ssize_t sid_read(struct file *fd, struct kobject *kobj,
> + struct bin_attribute *attr, char *buf,
> + loff_t pos, size_t size)
> +{
> + struct platform_device *pdev;
> + struct sunxi_sid_data *sid_data;
> + int i;
> +
> + pdev = to_platform_device(kobj_to_dev(kobj));
> + sid_data = platform_get_drvdata(pdev);
> +
> + if (pos < 0 || pos >= sid_data->keysize)
> + return 0;
> + if (size > sid_data->keysize - pos)
> + size = sid_data->keysize - pos;
> +
> + for (i = 0; i < size; i++)
> + buf[i] = sunxi_sid_read_byte(sid_data, pos + i);
> +
> + return i;
> +}
> +
> +static struct bin_attribute sid_bin_attr = {
> + .attr = { .name = "eeprom", .mode = S_IRUGO, },
> + .read = sid_read,
> +};
> +
> +static struct bin_attribute *sunxi_sid_bin_attrs[] = {
> + &sid_bin_attr,
> + NULL,
> +};
> +
> +static const struct attribute_group sunxi_sid_group = {
> + .bin_attrs = sunxi_sid_bin_attrs,
> +};
> +
> +/* fixme: Unused struct to be used with proper sysfs bin_attr groups */
> +static const struct attribute_group *sunxi_sid_groups[] = {
> + &sunxi_sid_group,
> + NULL,
> +};
> +
I don't see it used afterwards. Is it?
> +static int sunxi_sid_remove(struct platform_device *pdev)
> +{
> + struct sunxi_sid_data *sid_data;
> +
> + device_remove_bin_file(&pdev->dev, &sid_bin_attr); /* fixme: remove when using sysfs bin attrs */
> + sid_data = platform_get_drvdata(pdev);
> + dev_dbg(&pdev->dev, "driver unloaded\n");
> +
> + return 0;
> +}
> +
> +static const struct of_device_id sunxi_sid_of_match[] = {
> + { .compatible = "allwinner,sun4i-sid", .data = (void *)16},
> + { .compatible = "allwinner,sun7i-a20-sid", .data = (void *)512},
> + {/* sentinel */},
> +};
> +MODULE_DEVICE_TABLE(of, sunxi_sid_of_match);
> +
> +static int __init sunxi_sid_probe(struct platform_device *pdev)
This should be triggering a section mismatch warning when you compile
it.
The reason for this is that you reference your probe function (that has
a rather short life expectancy) from another structure (platform_driver)
that has a longer lifecycle. So, basically, after the init freeing, your
platform driver will still hold a reference to your probe function, that
has been freed, which is not a really good thing.
You can solve this in two ways:
- remove the __init
- or, in the platform driver case we're in, and if you're 100% sure
you won't need EPROBE_DEFER, you can use
module_platform_driver_probe.
> +{
> + struct sunxi_sid_data *sid_data;
> + struct resource *res;
> + const struct of_device_id *of_dev_id;
> + u8 *entropy;
> + unsigned int i;
> +
> + sid_data = devm_kzalloc(&pdev->dev, sizeof(struct sunxi_sid_data),
> + GFP_KERNEL);
> + if (!sid_data)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + sid_data->reg_base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(sid_data->reg_base))
> + return PTR_ERR(sid_data->reg_base);
> +
> + of_dev_id = of_match_device(sunxi_sid_of_match, &pdev->dev);
> + if (!of_dev_id)
> + return -ENODEV;
> + sid_data->keysize = (int)of_dev_id->data;
> +
> + platform_set_drvdata(pdev, sid_data);
> +
> + sid_bin_attr.size = sid_data->keysize; /* fixme: this should be properly set by the sysfs bin attr groups later */
> + if (device_create_bin_file(&pdev->dev, &sid_bin_attr)) /* fixme: this will need to be removed when using sysfs bin attr groups */
I'm sorry, this is not really what I had in mind. You still don't
explain why this is causing a problem, what problem, and why you can't
use the sysfs groups right from the start.
What I wanted was a (pretty long, I agree) summing up the whole
discussion we had.
Something like:
/*
* Be aware that this will generate a race with userspace. By the time
* it gets called, udev has already been notified of the device
* addition, and will never be notified that this file has been added.
*
* It should be properly fixed by having some way to attach a sysfs
* attribute group to the device structure before the probe.
* Unfortunately, this is both not possible for platform devices at the
* moment and not trivial to do, so this will need a proper fix later
* on.
*/
And just like the comment quoted above, you can remove the useless fixme
comments you added everywhere.
> + return -ENODEV;
> +
> + entropy = kzalloc(sizeof(u8) * sid_data->keysize, GFP_KERNEL);
> + for (i = 0; i < sid_data->keysize; i++)
> + entropy[i] = sunxi_sid_read_byte(sid_data, i);
> + add_device_randomness(entropy, sid_data->keysize);
> + kfree(entropy);
> +
> + dev_dbg(&pdev->dev, "loaded\n");
> +
> + return 0;
> +}
> +
> +static struct platform_driver sunxi_sid_driver = {
> + .probe = sunxi_sid_probe,
> + .remove = sunxi_sid_remove,
> + .driver = {
> + .name = DRV_NAME,
> + .owner = THIS_MODULE,
> + .of_match_table = sunxi_sid_of_match,
> + /* .groups = sunxi_sid_groups, fixme: this is the proper way using bin attr groups */
> + },
> +};
> +module_platform_driver(sunxi_sid_driver);
> +
> +MODULE_AUTHOR("Oliver Schinagl <[email protected]>");
> +MODULE_DESCRIPTION("Allwinner sunxi security id driver");
> +MODULE_LICENSE("GPL");
> --
> 1.8.1.5
>
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
On Mon, Sep 02, 2013 at 10:08:12AM +0200, Maxime Ripard wrote:
> > + platform_set_drvdata(pdev, sid_data);
> > +
> > + sid_bin_attr.size = sid_data->keysize; /* fixme: this should be properly set by the sysfs bin attr groups later */
> > + if (device_create_bin_file(&pdev->dev, &sid_bin_attr)) /* fixme: this will need to be removed when using sysfs bin attr groups */
>
> I'm sorry, this is not really what I had in mind. You still don't
> explain why this is causing a problem, what problem, and why you can't
> use the sysfs groups right from the start.
>
> What I wanted was a (pretty long, I agree) summing up the whole
> discussion we had.
>
> Something like:
>
> /*
> * Be aware that this will generate a race with userspace. By the time
> * it gets called, udev has already been notified of the device
> * addition, and will never be notified that this file has been added.
> *
> * It should be properly fixed by having some way to attach a sysfs
> * attribute group to the device structure before the probe.
> * Unfortunately, this is both not possible for platform devices at the
> * moment and not trivial to do, so this will need a proper fix later
> * on.
> */
>
> And just like the comment quoted above, you can remove the useless fixme
> comments you added everywhere.
There's no need for any of this, including the "FIXME" comments. I know
what the problem is here, and until the driver core changes get merged
upstream in order to fix it, there's no need to worry about "broken"
drivers, as everything is "broken".
I'll just take this as-is and remove the "fixme" comments, ok?
thanks,
greg k-h
Hi Greg,
On Mon, Sep 02, 2013 at 09:12:32AM -0700, Greg KH wrote:
> > What I wanted was a (pretty long, I agree) summing up the whole
> > discussion we had.
> >
> > Something like:
> >
> > /*
> > * Be aware that this will generate a race with userspace. By the time
> > * it gets called, udev has already been notified of the device
> > * addition, and will never be notified that this file has been added.
> > *
> > * It should be properly fixed by having some way to attach a sysfs
> > * attribute group to the device structure before the probe.
> > * Unfortunately, this is both not possible for platform devices at the
> > * moment and not trivial to do, so this will need a proper fix later
> > * on.
> > */
> >
> > And just like the comment quoted above, you can remove the useless fixme
> > comments you added everywhere.
>
> There's no need for any of this, including the "FIXME" comments. I know
> what the problem is here, and until the driver core changes get merged
> upstream in order to fix it, there's no need to worry about "broken"
> drivers, as everything is "broken".
>
> I'll just take this as-is and remove the "fixme" comments, ok?
It's your call :)
We had a few other comments, that will probably be fixed in the v7
oliver just sent, so you'd probably want to merge the v7 instead.
Thanks,
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com