16.05.2019 12:01, Joseph Lo пишет:
> On 5/15/19 9:50 PM, Dmitry Osipenko wrote:
>> 15.05.2019 10:17, Joseph Lo пишет:
>>> On 5/15/19 12:28 AM, Dmitry Osipenko wrote:
>>>> 10.05.2019 11:47, Joseph Lo пишет:
>>>>> Add the binding document for the external memory controller (EMC)
>>>>> which
>>>>> communicates with external LPDDR4 devices. It includes the bindings of
>>>>> the EMC node and a sub-node of EMC table which under the reserved
>>>>> memory
>>>>> node. The EMC table contains the data of the rates that EMC supported.
>>>>>
>>>>> Signed-off-by: Joseph Lo <josephl@xxxxxxxxxx>
>>>>> ---
>>>>> v3:
>>>>> - drop the bindings of EMC table
>>>>> - add memory-region and reserved-memory node for EMC table
>>>>> ---
>>>>> .../nvidia,tegra210-emc.txt | 55
>>>>> +++++++++++++++++++
>>>>> 1 file changed, 55 insertions(+)
>>>>> create mode 100644
>>>>> Documentation/devicetree/bindings/memory-controllers/nvidia,tegra210-emc.txt
>>>>>
>>>>>
>>>>>
>>>>> diff --git
>>>>> a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra210-emc.txt
>>>>>
>>>>> b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra210-emc.txt
>>>>>
>>>>>
>>>>> new file mode 100644
>>>>> index 000000000000..d65aeef2329c
>>>>> --- /dev/null
>>>>> +++
>>>>> b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra210-emc.txt
>>>>>
>>>>>
>>>>> @@ -0,0 +1,55 @@
>>>>> +NVIDIA Tegra210 SoC EMC (external memory controller)
>>>>> +====================================================
>>>>> +
>>>>> +Device node
>>>>> +===========
>>>>> +Required properties :
>>>>> +- compatible : should be "nvidia,tegra210-emc".
>>>>> +- reg : physical base address and length of the controller's
>>>>> registers.
>>>>> +- clocks : phandles of the possible source clocks.
>>>>> +- clock-names : names of the possible source clocks.
>>>>> +- interrupts : Should contain the EMC general interrupt.
>>>>> +- memory-region : phandle to the reserved memory (see
>>>>> +
>>>>> Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt)
>>>>> which
>>>>>
>>>>> + contains a sub-node of EMC table.
>>>>> +- nvidia,memory-controller : phandle of the memory controller.
>>>>> +
>>>>> +Reserved memory node
>>>>> +====================
>>>>> +Should contain a sub-node of EMC table with required properties:
>>>>> +- compatible : should be "nvidia,tegra210-emc-table".
>>>>> +- reg : physical address and length of the location of EMC table.
>>>>> +
>>>>> +Example:
>>>>> + reserved-memory {
>>>>> + #address-cells = <2>;
>>>>> + #size-cells = <2>;
>>>>> + ranges;
>>>>> +
>>>>> + emc_table: emc-table@8be00000 {
>>>>> + compatible = "nvidia,tegra210-emc-table";
>>>>> + reg = <0x0 0x8be00000 0x0 0x10000>;
>>>>> + status = "okay";
>>>>> + };
>>>>
>>>> You essentially moved the v1 binding into obscure and undocumented
>>>> blob,
>>>> ignoring previous review comments. This is a very odd move... please
>>>> explain what is going on.
>>>>
>>>
>>> Discussed with Thierry offline which way we prefer to pass the EMC table
>>> to the kernel. Some reasons below we decide to chose this one (via
>>> binary blob).
>>>
>>> - The EMC table is much bigger than the previous Tegra generations
>>> (LPDDR4 v.s. LPDDR2/3). It's harder to settle in the review process. And
>>> if there is a new fix of the table in the future, we'll need to go
>>> through that again.
>>
>> I don't think that this a very good excuse for not documenting the
>> blob's structure.
>
> The blob's structure is in patch 4 now that we originally wanted to
> describe below. Basically, the content is the same.
> http://patchwork.ozlabs.org/patch/1084467/
> http://patchwork.ozlabs.org/patch/1063879/
I'm not asking about what exactly it is, but saying that every supported
blob structure version should be documented in my opinion, otherwise the
documentation is not really useful.
