2023-01-20 03:05:18

by Xingyu Wu

[permalink] [raw]
Subject: [PATCH v1 00/11] Add new partial clock and reset drivers for StarFive JH7110

This patch serises are to add new partial clock drivers and reset
supports about System-Top-Group(STG), Image-Signal-Process(ISP)
and Video-Output(VOUT) for the StarFive JH7110 RISC-V SoC.

Patches 1 to 3 are about the System-Top-Group clock and reset
generator(STGCRG) part.
The first patch adds docunmentation to describe STG bindings, and
the second patch adds support about STG resets. The last patch adds
clock driver to support STG clocks for JH7110.

Patches 4 to 6 are about the Image-Signal-Process clock and reset
gennerator(ISPCRG) part.
The first patch adds docunmentation to describe ISP bindings, and
the second patch adds support about ISP resets. The last patch adds
clock driver to support ISP clocks for JH7110.

Patches 7 to 9 are about the Video-Output clock and reset
generator(VOUTCRG) part.
The first patch adds docunmentation to describe VOUT bindings, and
the second patch adds support about VOUT resets. The last patch adds
clock driver to support VOUT clocks for JH7110.

Patch 10 adds external clocks which ISP and VOUT clock driver need.
Patch 11 adds device node about STGCRG, ISPCRG and VOUTCRG to JH7110 dts.

Patches 2, 3, 5, 6, 8 and 9 are dependent on the patchset [1] which
is about JH71x0 clock and reset driver.
Patches 6 and 9 also are dependent on the patchset [2] which is about
JH7110 pmu driver.
Patchdes 10 and 11 are dependent on the patchset [3] which is about
JH7110 device tree.
This patchset should be applied after the patchset [1], [2], [3]:
[1] https://lore.kernel.org/all/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/
[3] https://lore.kernel.org/all/[email protected]/

Xingyu Wu (11):
dt-bindings: clock: Add StarFive JH7110 System-Top-Group clock and
reset generator
reset: starfive: jh7110: Add StarFive System-Top-Group reset support
clk: starfive: Add StarFive JH7110 System-Top-Group clock driver
dt-bindings: clock: Add StarFive JH7110 Image-Signal-Process clock and
reset generator
reset: starfive: jh7110: Add StarFive Image-Signal-Process reset
support
clk: starfive: Add StarFive JH7110 Image-Signal-Process clock driver
dt-bindings: clock: Add StarFive JH7110 Video-Output clock and reset
generator
reset: starfive: jh7110: Add StarFive Video-Output reset support
clk: starfive: Add StarFive JH7110 Video-Output clock driver
riscv: dts: starfive: jh7110: Add DVP and HDMI TX pixel external
clocks
riscv: dts: starfive: jh7110: Add STGCRG/ISPCRG/VOUTCRG nodes

.../clock/starfive,jh7110-ispcrg.yaml | 97 ++++++++
.../clock/starfive,jh7110-stgcrg.yaml | 82 +++++++
.../clock/starfive,jh7110-voutcrg.yaml | 96 ++++++++
MAINTAINERS | 2 +
.../jh7110-starfive-visionfive-2.dtsi | 8 +
arch/riscv/boot/dts/starfive/jh7110.dtsi | 69 ++++++
drivers/clk/starfive/Kconfig | 33 +++
drivers/clk/starfive/Makefile | 3 +
.../clk/starfive/clk-starfive-jh7110-isp.c | 218 +++++++++++++++++
.../clk/starfive/clk-starfive-jh7110-stg.c | 180 ++++++++++++++
.../clk/starfive/clk-starfive-jh7110-vout.c | 227 ++++++++++++++++++
.../reset/starfive/reset-starfive-jh7110.c | 30 +++
.../dt-bindings/clock/starfive,jh7110-crg.h | 74 ++++++
.../dt-bindings/reset/starfive,jh7110-crg.h | 60 +++++
14 files changed, 1179 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh7110-ispcrg.yaml
create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh7110-stgcrg.yaml
create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh7110-voutcrg.yaml
create mode 100644 drivers/clk/starfive/clk-starfive-jh7110-isp.c
create mode 100644 drivers/clk/starfive/clk-starfive-jh7110-stg.c
create mode 100644 drivers/clk/starfive/clk-starfive-jh7110-vout.c


base-commit: 830b3c68c1fb1e9176028d02ef86f3cf76aa2476
prerequisite-patch-id: 4dc515731ce237184553c1606ffb3afaeb51c3d8
prerequisite-patch-id: 09c98554df52d17ba5fd604125f8cdd62cbe80d1
prerequisite-patch-id: a798370d170dc2bcc79ed86f741c21c1e6d87c78
prerequisite-patch-id: bd9fd8b5cb2376dc7a5e08e1a1fbb969cf475926
prerequisite-patch-id: c57ebb83bc43ccd2a8366ff166eb499da1e1d2cf
prerequisite-patch-id: a1673a9e9f19d6fab5a51abb721e54e36636f067
prerequisite-patch-id: 94860423c7acc9025249d4bb36652a585bd0a797
prerequisite-patch-id: b5084253283929d9a6d0e66c350400c7c85d034d
prerequisite-patch-id: 6e369dbe9dca2785e4ea7d0b80e525e227a90a6e
prerequisite-patch-id: e08806183c152714c563f3a21c6d7b2f539c4d6e
prerequisite-patch-id: 79db8036abdc48fd36da227652ec62627a6b548b
prerequisite-patch-id: 06971b8e6bddc0e87e63bfdb0ce8bfb653bd73aa
prerequisite-patch-id: 16309a0e23811a2c55d2e56886de3e8eccc51554
prerequisite-patch-id: bf4f7ab0b6cfa90b6e49e66c7d75ed2eaaebbe78
prerequisite-patch-id: 38468d532e87867990055d3320679f18c5f52278
prerequisite-patch-id: 4710f2ac22dca0bdd9ff5d744d2c37cab3c74515
prerequisite-patch-id: 6bb9a780c62af3bcc2368dfd20303c7b1bc91e23
prerequisite-patch-id: 258ea5f9b8bf41b6981345dcc81795f25865d38f
prerequisite-patch-id: 8b6f2c9660c0ac0ee4e73e4c21aca8e6b75e81b9
prerequisite-patch-id: e3b986b9c60b2b93b7812ec174c9e1b4cfb14c97
prerequisite-patch-id: a2b3a9cff8a683422eb0ccf3a0850091401812d4
prerequisite-patch-id: e0ba7af0f8d3d41844da9fbcba14b548cbc18f55
prerequisite-patch-id: bc0176325c11a632c6abaa83e54e891cc92d1c74
prerequisite-patch-id: bd3076c3fde77b417bc5b402b97563f2928d623d
prerequisite-patch-id: 795c5ea9868c12fd62c14220140f0706f3db66ce
prerequisite-patch-id: 2ddada18ab6ea5cd1da14212aaf59632f5203d40
--
2.25.1


2023-01-20 03:06:01

by Xingyu Wu

[permalink] [raw]
Subject: [PATCH v1 06/11] clk: starfive: Add StarFive JH7110 Image-Signal-Process clock driver

Add driver for the StarFive JH7110 Image-Signal-Process clock controller.

Signed-off-by: Xingyu Wu <[email protected]>
---
drivers/clk/starfive/Kconfig | 11 +
drivers/clk/starfive/Makefile | 1 +
.../clk/starfive/clk-starfive-jh7110-isp.c | 218 ++++++++++++++++++
3 files changed, 230 insertions(+)
create mode 100644 drivers/clk/starfive/clk-starfive-jh7110-isp.c

diff --git a/drivers/clk/starfive/Kconfig b/drivers/clk/starfive/Kconfig
index a462b6e53543..59499acb95f7 100644
--- a/drivers/clk/starfive/Kconfig
+++ b/drivers/clk/starfive/Kconfig
@@ -53,3 +53,14 @@ config CLK_STARFIVE_JH7110_STG
help
Say yes here to support the System-Top-Group clock controller
on the StarFive JH7110 SoC.
+
+config CLK_STARFIVE_JH7110_ISP
+ tristate "StarFive JH7110 Image-Signal-Process clock support"
+ depends on CLK_STARFIVE_JH7110_SYS && JH71XX_PMU
+ select AUXILIARY_BUS
+ select CLK_STARFIVE_JH71X0
+ select RESET_STARFIVE_JH7110
+ default CLK_STARFIVE_JH7110_SYS
+ help
+ Say yes here to support the Image-Signal-Process clock controller
+ on the StarFive JH7110 SoC.
diff --git a/drivers/clk/starfive/Makefile b/drivers/clk/starfive/Makefile
index b81e97ee2659..76fb9f8d628b 100644
--- a/drivers/clk/starfive/Makefile
+++ b/drivers/clk/starfive/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_CLK_STARFIVE_JH7100_AUDIO) += clk-starfive-jh7100-audio.o
obj-$(CONFIG_CLK_STARFIVE_JH7110_SYS) += clk-starfive-jh7110-sys.o
obj-$(CONFIG_CLK_STARFIVE_JH7110_AON) += clk-starfive-jh7110-aon.o
obj-$(CONFIG_CLK_STARFIVE_JH7110_STG) += clk-starfive-jh7110-stg.o
+obj-$(CONFIG_CLK_STARFIVE_JH7110_ISP) += clk-starfive-jh7110-isp.o
diff --git a/drivers/clk/starfive/clk-starfive-jh7110-isp.c b/drivers/clk/starfive/clk-starfive-jh7110-isp.c
new file mode 100644
index 000000000000..f9fc94b4c6f8
--- /dev/null
+++ b/drivers/clk/starfive/clk-starfive-jh7110-isp.c
@@ -0,0 +1,218 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * StarFive JH7110 Image-Signal-Process Clock Driver
+ *
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/reset.h>
+
+#include <dt-bindings/clock/starfive,jh7110-crg.h>
+
+#include "clk-starfive-jh71x0.h"
+
+/* external clocks */
+#define JH7110_ISPCLK_ISP_TOP_CORE (JH7110_ISPCLK_END + 0)
+#define JH7110_ISPCLK_ISP_TOP_AXI (JH7110_ISPCLK_END + 1)
+#define JH7110_ISPCLK_NOC_BUS_ISP_AXI (JH7110_ISPCLK_END + 2)
+#define JH7110_ISPCLK_DVP_CLK (JH7110_ISPCLK_END + 3)
+
+static const struct jh71x0_clk_data jh7110_ispclk_data[] = {
+ /* syscon */
+ JH71X0__DIV(JH7110_ISPCLK_DOM4_APB_FUNC, "dom4_apb_func", 15,
+ JH7110_ISPCLK_ISP_TOP_AXI),
+ JH71X0__DIV(JH7110_ISPCLK_MIPI_RX0_PXL, "mipi_rx0_pxl", 8,
+ JH7110_ISPCLK_ISP_TOP_CORE),
+ JH71X0__INV(JH7110_ISPCLK_DVP_INV, "dvp_inv", JH7110_ISPCLK_DVP_CLK),
+ /* vin */
+ JH71X0__DIV(JH7110_ISPCLK_M31DPHY_CFGCLK_IN, "m31dphy_cfgclk_in", 16,
+ JH7110_ISPCLK_ISP_TOP_CORE),
+ JH71X0__DIV(JH7110_ISPCLK_M31DPHY_REFCLK_IN, "m31dphy_refclk_in", 16,
+ JH7110_ISPCLK_ISP_TOP_CORE),
+ JH71X0__DIV(JH7110_ISPCLK_M31DPHY_TXCLKESC_LAN0, "m31dphy_txclkesc_lan0", 60,
+ JH7110_ISPCLK_ISP_TOP_CORE),
+ JH71X0_GATE(JH7110_ISPCLK_VIN_PCLK, "vin_pclk", CLK_IGNORE_UNUSED,
+ JH7110_ISPCLK_DOM4_APB_FUNC),
+ JH71X0__DIV(JH7110_ISPCLK_VIN_SYS_CLK, "vin_sys_clk", 8, JH7110_ISPCLK_ISP_TOP_CORE),
+ JH71X0_GATE(JH7110_ISPCLK_VIN_PIXEL_CLK_IF0, "vin_pixel_clk_if0", CLK_IGNORE_UNUSED,
+ JH7110_ISPCLK_MIPI_RX0_PXL),
+ JH71X0_GATE(JH7110_ISPCLK_VIN_PIXEL_CLK_IF1, "vin_pixel_clk_if1", CLK_IGNORE_UNUSED,
+ JH7110_ISPCLK_MIPI_RX0_PXL),
+ JH71X0_GATE(JH7110_ISPCLK_VIN_PIXEL_CLK_IF2, "vin_pixel_clk_if2", CLK_IGNORE_UNUSED,
+ JH7110_ISPCLK_MIPI_RX0_PXL),
+ JH71X0_GATE(JH7110_ISPCLK_VIN_PIXEL_CLK_IF3, "vin_pixel_clk_if3", CLK_IGNORE_UNUSED,
+ JH7110_ISPCLK_MIPI_RX0_PXL),
+ JH71X0__MUX(JH7110_ISPCLK_VIN_CLK_P_AXIWR, "vin_clk_p_axiwr", 2,
+ JH7110_ISPCLK_MIPI_RX0_PXL,
+ JH7110_ISPCLK_DVP_INV),
+ /* ispv2_top_wrapper */
+ JH71X0_GMUX(JH7110_ISPCLK_ISPV2_TOP_WRAPPER_CLK_C, "ispv2_top_wrapper_clk_c",
+ CLK_IGNORE_UNUSED, 2,
+ JH7110_ISPCLK_MIPI_RX0_PXL,
+ JH7110_ISPCLK_DVP_INV),
+};
+
+struct isp_top_crg {
+ struct clk_bulk_data *top_clks;
+ struct reset_control *top_rsts;
+ int top_clks_num;
+};
+
+static struct clk_bulk_data jh7110_isp_top_clks[] = {
+ { .id = "isp_top_core" },
+ { .id = "isp_top_axi" }
+};
+
+static int jh7110_isp_top_crg_get(struct jh71x0_clk_priv *priv, struct isp_top_crg *top)
+{
+ int ret;
+
+ top->top_clks = jh7110_isp_top_clks;
+ top->top_clks_num = ARRAY_SIZE(jh7110_isp_top_clks);
+ ret = devm_clk_bulk_get(priv->dev, top->top_clks_num, top->top_clks);
+ if (ret) {
+ dev_err(priv->dev, "top clks get failed: %d\n", ret);
+ return ret;
+ }
+
+ /* The resets should be shared and other ISP modules will use its. */
+ top->top_rsts = devm_reset_control_array_get_shared(priv->dev);
+ if (IS_ERR(top->top_rsts)) {
+ dev_err(priv->dev, "top rsts get failed\n");
+ return PTR_ERR(top->top_rsts);
+ }
+
+ return 0;
+}
+
+static int jh7110_isp_top_crg_enable(struct isp_top_crg *top)
+{
+ int ret;
+
+ ret = clk_bulk_prepare_enable(top->top_clks_num, top->top_clks);
+ if (ret)
+ return ret;
+
+ return reset_control_deassert(top->top_rsts);
+}
+
+static struct clk_hw *jh7110_ispclk_get(struct of_phandle_args *clkspec, void *data)
+{
+ struct jh71x0_clk_priv *priv = data;
+ unsigned int idx = clkspec->args[0];
+
+ if (idx < JH7110_ISPCLK_END)
+ return &priv->reg[idx].hw;
+
+ return ERR_PTR(-EINVAL);
+}
+
+static int jh7110_ispcrg_probe(struct platform_device *pdev)
+{
+ struct jh71x0_clk_priv *priv;
+ struct isp_top_crg *top;
+ unsigned int idx;
+ int ret;
+
+ priv = devm_kzalloc(&pdev->dev,
+ struct_size(priv, reg, JH7110_ISPCLK_END),
+ GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ top = devm_kzalloc(&pdev->dev, sizeof(*top), GFP_KERNEL);
+ if (!top)
+ return -ENOMEM;
+
+ spin_lock_init(&priv->rmw_lock);
+ priv->dev = &pdev->dev;
+ priv->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(priv->base))
+ return PTR_ERR(priv->base);
+
+ dev_set_drvdata(priv->dev, priv->base);
+
+ pm_runtime_enable(priv->dev);
+ ret = pm_runtime_get_sync(priv->dev);
+ if (ret < 0) {
+ dev_err(priv->dev, "failed to turn power: %d\n", ret);
+ return ret;
+ }
+
+ ret = jh7110_isp_top_crg_get(priv, top);
+ if (ret)
+ return ret;
+
+ ret = jh7110_isp_top_crg_enable(top);
+ if (ret)
+ return ret;
+
+ for (idx = 0; idx < JH7110_ISPCLK_END; idx++) {
+ u32 max = jh7110_ispclk_data[idx].max;
+ struct clk_parent_data parents[4] = {};
+ struct clk_init_data init = {
+ .name = jh7110_ispclk_data[idx].name,
+ .ops = starfive_jh71x0_clk_ops(max),
+ .parent_data = parents,
+ .num_parents =
+ ((max & JH71X0_CLK_MUX_MASK) >> JH71X0_CLK_MUX_SHIFT) + 1,
+ .flags = jh7110_ispclk_data[idx].flags,
+ };
+ struct jh71x0_clk *clk = &priv->reg[idx];
+ unsigned int i;
+
+ for (i = 0; i < init.num_parents; i++) {
+ unsigned int pidx = jh7110_ispclk_data[idx].parents[i];
+
+ if (pidx < JH7110_ISPCLK_END)
+ parents[i].hw = &priv->reg[pidx].hw;
+ else if (pidx == JH7110_ISPCLK_ISP_TOP_CORE)
+ parents[i].fw_name = "isp_top_core";
+ else if (pidx == JH7110_ISPCLK_ISP_TOP_AXI)
+ parents[i].fw_name = "isp_top_axi";
+ else if (pidx == JH7110_ISPCLK_NOC_BUS_ISP_AXI)
+ parents[i].fw_name = "noc_bus_isp_axi";
+ else if (pidx == JH7110_ISPCLK_DVP_CLK)
+ parents[i].fw_name = "dvp_clk";
+ }
+
+ clk->hw.init = &init;
+ clk->idx = idx;
+ clk->max_div = max & JH71X0_CLK_DIV_MASK;
+
+ ret = devm_clk_hw_register(&pdev->dev, &clk->hw);
+ if (ret)
+ return ret;
+ }
+
+ ret = devm_of_clk_add_hw_provider(&pdev->dev, jh7110_ispclk_get, priv);
+ if (ret)
+ return ret;
+
+ return jh7110_reset_controller_register(priv, "reset-isp", 3);
+}
+
+static const struct of_device_id jh7110_ispcrg_match[] = {
+ { .compatible = "starfive,jh7110-ispcrg" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, jh7110_ispcrg_match);
+
+static struct platform_driver jh7110_ispcrg_driver = {
+ .probe = jh7110_ispcrg_probe,
+ .driver = {
+ .name = "clk-starfive-jh7110-isp",
+ .of_match_table = jh7110_ispcrg_match,
+ },
+};
+module_platform_driver(jh7110_ispcrg_driver);
+
+MODULE_AUTHOR("Xingyu Wu <[email protected]>");
+MODULE_DESCRIPTION("StarFive JH7110 Image-Signal-Process clock driver");
+MODULE_LICENSE("GPL");
--
2.25.1

2023-01-20 03:06:40

by Xingyu Wu

[permalink] [raw]
Subject: [PATCH v1 03/11] clk: starfive: Add StarFive JH7110 System-Top-Group clock driver

Add driver for the StarFive JH7110 System-Top-Group clock controller.

Signed-off-by: Xingyu Wu <[email protected]>
---
drivers/clk/starfive/Kconfig | 11 ++
drivers/clk/starfive/Makefile | 1 +
.../clk/starfive/clk-starfive-jh7110-stg.c | 180 ++++++++++++++++++
3 files changed, 192 insertions(+)
create mode 100644 drivers/clk/starfive/clk-starfive-jh7110-stg.c

diff --git a/drivers/clk/starfive/Kconfig b/drivers/clk/starfive/Kconfig
index 2aa664f2cdee..a462b6e53543 100644
--- a/drivers/clk/starfive/Kconfig
+++ b/drivers/clk/starfive/Kconfig
@@ -42,3 +42,14 @@ config CLK_STARFIVE_JH7110_AON
help
Say yes here to support the always-on clock controller on the
StarFive JH7110 SoC.
+
+config CLK_STARFIVE_JH7110_STG
+ tristate "StarFive JH7110 System-Top-Group clock support"
+ depends on CLK_STARFIVE_JH7110_SYS
+ select AUXILIARY_BUS
+ select CLK_STARFIVE_JH71X0
+ select RESET_STARFIVE_JH7110
+ default CLK_STARFIVE_JH7110_SYS
+ help
+ Say yes here to support the System-Top-Group clock controller
+ on the StarFive JH7110 SoC.
diff --git a/drivers/clk/starfive/Makefile b/drivers/clk/starfive/Makefile
index f3df7d957b1e..b81e97ee2659 100644
--- a/drivers/clk/starfive/Makefile
+++ b/drivers/clk/starfive/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_CLK_STARFIVE_JH7100_AUDIO) += clk-starfive-jh7100-audio.o

obj-$(CONFIG_CLK_STARFIVE_JH7110_SYS) += clk-starfive-jh7110-sys.o
obj-$(CONFIG_CLK_STARFIVE_JH7110_AON) += clk-starfive-jh7110-aon.o
+obj-$(CONFIG_CLK_STARFIVE_JH7110_STG) += clk-starfive-jh7110-stg.o
diff --git a/drivers/clk/starfive/clk-starfive-jh7110-stg.c b/drivers/clk/starfive/clk-starfive-jh7110-stg.c
new file mode 100644
index 000000000000..c2740f44e796
--- /dev/null
+++ b/drivers/clk/starfive/clk-starfive-jh7110-stg.c
@@ -0,0 +1,180 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * StarFive JH7110 System-Top-Group Clock Driver
+ *
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#include <dt-bindings/clock/starfive,jh7110-crg.h>
+
+#include "clk-starfive-jh71x0.h"
+
+/* external clocks */
+#define JH7110_STGCLK_OSC (JH7110_STGCLK_END + 0)
+#define JH7110_STGCLK_HIFI4_CORE (JH7110_STGCLK_END + 1)
+#define JH7110_STGCLK_STG_AXIAHB (JH7110_STGCLK_END + 2)
+#define JH7110_STGCLK_USB_125M (JH7110_STGCLK_END + 3)
+#define JH7110_STGCLK_CPU_BUS (JH7110_STGCLK_END + 4)
+#define JH7110_STGCLK_HIFI4_AXI (JH7110_STGCLK_END + 5)
+#define JH7110_STGCLK_NOCSTG_BUS (JH7110_STGCLK_END + 6)
+#define JH7110_STGCLK_APB_BUS (JH7110_STGCLK_END + 7)
+
+static const struct jh71x0_clk_data jh7110_stgclk_data[] = {
+ /* hifi4 */
+ JH71X0_GATE(JH7110_STGCLK_HIFI4_CLK_CORE, "hifi4_clk_core", 0,
+ JH7110_STGCLK_HIFI4_CORE),
+ /* usb */
+ JH71X0_GATE(JH7110_STGCLK_USB0_APB, "usb0_apb", 0, JH7110_STGCLK_APB_BUS),
+ JH71X0_GATE(JH7110_STGCLK_USB0_UTMI_APB, "usb0_utmi_apb", 0, JH7110_STGCLK_APB_BUS),
+ JH71X0_GATE(JH7110_STGCLK_USB0_AXI, "usb0_axi", 0, JH7110_STGCLK_STG_AXIAHB),
+ JH71X0_GDIV(JH7110_STGCLK_USB0_LPM, "usb0_lpm", 0, 2, JH7110_STGCLK_OSC),
+ JH71X0_GDIV(JH7110_STGCLK_USB0_STB, "usb0_stb", 0, 4, JH7110_STGCLK_OSC),
+ JH71X0_GATE(JH7110_STGCLK_USB0_APP_125, "usb0_app_125", 0, JH7110_STGCLK_USB_125M),
+ JH71X0__DIV(JH7110_STGCLK_USB0_REFCLK, "usb0_refclk", 2, JH7110_STGCLK_OSC),
+ /* pci-e */
+ JH71X0_GATE(JH7110_STGCLK_PCIE0_AXI_MST0, "pcie0_axi_mst0", 0,
+ JH7110_STGCLK_STG_AXIAHB),
+ JH71X0_GATE(JH7110_STGCLK_PCIE0_APB, "pcie0_apb", 0, JH7110_STGCLK_APB_BUS),
+ JH71X0_GATE(JH7110_STGCLK_PCIE0_TL, "pcie0_tl", 0, JH7110_STGCLK_STG_AXIAHB),
+ JH71X0_GATE(JH7110_STGCLK_PCIE1_AXI_MST0, "pcie1_axi_mst0", 0,
+ JH7110_STGCLK_STG_AXIAHB),
+ JH71X0_GATE(JH7110_STGCLK_PCIE1_APB, "pcie1_apb", 0, JH7110_STGCLK_APB_BUS),
+ JH71X0_GATE(JH7110_STGCLK_PCIE1_TL, "pcie1_tl", 0, JH7110_STGCLK_STG_AXIAHB),
+ JH71X0_GATE(JH7110_STGCLK_PCIE01_SLV_DEC_MAINCLK, "pcie01_slv_dec_mainclk",
+ CLK_IGNORE_UNUSED, JH7110_STGCLK_STG_AXIAHB),
+ /* security */
+ JH71X0_GATE(JH7110_STGCLK_SEC_HCLK, "sec_hclk", 0, JH7110_STGCLK_STG_AXIAHB),
+ JH71X0_GATE(JH7110_STGCLK_SEC_MISCAHB, "sec_miscahb", 0, JH7110_STGCLK_STG_AXIAHB),
+ /* stg mtrx */
+ JH71X0_GATE(JH7110_STGCLK_GRP0_MAIN, "mtrx_grp0_main",
+ CLK_IGNORE_UNUSED, JH7110_STGCLK_CPU_BUS),
+ JH71X0_GATE(JH7110_STGCLK_GRP0_BUS, "mtrx_grp0_bus",
+ CLK_IGNORE_UNUSED, JH7110_STGCLK_NOCSTG_BUS),
+ JH71X0_GATE(JH7110_STGCLK_GRP0_STG, "mtrx_grp0_stg",
+ CLK_IGNORE_UNUSED, JH7110_STGCLK_STG_AXIAHB),
+ JH71X0_GATE(JH7110_STGCLK_GRP1_MAIN, "mtrx_grp1_main",
+ CLK_IGNORE_UNUSED, JH7110_STGCLK_CPU_BUS),
+ JH71X0_GATE(JH7110_STGCLK_GRP1_BUS, "mtrx_grp1_bus",
+ CLK_IGNORE_UNUSED, JH7110_STGCLK_NOCSTG_BUS),
+ JH71X0_GATE(JH7110_STGCLK_GRP1_STG, "mtrx_grp1_stg",
+ CLK_IGNORE_UNUSED, JH7110_STGCLK_STG_AXIAHB),
+ JH71X0_GATE(JH7110_STGCLK_GRP1_HIFI, "mtrx_grp1_hifi",
+ CLK_IGNORE_UNUSED, JH7110_STGCLK_HIFI4_AXI),
+ /* e24_rvpi */
+ JH71X0_GDIV(JH7110_STGCLK_E2_RTC, "e2_rtc", 0, 24, JH7110_STGCLK_OSC),
+ JH71X0_GATE(JH7110_STGCLK_E2_CORE, "e2_core",
+ CLK_IGNORE_UNUSED, JH7110_STGCLK_STG_AXIAHB),
+ JH71X0_GATE(JH7110_STGCLK_E2_DBG, "e2_dbg", 0, JH7110_STGCLK_STG_AXIAHB),
+ /* dw_sgdma1p */
+ JH71X0_GATE(JH7110_STGCLK_DMA1P_AXI, "dma1p_axi", 0, JH7110_STGCLK_STG_AXIAHB),
+ JH71X0_GATE(JH7110_STGCLK_DMA1P_AHB, "dma1p_ahb", 0, JH7110_STGCLK_STG_AXIAHB),
+};
+
+static struct clk_hw *jh7110_stgclk_get(struct of_phandle_args *clkspec, void *data)
+{
+ struct jh71x0_clk_priv *priv = data;
+ unsigned int idx = clkspec->args[0];
+
+ if (idx < JH7110_STGCLK_END)
+ return &priv->reg[idx].hw;
+
+ return ERR_PTR(-EINVAL);
+}
+
+static int jh7110_stgcrg_probe(struct platform_device *pdev)
+{
+ struct jh71x0_clk_priv *priv;
+ unsigned int idx;
+ int ret;
+
+ priv = devm_kzalloc(&pdev->dev,
+ struct_size(priv, reg, JH7110_STGCLK_END),
+ GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ spin_lock_init(&priv->rmw_lock);
+ priv->dev = &pdev->dev;
+ priv->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(priv->base))
+ return PTR_ERR(priv->base);
+
+ dev_set_drvdata(priv->dev, priv->base);
+
+ for (idx = 0; idx < JH7110_STGCLK_END; idx++) {
+ u32 max = jh7110_stgclk_data[idx].max;
+ struct clk_parent_data parents[4] = {};
+ struct clk_init_data init = {
+ .name = jh7110_stgclk_data[idx].name,
+ .ops = starfive_jh71x0_clk_ops(max),
+ .parent_data = parents,
+ .num_parents =
+ ((max & JH71X0_CLK_MUX_MASK) >> JH71X0_CLK_MUX_SHIFT) + 1,
+ .flags = jh7110_stgclk_data[idx].flags,
+ };
+ struct jh71x0_clk *clk = &priv->reg[idx];
+ unsigned int i;
+
+ for (i = 0; i < init.num_parents; i++) {
+ unsigned int pidx = jh7110_stgclk_data[idx].parents[i];
+
+ if (pidx < JH7110_STGCLK_END)
+ parents[i].hw = &priv->reg[pidx].hw;
+ else if (pidx == JH7110_STGCLK_OSC)
+ parents[i].fw_name = "osc";
+ else if (pidx == JH7110_STGCLK_HIFI4_CORE)
+ parents[i].fw_name = "hifi4_core";
+ else if (pidx == JH7110_STGCLK_STG_AXIAHB)
+ parents[i].fw_name = "stg_axiahb";
+ else if (pidx == JH7110_STGCLK_USB_125M)
+ parents[i].fw_name = "usb_125m";
+ else if (pidx == JH7110_STGCLK_CPU_BUS)
+ parents[i].fw_name = "cpu_bus";
+ else if (pidx == JH7110_STGCLK_HIFI4_AXI)
+ parents[i].fw_name = "hifi4_axi";
+ else if (pidx == JH7110_STGCLK_NOCSTG_BUS)
+ parents[i].fw_name = "nocstg_bus";
+ else if (pidx == JH7110_STGCLK_APB_BUS)
+ parents[i].fw_name = "apb_bus";
+ }
+
+ clk->hw.init = &init;
+ clk->idx = idx;
+ clk->max_div = max & JH71X0_CLK_DIV_MASK;
+
+ ret = devm_clk_hw_register(&pdev->dev, &clk->hw);
+ if (ret)
+ return ret;
+ }
+
+ ret = devm_of_clk_add_hw_provider(&pdev->dev, jh7110_stgclk_get, priv);
+ if (ret)
+ return ret;
+
+ return jh7110_reset_controller_register(priv, "reset-stg", 2);
+}
+
+static const struct of_device_id jh7110_stgcrg_match[] = {
+ { .compatible = "starfive,jh7110-stgcrg" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, jh7110_stgcrg_match);
+
+static struct platform_driver jh7110_stgcrg_driver = {
+ .probe = jh7110_stgcrg_probe,
+ .driver = {
+ .name = "clk-starfive-jh7110-stg",
+ .of_match_table = jh7110_stgcrg_match,
+ },
+};
+module_platform_driver(jh7110_stgcrg_driver);
+
+MODULE_AUTHOR("Xingyu Wu <[email protected]>");
+MODULE_DESCRIPTION("StarFive JH7110 System-Top-Group clock driver");
+MODULE_LICENSE("GPL");
--
2.25.1

2023-01-20 03:34:45

by Xingyu Wu

[permalink] [raw]
Subject: [PATCH v1 02/11] reset: starfive: jh7110: Add StarFive System-Top-Group reset support

Add auxiliary_device_id to support StarFive JH7110 System-Top-Group resets
of which the auxiliary device name is "clk_starfive_jh71x0.reset-stg".

Signed-off-by: Xingyu Wu <[email protected]>
---
drivers/reset/starfive/reset-starfive-jh7110.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/drivers/reset/starfive/reset-starfive-jh7110.c b/drivers/reset/starfive/reset-starfive-jh7110.c
index 74bbc79f86af..580ec4ed055b 100644
--- a/drivers/reset/starfive/reset-starfive-jh7110.c
+++ b/drivers/reset/starfive/reset-starfive-jh7110.c
@@ -40,6 +40,12 @@ static const struct reset_info jh7110_aon_info = {
.status_offset = 0x3C,
};

+static const struct reset_info jh7110_stg_info = {
+ .nr_resets = JH7110_STGRST_END,
+ .assert_offset = 0x74,
+ .status_offset = 0x78,
+};
+
static const struct auxiliary_device_id jh7110_reset_ids[] = {
{
.name = "clk_starfive_jh71x0.reset-sys",
@@ -49,6 +55,10 @@ static const struct auxiliary_device_id jh7110_reset_ids[] = {
.name = "clk_starfive_jh71x0.reset-aon",
.driver_data = (kernel_ulong_t)&jh7110_aon_info,
},
+ {
+ .name = "clk_starfive_jh71x0.reset-stg",
+ .driver_data = (kernel_ulong_t)&jh7110_stg_info,
+ },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(auxiliary, jh7110_reset_ids);
--
2.25.1

2023-01-26 02:34:03

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v1 03/11] clk: starfive: Add StarFive JH7110 System-Top-Group clock driver

Quoting Xingyu Wu (2023-01-19 18:44:37)
> diff --git a/drivers/clk/starfive/clk-starfive-jh7110-stg.c b/drivers/clk/starfive/clk-starfive-jh7110-stg.c
> new file mode 100644
> index 000000000000..c2740f44e796
> --- /dev/null
> +++ b/drivers/clk/starfive/clk-starfive-jh7110-stg.c
> @@ -0,0 +1,180 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * StarFive JH7110 System-Top-Group Clock Driver
> + *
> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
> + */
> +
> +#include <linux/clk.h>

Is this include used? If not, please remove.

> +#include <linux/clk-provider.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +
> +#include <dt-bindings/clock/starfive,jh7110-crg.h>
> +
> +#include "clk-starfive-jh71x0.h"
> +
[...]
> +static int jh7110_stgcrg_probe(struct platform_device *pdev)
> +{
> + struct jh71x0_clk_priv *priv;
> + unsigned int idx;
> + int ret;
> +
> + priv = devm_kzalloc(&pdev->dev,
> + struct_size(priv, reg, JH7110_STGCLK_END),
> + GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + spin_lock_init(&priv->rmw_lock);
> + priv->dev = &pdev->dev;
> + priv->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(priv->base))
> + return PTR_ERR(priv->base);
> +
> + dev_set_drvdata(priv->dev, priv->base);
> +
> + for (idx = 0; idx < JH7110_STGCLK_END; idx++) {
> + u32 max = jh7110_stgclk_data[idx].max;
> + struct clk_parent_data parents[4] = {};
> + struct clk_init_data init = {
> + .name = jh7110_stgclk_data[idx].name,
> + .ops = starfive_jh71x0_clk_ops(max),
> + .parent_data = parents,
> + .num_parents =
> + ((max & JH71X0_CLK_MUX_MASK) >> JH71X0_CLK_MUX_SHIFT) + 1,
> + .flags = jh7110_stgclk_data[idx].flags,
> + };
> + struct jh71x0_clk *clk = &priv->reg[idx];
> + unsigned int i;
> +
> + for (i = 0; i < init.num_parents; i++) {
> + unsigned int pidx = jh7110_stgclk_data[idx].parents[i];
> +
> + if (pidx < JH7110_STGCLK_END)
> + parents[i].hw = &priv->reg[pidx].hw;
> + else if (pidx == JH7110_STGCLK_OSC)
> + parents[i].fw_name = "osc";
> + else if (pidx == JH7110_STGCLK_HIFI4_CORE)
> + parents[i].fw_name = "hifi4_core";
> + else if (pidx == JH7110_STGCLK_STG_AXIAHB)
> + parents[i].fw_name = "stg_axiahb";
> + else if (pidx == JH7110_STGCLK_USB_125M)
> + parents[i].fw_name = "usb_125m";
> + else if (pidx == JH7110_STGCLK_CPU_BUS)
> + parents[i].fw_name = "cpu_bus";
> + else if (pidx == JH7110_STGCLK_HIFI4_AXI)
> + parents[i].fw_name = "hifi4_axi";
> + else if (pidx == JH7110_STGCLK_NOCSTG_BUS)
> + parents[i].fw_name = "nocstg_bus";
> + else if (pidx == JH7110_STGCLK_APB_BUS)
> + parents[i].fw_name = "apb_bus";

Can this be an array lookup instead of a pile of conditions?

if (pidx < JH7110_STGCLK_END)
...
else
parents[i].fw_name = fw_table[pidx - JH7110_STGCLK_END];

Or even better, don't use strings at all and just make the 'pidx' number
(possibly minus the end constant) be the 'clocks' property index that
you want.

> + }
> +
> + clk->hw.init = &init;
> + clk->idx = idx;
> + clk->max_div = max & JH71X0_CLK_DIV_MASK;
> +
> + ret = devm_clk_hw_register(&pdev->dev, &clk->hw);
> + if (ret)
> + return ret;
> + }
> +
> + ret = devm_of_clk_add_hw_provider(&pdev->dev, jh7110_stgclk_get, priv);
> + if (ret)
> + return ret;
> +
> + return jh7110_reset_controller_register(priv, "reset-stg", 2);

Is this also devm-ified?

2023-01-26 02:35:22

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v1 06/11] clk: starfive: Add StarFive JH7110 Image-Signal-Process clock driver

Quoting Xingyu Wu (2023-01-19 18:44:40)
> diff --git a/drivers/clk/starfive/clk-starfive-jh7110-isp.c b/drivers/clk/starfive/clk-starfive-jh7110-isp.c
> new file mode 100644
> index 000000000000..f9fc94b4c6f8
> --- /dev/null
> +++ b/drivers/clk/starfive/clk-starfive-jh7110-isp.c
> @@ -0,0 +1,218 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * StarFive JH7110 Image-Signal-Process Clock Driver
> + *
> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
> + */
> +
> +#include <linux/clk.h>

Same include comment. This signals this is a clk consumer, which it
isn't?

2023-01-30 08:06:52

by Xingyu Wu

[permalink] [raw]
Subject: Re: [PATCH v1 03/11] clk: starfive: Add StarFive JH7110 System-Top-Group clock driver

On 2023/1/26 10:33, Stephen Boyd wrote:
> Quoting Xingyu Wu (2023-01-19 18:44:37)
>> diff --git a/drivers/clk/starfive/clk-starfive-jh7110-stg.c b/drivers/clk/starfive/clk-starfive-jh7110-stg.c
>> new file mode 100644
>> index 000000000000..c2740f44e796
>> --- /dev/null
>> +++ b/drivers/clk/starfive/clk-starfive-jh7110-stg.c
>> @@ -0,0 +1,180 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * StarFive JH7110 System-Top-Group Clock Driver
>> + *
>> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
>> + */
>> +
>> +#include <linux/clk.h>
>
> Is this include used? If not, please remove.

Will drop in next patch.


>
>> +#include <linux/clk-provider.h>
>> +#include <linux/io.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +
>> +#include <dt-bindings/clock/starfive,jh7110-crg.h>
>> +
>> +#include "clk-starfive-jh71x0.h"
>> +
> [...]
>> +static int jh7110_stgcrg_probe(struct platform_device *pdev)
>> +{
>> + struct jh71x0_clk_priv *priv;
>> + unsigned int idx;
>> + int ret;
>> +
>> + priv = devm_kzalloc(&pdev->dev,
>> + struct_size(priv, reg, JH7110_STGCLK_END),
>> + GFP_KERNEL);
>> + if (!priv)
>> + return -ENOMEM;
>> +
>> + spin_lock_init(&priv->rmw_lock);
>> + priv->dev = &pdev->dev;
>> + priv->base = devm_platform_ioremap_resource(pdev, 0);
>> + if (IS_ERR(priv->base))
>> + return PTR_ERR(priv->base);
>> +
>> + dev_set_drvdata(priv->dev, priv->base);
>> +
>> + for (idx = 0; idx < JH7110_STGCLK_END; idx++) {
>> + u32 max = jh7110_stgclk_data[idx].max;
>> + struct clk_parent_data parents[4] = {};
>> + struct clk_init_data init = {
>> + .name = jh7110_stgclk_data[idx].name,
>> + .ops = starfive_jh71x0_clk_ops(max),
>> + .parent_data = parents,
>> + .num_parents =
>> + ((max & JH71X0_CLK_MUX_MASK) >> JH71X0_CLK_MUX_SHIFT) + 1,
>> + .flags = jh7110_stgclk_data[idx].flags,
>> + };
>> + struct jh71x0_clk *clk = &priv->reg[idx];
>> + unsigned int i;
>> +
>> + for (i = 0; i < init.num_parents; i++) {
>> + unsigned int pidx = jh7110_stgclk_data[idx].parents[i];
>> +
>> + if (pidx < JH7110_STGCLK_END)
>> + parents[i].hw = &priv->reg[pidx].hw;
>> + else if (pidx == JH7110_STGCLK_OSC)
>> + parents[i].fw_name = "osc";
>> + else if (pidx == JH7110_STGCLK_HIFI4_CORE)
>> + parents[i].fw_name = "hifi4_core";
>> + else if (pidx == JH7110_STGCLK_STG_AXIAHB)
>> + parents[i].fw_name = "stg_axiahb";
>> + else if (pidx == JH7110_STGCLK_USB_125M)
>> + parents[i].fw_name = "usb_125m";
>> + else if (pidx == JH7110_STGCLK_CPU_BUS)
>> + parents[i].fw_name = "cpu_bus";
>> + else if (pidx == JH7110_STGCLK_HIFI4_AXI)
>> + parents[i].fw_name = "hifi4_axi";
>> + else if (pidx == JH7110_STGCLK_NOCSTG_BUS)
>> + parents[i].fw_name = "nocstg_bus";
>> + else if (pidx == JH7110_STGCLK_APB_BUS)
>> + parents[i].fw_name = "apb_bus";
>
> Can this be an array lookup instead of a pile of conditions?
>
> if (pidx < JH7110_STGCLK_END)
> ...
> else
> parents[i].fw_name = fw_table[pidx - JH7110_STGCLK_END];
>
> Or even better, don't use strings at all and just make the 'pidx' number
> (possibly minus the end constant) be the 'clocks' property index that
> you want.

It seen to be a good way that there uses an array.
Based on the another way, can I use the 'pidx' number to get the 'clock-names' property
to be the parent clock name?

>
>> + }
>> +
>> + clk->hw.init = &init;
>> + clk->idx = idx;
>> + clk->max_div = max & JH71X0_CLK_DIV_MASK;
>> +
>> + ret = devm_clk_hw_register(&pdev->dev, &clk->hw);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + ret = devm_of_clk_add_hw_provider(&pdev->dev, jh7110_stgclk_get, priv);
>> + if (ret)
>> + return ret;
>> +
>> + return jh7110_reset_controller_register(priv, "reset-stg", 2);
>
> Is this also devm-ified?

No, it need to be freed actively. I will advise Hal Feng this.


Best regards,
Xingyu Wu

2023-01-30 08:15:18

by Xingyu Wu

[permalink] [raw]
Subject: Re: [PATCH v1 06/11] clk: starfive: Add StarFive JH7110 Image-Signal-Process clock driver

On 2023/1/26 10:35, Stephen Boyd wrote:
> Quoting Xingyu Wu (2023-01-19 18:44:40)
>> diff --git a/drivers/clk/starfive/clk-starfive-jh7110-isp.c b/drivers/clk/starfive/clk-starfive-jh7110-isp.c
>> new file mode 100644
>> index 000000000000..f9fc94b4c6f8
>> --- /dev/null
>> +++ b/drivers/clk/starfive/clk-starfive-jh7110-isp.c
>> @@ -0,0 +1,218 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * StarFive JH7110 Image-Signal-Process Clock Driver
>> + *
>> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
>> + */
>> +
>> +#include <linux/clk.h>
>
> Same include comment. This signals this is a clk consumer, which it
> isn't?

This driver need to use 'clk_bulk_data' struct and some functions like
'devm_clk_bulk_get()' and 'clk_bulk_prepare_enable()' from this include.

Best regards,
Xingyu Wu

2023-01-31 00:36:12

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v1 03/11] clk: starfive: Add StarFive JH7110 System-Top-Group clock driver

Quoting Xingyu Wu (2023-01-30 00:02:28)
> On 2023/1/26 10:33, Stephen Boyd wrote:
> > Quoting Xingyu Wu (2023-01-19 18:44:37)
> >> diff --git a/drivers/clk/starfive/clk-starfive-jh7110-stg.c b/drivers/clk/starfive/clk-starfive-jh7110-stg.c
> >> new file mode 100644
> >> index 000000000000..c2740f44e796
> >> --- /dev/null
> >> +++ b/drivers/clk/starfive/clk-starfive-jh7110-stg.c
[...]
> >> + parents[i].fw_name = "nocstg_bus";
> >> + else if (pidx == JH7110_STGCLK_APB_BUS)
> >> + parents[i].fw_name = "apb_bus";
> >
> > Can this be an array lookup instead of a pile of conditions?
> >
> > if (pidx < JH7110_STGCLK_END)
> > ...
> > else
> > parents[i].fw_name = fw_table[pidx - JH7110_STGCLK_END];
> >
> > Or even better, don't use strings at all and just make the 'pidx' number
> > (possibly minus the end constant) be the 'clocks' property index that
> > you want.
>
> It seen to be a good way that there uses an array.
> Based on the another way, can I use the 'pidx' number to get the 'clock-names' property
> to be the parent clock name?

The binding is your design. It is incorrect if the binding is referencing clocks
provided by the same node though. If that's the case, simply use the hw
pointer directly.

>
> >
> >> + }
> >> +
> >> + clk->hw.init = &init;
> >> + clk->idx = idx;
> >> + clk->max_div = max & JH71X0_CLK_DIV_MASK;
> >> +
> >> + ret = devm_clk_hw_register(&pdev->dev, &clk->hw);
> >> + if (ret)
> >> + return ret;
> >> + }
> >> +
> >> + ret = devm_of_clk_add_hw_provider(&pdev->dev, jh7110_stgclk_get, priv);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + return jh7110_reset_controller_register(priv, "reset-stg", 2);
> >
> > Is this also devm-ified?
>
> No, it need to be freed actively. I will advise Hal Feng this.
>

Oh, that's not good.

2023-01-31 00:38:42

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v1 06/11] clk: starfive: Add StarFive JH7110 Image-Signal-Process clock driver

Quoting Xingyu Wu (2023-01-19 18:44:40)
> diff --git a/drivers/clk/starfive/clk-starfive-jh7110-isp.c b/drivers/clk/starfive/clk-starfive-jh7110-isp.c
> new file mode 100644
> index 000000000000..f9fc94b4c6f8
> --- /dev/null
> +++ b/drivers/clk/starfive/clk-starfive-jh7110-isp.c
> @@ -0,0 +1,218 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * StarFive JH7110 Image-Signal-Process Clock Driver
[...]
> +
> +static int jh7110_isp_top_crg_enable(struct isp_top_crg *top)
> +{
> + int ret;
> +
> + ret = clk_bulk_prepare_enable(top->top_clks_num, top->top_clks);
> + if (ret)
> + return ret;
> +
> + return reset_control_deassert(top->top_rsts);
> +}

This needs to be undone on driver remove.

2023-01-31 06:55:37

by Xingyu Wu

[permalink] [raw]
Subject: Re: [PATCH v1 03/11] clk: starfive: Add StarFive JH7110 System-Top-Group clock driver

On 2023/1/31 8:35, Stephen Boyd wrote:
> Quoting Xingyu Wu (2023-01-30 00:02:28)
>> On 2023/1/26 10:33, Stephen Boyd wrote:
>> > Quoting Xingyu Wu (2023-01-19 18:44:37)
>> >> diff --git a/drivers/clk/starfive/clk-starfive-jh7110-stg.c b/drivers/clk/starfive/clk-starfive-jh7110-stg.c
>> >> new file mode 100644
>> >> index 000000000000..c2740f44e796
>> >> --- /dev/null
>> >> +++ b/drivers/clk/starfive/clk-starfive-jh7110-stg.c
> [...]
>> >> + parents[i].fw_name = "nocstg_bus";
>> >> + else if (pidx == JH7110_STGCLK_APB_BUS)
>> >> + parents[i].fw_name = "apb_bus";
>> >
>> > Can this be an array lookup instead of a pile of conditions?
>> >
>> > if (pidx < JH7110_STGCLK_END)
>> > ...
>> > else
>> > parents[i].fw_name = fw_table[pidx - JH7110_STGCLK_END];
>> >
>> > Or even better, don't use strings at all and just make the 'pidx' number
>> > (possibly minus the end constant) be the 'clocks' property index that
>> > you want.
>>
>> It seen to be a good way that there uses an array.
>> Based on the another way, can I use the 'pidx' number to get the 'clock-names' property
>> to be the parent clock name?
>
> The binding is your design. It is incorrect if the binding is referencing clocks
> provided by the same node though. If that's the case, simply use the hw
> pointer directly.

There are external clocks and some of which belong to the SYS clock part.
Our clocks are divided into SYS, AON, STG, ISP and VOUT parts and they are different nodes.
So I think I use the clock names maybe better than use the hw pointer.

>
>>
>> >
>> >> + }
>> >> +
>> >> + clk->hw.init = &init;
>> >> + clk->idx = idx;
>> >> + clk->max_div = max & JH71X0_CLK_DIV_MASK;
>> >> +
>> >> + ret = devm_clk_hw_register(&pdev->dev, &clk->hw);
>> >> + if (ret)
>> >> + return ret;
>> >> + }
>> >> +
>> >> + ret = devm_of_clk_add_hw_provider(&pdev->dev, jh7110_stgclk_get, priv);
>> >> + if (ret)
>> >> + return ret;
>> >> +
>> >> + return jh7110_reset_controller_register(priv, "reset-stg", 2);
>> >
>> > Is this also devm-ified?
>>
>> No, it need to be freed actively. I will advise Hal Feng this.
>>
>
> Oh, that's not good.

Will add this in nest patch.


Best regards,
Xingyu Wu

2023-01-31 06:56:44

by Xingyu Wu

[permalink] [raw]
Subject: Re: [PATCH v1 06/11] clk: starfive: Add StarFive JH7110 Image-Signal-Process clock driver

On 2023/1/31 8:38, Stephen Boyd wrote:
> Quoting Xingyu Wu (2023-01-19 18:44:40)
>> diff --git a/drivers/clk/starfive/clk-starfive-jh7110-isp.c b/drivers/clk/starfive/clk-starfive-jh7110-isp.c
>> new file mode 100644
>> index 000000000000..f9fc94b4c6f8
>> --- /dev/null
>> +++ b/drivers/clk/starfive/clk-starfive-jh7110-isp.c
>> @@ -0,0 +1,218 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * StarFive JH7110 Image-Signal-Process Clock Driver
> [...]
>> +
>> +static int jh7110_isp_top_crg_enable(struct isp_top_crg *top)
>> +{
>> + int ret;
>> +
>> + ret = clk_bulk_prepare_enable(top->top_clks_num, top->top_clks);
>> + if (ret)
>> + return ret;
>> +
>> + return reset_control_deassert(top->top_rsts);
>> +}
>
> This needs to be undone on driver remove.

Will add in next patch.

Best regards,
Xingyu Wu