[PATCH v2 01/11] dt-bindings: clock: mobileye,eyeq5-clk: drop bindings

Théo Lebrun posted 11 patches 1 year, 9 months ago
There is a newer version of this series
[PATCH v2 01/11] dt-bindings: clock: mobileye,eyeq5-clk: drop bindings
Posted by Théo Lebrun 1 year, 9 months ago
Switch from sub-nodes in system-controller for each functionality to a
single node representing the entire OLB instance. dt-bindings is
unnecessary and soc/mobileye/mobileye,eyeq5-olb.yaml will inherit all
properties.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 .../bindings/clock/mobileye,eyeq5-clk.yaml         | 51 ----------------------
 1 file changed, 51 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml b/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml
deleted file mode 100644
index 2d4f2cde1e58..000000000000
--- a/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml
+++ /dev/null
@@ -1,51 +0,0 @@
-# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
-%YAML 1.2
----
-$id: http://devicetree.org/schemas/clock/mobileye,eyeq5-clk.yaml#
-$schema: http://devicetree.org/meta-schemas/core.yaml#
-
-title: Mobileye EyeQ5 clock controller
-
-description:
-  The EyeQ5 clock controller handles 10 read-only PLLs derived from the main
-  crystal clock. It also exposes one divider clock, a child of one of the PLLs.
-  Its registers live in a shared region called OLB.
-
-maintainers:
-  - Grégory Clement <gregory.clement@bootlin.com>
-  - Théo Lebrun <theo.lebrun@bootlin.com>
-  - Vladimir Kondratiev <vladimir.kondratiev@mobileye.com>
-
-properties:
-  compatible:
-    const: mobileye,eyeq5-clk
-
-  reg:
-    maxItems: 2
-
-  reg-names:
-    items:
-      - const: plls
-      - const: ospi
-
-  "#clock-cells":
-    const: 1
-
-  clocks:
-    maxItems: 1
-    description:
-      Input parent clock to all PLLs. Expected to be the main crystal.
-
-  clock-names:
-    items:
-      - const: ref
-
-required:
-  - compatible
-  - reg
-  - reg-names
-  - "#clock-cells"
-  - clocks
-  - clock-names
-
-additionalProperties: false

-- 
2.45.0

Re: [PATCH v2 01/11] dt-bindings: clock: mobileye,eyeq5-clk: drop bindings
Posted by Krzysztof Kozlowski 1 year, 9 months ago
On 03/05/2024 16:20, Théo Lebrun wrote:
> Switch from sub-nodes in system-controller for each functionality to a
> single node representing the entire OLB instance. dt-bindings is
> unnecessary and soc/mobileye/mobileye,eyeq5-olb.yaml will inherit all
> properties.

Why changing this? You just added these bindings not so long time ago...
This is very confusing to push bindings and then immediately ask to
remove them.

Best regards,
Krzysztof

Re: [PATCH v2 01/11] dt-bindings: clock: mobileye,eyeq5-clk: drop bindings
Posted by Krzysztof Kozlowski 1 year, 9 months ago
On 03/05/2024 17:57, Krzysztof Kozlowski wrote:
> On 03/05/2024 16:20, Théo Lebrun wrote:
>> Switch from sub-nodes in system-controller for each functionality to a
>> single node representing the entire OLB instance. dt-bindings is
>> unnecessary and soc/mobileye/mobileye,eyeq5-olb.yaml will inherit all
>> properties.
> 
> Why changing this? You just added these bindings not so long time ago...
> This is very confusing to push bindings and then immediately ask to
> remove them.

One more point - anyway this should be revert with clear explanation WHY
you are reverting bindings.

Same for second patch.

Best regards,
Krzysztof

Re: [PATCH v2 01/11] dt-bindings: clock: mobileye,eyeq5-clk: drop bindings
Posted by Théo Lebrun 1 year, 9 months ago
Hello,

On Fri May 3, 2024 at 6:05 PM CEST, Krzysztof Kozlowski wrote:
> On 03/05/2024 17:57, Krzysztof Kozlowski wrote:
> > On 03/05/2024 16:20, Théo Lebrun wrote:
> >> Switch from sub-nodes in system-controller for each functionality to a
> >> single node representing the entire OLB instance. dt-bindings is
> >> unnecessary and soc/mobileye/mobileye,eyeq5-olb.yaml will inherit all
> >> properties.
> > 
> > Why changing this? You just added these bindings not so long time ago...
> > This is very confusing to push bindings and then immediately ask to
> > remove them.

See this revision as a proposal of something that has been asked
multiple times in previous reviews. See message from Stephen Boyd on
last revision [0], or discussion with Rob Herring on much earlier
revision [1].

Proposal from Stephen Boyd of using auxiliary devices makes sense, that
could be the future direction of this series. It won't change the
dt-bindings aspect of it, only the driver implementations.

[0]: https://lore.kernel.org/lkml/daa732cb31d947c308513b535930c729.sboyd@kernel.org/
[1]: https://lore.kernel.org/lkml/20240124151405.GA930997-robh@kernel.org/

> One more point - anyway this should be revert with clear explanation WHY
> you are reverting bindings.

I'll make sure to use standard revert formatting and explain why it is
being done.

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH v2 01/11] dt-bindings: clock: mobileye,eyeq5-clk: drop bindings
Posted by Krzysztof Kozlowski 1 year, 9 months ago
On 07/05/2024 17:07, Théo Lebrun wrote:
> Hello,
> 
> On Fri May 3, 2024 at 6:05 PM CEST, Krzysztof Kozlowski wrote:
>> On 03/05/2024 17:57, Krzysztof Kozlowski wrote:
>>> On 03/05/2024 16:20, Théo Lebrun wrote:
>>>> Switch from sub-nodes in system-controller for each functionality to a
>>>> single node representing the entire OLB instance. dt-bindings is
>>>> unnecessary and soc/mobileye/mobileye,eyeq5-olb.yaml will inherit all
>>>> properties.
>>>
>>> Why changing this? You just added these bindings not so long time ago...
>>> This is very confusing to push bindings and then immediately ask to
>>> remove them.
> 
> See this revision as a proposal of something that has been asked
> multiple times in previous reviews. See message from Stephen Boyd on

That's driver, we talk about bindings.

> last revision [0], or discussion with Rob Herring on much earlier
> revision [1].
> 
> Proposal from Stephen Boyd of using auxiliary devices makes sense, that
> could be the future direction of this series. It won't change the
> dt-bindings aspect of it, only the driver implementations.
> 
> [0]: https://lore.kernel.org/lkml/daa732cb31d947c308513b535930c729.sboyd@kernel.org/
> [1]: https://lore.kernel.org/lkml/20240124151405.GA930997-robh@kernel.org/

So after Robs comment above, you still pushed the wrong approach and now
you revert it?

Why v7 was sent ignoring Rob's comments:
https://lore.kernel.org/all/20240221-mbly-clk-v7-3-31d4ce3630c3@bootlin.com/

Best regards,
Krzysztof

Re: [PATCH v2 01/11] dt-bindings: clock: mobileye,eyeq5-clk: drop bindings
Posted by Théo Lebrun 1 year, 7 months ago
Hello Krzysztof,

On Tue May 7, 2024 at 5:34 PM CEST, Krzysztof Kozlowski wrote:
> On 07/05/2024 17:07, Théo Lebrun wrote:
> > Proposal from Stephen Boyd of using auxiliary devices makes sense, that
> > could be the future direction of this series. It won't change the
> > dt-bindings aspect of it, only the driver implementations.
> > 
> > [0]: https://lore.kernel.org/lkml/daa732cb31d947c308513b535930c729.sboyd@kernel.org/
> > [1]: https://lore.kernel.org/lkml/20240124151405.GA930997-robh@kernel.org/
>
> So after Robs comment above, you still pushed the wrong approach and now
> you revert it?

Yes. The gist of it is that I had misunderstood the messages. Mostly, I
did not understand how to implement separate Linux driver with the
desired devicetree structure (no subnode on the syscon for each
feature). I was missing knowledge about Linux infrastructure allowing
for that. MFD and auxdevs are two approaches, with auxdevs being
preferred.

The latest revision finally takes those comments into account.

Thanks Krzysztof,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com