18.08.2021 16:59, Thierry Reding пишет:
> On Tue, Aug 17, 2021 at 04:27:26AM +0300, Dmitry Osipenko wrote:
>> Document tegra-clocks sub-node which describes Tegra SoC clocks that
>> require a higher voltage of the core power domain in order to operate
>> properly on a higher clock rates. Each node contains a phandle to OPP
>> table and power domain.
>>
>> The root PLLs and system clocks don't have any specific device dedicated
>> to them, clock controller is in charge of managing power for them.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@xxxxxxxxx>
>> ---
>> .../bindings/clock/nvidia,tegra20-car.yaml | 51 +++++++++++++++++++
>> 1 file changed, 51 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.yaml b/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.yaml
>> index 459d2a525393..7f5cd27e4ce0 100644
>> --- a/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.yaml
>> +++ b/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.yaml
>> @@ -42,6 +42,48 @@ properties:
>> "#reset-cells":
>> const: 1
>>
>> + tegra-clocks:
>> + description: child nodes are the output clocks from the CAR
>> + type: object
>> +
>> + patternProperties:
>> + "^[a-z]+[0-9]+$":
>> + type: object
>> + properties:
>> + compatible:
>> + allOf:
>> + - items:
>> + - enum:
>> + - nvidia,tegra20-sclk
>> + - nvidia,tegra30-sclk
>> + - nvidia,tegra30-pllc
>> + - nvidia,tegra30-plle
>> + - nvidia,tegra30-pllm
>> + - const: nvidia,tegra-clock
>> +
>> + operating-points-v2:
>> + $ref: /schemas/types.yaml#/definitions/phandle
>> + description:
>> + Phandle to OPP table that contains frequencies, voltages and
>> + opp-supported-hw property, which is a bitfield indicating
>> + SoC process or speedo ID mask.
>> +
>> + clocks:
>> + items:
>> + - description: node's clock
>> +
>> + power-domains:
>> + maxItems: 1
>> + description: phandle to the core SoC power domain
>> +
>> + required:
>> + - compatible
>> + - operating-points-v2
>> + - clocks
>> + - power-domains
>> +
>> + additionalProperties: false
>> +
>> required:
>> - compatible
>> - reg
>> @@ -59,6 +101,15 @@ examples:
>> reg = <0x60006000 0x1000>;
>> #clock-cells = <1>;
>> #reset-cells = <1>;
>> +
>> + tegra-clocks {
>> + sclk {
>> + compatible = "nvidia,tegra20-sclk", "nvidia,tegra-clock";
>> + operating-points-v2 = <&opp_table>;
>> + clocks = <&tegra_car TEGRA20_CLK_SCLK>;
>> + power-domains = <&domain>;
>> + };
>> + };
>
> I wonder if it'd be better to match on the name of the node rather than
> add an artificial compatible string. We usually use the compatible
> string to match a device, but here you're really trying to add
> information about a resource provided by the CAR controller.
>
> We do similar things for example in PMIC bindings where the individual
> regulators are represented in the device tree via nodes named after the
> regulator.
>
> You could then also leave out the clocks property, which is weird as it
> is because it's basically a self-reference. But you don't really need
> the reference here in the first place because the CAR is already the
> parent of SCLK.
We don't have a platform device for CaR. I don't see how it's going to
work. We need to create a platform device for each RPM-capable clock
because that's how RPM works. The compatible string is required for
instantiating OF-devices from a node, otherwise we will have to
re-invent the OF core.
> Also, I don't think the tegra- prefix is necessary here. The parent node
> is already identified as Tegra via the compatible string.
>
> In the case of CAR, I'd imagine something like:
>
> clocks {
> sclk {
> operating-points-v2 = <&opp_table>;
> power-domains = <&domain>;
> };
> };
>
> Now you've only got the bare minimum in here that you actually add. All
> the other data that you used to have is simply derived from the parent.
'clocks' is already a generic keyword in DT. It's probably not okay to
redefine it.
