Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221122085724.3245078-1-milkfafa@gmail.com> <20221122085724.3245078-3-milkfafa@gmail.com>
 <20221123000552.GA817752-robh@kernel.org>
In-Reply-To: <20221123000552.GA817752-robh@kernel.org>
From:   Kun-Fa Lin <milkfafa@gmail.com>
Date:   Thu, 24 Nov 2022 11:45:36 +0800
Message-ID: <CADnNmFq_1+OJNWCGSjmyV=sB39TNvAjKVs_1uLN8rOJ_RQNPSQ@mail.gmail.com>
Subject: Re: [PATCH v7 2/7] media: dt-binding: nuvoton: Add bindings for NPCM
 VCD and ECE engine
To:     Rob Herring <robh@kernel.org>
Cc:     mchehab@kernel.org, linux-media@vger.kernel.org,
        linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
        openbmc@lists.ozlabs.org, avifishman70@gmail.com,
        tmaimon77@gmail.com, tali.perry1@gmail.com, kwliu@nuvoton.com,
        kflin@nuvoton.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Rob,

Thanks for the review.

> > +title: Nuvoton NPCM Video Capture/Encode Engine Device Tree Bindings
>
> Drop ' Device Tree Bindings'
>
> > +
> > +maintainers:
> > +  - Joseph Liu <kwliu@nuvoton.com>
> > +  - Marvin Lin <kflin@nuvoton.com>
> > +
> > +description: |
> > +  Video Capture/Differentiation Engine (VCD) and Encoding Compression Engine
> > +  (ECE) present on Nuvoton NPCM SoCs.
>
> Sounds like 2 h/w blocks? If so, then it should be 2 separate nodes.
>
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - nuvoton,npcm750-video
> > +      - nuvoton,npcm845-video
>
> 'video' doesn't sound like the name of the h/w block(s).

VCD and ECE are 2 h/w blocks and our video capture driver needs to
control their registers.
If I separate them and store a phandle (referring to ece node) in vcd
node like this:

vcd: vcd@f0810000 {
    compatible = "nuvoton,npcm750-vcd";
    reg = <0xf0810000 0x10000>;
    ...
    nuvoton,enc-engine = <&ece>;
};

ece: ece@f0820000 {
    compatible = "nuvoton,npcm750-ece";
    reg = <0xf0820000 0x2000>;
    ...
};

Does it look good to you?

Regards,
Marvin