Hi Simon, On Wed, Feb 17, 2016 at 3:28 PM, Simon Horman <ho...@verge.net.au> wrote: > On Wed, Feb 17, 2016 at 11:33:27AM +0900, Magnus Damm wrote: >> Hi Geert, >> >> On Tue, Feb 16, 2016 at 10:11 PM, Geert Uytterhoeven >> <ge...@linux-m68k.org> wrote: >> > On Tue, Feb 16, 2016 at 8:17 AM, Magnus Damm <magnus.d...@gmail.com> wrote: >> >> From: Magnus Damm <damm+rene...@opensource.se> >> >> >> >> Add documentation for new separate CMT0 and CMT1 DT compatible strings >> >> for R-Car Gen2. These compat strings allow us to enable CMT1-specific >> >> features in the driver. The old compat strings will be deprecated in >> >> the not so distant future. >> >> >> >> Signed-off-by: Magnus Damm <damm+rene...@opensource.se> >> >> Acked-by: Geert Uytterhoeven <geert+rene...@glider.be> >> >> Acked-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com> >> >> Acked-by: Rob Herring <r...@kernel.org> >> >> --- >> >> >> >> Changes since V2: >> >> - Added Acked-by from Rob >> >> - Removed Tested-by tag from DT binding patch - duh! >> >> >> >> Changes since V1: >> >> - Added Acked-by and Tested-by from Geert >> >> - Added Acked-by from Laurent >> >> >> >> Documentation/devicetree/bindings/timer/renesas,cmt.txt | 3 +++ >> >> 1 file changed, 3 insertions(+) >> >> >> >> --- 0002/Documentation/devicetree/bindings/timer/renesas,cmt.txt >> >> +++ work/Documentation/devicetree/bindings/timer/renesas,cmt.txt >> >> 2015-09-17 17:26:57.440513000 +0900 >> >> @@ -36,6 +36,9 @@ Required Properties: >> >> (CMT1 on sh73a0 and r8a7740) >> >> This is a fallback for the above renesas,cmt-48-* entries. >> >> >> >> + - "renesas,cmt0-rcar-gen2" for 32-bit CMT0 devices included in R-Car >> >> Gen2. >> >> + - "renesas,cmt1-rcar-gen2" for 48-bit CMT1 devices included in R-Car >> >> Gen2. >> > >> > (advancing a few months always means more comments ;-) >> >> Indeed! >> >> > I'm wondering whether we should use e.g. "renesas,rcar-gen2-cmt0" instead? >> >> I have no strong feelings one way or the other, but I agree that >> aiming to make things more consistent must be good. > > FWIW, I agree with Geert's suggestion. > But I also think it is not particularly important. > >> Your proposal makes the fallback match with what we do for a bunch >> other devices on R-Car Gen2 like: >> "rcar-gen2-cpg-clocks" >> "rcar-gen2-scif" >> But we also seem to have: >> "pci-rcar-gen2" > > Bother, it looks like I got pci backwards :( > >> On R-Car Gen3 we seem to have the following per-SoC compat strings: >> "dmac-r8a7795" >> "etheravb-r8a7795" >> "gpio-r8a7795" >> "scif-r8a7795" > > I believe the above are to follow the existing pattern for > per-SoC compat strings in the same driver, which seems sane. > >> But we also have: >> "r8a7795-cpg-mssr" >> >> My only feeling is that it looks a tad odd if we follow >> "<vendor>,<family-generation>-<device>" for fallback strings but >> "<vendor>,<device>-<part-number>" for the per-soc binding. Why not >> using the same order? Maybe this is specified in some document >> somewhere? > > I believe that the problem is a historical one. Perhaps when > we started adding bindings for our hardware there were no clear > guidelines. But regardless we ended up with a mix as you describe.
Right, that seems to be the way things have happened. > In the mean time guidelines have emerged and we (or at least I) have > agreed with the device tree people (probably Rob) to use the > <vendor>,<chip>-<device> format for new bindings. My reading is that > applies even if it results in fallback and non-fallback bindings for the > same driver have different orders. Some precedence for this can now be found > in renesas,rcar-dmac.txt. Some sort of agreed format sounds good. Your proposal of <vendor>,<chip>-<device> sounds good to me. I'm however slightly more confused by seeing that your example renesas,rcar-dmac.txt included in renesas-drivers-2016-02-16-v4.5-rc4 does not match the proposed format: $ grep "renesas," Documentation/devicetree/bindings/dma/renesas,rcar-dmac.txt - compatible: "renesas,dmac-<soctype>", "renesas,rcar-dmac" as fallback. - "renesas,dmac-r8a7790" (R-Car H2) - "renesas,dmac-r8a7791" (R-Car M2-W) - "renesas,dmac-r8a7792" (R-Car V2H) - "renesas,dmac-r8a7793" (R-Car M2-N) - "renesas,dmac-r8a7794" (R-Car E2) - "renesas,dmac-r8a7795" (R-Car H3) compatible = "renesas,dmac-r8a7790", "renesas,rcar-dmac"; compatible = "renesas,dmac-r8a7790", "renesas,rcar-dmac"; $ > I don't, however, think it applies where we add more soc-specific to a > driver that already has such bindings. Or new fallback bindings to a driver > that already has such bindings. Right, but for rcar-dmac.c there is no matching on per-SoC strings in the driver so I guess we can pick anything we want? $ grep "renesas," drivers/dma/sh/rcar-dmac.c { .compatible = "renesas,rcar-dmac", }, $ >> I guess your take with "r8a7795-cpg-mssr" above is to follow the same >> order as for the fallback case? This seems sane to me, and if so then >> perhaps the per-soc compat strings for the CMT should also be updated? >> Same for other devices too then, like the recently added >> "dmac-r8a7795"? > > From my point of view it would be nice to clean things up and > re-order all the bindings. But I think the drivers would > need to maintain compatibility with the old strings. And I wonder > if it is really worth the effort. No need to rework existing stuff IMO. However once we rework DT bindings (CMT) or add new ones (SYS-DMAC) then we have a good opportunity to clean things up. Thanks, / magnus