Messages in this thread | | | Date | Thu, 19 Mar 2015 16:27:15 -0600 | From | Stephen Warren <> | Subject | Re: [PATCH 3/3] Documentation: DT bindings: Tegra AHB: note base address change |
| |
I guess pretend like I never made the suggestion.
On 03/19/2015 12:42 PM, Paul Walmsley wrote: > On Thu, 19 Mar 2015, Stephen Warren wrote: > >> On 03/19/2015 10:34 AM, Paul Walmsley wrote: >>> On Thu, 19 Mar 2015, Stephen Warren wrote: >>> >>>> On 03/19/2015 09:33 AM, Paul Walmsley wrote: >>>>> On Tue, 17 Mar 2015, Stephen Warren wrote: >>>>> >>>>>> On 03/17/2015 02:32 AM, Paul Walmsley wrote: >>>>>>> For Tegra132 and later chips, we can now use the correct hardware >>>>>>> base >>>>>>> address for the Tegra AHB IP block in the DT data. Update the DT >>>>>>> binding >>>>>>> documentation to reflect this change. >>>>>> >>>>>>> diff --git >>>>>>> a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-ahb.txt >>>>>>> b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-ahb.txt >>>>>>> index 067c979..7692b4c 100644 >>>>>>> --- >>>>>>> a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-ahb.txt >>>>>>> +++ >>>>>>> b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-ahb.txt >>>>>>> @@ -2,10 +2,15 @@ NVIDIA Tegra AHB >>>>>>> >>>>>>> Required properties: >>>>>>> - compatible : For Tegra20, must contain "nvidia,tegra20-ahb". >>>>>>> For >>>>>>> - Tegra30, must contain "nvidia,tegra30-ahb". Otherwise, must >>>>>>> contain >>>>>>> - '"nvidia,<chip>-ahb", "nvidia,tegra30-ahb"' where <chip> is >>>>>>> tegra124, >>>>>>> - tegra132, or tegra210. >>>>>>> -- reg : Should contain 1 register ranges(address and length) >>>>>>> + Tegra30, must contain "nvidia,tegra30-ahb". For Tegra114 and >>>>>>> Tegra124, >>>>>>> must >>>>>>> + contain '"nvidia,<chip>-ahb", "nvidia,tegra30-ahb"' where <chip> >>>>>>> is >>>>>>> tegra114 >>>>>>> + or tegra124. For Tegra132, the compatible string must contain >>>>>>> + "nvidia,tegra132-ahb". >>>>>>> + >>>>>>> +- reg : Should contain 1 register ranges(address and length). On >>>>>>> Tegra20, >>>>>>> + Tegra30, Tegra114, and Tegra124 chips, the low byte of the >>>>>>> physical >>>>>>> base >>>>>>> + address of the IP block must end in 0x04. On DT files for later >>>>>>> chips, >>>>>>> the >>>>>>> + actual hardware base address of the IP block should be used. >>>>>> >>>>>> A table-based approach rather than prose might make this more legible? >>>>>> >>>>>> - compatible: Must contain the following: >>>>>> Tegra20: "nvidia,tegra20-ahb" >>>>>> Tegra30: "nvidia,tegra30-ahb" >>>>>> Tegra114: "nvidia,tegra114-ahb", "nvidia,tegra30-ahb" >>>>>> Tegra124: "nvidia,tegra124-ahb", "nvidia,tegra30-ahb" >>>>>> Tegra132: "nvidia,tegra132-ahb" >>>>>> Tegra210: "nvidia,tegra210-ahb", "nvidia,tegra132-ahb" >>>>>> >>>>>> With any luck, we can extend that final item for future chips to be: >>>>>> >>>>>> Tegra210, TegraNNN: >>>>>> "nvidia,tegra<chip>-ahb", "nvidia,tegra132-ahb" >>>>>> >>>>>> Perhaps we format the 114/124 entry that way too. >>>>> >>>>> I think I'm just going to drop this patch, since Russell prefers that >>>>> the >>>>> workaround is applied in the driver. >>>>> >>>>> With regards to using tables rather than narrative descriptions: perhaps >>>>> consider a patch to >>>>> Documentation/devicetree/bindings/submitting-patches.txt ? I don't know >>>>> what the DT binding documentation maintainers' future plans are with >>>>> regards to automated documentation reflow, etc., but submitting a patch >>>>> there would stimulate at least some coordination on the issue. >>>> >>>> I don't think it's appropriate for that file to dictate that, in the same >>>> way >>>> that coding style documentation generally doesn't address that kind of >>>> detail >>>> regarding code structure. >>> >>> We do indeed specify details like this in our documentation guidelines. >>> Here are two examples: >>> >>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/kernel-doc-nano-HOWTO.txt#n103 >>> >>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/CodingStyle#n464 >> >> Perhaps I phrased my point slightly differently form the core of what I meant. >> >> I'm not aware that review feedback can't address topics that are not already >> addressed by the documentation. Is there such a rule? > > Not that I'm aware of, but I'm not sure that I understand the point you're > making. Care to rephrase to make it more explicit? > >> Equally, I think if you want the documentation to address a particular point, >> it's appropriate for you to submit a patch to the documentation to update it, >> rather than ask the reviewer to do so before accepting the review feedback. > > I guess my question is this: do you intend that the table-based > documentation approach you describe should apply generally to other DT > binding documents with similar per-chip support lists? Or is there > something about the Tegra AHB specifically that merits this format? > > If the former was intended -- in other words, you are proposing a policy > that should be followed in the general case -- then I would suggest that > the documentation policy should be described in a shared DT binding > CodingStyle or submitting-patches document, as we do elsewhere in the > kernel. > > For example, the guidance could read[*], using your earlier example: > > --- > If different values of a DT property are required for different chips > or different situations, these should be listed in the binding > documentation in the following format: > > - compatible: Must contain the following: > Tegra20: "nvidia,tegra20-ahb" > Tegra30: "nvidia,tegra30-ahb" > Tegra114: "nvidia,tegra114-ahb", "nvidia,tegra30-ahb" > (etc.) > > Each line in the list should be indented from the start of the section > describing the DT property by four spaces. There should be no blank > lines between each list row. > --- > > That way, the community can align on a common format for this table-based > format. Any automated parsing tools that read the DT documentation can > know what to expect; anyone who disagrees can speak up as the patch is > being considered; and the issue no longer needs to be a matter of taste: > it can be transformed into a matter of fact. > > Once the documentation format becomes a matter of fact, then patch > submitters have clear guidance to follow. Submitters can get the patches > right the first time and avoid wasting their time and reviewers' time. > Otherwise, there is the (quite present) risk that 'n' different reviewers > of the DT binding documentation could have 'n' different opinions about > how the data should be formatted, with each opinion conveying > minimal-to-no technical advantage over another. This just results in a > waste of time for everyone, time that is better spent on code. In my > view, every moment I spend reformatting documentation to standards that > aren't shared is not only wasted, it's time that's subtracted from my > ability to improve our actual upstream code and work on something that's > actually useful. > > > - Paul > > [*] I am neutral about the format or whether a narrative vs. a table > approach is best. Whatever it should be, it should just be common > guidance. >
| |