On 28/11/2023 09:32, Andreas Kemnade wrote:
> On Tue, 28 Nov 2023 09:00:16 +0100
> Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> 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
