Patchwork DTS: fix the bug and add the chip compatible for eSDHC

login
register
mail settings
Submitter Changming Huang
Date Dec. 23, 2011, 6:10 a.m.
Message ID <1324620659-7135-1-git-send-email-r66093@freescale.com>
Download mbox | patch
Permalink /patch/132968/
State Superseded
Delegated to: Kumar Gala
Headers show

Comments

Changming Huang - Dec. 23, 2011, 6:10 a.m.
From: Jerry Huang <Chang-Ming.Huang@freescale.com>

Accordint to latest kernel, the auto-cmd12 property should be
"sdhci,auto-cmd12", and according to the SDHC binding and the workaround for
the special chip, add the chip compatible for eSDHC: "fsl,p1022-esdhc",
"fsl,mpc8536-esdhc", "fsl,p2020-esdhc" and "fsl,p1010-esdhc".

Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
---
 arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi |    4 ++++
 arch/powerpc/boot/dts/fsl/p1010si-post.dtsi   |    3 ++-
 arch/powerpc/boot/dts/fsl/p1022si-post.dtsi   |    3 ++-
 arch/powerpc/boot/dts/fsl/p2020si-post.dtsi   |    4 ++++
 4 files changed, 12 insertions(+), 2 deletions(-)
Scott Wood - Jan. 2, 2012, 6:27 p.m.
On 12/23/2011 12:10 AM, r66093@freescale.com wrote:
> From: Jerry Huang <Chang-Ming.Huang@freescale.com>
> 
> Accordint to latest kernel, the auto-cmd12 property should be
> "sdhci,auto-cmd12", and according to the SDHC binding and the workaround for
> the special chip, add the chip compatible for eSDHC: "fsl,p1022-esdhc",
> "fsl,mpc8536-esdhc", "fsl,p2020-esdhc" and "fsl,p1010-esdhc".
> 
> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> ---
>  arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi |    4 ++++
>  arch/powerpc/boot/dts/fsl/p1010si-post.dtsi   |    3 ++-
>  arch/powerpc/boot/dts/fsl/p1022si-post.dtsi   |    3 ++-
>  arch/powerpc/boot/dts/fsl/p2020si-post.dtsi   |    4 ++++
>  4 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi b/arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi
> index 89af626..44e0ed9 100644
> --- a/arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi
> +++ b/arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi
> @@ -236,6 +236,10 @@
>  	};
>  
>  /include/ "pq3-esdhc-0.dtsi"
> +	sdhc@2e000 {
> +		compatible = "fsl,esdhc", "fsl,mpc8536-esdhc";
> +	};

More-specific compatible entries should come first.

-Scott
Tabi Timur-B04825 - Jan. 2, 2012, 8:30 p.m.
On Fri, Dec 23, 2011 at 12:10 AM,  <r66093@freescale.com> wrote:
>
> Accordint to latest kernel, the auto-cmd12 property should be
> "sdhci,auto-cmd12", and according to the SDHC binding and the workaround for
> the special chip, add the chip compatible for eSDHC: "fsl,p1022-esdhc",
> "fsl,mpc8536-esdhc", "fsl,p2020-esdhc" and "fsl,p1010-esdhc".

Please do not use the phrase "fix the bug" in patch summaries.
Changming Huang - Jan. 4, 2012, 3:11 a.m.
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Tuesday, January 03, 2012 2:28 AM
> To: Huang Changming-R66093
> Cc: linuxppc-dev@lists.ozlabs.org; Huang Changming-R66093
> Subject: Re: [PATCH] DTS: fix the bug and add the chip compatible for
> eSDHC
> 
> On 12/23/2011 12:10 AM, r66093@freescale.com wrote:
> > From: Jerry Huang <Chang-Ming.Huang@freescale.com>
> >
> > Accordint to latest kernel, the auto-cmd12 property should be
> > "sdhci,auto-cmd12", and according to the SDHC binding and the
> > workaround for the special chip, add the chip compatible for eSDHC:
> > "fsl,p1022-esdhc", "fsl,mpc8536-esdhc", "fsl,p2020-esdhc" and
> "fsl,p1010-esdhc".
> >
> > Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> > ---
> >  arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi |    4 ++++
> >  arch/powerpc/boot/dts/fsl/p1010si-post.dtsi   |    3 ++-
> >  arch/powerpc/boot/dts/fsl/p1022si-post.dtsi   |    3 ++-
> >  arch/powerpc/boot/dts/fsl/p2020si-post.dtsi   |    4 ++++
> >  4 files changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi
> > b/arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi
> > index 89af626..44e0ed9 100644
> > --- a/arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi
> > +++ b/arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi
> > @@ -236,6 +236,10 @@
> >  	};
> >
> >  /include/ "pq3-esdhc-0.dtsi"
> > +	sdhc@2e000 {
> > +		compatible = "fsl,esdhc", "fsl,mpc8536-esdhc";
> > +	};
> 
> More-specific compatible entries should come first.

I don't understand you, why more-specific compatible entries should come?
The Documentation/devicetree/bindings/mmc/fsl-esdhc.txt has introduced it:
  - compatible : should be
    "fsl,<chip>-esdhc", "fsl,esdhc"
I don't think I should introduce new entries.
Changming Huang - Jan. 4, 2012, 3:14 a.m.
> -----Original Message-----
> From: Tabi Timur-B04825
> Sent: Tuesday, January 03, 2012 4:30 AM
> To: Huang Changming-R66093
> Cc: linuxppc-dev@lists.ozlabs.org; Huang Changming-R66093
> Subject: Re: [PATCH] DTS: fix the bug and add the chip compatible for
> eSDHC
> 
> On Fri, Dec 23, 2011 at 12:10 AM,  <r66093@freescale.com> wrote:
> >
> > Accordint to latest kernel, the auto-cmd12 property should be
> > "sdhci,auto-cmd12", and according to the SDHC binding and the
> > workaround for the special chip, add the chip compatible for eSDHC:
> > "fsl,p1022-esdhc", "fsl,mpc8536-esdhc", "fsl,p2020-esdhc" and
> "fsl,p1010-esdhc".
> 
> Please do not use the phrase "fix the bug" in patch summaries.
> 
Then, could you tell me how to describe it?
Scott Wood - Jan. 4, 2012, 5:37 p.m.
On 01/03/2012 09:11 PM, Huang Changming-R66093 wrote:
> 
> 
>> -----Original Message-----
>> From: Wood Scott-B07421
>> Sent: Tuesday, January 03, 2012 2:28 AM
>> To: Huang Changming-R66093
>> Cc: linuxppc-dev@lists.ozlabs.org; Huang Changming-R66093
>> Subject: Re: [PATCH] DTS: fix the bug and add the chip compatible for
>> eSDHC
>>
>> On 12/23/2011 12:10 AM, r66093@freescale.com wrote:
>>> From: Jerry Huang <Chang-Ming.Huang@freescale.com>
>>>
>>> Accordint to latest kernel, the auto-cmd12 property should be
>>> "sdhci,auto-cmd12", and according to the SDHC binding and the
>>> workaround for the special chip, add the chip compatible for eSDHC:
>>> "fsl,p1022-esdhc", "fsl,mpc8536-esdhc", "fsl,p2020-esdhc" and
>> "fsl,p1010-esdhc".
>>>
>>> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
>>> ---
>>>  arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi |    4 ++++
>>>  arch/powerpc/boot/dts/fsl/p1010si-post.dtsi   |    3 ++-
>>>  arch/powerpc/boot/dts/fsl/p1022si-post.dtsi   |    3 ++-
>>>  arch/powerpc/boot/dts/fsl/p2020si-post.dtsi   |    4 ++++
>>>  4 files changed, 12 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi
>>> b/arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi
>>> index 89af626..44e0ed9 100644
>>> --- a/arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi
>>> +++ b/arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi
>>> @@ -236,6 +236,10 @@
>>>  	};
>>>
>>>  /include/ "pq3-esdhc-0.dtsi"
>>> +	sdhc@2e000 {
>>> +		compatible = "fsl,esdhc", "fsl,mpc8536-esdhc";
>>> +	};
>>
>> More-specific compatible entries should come first.
> 
> I don't understand you, why more-specific compatible entries should come?

Because the binding says so, as do ePAPR and the IEEE1275 generic names
recommendation.  It is relied on by some OS driver binding mechanisms to
resolve ambiguity in the event that drivers are present for both strings.

> The Documentation/devicetree/bindings/mmc/fsl-esdhc.txt has introduced it:
>   - compatible : should be
>     "fsl,<chip>-esdhc", "fsl,esdhc"
> I don't think I should introduce new entries.
> 

I'm not asking you to introduce a new entry.  I'm asking you to reverse
them as the above text specifies:

	compatible = "fsl,mpc8536-esdhc", "fsl,esdhc";

not:

	compatible = "fsl,esdhc", "fsl,mpc8536-esdhc";

-Scott
Scott Wood - Jan. 4, 2012, 7:19 p.m.
On 01/03/2012 09:14 PM, Huang Changming-R66093 wrote:
> 
> 
>> -----Original Message-----
>> From: Tabi Timur-B04825
>> Sent: Tuesday, January 03, 2012 4:30 AM
>> To: Huang Changming-R66093
>> Cc: linuxppc-dev@lists.ozlabs.org; Huang Changming-R66093
>> Subject: Re: [PATCH] DTS: fix the bug and add the chip compatible for
>> eSDHC
>>
>> On Fri, Dec 23, 2011 at 12:10 AM,  <r66093@freescale.com> wrote:
>>>
>>> Accordint to latest kernel, the auto-cmd12 property should be
>>> "sdhci,auto-cmd12", and according to the SDHC binding and the
>>> workaround for the special chip, add the chip compatible for eSDHC:
>>> "fsl,p1022-esdhc", "fsl,mpc8536-esdhc", "fsl,p2020-esdhc" and
>> "fsl,p1010-esdhc".
>>
>> Please do not use the phrase "fix the bug" in patch summaries.
>>
> Then, could you tell me how to describe it?

The subject line should be something like:

  powerpc/fsl: esdhc node fixes

Phrases like "Fix the bug" or "fix the issue" could apply to lots of
patches and don't really add anything, especially in the subject line
where there isn't much space and you want to clearly and concisely
identify what this patch is about.

-Scott

Patch

diff --git a/arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi b/arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi
index 89af626..44e0ed9 100644
--- a/arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/mpc8536si-post.dtsi
@@ -236,6 +236,10 @@ 
 	};
 
 /include/ "pq3-esdhc-0.dtsi"
+	sdhc@2e000 {
+		compatible = "fsl,esdhc", "fsl,mpc8536-esdhc";
+	};
+
 /include/ "pq3-sec3.0-0.dtsi"
 /include/ "pq3-mpic.dtsi"
 /include/ "pq3-mpic-timer-B.dtsi"
diff --git a/arch/powerpc/boot/dts/fsl/p1010si-post.dtsi b/arch/powerpc/boot/dts/fsl/p1010si-post.dtsi
index bd9e163..8ebe79c 100644
--- a/arch/powerpc/boot/dts/fsl/p1010si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/p1010si-post.dtsi
@@ -158,7 +158,8 @@ 
 /include/ "pq3-usb2-dr-0.dtsi"
 /include/ "pq3-esdhc-0.dtsi"
 	sdhc@2e000 {
-		fsl,sdhci-auto-cmd12;
+		compatible = "fsl,esdhc", "fsl,p1010-esdhc";
+		sdhci,auto-cmd12;
 	};
 
 /include/ "pq3-sec4.4-0.dtsi"
diff --git a/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi b/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
index 16239b1..9999e56 100644
--- a/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
@@ -203,7 +203,8 @@ 
 
 /include/ "pq3-esdhc-0.dtsi"
 	sdhc@2e000 {
-		fsl,sdhci-auto-cmd12;
+		compatible = "fsl,esdhc", "fsl,p1022-esdhc";
+		sdhci,auto-cmd12;
 	};
 
 /include/ "pq3-sec3.3-0.dtsi"
diff --git a/arch/powerpc/boot/dts/fsl/p2020si-post.dtsi b/arch/powerpc/boot/dts/fsl/p2020si-post.dtsi
index c041050..8ec1b13 100644
--- a/arch/powerpc/boot/dts/fsl/p2020si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/p2020si-post.dtsi
@@ -182,6 +182,10 @@ 
 /include/ "pq3-etsec1-1.dtsi"
 /include/ "pq3-etsec1-2.dtsi"
 /include/ "pq3-esdhc-0.dtsi"
+	sdhc@2e000 {
+		compatible = "fsl,esdhc", "fsl,p2020-esdhc";
+	};
+
 /include/ "pq3-sec3.1-0.dtsi"
 /include/ "pq3-mpic.dtsi"
 /include/ "pq3-mpic-timer-B.dtsi"