diff mbox series

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

Message ID 20240503-mbly-olb-v2-1-95ce5a1e18fe@bootlin.com
State New
Headers show
Series Add Mobileye EyeQ system controller support (clk, reset, pinctrl) | expand

Commit Message

Théo Lebrun May 3, 2024, 2:20 p.m. UTC
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(-)

Comments

Krzysztof Kozlowski May 3, 2024, 3:57 p.m. UTC | #1
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
Krzysztof Kozlowski May 3, 2024, 4:05 p.m. UTC | #2
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
Théo Lebrun May 7, 2024, 3:07 p.m. UTC | #3
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
Krzysztof Kozlowski May 7, 2024, 3:34 p.m. UTC | #4
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
diff mbox series

Patch

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