Re: [PATCH] powerpc/fsl: add device tree binding for QE firmware

[Timur Tabi <timur@freescale.com>]

Grant Likely wrote:

> +- fsl,firmware:
> +    Usage: Optional.
> +    Value type: <prop-encoded-array>, encoded array of bytes
> +    Definition: Contains the QUICC engine firmware blob.
> [plus any other properties needed for firmware metadata]

This would place the firmware metadata properties inside the QE node itself, which would break the QE binding.

> +Example:
> +       qe1: qe@e0080000 {
> +               compatible = "fsl,qe";
> +               fsl,firmware = /bininc/("firmware-blob.bin");  /* Or
> squirted in by firmware */
> +               ...
> +       }
> +
> +       qe@e0090000 {
> +               compatible = "fsl,qe";
> +               fsl,firmware-phandle = <&qe1>;
> +               ...
> +       }
> 
> Putting the blob into just one of the qe nodes keeps everything nicely
> contained with the device it actually applies to.  No debates about
> the best place to put device firmware blobs or new compatible values,
> and it is applicable to any device where firmware needs to be passed
> via the tree.

Except when you actually need to add metadata properties:

       qe1: qe@e0080000 {
               compatible = "fsl,qe";
               fsl,firmware = /bininc/("firmware-blob.bin"); 
               fsl,qe-firmware-eccr = <0x00000000 0x00001230>;
               ...
       }

(The ECCR is stored in the QE firmware blob, but let's pretend it isn't and I need to specify it)

Here, the fsl,qe-firmware-eccr property is associated with the QE itself.  This is why I want a compatible property for the firmware node, no matter where it is.  Then you can do this:

       qe1: qe@e0080000 {
               compatible = "fsl,qe";
               fsl,qe_firmware {
                       compatible="fsl,qe-firmware";
                       fsl,firmware = /bininc/("firmware-blob.bin"); 
                       fsl,qe-firmware-eccr = <0x00000000 0x00001230>;
               }
               ...
       }

Without the compatible property, the only way I'd know that the child node contains a firmware is to look at the actual name of the child node, which (as Scott and I believe) is not better than a compatible property.

-- 
Timur Tabi
Linux kernel developer at Freescale