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

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

[Sergei Shtylyov]

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 at SoC specific strings
currently at all. Thus describe "renesas,sh-mmcif" string as mandatory and the
others as optional.

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

---
 Documentation/devicetree/bindings/mmc/renesas,mmcif.txt |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: renesas/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
===================================================================
--- renesas.orig/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
+++ renesas/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
@@ -6,11 +6,11 @@ and the properties used by the MMCIF dev
 
 Required properties:
 
-- compatible: must contain one of the following
+- compatible: must contain "renesas,sh-mmcif"; may also contain one of
+  the following:
 	- "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
 
 - clocks: reference to the functional clock
 

Re: [PATCH] DT: mmc: sh_mmcif: fix "compatible" property text

[Sergei Shtylyov]

Hello.

On 07/30/2015 10:59 PM, 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 at SoC specific strings

    "For" not needed here.

> currently at all. Thus describe "renesas,sh-mmcif" string as mandatory and the
> others as optional.

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

    Do I need to repost?

MBR, Sergei

[PATCH] DT: mmc: sh_mmcif: document R8A779[34] support

[Sergei Shtylyov]

Renesas R8A7793 and R8A7793 SoC also have the MMCIF controller...

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
The patch is against Ulf Hansson's 'mmc.git'  repo's 'next' branch plus the
patch I posted earlier today...

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

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
@@ -11,6 +11,8 @@ 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,mmcif-r8a7793" for the MMCIF found in r8a7793 SoCs
+	- "renesas,mmcif-r8a7794" for the MMCIF found in r8a7794 SoCs
 
 - clocks: reference to the functional clock
 

Re: [PATCH] DT: mmc: sh_mmcif: fix "compatible" property text

[Magnus Damm]

Hi Sergei,

On Fri, Jul 31, 2015 at 4:59 AM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> 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 at SoC specific strings
> currently at all. Thus describe "renesas,sh-mmcif" string as mandatory and the
> others as optional.
>
> Fixes: b4c27763d749 ("mmc: sh_mmcif: Document DT bindings")
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Thanks for your efforts trying to improve the DT binding documentation.

> --- renesas.orig/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> +++ renesas/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> @@ -6,11 +6,11 @@ and the properties used by the MMCIF dev
>
>  Required properties:
>
> -- compatible: must contain one of the following
> +- compatible: must contain "renesas,sh-mmcif"; may also contain one of
> +  the following:
>         - "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

As you know, each SoC contains a wide range of on-chip devices and the
MMCIF device is just one of them. Exactly how to manage the DT
bindings must be up to each maintainer and of course this needs to be
aligned with the SoC maintainer and SoC vendor with policies used for
SoC support and BSPs and whatnot. Changing policy like this for a
single device without at least discussing this with the SoC
maintainers does not help.

For Renesas hardware we so far use both SoC part number and optionally
a generic binding as well. As commonly expected, the DT binding is
supposed to describe the hardware and if hardware devices are
compatible. Unless we use SoC part number in the compatible string
there is a risk that the SoC integrator simply copy-and-pastes generic
bindings "because it works" but this will result in DT binding based
on software compatibility and not hardware compatibility. Later when
the driver support is extended this may result in broken software due
to incorrect compatibility information through generic bindings.

If anything is unclear please ask and feel free to discuss this DT
topic with Simon, Laurent, Geert and/or me.

Thanks,

/ magnus

Re: [PATCH] DT: mmc: sh_mmcif: fix "compatible" property text

[Laurent Pinchart]

On Friday 31 July 2015 11:23:04 Magnus Damm wrote:
> On Fri, Jul 31, 2015 at 4:59 AM, 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 at SoC
> > specific strings currently at all. Thus describe "renesas,sh-mmcif"
> > string as mandatory and the others as optional.
> > 
> > Fixes: b4c27763d749 ("mmc: sh_mmcif: Document DT bindings")
> > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> Thanks for your efforts trying to improve the DT binding documentation.
> 
> > --- renesas.orig/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> > +++ renesas/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> > @@ -6,11 +6,11 @@ and the properties used by the MMCIF dev
> > 
> >  Required properties:
> > -- compatible: must contain one of the following
> > +- compatible: must contain "renesas,sh-mmcif"; may also contain one of
> > 
> > +  the following:
> >         - "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
> 
> As you know, each SoC contains a wide range of on-chip devices and the
> MMCIF device is just one of them. Exactly how to manage the DT
> bindings must be up to each maintainer and of course this needs to be
> aligned with the SoC maintainer and SoC vendor with policies used for
> SoC support and BSPs and whatnot. Changing policy like this for a
> single device without at least discussing this with the SoC
> maintainers does not help.
> 
> For Renesas hardware we so far use both SoC part number and optionally
> a generic binding as well. As commonly expected, the DT binding is
> supposed to describe the hardware and if hardware devices are
> compatible. Unless we use SoC part number in the compatible string
> there is a risk that the SoC integrator simply copy-and-pastes generic
> bindings "because it works" but this will result in DT binding based
> on software compatibility and not hardware compatibility. Later when
> the driver support is extended this may result in broken software due
> to incorrect compatibility information through generic bindings.
> 
> If anything is unclear please ask and feel free to discuss this DT
> topic with Simon, Laurent, Geert and/or me.

To clarify this, the current DT compatible strings policy for Renesas SoCs is 
to use a mandatory SoC-based string followed by a optional generic strings. 
Optional here refers to the fact that individual DT bindings can decide 
whether to use a generic string or not, based on hardware information. An IP 
core that has a different, incompatible implementation for each SoC it is 
present in can't make use of a generic compatible string. If a particular 
binding defines generic compatible strings those should be made mandatory by 
that binding.

In the MMCIF case, I would propose wording it as

- compatible: must contain one of the following
      - "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
  followed by "renesas,sh-mmcif".

-- 
Regards,

Laurent Pinchart

Re: [PATCH] DT: mmc: sh_mmcif: fix "compatible" property text

[Magnus Damm]

Hi Sergei,

On Fri, Jul 31, 2015 at 6:50 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Hello.
>
>
> On 7/31/2015 5:23 AM, Magnus Damm 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 at SoC specific
>>> strings
>>> currently at all. Thus describe "renesas,sh-mmcif" string as mandatory
>>> and the
>>> others as optional.
>
>
>>> Fixes: b4c27763d749 ("mmc: sh_mmcif: Document DT bindings")
>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
>
>> Thanks for your efforts trying to improve the DT binding documentation.
>
>
>>> --- renesas.orig/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
>>> +++ renesas/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
>>> @@ -6,11 +6,11 @@ and the properties used by the MMCIF dev
>>>
>>>   Required properties:
>>>
>>> -- compatible: must contain one of the following
>>> +- compatible: must contain "renesas,sh-mmcif"; may also contain one of
>>> +  the following:
>>>          - "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
>
>
>> As you know, each SoC contains a wide range of on-chip devices and the
>> MMCIF device is just one of them. Exactly how to manage the DT
>> bindings must be up to each maintainer and of course this needs to be
>> aligned with the SoC maintainer and SoC vendor with policies used for
>> SoC support and BSPs and whatnot. Changing policy like this for a
>> single device without at least discussing this with the SoC
>> maintainers does not help.
>
>
>    I'm not changing the policy, I'm making the binding actually reflect the
> driver reality (and even the example given in the binding).

Adjusting the binding after driver implementation seems a bit reverse to me.

The way I see it, first you make a DT binding describing the hardware.
Then as second step, you make sure the device driver support the
hardware and the DT binding. As third or second parallel step you
integrate via a DT.

If you are concerned about the order of the compatible strings, unless
specified by the SoC vendor documentation it is in my opinion up to
each driver maintainer and/or developer to judge if the hardware is
compatible or not. So the development activity of determining if the
devices are compatible shall result in a correct compat string order.
I'm not so sure if it is required that this order is supposed to be
documented in the DT binding. Being more clear does not hurt though.

>> For Renesas hardware we so far use both SoC part number and optionally
>> a generic binding as well. As commonly expected, the DT binding is
>> supposed to describe the hardware and if hardware devices are
>> compatible. Unless we use SoC part number in the compatible string
>> there is a risk that the SoC integrator simply copy-and-pastes generic
>> bindings "because it works" but this will result in DT binding based
>> on software compatibility and not hardware compatibility. Later when
>> the driver support is extended this may result in broken software due
>> to incorrect compatibility information through generic bindings.
>
>> If anything is unclear please ask and feel free to discuss this DT
>> topic with Simon, Laurent, Geert and/or me.
>
>    I didn't quite understand what you're proposing instead. Making SoC based
> strings mandatory? Changing the driver to look at the SoC based strings?

I think Laurent describes it pretty well. I'm not sure why you feel
that you need to change the driver though, so I wonder if there is
some misunderstanding going on here...

Please understand that all the compat strings included in the DT
binding document not necessarily have to be used by the driver. But
before merging DT integration code it seems customary that the DT
binding needs to be documented.

Thanks,

/ magnus

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

[Sergei Shtylyov]

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(-)

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
 

[PATCH v2] DT: mmc: sh_mmcif: document R8A779[34] support

[Sergei Shtylyov]

Renesas R8A7794 SoC also has the MMCIF controller...

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
The patch is against Ulf Hansson's 'mmc.git'  repo's 'next' branch plus the
patch I posted earlier today...

Changes in version 2:
- deferred R8A7793 support to the patch posted earlier by Ulrich Hecht;
- fixed typo in the changelog.

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

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,6 +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,mmcif-r8a7794" for the MMCIF found in r8a7794 SoCs
   followed by "renesas,sh-mmcif".
 
 - clocks: reference to the functional clock

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

[Simon Horman]

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
> 

Re: [PATCH v2] DT: mmc: sh_mmcif: document R8A779[34] support

[Simon Horman]

On Wed, Aug 12, 2015 at 01:57:45AM +0300, Sergei Shtylyov wrote:
> Renesas R8A7794 SoC also has the MMCIF controller...
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

This new compat string looks good to me.

Acked-by: Simon Horman <horms+renesas@verge.net.au>

> 
> ---
> The patch is against Ulf Hansson's 'mmc.git'  repo's 'next' branch plus the
> patch I posted earlier today...
> 
> Changes in version 2:
> - deferred R8A7793 support to the patch posted earlier by Ulrich Hecht;

Perhaps it would be best to co-ordinate the R8A7793 and R8A7794
changes to avoid tedious conflicts.

> - fixed typo in the changelog.
> 
>  Documentation/devicetree/bindings/mmc/renesas,mmcif.txt |    1 +
>  1 file changed, 1 insertion(+)
> 
> 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,6 +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,mmcif-r8a7794" for the MMCIF found in r8a7794 SoCs
>    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
> 

Re: [PATCH v2] DT: mmc: sh_mmcif: document R8A779[34] support

[Sergei Shtylyov]

Hello.

On 8/12/2015 3:59 AM, Simon Horman wrote:

>> Renesas R8A7794 SoC also has the MMCIF controller...

>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

> This new compat string looks good to me.

> Acked-by: Simon Horman <horms+renesas@verge.net.au>

    Thank you. I've messed up the subject, so need to re-post.

>> ---
>> The patch is against Ulf Hansson's 'mmc.git'  repo's 'next' branch plus the
>> patch I posted earlier today...
>>
>> Changes in version 2:
>> - deferred R8A7793 support to the patch posted earlier by Ulrich Hecht;

> Perhaps it would be best to co-ordinate the R8A7793 and R8A7794
> changes to avoid tedious conflicts.

    I thought about taking out Ulrich's binding patch and re-posting it along 
with this one but finally decided not to ruin his series. Do you mean that I 
should have posted his patch along with mine?

MBR, Sergei

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

[Sergei Shtylyov]

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

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

[Simon Horman]

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
> 

Re: [PATCH v2] DT: mmc: sh_mmcif: document R8A779[34] support

[Simon Horman]

[CC: Ulrich, Geert]

On Wed, Aug 12, 2015 at 01:38:02PM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 8/12/2015 3:59 AM, Simon Horman wrote:
> 
> >>Renesas R8A7794 SoC also has the MMCIF controller...
> 
> >>Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> >This new compat string looks good to me.
> 
> >Acked-by: Simon Horman <horms+renesas@verge.net.au>
> 
>    Thank you. I've messed up the subject, so need to re-post.
> 
> >>---
> >>The patch is against Ulf Hansson's 'mmc.git'  repo's 'next' branch plus the
> >>patch I posted earlier today...
> >>
> >>Changes in version 2:
> >>- deferred R8A7793 support to the patch posted earlier by Ulrich Hecht;
> 
> >Perhaps it would be best to co-ordinate the R8A7793 and R8A7794
> >changes to avoid tedious conflicts.
> 
>    I thought about taking out Ulrich's binding patch and re-posting it
>    along with this one but finally decided not to ruin his series. Do you
>    mean that I should have posted his patch along with mine?

I mean we sould talk about this to come up with a plan that makes things
easy for the maintainer to pick up the compat strings for the R8A7793 and
R8A7794.

Re: [PATCH v2] DT: mmc: sh_mmcif: document R8A779[34] support

[Ulrich Hecht]

On Wed, Aug 12, 2015 at 12:38 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
>    I thought about taking out Ulrich's binding patch and re-posting it along
> with this one but finally decided not to ruin his series. Do you mean that I
> should have posted his patch along with mine?

I wouldn't mind that; anything that gets stuff picked up is a win.

CU
Uli

[PATCH v3] DT: mmc: sh_mmcif: fix "compatible" property text

[Sergei Shtylyov]

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' or 'next' branches.

Changes in version 3:
- reworded the "compatible" property description to look like the majority of
  the Renesas bindings.

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 |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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
@@ -6,11 +6,11 @@ and the properties used by the MMCIF dev
 
 Required properties:
 
-- compatible: must contain one of the following
+- compatible: should be "renesas,mmcif-<soctype>", "renesas,sh-mmcif" as a
+  fallback. Examples with <soctype> are:
 	- "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
 
 - clocks: reference to the functional clock
 

[PATCH v3] DT: mmc: sh_mmcif: document R8A779[34] support

[Sergei Shtylyov]

Renesas R8A7794 SoC also has the MMCIF controller...

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
The patch is against Ulf Hansson's 'mmc.git' repo's 'next' branches plus the
patch I posted earlier today...

Changes in version 3:
- resolved reject.

Changes in version 2:
- deferred R8A7793 support to the patch posted earlier by Ulrich Hecht;
- fixed typo in the changelog.

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

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
@@ -11,6 +11,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,mmcif-r8a7794" for the MMCIF found in r8a7794 SoCs
 
 - clocks: reference to the functional clock
 

Re: [PATCH v3] DT: mmc: sh_mmcif: fix "compatible" property text

[Geert Uytterhoeven]

On Fri, Oct 16, 2015 at 12:39 AM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> 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>

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3] DT: mmc: sh_mmcif: document R8A779[34] support

[Geert Uytterhoeven]

On Fri, Oct 16, 2015 at 12:40 AM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Renesas R8A7794 SoC also has the MMCIF controller...
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3] DT: mmc: sh_mmcif: fix "compatible" property text

[Ulf Hansson]

On 16 October 2015 at 00:39, Sergei Shtylyov
<sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org> 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-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>

Thanks, applied for next.

Kind regards
Uffe

>
> ---
> The patch is against Ulf Hansson's 'mmc.git' repo's 'fixes' or 'next' branches.
>
> Changes in version 3:
> - reworded the "compatible" property description to look like the majority of
>   the Renesas bindings.
>
> 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 |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> 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
> @@ -6,11 +6,11 @@ and the properties used by the MMCIF dev
>
>  Required properties:
>
> -- compatible: must contain one of the following
> +- compatible: should be "renesas,mmcif-<soctype>", "renesas,sh-mmcif" as a
> +  fallback. Examples with <soctype> are:
>         - "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
>
>  - clocks: reference to the functional clock
>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] DT: mmc: sh_mmcif: document R8A779[34] support

[Ulf Hansson]

On 16 October 2015 at 00:40, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Renesas R8A7794 SoC also has the MMCIF controller...
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Thanks, applied for next.

Kind regards
Uffe


>
> ---
> The patch is against Ulf Hansson's 'mmc.git' repo's 'next' branches plus the
> patch I posted earlier today...
>
> Changes in version 3:
> - resolved reject.
>
> Changes in version 2:
> - deferred R8A7793 support to the patch posted earlier by Ulrich Hecht;
> - fixed typo in the changelog.
>
>  Documentation/devicetree/bindings/mmc/renesas,mmcif.txt |    1 +
>  1 file changed, 1 insertion(+)
>
> 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
> @@ -11,6 +11,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,mmcif-r8a7794" for the MMCIF found in r8a7794 SoCs
>
>  - clocks: reference to the functional clock
>
>

Re: [PATCH v3] DT: mmc: sh_mmcif: document R8A779[34] support

[Sergei Shtylyov]

Hello.

On 10/16/2015 04:07 PM, Ulf Hansson wrote:

>> Renesas R8A7794 SoC also has the MMCIF controller...
>>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

> Thanks, applied for next.

    Oops, just noticed the subject was stale. I wasn't adding R8A7793 support 
in that version. Is it possible to fix?

> Kind regards
> Uffe

[...]

MBR, Sergei

Re: [PATCH v3] DT: mmc: sh_mmcif: document R8A779[34] support

[Ulf Hansson]

On 20 October 2015 at 22:19, Sergei Shtylyov
<sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org> wrote:
> Hello.
>
> On 10/16/2015 04:07 PM, Ulf Hansson wrote:
>
>>> Renesas R8A7794 SoC also has the MMCIF controller...
>>>
>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>
>
>
>> Thanks, applied for next.
>
>
>    Oops, just noticed the subject was stale. I wasn't adding R8A7793 support
> in that version. Is it possible to fix?

I have updated the subject, no worries!

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] DT: mmc: sh_mmcif: document R8A779[34] support

[Sergei Shtylyov]

Hello.

On 10/21/2015 11:38 AM, Ulf Hansson wrote:

>>>> Renesas R8A7794 SoC also has the MMCIF controller...
>>>>
>>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>
>>
>>> Thanks, applied for next.
>>
>>
>>     Oops, just noticed the subject was stale. I wasn't adding R8A7793 support
>> in that version. Is it possible to fix?
>
> I have updated the subject, no worries!

    Perfect! Thanks a lot. :-)

> Kind regards
> Uffe

MBR, Sergei

Re: [PATCH v3] DT: mmc: sh_mmcif: document R8A779[34] support

[Geert Uytterhoeven]

Hi Sergei,

On Tue, Oct 20, 2015 at 10:19 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> On 10/16/2015 04:07 PM, Ulf Hansson wrote:
>>> Renesas R8A7794 SoC also has the MMCIF controller...
>>>
>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
>> Thanks, applied for next.
>
>    Oops, just noticed the subject was stale. I wasn't adding R8A7793 support
> in that version. Is it possible to fix?

Hah, and that made me mark Ulrich's patch for r8a7793 as obsolete in
patchwork :-(

Picked up and sent out.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds