diff mbox

[01/12] mmc: dt-bindings: update Mediatek MMC bindings

Message ID 1505207272-16983-2-git-send-email-chaotian.jing@mediatek.com (mailing list archive)
State New, archived
Headers show

Commit Message

Chaotian Jing (井朝天) Sept. 12, 2017, 9:07 a.m. UTC
Change the comptiable for support of multi-platform
Add description for reg
Add description for source_cg
Add description for mediatek,latch-ck

Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
---
 Documentation/devicetree/bindings/mmc/mtk-sd.txt | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

Comments

Rob Herring Sept. 13, 2017, 2:10 p.m. UTC | #1
On Tue, Sep 12, 2017 at 05:07:41PM +0800, Chaotian Jing wrote:
> Change the comptiable for support of multi-platform
> Add description for reg
> Add description for source_cg
> Add description for mediatek,latch-ck

This is at least the 3rd patch with exactly the same vague subject. 
Please make the subject somewhat unique.

> 
> Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
> ---
>  Documentation/devicetree/bindings/mmc/mtk-sd.txt | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.txt b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> index 4182ea3..405cd06 100644
> --- a/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> @@ -7,10 +7,15 @@ This file documents differences between the core properties in mmc.txt
>  and the properties used by the msdc driver.
>  
>  Required properties:
> -- compatible: Should be "mediatek,mt8173-mmc","mediatek,mt8135-mmc"
> +- compatible: value should be either of the following.
> +	"mediatek,mt8135-mmc": for mmc host ip compatible with mt8135
> +	"mediatek,mt8173-mmc": for mmc host ip compatible with mt8173
> +	"mediatek,mt2701-mmc": for mmc host ip compatible with mt2701
> +	"mediatek,mt2712-mmc": for mmc host ip compatible with mt2712
> +- reg: physical base address of the controller and length
>  - interrupts: Should contain MSDC interrupt number
> -- clocks: MSDC source clock, HCLK
> -- clock-names: "source", "hclk"
> +- clocks: MSDC source clock, HCLK, source_cg
> +- clock-names: "source", "hclk", "source_cg"

All chips support source_cg? That's not backwards compatible for 
existing compatible strings if the driver requires it.

>  - pinctrl-names: should be "default", "state_uhs"
>  - pinctrl-0: should contain default/high speed pin ctrl
>  - pinctrl-1: should contain uhs mode pin ctrl
> @@ -30,6 +35,8 @@ Optional properties:
>  - mediatek,hs400-cmd-resp-sel-rising:  HS400 command response sample selection
>  				       If present,HS400 command responses are sampled on rising edges.
>  				       If not present,HS400 command responses are sampled on falling edges.
> +- mediatek,latch-ck: Some projects do not support enhance_rx, need set correct latch-ck to avoid data crc

What's a project?

> +		     error caused by stop clock(fifo full)
>  
>  Examples:
>  mmc0: mmc@11230000 {
> -- 
> 1.8.1.1.dirty
>
Chaotian Jing (井朝天) Sept. 14, 2017, 2:10 a.m. UTC | #2
On Wed, 2017-09-13 at 09:10 -0500, Rob Herring wrote:
> On Tue, Sep 12, 2017 at 05:07:41PM +0800, Chaotian Jing wrote:
> > Change the comptiable for support of multi-platform
> > Add description for reg
> > Add description for source_cg
> > Add description for mediatek,latch-ck
> 
> This is at least the 3rd patch with exactly the same vague subject. 
> Please make the subject somewhat unique.
> 
Thx, will change the subject at next version
> > 
> > Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
> > ---
> >  Documentation/devicetree/bindings/mmc/mtk-sd.txt | 13 ++++++++++---
> >  1 file changed, 10 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.txt b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> > index 4182ea3..405cd06 100644
> > --- a/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> > +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> > @@ -7,10 +7,15 @@ This file documents differences between the core properties in mmc.txt
> >  and the properties used by the msdc driver.
> >  
> >  Required properties:
> > -- compatible: Should be "mediatek,mt8173-mmc","mediatek,mt8135-mmc"
> > +- compatible: value should be either of the following.
> > +	"mediatek,mt8135-mmc": for mmc host ip compatible with mt8135
> > +	"mediatek,mt8173-mmc": for mmc host ip compatible with mt8173
> > +	"mediatek,mt2701-mmc": for mmc host ip compatible with mt2701
> > +	"mediatek,mt2712-mmc": for mmc host ip compatible with mt2712
> > +- reg: physical base address of the controller and length
> >  - interrupts: Should contain MSDC interrupt number
> > -- clocks: MSDC source clock, HCLK
> > -- clock-names: "source", "hclk"
> > +- clocks: MSDC source clock, HCLK, source_cg
> > +- clock-names: "source", "hclk", "source_cg"
> 
> All chips support source_cg? That's not backwards compatible for 
> existing compatible strings if the driver requires it.
Not all chips support source_cg, for chips which do not support
source_cg, no need source_cg here, and the driver will parse it
to know if current chip support it.
> 
> >  - pinctrl-names: should be "default", "state_uhs"
> >  - pinctrl-0: should contain default/high speed pin ctrl
> >  - pinctrl-1: should contain uhs mode pin ctrl
> > @@ -30,6 +35,8 @@ Optional properties:
> >  - mediatek,hs400-cmd-resp-sel-rising:  HS400 command response sample selection
> >  				       If present,HS400 command responses are sampled on rising edges.
> >  				       If not present,HS400 command responses are sampled on falling edges.
> > +- mediatek,latch-ck: Some projects do not support enhance_rx, need set correct latch-ck to avoid data crc
> 
> What's a project?
eg, MT2701
> 
> > +		     error caused by stop clock(fifo full)
> >  
> >  Examples:
> >  mmc0: mmc@11230000 {
> > -- 
> > 1.8.1.1.dirty
> >
Linus Walleij Sept. 21, 2017, 11:38 a.m. UTC | #3
On Thu, Sep 14, 2017 at 4:10 AM, Chaotian Jing
<chaotian.jing@mediatek.com> wrote:
> On Wed, 2017-09-13 at 09:10 -0500, Rob Herring wrote:
>> On Tue, Sep 12, 2017 at 05:07:41PM +0800, Chaotian Jing wrote:

>> > +- mediatek,latch-ck: Some projects do not support enhance_rx, need set correct latch-ck to avoid data crc
>>
>> What's a project?
> eg, MT2701

Replace the word "project" with the word "system" or "SoC" in
your patch.

Yours,
Linus Walleij
Ulf Hansson Sept. 26, 2017, 10:33 p.m. UTC | #4
On 14 September 2017 at 04:10, Chaotian Jing <chaotian.jing@mediatek.com> wrote:
> On Wed, 2017-09-13 at 09:10 -0500, Rob Herring wrote:
>> On Tue, Sep 12, 2017 at 05:07:41PM +0800, Chaotian Jing wrote:
>> > Change the comptiable for support of multi-platform
>> > Add description for reg
>> > Add description for source_cg
>> > Add description for mediatek,latch-ck
>>
>> This is at least the 3rd patch with exactly the same vague subject.
>> Please make the subject somewhat unique.
>>
> Thx, will change the subject at next version
>> >
>> > Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
>> > ---
>> >  Documentation/devicetree/bindings/mmc/mtk-sd.txt | 13 ++++++++++---
>> >  1 file changed, 10 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.txt b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
>> > index 4182ea3..405cd06 100644
>> > --- a/Documentation/devicetree/bindings/mmc/mtk-sd.txt
>> > +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
>> > @@ -7,10 +7,15 @@ This file documents differences between the core properties in mmc.txt
>> >  and the properties used by the msdc driver.
>> >
>> >  Required properties:
>> > -- compatible: Should be "mediatek,mt8173-mmc","mediatek,mt8135-mmc"
>> > +- compatible: value should be either of the following.
>> > +   "mediatek,mt8135-mmc": for mmc host ip compatible with mt8135
>> > +   "mediatek,mt8173-mmc": for mmc host ip compatible with mt8173
>> > +   "mediatek,mt2701-mmc": for mmc host ip compatible with mt2701
>> > +   "mediatek,mt2712-mmc": for mmc host ip compatible with mt2712
>> > +- reg: physical base address of the controller and length
>> >  - interrupts: Should contain MSDC interrupt number
>> > -- clocks: MSDC source clock, HCLK
>> > -- clock-names: "source", "hclk"
>> > +- clocks: MSDC source clock, HCLK, source_cg
>> > +- clock-names: "source", "hclk", "source_cg"
>>
>> All chips support source_cg? That's not backwards compatible for
>> existing compatible strings if the driver requires it.
> Not all chips support source_cg, for chips which do not support
> source_cg, no need source_cg here, and the driver will parse it
> to know if current chip support it.

In such case you must not add add a required binding for it. I think
that is what Rob is trying to point out for you.

[...]

Kind regards
Uffe
Chaotian Jing (井朝天) Sept. 27, 2017, 1:18 a.m. UTC | #5
On Wed, 2017-09-27 at 00:33 +0200, Ulf Hansson wrote:
> On 14 September 2017 at 04:10, Chaotian Jing <chaotian.jing@mediatek.com> wrote:
> > On Wed, 2017-09-13 at 09:10 -0500, Rob Herring wrote:
> >> On Tue, Sep 12, 2017 at 05:07:41PM +0800, Chaotian Jing wrote:
> >> > Change the comptiable for support of multi-platform
> >> > Add description for reg
> >> > Add description for source_cg
> >> > Add description for mediatek,latch-ck
> >>
> >> This is at least the 3rd patch with exactly the same vague subject.
> >> Please make the subject somewhat unique.
> >>
> > Thx, will change the subject at next version
> >> >
> >> > Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
> >> > ---
> >> >  Documentation/devicetree/bindings/mmc/mtk-sd.txt | 13 ++++++++++---
> >> >  1 file changed, 10 insertions(+), 3 deletions(-)
> >> >
> >> > diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.txt b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> >> > index 4182ea3..405cd06 100644
> >> > --- a/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> >> > +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> >> > @@ -7,10 +7,15 @@ This file documents differences between the core properties in mmc.txt
> >> >  and the properties used by the msdc driver.
> >> >
> >> >  Required properties:
> >> > -- compatible: Should be "mediatek,mt8173-mmc","mediatek,mt8135-mmc"
> >> > +- compatible: value should be either of the following.
> >> > +   "mediatek,mt8135-mmc": for mmc host ip compatible with mt8135
> >> > +   "mediatek,mt8173-mmc": for mmc host ip compatible with mt8173
> >> > +   "mediatek,mt2701-mmc": for mmc host ip compatible with mt2701
> >> > +   "mediatek,mt2712-mmc": for mmc host ip compatible with mt2712
> >> > +- reg: physical base address of the controller and length
> >> >  - interrupts: Should contain MSDC interrupt number
> >> > -- clocks: MSDC source clock, HCLK
> >> > -- clock-names: "source", "hclk"
> >> > +- clocks: MSDC source clock, HCLK, source_cg
> >> > +- clock-names: "source", "hclk", "source_cg"
> >>
> >> All chips support source_cg? That's not backwards compatible for
> >> existing compatible strings if the driver requires it.
> > Not all chips support source_cg, for chips which do not support
> > source_cg, no need source_cg here, and the driver will parse it
> > to know if current chip support it.
> 
> In such case you must not add add a required binding for it. I think
> that is what Rob is trying to point out for you.
> 
> [...]
> 
> Kind regards
> Uffe
The source_cg is required(MUST) at MT2712 and future SoCs, but not
required(do not have it) at previous SoCs, so that put it at required
properties, let the driver to handle it.
Chaotian Jing (井朝天) Sept. 29, 2017, 1:56 a.m. UTC | #6
On Wed, 2017-09-27 at 09:18 +0800, Chaotian Jing wrote:
> On Wed, 2017-09-27 at 00:33 +0200, Ulf Hansson wrote:
> > On 14 September 2017 at 04:10, Chaotian Jing <chaotian.jing@mediatek.com> wrote:
> > > On Wed, 2017-09-13 at 09:10 -0500, Rob Herring wrote:
> > >> On Tue, Sep 12, 2017 at 05:07:41PM +0800, Chaotian Jing wrote:
> > >> > Change the comptiable for support of multi-platform
> > >> > Add description for reg
> > >> > Add description for source_cg
> > >> > Add description for mediatek,latch-ck
> > >>
> > >> This is at least the 3rd patch with exactly the same vague subject.
> > >> Please make the subject somewhat unique.
> > >>
> > > Thx, will change the subject at next version
> > >> >
> > >> > Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
> > >> > ---
> > >> >  Documentation/devicetree/bindings/mmc/mtk-sd.txt | 13 ++++++++++---
> > >> >  1 file changed, 10 insertions(+), 3 deletions(-)
> > >> >
> > >> > diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.txt b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> > >> > index 4182ea3..405cd06 100644
> > >> > --- a/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> > >> > +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> > >> > @@ -7,10 +7,15 @@ This file documents differences between the core properties in mmc.txt
> > >> >  and the properties used by the msdc driver.
> > >> >
> > >> >  Required properties:
> > >> > -- compatible: Should be "mediatek,mt8173-mmc","mediatek,mt8135-mmc"
> > >> > +- compatible: value should be either of the following.
> > >> > +   "mediatek,mt8135-mmc": for mmc host ip compatible with mt8135
> > >> > +   "mediatek,mt8173-mmc": for mmc host ip compatible with mt8173
> > >> > +   "mediatek,mt2701-mmc": for mmc host ip compatible with mt2701
> > >> > +   "mediatek,mt2712-mmc": for mmc host ip compatible with mt2712
> > >> > +- reg: physical base address of the controller and length
> > >> >  - interrupts: Should contain MSDC interrupt number
> > >> > -- clocks: MSDC source clock, HCLK
> > >> > -- clock-names: "source", "hclk"
> > >> > +- clocks: MSDC source clock, HCLK, source_cg
> > >> > +- clock-names: "source", "hclk", "source_cg"
> > >>
> > >> All chips support source_cg? That's not backwards compatible for
> > >> existing compatible strings if the driver requires it.
> > > Not all chips support source_cg, for chips which do not support
> > > source_cg, no need source_cg here, and the driver will parse it
> > > to know if current chip support it.
> > 
> > In such case you must not add add a required binding for it. I think
> > that is what Rob is trying to point out for you.
> > 
> > [...]
> > 
> > Kind regards
> > Uffe
> The source_cg is required(MUST) at MT2712 and future SoCs, but not
> required(do not have it) at previous SoCs, so that put it at required
> properties, let the driver to handle it.

Any other comments about it ? still must not add a required binding for
it ? if add a optional binding for it, how to add it ? as cannot
duplicate "clocks" & "clock-names" in one node.
Ulf Hansson Oct. 2, 2017, 6:53 a.m. UTC | #7
[...]

>> > >> >  Required properties:
>> > >> > -- compatible: Should be "mediatek,mt8173-mmc","mediatek,mt8135-mmc"
>> > >> > +- compatible: value should be either of the following.
>> > >> > +   "mediatek,mt8135-mmc": for mmc host ip compatible with mt8135
>> > >> > +   "mediatek,mt8173-mmc": for mmc host ip compatible with mt8173
>> > >> > +   "mediatek,mt2701-mmc": for mmc host ip compatible with mt2701
>> > >> > +   "mediatek,mt2712-mmc": for mmc host ip compatible with mt2712
>> > >> > +- reg: physical base address of the controller and length
>> > >> >  - interrupts: Should contain MSDC interrupt number
>> > >> > -- clocks: MSDC source clock, HCLK
>> > >> > -- clock-names: "source", "hclk"
>> > >> > +- clocks: MSDC source clock, HCLK, source_cg
>> > >> > +- clock-names: "source", "hclk", "source_cg"
>> > >>
>> > >> All chips support source_cg? That's not backwards compatible for
>> > >> existing compatible strings if the driver requires it.
>> > > Not all chips support source_cg, for chips which do not support
>> > > source_cg, no need source_cg here, and the driver will parse it
>> > > to know if current chip support it.
>> >
>> > In such case you must not add add a required binding for it. I think
>> > that is what Rob is trying to point out for you.
>> >
>> > [...]
>> >
>> > Kind regards
>> > Uffe
>> The source_cg is required(MUST) at MT2712 and future SoCs, but not
>> required(do not have it) at previous SoCs, so that put it at required
>> properties, let the driver to handle it.

Then you must explain that in the binding...

On 29 September 2017 at 03:56, Chaotian Jing <chaotian.jing@mediatek.com> wrote:
> On Wed, 2017-09-27 at 09:18 +0800, Chaotian Jing wrote:
>> On Wed, 2017-09-27 at 00:33 +0200, Ulf Hansson wrote:
>> > On 14 September 2017 at 04:10, Chaotian Jing <chaotian.jing@mediatek.com> wrote:
>> > > On Wed, 2017-09-13 at 09:10 -0500, Rob Herring wrote:
>> > >> On Tue, Sep 12, 2017 at 05:07:41PM +0800, Chaotian Jing wrote:
>> > >> > Change the comptiable for support of multi-platform
>> > >> > Add description for reg
>> > >> > Add description for source_cg
>> > >> > Add description for mediatek,latch-ck
>> > >>
>> > >> This is at least the 3rd patch with exactly the same vague subject.
>> > >> Please make the subject somewhat unique.
>> > >>
>> > > Thx, will change the subject at next version
>> > >> >
>> > >> > Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
>> > >> > ---
>> > >> >  Documentation/devicetree/bindings/mmc/mtk-sd.txt | 13 ++++++++++---
>> > >> >  1 file changed, 10 insertions(+), 3 deletions(-)
>> > >> >
>> > >> > diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.txt b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
>> > >> > index 4182ea3..405cd06 100644
>> > >> > --- a/Documentation/devicetree/bindings/mmc/mtk-sd.txt
>> > >> > +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
>> > >> > @@ -7,10 +7,15 @@ This file documents differences between the core properties in mmc.txt
>> > >> >  and the properties used by the msdc driver.
>> > >> >
>
> Any other comments about it ? still must not add a required binding for
> it ? if add a optional binding for it, how to add it ? as cannot
> duplicate "clocks" & "clock-names" in one node.
>
>

I suggest you keep the description of the new clock name part of the
"Required properties:" header. However, after each clock name,
explicit state when the clock is required/optional. Something along
the lines as sdhci-msm does it.
Documentation/devicetree/bindings/mmc/sdhci-msm.txt

Kind regards
Uffe
Yingjoe Chen Oct. 2, 2017, 10:49 a.m. UTC | #8
On Fri, 2017-09-29 at 09:56 +0800, Chaotian Jing wrote:
> On Wed, 2017-09-27 at 09:18 +0800, Chaotian Jing wrote:
> > On Wed, 2017-09-27 at 00:33 +0200, Ulf Hansson wrote:
> > > On 14 September 2017 at 04:10, Chaotian Jing <chaotian.jing@mediatek.com> wrote:
> > > > On Wed, 2017-09-13 at 09:10 -0500, Rob Herring wrote:
> > > >> On Tue, Sep 12, 2017 at 05:07:41PM +0800, Chaotian Jing wrote:
> > > >> > Change the comptiable for support of multi-platform
> > > >> > Add description for reg
> > > >> > Add description for source_cg
> > > >> > Add description for mediatek,latch-ck
> > > >>
> > > >> This is at least the 3rd patch with exactly the same vague subject.
> > > >> Please make the subject somewhat unique.
> > > >>
> > > > Thx, will change the subject at next version
> > > >> >
> > > >> > Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
> > > >> > ---
> > > >> >  Documentation/devicetree/bindings/mmc/mtk-sd.txt | 13 ++++++++++---
> > > >> >  1 file changed, 10 insertions(+), 3 deletions(-)
> > > >> >
> > > >> > diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.txt b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> > > >> > index 4182ea3..405cd06 100644
> > > >> > --- a/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> > > >> > +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> > > >> > @@ -7,10 +7,15 @@ This file documents differences between the core properties in mmc.txt
> > > >> >  and the properties used by the msdc driver.
> > > >> >
> > > >> >  Required properties:
> > > >> > -- compatible: Should be "mediatek,mt8173-mmc","mediatek,mt8135-mmc"
> > > >> > +- compatible: value should be either of the following.
> > > >> > +   "mediatek,mt8135-mmc": for mmc host ip compatible with mt8135
> > > >> > +   "mediatek,mt8173-mmc": for mmc host ip compatible with mt8173
> > > >> > +   "mediatek,mt2701-mmc": for mmc host ip compatible with mt2701
> > > >> > +   "mediatek,mt2712-mmc": for mmc host ip compatible with mt2712
> > > >> > +- reg: physical base address of the controller and length
> > > >> >  - interrupts: Should contain MSDC interrupt number
> > > >> > -- clocks: MSDC source clock, HCLK
> > > >> > -- clock-names: "source", "hclk"
> > > >> > +- clocks: MSDC source clock, HCLK, source_cg
> > > >> > +- clock-names: "source", "hclk", "source_cg"
> > > >>
> > > >> All chips support source_cg? That's not backwards compatible for
> > > >> existing compatible strings if the driver requires it.
> > > > Not all chips support source_cg, for chips which do not support
> > > > source_cg, no need source_cg here, and the driver will parse it
> > > > to know if current chip support it.
> > > 
> > > In such case you must not add add a required binding for it. I think
> > > that is what Rob is trying to point out for you.
> > > 
> > > [...]
> > > 
> > > Kind regards
> > > Uffe
> > The source_cg is required(MUST) at MT2712 and future SoCs, but not
> > required(do not have it) at previous SoCs, so that put it at required
> > properties, let the driver to handle it.
> 
> Any other comments about it ? still must not add a required binding for
> it ? if add a optional binding for it, how to add it ? as cannot
> duplicate "clocks" & "clock-names" in one node.


Chaotian,

You can follow examples of scpsys [1]:

- clock, clock-names: clocks according to the common clock binding.
                      These are clocks which hardware needs to be
                      enabled before enabling certain power domains.
	Required clocks for MT2701: "mm", "mfg", "ethif"
	Required clocks for MT6797: "mm", "mfg", "vdec"
	Required clocks for MT8173: "mm", "mfg", "venc", "venc_lt"

[1]:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/tree/Documentation/devicetree/bindings/soc/mediatek/scpsys.txt?h=v4.13.4

Joe.C
Chaotian Jing (井朝天) Oct. 9, 2017, 2:16 a.m. UTC | #9
On Mon, 2017-10-02 at 08:53 +0200, Ulf Hansson wrote:
> [...]
> 
> >> > >> >  Required properties:
> >> > >> > -- compatible: Should be "mediatek,mt8173-mmc","mediatek,mt8135-mmc"
> >> > >> > +- compatible: value should be either of the following.
> >> > >> > +   "mediatek,mt8135-mmc": for mmc host ip compatible with mt8135
> >> > >> > +   "mediatek,mt8173-mmc": for mmc host ip compatible with mt8173
> >> > >> > +   "mediatek,mt2701-mmc": for mmc host ip compatible with mt2701
> >> > >> > +   "mediatek,mt2712-mmc": for mmc host ip compatible with mt2712
> >> > >> > +- reg: physical base address of the controller and length
> >> > >> >  - interrupts: Should contain MSDC interrupt number
> >> > >> > -- clocks: MSDC source clock, HCLK
> >> > >> > -- clock-names: "source", "hclk"
> >> > >> > +- clocks: MSDC source clock, HCLK, source_cg
> >> > >> > +- clock-names: "source", "hclk", "source_cg"
> >> > >>
> >> > >> All chips support source_cg? That's not backwards compatible for
> >> > >> existing compatible strings if the driver requires it.
> >> > > Not all chips support source_cg, for chips which do not support
> >> > > source_cg, no need source_cg here, and the driver will parse it
> >> > > to know if current chip support it.
> >> >
> >> > In such case you must not add add a required binding for it. I think
> >> > that is what Rob is trying to point out for you.
> >> >
> >> > [...]
> >> >
> >> > Kind regards
> >> > Uffe
> >> The source_cg is required(MUST) at MT2712 and future SoCs, but not
> >> required(do not have it) at previous SoCs, so that put it at required
> >> properties, let the driver to handle it.
> 
> Then you must explain that in the binding...
> 
> On 29 September 2017 at 03:56, Chaotian Jing <chaotian.jing@mediatek.com> wrote:
> > On Wed, 2017-09-27 at 09:18 +0800, Chaotian Jing wrote:
> >> On Wed, 2017-09-27 at 00:33 +0200, Ulf Hansson wrote:
> >> > On 14 September 2017 at 04:10, Chaotian Jing <chaotian.jing@mediatek.com> wrote:
> >> > > On Wed, 2017-09-13 at 09:10 -0500, Rob Herring wrote:
> >> > >> On Tue, Sep 12, 2017 at 05:07:41PM +0800, Chaotian Jing wrote:
> >> > >> > Change the comptiable for support of multi-platform
> >> > >> > Add description for reg
> >> > >> > Add description for source_cg
> >> > >> > Add description for mediatek,latch-ck
> >> > >>
> >> > >> This is at least the 3rd patch with exactly the same vague subject.
> >> > >> Please make the subject somewhat unique.
> >> > >>
> >> > > Thx, will change the subject at next version
> >> > >> >
> >> > >> > Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
> >> > >> > ---
> >> > >> >  Documentation/devicetree/bindings/mmc/mtk-sd.txt | 13 ++++++++++---
> >> > >> >  1 file changed, 10 insertions(+), 3 deletions(-)
> >> > >> >
> >> > >> > diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.txt b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> >> > >> > index 4182ea3..405cd06 100644
> >> > >> > --- a/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> >> > >> > +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> >> > >> > @@ -7,10 +7,15 @@ This file documents differences between the core properties in mmc.txt
> >> > >> >  and the properties used by the msdc driver.
> >> > >> >
> >
> > Any other comments about it ? still must not add a required binding for
> > it ? if add a optional binding for it, how to add it ? as cannot
> > duplicate "clocks" & "clock-names" in one node.
> >
> >
> 
> I suggest you keep the description of the new clock name part of the
> "Required properties:" header. However, after each clock name,
> explicit state when the clock is required/optional. Something along
> the lines as sdhci-msm does it.
> Documentation/devicetree/bindings/mmc/sdhci-msm.txt
> 
> Kind regards
> Uffe

Thx, I will send a new series of patch v2 to review.
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.txt b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
index 4182ea3..405cd06 100644
--- a/Documentation/devicetree/bindings/mmc/mtk-sd.txt
+++ b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
@@ -7,10 +7,15 @@  This file documents differences between the core properties in mmc.txt
 and the properties used by the msdc driver.
 
 Required properties:
-- compatible: Should be "mediatek,mt8173-mmc","mediatek,mt8135-mmc"
+- compatible: value should be either of the following.
+	"mediatek,mt8135-mmc": for mmc host ip compatible with mt8135
+	"mediatek,mt8173-mmc": for mmc host ip compatible with mt8173
+	"mediatek,mt2701-mmc": for mmc host ip compatible with mt2701
+	"mediatek,mt2712-mmc": for mmc host ip compatible with mt2712
+- reg: physical base address of the controller and length
 - interrupts: Should contain MSDC interrupt number
-- clocks: MSDC source clock, HCLK
-- clock-names: "source", "hclk"
+- clocks: MSDC source clock, HCLK, source_cg
+- clock-names: "source", "hclk", "source_cg"
 - pinctrl-names: should be "default", "state_uhs"
 - pinctrl-0: should contain default/high speed pin ctrl
 - pinctrl-1: should contain uhs mode pin ctrl
@@ -30,6 +35,8 @@  Optional properties:
 - mediatek,hs400-cmd-resp-sel-rising:  HS400 command response sample selection
 				       If present,HS400 command responses are sampled on rising edges.
 				       If not present,HS400 command responses are sampled on falling edges.
+- mediatek,latch-ck: Some projects do not support enhance_rx, need set correct latch-ck to avoid data crc
+		     error caused by stop clock(fifo full)
 
 Examples:
 mmc0: mmc@11230000 {