Re: [PATCH RFC] ARM: BCM5301X: Add /device_id property including device ID string

Re: [PATCH RFC] ARM: BCM5301X: Add /device_id property including device ID string

[Mark Rutland]

Hi,

For DT patches, please send to devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; this affects
more than just ARM.

On Sun, Mar 29, 2015 at 11:14:48PM +0100, Rafał Miłecki wrote:
> 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.

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.

> 
> Signed-off-by: Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> 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.

> ---
>  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(+)
> 
> diff --git a/arch/arm/boot/dts/bcm4708-asus-rt-ac68u.dts b/arch/arm/boot/dts/bcm4708-asus-rt-ac68u.dts
> index 8b62836..9d2fac4 100644
> --- a/arch/arm/boot/dts/bcm4708-asus-rt-ac68u.dts
> +++ b/arch/arm/boot/dts/bcm4708-asus-rt-ac68u.dts
> @@ -14,6 +14,7 @@
>  / {
>  	compatible = "asus,rt-ac68u", "brcm,bcm4708";
>  	model = "Asus RT-AC68U (BCM4708)";
> +	device_id = "RT-AC68U";
>  
>  	chosen {
>  		bootargs = "console=ttyS0,115200";
> diff --git a/arch/arm/boot/dts/bcm4708-netgear-r6250.dts b/arch/arm/boot/dts/bcm4708-netgear-r6250.dts
> index 2ed9e57..98c8d63 100644
> --- a/arch/arm/boot/dts/bcm4708-netgear-r6250.dts
> +++ b/arch/arm/boot/dts/bcm4708-netgear-r6250.dts
> @@ -14,6 +14,7 @@
>  / {
>  	compatible = "netgear,r6250v1", "brcm,bcm4708";
>  	model = "Netgear R6250 V1 (BCM4708)";
> +	device_id = "U12H245T00_NETGEAR";
>  
>  	chosen {
>  		bootargs = "console=ttyS0,115200";
> diff --git a/arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts b/arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts
> index 3991042..c43173a 100644
> --- a/arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts
> +++ b/arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts
> @@ -14,6 +14,7 @@
>  / {
>  	compatible = "netgear,r6300v2", "brcm,bcm4708";
>  	model = "Netgear R6300 V2 (BCM4708)";
> +	device_id = "U12H240T00_NETGEAR";
>  
>  	chosen {
>  		bootargs = "console=ttyS0,115200";
> diff --git a/arch/arm/boot/dts/bcm47081-asus-rt-n18u.dts b/arch/arm/boot/dts/bcm47081-asus-rt-n18u.dts
> index 0ee85ea..876e2c0 100644
> --- a/arch/arm/boot/dts/bcm47081-asus-rt-n18u.dts
> +++ b/arch/arm/boot/dts/bcm47081-asus-rt-n18u.dts
> @@ -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";
> diff --git a/arch/arm/boot/dts/bcm4709-netgear-r8000.dts b/arch/arm/boot/dts/bcm4709-netgear-r8000.dts
> index ea26dd3..129df9a 100644
> --- a/arch/arm/boot/dts/bcm4709-netgear-r8000.dts
> +++ b/arch/arm/boot/dts/bcm4709-netgear-r8000.dts
> @@ -14,6 +14,7 @@
>  / {
>  	compatible = "netgear,r8000", "brcm,bcm4709", "brcm,bcm4708";
>  	model = "Netgear R8000 (BCM4709)";
> +	device_id = "U12H315T00_NETGEAR";
>  
>  	chosen {
>  		bootargs = "console=ttyS0,115200";
> -- 
> 1.8.4.5
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC] ARM: BCM5301X: Add /device_id property including device ID string

[Rafał Miłecki]

On 30 March 2015 at 12:37, Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> wrote:
> For DT patches, please send to devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; this affects
> more than just ARM.

Ofc, sorry.


> On Sun, Mar 29, 2015 at 11:14:48PM +0100, Rafał Miłecki wrote:
>> 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.
>
> 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)"
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC] ARM: BCM5301X: Add /device_id property including device ID string

[Mark Rutland]

> > On Sun, Mar 29, 2015 at 11:14:48PM +0100, Rafał Miłecki wrote:
> >> 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.
> >
> > 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)"

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.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC] ARM: BCM5301X: Add /device_id property including device ID string

[Arnd Bergmann]

On Monday 30 March 2015 11:58:29 Mark Rutland wrote:
> > > On Sun, Mar 29, 2015 at 11:14:48PM +0100, Rafał Miłecki wrote:
> > >> 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.
> > >
> > > 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)"
> 
> 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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC] ARM: BCM5301X: Add /device_id property including device ID string

[Rafał Miłecki]

On 10 April 2015 at 12:50, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> On Monday 30 March 2015 11:58:29 Mark Rutland wrote:
>> > > On Sun, Mar 29, 2015 at 11:14:48PM +0100, Rafał Miłecki wrote:
>> > >> 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.
>> > >
>> > > 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)"
>>
>> 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.

Thanks, I think it looks like the best alternative to my initial idea.

-- 
Rafał
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html