Re: [RFC 1/3] dt-bindings: soc: Add documentation for the MediaTek GCE unit

[Matthias Brugger]

On 22/01/16 04:38, Horng-Shyang Liao wrote:
> Hi Rob,
>
> On Wed, 2016-01-20 at 10:38 -0600, Rob Herring wrote:
> > On Wed, Jan 20, 2016 at 01:14:38PM +0800, hs.liao@xxxxxxxxxxxx wrote:
> > > From: HS Liao <hs.liao@xxxxxxxxxxxx>
> > >
> > > This adds documentation for the MediaTek Global Command Engine (GCE) unit
> > > found in MT8173 SoCs.
> > >
> > > Signed-off-by: HS Liao <hs.liao@xxxxxxxxxxxx>
> > > ---
> > >   .../devicetree/bindings/soc/mediatek/gce.txt       |   33 ++++++++++++++++++++
> > >   1 file changed, 33 insertions(+)
> > >   create mode 100644 Documentation/devicetree/bindings/soc/mediatek/gce.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/soc/mediatek/gce.txt b/Documentation/devicetree/bindings/soc/mediatek/gce.txt
> > > new file mode 100644
> > > index 0000000..878b11e
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/soc/mediatek/gce.txt
> > > @@ -0,0 +1,33 @@
> > > +MediaTek GCE
> > > +===============
> > > +
> > > +The Global Command Engine (GCE) is used to help read/write registers with
> > > +critical time limitation, such as updating display configuration during the
> > > +vblank. The GCE can be used to implement the Command Queue (CMDQ) driver.
> > > +Currently, the GCE only supports display related hardwares, but we expect
> > > +it can be extended to other hardwares for future requirements.
> >
> > That's a hardware limitation or just s/w is only using it for display?
> > If the latter, that's not really relevant to this binding and should be
> > removed.
>
> Just s/w is only using it for display.
> I will remove it from next patch.
>
> > > +
> > > +Required properties:
> > > +- compatible: Must be "mediatek,mt8173-gce"
> > > +- reg: Address range of the GCE unit
> > > +- interrupts: The interrupt signal from the GCE block
> > > +- clock: Clocks according to the common clock binding
> > > +- clock-names: Must be "gce" to stand for GCE clock
> > > +
> > > +Example:
> > > +
> > > +	gce: gce@10212000 {
> > > +		compatible = "mediatek,mt8173-gce";
> > > +		reg = <0 0x10212000 0 0x1000>;
> > > +		interrupts = <GIC_SPI 135 IRQ_TYPE_LEVEL_LOW>;
> > > +		clocks = <&infracfg CLK_INFRA_GCE>;
> > > +		clock-names = "gce";
> > > +	};
> > > +
> > > +	mmsys: clock-controller@14000000 {
> > > +		compatible = "mediatek,mt8173-mmsys", "syscon";
> > > +		reg = <0 0x14000000 0 0x1000>;
> > > +		power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
> > > +		#clock-cells = <1>;
> > > +		mediatek,gce = <&gce>;
> >
> > Not documented.
>
> It's just an example about how gce is used by display mmsys.
> After I discussed with Mediatek display owner,
> we think this can be moved to display device tree document.
> Do you agree with this suggestion?
> If so, I will remove it from next patch, too.

The problem is that mediatek,gce is not documented, but only 
mediatek,mt8173-gce.

Regards,
Matthias