[PATCH v3 0/4] clk: Add basic register clock controller

[PATCH v3 0/4] clk: Add basic register clock controller

[David Yang]

These clocks were provided in `include/linux/clk-provider.h`, but lacks DT
bindings. Add DT bindings for them.

v2: split clock controller and its clocks
v3:
  * rewrite commit message
  * rewrite descriptions in dt-bindings to describe the devices
  * fix format error of dt-bindings

Links:
v1: https://lore.kernel.org/r/20230414181302.986271-1-mmyangfl@gmail.com
v2: https://lore.kernel.org/r/20230416173302.1185683-1-mmyangfl@gmail.com

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 |  59 ++++
 .../clock/simple-clock-controller.yaml        |  39 +++
 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, 498 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
-- 
2.39.2

[PATCH v3 1/4] dt-bindings: clock: Add simple-clock-controller

[David Yang]

Add DT bindings documentation for simple clock controller, just a
contiguous region of multiple clock registers.

Signed-off-by: David Yang <mmyangfl@gmail.com>
---
 .../clock/simple-clock-controller.yaml        | 39 +++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/simple-clock-controller.yaml

diff --git a/Documentation/devicetree/bindings/clock/simple-clock-controller.yaml b/Documentation/devicetree/bindings/clock/simple-clock-controller.yaml
new file mode 100644
index 000000000000..3d9b436b0ef9
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/simple-clock-controller.yaml
@@ -0,0 +1,39 @@
+# 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 <mmyangfl@gmail.com>
+
+description: |
+  A contiguous register region of multiple different clocks. No operations are
+  required to enable or disable the clock controller.
+
+properties:
+  compatible:
+    const: simple-clock-controller
+
+  reg:
+    maxItems: 1
+
+patternProperties:
+  '.*clock.*':
+    type: object
+    description: Clock devices.
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    clock-controller@ffff000 {
+      compatible = "simple-clock-controller";
+      reg = <0xffff000 0x1000>;
+    };
-- 
2.39.2

[PATCH v3 2/4] clk: Add simple clock controller

[David Yang]

Basic common clocks were provided in `include/linux/clk-provider.h`, but
lacks DT bindings for direct declaration. To use them, a lock is required
to avoid operation conflict on a same register.

Add a clock controller to manage this lock.

Signed-off-by: David Yang <mmyangfl@gmail.com>
---
 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

diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index e3ca0d058a25..6cf0a888b673 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -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
diff --git a/drivers/clk/clk-of.c b/drivers/clk/clk-of.c
new file mode 100644
index 000000000000..3518ae848ed0
--- /dev/null
+++ b/drivers/clk/clk-of.c
@@ -0,0 +1,292 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Copyright (c) 2023 David Yang
+ *
+ * Simple straight-forward register clocks bindings
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+#include <linux/slab.h>
+
+#include "clk-of.h"
+
+struct of_clk_ctrl_priv {
+	spinlock_t lock;
+
+	struct reset_controller_dev rcdev;
+	void __iomem *base;
+	bool rst_set_to_disable;
+};
+
+static const struct of_clk_flag of_clk_common_flags[] = {
+	{ "set-rate-gate", CLK_SET_RATE_GATE },
+	{ "set-parent-gate", CLK_SET_PARENT_GATE },
+	{ "set-rate-parent", CLK_SET_RATE_PARENT },
+	{ "ignore-unused", CLK_IGNORE_UNUSED },
+	{ "get-rate-nocache", CLK_GET_RATE_NOCACHE },
+	{ "set-rate-no-reparent", CLK_SET_RATE_NO_REPARENT },
+	{ "get-accuracy-nocache", CLK_GET_ACCURACY_NOCACHE },
+	{ "recalc-new-rates", CLK_RECALC_NEW_RATES },
+	{ "set-rate-ungate", CLK_SET_RATE_UNGATE },
+	{ "critical", CLK_IS_CRITICAL },
+	{ "ops-parent-enable", CLK_OPS_PARENT_ENABLE },
+	{ "duty-cycle-parent", CLK_DUTY_CYCLE_PARENT },
+	{ }
+};
+
+void __iomem *of_clk_get_reg(struct device_node *np)
+{
+	u32 offset;
+	void __iomem *reg;
+
+	if (of_property_read_u32(np, "offset", &offset))
+		return NULL;
+
+	reg = of_iomap(np->parent, 0);
+	if (!reg)
+		return NULL;
+
+	return reg + offset;
+}
+EXPORT_SYMBOL_GPL(of_clk_get_reg);
+
+const char *of_clk_get_name(struct device_node *np)
+{
+	const char *name;
+
+	if (!of_property_read_string(np, "clock-output-name", &name))
+		return name;
+
+	return of_node_full_name(np);
+}
+EXPORT_SYMBOL_GPL(of_clk_get_name);
+
+unsigned long
+of_clk_get_flags(struct device_node *np, const struct of_clk_flag *defs)
+{
+	unsigned long flags = 0;
+
+	if (!defs)
+		defs = of_clk_common_flags;
+
+	for (int i = 0; defs[i].prop; i++)
+		if (of_property_read_bool(np, defs[i].prop))
+			flags |= defs[i].flag;
+
+	return flags;
+}
+EXPORT_SYMBOL_GPL(of_clk_get_flags);
+
+int of_clk_remove(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+
+	of_clk_del_provider(np);
+	clk_hw_unregister(np->data);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_clk_remove);
+
+int of_clk_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = pdev->dev.of_node;
+	int (*setup)(struct device_node *np) = of_device_get_match_data(dev);
+
+	return setup(np);
+}
+EXPORT_SYMBOL_GPL(of_clk_probe);
+
+/** of_rst_ctrl **/
+
+#if IS_ENABLED(CONFIG_RESET_CONTROLLER)
+static int
+of_rst_ctrl_assert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	struct of_clk_ctrl_priv *priv = container_of(rcdev, struct of_clk_ctrl_priv, rcdev);
+	unsigned long flags;
+	u32 offset = id >> 16;
+	u8 index = id & 0x1f;
+	u32 val;
+
+	if (WARN_ON(!priv->base))
+		return 0;
+
+	spin_lock_irqsave(&priv->lock, flags);
+
+	val = readl(priv->base + offset);
+	if (priv->rst_set_to_disable)
+		val &= ~BIT(index);
+	else
+		val |= BIT(index);
+	writel(val, priv->base + offset);
+
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	return 0;
+}
+
+static int
+of_rst_ctrl_deassert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	struct of_clk_ctrl_priv *priv = container_of(rcdev, struct of_clk_ctrl_priv, rcdev);
+	unsigned long flags;
+	u32 offset = id >> 16;
+	u8 index = id & 0x1f;
+	u32 val;
+
+	if (WARN_ON(!priv->base))
+		return 0;
+
+	spin_lock_irqsave(&priv->lock, flags);
+
+	val = readl(priv->base + offset);
+	if (priv->rst_set_to_disable)
+		val |= BIT(index);
+	else
+		val &= ~BIT(index);
+	writel(val, priv->base + offset);
+
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	return 0;
+}
+
+static const struct reset_control_ops of_rst_ctrl_ops = {
+	.assert = of_rst_ctrl_assert,
+	.deassert = of_rst_ctrl_deassert,
+};
+
+static int of_rst_ctrl_of_xlate(struct reset_controller_dev *rcdev,
+				 const struct of_phandle_args *reset_spec)
+{
+	return (reset_spec->args[0] << 16) | (reset_spec->args[1] & 0x1f);
+}
+
+static void of_rst_ctrl_unsetup(struct device_node *np)
+{
+	struct of_clk_ctrl_priv *priv = np->data;
+
+	reset_controller_unregister(&priv->rcdev);
+}
+
+static int of_rst_ctrl_setup(struct device_node *np, struct of_clk_ctrl_priv *priv)
+{
+	priv->base = of_iomap(np, 0);
+	priv->rst_set_to_disable = of_property_read_bool(np, "set-to-disable");
+
+	/* register no matter whether reg exists, to detect dts bug */
+	priv->rcdev.ops = &of_rst_ctrl_ops;
+	priv->rcdev.of_node = np;
+	priv->rcdev.of_reset_n_cells = 2;
+	priv->rcdev.of_xlate = of_rst_ctrl_of_xlate;
+	return reset_controller_register(&priv->rcdev);
+}
+#else
+static void of_rst_ctrl_unsetup(struct device_node *np)
+{
+}
+
+static int of_rst_ctrl_setup(struct device_node *np, struct of_clk_ctrl_priv *priv)
+{
+	return 0;
+}
+#endif
+
+/** of_crg_ctrl **/
+
+static void of_crg_ctrl_unsetup(struct device_node *np, bool crg)
+{
+	if (crg)
+		of_rst_ctrl_unsetup(np);
+
+	kfree(np->data);
+	np->data = NULL;
+}
+
+static int of_crg_ctrl_setup(struct device_node *np, bool crg)
+{
+	struct of_clk_ctrl_priv *priv;
+	int ret;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	np->data = priv;
+
+	spin_lock_init(&priv->lock);
+
+	if (crg) {
+		ret = of_rst_ctrl_setup(np, priv);
+		if (ret)
+			goto err;
+	}
+
+	return 0;
+
+err:
+	kfree(np->data);
+	np->data = NULL;
+	return ret;
+}
+
+/** driver **/
+
+static void __init of_clk_ctrl_init(struct device_node *np)
+{
+	of_crg_ctrl_setup(np, false);
+}
+CLK_OF_DECLARE(of_clk_ctrl, "simple-clock-controller", of_clk_ctrl_init);
+
+static void __init of_crg_ctrl_init(struct device_node *np)
+{
+	of_crg_ctrl_setup(np, true);
+}
+CLK_OF_DECLARE(of_crg_ctrl, "simple-clock-reset-controller", of_crg_ctrl_init);
+
+static int of_crg_ctrl_remove(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = pdev->dev.of_node;
+	bool crg = (bool) of_device_get_match_data(dev);
+
+	of_crg_ctrl_unsetup(np, crg);
+
+	return 0;
+}
+
+/* This function is not executed when of_clk_ctrl_init succeeded. */
+static int of_crg_ctrl_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = pdev->dev.of_node;
+	bool crg = (bool) of_device_get_match_data(dev);
+
+	return of_crg_ctrl_setup(np, crg);
+}
+
+static const struct of_device_id of_crg_ctrl_ids[] = {
+	{ .compatible = "simple-clock-controller", .data = (void *) false },
+	{ .compatible = "simple-clock-reset-controller", .data = (void *) true },
+	{ }
+};
+
+static struct platform_driver of_crg_ctrl_driver = {
+	.driver = {
+		.name = "clk_of",
+		.of_match_table = of_crg_ctrl_ids,
+	},
+	.probe = of_crg_ctrl_probe,
+	.remove = of_crg_ctrl_remove,
+};
+builtin_platform_driver(of_crg_ctrl_driver);
diff --git a/drivers/clk/clk-of.h b/drivers/clk/clk-of.h
new file mode 100644
index 000000000000..ddb1e57ec2f1
--- /dev/null
+++ b/drivers/clk/clk-of.h
@@ -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);
-- 
2.39.2

[PATCH v3 3/4] dt-bindings: clock: Add gate-clock

[David Yang]

Add DT bindings documentation for gate clock, which can gate its output.

Signed-off-by: David Yang <mmyangfl@gmail.com>
---
 .../devicetree/bindings/clock/gate-clock.yaml | 59 +++++++++++++++++++
 1 file changed, 59 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/gate-clock.yaml

diff --git a/Documentation/devicetree/bindings/clock/gate-clock.yaml b/Documentation/devicetree/bindings/clock/gate-clock.yaml
new file mode 100644
index 000000000000..bcd549dd9db1
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/gate-clock.yaml
@@ -0,0 +1,59 @@
+# 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 <mmyangfl@gmail.com>
+
+description: |
+  Clock which can gate its output, by toggling one bit in a register. Such
+  register may also control other clocks or reset requests.
+
+  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: Documentation/devicetree/bindings/clock/simple-clock-controller.yaml
+
+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:
+  - |
+    clock {
+      compatible = "gate-clock";
+      #clock-cells = <0>;
+      offset = <0xcc>;
+      bits = <3>;
+      clock-output-names = "my-clk";
+    };
-- 
2.39.2

[PATCH v3 4/4] clk: gate: Add DT binding

[David Yang]

Add DT binding for gate clock as "gate-clock".

Signed-off-by: David Yang <mmyangfl@gmail.com>
---
 drivers/clk/clk-gate.c | 81 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
index 64283807600b..a70df4a2a9a7 100644
--- a/drivers/clk/clk-gate.c
+++ b/drivers/clk/clk-gate.c
@@ -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
-- 
2.39.2

Re: [PATCH v3 0/4] clk: Add basic register clock controller

[Krzysztof Kozlowski]

On 16/04/2023 21:46, David Yang wrote:
> These clocks were provided in `include/linux/clk-provider.h`, but lacks DT
> bindings. Add DT bindings for them.
> 
> v2: split clock controller and its clocks

I gave you several comments on v1 and this vague changelog entry
suggests you ignored all of it.

Implement feedback and respond to each one that you ack it or disagree
with it.


Best regards,
Krzysztof

Re: [PATCH v3 1/4] dt-bindings: clock: Add simple-clock-controller

[Krzysztof Kozlowski]

On 16/04/2023 21:46, David Yang wrote:
> Add DT bindings documentation for simple clock controller, just a
> contiguous region of multiple clock registers.
> 
> Signed-off-by: David Yang <mmyangfl@gmail.com>
> ---
>  .../clock/simple-clock-controller.yaml        | 39 +++++++++++++++++++
>  1 file changed, 39 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/simple-clock-controller.yaml
> 
> diff --git a/Documentation/devicetree/bindings/clock/simple-clock-controller.yaml b/Documentation/devicetree/bindings/clock/simple-clock-controller.yaml
> new file mode 100644
> index 000000000000..3d9b436b0ef9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/simple-clock-controller.yaml
> @@ -0,0 +1,39 @@
> +# 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 <mmyangfl@gmail.com>
> +
> +description: |
> +  A contiguous register region of multiple different clocks. No operations are
> +  required to enable or disable the clock controller.
> +
> +properties:
> +  compatible:
> +    const: simple-clock-controller
> +
> +  reg:
> +    maxItems: 1
> +
> +patternProperties:
> +  '.*clock.*':
> +    type: object
> +    description: Clock devices.

I don't know what improved here... this is still without any refs.

It looks like you ignored my feedback from v1 and maybe v2, so respond
to each of it.

Best regards,
Krzysztof

Re: [PATCH v3 1/4] dt-bindings: clock: Add simple-clock-controller

[Krzysztof Kozlowski]

On 16/04/2023 21:46, David Yang wrote:

> +patternProperties:
> +  '.*clock.*':
> +    type: object
> +    description: Clock devices.
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    clock-controller@ffff000 {
> +      compatible = "simple-clock-controller";
> +      reg = <0xffff000 0x1000>;

BTW, this is just incomplete example. Fill it with proper children.

Best regards,
Krzysztof

Re: [PATCH v3 0/4] clk: Add basic register clock controller

[Yangfl]

Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 于2023年4月17日周一 14:27写道：
>
> On 16/04/2023 21:46, David Yang wrote:
> > These clocks were provided in `include/linux/clk-provider.h`, but lacks DT
> > bindings. Add DT bindings for them.
> >
> > v2: split clock controller and its clocks
>
> I gave you several comments on v1 and this vague changelog entry
> suggests you ignored all of it.
>
> Implement feedback and respond to each one that you ack it or disagree
> with it.
>
>
> Best regards,
> Krzysztof
>

All issues have been addressed in v3 patch. I omitted details in
changelog to reduce redundancy. If that is not the proper way to make
response in dt maillist, please let me know.

Responses for each comments in v1 and v2 have been sent against original email.

Re: [PATCH v3 3/4] dt-bindings: clock: Add gate-clock

[Stephen Boyd]

Quoting David Yang (2023-04-16 12:46:21)
> Add DT bindings documentation for gate clock, which can gate its output.
> 
> Signed-off-by: David Yang <mmyangfl@gmail.com>
> ---

NAK

We've been over this many times. The binding will not be accepted.

Re: [PATCH v3 2/4] clk: Add simple clock controller

[kernel test robot]

Hi David,

kernel test robot noticed the following build errors:

[auto build test ERROR on 7a934f4bd7d6f9da84c8812da3ba42ee10f5778e]

url:    https://github.com/intel-lab-lkp/linux/commits/David-Yang/dt-bindings-clock-Add-simple-clock-controller/20230417-034857
base:   7a934f4bd7d6f9da84c8812da3ba42ee10f5778e
patch link:    https://lore.kernel.org/r/20230416194624.1258860-3-mmyangfl%40gmail.com
patch subject: [PATCH v3 2/4] clk: Add simple clock controller
config: s390-randconfig-r036-20230416 (https://download.01.org/0day-ci/archive/20230419/202304190206.OjctYqwT-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 437b7602e4a998220871de78afcb020b9c14a661)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install s390 cross compiling tool for clang build
        # apt-get install binutils-s390x-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/242d6bab7339a967b40f9731b989913e8b2ea63c
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review David-Yang/dt-bindings-clock-Add-simple-clock-controller/20230417-034857
        git checkout 242d6bab7339a967b40f9731b989913e8b2ea63c
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304190206.OjctYqwT-lkp@intel.com/

All errors (new ones prefixed by >>):

   s390x-linux-ld: DWARF error: could not find abbrev number 115
   kernel/dma/coherent.o: in function `dma_declare_coherent_memory':
   coherent.c:(.text+0xc8): undefined reference to `memunmap'
   s390x-linux-ld: kernel/dma/coherent.o: in function `dma_init_coherent_memory':
   coherent.c:(.text+0x14a): undefined reference to `memremap'
   s390x-linux-ld: coherent.c:(.text+0x254): undefined reference to `memunmap'
   s390x-linux-ld: kernel/dma/coherent.o: in function `dma_release_coherent_memory':
   coherent.c:(.text+0x2c0): undefined reference to `memunmap'
   s390x-linux-ld: DWARF error: could not find abbrev number 53
   drivers/clk/clk-of.o: in function `of_clk_get_reg':
>> clk-of.c:(.text+0x6a): undefined reference to `of_iomap'
   s390x-linux-ld: drivers/clk/clk-of.o: in function `of_crg_ctrl_setup':
   clk-of.c:(.text+0x322): undefined reference to `of_iomap'
   s390x-linux-ld: DWARF error: could not find abbrev number 8446
   drivers/clk/clk-fixed-mmio.o: in function `fixed_mmio_clk_setup':
   clk-fixed-mmio.c:(.text+0x48): undefined reference to `of_iomap'
   s390x-linux-ld: clk-fixed-mmio.c:(.text+0x68): undefined reference to `iounmap'
   s390x-linux-ld: DWARF error: could not find abbrev number 15506
   drivers/char/xillybus/xillybus_of.o: in function `xilly_drv_probe':
   xillybus_of.c:(.text+0x64): undefined reference to `devm_platform_ioremap_resource'
   s390x-linux-ld: DWARF error: could not find abbrev number 93
   drivers/misc/open-dice.o: in function `open_dice_write':
   open-dice.c:(.text+0xe6): undefined reference to `devm_memremap'
   s390x-linux-ld: open-dice.c:(.text+0x176): undefined reference to `devm_memunmap'
   s390x-linux-ld: DWARF error: could not find abbrev number 67
   drivers/pcmcia/cistpl.o: in function `release_cis_mem':
   cistpl.c:(.text+0xe8): undefined reference to `iounmap'
   s390x-linux-ld: drivers/pcmcia/cistpl.o: in function `set_cis_map':
   cistpl.c:(.text+0x4a6): undefined reference to `ioremap'
   s390x-linux-ld: cistpl.c:(.text+0x520): undefined reference to `iounmap'
   s390x-linux-ld: cistpl.c:(.text+0x572): undefined reference to `iounmap'
   s390x-linux-ld: cistpl.c:(.text+0x59c): undefined reference to `ioremap'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH v3 0/4] clk: Add basic register clock controller

[Rob Herring]

On Tue, Apr 18, 2023 at 02:16:28AM +0800, Yangfl wrote:
> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 于2023年4月17日周一 14:27写道：
> >
> > On 16/04/2023 21:46, David Yang wrote:
> > > These clocks were provided in `include/linux/clk-provider.h`, but lacks DT
> > > bindings. Add DT bindings for them.
> > >
> > > v2: split clock controller and its clocks
> >
> > I gave you several comments on v1 and this vague changelog entry
> > suggests you ignored all of it.
> >
> > Implement feedback and respond to each one that you ack it or disagree
> > with it.
> >
> >
> > Best regards,
> > Krzysztof
> >
> 
> All issues have been addressed in v3 patch. I omitted details in
> changelog to reduce redundancy. If that is not the proper way to make
> response in dt maillist, please let me know.
> 
> Responses for each comments in v1 and v2 have been sent against original email.

Go look at v1 replies. This series is a hard no.

Rob