This patchset contains subsystem clocks support for Mediatek MT8173.
It also contains some bug fixes before adds new clocks support.
James Liao (4):
clk: mediatek: Fix apmixedsys clock registration
dt-bindings: ARM: Mediatek: Document devicetree bindings for clock
controllers
clk: mediatek: Add subsystem clocks of MT8173
clk: mediatek: Add USB clock support in MT8173 APMIXEDSYS
Sascha Hauer (1):
clk: mediatek: mt8173: Fix enabling of critical clocks
.../bindings/arm/mediatek/mediatek,imgsys.txt | 22 +
.../bindings/arm/mediatek/mediatek,mmsys.txt | 22 +
.../bindings/arm/mediatek/mediatek,vdecsys.txt | 22 +
.../bindings/arm/mediatek/mediatek,vencltsys.txt | 22 +
.../bindings/arm/mediatek/mediatek,vencsys.txt | 22 +
drivers/clk/mediatek/clk-mt8135.c | 2 +-
drivers/clk/mediatek/clk-mt8173.c | 454 ++++++++++++++++++++-
drivers/clk/mediatek/clk-pll.c | 7 +-
include/dt-bindings/clock/mt8173-clk.h | 94 ++++-
9 files changed, 652 insertions(+), 15 deletions(-)
create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,imgsys.txt
create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,vdecsys.txt
create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,vencltsys.txt
create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,vencsys.txt
--
1.8.1.1.dirty
The size of clk_data should be the same as CLK_APMIXED_NR_CLK
instead of ARRAY_SIZE(plls). CLK_APMIXED_* is numbered from 1, so
CLK_APMIXED_NR_CLK will be greater than ARRAY_SIZE(plls).
Signed-off-by: James Liao <[email protected]>
---
drivers/clk/mediatek/clk-mt8135.c | 2 +-
drivers/clk/mediatek/clk-mt8173.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/mediatek/clk-mt8135.c b/drivers/clk/mediatek/clk-mt8135.c
index a63435b..08b4b84 100644
--- a/drivers/clk/mediatek/clk-mt8135.c
+++ b/drivers/clk/mediatek/clk-mt8135.c
@@ -634,7 +634,7 @@ static void __init mtk_apmixedsys_init(struct device_node *node)
{
struct clk_onecell_data *clk_data;
- clk_data = mtk_alloc_clk_data(ARRAY_SIZE(plls));
+ clk_data = mtk_alloc_clk_data(CLK_APMIXED_NR_CLK);
if (!clk_data)
return;
diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c
index 357b080..4b9e04c 100644
--- a/drivers/clk/mediatek/clk-mt8173.c
+++ b/drivers/clk/mediatek/clk-mt8173.c
@@ -818,7 +818,7 @@ static void __init mtk_apmixedsys_init(struct device_node *node)
{
struct clk_onecell_data *clk_data;
- clk_data = mtk_alloc_clk_data(ARRAY_SIZE(plls));
+ clk_data = mtk_alloc_clk_data(CLK_APMIXED_NR_CLK);
if (!clk_data)
return;
--
1.8.1.1.dirty
From: Sascha Hauer <[email protected]>
On the MT8173 the clocks are provided by different units. To enable
the critical clocks we must be sure that all parent clocks are already
registered, otherwise the parents of the critical clocks end up being
unused and get disabled later. To find a place where all parents are
registered we try each time after we've registered some clocks if
all known providers are present now and only then we enable the critical
clocks
Signed-off-by: Sascha Hauer <[email protected]>
Signed-off-by: James Liao <[email protected]>
---
drivers/clk/mediatek/clk-mt8173.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c
index 4b9e04c..eb175ac 100644
--- a/drivers/clk/mediatek/clk-mt8173.c
+++ b/drivers/clk/mediatek/clk-mt8173.c
@@ -700,6 +700,20 @@ static const struct mtk_composite peri_clks[] __initconst = {
MUX(CLK_PERI_UART3_SEL, "uart3_ck_sel", uart_ck_sel_parents, 0x40c, 3, 1),
};
+static struct clk_onecell_data *mt8173_top_clk_data;
+static struct clk_onecell_data *mt8173_pll_clk_data;
+
+static void mtk_clk_enable_critical(void)
+{
+ if (!mt8173_top_clk_data || !mt8173_pll_clk_data)
+ return;
+
+ clk_prepare_enable(mt8173_top_clk_data->clks[CLK_TOP_MEM_SEL]);
+ clk_prepare_enable(mt8173_top_clk_data->clks[CLK_TOP_DDRPHYCFG_SEL]);
+ clk_prepare_enable(mt8173_top_clk_data->clks[CLK_TOP_CCI400_SEL]);
+ clk_prepare_enable(mt8173_top_clk_data->clks[CLK_TOP_RTC_SEL]);
+}
+
static void __init mtk_topckgen_init(struct device_node *node)
{
struct clk_onecell_data *clk_data;
@@ -712,19 +726,19 @@ static void __init mtk_topckgen_init(struct device_node *node)
return;
}
- clk_data = mtk_alloc_clk_data(CLK_TOP_NR_CLK);
+ mt8173_top_clk_data = clk_data = mtk_alloc_clk_data(CLK_TOP_NR_CLK);
mtk_clk_register_factors(root_clk_alias, ARRAY_SIZE(root_clk_alias), clk_data);
mtk_clk_register_factors(top_divs, ARRAY_SIZE(top_divs), clk_data);
mtk_clk_register_composites(top_muxes, ARRAY_SIZE(top_muxes), base,
&mt8173_clk_lock, clk_data);
- clk_prepare_enable(clk_data->clks[CLK_TOP_CCI400_SEL]);
-
r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
if (r)
pr_err("%s(): could not register clock provider: %d\n",
__func__, r);
+
+ mtk_clk_enable_critical();
}
CLK_OF_DECLARE(mtk_topckgen, "mediatek,mt8173-topckgen", mtk_topckgen_init);
@@ -818,13 +832,13 @@ static void __init mtk_apmixedsys_init(struct device_node *node)
{
struct clk_onecell_data *clk_data;
- clk_data = mtk_alloc_clk_data(CLK_APMIXED_NR_CLK);
+ mt8173_pll_clk_data = clk_data = mtk_alloc_clk_data(CLK_APMIXED_NR_CLK);
if (!clk_data)
return;
mtk_clk_register_plls(node, plls, ARRAY_SIZE(plls), clk_data);
- clk_prepare_enable(clk_data->clks[CLK_APMIXED_ARMCA15PLL]);
+ mtk_clk_enable_critical();
}
CLK_OF_DECLARE(mtk_apmixedsys, "mediatek,mt8173-apmixedsys",
mtk_apmixedsys_init);
--
1.8.1.1.dirty
This adds the binding documentation for the mmsys, imgsys, vdecsys,
vencsys and vencltsys controllers found on Mediatek SoCs.
Signed-off-by: James Liao <[email protected]>
---
.../bindings/arm/mediatek/mediatek,imgsys.txt | 22 ++++++++++++++++++++++
.../bindings/arm/mediatek/mediatek,mmsys.txt | 22 ++++++++++++++++++++++
.../bindings/arm/mediatek/mediatek,vdecsys.txt | 22 ++++++++++++++++++++++
.../bindings/arm/mediatek/mediatek,vencltsys.txt | 22 ++++++++++++++++++++++
.../bindings/arm/mediatek/mediatek,vencsys.txt | 22 ++++++++++++++++++++++
5 files changed, 110 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,imgsys.txt
create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,vdecsys.txt
create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,vencltsys.txt
create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,vencsys.txt
diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,imgsys.txt b/Documentation/devicetree/bindings/arm/mediatek/mediatek,imgsys.txt
new file mode 100644
index 0000000..7612bac
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,imgsys.txt
@@ -0,0 +1,22 @@
+Mediatek imgsys controller
+============================
+
+The Mediatek imgsys controller provides various clocks to the system.
+
+Required Properties:
+
+- compatible: Should be:
+ - "mediatek,mt8173-imgsys", "syscon"
+- #clock-cells: Must be 1
+
+The imgsys controller uses the common clk binding from
+Documentation/devicetree/bindings/clock/clock-bindings.txt
+The available clocks are defined in dt-bindings/clock/mt*-clk.h.
+
+Example:
+
+imgsys: imgsys@15000000 {
+ compatible = "mediatek,mt8173-imgsys", "syscon";
+ reg = <0 0x15000000 0 0x1000>;
+ #clock-cells = <1>;
+};
diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
new file mode 100644
index 0000000..b51e417
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
@@ -0,0 +1,22 @@
+Mediatek mmsys controller
+============================
+
+The Mediatek mmsys controller provides various clocks to the system.
+
+Required Properties:
+
+- compatible: Should be:
+ - "mediatek,mt8173-mmsys", "syscon"
+- #clock-cells: Must be 1
+
+The mmsys controller uses the common clk binding from
+Documentation/devicetree/bindings/clock/clock-bindings.txt
+The available clocks are defined in dt-bindings/clock/mt*-clk.h.
+
+Example:
+
+mmsys: mmsys@14000000 {
+ compatible = "mediatek,mt8173-mmsys", "syscon";
+ reg = <0 0x14000000 0 0x1000>;
+ #clock-cells = <1>;
+};
diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,vdecsys.txt b/Documentation/devicetree/bindings/arm/mediatek/mediatek,vdecsys.txt
new file mode 100644
index 0000000..a5b94a7
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,vdecsys.txt
@@ -0,0 +1,22 @@
+Mediatek vdecsys controller
+============================
+
+The Mediatek vdecsys controller provides various clocks to the system.
+
+Required Properties:
+
+- compatible: Should be:
+ - "mediatek,mt8173-vdecsys", "syscon"
+- #clock-cells: Must be 1
+
+The vdecsys controller uses the common clk binding from
+Documentation/devicetree/bindings/clock/clock-bindings.txt
+The available clocks are defined in dt-bindings/clock/mt*-clk.h.
+
+Example:
+
+vdecsys: vdecsys@16000000 {
+ compatible = "mediatek,mt8173-vdecsys", "syscon";
+ reg = <0 0x16000000 0 0x1000>;
+ #clock-cells = <1>;
+};
diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,vencltsys.txt b/Documentation/devicetree/bindings/arm/mediatek/mediatek,vencltsys.txt
new file mode 100644
index 0000000..3d4e8d8
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,vencltsys.txt
@@ -0,0 +1,22 @@
+Mediatek vencltsys controller
+============================
+
+The Mediatek vencltsys controller provides various clocks to the system.
+
+Required Properties:
+
+- compatible: Should be:
+ - "mediatek,mt8173-vencltsys", "syscon"
+- #clock-cells: Must be 1
+
+The vencltsys controller uses the common clk binding from
+Documentation/devicetree/bindings/clock/clock-bindings.txt
+The available clocks are defined in dt-bindings/clock/mt*-clk.h.
+
+Example:
+
+vencltsys: vencltsys@19000000 {
+ compatible = "mediatek,mt8173-vencltsys", "syscon";
+ reg = <0 0x19000000 0 0x1000>;
+ #clock-cells = <1>;
+};
diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,vencsys.txt b/Documentation/devicetree/bindings/arm/mediatek/mediatek,vencsys.txt
new file mode 100644
index 0000000..e5b72f5
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,vencsys.txt
@@ -0,0 +1,22 @@
+Mediatek vencsys controller
+============================
+
+The Mediatek vencsys controller provides various clocks to the system.
+
+Required Properties:
+
+- compatible: Should be:
+ - "mediatek,mt8173-vencsys", "syscon"
+- #clock-cells: Must be 1
+
+The vencsys controller uses the common clk binding from
+Documentation/devicetree/bindings/clock/clock-bindings.txt
+The available clocks are defined in dt-bindings/clock/mt*-clk.h.
+
+Example:
+
+vencsys: vencsys@18000000 {
+ compatible = "mediatek,mt8173-vencsys", "syscon";
+ reg = <0 0x18000000 0 0x1000>;
+ #clock-cells = <1>;
+};
--
1.8.1.1.dirty
Most multimedia subsystem clocks will be accessed by multiple
drivers, so it's a better way to manage these clocks in CCF.
This patch adds clock support for MM, IMG, VDEC, VENC and VENC_LT
subsystems.
Signed-off-by: James Liao <[email protected]>
---
drivers/clk/mediatek/clk-mt8173.c | 310 +++++++++++++++++++++++++++++++++
include/dt-bindings/clock/mt8173-clk.h | 87 +++++++++
2 files changed, 397 insertions(+)
diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c
index eb175ac..e2f40ba 100644
--- a/drivers/clk/mediatek/clk-mt8173.c
+++ b/drivers/clk/mediatek/clk-mt8173.c
@@ -700,6 +700,195 @@ static const struct mtk_composite peri_clks[] __initconst = {
MUX(CLK_PERI_UART3_SEL, "uart3_ck_sel", uart_ck_sel_parents, 0x40c, 3, 1),
};
+static struct mtk_gate_regs img_cg_regs = {
+ .set_ofs = 0x0004,
+ .clr_ofs = 0x0008,
+ .sta_ofs = 0x0000,
+};
+
+#define GATE_IMG(_id, _name, _parent, _shift) { \
+ .id = _id, \
+ .name = _name, \
+ .parent_name = _parent, \
+ .regs = &img_cg_regs, \
+ .shift = _shift, \
+ .ops = &mtk_clk_gate_ops_setclr, \
+ }
+
+static struct mtk_gate img_clks[] __initdata = {
+ GATE_IMG(CLK_IMG_LARB2_SMI, "img_larb2_smi", "mm_sel", 0),
+ GATE_IMG(CLK_IMG_CAM_SMI, "img_cam_smi", "mm_sel", 5),
+ GATE_IMG(CLK_IMG_CAM_CAM, "img_cam_cam", "mm_sel", 6),
+ GATE_IMG(CLK_IMG_SEN_TG, "img_sen_tg", "camtg_sel", 7),
+ GATE_IMG(CLK_IMG_SEN_CAM, "img_sen_cam", "mm_sel", 8),
+ GATE_IMG(CLK_IMG_CAM_SV, "img_cam_sv", "mm_sel", 9),
+ GATE_IMG(CLK_IMG_FD, "img_fd", "mm_sel", 11),
+};
+
+static struct mtk_gate_regs mm0_cg_regs = {
+ .set_ofs = 0x0104,
+ .clr_ofs = 0x0108,
+ .sta_ofs = 0x0100,
+};
+
+static struct mtk_gate_regs mm1_cg_regs = {
+ .set_ofs = 0x0114,
+ .clr_ofs = 0x0118,
+ .sta_ofs = 0x0110,
+};
+
+#define GATE_MM0(_id, _name, _parent, _shift) { \
+ .id = _id, \
+ .name = _name, \
+ .parent_name = _parent, \
+ .regs = &mm0_cg_regs, \
+ .shift = _shift, \
+ .ops = &mtk_clk_gate_ops_setclr, \
+ }
+
+#define GATE_MM1(_id, _name, _parent, _shift) { \
+ .id = _id, \
+ .name = _name, \
+ .parent_name = _parent, \
+ .regs = &mm1_cg_regs, \
+ .shift = _shift, \
+ .ops = &mtk_clk_gate_ops_setclr, \
+ }
+
+static struct mtk_gate mm_clks[] __initdata = {
+ /* MM0 */
+ GATE_MM0(CLK_MM_SMI_COMMON, "mm_smi_common", "mm_sel", 0),
+ GATE_MM0(CLK_MM_SMI_LARB0, "mm_smi_larb0", "mm_sel", 1),
+ GATE_MM0(CLK_MM_CAM_MDP, "mm_cam_mdp", "mm_sel", 2),
+ GATE_MM0(CLK_MM_MDP_RDMA0, "mm_mdp_rdma0", "mm_sel", 3),
+ GATE_MM0(CLK_MM_MDP_RDMA1, "mm_mdp_rdma1", "mm_sel", 4),
+ GATE_MM0(CLK_MM_MDP_RSZ0, "mm_mdp_rsz0", "mm_sel", 5),
+ GATE_MM0(CLK_MM_MDP_RSZ1, "mm_mdp_rsz1", "mm_sel", 6),
+ GATE_MM0(CLK_MM_MDP_RSZ2, "mm_mdp_rsz2", "mm_sel", 7),
+ GATE_MM0(CLK_MM_MDP_TDSHP0, "mm_mdp_tdshp0", "mm_sel", 8),
+ GATE_MM0(CLK_MM_MDP_TDSHP1, "mm_mdp_tdshp1", "mm_sel", 9),
+ GATE_MM0(CLK_MM_MDP_WDMA, "mm_mdp_wdma", "mm_sel", 11),
+ GATE_MM0(CLK_MM_MDP_WROT0, "mm_mdp_wrot0", "mm_sel", 12),
+ GATE_MM0(CLK_MM_MDP_WROT1, "mm_mdp_wrot1", "mm_sel", 13),
+ GATE_MM0(CLK_MM_FAKE_ENG, "mm_fake_eng", "mm_sel", 14),
+ GATE_MM0(CLK_MM_MUTEX_32K, "mm_mutex_32k", "rtc_sel", 15),
+ GATE_MM0(CLK_MM_DISP_OVL0, "mm_disp_ovl0", "mm_sel", 16),
+ GATE_MM0(CLK_MM_DISP_OVL1, "mm_disp_ovl1", "mm_sel", 17),
+ GATE_MM0(CLK_MM_DISP_RDMA0, "mm_disp_rdma0", "mm_sel", 18),
+ GATE_MM0(CLK_MM_DISP_RDMA1, "mm_disp_rdma1", "mm_sel", 19),
+ GATE_MM0(CLK_MM_DISP_RDMA2, "mm_disp_rdma2", "mm_sel", 20),
+ GATE_MM0(CLK_MM_DISP_WDMA0, "mm_disp_wdma0", "mm_sel", 21),
+ GATE_MM0(CLK_MM_DISP_WDMA1, "mm_disp_wdma1", "mm_sel", 22),
+ GATE_MM0(CLK_MM_DISP_COLOR0, "mm_disp_color0", "mm_sel", 23),
+ GATE_MM0(CLK_MM_DISP_COLOR1, "mm_disp_color1", "mm_sel", 24),
+ GATE_MM0(CLK_MM_DISP_AAL, "mm_disp_aal", "mm_sel", 25),
+ GATE_MM0(CLK_MM_DISP_GAMMA, "mm_disp_gamma", "mm_sel", 26),
+ GATE_MM0(CLK_MM_DISP_UFOE, "mm_disp_ufoe", "mm_sel", 27),
+ GATE_MM0(CLK_MM_DISP_SPLIT0, "mm_disp_split0", "mm_sel", 28),
+ GATE_MM0(CLK_MM_DISP_SPLIT1, "mm_disp_split1", "mm_sel", 29),
+ GATE_MM0(CLK_MM_DISP_MERGE, "mm_disp_merge", "mm_sel", 30),
+ GATE_MM0(CLK_MM_DISP_OD, "mm_disp_od", "mm_sel", 31),
+ /* MM1 */
+ GATE_MM1(CLK_MM_DISP_PWM0MM, "mm_disp_pwm0mm", "mm_sel", 0),
+ GATE_MM1(CLK_MM_DISP_PWM026M, "mm_disp_pwm026m", "pwm_sel", 1),
+ GATE_MM1(CLK_MM_DISP_PWM1MM, "mm_disp_pwm1mm", "mm_sel", 2),
+ GATE_MM1(CLK_MM_DISP_PWM126M, "mm_disp_pwm126m", "pwm_sel", 3),
+ GATE_MM1(CLK_MM_DSI0_ENGINE, "mm_dsi0_engine", "mm_sel", 4),
+ GATE_MM1(CLK_MM_DSI0_DIGITAL, "mm_dsi0_digital", "clk_null", 5),
+ GATE_MM1(CLK_MM_DSI1_ENGINE, "mm_dsi1_engine", "mm_sel", 6),
+ GATE_MM1(CLK_MM_DSI1_DIGITAL, "mm_dsi1_digital", "clk_null", 7),
+ GATE_MM1(CLK_MM_DPI_PIXEL, "mm_dpi_pixel", "dpi0_sel", 8),
+ GATE_MM1(CLK_MM_DPI_ENGINE, "mm_dpi_engine", "mm_sel", 9),
+ GATE_MM1(CLK_MM_DPI1_PIXEL, "mm_dpi1_pixel", "clk_null", 10),
+ GATE_MM1(CLK_MM_DPI1_ENGINE, "mm_dpi1_engine", "mm_sel", 11),
+ GATE_MM1(CLK_MM_HDMI_PIXEL, "mm_hdmi_pixel", "dpi0_sel", 12),
+ GATE_MM1(CLK_MM_HDMI_PLLCK, "mm_hdmi_pllck", "hdmi_sel", 13),
+ GATE_MM1(CLK_MM_HDMI_AUDIO, "mm_hdmi_audio", "apll1", 14),
+ GATE_MM1(CLK_MM_HDMI_SPDIF, "mm_hdmi_spdif", "apll2", 15),
+ GATE_MM1(CLK_MM_LVDS_PIXEL, "mm_lvds_pixel", "clk_null", 16),
+ GATE_MM1(CLK_MM_LVDS_CTS, "mm_lvds_cts", "clk_null", 17),
+ GATE_MM1(CLK_MM_SMI_LARB4, "mm_smi_larb4", "mm_sel", 18),
+ GATE_MM1(CLK_MM_HDMI_HDCP, "mm_hdmi_hdcp", "hdcp_sel", 19),
+ GATE_MM1(CLK_MM_HDMI_HDCP24M, "mm_hdmi_hdcp24m", "hdcp_24m_sel", 20),
+};
+
+static struct mtk_gate_regs vdec0_cg_regs = {
+ .set_ofs = 0x0000,
+ .clr_ofs = 0x0004,
+ .sta_ofs = 0x0000,
+};
+
+static struct mtk_gate_regs vdec1_cg_regs = {
+ .set_ofs = 0x0008,
+ .clr_ofs = 0x000c,
+ .sta_ofs = 0x0008,
+};
+
+#define GATE_VDEC0(_id, _name, _parent, _shift) { \
+ .id = _id, \
+ .name = _name, \
+ .parent_name = _parent, \
+ .regs = &vdec0_cg_regs, \
+ .shift = _shift, \
+ .ops = &mtk_clk_gate_ops_setclr_inv, \
+ }
+
+#define GATE_VDEC1(_id, _name, _parent, _shift) { \
+ .id = _id, \
+ .name = _name, \
+ .parent_name = _parent, \
+ .regs = &vdec1_cg_regs, \
+ .shift = _shift, \
+ .ops = &mtk_clk_gate_ops_setclr_inv, \
+ }
+
+static struct mtk_gate vdec_clks[] __initdata = {
+ GATE_VDEC0(CLK_VDEC_CKEN, "vdec_cken", "vdec_sel", 0),
+ GATE_VDEC1(CLK_VDEC_LARB_CKEN, "vdec_larb_cken", "mm_sel", 0),
+};
+
+static struct mtk_gate_regs venc_cg_regs = {
+ .set_ofs = 0x0004,
+ .clr_ofs = 0x0008,
+ .sta_ofs = 0x0000,
+};
+
+#define GATE_VENC(_id, _name, _parent, _shift) { \
+ .id = _id, \
+ .name = _name, \
+ .parent_name = _parent, \
+ .regs = &venc_cg_regs, \
+ .shift = _shift, \
+ .ops = &mtk_clk_gate_ops_setclr_inv, \
+ }
+
+static struct mtk_gate venc_clks[] __initdata = {
+ GATE_VENC(CLK_VENC_CKE0, "venc_cke0", "mm_sel", 0),
+ GATE_VENC(CLK_VENC_CKE1, "venc_cke1", "venc_sel", 4),
+ GATE_VENC(CLK_VENC_CKE2, "venc_cke2", "venc_sel", 8),
+ GATE_VENC(CLK_VENC_CKE3, "venc_cke3", "venc_sel", 12),
+};
+
+static struct mtk_gate_regs venclt_cg_regs = {
+ .set_ofs = 0x0004,
+ .clr_ofs = 0x0008,
+ .sta_ofs = 0x0000,
+};
+
+#define GATE_VENCLT(_id, _name, _parent, _shift) { \
+ .id = _id, \
+ .name = _name, \
+ .parent_name = _parent, \
+ .regs = &venclt_cg_regs, \
+ .shift = _shift, \
+ .ops = &mtk_clk_gate_ops_setclr_inv, \
+ }
+
+static struct mtk_gate venclt_clks[] __initdata = {
+ GATE_VENCLT(CLK_VENCLT_CKE0, "venclt_cke0", "mm_sel", 0),
+ GATE_VENCLT(CLK_VENCLT_CKE1, "venclt_cke1", "venclt_sel", 4),
+};
+
static struct clk_onecell_data *mt8173_top_clk_data;
static struct clk_onecell_data *mt8173_pll_clk_data;
@@ -842,3 +1031,124 @@ static void __init mtk_apmixedsys_init(struct device_node *node)
}
CLK_OF_DECLARE(mtk_apmixedsys, "mediatek,mt8173-apmixedsys",
mtk_apmixedsys_init);
+
+static void __init mtk_imgsys_init(struct device_node *node)
+{
+ struct clk_onecell_data *clk_data;
+ void __iomem *base;
+ int r;
+
+ base = of_iomap(node, 0);
+ if (!base) {
+ pr_err("%s(): ioremap failed\n", __func__);
+ return;
+ }
+
+ clk_data = mtk_alloc_clk_data(CLK_IMG_NR_CLK);
+
+ mtk_clk_register_gates(node, img_clks, ARRAY_SIZE(img_clks),
+ clk_data);
+
+ r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
+
+ if (r)
+ pr_err("%s(): could not register clock provider: %d\n",
+ __func__, r);
+}
+CLK_OF_DECLARE(mtk_imgsys, "mediatek,mt8173-imgsys", mtk_imgsys_init);
+
+static void __init mtk_mmsys_init(struct device_node *node)
+{
+ struct clk_onecell_data *clk_data;
+ void __iomem *base;
+ int r;
+
+ base = of_iomap(node, 0);
+ if (!base) {
+ pr_err("%s(): ioremap failed\n", __func__);
+ return;
+ }
+
+ clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK);
+
+ mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks),
+ clk_data);
+
+ r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
+ if (r)
+ pr_err("%s(): could not register clock provider: %d\n",
+ __func__, r);
+}
+CLK_OF_DECLARE(mtk_mmsys, "mediatek,mt8173-mmsys", mtk_mmsys_init);
+
+static void __init mtk_vdecsys_init(struct device_node *node)
+{
+ struct clk_onecell_data *clk_data;
+ void __iomem *base;
+ int r;
+
+ base = of_iomap(node, 0);
+ if (!base) {
+ pr_err("%s(): ioremap failed\n", __func__);
+ return;
+ }
+
+ clk_data = mtk_alloc_clk_data(CLK_VDEC_NR_CLK);
+
+ mtk_clk_register_gates(node, vdec_clks, ARRAY_SIZE(vdec_clks),
+ clk_data);
+
+ r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
+ if (r)
+ pr_err("%s(): could not register clock provider: %d\n",
+ __func__, r);
+}
+CLK_OF_DECLARE(mtk_vdecsys, "mediatek,mt8173-vdecsys", mtk_vdecsys_init);
+
+static void __init mtk_vencsys_init(struct device_node *node)
+{
+ struct clk_onecell_data *clk_data;
+ void __iomem *base;
+ int r;
+
+ base = of_iomap(node, 0);
+ if (!base) {
+ pr_err("%s(): ioremap failed\n", __func__);
+ return;
+ }
+
+ clk_data = mtk_alloc_clk_data(CLK_VENC_NR_CLK);
+
+ mtk_clk_register_gates(node, venc_clks, ARRAY_SIZE(venc_clks),
+ clk_data);
+
+ r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
+ if (r)
+ pr_err("%s(): could not register clock provider: %d\n",
+ __func__, r);
+}
+CLK_OF_DECLARE(mtk_vencsys, "mediatek,mt8173-vencsys", mtk_vencsys_init);
+
+static void __init mtk_vencltsys_init(struct device_node *node)
+{
+ struct clk_onecell_data *clk_data;
+ void __iomem *base;
+ int r;
+
+ base = of_iomap(node, 0);
+ if (!base) {
+ pr_err("%s(): ioremap failed\n", __func__);
+ return;
+ }
+
+ clk_data = mtk_alloc_clk_data(CLK_VENCLT_NR_CLK);
+
+ mtk_clk_register_gates(node, venclt_clks, ARRAY_SIZE(venclt_clks),
+ clk_data);
+
+ r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
+ if (r)
+ pr_err("%s(): could not register clock provider: %d\n",
+ __func__, r);
+}
+CLK_OF_DECLARE(mtk_vencltsys, "mediatek,mt8173-vencltsys", mtk_vencltsys_init);
diff --git a/include/dt-bindings/clock/mt8173-clk.h b/include/dt-bindings/clock/mt8173-clk.h
index 4ad76ed..94977e6 100644
--- a/include/dt-bindings/clock/mt8173-clk.h
+++ b/include/dt-bindings/clock/mt8173-clk.h
@@ -232,4 +232,91 @@
#define CLK_PERI_UART3_SEL 39
#define CLK_PERI_NR_CLK 40
+/* IMG_SYS */
+
+#define CLK_IMG_LARB2_SMI 1
+#define CLK_IMG_CAM_SMI 2
+#define CLK_IMG_CAM_CAM 3
+#define CLK_IMG_SEN_TG 4
+#define CLK_IMG_SEN_CAM 5
+#define CLK_IMG_CAM_SV 6
+#define CLK_IMG_FD 7
+#define CLK_IMG_NR_CLK 8
+
+/* MM_SYS */
+
+#define CLK_MM_SMI_COMMON 1
+#define CLK_MM_SMI_LARB0 2
+#define CLK_MM_CAM_MDP 3
+#define CLK_MM_MDP_RDMA0 4
+#define CLK_MM_MDP_RDMA1 5
+#define CLK_MM_MDP_RSZ0 6
+#define CLK_MM_MDP_RSZ1 7
+#define CLK_MM_MDP_RSZ2 8
+#define CLK_MM_MDP_TDSHP0 9
+#define CLK_MM_MDP_TDSHP1 10
+#define CLK_MM_MDP_WDMA 11
+#define CLK_MM_MDP_WROT0 12
+#define CLK_MM_MDP_WROT1 13
+#define CLK_MM_FAKE_ENG 14
+#define CLK_MM_MUTEX_32K 15
+#define CLK_MM_DISP_OVL0 16
+#define CLK_MM_DISP_OVL1 17
+#define CLK_MM_DISP_RDMA0 18
+#define CLK_MM_DISP_RDMA1 19
+#define CLK_MM_DISP_RDMA2 20
+#define CLK_MM_DISP_WDMA0 21
+#define CLK_MM_DISP_WDMA1 22
+#define CLK_MM_DISP_COLOR0 23
+#define CLK_MM_DISP_COLOR1 24
+#define CLK_MM_DISP_AAL 25
+#define CLK_MM_DISP_GAMMA 26
+#define CLK_MM_DISP_UFOE 27
+#define CLK_MM_DISP_SPLIT0 28
+#define CLK_MM_DISP_SPLIT1 29
+#define CLK_MM_DISP_MERGE 30
+#define CLK_MM_DISP_OD 31
+#define CLK_MM_DISP_PWM0MM 32
+#define CLK_MM_DISP_PWM026M 33
+#define CLK_MM_DISP_PWM1MM 34
+#define CLK_MM_DISP_PWM126M 35
+#define CLK_MM_DSI0_ENGINE 36
+#define CLK_MM_DSI0_DIGITAL 37
+#define CLK_MM_DSI1_ENGINE 38
+#define CLK_MM_DSI1_DIGITAL 39
+#define CLK_MM_DPI_PIXEL 40
+#define CLK_MM_DPI_ENGINE 41
+#define CLK_MM_DPI1_PIXEL 42
+#define CLK_MM_DPI1_ENGINE 43
+#define CLK_MM_HDMI_PIXEL 44
+#define CLK_MM_HDMI_PLLCK 45
+#define CLK_MM_HDMI_AUDIO 46
+#define CLK_MM_HDMI_SPDIF 47
+#define CLK_MM_LVDS_PIXEL 48
+#define CLK_MM_LVDS_CTS 49
+#define CLK_MM_SMI_LARB4 50
+#define CLK_MM_HDMI_HDCP 51
+#define CLK_MM_HDMI_HDCP24M 52
+#define CLK_MM_NR_CLK 53
+
+/* VDEC_SYS */
+
+#define CLK_VDEC_CKEN 1
+#define CLK_VDEC_LARB_CKEN 2
+#define CLK_VDEC_NR_CLK 3
+
+/* VENC_SYS */
+
+#define CLK_VENC_CKE0 1
+#define CLK_VENC_CKE1 2
+#define CLK_VENC_CKE2 3
+#define CLK_VENC_CKE3 4
+#define CLK_VENC_NR_CLK 5
+
+/* VENCLT_SYS */
+
+#define CLK_VENCLT_CKE0 1
+#define CLK_VENCLT_CKE1 2
+#define CLK_VENCLT_NR_CLK 3
+
#endif /* _DT_BINDINGS_CLK_MT8173_H */
--
1.8.1.1.dirty
Add REF2USB_TX clock support into MT8173 APMIXEDSYS. This clock
is needed by USB 3.0.
Signed-off-by: James Liao <[email protected]>
---
drivers/clk/mediatek/clk-mt8173.c | 120 +++++++++++++++++++++++++++++++++
drivers/clk/mediatek/clk-pll.c | 7 +-
include/dt-bindings/clock/mt8173-clk.h | 7 +-
3 files changed, 125 insertions(+), 9 deletions(-)
diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c
index e2f40ba..105844f 100644
--- a/drivers/clk/mediatek/clk-mt8173.c
+++ b/drivers/clk/mediatek/clk-mt8173.c
@@ -12,6 +12,7 @@
* GNU General Public License for more details.
*/
+#include <linux/delay.h>
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/slab.h>
@@ -978,6 +979,118 @@ static void __init mtk_pericfg_init(struct device_node *node)
}
CLK_OF_DECLARE(mtk_pericfg, "mediatek,mt8173-pericfg", mtk_pericfg_init);
+#define REF2USB_TX_EN BIT(0)
+#define REF2USB_TX_LPF_EN BIT(1)
+#define REF2USB_TX_OUT_EN BIT(2)
+#define REF2USB_EN_MASK (REF2USB_TX_EN | REF2USB_TX_LPF_EN | \
+ REF2USB_TX_OUT_EN)
+
+struct mtk_ref2usb_tx {
+ struct clk_hw hw;
+ void __iomem *base_addr;
+};
+
+static inline struct mtk_ref2usb_tx *to_mtk_ref2usb_tx(struct clk_hw *hw)
+{
+ return container_of(hw, struct mtk_ref2usb_tx, hw);
+}
+
+static int mtk_ref2usb_tx_is_prepared(struct clk_hw *hw)
+{
+ struct mtk_ref2usb_tx *tx = to_mtk_ref2usb_tx(hw);
+
+ return (readl(tx->base_addr) & REF2USB_EN_MASK) == REF2USB_EN_MASK;
+}
+
+static int mtk_ref2usb_tx_prepare(struct clk_hw *hw)
+{
+ struct mtk_ref2usb_tx *tx = to_mtk_ref2usb_tx(hw);
+ u32 val;
+
+ val = readl(tx->base_addr);
+
+ val |= REF2USB_TX_EN;
+ writel(val, tx->base_addr);
+ udelay(100);
+
+ val |= REF2USB_TX_LPF_EN;
+ writel(val, tx->base_addr);
+
+ val |= REF2USB_TX_OUT_EN;
+ writel(val, tx->base_addr);
+
+ return 0;
+}
+
+static void mtk_ref2usb_tx_unprepare(struct clk_hw *hw)
+{
+ struct mtk_ref2usb_tx *tx = to_mtk_ref2usb_tx(hw);
+ u32 val;
+
+ val = readl(tx->base_addr);
+ val &= ~REF2USB_EN_MASK;
+ writel(val, tx->base_addr);
+}
+
+static const struct clk_ops mtk_ref2usb_tx_ops = {
+ .is_prepared = mtk_ref2usb_tx_is_prepared,
+ .prepare = mtk_ref2usb_tx_prepare,
+ .unprepare = mtk_ref2usb_tx_unprepare,
+};
+
+static struct clk *mtk_clk_register_ref2usb_tx(const char *name,
+ void __iomem *reg)
+{
+ struct mtk_ref2usb_tx *tx;
+ struct clk_init_data init = {};
+ struct clk *clk;
+ const char *parent_name = "clk26m";
+
+ tx = kzalloc(sizeof(*tx), GFP_KERNEL);
+ if (!tx)
+ return ERR_PTR(-ENOMEM);
+
+ tx->base_addr = reg;
+ tx->hw.init = &init;
+
+ init.name = name;
+ init.ops = &mtk_ref2usb_tx_ops;
+ init.parent_names = &parent_name;
+ init.num_parents = 1;
+
+ clk = clk_register(NULL, &tx->hw);
+
+ if (IS_ERR(clk)) {
+ pr_err("Failed to register clk %s: %ld\n", name, PTR_ERR(clk));
+ kfree(tx);
+ }
+
+ return clk;
+}
+
+static void __init mtk_clk_register_apmixedsys_special(struct device_node *node,
+ struct clk_onecell_data *clk_data)
+{
+ void __iomem *base;
+ struct clk *clk;
+
+ base = of_iomap(node, 0);
+ if (!base) {
+ pr_err("%s(): ioremap failed\n", __func__);
+ return;
+ }
+
+ clk = mtk_clk_register_ref2usb_tx("ref2usb_tx", base + 0x8);
+
+ if (IS_ERR(clk)) {
+ pr_err("Failed to register clk ref2usb_tx: %ld\n",
+ PTR_ERR(clk));
+ return;
+ }
+
+ clk_data->clks[CLK_APMIXED_REF2USB_TX] = clk;
+}
+
#define MT8173_PLL_FMAX (3000UL * MHZ)
#define CON0_MT8173_RST_BAR BIT(24)
@@ -1020,12 +1133,19 @@ static const struct mtk_pll_data plls[] = {
static void __init mtk_apmixedsys_init(struct device_node *node)
{
struct clk_onecell_data *clk_data;
+ int r;
mt8173_pll_clk_data = clk_data = mtk_alloc_clk_data(CLK_APMIXED_NR_CLK);
if (!clk_data)
return;
mtk_clk_register_plls(node, plls, ARRAY_SIZE(plls), clk_data);
+ mtk_clk_register_apmixedsys_special(node, clk_data);
+
+ r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
+ if (r)
+ pr_err("%s(): could not register clock provider: %d\n",
+ __func__, r);
mtk_clk_enable_critical();
}
diff --git a/drivers/clk/mediatek/clk-pll.c b/drivers/clk/mediatek/clk-pll.c
index 44409e9..813f0c7 100644
--- a/drivers/clk/mediatek/clk-pll.c
+++ b/drivers/clk/mediatek/clk-pll.c
@@ -302,7 +302,7 @@ void __init mtk_clk_register_plls(struct device_node *node,
const struct mtk_pll_data *plls, int num_plls, struct clk_onecell_data *clk_data)
{
void __iomem *base;
- int r, i;
+ int i;
struct clk *clk;
base = of_iomap(node, 0);
@@ -324,9 +324,4 @@ void __init mtk_clk_register_plls(struct device_node *node,
clk_data->clks[pll->id] = clk;
}
-
- r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
- if (r)
- pr_err("%s(): could not register clock provider: %d\n",
- __func__, r);
}
diff --git a/include/dt-bindings/clock/mt8173-clk.h b/include/dt-bindings/clock/mt8173-clk.h
index 94977e6..7b7c555 100644
--- a/include/dt-bindings/clock/mt8173-clk.h
+++ b/include/dt-bindings/clock/mt8173-clk.h
@@ -158,8 +158,8 @@
/* APMIXED_SYS */
-#define CLK_APMIXED_ARMCA15PLL 1
-#define CLK_APMIXED_ARMCA7PLL 2
+#define CLK_APMIXED_ARMCA15PLL 1
+#define CLK_APMIXED_ARMCA7PLL 2
#define CLK_APMIXED_MAINPLL 3
#define CLK_APMIXED_UNIVPLL 4
#define CLK_APMIXED_MMPLL 5
@@ -172,7 +172,8 @@
#define CLK_APMIXED_APLL2 12
#define CLK_APMIXED_LVDSPLL 13
#define CLK_APMIXED_MSDCPLL2 14
-#define CLK_APMIXED_NR_CLK 15
+#define CLK_APMIXED_REF2USB_TX 15
+#define CLK_APMIXED_NR_CLK 16
/* INFRA_SYS */
--
1.8.1.1.dirty
Hi James,
On Thu, May 21, 2015 at 3:12 PM, James Liao <[email protected]> wrote:
> Most multimedia subsystem clocks will be accessed by multiple
> drivers, so it's a better way to manage these clocks in CCF.
> This patch adds clock support for MM, IMG, VDEC, VENC and VENC_LT
> subsystems.
Is there a reason why this patch (or patch set) does not also include
definitions for the SCP_SYS, AUDIO and MFG_SYS clocks?
-Dan
>
> Signed-off-by: James Liao <[email protected]>
> ---
> drivers/clk/mediatek/clk-mt8173.c | 310 +++++++++++++++++++++++++++++++++
> include/dt-bindings/clock/mt8173-clk.h | 87 +++++++++
> 2 files changed, 397 insertions(+)
>
> diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c
> index eb175ac..e2f40ba 100644
> --- a/drivers/clk/mediatek/clk-mt8173.c
> +++ b/drivers/clk/mediatek/clk-mt8173.c
> @@ -700,6 +700,195 @@ static const struct mtk_composite peri_clks[] __initconst = {
> MUX(CLK_PERI_UART3_SEL, "uart3_ck_sel", uart_ck_sel_parents, 0x40c, 3, 1),
> };
>
> +static struct mtk_gate_regs img_cg_regs = {
> + .set_ofs = 0x0004,
> + .clr_ofs = 0x0008,
> + .sta_ofs = 0x0000,
> +};
> +
> +#define GATE_IMG(_id, _name, _parent, _shift) { \
> + .id = _id, \
> + .name = _name, \
> + .parent_name = _parent, \
> + .regs = &img_cg_regs, \
> + .shift = _shift, \
> + .ops = &mtk_clk_gate_ops_setclr, \
> + }
> +
> +static struct mtk_gate img_clks[] __initdata = {
> + GATE_IMG(CLK_IMG_LARB2_SMI, "img_larb2_smi", "mm_sel", 0),
> + GATE_IMG(CLK_IMG_CAM_SMI, "img_cam_smi", "mm_sel", 5),
> + GATE_IMG(CLK_IMG_CAM_CAM, "img_cam_cam", "mm_sel", 6),
> + GATE_IMG(CLK_IMG_SEN_TG, "img_sen_tg", "camtg_sel", 7),
> + GATE_IMG(CLK_IMG_SEN_CAM, "img_sen_cam", "mm_sel", 8),
> + GATE_IMG(CLK_IMG_CAM_SV, "img_cam_sv", "mm_sel", 9),
> + GATE_IMG(CLK_IMG_FD, "img_fd", "mm_sel", 11),
> +};
> +
> +static struct mtk_gate_regs mm0_cg_regs = {
> + .set_ofs = 0x0104,
> + .clr_ofs = 0x0108,
> + .sta_ofs = 0x0100,
> +};
> +
> +static struct mtk_gate_regs mm1_cg_regs = {
> + .set_ofs = 0x0114,
> + .clr_ofs = 0x0118,
> + .sta_ofs = 0x0110,
> +};
> +
> +#define GATE_MM0(_id, _name, _parent, _shift) { \
> + .id = _id, \
> + .name = _name, \
> + .parent_name = _parent, \
> + .regs = &mm0_cg_regs, \
> + .shift = _shift, \
> + .ops = &mtk_clk_gate_ops_setclr, \
> + }
> +
> +#define GATE_MM1(_id, _name, _parent, _shift) { \
> + .id = _id, \
> + .name = _name, \
> + .parent_name = _parent, \
> + .regs = &mm1_cg_regs, \
> + .shift = _shift, \
> + .ops = &mtk_clk_gate_ops_setclr, \
> + }
> +
> +static struct mtk_gate mm_clks[] __initdata = {
> + /* MM0 */
> + GATE_MM0(CLK_MM_SMI_COMMON, "mm_smi_common", "mm_sel", 0),
> + GATE_MM0(CLK_MM_SMI_LARB0, "mm_smi_larb0", "mm_sel", 1),
> + GATE_MM0(CLK_MM_CAM_MDP, "mm_cam_mdp", "mm_sel", 2),
> + GATE_MM0(CLK_MM_MDP_RDMA0, "mm_mdp_rdma0", "mm_sel", 3),
> + GATE_MM0(CLK_MM_MDP_RDMA1, "mm_mdp_rdma1", "mm_sel", 4),
> + GATE_MM0(CLK_MM_MDP_RSZ0, "mm_mdp_rsz0", "mm_sel", 5),
> + GATE_MM0(CLK_MM_MDP_RSZ1, "mm_mdp_rsz1", "mm_sel", 6),
> + GATE_MM0(CLK_MM_MDP_RSZ2, "mm_mdp_rsz2", "mm_sel", 7),
> + GATE_MM0(CLK_MM_MDP_TDSHP0, "mm_mdp_tdshp0", "mm_sel", 8),
> + GATE_MM0(CLK_MM_MDP_TDSHP1, "mm_mdp_tdshp1", "mm_sel", 9),
> + GATE_MM0(CLK_MM_MDP_WDMA, "mm_mdp_wdma", "mm_sel", 11),
> + GATE_MM0(CLK_MM_MDP_WROT0, "mm_mdp_wrot0", "mm_sel", 12),
> + GATE_MM0(CLK_MM_MDP_WROT1, "mm_mdp_wrot1", "mm_sel", 13),
> + GATE_MM0(CLK_MM_FAKE_ENG, "mm_fake_eng", "mm_sel", 14),
> + GATE_MM0(CLK_MM_MUTEX_32K, "mm_mutex_32k", "rtc_sel", 15),
> + GATE_MM0(CLK_MM_DISP_OVL0, "mm_disp_ovl0", "mm_sel", 16),
> + GATE_MM0(CLK_MM_DISP_OVL1, "mm_disp_ovl1", "mm_sel", 17),
> + GATE_MM0(CLK_MM_DISP_RDMA0, "mm_disp_rdma0", "mm_sel", 18),
> + GATE_MM0(CLK_MM_DISP_RDMA1, "mm_disp_rdma1", "mm_sel", 19),
> + GATE_MM0(CLK_MM_DISP_RDMA2, "mm_disp_rdma2", "mm_sel", 20),
> + GATE_MM0(CLK_MM_DISP_WDMA0, "mm_disp_wdma0", "mm_sel", 21),
> + GATE_MM0(CLK_MM_DISP_WDMA1, "mm_disp_wdma1", "mm_sel", 22),
> + GATE_MM0(CLK_MM_DISP_COLOR0, "mm_disp_color0", "mm_sel", 23),
> + GATE_MM0(CLK_MM_DISP_COLOR1, "mm_disp_color1", "mm_sel", 24),
> + GATE_MM0(CLK_MM_DISP_AAL, "mm_disp_aal", "mm_sel", 25),
> + GATE_MM0(CLK_MM_DISP_GAMMA, "mm_disp_gamma", "mm_sel", 26),
> + GATE_MM0(CLK_MM_DISP_UFOE, "mm_disp_ufoe", "mm_sel", 27),
> + GATE_MM0(CLK_MM_DISP_SPLIT0, "mm_disp_split0", "mm_sel", 28),
> + GATE_MM0(CLK_MM_DISP_SPLIT1, "mm_disp_split1", "mm_sel", 29),
> + GATE_MM0(CLK_MM_DISP_MERGE, "mm_disp_merge", "mm_sel", 30),
> + GATE_MM0(CLK_MM_DISP_OD, "mm_disp_od", "mm_sel", 31),
> + /* MM1 */
> + GATE_MM1(CLK_MM_DISP_PWM0MM, "mm_disp_pwm0mm", "mm_sel", 0),
> + GATE_MM1(CLK_MM_DISP_PWM026M, "mm_disp_pwm026m", "pwm_sel", 1),
> + GATE_MM1(CLK_MM_DISP_PWM1MM, "mm_disp_pwm1mm", "mm_sel", 2),
> + GATE_MM1(CLK_MM_DISP_PWM126M, "mm_disp_pwm126m", "pwm_sel", 3),
> + GATE_MM1(CLK_MM_DSI0_ENGINE, "mm_dsi0_engine", "mm_sel", 4),
> + GATE_MM1(CLK_MM_DSI0_DIGITAL, "mm_dsi0_digital", "clk_null", 5),
> + GATE_MM1(CLK_MM_DSI1_ENGINE, "mm_dsi1_engine", "mm_sel", 6),
> + GATE_MM1(CLK_MM_DSI1_DIGITAL, "mm_dsi1_digital", "clk_null", 7),
> + GATE_MM1(CLK_MM_DPI_PIXEL, "mm_dpi_pixel", "dpi0_sel", 8),
> + GATE_MM1(CLK_MM_DPI_ENGINE, "mm_dpi_engine", "mm_sel", 9),
> + GATE_MM1(CLK_MM_DPI1_PIXEL, "mm_dpi1_pixel", "clk_null", 10),
> + GATE_MM1(CLK_MM_DPI1_ENGINE, "mm_dpi1_engine", "mm_sel", 11),
> + GATE_MM1(CLK_MM_HDMI_PIXEL, "mm_hdmi_pixel", "dpi0_sel", 12),
> + GATE_MM1(CLK_MM_HDMI_PLLCK, "mm_hdmi_pllck", "hdmi_sel", 13),
> + GATE_MM1(CLK_MM_HDMI_AUDIO, "mm_hdmi_audio", "apll1", 14),
> + GATE_MM1(CLK_MM_HDMI_SPDIF, "mm_hdmi_spdif", "apll2", 15),
> + GATE_MM1(CLK_MM_LVDS_PIXEL, "mm_lvds_pixel", "clk_null", 16),
> + GATE_MM1(CLK_MM_LVDS_CTS, "mm_lvds_cts", "clk_null", 17),
> + GATE_MM1(CLK_MM_SMI_LARB4, "mm_smi_larb4", "mm_sel", 18),
> + GATE_MM1(CLK_MM_HDMI_HDCP, "mm_hdmi_hdcp", "hdcp_sel", 19),
> + GATE_MM1(CLK_MM_HDMI_HDCP24M, "mm_hdmi_hdcp24m", "hdcp_24m_sel", 20),
> +};
> +
> +static struct mtk_gate_regs vdec0_cg_regs = {
> + .set_ofs = 0x0000,
> + .clr_ofs = 0x0004,
> + .sta_ofs = 0x0000,
> +};
> +
> +static struct mtk_gate_regs vdec1_cg_regs = {
> + .set_ofs = 0x0008,
> + .clr_ofs = 0x000c,
> + .sta_ofs = 0x0008,
> +};
> +
> +#define GATE_VDEC0(_id, _name, _parent, _shift) { \
> + .id = _id, \
> + .name = _name, \
> + .parent_name = _parent, \
> + .regs = &vdec0_cg_regs, \
> + .shift = _shift, \
> + .ops = &mtk_clk_gate_ops_setclr_inv, \
> + }
> +
> +#define GATE_VDEC1(_id, _name, _parent, _shift) { \
> + .id = _id, \
> + .name = _name, \
> + .parent_name = _parent, \
> + .regs = &vdec1_cg_regs, \
> + .shift = _shift, \
> + .ops = &mtk_clk_gate_ops_setclr_inv, \
> + }
> +
> +static struct mtk_gate vdec_clks[] __initdata = {
> + GATE_VDEC0(CLK_VDEC_CKEN, "vdec_cken", "vdec_sel", 0),
> + GATE_VDEC1(CLK_VDEC_LARB_CKEN, "vdec_larb_cken", "mm_sel", 0),
> +};
> +
> +static struct mtk_gate_regs venc_cg_regs = {
> + .set_ofs = 0x0004,
> + .clr_ofs = 0x0008,
> + .sta_ofs = 0x0000,
> +};
> +
> +#define GATE_VENC(_id, _name, _parent, _shift) { \
> + .id = _id, \
> + .name = _name, \
> + .parent_name = _parent, \
> + .regs = &venc_cg_regs, \
> + .shift = _shift, \
> + .ops = &mtk_clk_gate_ops_setclr_inv, \
> + }
> +
> +static struct mtk_gate venc_clks[] __initdata = {
> + GATE_VENC(CLK_VENC_CKE0, "venc_cke0", "mm_sel", 0),
> + GATE_VENC(CLK_VENC_CKE1, "venc_cke1", "venc_sel", 4),
> + GATE_VENC(CLK_VENC_CKE2, "venc_cke2", "venc_sel", 8),
> + GATE_VENC(CLK_VENC_CKE3, "venc_cke3", "venc_sel", 12),
> +};
> +
> +static struct mtk_gate_regs venclt_cg_regs = {
> + .set_ofs = 0x0004,
> + .clr_ofs = 0x0008,
> + .sta_ofs = 0x0000,
> +};
> +
> +#define GATE_VENCLT(_id, _name, _parent, _shift) { \
> + .id = _id, \
> + .name = _name, \
> + .parent_name = _parent, \
> + .regs = &venclt_cg_regs, \
> + .shift = _shift, \
> + .ops = &mtk_clk_gate_ops_setclr_inv, \
> + }
> +
> +static struct mtk_gate venclt_clks[] __initdata = {
> + GATE_VENCLT(CLK_VENCLT_CKE0, "venclt_cke0", "mm_sel", 0),
> + GATE_VENCLT(CLK_VENCLT_CKE1, "venclt_cke1", "venclt_sel", 4),
> +};
> +
> static struct clk_onecell_data *mt8173_top_clk_data;
> static struct clk_onecell_data *mt8173_pll_clk_data;
>
> @@ -842,3 +1031,124 @@ static void __init mtk_apmixedsys_init(struct device_node *node)
> }
> CLK_OF_DECLARE(mtk_apmixedsys, "mediatek,mt8173-apmixedsys",
> mtk_apmixedsys_init);
> +
> +static void __init mtk_imgsys_init(struct device_node *node)
> +{
> + struct clk_onecell_data *clk_data;
> + void __iomem *base;
> + int r;
> +
> + base = of_iomap(node, 0);
> + if (!base) {
> + pr_err("%s(): ioremap failed\n", __func__);
> + return;
> + }
> +
> + clk_data = mtk_alloc_clk_data(CLK_IMG_NR_CLK);
> +
> + mtk_clk_register_gates(node, img_clks, ARRAY_SIZE(img_clks),
> + clk_data);
> +
> + r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
> +
> + if (r)
> + pr_err("%s(): could not register clock provider: %d\n",
> + __func__, r);
> +}
> +CLK_OF_DECLARE(mtk_imgsys, "mediatek,mt8173-imgsys", mtk_imgsys_init);
> +
> +static void __init mtk_mmsys_init(struct device_node *node)
> +{
> + struct clk_onecell_data *clk_data;
> + void __iomem *base;
> + int r;
> +
> + base = of_iomap(node, 0);
> + if (!base) {
> + pr_err("%s(): ioremap failed\n", __func__);
> + return;
> + }
> +
> + clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK);
> +
> + mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks),
> + clk_data);
> +
> + r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
> + if (r)
> + pr_err("%s(): could not register clock provider: %d\n",
> + __func__, r);
> +}
> +CLK_OF_DECLARE(mtk_mmsys, "mediatek,mt8173-mmsys", mtk_mmsys_init);
> +
> +static void __init mtk_vdecsys_init(struct device_node *node)
> +{
> + struct clk_onecell_data *clk_data;
> + void __iomem *base;
> + int r;
> +
> + base = of_iomap(node, 0);
> + if (!base) {
> + pr_err("%s(): ioremap failed\n", __func__);
> + return;
> + }
> +
> + clk_data = mtk_alloc_clk_data(CLK_VDEC_NR_CLK);
> +
> + mtk_clk_register_gates(node, vdec_clks, ARRAY_SIZE(vdec_clks),
> + clk_data);
> +
> + r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
> + if (r)
> + pr_err("%s(): could not register clock provider: %d\n",
> + __func__, r);
> +}
> +CLK_OF_DECLARE(mtk_vdecsys, "mediatek,mt8173-vdecsys", mtk_vdecsys_init);
> +
> +static void __init mtk_vencsys_init(struct device_node *node)
> +{
> + struct clk_onecell_data *clk_data;
> + void __iomem *base;
> + int r;
> +
> + base = of_iomap(node, 0);
> + if (!base) {
> + pr_err("%s(): ioremap failed\n", __func__);
> + return;
> + }
> +
> + clk_data = mtk_alloc_clk_data(CLK_VENC_NR_CLK);
> +
> + mtk_clk_register_gates(node, venc_clks, ARRAY_SIZE(venc_clks),
> + clk_data);
> +
> + r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
> + if (r)
> + pr_err("%s(): could not register clock provider: %d\n",
> + __func__, r);
> +}
> +CLK_OF_DECLARE(mtk_vencsys, "mediatek,mt8173-vencsys", mtk_vencsys_init);
> +
> +static void __init mtk_vencltsys_init(struct device_node *node)
> +{
> + struct clk_onecell_data *clk_data;
> + void __iomem *base;
> + int r;
> +
> + base = of_iomap(node, 0);
> + if (!base) {
> + pr_err("%s(): ioremap failed\n", __func__);
> + return;
> + }
> +
> + clk_data = mtk_alloc_clk_data(CLK_VENCLT_NR_CLK);
> +
> + mtk_clk_register_gates(node, venclt_clks, ARRAY_SIZE(venclt_clks),
> + clk_data);
> +
> + r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
> + if (r)
> + pr_err("%s(): could not register clock provider: %d\n",
> + __func__, r);
> +}
> +CLK_OF_DECLARE(mtk_vencltsys, "mediatek,mt8173-vencltsys", mtk_vencltsys_init);
> diff --git a/include/dt-bindings/clock/mt8173-clk.h b/include/dt-bindings/clock/mt8173-clk.h
> index 4ad76ed..94977e6 100644
> --- a/include/dt-bindings/clock/mt8173-clk.h
> +++ b/include/dt-bindings/clock/mt8173-clk.h
> @@ -232,4 +232,91 @@
> #define CLK_PERI_UART3_SEL 39
> #define CLK_PERI_NR_CLK 40
>
> +/* IMG_SYS */
> +
> +#define CLK_IMG_LARB2_SMI 1
> +#define CLK_IMG_CAM_SMI 2
> +#define CLK_IMG_CAM_CAM 3
> +#define CLK_IMG_SEN_TG 4
> +#define CLK_IMG_SEN_CAM 5
> +#define CLK_IMG_CAM_SV 6
> +#define CLK_IMG_FD 7
> +#define CLK_IMG_NR_CLK 8
> +
> +/* MM_SYS */
> +
> +#define CLK_MM_SMI_COMMON 1
> +#define CLK_MM_SMI_LARB0 2
> +#define CLK_MM_CAM_MDP 3
> +#define CLK_MM_MDP_RDMA0 4
> +#define CLK_MM_MDP_RDMA1 5
> +#define CLK_MM_MDP_RSZ0 6
> +#define CLK_MM_MDP_RSZ1 7
> +#define CLK_MM_MDP_RSZ2 8
> +#define CLK_MM_MDP_TDSHP0 9
> +#define CLK_MM_MDP_TDSHP1 10
> +#define CLK_MM_MDP_WDMA 11
> +#define CLK_MM_MDP_WROT0 12
> +#define CLK_MM_MDP_WROT1 13
> +#define CLK_MM_FAKE_ENG 14
> +#define CLK_MM_MUTEX_32K 15
> +#define CLK_MM_DISP_OVL0 16
> +#define CLK_MM_DISP_OVL1 17
> +#define CLK_MM_DISP_RDMA0 18
> +#define CLK_MM_DISP_RDMA1 19
> +#define CLK_MM_DISP_RDMA2 20
> +#define CLK_MM_DISP_WDMA0 21
> +#define CLK_MM_DISP_WDMA1 22
> +#define CLK_MM_DISP_COLOR0 23
> +#define CLK_MM_DISP_COLOR1 24
> +#define CLK_MM_DISP_AAL 25
> +#define CLK_MM_DISP_GAMMA 26
> +#define CLK_MM_DISP_UFOE 27
> +#define CLK_MM_DISP_SPLIT0 28
> +#define CLK_MM_DISP_SPLIT1 29
> +#define CLK_MM_DISP_MERGE 30
> +#define CLK_MM_DISP_OD 31
> +#define CLK_MM_DISP_PWM0MM 32
> +#define CLK_MM_DISP_PWM026M 33
> +#define CLK_MM_DISP_PWM1MM 34
> +#define CLK_MM_DISP_PWM126M 35
> +#define CLK_MM_DSI0_ENGINE 36
> +#define CLK_MM_DSI0_DIGITAL 37
> +#define CLK_MM_DSI1_ENGINE 38
> +#define CLK_MM_DSI1_DIGITAL 39
> +#define CLK_MM_DPI_PIXEL 40
> +#define CLK_MM_DPI_ENGINE 41
> +#define CLK_MM_DPI1_PIXEL 42
> +#define CLK_MM_DPI1_ENGINE 43
> +#define CLK_MM_HDMI_PIXEL 44
> +#define CLK_MM_HDMI_PLLCK 45
> +#define CLK_MM_HDMI_AUDIO 46
> +#define CLK_MM_HDMI_SPDIF 47
> +#define CLK_MM_LVDS_PIXEL 48
> +#define CLK_MM_LVDS_CTS 49
> +#define CLK_MM_SMI_LARB4 50
> +#define CLK_MM_HDMI_HDCP 51
> +#define CLK_MM_HDMI_HDCP24M 52
> +#define CLK_MM_NR_CLK 53
> +
> +/* VDEC_SYS */
> +
> +#define CLK_VDEC_CKEN 1
> +#define CLK_VDEC_LARB_CKEN 2
> +#define CLK_VDEC_NR_CLK 3
> +
> +/* VENC_SYS */
> +
> +#define CLK_VENC_CKE0 1
> +#define CLK_VENC_CKE1 2
> +#define CLK_VENC_CKE2 3
> +#define CLK_VENC_CKE3 4
> +#define CLK_VENC_NR_CLK 5
> +
> +/* VENCLT_SYS */
> +
> +#define CLK_VENCLT_CKE0 1
> +#define CLK_VENCLT_CKE1 2
> +#define CLK_VENCLT_NR_CLK 3
> +
> #endif /* _DT_BINDINGS_CLK_MT8173_H */
> --
> 1.8.1.1.dirty
>
Hi Daniel,
On Fri, 2015-05-22 at 12:22 +0800, Daniel Kurtz wrote:
> On Thu, May 21, 2015 at 3:12 PM, James Liao <[email protected]> wrote:
> > Most multimedia subsystem clocks will be accessed by multiple
> > drivers, so it's a better way to manage these clocks in CCF.
> > This patch adds clock support for MM, IMG, VDEC, VENC and VENC_LT
> > subsystems.
>
> Is there a reason why this patch (or patch set) does not also include
> definitions for the SCP_SYS, AUDIO and MFG_SYS clocks?
Clocks of SCP_SYS is a workaround in our previous internal
implementation. It should be replaced with new mtk-scpsys driver which
implemented in pm_domain framework because they are used to power on/off
subsystem domains.
I had discussed with Sascha, and he said it not worth the overhead of
CCF for clocks that are not shared by different units. As I know AUDIO
and MFG clocks are only used by their own drivers. So I don't include
them in this patch.
Best regards,
James
On Thu, May 21, 2015 at 03:12:52PM +0800, James Liao wrote:
> The size of clk_data should be the same as CLK_APMIXED_NR_CLK
> instead of ARRAY_SIZE(plls). CLK_APMIXED_* is numbered from 1, so
> CLK_APMIXED_NR_CLK will be greater than ARRAY_SIZE(plls).
>
> Signed-off-by: James Liao <[email protected]>
Acked-by: Sascha Hauer <[email protected]>
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Thu, May 21, 2015 at 03:12:53PM +0800, James Liao wrote:
> From: Sascha Hauer <[email protected]>
>
> On the MT8173 the clocks are provided by different units. To enable
> the critical clocks we must be sure that all parent clocks are already
> registered, otherwise the parents of the critical clocks end up being
> unused and get disabled later. To find a place where all parents are
> registered we try each time after we've registered some clocks if
> all known providers are present now and only then we enable the critical
> clocks
>
> Signed-off-by: Sascha Hauer <[email protected]>
> Signed-off-by: James Liao <[email protected]>
> ---
> drivers/clk/mediatek/clk-mt8173.c | 24 +++++++++++++++++++-----
> 1 file changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c
> index 4b9e04c..eb175ac 100644
> --- a/drivers/clk/mediatek/clk-mt8173.c
> +++ b/drivers/clk/mediatek/clk-mt8173.c
> @@ -700,6 +700,20 @@ static const struct mtk_composite peri_clks[] __initconst = {
> MUX(CLK_PERI_UART3_SEL, "uart3_ck_sel", uart_ck_sel_parents, 0x40c, 3, 1),
> };
>
> +static struct clk_onecell_data *mt8173_top_clk_data;
> +static struct clk_onecell_data *mt8173_pll_clk_data;
> +
> +static void mtk_clk_enable_critical(void)
> +{
> + if (!mt8173_top_clk_data || !mt8173_pll_clk_data)
> + return;
> +
> + clk_prepare_enable(mt8173_top_clk_data->clks[CLK_TOP_MEM_SEL]);
> + clk_prepare_enable(mt8173_top_clk_data->clks[CLK_TOP_DDRPHYCFG_SEL]);
> + clk_prepare_enable(mt8173_top_clk_data->clks[CLK_TOP_CCI400_SEL]);
> + clk_prepare_enable(mt8173_top_clk_data->clks[CLK_TOP_RTC_SEL]);
Is CLK_TOP_RTC_SEL really a critical clock?
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Thu, May 21, 2015 at 03:12:54PM +0800, James Liao wrote:
> This adds the binding documentation for the mmsys, imgsys, vdecsys,
> vencsys and vencltsys controllers found on Mediatek SoCs.
>
> index 0000000..a5b94a7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,vdecsys.txt
> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,vencltsys.txt
> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,vencsys.txt
Do these really become multiple drivers so that it's worth abstracting
them in the clock framework?
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Thu, May 21, 2015 at 03:12:56PM +0800, James Liao wrote:
> Add REF2USB_TX clock support into MT8173 APMIXEDSYS. This clock
> is needed by USB 3.0.
>
> +
> +static struct clk *mtk_clk_register_ref2usb_tx(const char *name,
> + void __iomem *reg)
> +{
> + struct mtk_ref2usb_tx *tx;
> + struct clk_init_data init = {};
> + struct clk *clk;
> + const char *parent_name = "clk26m";
> +
> + tx = kzalloc(sizeof(*tx), GFP_KERNEL);
> + if (!tx)
> + return ERR_PTR(-ENOMEM);
> +
> + tx->base_addr = reg;
> + tx->hw.init = &init;
> +
> + init.name = name;
> + init.ops = &mtk_ref2usb_tx_ops;
> + init.parent_names = &parent_name;
> + init.num_parents = 1;
> +
> + clk = clk_register(NULL, &tx->hw);
> +
> + if (IS_ERR(clk)) {
> + pr_err("Failed to register clk %s: %ld\n", name, PTR_ERR(clk));
> + kfree(tx);
> + }
> +
> + return clk;
> +}
> +
> +static void __init mtk_clk_register_apmixedsys_special(struct device_node *node,
> + struct clk_onecell_data *clk_data)
> +{
> + void __iomem *base;
> + struct clk *clk;
> +
> + base = of_iomap(node, 0);
> + if (!base) {
> + pr_err("%s(): ioremap failed\n", __func__);
> + return;
> + }
> +
> + clk = mtk_clk_register_ref2usb_tx("ref2usb_tx", base + 0x8);
The function seems to be for one special clock only. Why do you pass the
name to it? They will never be called with another name, right?
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Tue, 2015-05-26 at 09:46 +0200, Sascha Hauer wrote:
> > +static struct clk_onecell_data *mt8173_top_clk_data;
> > +static struct clk_onecell_data *mt8173_pll_clk_data;
> > +
> > +static void mtk_clk_enable_critical(void)
> > +{
> > + if (!mt8173_top_clk_data || !mt8173_pll_clk_data)
> > + return;
> > +
> > + clk_prepare_enable(mt8173_top_clk_data->clks[CLK_TOP_MEM_SEL]);
> > + clk_prepare_enable(mt8173_top_clk_data->clks[CLK_TOP_DDRPHYCFG_SEL]);
> > + clk_prepare_enable(mt8173_top_clk_data->clks[CLK_TOP_CCI400_SEL]);
> > + clk_prepare_enable(mt8173_top_clk_data->clks[CLK_TOP_RTC_SEL]);
>
> Is CLK_TOP_RTC_SEL really a critical clock?
CLK_TOP_RTC_SEL is the main 32k clock used by some system hardware such
sleep controller on MT8173. This clock should not be turned off even
when software/CPU is sleeping. So it's a better way to set
CLK_TOP_RTC_SEL as a critical clock (an always on clock).
Best regards,
James
Hi Sascha,
On Tue, 2015-05-26 at 09:56 +0200, Sascha Hauer wrote:
> On Thu, May 21, 2015 at 03:12:54PM +0800, James Liao wrote:
> > This adds the binding documentation for the mmsys, imgsys, vdecsys,
> > vencsys and vencltsys controllers found on Mediatek SoCs.
> >
> > index 0000000..a5b94a7
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,vdecsys.txt
> > +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,vencltsys.txt
> > +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,vencsys.txt
>
> Do these really become multiple drivers so that it's worth abstracting
> them in the clock framework?
These clocks need to be controlled among several drivers. For example,
vdecsys clocks will be controlled by VDEC driver (not ready yet) and
MT8173 SMI driver [1]. That means these clocks need a mechanism to share
between these 2 drivers. CCF share clocks by using of reference count,
so I think it's suitable to implement these subsystem clocks.
As I know SMI driver need to access clocks among mmsys, imgsys, vdecsys,
vencsys and vencltsys. So in this patch I added clocks of these
subsystems into CCF.
[1]
http://lists.infradead.org/pipermail/linux-mediatek/2015-March/000058.html
Best regards,
James
Hi Sascha,
On Tue, 2015-05-26 at 10:05 +0200, Sascha Hauer wrote:
> On Thu, May 21, 2015 at 03:12:56PM +0800, James Liao wrote:
> > +static void __init mtk_clk_register_apmixedsys_special(struct device_node *node,
> > + struct clk_onecell_data *clk_data)
> > +{
> > + void __iomem *base;
> > + struct clk *clk;
> > +
> > + base = of_iomap(node, 0);
> > + if (!base) {
> > + pr_err("%s(): ioremap failed\n", __func__);
> > + return;
> > + }
> > +
> > + clk = mtk_clk_register_ref2usb_tx("ref2usb_tx", base + 0x8);
>
> The function seems to be for one special clock only. Why do you pass the
> name to it? They will never be called with another name, right?
This function decides clock name and associates clock ID for special
clocks. In fact there may be another "special clocks" need to add into
apmixedsys. I think it's a better way to group clock names and clock IDs
in the same function for maintenance.
Best regards,
James
On Tue, May 26, 2015 at 05:11:15PM +0800, James Liao wrote:
> Hi Sascha,
>
> On Tue, 2015-05-26 at 10:05 +0200, Sascha Hauer wrote:
> > On Thu, May 21, 2015 at 03:12:56PM +0800, James Liao wrote:
> > > +static void __init mtk_clk_register_apmixedsys_special(struct device_node *node,
> > > + struct clk_onecell_data *clk_data)
> > > +{
> > > + void __iomem *base;
> > > + struct clk *clk;
> > > +
> > > + base = of_iomap(node, 0);
> > > + if (!base) {
> > > + pr_err("%s(): ioremap failed\n", __func__);
> > > + return;
> > > + }
> > > +
> > > + clk = mtk_clk_register_ref2usb_tx("ref2usb_tx", base + 0x8);
> >
> > The function seems to be for one special clock only. Why do you pass the
> > name to it? They will never be called with another name, right?
>
> This function decides clock name and associates clock ID for special
> clocks. In fact there may be another "special clocks" need to add into
> apmixedsys. I think it's a better way to group clock names and clock IDs
> in the same function for maintenance.
How can a function with ref2usb_tx in its name ever register a clock
with another name? Then it seems the function name is wrong.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Hi Sascha,
On Tue, 2015-05-26 at 11:41 +0200, Sascha Hauer wrote:
> On Tue, May 26, 2015 at 05:11:15PM +0800, James Liao wrote:
> > On Tue, 2015-05-26 at 10:05 +0200, Sascha Hauer wrote:
> > > On Thu, May 21, 2015 at 03:12:56PM +0800, James Liao wrote:
> > > > +static void __init mtk_clk_register_apmixedsys_special(struct device_node *node,
> > > > + struct clk_onecell_data *clk_data)
> > > > +{
> > > > + void __iomem *base;
> > > > + struct clk *clk;
> > > > +
> > > > + base = of_iomap(node, 0);
> > > > + if (!base) {
> > > > + pr_err("%s(): ioremap failed\n", __func__);
> > > > + return;
> > > > + }
> > > > +
> > > > + clk = mtk_clk_register_ref2usb_tx("ref2usb_tx", base + 0x8);
> > >
> > > The function seems to be for one special clock only. Why do you pass the
> > > name to it? They will never be called with another name, right?
> >
> > This function decides clock name and associates clock ID for special
> > clocks. In fact there may be another "special clocks" need to add into
> > apmixedsys. I think it's a better way to group clock names and clock IDs
> > in the same function for maintenance.
>
> How can a function with ref2usb_tx in its name ever register a clock
> with another name? Then it seems the function name is wrong.
I mean mtk_clk_register_apmixedsys_special() decides the clock names and
clock ID bindings.
mtk_clk_register_ref2usb_tx() is only used by ref2usb_tx clock. Other
special clocks will not share the implementation with ref2usb_tx. But we
can set names and clock IDs for different special clocks in the same
function (mtk_clk_register_apmixedsys_special()) to avoid inconsistence
naming.
Best regards,
James
On Tue, May 26, 2015 at 04:55:36PM +0800, James Liao wrote:
> Hi Sascha,
>
> On Tue, 2015-05-26 at 09:56 +0200, Sascha Hauer wrote:
> > On Thu, May 21, 2015 at 03:12:54PM +0800, James Liao wrote:
> > > This adds the binding documentation for the mmsys, imgsys, vdecsys,
> > > vencsys and vencltsys controllers found on Mediatek SoCs.
> > >
> > > index 0000000..a5b94a7
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,vdecsys.txt
> > > +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,vencltsys.txt
> > > +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,vencsys.txt
> >
> > Do these really become multiple drivers so that it's worth abstracting
> > them in the clock framework?
>
> These clocks need to be controlled among several drivers. For example,
> vdecsys clocks will be controlled by VDEC driver (not ready yet) and
> MT8173 SMI driver [1]. That means these clocks need a mechanism to share
> between these 2 drivers. CCF share clocks by using of reference count,
> so I think it's suitable to implement these subsystem clocks.
>
> As I know SMI driver need to access clocks among mmsys, imgsys, vdecsys,
> vencsys and vencltsys. So in this patch I added clocks of these
> subsystems into CCF.
>
> [1]
> http://lists.infradead.org/pipermail/linux-mediatek/2015-March/000058.html
Looking at the 3.18 tree we have this:
vdecsys: vdecsys@16000000 {
compatible = "mediatek,mt8173-vdecsys", "syscon";
reg = <0 0x16000000 0 0x1000>;
#clock-cells = <1>;
};
larb1:larb@16010000 {
compatible = "mediatek,mt8173-smi-larb";
reg = <0 0x16010000 0 0x1000>;
clocks = <&mmsys MM_SMI_COMMON>,
<&vdecsys VDEC_CKEN>,
<&vdecsys VDEC_LARB_CKEN>;
clock-names = "larb_sub0", "larb_sub1", "larb_sub2";
};
I believe that the larb needs the MM_SMI_COMMON clock to modify the larb
registers, but is it really necessary to enable VDEC_CKEN and
VDEC_LARB_CKEN just to set the F_SMI_MMU_EN bit in the larb?
With the above we have the situation that the vdec driver calls into the
iommu driver which then calls into the larb driver which calls back into
the vdec driver via the clk API. This seems very suspicious.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On 05/29, Sascha Hauer wrote:
> On Fri, May 29, 2015 at 10:47:29AM +0800, James Liao wrote:
> > Hi Sascha,
> >
> > > And really the driver matching "mediatek,mt8173-vencsys" should register
> > > the necessary clocks and reset lines and call of_platform_populate on
> > > the subnodes. The driver should also be a real driver, not something
> > > matched by CLK_OF_DECLARE. The "mediatek,mt8173-vencsys" driver now has
> > > the possibility to manage the toplevel vencsys unit, do runtime pm, turn
> > > the whole thing off and on. Using CCF for abstracting these clocks may
> > > be the right thing, but I believe that we should keep the code for the
> > > toplevel vencsys register space together in a single file and not put
> > > the clk bits in drivers/clk/mediatek/mt8173.c, the reset bits in
> > > drivers/reset/ and the remaining misc stuff in drivers/soc/mediatek/.
> > >
> > > So I think we should have a drivers/soc/mediatek/mtk-vencsys.c which
> > > is a regular driver, calls clk_register() for its clocks, calls
> > > reset_controller_register() for the reset bits, provides plain functions
> > > for the remaining bits which are not handled by any Linux framework.
> > > Finally of_platform_populate will register the child devices.
> > >
> > > I showed this using the vencsys example, but it's the same for vdecsys,
> > > vencltsys, imgsys and mmsys.
> >
> > So you agree to manage these subsystem clocks in CCF, but they should be
> > provided by their own (globalcon) drivers, right?
>
> Yes. I previously got the impression that the subsystem clocks are not
> directly associated to the larbs, but needed to be handled by the larb
> code due to some side effect. Now that I saw that the larbs are directly
> in the subsystem register space it all makes sense.
>
> Note that the way Mediatek SoCs are designed around sub modules is bit
> unusual and does not fit very well in the Linux directory structure.
> Normally SoCs have a single clocks controller which controls all clocks
> in the SoC. Then you often have a reset controller providing reset lines
> in the SoC. In this case it's clear that the clk driver goes to
> drivers/clk/, the reset controller driver to drivers/reset/. Mediatek
> SoCs instead have several blocks, each with its own clock and reset
> controller. Splitting each block up into parts in drivers/clk/ and
> drivers/reset/ leads to quite a code fragmentation.
> This is my opinion, it would be great to hear something from others.
> Matthias? I'd like to avoid running into a direction that is not
> acceptable in the end.
We already have drivers registering clocks and resets under
drivers/clk, so it's not unheard of. An alternative solution is
to make child devices for the clock part and the reset part at
runtime in the toplevel driver for the vencsys device (don't do
any sort of DT description for this) and use regmap to mediate
the register accesses and locking. That way we can put the clk
driver in drivers/clk/, the reset driver in drivers/reset, etc.
so that logically related code is grouped.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On 05/21, James Liao wrote:
> The size of clk_data should be the same as CLK_APMIXED_NR_CLK
> instead of ARRAY_SIZE(plls). CLK_APMIXED_* is numbered from 1, so
> CLK_APMIXED_NR_CLK will be greater than ARRAY_SIZE(plls).
>
> Signed-off-by: James Liao <[email protected]>
> ---
Applied to clk-next
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
Hi Stephen,
On Thu, 2015-06-04 at 14:02 -0700, Stephen Boyd wrote:
> On 05/29, Sascha Hauer wrote:
> > Yes. I previously got the impression that the subsystem clocks are not
> > directly associated to the larbs, but needed to be handled by the larb
> > code due to some side effect. Now that I saw that the larbs are directly
> > in the subsystem register space it all makes sense.
> >
> > Note that the way Mediatek SoCs are designed around sub modules is bit
> > unusual and does not fit very well in the Linux directory structure.
> > Normally SoCs have a single clocks controller which controls all clocks
> > in the SoC. Then you often have a reset controller providing reset lines
> > in the SoC. In this case it's clear that the clk driver goes to
> > drivers/clk/, the reset controller driver to drivers/reset/. Mediatek
> > SoCs instead have several blocks, each with its own clock and reset
> > controller. Splitting each block up into parts in drivers/clk/ and
> > drivers/reset/ leads to quite a code fragmentation.
> > This is my opinion, it would be great to hear something from others.
> > Matthias? I'd like to avoid running into a direction that is not
> > acceptable in the end.
>
> We already have drivers registering clocks and resets under
> drivers/clk, so it's not unheard of. An alternative solution is
> to make child devices for the clock part and the reset part at
> runtime in the toplevel driver for the vencsys device (don't do
> any sort of DT description for this) and use regmap to mediate
> the register accesses and locking. That way we can put the clk
> driver in drivers/clk/, the reset driver in drivers/reset, etc.
> so that logically related code is grouped.
I have a question about the alternative way you mentioned. Currently
clock providers and consumers describe what clocks they will provide /
consume in device tree. If we don't describe vencsys clocks in device
tree, how to get vencsys clocks for drivers that need to control them?
Best regards,
James
On 06/05, James Liao wrote:
> Hi Stephen,
>
> On Thu, 2015-06-04 at 14:02 -0700, Stephen Boyd wrote:
> > On 05/29, Sascha Hauer wrote:
> > > Yes. I previously got the impression that the subsystem clocks are not
> > > directly associated to the larbs, but needed to be handled by the larb
> > > code due to some side effect. Now that I saw that the larbs are directly
> > > in the subsystem register space it all makes sense.
> > >
> > > Note that the way Mediatek SoCs are designed around sub modules is bit
> > > unusual and does not fit very well in the Linux directory structure.
> > > Normally SoCs have a single clocks controller which controls all clocks
> > > in the SoC. Then you often have a reset controller providing reset lines
> > > in the SoC. In this case it's clear that the clk driver goes to
> > > drivers/clk/, the reset controller driver to drivers/reset/. Mediatek
> > > SoCs instead have several blocks, each with its own clock and reset
> > > controller. Splitting each block up into parts in drivers/clk/ and
> > > drivers/reset/ leads to quite a code fragmentation.
> > > This is my opinion, it would be great to hear something from others.
> > > Matthias? I'd like to avoid running into a direction that is not
> > > acceptable in the end.
> >
> > We already have drivers registering clocks and resets under
> > drivers/clk, so it's not unheard of. An alternative solution is
> > to make child devices for the clock part and the reset part at
> > runtime in the toplevel driver for the vencsys device (don't do
> > any sort of DT description for this) and use regmap to mediate
> > the register accesses and locking. That way we can put the clk
> > driver in drivers/clk/, the reset driver in drivers/reset, etc.
> > so that logically related code is grouped.
>
> I have a question about the alternative way you mentioned. Currently
> clock providers and consumers describe what clocks they will provide /
> consume in device tree. If we don't describe vencsys clocks in device
> tree, how to get vencsys clocks for drivers that need to control them?
>
Perhaps an example would be best. In DT we would have:
vencsys: vencsys@10000 {
compatible = "mtk,vencsys";
reg = <0x10000 0x1000>;
#clock-cells = <1>;
#reset-cells = <1>;
};
myconsumer@12000 {
compatible = "mtk,vencsys";
reg = <0x12000 0x100>;
clocks = <&vencsys 10>;
clock-names = "core";
};
(Or are the consumers only children of the subsystem?
It's not clear to me)
And then in the mtk,vencsys driver we would create a platform
device named something like "mtk-vencsys-clk" and assign the
of_node of the device to be the of_node that is assigned to the
mtk,vencsys device.
static int vencsys_probe(struct platform_device *pdev)
{
int ret;
struct device_node *np = pdev->dev.of_node;
struct platform_device *clk_pdev;
clk_pdev = platform_device_alloc("mtk-vencsys-clk", -1);
clk_pdev->dev.of_node = of_node;
ret = platform_device_add(clk_pdev);
if (ret)
return ret;
}
Then we could put a mtk-vencsys-clk driver in drivers/clk/ that
does the clk driver part...
static int clk_vencsys_probe(struct platform_device *pdev)
{
int ret;
struct device_node *np = pdev->dev.of_node;
struct regmap *regmap;
ret = of_clk_add_provider(np, of_clk_src_onecell_get, ..);
if (ret)
return ret;
regmap = dev_get_regmap(pdev->dev.parent, NULL);
}
And similar things could be done for the reset driver.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
Hi Stephen,
On Fri, 2015-06-05 at 17:59 -0700, Stephen Boyd wrote:
> Perhaps an example would be best. In DT we would have:
>
> vencsys: vencsys@10000 {
> compatible = "mtk,vencsys";
> reg = <0x10000 0x1000>;
> #clock-cells = <1>;
> #reset-cells = <1>;
> };
>
> myconsumer@12000 {
> compatible = "mtk,vencsys";
> reg = <0x12000 0x100>;
> clocks = <&vencsys 10>;
> clock-names = "core";
> };
>
> (Or are the consumers only children of the subsystem?
> It's not clear to me)
In our case the vencsys clocks may use used by its own driver and other
drivers (such as SMI driver).
> And then in the mtk,vencsys driver we would create a platform
> device named something like "mtk-vencsys-clk" and assign the
> of_node of the device to be the of_node that is assigned to the
> mtk,vencsys device.
>
> static int vencsys_probe(struct platform_device *pdev)
> {
> int ret;
> struct device_node *np = pdev->dev.of_node;
> struct platform_device *clk_pdev;
>
> clk_pdev = platform_device_alloc("mtk-vencsys-clk", -1);
> clk_pdev->dev.of_node = of_node;
> ret = platform_device_add(clk_pdev);
> if (ret)
> return ret;
> }
>
> Then we could put a mtk-vencsys-clk driver in drivers/clk/ that
> does the clk driver part...
>
> static int clk_vencsys_probe(struct platform_device *pdev)
> {
> int ret;
> struct device_node *np = pdev->dev.of_node;
> struct regmap *regmap;
>
> ret = of_clk_add_provider(np, of_clk_src_onecell_get, ..);
> if (ret)
> return ret;
>
> regmap = dev_get_regmap(pdev->dev.parent, NULL);
>
> }
>
> And similar things could be done for the reset driver.
>
It looks like there is only one DT node (vencsys@10000) but there are
several drivers refer to this DT node, and then we use a dummy driver
(or a "top driver") to aggregate drivers with different features (clk,
reset etc.).
It means we need to provide vencsys clk driver in drivers/clk and
vencsys reset driver in drivers/reset, and also to provide a vencsys top
driver in, for example, soc/mediatek, right?
Now we have 3 choice:
1. implement vencsys clock driver and reset driver in drivers/clk.
2. implement vencsys clock driver and reset driver in drivers/soc.
3. implement vencsys clock driver in drivers/clk, vencsys reset driver
in drivers/reset, and vencsys top driver in drivers/soc.
Sascha, do you have comments for these 3 solutions?
Best regards,
James
On Fri, Jun 05, 2015 at 05:59:12PM -0700, Stephen Boyd wrote:
> On 06/05, James Liao wrote:
> > Hi Stephen,
> >
> > On Thu, 2015-06-04 at 14:02 -0700, Stephen Boyd wrote:
> > > On 05/29, Sascha Hauer wrote:
> > > > Yes. I previously got the impression that the subsystem clocks are not
> > > > directly associated to the larbs, but needed to be handled by the larb
> > > > code due to some side effect. Now that I saw that the larbs are directly
> > > > in the subsystem register space it all makes sense.
> > > >
> > > > Note that the way Mediatek SoCs are designed around sub modules is bit
> > > > unusual and does not fit very well in the Linux directory structure.
> > > > Normally SoCs have a single clocks controller which controls all clocks
> > > > in the SoC. Then you often have a reset controller providing reset lines
> > > > in the SoC. In this case it's clear that the clk driver goes to
> > > > drivers/clk/, the reset controller driver to drivers/reset/. Mediatek
> > > > SoCs instead have several blocks, each with its own clock and reset
> > > > controller. Splitting each block up into parts in drivers/clk/ and
> > > > drivers/reset/ leads to quite a code fragmentation.
> > > > This is my opinion, it would be great to hear something from others.
> > > > Matthias? I'd like to avoid running into a direction that is not
> > > > acceptable in the end.
> > >
> > > We already have drivers registering clocks and resets under
> > > drivers/clk, so it's not unheard of. An alternative solution is
> > > to make child devices for the clock part and the reset part at
> > > runtime in the toplevel driver for the vencsys device (don't do
> > > any sort of DT description for this) and use regmap to mediate
> > > the register accesses and locking. That way we can put the clk
> > > driver in drivers/clk/, the reset driver in drivers/reset, etc.
> > > so that logically related code is grouped.
> >
> > I have a question about the alternative way you mentioned. Currently
> > clock providers and consumers describe what clocks they will provide /
> > consume in device tree. If we don't describe vencsys clocks in device
> > tree, how to get vencsys clocks for drivers that need to control them?
> >
>
> Perhaps an example would be best. In DT we would have:
>
> vencsys: vencsys@10000 {
> compatible = "mtk,vencsys";
> reg = <0x10000 0x1000>;
> #clock-cells = <1>;
> #reset-cells = <1>;
> };
>
> myconsumer@12000 {
> compatible = "mtk,vencsys";
> reg = <0x12000 0x100>;
> clocks = <&vencsys 10>;
> clock-names = "core";
> };
>
> (Or are the consumers only children of the subsystem?
> It's not clear to me)
>
> And then in the mtk,vencsys driver we would create a platform
> device named something like "mtk-vencsys-clk" and assign the
> of_node of the device to be the of_node that is assigned to the
> mtk,vencsys device.
>
> static int vencsys_probe(struct platform_device *pdev)
> {
> int ret;
> struct device_node *np = pdev->dev.of_node;
> struct platform_device *clk_pdev;
>
> clk_pdev = platform_device_alloc("mtk-vencsys-clk", -1);
> clk_pdev->dev.of_node = of_node;
> ret = platform_device_add(clk_pdev);
> if (ret)
> return ret;
> }
>
> Then we could put a mtk-vencsys-clk driver in drivers/clk/ that
> does the clk driver part...
>
> static int clk_vencsys_probe(struct platform_device *pdev)
> {
> int ret;
> struct device_node *np = pdev->dev.of_node;
> struct regmap *regmap;
>
> ret = of_clk_add_provider(np, of_clk_src_onecell_get, ..);
> if (ret)
> return ret;
>
> regmap = dev_get_regmap(pdev->dev.parent, NULL);
>
> }
>
> And similar things could be done for the reset driver.
The problem I see with this approach is that we scatter the code for a
otherwise simple driver over a bunch of directories. We would have
drivers/clk/mediatek/vencsys.c
drivers/reset/mediatek/vencsys.c
drivers/soc/mediatek/vencsys.c
The same must be added for vdecsys, imgsys and vencltsys. That will make
12 drivers and three maintainers for 12 registers. I think this will be
a pain to maintain, hence my suggestion to put the vencsys code into a
single file and not split this up into more subsystem specific files.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On 06/08, Sascha Hauer wrote:
> On Fri, Jun 05, 2015 at 05:59:12PM -0700, Stephen Boyd wrote:
> >
> > And similar things could be done for the reset driver.
>
> The problem I see with this approach is that we scatter the code for a
> otherwise simple driver over a bunch of directories. We would have
>
> drivers/clk/mediatek/vencsys.c
> drivers/reset/mediatek/vencsys.c
> drivers/soc/mediatek/vencsys.c
>
> The same must be added for vdecsys, imgsys and vencltsys. That will make
> 12 drivers and three maintainers for 12 registers. I think this will be
> a pain to maintain, hence my suggestion to put the vencsys code into a
> single file and not split this up into more subsystem specific files.
>
I probably don't have enough information here, but why is it a
pain to maintain? It seems more like a pain to setup the first
time and then little to no pain to maintain because we clearly
split functionality based on subsystem. No merge conflicts, clear
division of functionality, etc. But again, I don't think it
matters much either way given that reset and clk drivers are
combined sometimes and don't always reside in drivers/clk or
drivers/reset either.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On Thursday, June 11, 2015 04:52:12 PM Stephen Boyd wrote:
> On 06/08, Sascha Hauer wrote:
> > On Fri, Jun 05, 2015 at 05:59:12PM -0700, Stephen Boyd wrote:
> > > And similar things could be done for the reset driver.
> >
> > The problem I see with this approach is that we scatter the code for a
> > otherwise simple driver over a bunch of directories. We would have
> >
> > drivers/clk/mediatek/vencsys.c
> > drivers/reset/mediatek/vencsys.c
> > drivers/soc/mediatek/vencsys.c
> >
> > The same must be added for vdecsys, imgsys and vencltsys. That will make
> > 12 drivers and three maintainers for 12 registers. I think this will be
> > a pain to maintain, hence my suggestion to put the vencsys code into a
> > single file and not split this up into more subsystem specific files.
>
> I probably don't have enough information here, but why is it a
> pain to maintain? It seems more like a pain to setup the first
> time and then little to no pain to maintain because we clearly
> split functionality based on subsystem. No merge conflicts, clear
> division of functionality, etc. But again, I don't think it
> matters much either way given that reset and clk drivers are
> combined sometimes and don't always reside in drivers/clk or
> drivers/reset either.
Actually I would prefer to have the clock and resets in drivers/clk, just as
we already did for Mediatek up to now.
On Thursday, May 21, 2015 03:12:55 PM James Liao wrote:
> Most multimedia subsystem clocks will be accessed by multiple
> drivers, so it's a better way to manage these clocks in CCF.
> This patch adds clock support for MM, IMG, VDEC, VENC and VENC_LT
> subsystems.
>
> Signed-off-by: James Liao <[email protected]>
> ---
> drivers/clk/mediatek/clk-mt8173.c | 310
> +++++++++++++++++++++++++++++++++ include/dt-bindings/clock/mt8173-clk.h |
> 87 +++++++++
> 2 files changed, 397 insertions(+)
>
> diff --git a/drivers/clk/mediatek/clk-mt8173.c
> b/drivers/clk/mediatek/clk-mt8173.c index eb175ac..e2f40ba 100644
> --- a/drivers/clk/mediatek/clk-mt8173.c
> +++ b/drivers/clk/mediatek/clk-mt8173.c
> @@ -700,6 +700,195 @@ static const struct mtk_composite peri_clks[]
> __initconst = { MUX(CLK_PERI_UART3_SEL, "uart3_ck_sel",
> uart_ck_sel_parents, 0x40c, 3, 1), };
>
> +static struct mtk_gate_regs img_cg_regs = {
> + .set_ofs = 0x0004,
> + .clr_ofs = 0x0008,
> + .sta_ofs = 0x0000,
> +};
> +
> +#define GATE_IMG(_id, _name, _parent, _shift) { \
> + .id = _id, \
> + .name = _name, \
> + .parent_name = _parent, \
> + .regs = &img_cg_regs, \
> + .shift = _shift, \
> + .ops = &mtk_clk_gate_ops_setclr, \
> + }
> +
[...]
> +
> +static struct mtk_gate_regs venc_cg_regs = {
> + .set_ofs = 0x0004,
> + .clr_ofs = 0x0008,
> + .sta_ofs = 0x0000,
> +};
> +
> +#define GATE_VENC(_id, _name, _parent, _shift) { \
> + .id = _id, \
> + .name = _name, \
> + .parent_name = _parent, \
> + .regs = &venc_cg_regs, \
> + .shift = _shift, \
> + .ops = &mtk_clk_gate_ops_setclr_inv, \
> + }
> +
> +static struct mtk_gate venc_clks[] __initdata = {
> + GATE_VENC(CLK_VENC_CKE0, "venc_cke0", "mm_sel", 0),
> + GATE_VENC(CLK_VENC_CKE1, "venc_cke1", "venc_sel", 4),
> + GATE_VENC(CLK_VENC_CKE2, "venc_cke2", "venc_sel", 8),
> + GATE_VENC(CLK_VENC_CKE3, "venc_cke3", "venc_sel", 12),
> +};
> +
> +static struct mtk_gate_regs venclt_cg_regs = {
> + .set_ofs = 0x0004,
> + .clr_ofs = 0x0008,
> + .sta_ofs = 0x0000,
> +};
The register for imagesys, vencsys and vencltsys have all the same offset.
We could use just one struct for all of them.
Cheers,
Matthias
On Fri, 2015-06-12 at 19:09 +0200, Matthias Brugger wrote:
> On Thursday, May 21, 2015 03:12:55 PM James Liao wrote:
> > +
> > +static struct mtk_gate_regs venclt_cg_regs = {
> > + .set_ofs = 0x0004,
> > + .clr_ofs = 0x0008,
> > + .sta_ofs = 0x0000,
> > +};
>
>
> The register for imagesys, vencsys and vencltsys have all the same offset.
> We could use just one struct for all of them.
OK. I'll merge them in next patch.
Best regards,
James