Re: [PATCH v3 1/3] dt-bindings: mediatek: Add a binding for Mediatek JPEG Decoder

Rick Chang
2016-11-05T08:19:57Z

Hi Laurent,

Thanks for your prompt reply. I will fix them in patch v4.

On Sat, 2016-11-05 at 01:32 +0200, Laurent Pinchart wrote:
> Hi Rick,
>
> Thank you for the patch.
>
> On Friday 04 Nov 2016 13:51:18 Rick Chang wrote:
> > Add a DT binding documentation for Mediatek JPEG Decoder of
> > MT2701 SoC.
>
> This version looks much better !

I'm glad to hear that!

> > Signed-off-by: Rick Chang <rick.chang@mediatek.com>
> > Signed-off-by: Minghsiu Tsai <minghsiu.tsai@mediatek.com>
> > ---
> > .../bindings/media/mediatek-jpeg-codec.txt | 35 +++++++++++++++++++
> > 1 file changed, 35 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/media/mediatek-jpeg-codec.txt
> >
> > diff --git a/Documentation/devicetree/bindings/media/mediatek-jpeg-codec.txt
> > b/Documentation/devicetree/bindings/media/mediatek-jpeg-codec.txt new file
> > mode 100644
> > index 0000000..b2b19ed
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/mediatek-jpeg-codec.txt
> > @@ -0,0 +1,35 @@
> > +* Mediatek JPEG Decoder
> > +
> > +Mediatek JPEG Decoder is the JPEG decode hw present in Mediatek SoCs
>
> Nitpicking, I'd write hardware instead of hw.

OK.

> > +Required properties:
> > +- compatible : "mediatek,mt2701-jpgdec"
>
> As commented on the previous version, is this JPEG decoder unique to the
> MT2701, or is it also used (possibly with different interrupts, clocks, ...)
> in other SoCs ? In the latter case, if the JPEG decoder IP core is identical
> in multiple SoCs, a more generic compatible string would be better.

I understand your question. The essence of this patch series targeting
is the JPEG decoder IP which is integrated in mt2701.I will check it
with our hardware engineer and making a more generic compatible string
if the IP is used in multiple SoCs.

> > +- reg : physical base address of the jpeg decoder registers and length of
> > + memory mapped region.
> > +- interrupts : interrupt number to the interrupt controller.
> > +- clocks: device clocks, see
> > + Documentation/devicetree/bindings/clock/clock-bindings.txt for details.
> > +- clock-names: must contain "jpgdec-smi" and "jpgdec".
> > +- power-domains: a phandle to the power domain, see
> > + Documentation/devicetree/bindings/power/power_domain.txt for details.
> > +- mediatek,larb: must contain the local arbiters in the current Socs, see
> > +
> > Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
> > + for details.
> > +- iommus: should point to the respective IOMMU block with master port as
> > + argument, see Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
> > + for details.
> > +
> > +Example:
> > + jpegdec: jpegdec@15004000 {
> > + compatible = "mediatek,mt2701-jpgdec";
> > + reg = <0 0x15004000 0 0x1000>;
> > + interrupts = <GIC_SPI 143 IRQ_TYPE_LEVEL_LOW>;
> > + clocks = <&imgsys CLK_IMG_JPGDEC_SMI>,
> > + <&imgsys CLK_IMG_JPGDEC>;
> > + clock-names = "jpgdec-smi",
> > + "jpgdec";
> > + power-domains = <&scpsys MT2701_POWER_DOMAIN_ISP>;
> > + mediatek,larb = <&larb2>;
> > + iommus = <&iommu MT2701_M4U_PORT_JPGDEC_WDMA>,
> > + <&iommu MT2701_M4U_PORT_JPGDEC_BSDMA>;
> > + };
>