Re: [PATCH 2/2] dt-bindings: clock: intel,cgu-lgm: add mxl,control-gate option

From: Florian Eckert
Date: Tue Aug 01 2023 - 04:10:01 EST


Hello Krzysztof,

You described the desired Linux feature or behavior, not the actual
hardware. The bindings are about the latter, so instead you need to
rephrase the property and its description to match actual hardware
capabilities/features/configuration etc.

You have correctly identified that this is not a hardware configuration,
but a driver configuration. Currently, the driver is configured so that
the gates cannot be switched via the clk subsystem callbacks. When
registering the data structures from the driver, I have to pass a flag
GATE_CLK_HW so that the gate is managed by the driver.

I didn't want to always change the source of the driver when it has to
take
care of the GATE, so I wanted to map this via the dts.

I have a board support package from Maxlinear for the Lightning Mountain
Soc
with other drivers that are not upstream now. Some of them use the
clock framework some of them does not.

Due to missing documents it is not possible to send these drivers
upstream.

So when you upstream them, the binding becomes wrong or not needed?
Sorry, bindings are entirely independent of OS, so using this as an
argument is clear no-go.

Yes, that would probably be the case, as the maxlinear drivers are at
an early stage and are not yet upstreamable in my opinion. If I had the
documents, I would take a closer look. But they are developing behind
closed doors. Nothing can be contributed. Not until the drivers are
hopefully upstream at some point as the cgu-lgm.

Strictly speaking, this is about the gptc and the watchdog.

Since it is a buildin_platform driver, it can also not work via
module parameters.

None of this explains any hardware related part of this binding. You
created now policy for one specific OS. Devicetree, which is OS
independent, is not for such purposes.

Yes this would be the case. Maybe I need to patch the cgu-lgm.c [1]
and send it upstream to restore the old behavior.
Because the following commit has changed the behaviour [2].
Unfortunately, it is also included in 5.15 stable branch.
Which in my opinion should not have happened!

Best regards,
Florian

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/x86/clk-lgm.c?h=v6.5-rc4
[2] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/clk/x86/clk-cgu.c?h=v5.15.123&id=a0583edea4fdb7b5b87a077263dddab476e9f138