clk: Add basic register clock controller
Subsystems (2)
v2 cover clk: Add basic register clock controller
These clocks were provided in include/linux/clk-provider.h, but lacks DT bindings. Add a clock controller to avoid operation conflict on same register.
v2: split clock controller and its clocks
Links:
v1: https://lore.kernel.org/r/[email protected]
David Yang (4):
dt-bindings: clock: Add simple-clock-controller
clk: Add simple clock controller
dt-bindings: clock: Add gate-clock
clk: gate: Add DT binding
.../devicetree/bindings/clock/gate-clock.yaml | 58 ++++ .../clock/simple-clock-controller.yaml | 50 +++ drivers/clk/Makefile | 1 + drivers/clk/clk-gate.c | 81 +++++ drivers/clk/clk-of.c | 292 ++++++++++++++++++ drivers/clk/clk-of.h | 26 ++ 6 files changed, 508 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/gate-clock.yaml create mode 100644 Documentation/devicetree/bindings/clock/simple-clock-controller.yaml create mode 100644 drivers/clk/clk-of.c create mode 100644 drivers/clk/clk-of.h
base-commit: 7a934f4bd7d6f9da84c8812da3ba42ee10f5778e
Signature · 1 line
2.39.2
PATCH v2 1/4 dt-bindings: clock: Add simple-clock-controller
Add DT bindings documentation for simple-clock-controller, mutex controller for clocks.
--- .../clock/simple-clock-controller.yaml | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/simple-clock-controller.yaml
Documentation/devicetree/bindings/clock/simple-clock-controller.yaml new file +50 -0
@@ -0,0 +1,50 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/clock/simple-clock-controller.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Simple clock controller + +maintainers: + - David Yang <[email protected]> + +description: | + Driver (lock provider) for real clocks. + + Usually one register controls more than one clocks. This controller avoids + write conflicts by imposing a write lock, so that two operations on the same + register will not happen at the same time. + +properties: + compatible: + items: + - oneOf: + - const: simple-clock-controller + - const: simple-clock-reset-controller + - const: syscon + - const: simple-mfd + + reg: + maxItems: 1 + + '#reset-cells': + const: 2 + +patternProperties: + "clock@.*": + type: object + description: Clock nodes. + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + clock-controller@ffff000 { + compatible = "simple-clock-controller", "syscon", "simple-mfd"; + reg = <0xffff000 0x1000>; + };
Signature · 1 line
2.39.2
Re: [PATCH v2 1/4] dt-bindings: clock: Add simple-clock-controller
Quoted 6 lines · David Yang
Add DT bindings documentation for simple-clock-controller, mutex controller for clocks.
Signed-off-by David Yang <[email protected]>--- .../clock/simple-clock-controller.yaml | 50 +++++++++++++++++++
Where is the changelog?
Quoted 12 lines
1 file changed, 50 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/simple-clock-controller.yamlDocumentation/devicetree/bindings/clock/simple-clock-controller.yaml new file +3
-0@@ -0,0 +1,50 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +---
Documentation/devicetree/bindings/clock/simple-clock-controller.yaml line 13
Quoted 10 lines
Documentation/devicetree/bindings/clock/simple-clock-controller.yaml new file +10
-0+$id: http://devicetree.org/schemas/clock/simple-clock-controller.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Simple clock controller + +maintainers: + - David Yang <[email protected]> + +description: | + Driver (lock provider) for real clocks.
Drop driver references. Typo: clock, not lock.
What is a real clock? What is an unreal clock?
Documentation/devicetree/bindings/clock/simple-clock-controller.yaml line 17
Quoted 4 lines
Documentation/devicetree/bindings/clock/simple-clock-controller.yaml new file +4
-0+ + Usually one register controls more than one clocks. This controller avoids + write conflicts by imposing a write lock, so that two operations on the same + register will not happen at the same time.
Interesting. How the clock controller imposes write locks? Aren't you now mixing drivers and hardware?
Documentation/devicetree/bindings/clock/simple-clock-controller.yaml line 24
Quoted 7 lines
Documentation/devicetree/bindings/clock/simple-clock-controller.yaml new file +7
-0+ +properties: + compatible: + items: + - oneOf: + - const: simple-clock-controller + - const: simple-clock-reset-controller
Why two?
Documentation/devicetree/bindings/clock/simple-clock-controller.yaml line 26
Quoted 2 lines
Documentation/devicetree/bindings/clock/simple-clock-controller.yaml new file +2
-0+ - const: syscon + - const: simple-mfd
Why do you need syscon and simple-mfd?
Documentation/devicetree/bindings/clock/simple-clock-controller.yaml line 35
Quoted 9 lines
Documentation/devicetree/bindings/clock/simple-clock-controller.yaml new file +9
-0+ + reg: + maxItems: 1 + + '#reset-cells': + const: 2 + +patternProperties: + "clock@.*":
Use consistent quotes.
Anyway, I don't understand what is happening here and why such changes. Nothing is explained...
Best regards,
Krzysztof
Re: [PATCH v2 1/4] dt-bindings: clock: Add simple-clock-controller
Quoted 10 lines · Krzysztof Kozlowski <[email protected]> 于2023年4月17日周一 01:38写道：
Quoted 6 lines · David Yang
Add DT bindings documentation for simple-clock-controller, mutex controller for clocks.
Signed-off-by David Yang <[email protected]>--- .../clock/simple-clock-controller.yaml | 50 +++++++++++++++++++Where is the changelog?
What changelog? Series changelog already included in series cover.
Re: [PATCH v2 1/4] dt-bindings: clock: Add simple-clock-controller
Quoted 6 lines · Yangfl
Quoted 5 lines · Krzysztof Kozlowski <[email protected]> 于2023年4月17日周一 01:38写道：
Quoted 3 lines · David Yang
Add DT bindings documentation for simple-clock-controller, mutex controller for clocks.
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Signed-off-by David Yang <[email protected]>--- .../clock/simple-clock-controller.yaml | 50 +++++++++++++++++++Where is the changelog?
What changelog? Series changelog already included in series cover.
Changelog of each patch, since there is no cover letter. If you skip some people from CC, then be sure they get cover letter (if such exists).
Best regards,
Krzysztof
Re: [PATCH v2 1/4] dt-bindings: clock: Add simple-clock-controller
In reply to Krzysztof KozlowskiQuoted 11 lines · Krzysztof Kozlowski <[email protected]> 于2023年4月17日周一 01:38写道：
Quoted 6 lines · David Yang
Add DT bindings documentation for simple-clock-controller, mutex controller for clocks.
Signed-off-by David Yang <[email protected]>--- .../clock/simple-clock-controller.yaml | 50 +++++++++++++++++++Where is the changelog?
Cover now send with v3.
Quoted 25 lines
Quoted 12 lines
1 file changed, 50 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/simple-clock-controller.yamlDocumentation/devicetree/bindings/clock/simple-clock-controller.yaml new file +3
-0@@ -0,0 +1,50 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +---Documentation/devicetree/bindings/clock/simple-clock-controller.yaml line 13
Quoted 10 lines
Documentation/devicetree/bindings/clock/simple-clock-controller.yaml new file +10
-0+$id: http://devicetree.org/schemas/clock/simple-clock-controller.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Simple clock controller + +maintainers: + - David Yang <[email protected]> + +description: | + Driver (lock provider) for real clocks.Drop driver references. Typo: clock, not lock.
Quoted 2 lines
What is a real clock? What is an unreal clock?
Rewrite description in v3. The controller is kinda unreal since it does not require any operation to "enable" the controller, but such description is avoided in v3.
Quoted 7 lines
Documentation/devicetree/bindings/clock/simple-clock-controller.yaml line 17
Quoted 4 lines
Documentation/devicetree/bindings/clock/simple-clock-controller.yaml new file +4
-0+ + Usually one register controls more than one clocks. This controller avoids + write conflicts by imposing a write lock, so that two operations on the same + register will not happen at the same time.Interesting. How the clock controller imposes write locks? Aren't you now mixing drivers and hardware?
Avoided driver details in device description in v3.
Quoted 11 lines
Documentation/devicetree/bindings/clock/simple-clock-controller.yaml line 24
Quoted 9 lines
Documentation/devicetree/bindings/clock/simple-clock-controller.yaml new file +7
-0+ +properties: + compatible: + items: + - oneOf: + - const: simple-clock-controller + - const: simple-clock-reset-controllerWhy two?
It may also handle reset requests. But removed in v3 for further consideration.
Quoted 5 lines
Documentation/devicetree/bindings/clock/simple-clock-controller.yaml line 26
Quoted 3 lines
Documentation/devicetree/bindings/clock/simple-clock-controller.yaml new file +2
-0+ - const: syscon + - const: simple-mfdWhy do you need syscon and simple-mfd?
Kinda typo. Removed in v3.
Quoted 12 lines
Documentation/devicetree/bindings/clock/simple-clock-controller.yaml line 35
Quoted 10 lines
Documentation/devicetree/bindings/clock/simple-clock-controller.yaml new file +9
-0+ + reg: + maxItems: 1 + + '#reset-cells': + const: 2 + +patternProperties: + "clock@.*":Use consistent quotes.
Fixed in v3.
Quoted 8 lines
Anyway, I don't understand what is happening here and why such changes. Nothing is explained...
Best regards,
Krzysztof
PATCH v2 2/4 clk: Add simple clock controller
In reply to David YangThese clocks were provided in include/linux/clk-provider.h, but lacks DT bindings. Add a clock controller to avoid operation conflict on same register.
--- drivers/clk/Makefile | 1 + drivers/clk/clk-of.c | 292 +++++++++++++++++++++++++++++++++++++++++++ drivers/clk/clk-of.h | 26 ++++ 3 files changed, 319 insertions(+) create mode 100644 drivers/clk/clk-of.c create mode 100644 drivers/clk/clk-of.h
drivers/clk/Makefile +1 -0
@@ -15,6 +15,7 @@ obj-$(CONFIG_COMMON_CLK) += clk-fractional-divider.o obj-$(CONFIG_COMMON_CLK) += clk-gpio.o ifeq ($(CONFIG_OF), y) obj-$(CONFIG_COMMON_CLK) += clk-conf.o +obj-$(CONFIG_COMMON_CLK) += clk-of.o endif # hardware specific clock types
drivers/clk/clk-of.c new file +292 -0
292 lines not on this page · raw message
drivers/clk/clk-of.h new file +26 -0
@@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later OR MIT */ +/* +* Copyright (c) 2023 David Yang +*/ + +#include <linux/spinlock_types.h> + +struct device_node ; +struct platform_device ; + +struct of_clk_ctrl { +spinlock_t lock; +}; + +struct of_clk_flag { +const char *prop; +unsigned long flag; +}; + +void __iomem *of_clk_get_reg (struct device_node *np); +const char *of_clk_get_name (struct device_node *np); +unsigned long +of_clk_get_flags (struct device_node *np,const struct of_clk_flag *defs); + +int of_clk_remove (struct platform_device *pdev); +int of_clk_probe (struct platform_device *pdev);
Signature · 1 line
2.39.2
PATCH v2 3/4 dt-bindings: clock: Add gate-clock
In reply to David YangAdd DT bindings documentation for gate-clock, which can gate its output.
--- .../devicetree/bindings/clock/gate-clock.yaml | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/gate-clock.yaml
Documentation/devicetree/bindings/clock/gate-clock.yaml new file +58 -0
@@ -0,0 +1,58 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/clock/gate-clock.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Clock which can gate its output + +maintainers: + - David Yang <[email protected]> + +description: | + Clock which can gate its output. + + The registers map is retrieved from the parental dt-node. So the clock node + should be represented as a sub-node of a "clock-controller" node. + + See also: linux/clk-provider.h + +properties: + compatible: + const: gate-clock + + '#clock-cells': + const: 0 + + clocks: + maxItems: 1 + description: Parent clock. + + offset: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Offset in the register map for the control register (in bytes). + + bits: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Bit index which controls the output. + + clock-output-names: + maxItems: 1 + +required: + - compatible + - '#clock-cells' + - offset + - bits + +additionalProperties: false + +examples: + - | + [email protected] { + compatible = "gate-clock"; + #clock-cells = <0>; + offset = <0xcc>; + bits = <3>; + clock-output-names = "my-clk"; + };
Signature · 1 line
2.39.2
Re: [PATCH v2 3/4] dt-bindings: clock: Add gate-clock
Quoted 7 lines · David Yang
Add DT bindings documentation for gate-clock, which can gate its output.
Signed-off-by David Yang <[email protected]>--- .../devicetree/bindings/clock/gate-clock.yaml | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/gate-clock.yaml
Where is the changelog? What happened here?
Quoted 13 lines
Documentation/devicetree/bindings/clock/gate-clock.yaml new file +6
-0@@ -0,0 +1,58 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/clock/gate-clock.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# +
Documentation/devicetree/bindings/clock/gate-clock.yaml line 16
Quoted 10 lines
Documentation/devicetree/bindings/clock/gate-clock.yaml new file +10
-0+title: Clock which can gate its output + +maintainers: + - David Yang <[email protected]> + +description: | + Clock which can gate its output. + + The registers map is retrieved from the parental dt-node. So the clock node + should be represented as a sub-node of a "clock-controller" node.
If this is supposed to be used in parent schema, then reference it there.
Documentation/devicetree/bindings/clock/gate-clock.yaml line 18
Quoted 2 lines
Documentation/devicetree/bindings/clock/gate-clock.yaml new file +2
-0+ + See also: linux/clk-provider.h
How is this related to hardware? Also, referencing linux headers is usually not good idea for bindings.
Quoted 24 lines
Documentation/devicetree/bindings/clock/gate-clock.yaml new file +24
-0+ +properties: + compatible: + const: gate-clock + + '#clock-cells': + const: 0 + + clocks: + maxItems: 1 + description: Parent clock. + + offset: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Offset in the register map for the control register (in bytes). + + bits: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Bit index which controls the output. + + clock-output-names: + maxItems: 1 + +required:
Documentation/devicetree/bindings/clock/gate-clock.yaml line 52
Quoted 10 lines
Documentation/devicetree/bindings/clock/gate-clock.yaml new file +10
-0+ - compatible + - '#clock-cells' + - offset + - bits + +additionalProperties: false + +examples: + - | + [email protected] {
So you keep ignoring the comments... I don't know what happened here but this code for sure looks wrong.
Did you test the changes?
Best regards,
Krzysztof
Re: [PATCH v2 3/4] dt-bindings: clock: Add gate-clock
Quoted 2 lines · Krzysztof Kozlowski <[email protected]> 于2023年4月17日周一 01:41写道：
Did you test the changes?
I don't know what "test" is here. make dt_binding_check passed and it can work with the driver.
Re: [PATCH v2 3/4] dt-bindings: clock: Add gate-clock
Quoted 6 lines · Yangfl
Quoted 2 lines · Krzysztof Kozlowski <[email protected]> 于2023年4月17日周一 01:41写道：
Did you test the changes?
I don't know what "test" is here. make
dt_binding_checkpassed and it can work with the driver.
I don't think so. If you tested it, for sure you would see warnings like "node has a unit name, but no reg or ranges property".
Best regards,
Krzysztof
PATCH v2 4/4 clk: gate: Add DT binding
In reply to David YangAdd DT binding for gate clock as "gate-clock".
--- drivers/clk/clk-gate.c | 81 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+)
drivers/clk/clk-gate.c +81 -0
@@ -12,8 +12,11 @@#include <linux/slab.h> #include <linux/io.h> #include <linux/err.h> +#include <linux/platform_device.h> #include <linux/string.h> +#include "clk-of.h" +/** * DOC: basic gatable clock which can gate and ungate it's ouput *
@@ -257,3 +260,81 @@ struct clk_hw *__devm_clk_hw_register_gate(struct device *dev,return hw; }EXPORT_SYMBOL_GPL (__devm_clk_hw_register_gate); + +#if IS_ENABLED (CONFIG_OF ) +static const struct of_clk_flag of_clk_gate_flags[] = { + {"set-to-disable" ,CLK_GATE_SET_TO_DISABLE }, + {"hiword-mask" ,CLK_GATE_HIWORD_MASK }, + {"big-endian" ,CLK_GATE_BIG_ENDIAN }, + { } +}; + +static int of_clk_gate_setup (struct device_node *np) +{ +struct of_clk_ctrl *ctrl = np->parent->data; +const char *name; +void __iomem *reg; +u32 bit_idx; + +const char *property; +struct clk_hw *hw; +int ret; + + reg =of_clk_get_reg (np); +if (!reg) +return -ENOMEM ; + name =of_clk_get_name (np); +if (!name) +return -EINVAL ; + + property ="bits" ; +if (of_property_read_u32 (np, property, &bit_idx)) + goto err_property; + + hw =__clk_hw_register_gate (NULL , np, name, +of_clk_get_parent_name (np,0 ), +NULL ,NULL ,of_clk_get_flags (np,NULL ), + reg, bit_idx, +of_clk_get_flags (np, of_clk_gate_flags), + &ctrl->lock); +if (IS_ERR (hw)) +return PTR_ERR (hw); + + ret =of_clk_add_hw_provider (np, of_clk_hw_simple_get, hw); +if (ret) + goto err_register; + + np->data = hw; +return 0 ; + +err_register: +clk_hw_unregister (hw); +return ret; + +err_property: +pr_err ("%s: clock %s missing required property \"%s\"\n" , + __func__, name, property); +return -EINVAL ; +} + +static void __init of_clk_gate_init (struct device_node *np) +{ +of_clk_gate_setup (np); +} +CLK_OF_DECLARE (of_clk_gate,"gate-clock" , of_clk_gate_init); + +static const struct of_device_id of_clk_gate_ids[] = { + { .compatible ="gate-clock" , .data = of_clk_gate_setup }, + { } +}; + +static struct platform_driver of_clk_gate_driver = { + .driver = { + .name ="clk_gate" , + .of_match_table = of_clk_gate_ids, + }, + .probe = of_clk_probe, + .remove = of_clk_remove, +}; +builtin_platform_driver (of_clk_gate_driver); +#endif
Signature · 1 line
2.39.2