From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Subject: Re: [PATCH] DT: mmc: sh_mmcif: fix "compatible" property text References: <201404250239.39150.sergei.shtylyov@cogentembedded.com> <3979266.lQrOKbu0yo@wasted.cogentembedded.com> <55BB44DC.8040706@cogentembedded.com> From: Sergei Shtylyov Message-ID: <55BF86DE.3010001@cogentembedded.com> Date: Mon, 3 Aug 2015 18:21:02 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit To: Magnus Damm Cc: Rob Herring , Pawel Moll , Mark Rutland , "ijc+devicetree@hellion.org.uk" , Kumar Gala , "devicetree@vger.kernel.org" , "ulf.hansson" , linux-mmc@vger.kernel.org, SH-Linux , Simon, Horms, horms@verge.net.au, Geert Uytterhoeven , Laurent Pinchart List-ID: Hello. On 08/03/2015 04:09 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 >>> 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. Everything was done backwards with this binding: first, the driver implemented the device tree probing, then the bindings were (incorrectly) documented by Laurent. > 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. The questions now is why he didn't do exactly this when creating the binding. :-) > 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... I hadn't felt that I need to change the driver before your comment. :-) > Please understand that all the compat strings included in the DT > binding document not necessarily have to be used by the driver. I understand. > But > before merging DT integration code it seems customary that the DT > binding needs to be documented. It was documented plainly incorrect, that's what all the fuzz was about on my side. > Thanks, > / magnus MBR, Sergei