On 18/09/2023 06:20, Yi Chou wrote: > On Sun, Sep 17, 2023 at 4:40 PM Krzysztof Kozlowski > <krzysztof.kozlowski@xxxxxxxxxx> wrote: >> >> On 08/09/2023 12:15, Yi Chou wrote: >>> The necessary fields to initialize the widevine related functions in >>> OP-TEE. >>> >>> Signed-off-by: Yi Chou <yich@xxxxxxxxxx> >>> Reviewed-by: Simon Glass <sjg@xxxxxxxxxxxx> >>> --- >>> .../bindings/options/google,widevine.yaml | 124 ++++++++++++++++++ >>> 1 file changed, 124 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/options/google,widevine.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/options/google,widevine.yaml b/Documentation/devicetree/bindings/options/google,widevine.yaml >>> new file mode 100644 >>> index 0000000000000..bf2b834cb1454 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/options/google,widevine.yaml >> >> There is no such hardware as "options". What is this supposed to be for? >> firmware? > > These DT fields would not be consumed by the OS. > https://www.spinics.net/lists/devicetree-spec/msg01195.html > The previous discussion tended to use the "options" node. > Do we have any better place for these widevine related fields? I'll let Rob comment on this in such case. > >> >>> @@ -0,0 +1,124 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/options/google,widevine.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Google Widevine initialization parameters. >> >> This is a title, drop full stop. > > Got it, will be fixed in the next patch. > >> >>> + >>> +maintainers: >>> + - Jeffrey Kardatzke <jkardatzke@xxxxxxxxxxxx> >>> + - Yi Chou <yich@xxxxxxxxxxxx> >>> + >>> +description: >>> + The necessary fields to initialize the widevine related functions in >>> + OP-TEE. This node does not represent a real device, but serves as a >>> + place for passing data between firmware and OP-TEE. >>> + The public fields (e.g. tpm-auth-public-key & root-of-trust-cert) can >>> + be ignored because it's safe to pass the public information with the >>> + other methods(e.g. userland OP-TEE plugins). >> >> Then why isn't this a property of optee node? > > Are you talking about the /firmware/optee node? > If I understand correctly, that node was talking about how the kernel > communicates with the OP-TEE. > But what we are doing here is passing some secrets from trusted > firmware into OP-TEE, and the data would not go through the linux > kernel. > I'm not sure if it is a good idea to mix two different purpose fields > in the same node... > >> >>> + >>> +properties: >>> + compatible: >>> + const: google,widevine >> >> From the description I have no clue what is "widevine". The more >> surprising is to see it as "not hardware" but having its node and >> compatible, like it was a hardware node. > > We already have a "chosen" node that is "not hardware" in the DT. > Should we just remove the compatible field from this node? > > BTW, Widevine is a digital rights management (DRM) system to make sure > the video stream can only be decoded on the valid devices. Then describe it in the description. Best regards, Krzysztof
