Skip to contentSkip to conversation

Cirrus Logic CS2600 clock device

PATCH In review Richard Fitzgerald 4 participants 8 messages Aug 7 – Aug 10

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

v7 cover Cirrus Logic CS2600 clock device

This patch set provides Common Clock Framework support for the Cirrus Logic CS2600 Fractional-N Clock Synthesizer and Multiplier.

KUnit testing is included in the series.

Major changes:

v7 changes:
- Fix issues reported by Sashiko.
- Remove most examples from DT binding.

v6 changes:
- Convert DT smart mode selection from an enum property to two booleans.
- Fix DT clock names to match the datasheet names.
- Add DT property to explicitly flag that the internal oscillator should be used instead of inferring it from having a variable definition of clock-names.
- Add DT support for setting BCLK invert, FSYNC invert, and FSYNC duty cycles.
- More DT examples
- Merged and factored-out duplicated code in the driver.
- Fixed rounding errors that caused the calculated frequency to drift when it was passed into clock core and then passed back to the driver.
- Fixed driver bugs where the functions for one clock relied on the register values of other clocks being set already, which is not guaranteed.
- Fix a bug where the (mandatory) REF_CLK_IN wasn't requested and enabled when using CLK_IN as the frequency reference in manual mode.
- Lots of code cleanup in the driver
- Fix driver bugs.
- Added KUnit test.

v5 changes:
- Change dt-binding to dt-bindings in commit message
- Add minItems to clocks
- Remove unneeded comments for the cirrus,aux-output-source enums

v4 changes:
- Change DT discription to add more detial.
- Move reg property to keep consistent with the required list.
- Add enum to string DT inputs and change the input values to lower case.
- Change unevauatedProperties to additionalProperties
- Reove black line.

v3 changes:
- Changed clock-names in device tree to an emum since the both clocks are not always required.
- Changed cirrus,aux-output-source to a string input.
- Added cirrus,clock-mode as a string input.
- Fixed ordering in DT example.
- Removed dt-bindings .h file.
- Add in_range to allow for acceptable ranges for clock rates.

v2 changes:
- Remove | from after "discription" in DT binding.
- Remove discription from "clocks" in DT binding.
- Change ref_clk to xti
- Clarify auxiliary clock pin
- Change DT example to not have "reg" for the bus
- Change DT example to change i2c@0 to i2c
- Remove "OUTPUT" from the clock ouputs in the dt binding include file.
- Remove AUX_OUT defines
- Use do_div when dividing u64 by a u32
- Include the dt bindings include file
- Use the includes from the dt bindings include file aux output.
- Move of_device_id and i2c_device_id tables next to i2c_driver struct.

Paul Handrigan (2):
dt-bindings: clock: cs2600: Add support for the CS2600
clk: cs2600: Add Fractional-N clock driver

Richard Fitzgerald (1):
clk: cs2600: Add KUnit test for CS2600 driver

 .../bindings/clock/cirrus,cs2600.yaml         |  170 ++
 MAINTAINERS                                   |    8 +
 drivers/clk/Kconfig                           |    1 +
 drivers/clk/Makefile                          |    1 +
 drivers/clk/cirrus/Kconfig                    |   24 +
 drivers/clk/cirrus/Makefile                   |   27 +
 drivers/clk/cirrus/clk-cs2600-kunit.c         | 2526 +++++++++++++++++
 drivers/clk/cirrus/clk-cs2600-kunit.h         |   18 +
 drivers/clk/cirrus/clk-cs2600.c               | 1229 ++++++++
 drivers/clk/cirrus/clk-cs2600.h               |  163 ++
 drivers/clk/cirrus/kunit_clk_cs2600.dtsi      |   40 +
 .../clk/cirrus/kunit_clk_cs2600_assigned.dtso |   27 +
 .../kunit_clk_cs2600_aux1_out_clk_in.dtso     |   17 +
 ...it_clk_cs2600_aux1_out_clk_in_missing.dtso |   17 +
 ...kunit_clk_cs2600_aux1_out_freq_unlock.dtso |   17 +
 ...unit_clk_cs2600_aux1_out_phase_unlock.dtso |   17 +
 .../kunit_clk_cs2600_aux1_out_ref_clk_in.dtso |   17 +
 .../cirrus/kunit_clk_cs2600_bclk_invert.dtso  |   17 +
 .../cirrus/kunit_clk_cs2600_fsync_duty_1.dtso |   17 +
 .../cirrus/kunit_clk_cs2600_fsync_duty_2.dtso |   17 +
 .../kunit_clk_cs2600_fsync_duty_32.dtso       |   17 +
 .../cirrus/kunit_clk_cs2600_fsync_invert.dtso |   17 +
 .../kunit_clk_cs2600_manual_intosc_clkin.dtso |   17 +
 .../kunit_clk_cs2600_manual_intosc_only.dtso  |   17 +
 .../kunit_clk_cs2600_manual_refclk_clkin.dtso |   16 +
 .../kunit_clk_cs2600_manual_refclk_only.dtso  |   16 +
 .../kunit_clk_cs2600_name_output_clocks.dtso  |   23 +
 ...it_clk_cs2600_smart_clkin_only_intosc.dtso |   19 +
 ...it_clk_cs2600_smart_clkin_only_refclk.dtso |   18 +
 .../cirrus/kunit_clk_cs2600_smart_intosc.dtso |   18 +
 .../cirrus/kunit_clk_cs2600_smart_refclk.dtso |   17 +
 .../dt-bindings/clock/cirrus,cs2600-clock.h   |   18 +
 32 files changed, 4583 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/cirrus,cs2600.yaml
 create mode 100644 drivers/clk/cirrus/Kconfig
 create mode 100644 drivers/clk/cirrus/Makefile
 create mode 100644 drivers/clk/cirrus/clk-cs2600-kunit.c
 create mode 100644 drivers/clk/cirrus/clk-cs2600-kunit.h
 create mode 100644 drivers/clk/cirrus/clk-cs2600.c
 create mode 100644 drivers/clk/cirrus/clk-cs2600.h
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600.dtsi
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_assigned.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_aux1_out_clk_in.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_aux1_out_clk_in_missing.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_aux1_out_freq_unlock.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_aux1_out_phase_unlock.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_aux1_out_ref_clk_in.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_bclk_invert.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_fsync_duty_1.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_fsync_duty_2.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_fsync_duty_32.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_fsync_invert.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_manual_intosc_clkin.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_manual_intosc_only.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_manual_refclk_clkin.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_manual_refclk_only.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_name_output_clocks.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_smart_clkin_only_intosc.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_smart_clkin_only_refclk.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_smart_intosc.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_smart_refclk.dtso
 create mode 100644 include/dt-bindings/clock/cirrus,cs2600-clock.h
Signature · 1 line
2.47.3
#1 lore raw

PATCH v7 1/3 dt-bindings: clock: cs2600: Add support for the CS2600

From: Paul Handrigan <[email protected]>

Add device tree schema for the Cirrus Logic CS2600 clock generator.

The majority of the schema is typical clock, power and I2C properties.

---

Changes in V7:
- Reduced to one example of a typical configuration.

Changes in V6:
- Fixed clock naming in descriptions to match datasheet.
- Fixed clock-names property values to match datasheet names.

- Added cirrus,internal-oscillator boolean to flag that the internal oscillator is the clock source. Previously the driver inferred this if clock-names did not contain "ref_clk_in", but this made it difficult to enforce dts correctness because there was no way to tell whether ref_clk_in was intentionally or accidentally missing.

- Changed the cirrus,clock-mode enum property into two booleans, since effectively it was two separate features masquerading as an enum:
- cirrus,smart-mode present to enable smart mode.
- cirrus,smart-mode-clkin-only to enable a feature where the output will be suppressed until both input clocks are present.

- Changes to cirrus,aux-output-source property:
- Renamed to cirrus,aux1-output-source because it's for the AUX1 pin.
- Added more options.
- Renamed the "no_clkin" option to "clkin_missing".
- Reformatted the description as a list instead of one long sentence.

- Changed clock-names from an enum to an ordered list of const. This implicitly ensures ref_clk_in is always required.

- Added properties to invert bclk and fsync outputs.
- Added property cirrus,fsync-duty-cycles.
- Added #clock-cells and vdd-supply to the list of required properies.
- Rewritten description description section.
- Reordered the property list to put common properties before custom cirrus properties.
- Added more examples.
- Added header file to define the clock indexes for DT consumers of the CS2600 clocks.

Note:
V5 was Reviewed-by: Krzysztof Kozlowski <[email protected]> but I haven't carried this forward because the changes in V6 are large.

Changes in V5:
- Change dt-binding to dt-bindings in commit message
- Add minItems to clocks
- Remove unneeded comments for the cirrus,aux-output-source enums

Changes in V4:
- Change DT description to add more detail.
- Move reg property to keep consistant with the required list.
- Add enum to string DT inputs and change the input values to lower case.
- Change unevaluatedProperties to additionalProperties

Changes in V3:
- Changed clock-names in device tree to an enum since the both clocks are not always required.
- Changed cirrus,aux-output-source to a string input.
- Added cirrus,clock-mode as a string input.
- Fixed ordering in DT example.
- Removed dt-bindings .h file.

Changes in V2:
- Remove | from after "description" in DT binding.
- Remove description from "clocks" in DT binding.
- Change ref_clk to xti
- Clarify auxiliary clock pin
- Change DT example to not have "reg" for the bus
- Change DT example to change i2c@0 to i2c
- Remove "OUTPUT" from the clock ouputs in the dt binding include file.

 .../bindings/clock/cirrus,cs2600.yaml         | 170 ++++++++++++++++++
 MAINTAINERS                                   |   7 +
 .../dt-bindings/clock/cirrus,cs2600-clock.h   |  18 ++
 3 files changed, 195 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/cirrus,cs2600.yaml
 create mode 100644 include/dt-bindings/clock/cirrus,cs2600-clock.h
Documentation/devicetree/bindings/clock/cirrus,cs2600.yaml new file +170 -0
new file mode 100644
index 000000000000..630be30c160c
@@ -0,0 +1,170 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/cirrus,cs2600.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Cirrus Logic Fractional-N Clock Synthesizer & Clock Multiplier
+
+maintainers:
+  - [email protected]
+
+description: |
+  The CS2600 is a system-clocking device that enables frequency synthesis
+  and clock generation from a stable timing reference clock. The device can
+  generate low-jitter clocks from a noisy clock reference at frequencies as
+  low as 50 Hz. The CS2600 has two clock inputs: REF_CLK_IN and CLK_IN.
+  REF_CLK_IN can be either a crystal or an external clock source.
+  REF_CLK_IN is always required to clock the PLL. Either REF_CLK_IN or
+  CLK_IN can be used as the reference frequency from which the output
+  frequency is derived.
+
+  The clock tree of the CS2600 is:
+
+                 +------------+
+  REF_CLK_IN ----| Hybrid PLL | PLL_OUT    +--------+
+                 |   + FLL    |-----+------|  GATE  |--------- CLK_OUT
+      CLK_IN ----|            |     |      +--------+
+                 +------------+     |
+                                    |      +---------+
+                                    +------| DIVIDER |-------- BCLK_OUT
+                                    |      +---------+
+                                    |
+                                    |      +---------+
+                                    +------| DIVIDER |-------- FSYNC_OUT
+                                           +---------+
+
+  Clock output names are defined in <dt-bindings/clock/cirrus,cs2600-clock.h>
+
+properties:
+  compatible:
+    enum:
+      - cirrus,cs2600
+
+  reg:
+    maxItems: 1
+
+  '#clock-cells':
+    const: 1
+
+  clocks:
+    minItems: 1
+    maxItems: 2
+
+  clock-names:
+    minItems: 1
+    items:
+      - const: ref_clk_in
+      - const: clk_in
+
+  vdd-supply:
+    description: Power Supply
+
+  clock-output-names:
+    maxItems: 4
+    description:
+      Names for the output clocks corresponding to PLL_OUT, CLK_OUT,
+      BCLK_OUT and FSYNC_OUT.
+
+  cirrus,aux1-output-source:
+    description: |
+      Selects the signal  routed to the AUX1_OUT pin:
+        - "ref_clk_in"    the clock on REF_CLK_IN
+        - "clk_in"        the clock on CLK_IN
+        - "phase_unlock"  indicate the input and output clocks are not in phase,
+        - "freq_unlock"   indicate the PLL is unlocked,
+        - "clkin_missing" indicate no clock present at clk_in pin.
+    $ref: /schemas/types.yaml#/definitions/string
+    enum:
+      - ref_clk_in
+      - clk_in
+      - phase_unlock
+      - freq_unlock
+      - clkin_missing
+    default: ref_clk_in
+
+  cirrus,internal-oscillator:
+    description:
+      Use the internal oscillator as the source of REF_CLK_IN. The clocks
+      and clock-names properties must provide a fixed-rate clock of exactly
+      12000000 Hz as the ref_clk_in clock.
+    type: boolean
+
+  cirrus,smart-mode:
+    description:
+      Use REF_CLK_IN as the frequency reference until CLK_IN is present,
+      with a glitchless switch to CLK_IN as the frequency reference when
+      CLK_IN is present.
+    type: boolean
+
+  cirrus,smart-mode-clkin-only:
+    description:
+      In Smart Mode keep PLL_OUT disabled until there is clock on CLK_IN.
+    type: boolean
+
+  cirrus,bclk-invert:
+    description:
+      Invert the BCLK output.
+    type: boolean
+
+  cirrus,fsync-invert:
+    description:
+      Invert the FSYNC output.
+    type: boolean
+
+  cirrus,fsync-duty-cycles:
+    description:
+      FSYNC output duty. The number of BCLK cycles that FSYNC is asserted.
+      If not present the duty is 50%.
+      This is determined by the type of audio port that is being clocked.
+      For example TDM formats typically define FSYNC is a pulse of 1 BCLK
+      at the start of a frame.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [ 1, 2, 4, 8, 16, 32 ]
+
+required:
+  - compatible
+  - reg
+  - '#clock-cells'
+  - clocks
+  - clock-names
+  - vdd-supply
+
+allOf:
+  - if:
+      required:
+        - cirrus,smart-mode-clkin-only
+    then:
+      required:
+        - cirrus,smart-mode
+
+  # Require clk_in for smart mode
+  - if:
+      required:
+        - cirrus,smart-mode
+    then:
+      properties:
+        clocks:
+          minItems: 2
+        clock-names:
+          minItems: 2
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      clock-controller@2c {
+        compatible = "cirrus,cs2600";
+        reg = <0x2c>;
+        #clock-cells = <1>;
+        clocks = <&xtl_clk>, <&sync_clock>;
+        clock-names = "ref_clk_in", "clk_in";
+        vdd-supply = <&vreg>;
+        cirrus,smart-mode;
+        cirrus,smart-mode-clkin-only;
+      };
+    };
MAINTAINERS +7 -0
index 4a8b0fd665ce..c2328066fede 100644
@@ -6213,6 +6213,13 @@ F:	sound/hda/codecs/side-codecs/cs*
 F:	sound/hda/codecs/side-codecs/hda_component*
 F:	sound/soc/codecs/cs*
 
+CIRRUS LOGIC CLOCK DRIVERS
+M:	[email protected]
+L:	[email protected]
+S:	Supported
+F:	Documentation/devicetree/bindings/clock/cirrus,cs2600*
+F:	include/dt-bindings/clock/cirrus,cs2600*
+
 CIRRUS LOGIC HAPTIC DRIVERS
 M:	James Ogletree <[email protected]>
 M:	Fred Treven <[email protected]>
include/dt-bindings/clock/cirrus,cs2600-clock.h new file +18 -0
new file mode 100644
index 000000000000..bde505e17813
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause*/
+/*
+ * CS2600 clock driver
+ *
+ * Copyright (C) 2023-2025 Cirrus Logic, Inc. and
+ *                         Cirrus Logic International Semiconductor Ltd.
+ */
+
+#ifndef __DT_BINDINGS_CLOCK_CS2600_H
+#define __DT_BINDINGS_CLOCK_CS2600_H
+
+/* Clock output indexes */
+#define CS2600_PLL_OUT			0
+#define CS2600_CLK_OUT			1
+#define CS2600_BCLK_OUT			2
+#define CS2600_FSYNC_OUT		3
+
+#endif /* __DT_BINDINGS_CLOCK_CS2600_H */
Signature · 1 line
2.47.3
#2 lore raw

Re: [PATCH v7 1/3] dt-bindings: clock: cs2600: Add support for the CS2600

Quoted 6 lines · Richard Fitzgerald
Documentation/devicetree/bindings/clock/cirrus,cs2600.yaml new file +0 -0
new file mode 100644
index 000000000000..630be30c160c

Documentation/devicetree/bindings/clock/cirrus,cs2600.yaml line 10

Quoted 10 lines
Documentation/devicetree/bindings/clock/cirrus,cs2600.yaml new file +10 -0
@@ -0,0 +1,170 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/cirrus,cs2600.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Cirrus Logic Fractional-N Clock Synthesizer & Clock Multiplier
+
+maintainers:
+  - [email protected]

No group mailboxes are accepted. List here people which we can actually contact.

I did not notice this in v5.

...

Quoted 22 lines
Documentation/devicetree/bindings/clock/cirrus,cs2600.yaml new file +17 -0
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      clock-controller@2c {
+        compatible = "cirrus,cs2600";
+        reg = <0x2c>;
+        #clock-cells = <1>;
+        clocks = <&xtl_clk>, <&sync_clock>;
+        clock-names = "ref_clk_in", "clk_in";
+        vdd-supply = <&vreg>;
+        cirrus,smart-mode;
+        cirrus,smart-mode-clkin-only;
+      };
+    };
MAINTAINERS +0 -0
index 4a8b0fd665ce..c2328066fede 100644

MAINTAINERS F: sound/hda/codecs/side-codecs/cs* · line 6217

Quoted 5 lines
MAINTAINERS +2 -0
@@ -6213,6 +6213,13 @@ F:	sound/hda/codecs/side-codecs/cs*
 F:	sound/hda/codecs/side-codecs/hda_component*
 F:	sound/soc/codecs/cs*
 
+CIRRUS LOGIC CLOCK DRIVERS
+M:	[email protected]

Company drop boxes are not really working for us. I understand it does work for you, though, but that's not what we want. Please come up with list of people REALLY responsible for this.

Otherwise why would we want it in the Linux kernel?

include/dt-bindings/clock/cirrus,cs2600-clock.h

Quoted 13 lines
MAINTAINERS +5 -0
+L:	[email protected]
+S:	Supported
+F:	Documentation/devicetree/bindings/clock/cirrus,cs2600*
+F:	include/dt-bindings/clock/cirrus,cs2600*
+
 CIRRUS LOGIC HAPTIC DRIVERS
 M:	James Ogletree <[email protected]>
 M:	Fred Treven <[email protected]>
include/dt-bindings/clock/cirrus,cs2600-clock.h new file +0 -0
new file mode 100644
index 000000000000..bde505e17813

Same filename as the binding.

Best regards,
Krzysztof

#3 lore raw

PATCH v7 2/3 clk: cs2600: Add Fractional-N clock driver

In reply to Richard Fitzgerald

From: Paul Handrigan <[email protected]>

Add driver for the Cirrus Logic CS2600 fractional-N clock synthesizer and multiplier.

The CS2600 is a system-clocking device using a hybrid fractional-N analog PLL and a digital frequency-locked loop (FLL). The CS2600 enables frequency synthesis and clock generation from a stable timing reference clock. The device can generate low-jitter clocks between 6 MHz and 75 MHz from a noisy clock reference between 50 Hz and 30 MHz, or from a stable clock reference between 8 MHz and 75 MHz.

The timing reference clock (REF_CLK_IN) can be an external clock source or the internal oscillator. This clock drives the PLL and is always required to generate output clocks, even if it is not used as the frequency reference.

The frequency reference source can be either REF_CLK_IN or an external clock on CLK_IN.

The PLL_OUT clock frequency is a ratio of the chosen frequency reference. The ratio is a fixed-point number of either 12.20 or 20.12 precision. The lower-precision high-multiplication 20.12 ratios are only available when CLK_IN is the frequency reference.

In smart mode the CS2600 will synthesize the output based on REF_CLK_IN until a clock is present on CLK_IN; it then performs a glitchless switch to CLK_IN as the frequency reference. A variant of smart mode automatically enables the PLL output only when CLK_IN becomes available.

Three output pins are derived from PLL_OUT:

CLK_OUT: Gated output from PLL_OUT.
BCLK_OUT: Divided from PLL_OUT.
FSYNC_OUT: Divided from PLL_OUT.

The BCLK_OUT and FSYNC_OUT have different sets of available divide ratios intended to provide the typical frequency relation between bit clock and frame clock on I2S/TDM audio interfaces. They can also be inverted with respect to CLK_OUT to support various formats of serial audio data. FSYNC_OUT has the additional feature that it can be either a 50:50 duty cycle or a pulse of a programmable number of BCLK cycles.

This configuration is modeled in the driver as:

                 +------------+
  REF_CLK_IN ----| Hybrid PLL | PLL_OUT    +--------+
                 |   + FLL    |-----+------|  GATE  |--------- CLK_OUT
      CLK_IN ----|            |     |      +--------+
                 +------------+     |
                                    |      +---------+
                                    +------| DIVIDER |-------- BCLK_OUT
                                    |      +---------+
                                    |
                                    |      +---------+
                                    +------| DIVIDER |-------- FSYNC_OUT
                                           +---------+

OF properties define the hardware-level configuration to match the hardware that the CS2600 is driving.

The CS2600_ERR_xxx register bits are not used in the driver but are defined in the header file as a convenience for anyone debugging a system.

---
Changes in V7:
- Make use of the holding buffer in the CS2600 for writes to frozen bits. The removes the need to clear and set FREEZE_EN around those writes, instead FREEZE_EN is toggled after the writes.

- Set req->rate in cs2600_bclk_determine_rate() and cs2600_fsync_determine_rate()

- Deleted the spurious check of PLL_EN1 in cs2600_pll_out_prepare(). It would always be 0 so this code did nothing.

- Use regmap_multi_reg_write() two write both ratio registers in one function call. Makes the error handling tidier because there's only one error return.

- Set CLK_SET_RATE_GATE on the PLL_OUT and CLK_OUT clocks.

- Set reg_stride in the regmap.

- Delete if statements that checked cs2600->ref_clk for NULL. It's enforced during probe() that this cannot be NULL.

- Check in cs2600_clk_register() that there is at least one parent.

- Delete the check in cs2600_pll_out_set_parent() that the parent index fits in the register field. The call to clk_hw_get_parent_by_index() already checked that the index is valid.

- Added some more error checks on regmap writes.

- Check return value of regmap_test_bits() for error.

- Log detected device ID and revision at dev_dbg level instead of dev_info.

- Renamed two local variables that hold the divide index to 'div_index' (instead of 'div_val').

- Order local variables in cs2600_clk_register() by line length.

Changes in V6:
- Merged the functions to calculate synth mode and mult mode ratios. The math is the same, only the fixed-point precision is different. The new cs2600_calc_ratio() function calculates at 20:20 fixed point and then converts to 20:12 if the value is too large for 12:20.
This also simplifies the calling code because it doesn't need all the checks of which ratio function it should be calling.
- Re-worked the rounding of calculated rates so that they didn't drift away from the best rate when the clock core passed the rounded rate from determine_rate() back into set_rate().
- Changed various parts of the recalc_rate()/determine_rate()/set_rate() functions that relied on the state of the other clock registers to make decisions. There's no guarantee whether those other clocks have been configured yet so the decision making could be invalid.
- Removed the use of different ratio slots depending on the mode. The The CS2600 doesn't care which slots are used for synth and mult ratio. Using fixed slots simplifies the code.
- Split the large cs2600_pll_out_prepare() into helper functions.
- Request ref_clk when starting the PLL. It is the system clock of the CS2600 so it is always required to clock the chip even if it isn't the frequency reference parent. In previous versions ref_clk was only enabled by being the parent, which meant the CS2600 wasn't clocked when using mult mode with clk_in as the frequency reference parent.
- Fix cs2600_pll_out_set_parent() to properly reconfigure the PLL when changing parents. The parent selection isn't a simple mux, it changes PLL mode and selected ratio registers. Previously the mode bit was written but the mode ratio registers weren't updated to match.
- Factor out the BCLK and FSYNC divider lookup into a single function. The lookup code is the same apart from the divide value that each register field value represents. The divide values are provided as an array.
- Don't attempt to set a best_parent_rate when setting BCLK/FSYNC rate. The parent (CLK_OUT) rate is not arbitrary, it must match the requirements of the device being clocked, and only the consumer driver will know what is correct.
- Set CLK_SET_PARENT_GATE on the PLL. Its configuration cannot be updated atomically, so it must be stopped to reconfigure it.
- Split several functions into separate functions for calculating a value and get/set the register.
- Use FIELD_PREP() and FIELD_GET() instead of multiple custom macros.
- Fix inverted register unfreeze and freeze that meant the bits affected by FREEZE_EN were not actually updated.
- Use a DEFINE_GUARD to implement the register field unfreeze.
- Rename cs2600_ref_clk_bound_rate() to cs2600_ref_clk_set_divider() to indicate it's actually setting the rate, not just finding a rounded value.
- Remove inappropriate use of in_range() that required some confusing math to pass the correct value into in_range(). It's clearer to do normal min/max comparison.
- Use DIV_ROUND_UP_ULL() to convert PPM to a frequency delta instead of open-coding it.
- Initialize all the clocks using a single common loop.
- Use clk_init_data.parent_hws to set FSYNC, BCLK and CLK_OUT parent as PLL_OUT, instead of looking up PLL_OUT by name and setting FSYNC, BCLK and CLK_OUT parent to that name.
- Smart mode selection is now two boolean properties instead of an enum property. One property select smart mode, the other property enables suppression of PLL_OUT until there is clock on CLK_IN.
- Added properties to invert FSYNC and BCLK relative to CLK_OUT.
- Added property to set the duty cycle of FSYNC (required when clocking TDM audio formats).
- Use match_string() instead of a string of strcmp() to lookup cirrus,aux1-output-source value.
- Use of the internal oscillator is now explictly flagged by the presence of the cirrus,internal-oscillator property. Previously the driver inferred it from the absence of a ref_clk in clock-names.
- More error checking.

Changes before V6:
Sorry, the previously upstreamed versions don't have a changelist for me to copy into here.

 MAINTAINERS                     |    1 +
 drivers/clk/Kconfig             |    1 +
 drivers/clk/Makefile            |    1 +
 drivers/clk/cirrus/Kconfig      |   10 +
 drivers/clk/cirrus/Makefile     |    3 +
 drivers/clk/cirrus/clk-cs2600.c | 1229 +++++++++++++++++++++++++++++++
 drivers/clk/cirrus/clk-cs2600.h |  163 ++++
 7 files changed, 1408 insertions(+)
 create mode 100644 drivers/clk/cirrus/Kconfig
 create mode 100644 drivers/clk/cirrus/Makefile
 create mode 100644 drivers/clk/cirrus/clk-cs2600.c
 create mode 100644 drivers/clk/cirrus/clk-cs2600.h
MAINTAINERS +1 -0
index c2328066fede..9523e6d61bc6 100644
@@ -6218,6 +6218,7 @@ M:	[email protected]
 L:	[email protected]
 S:	Supported
 F:	Documentation/devicetree/bindings/clock/cirrus,cs2600*
+F:	drivers/clk/cirrus/*
 F:	include/dt-bindings/clock/cirrus,cs2600*
 
 CIRRUS LOGIC HAPTIC DRIVERS
drivers/clk/Kconfig +1 -0
index 1717ce75a907..e576c6cd33f4 100644
@@ -509,6 +509,7 @@ source "drivers/clk/actions/Kconfig"
 source "drivers/clk/analogbits/Kconfig"
 source "drivers/clk/aspeed/Kconfig"
 source "drivers/clk/bcm/Kconfig"
+source "drivers/clk/cirrus/Kconfig"
 source "drivers/clk/eswin/Kconfig"
 source "drivers/clk/hisilicon/Kconfig"
 source "drivers/clk/imgtec/Kconfig"
drivers/clk/Makefile +1 -0
index cc108a75a900..b1e3bb794d7f 100644
@@ -119,6 +119,7 @@ obj-$(CONFIG_ARCH_ARTPEC)		+= axis/
 obj-$(CONFIG_ARC_PLAT_AXS10X)		+= axs10x/
 obj-y					+= bcm/
 obj-$(CONFIG_ARCH_BERLIN)		+= berlin/
+obj-y					+= cirrus/
 obj-$(CONFIG_ARCH_DAVINCI)		+= davinci/
 obj-$(CONFIG_COMMON_CLK_ESWIN)		+= eswin/
 obj-$(CONFIG_ARCH_HISI)			+= hisilicon/
drivers/clk/cirrus/Kconfig new file +10 -0
new file mode 100644
index 000000000000..5674d6554f96
@@ -0,0 +1,10 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+config COMMON_CLK_CS2600
+	tristate "Clock driver for CS2600 Fractional-N Clock Synthesizer & Clock Multiplier"
+	depends on I2C
+	depends on OF
+	select REGMAP_I2C
+	help
+	  If you say yes here you get support for the CS2600 clock synthesizer
+	  and multiplier.
drivers/clk/cirrus/Makefile new file +3 -0
new file mode 100644
index 000000000000..182612dbb96a
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_COMMON_CLK_CS2600)		+= clk-cs2600.o
drivers/clk/cirrus/clk-cs2600.c new file +1229 -0

1,229 lines not on this page · raw message

drivers/clk/cirrus/clk-cs2600.h new file +163 -0

163 lines not on this page · raw message

Signature · 1 line
2.47.3
#4 lore raw

Re: [PATCH v7 2/3] clk: cs2600: Add Fractional-N clock driver

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [High] The driver fails to toggle the FREEZE_EN bit after writing to frozen registers, preventing new…

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The driver fails to toggle the FREEZE_EN bit after writing to frozen registers, preventing new configurations from taking effect.
- [Medium] cs2600_pll_out_recalc_rate computes a new theoretical ratio instead of querying the actual hardware frequency.
- [Medium] The driver exposes the internal PLL_OUT clock to consumers despite explicit comments intending to restrict it.
- [Medium] dev_err is inappropriately called in determine_rate and ratio calculations for unsupported parameters, causing log spam. --

--- Patch [2]: [PATCH v7 2/3] clk: cs2600: Add Fractional-N clock driver --- commit 75202f90081b654ba7061c4d517a6849df860547
Author: Paul Handrigan <[email protected]>

clk: cs2600: Add Fractional-N clock driver

Add driver for the Cirrus Logic CS2600 fractional-N clock synthesizer and multiplier.

Quoted 5 lines
drivers/clk/cirrus/clk-cs2600.c new file +0 -0
new file mode 100644
index 0000000000000..977a4bf3c31dc

[ ... ]

Quoted 6 lines
drivers/clk/cirrus/clk-cs2600.c new file +6 -0
+static int cs2600_calc_ratio(struct cs2600 *cs2600,
+			     unsigned long rate,
+			     unsigned long parent_rate,
+			     unsigned int *ratio_out,
+			     bool *hi_res)
+{

[ ... ]

drivers/clk/cirrus/clk-cs2600.c line 171

Quoted 6 lines
drivers/clk/cirrus/clk-cs2600.c new file +6 -0
+	if (ratio > CS2600_12_20_RATIO_MAX) {
+		*hi_res = false;
+		ratio >>= (CS2600_12_20_SHIFT - CS2600_20_12_SHIFT);
+
+		if (ratio > CS2600_20_12_RATIO_MAX) {
+			dev_err(cs2600->dev, "Ratio %lu:%lu too large\n", parent_rate, rate);

[Severity: Medium]
Will this dev_err() trigger during normal speculative CCF operations like clk_round_rate()? If determine_rate() passes an unachievable rate, this could spam the kernel log.

[ ... ]

drivers/clk/cirrus/clk-cs2600.c line 241

Quoted 13 lines
drivers/clk/cirrus/clk-cs2600.c new file +13 -0
+static unsigned long cs2600_pll_out_recalc_rate(struct clk_hw *hw,
+						unsigned long parent_rate)
+{
+	struct cs2600_clk_hw *clk_hw = hw_to_cs2600_clk(hw);
+	struct cs2600 *cs2600 = clk_hw->priv;
+
+	if ((parent_rate == 0) || (cs2600->pll_target_rate == 0))
+		return 0;
+
+	if (parent_rate == cs2600->pll_parent_rate)
+		return cs2600->pll_target_rate;
+
+	return cs2600_calc_rounded_integer_rate(cs2600, parent_rate, cs2600->pll_target_rate);

[Severity: Medium]
Does this bypass hardware querying? The CCF expects recalc_rate() to query the physical hardware to inherit settings (like those from the bootloader) and report the current state.

By returning a computed rate based on the uninitialized software variable pll_target_rate, won't CCF assume the hardware is running at 0Hz until set_rate() is explicitly called?

[ ... ]

Quoted 3 lines
drivers/clk/cirrus/clk-cs2600.c new file +3 -0
+static int cs2600_pll_out_set_rate(struct clk_hw *hw, unsigned long rate,
+				   unsigned long parent_rate)
+{

[ ... ]

drivers/clk/cirrus/clk-cs2600.c line 339

Quoted 1 line
drivers/clk/cirrus/clk-cs2600.c new file +1 -0
+	ret = cs2600_write_ratio(cs2600, CS2600_RATIO_SLOT_1, ratio);

[Severity: High]
Is a step to toggle FREEZE_EN missing here? The commit message states FREEZE_EN is toggled after writing to frozen bits. Since CS2600_FREEZE_EN is never actually used in the code, won't these writes remain trapped in the holding buffer and fail to update the active hardware registers?

[ ... ]

Quoted 2 lines
drivers/clk/cirrus/clk-cs2600.c new file +2 -0
+static int cs2600_pll_out_set_parent(struct clk_hw *hw, u8 index)
+{

[ ... ]

drivers/clk/cirrus/clk-cs2600.c line 498

Quoted 2 lines
drivers/clk/cirrus/clk-cs2600.c new file +2 -0
+	ret = regmap_update_bits(regmap, CS2600_PLL_CFG2, CS2600_PLL_MODE_SEL,
+				 index);

[Severity: High]
Does this also need to toggle FREEZE_EN to latch the mode selection into the active registers?

[ ... ]

Quoted 3 lines
drivers/clk/cirrus/clk-cs2600.c new file +3 -0
+static int cs2600_bclk_determine_rate(struct clk_hw *hw,
+				      struct clk_rate_request *req)
+{

[ ... ]

drivers/clk/cirrus/clk-cs2600.c line 623

Quoted 3 lines
drivers/clk/cirrus/clk-cs2600.c new file +3 -0
+	div_index = cs2600_get_clk_div_index(cs2600_bclk_div, req->rate, req->best_parent_rate);
+	if (div_index < 0) {
+		dev_err(cs2600->dev, "BCLK_OUT cannot be derived from the parent rate");

[Severity: Medium]
Since determine_rate() is often called speculatively, could this dev_err() fill the kernel log when CCF tests unachievable rate requests?

[ ... ]

Quoted 3 lines
drivers/clk/cirrus/clk-cs2600.c new file +3 -0
+static int cs2600_fsync_determine_rate(struct clk_hw *hw,
+				       struct clk_rate_request *req)
+{

[ ... ]

drivers/clk/cirrus/clk-cs2600.c line 744

Quoted 4 lines
drivers/clk/cirrus/clk-cs2600.c new file +4 -0
+	div_index = cs2600_get_clk_div_index(cs2600_fsync_div, req->rate, req->best_parent_rate);
+	if (div_index < 0) {
+		if (req->best_parent_rate)
+			dev_err(cs2600->dev, "FSYNC_OUT cannot be derived from the parent rate");

[Severity: Medium]
Could this dev_err() also spam the log during speculative rate queries?

[ ... ]

drivers/clk/cirrus/clk-cs2600.c line 910

Quoted 8 lines
drivers/clk/cirrus/clk-cs2600.c new file +8 -0
+static struct clk_hw *cs2600_of_clk_get(struct of_phandle_args *clkspec,
+					void *data)
+{
+	unsigned int index = clkspec->args[0];
+	struct cs2600 *cs2600 = data;
+
+	/* PLL_OUT is an internal clock */
+	if (index > CS2600_OUT_CLK_MAX - 1) {

[Severity: Medium]
Does this actually prevent consumers from requesting the internal PLL_OUT clock?

The bounds check allows index == CS2600_PLL_OUT (which is 0), potentially allowing external device tree nodes to bypass the output clocks and manipulate the internal PLL directly.

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
#5 lore raw

Re: [PATCH v7 2/3] clk: cs2600: Add Fractional-N clock driver

In reply to Richard Fitzgerald

drivers/clk/cirrus/clk-cs2600.c line 17

Quoted 10 lines · Richard Fitzgerald
drivers/clk/cirrus/clk-cs2600.c new file +10 -0
+#include <dt-bindings/clock/cirrus,cs2600-clock.h>
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/container_of.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/math.h>
+#include <linux/math64.h>
+#include <linux/mod_devicetable.h>

Please drop <linux/mod_devicetable.h>. You can rely on <linux/i2c.h> to provide struct i2c_device_id and of_device_id.

(Background of this request is that <linux/mod_devicetable.h> should go away soon.)

drivers/clk/cirrus/clk-cs2600.c

Quoted 8 lines
drivers/clk/cirrus/clk-cs2600.c new file +7 -0
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+#include <linux/string.h>
+
[...]
+static const struct of_device_id cs2600_of_match[] = {
+	{ .compatible = "cirrus,cs2600", },

No comma before a closing } in the same line please.

Review

Quoted 1 line
(quoted diff) +1 -0
+	{}

{ } is the most common way to write the terminator.

drivers/clk/cirrus/clk-cs2600.c line 1213

Quoted 7 lines
drivers/clk/cirrus/clk-cs2600.c new file +7 -0
+};
+MODULE_DEVICE_TABLE(of, cs2600_of_match);
+
+static const struct i2c_device_id cs2600_id[] = {
+	{ .name = "cs2600", },
+	{}
+};

Here the same please.

drivers/clk/cirrus/clk-cs2600.c line 1222

Quoted 9 lines
drivers/clk/cirrus/clk-cs2600.c new file +9 -0
+MODULE_DEVICE_TABLE(i2c, cs2600_id);
+
+static struct i2c_driver cs2600_driver = {
+	.driver = {
+		.name = "cs2600",
+		.of_match_table = cs2600_of_match,
+	},
+	.probe		= cs2600_i2c_probe,
+	.id_table	= cs2600_id,

I'm not a fan of aligning =s as it gets inconsistent over time. Here it's wrong already from the start. Only 2 out of 3 members of the i2c_driver use alignment and .driver doesn't. If you ask me, just use a single space before each =.

drivers/clk/cirrus/clk-cs2600.c line 1225

Quoted 3 lines
drivers/clk/cirrus/clk-cs2600.c new file +3 -0
+};
+
+module_i2c_driver(cs2600_driver);

No empty line before module_i2c_driver() please.

Quoted 4 lines
drivers/clk/cirrus/clk-cs2600.c new file +4 -0
+
+MODULE_DESCRIPTION("CS2600 clock driver");
+MODULE_AUTHOR("Paul Handrigan <[email protected]>");
+MODULE_LICENSE("GPL");
#6 lore raw

PATCH v7 3/3 clk: cs2600: Add KUnit test for CS2600 driver

In reply to Richard Fitzgerald

Add a KUnit test for the CS2600 clock driver.

This test uses a mock I2C host driver and mock regmap to partially emulate the hardware. The regmap is permanently in cache-only mode so that it will act as an emulation of a set of device registers.

The bulk of the testing is to confirm that the frequency ratio and PLL configuration register fields are set correctly. The PLL configuration and ratio precision depend on several factors:

- The frequency reference source (REF_CLK_IN or CLK_IN).
- The magnitude of frequency difference between input and output clocks.
- Whether the CS2600 is operating in smart mode.

The test uses hardcoded tables of the expected ratio register settings for pairs of IN:OUT clock frequencies in various modes. The values in these tables were derived independently from the calculation code in the clk-cs2600.c driver. The purpose of this is to avoid false test passes caused by the test code having the same calculation bug as the code it is testing. Using an independent source of ratios reduces this risk. These tables do not attempt to test every possible ratio, but focus on (a) typical frequencies, and (b) edge cases where rounding is expected.

Devicetree overlay files are used to configure properties for a variety of possible hardware configurations. There are test cases to validate that the properties are applied correctly.

---
Changes in V7:
- Fixed missing clock disable on each loop in cs2600_kunit_synth_ratio(), cs2600_kunit_mult_ratio(), cs2600_kunit_smart_ratio() and cs2600_kunit_smart_clkin_only_ratio().

- In cs2600_kunit_change_parent_same_clkout() re-read config registers after calling clk_set_rate().

- In cs2600_kunit_bclk_divider() and cs2600_kunit_fsync_divider() use a 64-bit multiple to calculate the delta so that illegal values can't make it overflow.

- Fix use-after-put of the device_node in cs2600_kunit_case_common_init().

- Validate arguments in cs2600_kunit_i2c_xfer() instead of assuming they cannot be wrong.

- Delete unused local variable in cs2600_kunit_smart_clkin_only_ratio().

- Depend on CONFIG_DTC.

New in V6 series.

 drivers/clk/cirrus/Kconfig                    |   14 +
 drivers/clk/cirrus/Makefile                   |   24 +
 drivers/clk/cirrus/clk-cs2600-kunit.c         | 2526 +++++++++++++++++
 drivers/clk/cirrus/clk-cs2600-kunit.h         |   18 +
 drivers/clk/cirrus/kunit_clk_cs2600.dtsi      |   40 +
 .../clk/cirrus/kunit_clk_cs2600_assigned.dtso |   27 +
 .../kunit_clk_cs2600_aux1_out_clk_in.dtso     |   17 +
 ...it_clk_cs2600_aux1_out_clk_in_missing.dtso |   17 +
 ...kunit_clk_cs2600_aux1_out_freq_unlock.dtso |   17 +
 ...unit_clk_cs2600_aux1_out_phase_unlock.dtso |   17 +
 .../kunit_clk_cs2600_aux1_out_ref_clk_in.dtso |   17 +
 .../cirrus/kunit_clk_cs2600_bclk_invert.dtso  |   17 +
 .../cirrus/kunit_clk_cs2600_fsync_duty_1.dtso |   17 +
 .../cirrus/kunit_clk_cs2600_fsync_duty_2.dtso |   17 +
 .../kunit_clk_cs2600_fsync_duty_32.dtso       |   17 +
 .../cirrus/kunit_clk_cs2600_fsync_invert.dtso |   17 +
 .../kunit_clk_cs2600_manual_intosc_clkin.dtso |   17 +
 .../kunit_clk_cs2600_manual_intosc_only.dtso  |   17 +
 .../kunit_clk_cs2600_manual_refclk_clkin.dtso |   16 +
 .../kunit_clk_cs2600_manual_refclk_only.dtso  |   16 +
 .../kunit_clk_cs2600_name_output_clocks.dtso  |   23 +
 ...it_clk_cs2600_smart_clkin_only_intosc.dtso |   19 +
 ...it_clk_cs2600_smart_clkin_only_refclk.dtso |   18 +
 .../cirrus/kunit_clk_cs2600_smart_intosc.dtso |   18 +
 .../cirrus/kunit_clk_cs2600_smart_refclk.dtso |   17 +
 25 files changed, 2980 insertions(+)
 create mode 100644 drivers/clk/cirrus/clk-cs2600-kunit.c
 create mode 100644 drivers/clk/cirrus/clk-cs2600-kunit.h
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600.dtsi
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_assigned.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_aux1_out_clk_in.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_aux1_out_clk_in_missing.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_aux1_out_freq_unlock.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_aux1_out_phase_unlock.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_aux1_out_ref_clk_in.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_bclk_invert.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_fsync_duty_1.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_fsync_duty_2.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_fsync_duty_32.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_fsync_invert.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_manual_intosc_clkin.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_manual_intosc_only.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_manual_refclk_clkin.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_manual_refclk_only.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_name_output_clocks.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_smart_clkin_only_intosc.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_smart_clkin_only_refclk.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_smart_intosc.dtso
 create mode 100644 drivers/clk/cirrus/kunit_clk_cs2600_smart_refclk.dtso
drivers/clk/cirrus/Kconfig +14 -0
index 5674d6554f96..fbf1ec8262be 100644
@@ -8,3 +8,17 @@ config COMMON_CLK_CS2600
 	help
 	  If you say yes here you get support for the CS2600 clock synthesizer
 	  and multiplier.
+
+config COMMON_CLK_CS2600_KUNIT_TEST
+	tristate "KUnit test for CS2600 clock driver" if !KUNIT_ALL_TESTS
+	depends on COMMON_CLK_CS2600
+	depends on KUNIT
+	depends on DTC
+	depends on REGULATOR
+	depends on REGULATOR_FIXED_VOLTAGE
+	default KUNIT_ALL_TESTS
+	help
+	  Build the KUnit tests for the Cirrus Logic CS2600
+	  clock driver.
+
+	  If unsure say N.
drivers/clk/cirrus/Makefile +24 -0
index 182612dbb96a..be59a601d936 100644
@@ -1,3 +1,27 @@
 # SPDX-License-Identifier: GPL-2.0
 
 obj-$(CONFIG_COMMON_CLK_CS2600)		+= clk-cs2600.o
+
+clk-cs2600_kunit-y :=		clk-cs2600-kunit.o \
+				kunit_clk_cs2600_assigned.dtbo.o \
+				kunit_clk_cs2600_aux1_out_clk_in.dtbo.o \
+				kunit_clk_cs2600_aux1_out_clk_in_missing.dtbo.o \
+				kunit_clk_cs2600_aux1_out_freq_unlock.dtbo.o \
+				kunit_clk_cs2600_aux1_out_phase_unlock.dtbo.o \
+				kunit_clk_cs2600_aux1_out_ref_clk_in.dtbo.o \
+				kunit_clk_cs2600_bclk_invert.dtbo.o \
+				kunit_clk_cs2600_fsync_duty_1.dtbo.o \
+				kunit_clk_cs2600_fsync_duty_2.dtbo.o \
+				kunit_clk_cs2600_fsync_duty_32.dtbo.o \
+				kunit_clk_cs2600_fsync_invert.dtbo.o \
+				kunit_clk_cs2600_manual_intosc_clkin.dtbo.o \
+				kunit_clk_cs2600_manual_intosc_only.dtbo.o \
+				kunit_clk_cs2600_manual_refclk_clkin.dtbo.o \
+				kunit_clk_cs2600_manual_refclk_only.dtbo.o \
+				kunit_clk_cs2600_name_output_clocks.dtbo.o \
+				kunit_clk_cs2600_smart_clkin_only_intosc.dtbo.o \
+				kunit_clk_cs2600_smart_clkin_only_refclk.dtbo.o \
+				kunit_clk_cs2600_smart_intosc.dtbo.o \
+				kunit_clk_cs2600_smart_refclk.dtbo.o
+
+obj-$(CONFIG_COMMON_CLK_CS2600_KUNIT_TEST)	+= clk-cs2600_kunit.o
drivers/clk/cirrus/clk-cs2600-kunit.c new file +2526 -0

2,526 lines not on this page · raw message

drivers/clk/cirrus/clk-cs2600-kunit.h new file +18 -0
new file mode 100644
index 000000000000..2387610cc23b
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * KUnit test for the Cirrus Logic CS2600 clock driver.
+ *
+ * Copyright (C) 2026 Cirrus Logic, Inc. and
+ *                    Cirrus Logic International Semiconductor Ltd.
+ */
+
+#define CS2600_KUNIT_PLLOUT_RENAME	"renamed_pll"
+#define CS2600_KUNIT_CLKOUT_RENAME	"renamed_clk"
+#define CS2600_KUNIT_BCLK_RENAME	"renamed_bs"
+#define CS2600_KUNIT_FSYNC_RENAME	"renamed_fs"
+
+#define CS2600_KUNIT_CLKOUT_ASSIGNED	6144000
+#define CS2600_KUNIT_BCLK_ASSIGNED	1536000
+#define CS2600_KUNIT_FSYNC_ASSIGNED	96000
+#define CS2600_KUNIT_BCLK_DIV_INDEX	3
+#define CS2600_KUNIT_FSYNC_DIV_INDEX	2
drivers/clk/cirrus/kunit_clk_cs2600.dtsi new file +40 -0
new file mode 100644
index 000000000000..9fd11d322837
@@ -0,0 +1,40 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+#include <dt-bindings/clock/cirrus,cs2600-clock.h>
+
+&{/} {
+	dummy_vreg: VDD-DUMMY {
+		compatible = "regulator-fixed";
+		regulator-name = "vdd";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
+
+	cs2600_kunit_refclk: cs2600_kunit_refclk {
+		compatible = "cirrus,cs2600-kunit-refclk";
+		#clock-cells = <0>;
+	};
+
+	cs2600_kunit_clkin: cs2600_kunit_clkin {
+		compatible = "cirrus,cs2600-kunit-clkin";
+		#clock-cells = <0>;
+	};
+
+	cs2600_kunit_i2c: cs2600-kunit-i2c {
+		compatible = "cirrus,cs2600-kunit-i2c";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		clock-frequency = <400000>;
+	};
+
+	cs2600_test_consumer: cs2600-test-consumer {
+		compatible = "cirrus,cs2600-kunit-consumer";
+		clocks = <&cs2600 CS2600_PLL_OUT>, <&cs2600 CS2600_CLK_OUT>,
+			 <&cs2600 CS2600_BCLK_OUT>, <&cs2600 CS2600_FSYNC_OUT>;
+		clock-names = "pllout", "clkout", "bclk", "fsync";
+	};
+};
drivers/clk/cirrus/kunit_clk_cs2600_assigned.dtso new file +27 -0
new file mode 100644
index 000000000000..fd2d7b3b5b10
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+#include "kunit_clk_cs2600.dtsi"
+#include "clk-cs2600-kunit.h"
+
+&cs2600_kunit_i2c {
+	cs2600: cs2600@2c {
+		compatible = "cirrus,cs2600";
+		reg = <0x2c>;
+		#clock-cells = <1>;
+		clocks = <&cs2600_kunit_refclk>;
+		clock-names = "ref_clk_in";
+		vdd-supply = <&dummy_vreg>;
+	};
+};
+
+&cs2600_test_consumer {
+	assigned-clocks = <&cs2600 CS2600_CLK_OUT>,
+			  <&cs2600 CS2600_BCLK_OUT>,
+			  <&cs2600 CS2600_FSYNC_OUT>;
+	assigned-clock-rates = <CS2600_KUNIT_CLKOUT_ASSIGNED
+				CS2600_KUNIT_BCLK_ASSIGNED
+				CS2600_KUNIT_FSYNC_ASSIGNED>;
+};
+
drivers/clk/cirrus/kunit_clk_cs2600_aux1_out_clk_in.dtso new file +17 -0
new file mode 100644
index 000000000000..aeaa3740d6fd
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+#include "kunit_clk_cs2600.dtsi"
+
+&cs2600_kunit_i2c {
+	cs2600: cs2600@2c {
+		compatible = "cirrus,cs2600";
+		reg = <0x2c>;
+		#clock-cells = <1>;
+		clocks = <&cs2600_kunit_refclk>;
+		clock-names = "ref_clk_in";
+		vdd-supply = <&dummy_vreg>;
+		cirrus,aux1-output-source = "clk_in";
+	};
+};
drivers/clk/cirrus/kunit_clk_cs2600_aux1_out_clk_in_missing.dtso new file +17 -0
new file mode 100644
index 000000000000..809e55f22d4d
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+#include "kunit_clk_cs2600.dtsi"
+
+&cs2600_kunit_i2c {
+	cs2600: cs2600@2c {
+		compatible = "cirrus,cs2600";
+		reg = <0x2c>;
+		#clock-cells = <1>;
+		clocks = <&cs2600_kunit_refclk>;
+		clock-names = "ref_clk_in";
+		vdd-supply = <&dummy_vreg>;
+		cirrus,aux1-output-source = "clkin_missing";
+	};
+};
drivers/clk/cirrus/kunit_clk_cs2600_aux1_out_freq_unlock.dtso new file +17 -0
new file mode 100644
index 000000000000..2d295792ef68
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+#include "kunit_clk_cs2600.dtsi"
+
+&cs2600_kunit_i2c {
+	cs2600: cs2600@2c {
+		compatible = "cirrus,cs2600";
+		reg = <0x2c>;
+		#clock-cells = <1>;
+		clocks = <&cs2600_kunit_refclk>;
+		clock-names = "ref_clk_in";
+		vdd-supply = <&dummy_vreg>;
+		cirrus,aux1-output-source = "freq_unlock";
+	};
+};
drivers/clk/cirrus/kunit_clk_cs2600_aux1_out_phase_unlock.dtso new file +17 -0
new file mode 100644
index 000000000000..5c242cd06707
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+#include "kunit_clk_cs2600.dtsi"
+
+&cs2600_kunit_i2c {
+	cs2600: cs2600@2c {
+		compatible = "cirrus,cs2600";
+		reg = <0x2c>;
+		#clock-cells = <1>;
+		clocks = <&cs2600_kunit_refclk>;
+		clock-names = "ref_clk_in";
+		vdd-supply = <&dummy_vreg>;
+		cirrus,aux1-output-source = "phase_unlock";
+	};
+};
drivers/clk/cirrus/kunit_clk_cs2600_aux1_out_ref_clk_in.dtso new file +17 -0
new file mode 100644
index 000000000000..4e9205789ef4
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+#include "kunit_clk_cs2600.dtsi"
+
+&cs2600_kunit_i2c {
+	cs2600: cs2600@2c {
+		compatible = "cirrus,cs2600";
+		reg = <0x2c>;
+		#clock-cells = <1>;
+		clocks = <&cs2600_kunit_refclk>;
+		clock-names = "ref_clk_in";
+		vdd-supply = <&dummy_vreg>;
+		cirrus,aux1-output-source = "ref_clk_in";
+	};
+};
drivers/clk/cirrus/kunit_clk_cs2600_bclk_invert.dtso new file +17 -0
new file mode 100644
index 000000000000..c3d89d7c8020
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+#include "kunit_clk_cs2600.dtsi"
+
+&cs2600_kunit_i2c {
+	cs2600: cs2600@2c {
+		compatible = "cirrus,cs2600";
+		reg = <0x2c>;
+		#clock-cells = <1>;
+		clocks = <&cs2600_kunit_refclk>;
+		clock-names = "ref_clk_in";
+		vdd-supply = <&dummy_vreg>;
+		cirrus,bclk-invert;
+	};
+};
drivers/clk/cirrus/kunit_clk_cs2600_fsync_duty_1.dtso new file +17 -0
new file mode 100644
index 000000000000..f3e175c93c65
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+#include "kunit_clk_cs2600.dtsi"
+
+&cs2600_kunit_i2c {
+	cs2600: cs2600@2c {
+		compatible = "cirrus,cs2600";
+		reg = <0x2c>;
+		#clock-cells = <1>;
+		clocks = <&cs2600_kunit_refclk>;
+		clock-names = "ref_clk_in";
+		vdd-supply = <&dummy_vreg>;
+		cirrus,fsync-duty-cycles = <1>;
+	};
+};
drivers/clk/cirrus/kunit_clk_cs2600_fsync_duty_2.dtso new file +17 -0
new file mode 100644
index 000000000000..11f308e49ff2
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+#include "kunit_clk_cs2600.dtsi"
+
+&cs2600_kunit_i2c {
+	cs2600: cs2600@2c {
+		compatible = "cirrus,cs2600";
+		reg = <0x2c>;
+		#clock-cells = <1>;
+		clocks = <&cs2600_kunit_refclk>;
+		clock-names = "ref_clk_in";
+		vdd-supply = <&dummy_vreg>;
+		cirrus,fsync-duty-cycles = <2>;
+	};
+};
drivers/clk/cirrus/kunit_clk_cs2600_fsync_duty_32.dtso new file +17 -0
new file mode 100644
index 000000000000..00656af862d7
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+#include "kunit_clk_cs2600.dtsi"
+
+&cs2600_kunit_i2c {
+	cs2600: cs2600@2c {
+		compatible = "cirrus,cs2600";
+		reg = <0x2c>;
+		#clock-cells = <1>;
+		clocks = <&cs2600_kunit_refclk>;
+		clock-names = "ref_clk_in";
+		vdd-supply = <&dummy_vreg>;
+		cirrus,fsync-duty-cycles = <32>;
+	};
+};
drivers/clk/cirrus/kunit_clk_cs2600_fsync_invert.dtso new file +17 -0
new file mode 100644
index 000000000000..d85b5d851266
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+#include "kunit_clk_cs2600.dtsi"
+
+&cs2600_kunit_i2c {
+	cs2600: cs2600@2c {
+		compatible = "cirrus,cs2600";
+		reg = <0x2c>;
+		#clock-cells = <1>;
+		clocks = <&cs2600_kunit_refclk>;
+		clock-names = "ref_clk_in";
+		vdd-supply = <&dummy_vreg>;
+		cirrus,fsync-invert;
+	};
+};
drivers/clk/cirrus/kunit_clk_cs2600_manual_intosc_clkin.dtso new file +17 -0
new file mode 100644
index 000000000000..859b28daf25c
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+#include "kunit_clk_cs2600.dtsi"
+
+&cs2600_kunit_i2c {
+	cs2600: cs2600@2c {
+		compatible = "cirrus,cs2600";
+		reg = <0x2c>;
+		#clock-cells = <1>;
+		clocks = <&cs2600_kunit_refclk>, <&cs2600_kunit_clkin>;
+		clock-names = "ref_clk_in", "clk_in";
+		cirrus,internal-oscillator;
+		vdd-supply = <&dummy_vreg>;
+	};
+};
drivers/clk/cirrus/kunit_clk_cs2600_manual_intosc_only.dtso new file +17 -0
new file mode 100644
index 000000000000..dba29f398774
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+#include "kunit_clk_cs2600.dtsi"
+
+&cs2600_kunit_i2c {
+	cs2600: cs2600@2c {
+		compatible = "cirrus,cs2600";
+		reg = <0x2c>;
+		#clock-cells = <1>;
+		clocks = <&cs2600_kunit_refclk>;
+		clock-names = "ref_clk_in";
+		cirrus,internal-oscillator;
+		vdd-supply = <&dummy_vreg>;
+	};
+};
drivers/clk/cirrus/kunit_clk_cs2600_manual_refclk_clkin.dtso new file +16 -0
new file mode 100644
index 000000000000..5af4eb45fee0
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+#include "kunit_clk_cs2600.dtsi"
+
+&cs2600_kunit_i2c {
+	cs2600: cs2600@2c {
+		compatible = "cirrus,cs2600";
+		reg = <0x2c>;
+		#clock-cells = <1>;
+		clocks = <&cs2600_kunit_refclk>, <&cs2600_kunit_clkin>;
+		clock-names = "ref_clk_in", "clk_in";
+		vdd-supply = <&dummy_vreg>;
+	};
+};
drivers/clk/cirrus/kunit_clk_cs2600_manual_refclk_only.dtso new file +16 -0
new file mode 100644
index 000000000000..dc2bf6f4a90a
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+#include "kunit_clk_cs2600.dtsi"
+
+&cs2600_kunit_i2c {
+	cs2600: cs2600@2c {
+		compatible = "cirrus,cs2600";
+		reg = <0x2c>;
+		#clock-cells = <1>;
+		clocks = <&cs2600_kunit_refclk>;
+		clock-names = "ref_clk_in";
+		vdd-supply = <&dummy_vreg>;
+	};
+};
drivers/clk/cirrus/kunit_clk_cs2600_name_output_clocks.dtso new file +23 -0
new file mode 100644
index 000000000000..d05a69512bc6
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+#include "clk-cs2600-kunit.h"
+#include "kunit_clk_cs2600.dtsi"
+
+&cs2600_kunit_i2c {
+	cs2600: cs2600@2c {
+		compatible = "cirrus,cs2600";
+		reg = <0x2c>;
+		#clock-cells = <1>;
+		clocks = <&cs2600_kunit_refclk>, <&cs2600_kunit_clkin>;
+		clock-names = "ref_clk_in", "clk_in";
+		vdd-supply = <&dummy_vreg>;
+
+		clock-output-names =
+			CS2600_KUNIT_PLLOUT_RENAME,
+			CS2600_KUNIT_CLKOUT_RENAME,
+			CS2600_KUNIT_BCLK_RENAME,
+			CS2600_KUNIT_FSYNC_RENAME;
+	};
+};
drivers/clk/cirrus/kunit_clk_cs2600_smart_clkin_only_intosc.dtso new file +19 -0
new file mode 100644
index 000000000000..76fe2e5a01c9
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+#include "kunit_clk_cs2600.dtsi"
+
+&cs2600_kunit_i2c {
+	cs2600: cs2600@2c {
+		compatible = "cirrus,cs2600";
+		reg = <0x2c>;
+		#clock-cells = <1>;
+		clocks = <&cs2600_kunit_refclk>, <&cs2600_kunit_clkin>;
+		clock-names = "ref_clk_in", "clk_in";
+		cirrus,internal-oscillator;
+		cirrus,smart-mode;
+		cirrus,smart-mode-clkin-only;
+		vdd-supply = <&dummy_vreg>;
+	};
+};
drivers/clk/cirrus/kunit_clk_cs2600_smart_clkin_only_refclk.dtso new file +18 -0
new file mode 100644
index 000000000000..83e1b57deeaa
@@ -0,0 +1,18 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+#include "kunit_clk_cs2600.dtsi"
+
+&cs2600_kunit_i2c {
+	cs2600: cs2600@2c {
+		compatible = "cirrus,cs2600";
+		reg = <0x2c>;
+		#clock-cells = <1>;
+		clocks = <&cs2600_kunit_refclk>, <&cs2600_kunit_clkin>;
+		clock-names = "ref_clk_in", "clk_in";
+		cirrus,smart-mode;
+		cirrus,smart-mode-clkin-only;
+		vdd-supply = <&dummy_vreg>;
+	};
+};
drivers/clk/cirrus/kunit_clk_cs2600_smart_intosc.dtso new file +18 -0
new file mode 100644
index 000000000000..893406b49f1c
@@ -0,0 +1,18 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+#include "kunit_clk_cs2600.dtsi"
+
+&cs2600_kunit_i2c {
+	cs2600: cs2600@2c {
+		compatible = "cirrus,cs2600";
+		reg = <0x2c>;
+		#clock-cells = <1>;
+		clocks = <&cs2600_kunit_refclk>, <&cs2600_kunit_clkin>;
+		clock-names = "ref_clk_in", "clk_in";
+		cirrus,internal-oscillator;
+		cirrus,smart-mode;
+		vdd-supply = <&dummy_vreg>;
+	};
+};
drivers/clk/cirrus/kunit_clk_cs2600_smart_refclk.dtso new file +17 -0
new file mode 100644
index 000000000000..6f46834cd2de
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+#include "kunit_clk_cs2600.dtsi"
+
+&cs2600_kunit_i2c {
+	cs2600: cs2600@2c {
+		compatible = "cirrus,cs2600";
+		reg = <0x2c>;
+		#clock-cells = <1>;
+		clocks = <&cs2600_kunit_refclk>, <&cs2600_kunit_clkin>;
+		clock-names = "ref_clk_in", "clk_in";
+		cirrus,smart-mode;
+		vdd-supply = <&dummy_vreg>;
+	};
+};
Signature · 1 line
2.47.3
#7 lore raw

Re: [PATCH v7 3/3] clk: cs2600: Add KUnit test for CS2600 driver

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] Missing length validation for read messages in the dummy I2C transfer function allows a heap…

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing length validation for read messages in the dummy I2C transfer function allows a heap buffer overflow.
- [Medium] Integer overflow in delta frequency calculation causes improperly strict test failure bounds.
- [Medium] Device node reference leak on KUnit assertion failure.
--

--- Patch [3]: [PATCH v7 3/3] clk: cs2600: Add KUnit test for CS2600 driver --- commit 2362317ae5c3adcc9e6eb5a18429400bcf90b30f
Author: Richard Fitzgerald <[email protected]>

clk: cs2600: Add KUnit test for CS2600 driver

This commit adds a KUnit test for the CS2600 clock driver using a mock I2C host driver and regmap. It validates the frequency ratio and PLL configuration register fields against hardcoded tables of expected settings.

Quoted 5 lines
drivers/clk/cirrus/clk-cs2600-kunit.c new file +0 -0
new file mode 100644
index 0000000000000..2fd3f3ff4ef3f

[ ... ]

Quoted 1 line
drivers/clk/cirrus/clk-cs2600-kunit.c new file +1 -0
+	for (i = 0; cs2600_kunit_bclk_divs[i].pll_out != 0; i++) {

[ ... ]

drivers/clk/cirrus/clk-cs2600-kunit.c line 1458

Quoted 5 lines
drivers/clk/cirrus/clk-cs2600-kunit.c new file +5 -0
+		/*
+		 * Actual reported frequency can be slightly out because of
+		 * PLL resolution, but should be within worst-case PPM.
+		 */
+		fdelta = (cs2600_kunit_bclk_divs[i].bf_out * CS2600_20_12_PPM) >> 20;

[Severity: Medium]
Could this 32-bit multiplication overflow for large values of bf_out before the result is stored in the 64-bit fdelta?

[ ... ]

Quoted 1 line
drivers/clk/cirrus/clk-cs2600-kunit.c new file +1 -0
+	for (i = 0; cs2600_kunit_fsync_divs[i].pll_out != 0; i++) {

[ ... ]

drivers/clk/cirrus/clk-cs2600-kunit.c line 1536

Quoted 5 lines
drivers/clk/cirrus/clk-cs2600-kunit.c new file +5 -0
+		/*
+		 * Actual reported frequency can be slightly out because of
+		 * PLL resolution, but should be within worst-case PPM.
+		 */
+		fdelta = (cs2600_kunit_fsync_divs[i].bf_out * CS2600_20_12_PPM) >> 20;

[Severity: Medium]
Does this have the same 32-bit overflow vulnerability as the BCLK divider calculation above?

[ ... ]

drivers/clk/cirrus/clk-cs2600-kunit.c line 1697

Quoted 8 lines
drivers/clk/cirrus/clk-cs2600-kunit.c new file +8 -0
+	/* If there is a second packet it is a read */
+	if (num == 2) {
+		ret = cs2600_kunit_i2c_read(priv, addr, &val);
+		if (ret < 0)
+			return ret;
+
+		bebuf = (__force __be16 *)msgs[1].buf;
+		bebuf[0] = cpu_to_be16(val);

[Severity: High]
Could this write out of bounds if the read message length is less than 2 bytes, since msgs[1].len is not validated before the write?

Quoted 3 lines
drivers/clk/cirrus/clk-cs2600-kunit.c new file +3 -0
+
+		return 2;
+	}

[ ... ]

Quoted 2 lines
drivers/clk/cirrus/clk-cs2600-kunit.c new file +2 -0
+static int cs2600_kunit_case_common_init(struct kunit *test)
+{

[ ... ]

drivers/clk/cirrus/clk-cs2600-kunit.c line 1917

Quoted 5 lines
drivers/clk/cirrus/clk-cs2600-kunit.c new file +5 -0
+	/* Wait for probes if the test case init already loaded the overlay */
+	np = of_find_node_by_name(NULL, "cs2600-test-consumer");
+	if (!IS_ERR_OR_NULL(np))
+		cs2600_kunit_wait_for_probes(test);
+	of_node_put(np);

[Severity: Medium]
If an assertion inside cs2600_kunit_wait_for_probes() fails, the KUnit framework aborts the test thread. Does this result in a leaked device node reference since the of_node_put() call would be bypassed?

Quoted 3 lines
drivers/clk/cirrus/clk-cs2600-kunit.c new file +3 -0
+
+	return 0;
+}
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3