* [PATCH 0/6] Binding and driver for "dumb" clock generators
@ 2024-07-09 12:31 Heiko Stuebner
2024-07-09 12:31 ` [PATCH 1/6] dt-bindings: clocks: add binding for generic clock-generators Heiko Stuebner
` (6 more replies)
0 siblings, 7 replies; 17+ messages in thread
From: Heiko Stuebner @ 2024-07-09 12:31 UTC (permalink / raw)
To: mturquette, sboyd
Cc: robh, krzk+dt, conor+dt, heiko, quentin.schulz, linux-clk,
devicetree, linux-kernel, linux-arm-kernel, linux-rockchip
Rockchip boards with PCIe3 controllers inside the soc (rk3568, rk3588) have
external clock generators on the board to generate the needed 100MHz
reference clock the PCIe3 controller needs.
Often these clock generators need supplies to be enabled to run.
Modelling this clock has taken a number of shapes:
- The rk3568 Rock-3a modelled the generator-regulator as "phy-supply" [0]
&pcie30phy {
phy-supply = <&vcc3v3_pi6c_03>;
status = "okay";
};
which is of course not part of the binding
- On the Rock-5-ITX the supply of the clock generator is controlled by
the same gpio as the regulator supplying the the port connected to the
pcie30x4 controller, so if this controller probes first, both
controllers will just run. But if the pcie30x2 controller probes first
(which has a different supply), the controller will stall at the first
dbi read.
- Recent Theobroma-Systems boards (Jaguar and Tiger) modelled their
generator as a combination of fixed clock and gpio-gate, which works
because the generator's vdd-supply is always on and only the output-
enable pin needs to be toggled.
So this series attempts to improve the situation by allowing to model
these clock generators as actual entities in the devicetree, to not have
them just accidentially work or break bindings.
[0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts#n605
Heiko Stuebner (6):
dt-bindings: clocks: add binding for generic clock-generators
clk: add driver for generic clock generators
arm64: dts: rockchip: fix the pcie clock generator on Rock 5 ITX
arm64: dts: rockchip: use clock-generator for pcie-refclk on
rk3588-jaguar
arm64: dts: rockchip: use clock-generator for pcie-refclk on
rk3588-tiger
arm64: dts: rockchip: add pinctrl for clk-generator gpio on
rk3588-tiger
.../bindings/clock/clock-generator.yaml | 62 ++++++++
.../arm64/boot/dts/rockchip/rk3588-jaguar.dts | 13 +-
.../boot/dts/rockchip/rk3588-rock-5itx.dts | 34 ++++-
.../arm64/boot/dts/rockchip/rk3588-tiger.dtsi | 21 +--
drivers/clk/Kconfig | 7 +
drivers/clk/Makefile | 1 +
drivers/clk/clk-generator.c | 133 ++++++++++++++++++
7 files changed, 251 insertions(+), 20 deletions(-)
create mode 100644 Documentation/devicetree/bindings/clock/clock-generator.yaml
create mode 100644 drivers/clk/clk-generator.c
--
2.39.2
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/6] dt-bindings: clocks: add binding for generic clock-generators
2024-07-09 12:31 [PATCH 0/6] Binding and driver for "dumb" clock generators Heiko Stuebner
@ 2024-07-09 12:31 ` Heiko Stuebner
2024-07-09 21:45 ` Stephen Boyd
2024-07-10 7:02 ` Alexander Stein
2024-07-09 12:31 ` [PATCH 2/6] clk: add driver for generic clock generators Heiko Stuebner
` (5 subsequent siblings)
6 siblings, 2 replies; 17+ messages in thread
From: Heiko Stuebner @ 2024-07-09 12:31 UTC (permalink / raw)
To: mturquette, sboyd
Cc: robh, krzk+dt, conor+dt, heiko, quentin.schulz, linux-clk,
devicetree, linux-kernel, linux-arm-kernel, linux-rockchip
In contrast to fixed clocks that are described as ungateable, boards
sometimes use additional clock generators for things like PCIe reference
clocks, that need actual supplies to get enabled and enable-gpios to be
toggled for them to work.
This adds a binding for such clock generators that are not configurable
themself, but need to handle supplies for them to work.
While in a lot of cases the type of the IC used is described in board
schematics, in some cases just a generic type description like
"100MHz, 3.3V" might also be used. The binding therefore allows both
cases. Specifying the type is of course preferred.
The clock-frequency is set in devicetree, because while some clock
generators have pins to decide between multipls output rates, those
are generally set statically on the board-layout-level.
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
.../bindings/clock/clock-generator.yaml | 62 +++++++++++++++++++
1 file changed, 62 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/clock-generator.yaml
diff --git a/Documentation/devicetree/bindings/clock/clock-generator.yaml b/Documentation/devicetree/bindings/clock/clock-generator.yaml
new file mode 100644
index 0000000000000..f44e61e414e89
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/clock-generator.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/clock-generator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Simple clock generators
+
+maintainers:
+ - Heiko Stuebner <heiko@sntech.de>
+
+properties:
+ $nodename:
+ anyOf:
+ - description:
+ Preferred name is 'clock-<freq>' with <freq> being the output
+ frequency as defined in the 'clock-frequency' property.
+ pattern: "^clock-([0-9]+|[a-z0-9-]+)$"
+ - description: Any name allowed
+ deprecated: true
+
+ compatible:
+ oneOf:
+ - const: clock-generator
+ - items:
+ - enum:
+ - diodes,pi6c557-03b
+ - diodes,pi6c557-05b
+ - const: clock-generator
+
+ "#clock-cells":
+ const: 0
+
+ clock-frequency: true
+
+ clock-output-names:
+ maxItems: 1
+
+ enable-gpios:
+ description:
+ Contains a single GPIO specifier for the GPIO that enables and disables
+ the clock generator.
+ maxItems: 1
+
+ vdd-supply:
+ description: handle of the regulator that provides the supply voltage
+
+required:
+ - compatible
+ - "#clock-cells"
+ - clock-frequency
+
+additionalProperties: false
+
+examples:
+ - |
+ clock {
+ compatible = "clock-generator";
+ #clock-cells = <0>;
+ clock-frequency = <1000000000>;
+ };
+...
--
2.39.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/6] clk: add driver for generic clock generators
2024-07-09 12:31 [PATCH 0/6] Binding and driver for "dumb" clock generators Heiko Stuebner
2024-07-09 12:31 ` [PATCH 1/6] dt-bindings: clocks: add binding for generic clock-generators Heiko Stuebner
@ 2024-07-09 12:31 ` Heiko Stuebner
2024-07-09 12:31 ` [PATCH 3/6] arm64: dts: rockchip: fix the pcie clock generator on Rock 5 ITX Heiko Stuebner
` (4 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Heiko Stuebner @ 2024-07-09 12:31 UTC (permalink / raw)
To: mturquette, sboyd
Cc: robh, krzk+dt, conor+dt, heiko, quentin.schulz, linux-clk,
devicetree, linux-kernel, linux-arm-kernel, linux-rockchip
In contrast to fixed clocks that are described as ungateable, boards
sometimes use additional clock generators for things like PCIe reference
clocks, that need actual supplies to get enabled and enable-gpios to be
toggled for them to work.
This adds a driver for those generic unconfigurable clock generators.
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
drivers/clk/Kconfig | 7 ++
drivers/clk/Makefile | 1 +
drivers/clk/clk-generator.c | 133 ++++++++++++++++++++++++++++++++++++
3 files changed, 141 insertions(+)
create mode 100644 drivers/clk/clk-generator.c
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 3e9099504fad4..76c53ddc472ce 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -247,6 +247,13 @@ config COMMON_CLK_GEMINI
This driver supports the SoC clocks on the Cortina Systems Gemini
platform, also known as SL3516 or CS3516.
+config COMMON_CLK_GENERATOR
+ tristate "Clock driver for generic clock generators"
+ depends on GPIOLIB && REGULATOR
+ help
+ This driver supports generic clock generators that are not
+ configurable but need supplies to be enabled to run.
+
config COMMON_CLK_LAN966X
tristate "Generic Clock Controller driver for LAN966X SoC"
depends on HAS_IOMEM
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 4abe16c8ccdfe..9cb0801155771 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -36,6 +36,7 @@ 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
obj-$(CONFIG_COMMON_CLK_GEMINI) += clk-gemini.o
+obj-$(CONFIG_COMMON_CLK_GENERATOR) += clk-generator.o
obj-$(CONFIG_COMMON_CLK_ASPEED) += clk-aspeed.o
obj-$(CONFIG_MACH_ASPEED_G6) += clk-ast2600.o
obj-$(CONFIG_ARCH_HIGHBANK) += clk-highbank.o
diff --git a/drivers/clk/clk-generator.c b/drivers/clk/clk-generator.c
new file mode 100644
index 0000000000000..99737bab1f5ad
--- /dev/null
+++ b/drivers/clk/clk-generator.c
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2024 Heiko Stuebner <heiko@sntech.de>
+ *
+ * Generic unconfigurable clock generators
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+
+struct clk_generator {
+ struct device *dev;
+ struct clk_hw hw;
+ u32 rate;
+ struct regulator *supply;
+ struct gpio_desc *enable_gpio;
+};
+
+#define to_clk_generator(_hw) container_of(_hw, struct clk_generator, hw)
+
+static int clk_generator_prepare(struct clk_hw *hw)
+{
+ return regulator_enable(to_clk_generator(hw)->supply);
+}
+
+static void clk_generator_unprepare(struct clk_hw *hw)
+{
+ regulator_disable(to_clk_generator(hw)->supply);
+}
+
+static int clk_generator_enable(struct clk_hw *hw)
+{
+ gpiod_set_value(to_clk_generator(hw)->enable_gpio, 1);
+ return 0;
+}
+
+static void clk_generator_disable(struct clk_hw *hw)
+{
+ gpiod_set_value(to_clk_generator(hw)->enable_gpio, 0);
+}
+
+static unsigned long clk_generator_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ return to_clk_generator(hw)->rate;
+}
+
+const struct clk_ops clk_generator_ops = {
+ .prepare = clk_generator_prepare,
+ .unprepare = clk_generator_unprepare,
+ .enable = clk_generator_enable,
+ .disable = clk_generator_disable,
+ .recalc_rate = clk_generator_recalc_rate,
+};
+
+static int clk_generator_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct clk_generator *clkgen;
+ const char *clk_name;
+ int ret;
+
+ clkgen = devm_kzalloc(dev, sizeof(*clkgen), GFP_KERNEL);
+ if (!clkgen)
+ return -ENOMEM;
+
+ clkgen->dev = dev;
+
+ if (device_property_read_u32(dev, "clock-frequency", &clkgen->rate))
+ return -EIO;
+
+ ret = device_property_read_string(dev, "clock-output-names", &clk_name);
+ if (ret)
+ clk_name = fwnode_get_name(dev->fwnode);
+
+ clkgen->supply = devm_regulator_get_optional(dev, "vdd");
+ if (IS_ERR(clkgen->supply)) {
+ if (PTR_ERR(clkgen->supply) != -ENODEV)
+ return dev_err_probe(dev, PTR_ERR(clkgen->supply),
+ "failed to get regulator\n");
+ clkgen->supply = NULL;
+ }
+
+ clkgen->enable_gpio = devm_gpiod_get_optional(dev, "enable",
+ GPIOD_OUT_LOW);
+ if (IS_ERR(clkgen->enable_gpio))
+ return dev_err_probe(dev, PTR_ERR(clkgen->enable_gpio),
+ "failed to get gpio\n");
+
+ ret = gpiod_direction_output(clkgen->enable_gpio, 0);
+ if (ret < 0)
+ return ret;
+
+ clkgen->hw.init = CLK_HW_INIT_NO_PARENT(clk_name, &clk_generator_ops, 0);
+
+ /* register the clock */
+ ret = devm_clk_hw_register(dev, &clkgen->hw);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "failed to register clock\n");
+
+ ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get,
+ &clkgen->hw);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "failed to register clock provider\n");
+
+ return 0;
+}
+
+static const struct of_device_id clk_generator_ids[] = {
+ { .compatible = "clock-generator" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, clk_generator_ids);
+
+static struct platform_driver clk_generator_driver = {
+ .driver = {
+ .name = "clk_generator",
+ .of_match_table = clk_generator_ids,
+ },
+ .probe = clk_generator_probe,
+};
+module_platform_driver(clk_generator_driver);
+
+MODULE_AUTHOR("Heiko Stuebner <heiko@sntech.de>");
+MODULE_DESCRIPTION("Clock-generator driver");
+MODULE_LICENSE("GPL");
--
2.39.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/6] arm64: dts: rockchip: fix the pcie clock generator on Rock 5 ITX
2024-07-09 12:31 [PATCH 0/6] Binding and driver for "dumb" clock generators Heiko Stuebner
2024-07-09 12:31 ` [PATCH 1/6] dt-bindings: clocks: add binding for generic clock-generators Heiko Stuebner
2024-07-09 12:31 ` [PATCH 2/6] clk: add driver for generic clock generators Heiko Stuebner
@ 2024-07-09 12:31 ` Heiko Stuebner
2024-07-09 12:31 ` [PATCH 4/6] arm64: dts: rockchip: use clock-generator for pcie-refclk on rk3588-jaguar Heiko Stuebner
` (3 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Heiko Stuebner @ 2024-07-09 12:31 UTC (permalink / raw)
To: mturquette, sboyd
Cc: robh, krzk+dt, conor+dt, heiko, quentin.schulz, linux-clk,
devicetree, linux-kernel, linux-arm-kernel, linux-rockchip
The Rock 5 ITX uses two PCIe controllers to drive both a M.2 slot and its
SATA controller with 2 lanes each. The supply for the clock generator is
the same that supplies the M.2 slot, but the SATA controller port is
supplied by a different rail.
This leads to the effect that if the PCIe30x4 controller for the M.2
probes first, everything works normally. But if the PCIe30x2 controller
that is connected to the SATA controller probes first, it will hang on
the first DBI read as nothing will have enabled the refclock before.
Fix this by describing the clock generator with its supplies so that
both controllers can reference it as needed.
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
.../boot/dts/rockchip/rk3588-rock-5itx.dts | 34 +++++++++++++++++--
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5itx.dts b/arch/arm64/boot/dts/rockchip/rk3588-rock-5itx.dts
index 41d92ceeeb09c..21fb3d940b489 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5itx.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5itx.dts
@@ -77,6 +77,15 @@ hdd-led2 {
};
};
+ /* Unnamed clock generator: 100MHz,3.3V,3225 */
+ pcie30_port0_refclk: pcie30_port1_refclk: pcie-clock-generator {
+ compatible = "clock-generator";
+ #clock-cells = <0>;
+ clock-frequency = <100000000>;
+ clock-output-names = "pcie30_refclk";
+ vdd-supply = <&vcc3v3_pi6c_05>;
+ };
+
fan0: pwm-fan {
compatible = "pwm-fan";
#cooling-cells = <2>;
@@ -151,13 +160,14 @@ vcc3v3_lan: vcc3v3_lan_phy2: regulator-vcc3v3-lan {
vin-supply = <&vcc_3v3_s3>;
};
- vcc3v3_mkey: regulator-vcc3v3-mkey {
+ /* The PCIE30x4_PWREN_H controls two regulators */
+ vcc3v3_mkey: vcc3v3_pi6c_05: regulator-vcc3v3-pi6c-05 {
compatible = "regulator-fixed";
enable-active-high;
gpios = <&gpio1 RK_PA4 GPIO_ACTIVE_HIGH>;
pinctrl-names = "default";
pinctrl-0 = <&pcie30x4_pwren_h>;
- regulator-name = "vcc3v3_mkey";
+ regulator-name = "vcc3v3_pi6c_05";
regulator-min-microvolt = <3300000>;
regulator-max-microvolt = <3300000>;
startup-delay-us = <5000>;
@@ -519,6 +529,14 @@ &pcie30phy {
/* ASMedia ASM1164 Sata controller */
&pcie3x2 {
+ clocks = <&cru ACLK_PCIE_2L_MSTR>, <&cru ACLK_PCIE_2L_SLV>,
+ <&cru ACLK_PCIE_2L_DBI>, <&cru PCLK_PCIE_2L>,
+ <&cru CLK_PCIE_AUX1>, <&cru CLK_PCIE2L_PIPE>,
+ <&pcie30_port1_refclk>;
+ clock-names = "aclk_mst", "aclk_slv",
+ "aclk_dbi", "pclk",
+ "aux", "pipe",
+ "ref";
pinctrl-names = "default";
pinctrl-0 = <&pcie30x2_perstn_m1_l>;
reset-gpios = <&gpio4 RK_PB0 GPIO_ACTIVE_HIGH>;
@@ -528,6 +546,18 @@ &pcie3x2 {
/* M.2 M.key */
&pcie3x4 {
+ /*
+ * The board has a gpio-controlled "pcie_refclk" generator,
+ * so add it to the list of clocks.
+ */
+ clocks = <&cru ACLK_PCIE_4L_MSTR>, <&cru ACLK_PCIE_4L_SLV>,
+ <&cru ACLK_PCIE_4L_DBI>, <&cru PCLK_PCIE_4L>,
+ <&cru CLK_PCIE_AUX0>, <&cru CLK_PCIE4L_PIPE>,
+ <&pcie30_port0_refclk>;
+ clock-names = "aclk_mst", "aclk_slv",
+ "aclk_dbi", "pclk",
+ "aux", "pipe",
+ "ref";
num-lanes = <2>;
pinctrl-names = "default";
pinctrl-0 = <&pcie30x4_perstn_m1_l>;
--
2.39.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/6] arm64: dts: rockchip: use clock-generator for pcie-refclk on rk3588-jaguar
2024-07-09 12:31 [PATCH 0/6] Binding and driver for "dumb" clock generators Heiko Stuebner
` (2 preceding siblings ...)
2024-07-09 12:31 ` [PATCH 3/6] arm64: dts: rockchip: fix the pcie clock generator on Rock 5 ITX Heiko Stuebner
@ 2024-07-09 12:31 ` Heiko Stuebner
2024-07-09 12:31 ` [PATCH 5/6] arm64: dts: rockchip: use clock-generator for pcie-refclk on rk3588-tiger Heiko Stuebner
` (2 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Heiko Stuebner @ 2024-07-09 12:31 UTC (permalink / raw)
To: mturquette, sboyd
Cc: robh, krzk+dt, conor+dt, heiko, quentin.schulz, linux-clk,
devicetree, linux-kernel, linux-arm-kernel, linux-rockchip
Using a combination of fixed clock and gpio-gate clock works but does
not describe the actual hardware. Use the new clock-generator binding
to describe this in a nicer way.
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts b/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
index 71999f4f170af..b3c2aaedacf57 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588-jaguar.dts
@@ -88,21 +88,16 @@ led-1 {
* 100MHz reference clock for PCIe peripherals from PI6C557-05BLE
* clock generator.
* The clock output is gated via the OE pin on the clock generator.
- * This is modeled as a fixed-clock plus a gpio-gate-clock.
*/
- pcie_refclk_gen: pcie-refclk-gen-clock {
- compatible = "fixed-clock";
+ pcie_refclk: pcie-clock-generator {
+ compatible = "diodes,pi6c557-05b", "clock-generator";
#clock-cells = <0>;
clock-frequency = <100000000>;
- };
-
- pcie_refclk: pcie-refclk-clock {
- compatible = "gpio-gate-clock";
- clocks = <&pcie_refclk_gen>;
- #clock-cells = <0>;
+ clock-output-names = "pcie3_refclk";
enable-gpios = <&gpio0 RK_PC6 GPIO_ACTIVE_LOW>; /* PCIE30X4_CLKREQN_M0 */
pinctrl-names = "default";
pinctrl-0 = <&pcie30x4_clkreqn_m0>;
+ vdd-supply = <&vcca_3v3_s0>;
};
pps {
--
2.39.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 5/6] arm64: dts: rockchip: use clock-generator for pcie-refclk on rk3588-tiger
2024-07-09 12:31 [PATCH 0/6] Binding and driver for "dumb" clock generators Heiko Stuebner
` (3 preceding siblings ...)
2024-07-09 12:31 ` [PATCH 4/6] arm64: dts: rockchip: use clock-generator for pcie-refclk on rk3588-jaguar Heiko Stuebner
@ 2024-07-09 12:31 ` Heiko Stuebner
2024-07-09 12:31 ` [PATCH 6/6] arm64: dts: rockchip: add pinctrl for clk-generator gpio " Heiko Stuebner
2024-07-10 3:02 ` [PATCH 0/6] Binding and driver for "dumb" clock generators Anand Moon
6 siblings, 0 replies; 17+ messages in thread
From: Heiko Stuebner @ 2024-07-09 12:31 UTC (permalink / raw)
To: mturquette, sboyd
Cc: robh, krzk+dt, conor+dt, heiko, quentin.schulz, linux-clk,
devicetree, linux-kernel, linux-arm-kernel, linux-rockchip
Using a combination of fixed clock and gpio-gate clock works but does
not describe the actual hardware. Use the new clock-generator binding
to describe this in a nicer way.
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi
index f870f84da1e6d..4c5be356fa7fe 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi
@@ -49,19 +49,14 @@ led-1 {
* 100MHz reference clock for PCIe peripherals from PI6C557-05BLE
* clock generator.
* The clock output is gated via the OE pin on the clock generator.
- * This is modeled as a fixed-clock plus a gpio-gate-clock.
*/
- pcie_refclk_gen: pcie-refclk-gen-clock {
- compatible = "fixed-clock";
+ pcie_refclk: pcie-clock-generator {
+ compatible = "diodes,pi6c557-05b", "clock-generator";
#clock-cells = <0>;
clock-frequency = <100000000>;
- };
-
- pcie_refclk: pcie-refclk-clock {
- compatible = "gpio-gate-clock";
- clocks = <&pcie_refclk_gen>;
- #clock-cells = <0>;
+ clock-output-names = "pcie3_refclk";
enable-gpios = <&gpio4 RK_PB4 GPIO_ACTIVE_HIGH>; /* PCIE30X4_CLKREQN_M1_L */
+ vdd-supply = <&vcca_3v3_s0>;
};
vcc_1v1_nldo_s3: vcc-1v1-nldo-s3-regulator {
--
2.39.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 6/6] arm64: dts: rockchip: add pinctrl for clk-generator gpio on rk3588-tiger
2024-07-09 12:31 [PATCH 0/6] Binding and driver for "dumb" clock generators Heiko Stuebner
` (4 preceding siblings ...)
2024-07-09 12:31 ` [PATCH 5/6] arm64: dts: rockchip: use clock-generator for pcie-refclk on rk3588-tiger Heiko Stuebner
@ 2024-07-09 12:31 ` Heiko Stuebner
2024-07-10 3:02 ` [PATCH 0/6] Binding and driver for "dumb" clock generators Anand Moon
6 siblings, 0 replies; 17+ messages in thread
From: Heiko Stuebner @ 2024-07-09 12:31 UTC (permalink / raw)
To: mturquette, sboyd
Cc: robh, krzk+dt, conor+dt, heiko, quentin.schulz, linux-clk,
devicetree, linux-kernel, linux-arm-kernel, linux-rockchip
Having pinctrl entries defined for used gpios is helpful as it makes
sure the pin isn't used anywhere else.
The somewhat similar rk3588-jaguar board has a pinctrl entry already,
so add the same for rk3588-tiger.
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi
index 4c5be356fa7fe..fb5f1fa25fb9e 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-tiger.dtsi
@@ -56,6 +56,8 @@ pcie_refclk: pcie-clock-generator {
clock-frequency = <100000000>;
clock-output-names = "pcie3_refclk";
enable-gpios = <&gpio4 RK_PB4 GPIO_ACTIVE_HIGH>; /* PCIE30X4_CLKREQN_M1_L */
+ pinctrl-names = "default";
+ pinctrl-0 = <&pcie30x4_clkreqn_m1_l>;
vdd-supply = <&vcca_3v3_s0>;
};
@@ -339,6 +341,12 @@ module_led_pin: module-led-pin {
};
};
+ pcie30x4 {
+ pcie30x4_clkreqn_m1_l: pcie30x4-clkreqn-m1-l {
+ rockchip,pins = <4 RK_PB4 RK_FUNC_GPIO &pcfg_pull_none>;
+ };
+ };
+
usb3 {
usb3_id: usb3-id {
rockchip,pins =
--
2.39.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/6] dt-bindings: clocks: add binding for generic clock-generators
2024-07-09 12:31 ` [PATCH 1/6] dt-bindings: clocks: add binding for generic clock-generators Heiko Stuebner
@ 2024-07-09 21:45 ` Stephen Boyd
2024-07-10 8:02 ` Heiko Stübner
2024-07-10 7:02 ` Alexander Stein
1 sibling, 1 reply; 17+ messages in thread
From: Stephen Boyd @ 2024-07-09 21:45 UTC (permalink / raw)
To: Heiko Stuebner, mturquette
Cc: robh, krzk+dt, conor+dt, heiko, quentin.schulz, linux-clk,
devicetree, linux-kernel, linux-arm-kernel, linux-rockchip
Quoting Heiko Stuebner (2024-07-09 05:31:16)
> In contrast to fixed clocks that are described as ungateable, boards
> sometimes use additional clock generators for things like PCIe reference
> clocks, that need actual supplies to get enabled and enable-gpios to be
> toggled for them to work.
>
> This adds a binding for such clock generators that are not configurable
> themself, but need to handle supplies for them to work.
Sounds like vdd-supply is required?
>
> While in a lot of cases the type of the IC used is described in board
> schematics, in some cases just a generic type description like
> "100MHz, 3.3V" might also be used. The binding therefore allows both
> cases. Specifying the type is of course preferred.
>
> The clock-frequency is set in devicetree, because while some clock
> generators have pins to decide between multipls output rates, those
> are generally set statically on the board-layout-level.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
> .../bindings/clock/clock-generator.yaml | 62 +++++++++++++++++++
> 1 file changed, 62 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/clock-generator.yaml
>
> diff --git a/Documentation/devicetree/bindings/clock/clock-generator.yaml b/Documentation/devicetree/bindings/clock/clock-generator.yaml
> new file mode 100644
> index 0000000000000..f44e61e414e89
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/clock-generator.yaml
> @@ -0,0 +1,62 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/clock-generator.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Simple clock generators
> +
> +maintainers:
> + - Heiko Stuebner <heiko@sntech.de>
> +
> +properties:
> + $nodename:
> + anyOf:
> + - description:
> + Preferred name is 'clock-<freq>' with <freq> being the output
> + frequency as defined in the 'clock-frequency' property.
> + pattern: "^clock-([0-9]+|[a-z0-9-]+)$"
> + - description: Any name allowed
> + deprecated: true
Drop the deprecated stuff from the fixed-clock binding?
> +
> + compatible:
> + oneOf:
> + - const: clock-generator
> + - items:
> + - enum:
> + - diodes,pi6c557-03b
I see this datasheet[1]. Can that link be included somewhere? That shows
there's a clock input pin, which means I expect a 'clocks' property.
Maybe instead of creating a generic binding just make a binding for
these diodes parts? It certainly looks like a generic binding could come
later when another vendor supports the same binding.
> + - diodes,pi6c557-05b
> + - const: clock-generator
> +
> + "#clock-cells":
> + const: 0
> +
> + clock-frequency: true
> +
> + clock-output-names:
> + maxItems: 1
> +
> + enable-gpios:
> + description:
> + Contains a single GPIO specifier for the GPIO that enables and disables
> + the clock generator.
> + maxItems: 1
> +
> + vdd-supply:
> + description: handle of the regulator that provides the supply voltage
> +
> +required:
> + - compatible
> + - "#clock-cells"
> + - clock-frequency
This is the same required properties as fixed rate clocks. I'd guess
that at least 'enable-gpios' or 'vdd-supply' should also be required, or
the node would simply use fixed-clock compatible.
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + clock {
> + compatible = "clock-generator";
> + #clock-cells = <0>;
> + clock-frequency = <1000000000>;
> + };
[1] https://diodes.com/assets/Datasheets/PI6C557-03B.pdf
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/6] Binding and driver for "dumb" clock generators
2024-07-09 12:31 [PATCH 0/6] Binding and driver for "dumb" clock generators Heiko Stuebner
` (5 preceding siblings ...)
2024-07-09 12:31 ` [PATCH 6/6] arm64: dts: rockchip: add pinctrl for clk-generator gpio " Heiko Stuebner
@ 2024-07-10 3:02 ` Anand Moon
2024-07-10 7:50 ` Heiko Stübner
6 siblings, 1 reply; 17+ messages in thread
From: Anand Moon @ 2024-07-10 3:02 UTC (permalink / raw)
To: Heiko Stuebner
Cc: mturquette, sboyd, robh, krzk+dt, conor+dt, quentin.schulz,
linux-clk, devicetree, linux-kernel, linux-arm-kernel,
linux-rockchip
Hi Heiko,
On Tue, 9 Jul 2024 at 18:02, Heiko Stuebner <heiko@sntech.de> wrote:
>
> Rockchip boards with PCIe3 controllers inside the soc (rk3568, rk3588) have
> external clock generators on the board to generate the needed 100MHz
> reference clock the PCIe3 controller needs.
>
> Often these clock generators need supplies to be enabled to run.
>
> Modelling this clock has taken a number of shapes:
> - The rk3568 Rock-3a modelled the generator-regulator as "phy-supply" [0]
> &pcie30phy {
> phy-supply = <&vcc3v3_pi6c_03>;
> status = "okay";
> };
> which is of course not part of the binding
>
> - On the Rock-5-ITX the supply of the clock generator is controlled by
> the same gpio as the regulator supplying the the port connected to the
> pcie30x4 controller, so if this controller probes first, both
> controllers will just run. But if the pcie30x2 controller probes first
> (which has a different supply), the controller will stall at the first
> dbi read.
>
> - Recent Theobroma-Systems boards (Jaguar and Tiger) modelled their
> generator as a combination of fixed clock and gpio-gate, which works
> because the generator's vdd-supply is always on and only the output-
> enable pin needs to be toggled.
>
>
> So this series attempts to improve the situation by allowing to model
> these clock generators as actual entities in the devicetree, to not have
> them just accidentially work or break bindings.
>
I was wondering if these changes apply to Radxa Rock 5b SbC, it does not have
pi6c557 clock generator but the schematic supports GEN_CLK_100MHZ is
input to CLKin0 which is generated via the VCC3V3_PI6C_05 (100MHz,3.3V,3225)
regulator.
[1] https://dl.radxa.com/rock5/5b/docs/hw/radxa_rock5b_v13_sch.pdf
Thanks
-Anand
>
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts#n605
>
> Heiko Stuebner (6):
> dt-bindings: clocks: add binding for generic clock-generators
> clk: add driver for generic clock generators
> arm64: dts: rockchip: fix the pcie clock generator on Rock 5 ITX
> arm64: dts: rockchip: use clock-generator for pcie-refclk on
> rk3588-jaguar
> arm64: dts: rockchip: use clock-generator for pcie-refclk on
> rk3588-tiger
> arm64: dts: rockchip: add pinctrl for clk-generator gpio on
> rk3588-tiger
>
> .../bindings/clock/clock-generator.yaml | 62 ++++++++
> .../arm64/boot/dts/rockchip/rk3588-jaguar.dts | 13 +-
> .../boot/dts/rockchip/rk3588-rock-5itx.dts | 34 ++++-
> .../arm64/boot/dts/rockchip/rk3588-tiger.dtsi | 21 +--
> drivers/clk/Kconfig | 7 +
> drivers/clk/Makefile | 1 +
> drivers/clk/clk-generator.c | 133 ++++++++++++++++++
> 7 files changed, 251 insertions(+), 20 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/clock/clock-generator.yaml
> create mode 100644 drivers/clk/clk-generator.c
>
> --
> 2.39.2
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/6] dt-bindings: clocks: add binding for generic clock-generators
2024-07-09 12:31 ` [PATCH 1/6] dt-bindings: clocks: add binding for generic clock-generators Heiko Stuebner
2024-07-09 21:45 ` Stephen Boyd
@ 2024-07-10 7:02 ` Alexander Stein
2024-07-10 7:45 ` Heiko Stübner
1 sibling, 1 reply; 17+ messages in thread
From: Alexander Stein @ 2024-07-10 7:02 UTC (permalink / raw)
To: mturquette, sboyd, Heiko Stuebner
Cc: robh, krzk+dt, conor+dt, heiko, quentin.schulz, linux-clk,
devicetree, linux-kernel, linux-arm-kernel, linux-rockchip
Hello Heiko,
Am Dienstag, 9. Juli 2024, 14:31:16 CEST schrieb Heiko Stuebner:
> In contrast to fixed clocks that are described as ungateable, boards
> sometimes use additional clock generators for things like PCIe reference
> clocks, that need actual supplies to get enabled and enable-gpios to be
> toggled for them to work.
Fixed clocks are intended to be ungateable? Where does this come from?
> This adds a binding for such clock generators that are not configurable
> themself, but need to handle supplies for them to work.
>
> While in a lot of cases the type of the IC used is described in board
> schematics, in some cases just a generic type description like
> "100MHz, 3.3V" might also be used. The binding therefore allows both
> cases. Specifying the type is of course preferred.
>
> The clock-frequency is set in devicetree, because while some clock
> generators have pins to decide between multipls output rates, those
> are generally set statically on the board-layout-level.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
> .../bindings/clock/clock-generator.yaml | 62 +++++++++++++++++++
> 1 file changed, 62 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/clock-generator.yaml
>
> diff --git a/Documentation/devicetree/bindings/clock/clock-generator.yaml b/Documentation/devicetree/bindings/clock/clock-generator.yaml
> new file mode 100644
> index 0000000000000..f44e61e414e89
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/clock-generator.yaml
> @@ -0,0 +1,62 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/clock-generator.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Simple clock generators
> +
> +maintainers:
> + - Heiko Stuebner <heiko@sntech.de>
> +
> +properties:
> + $nodename:
> + anyOf:
> + - description:
> + Preferred name is 'clock-<freq>' with <freq> being the output
> + frequency as defined in the 'clock-frequency' property.
> + pattern: "^clock-([0-9]+|[a-z0-9-]+)$"
> + - description: Any name allowed
> + deprecated: true
> +
> + compatible:
> + oneOf:
> + - const: clock-generator
> + - items:
> + - enum:
> + - diodes,pi6c557-03b
> + - diodes,pi6c557-05b
> + - const: clock-generator
> +
> + "#clock-cells":
> + const: 0
> +
> + clock-frequency: true
> +
> + clock-output-names:
> + maxItems: 1
> +
> + enable-gpios:
> + description:
> + Contains a single GPIO specifier for the GPIO that enables and disables
> + the clock generator.
> + maxItems: 1
> +
> + vdd-supply:
> + description: handle of the regulator that provides the supply voltage
So essentially only enable-gpios and vdd-supply is added in comparison to
fixed-clock. Does it make sense to add that to the fixed-clocks instead?
Similar to fixed-regulator.
> +
> +required:
> + - compatible
> + - "#clock-cells"
> + - clock-frequency
With this list it's essentially the same as fixed-clock.
Best regards,
Alexander
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + clock {
> + compatible = "clock-generator";
> + #clock-cells = <0>;
> + clock-frequency = <1000000000>;
> + };
> +...
>
--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/6] dt-bindings: clocks: add binding for generic clock-generators
2024-07-10 7:02 ` Alexander Stein
@ 2024-07-10 7:45 ` Heiko Stübner
2024-07-10 23:21 ` Stephen Boyd
0 siblings, 1 reply; 17+ messages in thread
From: Heiko Stübner @ 2024-07-10 7:45 UTC (permalink / raw)
To: mturquette, sboyd, Alexander Stein
Cc: robh, krzk+dt, conor+dt, quentin.schulz, linux-clk, devicetree,
linux-kernel, linux-arm-kernel, linux-rockchip
Hi,
Am Mittwoch, 10. Juli 2024, 09:02:34 CEST schrieb Alexander Stein:
> Am Dienstag, 9. Juli 2024, 14:31:16 CEST schrieb Heiko Stuebner:
> > In contrast to fixed clocks that are described as ungateable, boards
> > sometimes use additional clock generators for things like PCIe reference
> > clocks, that need actual supplies to get enabled and enable-gpios to be
> > toggled for them to work.
>
> Fixed clocks are intended to be ungateable? Where does this come from?
"DOC: basic fixed-rate clock that cannot gate"
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/clk-fixed-rate.c#n18
> > This adds a binding for such clock generators that are not configurable
> > themself, but need to handle supplies for them to work.
> >
> > While in a lot of cases the type of the IC used is described in board
> > schematics, in some cases just a generic type description like
> > "100MHz, 3.3V" might also be used. The binding therefore allows both
> > cases. Specifying the type is of course preferred.
> >
> > The clock-frequency is set in devicetree, because while some clock
> > generators have pins to decide between multipls output rates, those
> > are generally set statically on the board-layout-level.
> >
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > ---
> > .../bindings/clock/clock-generator.yaml | 62 +++++++++++++++++++
> > 1 file changed, 62 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/clock/clock-generator.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/clock/clock-generator.yaml b/Documentation/devicetree/bindings/clock/clock-generator.yaml
> > new file mode 100644
> > index 0000000000000..f44e61e414e89
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/clock-generator.yaml
> > @@ -0,0 +1,62 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/clock-generator.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Simple clock generators
> > +
> > +maintainers:
> > + - Heiko Stuebner <heiko@sntech.de>
> > +
> > +properties:
> > + $nodename:
> > + anyOf:
> > + - description:
> > + Preferred name is 'clock-<freq>' with <freq> being the output
> > + frequency as defined in the 'clock-frequency' property.
> > + pattern: "^clock-([0-9]+|[a-z0-9-]+)$"
> > + - description: Any name allowed
> > + deprecated: true
> > +
> > + compatible:
> > + oneOf:
> > + - const: clock-generator
> > + - items:
> > + - enum:
> > + - diodes,pi6c557-03b
> > + - diodes,pi6c557-05b
> > + - const: clock-generator
> > +
> > + "#clock-cells":
> > + const: 0
> > +
> > + clock-frequency: true
> > +
> > + clock-output-names:
> > + maxItems: 1
> > +
> > + enable-gpios:
> > + description:
> > + Contains a single GPIO specifier for the GPIO that enables and disables
> > + the clock generator.
> > + maxItems: 1
> > +
> > + vdd-supply:
> > + description: handle of the regulator that provides the supply voltage
>
> So essentially only enable-gpios and vdd-supply is added in comparison to
> fixed-clock. Does it make sense to add that to the fixed-clocks instead?
> Similar to fixed-regulator.
I wasn't that sure which way to go in the first place.
The deciding point was reading that line about the fixed clock not
being gateable, so I opted to not touch the fixed-clock.
But you're definitly right, this _could_ live inside the fixed-clock
as well, if we decide to get rid of the not-gateable thing above.
Heiko
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/6] Binding and driver for "dumb" clock generators
2024-07-10 3:02 ` [PATCH 0/6] Binding and driver for "dumb" clock generators Anand Moon
@ 2024-07-10 7:50 ` Heiko Stübner
0 siblings, 0 replies; 17+ messages in thread
From: Heiko Stübner @ 2024-07-10 7:50 UTC (permalink / raw)
To: Anand Moon
Cc: mturquette, sboyd, robh, krzk+dt, conor+dt, quentin.schulz,
linux-clk, devicetree, linux-kernel, linux-arm-kernel,
linux-rockchip
Hi Anand,
Am Mittwoch, 10. Juli 2024, 05:02:57 CEST schrieb Anand Moon:
> On Tue, 9 Jul 2024 at 18:02, Heiko Stuebner <heiko@sntech.de> wrote:
> >
> > Rockchip boards with PCIe3 controllers inside the soc (rk3568, rk3588) have
> > external clock generators on the board to generate the needed 100MHz
> > reference clock the PCIe3 controller needs.
> >
> > Often these clock generators need supplies to be enabled to run.
> >
> > Modelling this clock has taken a number of shapes:
> > - The rk3568 Rock-3a modelled the generator-regulator as "phy-supply" [0]
> > &pcie30phy {
> > phy-supply = <&vcc3v3_pi6c_03>;
> > status = "okay";
> > };
> > which is of course not part of the binding
> >
> > - On the Rock-5-ITX the supply of the clock generator is controlled by
> > the same gpio as the regulator supplying the the port connected to the
> > pcie30x4 controller, so if this controller probes first, both
> > controllers will just run. But if the pcie30x2 controller probes first
> > (which has a different supply), the controller will stall at the first
> > dbi read.
> >
> > - Recent Theobroma-Systems boards (Jaguar and Tiger) modelled their
> > generator as a combination of fixed clock and gpio-gate, which works
> > because the generator's vdd-supply is always on and only the output-
> > enable pin needs to be toggled.
> >
> >
> > So this series attempts to improve the situation by allowing to model
> > these clock generators as actual entities in the devicetree, to not have
> > them just accidentially work or break bindings.
> >
>
> I was wondering if these changes apply to Radxa Rock 5b SbC, it does not have
> pi6c557 clock generator but the schematic supports GEN_CLK_100MHZ is
> input to CLKin0 which is generated via the VCC3V3_PI6C_05 (100MHz,3.3V,3225)
> regulator.
yes, that is the same setup as on the Rock 5 ITX, see it's patch in
this series.
The difference being that for your Rock 5b it already works by accident,
as the pcie30x4 port-supply and the supply for the clock-generator are
controlled by the same gpio.
So it does make sense to model this correctly for correctness sake, but
there is no immediate need for action, as there is no fault happening
like on the Rock 5 ITX with its two separate ports.
Heiko
> > [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts#n605
> >
> > Heiko Stuebner (6):
> > dt-bindings: clocks: add binding for generic clock-generators
> > clk: add driver for generic clock generators
> > arm64: dts: rockchip: fix the pcie clock generator on Rock 5 ITX
> > arm64: dts: rockchip: use clock-generator for pcie-refclk on
> > rk3588-jaguar
> > arm64: dts: rockchip: use clock-generator for pcie-refclk on
> > rk3588-tiger
> > arm64: dts: rockchip: add pinctrl for clk-generator gpio on
> > rk3588-tiger
> >
> > .../bindings/clock/clock-generator.yaml | 62 ++++++++
> > .../arm64/boot/dts/rockchip/rk3588-jaguar.dts | 13 +-
> > .../boot/dts/rockchip/rk3588-rock-5itx.dts | 34 ++++-
> > .../arm64/boot/dts/rockchip/rk3588-tiger.dtsi | 21 +--
> > drivers/clk/Kconfig | 7 +
> > drivers/clk/Makefile | 1 +
> > drivers/clk/clk-generator.c | 133 ++++++++++++++++++
> > 7 files changed, 251 insertions(+), 20 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/clock/clock-generator.yaml
> > create mode 100644 drivers/clk/clk-generator.c
> >
> > --
> > 2.39.2
> >
> >
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/6] dt-bindings: clocks: add binding for generic clock-generators
2024-07-09 21:45 ` Stephen Boyd
@ 2024-07-10 8:02 ` Heiko Stübner
2024-07-10 23:56 ` Stephen Boyd
0 siblings, 1 reply; 17+ messages in thread
From: Heiko Stübner @ 2024-07-10 8:02 UTC (permalink / raw)
To: mturquette, Stephen Boyd
Cc: robh, krzk+dt, conor+dt, quentin.schulz, linux-clk, devicetree,
linux-kernel, linux-arm-kernel, linux-rockchip
Hi Stephen,
Am Dienstag, 9. Juli 2024, 23:45:20 CEST schrieb Stephen Boyd:
> Quoting Heiko Stuebner (2024-07-09 05:31:16)
> > In contrast to fixed clocks that are described as ungateable, boards
> > sometimes use additional clock generators for things like PCIe reference
> > clocks, that need actual supplies to get enabled and enable-gpios to be
> > toggled for them to work.
> >
> > This adds a binding for such clock generators that are not configurable
> > themself, but need to handle supplies for them to work.
>
> Sounds like vdd-supply is required?
yeah, I guess this could move to required
> >
> > While in a lot of cases the type of the IC used is described in board
> > schematics, in some cases just a generic type description like
> > "100MHz, 3.3V" might also be used. The binding therefore allows both
> > cases. Specifying the type is of course preferred.
> >
> > The clock-frequency is set in devicetree, because while some clock
> > generators have pins to decide between multipls output rates, those
> > are generally set statically on the board-layout-level.
> >
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > ---
> > .../bindings/clock/clock-generator.yaml | 62 +++++++++++++++++++
> > 1 file changed, 62 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/clock/clock-generator.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/clock/clock-generator.yaml b/Documentation/devicetree/bindings/clock/clock-generator.yaml
> > new file mode 100644
> > index 0000000000000..f44e61e414e89
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/clock-generator.yaml
> > @@ -0,0 +1,62 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/clock-generator.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Simple clock generators
> > +
> > +maintainers:
> > + - Heiko Stuebner <heiko@sntech.de>
> > +
> > +properties:
> > + $nodename:
> > + anyOf:
> > + - description:
> > + Preferred name is 'clock-<freq>' with <freq> being the output
> > + frequency as defined in the 'clock-frequency' property.
> > + pattern: "^clock-([0-9]+|[a-z0-9-]+)$"
> > + - description: Any name allowed
> > + deprecated: true
>
> Drop the deprecated stuff from the fixed-clock binding?
ok
>
> > +
> > + compatible:
> > + oneOf:
> > + - const: clock-generator
> > + - items:
> > + - enum:
> > + - diodes,pi6c557-03b
>
> I see this datasheet[1]. Can that link be included somewhere? That shows
> there's a clock input pin, which means I expect a 'clocks' property.
ok, I guess we could add a clock property and make it mandatory for the
Diodes compatibles.
> Maybe instead of creating a generic binding just make a binding for
> these diodes parts? It certainly looks like a generic binding could come
> later when another vendor supports the same binding.
I was actually primarily aiming at solving the Rock 5 ITX clock generator
issue described in patch 5, where the 100 MHz clock generator is just
described as "100MHz,3.3V,3225" in the schematics, but definitly needs
the supply regulator to be enabled [1].
The Theobroma boards were addons because I felt they could also describe
the hardware better instead of lumping together 2 existing bindings.
> > + - diodes,pi6c557-05b
> > + - const: clock-generator
> > +
> > + "#clock-cells":
> > + const: 0
> > +
> > + clock-frequency: true
> > +
> > + clock-output-names:
> > + maxItems: 1
> > +
> > + enable-gpios:
> > + description:
> > + Contains a single GPIO specifier for the GPIO that enables and disables
> > + the clock generator.
> > + maxItems: 1
> > +
> > + vdd-supply:
> > + description: handle of the regulator that provides the supply voltage
> > +
> > +required:
> > + - compatible
> > + - "#clock-cells"
> > + - clock-frequency
>
> This is the same required properties as fixed rate clocks. I'd guess
> that at least 'enable-gpios' or 'vdd-supply' should also be required, or
> the node would simply use fixed-clock compatible.
ok, as said above, having the vdd-supply as requirement does make sense.
Heiko
[1] https://dl.radxa.com/rock5/5itx/radxa_rock_5_itx_X1100_schematic.pdf
page 35, bottom left, to the left of the big Au5426 "clock buffer".
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Re: [PATCH 1/6] dt-bindings: clocks: add binding for generic clock-generators
2024-07-10 7:45 ` Heiko Stübner
@ 2024-07-10 23:21 ` Stephen Boyd
2024-07-11 5:27 ` Alexander Stein
0 siblings, 1 reply; 17+ messages in thread
From: Stephen Boyd @ 2024-07-10 23:21 UTC (permalink / raw)
To: Alexander Stein, Heiko Stübner, mturquette
Cc: robh, krzk+dt, conor+dt, quentin.schulz, linux-clk, devicetree,
linux-kernel, linux-arm-kernel, linux-rockchip
Quoting Heiko Stübner (2024-07-10 00:45:17)
> Am Mittwoch, 10. Juli 2024, 09:02:34 CEST schrieb Alexander Stein:
> >
> > So essentially only enable-gpios and vdd-supply is added in comparison to
> > fixed-clock. Does it make sense to add that to the fixed-clocks instead?
> > Similar to fixed-regulator.
>
> I wasn't that sure which way to go in the first place.
> The deciding point was reading that line about the fixed clock not
> being gateable, so I opted to not touch the fixed-clock.
>
> But you're definitly right, this _could_ live inside the fixed-clock
> as well, if we decide to get rid of the not-gateable thing above.
It's probably more complicated to combine it with the fixed-clock
binding after making properties required like vdd-supply. I'd just make
a new binding and look at combining later.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Re: [PATCH 1/6] dt-bindings: clocks: add binding for generic clock-generators
2024-07-10 8:02 ` Heiko Stübner
@ 2024-07-10 23:56 ` Stephen Boyd
0 siblings, 0 replies; 17+ messages in thread
From: Stephen Boyd @ 2024-07-10 23:56 UTC (permalink / raw)
To: Heiko Stübner, mturquette
Cc: robh, krzk+dt, conor+dt, quentin.schulz, linux-clk, devicetree,
linux-kernel, linux-arm-kernel, linux-rockchip
Quoting Heiko Stübner (2024-07-10 01:02:57)
> Am Dienstag, 9. Juli 2024, 23:45:20 CEST schrieb Stephen Boyd:
> > Quoting Heiko Stuebner (2024-07-09 05:31:16)
> > > diff --git a/Documentation/devicetree/bindings/clock/clock-generator.yaml b/Documentation/devicetree/bindings/clock/clock-generator.yaml
> > > new file mode 100644
> > > index 0000000000000..f44e61e414e89
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/clock/clock-generator.yaml
> > > @@ -0,0 +1,62 @@
[...]
>
>
> > Maybe instead of creating a generic binding just make a binding for
> > these diodes parts? It certainly looks like a generic binding could come
> > later when another vendor supports the same binding.
>
> I was actually primarily aiming at solving the Rock 5 ITX clock generator
> issue described in patch 5, where the 100 MHz clock generator is just
> described as "100MHz,3.3V,3225" in the schematics, but definitly needs
> the supply regulator to be enabled [1].
That looks like a VCO (voltage controlled oscillator). Maybe the
compatible string can be "voltage-oscillator" or "clock-vco" and it can
require the vdd-supply.
Those diodes parts look different. They look like PLLs that have a
reference clock, hence the 'clocks' property I was expecting to see.
That would use the VCO you have to make the PCIE reference clk
frequencies. A generic compatible for those diodes parts is likely
"phase-locked-loop", or "clock-pll", but I'd avoid that given that PLLs
are almost always complicated and can have multiple output frequencies
if they have post and pre-dividers, etc. It's easier to be specific
here and make a binding for the part you have.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/6] dt-bindings: clocks: add binding for generic clock-generators
2024-07-10 23:21 ` Stephen Boyd
@ 2024-07-11 5:27 ` Alexander Stein
2024-07-15 10:59 ` Heiko Stübner
0 siblings, 1 reply; 17+ messages in thread
From: Alexander Stein @ 2024-07-11 5:27 UTC (permalink / raw)
To: Heiko Stübner, mturquette, Stephen Boyd
Cc: robh, krzk+dt, conor+dt, quentin.schulz, linux-clk, devicetree,
linux-kernel, linux-arm-kernel, linux-rockchip
Am Donnerstag, 11. Juli 2024, 01:21:15 CEST schrieb Stephen Boyd:
> Quoting Heiko Stübner (2024-07-10 00:45:17)
> > Am Mittwoch, 10. Juli 2024, 09:02:34 CEST schrieb Alexander Stein:
> > >
> > > So essentially only enable-gpios and vdd-supply is added in comparison to
> > > fixed-clock. Does it make sense to add that to the fixed-clocks instead?
> > > Similar to fixed-regulator.
> >
> > I wasn't that sure which way to go in the first place.
> > The deciding point was reading that line about the fixed clock not
> > being gateable, so I opted to not touch the fixed-clock.
> >
> > But you're definitly right, this _could_ live inside the fixed-clock
> > as well, if we decide to get rid of the not-gateable thing above.
>
> It's probably more complicated to combine it with the fixed-clock
> binding after making properties required like vdd-supply. I'd just make
> a new binding and look at combining later.
Maybe I am missing something IMHO adding optional vdd-supply and
enable-gpios doesn't seem a big deal.
Anyway I don't have a hard opinion here. To me fixed-clocks still
seems very appropriate for having a controlling GPIO and power supply.
I just would get rid of the (comment only) hint they are ungatable.
Best regards,
Alexander
--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/6] dt-bindings: clocks: add binding for generic clock-generators
2024-07-11 5:27 ` Alexander Stein
@ 2024-07-15 10:59 ` Heiko Stübner
0 siblings, 0 replies; 17+ messages in thread
From: Heiko Stübner @ 2024-07-15 10:59 UTC (permalink / raw)
To: mturquette, Stephen Boyd, Alexander Stein
Cc: robh, krzk+dt, conor+dt, quentin.schulz, linux-clk, devicetree,
linux-kernel, linux-arm-kernel, linux-rockchip
Am Donnerstag, 11. Juli 2024, 07:27:40 CEST schrieb Alexander Stein:
> Am Donnerstag, 11. Juli 2024, 01:21:15 CEST schrieb Stephen Boyd:
> > Quoting Heiko Stübner (2024-07-10 00:45:17)
> > > Am Mittwoch, 10. Juli 2024, 09:02:34 CEST schrieb Alexander Stein:
> > > >
> > > > So essentially only enable-gpios and vdd-supply is added in comparison to
> > > > fixed-clock. Does it make sense to add that to the fixed-clocks instead?
> > > > Similar to fixed-regulator.
> > >
> > > I wasn't that sure which way to go in the first place.
> > > The deciding point was reading that line about the fixed clock not
> > > being gateable, so I opted to not touch the fixed-clock.
> > >
> > > But you're definitly right, this _could_ live inside the fixed-clock
> > > as well, if we decide to get rid of the not-gateable thing above.
> >
> > It's probably more complicated to combine it with the fixed-clock
> > binding after making properties required like vdd-supply. I'd just make
> > a new binding and look at combining later.
>
> Maybe I am missing something IMHO adding optional vdd-supply and
> enable-gpios doesn't seem a big deal.
> Anyway I don't have a hard opinion here. To me fixed-clocks still
> seems very appropriate for having a controlling GPIO and power supply.
> I just would get rid of the (comment only) hint they are ungatable.
I think the main issue is that the fixed-rate code is not limited to the
actual fixed-rate clock.
The clk_fixed_rate_ops is exported and used itself in a number of
completely different clock drivers. The same is true for the
clk_register_fixed_rate function, also exported and used in even more
places throughout the kernel while implicitly using clk_fixed_rate_ops.
For just being a simple always-on fixed rate clock, the clk-fixed-rate.c is
already pretty complex and adding supply handling will entail modifying
the shared clk-ops, or defining a separate clk-ops and clk-register
function, which is what we're doing here already.
Heiko
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2024-07-15 10:59 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-09 12:31 [PATCH 0/6] Binding and driver for "dumb" clock generators Heiko Stuebner
2024-07-09 12:31 ` [PATCH 1/6] dt-bindings: clocks: add binding for generic clock-generators Heiko Stuebner
2024-07-09 21:45 ` Stephen Boyd
2024-07-10 8:02 ` Heiko Stübner
2024-07-10 23:56 ` Stephen Boyd
2024-07-10 7:02 ` Alexander Stein
2024-07-10 7:45 ` Heiko Stübner
2024-07-10 23:21 ` Stephen Boyd
2024-07-11 5:27 ` Alexander Stein
2024-07-15 10:59 ` Heiko Stübner
2024-07-09 12:31 ` [PATCH 2/6] clk: add driver for generic clock generators Heiko Stuebner
2024-07-09 12:31 ` [PATCH 3/6] arm64: dts: rockchip: fix the pcie clock generator on Rock 5 ITX Heiko Stuebner
2024-07-09 12:31 ` [PATCH 4/6] arm64: dts: rockchip: use clock-generator for pcie-refclk on rk3588-jaguar Heiko Stuebner
2024-07-09 12:31 ` [PATCH 5/6] arm64: dts: rockchip: use clock-generator for pcie-refclk on rk3588-tiger Heiko Stuebner
2024-07-09 12:31 ` [PATCH 6/6] arm64: dts: rockchip: add pinctrl for clk-generator gpio " Heiko Stuebner
2024-07-10 3:02 ` [PATCH 0/6] Binding and driver for "dumb" clock generators Anand Moon
2024-07-10 7:50 ` Heiko Stübner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).