On Wed, Feb 17, 2016 at 03:45:19PM +0900, Magnus Damm wrote: > 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"; > $
It looks like I have been making the mess worse :( Possibly I prepared the patch in question, though recently, before I was properly aware of the preferred order. > > 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", }, > $ Pretty much. > >> 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. Understood. From my point of view that seems like an opportunity worth taking.