ARM: BCM5301X: Add /device_id property including device ID string
RFC PATCH 1/1 ARM: BCM5301X: Add /device_id property including device ID string
Device vendors often assign IDs to their devices to allow comparing firmware image with device model. This is required to prevent users from flashing incompatible image and soft-bricking device. Add device_id property to DTs to allow user space (and optionally bootloader) verifying firmware images.
---
Hi guys,
I think my commit message explains pretty well what I'm trying to do, however I'm not sure if I'm using a right place for that.
I also didn't document this news property, as I can't find a place where root-properties are currenty described. Is there any such place? I couldn't find a file describing e.g. "model" property.
--- arch/arm/boot/dts/bcm4708-asus-rt-ac68u.dts | 1 + arch/arm/boot/dts/bcm4708-netgear-r6250.dts | 1 + arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts | 1 + arch/arm/boot/dts/bcm47081-asus-rt-n18u.dts | 1 + arch/arm/boot/dts/bcm4709-netgear-r8000.dts | 1 + 5 files changed, 5 insertions(+)
arch/arm/boot/dts/bcm4708-asus-rt-ac68u.dts +1 -0
@@ -14,6 +14,7 @@ / { compatible = "asus,rt-ac68u", "brcm,bcm4708"; model = "Asus RT-AC68U (BCM4708)"; + device_id = "RT-AC68U"; chosen { bootargs = "console=ttyS0,115200";
arch/arm/boot/dts/bcm4708-netgear-r6250.dts +1 -0
@@ -14,6 +14,7 @@ / { compatible = "netgear,r6250v1", "brcm,bcm4708"; model = "Netgear R6250 V1 (BCM4708)"; + device_id = "U12H245T00_NETGEAR"; chosen { bootargs = "console=ttyS0,115200";
arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts +1 -0
@@ -14,6 +14,7 @@ / { compatible = "netgear,r6300v2", "brcm,bcm4708"; model = "Netgear R6300 V2 (BCM4708)"; + device_id = "U12H240T00_NETGEAR"; chosen { bootargs = "console=ttyS0,115200";
arch/arm/boot/dts/bcm47081-asus-rt-n18u.dts +1 -0
@@ -14,6 +14,7 @@ / { compatible = "asus,rt-n18u", "brcm,bcm47081", "brcm,bcm4708"; model = "Asus RT-N18U (BCM47081)"; + device_id = "RT-N18U"; chosen { bootargs = "console=ttyS0,115200";
arch/arm/boot/dts/bcm4709-netgear-r8000.dts +1 -0
@@ -14,6 +14,7 @@ / { compatible = "netgear,r8000", "brcm,bcm4709", "brcm,bcm4708"; model = "Netgear R8000 (BCM4709)"; + device_id = "U12H315T00_NETGEAR"; chosen { bootargs = "console=ttyS0,115200";
Signature · 1 line
1.8.4.5
RFC PATCH 1/1 ARM: BCM5301X: Add /device_id property including device ID string
Quoted 7 lines · Rafa? Mi?ecki
Device vendors often assign IDs to their devices to allow comparing firmware image with device model. This is required to prevent users from flashing incompatible image and soft-bricking device. Add
device_idproperty to DTs to allow user space (and optionally bootloader) verifying firmware images.Signed-off-by Rafa? Mi?ecki <[email protected]>
Quoted 8 lines
---
Hi guys,I think my commit message explains pretty well what I'm trying to do, however I'm not sure if I'm using a right place for that.
I also didn't document this news property, as I can't find a place where root-properties are currenty described. Is there any such place? I couldn't find a file describing e.g. "model" property.
What does this do which the top-level "compatible" doesn't do?
arch/arm/boot/dts/bcm4708-asus-rt-ac68u.dts line 17
Quoted 9 lines
arch/arm/boot/dts/bcm4708-asus-rt-ac68u.dts +1
-0@@ -14,6 +14,7 @@ / { compatible = "asus,rt-ac68u", "brcm,bcm4708"; model = "Asus RT-AC68U (BCM4708)"; + device_id = "RT-AC68U";
"asus,rt-ac68u" is more specific than your device ID - it's entirely possible that two manufacturers could choose the same model number for their device - matching on the model number alone is opening this up to exactly the issue you're trying to prevent.
If you have your validator check for "asus,rt-ac68u" on such a device rather than the magic string "RT-AC68U", it should be much better.
arch/arm/boot/dts/bcm4708-netgear-r6250.dts line 17
Quoted 9 lines
arch/arm/boot/dts/bcm4708-netgear-r6250.dts +1
-0@@ -14,6 +14,7 @@ / { compatible = "netgear,r6250v1", "brcm,bcm4708"; model = "Netgear R6250 V1 (BCM4708)"; + device_id = "U12H245T00_NETGEAR";
Here you decide to include the manufacturer at the end of the name. If the model number is "U12H245T00" why doesn't the compatible string reflect this? That sounds like a mistake to me.
I quote the definition of the "compatible" property:
Property: compatible
Value type: <stringlist>
Description: The compatible property value consists of one or more
strings that define the specific programming model for the device. This list of strings should be used by a client program for device driver selection. The property value consists of a concatenated list of null terminated strings, from most specific to most general. They allow a device to express its compatibility with a family of similar devices, potentially allowing a single device driver to match against several devices. The recommended format is ?manufacturer,model?, where manufacturer is a string describing the name of the manufacturer (such as a stock ticker symbol), and model specifies the model number.
So, checking the top-level compatible _should_ do everything you want, provided it follows the requirements of ePAPR.
Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.
RFC PATCH 1/1 ARM: BCM5301X: Add /device_id property including device ID string
Quoted 11 lines · Russell King - ARM Linux
Quoted 10 lines · Rafa? Mi?ecki
Device vendors often assign IDs to their devices to allow comparing firmware image with device model. This is required to prevent users from flashing incompatible image and soft-bricking device. Add
device_idproperty to DTs to allow user space (and optionally bootloader) verifying firmware images.Signed-off-by Rafa? Mi?ecki <[email protected]>---
Hi guys,
Quoted 7 lines
Quoted 5 lines
I think my commit message explains pretty well what I'm trying to do, however I'm not sure if I'm using a right place for that.
I also didn't document this news property, as I can't find a place where root-properties are currenty described. Is there any such place? I couldn't find a file describing e.g. "model" property.What does this do which the top-level "compatible" doesn't do?
Let me just answer to this one, it should make rest answers redundant. I obviously forgot to explain what was clear to me.
What we use in "compatible" is some sane-looking string describing machine in a friendly way. Why my patch adds is some vendor-picked ID which is more like a MAGIC specific for a device.
If you download firmware image from vendor's site (Netgear, Asus, others too), it includes some extra header/tail with a vendor picked ID (or better: MAGIC). Vendor's bootloader and default-firmware-web-UI requires that MAGIC or will reject the firmware. It also means that:
1) Generating device-compatible (and so flashable) firmware (like OpenWrt or similar) requires using the same vendor-picked MAGIC.
2) Flashing vendor-provided firmware image from custom firmware (e.g. OpenWrt) requires comparing provided file with a device-specific MAGIC. Since vendors (Netgear, Asus, others) provide firmwares with their own picked MAGICs, we can't compare them against "asus,rt-ac68u", "netgear,r6250v1", or whatever we use. That also means comparing it against some more-or-less silly strings like "U12H245T00_NETGEAR".
Let me know if that explains my purpose of device_id a bit.
Signature · 1 line
Rafa?
RFC PATCH 1/1 ARM: BCM5301X: Add /device_id property including device ID string
Quoted 44 lines · Rafa? Mi?ecki
Quoted 11 lines · Russell King - ARM Linux
Quoted 10 lines · Rafa? Mi?ecki
Device vendors often assign IDs to their devices to allow comparing firmware image with device model. This is required to prevent users from flashing incompatible image and soft-bricking device. Add
device_idproperty to DTs to allow user space (and optionally bootloader) verifying firmware images.Signed-off-by Rafa? Mi?ecki <[email protected]>---
Hi guys,Quoted 7 lines
Quoted 5 lines
I think my commit message explains pretty well what I'm trying to do, however I'm not sure if I'm using a right place for that.
I also didn't document this news property, as I can't find a place where root-properties are currenty described. Is there any such place? I couldn't find a file describing e.g. "model" property.What does this do which the top-level "compatible" doesn't do?
Let me just answer to this one, it should make rest answers redundant. I obviously forgot to explain what was clear to me.
What we use in "compatible" is some sane-looking string describing machine in a friendly way. Why my patch adds is some vendor-picked ID which is more like a MAGIC specific for a device.
If you download firmware image from vendor's site (Netgear, Asus, others too), it includes some extra header/tail with a vendor picked ID (or better: MAGIC). Vendor's bootloader and default-firmware-web-UI requires that MAGIC or will reject the firmware. It also means that:
1) Generating device-compatible (and so flashable) firmware (like OpenWrt or similar) requires using the same vendor-picked MAGIC.
2) Flashing vendor-provided firmware image from custom firmware (e.g. OpenWrt) requires comparing provided file with a device-specific MAGIC. Since vendors (Netgear, Asus, others) provide firmwares with their own picked MAGICs, we can't compare them against "asus,rt-ac68u", "netgear,r6250v1", or whatever we use. That also means comparing it against some more-or-less silly strings like "
U12H245T00_NETGEAR".
Quoted 1 line
Let me know if that explains my purpose of
device_ida bit.
Yes. Now you need to add a description of the new property to the appropriate place in Documentation/devicetree/bindings first before you can add it to the device tree descriptions themselves, and have the DT maintainers review your new description.
You'll also want to read
Documentation/devicetree/bindings/submitting-patches.txt
before making your submission.
I'd suggest explaining it fully - maybe this would be better:
Device vendors often assign their own IDs to their devices to allow comparing the firmware image with device model. This is required to prevent users from flashing an incompatible image and soft-bricking device.
Add a device_id property to contain this vendor assigned ID, which is intended to inform userspace of the appropriate ID to verify in user supplied firmware images prior to programming.
This makes it clear that the purpose here is to supply an identifier to userspace in order to verify user supplied firmware images, rather than this property being used to identify a firmware image.
I'll leave it up to DT people to decide whether this is something they want to accept; they'll probably only comment with a properly submitted binding proposal.
Thanks.
Signature · 2 lines
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net.
RFC PATCH 1/1 ARM: BCM5301X: Add /device_id property including device ID string
In reply to Rafał MiłeckiHi,
For DT patches, please send to devicetree at vger.kernel.org; this affects more than just ARM.
Quoted 5 lines · Rafa? Mi?ecki
Device vendors often assign IDs to their devices to allow comparing firmware image with device model. This is required to prevent users from flashing incompatible image and soft-bricking device. Add
device_idproperty to DTs to allow user space (and optionally bootloader) verifying firmware images.
This sounds like exactly what the "model" property is meant to be (per ePAPR) -- a string that specifices the manufacturer's model number of the device, ideally in "manufacturer,model" format.
Quoted 10 lines
Signed-off-by Rafa? Mi?ecki <[email protected]>---
Hi guys,I think my commit message explains pretty well what I'm trying to do, however I'm not sure if I'm using a right place for that.
I also didn't document this news property, as I can't find a place where root-properties are currenty described. Is there any such place? I couldn't find a file describing e.g. "model" property.
It's documented in ePAPR (embedded power achitecture platform requirements), and possibly earlier device tree and/or open firmware documents.
Unfortuantely ePAPR is now behind a registration page on power.org, and I'm not sure where most other documents live (playground.sun.com and other sources are dead).
Mark.
Quoted 76 lines
--- arch/arm/boot/dts/bcm4708-asus-rt-ac68u.dts | 1 + arch/arm/boot/dts/bcm4708-netgear-r6250.dts | 1 + arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts | 1 + arch/arm/boot/dts/bcm47081-asus-rt-n18u.dts | 1 + arch/arm/boot/dts/bcm4709-netgear-r8000.dts | 1 + 5 files changed, 5 insertions(+)arch/arm/boot/dts/bcm4708-asus-rt-ac68u.dts +1
-0@@ -14,6 +14,7 @@ / { compatible = "asus,rt-ac68u", "brcm,bcm4708"; model = "Asus RT-AC68U (BCM4708)"; + device_id = "RT-AC68U"; chosen { bootargs = "console=ttyS0,115200";arch/arm/boot/dts/bcm4708-netgear-r6250.dts +1
-0@@ -14,6 +14,7 @@ / { compatible = "netgear,r6250v1", "brcm,bcm4708"; model = "Netgear R6250 V1 (BCM4708)"; + device_id = "U12H245T00_NETGEAR"; chosen { bootargs = "console=ttyS0,115200";arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts +1
-0@@ -14,6 +14,7 @@ / { compatible = "netgear,r6300v2", "brcm,bcm4708"; model = "Netgear R6300 V2 (BCM4708)"; + device_id = "U12H240T00_NETGEAR"; chosen { bootargs = "console=ttyS0,115200";arch/arm/boot/dts/bcm47081-asus-rt-n18u.dts +1
-0@@ -14,6 +14,7 @@ / { compatible = "asus,rt-n18u", "brcm,bcm47081", "brcm,bcm4708"; model = "Asus RT-N18U (BCM47081)"; + device_id = "RT-N18U"; chosen { bootargs = "console=ttyS0,115200";arch/arm/boot/dts/bcm4709-netgear-r8000.dts +1
-0@@ -14,6 +14,7 @@ / { compatible = "netgear,r8000", "brcm,bcm4709", "brcm,bcm4708"; model = "Netgear R8000 (BCM4709)"; + device_id = "U12H315T00_NETGEAR"; chosen { bootargs = "console=ttyS0,115200";Signature · 7 lines
1.8.4.5 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
RFC PATCH 1/1 ARM: BCM5301X: Add /device_id property including device ID string
Quoted 2 lines · Mark Rutland
For DT patches, please send to devicetree at vger.kernel.org; this affects more than just ARM.
Ofc, sorry.
Quoted 10 lines · Rafa? Mi?ecki
Quoted 5 lines
Device vendors often assign IDs to their devices to allow comparing firmware image with device model. This is required to prevent users from flashing incompatible image and soft-bricking device. Add
device_idproperty to DTs to allow user space (and optionally bootloader) verifying firmware images.This sounds like exactly what the "model" property is meant to be (per ePAPR) -- a string that specifices the manufacturer's model number of the device, ideally in "manufacturer,model" format.
What if manufacturer decided to use some totally unfriendly ID for their low-level (firmware upgrade) model identification? I can't really see us using
model = "U12H245T00_NETGEAR"
which would mean nothing compared to the current friendly:
model = "Netgear R6250 V1 (BCM4708)"
RFC PATCH 1/1 ARM: BCM5301X: Add /device_id property including device ID string
Quoted 11 lines
Quoted 10 lines · Rafa? Mi?ecki
Quoted 5 lines
Device vendors often assign IDs to their devices to allow comparing firmware image with device model. This is required to prevent users from flashing incompatible image and soft-bricking device. Add
device_idproperty to DTs to allow user space (and optionally bootloader) verifying firmware images.This sounds like exactly what the "model" property is meant to be (per ePAPR) -- a string that specifices the manufacturer's model number of the device, ideally in "manufacturer,model" format.
Quoted 6 lines
What if manufacturer decided to use some totally unfriendly ID for their low-level (firmware upgrade) model identification? I can't really see us using
model = "U12H245T00_NETGEAR"
which would mean nothing compared to the current friendly:
model = "Netgear R6250 V1 (BCM4708)"
If it uniquely identifies the model, it's fit to be a model string. If being "friendly" means that we lose that, then the property is useless anyway.
Note that both examples above deviate from the recommended format, and something like: "netgear,U12H245T00" would better align with the recommendation.
Mark.
RFC PATCH 1/1 ARM: BCM5301X: Add /device_id property including device ID string
Quoted 18 lines · On Monday 30 March 2015 11:58:29 Mark Rutland wrote:
Quoted 11 lines
Quoted 10 lines · Rafa? Mi?ecki
Quoted 5 lines
Device vendors often assign IDs to their devices to allow comparing firmware image with device model. This is required to prevent users from flashing incompatible image and soft-bricking device. Add
device_idproperty to DTs to allow user space (and optionally bootloader) verifying firmware images.This sounds like exactly what the "model" property is meant to be (per ePAPR) -- a string that specifices the manufacturer's model number of the device, ideally in "manufacturer,model" format.
Quoted 6 lines
What if manufacturer decided to use some totally unfriendly ID for their low-level (firmware upgrade) model identification? I can't really see us using
model "U12H245T00_NETGEAR"
which would mean nothing compared to the current friendly:
model "Netgear R6250 V1 (BCM4708)"
Quoted 7 lines
If it uniquely identifies the model, it's fit to be a model string. If being "friendly" means that we lose that, then the property is useless anyway.
Note that both examples above deviate from the recommended format, and something like: "netgear,U12H245T00" would better align with the recommendation.
Sorry for jumping in late here, but this still sounds like more fitting for the "compatible" property than "model". For compatible, you can already have multiple strings, and they come with a vendor prefix.
Any of these would seem reasonable to me:
a)
compatible = "asus,rt-ac68u", "openwrt,RT-AC68U", "brcm,bcm4708";
model = "Asus RT-AC68U (BCM4708)";b)
compatible = "asus,rt-ac68u", "brcm,RT-AC68U", "brcm,bcm4708";
model = "Asus RT-AC68U (BCM4708)";c)
compatible = "asus,rt-ac68u", "asus,RT-AC68U", "brcm,bcm4708";
model = "Asus RT-AC68U (BCM4708)";depending on which party maintains the list of these identifiers.
Arnd
RFC PATCH 1/1 ARM: BCM5301X: Add /device_id property including device ID string
Quoted 38 lines · Arnd Bergmann
Quoted 18 lines · On Monday 30 March 2015 11:58:29 Mark Rutland wrote:
Quoted 11 lines
Quoted 10 lines · Rafa? Mi?ecki
Quoted 5 lines
Device vendors often assign IDs to their devices to allow comparing firmware image with device model. This is required to prevent users from flashing incompatible image and soft-bricking device. Add
device_idproperty to DTs to allow user space (and optionally bootloader) verifying firmware images.This sounds like exactly what the "model" property is meant to be (per ePAPR) -- a string that specifices the manufacturer's model number of the device, ideally in "manufacturer,model" format.
Quoted 6 lines
What if manufacturer decided to use some totally unfriendly ID for their low-level (firmware upgrade) model identification? I can't really see us using
model "U12H245T00_NETGEAR"
which would mean nothing compared to the current friendly:
model "Netgear R6250 V1 (BCM4708)"Quoted 7 lines
If it uniquely identifies the model, it's fit to be a model string. If being "friendly" means that we lose that, then the property is useless anyway.
Note that both examples above deviate from the recommended format, and something like: "netgear,U12H245T00" would better align with the recommendation.
Sorry for jumping in late here, but this still sounds like more fitting for the "compatible" property than "model". For compatible, you can already have multiple strings, and they come with a vendor prefix.
Any of these would seem reasonable to me:
a)
compatible = "asus,rt-ac68u", "openwrt,RT-AC68U", "brcm,bcm4708"; model = "Asus RT-AC68U (BCM4708)";b)
Quoted 8 lines
compatible = "asus,rt-ac68u", "brcm,RT-AC68U", "brcm,bcm4708"; model = "Asus RT-AC68U (BCM4708)";c)
compatible = "asus,rt-ac68u", "asus,RT-AC68U", "brcm,bcm4708"; model = "Asus RT-AC68U (BCM4708)";depending on which party maintains the list of these identifiers.
Thanks, I think it looks like the best alternative to my initial idea.
Signature · 1 line
Rafa?