| Message ID | 1448954672-27236-3-git-send-email-horms+renesas@verge.net.au |
|---|---|
| State | Superseded, archived |
| Headers | show |
Hi Simon, On Tue, Dec 01, 2015 at 04:24:30PM +0900, Simon Horman wrote: > Simply document new compat strings. > There appears to be no need for a driver updates. > > Signed-off-by: Simon Horman <horms+renesas@verge.net.au> > --- > Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt b/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt > index b19be08a8113..7c231b3e5872 100644 > --- a/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt > +++ b/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt > @@ -8,6 +8,7 @@ OHCI and EHCI controllers. > Required properties: > - compatible: "renesas,pci-r8a7790" for the R8A7790 SoC; > "renesas,pci-r8a7791" for the R8A7791 SoC; > + "renesas,pci-r8a7793" for the R8A7793 SoC; What's the benefit of adding a string here if the driver doesn't check for it? Since the driver doesn't look for it, there's no way to test anything. It doesn't seem like this file is an authoritative source of names, so if we add it here, there's the possibility the r8a7793 will be canceled or renamed, and then we'd have to update this if the driver ever did need an r8a7793-specific quirk. If we waited until the driver actually needs a quirk, then we'd know exactly what name to look for and we could update the driver, DT, and doc all together. > "renesas,pci-r8a7794" for the R8A7794 SoC; > "renesas,pci-gen2" for a generic R-Car Gen2 compatible device. > > -- > 2.1.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Bjorn, On Fri, Dec 4, 2015 at 10:26 PM, Bjorn Helgaas <helgaas@kernel.org> wrote: > On Tue, Dec 01, 2015 at 04:24:30PM +0900, Simon Horman wrote: >> Simply document new compat strings. >> There appears to be no need for a driver updates. >> >> Signed-off-by: Simon Horman <horms+renesas@verge.net.au> >> --- >> Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt b/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt >> index b19be08a8113..7c231b3e5872 100644 >> --- a/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt >> +++ b/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt >> @@ -8,6 +8,7 @@ OHCI and EHCI controllers. >> Required properties: >> - compatible: "renesas,pci-r8a7790" for the R8A7790 SoC; >> "renesas,pci-r8a7791" for the R8A7791 SoC; >> + "renesas,pci-r8a7793" for the R8A7793 SoC; > > What's the benefit of adding a string here if the driver doesn't check > for it? Since the driver doesn't look for it, there's no way to test > anything. If we ever discover a difference between PCI on r8a7793 compared to PCI on other SoCs of the R-Car Gen2 family, we can handle that difference in the driver. > It doesn't seem like this file is an authoritative source of names, so Yes it is. When adding compatible values to a DTS, checkpatch.pl will check for their existence in the binding documentation. Hence we always add the compatible values to the DT binding docs, before we start using them. > if we add it here, there's the possibility the r8a7793 will be > canceled or renamed, and then we'd have to update this if the driver > ever did need an r8a7793-specific quirk. If we waited until the I don't understand what you mean here. > driver actually needs a quirk, then we'd know exactly what name to > look for and we could update the driver, DT, and doc all together. If we update driver, DT, and doc only after we discover the need for a quirk, it's already too late, due to stable DT rules. Hence we always document and use SoC-specific compatible values, sometimes combined with family-specific compatible values. The driver only matches to the as least specific value as possible. I hope this clears up your worries. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org 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 devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Dec 05, 2015 at 11:38:37AM +0100, Geert Uytterhoeven wrote: > Hi Bjorn, > > On Fri, Dec 4, 2015 at 10:26 PM, Bjorn Helgaas <helgaas@kernel.org> wrote: > > On Tue, Dec 01, 2015 at 04:24:30PM +0900, Simon Horman wrote: > >> Simply document new compat strings. > >> There appears to be no need for a driver updates. > >> > >> Signed-off-by: Simon Horman <horms+renesas@verge.net.au> > >> --- > >> Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt b/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt > >> index b19be08a8113..7c231b3e5872 100644 > >> --- a/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt > >> +++ b/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt > >> @@ -8,6 +8,7 @@ OHCI and EHCI controllers. > >> Required properties: > >> - compatible: "renesas,pci-r8a7790" for the R8A7790 SoC; > >> "renesas,pci-r8a7791" for the R8A7791 SoC; > >> + "renesas,pci-r8a7793" for the R8A7793 SoC; > > > > What's the benefit of adding a string here if the driver doesn't check > > for it? Since the driver doesn't look for it, there's no way to test > > anything. > > If we ever discover a difference between PCI on r8a7793 compared to PCI on > other SoCs of the R-Car Gen2 family, we can handle that difference in the > driver. > > > It doesn't seem like this file is an authoritative source of names, so > > Yes it is. When adding compatible values to a DTS, checkpatch.pl will check > for their existence in the binding documentation. Hence we always add the > compatible values to the DT binding docs, before we start using them. > > > if we add it here, there's the possibility the r8a7793 will be > > canceled or renamed, and then we'd have to update this if the driver > > ever did need an r8a7793-specific quirk. If we waited until the > > I don't understand what you mean here. > > > driver actually needs a quirk, then we'd know exactly what name to > > look for and we could update the driver, DT, and doc all together. > > If we update driver, DT, and doc only after we discover the need for a quirk, > it's already too late, due to stable DT rules. Hence we always document and > use SoC-specific compatible values, sometimes combined with family-specific > compatible values. The driver only matches to the as least specific value as > possible. The stable DT rules seem to be the key here, but I don't know what they are. I found Documentation/devicetree/bindings/ABI.txt, but it doesn't clear it up for me. I was assuming vendors could ship a DT in a platform ROM, in the same way platforms ship with an ACPI system description. For ACPI, the _HID/_CID in the ROM in the field is authoritative in the sense that we have to regard it as a fixed, unchangeable feature of the platform. If we want a driver to recognize that device, we have to build the ID from the ROM into the driver, and it doesn't matter what is documented in the spec or in the kernel source. If we had a one-sentence description of why adding the doc update when we actually need it is too late, that would probably be enough. I'm perfectly happy with the PCI changes, so if somebody else wants to just merge all these via a DT tree, I'm happy to ack the PCI ones. Bjorn -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Dec 07, 2015 at 09:38:33AM -0600, Bjorn Helgaas wrote: > On Sat, Dec 05, 2015 at 11:38:37AM +0100, Geert Uytterhoeven wrote: > > Hi Bjorn, > > > > On Fri, Dec 4, 2015 at 10:26 PM, Bjorn Helgaas <helgaas@kernel.org> wrote: > > > On Tue, Dec 01, 2015 at 04:24:30PM +0900, Simon Horman wrote: > > >> Simply document new compat strings. > > >> There appears to be no need for a driver updates. > > >> > > >> Signed-off-by: Simon Horman <horms+renesas@verge.net.au> > > >> --- > > >> Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt | 1 + > > >> 1 file changed, 1 insertion(+) > > >> > > >> diff --git a/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt b/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt > > >> index b19be08a8113..7c231b3e5872 100644 > > >> --- a/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt > > >> +++ b/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt > > >> @@ -8,6 +8,7 @@ OHCI and EHCI controllers. > > >> Required properties: > > >> - compatible: "renesas,pci-r8a7790" for the R8A7790 SoC; > > >> "renesas,pci-r8a7791" for the R8A7791 SoC; > > >> + "renesas,pci-r8a7793" for the R8A7793 SoC; > > > > > > What's the benefit of adding a string here if the driver doesn't check > > > for it? Since the driver doesn't look for it, there's no way to test > > > anything. > > > > If we ever discover a difference between PCI on r8a7793 compared to PCI on > > other SoCs of the R-Car Gen2 family, we can handle that difference in the > > driver. > > > > > It doesn't seem like this file is an authoritative source of names, so > > > > Yes it is. When adding compatible values to a DTS, checkpatch.pl will check > > for their existence in the binding documentation. Hence we always add the > > compatible values to the DT binding docs, before we start using them. > > > > > if we add it here, there's the possibility the r8a7793 will be > > > canceled or renamed, and then we'd have to update this if the driver > > > ever did need an r8a7793-specific quirk. If we waited until the > > > > I don't understand what you mean here. > > > > > driver actually needs a quirk, then we'd know exactly what name to > > > look for and we could update the driver, DT, and doc all together. > > > > If we update driver, DT, and doc only after we discover the need for a quirk, > > it's already too late, due to stable DT rules. Hence we always document and > > use SoC-specific compatible values, sometimes combined with family-specific > > compatible values. The driver only matches to the as least specific value as > > possible. > > The stable DT rules seem to be the key here, but I don't know what > they are. I found Documentation/devicetree/bindings/ABI.txt, but it > doesn't clear it up for me. > > I was assuming vendors could ship a DT in a platform ROM, in the same > way platforms ship with an ACPI system description. For ACPI, the > _HID/_CID in the ROM in the field is authoritative in the sense that > we have to regard it as a fixed, unchangeable feature of the platform. > If we want a driver to recognize that device, we have to build the ID > from the ROM into the driver, and it doesn't matter what is documented > in the spec or in the kernel source. > > If we had a one-sentence description of why adding the doc update when > we actually need it is too late, that would probably be enough. I agree that would be useful. > I'm perfectly happy with the PCI changes, so if somebody else wants to > just merge all these via a DT tree, I'm happy to ack the PCI ones. I have no fixed ideas about who should take these changes but I think you could take them through the PCI tree if you are so inclined. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Dec 08, 2015 at 09:45:19AM +0900, Simon Horman wrote: > On Mon, Dec 07, 2015 at 09:38:33AM -0600, Bjorn Helgaas wrote: > > On Sat, Dec 05, 2015 at 11:38:37AM +0100, Geert Uytterhoeven wrote: > > > Hi Bjorn, > > > > > > On Fri, Dec 4, 2015 at 10:26 PM, Bjorn Helgaas <helgaas@kernel.org> wrote: > > > > On Tue, Dec 01, 2015 at 04:24:30PM +0900, Simon Horman wrote: > > > >> Simply document new compat strings. > > > >> There appears to be no need for a driver updates. > > > >> > > > >> Signed-off-by: Simon Horman <horms+renesas@verge.net.au> > > > >> --- > > > >> Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt | 1 + > > > >> 1 file changed, 1 insertion(+) > > > >> > > > >> diff --git a/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt b/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt > > > >> index b19be08a8113..7c231b3e5872 100644 > > > >> --- a/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt > > > >> +++ b/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt > > > >> @@ -8,6 +8,7 @@ OHCI and EHCI controllers. > > > >> Required properties: > > > >> - compatible: "renesas,pci-r8a7790" for the R8A7790 SoC; > > > >> "renesas,pci-r8a7791" for the R8A7791 SoC; > > > >> + "renesas,pci-r8a7793" for the R8A7793 SoC; > > > > > > > > What's the benefit of adding a string here if the driver doesn't check > > > > for it? Since the driver doesn't look for it, there's no way to test > > > > anything. > > > > > > If we ever discover a difference between PCI on r8a7793 compared to PCI on > > > other SoCs of the R-Car Gen2 family, we can handle that difference in the > > > driver. > > > > > > > It doesn't seem like this file is an authoritative source of names, so > > > > > > Yes it is. When adding compatible values to a DTS, checkpatch.pl will check > > > for their existence in the binding documentation. Hence we always add the > > > compatible values to the DT binding docs, before we start using them. > > > > > > > if we add it here, there's the possibility the r8a7793 will be > > > > canceled or renamed, and then we'd have to update this if the driver > > > > ever did need an r8a7793-specific quirk. If we waited until the > > > > > > I don't understand what you mean here. > > > > > > > driver actually needs a quirk, then we'd know exactly what name to > > > > look for and we could update the driver, DT, and doc all together. > > > > > > If we update driver, DT, and doc only after we discover the need for a quirk, > > > it's already too late, due to stable DT rules. Hence we always document and > > > use SoC-specific compatible values, sometimes combined with family-specific > > > compatible values. The driver only matches to the as least specific value as > > > possible. > > > > The stable DT rules seem to be the key here, but I don't know what > > they are. I found Documentation/devicetree/bindings/ABI.txt, but it > > doesn't clear it up for me. > > > > I was assuming vendors could ship a DT in a platform ROM, in the same > > way platforms ship with an ACPI system description. For ACPI, the > > _HID/_CID in the ROM in the field is authoritative in the sense that > > we have to regard it as a fixed, unchangeable feature of the platform. > > If we want a driver to recognize that device, we have to build the ID > > from the ROM into the driver, and it doesn't matter what is documented > > in the spec or in the kernel source. > > > > If we had a one-sentence description of why adding the doc update when > > we actually need it is too late, that would probably be enough. > > I agree that would be useful. Sorry for letting this slip. How about the following in the relevant changelogs: By documenting this compat sting it may be used in DTSs shipped, for example as part of ROMs. It must be used in conjunction with the Gen2 fallback compat string. At this time there are no known differences between the r8a7793 IP block and that implemented by the driver for the Gen2 fallback compat string. Thus there is no need to update the driver as the use of the Gen2 fallback compat string will activate the correct code in the current driver while leaving the option for r8a7793-specific driver code to be activated in an updated driver should the need arise. > > I'm perfectly happy with the PCI changes, so if somebody else wants to > > just merge all these via a DT tree, I'm happy to ack the PCI ones. > > I have no fixed ideas about who should take these changes > but I think you could take them through the PCI tree if you > are so inclined. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt b/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt index b19be08a8113..7c231b3e5872 100644 --- a/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt +++ b/Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt @@ -8,6 +8,7 @@ OHCI and EHCI controllers. Required properties: - compatible: "renesas,pci-r8a7790" for the R8A7790 SoC; "renesas,pci-r8a7791" for the R8A7791 SoC; + "renesas,pci-r8a7793" for the R8A7793 SoC; "renesas,pci-r8a7794" for the R8A7794 SoC; "renesas,pci-gen2" for a generic R-Car Gen2 compatible device.
Simply document new compat strings. There appears to be no need for a driver updates. Signed-off-by: Simon Horman <horms+renesas@verge.net.au> --- Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt | 1 + 1 file changed, 1 insertion(+)