Hi Evan, Thanks very much for reviewing this patchset. On Wed, 2019-01-30 at 10:27 -0800, Evan Green wrote: > On Mon, Dec 31, 2018 at 7:56 PM Yong Wu <yong.wu@xxxxxxxxxxxx> wrote: > > > > This patch adds decriptions for mt8183 IOMMU and SMI. > > > > mt8183 has only one M4U like mt8173 and is also MTK IOMMU gen2 which > > uses ARM Short-Descriptor translation table format. > > > > The mt8183 M4U-SMI HW diagram is as below: > > > > EMI > > | > > M4U > > | > > ---------- > > | | > > gals0-rx gals1-rx > > | | > > | | > > gals0-tx gals1-tx > > | | > > ------------ > > SMI Common > > ------------ > > | > > +-----+-----+--------+-----+-----+-------+-------+ > > | | | | | | | | > > | | gals-rx gals-rx | gals-rx gals-rx gals-rx > > | | | | | | | | > > | | | | | | | | > > | | gals-tx gals-tx | gals-tx gals-tx gals-tx > > | | | | | | | | > > larb0 larb1 IPU0 IPU1 larb4 larb5 larb6 CCU > > disp vdec img cam venc img cam > > It might be cool to put the gals in the picture in the bindings. Not a > big deal though. OK. the picture in the binding should be more generic, I will try add gals in the binding picture in next version. > > > > > All the connections are HW fixed, SW can NOT adjust it. > > > > Compared with mt8173, we add a GALS(Global Async Local Sync) module > > between SMI-common and M4U, and additional GALS between larb2/3/5/6 > > and SMI-common. GALS can help synchronize for the modules in different > > clock frequency, it can be seen as a "asynchronous fifo". > > > > GALS can only help transfer the command/data while it doesn't have > > the configuring register, thus it has the special "smi" clock and it > > doesn't have the "apb" clock. From the diagram above, we add "gals0" > > and "gals1" clocks for smi-common and add a "gals" clock for smi-larb. > > > > From the diagram above, IPU0/IPU1(Image Processor Unit) and CCU(Camera > > Control Unit) is connected with smi-common directly, we can take them > > as "larb2", "larb3" and "larb7", and their register spaces are > > different with the normal larb. > > > > Signed-off-by: Yong Wu <yong.wu@xxxxxxxxxxxx> > > Reviewed-by: Rob Herring <robh@xxxxxxxxxx> > > --- > > .../devicetree/bindings/iommu/mediatek,iommu.txt | 15 ++- > > .../memory-controllers/mediatek,smi-common.txt | 11 +- > > .../memory-controllers/mediatek,smi-larb.txt | 3 + > > include/dt-bindings/memory/mt8183-larb-port.h | 130 +++++++++++++++++++++ > > 4 files changed, 153 insertions(+), 6 deletions(-) > > create mode 100644 include/dt-bindings/memory/mt8183-larb-port.h > > > > diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.txt b/Documentation/devicetree/bindings/iommu/mediatek,iommu.txt > > index 6922db5..6e758996 100644 > > --- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.txt > > +++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.txt > > @@ -36,6 +36,10 @@ each local arbiter. > > like display, video decode, and camera. And there are different ports > > in each larb. Take a example, There are many ports like MC, PP, VLD in the > > video decode local arbiter, all these ports are according to the video HW. > > + In some SoCs, there may be a GALS(Global Async Local Sync) module between > > +smi-common and m4u, and additional GALS module between smi-larb and > > +smi-common. GALS can been seen as a "asynchronous fifo" which could help > > +synchronize for the modules in different clock frequency. > > > > Required properties: > > - compatible : must be one of the following string: > > @@ -44,18 +48,23 @@ Required properties: > > "mediatek,mt7623-m4u", "mediatek,mt2701-m4u" for mt7623 which uses > > generation one m4u HW. > > "mediatek,mt8173-m4u" for mt8173 which uses generation two m4u HW. > > + "mediatek,mt8183-m4u" for mt8183 which uses generation two m4u HW. > > - reg : m4u register base and size. > > - interrupts : the interrupt of m4u. > > - clocks : must contain one entry for each clock-names. > > -- clock-names : must be "bclk", It is the block clock of m4u. > > +- clock-names : Only 1 optional clock: > > + - "bclk": the block clock of m4u. > > + Note that m4u use the EMI clock which always has been enabled before kernel > > + if there is no this "bclk". > > Ideally bclk could be specified a little more crisply, as this is > actually required for some SoCs and not used at all on others (as in > patch 7). OK. I will add like this: + Here is the list which require the "bclk": + - mt2701, mt2712, mt7623 and mt8173. > > > - mediatek,larbs : List of phandle to the local arbiters in the current Socs. > > Refer to bindings/memory-controllers/mediatek,smi-larb.txt. It must sort > > according to the local arbiter index, like larb0, larb1, larb2... > > - iommu-cells : must be 1. This is the mtk_m4u_id according to the HW. > > Specifies the mtk_m4u_id as defined in > > dt-binding/memory/mt2701-larb-port.h for mt2701, mt7623 > > - dt-binding/memory/mt2712-larb-port.h for mt2712, and > > - dt-binding/memory/mt8173-larb-port.h for mt8173. > > + dt-binding/memory/mt2712-larb-port.h for mt2712, > > + dt-binding/memory/mt8173-larb-port.h for mt8173, and > > + dt-binding/memory/mt8183-larb-port.h for mt8183. > > > > Example: > > iommu: iommu@10205000 { > > diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt > > index e937ddd..8d3240a 100644 > > --- a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt > > +++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt > > @@ -2,9 +2,10 @@ SMI (Smart Multimedia Interface) Common > > > > The hardware block diagram please check bindings/iommu/mediatek,iommu.txt > > > > -Mediatek SMI have two generations of HW architecture, mt2712 and mt8173 use > > -the second generation of SMI HW while mt2701 uses the first generation HW of > > -SMI. > > +Mediatek SMI have two generations of HW architecture, here is the list > > +which generation the Socs use: > > +generation 1: mt2701 and mt7623. > > +generation 2: mt2712, mt8173 and mt8183. > > > > There's slight differences between the two SMI, for generation 2, the > > register which control the iommu port is at each larb's register base. But > > @@ -19,6 +20,7 @@ Required properties: > > "mediatek,mt2712-smi-common" > > "mediatek,mt7623-smi-common", "mediatek,mt2701-smi-common" > > "mediatek,mt8173-smi-common" > > + "mediatek,mt8183-smi-common" > > - reg : the register and size of the SMI block. > > - power-domains : a phandle to the power domain of this local arbiter. > > - clocks : Must contain an entry for each entry in clock-names. > > @@ -30,6 +32,9 @@ Required properties: > > They may be the same if both source clocks are the same. > > - "async" : asynchronous clock, it help transform the smi clock into the emi > > clock domain, this clock is only needed by generation 1 smi HW. > > + and these 2 option clocks for generation 2 smi HW: > > + - "gals0": the path0 clock of GALS(Global Async Local Sync). > > + - "gals1": the path1 clock of GALS(Global Async Local Sync). > > It would also be nice to specify these more clearly too, since these > are also required for some SoCs and not used on others (as in patch > 12). I will add: + Here is the list which has the "gals": mt8183.