On Fri, Feb 05, 2016 at 08:43:55AM +0100, Geert Uytterhoeven wrote: > On Thu, Feb 4, 2016 at 11:01 PM, Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote: > > On Fri, Dec 11, 2015 at 04:58:06PM +0100, Geert Uytterhoeven wrote: > >> On Fri, Dec 11, 2015 at 3:59 PM, Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote: > >> > On Fri, Dec 11, 2015 at 11:14:34AM +0900, Simon Horman wrote: > >> >> On Wed, Dec 09, 2015 at 12:37:43PM -0600, Bjorn Helgaas wrote: > >> >> > On Thu, Dec 03, 2015 at 07:51:36AM +0900, Simon Horman wrote: > >> >> > > this short series adds generic gen2 and SoC-specific r8a7793 compatibility > >> >> > > strings to the rcar PCI and rcar-gen2 PCIE drivers. The intention is to > >> >> > > provide a complete set of compatibility strings for known Gen2 SoCs. > >> >> > > > >> >> > > Key Changes in v2: > >> >> > > * Include "rcar-" in generic bindings > >> >> > > > >> >> > > Simon Horman (4): > >> >> > > PCI: rcar-gen2: add gen2 fallback compatibility string > >> >> > > PCI: rcar-gen2: add device tree support for r8a7793 > >> >> > > PCI: rcar: add gen2 fallback compatibility string > >> >> > > PCI: rcar: add device tree support for r8a7793 > >> >> > > > >> >> > > Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt | 12 ++++++++++-- > >> >> > > Documentation/devicetree/bindings/pci/rcar-pci.txt | 14 +++++++++++--- > >> >> > > drivers/pci/host/pci-rcar-gen2.c | 1 + > >> >> > > drivers/pci/host/pcie-rcar.c | 1 + > >> >> > > 4 files changed, 23 insertions(+), 5 deletions(-) > >> >> > > >> >> > I applied these: > >> >> > > >> >> > > PCI: rcar-gen2: add gen2 fallback compatibility string > >> >> > > PCI: rcar: add gen2 fallback compatibility string > >> >> > > >> >> > to pci/host-rcar for v4.5, thanks! > >> >> > > >> >> > I haven't applied the R8A7793 binding documentation updates yet, but > >> >> > I'll be happy to do so given a short description of why they're > >> >> > useful (since they don't update a DT or the driver). Or they could be > >> >> > merged via a DT tree. > >> >> > >> >> To clarify: you would like a description in the changelog? > >> > > >> > Yes, please. The email discussion so far hasn't contained what I'm > >> > looking for (if it had, I would have just inserted it and been done > >> > with it). > >> > > >> > Apparently it has to do with the stable DT rules, which I don't know. > >> > A concrete example would probably help clear it up. > >> > >> The stable DT rules mean that an old DTS should keep on working with > >> newer kernels. > >> > >> Suppose we have two SoCs, that both contain "foo" modules, which look > >> identical. Hence the DTS for both declares the devices to be compatible > >> with "vendor,foo". > >> > >> Later, we discover a difference between the two "foo" modules in the two > >> SoCs (e.g. a feature present in one of them, or worse, a bug), which we > >> need to handle in the driver. But how can we distinguish between them? > >> We can change the compatible value in the DTS, but that means the user > >> has to update the DTS when updating the kernel. For a new feature that > >> may be deemed acceptable, for a bug fix that's not acceptable. > >> > >> Hence we always use an SoC-specific compatible value, which may or may > >> not be accompanied by a family-specific and/or generic compatible value. > >> As long as everything can be handled the same, the driver will just match > >> against the most generic compatible value used. But if needed later, the > >> driver can be updated to match against a more specific compatible value, > >> and can have special handling for a module in a specific SoC. > >> > >> So that's why we want to have compatible value in the DT bindings that > >> are not necessarily used by the driver. > >> > >> In a perfect world, where all hardware is properly documented, or even > >> Open Source, we wouldn't need this. There we could just declare a device > >> compatible with what it really is, based on the module's internal version ID > >> (ideally a git commit ID of its HDL source ;-). > >> > >> I hope the above explains it. If you have more questions, feel free to ask! > > > > The above is all pretty standard device identification technique. > > That's not what I've been missing. > > > > But I just had an epiphany. Tell me if I'm talking sense or > > gibberish: > > > > If a patch adds a use of "renesas,pcie-r8a7793" in a .dts, .c, or .h > > file, e.g., in arch/arm/boot/dts/ or in the driver, and somebody > > runs checkpatch on that patch, checkpatch will complain unless > > "renesas,pcie-r8a7793" appears somewhere in > > devicetree/bindings/pci/. > > > > So if I understand correctly, the only point of this patch is to shut > > up checkpatch in that situation. It doesn't seem terribly useful to > > me because checkpatch is only relevant to Linux kernel patches, and > > even there it's optional and advisory. > > > > The latest changelog says "By documenting this compat string it may be > > used in DTSs shipped, for example as part of ROMs", which seems to be > > saying a DTS may not be shipped unless checkpatch approves of it. > > That's a social and procedural question, not a coding question. > > > > It's fine with me if you want to try to enforce that via checkpatch, > > but I'd just like to be clear on the mechanism. > > A DT binding (e.g. a compatible value) may only be used in a DTS after it's > been documented and approved in a DT binding document. > > Checkpath is just a tool. AFAIK there's no other tool enforcing the above. > > As both DTSes and DT binding documents are (still) in the kernel, and changes > to them go in the kernel through patches, and most people run checkpatch > on patches, I think having the check in checkpatch makes perfect sense. > Adding and using yet another tool would complicate matters. Like I said, it's fine with me if your DTS process relies on checkpatch, and I'm not suggesting any other tool. I'm just trying to get clarity on the nitty-gritty technical benefits of this patch, and I think it is "this patch enables people to add a DTS containing 'renesas,pcie-r8a7793' to the kernel source without having checkpatch complain." If the convention is that OEMs can't ship a DT unless it's in the kernel source and has passed checkpatch, that's fine, but it can't be enforced by a kernel patch. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
