diff mbox

[v2] DT: mmc: sh_mmcif: fix "compatible" property text

Message ID 1643858.nkRxsQo3O3@wasted.cogentembedded.com (mailing list archive)
State New, archived
Headers show

Commit Message

Sergei Shtylyov Aug. 11, 2015, 10:37 p.m. UTC
The "compatible" property text contradicts even the example given in the MMCIF
binding document itself;  moreover, the Renesas MMCIF driver only matches on
the generic "compatible" string and doesn't look for the SoC specific strings
at all. Thus describe "renesas,sh-mmcif" as a fallback value.

Fixes: b4c27763d749 ("mmc: sh_mmcif: Document DT bindings")
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
The patch is against Ulf Hansson's 'mmc.git' repo's 'fixes' and 'next' branches.

Changes in version 2:
- kept the SoC specific "compatible" property values mandatory and made the
  generic string a fallback.

 Documentation/devicetree/bindings/mmc/renesas,mmcif.txt |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Simon Horman Aug. 12, 2015, 12:56 a.m. UTC | #1
Hi Sergei,

On Wed, Aug 12, 2015 at 01:37:28AM +0300, Sergei Shtylyov wrote:
> The "compatible" property text contradicts even the example given in the MMCIF
> binding document itself;  moreover, the Renesas MMCIF driver only matches on
> the generic "compatible" string and doesn't look for the SoC specific strings
> at all. Thus describe "renesas,sh-mmcif" as a fallback value.
> 
> Fixes: b4c27763d749 ("mmc: sh_mmcif: Document DT bindings")
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

I don't believe this file is the appropriate place to describe
best-practice for the ordering of compatible strings which must surely be
documented elsewhere.

> ---
> The patch is against Ulf Hansson's 'mmc.git' repo's 'fixes' and 'next' branches.
> 
> Changes in version 2:
> - kept the SoC specific "compatible" property values mandatory and made the
>   generic string a fallback.
> 
>  Documentation/devicetree/bindings/mmc/renesas,mmcif.txt |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: mmc/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> ===================================================================
> --- mmc.orig/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> +++ mmc/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> @@ -10,7 +10,7 @@ Required properties:
>  	- "renesas,mmcif-r8a7740" for the MMCIF found in r8a7740 SoCs
>  	- "renesas,mmcif-r8a7790" for the MMCIF found in r8a7790 SoCs
>  	- "renesas,mmcif-r8a7791" for the MMCIF found in r8a7791 SoCs
> -	- "renesas,sh-mmcif" for the generic MMCIF
> +  followed by "renesas,sh-mmcif".
>  
>  - clocks: reference to the functional clock
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergei Shtylyov Aug. 12, 2015, 10:41 a.m. UTC | #2
On 8/12/2015 3:56 AM, Simon Horman wrote:

>> The "compatible" property text contradicts even the example given in the MMCIF
>> binding document itself;  moreover, the Renesas MMCIF driver only matches on
>> the generic "compatible" string and doesn't look for the SoC specific strings
>> at all. Thus describe "renesas,sh-mmcif" as a fallback value.

>> Fixes: b4c27763d749 ("mmc: sh_mmcif: Document DT bindings")
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

> I don't believe this file is the appropriate place to describe
> best-practice for the ordering of compatible strings which must surely be
> documented elsewhere.

    Where? I have no idea what you mean...
    And let me reiterate: this text is *wrong* and needs to be fixed anyway.

>> ---
>> The patch is against Ulf Hansson's 'mmc.git' repo's 'fixes' and 'next' branches.
>>
>> Changes in version 2:
>> - kept the SoC specific "compatible" property values mandatory and made the
>>    generic string a fallback.
>>
>>   Documentation/devicetree/bindings/mmc/renesas,mmcif.txt |    2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> Index: mmc/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
>> ===================================================================
>> --- mmc.orig/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
>> +++ mmc/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
>> @@ -10,7 +10,7 @@ Required properties:
>>   	- "renesas,mmcif-r8a7740" for the MMCIF found in r8a7740 SoCs
>>   	- "renesas,mmcif-r8a7790" for the MMCIF found in r8a7790 SoCs
>>   	- "renesas,mmcif-r8a7791" for the MMCIF found in r8a7791 SoCs
>> -	- "renesas,sh-mmcif" for the generic MMCIF
>> +  followed by "renesas,sh-mmcif".
 >>
>>   - clocks: reference to the functional clock

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Simon Horman Aug. 12, 2015, 11:58 p.m. UTC | #3
On Wed, Aug 12, 2015 at 01:41:40PM +0300, Sergei Shtylyov wrote:
> On 8/12/2015 3:56 AM, Simon Horman wrote:
> 
> >>The "compatible" property text contradicts even the example given in the MMCIF
> >>binding document itself;  moreover, the Renesas MMCIF driver only matches on
> >>the generic "compatible" string and doesn't look for the SoC specific strings
> >>at all. Thus describe "renesas,sh-mmcif" as a fallback value.
> 
> >>Fixes: b4c27763d749 ("mmc: sh_mmcif: Document DT bindings")
> >>Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> >I don't believe this file is the appropriate place to describe
> >best-practice for the ordering of compatible strings which must surely be
> >documented elsewhere.
> 
>    Where? I have no idea what you mean...
>    And let me reiterate: this text is *wrong* and needs to be fixed anyway.

The document lists the acceptable compat strings for the driver.
Some are currently implemented by the driver. Some are not.
This provides a mechanism for enhancing the driver while maintaining
compatibility with existing DT blobs.

The document does not describe the way to order the compat strings, which I
believe is a more generic issue as many drivers have more and less specific
compat strings to describe hardware which is compatible with each other. To
some extent I believe it is up to the user, that is the person writing DT
files, to understand what the hardware they are dealing with is compatible
with. And to some extent I believe the ordering is a best-practice that
ought to be described in a high-level document if it is not already.

Your proposed update assumes that all past and future hardware handled by
current and future versions of the driver will be compatible with
"renesas,sh-mmcif". How can you possibly know that is true?

> >>---
> >>The patch is against Ulf Hansson's 'mmc.git' repo's 'fixes' and 'next' branches.
> >>
> >>Changes in version 2:
> >>- kept the SoC specific "compatible" property values mandatory and made the
> >>   generic string a fallback.
> >>
> >>  Documentation/devicetree/bindings/mmc/renesas,mmcif.txt |    2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >>Index: mmc/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> >>===================================================================
> >>--- mmc.orig/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> >>+++ mmc/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> >>@@ -10,7 +10,7 @@ Required properties:
> >>  	- "renesas,mmcif-r8a7740" for the MMCIF found in r8a7740 SoCs
> >>  	- "renesas,mmcif-r8a7790" for the MMCIF found in r8a7790 SoCs
> >>  	- "renesas,mmcif-r8a7791" for the MMCIF found in r8a7791 SoCs
> >>-	- "renesas,sh-mmcif" for the generic MMCIF
> >>+  followed by "renesas,sh-mmcif".
> >>
> >>  - clocks: reference to the functional clock
> 
> MBR, Sergei
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

Index: mmc/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
===================================================================
--- mmc.orig/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
+++ mmc/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
@@ -10,7 +10,7 @@  Required properties:
 	- "renesas,mmcif-r8a7740" for the MMCIF found in r8a7740 SoCs
 	- "renesas,mmcif-r8a7790" for the MMCIF found in r8a7790 SoCs
 	- "renesas,mmcif-r8a7791" for the MMCIF found in r8a7791 SoCs
-	- "renesas,sh-mmcif" for the generic MMCIF
+  followed by "renesas,sh-mmcif".
 
 - clocks: reference to the functional clock