| Message ID | 1425638900-24989-4-git-send-email-yong.wu@mediatek.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Fri, Mar 06, 2015 at 10:48:18AM +0000, yong.wu@mediatek.com wrote: > From: Yong Wu <yong.wu@mediatek.com> > > This patch add smi binding document. Please move binding documents to the start of the series. It makes things far easier to review. > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > --- > .../devicetree/bindings/soc/mediatek/mediatek,smi.txt | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > create mode 100644 Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt > > diff --git a/Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt b/Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt > new file mode 100644 > index 0000000..0fc9d1a > --- /dev/null > +++ b/Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt > @@ -0,0 +1,17 @@ > +SMI hardware block diagram please help check <bindings/iommu/mediatek,iommu.txt> > + > +Required properties: > +- compatible : must be "mediatek,mediatek,mt8173-smi-larb" Double vendor prefix? What does "larb" mean? It would be nice for the intorductory paragraph in this file to explain. > +- reg : the register of each local arbiter > +- clocks : the clocks of each local arbiter > +- clock-name: larb_sub*(3 clockes at most) The names required _must_ be specified here, or clock-names is pointless. The clock names should be from the PoV of _this_ device (i.e. they should be the names of the inputs) not from the PoV of the provider (i.e. they should not be the names of the outputs from the provider). Mark. > + > +Example: > + larb1:larb@16010000 { > + compatible = "mediatek,mt8173-smi-larb"; > + reg = <0 0x16010000 0 0x1000>; > + clocks = <&mmsys MM_SMI_COMMON>, > + <&vdecsys VDEC_CKEN>, > + <&vdecsys VDEC_LARB_CKEN>; > + clock-names = "larb_sub0", "larb_sub1", "larb_sub2"; > + }; > -- > 1.8.1.1.dirty > > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 3/6/2015 1:48 PM, yong.wu@mediatek.com wrote: > From: Yong Wu <yong.wu@mediatek.com> > This patch add smi binding document. > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > --- > .../devicetree/bindings/soc/mediatek/mediatek,smi.txt | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > create mode 100644 Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt > diff --git a/Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt b/Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt > new file mode 100644 > index 0000000..0fc9d1a > --- /dev/null > +++ b/Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt > @@ -0,0 +1,17 @@ > +SMI hardware block diagram please help check <bindings/iommu/mediatek,iommu.txt> > + > +Required properties: > +- compatible : must be "mediatek,mediatek,mt8173-smi-larb" One "mediatek," is enough. :-) > +- reg : the register of each local arbiter > +- clocks : the clocks of each local arbiter > +- clock-name: larb_sub*(3 clockes at most) clock-names. > + > +Example: > + larb1:larb@16010000 { > + compatible = "mediatek,mt8173-smi-larb"; > + reg = <0 0x16010000 0 0x1000>; > + clocks = <&mmsys MM_SMI_COMMON>, > + <&vdecsys VDEC_CKEN>, Please align with the < above. > + <&vdecsys VDEC_LARB_CKEN>; Use tabs instead of spaces, please. > + clock-names = "larb_sub0", "larb_sub1", "larb_sub2"; > + }; WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear Sergei,
Thanks very much for suggestion.
I will fix them in the next version.
On Fri, 2015-03-06 at 17:48 +0300, Sergei Shtylyov wrote:
> Hello.
>
> On 3/6/2015 1:48 PM, yong.wu@mediatek.com wrote:
>
> > From: Yong Wu <yong.wu@mediatek.com>
>
> > This patch add smi binding document.
>
> > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > ---
> > .../devicetree/bindings/soc/mediatek/mediatek,smi.txt | 17 +++++++++++++++++
> > 1 file changed, 17 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt
>
> > diff --git a/Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt b/Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt
> > new file mode 100644
> > index 0000000..0fc9d1a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt
> > @@ -0,0 +1,17 @@
> > +SMI hardware block diagram please help check <bindings/iommu/mediatek,iommu.txt>
> > +
> > +Required properties:
> > +- compatible : must be "mediatek,mediatek,mt8173-smi-larb"
>
> One "mediatek," is enough. :-)
>
> > +- reg : the register of each local arbiter
> > +- clocks : the clocks of each local arbiter
> > +- clock-name: larb_sub*(3 clockes at most)
>
> clock-names.
>
> > +
> > +Example:
> > + larb1:larb@16010000 {
> > + compatible = "mediatek,mt8173-smi-larb";
> > + reg = <0 0x16010000 0 0x1000>;
> > + clocks = <&mmsys MM_SMI_COMMON>,
> > + <&vdecsys VDEC_CKEN>,
>
> Please align with the < above.
>
> > + <&vdecsys VDEC_LARB_CKEN>;
>
> Use tabs instead of spaces, please.
>
> > + clock-names = "larb_sub0", "larb_sub1", "larb_sub2";
> > + };
>
> WBR, Sergei
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear Mark,
Thanks very much for you suggestion.
On Fri, 2015-03-06 at 11:13 +0000, Mark Rutland wrote:
> On Fri, Mar 06, 2015 at 10:48:18AM +0000, yong.wu@mediatek.com wrote:
> > From: Yong Wu <yong.wu@mediatek.com>
> >
> > This patch add smi binding document.
>
> Please move binding documents to the start of the series. It makes
> things far easier to review.
>
> >
> > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > ---
> > .../devicetree/bindings/soc/mediatek/mediatek,smi.txt | 17 +++++++++++++++++
> > 1 file changed, 17 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt
> >
> > diff --git a/Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt b/Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt
> > new file mode 100644
> > index 0000000..0fc9d1a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt
> > @@ -0,0 +1,17 @@
> > +SMI hardware block diagram please help check <bindings/iommu/mediatek,iommu.txt>
> > +
> > +Required properties:
> > +- compatible : must be "mediatek,mediatek,mt8173-smi-larb"
>
> Double vendor prefix?
Yes. I will change this.
>
> What does "larb" mean? It would be nice for the intorductory paragraph
> in this file to explain.
That is local arbiter. I will add a link like this, is it ok?
compatible : must be "mediatek,mt8173-smi-larb", about the "larb",
please check the <bindings/iommu/mediatek,iommu.txt>.
>
> > +- reg : the register of each local arbiter
> > +- clocks : the clocks of each local arbiter
> > +- clock-name: larb_sub*(3 clockes at most)
>
> The names required _must_ be specified here, or clock-names is
> pointless.
>
> The clock names should be from the PoV of _this_ device (i.e. they
> should be the names of the inputs) not from the PoV of the provider
> (i.e. they should not be the names of the outputs from the provider).
I use larb_sub0, larb_sub1,larb_sub2 for more easily to control the
clocks. if we use the PoV, We should list all the clocks name, it will
has a little code. Anyway I will change this in the next version.
>
> Mark.
>
> > +
> > +Example:
> > + larb1:larb@16010000 {
> > + compatible = "mediatek,mt8173-smi-larb";
> > + reg = <0 0x16010000 0 0x1000>;
> > + clocks = <&mmsys MM_SMI_COMMON>,
> > + <&vdecsys VDEC_CKEN>,
> > + <&vdecsys VDEC_LARB_CKEN>;
> > + clock-names = "larb_sub0", "larb_sub1", "larb_sub2";
> > + };
> > --
> > 1.8.1.1.dirty
> >
> >
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Mark,
Thanks very much for review.
About the clock name should be the PoV of _this_ device. Could you
help check below?
On Fri, 2015-03-06 at 11:13 +0000, Mark Rutland wrote:
> On Fri, Mar 06, 2015 at 10:48:18AM +0000, yong.wu@mediatek.com wrote:
> > From: Yong Wu <yong.wu@mediatek.com>
> >
> > This patch add smi binding document.
>
> Please move binding documents to the start of the series. It makes
> things far easier to review.
>
> >
> > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > +++ b/Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt
> > @@ -0,0 +1,17 @@
> > +SMI hardware block diagram please help check <bindings/iommu/mediatek,iommu.txt>
> > +
> > +Required properties:
> > +- compatible : must be "mediatek,mediatek,mt8173-smi-larb"
>
> Double vendor prefix?
>
> What does "larb" mean? It would be nice for the intorductory paragraph
> in this file to explain.
>
> > +- reg : the register of each local arbiter
> > +- clocks : the clocks of each local arbiter
> > +- clock-name: larb_sub*(3 clockes at most)
>
> The names required _must_ be specified here, or clock-names is
> pointless.
>
> The clock names should be from the PoV of _this_ device (i.e. they
> should be the names of the inputs) not from the PoV of the provider
> (i.e. they should not be the names of the outputs from the provider).
>
> Mark.
>
After we check with our SMI Designer. Every SMI local arbiter need
two clocks, which is called APB clocks and SMI clock.
APB clock : Advanced Peripheral Bus Clock. It is the clock for
setting the register of local arbiter.
SMI clock : Smart Multimedia Interface Clock, It is the clock for
transfering the data and command.
And all the local arbiters need the smi common clock, so we
separate it.
Then I prepare to design the smi the dtsi like this:
smi_common:smi@14022000 {
compatible = “mediate, mt8173-smi”;
reg = <0 0x14022000 0 0x1000>;
clocks = <&mmsys MM_SMI_COMMON>;
clocks-names = “smi_common”;
};
larb0: larb@14021000 {
compatible = “mediate, mt8173-smi-larb”;
reg = <0 0x14021000 0 0x1000>;
smi = <&smi_common>;
clocks = <&mmsys MM_SMI_LARB0>,
<&mmsys MM_SMI_LARB0>;
clocks-names = “apb_clk”, “smi_clk”;
};
larb1: larb@16010000 {
compatible = “mediate, mt8173-smi-larb”;
reg = <0 0x16010000 0 0x1000>;
smi = <&smi_common>;
clocks = <&vdecsys VDEC_CKEN>,
<&mmsys VDEC_LARB_CKEN>;
clocks-names = “apb_clk”, “smi_clk”;
};
…
In some local arbiter, the source clock of the APB clock and the
SMI clock may be the same, like larb0. so the two clocks are the same.
And they may be different in other local arbiteres, like larb1.
If it is designed like this, is it ok?
> > +
> > +Example:
> > + larb1:larb@16010000 {
> > + compatible = "mediatek,mt8173-smi-larb";
> > + reg = <0 0x16010000 0 0x1000>;
> > + clocks = <&mmsys MM_SMI_COMMON>,
> > + <&vdecsys VDEC_CKEN>,
> > + <&vdecsys VDEC_LARB_CKEN>;
> > + clock-names = "larb_sub0", "larb_sub1", "larb_sub2";
> > + };
> > --
> > 1.8.1.1.dirty
> >
> >
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 14, 2015 at 10:07:54AM +0100, Yong Wu wrote: > Hi Mark, > Thanks very much for review. > About the clock name should be the PoV of _this_ device. Could you > help check below? > > On Fri, 2015-03-06 at 11:13 +0000, Mark Rutland wrote: > > On Fri, Mar 06, 2015 at 10:48:18AM +0000, yong.wu@mediatek.com wrote: > > > From: Yong Wu <yong.wu@mediatek.com> > > > > > > This patch add smi binding document. > > > > Please move binding documents to the start of the series. It makes > > things far easier to review. > > > > > > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > > > +++ b/Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt > > > @@ -0,0 +1,17 @@ > > > +SMI hardware block diagram please help check <bindings/iommu/mediatek,iommu.txt> > > > + > > > +Required properties: > > > +- compatible : must be "mediatek,mediatek,mt8173-smi-larb" > > > > Double vendor prefix? > > > > What does "larb" mean? It would be nice for the intorductory paragraph > > in this file to explain. > > > > > +- reg : the register of each local arbiter > > > +- clocks : the clocks of each local arbiter > > > +- clock-name: larb_sub*(3 clockes at most) > > > > The names required _must_ be specified here, or clock-names is > > pointless. > > > > The clock names should be from the PoV of _this_ device (i.e. they > > should be the names of the inputs) not from the PoV of the provider > > (i.e. they should not be the names of the outputs from the provider). > > > > Mark. > > > After we check with our SMI Designer. Every SMI local arbiter need > two clocks, which is called APB clocks and SMI clock. > APB clock : Advanced Peripheral Bus Clock. It is the clock for > setting the register of local arbiter. > SMI clock : Smart Multimedia Interface Clock, It is the clock for > transfering the data and command. > > And all the local arbiters need the smi common clock, so we > separate it. > > Then I prepare to design the smi the dtsi like this: > > smi_common:smi@14022000 { > compatible = “mediate, mt8173-smi”; > reg = <0 0x14022000 0 0x1000>; > clocks = <&mmsys MM_SMI_COMMON>; > clocks-names = “smi_common”; > }; > > larb0: larb@14021000 { > compatible = “mediate, mt8173-smi-larb”; > reg = <0 0x14021000 0 0x1000>; > smi = <&smi_common>; > clocks = <&mmsys MM_SMI_LARB0>, > <&mmsys MM_SMI_LARB0>; > clocks-names = “apb_clk”, “smi_clk”; > }; > > larb1: larb@16010000 { > compatible = “mediate, mt8173-smi-larb”; > reg = <0 0x16010000 0 0x1000>; > smi = <&smi_common>; > clocks = <&vdecsys VDEC_CKEN>, > <&mmsys VDEC_LARB_CKEN>; > clocks-names = “apb_clk”, “smi_clk”; > }; > … > In some local arbiter, the source clock of the APB clock and the > SMI clock may be the same, like larb0. so the two clocks are the same. > And they may be different in other local arbiteres, like larb1. > > If it is designed like this, is it ok? That looks pretty good; the clocks and names on the larb nodes seem sensible. The naming of the "smi_common" clock on the smi_common node looks a bit odd though. Is that really what the clock input is called? Mark. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Mark, On Tue, 2015-04-14 at 11:06 +0100, Mark Rutland wrote: > On Tue, Apr 14, 2015 at 10:07:54AM +0100, Yong Wu wrote: > > Hi Mark, > > Thanks very much for review. > > About the clock name should be the PoV of _this_ device. Could you > > help check below? > > > > On Fri, 2015-03-06 at 11:13 +0000, Mark Rutland wrote: > > > On Fri, Mar 06, 2015 at 10:48:18AM +0000, yong.wu@mediatek.com wrote: > > > > From: Yong Wu <yong.wu@mediatek.com> > > > > > > > > This patch add smi binding document. > > > > > > Please move binding documents to the start of the series. It makes > > > things far easier to review. > > > > > > What does "larb" mean? It would be nice for the intorductory paragraph > > > in this file to explain. > > > > > > > +- reg : the register of each local arbiter > > > > +- clocks : the clocks of each local arbiter > > > > +- clock-name: larb_sub*(3 clockes at most) > > > > > > The names required _must_ be specified here, or clock-names is > > > pointless. > > > > > > The clock names should be from the PoV of _this_ device (i.e. they > > > should be the names of the inputs) not from the PoV of the provider > > > (i.e. they should not be the names of the outputs from the provider). > > > > > > Mark. > > > > > After we check with our SMI Designer. Every SMI local arbiter need > > two clocks, which is called APB clocks and SMI clock. > > APB clock : Advanced Peripheral Bus Clock. It is the clock for > > setting the register of local arbiter. > > SMI clock : Smart Multimedia Interface Clock, It is the clock for > > transfering the data and command. > > > > And all the local arbiters need the smi common clock, so we > > separate it. > > > > Then I prepare to design the smi the dtsi like this: > > > > smi_common:smi@14022000 { > > compatible = “mediate, mt8173-smi”; > > reg = <0 0x14022000 0 0x1000>; > > clocks = <&mmsys MM_SMI_COMMON>; > > clocks-names = “smi_common”; > > }; > > > > larb0: larb@14021000 { > > compatible = “mediate, mt8173-smi-larb”; > > reg = <0 0x14021000 0 0x1000>; > > smi = <&smi_common>; > > clocks = <&mmsys MM_SMI_LARB0>, > > <&mmsys MM_SMI_LARB0>; > > clocks-names = “apb_clk”, “smi_clk”; > > }; > > > > larb1: larb@16010000 { > > compatible = “mediate, mt8173-smi-larb”; > > reg = <0 0x16010000 0 0x1000>; > > smi = <&smi_common>; > > clocks = <&vdecsys VDEC_CKEN>, > > <&mmsys VDEC_LARB_CKEN>; > > clocks-names = “apb_clk”, “smi_clk”; > > }; > > … > > In some local arbiter, the source clock of the APB clock and the > > SMI clock may be the same, like larb0. so the two clocks are the same. > > And they may be different in other local arbiteres, like larb1. > > > > If it is designed like this, is it ok? > > That looks pretty good; the clocks and names on the larb nodes seem > sensible. > > The naming of the "smi_common" clock on the smi_common node looks a bit > odd though. Is that really what the clock input is called? > > Mark. After check with DE, the smi_common clock also have its APB clock and the smi clock(they have the same clock source). And I prepare to delete "_clk" in all the clock-names. So it may be like this: smi_common:smi@14022000 { compatible = “mediate, mt8173-smi”; reg = <0 0x14022000 0 0x1000>; clocks = <&mmsys MM_SMI_COMMON>, <&mmsys MM_SMI_COMMON>; clocks-names = “apk”,"smi"; }; How about this? -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2015-04-14 at 21:49 +0800, Yong Wu wrote: > Hi Mark, > > On Tue, 2015-04-14 at 11:06 +0100, Mark Rutland wrote: > > On Tue, Apr 14, 2015 at 10:07:54AM +0100, Yong Wu wrote: > > > Hi Mark, > > > Thanks very much for review. > > > About the clock name should be the PoV of _this_ device. Could you > > > help check below? > > > > > > On Fri, 2015-03-06 at 11:13 +0000, Mark Rutland wrote: > > > > On Fri, Mar 06, 2015 at 10:48:18AM +0000, yong.wu@mediatek.com wrote: > > > > > From: Yong Wu <yong.wu@mediatek.com> > > > > > > > > > > This patch add smi binding document. > > > > > > > > Please move binding documents to the start of the series. It makes > > > > things far easier to review. > > > > > > > > What does "larb" mean? It would be nice for the intorductory paragraph > > > > in this file to explain. > > > > > > > > > +- reg : the register of each local arbiter > > > > > +- clocks : the clocks of each local arbiter > > > > > +- clock-name: larb_sub*(3 clockes at most) > > > > > > > > The names required _must_ be specified here, or clock-names is > > > > pointless. > > > > > > > > The clock names should be from the PoV of _this_ device (i.e. they > > > > should be the names of the inputs) not from the PoV of the provider > > > > (i.e. they should not be the names of the outputs from the provider). > > > > > > > > Mark. > > > > > > > After we check with our SMI Designer. Every SMI local arbiter need > > > two clocks, which is called APB clocks and SMI clock. > > > APB clock : Advanced Peripheral Bus Clock. It is the clock for > > > setting the register of local arbiter. > > > SMI clock : Smart Multimedia Interface Clock, It is the clock for > > > transfering the data and command. > > > > > > And all the local arbiters need the smi common clock, so we > > > separate it. > > > > > > Then I prepare to design the smi the dtsi like this: > > > > > > smi_common:smi@14022000 { > > > compatible = “mediate, mt8173-smi”; > > > reg = <0 0x14022000 0 0x1000>; > > > clocks = <&mmsys MM_SMI_COMMON>; > > > clocks-names = “smi_common”; > > > }; > > > > > > larb0: larb@14021000 { > > > compatible = “mediate, mt8173-smi-larb”; > > > reg = <0 0x14021000 0 0x1000>; > > > smi = <&smi_common>; > > > clocks = <&mmsys MM_SMI_LARB0>, > > > <&mmsys MM_SMI_LARB0>; > > > clocks-names = “apb_clk”, “smi_clk”; > > > }; > > > > > > larb1: larb@16010000 { > > > compatible = “mediate, mt8173-smi-larb”; > > > reg = <0 0x16010000 0 0x1000>; > > > smi = <&smi_common>; > > > clocks = <&vdecsys VDEC_CKEN>, > > > <&mmsys VDEC_LARB_CKEN>; > > > clocks-names = “apb_clk”, “smi_clk”; > > > }; > > > … > > > In some local arbiter, the source clock of the APB clock and the > > > SMI clock may be the same, like larb0. so the two clocks are the same. > > > And they may be different in other local arbiteres, like larb1. > > > > > > If it is designed like this, is it ok? > > > > That looks pretty good; the clocks and names on the larb nodes seem > > sensible. > > > > The naming of the "smi_common" clock on the smi_common node looks a bit > > odd though. Is that really what the clock input is called? > > > > Mark. > After check with DE, the smi_common clock also have its APB clock > and the smi clock(they have the same clock source). > And I prepare to delete "_clk" in all the clock-names. > So it may be like this: > smi_common:smi@14022000 { > compatible = “mediate, mt8173-smi”; > reg = <0 0x14022000 0 0x1000>; > clocks = <&mmsys MM_SMI_COMMON>, > <&mmsys MM_SMI_COMMON>; > clocks-names = “apk”,"smi"; > }; > How about this? Sorry, I wrote wrong, it should be clock-names = “apb”,"smi"; > > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > > smi_common:smi@14022000 { > > > compatible = “mediate, mt8173-smi”; > > > reg = <0 0x14022000 0 0x1000>; > > > clocks = <&mmsys MM_SMI_COMMON>; > > > clocks-names = “smi_common”; > > > }; > > > > > > larb0: larb@14021000 { > > > compatible = “mediate, mt8173-smi-larb”; > > > reg = <0 0x14021000 0 0x1000>; > > > smi = <&smi_common>; > > > clocks = <&mmsys MM_SMI_LARB0>, > > > <&mmsys MM_SMI_LARB0>; > > > clocks-names = “apb_clk”, “smi_clk”; > > > }; > > > > > > larb1: larb@16010000 { > > > compatible = “mediate, mt8173-smi-larb”; > > > reg = <0 0x16010000 0 0x1000>; > > > smi = <&smi_common>; > > > clocks = <&vdecsys VDEC_CKEN>, > > > <&mmsys VDEC_LARB_CKEN>; > > > clocks-names = “apb_clk”, “smi_clk”; > > > }; > > > … > > > In some local arbiter, the source clock of the APB clock and the > > > SMI clock may be the same, like larb0. so the two clocks are the same. > > > And they may be different in other local arbiteres, like larb1. > > > > > > If it is designed like this, is it ok? > > > > That looks pretty good; the clocks and names on the larb nodes seem > > sensible. > > > > The naming of the "smi_common" clock on the smi_common node looks a bit > > odd though. Is that really what the clock input is called? > > > > Mark. > After check with DE, the smi_common clock also have its APB clock > and the smi clock(they have the same clock source). > And I prepare to delete "_clk" in all the clock-names. > So it may be like this: > smi_common:smi@14022000 { > compatible = “mediate, mt8173-smi”; > reg = <0 0x14022000 0 0x1000>; > clocks = <&mmsys MM_SMI_COMMON>, > <&mmsys MM_SMI_COMMON>; > clocks-names = “apk”,"smi"; > }; > How about this? That looks fine to me. I assume "apk" should be "apb" in the last example. Mark. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt b/Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt new file mode 100644 index 0000000..0fc9d1a --- /dev/null +++ b/Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt @@ -0,0 +1,17 @@ +SMI hardware block diagram please help check <bindings/iommu/mediatek,iommu.txt> + +Required properties: +- compatible : must be "mediatek,mediatek,mt8173-smi-larb" +- reg : the register of each local arbiter +- clocks : the clocks of each local arbiter +- clock-name: larb_sub*(3 clockes at most) + +Example: + larb1:larb@16010000 { + compatible = "mediatek,mt8173-smi-larb"; + reg = <0 0x16010000 0 0x1000>; + clocks = <&mmsys MM_SMI_COMMON>, + <&vdecsys VDEC_CKEN>, + <&vdecsys VDEC_LARB_CKEN>; + clock-names = "larb_sub0", "larb_sub1", "larb_sub2"; + };