diff mbox

[v4,1/3] mmc: dt-bindings: update Mediatek MMC bindings

Message ID 1484988923-1543-2-git-send-email-yong.mao@mediatek.com (mailing list archive)
State New, archived
Headers show

Commit Message

Yong Mao Jan. 21, 2017, 8:55 a.m. UTC
From: yong mao <yong.mao@mediatek.com>

Add description for mediatek,hs200-cmd-int-delay
Add description for mediatek,hs400-cmd-int-delay
Add description for mediatek,hs400-cmd-resp-sel-rising

Signed-off-by: Yong Mao <yong.mao@mediatek.com>
---
 Documentation/devicetree/bindings/mmc/mtk-sd.txt |   12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Ulf Hansson Jan. 24, 2017, 8:02 a.m. UTC | #1
+Rob, devicetree

On 21 January 2017 at 09:55, Yong Mao <yong.mao@mediatek.com> wrote:
> From: yong mao <yong.mao@mediatek.com>
>
> Add description for mediatek,hs200-cmd-int-delay
> Add description for mediatek,hs400-cmd-int-delay
> Add description for mediatek,hs400-cmd-resp-sel-rising
>
> Signed-off-by: Yong Mao <yong.mao@mediatek.com>

You need an ack from a DT maintainer for this. I have added Rob and
the devicetree list on cc.

Kind regards
Uffe

> ---
>  Documentation/devicetree/bindings/mmc/mtk-sd.txt |   12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.txt b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> index 0120c7f..4182ea3 100644
> --- a/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> @@ -21,6 +21,15 @@ Optional properties:
>  - assigned-clocks: PLL of the source clock
>  - assigned-clock-parents: parent of source clock, used for HS400 mode to get 400Mhz source clock
>  - hs400-ds-delay: HS400 DS delay setting
> +- mediatek,hs200-cmd-int-delay: HS200 command internal delay setting.
> +                               This field has total 32 stages.
> +                               The value is an integer from 0 to 31.
> +- mediatek,hs400-cmd-int-delay: HS400 command internal delay setting
> +                               This field has total 32 stages.
> +                               The value is an integer from 0 to 31.
> +- 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.
>
>  Examples:
>  mmc0: mmc@11230000 {
> @@ -38,4 +47,7 @@ mmc0: mmc@11230000 {
>         assigned-clocks = <&topckgen CLK_TOP_MSDC50_0_SEL>;
>         assigned-clock-parents = <&topckgen CLK_TOP_MSDCPLL_D2>;
>         hs400-ds-delay = <0x14015>;
> +       mediatek,hs200-cmd-int-delay = <26>;
> +       mediatek,hs400-cmd-int-delay = <14>;
> +       mediatek,hs400-cmd-resp-sel-rising;
>  };
> --
> 1.7.9.5
>
Yong Mao Feb. 24, 2017, 9:59 a.m. UTC | #2
Dear Rob,

Could you please help to make comments for this patch?
Thanks.

On Tue, 2017-01-24 at 09:02 +0100, Ulf Hansson wrote:
> +Rob, devicetree
> 
> On 21 January 2017 at 09:55, Yong Mao <yong.mao@mediatek.com> wrote:
> > From: yong mao <yong.mao@mediatek.com>
> >
> > Add description for mediatek,hs200-cmd-int-delay
> > Add description for mediatek,hs400-cmd-int-delay
> > Add description for mediatek,hs400-cmd-resp-sel-rising
> >
> > Signed-off-by: Yong Mao <yong.mao@mediatek.com>
> 
> You need an ack from a DT maintainer for this. I have added Rob and
> the devicetree list on cc.
> 
> Kind regards
> Uffe
> 
> > ---
> >  Documentation/devicetree/bindings/mmc/mtk-sd.txt |   12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.txt b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> > index 0120c7f..4182ea3 100644
> > --- a/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> > +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> > @@ -21,6 +21,15 @@ Optional properties:
> >  - assigned-clocks: PLL of the source clock
> >  - assigned-clock-parents: parent of source clock, used for HS400 mode to get 400Mhz source clock
> >  - hs400-ds-delay: HS400 DS delay setting
> > +- mediatek,hs200-cmd-int-delay: HS200 command internal delay setting.
> > +                               This field has total 32 stages.
> > +                               The value is an integer from 0 to 31.
> > +- mediatek,hs400-cmd-int-delay: HS400 command internal delay setting
> > +                               This field has total 32 stages.
> > +                               The value is an integer from 0 to 31.
> > +- 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.
> >
> >  Examples:
> >  mmc0: mmc@11230000 {
> > @@ -38,4 +47,7 @@ mmc0: mmc@11230000 {
> >         assigned-clocks = <&topckgen CLK_TOP_MSDC50_0_SEL>;
> >         assigned-clock-parents = <&topckgen CLK_TOP_MSDCPLL_D2>;
> >         hs400-ds-delay = <0x14015>;
> > +       mediatek,hs200-cmd-int-delay = <26>;
> > +       mediatek,hs400-cmd-int-delay = <14>;
> > +       mediatek,hs400-cmd-resp-sel-rising;
> >  };
> > --
> > 1.7.9.5
> >
Rob Herring (Arm) Feb. 24, 2017, 10:47 p.m. UTC | #3
On Fri, Feb 24, 2017 at 3:59 AM, Yong Mao <yong.mao@mediatek.com> wrote:
> Dear Rob,
>
> Could you please help to make comments for this patch?
> Thanks.

I already did comment. It's still wrong as Ulf commented. So fix and
send a new version. It has to go to the DT list if you want to be in
my queue.

Rob
Yong Mao March 3, 2017, 3:49 a.m. UTC | #4
On Fri, 2017-02-24 at 16:47 -0600, Rob Herring wrote:
> On Fri, Feb 24, 2017 at 3:59 AM, Yong Mao <yong.mao@mediatek.com> wrote:
> > Dear Rob,
> >
> > Could you please help to make comments for this patch?
> > Thanks.
> 
> I already did comment. It's still wrong as Ulf commented. So fix and
> send a new version. It has to go to the DT list if you want to be in
> my queue.
> 
> Rob

After reviewing the history, We guess your mentioned Ulf's comments is as below.
  
"> +- mtk-hs200-cmd-int-delay: HS200 command internal delay setting.
> + The value is an integer from 0 to 31

Please change to:

mediatek,hs200-cmd-delay

... and if there is a unit, like ns or us, please add that a suffix.

> +- mtk-hs400-cmd-int-delay: HS400 command internal delay setting
> + The value is an integer from 0 to 31

mediatek,hs400-cmd-delay and add unit if applicable.

> +- mtk-hs400-cmd-resp-sel: HS400 command response sample selection
> + The value is an integer from 0 to 1

mediatek,hs400-cmd-resp-sel

And make it a boolean value instead!"

==> We already fix this comment in v4. 
We use "mediatek,hs200-cmd-int-delay" to replace "mtk-hs200-cmd-int-delay", 
but not use "mediatek,hs200-cmd-delay". This is because "-int-" here means internal.
We should not drop it.

And this field does not have unit, it only have total 32 stages.
We also change the description in v4.

For comment about "mtk-hs400-cmd-resp-sel", we also make it a boolean value in v4.
And re-name it as "mediatek,hs400-cmd-resp-rising".

Please help to point out where we need to modify.
Thanks.
Yong Mao March 7, 2017, 8:05 a.m. UTC | #5
On Fri, 2017-03-03 at 11:49 +0800, Yong Mao wrote:
> On Fri, 2017-02-24 at 16:47 -0600, Rob Herring wrote:
> > On Fri, Feb 24, 2017 at 3:59 AM, Yong Mao <yong.mao@mediatek.com> wrote:
> > > Dear Rob,
> > >
> > > Could you please help to make comments for this patch?
> > > Thanks.
> > 
> > I already did comment. It's still wrong as Ulf commented. So fix and
> > send a new version. It has to go to the DT list if you want to be in
> > my queue.
> > 
> > Rob
> 
> After reviewing the history, We guess your mentioned Ulf's comments is as below.
>   
> "> +- mtk-hs200-cmd-int-delay: HS200 command internal delay setting.
> > + The value is an integer from 0 to 31
> 
> Please change to:
> 
> mediatek,hs200-cmd-delay
> 
> ... and if there is a unit, like ns or us, please add that a suffix.
> 
> > +- mtk-hs400-cmd-int-delay: HS400 command internal delay setting
> > + The value is an integer from 0 to 31
> 
> mediatek,hs400-cmd-delay and add unit if applicable.
> 
> > +- mtk-hs400-cmd-resp-sel: HS400 command response sample selection
> > + The value is an integer from 0 to 1
> 
> mediatek,hs400-cmd-resp-sel
> 
> And make it a boolean value instead!"
> 
> ==> We already fix this comment in v4. 
> We use "mediatek,hs200-cmd-int-delay" to replace "mtk-hs200-cmd-int-delay", 
> but not use "mediatek,hs200-cmd-delay". This is because "-int-" here means internal.
> We should not drop it.
> 
> And this field does not have unit, it only have total 32 stages.
> We also change the description in v4.
> 
> For comment about "mtk-hs400-cmd-resp-sel", we also make it a boolean value in v4.
> And re-name it as "mediatek,hs400-cmd-resp-rising".
> 
> Please help to point out where we need to modify.
> Thanks.
> 
> 

Please help to point out a bright way for this patch.
We are anxiously waiting for your response.

Best Regards,
Yong Mao.
Ulf Hansson March 14, 2017, 2:51 p.m. UTC | #6
[...]

>>
>
> Please help to point out a bright way for this patch.
> We are anxiously waiting for your response.

Yong, I suggest you make resend of the series. This time make sure you
send to DT maintainers as well.

Kind regards
Uffe
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.txt b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
index 0120c7f..4182ea3 100644
--- a/Documentation/devicetree/bindings/mmc/mtk-sd.txt
+++ b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
@@ -21,6 +21,15 @@  Optional properties:
 - assigned-clocks: PLL of the source clock
 - assigned-clock-parents: parent of source clock, used for HS400 mode to get 400Mhz source clock
 - hs400-ds-delay: HS400 DS delay setting
+- mediatek,hs200-cmd-int-delay: HS200 command internal delay setting.
+				This field has total 32 stages.
+				The value is an integer from 0 to 31.
+- mediatek,hs400-cmd-int-delay: HS400 command internal delay setting
+				This field has total 32 stages.
+				The value is an integer from 0 to 31.
+- 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.
 
 Examples:
 mmc0: mmc@11230000 {
@@ -38,4 +47,7 @@  mmc0: mmc@11230000 {
 	assigned-clocks = <&topckgen CLK_TOP_MSDC50_0_SEL>;
 	assigned-clock-parents = <&topckgen CLK_TOP_MSDCPLL_D2>;
 	hs400-ds-delay = <0x14015>;
+	mediatek,hs200-cmd-int-delay = <26>;
+	mediatek,hs400-cmd-int-delay = <14>;
+	mediatek,hs400-cmd-resp-sel-rising;
 };