linux-kernel - Re: [PATCH] dt-bindings: clock: ti: Convert interface.txt to json-schema

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7361082a-f271-4ef4-9dad-06ee7445c749@linaro.org>
Date:   Tue, 28 Nov 2023 09:41:23 +0100
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Andreas Kemnade <andreas@...nade.info>
Cc:     mturquette@...libre.com, sboyd@...nel.org, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
        kristo@...nel.org, linux-clk@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-omap@...r.kernel.org
Subject: Re: [PATCH] dt-bindings: clock: ti: Convert interface.txt to
 json-schema

On 28/11/2023 09:32, Andreas Kemnade wrote:
> On Tue, 28 Nov 2023 09:00:16 +0100
> Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org> wrote:
> 
>>> +required:
>>> +  - compatible
>>> +  - clocks
>>> +  - '#clock-cells'  
>>
>> reg is required. Device cannot take "reg" from parent, DTS does not work
>> like this.
> 
> Well, apparently they do... and I am just dealing with status quo and not
> how it should be.
> Look at commit 31fc1c63c2ae4a542e3c7ac572a10a59ece45c24

Who designed clock-controller binding with a device node per each clock?
This is ridiculous (although of course not your fault here)! Looking at
omap3xxx-clocks.dtsi - all its children should be just defined by the
driver, not by DTSI.


> for the reasoning of not having reg.
> 
> 
> well, look at drivers/clk/ti/clk.c
> ti_clk_get_reg_addr();

That's a driver implementation, not bindings, thus confusion.

> 
> ...
> 
>        if (of_property_read_u32_index(node, "reg", index, &val)) {
>                 if (of_property_read_u32_index(node->parent, "reg",
>                                                index, &val)) {
>                         pr_err("%pOFn or parent must have reg[%d]!\n",
>                                node, index);
>                         return -EINVAL;
>                 }
>         }
> 
> 
> We have two usecases here (status quo in dts usage and code):
> If these interface clocks are below a ti,clksel then we are describing 
> multiple bits in the same register and therefore every child of ti,clksel
> would have the same reg.

Regs can have bits, so that could still work.

> If the interface clock is not below a ti,clksel then we have reg.

This should be expressed in the bindings. It's fine to make the reg
optional (skip the description, it's confusing), but the ti,clksel
should reference this schema and enforce it on the children.

Best regards,
Krzysztof