[PATCH 00/11] Add Mobileye EyeQ system controller support (clk, reset, pinctrl)

[PATCH 00/11] Add Mobileye EyeQ system controller support (clk, reset, pinctrl)

[Théo Lebrun]

Hello,

This builds on previous EyeQ5 system-controller revisions [0] and adds
EyeQ6L + EyeQ6H support. Pinctrl is not affected because it is not
handled in the same manner on those new platforms; it will be dealt
with with pinctrl-single. Only clk and reset expand to support those
new platform.

Changes in drivers are pretty massive, which explains why I started a
new revision count.

EyeQ6H is particular in that it has not one but seven OLB instances.
All seven host a (different) clock controller. Three host a reset
controller.

Patches are targeting MIPS, clk, reset and pinctrl trees independently.

 - dt-bindings: soc: mobileye: add EyeQ5 OLB system controller
 - MIPS: mobileye: eyeq5: add OLB syscon node
 - MIPS: mobileye: eyeq5: use OLB clocks controller node
 - MIPS: mobileye: eyeq5: add OLB reset controller node
 - MIPS: mobileye: eyeq5: add pinctrl node & pinmux function nodes

   Patches have no dependencies on this series. All required devicetrees
   and bindings got in the last release. Bindings changes below are
   only related to EyeQ6L/EyeQ6H.

 - dt-bindings: clock: mobileye,eyeq5-clk: add EyeQ6L and EyeQ6H
 - clk: divider: Introduce CLK_DIVIDER_EVEN_INTEGERS flag
 - clk: eyeq: add driver

   Clocks on all three platforms are rather similar. We have a bunch of
   PLLs children to the main crystal, some being required early
   (at of_clk_init(), before platform bus init).

   We have a few divider clocks in some instances. A custom clk-divider
   flag is introduced for the divisor because one clk would have a 256
   entries table otherwise.

   Compatible match data stores it all, nothing is declared in DT. Match
   data has three arrays for the three types of clocks: early PLLs,
   standard PLLs and divider clks.

 - dt-bindings: reset: mobileye,eyeq5-reset: add EyeQ6L and EyeQ6H
 - reset: eyeq: add platform driver

   Resets on all three platforms are rather similar. We therefore
   declare reset domain types and assign compatibles to an array of
   domains, with types and valid masks associated. The rest is pretty
   similar to previous code.

   EyeQ6H west and east OLB each host an instance of the same compat.

 - pinctrl: eyeq5: add platform driver

   Not affected by EyeQ6L/EyeQ6H additions. It has not changed.

Have a nice day,
Théo

[0]: https://lore.kernel.org/lkml/20240301-mbly-clk-v9-0-cbf06eb88708@bootlin.com/

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
Théo Lebrun (11):
      dt-bindings: soc: mobileye: add EyeQ5 OLB system controller
      dt-bindings: clock: mobileye,eyeq5-clk: add EyeQ6L and EyeQ6H
      dt-bindings: reset: mobileye,eyeq5-reset: add EyeQ6L and EyeQ6H
      clk: divider: Introduce CLK_DIVIDER_EVEN_INTEGERS flag
      clk: eyeq: add driver
      reset: eyeq: add platform driver
      pinctrl: eyeq5: add platform driver
      MIPS: mobileye: eyeq5: add OLB syscon node
      MIPS: mobileye: eyeq5: use OLB clocks controller node
      MIPS: mobileye: eyeq5: add OLB reset controller node
      MIPS: mobileye: eyeq5: add pinctrl node & pinmux function nodes

 .../bindings/clock/mobileye,eyeq5-clk.yaml         | 103 +++-
 .../bindings/reset/mobileye,eyeq5-reset.yaml       |  88 ++-
 .../bindings/soc/mobileye/mobileye,eyeq5-olb.yaml  | 125 ++++
 MAINTAINERS                                        |   8 +
 .../{eyeq5-fixed-clocks.dtsi => eyeq5-clocks.dtsi} |  54 +-
 arch/mips/boot/dts/mobileye/eyeq5-pins.dtsi        | 125 ++++
 arch/mips/boot/dts/mobileye/eyeq5.dtsi             |  42 +-
 drivers/clk/Kconfig                                |  11 +
 drivers/clk/Makefile                               |   1 +
 drivers/clk/clk-divider.c                          |  12 +-
 drivers/clk/clk-eyeq.c                             | 644 +++++++++++++++++++++
 drivers/pinctrl/Kconfig                            |  14 +
 drivers/pinctrl/Makefile                           |   1 +
 drivers/pinctrl/pinctrl-eyeq5.c                    | 579 ++++++++++++++++++
 drivers/reset/Kconfig                              |  13 +
 drivers/reset/Makefile                             |   1 +
 drivers/reset/reset-eyeq.c                         | 543 +++++++++++++++++
 include/dt-bindings/clock/mobileye,eyeq5-clk.h     |  21 +
 include/linux/clk-provider.h                       |  11 +-
 19 files changed, 2322 insertions(+), 74 deletions(-)
---
base-commit: c8e31f416e99bd460f6f8847709bf69c72a3e146
change-id: 20240408-mbly-olb-75a85f5cfde3

Best regards,
-- 
Théo Lebrun <theo.lebrun@bootlin.com>

[PATCH 01/11] dt-bindings: soc: mobileye: add EyeQ5 OLB system controller

[Théo Lebrun]

Add documentation to describe the "Other Logic Block" syscon.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 .../bindings/soc/mobileye/mobileye,eyeq5-olb.yaml  | 125 +++++++++++++++++++++
 MAINTAINERS                                        |   1 +
 2 files changed, 126 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml b/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml
new file mode 100644
index 000000000000..c4e33a167fab
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml
@@ -0,0 +1,125 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/mobileye/mobileye,eyeq5-olb.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mobileye EyeQ SoC system controller
+
+maintainers:
+  - Grégory Clement <gregory.clement@bootlin.com>
+  - Théo Lebrun <theo.lebrun@bootlin.com>
+  - Vladimir Kondratiev <vladimir.kondratiev@mobileye.com>
+
+description:
+  OLB ("Other Logic Block") is a hardware block grouping smaller blocks. Clocks,
+  resets, pinctrl are being handled from here. EyeQ5 and EyeQ6L host a single
+  instance. EyeQ6H hosts seven instances.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - mobileye,eyeq5-olb
+          - mobileye,eyeq6l-olb
+          - mobileye,eyeq6h-olb
+      - const: syscon
+      - const: simple-mfd
+
+  reg:
+    maxItems: 1
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 1
+
+  ranges: true
+
+patternProperties:
+  '^clock-controller@[0-9a-f]+$':
+    $ref: /schemas/clock/mobileye,eyeq5-clk.yaml#
+
+  '^reset-controller@[0-9a-f]+$':
+    $ref: /schemas/reset/mobileye,eyeq5-reset.yaml#
+
+  '^pinctrl@[0-9a-f]+$':
+    $ref: /schemas/pinctrl/mobileye,eyeq5-pinctrl.yaml#
+
+required:
+  - compatible
+  - reg
+  - '#address-cells'
+  - '#size-cells'
+  - ranges
+
+additionalProperties: false
+
+examples:
+  - |
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      system-controller@e00000 {
+        compatible = "mobileye,eyeq5-olb", "syscon", "simple-mfd";
+        reg = <0x0 0xe00000 0x0 0x400>;
+        ranges = <0x0 0x0 0xe00000 0x400>;
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        reset-controller@e00000 {
+          compatible = "mobileye,eyeq5-reset";
+          reg = <0x000 0x0c>, <0x200 0x34>, <0x120 0x04>;
+          reg-names = "d0", "d1", "d2";
+          #reset-cells = <2>;
+        };
+
+        clock-controller@e0002c {
+          compatible = "mobileye,eyeq5-clk";
+          reg = <0x02c 0x50>, <0x11c 0x04>;
+          reg-names = "plls", "ospi";
+          #clock-cells = <1>;
+          clocks = <&xtal>;
+          clock-names = "ref";
+        };
+
+        pinctrl@e000b0 {
+          compatible = "mobileye,eyeq5-pinctrl";
+          reg = <0x0b0 0x30>;
+
+          uart2_pins: uart2-pins {
+            function = "uart2";
+            pins = "PB8", "PB9";
+          };
+        };
+      };
+    };
+  - |
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      system-controller@d2003000 {
+        compatible = "mobileye,eyeq6h-olb", "syscon", "simple-mfd";
+        reg = <0x0 0xd2003000 0x0 0x1000>;
+        ranges = <0x0 0x0 0xd2003000 0x1000>;
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        reset-controller@d2003000 {
+          compatible = "mobileye,eyeq6h-acc-reset";
+          reg = <0x0 0x3c>;
+          #reset-cells = <1>;
+        };
+
+        clock-controller@d2003040 {
+          compatible = "mobileye,eyeq6h-acc-clk";
+          reg = <0x40 0x38>;
+          #clock-cells = <1>;
+          clocks = <&xtal>;
+          clock-names = "ref";
+        };
+      };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index aa3b947fb080..30dfbee84007 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14926,6 +14926,7 @@ M:	Théo Lebrun <theo.lebrun@bootlin.com>
 L:	linux-mips@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/mips/mobileye.yaml
+F:	Documentation/devicetree/bindings/soc/mobileye/
 F:	arch/mips/boot/dts/mobileye/
 F:	arch/mips/configs/eyeq5_defconfig
 F:	arch/mips/mobileye/board-epm5.its.S

-- 
2.44.0

Re: [PATCH 01/11] dt-bindings: soc: mobileye: add EyeQ5 OLB system controller

[Rob Herring]

On Wed, 10 Apr 2024 19:12:30 +0200, Théo Lebrun wrote:
> Add documentation to describe the "Other Logic Block" syscon.
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  .../bindings/soc/mobileye/mobileye,eyeq5-olb.yaml  | 125 +++++++++++++++++++++
>  MAINTAINERS                                        |   1 +
>  2 files changed, 126 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.example.dtb: system-controller@d2003000: clock-controller@d2003040:compatible:0: 'mobileye,eyeq5-clk' was expected
	from schema $id: http://devicetree.org/schemas/soc/mobileye/mobileye,eyeq5-olb.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.example.dtb: system-controller@d2003000: clock-controller@d2003040:reg: [[64, 56]] is too short
	from schema $id: http://devicetree.org/schemas/soc/mobileye/mobileye,eyeq5-olb.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.example.dtb: system-controller@d2003000: clock-controller@d2003040: 'reg-names' is a required property
	from schema $id: http://devicetree.org/schemas/soc/mobileye/mobileye,eyeq5-olb.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.example.dtb: system-controller@d2003000: reset-controller@d2003000:compatible:0: 'mobileye,eyeq5-reset' was expected
	from schema $id: http://devicetree.org/schemas/soc/mobileye/mobileye,eyeq5-olb.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.example.dtb: system-controller@d2003000: reset-controller@d2003000:reg: [[0, 60]] is too short
	from schema $id: http://devicetree.org/schemas/soc/mobileye/mobileye,eyeq5-olb.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.example.dtb: system-controller@d2003000: reset-controller@d2003000:#reset-cells:0:0: 2 was expected
	from schema $id: http://devicetree.org/schemas/soc/mobileye/mobileye,eyeq5-olb.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.example.dtb: system-controller@d2003000: reset-controller@d2003000: 'reg-names' is a required property
	from schema $id: http://devicetree.org/schemas/soc/mobileye/mobileye,eyeq5-olb.yaml#
Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.example.dtb: /example-1/soc/system-controller@d2003000/reset-controller@d2003000: failed to match any schema with compatible: ['mobileye,eyeq6h-acc-reset']
Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.example.dtb: /example-1/soc/system-controller@d2003000/clock-controller@d2003040: failed to match any schema with compatible: ['mobileye,eyeq6h-acc-clk']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240410-mbly-olb-v1-1-335e496d7be3@bootlin.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 01/11] dt-bindings: soc: mobileye: add EyeQ5 OLB system controller

[Théo Lebrun]

Hello,

On Wed Apr 10, 2024 at 8:52 PM CEST, Rob Herring wrote:
> On Wed, 10 Apr 2024 19:12:30 +0200, Théo Lebrun wrote:
> > Add documentation to describe the "Other Logic Block" syscon.
> > 
> > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> > ---
> >  .../bindings/soc/mobileye/mobileye,eyeq5-olb.yaml  | 125 +++++++++++++++++++++
> >  MAINTAINERS                                        |   1 +
> >  2 files changed, 126 insertions(+)
>
> My bot found errors running 'make dt_binding_check' on your patch:
>
> yamllint warnings/errors:

[...]

I made the mistake of putting an EyeQ6H example inside
Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml

I will remove it to get this error fixed. Else there would be
dependencies on clk and reset patches from this series:

   dt-bindings: clock: mobileye,eyeq5-clk: add EyeQ6L and EyeQ6H
   dt-bindings: reset: mobileye,eyeq5-reset: add EyeQ6L and EyeQ6H

Regards,

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

Re: [PATCH 01/11] dt-bindings: soc: mobileye: add EyeQ5 OLB system controller

[Stephen Boyd]

Quoting Théo Lebrun (2024-04-10 10:12:30)
> diff --git a/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml b/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml
> new file mode 100644
> index 000000000000..c4e33a167fab
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml
> @@ -0,0 +1,125 @@
[..]
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 1
> +
> +  ranges: true
> +
> +patternProperties:
> +  '^clock-controller@[0-9a-f]+$':
> +    $ref: /schemas/clock/mobileye,eyeq5-clk.yaml#
> +
> +  '^reset-controller@[0-9a-f]+$':
> +    $ref: /schemas/reset/mobileye,eyeq5-reset.yaml#
> +
> +  '^pinctrl@[0-9a-f]+$':
> +    $ref: /schemas/pinctrl/mobileye,eyeq5-pinctrl.yaml#

Yep, there shouldn't be subnodes for these. Instead, olb should have
#clock-cells, #reset-cells, etc. and the driver registers auxiliary
devices for each driver like the clk driver, reset driver, pinctrl
driver. Then we don't need syscon or simple-mfd and random other drivers
can't use the regmap.

[PATCH 02/11] dt-bindings: clock: mobileye,eyeq5-clk: add EyeQ6L and EyeQ6H

[Théo Lebrun]

Add bindings describing EyeQ6L and EyeQ6H clock controllers.
Add constants to index clocks.

Bindings are conditional for two reasons:
 - Some compatibles expose a single clock; they do not take clock cells.
 - All compatibles take a PLLs resource, not all take others (aimed at
   divider clocks). Those that only take a resource for PLLs do not
   require named resources.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 .../bindings/clock/mobileye,eyeq5-clk.yaml         | 103 ++++++++++++++++++---
 MAINTAINERS                                        |   2 +
 include/dt-bindings/clock/mobileye,eyeq5-clk.h     |  21 +++++
 3 files changed, 113 insertions(+), 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml b/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml
index 2d4f2cde1e58..a1651fcce258 100644
--- a/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml
+++ b/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml
@@ -4,12 +4,13 @@
 $id: http://devicetree.org/schemas/clock/mobileye,eyeq5-clk.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Mobileye EyeQ5 clock controller
+title: Mobileye EyeQ 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.
+  EyeQ clock controllers expose read-only PLLs derived from main crystal clock.
+  Some also expose divider clocks, children of specific PLLs. Its registers
+  live in a shared region called OLB. EyeQ5 and EyeQ6L have a single OLB
+  instance while EyeQ6H have seven, leading to seven clock controllers.
 
 maintainers:
   - Grégory Clement <gregory.clement@bootlin.com>
@@ -18,18 +19,23 @@ maintainers:
 
 properties:
   compatible:
-    const: mobileye,eyeq5-clk
+    enum:
+      - mobileye,eyeq5-clk
+      - mobileye,eyeq6l-clk
+      - mobileye,eyeq6h-central-clk
+      - mobileye,eyeq6h-west-clk
+      - mobileye,eyeq6h-east-clk
+      - mobileye,eyeq6h-south-clk
+      - mobileye,eyeq6h-ddr0-clk
+      - mobileye,eyeq6h-ddr1-clk
+      - mobileye,eyeq6h-acc-clk
 
-  reg:
-    maxItems: 2
+  reg: true
 
-  reg-names:
-    items:
-      - const: plls
-      - const: ospi
+  reg-names: true
 
   "#clock-cells":
-    const: 1
+    enum: [0, 1]
 
   clocks:
     maxItems: 1
@@ -43,9 +49,80 @@ properties:
 required:
   - compatible
   - reg
-  - reg-names
   - "#clock-cells"
   - clocks
   - clock-names
 
+allOf:
+  # "mobileye,eyeq5-clk" provides:
+  #  - PLLs and,
+  #  - One divider clock related to ospi.
+  - if:
+      properties:
+        compatible:
+          const: mobileye,eyeq5-clk
+    then:
+      properties:
+        reg:
+          minItems: 2
+          maxItems: 2
+        reg-names:
+          minItems: 2
+          maxItems: 2
+          items:
+            enum: [ plls, ospi ]
+      required:
+        - reg-names
+
+  # "mobileye,eyeq6h-south-clk" provides:
+  #  - PLLs and,
+  #  - Four divider clocks related to emmc, ospi and tsu.
+  - if:
+      properties:
+        compatible:
+          const: mobileye,eyeq6h-south-clk
+    then:
+      properties:
+        reg:
+          minItems: 4
+          maxItems: 4
+        reg-names:
+          minItems: 4
+          maxItems: 4
+          items:
+            enum: [ plls, emmc, ospi, tsu ]
+      required:
+        - reg-names
+
+  # Other compatibles only provide PLLs. Do not ask for named resources.
+  - if:
+      not:
+        required:
+          - reg-names
+    then:
+      properties:
+        reg:
+          minItems: 1
+          maxItems: 1
+        reg-names: false
+
+  # Some compatibles provide a single clock; they do not take a clock cell.
+  - if:
+      properties:
+        compatible:
+          enum:
+            - mobileye,eyeq6h-central-clk
+            - mobileye,eyeq6h-west-clk
+            - mobileye,eyeq6h-east-clk
+            - mobileye,eyeq6h-ddr0-clk
+            - mobileye,eyeq6h-ddr1-clk
+    then:
+      properties:
+        "#clock-cells":
+          const: 0
+    else:
+      properties:
+        "#clock-cells":
+          const: 1
+
 additionalProperties: false
diff --git a/MAINTAINERS b/MAINTAINERS
index 30dfbee84007..f5a488331b38 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14925,11 +14925,13 @@ M:	Gregory CLEMENT <gregory.clement@bootlin.com>
 M:	Théo Lebrun <theo.lebrun@bootlin.com>
 L:	linux-mips@vger.kernel.org
 S:	Maintained
+F:	Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml
 F:	Documentation/devicetree/bindings/mips/mobileye.yaml
 F:	Documentation/devicetree/bindings/soc/mobileye/
 F:	arch/mips/boot/dts/mobileye/
 F:	arch/mips/configs/eyeq5_defconfig
 F:	arch/mips/mobileye/board-epm5.its.S
+F:	include/dt-bindings/clock/mobileye,eyeq5-clk.h
 
 MODULE SUPPORT
 M:	Luis Chamberlain <mcgrof@kernel.org>
diff --git a/include/dt-bindings/clock/mobileye,eyeq5-clk.h b/include/dt-bindings/clock/mobileye,eyeq5-clk.h
index 26d8930335e4..b433c1772c28 100644
--- a/include/dt-bindings/clock/mobileye,eyeq5-clk.h
+++ b/include/dt-bindings/clock/mobileye,eyeq5-clk.h
@@ -19,4 +19,25 @@
 
 #define EQ5C_DIV_OSPI	10
 
+#define EQ6LC_PLL_DDR		0
+#define EQ6LC_PLL_CPU		1
+#define EQ6LC_PLL_PER		2
+#define EQ6LC_PLL_VDI		3
+
+#define EQ6HC_SOUTH_PLL_VDI		0
+#define EQ6HC_SOUTH_PLL_PCIE		1
+#define EQ6HC_SOUTH_PLL_PER		2
+#define EQ6HC_SOUTH_PLL_ISP		3
+
+#define EQ6HC_SOUTH_DIV_EMMC		4
+#define EQ6HC_SOUTH_DIV_OSPI_REF	5
+#define EQ6HC_SOUTH_DIV_OSPI_SYS	6
+#define EQ6HC_SOUTH_DIV_TSU		7
+
+#define EQ6HC_ACC_PLL_XNN		0
+#define EQ6HC_ACC_PLL_VMP		1
+#define EQ6HC_ACC_PLL_PMA		2
+#define EQ6HC_ACC_PLL_MPC		3
+#define EQ6HC_ACC_PLL_NOC		4
+
 #endif

-- 
2.44.0

Re: [PATCH 02/11] dt-bindings: clock: mobileye,eyeq5-clk: add EyeQ6L and EyeQ6H

[Krzysztof Kozlowski]

On 10/04/2024 19:12, Théo Lebrun wrote:
> Add bindings describing EyeQ6L and EyeQ6H clock controllers.
> Add constants to index clocks.
> 
> Bindings are conditional for two reasons:
>  - Some compatibles expose a single clock; they do not take clock cells.
>  - All compatibles take a PLLs resource, not all take others (aimed at
>    divider clocks). Those that only take a resource for PLLs do not
>    require named resources.
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  .../bindings/clock/mobileye,eyeq5-clk.yaml         | 103 ++++++++++++++++++---
>  MAINTAINERS                                        |   2 +
>  include/dt-bindings/clock/mobileye,eyeq5-clk.h     |  21 +++++
>  3 files changed, 113 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml b/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml
> index 2d4f2cde1e58..a1651fcce258 100644
> --- a/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml
> +++ b/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml
> @@ -4,12 +4,13 @@
>  $id: http://devicetree.org/schemas/clock/mobileye,eyeq5-clk.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>  
> -title: Mobileye EyeQ5 clock controller
> +title: Mobileye EyeQ 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.
> +  EyeQ clock controllers expose read-only PLLs derived from main crystal clock.
> +  Some also expose divider clocks, children of specific PLLs. Its registers
> +  live in a shared region called OLB. EyeQ5 and EyeQ6L have a single OLB
> +  instance while EyeQ6H have seven, leading to seven clock controllers.
>  
>  maintainers:
>    - Grégory Clement <gregory.clement@bootlin.com>
> @@ -18,18 +19,23 @@ maintainers:
>  
>  properties:
>    compatible:
> -    const: mobileye,eyeq5-clk
> +    enum:
> +      - mobileye,eyeq5-clk
> +      - mobileye,eyeq6l-clk
> +      - mobileye,eyeq6h-central-clk
> +      - mobileye,eyeq6h-west-clk
> +      - mobileye,eyeq6h-east-clk
> +      - mobileye,eyeq6h-south-clk
> +      - mobileye,eyeq6h-ddr0-clk
> +      - mobileye,eyeq6h-ddr1-clk
> +      - mobileye,eyeq6h-acc-clk
>  
> -  reg:
> -    maxItems: 2
> +  reg: true

No, you must leave widest constraints here.

>  
> -  reg-names:
> -    items:
> -      - const: plls
> -      - const: ospi
> +  reg-names: true

No, you must leave widest constraints here.


>  
>    "#clock-cells":
> -    const: 1
> +    enum: [0, 1]

Looks like you squash here quite different devices...

>  
>    clocks:
>      maxItems: 1
> @@ -43,9 +49,80 @@ properties:
>  required:
>    - compatible
>    - reg
> -  - reg-names
>    - "#clock-cells"
>    - clocks
>    - clock-names
>  
> +allOf:
> +  # "mobileye,eyeq5-clk" provides:
> +  #  - PLLs and,
> +  #  - One divider clock related to ospi.
> +  - if:
> +      properties:
> +        compatible:
> +          const: mobileye,eyeq5-clk
> +    then:
> +      properties:
> +        reg:
> +          minItems: 2
> +          maxItems: 2
> +        reg-names:
> +          minItems: 2
> +          maxItems: 2

So any name is now valid? Like "yellow-pony"?

> +          items:
> +            enum: [ plls, ospi ]
> +      required:
> +        - reg-names
> +
> +  # "mobileye,eyeq6h-south-clk" provides:
> +  #  - PLLs and,
> +  #  - Four divider clocks related to emmc, ospi and tsu.
> +  - if:
> +      properties:
> +        compatible:
> +          const: mobileye,eyeq6h-south-clk
> +    then:
> +      properties:
> +        reg:
> +          minItems: 4
> +          maxItems: 4
> +        reg-names:
> +          minItems: 4
> +          maxItems: 4
> +          items:
> +            enum: [ plls, emmc, ospi, tsu ]
> +      required:
> +        - reg-names
> +
> +  # Other compatibles only provide PLLs. Do not ask for named resources.
> +  - if:
> +      not:
> +        required:
> +          - reg-names
> +    then:
> +      properties:
> +        reg:
> +          minItems: 1
> +          maxItems: 1

No, just restrict properly reg per variant.


> +        reg-names: false

That's redundant. Drop entire if.


> +
> +  # Some compatibles provide a single clock; they do not take a clock cell.
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - mobileye,eyeq6h-central-clk
> +            - mobileye,eyeq6h-west-clk
> +            - mobileye,eyeq6h-east-clk
> +            - mobileye,eyeq6h-ddr0-clk
> +            - mobileye,eyeq6h-ddr1-clk
> +    then:
> +      properties:
> +        "#clock-cells":
> +          const: 0

Wait, so you define device-per-clock? That's a terrible idea. We also
discussed it many times and it was rejected many times.

You have one device, not 5.



Best regards,
Krzysztof

Re: [PATCH 02/11] dt-bindings: clock: mobileye,eyeq5-clk: add EyeQ6L and EyeQ6H

[Théo Lebrun]

Hello Krzysztof,

On Thu Apr 11, 2024 at 8:14 AM CEST, Krzysztof Kozlowski wrote:
> On 10/04/2024 19:12, Théo Lebrun wrote:
> > Add bindings describing EyeQ6L and EyeQ6H clock controllers.
> > Add constants to index clocks.
> > 
> > Bindings are conditional for two reasons:
> >  - Some compatibles expose a single clock; they do not take clock cells.
> >  - All compatibles take a PLLs resource, not all take others (aimed at
> >    divider clocks). Those that only take a resource for PLLs do not
> >    require named resources.
> > 
> > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> > ---
> >  .../bindings/clock/mobileye,eyeq5-clk.yaml         | 103 ++++++++++++++++++---
> >  MAINTAINERS                                        |   2 +
> >  include/dt-bindings/clock/mobileye,eyeq5-clk.h     |  21 +++++
> >  3 files changed, 113 insertions(+), 13 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml b/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml

[...]

> >  properties:
> >    compatible:
> > -    const: mobileye,eyeq5-clk
> > +    enum:
> > +      - mobileye,eyeq5-clk
> > +      - mobileye,eyeq6l-clk
> > +      - mobileye,eyeq6h-central-clk
> > +      - mobileye,eyeq6h-west-clk
> > +      - mobileye,eyeq6h-east-clk
> > +      - mobileye,eyeq6h-south-clk
> > +      - mobileye,eyeq6h-ddr0-clk
> > +      - mobileye,eyeq6h-ddr1-clk
> > +      - mobileye,eyeq6h-acc-clk
> >  
> > -  reg:
> > -    maxItems: 2
> > +  reg: true
>
> No, you must leave widest constraints here.

Noted, will do.

> > -  reg-names:
> > -    items:
> > -      - const: plls
> > -      - const: ospi
> > +  reg-names: true
>
> No, you must leave widest constraints here.

Noted, will do.

> >    "#clock-cells":
> > -    const: 1
> > +    enum: [0, 1]
>
> Looks like you squash here quite different devices...

They are the same controllers but some only expose a single clock. It is
EyeQ6H that has 7 OLB instances, so some don't deal with many clocks.

I started with a more generic approach of #clock-cells = <1> and only
have index zero available for those that have a single clock.
I am not a fan of this however.

> >    clocks:
> >      maxItems: 1
> > @@ -43,9 +49,80 @@ properties:
> >  required:
> >    - compatible
> >    - reg
> > -  - reg-names
> >    - "#clock-cells"
> >    - clocks
> >    - clock-names
> >  
> > +allOf:
> > +  # "mobileye,eyeq5-clk" provides:
> > +  #  - PLLs and,
> > +  #  - One divider clock related to ospi.
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          const: mobileye,eyeq5-clk
> > +    then:
> > +      properties:
> > +        reg:
> > +          minItems: 2
> > +          maxItems: 2
> > +        reg-names:
> > +          minItems: 2
> > +          maxItems: 2
>
> So any name is now valid? Like "yellow-pony"?

I do not understand what implies this. Below "items: enum: [...]"
ensures only two allowed values. dtbs_check agrees:

⟩ git diff
diff --git a/arch/mips/boot/dts/mobileye/eyeq5.dtsi
           b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
index 8d4f65ec912d..5031eb8b4270 100644
--- a/arch/mips/boot/dts/mobileye/eyeq5.dtsi
+++ b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
@@ -126,7 +126,7 @@ reset: reset-controller@e00000 {
                        clocks: clock-controller@e0002c {
                                compatible = "mobileye,eyeq5-clk";
                                reg = <0x02c 0x50>, <0x11c 0x04>;
-                               reg-names = "plls", "ospi";
+                               reg-names = "plls", "yellow-pony";
                                #clock-cells = <1>;
                                clocks = <&xtal>;
                                clock-names = "ref";

⟩ make dtbs_check DT_SCHEMA_FILES=mobileye DT_CHECKER_FLAGS=-m
  UPD     include/config/kernel.release
  DTC_CHK arch/mips/boot/dts/mobileye/eyeq5-epm5.dtb
arch/mips/boot/dts/mobileye/eyeq5-epm5.dtb: system-controller@e00000:
  clock-controller@e0002c:reg-names:1:
  'yellow-pony' is not one of ['plls', 'ospi']
  from schema $id:
    http://devicetree.org/schemas/soc/mobileye/mobileye,eyeq5-olb.yaml#

> > +          items:
> > +            enum: [ plls, ospi ]
> > +      required:
> > +        - reg-names
> > +
> > +  # "mobileye,eyeq6h-south-clk" provides:
> > +  #  - PLLs and,
> > +  #  - Four divider clocks related to emmc, ospi and tsu.
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          const: mobileye,eyeq6h-south-clk
> > +    then:
> > +      properties:
> > +        reg:
> > +          minItems: 4
> > +          maxItems: 4
> > +        reg-names:
> > +          minItems: 4
> > +          maxItems: 4
> > +          items:
> > +            enum: [ plls, emmc, ospi, tsu ]
> > +      required:
> > +        - reg-names
> > +
> > +  # Other compatibles only provide PLLs. Do not ask for named resources.
> > +  - if:
> > +      not:
> > +        required:
> > +          - reg-names
> > +    then:
> > +      properties:
> > +        reg:
> > +          minItems: 1
> > +          maxItems: 1
>
> No, just restrict properly reg per variant.

Noted, will do.

> > +        reg-names: false
>
> That's redundant. Drop entire if.

Ah, yes. Will fix that.

> > +
> > +  # Some compatibles provide a single clock; they do not take a clock cell.
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          enum:
> > +            - mobileye,eyeq6h-central-clk
> > +            - mobileye,eyeq6h-west-clk
> > +            - mobileye,eyeq6h-east-clk
> > +            - mobileye,eyeq6h-ddr0-clk
> > +            - mobileye,eyeq6h-ddr1-clk
> > +    then:
> > +      properties:
> > +        "#clock-cells":
> > +          const: 0
>
> Wait, so you define device-per-clock? That's a terrible idea. We also
> discussed it many times and it was rejected many times.
>
> You have one device, not 5.

Each region must be a syscon to make its various registers accessible to
drivers that'll need it. Following that, I have a hard time seeing what
would be the DT structure of 7 OLB system-controllers but a single
clock node?

Regards,

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

Re: [PATCH 02/11] dt-bindings: clock: mobileye,eyeq5-clk: add EyeQ6L and EyeQ6H

[Krzysztof Kozlowski]

On 11/04/2024 15:49, Théo Lebrun wrote:
>>> +    then:
>>> +      properties:
>>> +        reg:
>>> +          minItems: 2
>>> +          maxItems: 2
>>> +        reg-names:
>>> +          minItems: 2
>>> +          maxItems: 2
>>
>> So any name is now valid? Like "yellow-pony"?
> 
> I do not understand what implies this. Below "items: enum: [...]"
> ensures only two allowed values. dtbs_check agrees:
> 
> ⟩ git diff
> diff --git a/arch/mips/boot/dts/mobileye/eyeq5.dtsi
>            b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
> index 8d4f65ec912d..5031eb8b4270 100644
> --- a/arch/mips/boot/dts/mobileye/eyeq5.dtsi
> +++ b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
> @@ -126,7 +126,7 @@ reset: reset-controller@e00000 {
>                         clocks: clock-controller@e0002c {
>                                 compatible = "mobileye,eyeq5-clk";
>                                 reg = <0x02c 0x50>, <0x11c 0x04>;
> -                               reg-names = "plls", "ospi";
> +                               reg-names = "plls", "yellow-pony";
>                                 #clock-cells = <1>;
>                                 clocks = <&xtal>;
>                                 clock-names = "ref";
> 
> ⟩ make dtbs_check DT_SCHEMA_FILES=mobileye DT_CHECKER_FLAGS=-m
>   UPD     include/config/kernel.release
>   DTC_CHK arch/mips/boot/dts/mobileye/eyeq5-epm5.dtb
> arch/mips/boot/dts/mobileye/eyeq5-epm5.dtb: system-controller@e00000:
>   clock-controller@e0002c:reg-names:1:
>   'yellow-pony' is not one of ['plls', 'ospi']
>   from schema $id:
>     http://devicetree.org/schemas/soc/mobileye/mobileye,eyeq5-olb.yaml#

Ah, so you defined the items but made them an random order? No, please
keep same syntax which is what we always recommend anyway:

https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L132

...

>>> +
>>> +  # Some compatibles provide a single clock; they do not take a clock cell.
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          enum:
>>> +            - mobileye,eyeq6h-central-clk
>>> +            - mobileye,eyeq6h-west-clk
>>> +            - mobileye,eyeq6h-east-clk
>>> +            - mobileye,eyeq6h-ddr0-clk
>>> +            - mobileye,eyeq6h-ddr1-clk
>>> +    then:
>>> +      properties:
>>> +        "#clock-cells":
>>> +          const: 0
>>
>> Wait, so you define device-per-clock? That's a terrible idea. We also
>> discussed it many times and it was rejected many times.
>>
>> You have one device, not 5.
> 
> Each region must be a syscon to make its various registers accessible to
> drivers that'll need it. Following that, I have a hard time seeing what
> would be the DT structure of 7 OLB system-controllers but a single
> clock node?

I assumed all these are in one syscon. Lack of DTS (example is quite
limited, which is expected) does not help. Please link full DTS so we
can see what you want to achieve.

Best regards,
Krzysztof

[PATCH 03/11] dt-bindings: reset: mobileye,eyeq5-reset: add EyeQ6L and EyeQ6H

[Théo Lebrun]

Add bindings for EyeQ6L and EyeQ6H reset controllers.

Some controllers host a single domain, meaning a single cell is enough.
We do not enforce reg-names for such nodes.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 .../bindings/reset/mobileye,eyeq5-reset.yaml       | 88 ++++++++++++++++++----
 MAINTAINERS                                        |  1 +
 2 files changed, 74 insertions(+), 15 deletions(-)

diff --git a/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml b/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml
index 062b4518347b..799bcf15bed9 100644
--- a/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml
+++ b/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml
@@ -4,11 +4,13 @@
 $id: http://devicetree.org/schemas/reset/mobileye,eyeq5-reset.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Mobileye EyeQ5 reset controller
+title: Mobileye EyeQ reset controller
 
 description:
-  The EyeQ5 reset driver handles three reset domains. Its registers live in a
-  shared region called OLB.
+  EyeQ reset controller handles one or more reset domains. They live in shared
+  regions called OLB. EyeQ5 and EyeQ6L host one OLB each, each with one reset
+  instance. EyeQ6H hosts 7 OLB regions; three of those (west, east,
+  accelerator) host reset controllers. West and east are duplicates.
 
 maintainers:
   - Grégory Clement <gregory.clement@bootlin.com>
@@ -17,27 +19,83 @@ maintainers:
 
 properties:
   compatible:
-    const: mobileye,eyeq5-reset
+    enum:
+      - mobileye,eyeq5-reset
+      - mobileye,eyeq6l-reset
+      - mobileye,eyeq6h-we-reset
+      - mobileye,eyeq6h-acc-reset
 
-  reg:
-    maxItems: 3
+  reg: true
 
-  reg-names:
-    items:
-      - const: d0
-      - const: d1
-      - const: d2
+  reg-names: true
 
   "#reset-cells":
-    const: 2
     description:
-      The first cell is the domain (0 to 2 inclusive) and the second one is the
-      reset index inside that domain.
+      First cell is domain, second is reset index inside that domain. If
+      controller has a single domain, first cell is implicitly zero.
+    enum: [ 1, 2 ]
 
 required:
   - compatible
   - reg
-  - reg-names
   - "#reset-cells"
 
+allOf:
+  # EyeQ5 and EyeQ6L have multiple domains, other compatibles have one.
+  # Multiple domains means named resources and two reset cells.
+  # Single domain means a single unnamed resource and one reset cell.
+  - if:
+      properties:
+        compatible:
+          enum:
+            - mobileye,eyeq5-reset
+            - mobileye,eyeq6l-reset
+    then:
+      properties:
+        "#reset-cells":
+          const: 2
+      required:
+        - reg-names
+    else:
+      properties:
+        reg:
+          maxItems: 1
+        reg-names: false
+        "#reset-cells":
+          const: 1
+
+  # EyeQ5 has three domains.
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: mobileye,eyeq5-reset
+    then:
+      properties:
+        reg:
+          minItems: 3
+          maxItems: 3
+        reg-names:
+          minItems: 3
+          maxItems: 3
+          items:
+            enum: [ d0, d1, d2 ]
+
+  # EyeQ6L has two domains.
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: mobileye,eyeq6l-reset
+    then:
+      properties:
+        reg:
+          minItems: 2
+          maxItems: 2
+        reg-names:
+          minItems: 2
+          maxItems: 2
+          items:
+            enum: [ d0, d1 ]
+
 additionalProperties: false
diff --git a/MAINTAINERS b/MAINTAINERS
index f5a488331b38..42553da10be9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14927,6 +14927,7 @@ L:	linux-mips@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml
 F:	Documentation/devicetree/bindings/mips/mobileye.yaml
+F:	Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml
 F:	Documentation/devicetree/bindings/soc/mobileye/
 F:	arch/mips/boot/dts/mobileye/
 F:	arch/mips/configs/eyeq5_defconfig

-- 
2.44.0

Re: [PATCH 03/11] dt-bindings: reset: mobileye,eyeq5-reset: add EyeQ6L and EyeQ6H

[Krzysztof Kozlowski]

On 10/04/2024 19:12, Théo Lebrun wrote:
> Add bindings for EyeQ6L and EyeQ6H reset controllers.
> 
> Some controllers host a single domain, meaning a single cell is enough.
> We do not enforce reg-names for such nodes.
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  .../bindings/reset/mobileye,eyeq5-reset.yaml       | 88 ++++++++++++++++++----
>  MAINTAINERS                                        |  1 +
>  2 files changed, 74 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml b/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml
> index 062b4518347b..799bcf15bed9 100644
> --- a/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml
> +++ b/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml
> @@ -4,11 +4,13 @@
>  $id: http://devicetree.org/schemas/reset/mobileye,eyeq5-reset.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>  
> -title: Mobileye EyeQ5 reset controller
> +title: Mobileye EyeQ reset controller
>  
>  description:
> -  The EyeQ5 reset driver handles three reset domains. Its registers live in a
> -  shared region called OLB.
> +  EyeQ reset controller handles one or more reset domains. They live in shared
> +  regions called OLB. EyeQ5 and EyeQ6L host one OLB each, each with one reset
> +  instance. EyeQ6H hosts 7 OLB regions; three of those (west, east,
> +  accelerator) host reset controllers. West and east are duplicates.
>  
>  maintainers:
>    - Grégory Clement <gregory.clement@bootlin.com>
> @@ -17,27 +19,83 @@ maintainers:
>  
>  properties:
>    compatible:
> -    const: mobileye,eyeq5-reset
> +    enum:
> +      - mobileye,eyeq5-reset
> +      - mobileye,eyeq6l-reset
> +      - mobileye,eyeq6h-we-reset
> +      - mobileye,eyeq6h-acc-reset
>  
> -  reg:
> -    maxItems: 3
> +  reg: true

Same mistakes. Please open existing bindings with multiple variants,
e.g. some Qualcomm, and take a look how it is done there.

Best regards,
Krzysztof

Re: [PATCH 03/11] dt-bindings: reset: mobileye,eyeq5-reset: add EyeQ6L and EyeQ6H

[Théo Lebrun]

Hello,

On Thu Apr 11, 2024 at 8:14 AM CEST, Krzysztof Kozlowski wrote:
> On 10/04/2024 19:12, Théo Lebrun wrote:
> > Add bindings for EyeQ6L and EyeQ6H reset controllers.
> > 
> > Some controllers host a single domain, meaning a single cell is enough.
> > We do not enforce reg-names for such nodes.
> > 
> > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> > ---
> >  .../bindings/reset/mobileye,eyeq5-reset.yaml       | 88 ++++++++++++++++++----
> >  MAINTAINERS                                        |  1 +
> >  2 files changed, 74 insertions(+), 15 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml b/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml
> > index 062b4518347b..799bcf15bed9 100644
> > --- a/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml
> > +++ b/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml
> > @@ -4,11 +4,13 @@
> >  $id: http://devicetree.org/schemas/reset/mobileye,eyeq5-reset.yaml#
> >  $schema: http://devicetree.org/meta-schemas/core.yaml#
> >  
> > -title: Mobileye EyeQ5 reset controller
> > +title: Mobileye EyeQ reset controller
> >  
> >  description:
> > -  The EyeQ5 reset driver handles three reset domains. Its registers live in a
> > -  shared region called OLB.
> > +  EyeQ reset controller handles one or more reset domains. They live in shared
> > +  regions called OLB. EyeQ5 and EyeQ6L host one OLB each, each with one reset
> > +  instance. EyeQ6H hosts 7 OLB regions; three of those (west, east,
> > +  accelerator) host reset controllers. West and east are duplicates.
> >  
> >  maintainers:
> >    - Grégory Clement <gregory.clement@bootlin.com>
> > @@ -17,27 +19,83 @@ maintainers:
> >  
> >  properties:
> >    compatible:
> > -    const: mobileye,eyeq5-reset
> > +    enum:
> > +      - mobileye,eyeq5-reset
> > +      - mobileye,eyeq6l-reset
> > +      - mobileye,eyeq6h-we-reset
> > +      - mobileye,eyeq6h-acc-reset
> >  
> > -  reg:
> > -    maxItems: 3
> > +  reg: true
>
> Same mistakes. Please open existing bindings with multiple variants,
> e.g. some Qualcomm, and take a look how it is done there.

Thanks for the pointer to good example, that is useful! So if we take
one random binding matching
Documentation/devicetree/bindings/clock/qcom,*.yaml and that contains
the "reg-names" string, we see:

  reg:
    items:
      - description: LPASS qdsp6ss register
      - description: LPASS top-cc register

  reg-names:
    items:
      - const: qdsp6ss
      - const: top_cc

I don't understand one thing; this doesn't tell you:

   You can provide 2 MMIO blocks, which must be qdsp6ss and top_cc.

But it tells you:

   Block zero must be qdsp6ss.
   Block one must be top_cc.

If we do that I do not get the point of reg-names; we put more
information in our devicetree that is in any case imposed.

This is why I went with a different approach looking like:

  reg:
    minItems: 2
    maxItems: 2
  reg-names:
    minItems: 2
    maxItems: 2
    items:
      enum: [ d0, d1 ]

I know this is not perfect, but at least you don't enforce an order for
no reason. If "items: const..." approach should be taken, then I'll
remove reg-names which bring no benefit.

Thanks Krzysztof,

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

Re: [PATCH 03/11] dt-bindings: reset: mobileye,eyeq5-reset: add EyeQ6L and EyeQ6H

[Krzysztof Kozlowski]

On 11/04/2024 16:04, Théo Lebrun wrote:
> Hello,
> 
> On Thu Apr 11, 2024 at 8:14 AM CEST, Krzysztof Kozlowski wrote:
>> On 10/04/2024 19:12, Théo Lebrun wrote:
>>> Add bindings for EyeQ6L and EyeQ6H reset controllers.
>>>
>>> Some controllers host a single domain, meaning a single cell is enough.
>>> We do not enforce reg-names for such nodes.
>>>
>>> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
>>> ---
>>>  .../bindings/reset/mobileye,eyeq5-reset.yaml       | 88 ++++++++++++++++++----
>>>  MAINTAINERS                                        |  1 +
>>>  2 files changed, 74 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml b/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml
>>> index 062b4518347b..799bcf15bed9 100644
>>> --- a/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml
>>> +++ b/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml
>>> @@ -4,11 +4,13 @@
>>>  $id: http://devicetree.org/schemas/reset/mobileye,eyeq5-reset.yaml#
>>>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>>>  
>>> -title: Mobileye EyeQ5 reset controller
>>> +title: Mobileye EyeQ reset controller
>>>  
>>>  description:
>>> -  The EyeQ5 reset driver handles three reset domains. Its registers live in a
>>> -  shared region called OLB.
>>> +  EyeQ reset controller handles one or more reset domains. They live in shared
>>> +  regions called OLB. EyeQ5 and EyeQ6L host one OLB each, each with one reset
>>> +  instance. EyeQ6H hosts 7 OLB regions; three of those (west, east,
>>> +  accelerator) host reset controllers. West and east are duplicates.
>>>  
>>>  maintainers:
>>>    - Grégory Clement <gregory.clement@bootlin.com>
>>> @@ -17,27 +19,83 @@ maintainers:
>>>  
>>>  properties:
>>>    compatible:
>>> -    const: mobileye,eyeq5-reset
>>> +    enum:
>>> +      - mobileye,eyeq5-reset
>>> +      - mobileye,eyeq6l-reset
>>> +      - mobileye,eyeq6h-we-reset
>>> +      - mobileye,eyeq6h-acc-reset
>>>  
>>> -  reg:
>>> -    maxItems: 3
>>> +  reg: true
>>
>> Same mistakes. Please open existing bindings with multiple variants,
>> e.g. some Qualcomm, and take a look how it is done there.
> 
> Thanks for the pointer to good example, that is useful! So if we take
> one random binding matching
> Documentation/devicetree/bindings/clock/qcom,*.yaml and that contains
> the "reg-names" string, we see:
> 
>   reg:
>     items:
>       - description: LPASS qdsp6ss register
>       - description: LPASS top-cc register
> 
>   reg-names:
>     items:
>       - const: qdsp6ss
>       - const: top_cc
> 
> I don't understand one thing; this doesn't tell you:
> 
>    You can provide 2 MMIO blocks, which must be qdsp6ss and top_cc.

No, it tells you exactly this, with difference: s/can/must/

> 
> But it tells you:
> 
>    Block zero must be qdsp6ss.
>    Block one must be top_cc.
> 
> If we do that I do not get the point of reg-names; we put more
> information in our devicetree that is in any case imposed.

Same old argument. Order is not flexible. Order is fixed.

Why do you need names? I don't need, it's purely your optional choice.
Maybe you find it more readable, up to you.


> 
> This is why I went with a different approach looking like:
> 
>   reg:
>     minItems: 2
>     maxItems: 2
>   reg-names:
>     minItems: 2
>     maxItems: 2
>     items:
>       enum: [ d0, d1 ]

No, order is fixed.

> 
> I know this is not perfect, but at least you don't enforce an order for
> no reason. If "items: const..." approach should be taken, then I'll
> remove reg-names which bring no benefit.

You can remove it, you can keep it, whatever makes code more readable,
but order is fixed.

Best regards,
Krzysztof

[PATCH 04/11] clk: divider: Introduce CLK_DIVIDER_EVEN_INTEGERS flag

[Théo Lebrun]

Add CLK_DIVIDER_EVEN_INTEGERS flag to support divisor of 2, 4, 6, etc.
The same divisor can be done using a table, which would be big and
wasteful for a clock dividor of width 8 (256 entries).

Require increasing flags size from u8 to u16 because
CLK_DIVIDER_EVEN_INTEGERS is the eighth flag.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/clk/clk-divider.c    | 12 +++++++++---
 include/linux/clk-provider.h | 11 +++++++----
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index a2c2b5203b0a..b6654c5c36d2 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -72,6 +72,8 @@ static unsigned int _get_maxdiv(const struct clk_div_table *table, u8 width,
 		return clk_div_mask(width);
 	if (flags & CLK_DIVIDER_POWER_OF_TWO)
 		return 1 << clk_div_mask(width);
+	if (flags & CLK_DIVIDER_EVEN_INTEGERS)
+		return 2 * (clk_div_mask(width) + 1);
 	if (table)
 		return _get_table_maxdiv(table, width);
 	return clk_div_mask(width) + 1;
@@ -97,6 +99,8 @@ static unsigned int _get_div(const struct clk_div_table *table,
 		return 1 << val;
 	if (flags & CLK_DIVIDER_MAX_AT_ZERO)
 		return val ? val : clk_div_mask(width) + 1;
+	if (flags & CLK_DIVIDER_EVEN_INTEGERS)
+		return 2 * (val + 1);
 	if (table)
 		return _get_table_div(table, val);
 	return val + 1;
@@ -122,6 +126,8 @@ static unsigned int _get_val(const struct clk_div_table *table,
 		return __ffs(div);
 	if (flags & CLK_DIVIDER_MAX_AT_ZERO)
 		return (div == clk_div_mask(width) + 1) ? 0 : div;
+	if (flags & CLK_DIVIDER_EVEN_INTEGERS)
+		return (div >> 1) - 1;
 	if (table)
 		return  _get_table_val(table, div);
 	return div - 1;
@@ -538,7 +544,7 @@ struct clk_hw *__clk_hw_register_divider(struct device *dev,
 		struct device_node *np, const char *name,
 		const char *parent_name, const struct clk_hw *parent_hw,
 		const struct clk_parent_data *parent_data, unsigned long flags,
-		void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags,
+		void __iomem *reg, u8 shift, u8 width, u16 clk_divider_flags,
 		const struct clk_div_table *table, spinlock_t *lock)
 {
 	struct clk_divider *div;
@@ -610,7 +616,7 @@ EXPORT_SYMBOL_GPL(__clk_hw_register_divider);
 struct clk *clk_register_divider_table(struct device *dev, const char *name,
 		const char *parent_name, unsigned long flags,
 		void __iomem *reg, u8 shift, u8 width,
-		u8 clk_divider_flags, const struct clk_div_table *table,
+		u16 clk_divider_flags, const struct clk_div_table *table,
 		spinlock_t *lock)
 {
 	struct clk_hw *hw;
@@ -664,7 +670,7 @@ struct clk_hw *__devm_clk_hw_register_divider(struct device *dev,
 		struct device_node *np, const char *name,
 		const char *parent_name, const struct clk_hw *parent_hw,
 		const struct clk_parent_data *parent_data, unsigned long flags,
-		void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags,
+		void __iomem *reg, u8 shift, u8 width, u16 clk_divider_flags,
 		const struct clk_div_table *table, spinlock_t *lock)
 {
 	struct clk_hw **ptr, *hw;
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 4a537260f655..cb348e502e41 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -675,13 +675,15 @@ struct clk_div_table {
  * CLK_DIVIDER_BIG_ENDIAN - By default little endian register accesses are used
  *	for the divider register.  Setting this flag makes the register accesses
  *	big endian.
+ * CLK_DIVIDER_EVEN_INTEGERS - clock divisor is 2, 4, 6, 8, 10, etc.
+ *	Formula is 2 * (value read from hardware + 1).
  */
 struct clk_divider {
 	struct clk_hw	hw;
 	void __iomem	*reg;
 	u8		shift;
 	u8		width;
-	u8		flags;
+	u16		flags;
 	const struct clk_div_table	*table;
 	spinlock_t	*lock;
 };
@@ -697,6 +699,7 @@ struct clk_divider {
 #define CLK_DIVIDER_READ_ONLY		BIT(5)
 #define CLK_DIVIDER_MAX_AT_ZERO		BIT(6)
 #define CLK_DIVIDER_BIG_ENDIAN		BIT(7)
+#define CLK_DIVIDER_EVEN_INTEGERS	BIT(8)
 
 extern const struct clk_ops clk_divider_ops;
 extern const struct clk_ops clk_divider_ro_ops;
@@ -726,18 +729,18 @@ struct clk_hw *__clk_hw_register_divider(struct device *dev,
 		struct device_node *np, const char *name,
 		const char *parent_name, const struct clk_hw *parent_hw,
 		const struct clk_parent_data *parent_data, unsigned long flags,
-		void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags,
+		void __iomem *reg, u8 shift, u8 width, u16 clk_divider_flags,
 		const struct clk_div_table *table, spinlock_t *lock);
 struct clk_hw *__devm_clk_hw_register_divider(struct device *dev,
 		struct device_node *np, const char *name,
 		const char *parent_name, const struct clk_hw *parent_hw,
 		const struct clk_parent_data *parent_data, unsigned long flags,
-		void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags,
+		void __iomem *reg, u8 shift, u8 width, u16 clk_divider_flags,
 		const struct clk_div_table *table, spinlock_t *lock);
 struct clk *clk_register_divider_table(struct device *dev, const char *name,
 		const char *parent_name, unsigned long flags,
 		void __iomem *reg, u8 shift, u8 width,
-		u8 clk_divider_flags, const struct clk_div_table *table,
+		u16 clk_divider_flags, const struct clk_div_table *table,
 		spinlock_t *lock);
 /**
  * clk_register_divider - register a divider clock with the clock framework

-- 
2.44.0

Re: [PATCH 04/11] clk: divider: Introduce CLK_DIVIDER_EVEN_INTEGERS flag

[Stephen Boyd]

Quoting Théo Lebrun (2024-04-10 10:12:33)
> index 4a537260f655..cb348e502e41 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -675,13 +675,15 @@ struct clk_div_table {
>   * CLK_DIVIDER_BIG_ENDIAN - By default little endian register accesses are used
>   *     for the divider register.  Setting this flag makes the register accesses
>   *     big endian.
> + * CLK_DIVIDER_EVEN_INTEGERS - clock divisor is 2, 4, 6, 8, 10, etc.
> + *     Formula is 2 * (value read from hardware + 1).
>   */
>  struct clk_divider {
>         struct clk_hw   hw;
>         void __iomem    *reg;
>         u8              shift;
>         u8              width;
> -       u8              flags;
> +       u16             flags;

This can stay u8

>         const struct clk_div_table      *table;
>         spinlock_t      *lock;
>  };

We should add a kunit test.

Re: [PATCH 04/11] clk: divider: Introduce CLK_DIVIDER_EVEN_INTEGERS flag

[Théo Lebrun]

Hello,

On Thu Apr 11, 2024 at 5:06 AM CEST, Stephen Boyd wrote:
> Quoting Théo Lebrun (2024-04-10 10:12:33)
> > index 4a537260f655..cb348e502e41 100644
> > --- a/include/linux/clk-provider.h
> > +++ b/include/linux/clk-provider.h
> > @@ -675,13 +675,15 @@ struct clk_div_table {
> >   * CLK_DIVIDER_BIG_ENDIAN - By default little endian register accesses are used
> >   *     for the divider register.  Setting this flag makes the register accesses
> >   *     big endian.
> > + * CLK_DIVIDER_EVEN_INTEGERS - clock divisor is 2, 4, 6, 8, 10, etc.
> > + *     Formula is 2 * (value read from hardware + 1).
> >   */
> >  struct clk_divider {
> >         struct clk_hw   hw;
> >         void __iomem    *reg;
> >         u8              shift;
> >         u8              width;
> > -       u8              flags;
> > +       u16             flags;
>
> This can stay u8

It is unclear to me why it can stay u8? __clk_hw_register_divider() puts
clk_divider_flags into flags field of struct clk_divider.
BIT(8) overflows u8.

>
> >         const struct clk_div_table      *table;
> >         spinlock_t      *lock;
> >  };
>
> We should add a kunit test.

Will look into how this works and try something for next revision. I
guess you are talking about adding clk_divider tests, not only tests
for this flag? I cannot find any existing kunit tests for clk_divider.

Thanks,

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

Re: [PATCH 04/11] clk: divider: Introduce CLK_DIVIDER_EVEN_INTEGERS flag

[Stephen Boyd]

Quoting Théo Lebrun (2024-04-11 03:14:09)
> Hello,
> 
> On Thu Apr 11, 2024 at 5:06 AM CEST, Stephen Boyd wrote:
> > Quoting Théo Lebrun (2024-04-10 10:12:33)
> > > index 4a537260f655..cb348e502e41 100644
> > > --- a/include/linux/clk-provider.h
> > > +++ b/include/linux/clk-provider.h
> > > @@ -675,13 +675,15 @@ struct clk_div_table {
> > >   * CLK_DIVIDER_BIG_ENDIAN - By default little endian register accesses are used
> > >   *     for the divider register.  Setting this flag makes the register accesses
> > >   *     big endian.
> > > + * CLK_DIVIDER_EVEN_INTEGERS - clock divisor is 2, 4, 6, 8, 10, etc.
> > > + *     Formula is 2 * (value read from hardware + 1).
> > >   */
> > >  struct clk_divider {
> > >         struct clk_hw   hw;
> > >         void __iomem    *reg;
> > >         u8              shift;
> > >         u8              width;
> > > -       u8              flags;
> > > +       u16             flags;
> >
> > This can stay u8
> 
> It is unclear to me why it can stay u8? __clk_hw_register_divider() puts
> clk_divider_flags into flags field of struct clk_divider.
> BIT(8) overflows u8.

Oh, I missed that part.

> 
> >
> > >         const struct clk_div_table      *table;
> > >         spinlock_t      *lock;
> > >  };
> >
> > We should add a kunit test.
> 
> Will look into how this works and try something for next revision. I
> guess you are talking about adding clk_divider tests, not only tests
> for this flag? I cannot find any existing kunit tests for clk_divider.
> 

Right, there aren't any tests today. Thanks.

[PATCH 05/11] clk: eyeq: add driver

[Théo Lebrun]

Add Mobileye EyeQ5, EyeQ6L and EyeQ6H clock controller driver. It is
both a platform driver and a hook onto of_clk_init() used for clocks
required early (GIC timer, UARTs).

For some compatible, it is both at the same time. eqc_init() initialises
early PLLs and stores clock array in a static linked list. It marks
other clocks as deferred. eqc_probe() retrieves the clock array and
adds all remaining clocks.

It exposes read-only PLLs derived from the main crystal on board. It
also exposes another type of clocks: divider clocks. They always have
even divisors and have one PLL as parent.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 MAINTAINERS            |   1 +
 drivers/clk/Kconfig    |  11 +
 drivers/clk/Makefile   |   1 +
 drivers/clk/clk-eyeq.c | 644 +++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 657 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 42553da10be9..33168ebf3cc5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14932,6 +14932,7 @@ F:	Documentation/devicetree/bindings/soc/mobileye/
 F:	arch/mips/boot/dts/mobileye/
 F:	arch/mips/configs/eyeq5_defconfig
 F:	arch/mips/mobileye/board-epm5.its.S
+F:	drivers/clk/clk-eyeq5.c
 F:	include/dt-bindings/clock/mobileye,eyeq5-clk.h
 
 MODULE SUPPORT
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 50af5fc7f570..1eb6e70977a3 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -218,6 +218,17 @@ config COMMON_CLK_EN7523
 	  This driver provides the fixed clocks and gates present on Airoha
 	  ARM silicon.
 
+config COMMON_CLK_EYEQ
+	bool "Clock driver for the Mobileye EyeQ platform"
+	depends on OF || COMPILE_TEST
+	depends on MACH_EYEQ5 || MACH_EYEQ6H || COMPILE_TEST
+	default MACH_EYEQ5 || MACH_EYEQ6H
+	help
+	  This driver provides clocks found on Mobileye EyeQ5, EyeQ6L and Eye6H
+	  SoCs. Controllers live in shared register regions called OLB. Driver
+	  provides read-only PLLs, derived from the main crystal clock (which
+	  must be constant). It also exposes some divider clocks.
+
 config COMMON_CLK_FSL_FLEXSPI
 	tristate "Clock driver for FlexSPI on Layerscape SoCs"
 	depends on ARCH_LAYERSCAPE || COMPILE_TEST
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 14fa8d4ecc1f..52de92309aa8 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_ARCH_CLPS711X)		+= clk-clps711x.o
 obj-$(CONFIG_COMMON_CLK_CS2000_CP)	+= clk-cs2000-cp.o
 obj-$(CONFIG_ARCH_SPARX5)		+= clk-sparx5.o
 obj-$(CONFIG_COMMON_CLK_EN7523)		+= clk-en7523.o
+obj-$(CONFIG_COMMON_CLK_EYEQ)		+= clk-eyeq.o
 obj-$(CONFIG_COMMON_CLK_FIXED_MMIO)	+= clk-fixed-mmio.o
 obj-$(CONFIG_COMMON_CLK_FSL_FLEXSPI)	+= clk-fsl-flexspi.o
 obj-$(CONFIG_COMMON_CLK_FSL_SAI)	+= clk-fsl-sai.o
diff --git a/drivers/clk/clk-eyeq.c b/drivers/clk/clk-eyeq.c
new file mode 100644
index 000000000000..bb2535010ae6
--- /dev/null
+++ b/drivers/clk/clk-eyeq.c
@@ -0,0 +1,644 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * PLL clock driver for the Mobileye EyeQ5, EyeQ6L and EyeQ6H platforms.
+ *
+ * This controller handles read-only PLLs, all derived from the same main
+ * crystal clock. It also exposes divider clocks, those are children to PLLs.
+ * Parent clock is expected to be constant. This driver's registers live in
+ * a shared region called OLB. Some PLLs are initialised early by of_clk_init().
+ *
+ * We use eqc_ as prefix, as-in "EyeQ Clock", but way shorter.
+ *
+ * Copyright (C) 2024 Mobileye Vision Technologies Ltd.
+ */
+
+#define pr_fmt(fmt) "clk-eyeq: " fmt
+
+#include <linux/array_size.h>
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/clk-provider.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/overflow.h>
+#include <linux/platform_device.h>
+#include <linux/printk.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+
+#include <dt-bindings/clock/mobileye,eyeq5-clk.h>
+
+#define EQC_MAX_DIV_COUNT		4
+
+/* In frac mode, it enables fractional noise canceling DAC. Else, no function. */
+#define PCSR0_DAC_EN			BIT(0)
+/* Fractional or integer mode */
+#define PCSR0_DSM_EN			BIT(1)
+#define PCSR0_PLL_EN			BIT(2)
+/* All clocks output held at 0 */
+#define PCSR0_FOUTPOSTDIV_EN		BIT(3)
+#define PCSR0_POST_DIV1			GENMASK(6, 4)
+#define PCSR0_POST_DIV2			GENMASK(9, 7)
+#define PCSR0_REF_DIV			GENMASK(15, 10)
+#define PCSR0_INTIN			GENMASK(27, 16)
+#define PCSR0_BYPASS			BIT(28)
+/* Bits 30..29 are reserved */
+#define PCSR0_PLL_LOCKED		BIT(31)
+
+#define PCSR1_RESET			BIT(0)
+#define PCSR1_SSGC_DIV			GENMASK(4, 1)
+/* Spread amplitude (% = 0.1 * SPREAD[4:0]) */
+#define PCSR1_SPREAD			GENMASK(9, 5)
+#define PCSR1_DIS_SSCG			BIT(10)
+/* Down-spread or center-spread */
+#define PCSR1_DOWN_SPREAD		BIT(11)
+#define PCSR1_FRAC_IN			GENMASK(31, 12)
+
+/*
+ * Driver might register clock provider from eqc_init() if PLLs are required
+ * early (before platform bus is ready). Store struct eqc_priv inside linked
+ * list to pass clock provider from eqc_init() to eqc_probe() and register
+ * remaining clocks from platform device probe.
+ *
+ * Clock provider is NOT created by eqc_init() if no early clock is required.
+ * Store as linked list because EyeQ6H has multiple clock controller instances.
+ * Matching is done based on devicetree node pointer.
+ */
+static DEFINE_SPINLOCK(eqc_list_slock);
+static LIST_HEAD(eqc_list);
+
+struct eqc_pll {
+	unsigned int	index;
+	const char	*name;
+	u32		reg64;
+};
+
+/*
+ * Divider clock. Divider is 2*(v+1), with v the register value.
+ * Min divider is 2, max is 2*(2^width).
+ */
+struct eqc_div {
+	unsigned int	index;
+	const char	*name;
+	unsigned int	parent;
+	const char	*resource_name;
+	u8		shift;
+	u8		width;
+};
+
+struct eqc_match_data {
+	unsigned int		early_pll_count;
+	const struct eqc_pll	*early_plls;
+
+	unsigned int		pll_count;
+	const struct eqc_pll	*plls;
+
+	unsigned int		div_count;
+	const struct eqc_div	*divs;
+};
+
+struct eqc_priv {
+	struct clk_hw_onecell_data	*cells;
+	const struct eqc_match_data	*data;
+	void __iomem			*base_plls;
+	struct device_node		*np;
+	struct list_head		list;
+};
+
+static int eqc_pll_parse_registers(u32 r0, u32 r1, unsigned long *mult,
+				   unsigned long *div, unsigned long *acc)
+{
+	if (r0 & PCSR0_BYPASS) {
+		*mult = 1;
+		*div = 1;
+		*acc = 0;
+		return 0;
+	}
+
+	if (!(r0 & PCSR0_PLL_LOCKED))
+		return -EINVAL;
+
+	*mult = FIELD_GET(PCSR0_INTIN, r0);
+	*div = FIELD_GET(PCSR0_REF_DIV, r0);
+	if (r0 & PCSR0_FOUTPOSTDIV_EN)
+		*div *= FIELD_GET(PCSR0_POST_DIV1, r0) * FIELD_GET(PCSR0_POST_DIV2, r0);
+
+	/* Fractional mode, in 2^20 (0x100000) parts. */
+	if (r0 & PCSR0_DSM_EN) {
+		*div *= 0x100000;
+		*mult = *mult * 0x100000 + FIELD_GET(PCSR1_FRAC_IN, r1);
+	}
+
+	if (!*mult || !*div)
+		return -EINVAL;
+
+	/* Spread spectrum. */
+	if (!(r1 & (PCSR1_RESET | PCSR1_DIS_SSCG))) {
+		/*
+		 * Spread is 1/1000 parts of frequency, accuracy is half of
+		 * that. To get accuracy, convert to ppb (parts per billion).
+		 */
+		u32 spread = FIELD_GET(PCSR1_SPREAD, r1);
+
+		*acc = spread * 500000;
+		if (r1 & PCSR1_DOWN_SPREAD) {
+			/*
+			 * Downspreading: the central frequency is half a
+			 * spread lower.
+			 */
+			*mult *= 2000 - spread;
+			*div *= 2000;
+		}
+	} else {
+		*acc = 0;
+	}
+
+	return 0;
+}
+
+static unsigned int eqc_compute_clock_count(const struct eqc_match_data *data)
+{
+	unsigned int i, nb_clks = 0;
+
+	for (i = 0; i < data->early_pll_count; i++)
+		if (data->early_plls[i].index >= nb_clks)
+			nb_clks = data->early_plls[i].index + 1;
+	for (i = 0; i < data->pll_count; i++)
+		if (data->plls[i].index >= nb_clks)
+			nb_clks = data->plls[i].index + 1;
+	for (i = 0; i < data->div_count; i++)
+		if (data->divs[i].index >= nb_clks)
+			nb_clks = data->divs[i].index + 1;
+
+	/* We expect the biggest clock index to be 1 below the clock count. */
+	WARN_ON(nb_clks != data->early_pll_count + data->pll_count + data->div_count);
+
+	return nb_clks;
+}
+
+static int eqc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	void __iomem *div_resources[EQC_MAX_DIV_COUNT];
+	struct device_node *np = dev->of_node;
+	const struct eqc_match_data *data;
+	struct eqc_priv *priv = NULL;
+	struct clk_hw *hw;
+	unsigned int i;
+
+	data = device_get_match_data(dev);
+	if (!data)
+		return -ENODEV;
+
+	if (data->early_pll_count) {
+		/* Device got inited early. Retrieve clock provider from list. */
+		struct eqc_priv *entry;
+
+		spin_lock(&eqc_list_slock);
+		list_for_each_entry(entry, &eqc_list, list) {
+			if (entry->np == np) {
+				priv = entry;
+				break;
+			}
+		}
+		spin_unlock(&eqc_list_slock);
+
+		if (!priv)
+			return -ENODEV;
+	} else {
+		/* Device did NOT get init early. Do it now. */
+		unsigned int nb_clks;
+
+		priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+		if (!priv)
+			return -ENOMEM;
+
+		priv->np = np;
+		priv->data = data;
+
+		nb_clks = eqc_compute_clock_count(data);
+		priv->cells = devm_kzalloc(dev, struct_size(priv->cells, hws, nb_clks),
+					   GFP_KERNEL);
+		if (!priv->cells)
+			return -ENOMEM;
+
+		priv->cells->num = nb_clks;
+
+		/*
+		 * We expect named resources if divider clocks are present.
+		 * Else, we only expect one resource.
+		 */
+		if (data->div_count)
+			priv->base_plls = devm_platform_ioremap_resource_byname(pdev, "plls");
+		else
+			priv->base_plls = devm_platform_ioremap_resource(pdev, 0);
+		if (IS_ERR(priv->base_plls))
+			return PTR_ERR(priv->base_plls);
+	}
+
+	for (i = 0; i < data->pll_count; i++) {
+		const struct eqc_pll *pll = &data->plls[i];
+		unsigned long mult, div, acc;
+		u32 r0, r1;
+		u64 val;
+		int ret;
+
+		val = readq(priv->base_plls + pll->reg64);
+		r0 = val;
+		r1 = val >> 32;
+
+		ret = eqc_pll_parse_registers(r0, r1, &mult, &div, &acc);
+		if (ret) {
+			dev_warn(dev, "failed parsing state of %s\n", pll->name);
+			priv->cells->hws[pll->index] = ERR_PTR(ret);
+			continue;
+		}
+
+		hw = clk_hw_register_fixed_factor_with_accuracy_fwname(dev,
+				dev->of_node, pll->name, "ref", 0, mult, div, acc);
+		priv->cells->hws[pll->index] = hw;
+		if (IS_ERR(hw))
+			dev_warn(dev, "failed registering %s: %pe\n", pll->name, hw);
+	}
+
+	BUG_ON(ARRAY_SIZE(div_resources) < data->div_count);
+
+	for (i = 0; i < data->div_count; i++) {
+		const struct eqc_div *div = &data->divs[i];
+		void __iomem *base = NULL;
+		struct clk_hw *parent;
+		unsigned int j;
+
+		/*
+		 * Multiple divider clocks can request the same resource. Store
+		 * resource pointers during probe(). For each divider clock,
+		 * check if previous clocks referenced the same resource name.
+		 *
+		 * See EQ6HC_SOUTH_DIV_OSPI_REF and EQ6HC_SOUTH_DIV_OSPI_SYS.
+		 */
+		for (j = 0; j < i; j++) {
+			if (strcmp(data->divs[j].resource_name, div->resource_name) == 0) {
+				base = div_resources[j];
+				break;
+			}
+		}
+
+		/* Resource is first encountered. */
+		if (!base) {
+			base = devm_platform_ioremap_resource_byname(pdev, div->resource_name);
+			if (IS_ERR(base)) {
+				dev_warn(dev, "failed to iomap resource for %s\n", div->name);
+				priv->cells->hws[div->index] = base;
+				continue;
+			}
+		}
+
+		div_resources[i] = base;
+
+		parent = priv->cells->hws[div->parent];
+		hw = clk_hw_register_divider_table_parent_hw(dev, div->name,
+				parent, 0, base, div->shift, div->width,
+				CLK_DIVIDER_EVEN_INTEGERS, NULL, NULL);
+		priv->cells->hws[div->index] = hw;
+		if (IS_ERR(hw))
+			dev_warn(dev, "failed registering %s: %pe\n",
+				 div->name, hw);
+	}
+
+	/* Clock provider has not been registered by eqc_init(). Do it now. */
+	if (data->early_pll_count == 0) {
+		/* When providing a single clock, require no cell. */
+		if (priv->cells->num == 1)
+			return devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get,
+							   priv->cells->hws);
+		else
+			return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
+							   priv->cells);
+	}
+
+	return 0;
+}
+
+/* Required early for GIC timer (pll-cpu) and UARTs (pll-per). */
+static const struct eqc_pll eqc_eyeq5_early_plls[] = {
+	{ .index = EQ5C_PLL_CPU, .name = "pll-cpu",  .reg64 = 0x00, },
+	{ .index = EQ5C_PLL_PER, .name = "pll-per",  .reg64 = 0x30, },
+};
+
+static const struct eqc_pll eqc_eyeq5_plls[] = {
+	{ .index = EQ5C_PLL_VMP,  .name = "pll-vmp",  .reg64 = 0x08, },
+	{ .index = EQ5C_PLL_PMA,  .name = "pll-pma",  .reg64 = 0x10, },
+	{ .index = EQ5C_PLL_VDI,  .name = "pll-vdi",  .reg64 = 0x18, },
+	{ .index = EQ5C_PLL_DDR0, .name = "pll-ddr0", .reg64 = 0x20, },
+	{ .index = EQ5C_PLL_PCI,  .name = "pll-pci",  .reg64 = 0x28, },
+	{ .index = EQ5C_PLL_PMAC, .name = "pll-pmac", .reg64 = 0x38, },
+	{ .index = EQ5C_PLL_MPC,  .name = "pll-mpc",  .reg64 = 0x40, },
+	{ .index = EQ5C_PLL_DDR1, .name = "pll-ddr1", .reg64 = 0x48, },
+};
+
+static const struct eqc_div eqc_eyeq5_divs[] = {
+	{
+		.index = EQ5C_DIV_OSPI,
+		.name = "div-ospi",
+		.parent = EQ5C_PLL_PER,
+		.resource_name = "ospi",
+		.shift = 0,
+		.width = 4,
+	},
+};
+
+static const struct eqc_match_data eqc_eyeq5_match_data = {
+	.early_pll_count	= ARRAY_SIZE(eqc_eyeq5_early_plls),
+	.early_plls		= eqc_eyeq5_early_plls,
+
+	.pll_count	= ARRAY_SIZE(eqc_eyeq5_plls),
+	.plls		= eqc_eyeq5_plls,
+
+	.div_count	= ARRAY_SIZE(eqc_eyeq5_divs),
+	.divs		= eqc_eyeq5_divs,
+};
+
+static const struct eqc_pll eqc_eyeq6l_plls[] = {
+	{ .index = EQ6LC_PLL_DDR, .name = "pll-ddr", .reg64 = 0x2C },
+	{ .index = EQ6LC_PLL_CPU, .name = "pll-cpu", .reg64 = 0x34 }, /* also acc */
+	{ .index = EQ6LC_PLL_PER, .name = "pll-per", .reg64 = 0x3C },
+	{ .index = EQ6LC_PLL_VDI, .name = "pll-vdi", .reg64 = 0x44 },
+};
+
+static const struct eqc_match_data eqc_eyeq6l_match_data = {
+	.pll_count	= ARRAY_SIZE(eqc_eyeq6l_plls),
+	.plls		= eqc_eyeq6l_plls,
+};
+
+/* Required early for GIC timer. */
+static const struct eqc_pll eqc_eyeq6h_central_early_plls[] = {
+	{ .index = 0, .name = "pll-cpu", .reg64 = 0x00 },
+};
+
+static const struct eqc_match_data eqc_eyeq6h_central_match_data = {
+	.early_pll_count	= ARRAY_SIZE(eqc_eyeq6h_central_early_plls),
+	.early_plls		= eqc_eyeq6h_central_early_plls,
+};
+
+/* Required early for UART. */
+static const struct eqc_pll eqc_eyeq6h_west_early_plls[] = {
+	{ .index = 0, .name = "pll-west", .reg64 = 0x00 },
+};
+
+static const struct eqc_match_data eqc_eyeq6h_west_match_data = {
+	.early_pll_count	= ARRAY_SIZE(eqc_eyeq6h_west_early_plls),
+	.early_plls		= eqc_eyeq6h_west_early_plls,
+};
+
+static const struct eqc_pll eqc_eyeq6h_east_plls[] = {
+	{ .index = 0, .name = "pll-east", .reg64 = 0x00 },
+};
+
+static const struct eqc_match_data eqc_eyeq6h_east_match_data = {
+	.pll_count	= ARRAY_SIZE(eqc_eyeq6h_east_plls),
+	.plls		= eqc_eyeq6h_east_plls,
+};
+
+static const struct eqc_pll eqc_eyeq6h_south_plls[] = {
+	{ .index = EQ6HC_SOUTH_PLL_VDI,  .name = "pll-vdi",  .reg64 = 0x00 },
+	{ .index = EQ6HC_SOUTH_PLL_PCIE, .name = "pll-pcie", .reg64 = 0x08 },
+	{ .index = EQ6HC_SOUTH_PLL_PER,  .name = "pll-per",  .reg64 = 0x10 },
+	{ .index = EQ6HC_SOUTH_PLL_ISP,  .name = "pll-isp",  .reg64 = 0x18 },
+};
+
+static const struct eqc_div eqc_eyeq6h_south_divs[] = {
+	{
+		.index = EQ6HC_SOUTH_DIV_EMMC,
+		.name = "div-emmc",
+		.parent = EQ6HC_SOUTH_PLL_PER,
+		.resource_name = "emmc",
+		.shift = 4,
+		.width = 4,
+	},
+	{
+		.index = EQ6HC_SOUTH_DIV_OSPI_REF,
+		.name = "div-ospi-ref",
+		.parent = EQ6HC_SOUTH_PLL_PER,
+		.resource_name = "ospi",
+		.shift = 4,
+		.width = 4,
+	},
+	{
+		.index = EQ6HC_SOUTH_DIV_OSPI_SYS,
+		.name = "div-ospi-sys",
+		.parent = EQ6HC_SOUTH_PLL_PER,
+		.resource_name = "ospi",
+		.shift = 8,
+		.width = 1,
+	},
+	{
+		.index = EQ6HC_SOUTH_DIV_TSU,
+		.name = "div-tsu",
+		.parent = EQ6HC_SOUTH_PLL_PCIE,
+		.resource_name = "tsu",
+		.shift = 4,
+		.width = 8,
+	},
+};
+
+static const struct eqc_match_data eqc_eyeq6h_south_match_data = {
+	.pll_count	= ARRAY_SIZE(eqc_eyeq6h_south_plls),
+	.plls		= eqc_eyeq6h_south_plls,
+
+	.div_count	= ARRAY_SIZE(eqc_eyeq6h_south_divs),
+	.divs		= eqc_eyeq6h_south_divs,
+};
+
+static const struct eqc_pll eqc_eyeq6h_ddr0_plls[] = {
+	{ .index = 0, .name = "pll-ddr0", .reg64 = 0x00 },
+};
+
+static const struct eqc_match_data eqc_eyeq6h_ddr0_match_data = {
+	.pll_count	= ARRAY_SIZE(eqc_eyeq6h_ddr0_plls),
+	.plls		= eqc_eyeq6h_ddr0_plls,
+};
+
+static const struct eqc_pll eqc_eyeq6h_ddr1_plls[] = {
+	{ .index = 0, .name = "pll-ddr1", .reg64 = 0x00 },
+};
+
+static const struct eqc_match_data eqc_eyeq6h_ddr1_match_data = {
+	.pll_count	= ARRAY_SIZE(eqc_eyeq6h_ddr1_plls),
+	.plls		= eqc_eyeq6h_ddr1_plls,
+};
+
+static const struct eqc_pll eqc_eyeq6h_acc_plls[] = {
+	{ .index = EQ6HC_ACC_PLL_XNN, .name = "pll-xnn", .reg64 = 0x00 },
+	{ .index = EQ6HC_ACC_PLL_VMP, .name = "pll-vmp", .reg64 = 0x10 },
+	{ .index = EQ6HC_ACC_PLL_PMA, .name = "pll-pma", .reg64 = 0x1C },
+	{ .index = EQ6HC_ACC_PLL_MPC, .name = "pll-mpc", .reg64 = 0x28 },
+	{ .index = EQ6HC_ACC_PLL_NOC, .name = "pll-noc", .reg64 = 0x30 },
+};
+
+static const struct eqc_match_data eqc_eyeq6h_acc_match_data = {
+	.pll_count	= ARRAY_SIZE(eqc_eyeq6h_acc_plls),
+	.plls		= eqc_eyeq6h_acc_plls,
+};
+
+static const struct of_device_id eqc_match_table[] = {
+	{ .compatible = "mobileye,eyeq5-clk", .data = &eqc_eyeq5_match_data },
+	{ .compatible = "mobileye,eyeq6l-clk", .data = &eqc_eyeq6l_match_data },
+	{ .compatible = "mobileye,eyeq6h-central-clk", .data = &eqc_eyeq6h_central_match_data },
+	{ .compatible = "mobileye,eyeq6h-west-clk", .data = &eqc_eyeq6h_west_match_data },
+	{ .compatible = "mobileye,eyeq6h-east-clk", .data = &eqc_eyeq6h_east_match_data },
+	{ .compatible = "mobileye,eyeq6h-south-clk", .data = &eqc_eyeq6h_south_match_data },
+	{ .compatible = "mobileye,eyeq6h-ddr0-clk", .data = &eqc_eyeq6h_ddr0_match_data },
+	{ .compatible = "mobileye,eyeq6h-ddr1-clk", .data = &eqc_eyeq6h_ddr1_match_data },
+	{ .compatible = "mobileye,eyeq6h-acc-clk", .data = &eqc_eyeq6h_acc_match_data },
+	{}
+};
+MODULE_DEVICE_TABLE(of, eqc_match_table);
+
+static struct platform_driver eqc_driver = {
+	.probe = eqc_probe,
+	.driver = {
+		.name = "clk-eyeq",
+		.of_match_table = eqc_match_table,
+	},
+};
+builtin_platform_driver(eqc_driver);
+
+static void __init eqc_init(struct device_node *np)
+{
+	const struct eqc_match_data *data;
+	unsigned int nb_clks = 0;
+	struct eqc_priv *priv;
+	unsigned int i;
+	int ret;
+
+	data = of_match_node(eqc_match_table, np)->data;
+
+	/* No reason to early init this clock provider. Do it at probe. */
+	if (data->early_pll_count == 0)
+		return;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	priv->np = np;
+	priv->data = data;
+
+	nb_clks = eqc_compute_clock_count(data);
+	priv->cells = kzalloc(struct_size(priv->cells, hws, nb_clks), GFP_KERNEL);
+	if (!priv->cells) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	priv->cells->num = nb_clks;
+
+	/*
+	 * Mark non-early clocks as deferred; they'll be registered at platform
+	 * device probe.
+	 */
+	for (i = 0; i < data->pll_count; i++)
+		priv->cells->hws[data->plls[i].index] = ERR_PTR(-EPROBE_DEFER);
+	for (i = 0; i < data->div_count; i++)
+		priv->cells->hws[data->divs[i].index] = ERR_PTR(-EPROBE_DEFER);
+
+	/*
+	 * We expect named resources if divider clocks are present.
+	 * Else, we only expect one resource.
+	 */
+	if (data->div_count)
+		ret = of_property_match_string(np, "reg-names", "plls");
+	else
+		ret = 0;
+	if (ret < 0)
+		goto err;
+
+	priv->base_plls = of_iomap(np, ret);
+	if (!priv->base_plls) {
+		ret = -ENODEV;
+		goto err;
+	}
+
+	for (i = 0; i < data->early_pll_count; i++) {
+		const struct eqc_pll *pll = &data->early_plls[i];
+		unsigned long mult, div, acc;
+		struct clk_hw *hw;
+		u32 r0, r1;
+		u64 val;
+
+		val = readq(priv->base_plls + pll->reg64);
+		r0 = val;
+		r1 = val >> 32;
+
+		ret = eqc_pll_parse_registers(r0, r1, &mult, &div, &acc);
+		if (ret) {
+			pr_err("failed parsing state of %s\n", pll->name);
+			goto err;
+		}
+
+		hw = clk_hw_register_fixed_factor_with_accuracy_fwname(NULL,
+				np, pll->name, "ref", 0, mult, div, acc);
+		priv->cells->hws[pll->index] = hw;
+		if (IS_ERR(hw)) {
+			pr_err("failed registering %s: %pe\n", pll->name, hw);
+			ret = PTR_ERR(hw);
+			goto err;
+		}
+	}
+
+	/* When providing a single clock, require no cell. */
+	if (nb_clks == 1)
+		ret = of_clk_add_hw_provider(np, of_clk_hw_simple_get, priv->cells->hws);
+	else
+		ret = of_clk_add_hw_provider(np, of_clk_hw_onecell_get, priv->cells);
+	if (ret) {
+		pr_err("failed registering clk provider: %d\n", ret);
+		goto err;
+	}
+
+	spin_lock(&eqc_list_slock);
+	list_add_tail(&priv->list, &eqc_list);
+	spin_unlock(&eqc_list_slock);
+
+	return;
+
+err:
+	/*
+	 * We are doomed. The system will not be able to boot.
+	 *
+	 * Let's still try to be good citizens by freeing resources and print
+	 * a last error message that might help debugging.
+	 */
+
+	if (priv && priv->cells) {
+		of_clk_del_provider(np);
+
+		for (i = 0; i < data->early_pll_count; i++) {
+			const struct eqc_pll *pll = &data->early_plls[i];
+			struct clk_hw *hw = priv->cells->hws[pll->index];
+
+			if (!IS_ERR_OR_NULL(hw))
+				clk_hw_unregister_fixed_factor(hw);
+		}
+
+		kfree(priv->cells);
+	}
+
+	kfree(priv);
+
+	pr_err("failed clk init: %d\n", ret);
+}
+
+CLK_OF_DECLARE_DRIVER(eqc_eyeq5, "mobileye,eyeq5-clk", eqc_init);
+CLK_OF_DECLARE_DRIVER(eqc_eyeq6h_central, "mobileye,eyeq6h-central-clk", eqc_init);
+CLK_OF_DECLARE_DRIVER(eqc_eyeq6h_west, "mobileye,eyeq6h-west-clk", eqc_init);

-- 
2.44.0

Re: [PATCH 05/11] clk: eyeq: add driver

[Stephen Boyd]

Quoting Théo Lebrun (2024-04-10 10:12:34)
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 50af5fc7f570..1eb6e70977a3 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -218,6 +218,17 @@ config COMMON_CLK_EN7523
>           This driver provides the fixed clocks and gates present on Airoha
>           ARM silicon.
>  
> +config COMMON_CLK_EYEQ
> +       bool "Clock driver for the Mobileye EyeQ platform"
> +       depends on OF || COMPILE_TEST

The OF build dependency looks useless as we have the MACH_ dependency
below.

> +       depends on MACH_EYEQ5 || MACH_EYEQ6H || COMPILE_TEST
> +       default MACH_EYEQ5 || MACH_EYEQ6H
> +       help
> +         This driver provides clocks found on Mobileye EyeQ5, EyeQ6L and Eye6H
> +         SoCs. Controllers live in shared register regions called OLB. Driver
> +         provides read-only PLLs, derived from the main crystal clock (which
> +         must be constant). It also exposes some divider clocks.
> +
>  config COMMON_CLK_FSL_FLEXSPI
>         tristate "Clock driver for FlexSPI on Layerscape SoCs"
>         depends on ARCH_LAYERSCAPE || COMPILE_TEST
> diff --git a/drivers/clk/clk-eyeq.c b/drivers/clk/clk-eyeq.c
> new file mode 100644
> index 000000000000..bb2535010ae6
> --- /dev/null
> +++ b/drivers/clk/clk-eyeq.c
> @@ -0,0 +1,644 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * PLL clock driver for the Mobileye EyeQ5, EyeQ6L and EyeQ6H platforms.
> + *
> + * This controller handles read-only PLLs, all derived from the same main
> + * crystal clock. It also exposes divider clocks, those are children to PLLs.
> + * Parent clock is expected to be constant. This driver's registers live in
> + * a shared region called OLB. Some PLLs are initialised early by of_clk_init().

Is OLB a different DT node? It sounds like maybe this is trying to jam a
driver into DT when the OLB node should be a #clock-cells node.

> + *
> + * We use eqc_ as prefix, as-in "EyeQ Clock", but way shorter.
> + *
> + * Copyright (C) 2024 Mobileye Vision Technologies Ltd.
> + */
> +
> +#define pr_fmt(fmt) "clk-eyeq: " fmt
> +
> +#include <linux/array_size.h>
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/clk-provider.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/list.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/overflow.h>
> +#include <linux/platform_device.h>
> +#include <linux/printk.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +#include <linux/types.h>
> +
> +#include <dt-bindings/clock/mobileye,eyeq5-clk.h>
> +
> +#define EQC_MAX_DIV_COUNT              4
> +
> +/* In frac mode, it enables fractional noise canceling DAC. Else, no function. */
> +#define PCSR0_DAC_EN                   BIT(0)
> +/* Fractional or integer mode */
> +#define PCSR0_DSM_EN                   BIT(1)
> +#define PCSR0_PLL_EN                   BIT(2)
> +/* All clocks output held at 0 */
> +#define PCSR0_FOUTPOSTDIV_EN           BIT(3)
> +#define PCSR0_POST_DIV1                        GENMASK(6, 4)
> +#define PCSR0_POST_DIV2                        GENMASK(9, 7)
> +#define PCSR0_REF_DIV                  GENMASK(15, 10)
> +#define PCSR0_INTIN                    GENMASK(27, 16)
> +#define PCSR0_BYPASS                   BIT(28)
> +/* Bits 30..29 are reserved */
> +#define PCSR0_PLL_LOCKED               BIT(31)
> +
> +#define PCSR1_RESET                    BIT(0)
> +#define PCSR1_SSGC_DIV                 GENMASK(4, 1)
> +/* Spread amplitude (% = 0.1 * SPREAD[4:0]) */
> +#define PCSR1_SPREAD                   GENMASK(9, 5)
> +#define PCSR1_DIS_SSCG                 BIT(10)
> +/* Down-spread or center-spread */
> +#define PCSR1_DOWN_SPREAD              BIT(11)
> +#define PCSR1_FRAC_IN                  GENMASK(31, 12)
> +
> +/*
> + * Driver might register clock provider from eqc_init() if PLLs are required
> + * early (before platform bus is ready). Store struct eqc_priv inside linked
> + * list to pass clock provider from eqc_init() to eqc_probe() and register
> + * remaining clocks from platform device probe.
> + *
> + * Clock provider is NOT created by eqc_init() if no early clock is required.
> + * Store as linked list because EyeQ6H has multiple clock controller instances.
> + * Matching is done based on devicetree node pointer.
> + */
> +static DEFINE_SPINLOCK(eqc_list_slock);
> +static LIST_HEAD(eqc_list);
> +
> +struct eqc_pll {
> +       unsigned int    index;
> +       const char      *name;
> +       u32             reg64;
> +};
> +
> +/*
> + * Divider clock. Divider is 2*(v+1), with v the register value.
> + * Min divider is 2, max is 2*(2^width).
> + */
> +struct eqc_div {
> +       unsigned int    index;
> +       const char      *name;
> +       unsigned int    parent;
> +       const char      *resource_name;
> +       u8              shift;
> +       u8              width;
> +};
> +
> +struct eqc_match_data {
> +       unsigned int            early_pll_count;
> +       const struct eqc_pll    *early_plls;
> +
> +       unsigned int            pll_count;
> +       const struct eqc_pll    *plls;
> +
> +       unsigned int            div_count;
> +       const struct eqc_div    *divs;
> +};
> +
> +struct eqc_priv {
> +       struct clk_hw_onecell_data      *cells;
> +       const struct eqc_match_data     *data;
> +       void __iomem                    *base_plls;
> +       struct device_node              *np;
> +       struct list_head                list;
> +};
> +
> +static int eqc_pll_parse_registers(u32 r0, u32 r1, unsigned long *mult,
> +                                  unsigned long *div, unsigned long *acc)
> +{
> +       if (r0 & PCSR0_BYPASS) {
> +               *mult = 1;
> +               *div = 1;
> +               *acc = 0;
> +               return 0;
> +       }
> +
> +       if (!(r0 & PCSR0_PLL_LOCKED))
> +               return -EINVAL;
> +
> +       *mult = FIELD_GET(PCSR0_INTIN, r0);
> +       *div = FIELD_GET(PCSR0_REF_DIV, r0);
> +       if (r0 & PCSR0_FOUTPOSTDIV_EN)
> +               *div *= FIELD_GET(PCSR0_POST_DIV1, r0) * FIELD_GET(PCSR0_POST_DIV2, r0);
> +
> +       /* Fractional mode, in 2^20 (0x100000) parts. */
> +       if (r0 & PCSR0_DSM_EN) {
> +               *div *= 0x100000;
> +               *mult = *mult * 0x100000 + FIELD_GET(PCSR1_FRAC_IN, r1);
> +       }
> +
> +       if (!*mult || !*div)
> +               return -EINVAL;
> +
> +       /* Spread spectrum. */
> +       if (!(r1 & (PCSR1_RESET | PCSR1_DIS_SSCG))) {
> +               /*
> +                * Spread is 1/1000 parts of frequency, accuracy is half of
> +                * that. To get accuracy, convert to ppb (parts per billion).
> +                */
> +               u32 spread = FIELD_GET(PCSR1_SPREAD, r1);
> +
> +               *acc = spread * 500000;
> +               if (r1 & PCSR1_DOWN_SPREAD) {
> +                       /*
> +                        * Downspreading: the central frequency is half a
> +                        * spread lower.
> +                        */
> +                       *mult *= 2000 - spread;
> +                       *div *= 2000;
> +               }
> +       } else {
> +               *acc = 0;
> +       }
> +
> +       return 0;
> +}
> +
> +static unsigned int eqc_compute_clock_count(const struct eqc_match_data *data)
> +{
> +       unsigned int i, nb_clks = 0;
> +
> +       for (i = 0; i < data->early_pll_count; i++)
> +               if (data->early_plls[i].index >= nb_clks)
> +                       nb_clks = data->early_plls[i].index + 1;
> +       for (i = 0; i < data->pll_count; i++)
> +               if (data->plls[i].index >= nb_clks)
> +                       nb_clks = data->plls[i].index + 1;
> +       for (i = 0; i < data->div_count; i++)
> +               if (data->divs[i].index >= nb_clks)
> +                       nb_clks = data->divs[i].index + 1;
> +
> +       /* We expect the biggest clock index to be 1 below the clock count. */
> +       WARN_ON(nb_clks != data->early_pll_count + data->pll_count + data->div_count);
> +
> +       return nb_clks;
> +}
> +
> +static int eqc_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       void __iomem *div_resources[EQC_MAX_DIV_COUNT];
> +       struct device_node *np = dev->of_node;
> +       const struct eqc_match_data *data;
> +       struct eqc_priv *priv = NULL;
> +       struct clk_hw *hw;
> +       unsigned int i;
> +
> +       data = device_get_match_data(dev);
> +       if (!data)
> +               return -ENODEV;
> +
> +       if (data->early_pll_count) {
> +               /* Device got inited early. Retrieve clock provider from list. */
> +               struct eqc_priv *entry;
> +
> +               spin_lock(&eqc_list_slock);
> +               list_for_each_entry(entry, &eqc_list, list) {
> +                       if (entry->np == np) {
> +                               priv = entry;
> +                               break;
> +                       }
> +               }
> +               spin_unlock(&eqc_list_slock);
> +
> +               if (!priv)
> +                       return -ENODEV;

This can be a sub-function.

> +       } else {
> +               /* Device did NOT get init early. Do it now. */
> +               unsigned int nb_clks;
> +
> +               priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +               if (!priv)
> +                       return -ENOMEM;
> +
> +               priv->np = np;
> +               priv->data = data;
> +
> +               nb_clks = eqc_compute_clock_count(data);
> +               priv->cells = devm_kzalloc(dev, struct_size(priv->cells, hws, nb_clks),
> +                                          GFP_KERNEL);
> +               if (!priv->cells)
> +                       return -ENOMEM;
> +
> +               priv->cells->num = nb_clks;
> +
> +               /*
> +                * We expect named resources if divider clocks are present.
> +                * Else, we only expect one resource.
> +                */
> +               if (data->div_count)
> +                       priv->base_plls = devm_platform_ioremap_resource_byname(pdev, "plls");
> +               else
> +                       priv->base_plls = devm_platform_ioremap_resource(pdev, 0);
> +               if (IS_ERR(priv->base_plls))
> +                       return PTR_ERR(priv->base_plls);
> +       }
> +
> +       for (i = 0; i < data->pll_count; i++) {
> +               const struct eqc_pll *pll = &data->plls[i];
> +               unsigned long mult, div, acc;
> +               u32 r0, r1;
> +               u64 val;
> +               int ret;

All variables should be declared at the start of the function. Once it
becomes "too heavy" you can split it up into smaller functions, that
again have all variables declared at the start of the function.

> +
> +               val = readq(priv->base_plls + pll->reg64);
> +               r0 = val;
> +               r1 = val >> 32;
> +
> +               ret = eqc_pll_parse_registers(r0, r1, &mult, &div, &acc);
> +               if (ret) {
> +                       dev_warn(dev, "failed parsing state of %s\n", pll->name);
> +                       priv->cells->hws[pll->index] = ERR_PTR(ret);
> +                       continue;
> +               }
> +
> +               hw = clk_hw_register_fixed_factor_with_accuracy_fwname(dev,
> +                               dev->of_node, pll->name, "ref", 0, mult, div, acc);
> +               priv->cells->hws[pll->index] = hw;
> +               if (IS_ERR(hw))
> +                       dev_warn(dev, "failed registering %s: %pe\n", pll->name, hw);
> +       }
> +
> +       BUG_ON(ARRAY_SIZE(div_resources) < data->div_count);

Can this be a static assert instead on the arrays these are based on?
Put some static_assert() near the match data macros.

> +
> +       for (i = 0; i < data->div_count; i++) {
> +               const struct eqc_div *div = &data->divs[i];
> +               void __iomem *base = NULL;
> +               struct clk_hw *parent;
> +               unsigned int j;
> +
> +               /*
> +                * Multiple divider clocks can request the same resource. Store
> +                * resource pointers during probe(). For each divider clock,
> +                * check if previous clocks referenced the same resource name.
> +                *
> +                * See EQ6HC_SOUTH_DIV_OSPI_REF and EQ6HC_SOUTH_DIV_OSPI_SYS.
> +                */
> +               for (j = 0; j < i; j++) {
> +                       if (strcmp(data->divs[j].resource_name, div->resource_name) == 0) {
> +                               base = div_resources[j];
> +                               break;
> +                       }
> +               }
> +
> +               /* Resource is first encountered. */
> +               if (!base) {
> +                       base = devm_platform_ioremap_resource_byname(pdev, div->resource_name);
> +                       if (IS_ERR(base)) {
> +                               dev_warn(dev, "failed to iomap resource for %s\n", div->name);
> +                               priv->cells->hws[div->index] = base;
> +                               continue;
> +                       }
> +               }

I don't get this code at all. The driver should simply map the
resources because it knows that there's an io resource. I'll look at the
binding which is probably wrong and causing the driver to be written
this way.

> +
> +               div_resources[i] = base;
> +
> +               parent = priv->cells->hws[div->parent];
> +               hw = clk_hw_register_divider_table_parent_hw(dev, div->name,
> +                               parent, 0, base, div->shift, div->width,
> +                               CLK_DIVIDER_EVEN_INTEGERS, NULL, NULL);
> +               priv->cells->hws[div->index] = hw;
> +               if (IS_ERR(hw))
> +                       dev_warn(dev, "failed registering %s: %pe\n",
> +                                div->name, hw);
> +       }
> +
> +       /* Clock provider has not been registered by eqc_init(). Do it now. */
> +       if (data->early_pll_count == 0) {
> +               /* When providing a single clock, require no cell. */
> +               if (priv->cells->num == 1)
> +                       return devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get,
> +                                                          priv->cells->hws);
> +               else
> +                       return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
> +                                                          priv->cells);
> +       }
> +
> +       return 0;
> +}
> +
> +/* Required early for GIC timer (pll-cpu) and UARTs (pll-per). */
> +static const struct eqc_pll eqc_eyeq5_early_plls[] = {
> +       { .index = EQ5C_PLL_CPU, .name = "pll-cpu",  .reg64 = 0x00, },
> +       { .index = EQ5C_PLL_PER, .name = "pll-per",  .reg64 = 0x30, },
> +};
> +
> +static const struct eqc_pll eqc_eyeq5_plls[] = {
> +       { .index = EQ5C_PLL_VMP,  .name = "pll-vmp",  .reg64 = 0x08, },
> +       { .index = EQ5C_PLL_PMA,  .name = "pll-pma",  .reg64 = 0x10, },
> +       { .index = EQ5C_PLL_VDI,  .name = "pll-vdi",  .reg64 = 0x18, },
> +       { .index = EQ5C_PLL_DDR0, .name = "pll-ddr0", .reg64 = 0x20, },
> +       { .index = EQ5C_PLL_PCI,  .name = "pll-pci",  .reg64 = 0x28, },
> +       { .index = EQ5C_PLL_PMAC, .name = "pll-pmac", .reg64 = 0x38, },
> +       { .index = EQ5C_PLL_MPC,  .name = "pll-mpc",  .reg64 = 0x40, },
> +       { .index = EQ5C_PLL_DDR1, .name = "pll-ddr1", .reg64 = 0x48, },
> +};
> +
> +static const struct eqc_div eqc_eyeq5_divs[] = {
> +       {
> +               .index = EQ5C_DIV_OSPI,
> +               .name = "div-ospi",
> +               .parent = EQ5C_PLL_PER,
> +               .resource_name = "ospi",
> +               .shift = 0,
> +               .width = 4,
> +       },
> +};
> +
> +static const struct eqc_match_data eqc_eyeq5_match_data = {
> +       .early_pll_count        = ARRAY_SIZE(eqc_eyeq5_early_plls),
> +       .early_plls             = eqc_eyeq5_early_plls,
> +
> +       .pll_count      = ARRAY_SIZE(eqc_eyeq5_plls),
> +       .plls           = eqc_eyeq5_plls,
> +
> +       .div_count      = ARRAY_SIZE(eqc_eyeq5_divs),
> +       .divs           = eqc_eyeq5_divs,
> +};
> +
> +static const struct eqc_pll eqc_eyeq6l_plls[] = {
> +       { .index = EQ6LC_PLL_DDR, .name = "pll-ddr", .reg64 = 0x2C },
> +       { .index = EQ6LC_PLL_CPU, .name = "pll-cpu", .reg64 = 0x34 }, /* also acc */
> +       { .index = EQ6LC_PLL_PER, .name = "pll-per", .reg64 = 0x3C },
> +       { .index = EQ6LC_PLL_VDI, .name = "pll-vdi", .reg64 = 0x44 },
> +};
> +
> +static const struct eqc_match_data eqc_eyeq6l_match_data = {
> +       .pll_count      = ARRAY_SIZE(eqc_eyeq6l_plls),
> +       .plls           = eqc_eyeq6l_plls,
> +};
> +
> +/* Required early for GIC timer. */
> +static const struct eqc_pll eqc_eyeq6h_central_early_plls[] = {
> +       { .index = 0, .name = "pll-cpu", .reg64 = 0x00 },
> +};
> +
> +static const struct eqc_match_data eqc_eyeq6h_central_match_data = {
> +       .early_pll_count        = ARRAY_SIZE(eqc_eyeq6h_central_early_plls),
> +       .early_plls             = eqc_eyeq6h_central_early_plls,
> +};
> +
> +/* Required early for UART. */
> +static const struct eqc_pll eqc_eyeq6h_west_early_plls[] = {
> +       { .index = 0, .name = "pll-west", .reg64 = 0x00 },
> +};
> +
> +static const struct eqc_match_data eqc_eyeq6h_west_match_data = {
> +       .early_pll_count        = ARRAY_SIZE(eqc_eyeq6h_west_early_plls),
> +       .early_plls             = eqc_eyeq6h_west_early_plls,
> +};
> +
> +static const struct eqc_pll eqc_eyeq6h_east_plls[] = {
> +       { .index = 0, .name = "pll-east", .reg64 = 0x00 },
> +};
> +
> +static const struct eqc_match_data eqc_eyeq6h_east_match_data = {
> +       .pll_count      = ARRAY_SIZE(eqc_eyeq6h_east_plls),
> +       .plls           = eqc_eyeq6h_east_plls,
> +};
> +
> +static const struct eqc_pll eqc_eyeq6h_south_plls[] = {
> +       { .index = EQ6HC_SOUTH_PLL_VDI,  .name = "pll-vdi",  .reg64 = 0x00 },
> +       { .index = EQ6HC_SOUTH_PLL_PCIE, .name = "pll-pcie", .reg64 = 0x08 },
> +       { .index = EQ6HC_SOUTH_PLL_PER,  .name = "pll-per",  .reg64 = 0x10 },
> +       { .index = EQ6HC_SOUTH_PLL_ISP,  .name = "pll-isp",  .reg64 = 0x18 },
> +};
> +
> +static const struct eqc_div eqc_eyeq6h_south_divs[] = {
> +       {
> +               .index = EQ6HC_SOUTH_DIV_EMMC,
> +               .name = "div-emmc",
> +               .parent = EQ6HC_SOUTH_PLL_PER,
> +               .resource_name = "emmc",
> +               .shift = 4,
> +               .width = 4,
> +       },
> +       {
> +               .index = EQ6HC_SOUTH_DIV_OSPI_REF,
> +               .name = "div-ospi-ref",
> +               .parent = EQ6HC_SOUTH_PLL_PER,
> +               .resource_name = "ospi",
> +               .shift = 4,
> +               .width = 4,
> +       },
> +       {
> +               .index = EQ6HC_SOUTH_DIV_OSPI_SYS,
> +               .name = "div-ospi-sys",
> +               .parent = EQ6HC_SOUTH_PLL_PER,
> +               .resource_name = "ospi",
> +               .shift = 8,
> +               .width = 1,
> +       },
> +       {
> +               .index = EQ6HC_SOUTH_DIV_TSU,
> +               .name = "div-tsu",
> +               .parent = EQ6HC_SOUTH_PLL_PCIE,
> +               .resource_name = "tsu",
> +               .shift = 4,
> +               .width = 8,
> +       },
> +};
> +
> +static const struct eqc_match_data eqc_eyeq6h_south_match_data = {
> +       .pll_count      = ARRAY_SIZE(eqc_eyeq6h_south_plls),
> +       .plls           = eqc_eyeq6h_south_plls,
> +
> +       .div_count      = ARRAY_SIZE(eqc_eyeq6h_south_divs),
> +       .divs           = eqc_eyeq6h_south_divs,
> +};
> +
> +static const struct eqc_pll eqc_eyeq6h_ddr0_plls[] = {
> +       { .index = 0, .name = "pll-ddr0", .reg64 = 0x00 },
> +};
> +
> +static const struct eqc_match_data eqc_eyeq6h_ddr0_match_data = {
> +       .pll_count      = ARRAY_SIZE(eqc_eyeq6h_ddr0_plls),
> +       .plls           = eqc_eyeq6h_ddr0_plls,
> +};
> +
> +static const struct eqc_pll eqc_eyeq6h_ddr1_plls[] = {
> +       { .index = 0, .name = "pll-ddr1", .reg64 = 0x00 },
> +};
> +
> +static const struct eqc_match_data eqc_eyeq6h_ddr1_match_data = {
> +       .pll_count      = ARRAY_SIZE(eqc_eyeq6h_ddr1_plls),
> +       .plls           = eqc_eyeq6h_ddr1_plls,
> +};
> +
> +static const struct eqc_pll eqc_eyeq6h_acc_plls[] = {
> +       { .index = EQ6HC_ACC_PLL_XNN, .name = "pll-xnn", .reg64 = 0x00 },
> +       { .index = EQ6HC_ACC_PLL_VMP, .name = "pll-vmp", .reg64 = 0x10 },
> +       { .index = EQ6HC_ACC_PLL_PMA, .name = "pll-pma", .reg64 = 0x1C },
> +       { .index = EQ6HC_ACC_PLL_MPC, .name = "pll-mpc", .reg64 = 0x28 },
> +       { .index = EQ6HC_ACC_PLL_NOC, .name = "pll-noc", .reg64 = 0x30 },
> +};
> +
> +static const struct eqc_match_data eqc_eyeq6h_acc_match_data = {
> +       .pll_count      = ARRAY_SIZE(eqc_eyeq6h_acc_plls),
> +       .plls           = eqc_eyeq6h_acc_plls,
> +};
> +
> +static const struct of_device_id eqc_match_table[] = {
> +       { .compatible = "mobileye,eyeq5-clk", .data = &eqc_eyeq5_match_data },
> +       { .compatible = "mobileye,eyeq6l-clk", .data = &eqc_eyeq6l_match_data },
> +       { .compatible = "mobileye,eyeq6h-central-clk", .data = &eqc_eyeq6h_central_match_data },
> +       { .compatible = "mobileye,eyeq6h-west-clk", .data = &eqc_eyeq6h_west_match_data },
> +       { .compatible = "mobileye,eyeq6h-east-clk", .data = &eqc_eyeq6h_east_match_data },
> +       { .compatible = "mobileye,eyeq6h-south-clk", .data = &eqc_eyeq6h_south_match_data },
> +       { .compatible = "mobileye,eyeq6h-ddr0-clk", .data = &eqc_eyeq6h_ddr0_match_data },
> +       { .compatible = "mobileye,eyeq6h-ddr1-clk", .data = &eqc_eyeq6h_ddr1_match_data },
> +       { .compatible = "mobileye,eyeq6h-acc-clk", .data = &eqc_eyeq6h_acc_match_data },
> +       {}
> +};
> +MODULE_DEVICE_TABLE(of, eqc_match_table);
> +
> +static struct platform_driver eqc_driver = {
> +       .probe = eqc_probe,
> +       .driver = {
> +               .name = "clk-eyeq",
> +               .of_match_table = eqc_match_table,
> +       },
> +};
> +builtin_platform_driver(eqc_driver);
> +
> +static void __init eqc_init(struct device_node *np)
> +{
> +       const struct eqc_match_data *data;
> +       unsigned int nb_clks = 0;
> +       struct eqc_priv *priv;
> +       unsigned int i;
> +       int ret;
> +
> +       data = of_match_node(eqc_match_table, np)->data;
> +
> +       /* No reason to early init this clock provider. Do it at probe. */
> +       if (data->early_pll_count == 0)

You can have a different match table for this function then.

> +               return;
> +
> +       priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> +       if (!priv) {
> +               ret = -ENOMEM;
> +               goto err;
> +       }
> +
> +       priv->np = np;
> +       priv->data = data;
> +
> +       nb_clks = eqc_compute_clock_count(data);
> +       priv->cells = kzalloc(struct_size(priv->cells, hws, nb_clks), GFP_KERNEL);
> +       if (!priv->cells) {
> +               ret = -ENOMEM;
> +               goto err;
> +       }
> +
> +       priv->cells->num = nb_clks;
> +
> +       /*
> +        * Mark non-early clocks as deferred; they'll be registered at platform
> +        * device probe.
> +        */
> +       for (i = 0; i < data->pll_count; i++)
> +               priv->cells->hws[data->plls[i].index] = ERR_PTR(-EPROBE_DEFER);
> +       for (i = 0; i < data->div_count; i++)
> +               priv->cells->hws[data->divs[i].index] = ERR_PTR(-EPROBE_DEFER);
> +
> +       /*
> +        * We expect named resources if divider clocks are present.
> +        * Else, we only expect one resource.
> +        */

Please avoid named resources. They give the false sense of hope that the
binding can re-order the reg property when that can't be done. Instead,
just index and know which index to use in the driver.

Re: [PATCH 05/11] clk: eyeq: add driver

[Théo Lebrun]

Hello,

On Thu Apr 11, 2024 at 5:22 AM CEST, Stephen Boyd wrote:
> Quoting Théo Lebrun (2024-04-10 10:12:34)
> > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> > index 50af5fc7f570..1eb6e70977a3 100644
> > --- a/drivers/clk/Kconfig
> > +++ b/drivers/clk/Kconfig
> > @@ -218,6 +218,17 @@ config COMMON_CLK_EN7523
> >           This driver provides the fixed clocks and gates present on Airoha
> >           ARM silicon.
> >  
> > +config COMMON_CLK_EYEQ
> > +       bool "Clock driver for the Mobileye EyeQ platform"
> > +       depends on OF || COMPILE_TEST
>
> The OF build dependency looks useless as we have the MACH_ dependency
> below.

Indeed. I thought explicit dependency could be useful. Will remove.

> > +       depends on MACH_EYEQ5 || MACH_EYEQ6H || COMPILE_TEST
> > +       default MACH_EYEQ5 || MACH_EYEQ6H
> > +       help
> > +         This driver provides clocks found on Mobileye EyeQ5, EyeQ6L and Eye6H
> > +         SoCs. Controllers live in shared register regions called OLB. Driver
> > +         provides read-only PLLs, derived from the main crystal clock (which
> > +         must be constant). It also exposes some divider clocks.
> > +
> >  config COMMON_CLK_FSL_FLEXSPI
> >         tristate "Clock driver for FlexSPI on Layerscape SoCs"
> >         depends on ARCH_LAYERSCAPE || COMPILE_TEST
> > diff --git a/drivers/clk/clk-eyeq.c b/drivers/clk/clk-eyeq.c
> > new file mode 100644
> > index 000000000000..bb2535010ae6
> > --- /dev/null
> > +++ b/drivers/clk/clk-eyeq.c
> > @@ -0,0 +1,644 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * PLL clock driver for the Mobileye EyeQ5, EyeQ6L and EyeQ6H platforms.
> > + *
> > + * This controller handles read-only PLLs, all derived from the same main
> > + * crystal clock. It also exposes divider clocks, those are children to PLLs.
> > + * Parent clock is expected to be constant. This driver's registers live in
> > + * a shared region called OLB. Some PLLs are initialised early by of_clk_init().
>
> Is OLB a different DT node? It sounds like maybe this is trying to jam a
> driver into DT when the OLB node should be a #clock-cells node.

Yes OLB is a different DT node. It looks like on EyeQ5:

	olb: system-controller@e00000 {
		compatible = "mobileye,eyeq5-olb", "syscon", "simple-mfd";
		reg = <0 0xe00000 0x0 0x400>;
		ranges = <0x0 0x0 0xe00000 0x400>;
		#address-cells = <1>;
		#size-cells = <1>;

		reset: reset-controller@e00000 {
			compatible = "mobileye,eyeq5-reset";
			reg = <0x000 0x0c>, <0x200 0x34>, <0x120 0x04>;
			reg-names = "d0", "d1", "d2";
			#reset-cells = <2>;
		};

		clocks: clock-controller@e0002c {
			compatible = "mobileye,eyeq5-clk";
			reg = <0x02c 0x50>, <0x11c 0x04>;
			reg-names = "plls", "ospi";
			#clock-cells = <1>;
			clocks = <&xtal>;
			clock-names = "ref";
		};

		pinctrl: pinctrl@e000b0 {
			compatible = "mobileye,eyeq5-pinctrl";
			reg = <0x0b0 0x30>;
		};
	};

Keep in mind OLB is a complex beast. On EyeQ5, it hosts something like
150 registers, describing 20ish various hardware features. We have to
expose registers to drivers for one-off reads/writes. One example found
upstream: I2C speed mode register. Others will be Ethernet, eMMC DMA
config, etc. A syscon makes sense.

I2C looks like like this for example, look at mobileye,olb.

	i2c@300000 {
		compatible = "mobileye,eyeq5-i2c", "arm,primecell";
		reg = <0x300000 0x1000>;
		interrupt-parent = <&gic>;
		interrupts = <GIC_SHARED 1 IRQ_TYPE_LEVEL_HIGH>;
		clock-frequency = <400000>;
		#address-cells = <1>;
		#size-cells = <0>;
		clocks = <&i2c_ser_clk>, <&i2c_clk>;
		clock-names = "i2cclk", "apb_pclk";
		mobileye,olb = <&olb 0>;
	};

See commits 7d4c57abb928 and 1b9a8e8af0d9:
  i2c: nomadik: support Mobileye EyeQ5 I2C controller
  dt-bindings: i2c: nomadik: add mobileye,eyeq5-i2c bindings and example

> > +
> > +static int eqc_probe(struct platform_device *pdev)
> > +{
> > +       struct device *dev = &pdev->dev;
> > +       void __iomem *div_resources[EQC_MAX_DIV_COUNT];
> > +       struct device_node *np = dev->of_node;
> > +       const struct eqc_match_data *data;
> > +       struct eqc_priv *priv = NULL;
> > +       struct clk_hw *hw;
> > +       unsigned int i;
> > +
> > +       data = device_get_match_data(dev);
> > +       if (!data)
> > +               return -ENODEV;
> > +
> > +       if (data->early_pll_count) {
> > +               /* Device got inited early. Retrieve clock provider from list. */
> > +               struct eqc_priv *entry;
> > +
> > +               spin_lock(&eqc_list_slock);
> > +               list_for_each_entry(entry, &eqc_list, list) {
> > +                       if (entry->np == np) {
> > +                               priv = entry;
> > +                               break;
> > +                       }
> > +               }
> > +               spin_unlock(&eqc_list_slock);
> > +
> > +               if (!priv)
> > +                       return -ENODEV;
>
> This can be a sub-function.

Will do.

[...]

> > +       for (i = 0; i < data->pll_count; i++) {
> > +               const struct eqc_pll *pll = &data->plls[i];
> > +               unsigned long mult, div, acc;
> > +               u32 r0, r1;
> > +               u64 val;
> > +               int ret;
>
> All variables should be declared at the start of the function. Once it
> becomes "too heavy" you can split it up into smaller functions, that
> again have all variables declared at the start of the function.

Will avoid variables declarations at start of loops.

> > +
> > +               val = readq(priv->base_plls + pll->reg64);
> > +               r0 = val;
> > +               r1 = val >> 32;
> > +
> > +               ret = eqc_pll_parse_registers(r0, r1, &mult, &div, &acc);
> > +               if (ret) {
> > +                       dev_warn(dev, "failed parsing state of %s\n", pll->name);
> > +                       priv->cells->hws[pll->index] = ERR_PTR(ret);
> > +                       continue;
> > +               }
> > +
> > +               hw = clk_hw_register_fixed_factor_with_accuracy_fwname(dev,
> > +                               dev->of_node, pll->name, "ref", 0, mult, div, acc);
> > +               priv->cells->hws[pll->index] = hw;
> > +               if (IS_ERR(hw))
> > +                       dev_warn(dev, "failed registering %s: %pe\n", pll->name, hw);
> > +       }
> > +
> > +       BUG_ON(ARRAY_SIZE(div_resources) < data->div_count);
>
> Can this be a static assert instead on the arrays these are based on?
> Put some static_assert() near the match data macros.

I hesitated before sending. Will update.

> > +
> > +       for (i = 0; i < data->div_count; i++) {
> > +               const struct eqc_div *div = &data->divs[i];
> > +               void __iomem *base = NULL;
> > +               struct clk_hw *parent;
> > +               unsigned int j;
> > +
> > +               /*
> > +                * Multiple divider clocks can request the same resource. Store
> > +                * resource pointers during probe(). For each divider clock,
> > +                * check if previous clocks referenced the same resource name.
> > +                *
> > +                * See EQ6HC_SOUTH_DIV_OSPI_REF and EQ6HC_SOUTH_DIV_OSPI_SYS.
> > +                */
> > +               for (j = 0; j < i; j++) {
> > +                       if (strcmp(data->divs[j].resource_name, div->resource_name) == 0) {
> > +                               base = div_resources[j];
> > +                               break;
> > +                       }
> > +               }
> > +
> > +               /* Resource is first encountered. */
> > +               if (!base) {
> > +                       base = devm_platform_ioremap_resource_byname(pdev, div->resource_name);
> > +                       if (IS_ERR(base)) {
> > +                               dev_warn(dev, "failed to iomap resource for %s\n", div->name);
> > +                               priv->cells->hws[div->index] = base;
> > +                               continue;
> > +                       }
> > +               }
>
> I don't get this code at all. The driver should simply map the
> resources because it knows that there's an io resource. I'll look at the
> binding which is probably wrong and causing the driver to be written
> this way.

This is here for a single reason: EyeQ6H south OLB has two clocks that
live in the same register:

 - div-ospi-ref, reg offset 0x90, mask GENMASK(9,  8) == 0x300.
 - div-ospi-sys, reg offset 0x90, mask GENMASK(12, 4) == 0x1FF0.

Calling twice devm_platform_ioremap_resource_byname() with the same
resource name gives an error. So we need to buffer resources already
requested.

If there is a simpler & better solution I'd be happy to take it.

[...]

> > +static void __init eqc_init(struct device_node *np)
> > +{
> > +       const struct eqc_match_data *data;
> > +       unsigned int nb_clks = 0;
> > +       struct eqc_priv *priv;
> > +       unsigned int i;
> > +       int ret;
> > +
> > +       data = of_match_node(eqc_match_table, np)->data;
> > +
> > +       /* No reason to early init this clock provider. Do it at probe. */
> > +       if (data->early_pll_count == 0)
>
> You can have a different match table for this function then.

Ah, clever. Will do.

[...]

> > +       /*
> > +        * We expect named resources if divider clocks are present.
> > +        * Else, we only expect one resource.
> > +        */
>
> Please avoid named resources. They give the false sense of hope that the
> binding can re-order the reg property when that can't be done. Instead,
> just index and know which index to use in the driver.

It is unclear what you mean by not being able to re-order reg property?
Are you talking about reg-names being most often defined as items const
list and therefore cannot be reordered? Here binding declare things
using minItems/maxItems/enum so it can be reordered, looking like:

  properties:
    reg:
      minItems: 2
      maxItems: 2
    reg-names:
      minItems: 2
      maxItems: 2
      items:
        enum: [ plls, ospi ]

If this is not what you are talking about then I rambled about garbage
and I'll use indexed resources.

Thanks,

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

Re: [PATCH 05/11] clk: eyeq: add driver

[Stephen Boyd]

Quoting Théo Lebrun (2024-04-11 03:46:04)
> Hello,
> 
> On Thu Apr 11, 2024 at 5:22 AM CEST, Stephen Boyd wrote:
> > Quoting Théo Lebrun (2024-04-10 10:12:34)
> > > diff --git a/drivers/clk/clk-eyeq.c b/drivers/clk/clk-eyeq.c
> > > new file mode 100644
> > > index 000000000000..bb2535010ae6
> > > --- /dev/null
> > > +++ b/drivers/clk/clk-eyeq.c
> > > @@ -0,0 +1,644 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * PLL clock driver for the Mobileye EyeQ5, EyeQ6L and EyeQ6H platforms.
> > > + *
> > > + * This controller handles read-only PLLs, all derived from the same main
> > > + * crystal clock. It also exposes divider clocks, those are children to PLLs.
> > > + * Parent clock is expected to be constant. This driver's registers live in
> > > + * a shared region called OLB. Some PLLs are initialised early by of_clk_init().
> >
> > Is OLB a different DT node? It sounds like maybe this is trying to jam a
> > driver into DT when the OLB node should be a #clock-cells node.
> 
> Yes OLB is a different DT node. It looks like on EyeQ5:
> 
>         olb: system-controller@e00000 {
>                 compatible = "mobileye,eyeq5-olb", "syscon", "simple-mfd";
>                 reg = <0 0xe00000 0x0 0x400>;
>                 ranges = <0x0 0x0 0xe00000 0x400>;
>                 #address-cells = <1>;
>                 #size-cells = <1>;
> 
>                 reset: reset-controller@e00000 {
>                         compatible = "mobileye,eyeq5-reset";
>                         reg = <0x000 0x0c>, <0x200 0x34>, <0x120 0x04>;
>                         reg-names = "d0", "d1", "d2";
>                         #reset-cells = <2>;
>                 };
> 
>                 clocks: clock-controller@e0002c {
>                         compatible = "mobileye,eyeq5-clk";
>                         reg = <0x02c 0x50>, <0x11c 0x04>;

Is this reg property always the same value '0x2c'?

>                         reg-names = "plls", "ospi";
>                         #clock-cells = <1>;
>                         clocks = <&xtal>;
>                         clock-names = "ref";
>                 };
> 
>                 pinctrl: pinctrl@e000b0 {
>                         compatible = "mobileye,eyeq5-pinctrl";
>                         reg = <0x0b0 0x30>;
>                 };
>         };


> 
> Keep in mind OLB is a complex beast. On EyeQ5, it hosts something like
> 150 registers, describing 20ish various hardware features. We have to
> expose registers to drivers for one-off reads/writes. One example found
> upstream: I2C speed mode register. Others will be Ethernet, eMMC DMA
> config, etc. A syscon makes sense.

Syscons are a slippery slope. It makes it easy to give up abstracting
the 20ish hardware features and makes the resulting drivers which use
the syscon highly platform specific.

Regardless of having a syscon or not, the binding should collapse the
sub-nodes into the olb node. If that requires making a different
compatible for different olb nodes, then that's actually better because
there may be some quirk for one of the olbs and not the other and we
won't be able to fix that without a compatible string update. It would
also make the reg-names property go away, because the sub-functionality
drivers would have the register offsets hard-coded as some offset from
the base of olb, instead of encoding that in DT.

> 
> I2C looks like like this for example, look at mobileye,olb.
> 
>         i2c@300000 {
>                 compatible = "mobileye,eyeq5-i2c", "arm,primecell";
>                 reg = <0x300000 0x1000>;
>                 interrupt-parent = <&gic>;
>                 interrupts = <GIC_SHARED 1 IRQ_TYPE_LEVEL_HIGH>;
>                 clock-frequency = <400000>;
>                 #address-cells = <1>;
>                 #size-cells = <0>;
>                 clocks = <&i2c_ser_clk>, <&i2c_clk>;
>                 clock-names = "i2cclk", "apb_pclk";
>                 mobileye,olb = <&olb 0>;
>         };
> 
> See commits 7d4c57abb928 and 1b9a8e8af0d9:
>   i2c: nomadik: support Mobileye EyeQ5 I2C controller
>   dt-bindings: i2c: nomadik: add mobileye,eyeq5-i2c bindings and example

Why isn't i2c speed mode another clk exposed by OLB that rounds to the
different rates?

> 
> > > +
> > > +       for (i = 0; i < data->div_count; i++) {
> > > +               const struct eqc_div *div = &data->divs[i];
> > > +               void __iomem *base = NULL;
> > > +               struct clk_hw *parent;
> > > +               unsigned int j;
> > > +
> > > +               /*
> > > +                * Multiple divider clocks can request the same resource. Store
> > > +                * resource pointers during probe(). For each divider clock,
> > > +                * check if previous clocks referenced the same resource name.
> > > +                *
> > > +                * See EQ6HC_SOUTH_DIV_OSPI_REF and EQ6HC_SOUTH_DIV_OSPI_SYS.
> > > +                */
> > > +               for (j = 0; j < i; j++) {
> > > +                       if (strcmp(data->divs[j].resource_name, div->resource_name) == 0) {
> > > +                               base = div_resources[j];
> > > +                               break;
> > > +                       }
> > > +               }
> > > +
> > > +               /* Resource is first encountered. */
> > > +               if (!base) {
> > > +                       base = devm_platform_ioremap_resource_byname(pdev, div->resource_name);
> > > +                       if (IS_ERR(base)) {
> > > +                               dev_warn(dev, "failed to iomap resource for %s\n", div->name);
> > > +                               priv->cells->hws[div->index] = base;
> > > +                               continue;
> > > +                       }
> > > +               }
> >
> > I don't get this code at all. The driver should simply map the
> > resources because it knows that there's an io resource. I'll look at the
> > binding which is probably wrong and causing the driver to be written
> > this way.
> 
> This is here for a single reason: EyeQ6H south OLB has two clocks that
> live in the same register:
> 
>  - div-ospi-ref, reg offset 0x90, mask GENMASK(9,  8) == 0x300.
>  - div-ospi-sys, reg offset 0x90, mask GENMASK(12, 4) == 0x1FF0.
> 
> Calling twice devm_platform_ioremap_resource_byname() with the same
> resource name gives an error. So we need to buffer resources already
> requested.
> 
> If there is a simpler & better solution I'd be happy to take it.

Sure, don't call platform_ioremap_resource() and friends more than once
per index. But why is the code written in a way that that is happening?
Maybe the driver can ioremap resources, and then register clks for those
resources. I suspect the only way of getting here is that the driver is
focused on registering clks, and ioremapping resources while registering
clks. Don't do that, because then you have to write code to track
resources.

> 
> 
> [...]
> 
> > > +       /*
> > > +        * We expect named resources if divider clocks are present.
> > > +        * Else, we only expect one resource.
> > > +        */
> >
> > Please avoid named resources. They give the false sense of hope that the
> > binding can re-order the reg property when that can't be done. Instead,
> > just index and know which index to use in the driver.
> 
> It is unclear what you mean by not being able to re-order reg property?
> Are you talking about reg-names being most often defined as items const
> list and therefore cannot be reordered? Here binding declare things
> using minItems/maxItems/enum so it can be reordered, looking like:

Yes, that's wrong.

> 
>   properties:
>     reg:
>       minItems: 2
>       maxItems: 2
>     reg-names:
>       minItems: 2
>       maxItems: 2
>       items:
>         enum: [ plls, ospi ]
> 
> If this is not what you are talking about then I rambled about garbage
> and I'll use indexed resources.
> 

You cannot reorder strings in a DT binding property after the fact.
While the code will keep working if the reg-names elements are
re-ordered, the binding will be backwards incompatible, because the
reg-names property must have the same order. It can be convenient to use
reg-names if you have a long list of reg properties to map, but having
two or one elements isn't a very strong argument.

Re: [PATCH 05/11] clk: eyeq: add driver

[Théo Lebrun]

Hello,

On Fri Apr 12, 2024 at 7:46 AM CEST, Stephen Boyd wrote:
> Quoting Théo Lebrun (2024-04-11 03:46:04)
> > On Thu Apr 11, 2024 at 5:22 AM CEST, Stephen Boyd wrote:
> > > Quoting Théo Lebrun (2024-04-10 10:12:34)
> > > > diff --git a/drivers/clk/clk-eyeq.c b/drivers/clk/clk-eyeq.c
> > > > new file mode 100644
> > > > index 000000000000..bb2535010ae6
> > > > --- /dev/null
> > > > +++ b/drivers/clk/clk-eyeq.c
> > > > @@ -0,0 +1,644 @@
> > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > +/*
> > > > + * PLL clock driver for the Mobileye EyeQ5, EyeQ6L and EyeQ6H platforms.
> > > > + *
> > > > + * This controller handles read-only PLLs, all derived from the same main
> > > > + * crystal clock. It also exposes divider clocks, those are children to PLLs.
> > > > + * Parent clock is expected to be constant. This driver's registers live in
> > > > + * a shared region called OLB. Some PLLs are initialised early by of_clk_init().
> > >
> > > Is OLB a different DT node? It sounds like maybe this is trying to jam a
> > > driver into DT when the OLB node should be a #clock-cells node.
> > 
> > Yes OLB is a different DT node. It looks like on EyeQ5:
> > 
> >         olb: system-controller@e00000 {
> >                 compatible = "mobileye,eyeq5-olb", "syscon", "simple-mfd";
> >                 reg = <0 0xe00000 0x0 0x400>;
> >                 ranges = <0x0 0x0 0xe00000 0x400>;
> >                 #address-cells = <1>;
> >                 #size-cells = <1>;
> > 
> >                 reset: reset-controller@e00000 {
> >                         compatible = "mobileye,eyeq5-reset";
> >                         reg = <0x000 0x0c>, <0x200 0x34>, <0x120 0x04>;
> >                         reg-names = "d0", "d1", "d2";
> >                         #reset-cells = <2>;
> >                 };
> > 
> >                 clocks: clock-controller@e0002c {
> >                         compatible = "mobileye,eyeq5-clk";
> >                         reg = <0x02c 0x50>, <0x11c 0x04>;
>
> Is this reg property always the same value '0x2c'?

On EyeQ5 yes.
On EyeQ6L and EyeQ6H (next revisions, different compatible), no.

> >                         reg-names = "plls", "ospi";
> >                         #clock-cells = <1>;
> >                         clocks = <&xtal>;
> >                         clock-names = "ref";
> >                 };
> > 
> >                 pinctrl: pinctrl@e000b0 {
> >                         compatible = "mobileye,eyeq5-pinctrl";
> >                         reg = <0x0b0 0x30>;
> >                 };
> >         };
>
>
> > 
> > Keep in mind OLB is a complex beast. On EyeQ5, it hosts something like
> > 150 registers, describing 20ish various hardware features. We have to
> > expose registers to drivers for one-off reads/writes. One example found
> > upstream: I2C speed mode register. Others will be Ethernet, eMMC DMA
> > config, etc. A syscon makes sense.
>
> Syscons are a slippery slope. It makes it easy to give up abstracting
> the 20ish hardware features and makes the resulting drivers which use
> the syscon highly platform specific.

I see where you are coming from. In my mind syscons make sense here
because most features in the block are only small additions/tweaks to
existing blocks. A few examples:

 - eMMC DMA integration config
 - eMMC high impedance register
 - OSPI high impedance register

Those cannot be abstracted. The main node is elsewhere and it needs
access to those registers.

Some other registers however will be able to be abstracted away, as
custom clocks for example (eg I'm seeing SGMII PLL control register).
That is not the case for all however.

> Regardless of having a syscon or not, the binding should collapse the
> sub-nodes into the olb node. If that requires making a different
> compatible for different olb nodes, then that's actually better because
> there may be some quirk for one of the olbs and not the other and we
> won't be able to fix that without a compatible string update. It would
> also make the reg-names property go away, because the sub-functionality
> drivers would have the register offsets hard-coded as some offset from
> the base of olb, instead of encoding that in DT.

Do you have examples of existing nodes that both are syscons and expose
multiple resources (clocks, resets, etc)? I did not envision this as
possible which is why I didn't go this route. Also, this would require
big changes to dt-bindings currently in v6.9-rc.

To be honest, I do not comprehend what aggregating sub-nodes into a
single one brings. Here we have one node per feature, each exposing
their small feature set, with a single driver handling each. Devicetree
is longer but more straight forward, with small resource provider nodes
versus a big behemoth handling all of it. Simple and easy: my brain
has an easy time grasping it all.

What you are proposing sounds more complex, for no clear benefit to my
eyes. Can you expand on what that brings? I guess answer has been given
elsewhere in depth, so answer might just be a link to the right
resource. This is an open question, not reserved to Stephen. Thanks!

> > I2C looks like like this for example, look at mobileye,olb.
> > 
> >         i2c@300000 {
> >                 compatible = "mobileye,eyeq5-i2c", "arm,primecell";
> >                 reg = <0x300000 0x1000>;
> >                 interrupt-parent = <&gic>;
> >                 interrupts = <GIC_SHARED 1 IRQ_TYPE_LEVEL_HIGH>;
> >                 clock-frequency = <400000>;
> >                 #address-cells = <1>;
> >                 #size-cells = <0>;
> >                 clocks = <&i2c_ser_clk>, <&i2c_clk>;
> >                 clock-names = "i2cclk", "apb_pclk";
> >                 mobileye,olb = <&olb 0>;
> >         };
> > 
> > See commits 7d4c57abb928 and 1b9a8e8af0d9:
> >   i2c: nomadik: support Mobileye EyeQ5 I2C controller
> >   dt-bindings: i2c: nomadik: add mobileye,eyeq5-i2c bindings and example
>
> Why isn't i2c speed mode another clk exposed by OLB that rounds to the
> different rates?

It would be a new clock referenced from DT on which we would only do
clk_set_rate(priv->clk_freq). How would it fit in the clock tree? It
wouldn't have any parent, even though in practice is it parented to an
internal Nomadik I2C clock that is not being exposed.

I thought it would be an issue to have a clock modeled as having no
parent (because its true parent is not exposed) which is a blatant lie.

> > > > +       for (i = 0; i < data->div_count; i++) {
> > > > +               const struct eqc_div *div = &data->divs[i];
> > > > +               void __iomem *base = NULL;
> > > > +               struct clk_hw *parent;
> > > > +               unsigned int j;
> > > > +
> > > > +               /*
> > > > +                * Multiple divider clocks can request the same resource. Store
> > > > +                * resource pointers during probe(). For each divider clock,
> > > > +                * check if previous clocks referenced the same resource name.
> > > > +                *
> > > > +                * See EQ6HC_SOUTH_DIV_OSPI_REF and EQ6HC_SOUTH_DIV_OSPI_SYS.
> > > > +                */
> > > > +               for (j = 0; j < i; j++) {
> > > > +                       if (strcmp(data->divs[j].resource_name, div->resource_name) == 0) {
> > > > +                               base = div_resources[j];
> > > > +                               break;
> > > > +                       }
> > > > +               }
> > > > +
> > > > +               /* Resource is first encountered. */
> > > > +               if (!base) {
> > > > +                       base = devm_platform_ioremap_resource_byname(pdev, div->resource_name);
> > > > +                       if (IS_ERR(base)) {
> > > > +                               dev_warn(dev, "failed to iomap resource for %s\n", div->name);
> > > > +                               priv->cells->hws[div->index] = base;
> > > > +                               continue;
> > > > +                       }
> > > > +               }
> > >
> > > I don't get this code at all. The driver should simply map the
> > > resources because it knows that there's an io resource. I'll look at the
> > > binding which is probably wrong and causing the driver to be written
> > > this way.
> > 
> > This is here for a single reason: EyeQ6H south OLB has two clocks that
> > live in the same register:
> > 
> >  - div-ospi-ref, reg offset 0x90, mask GENMASK(9,  8) == 0x300.
> >  - div-ospi-sys, reg offset 0x90, mask GENMASK(12, 4) == 0x1FF0.
> > 
> > Calling twice devm_platform_ioremap_resource_byname() with the same
> > resource name gives an error. So we need to buffer resources already
> > requested.
> > 
> > If there is a simpler & better solution I'd be happy to take it.
>
> Sure, don't call platform_ioremap_resource() and friends more than once
> per index. But why is the code written in a way that that is happening?
> Maybe the driver can ioremap resources, and then register clks for those
> resources. I suspect the only way of getting here is that the driver is
> focused on registering clks, and ioremapping resources while registering
> clks. Don't do that, because then you have to write code to track
> resources.

So code would look like (removing reg-names as it brings nothing, and
error checking for brevity):

void __iomem *div_resources[EQC_MAX_DIV_COUNT];
unsigned int max_div_resource_index = 0;
const struct eqc_div *div;
struct clk_hw *parent;
void __iomem *base;
struct clk_hw *hw;

// Learn what resources should be acquired
for (i = 0; i < data->div_count; i++) {
	div = &data->divs[i];

	if (div->resource_index > max_div_resource_index)
		max_div_resource_index = div->resource_index;
}

// Grab resources (starting at 1 because 0 is for PLLs)
for (i = 1; i < max_div_resource_index; i++) {
	div_resources[i] = devm_platform_ioremap_resource(pdev, i);
	// TODO: error checking
}

// Register clocks
for (i = 0; i < data->div_count; i++) {
	div = &data->divs[i];
	base = div_resources[div->resource_index];

	parent = priv->cells->hws[div->parent];
	hw = clk_hw_register_divider_table_parent_hw(dev, div->name,
			parent, 0, base, div->shift, div->width,
			CLK_DIVIDER_EVEN_INTEGERS, NULL, NULL);
	priv->cells->hws[div->index] = hw;
	// TODO: error checking
}

>
> > 
> > 
> > [...]
> > 
> > > > +       /*
> > > > +        * We expect named resources if divider clocks are present.
> > > > +        * Else, we only expect one resource.
> > > > +        */
> > >
> > > Please avoid named resources. They give the false sense of hope that the
> > > binding can re-order the reg property when that can't be done. Instead,
> > > just index and know which index to use in the driver.
> > 
> > It is unclear what you mean by not being able to re-order reg property?
> > Are you talking about reg-names being most often defined as items const
> > list and therefore cannot be reordered? Here binding declare things
> > using minItems/maxItems/enum so it can be reordered, looking like:
>
> Yes, that's wrong.
>
> > 
> >   properties:
> >     reg:
> >       minItems: 2
> >       maxItems: 2
> >     reg-names:
> >       minItems: 2
> >       maxItems: 2
> >       items:
> >         enum: [ plls, ospi ]
> > 
> > If this is not what you are talking about then I rambled about garbage
> > and I'll use indexed resources.
> > 
>
> You cannot reorder strings in a DT binding property after the fact.
> While the code will keep working if the reg-names elements are
> re-ordered, the binding will be backwards incompatible, because the
> reg-names property must have the same order. It can be convenient to use
> reg-names if you have a long list of reg properties to map, but having
> two or one elements isn't a very strong argument.

I didn't know that beforehands. I completely get what you mean now.
Why it is that way is unclear to me but whatever. It is even documented:
https://elixir.bootlin.com/linux/v6.8.6/source/Documentation/devicetree/bindings/writing-bindings.rst#L64

Thanks Stephen,

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

[PATCH 06/11] reset: eyeq: add platform driver

[Théo Lebrun]

Add Mobileye EyeQ reset controller driver, for EyeQ5, EyeQ6L and EyeQ6H
SoCs. Instances belong to a shared register region called OLB.

There is one OLB instance for EyeQ5 and EyeQ6L. There are seven OLB
instances on EyeQ6H; three have a reset controller embedded:
 - West and east get handled by the same compatible.
 - Acc is another one.

Each instance vary in the number and types of reset domains.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 MAINTAINERS                |   1 +
 drivers/reset/Kconfig      |  13 ++
 drivers/reset/Makefile     |   1 +
 drivers/reset/reset-eyeq.c | 543 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 558 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 33168ebf3cc5..724f20ea0411 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14933,6 +14933,7 @@ F:	arch/mips/boot/dts/mobileye/
 F:	arch/mips/configs/eyeq5_defconfig
 F:	arch/mips/mobileye/board-epm5.its.S
 F:	drivers/clk/clk-eyeq5.c
+F:	drivers/reset/reset-eyeq5.c
 F:	include/dt-bindings/clock/mobileye,eyeq5-clk.h
 
 MODULE SUPPORT
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 85b27c42cf65..18ee99ed8ecc 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -66,6 +66,19 @@ config RESET_BRCMSTB_RESCAL
 	  This enables the RESCAL reset controller for SATA, PCIe0, or PCIe1 on
 	  BCM7216.
 
+config RESET_EYEQ
+	bool "Mobileye EyeQ reset controller"
+	depends on MFD_SYSCON
+	depends on MACH_EYEQ5 || MACH_EYEQ6H || COMPILE_TEST
+	default MACH_EYEQ5 || MACH_EYEQ6H
+	help
+	  This enables the Mobileye EyeQ reset controller, used in EyeQ5, EyeQ6L
+	  and EyeQ6H SoCs.
+
+	  It has one or more domains, with a varying number of resets in each.
+	  Registers are located in a shared register region called OLB. EyeQ6H
+	  has multiple reset instances.
+
 config RESET_GPIO
 	tristate "GPIO reset controller"
 	help
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index fd8b49fa46fc..a4e6fea29800 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_RESET_BCM6345) += reset-bcm6345.o
 obj-$(CONFIG_RESET_BERLIN) += reset-berlin.o
 obj-$(CONFIG_RESET_BRCMSTB) += reset-brcmstb.o
 obj-$(CONFIG_RESET_BRCMSTB_RESCAL) += reset-brcmstb-rescal.o
+obj-$(CONFIG_RESET_EYEQ) += reset-eyeq.o
 obj-$(CONFIG_RESET_GPIO) += reset-gpio.o
 obj-$(CONFIG_RESET_HSDK) += reset-hsdk.o
 obj-$(CONFIG_RESET_IMX7) += reset-imx7.o
diff --git a/drivers/reset/reset-eyeq.c b/drivers/reset/reset-eyeq.c
new file mode 100644
index 000000000000..b86930145256
--- /dev/null
+++ b/drivers/reset/reset-eyeq.c
@@ -0,0 +1,543 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Reset driver for the Mobileye EyeQ5, EyeQ6L and EyeQ6H platforms.
+ *
+ * Controllers live in a shared register region called OLB. EyeQ5 and EyeQ6L
+ * have a single OLB instance for a single reset controller. EyeQ6H has seven
+ * OLB instances; three host reset controllers.
+ *
+ * Each reset controller has one or more domain. Domains are of a given type
+ * (see enum eqr_domain_type), with a valid offset mask (up to 32 resets per
+ * domain).
+ *
+ * Domain types define expected behavior: one-register-per-reset,
+ * one-bit-per-reset, status detection method, busywait duration, etc.
+ *
+ * We use eqr_ as prefix, as-in "EyeQ Reset", but way shorter.
+ *
+ * Known resets in EyeQ5 domain 0 (type EQR_EYEQ5_SARCR):
+ *  3. CAN0	 4. CAN1	 5. CAN2	 6. SPI0
+ *  7. SPI1	 8. SPI2	 9. SPI3	10. UART0
+ * 11. UART1	12. UART2	13. I2C0	14. I2C1
+ * 15. I2C2	16. I2C3	17. I2C4	18. TIMER0
+ * 19. TIMER1	20. TIMER2	21. TIMER3	22. TIMER4
+ * 23. WD0	24. EXT0	25. EXT1	26. GPIO
+ * 27. WD1
+ *
+ * Known resets in EyeQ5 domain 1 (type EQR_EYEQ5_ACRP):
+ *  0. VMP0	 1. VMP1	 2. VMP2	 3. VMP3
+ *  4. PMA0	 5. PMA1	 6. PMAC0	 7. PMAC1
+ *  8. MPC0	 9. MPC1	10. MPC2	11. MPC3
+ * 12. MPC4
+ *
+ * Known resets in EyeQ5 domain 2 (type EQR_EYEQ5_PCIE):
+ *  0. PCIE0_CORE	 1. PCIE0_APB		 2. PCIE0_LINK_AXI	 3. PCIE0_LINK_MGMT
+ *  4. PCIE0_LINK_HOT	 5. PCIE0_LINK_PIPE	 6. PCIE1_CORE		 7. PCIE1_APB
+ *  8. PCIE1_LINK_AXI	 9. PCIE1_LINK_MGMT	10. PCIE1_LINK_HOT	11. PCIE1_LINK_PIPE
+ * 12. MULTIPHY		13. MULTIPHY_APB	15. PCIE0_LINK_MGMT	16. PCIE1_LINK_MGMT
+ * 17. PCIE0_LINK_PM	18. PCIE1_LINK_PM
+ *
+ * Known resets in EyeQ6L domain 0 (type EQR_EYEQ5_SARCR):
+ *  0. SPI0	 1. SPI1	 2. UART0	 3. I2C0
+ *  4. I2C1	 5. TIMER0	 6. TIMER1	 7. TIMER2
+ *  8. TIMER3	 9. WD0		10. WD1		11. EXT0
+ * 12. EXT1	13. GPIO
+ *
+ * Known resets in EyeQ6L domain 1 (type EQR_EYEQ5_ACRP):
+ *  0. VMP0	 1. VMP1	 2. VMP2	 3. VMP3
+ *  4. PMA0	 5. PMA1	 6. PMAC0	 7. PMAC1
+ *  8. MPC0	 9. MPC1	10. MPC2	11. MPC3
+ * 12. MPC4
+ *
+ * Known resets in EyeQ6H west/east (type EQR_EYEQ6H_SARCR):
+ *  0. CAN	 1. SPI0	 2. SPI1	 3. UART0
+ *  4. UART1	 5. I2C0	 6. I2C1	 7. -hole-
+ *  8. TIMER0	 9. TIMER1	10. WD		11. EXT TIMER
+ * 12. GPIO
+ *
+ * Known resets in EyeQ6H acc (type EQR_EYEQ5_ACRP):
+ *  1. XNN0	 2. XNN1	 3. XNN2	 4. XNN3
+ *  5. VMP0	 6. VMP1	 7. VMP2	 8. VMP3
+ *  9. PMA0	10. PMA1	11. MPC0	12. MPC1
+ * 13. MPC2	14. MPC3	15. PERIPH
+ *
+ * Abbreviations:
+ *  - PMA: Programmable Macro Array
+ *  - MPC: Multi-threaded Processing Clusters
+ *  - VMP: Vector Microcode Processors
+ *
+ * Copyright (C) 2024 Mobileye Vision Technologies Ltd.
+ */
+
+#include <linux/array_size.h>
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/bug.h>
+#include <linux/cleanup.h>
+#include <linux/container_of.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/lockdep.h>
+#include <linux/mod_devicetable.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+/*
+ * A reset ID, as returned by eqr_of_xlate, is a (domain, offset) pair.
+ * Low byte is domain, rest is offset.
+ */
+#define ID_DOMAIN_MASK	GENMASK(7, 0)
+#define ID_OFFSET_MASK	GENMASK(31, 8)
+
+enum eqr_domain_type {
+	EQR_EYEQ5_SARCR,
+	EQR_EYEQ5_ACRP,
+	EQR_EYEQ5_PCIE,
+	EQR_EYEQ6H_SARCR,
+};
+
+/*
+ * Domain type EQR_EYEQ5_SARCR register offsets.
+ */
+#define EQR_EYEQ5_SARCR_REQUEST	(0x004)
+#define EQR_EYEQ5_SARCR_STATUS		(0x008)
+
+/*
+ * Domain type EQR_EYEQ5_ACRP register masks.
+ * Registers are: base + 4 * offset.
+ */
+#define EQR_EYEQ5_ACRP_PD_REQ		BIT(0)
+#define EQR_EYEQ5_ACRP_ST_POWER_DOWN	BIT(27)
+#define EQR_EYEQ5_ACRP_ST_ACTIVE	BIT(29)
+
+/*
+ * Domain type EQR_EYEQ6H_SARCR register offsets.
+ */
+#define EQR_EYEQ6H_SARCR_RST_REQUEST	(0x004)
+#define EQR_EYEQ6H_SARCR_CLK_STATUS	(0x008)
+#define EQR_EYEQ6H_SARCR_RST_STATUS	(0x00C)
+#define EQR_EYEQ6H_SARCR_CLK_REQUEST	(0x010)
+
+struct eqr_busy_wait_timings {
+	unsigned long sleep_us;
+	unsigned long timeout_us;
+};
+
+static const struct eqr_busy_wait_timings eqr_timings[] = {
+	[EQR_EYEQ5_SARCR]	= {1, 10},
+	[EQR_EYEQ5_ACRP]	= {1, 40 * USEC_PER_MSEC}, /* LBIST implies long timeout. */
+	/* EQR_EYEQ5_PCIE does no busy waiting. */
+	[EQR_EYEQ6H_SARCR]	= {1, 400},
+};
+
+#define EQR_MAX_DOMAIN_COUNT 3
+
+struct eqr_domain_descriptor {
+	const char		*resource_name;
+	enum eqr_domain_type	type;
+	u32			valid_mask;
+};
+
+struct eqr_match_data {
+	unsigned int				domain_count;
+	const struct eqr_domain_descriptor	*domains;
+};
+
+struct eqr_private {
+	struct mutex			mutexes[EQR_MAX_DOMAIN_COUNT];
+	void __iomem			*bases[EQR_MAX_DOMAIN_COUNT];
+	const struct eqr_match_data	*data;
+	struct reset_controller_dev	rcdev;
+};
+
+#define rcdev_to_priv(rcdev) container_of(rcdev, struct eqr_private, rcdev)
+
+static u32 eqr_double_readl(void __iomem *addr_a, void __iomem *addr_b,
+			    u32 *dest_a, u32 *dest_b)
+{
+	*dest_a = readl(addr_a);
+	*dest_b = readl(addr_b);
+	return 0; /* read_poll_timeout() op argument must return something. */
+}
+
+static int eqr_busy_wait_locked(struct eqr_private *priv, struct device *dev,
+				u32 domain, u32 offset, bool assert)
+{
+	enum eqr_domain_type domain_type = priv->data->domains[domain].type;
+	unsigned long sleep_us, timeout_us;
+	u32 val, mask, val0, val1;
+	void __iomem *base, *reg;
+	int ret;
+
+	lockdep_assert_held(&priv->mutexes[domain]);
+
+	base = priv->bases[domain];
+	sleep_us = eqr_timings[domain_type].sleep_us;
+	timeout_us = eqr_timings[domain_type].timeout_us;
+
+	switch (domain_type) {
+	case EQR_EYEQ5_SARCR:
+		reg = base + EQR_EYEQ5_SARCR_STATUS;
+		mask = BIT(offset);
+
+		ret = readl_poll_timeout(reg, val, !(val & mask) == assert,
+					 sleep_us, timeout_us);
+		break;
+
+	case EQR_EYEQ5_ACRP:
+		reg = base + 4 * offset;
+		if (assert)
+			mask = EQR_EYEQ5_ACRP_ST_POWER_DOWN;
+		else
+			mask = EQR_EYEQ5_ACRP_ST_ACTIVE;
+
+		ret = readl_poll_timeout(reg, val, !!(val & mask),
+					 sleep_us, timeout_us);
+		break;
+
+	case EQR_EYEQ5_PCIE:
+		ret = 0; /* No busy waiting. */
+		break;
+
+	case EQR_EYEQ6H_SARCR:
+		/*
+		 * Wait until both bits change:
+		 *	readl(base + EQR_EYEQ6H_SARCR_RST_STATUS) & BIT(offset)
+		 *	readl(base + EQR_EYEQ6H_SARCR_CLK_STATUS) & BIT(offset)
+		 */
+		mask = BIT(offset);
+		ret = read_poll_timeout(eqr_double_readl, val,
+					(!(val0 & mask) == assert) &&
+						(!(val1 & mask) == assert),
+					sleep_us, timeout_us, false,
+					base + EQR_EYEQ6H_SARCR_RST_STATUS,
+					base + EQR_EYEQ6H_SARCR_CLK_STATUS,
+					&val0, &val1);
+		break;
+
+	default:
+		WARN_ON(1);
+		ret = -EINVAL;
+		break;
+	}
+
+	if (ret == -ETIMEDOUT)
+		dev_dbg(dev, "%u-%u: timeout\n", domain, offset);
+	return ret;
+}
+
+static void eqr_assert_locked(struct eqr_private *priv, u32 domain,
+			      u32 offset)
+{
+	enum eqr_domain_type domain_type = priv->data->domains[domain].type;
+	void __iomem *base, *reg;
+	u32 val;
+
+	lockdep_assert_held(&priv->mutexes[domain]);
+
+	base = priv->bases[domain];
+
+	switch (domain_type) {
+	case EQR_EYEQ5_SARCR:
+		reg = base + EQR_EYEQ5_SARCR_REQUEST;
+		writel(readl(reg) & ~BIT(offset), reg);
+		break;
+
+	case EQR_EYEQ5_ACRP:
+		reg = base + 4 * offset;
+		writel(readl(reg) | EQR_EYEQ5_ACRP_PD_REQ, reg);
+		break;
+
+	case EQR_EYEQ5_PCIE:
+		writel(readl(base) & ~BIT(offset), base);
+		break;
+
+	case EQR_EYEQ6H_SARCR:
+		val = readl(base + EQR_EYEQ6H_SARCR_RST_REQUEST);
+		val &= ~BIT(offset);
+		writel(val, base + EQR_EYEQ6H_SARCR_RST_REQUEST);
+		writel(val, base + EQR_EYEQ6H_SARCR_CLK_REQUEST);
+		break;
+
+	default:
+		WARN_ON(1);
+		break;
+	}
+}
+
+static int eqr_assert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	struct eqr_private *priv = rcdev_to_priv(rcdev);
+	u32 domain = FIELD_GET(ID_DOMAIN_MASK, id);
+	u32 offset = FIELD_GET(ID_OFFSET_MASK, id);
+
+	dev_dbg(rcdev->dev, "%u-%u: assert request\n", domain, offset);
+
+	guard(mutex)(&priv->mutexes[domain]);
+	eqr_assert_locked(priv, domain, offset);
+	return eqr_busy_wait_locked(priv, rcdev->dev, domain, offset, true);
+}
+
+static void eqr_deassert_locked(struct eqr_private *priv, u32 domain,
+				u32 offset)
+{
+	enum eqr_domain_type domain_type = priv->data->domains[domain].type;
+	void __iomem *base, *reg;
+	u32 val;
+
+	lockdep_assert_held(&priv->mutexes[domain]);
+
+	base = priv->bases[domain];
+
+	switch (domain_type) {
+	case EQR_EYEQ5_SARCR:
+		reg = base + EQR_EYEQ5_SARCR_REQUEST;
+		writel(readl(reg) | BIT(offset), reg);
+		break;
+
+	case EQR_EYEQ5_ACRP:
+		reg = base + 4 * offset;
+		writel(readl(reg) & ~EQR_EYEQ5_ACRP_PD_REQ, reg);
+		break;
+
+	case EQR_EYEQ5_PCIE:
+		writel(readl(base) | BIT(offset), base);
+		break;
+
+	case EQR_EYEQ6H_SARCR:
+		val = readl(base + EQR_EYEQ6H_SARCR_RST_REQUEST);
+		val |= BIT(offset);
+		writel(val, base + EQR_EYEQ6H_SARCR_RST_REQUEST);
+		writel(val, base + EQR_EYEQ6H_SARCR_CLK_REQUEST);
+		break;
+
+	default:
+		WARN_ON(1);
+		break;
+	}
+}
+
+static int eqr_deassert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	struct eqr_private *priv = rcdev_to_priv(rcdev);
+	u32 domain = FIELD_GET(ID_DOMAIN_MASK, id);
+	u32 offset = FIELD_GET(ID_OFFSET_MASK, id);
+
+	dev_dbg(rcdev->dev, "%u-%u: deassert request\n", domain, offset);
+
+	guard(mutex)(&priv->mutexes[domain]);
+	eqr_deassert_locked(priv, domain, offset);
+	return eqr_busy_wait_locked(priv, rcdev->dev, domain, offset, false);
+}
+
+static int eqr_status(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	u32 domain = FIELD_GET(ID_DOMAIN_MASK, id);
+	struct eqr_private *priv = rcdev_to_priv(rcdev);
+	enum eqr_domain_type domain_type = priv->data->domains[domain].type;
+	u32 offset = FIELD_GET(ID_OFFSET_MASK, id);
+	void __iomem *base, *reg;
+
+	dev_dbg(rcdev->dev, "%u-%u: status request\n", domain, offset);
+
+	guard(mutex)(&priv->mutexes[domain]);
+
+	base = priv->bases[domain];
+
+	switch (domain_type) {
+	case EQR_EYEQ5_SARCR:
+		reg = base + EQR_EYEQ5_SARCR_STATUS;
+		return !(readl(reg) & BIT(offset));
+	case EQR_EYEQ5_ACRP:
+		reg = base + 4 * offset;
+		return !(readl(reg) & EQR_EYEQ5_ACRP_ST_ACTIVE);
+	case EQR_EYEQ5_PCIE:
+		return !(readl(base) & BIT(offset));
+	case EQR_EYEQ6H_SARCR:
+		reg = base + EQR_EYEQ6H_SARCR_RST_STATUS;
+		return !(readl(reg) & BIT(offset));
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct reset_control_ops eqr_ops = {
+	.assert	  = eqr_assert,
+	.deassert = eqr_deassert,
+	.status	  = eqr_status,
+};
+
+static int eqr_of_xlate(struct reset_controller_dev *rcdev,
+			const struct of_phandle_args *reset_spec)
+{
+	struct eqr_private *priv = rcdev_to_priv(rcdev);
+	u32 domain, offset;
+
+	/* Args count is expected to be 1 iff domain count is 1. */
+	if (reset_spec->args_count == 1) {
+		WARN_ON(priv->data->domain_count > 1);
+		domain = 0;
+		offset = reset_spec->args[0];
+	} else if (reset_spec->args_count == 2) {
+		WARN_ON(priv->data->domain_count < 2);
+		domain = reset_spec->args[0];
+		offset = reset_spec->args[1];
+	} else {
+		return -EINVAL;
+	}
+
+	if (domain >= priv->data->domain_count || offset > 31 ||
+	    !(priv->data->domains[domain].valid_mask & BIT(offset))) {
+		dev_err(rcdev->dev, "%u-%u: invalid reset\n", domain, offset);
+		return -EINVAL;
+	}
+
+	return FIELD_PREP(ID_DOMAIN_MASK, domain) | FIELD_PREP(ID_OFFSET_MASK, offset);
+}
+
+static int eqr_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct eqr_private *priv;
+	unsigned int i;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->data = device_get_match_data(dev);
+	if (!priv->data)
+		return -ENODEV;
+
+	priv->rcdev.ops = &eqr_ops;
+	priv->rcdev.owner = THIS_MODULE;
+	priv->rcdev.dev = dev;
+	priv->rcdev.of_node = np;
+	priv->rcdev.of_xlate = eqr_of_xlate;
+
+	if (priv->data->domain_count == 1) {
+		priv->rcdev.of_reset_n_cells = 1;
+
+		/* Single domain means single unnamed resource. Ignore resource name. */
+		priv->bases[0] = devm_platform_ioremap_resource(pdev, 0);
+		if (IS_ERR(priv->bases[0]))
+			return PTR_ERR(priv->bases[0]);
+	} else {
+		priv->rcdev.of_reset_n_cells = 2;
+
+		/* Multiple domains means named resources. */
+		for (i = 0; i < priv->data->domain_count; i++) {
+			const char *res_name = priv->data->domains[i].resource_name;
+
+			priv->bases[i] = devm_platform_ioremap_resource_byname(pdev, res_name);
+			if (IS_ERR(priv->bases[i]))
+				return PTR_ERR(priv->bases[i]);
+		}
+	}
+
+	for (i = 0; i < priv->data->domain_count; i++)
+		mutex_init(&priv->mutexes[i]);
+
+	priv->rcdev.nr_resets = 0;
+	for (i = 0; i < priv->data->domain_count; i++)
+		priv->rcdev.nr_resets += hweight32(priv->data->domains[i].valid_mask);
+
+	ret = devm_reset_controller_register(dev, &priv->rcdev);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed registering reset controller\n");
+
+	return 0;
+}
+
+static const struct eqr_domain_descriptor eqr_eyeq5_domains[] = {
+	{
+		.resource_name = "d0",
+		.type = EQR_EYEQ5_SARCR,
+		.valid_mask = 0xFFFFFF8,
+	},
+	{
+		.resource_name = "d1",
+		.type = EQR_EYEQ5_ACRP,
+		.valid_mask = 0x0001FFF,
+	},
+	{
+		.resource_name = "d2",
+		.type = EQR_EYEQ5_PCIE,
+		.valid_mask = 0x007BFFF,
+	},
+};
+
+static const struct eqr_match_data eqr_eyeq5_data = {
+	.domain_count	= ARRAY_SIZE(eqr_eyeq5_domains),
+	.domains	= eqr_eyeq5_domains,
+};
+
+static const struct eqr_domain_descriptor eqr_eyeq6l_domains[] = {
+	{
+		.resource_name = "d0",
+		.type = EQR_EYEQ5_SARCR,
+		.valid_mask = 0x3FFF,
+	},
+	{
+		.resource_name = "d1",
+		.type = EQR_EYEQ5_ACRP,
+		.valid_mask = 0x00FF,
+	},
+};
+
+static const struct eqr_match_data eqr_eyeq6l_data = {
+	.domain_count	= ARRAY_SIZE(eqr_eyeq6l_domains),
+	.domains	= eqr_eyeq6l_domains,
+};
+
+/* West and east OLBs each have an instance. */
+static const struct eqr_domain_descriptor eqr_eyeq6h_we_domains[] = {
+	{
+		.type = EQR_EYEQ6H_SARCR,
+		.valid_mask = 0x1F7F,
+	},
+};
+
+static const struct eqr_match_data eqr_eyeq6h_we_data = {
+	.domain_count	= ARRAY_SIZE(eqr_eyeq6h_we_domains),
+	.domains	= eqr_eyeq6h_we_domains,
+};
+
+static const struct eqr_domain_descriptor eqr_eyeq6h_acc_domains[] = {
+	{
+		.type = EQR_EYEQ5_ACRP,
+		.valid_mask = 0x7FFF,
+	},
+};
+
+static const struct eqr_match_data eqr_eyeq6h_acc_data = {
+	.domain_count	= ARRAY_SIZE(eqr_eyeq6h_acc_domains),
+	.domains	= eqr_eyeq6h_acc_domains,
+};
+
+static const struct of_device_id eqr_match_table[] = {
+	{ .compatible = "mobileye,eyeq5-reset", .data = &eqr_eyeq5_data },
+	{ .compatible = "mobileye,eyeq6l-reset", .data = &eqr_eyeq6l_data },
+	{ .compatible = "mobileye,eyeq6h-we-reset", .data = &eqr_eyeq6h_we_data },
+	{ .compatible = "mobileye,eyeq6h-acc-reset", .data = &eqr_eyeq6h_acc_data },
+	{}
+};
+
+static struct platform_driver eqr_driver = {
+	.probe = eqr_probe,
+	.driver = {
+		.name = "eyeq5-reset",
+		.of_match_table = eqr_match_table,
+	},
+};
+builtin_platform_driver(eqr_driver);

-- 
2.44.0

[PATCH 07/11] pinctrl: eyeq5: add platform driver

[Théo Lebrun]

Add the Mobileye EyeQ5 pin controller driver. It belongs to a syscon
region called OLB.

Existing pins and their function live statically in the driver code
rather than in the devicetree, see compatible match data.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 MAINTAINERS                     |   2 +
 drivers/pinctrl/Kconfig         |  14 +
 drivers/pinctrl/Makefile        |   1 +
 drivers/pinctrl/pinctrl-eyeq5.c | 579 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 596 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 724f20ea0411..5cede8039c1b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14927,12 +14927,14 @@ L:	linux-mips@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml
 F:	Documentation/devicetree/bindings/mips/mobileye.yaml
+F:	Documentation/devicetree/bindings/pinctrl/mobileye,eyeq5-pinctrl.yaml
 F:	Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml
 F:	Documentation/devicetree/bindings/soc/mobileye/
 F:	arch/mips/boot/dts/mobileye/
 F:	arch/mips/configs/eyeq5_defconfig
 F:	arch/mips/mobileye/board-epm5.its.S
 F:	drivers/clk/clk-eyeq5.c
+F:	drivers/pinctrl/pinctrl-eyeq5.c
 F:	drivers/reset/reset-eyeq5.c
 F:	include/dt-bindings/clock/mobileye,eyeq5-clk.h
 
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index d45657aa986a..b266326372e2 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -213,6 +213,20 @@ config PINCTRL_EQUILIBRIUM
 	  desired pin functions, configure GPIO attributes for LGM SoC pins.
 	  Pin muxing and pin config settings are retrieved from device tree.
 
+config PINCTRL_EYEQ5
+	bool "Mobileye EyeQ5 pinctrl driver"
+	depends on MACH_EYEQ5 || COMPILE_TEST
+	select PINMUX
+	select GENERIC_PINCONF
+	select MFD_SYSCON
+	default MACH_EYEQ5
+	help
+	  Pin controller driver for the Mobileye EyeQ5 platform. It does both
+	  pin config & pin muxing. It does not handle GPIO.
+
+	  Pin muxing supports two functions for each pin: first is GPIO, second
+	  is pin-dependent. Pin config is about bias & drive strength.
+
 config PINCTRL_GEMINI
 	bool
 	depends on ARCH_GEMINI
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index 2152539b53d5..6113e980cdb2 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_PINCTRL_DA850_PUPD) += pinctrl-da850-pupd.o
 obj-$(CONFIG_PINCTRL_DA9062)	+= pinctrl-da9062.o
 obj-$(CONFIG_PINCTRL_DIGICOLOR)	+= pinctrl-digicolor.o
 obj-$(CONFIG_PINCTRL_EQUILIBRIUM)   += pinctrl-equilibrium.o
+obj-$(CONFIG_PINCTRL_EYEQ5)	+= pinctrl-eyeq5.o
 obj-$(CONFIG_PINCTRL_GEMINI)	+= pinctrl-gemini.o
 obj-$(CONFIG_PINCTRL_INGENIC)	+= pinctrl-ingenic.o
 obj-$(CONFIG_PINCTRL_K210)	+= pinctrl-k210.o
diff --git a/drivers/pinctrl/pinctrl-eyeq5.c b/drivers/pinctrl/pinctrl-eyeq5.c
new file mode 100644
index 000000000000..3228fc2a026e
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-eyeq5.c
@@ -0,0 +1,579 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Pinctrl driver for the Mobileye EyeQ5 platform.
+ *
+ * The registers are located in a syscon region called OLB. There are two pin
+ * banks, each being controlled by 5 registers (see enum eq5p_regs) for
+ * pull-down, pull-up, drive strength and muxing.
+ *
+ * For each pin, muxing is between two functions: (0) GPIO or (1) another one
+ * that is pin-dependent. Functions are declared statically in this driver.
+ *
+ * We create pinctrl groups that are 1:1 equivalent to pins: each group has a
+ * single pin, and its index/selector is the pin number.
+ *
+ * We use eq5p_ as prefix, as-in "EyeQ5 Pinctrl", but way shorter.
+ *
+ * Copyright (C) 2024 Mobileye Vision Technologies Ltd.
+ */
+#include <linux/array_size.h>
+#include <linux/bits.h>
+#include <linux/bug.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/io.h>
+#include <linux/mod_devicetable.h>
+#include <linux/platform_device.h>
+#include <linux/seq_file.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+
+#include "core.h"
+#include "pinctrl-utils.h"
+
+struct eq5p_pinctrl {
+	struct pinctrl_desc	desc;
+	void __iomem		*base;
+};
+
+enum eq5p_bank {
+	EQ5P_BANK_A,
+	EQ5P_BANK_B,
+
+	EQ5P_BANK_COUNT,
+};
+
+enum eq5p_regs {
+	EQ5P_PD,
+	EQ5P_PU,
+	EQ5P_DS_LOW,
+	EQ5P_DS_HIGH,
+	EQ5P_IOCR,
+
+	EQ5P_REG_COUNT,
+};
+
+static const unsigned int eq5p_regs[EQ5P_BANK_COUNT][EQ5P_REG_COUNT] = {
+	[EQ5P_BANK_A] = {0x010, 0x014, 0x020, 0x024, 0x000},
+	[EQ5P_BANK_B] = {0x018, 0x01C, 0x028, 0x02C, 0x004},
+};
+
+/*
+ * Drive strength; two bits per pin.
+ */
+#define EQ5P_DS_MASK	GENMASK(1, 0)
+
+/*
+ * Comments to the right of each pin are the "signal name" in the datasheet.
+ */
+static const struct pinctrl_pin_desc eq5p_pins[] = {
+	/* Bank A */
+	PINCTRL_PIN(0,  "PA0"),  /* A0_TIMER0_CK */
+	PINCTRL_PIN(1,  "PA1"),  /* A1_TIMER0_EOC */
+	PINCTRL_PIN(2,  "PA2"),  /* A2_TIMER1_CK */
+	PINCTRL_PIN(3,  "PA3"),  /* A3_TIMER1_EOC */
+	PINCTRL_PIN(4,  "PA4"),  /* A4_TIMER2_CK */
+	PINCTRL_PIN(5,  "PA5"),  /* A5_TIMER2_EOC */
+	PINCTRL_PIN(6,  "PA6"),  /* A6_TIMER5_EXT_INCAP1 */
+	PINCTRL_PIN(7,  "PA7"),  /* A7_TIMER5_EXT_INCAP2 */
+	PINCTRL_PIN(8,  "PA8"),  /* A8_TIMER5_EXT_OUTCMP1 */
+	PINCTRL_PIN(9,  "PA9"),  /* A9_TIMER5_EXT_OUTCMP2 */
+	PINCTRL_PIN(10, "PA10"), /* A10_UART_0_TX */
+	PINCTRL_PIN(11, "PA11"), /* A11_UART_0_RX */
+	PINCTRL_PIN(12, "PA12"), /* A12_UART_1_TX */
+	PINCTRL_PIN(13, "PA13"), /* A13_UART_1_RX */
+	PINCTRL_PIN(14, "PA14"), /* A14_CAN_0_TX */
+	PINCTRL_PIN(15, "PA15"), /* A15_CAN_0_RX */
+	PINCTRL_PIN(16, "PA16"), /* A16_CAN_1_TX */
+	PINCTRL_PIN(17, "PA17"), /* A17_CAN_1_RX */
+	PINCTRL_PIN(18, "PA18"), /* A18_SPI_0_DO */
+	PINCTRL_PIN(19, "PA19"), /* A19_SPI_0_DI */
+	PINCTRL_PIN(20, "PA20"), /* A20_SPI_0_CK */
+	PINCTRL_PIN(21, "PA21"), /* A21_SPI_0_CS0 */
+	PINCTRL_PIN(22, "PA22"), /* A22_SPI_0_CS1 */
+	PINCTRL_PIN(23, "PA23"), /* A23_SPI_1_DO */
+	PINCTRL_PIN(24, "PA24"), /* A24_SPI_1_DI */
+	PINCTRL_PIN(25, "PA25"), /* A25_SPI_1_CK */
+	PINCTRL_PIN(26, "PA26"), /* A26_SPI_1_CS0 */
+	PINCTRL_PIN(27, "PA27"), /* A27_SPI_1_CS1 */
+	PINCTRL_PIN(28, "PA28"), /* A28_REF_CLK0 */
+
+#define EQ5P_PIN_OFFSET_BANK_B	29
+
+	/* Bank B */
+	PINCTRL_PIN(EQ5P_PIN_OFFSET_BANK_B + 0,  "PB0"),  /* B0_TIMER3_CK */
+	PINCTRL_PIN(EQ5P_PIN_OFFSET_BANK_B + 1,  "PB1"),  /* B1_TIMER3_EOC */
+	PINCTRL_PIN(EQ5P_PIN_OFFSET_BANK_B + 2,  "PB2"),  /* B2_TIMER4_CK */
+	PINCTRL_PIN(EQ5P_PIN_OFFSET_BANK_B + 3,  "PB3"),  /* B3_TIMER4_EOC */
+	PINCTRL_PIN(EQ5P_PIN_OFFSET_BANK_B + 4,  "PB4"),  /* B4_TIMER6_EXT_INCAP1 */
+	PINCTRL_PIN(EQ5P_PIN_OFFSET_BANK_B + 5,  "PB5"),  /* B5_TIMER6_EXT_INCAP2 */
+	PINCTRL_PIN(EQ5P_PIN_OFFSET_BANK_B + 6,  "PB6"),  /* B6_TIMER6_EXT_OUTCMP1 */
+	PINCTRL_PIN(EQ5P_PIN_OFFSET_BANK_B + 7,  "PB7"),  /* B7_TIMER6_EXT_OUTCMP2 */
+	PINCTRL_PIN(EQ5P_PIN_OFFSET_BANK_B + 8,  "PB8"),  /* B8_UART_2_TX */
+	PINCTRL_PIN(EQ5P_PIN_OFFSET_BANK_B + 9,  "PB9"),  /* B9_UART_2_RX */
+	PINCTRL_PIN(EQ5P_PIN_OFFSET_BANK_B + 10, "PB10"), /* B10_CAN_2_TX */
+	PINCTRL_PIN(EQ5P_PIN_OFFSET_BANK_B + 11, "PB11"), /* B11_CAN_2_RX */
+	PINCTRL_PIN(EQ5P_PIN_OFFSET_BANK_B + 12, "PB12"), /* B12_SPI_2_DO */
+	PINCTRL_PIN(EQ5P_PIN_OFFSET_BANK_B + 13, "PB13"), /* B13_SPI_2_DI */
+	PINCTRL_PIN(EQ5P_PIN_OFFSET_BANK_B + 14, "PB14"), /* B14_SPI_2_CK */
+	PINCTRL_PIN(EQ5P_PIN_OFFSET_BANK_B + 15, "PB15"), /* B15_SPI_2_CS0 */
+	PINCTRL_PIN(EQ5P_PIN_OFFSET_BANK_B + 16, "PB16"), /* B16_SPI_2_CS1 */
+	PINCTRL_PIN(EQ5P_PIN_OFFSET_BANK_B + 17, "PB17"), /* B17_SPI_3_DO */
+	PINCTRL_PIN(EQ5P_PIN_OFFSET_BANK_B + 18, "PB18"), /* B18_SPI_3_DI */
+	PINCTRL_PIN(EQ5P_PIN_OFFSET_BANK_B + 19, "PB19"), /* B19_SPI_3_CK */
+	PINCTRL_PIN(EQ5P_PIN_OFFSET_BANK_B + 20, "PB20"), /* B20_SPI_3_CS0 */
+	PINCTRL_PIN(EQ5P_PIN_OFFSET_BANK_B + 21, "PB21"), /* B21_SPI_3_CS1 */
+	PINCTRL_PIN(EQ5P_PIN_OFFSET_BANK_B + 22, "PB22"), /* B22_MCLK0 */
+};
+
+static const char * const gpio_groups[] = {
+	/* Bank A */
+	"PA0",  "PA1",  "PA2",  "PA3",  "PA4",  "PA5",  "PA6",  "PA7",
+	"PA8",  "PA9",  "PA10", "PA11", "PA12", "PA13", "PA14", "PA15",
+	"PA16", "PA17", "PA18", "PA19", "PA20", "PA21", "PA22", "PA23",
+	"PA24", "PA25", "PA26", "PA27", "PA28",
+
+	/* Bank B */
+	"PB0",  "PB1",  "PB2",  "PB3",  "PB4",  "PB5",  "PB6",  "PB7",
+	"PB8",  "PB9",  "PB10", "PB11", "PB12", "PB13", "PB14", "PB15",
+	"PB16", "PB17", "PB18", "PB19", "PB20", "PB21", "PB22",
+};
+
+/* Groups of functions on bank A */
+static const char * const timer0_groups[] = { "PA0", "PA1" };
+static const char * const timer1_groups[] = { "PA2", "PA3" };
+static const char * const timer2_groups[] = { "PA4", "PA5" };
+static const char * const timer5_groups[] = { "PA6", "PA7", "PA8", "PA9" };
+static const char * const uart0_groups[] = { "PA10", "PA11" };
+static const char * const uart1_groups[] = { "PA12", "PA13" };
+static const char * const can0_groups[] = { "PA14", "PA15" };
+static const char * const can1_groups[] = { "PA16", "PA17" };
+static const char * const spi0_groups[] = { "PA18", "PA19", "PA20", "PA21", "PA22" };
+static const char * const spi1_groups[] = { "PA23", "PA24", "PA25", "PA26", "PA27" };
+static const char * const refclk0_groups[] = { "PA28" };
+
+/* Groups of functions on bank B */
+static const char * const timer3_groups[] = { "PB0", "PB1" };
+static const char * const timer4_groups[] = { "PB2", "PB3" };
+static const char * const timer6_groups[] = { "PB4", "PB5", "PB6", "PB7" };
+static const char * const uart2_groups[] = { "PB8", "PB9" };
+static const char * const can2_groups[] = { "PB10", "PB11" };
+static const char * const spi2_groups[] = { "PB12", "PB13", "PB14", "PB15", "PB16" };
+static const char * const spi3_groups[] = { "PB17", "PB18", "PB19", "PB20", "PB21" };
+static const char * const mclk0_groups[] = { "PB22" };
+
+static const struct pinfunction eq5p_functions[] = {
+	/* GPIO having a fixed index is depended upon, see GPIO_FUNC_SELECTOR. */
+	PINCTRL_PINFUNCTION("gpio", gpio_groups, ARRAY_SIZE(gpio_groups)),
+#define GPIO_FUNC_SELECTOR 0
+
+	/* Bank A functions */
+	PINCTRL_PINFUNCTION("timer0", timer0_groups, ARRAY_SIZE(timer0_groups)),
+	PINCTRL_PINFUNCTION("timer1", timer1_groups, ARRAY_SIZE(timer1_groups)),
+	PINCTRL_PINFUNCTION("timer2", timer2_groups, ARRAY_SIZE(timer2_groups)),
+	PINCTRL_PINFUNCTION("timer5", timer5_groups, ARRAY_SIZE(timer5_groups)),
+	PINCTRL_PINFUNCTION("uart0", uart0_groups, ARRAY_SIZE(uart0_groups)),
+	PINCTRL_PINFUNCTION("uart1", uart1_groups, ARRAY_SIZE(uart1_groups)),
+	PINCTRL_PINFUNCTION("can0", can0_groups, ARRAY_SIZE(can0_groups)),
+	PINCTRL_PINFUNCTION("can1", can1_groups, ARRAY_SIZE(can1_groups)),
+	PINCTRL_PINFUNCTION("spi0", spi0_groups, ARRAY_SIZE(spi0_groups)),
+	PINCTRL_PINFUNCTION("spi1", spi1_groups, ARRAY_SIZE(spi1_groups)),
+	PINCTRL_PINFUNCTION("refclk0", refclk0_groups, ARRAY_SIZE(refclk0_groups)),
+
+	/* Bank B functions */
+	PINCTRL_PINFUNCTION("timer3", timer3_groups, ARRAY_SIZE(timer3_groups)),
+	PINCTRL_PINFUNCTION("timer4", timer4_groups, ARRAY_SIZE(timer4_groups)),
+	PINCTRL_PINFUNCTION("timer6", timer6_groups, ARRAY_SIZE(timer6_groups)),
+	PINCTRL_PINFUNCTION("uart2", uart2_groups, ARRAY_SIZE(uart2_groups)),
+	PINCTRL_PINFUNCTION("can2", can2_groups, ARRAY_SIZE(can2_groups)),
+	PINCTRL_PINFUNCTION("spi2", spi2_groups, ARRAY_SIZE(spi2_groups)),
+	PINCTRL_PINFUNCTION("spi3", spi3_groups, ARRAY_SIZE(spi3_groups)),
+	PINCTRL_PINFUNCTION("mclk0", mclk0_groups, ARRAY_SIZE(mclk0_groups)),
+};
+
+static void eq5p_update_bits(const struct eq5p_pinctrl *pctrl,
+			     enum eq5p_bank bank, enum eq5p_regs reg,
+			     u32 mask, u32 val)
+{
+	void __iomem *ptr = pctrl->base + eq5p_regs[bank][reg];
+
+	writel((readl(ptr) & ~mask) | (val & mask), ptr);
+}
+
+static bool eq5p_test_bit(const struct eq5p_pinctrl *pctrl,
+			  enum eq5p_bank bank, enum eq5p_regs reg, int offset)
+{
+	u32 val = readl(pctrl->base + eq5p_regs[bank][reg]);
+
+	if (WARN_ON(offset > 31))
+		return false;
+
+	return (val & BIT(offset)) != 0;
+}
+
+static enum eq5p_bank eq5p_pin_to_bank(unsigned int pin)
+{
+	if (pin < EQ5P_PIN_OFFSET_BANK_B)
+		return EQ5P_BANK_A;
+	else
+		return EQ5P_BANK_B;
+}
+
+static unsigned int eq5p_pin_to_offset(unsigned int pin)
+{
+	if (pin < EQ5P_PIN_OFFSET_BANK_B)
+		return pin;
+	else
+		return pin - EQ5P_PIN_OFFSET_BANK_B;
+}
+
+static int eq5p_pinctrl_get_groups_count(struct pinctrl_dev *pctldev)
+{
+	return ARRAY_SIZE(eq5p_pins);
+}
+
+static const char *eq5p_pinctrl_get_group_name(struct pinctrl_dev *pctldev,
+					       unsigned int selector)
+{
+	return pctldev->desc->pins[selector].name;
+}
+
+static int eq5p_pinctrl_get_group_pins(struct pinctrl_dev *pctldev,
+				       unsigned int selector,
+				       const unsigned int **pins,
+				       unsigned int *num_pins)
+{
+	*pins = &pctldev->desc->pins[selector].number;
+	*num_pins = 1;
+	return 0;
+}
+
+static int eq5p_pinconf_get(struct pinctrl_dev *pctldev, unsigned int pin,
+			    unsigned long *config)
+{
+	enum pin_config_param param = pinconf_to_config_param(*config);
+	struct eq5p_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
+	unsigned int offset = eq5p_pin_to_offset(pin);
+	enum eq5p_bank bank = eq5p_pin_to_bank(pin);
+	u32 val_ds, arg;
+	bool pd, pu;
+
+	pd = eq5p_test_bit(pctrl, bank, EQ5P_PD, offset);
+	pu = eq5p_test_bit(pctrl, bank, EQ5P_PU, offset);
+
+	switch (param) {
+	case PIN_CONFIG_BIAS_DISABLE:
+		arg = !(pd || pu);
+		break;
+	case PIN_CONFIG_BIAS_PULL_DOWN:
+		arg = pd;
+		break;
+	case PIN_CONFIG_BIAS_PULL_UP:
+		arg = pu;
+		break;
+	case PIN_CONFIG_DRIVE_STRENGTH:
+		offset *= 2; /* two bits per pin */
+		if (offset >= 32) {
+			val_ds = readl(pctrl->base + eq5p_regs[bank][EQ5P_DS_HIGH]);
+			offset -= 32;
+		} else {
+			val_ds = readl(pctrl->base + eq5p_regs[bank][EQ5P_DS_LOW]);
+		}
+		arg = (val_ds >> offset) & EQ5P_DS_MASK;
+		break;
+	default:
+		return -ENOTSUPP;
+	}
+
+	*config = pinconf_to_config_packed(param, arg);
+	return 0;
+}
+
+static void eq5p_pinctrl_pin_dbg_show(struct pinctrl_dev *pctldev,
+				      struct seq_file *s,
+				      unsigned int pin)
+{
+	struct eq5p_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
+	const char *pin_name = pctrl->desc.pins[pin].name;
+	unsigned int offset = eq5p_pin_to_offset(pin);
+	enum eq5p_bank bank = eq5p_pin_to_bank(pin);
+	const char *func_name, *bias;
+	unsigned long ds_config;
+	u32 drive_strength;
+	bool pd, pu;
+	int i, j;
+
+	/*
+	 * First, let's get the function name. All pins have only two functions:
+	 * GPIO (IOCR == 0) and something else (IOCR == 1).
+	 */
+	if (eq5p_test_bit(pctrl, bank, EQ5P_IOCR, offset)) {
+		func_name = NULL;
+		for (i = 0; i < ARRAY_SIZE(eq5p_functions); i++) {
+			if (i == GPIO_FUNC_SELECTOR)
+				continue;
+
+			for (j = 0; j < eq5p_functions[i].ngroups; j++) {
+				/* Groups and pins are the same thing for us. */
+				const char *x = eq5p_functions[i].groups[j];
+
+				if (strcmp(x, pin_name) == 0) {
+					func_name = eq5p_functions[i].name;
+					break;
+				}
+			}
+
+			if (func_name)
+				break;
+		}
+
+		/*
+		 * We have not found the function attached to this pin, this
+		 * should never occur as all pins have exactly two functions.
+		 */
+		if (!func_name)
+			func_name = "unknown";
+	} else {
+		func_name = eq5p_functions[GPIO_FUNC_SELECTOR].name;
+	}
+
+	/* Second, we retrieve the bias. */
+	pd = eq5p_test_bit(pctrl, bank, EQ5P_PD, offset);
+	pu = eq5p_test_bit(pctrl, bank, EQ5P_PU, offset);
+	if (pd && pu)
+		bias = "both";
+	else if (pd && !pu)
+		bias = "pulldown";
+	else if (!pd && pu)
+		bias = "pullup";
+	else
+		bias = "none";
+
+	/* Third, we get the drive strength. */
+	ds_config = pinconf_to_config_packed(PIN_CONFIG_DRIVE_STRENGTH, 0);
+	eq5p_pinconf_get(pctldev, pin, &ds_config);
+	drive_strength = pinconf_to_config_argument(ds_config);
+
+	seq_printf(s, "function=%s bias=%s drive_strength=%d",
+		   func_name, bias, drive_strength);
+}
+
+static const struct pinctrl_ops eq5p_pinctrl_ops = {
+	.get_groups_count	= eq5p_pinctrl_get_groups_count,
+	.get_group_name		= eq5p_pinctrl_get_group_name,
+	.get_group_pins		= eq5p_pinctrl_get_group_pins,
+	.pin_dbg_show		= eq5p_pinctrl_pin_dbg_show,
+	.dt_node_to_map		= pinconf_generic_dt_node_to_map_pin,
+	.dt_free_map		= pinctrl_utils_free_map,
+};
+
+static int eq5p_pinmux_get_functions_count(struct pinctrl_dev *pctldev)
+{
+	return ARRAY_SIZE(eq5p_functions);
+}
+
+static const char *eq5p_pinmux_get_function_name(struct pinctrl_dev *pctldev,
+						 unsigned int selector)
+{
+	return eq5p_functions[selector].name;
+}
+
+static int eq5p_pinmux_get_function_groups(struct pinctrl_dev *pctldev,
+					   unsigned int selector,
+					   const char * const **groups,
+					   unsigned int *num_groups)
+{
+	*groups = eq5p_functions[selector].groups;
+	*num_groups = eq5p_functions[selector].ngroups;
+	return 0;
+}
+
+static int eq5p_pinmux_set_mux(struct pinctrl_dev *pctldev,
+			       unsigned int func_selector, unsigned int pin)
+{
+	struct eq5p_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
+	const char *func_name = eq5p_functions[func_selector].name;
+	const char *group_name = pctldev->desc->pins[pin].name;
+	bool is_gpio = func_selector == GPIO_FUNC_SELECTOR;
+	unsigned int offset = eq5p_pin_to_offset(pin);
+	enum eq5p_bank bank = eq5p_pin_to_bank(pin);
+	u32 mask, val;
+
+	dev_dbg(pctldev->dev, "func=%s group=%s\n", func_name, group_name);
+
+	mask = BIT(offset);
+	val = is_gpio ? 0 : mask;
+	eq5p_update_bits(pctrl, bank, EQ5P_IOCR, mask, val);
+	return 0;
+}
+
+static int eq5p_pinmux_gpio_request_enable(struct pinctrl_dev *pctldev,
+					   struct pinctrl_gpio_range *range,
+					   unsigned int pin)
+{
+	/* Pin numbers and group selectors are the same thing in our case. */
+	return eq5p_pinmux_set_mux(pctldev, GPIO_FUNC_SELECTOR, pin);
+}
+
+static const struct pinmux_ops eq5p_pinmux_ops = {
+	.get_functions_count	= eq5p_pinmux_get_functions_count,
+	.get_function_name	= eq5p_pinmux_get_function_name,
+	.get_function_groups	= eq5p_pinmux_get_function_groups,
+	.set_mux		= eq5p_pinmux_set_mux,
+	.gpio_request_enable	= eq5p_pinmux_gpio_request_enable,
+	.strict			= true,
+};
+
+static int eq5p_pinconf_set_drive_strength(struct pinctrl_dev *pctldev,
+					   unsigned int pin, u32 arg)
+{
+	struct eq5p_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
+	unsigned int offset = eq5p_pin_to_offset(pin);
+	enum eq5p_bank bank = eq5p_pin_to_bank(pin);
+	unsigned int reg;
+	u32 mask, val;
+
+	if (arg & ~EQ5P_DS_MASK) {
+		dev_err(pctldev->dev, "Unsupported drive strength: %u\n", arg);
+		return -EINVAL;
+	}
+
+	offset *= 2; /* two bits per pin */
+
+	if (offset >= 32) {
+		reg = EQ5P_DS_HIGH;
+		offset -= 32;
+	} else {
+		reg = EQ5P_DS_LOW;
+	}
+
+	mask = EQ5P_DS_MASK << offset;
+	val = arg << offset;
+	eq5p_update_bits(pctrl, bank, reg, mask, val);
+	return 0;
+}
+
+static int eq5p_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
+			    unsigned long *configs, unsigned int num_configs)
+{
+	struct eq5p_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
+	const char *pin_name = pctldev->desc->pins[pin].name;
+	unsigned int offset = eq5p_pin_to_offset(pin);
+	enum eq5p_bank bank = eq5p_pin_to_bank(pin);
+	struct device *dev = pctldev->dev;
+	u32 val = BIT(offset);
+	unsigned int i;
+
+	for (i = 0; i < num_configs; i++) {
+		enum pin_config_param param = pinconf_to_config_param(configs[i]);
+		u32 arg = pinconf_to_config_argument(configs[i]);
+
+		switch (param) {
+		case PIN_CONFIG_BIAS_DISABLE:
+			dev_dbg(dev, "pin=%s bias_disable\n", pin_name);
+
+			eq5p_update_bits(pctrl, bank, EQ5P_PD, val, 0);
+			eq5p_update_bits(pctrl, bank, EQ5P_PU, val, 0);
+			break;
+
+		case PIN_CONFIG_BIAS_PULL_DOWN:
+			dev_dbg(dev, "pin=%s bias_pull_down arg=%u\n",
+				pin_name, arg);
+
+			if (arg == 0) /* cannot connect to GND */
+				return -ENOTSUPP;
+
+			eq5p_update_bits(pctrl, bank, EQ5P_PD, val, val);
+			eq5p_update_bits(pctrl, bank, EQ5P_PU, val, 0);
+			break;
+
+		case PIN_CONFIG_BIAS_PULL_UP:
+			dev_dbg(dev, "pin=%s bias_pull_up arg=%u\n",
+				pin_name, arg);
+
+			if (arg == 0) /* cannot connect to VDD */
+				return -ENOTSUPP;
+
+			eq5p_update_bits(pctrl, bank, EQ5P_PD, val, 0);
+			eq5p_update_bits(pctrl, bank, EQ5P_PU, val, val);
+			break;
+
+		case PIN_CONFIG_DRIVE_STRENGTH:
+			dev_dbg(dev, "pin=%s drive_strength arg=%u\n",
+				pin_name, arg);
+
+			eq5p_pinconf_set_drive_strength(pctldev, pin, arg);
+			break;
+
+		default:
+			dev_err(dev, "Unsupported pinconf %u\n", param);
+			return -ENOTSUPP;
+		}
+	}
+
+	return 0;
+}
+
+static const struct pinconf_ops eq5p_pinconf_ops = {
+	.is_generic = true,
+	.pin_config_get = eq5p_pinconf_get,
+	.pin_config_set = eq5p_pinconf_set,
+	/* Pins and groups are equivalent in this driver. */
+	.pin_config_group_get = eq5p_pinconf_get,
+	.pin_config_group_set = eq5p_pinconf_set,
+};
+
+static int eq5p_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct pinctrl_dev *pctldev;
+	struct eq5p_pinctrl *pctrl;
+	int ret;
+
+	pctrl = devm_kzalloc(dev, sizeof(*pctrl), GFP_KERNEL);
+	if (!pctrl)
+		return -ENOMEM;
+
+	pctrl->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(pctrl->base))
+		return PTR_ERR(pctrl->base);
+
+	pctrl->desc.name = dev_name(dev);
+	pctrl->desc.pins = eq5p_pins;
+	pctrl->desc.npins = ARRAY_SIZE(eq5p_pins);
+	pctrl->desc.pctlops = &eq5p_pinctrl_ops;
+	pctrl->desc.pmxops = &eq5p_pinmux_ops;
+	pctrl->desc.confops = &eq5p_pinconf_ops;
+	pctrl->desc.owner = THIS_MODULE;
+
+	ret = devm_pinctrl_register_and_init(dev, &pctrl->desc, pctrl, &pctldev);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed registering pinctrl device\n");
+
+	ret = pinctrl_enable(pctldev);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed enabling pinctrl device\n");
+
+	return 0;
+}
+
+static const struct of_device_id eq5p_match[] = {
+	{ .compatible = "mobileye,eyeq5-pinctrl" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, eq5p_match);
+
+static struct platform_driver eq5p_driver = {
+	.driver = {
+		.name = "eyeq5-pinctrl",
+		.of_match_table = eq5p_match,
+	},
+	.probe = eq5p_probe,
+};
+builtin_platform_driver(eq5p_driver);

-- 
2.44.0

[PATCH 08/11] MIPS: mobileye: eyeq5: add OLB syscon node

[Théo Lebrun]

The OLB ("Other Logic Block") is a syscon region hosting clock, reset
and pin controllers. It contains registers such as I2C speed mode that
need to be accessible by other nodes.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 arch/mips/boot/dts/mobileye/eyeq5.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/mips/boot/dts/mobileye/eyeq5.dtsi b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
index 6cc5980e2fa1..e82d2a57f6da 100644
--- a/arch/mips/boot/dts/mobileye/eyeq5.dtsi
+++ b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
@@ -100,6 +100,14 @@ uart2: serial@a00000 {
 			clock-names = "uartclk", "apb_pclk";
 		};
 
+		olb: system-controller@e00000 {
+			compatible = "mobileye,eyeq5-olb", "syscon", "simple-mfd";
+			reg = <0 0xe00000 0x0 0x400>;
+			ranges = <0x0 0x0 0xe00000 0x400>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+		};
+
 		gic: interrupt-controller@140000 {
 			compatible = "mti,gic";
 			reg = <0x0 0x140000 0x0 0x20000>;

-- 
2.44.0

Re: [PATCH 08/11] MIPS: mobileye: eyeq5: add OLB syscon node

[Krzysztof Kozlowski]

On 10/04/2024 19:12, Théo Lebrun wrote:
> The OLB ("Other Logic Block") is a syscon region hosting clock, reset
> and pin controllers. It contains registers such as I2C speed mode that
> need to be accessible by other nodes.
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  arch/mips/boot/dts/mobileye/eyeq5.dtsi | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/mips/boot/dts/mobileye/eyeq5.dtsi b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
> index 6cc5980e2fa1..e82d2a57f6da 100644
> --- a/arch/mips/boot/dts/mobileye/eyeq5.dtsi
> +++ b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
> @@ -100,6 +100,14 @@ uart2: serial@a00000 {
>  			clock-names = "uartclk", "apb_pclk";
>  		};
>  
> +		olb: system-controller@e00000 {
> +			compatible = "mobileye,eyeq5-olb", "syscon", "simple-mfd";
> +			reg = <0 0xe00000 0x0 0x400>;
> +			ranges = <0x0 0x0 0xe00000 0x400>;
> +			#address-cells = <1>;
> +			#size-cells = <1>;

Do not add incomplete node. ranges, address/size-cells are incorrect in
this context and you will have warnings.

Add complete node, so these properties make sense.

Best regards,
Krzysztof

Re: [PATCH 08/11] MIPS: mobileye: eyeq5: add OLB syscon node

[Théo Lebrun]

Hello,

On Thu Apr 11, 2024 at 8:15 AM CEST, Krzysztof Kozlowski wrote:
> On 10/04/2024 19:12, Théo Lebrun wrote:
> > The OLB ("Other Logic Block") is a syscon region hosting clock, reset
> > and pin controllers. It contains registers such as I2C speed mode that
> > need to be accessible by other nodes.
> > 
> > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> > ---
> >  arch/mips/boot/dts/mobileye/eyeq5.dtsi | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/arch/mips/boot/dts/mobileye/eyeq5.dtsi b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
> > index 6cc5980e2fa1..e82d2a57f6da 100644
> > --- a/arch/mips/boot/dts/mobileye/eyeq5.dtsi
> > +++ b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
> > @@ -100,6 +100,14 @@ uart2: serial@a00000 {
> >  			clock-names = "uartclk", "apb_pclk";
> >  		};
> >  
> > +		olb: system-controller@e00000 {
> > +			compatible = "mobileye,eyeq5-olb", "syscon", "simple-mfd";
> > +			reg = <0 0xe00000 0x0 0x400>;
> > +			ranges = <0x0 0x0 0xe00000 0x400>;
> > +			#address-cells = <1>;
> > +			#size-cells = <1>;
>
> Do not add incomplete node. ranges, address/size-cells are incorrect in
> this context and you will have warnings.
>
> Add complete node, so these properties make sense.

I'll squash all four commits into one. For reference, commits are:

 - MIPS: mobileye: eyeq5: add OLB syscon node
 - MIPS: mobileye: eyeq5: use OLB clocks controller node
 - MIPS: mobileye: eyeq5: add OLB reset controller node
 - MIPS: mobileye: eyeq5: add pinctrl node & pinmux function nodes

This means two things: (1) it won't be partially applicable and (2) it
will make one big commit adding pins and editing clocks.

Thanks,

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

Re: [PATCH 08/11] MIPS: mobileye: eyeq5: add OLB syscon node

[Krzysztof Kozlowski]

On 11/04/2024 16:34, Théo Lebrun wrote:
> Hello,
> 
> On Thu Apr 11, 2024 at 8:15 AM CEST, Krzysztof Kozlowski wrote:
>> On 10/04/2024 19:12, Théo Lebrun wrote:
>>> The OLB ("Other Logic Block") is a syscon region hosting clock, reset
>>> and pin controllers. It contains registers such as I2C speed mode that
>>> need to be accessible by other nodes.
>>>
>>> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
>>> ---
>>>  arch/mips/boot/dts/mobileye/eyeq5.dtsi | 8 ++++++++
>>>  1 file changed, 8 insertions(+)
>>>
>>> diff --git a/arch/mips/boot/dts/mobileye/eyeq5.dtsi b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
>>> index 6cc5980e2fa1..e82d2a57f6da 100644
>>> --- a/arch/mips/boot/dts/mobileye/eyeq5.dtsi
>>> +++ b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
>>> @@ -100,6 +100,14 @@ uart2: serial@a00000 {
>>>  			clock-names = "uartclk", "apb_pclk";
>>>  		};
>>>  
>>> +		olb: system-controller@e00000 {
>>> +			compatible = "mobileye,eyeq5-olb", "syscon", "simple-mfd";
>>> +			reg = <0 0xe00000 0x0 0x400>;
>>> +			ranges = <0x0 0x0 0xe00000 0x400>;
>>> +			#address-cells = <1>;
>>> +			#size-cells = <1>;
>>
>> Do not add incomplete node. ranges, address/size-cells are incorrect in
>> this context and you will have warnings.
>>
>> Add complete node, so these properties make sense.
> 
> I'll squash all four commits into one. For reference, commits are:
> 
>  - MIPS: mobileye: eyeq5: add OLB syscon node
>  - MIPS: mobileye: eyeq5: use OLB clocks controller node
>  - MIPS: mobileye: eyeq5: add OLB reset controller node
>  - MIPS: mobileye: eyeq5: add pinctrl node & pinmux function nodes
> 
> This means two things: (1) it won't be partially applicable and (2) it

Why?

> will make one big commit adding pins and editing clocks.

It never was partially applicable. Causing warnings does not make things
partially applicable. If node is too big, although I personally do not
agree, it's quite moderate size chunk, then sure, split pinctrl groups
or pinctrl node to additional patch.

Best regards,
Krzysztof

Re: [PATCH 08/11] MIPS: mobileye: eyeq5: add OLB syscon node

[Théo Lebrun]

Hello,

On Thu Apr 11, 2024 at 5:07 PM CEST, Krzysztof Kozlowski wrote:
> On 11/04/2024 16:34, Théo Lebrun wrote:
> > Hello,
> > 
> > On Thu Apr 11, 2024 at 8:15 AM CEST, Krzysztof Kozlowski wrote:
> >> On 10/04/2024 19:12, Théo Lebrun wrote:
> >>> The OLB ("Other Logic Block") is a syscon region hosting clock, reset
> >>> and pin controllers. It contains registers such as I2C speed mode that
> >>> need to be accessible by other nodes.
> >>>
> >>> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> >>> ---
> >>>  arch/mips/boot/dts/mobileye/eyeq5.dtsi | 8 ++++++++
> >>>  1 file changed, 8 insertions(+)
> >>>
> >>> diff --git a/arch/mips/boot/dts/mobileye/eyeq5.dtsi b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
> >>> index 6cc5980e2fa1..e82d2a57f6da 100644
> >>> --- a/arch/mips/boot/dts/mobileye/eyeq5.dtsi
> >>> +++ b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
> >>> @@ -100,6 +100,14 @@ uart2: serial@a00000 {
> >>>  			clock-names = "uartclk", "apb_pclk";
> >>>  		};
> >>>  
> >>> +		olb: system-controller@e00000 {
> >>> +			compatible = "mobileye,eyeq5-olb", "syscon", "simple-mfd";
> >>> +			reg = <0 0xe00000 0x0 0x400>;
> >>> +			ranges = <0x0 0x0 0xe00000 0x400>;
> >>> +			#address-cells = <1>;
> >>> +			#size-cells = <1>;
> >>
> >> Do not add incomplete node. ranges, address/size-cells are incorrect in
> >> this context and you will have warnings.
> >>
> >> Add complete node, so these properties make sense.
> > 
> > I'll squash all four commits into one. For reference, commits are:
> > 
> >  - MIPS: mobileye: eyeq5: add OLB syscon node
> >  - MIPS: mobileye: eyeq5: use OLB clocks controller node
> >  - MIPS: mobileye: eyeq5: add OLB reset controller node
> >  - MIPS: mobileye: eyeq5: add pinctrl node & pinmux function nodes
> > 
> > This means two things: (1) it won't be partially applicable and (2) it
>
> Why?
>
> > will make one big commit adding pins and editing clocks.
>
> It never was partially applicable. Causing warnings does not make things
> partially applicable. If node is too big, although I personally do not
> agree, it's quite moderate size chunk, then sure, split pinctrl groups
> or pinctrl node to additional patch.

Thanks for feedback; it'll become a single patch as it is fine with you.

Regards,

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

[PATCH 09/11] MIPS: mobileye: eyeq5: use OLB clocks controller node

[Théo Lebrun]

We add the clock controller inside the OLB syscon region and remove
previous fixed devicetree nodes representing PLLs exposed by the clock
controller.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 .../{eyeq5-fixed-clocks.dtsi => eyeq5-clocks.dtsi} | 54 +++++++---------------
 arch/mips/boot/dts/mobileye/eyeq5.dtsi             | 11 ++++-
 2 files changed, 26 insertions(+), 39 deletions(-)

diff --git a/arch/mips/boot/dts/mobileye/eyeq5-fixed-clocks.dtsi b/arch/mips/boot/dts/mobileye/eyeq5-clocks.dtsi
similarity index 88%
rename from arch/mips/boot/dts/mobileye/eyeq5-fixed-clocks.dtsi
rename to arch/mips/boot/dts/mobileye/eyeq5-clocks.dtsi
index 78f5533a95c6..aa6db704a786 100644
--- a/arch/mips/boot/dts/mobileye/eyeq5-fixed-clocks.dtsi
+++ b/arch/mips/boot/dts/mobileye/eyeq5-clocks.dtsi
@@ -3,42 +3,20 @@
  * Copyright 2023 Mobileye Vision Technologies Ltd.
  */
 
+#include <dt-bindings/clock/mobileye,eyeq5-clk.h>
+
 / {
 	/* Fixed clock */
-	pll_cpu: pll-cpu {
-		compatible = "fixed-clock";
-		#clock-cells = <0>;
-		clock-frequency = <1500000000>;
-	};
-
-	pll_vdi: pll-vdi {
-		compatible = "fixed-clock";
-		#clock-cells = <0>;
-		clock-frequency = <1280000000>;
-	};
-
-	pll_per: pll-per {
-		compatible = "fixed-clock";
-		#clock-cells = <0>;
-		clock-frequency = <2000000000>;
-	};
-
-	pll_ddr0: pll-ddr0 {
-		compatible = "fixed-clock";
-		#clock-cells = <0>;
-		clock-frequency = <1857210000>;
-	};
-
-	pll_ddr1: pll-ddr1 {
+	xtal: xtal {
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
-		clock-frequency = <1857210000>;
+		clock-frequency = <30000000>;
 	};
 
 /* PLL_CPU derivatives */
 	occ_cpu: occ-cpu {
 		compatible = "fixed-factor-clock";
-		clocks = <&pll_cpu>;
+		clocks = <&clocks EQ5C_PLL_CPU>;
 		#clock-cells = <0>;
 		clock-div = <1>;
 		clock-mult = <1>;
@@ -101,7 +79,7 @@ mem_clk: mem-clk {
 	};
 	occ_isram: occ-isram {
 		compatible = "fixed-factor-clock";
-		clocks = <&pll_cpu>;
+		clocks = <&clocks EQ5C_PLL_CPU>;
 		#clock-cells = <0>;
 		clock-div = <2>;
 		clock-mult = <1>;
@@ -115,7 +93,7 @@ isram_clk: isram-clk { /* gate ClkRstGen_isram */
 	};
 	occ_dbu: occ-dbu {
 		compatible = "fixed-factor-clock";
-		clocks = <&pll_cpu>;
+		clocks = <&clocks EQ5C_PLL_CPU>;
 		#clock-cells = <0>;
 		clock-div = <10>;
 		clock-mult = <1>;
@@ -130,7 +108,7 @@ si_dbu_tp_pclk: si-dbu-tp-pclk { /* gate ClkRstGen_dbu */
 /* PLL_VDI derivatives */
 	occ_vdi: occ-vdi {
 		compatible = "fixed-factor-clock";
-		clocks = <&pll_vdi>;
+		clocks = <&clocks EQ5C_PLL_VDI>;
 		#clock-cells = <0>;
 		clock-div = <2>;
 		clock-mult = <1>;
@@ -144,7 +122,7 @@ vdi_clk: vdi-clk { /* gate ClkRstGen_vdi */
 	};
 	occ_can_ser: occ-can-ser {
 		compatible = "fixed-factor-clock";
-		clocks = <&pll_vdi>;
+		clocks = <&clocks EQ5C_PLL_VDI>;
 		#clock-cells = <0>;
 		clock-div = <16>;
 		clock-mult = <1>;
@@ -158,7 +136,7 @@ can_ser_clk: can-ser-clk { /* gate ClkRstGen_can_ser */
 	};
 	i2c_ser_clk: i2c-ser-clk {
 		compatible = "fixed-factor-clock";
-		clocks = <&pll_vdi>;
+		clocks = <&clocks EQ5C_PLL_VDI>;
 		#clock-cells = <0>;
 		clock-div = <20>;
 		clock-mult = <1>;
@@ -166,7 +144,7 @@ i2c_ser_clk: i2c-ser-clk {
 /* PLL_PER derivatives */
 	occ_periph: occ-periph {
 		compatible = "fixed-factor-clock";
-		clocks = <&pll_per>;
+		clocks = <&clocks EQ5C_PLL_PER>;
 		#clock-cells = <0>;
 		clock-div = <16>;
 		clock-mult = <1>;
@@ -225,7 +203,7 @@ gpio_clk: gpio-clk {
 	};
 	emmc_sys_clk: emmc-sys-clk {
 		compatible = "fixed-factor-clock";
-		clocks = <&pll_per>;
+		clocks = <&clocks EQ5C_PLL_PER>;
 		#clock-cells = <0>;
 		clock-div = <10>;
 		clock-mult = <1>;
@@ -233,7 +211,7 @@ emmc_sys_clk: emmc-sys-clk {
 	};
 	ccf_ctrl_clk: ccf-ctrl-clk {
 		compatible = "fixed-factor-clock";
-		clocks = <&pll_per>;
+		clocks = <&clocks EQ5C_PLL_PER>;
 		#clock-cells = <0>;
 		clock-div = <4>;
 		clock-mult = <1>;
@@ -241,7 +219,7 @@ ccf_ctrl_clk: ccf-ctrl-clk {
 	};
 	occ_mjpeg_core: occ-mjpeg-core {
 		compatible = "fixed-factor-clock";
-		clocks = <&pll_per>;
+		clocks = <&clocks EQ5C_PLL_PER>;
 		#clock-cells = <0>;
 		clock-div = <2>;
 		clock-mult = <1>;
@@ -265,7 +243,7 @@ mjpeg_core_clk: mjpeg-core-clk { /* gate ClkRstGen_mjpeg_gen */
 	};
 	fcmu_a_clk: fcmu-a-clk {
 		compatible = "fixed-factor-clock";
-		clocks = <&pll_per>;
+		clocks = <&clocks EQ5C_PLL_PER>;
 		#clock-cells = <0>;
 		clock-div = <20>;
 		clock-mult = <1>;
@@ -273,7 +251,7 @@ fcmu_a_clk: fcmu-a-clk {
 	};
 	occ_pci_sys: occ-pci-sys {
 		compatible = "fixed-factor-clock";
-		clocks = <&pll_per>;
+		clocks = <&clocks EQ5C_PLL_PER>;
 		#clock-cells = <0>;
 		clock-div = <8>;
 		clock-mult = <1>;
diff --git a/arch/mips/boot/dts/mobileye/eyeq5.dtsi b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
index e82d2a57f6da..1a65b43e13b1 100644
--- a/arch/mips/boot/dts/mobileye/eyeq5.dtsi
+++ b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
@@ -5,7 +5,7 @@
 
 #include <dt-bindings/interrupt-controller/mips-gic.h>
 
-#include "eyeq5-fixed-clocks.dtsi"
+#include "eyeq5-clocks.dtsi"
 
 / {
 	#address-cells = <2>;
@@ -106,6 +106,15 @@ olb: system-controller@e00000 {
 			ranges = <0x0 0x0 0xe00000 0x400>;
 			#address-cells = <1>;
 			#size-cells = <1>;
+
+			clocks: clock-controller@e0002c {
+				compatible = "mobileye,eyeq5-clk";
+				reg = <0x02c 0x50>, <0x11c 0x04>;
+				reg-names = "plls", "ospi";
+				#clock-cells = <1>;
+				clocks = <&xtal>;
+				clock-names = "ref";
+			};
 		};
 
 		gic: interrupt-controller@140000 {

-- 
2.44.0

[PATCH 10/11] MIPS: mobileye: eyeq5: add OLB reset controller node

[Théo Lebrun]

Add devicetree node for the reset controller on Mobileye EyeQ5.
It appears as a subnode to the OLB syscon as its registers are located
in this shared register region.

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 arch/mips/boot/dts/mobileye/eyeq5.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/mips/boot/dts/mobileye/eyeq5.dtsi b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
index 1a65b43e13b1..76935f237ab5 100644
--- a/arch/mips/boot/dts/mobileye/eyeq5.dtsi
+++ b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
@@ -78,6 +78,7 @@ uart0: serial@800000 {
 			interrupts = <GIC_SHARED 6 IRQ_TYPE_LEVEL_HIGH>;
 			clocks  = <&uart_clk>, <&occ_periph>;
 			clock-names = "uartclk", "apb_pclk";
+			resets = <&reset 0 10>;
 		};
 
 		uart1: serial@900000 {
@@ -88,6 +89,7 @@ uart1: serial@900000 {
 			interrupts = <GIC_SHARED 6 IRQ_TYPE_LEVEL_HIGH>;
 			clocks  = <&uart_clk>, <&occ_periph>;
 			clock-names = "uartclk", "apb_pclk";
+			resets = <&reset 0 11>;
 		};
 
 		uart2: serial@a00000 {
@@ -98,6 +100,7 @@ uart2: serial@a00000 {
 			interrupts = <GIC_SHARED 6 IRQ_TYPE_LEVEL_HIGH>;
 			clocks  = <&uart_clk>, <&occ_periph>;
 			clock-names = "uartclk", "apb_pclk";
+			resets = <&reset 0 12>;
 		};
 
 		olb: system-controller@e00000 {
@@ -107,6 +110,13 @@ olb: system-controller@e00000 {
 			#address-cells = <1>;
 			#size-cells = <1>;
 
+			reset: reset-controller@e00000 {
+				compatible = "mobileye,eyeq5-reset";
+				reg = <0x000 0x0c>, <0x200 0x34>, <0x120 0x04>;
+				reg-names = "d0", "d1", "d2";
+				#reset-cells = <2>;
+			};
+
 			clocks: clock-controller@e0002c {
 				compatible = "mobileye,eyeq5-clk";
 				reg = <0x02c 0x50>, <0x11c 0x04>;

-- 
2.44.0

[PATCH 11/11] MIPS: mobileye: eyeq5: add pinctrl node & pinmux function nodes

[Théo Lebrun]

Pins on this platform have two functions: GPIO or something-else. We
create function nodes for each something-else based on functions.

UART nodes are present in the platform devicetree. Add pinctrl to them
now that the pin controller is supported.

Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 arch/mips/boot/dts/mobileye/eyeq5-pins.dtsi | 125 ++++++++++++++++++++++++++++
 arch/mips/boot/dts/mobileye/eyeq5.dtsi      |  13 +++
 2 files changed, 138 insertions(+)

diff --git a/arch/mips/boot/dts/mobileye/eyeq5-pins.dtsi b/arch/mips/boot/dts/mobileye/eyeq5-pins.dtsi
new file mode 100644
index 000000000000..42acda13e57a
--- /dev/null
+++ b/arch/mips/boot/dts/mobileye/eyeq5-pins.dtsi
@@ -0,0 +1,125 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+
+/*
+ * Default pin configuration for Mobileye EyeQ5 boards. We mostly create one
+ * pin configuration node per function.
+ */
+
+&pinctrl {
+	timer0_pins: timer0-pins {
+		function = "timer0";
+		pins = "PA0", "PA1";
+	};
+	timer1_pins: timer1-pins {
+		function = "timer1";
+		pins = "PA2", "PA3";
+	};
+	timer2_pins: timer2-pins {
+		function = "timer2";
+		pins = "PA4", "PA5";
+	};
+	pps0_pins: pps0-pin {
+		function = "timer2";
+		pins = "PA4";
+	};
+	pps1_pins: pps1-pin {
+		function = "timer2";
+		pins = "PA5";
+	};
+	timer5_ext_pins: timer5-ext-pins {
+		function = "timer5";
+		pins = "PA6", "PA7", "PA8", "PA9";
+	};
+	timer5_ext_input_pins: timer5-ext-input-pins {
+		function = "timer5";
+		pins = "PA6", "PA7";
+	};
+	timer5_ext_incap_a_pins: timer5-ext-incap-a-pin {
+		function = "timer5";
+		pins = "PA6";
+	};
+	timer5_ext_incap_b_pins: timer5-ext-incap-b-pin {
+		function = "timer5";
+		pins = "PA7";
+	};
+	can0_pins: can0-pins {
+		function = "can0";
+		pins = "PA14", "PA15";
+	};
+	can1_pins: can1-pins {
+		function = "can1";
+		pins = "PA16", "PA17";
+	};
+	uart0_pins: uart0-pins {
+		function = "uart0";
+		pins = "PA10", "PA11";
+	};
+	uart1_pins: uart1-pins {
+		function = "uart1";
+		pins = "PA12", "PA13";
+	};
+	spi0_pins: spi0-pins {
+		function = "spi0";
+		pins = "PA18", "PA19", "PA20", "PA21", "PA22";
+	};
+	spi1_pins: spi1-pins {
+		function = "spi1";
+		pins = "PA23", "PA24", "PA25", "PA26", "PA27";
+	};
+	spi1_slave_pins: spi1-slave-pins {
+		function = "spi1";
+		pins = "PA24", "PA25", "PA26";
+	};
+	refclk0_pins: refclk0-pin {
+		function = "refclk0";
+		pins = "PA28";
+	};
+	timer3_pins: timer3-pins {
+		function = "timer3";
+		pins = "PB0", "PB1";
+	};
+	timer4_pins: timer4-pins {
+		function = "timer4";
+		pins = "PB2", "PB3";
+	};
+	timer6_ext_pins: timer6-ext-pins {
+		function = "timer6";
+		pins = "PB4", "PB5", "PB6", "PB7";
+	};
+	timer6_ext_input_pins: timer6-ext-input-pins {
+		function = "timer6";
+		pins = "PB4", "PB5";
+	};
+	timer6_ext_incap_a_pins: timer6-ext-incap-a-pin {
+		function = "timer6";
+		pins = "PB4";
+	};
+	timer6_ext_incap_b_pins: timer6-ext-incap-b-pin {
+		function = "timer6";
+		pins = "PB5";
+	};
+	can2_pins: can2-pins {
+		function = "can2";
+		pins = "PB10", "PB11";
+	};
+	uart2_pins: uart2-pins {
+		function = "uart2";
+		pins = "PB8", "PB9";
+	};
+	spi2_pins: spi2-pins {
+		function = "spi2";
+		pins = "PB12", "PB13", "PB14", "PB15", "PB16";
+	};
+	spi3_pins: spi3-pins {
+		function = "spi3";
+		pins = "PB17", "PB18", "PB19", "PB20", "PB21";
+	};
+	spi3_slave_pins: spi3-slave-pins {
+		function = "spi3";
+		pins = "PB18", "PB19", "PB20";
+	};
+	mclk0_pins: mclk0-pin {
+		function = "mclk0";
+		pins = "PB22";
+	};
+};
diff --git a/arch/mips/boot/dts/mobileye/eyeq5.dtsi b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
index 76935f237ab5..8d4f65ec912d 100644
--- a/arch/mips/boot/dts/mobileye/eyeq5.dtsi
+++ b/arch/mips/boot/dts/mobileye/eyeq5.dtsi
@@ -79,6 +79,8 @@ uart0: serial@800000 {
 			clocks  = <&uart_clk>, <&occ_periph>;
 			clock-names = "uartclk", "apb_pclk";
 			resets = <&reset 0 10>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&uart0_pins>;
 		};
 
 		uart1: serial@900000 {
@@ -90,6 +92,8 @@ uart1: serial@900000 {
 			clocks  = <&uart_clk>, <&occ_periph>;
 			clock-names = "uartclk", "apb_pclk";
 			resets = <&reset 0 11>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&uart1_pins>;
 		};
 
 		uart2: serial@a00000 {
@@ -101,6 +105,8 @@ uart2: serial@a00000 {
 			clocks  = <&uart_clk>, <&occ_periph>;
 			clock-names = "uartclk", "apb_pclk";
 			resets = <&reset 0 12>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&uart2_pins>;
 		};
 
 		olb: system-controller@e00000 {
@@ -125,6 +131,11 @@ clocks: clock-controller@e0002c {
 				clocks = <&xtal>;
 				clock-names = "ref";
 			};
+
+			pinctrl: pinctrl@e000b0 {
+				compatible = "mobileye,eyeq5-pinctrl";
+				reg = <0x0b0 0x30>;
+			};
 		};
 
 		gic: interrupt-controller@140000 {
@@ -149,3 +160,5 @@ timer {
 		};
 	};
 };
+
+#include "eyeq5-pins.dtsi"

-- 
2.44.0