Hi Bjorn,
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.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
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
