Message ID | 1643858.nkRxsQo3O3@wasted.cogentembedded.com (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Geert Uytterhoeven |
Headers | show |
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-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html