The Amlogic G12A, G12B & SM1 SoCs embeds a Synopsys DW-MIPI-DSI transceiver (ver 1.21a),
with a custom glue managing the IP resets, clock and data input similar to the DW-HDMI
glue on the same Amlogic SoCs.
This is a follow-up of v5 now the DRM patches are applied, the clk & DT changes
remains for a full DSI support on G12A & SM1 platforms.
The DW-MIPI-DSI transceiver + D-PHY are clocked by the GP0 PLL, and the ENCL encoder + VIU
pixel reader by the VCLK2 clock using the HDMI PLL.
The DW-MIPI-DSI transceiver gets this pixel stream as input clocked with the VCLK2 clock.
An optional "MEAS" clock can be enabled to measure the delay between each vsync feeding the
DW-MIPI-DSI transceiver.
The clock setup has been redesigned to use CCF, a common PLL (GP0) and the VCLK2 clock
path for DSI in preparation of full CCF support and possibly dual display with HDMI.
The change from v5 is that now we use a "VCLK" driver instead of notifier and rely
on CLK_SET_RATE_GATE to ensure the VCLK gate operation are called.
Signed-off-by: Neil Armstrong <[email protected]>
---
Changes in v12:
- fix parameters alignment in patch 2
- update g12a_mipi_dsi_pxclk_div_table comment with jerome's suggestions
- fix dtbs overlay build, fix missed v11... thx khadas for reporting it off-list & testing
- Link to v11: https://lore.kernel.org/r/20240325-amlogic-v6-4-upstream-dsi-ccf-vim3-v11-0-04f55de44604@linaro.org
Changes in v11:
- Rebased on v6.9-rc1
- Fixed overlay handling/creation
- Link to v10: https://lore.kernel.org/r/20240205-amlogic-v6-4-upstream-dsi-ccf-vim3-v10-0-dc06073d5330@linaro.org
Changes in v10:
- Rename regmap_vclk to meson_clk and add _gate for the gate
- Move COMMON_CLK_MESON_VCLK to following patch
- Remove CLK_SET_RATE_PARENT from g12a_vclk2_sel, keep it only on mipi_dsi_pxclk_sel
- Add more info on commit message to specify how clock setup is designed
- Remove forgotten CLK_IGNORE_UNUSED on g12a_vclk2_input
- Remove useless CLK_SET_RATE_PARENT on g12a_vclk2_div to stop propagatting rate _after_ vclk2_div
- Remove invalid CLK_SET_RATE_GATE on g12a_vclk2 since it's not a divider...
- Drop already applied patches
- move Khadas TS050 changes as an overlay
- Link to v9: https://lore.kernel.org/r/20231124-amlogic-v6-4-upstream-dsi-ccf-vim3-v9-0-95256ed139e6@linaro.org
Changes in v9:
- Colledte reviewed-bys
- Fixed patches 2 & 4, commit messages and bindings format
- Link to v8: https://lore.kernel.org/r/20231109-amlogic-v6-4-upstream-dsi-ccf-vim3-v8-0-81e4aeeda193@linaro.org
Changes in v8:
- Switch vclk clk driver to parm as requested by Jerome
- Added bindings fixes to amlogic,meson-axg-mipi-pcie-analog & amlogic,g12a-mipi-dphy-analog
- Fixed DT errors in vim3 example and MNT Reform DT
- Rebased on next-20231107, successfully tested on VIM3L
- Link to v7: https://lore.kernel.org/r/20230803-amlogic-v6-4-upstream-dsi-ccf-vim3-v7-0-762219fc5b28@linaro.org
Changes in v7:
- Added review tags
- Fixed patch 5 thanks to George
- Link to v6: https://lore.kernel.org/r/20230512-amlogic-v6-4-upstream-dsi-ccf-vim3-v6-0-fd2ac9845472@linaro.org
Changes in v6:
- dropped applied DRM patches
- dropped clk private prefix patches
- rebased on top of 20230607-topic-amlogic-upstream-clkid-public-migration-v2-0-38172d17c27a@linaro.org
- re-ordered/cleaned ENCL patches to match clkid public migration
- Added new "vclk" driver
- uses vclk driver instead of notifier
- cleaned VCLK2 clk flags
- add px_clk gating from DSI driver
- Link to v5: https://lore.kernel.org/r/20230512-amlogic-v6-4-upstream-dsi-ccf-vim3-v5-0-56eb7a4d5b8e@linaro.org
Changes in v5:
- Aded PRIV all the G12 internal clk IDS to simplify public exposing
- Fixed the DSI bindings
- Fixed the DSI HSYNC/VSYNC polarity handling
- Fixed the DSI clock setup
- Fixed the DSI phy timings
- Dropped components for DSI, only keeping it for HDMI
- Added MNT Reform 2 CM4 DT
- Dropped already applied PHY fix
- Link to v4: https://lore.kernel.org/r/20230512-amlogic-v6-4-upstream-dsi-ccf-vim3-v4-0-2592c29ea263@linaro.org
Changes from v3 at [3]:
- switched all clk setup via CCF
- using single PLL for DSI controller & ENCL encoder
- added ENCL clocks to CCF
- make the VCLK2 clocks configuration by CCF
- fixed probe/bind of DSI controller to work with panels & bridges
- added bit_clk to controller to it can setup the BIT clock aswell
- added fix for components unbind
- added fix for analog phy setup value
- added TS050 timings fix
- dropped previous clk control patch
Changes from v2 at [2]:
- Fixed patch 3
- Added reviews from Jagan
- Rebased on v5.19-rc1
Changes from v1 at [1]:
- fixed DSI host bindings
- add reviewed-by tags for bindings
- moved magic values to defines thanks to Martin's searches
- added proper prefixes to defines
- moved phy_configure to phy_init() dw-mipi-dsi callback
- moved phy_on to a new phy_power_on() dw-mipi-dsi callback
- correctly return phy_init/configure errors to callback returns
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]
---
Neil Armstrong (7):
dt-bindings: arm: amlogic: Document the MNT Reform 2 CM4 adapter with a BPI-CM4 Module
clk: meson: add vclk driver
clk: meson: g12a: make VCLK2 and ENCL clock path configurable by CCF
drm/meson: gate px_clk when setting rate
arm64: meson: g12-common: add the MIPI DSI nodes
arm64: meson: khadas-vim3l: add TS050 DSI panel overlay
arm64: dts: amlogic: meson-g12b-bananapi-cm4: add support for MNT Reform2 with CM4 adaper
Documentation/devicetree/bindings/arm/amlogic.yaml | 1 +
arch/arm64/boot/dts/amlogic/Makefile | 5 +
arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi | 70 ++++
.../meson-g12b-bananapi-cm4-mnt-reform2.dts | 384 +++++++++++++++++++++
.../boot/dts/amlogic/meson-khadas-vim3-ts050.dtso | 108 ++++++
drivers/clk/meson/Kconfig | 5 +
drivers/clk/meson/Makefile | 1 +
drivers/clk/meson/g12a.c | 76 ++--
drivers/clk/meson/vclk.c | 141 ++++++++
drivers/clk/meson/vclk.h | 51 +++
drivers/gpu/drm/meson/meson_dw_mipi_dsi.c | 7 +
11 files changed, 829 insertions(+), 20 deletions(-)
---
base-commit: 4cece764965020c22cff7665b18a012006359095
change-id: 20230512-amlogic-v6-4-upstream-dsi-ccf-vim3-b8e5217e1f4a
Best regards,
--
Neil Armstrong <[email protected]>
The VCLK and VCLK_DIV clocks have supplementary bits.
The VCLK gate has a "SOFT RESET" bit to toggle after the whole
VCLK sub-tree rate has been set, this is implemented in
the gate enable callback.
The VCLK_DIV clocks as enable and reset bits used to disable
and reset the divider, associated with CLK_SET_RATE_GATE it ensures
the rate is set while the divider is disabled and in reset mode.
The VCLK_DIV enable bit isn't implemented as a gate since it's part
of the divider logic and vendor does this exact sequence to ensure
the divider is correctly set.
Signed-off-by: Neil Armstrong <[email protected]>
---
drivers/clk/meson/Kconfig | 4 ++
drivers/clk/meson/Makefile | 1 +
drivers/clk/meson/vclk.c | 141 +++++++++++++++++++++++++++++++++++++++++++++
drivers/clk/meson/vclk.h | 51 ++++++++++++++++
4 files changed, 197 insertions(+)
diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
index 29ffd14d267b..8a9823789fa3 100644
--- a/drivers/clk/meson/Kconfig
+++ b/drivers/clk/meson/Kconfig
@@ -30,6 +30,10 @@ config COMMON_CLK_MESON_VID_PLL_DIV
tristate
select COMMON_CLK_MESON_REGMAP
+config COMMON_CLK_MESON_VCLK
+ tristate
+ select COMMON_CLK_MESON_REGMAP
+
config COMMON_CLK_MESON_CLKC_UTILS
tristate
diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index 9ee4b954c896..9ba43fe7a07a 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_COMMON_CLK_MESON_PLL) += clk-pll.o
obj-$(CONFIG_COMMON_CLK_MESON_REGMAP) += clk-regmap.o
obj-$(CONFIG_COMMON_CLK_MESON_SCLK_DIV) += sclk-div.o
obj-$(CONFIG_COMMON_CLK_MESON_VID_PLL_DIV) += vid-pll-div.o
+obj-$(CONFIG_COMMON_CLK_MESON_VCLK) += vclk.o
# Amlogic Clock controllers
diff --git a/drivers/clk/meson/vclk.c b/drivers/clk/meson/vclk.c
new file mode 100644
index 000000000000..45dc216941ea
--- /dev/null
+++ b/drivers/clk/meson/vclk.c
@@ -0,0 +1,141 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2024 Neil Armstrong <[email protected]>
+ */
+
+#include <linux/module.h>
+#include "vclk.h"
+
+/* The VCLK gate has a supplementary reset bit to pulse after ungating */
+
+static inline struct meson_vclk_gate_data *
+clk_get_meson_vclk_gate_data(struct clk_regmap *clk)
+{
+ return (struct meson_vclk_gate_data *)clk->data;
+}
+
+static int meson_vclk_gate_enable(struct clk_hw *hw)
+{
+ struct clk_regmap *clk = to_clk_regmap(hw);
+ struct meson_vclk_gate_data *vclk = clk_get_meson_vclk_gate_data(clk);
+
+ meson_parm_write(clk->map, &vclk->enable, 1);
+
+ /* Do a reset pulse */
+ meson_parm_write(clk->map, &vclk->reset, 1);
+ meson_parm_write(clk->map, &vclk->reset, 0);
+
+ return 0;
+}
+
+static void meson_vclk_gate_disable(struct clk_hw *hw)
+{
+ struct clk_regmap *clk = to_clk_regmap(hw);
+ struct meson_vclk_gate_data *vclk = clk_get_meson_vclk_gate_data(clk);
+
+ meson_parm_write(clk->map, &vclk->enable, 0);
+}
+
+static int meson_vclk_gate_is_enabled(struct clk_hw *hw)
+{
+ struct clk_regmap *clk = to_clk_regmap(hw);
+ struct meson_vclk_gate_data *vclk = clk_get_meson_vclk_gate_data(clk);
+
+ return meson_parm_read(clk->map, &vclk->enable);
+}
+
+const struct clk_ops meson_vclk_gate_ops = {
+ .enable = meson_vclk_gate_enable,
+ .disable = meson_vclk_gate_disable,
+ .is_enabled = meson_vclk_gate_is_enabled,
+};
+EXPORT_SYMBOL_GPL(meson_vclk_gate_ops);
+
+/* The VCLK Divider has supplementary reset & enable bits */
+
+static inline struct meson_vclk_div_data *
+clk_get_meson_vclk_div_data(struct clk_regmap *clk)
+{
+ return (struct meson_vclk_div_data *)clk->data;
+}
+
+static unsigned long meson_vclk_div_recalc_rate(struct clk_hw *hw,
+ unsigned long prate)
+{
+ struct clk_regmap *clk = to_clk_regmap(hw);
+ struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
+
+ return divider_recalc_rate(hw, prate, meson_parm_read(clk->map, &vclk->div),
+ vclk->table, vclk->flags, vclk->div.width);
+}
+
+static int meson_vclk_div_determine_rate(struct clk_hw *hw,
+ struct clk_rate_request *req)
+{
+ struct clk_regmap *clk = to_clk_regmap(hw);
+ struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
+
+ return divider_determine_rate(hw, req, vclk->table, vclk->div.width,
+ vclk->flags);
+}
+
+static int meson_vclk_div_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ struct clk_regmap *clk = to_clk_regmap(hw);
+ struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
+ int ret;
+
+ ret = divider_get_val(rate, parent_rate, vclk->table, vclk->div.width,
+ vclk->flags);
+ if (ret < 0)
+ return ret;
+
+ meson_parm_write(clk->map, &vclk->div, ret);
+
+ return 0;
+};
+
+static int meson_vclk_div_enable(struct clk_hw *hw)
+{
+ struct clk_regmap *clk = to_clk_regmap(hw);
+ struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
+
+ /* Unreset the divider when ungating */
+ meson_parm_write(clk->map, &vclk->reset, 0);
+ meson_parm_write(clk->map, &vclk->enable, 1);
+
+ return 0;
+}
+
+static void meson_vclk_div_disable(struct clk_hw *hw)
+{
+ struct clk_regmap *clk = to_clk_regmap(hw);
+ struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
+
+ /* Reset the divider when gating */
+ meson_parm_write(clk->map, &vclk->enable, 0);
+ meson_parm_write(clk->map, &vclk->reset, 1);
+}
+
+static int meson_vclk_div_is_enabled(struct clk_hw *hw)
+{
+ struct clk_regmap *clk = to_clk_regmap(hw);
+ struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
+
+ return meson_parm_read(clk->map, &vclk->enable);
+}
+
+const struct clk_ops meson_vclk_div_ops = {
+ .recalc_rate = meson_vclk_div_recalc_rate,
+ .determine_rate = meson_vclk_div_determine_rate,
+ .set_rate = meson_vclk_div_set_rate,
+ .enable = meson_vclk_div_enable,
+ .disable = meson_vclk_div_disable,
+ .is_enabled = meson_vclk_div_is_enabled,
+};
+EXPORT_SYMBOL_GPL(meson_vclk_div_ops);
+
+MODULE_DESCRIPTION("Amlogic vclk clock driver");
+MODULE_AUTHOR("Neil Armstrong <[email protected]>");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/clk/meson/vclk.h b/drivers/clk/meson/vclk.h
new file mode 100644
index 000000000000..20b0b181db09
--- /dev/null
+++ b/drivers/clk/meson/vclk.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2024 Neil Armstrong <[email protected]>
+ */
+
+#ifndef __VCLK_H
+#define __VCLK_H
+
+#include "clk-regmap.h"
+#include "parm.h"
+
+/**
+ * struct meson_vclk_gate_data - vclk_gate regmap backed specific data
+ *
+ * @enable: vclk enable field
+ * @reset: vclk reset field
+ * @flags: hardware-specific flags
+ *
+ * Flags:
+ * Same as clk_gate except CLK_GATE_HIWORD_MASK which is ignored
+ */
+struct meson_vclk_gate_data {
+ struct parm enable;
+ struct parm reset;
+ u8 flags;
+};
+
+extern const struct clk_ops meson_vclk_gate_ops;
+
+/**
+ * struct meson_vclk_div_data - vclk_div regmap back specific data
+ *
+ * @div: divider field
+ * @enable: vclk divider enable field
+ * @reset: vclk divider reset field
+ * @table: array of value/divider pairs, last entry should have div = 0
+ *
+ * Flags:
+ * Same as clk_divider except CLK_DIVIDER_HIWORD_MASK which is ignored
+ */
+struct meson_vclk_div_data {
+ struct parm div;
+ struct parm enable;
+ struct parm reset;
+ const struct clk_div_table *table;
+ u8 flags;
+};
+
+extern const struct clk_ops meson_vclk_div_ops;
+
+#endif /* __VCLK_H */
--
2.34.1
Disable the px_clk when setting the rate to recover a fully
configured and correctly reset VCLK clock tree after the rate
is set.
Fixes: 77d9e1e6b846 ("drm/meson: add support for MIPI-DSI transceiver")
Signed-off-by: Neil Armstrong <[email protected]>
---
drivers/gpu/drm/meson/meson_dw_mipi_dsi.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c b/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c
index a6bc1bdb3d0d..a10cff3ca1fe 100644
--- a/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c
+++ b/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c
@@ -95,6 +95,7 @@ static int dw_mipi_dsi_phy_init(void *priv_data)
return ret;
}
+ clk_disable_unprepare(mipi_dsi->px_clk);
ret = clk_set_rate(mipi_dsi->px_clk, mipi_dsi->mode->clock * 1000);
if (ret) {
@@ -103,6 +104,12 @@ static int dw_mipi_dsi_phy_init(void *priv_data)
return ret;
}
+ ret = clk_prepare_enable(mipi_dsi->px_clk);
+ if (ret) {
+ dev_err(mipi_dsi->dev, "Failed to enable DSI Pixel clock (ret %d)\n", ret);
+ return ret;
+ }
+
switch (mipi_dsi->dsi_device->format) {
case MIPI_DSI_FMT_RGB888:
dpi_data_format = DPI_COLOR_24BIT;
--
2.34.1
In order to setup the DSI clock, let's make the unused VCLK2 clock path
configuration via CCF.
The nocache option is removed from following clocks:
- vclk2_sel
- vclk2_input
- vclk2_div
- vclk2
- vclk_div1
- vclk2_div2_en
- vclk2_div4_en
- vclk2_div6_en
- vclk2_div12_en
- vclk2_div2
- vclk2_div4
- vclk2_div6
- vclk2_div12
- cts_encl_sel
vclk2 and vclk2_div uses the newly introduced vclk regmap driver
to handle the enable and reset bits.
In order to set a rate on cts_encl via the vclk2 clock path,
the NO_REPARENT flag is set on cts_encl_sel & vclk2_sel in order
to keep CCF from selection a parent.
The parents of cts_encl_sel & vclk2_sel are expected to be defined
in DT or manually set by the display driver at some point.
The following clock scheme is to be used for DSI:
xtal
\_ gp0_pll_dco
\_ gp0_pll
|- vclk2_sel
| \_ vclk2_input
| \_ vclk2_div
| \_ vclk2
| \_ vclk2_div1
| \_ cts_encl_sel
| \_ cts_encl -> to VPU LCD Encoder
|- mipi_dsi_pxclk_sel
\_ mipi_dsi_pxclk_div
\_ mipi_dsi_pxclk -> to DSI controller
The mipi_dsi_pxclk_div is set as bypass with a single /1 entry in div_table
in order to use the same GP0 for mipi_dsi_pxclk and vclk2_input.
The SET_RATE_PARENT is only set on the mipi_dsi_pxclk_sel clock so the
DSI bitclock is the reference base clock to calculate the vclk2_div value
when pixel clock is set on the cts_encl endpoint.
Signed-off-by: Neil Armstrong <[email protected]>
---
drivers/clk/meson/Kconfig | 1 +
drivers/clk/meson/g12a.c | 76 ++++++++++++++++++++++++++++++++++-------------
2 files changed, 57 insertions(+), 20 deletions(-)
diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
index 8a9823789fa3..59a40a49f8e1 100644
--- a/drivers/clk/meson/Kconfig
+++ b/drivers/clk/meson/Kconfig
@@ -144,6 +144,7 @@ config COMMON_CLK_G12A
select COMMON_CLK_MESON_EE_CLKC
select COMMON_CLK_MESON_CPU_DYNDIV
select COMMON_CLK_MESON_VID_PLL_DIV
+ select COMMON_CLK_MESON_VCLK
select MFD_SYSCON
help
Support for the clock controller on Amlogic S905D2, S905X2 and S905Y2
diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c
index 90f4c6103014..df7e17c850d8 100644
--- a/drivers/clk/meson/g12a.c
+++ b/drivers/clk/meson/g12a.c
@@ -22,6 +22,7 @@
#include "clk-regmap.h"
#include "clk-cpu-dyndiv.h"
#include "vid-pll-div.h"
+#include "vclk.h"
#include "meson-eeclk.h"
#include "g12a.h"
@@ -3165,7 +3166,7 @@ static struct clk_regmap g12a_vclk2_sel = {
.ops = &clk_regmap_mux_ops,
.parent_hws = g12a_vclk_parent_hws,
.num_parents = ARRAY_SIZE(g12a_vclk_parent_hws),
- .flags = CLK_SET_RATE_NO_REPARENT | CLK_GET_RATE_NOCACHE,
+ .flags = CLK_SET_RATE_NO_REPARENT,
},
};
@@ -3193,7 +3194,6 @@ static struct clk_regmap g12a_vclk2_input = {
.ops = &clk_regmap_gate_ops,
.parent_hws = (const struct clk_hw *[]) { &g12a_vclk2_sel.hw },
.num_parents = 1,
- .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
},
};
@@ -3215,19 +3215,32 @@ static struct clk_regmap g12a_vclk_div = {
};
static struct clk_regmap g12a_vclk2_div = {
- .data = &(struct clk_regmap_div_data){
- .offset = HHI_VIID_CLK_DIV,
- .shift = 0,
- .width = 8,
+ .data = &(struct meson_vclk_div_data){
+ .div = {
+ .reg_off = HHI_VIID_CLK_DIV,
+ .shift = 0,
+ .width = 8,
+ },
+ .enable = {
+ .reg_off = HHI_VIID_CLK_DIV,
+ .shift = 16,
+ .width = 1,
+ },
+ .reset = {
+ .reg_off = HHI_VIID_CLK_DIV,
+ .shift = 17,
+ .width = 1,
+ },
+ .flags = CLK_DIVIDER_ROUND_CLOSEST,
},
.hw.init = &(struct clk_init_data){
.name = "vclk2_div",
- .ops = &clk_regmap_divider_ops,
+ .ops = &meson_vclk_div_ops,
.parent_hws = (const struct clk_hw *[]) {
&g12a_vclk2_input.hw
},
.num_parents = 1,
- .flags = CLK_GET_RATE_NOCACHE,
+ .flags = CLK_SET_RATE_GATE,
},
};
@@ -3246,16 +3259,24 @@ static struct clk_regmap g12a_vclk = {
};
static struct clk_regmap g12a_vclk2 = {
- .data = &(struct clk_regmap_gate_data){
- .offset = HHI_VIID_CLK_CNTL,
- .bit_idx = 19,
+ .data = &(struct meson_vclk_gate_data){
+ .enable = {
+ .reg_off = HHI_VIID_CLK_CNTL,
+ .shift = 19,
+ .width = 1,
+ },
+ .reset = {
+ .reg_off = HHI_VIID_CLK_CNTL,
+ .shift = 15,
+ .width = 1,
+ },
},
.hw.init = &(struct clk_init_data) {
.name = "vclk2",
- .ops = &clk_regmap_gate_ops,
+ .ops = &meson_vclk_gate_ops,
.parent_hws = (const struct clk_hw *[]) { &g12a_vclk2_div.hw },
.num_parents = 1,
- .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
+ .flags = CLK_SET_RATE_PARENT,
},
};
@@ -3339,7 +3360,7 @@ static struct clk_regmap g12a_vclk2_div1 = {
.ops = &clk_regmap_gate_ops,
.parent_hws = (const struct clk_hw *[]) { &g12a_vclk2.hw },
.num_parents = 1,
- .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
+ .flags = CLK_SET_RATE_PARENT,
},
};
@@ -3353,7 +3374,7 @@ static struct clk_regmap g12a_vclk2_div2_en = {
.ops = &clk_regmap_gate_ops,
.parent_hws = (const struct clk_hw *[]) { &g12a_vclk2.hw },
.num_parents = 1,
- .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
+ .flags = CLK_SET_RATE_PARENT,
},
};
@@ -3367,7 +3388,7 @@ static struct clk_regmap g12a_vclk2_div4_en = {
.ops = &clk_regmap_gate_ops,
.parent_hws = (const struct clk_hw *[]) { &g12a_vclk2.hw },
.num_parents = 1,
- .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
+ .flags = CLK_SET_RATE_PARENT,
},
};
@@ -3381,7 +3402,7 @@ static struct clk_regmap g12a_vclk2_div6_en = {
.ops = &clk_regmap_gate_ops,
.parent_hws = (const struct clk_hw *[]) { &g12a_vclk2.hw },
.num_parents = 1,
- .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
+ .flags = CLK_SET_RATE_PARENT,
},
};
@@ -3395,7 +3416,7 @@ static struct clk_regmap g12a_vclk2_div12_en = {
.ops = &clk_regmap_gate_ops,
.parent_hws = (const struct clk_hw *[]) { &g12a_vclk2.hw },
.num_parents = 1,
- .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
+ .flags = CLK_SET_RATE_PARENT,
},
};
@@ -3461,6 +3482,7 @@ static struct clk_fixed_factor g12a_vclk2_div2 = {
&g12a_vclk2_div2_en.hw
},
.num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
},
};
@@ -3474,6 +3496,7 @@ static struct clk_fixed_factor g12a_vclk2_div4 = {
&g12a_vclk2_div4_en.hw
},
.num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
},
};
@@ -3487,6 +3510,7 @@ static struct clk_fixed_factor g12a_vclk2_div6 = {
&g12a_vclk2_div6_en.hw
},
.num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
},
};
@@ -3500,6 +3524,7 @@ static struct clk_fixed_factor g12a_vclk2_div12 = {
&g12a_vclk2_div12_en.hw
},
.num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
},
};
@@ -3561,7 +3586,7 @@ static struct clk_regmap g12a_cts_encl_sel = {
.ops = &clk_regmap_mux_ops,
.parent_hws = g12a_cts_parent_hws,
.num_parents = ARRAY_SIZE(g12a_cts_parent_hws),
- .flags = CLK_SET_RATE_NO_REPARENT | CLK_GET_RATE_NOCACHE,
+ .flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_NO_REPARENT,
},
};
@@ -3717,15 +3742,26 @@ static struct clk_regmap g12a_mipi_dsi_pxclk_sel = {
.ops = &clk_regmap_mux_ops,
.parent_hws = g12a_mipi_dsi_pxclk_parent_hws,
.num_parents = ARRAY_SIZE(g12a_mipi_dsi_pxclk_parent_hws),
- .flags = CLK_SET_RATE_NO_REPARENT,
+ .flags = CLK_SET_RATE_NO_REPARENT | CLK_SET_RATE_PARENT,
},
};
+/*
+ * FIXME: Force as bypass by forcing a single /1 table entry, and doensn't on boot value
+ * when setting a clock whith this node in the clock path, but doesn't garantee the divider
+ * is at /1 at boot until a rate is set.
+ */
+static const struct clk_div_table g12a_mipi_dsi_pxclk_div_table[] = {
+ { .val = 0, .div = 1 },
+ { /* sentinel */ },
+};
+
static struct clk_regmap g12a_mipi_dsi_pxclk_div = {
.data = &(struct clk_regmap_div_data){
.offset = HHI_MIPIDSI_PHY_CLK_CNTL,
.shift = 0,
.width = 7,
+ .table = g12a_mipi_dsi_pxclk_div_table,
},
.hw.init = &(struct clk_init_data){
.name = "mipi_dsi_pxclk_div",
--
2.34.1
The MNT Reform 2 CM4 adapter can be populated with any Raspberry Pi CM4
compatible module such as a BPI-CM4 Module, document that.
Acked-by: Conor Dooley <[email protected]>
Signed-off-by: Neil Armstrong <[email protected]>
---
Documentation/devicetree/bindings/arm/amlogic.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/arm/amlogic.yaml b/Documentation/devicetree/bindings/arm/amlogic.yaml
index 949537cea6be..b66b93b8bfd3 100644
--- a/Documentation/devicetree/bindings/arm/amlogic.yaml
+++ b/Documentation/devicetree/bindings/arm/amlogic.yaml
@@ -157,6 +157,7 @@ properties:
items:
- enum:
- bananapi,bpi-cm4io
+ - mntre,reform2-cm4
- const: bananapi,bpi-cm4
- const: amlogic,a311d
- const: amlogic,g12b
--
2.34.1
Add the MIPI DSI Analog & Digital PHY nodes and the DSI control
nodes with proper port endpoint to the VPU.
Signed-off-by: Neil Armstrong <[email protected]>
---
arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi | 70 +++++++++++++++++++++++
1 file changed, 70 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
index 9d5eab6595d0..b058ed78faf0 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi
@@ -1663,9 +1663,28 @@ pwrc: power-controller {
<250000000>,
<0>; /* Do Nothing */
};
+
+ mipi_analog_dphy: phy {
+ compatible = "amlogic,g12a-mipi-dphy-analog";
+ #phy-cells = <0>;
+ status = "disabled";
+ };
};
};
+ mipi_dphy: phy@44000 {
+ compatible = "amlogic,axg-mipi-dphy";
+ reg = <0x0 0x44000 0x0 0x2000>;
+ clocks = <&clkc CLKID_MIPI_DSI_PHY>;
+ clock-names = "pclk";
+ resets = <&reset RESET_MIPI_DSI_PHY>;
+ reset-names = "phy";
+ phys = <&mipi_analog_dphy>;
+ phy-names = "analog";
+ #phy-cells = <0>;
+ status = "disabled";
+ };
+
usb3_pcie_phy: phy@46000 {
compatible = "amlogic,g12a-usb3-pcie-phy";
reg = <0x0 0x46000 0x0 0x2000>;
@@ -2152,6 +2171,15 @@ hdmi_tx_out: endpoint {
remote-endpoint = <&hdmi_tx_in>;
};
};
+
+ /* DPI output port */
+ dpi_port: port@2 {
+ reg = <2>;
+
+ dpi_out: endpoint {
+ remote-endpoint = <&mipi_dsi_in>;
+ };
+ };
};
gic: interrupt-controller@ffc01000 {
@@ -2189,6 +2217,48 @@ gpio_intc: interrupt-controller@f080 {
amlogic,channel-interrupts = <64 65 66 67 68 69 70 71>;
};
+ mipi_dsi: dsi@7000 {
+ compatible = "amlogic,meson-g12a-dw-mipi-dsi";
+ reg = <0x0 0x7000 0x0 0x1000>;
+ resets = <&reset RESET_MIPI_DSI_HOST>;
+ reset-names = "top";
+ clocks = <&clkc CLKID_MIPI_DSI_HOST>,
+ <&clkc CLKID_MIPI_DSI_PXCLK>,
+ <&clkc CLKID_CTS_ENCL>;
+ clock-names = "pclk", "bit", "px";
+ phys = <&mipi_dphy>;
+ phy-names = "dphy";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+
+ assigned-clocks = <&clkc CLKID_MIPI_DSI_PXCLK_SEL>,
+ <&clkc CLKID_CTS_ENCL_SEL>,
+ <&clkc CLKID_VCLK2_SEL>;
+ assigned-clock-parents = <&clkc CLKID_GP0_PLL>,
+ <&clkc CLKID_VCLK2_DIV1>,
+ <&clkc CLKID_GP0_PLL>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ /* VPU VENC Input */
+ mipi_dsi_venc_port: port@0 {
+ reg = <0>;
+
+ mipi_dsi_in: endpoint {
+ remote-endpoint = <&dpi_out>;
+ };
+ };
+
+ /* DSI Output */
+ mipi_dsi_panel_port: port@1 {
+ reg = <1>;
+ };
+ };
+ };
+
watchdog: watchdog@f0d0 {
compatible = "amlogic,meson-gxbb-wdt";
reg = <0x0 0xf0d0 0x0 0x10>;
--
2.34.1
This add dtbo overlay to support the Khadas TS050 panel on the
Khadas VIM3 & VIM3L boards.
Signed-off-by: Neil Armstrong <[email protected]>
---
arch/arm64/boot/dts/amlogic/Makefile | 4 +
.../boot/dts/amlogic/meson-khadas-vim3-ts050.dtso | 108 +++++++++++++++++++++
2 files changed, 112 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/Makefile b/arch/arm64/boot/dts/amlogic/Makefile
index 1ab160bf928a..0b7961de3db7 100644
--- a/arch/arm64/boot/dts/amlogic/Makefile
+++ b/arch/arm64/boot/dts/amlogic/Makefile
@@ -16,6 +16,7 @@ dtb-$(CONFIG_ARCH_MESON) += meson-g12a-u200.dtb
dtb-$(CONFIG_ARCH_MESON) += meson-g12a-x96-max.dtb
dtb-$(CONFIG_ARCH_MESON) += meson-g12b-a311d-bananapi-m2s.dtb
dtb-$(CONFIG_ARCH_MESON) += meson-g12b-a311d-khadas-vim3.dtb
+dtb-$(CONFIG_ARCH_MESON) += meson-g12b-a311d-khadas-vim3-ts050.dtb
dtb-$(CONFIG_ARCH_MESON) += meson-g12b-bananapi-cm4-cm4io.dtb
dtb-$(CONFIG_ARCH_MESON) += meson-g12b-gsking-x.dtb
dtb-$(CONFIG_ARCH_MESON) += meson-g12b-gtking-pro.dtb
@@ -76,6 +77,7 @@ dtb-$(CONFIG_ARCH_MESON) += meson-sm1-bananapi-m2-pro.dtb
dtb-$(CONFIG_ARCH_MESON) += meson-sm1-bananapi-m5.dtb
dtb-$(CONFIG_ARCH_MESON) += meson-sm1-h96-max.dtb
dtb-$(CONFIG_ARCH_MESON) += meson-sm1-khadas-vim3l.dtb
+dtb-$(CONFIG_ARCH_MESON) += meson-sm1-khadas-vim3l-ts050.dtb
dtb-$(CONFIG_ARCH_MESON) += meson-sm1-s905d3-libretech-cc.dtb
dtb-$(CONFIG_ARCH_MESON) += meson-sm1-odroid-c4.dtb
dtb-$(CONFIG_ARCH_MESON) += meson-sm1-odroid-hc4.dtb
@@ -86,3 +88,5 @@ dtb-$(CONFIG_ARCH_MESON) += meson-sm1-x96-air.dtb
# Overlays
meson-g12a-fbx8am-brcm-dtbs := meson-g12a-fbx8am.dtb meson-g12a-fbx8am-brcm.dtbo
meson-g12a-fbx8am-realtek-dtbs := meson-g12a-fbx8am.dtb meson-g12a-fbx8am-realtek.dtbo
+meson-g12b-a311d-khadas-vim3-ts050-dtbs := meson-g12b-a311d-khadas-vim3.dtb meson-khadas-vim3-ts050.dtbo
+meson-sm1-khadas-vim3l-ts050-dtbs := meson-sm1-khadas-vim3l.dtb meson-khadas-vim3-ts050.dtbo
diff --git a/arch/arm64/boot/dts/amlogic/meson-khadas-vim3-ts050.dtso b/arch/arm64/boot/dts/amlogic/meson-khadas-vim3-ts050.dtso
new file mode 100644
index 000000000000..a41b4e619580
--- /dev/null
+++ b/arch/arm64/boot/dts/amlogic/meson-khadas-vim3-ts050.dtso
@@ -0,0 +1,108 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2019 BayLibre, SAS
+ * Author: Neil Armstrong <[email protected]>
+ */
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/clock/g12a-clkc.h>
+#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/interrupt-controller/amlogic,meson-g12a-gpio-intc.h>
+
+/dts-v1/;
+/plugin/;
+
+/*
+ * Enable Khadas TS050 DSI Panel + Touch Controller
+ * on Khadas VIM3 (A311D) and VIM3L (S905D3)
+ */
+
+&{/} {
+ panel_backlight: backlight {
+ compatible = "pwm-backlight";
+ pwms = <&pwm_AO_cd 0 25000 0>;
+ brightness-levels = <0 255>;
+ num-interpolated-steps = <255>;
+ default-brightness-level = <200>;
+ };
+};
+
+&i2c3 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ pinctrl-0 = <&i2c3_sda_a_pins>, <&i2c3_sck_a_pins>;
+ pinctrl-names = "default";
+ status = "okay";
+
+ touch-controller@38 {
+ compatible = "edt,edt-ft5206";
+ reg = <0x38>;
+ interrupt-parent = <&gpio_intc>;
+ interrupts = <IRQID_GPIOA_5 IRQ_TYPE_EDGE_FALLING>;
+ reset-gpios = <&gpio_expander 6 GPIO_ACTIVE_LOW>;
+ touchscreen-size-x = <1080>;
+ touchscreen-size-y = <1920>;
+ status = "okay";
+ };
+};
+
+&mipi_dsi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "okay";
+
+ assigned-clocks = <&clkc CLKID_GP0_PLL>,
+ <&clkc CLKID_MIPI_DSI_PXCLK_SEL>,
+ <&clkc CLKID_MIPI_DSI_PXCLK>,
+ <&clkc CLKID_CTS_ENCL_SEL>,
+ <&clkc CLKID_VCLK2_SEL>;
+ assigned-clock-parents = <0>,
+ <&clkc CLKID_GP0_PLL>,
+ <0>,
+ <&clkc CLKID_VCLK2_DIV1>,
+ <&clkc CLKID_GP0_PLL>;
+ assigned-clock-rates = <960000000>,
+ <0>,
+ <960000000>,
+ <0>,
+ <0>;
+
+ panel@0 {
+ compatible = "khadas,ts050";
+ reset-gpios = <&gpio_expander 0 GPIO_ACTIVE_LOW>;
+ enable-gpios = <&gpio_expander 1 GPIO_ACTIVE_HIGH>;
+ power-supply = <&vcc_3v3>;
+ backlight = <&panel_backlight>;
+ reg = <0>;
+
+ port {
+ mipi_in_panel: endpoint {
+ remote-endpoint = <&mipi_out_panel>;
+ };
+ };
+ };
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@1 {
+ mipi_out_panel: endpoint {
+ remote-endpoint = <&mipi_in_panel>;
+ };
+ };
+ };
+};
+
+&mipi_analog_dphy {
+ status = "okay";
+};
+
+&mipi_dphy {
+ status = "okay";
+};
+
+&pwm_AO_cd {
+ pinctrl-0 = <&pwm_ao_c_6_pins>, <&pwm_ao_d_e_pins>;
+};
--
2.34.1
This adds a basic devicetree for the MNT Reform2 DIY laptop when using a
CM4 adapter and a BPI-CM4 module.
Co-developed-by: Lukas F. Hartmann <[email protected]>
Signed-off-by: Neil Armstrong <[email protected]>
---
arch/arm64/boot/dts/amlogic/Makefile | 1 +
.../meson-g12b-bananapi-cm4-mnt-reform2.dts | 384 +++++++++++++++++++++
2 files changed, 385 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/Makefile b/arch/arm64/boot/dts/amlogic/Makefile
index 0b7961de3db7..d525e5123fbc 100644
--- a/arch/arm64/boot/dts/amlogic/Makefile
+++ b/arch/arm64/boot/dts/amlogic/Makefile
@@ -18,6 +18,7 @@ dtb-$(CONFIG_ARCH_MESON) += meson-g12b-a311d-bananapi-m2s.dtb
dtb-$(CONFIG_ARCH_MESON) += meson-g12b-a311d-khadas-vim3.dtb
dtb-$(CONFIG_ARCH_MESON) += meson-g12b-a311d-khadas-vim3-ts050.dtb
dtb-$(CONFIG_ARCH_MESON) += meson-g12b-bananapi-cm4-cm4io.dtb
+dtb-$(CONFIG_ARCH_MESON) += meson-g12b-bananapi-cm4-mnt-reform2.dtb
dtb-$(CONFIG_ARCH_MESON) += meson-g12b-gsking-x.dtb
dtb-$(CONFIG_ARCH_MESON) += meson-g12b-gtking-pro.dtb
dtb-$(CONFIG_ARCH_MESON) += meson-g12b-gtking.dtb
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-bananapi-cm4-mnt-reform2.dts b/arch/arm64/boot/dts/amlogic/meson-g12b-bananapi-cm4-mnt-reform2.dts
new file mode 100644
index 000000000000..003efed529ba
--- /dev/null
+++ b/arch/arm64/boot/dts/amlogic/meson-g12b-bananapi-cm4-mnt-reform2.dts
@@ -0,0 +1,384 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2023 Neil Armstrong <[email protected]>
+ * Copyright 2023 MNT Research GmbH
+ */
+
+/dts-v1/;
+
+#include "meson-g12b-bananapi-cm4.dtsi"
+#include <dt-bindings/input/input.h>
+#include <dt-bindings/leds/common.h>
+#include <dt-bindings/sound/meson-g12a-tohdmitx.h>
+
+/ {
+ model = "MNT Reform 2 with BPI-CM4 Module";
+ compatible = "mntre,reform2-cm4", "bananapi,bpi-cm4", "amlogic,a311d", "amlogic,g12b";
+ chassis-type = "laptop";
+
+ aliases {
+ ethernet0 = ðmac;
+ i2c0 = &i2c1;
+ i2c1 = &i2c3;
+ };
+
+ hdmi_connector: hdmi-connector {
+ compatible = "hdmi-connector";
+ type = "a";
+
+ port {
+ hdmi_connector_in: endpoint {
+ remote-endpoint = <&hdmi_tx_tmds_out>;
+ };
+ };
+ };
+
+ leds {
+ compatible = "gpio-leds";
+
+ led-blue {
+ color = <LED_COLOR_ID_BLUE>;
+ function = LED_FUNCTION_STATUS;
+ gpios = <&gpio_ao GPIOAO_7 GPIO_ACTIVE_HIGH>;
+ linux,default-trigger = "heartbeat";
+ };
+
+ led-green {
+ color = <LED_COLOR_ID_GREEN>;
+ function = LED_FUNCTION_STATUS;
+ gpios = <&gpio_ao GPIOAO_2 GPIO_ACTIVE_HIGH>;
+ };
+ };
+
+ sound {
+ compatible = "amlogic,axg-sound-card";
+ model = "MNT-REFORM2-BPI-CM4";
+ audio-widgets = "Headphone", "Headphone Jack",
+ "Speaker", "External Speaker",
+ "Microphone", "Mic Jack";
+ audio-aux-devs = <&tdmout_a>, <&tdmout_b>, <&tdmin_b>;
+ audio-routing = "TDMOUT_A IN 0", "FRDDR_A OUT 0",
+ "TDMOUT_A IN 1", "FRDDR_B OUT 0",
+ "TDMOUT_A IN 2", "FRDDR_C OUT 0",
+ "TDM_A Playback", "TDMOUT_A OUT",
+ "TDMOUT_B IN 0", "FRDDR_A OUT 1",
+ "TDMOUT_B IN 1", "FRDDR_B OUT 1",
+ "TDMOUT_B IN 2", "FRDDR_C OUT 1",
+ "TDM_B Playback", "TDMOUT_B OUT",
+ "TDMIN_B IN 1", "TDM_B Capture",
+ "TDMIN_B IN 4", "TDM_B Loopback",
+ "TODDR_A IN 1", "TDMIN_B OUT",
+ "TODDR_B IN 1", "TDMIN_B OUT",
+ "TODDR_C IN 1", "TDMIN_B OUT",
+ "Headphone Jack", "HP_L",
+ "Headphone Jack", "HP_R",
+ "External Speaker", "SPK_LP",
+ "External Speaker", "SPK_LN",
+ "External Speaker", "SPK_RP",
+ "External Speaker", "SPK_RN",
+ "LINPUT1", "Mic Jack",
+ "Mic Jack", "MICB";
+
+ assigned-clocks = <&clkc CLKID_MPLL2>,
+ <&clkc CLKID_MPLL0>,
+ <&clkc CLKID_MPLL1>;
+ assigned-clock-parents = <0>, <0>, <0>;
+ assigned-clock-rates = <294912000>,
+ <270950400>,
+ <393216000>;
+
+ dai-link-0 {
+ sound-dai = <&frddr_a>;
+ };
+
+ dai-link-1 {
+ sound-dai = <&frddr_b>;
+ };
+
+ dai-link-2 {
+ sound-dai = <&frddr_c>;
+ };
+
+ dai-link-3 {
+ sound-dai = <&toddr_a>;
+ };
+
+ dai-link-4 {
+ sound-dai = <&toddr_b>;
+ };
+
+ dai-link-5 {
+ sound-dai = <&toddr_c>;
+ };
+
+ /* 8ch hdmi interface */
+ dai-link-6 {
+ sound-dai = <&tdmif_a>;
+ dai-format = "i2s";
+ dai-tdm-slot-tx-mask-0 = <1 1>;
+ dai-tdm-slot-tx-mask-1 = <1 1>;
+ dai-tdm-slot-tx-mask-2 = <1 1>;
+ dai-tdm-slot-tx-mask-3 = <1 1>;
+ mclk-fs = <256>;
+
+ codec {
+ sound-dai = <&tohdmitx TOHDMITX_I2S_IN_A>;
+ };
+ };
+
+ /* Analog Audio */
+ dai-link-7 {
+ sound-dai = <&tdmif_b>;
+ dai-format = "i2s";
+ dai-tdm-slot-tx-mask-0 = <1 1>;
+ mclk-fs = <256>;
+
+ codec {
+ sound-dai = <&wm8960>;
+ };
+ };
+
+ /* hdmi glue */
+ dai-link-8 {
+ sound-dai = <&tohdmitx TOHDMITX_I2S_OUT>;
+
+ codec {
+ sound-dai = <&hdmi_tx>;
+ };
+ };
+ };
+
+ reg_main_1v8: regulator-main-1v8 {
+ compatible = "regulator-fixed";
+ regulator-name = "1V8";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ vin-supply = <®_main_3v3>;
+ };
+
+ reg_main_1v2: regulator-main-1v2 {
+ compatible = "regulator-fixed";
+ regulator-name = "1V2";
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
+ vin-supply = <®_main_5v>;
+ };
+
+ reg_main_3v3: regulator-main-3v3 {
+ compatible = "regulator-fixed";
+ regulator-name = "3V3";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ };
+
+ reg_main_5v: regulator-main-5v {
+ compatible = "regulator-fixed";
+ regulator-name = "5V";
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+ };
+
+ reg_main_usb: regulator-main-usb {
+ compatible = "regulator-fixed";
+ regulator-name = "USB_PWR";
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+ vin-supply = <®_main_5v>;
+ };
+
+ backlight: backlight {
+ compatible = "pwm-backlight";
+ pwms = <&pwm_AO_ab 0 10000 0>;
+ power-supply = <®_main_usb>;
+ enable-gpios = <&gpio 58 GPIO_ACTIVE_HIGH>;
+ brightness-levels = <0 32 64 128 160 200 255>;
+ default-brightness-level = <6>;
+ };
+
+ panel {
+ compatible = "innolux,n125hce-gn1";
+ power-supply = <®_main_3v3>;
+ backlight = <&backlight>;
+ no-hpd;
+
+ port {
+ panel_in: endpoint {
+ remote-endpoint = <&edp_bridge_out>;
+ };
+ };
+ };
+
+ clock_12288: clock_12288 {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <12288000>;
+ };
+};
+
+&mipi_analog_dphy {
+ status = "okay";
+};
+
+&mipi_dphy {
+ status = "okay";
+};
+
+&mipi_dsi {
+ status = "okay";
+
+ assigned-clocks = <&clkc CLKID_GP0_PLL>,
+ <&clkc CLKID_MIPI_DSI_PXCLK_SEL>,
+ <&clkc CLKID_MIPI_DSI_PXCLK>,
+ <&clkc CLKID_CTS_ENCL_SEL>,
+ <&clkc CLKID_VCLK2_SEL>;
+ assigned-clock-parents = <0>,
+ <&clkc CLKID_GP0_PLL>,
+ <0>,
+ <&clkc CLKID_VCLK2_DIV1>,
+ <&clkc CLKID_GP0_PLL>;
+ assigned-clock-rates = <936000000>,
+ <0>,
+ <936000000>,
+ <0>,
+ <0>;
+};
+
+&mipi_dsi_panel_port {
+ mipi_dsi_out: endpoint {
+ remote-endpoint = <&edp_bridge_in>;
+ };
+};
+
+&cecb_AO {
+ status = "okay";
+};
+
+ðmac {
+ status = "okay";
+};
+
+&hdmi_tx {
+ status = "okay";
+};
+
+&hdmi_tx_tmds_port {
+ hdmi_tx_tmds_out: endpoint {
+ remote-endpoint = <&hdmi_connector_in>;
+ };
+};
+
+&pwm_AO_ab {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pwm_ao_a_pins>;
+ status = "okay";
+};
+
+&i2c0 {
+ status = "okay";
+};
+
+&i2c3 {
+ status = "okay";
+
+ edp_bridge: bridge@2c {
+ compatible = "ti,sn65dsi86";
+ reg = <0x2c>;
+ enable-gpios = <&gpio GPIOX_10 GPIO_ACTIVE_HIGH>; // PIN_24 / GPIO8
+ vccio-supply = <®_main_1v8>;
+ vpll-supply = <®_main_1v8>;
+ vcca-supply = <®_main_1v2>;
+ vcc-supply = <®_main_1v2>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+
+ edp_bridge_in: endpoint {
+ remote-endpoint = <&mipi_dsi_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+
+ edp_bridge_out: endpoint {
+ remote-endpoint = <&panel_in>;
+ };
+ };
+ };
+ };
+};
+
+&i2c2 {
+ status = "okay";
+
+ wm8960: codec@1a {
+ compatible = "wlf,wm8960";
+ reg = <0x1a>;
+ clocks = <&clock_12288>;
+ clock-names = "mclk";
+ #sound-dai-cells = <0>;
+ wlf,shared-lrclk;
+ };
+
+ rtc@68 {
+ compatible = "nxp,pcf8523";
+ reg = <0x68>;
+ };
+};
+
+&pcie {
+ status = "okay";
+};
+
+&sd_emmc_b {
+ status = "okay";
+};
+
+&tdmif_a {
+ status = "okay";
+};
+
+&tdmout_a {
+ status = "okay";
+};
+
+&tdmif_b {
+ pinctrl-0 = <&tdm_b_dout0_pins>, <&tdm_b_fs_pins>, <&tdm_b_sclk_pins>, <&tdm_b_din1_pins>;
+ pinctrl-names = "default";
+
+ assigned-clocks = <&clkc_audio AUD_CLKID_TDM_SCLK_PAD1>,
+ <&clkc_audio AUD_CLKID_TDM_LRCLK_PAD1>;
+ assigned-clock-parents = <&clkc_audio AUD_CLKID_MST_B_SCLK>,
+ <&clkc_audio AUD_CLKID_MST_B_LRCLK>;
+ assigned-clock-rates = <0>, <0>;
+};
+
+&tdmin_b {
+ status = "okay";
+};
+
+&toddr_a {
+ status = "okay";
+};
+
+&toddr_b {
+ status = "okay";
+};
+
+&toddr_c {
+ status = "okay";
+};
+
+&tohdmitx {
+ status = "okay";
+};
+
+&usb {
+ dr_mode = "host";
+
+ status = "okay";
+};
--
2.34.1
On Wed, 03 Apr 2024 09:46:31 +0200, Neil Armstrong wrote:
> The Amlogic G12A, G12B & SM1 SoCs embeds a Synopsys DW-MIPI-DSI transceiver (ver 1.21a),
> with a custom glue managing the IP resets, clock and data input similar to the DW-HDMI
> glue on the same Amlogic SoCs.
>
> This is a follow-up of v5 now the DRM patches are applied, the clk & DT changes
> remains for a full DSI support on G12A & SM1 platforms.
>
> The DW-MIPI-DSI transceiver + D-PHY are clocked by the GP0 PLL, and the ENCL encoder + VIU
> pixel reader by the VCLK2 clock using the HDMI PLL.
>
> The DW-MIPI-DSI transceiver gets this pixel stream as input clocked with the VCLK2 clock.
>
> An optional "MEAS" clock can be enabled to measure the delay between each vsync feeding the
> DW-MIPI-DSI transceiver.
>
> The clock setup has been redesigned to use CCF, a common PLL (GP0) and the VCLK2 clock
> path for DSI in preparation of full CCF support and possibly dual display with HDMI.
>
> The change from v5 is that now we use a "VCLK" driver instead of notifier and rely
> on CLK_SET_RATE_GATE to ensure the VCLK gate operation are called.
>
> Signed-off-by: Neil Armstrong <[email protected]>
> ---
> Changes in v12:
> - fix parameters alignment in patch 2
> - update g12a_mipi_dsi_pxclk_div_table comment with jerome's suggestions
> - fix dtbs overlay build, fix missed v11... thx khadas for reporting it off-list & testing
> - Link to v11: https://lore.kernel.org/r/20240325-amlogic-v6-4-upstream-dsi-ccf-vim3-v11-0-04f55de44604@linaro.org
>
> Changes in v11:
> - Rebased on v6.9-rc1
> - Fixed overlay handling/creation
> - Link to v10: https://lore.kernel.org/r/20240205-amlogic-v6-4-upstream-dsi-ccf-vim3-v10-0-dc06073d5330@linaro.org
>
> Changes in v10:
> - Rename regmap_vclk to meson_clk and add _gate for the gate
> - Move COMMON_CLK_MESON_VCLK to following patch
> - Remove CLK_SET_RATE_PARENT from g12a_vclk2_sel, keep it only on mipi_dsi_pxclk_sel
> - Add more info on commit message to specify how clock setup is designed
> - Remove forgotten CLK_IGNORE_UNUSED on g12a_vclk2_input
> - Remove useless CLK_SET_RATE_PARENT on g12a_vclk2_div to stop propagatting rate _after_ vclk2_div
> - Remove invalid CLK_SET_RATE_GATE on g12a_vclk2 since it's not a divider...
> - Drop already applied patches
> - move Khadas TS050 changes as an overlay
> - Link to v9: https://lore.kernel.org/r/20231124-amlogic-v6-4-upstream-dsi-ccf-vim3-v9-0-95256ed139e6@linaro.org
>
> Changes in v9:
> - Colledte reviewed-bys
> - Fixed patches 2 & 4, commit messages and bindings format
> - Link to v8: https://lore.kernel.org/r/20231109-amlogic-v6-4-upstream-dsi-ccf-vim3-v8-0-81e4aeeda193@linaro.org
>
> Changes in v8:
> - Switch vclk clk driver to parm as requested by Jerome
> - Added bindings fixes to amlogic,meson-axg-mipi-pcie-analog & amlogic,g12a-mipi-dphy-analog
> - Fixed DT errors in vim3 example and MNT Reform DT
> - Rebased on next-20231107, successfully tested on VIM3L
> - Link to v7: https://lore.kernel.org/r/20230803-amlogic-v6-4-upstream-dsi-ccf-vim3-v7-0-762219fc5b28@linaro.org
>
> Changes in v7:
> - Added review tags
> - Fixed patch 5 thanks to George
> - Link to v6: https://lore.kernel.org/r/20230512-amlogic-v6-4-upstream-dsi-ccf-vim3-v6-0-fd2ac9845472@linaro.org
>
> Changes in v6:
> - dropped applied DRM patches
> - dropped clk private prefix patches
> - rebased on top of 20230607-topic-amlogic-upstream-clkid-public-migration-v2-0-38172d17c27a@linaro.org
> - re-ordered/cleaned ENCL patches to match clkid public migration
> - Added new "vclk" driver
> - uses vclk driver instead of notifier
> - cleaned VCLK2 clk flags
> - add px_clk gating from DSI driver
> - Link to v5: https://lore.kernel.org/r/20230512-amlogic-v6-4-upstream-dsi-ccf-vim3-v5-0-56eb7a4d5b8e@linaro.org
>
> Changes in v5:
> - Aded PRIV all the G12 internal clk IDS to simplify public exposing
> - Fixed the DSI bindings
> - Fixed the DSI HSYNC/VSYNC polarity handling
> - Fixed the DSI clock setup
> - Fixed the DSI phy timings
> - Dropped components for DSI, only keeping it for HDMI
> - Added MNT Reform 2 CM4 DT
> - Dropped already applied PHY fix
> - Link to v4: https://lore.kernel.org/r/20230512-amlogic-v6-4-upstream-dsi-ccf-vim3-v4-0-2592c29ea263@linaro.org
>
> Changes from v3 at [3]:
> - switched all clk setup via CCF
> - using single PLL for DSI controller & ENCL encoder
> - added ENCL clocks to CCF
> - make the VCLK2 clocks configuration by CCF
> - fixed probe/bind of DSI controller to work with panels & bridges
> - added bit_clk to controller to it can setup the BIT clock aswell
> - added fix for components unbind
> - added fix for analog phy setup value
> - added TS050 timings fix
> - dropped previous clk control patch
>
> Changes from v2 at [2]:
> - Fixed patch 3
> - Added reviews from Jagan
> - Rebased on v5.19-rc1
>
> Changes from v1 at [1]:
> - fixed DSI host bindings
> - add reviewed-by tags for bindings
> - moved magic values to defines thanks to Martin's searches
> - added proper prefixes to defines
> - moved phy_configure to phy_init() dw-mipi-dsi callback
> - moved phy_on to a new phy_power_on() dw-mipi-dsi callback
> - correctly return phy_init/configure errors to callback returns
>
> [1] https://lore.kernel.org/r/[email protected]
> [2] https://lore.kernel.org/r/[email protected]
> [3] https://lore.kernel.org/r/[email protected]
>
> ---
> Neil Armstrong (7):
> dt-bindings: arm: amlogic: Document the MNT Reform 2 CM4 adapter with a BPI-CM4 Module
> clk: meson: add vclk driver
> clk: meson: g12a: make VCLK2 and ENCL clock path configurable by CCF
> drm/meson: gate px_clk when setting rate
> arm64: meson: g12-common: add the MIPI DSI nodes
> arm64: meson: khadas-vim3l: add TS050 DSI panel overlay
> arm64: dts: amlogic: meson-g12b-bananapi-cm4: add support for MNT Reform2 with CM4 adaper
>
> Documentation/devicetree/bindings/arm/amlogic.yaml | 1 +
> arch/arm64/boot/dts/amlogic/Makefile | 5 +
> arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi | 70 ++++
> .../meson-g12b-bananapi-cm4-mnt-reform2.dts | 384 +++++++++++++++++++++
> .../boot/dts/amlogic/meson-khadas-vim3-ts050.dtso | 108 ++++++
> drivers/clk/meson/Kconfig | 5 +
> drivers/clk/meson/Makefile | 1 +
> drivers/clk/meson/g12a.c | 76 ++--
> drivers/clk/meson/vclk.c | 141 ++++++++
> drivers/clk/meson/vclk.h | 51 +++
> drivers/gpu/drm/meson/meson_dw_mipi_dsi.c | 7 +
> 11 files changed, 829 insertions(+), 20 deletions(-)
> ---
> base-commit: 4cece764965020c22cff7665b18a012006359095
> change-id: 20230512-amlogic-v6-4-upstream-dsi-ccf-vim3-b8e5217e1f4a
>
> Best regards,
> --
> Neil Armstrong <[email protected]>
>
>
>
My bot found new DTB warnings on the .dts files added or changed in this
series.
Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.
If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:
pip3 install dtschema --upgrade
New warnings running 'make CHECK_DTBS=y amlogic/meson-g12b-bananapi-cm4-mnt-reform2.dtb' for 20240403-amlogic-v6-4-upstream-dsi-ccf-vim3-v12-0-99ecdfdc87fc@linaro.org:
arch/arm64/boot/dts/amlogic/meson-g12b-bananapi-cm4-mnt-reform2.dtb: /soc/bus@ff600000/bus@42000/clock-controller@0: failed to match any schema with compatible: ['amlogic,g12a-audio-clkc']
arch/arm64/boot/dts/amlogic/meson-g12b-bananapi-cm4-mnt-reform2.dtb: /soc/bus@ff600000/bus@42000/audio-controller@744: failed to match any schema with compatible: ['amlogic,g12a-tohdmitx']
arch/arm64/boot/dts/amlogic/meson-g12b-bananapi-cm4-mnt-reform2.dtb: sys-ctrl@0: '#address-cells', '#size-cells', 'ranges' do not match any of the regexes: 'pinctrl-[0-9]+'
from schema $id: http://devicetree.org/schemas/soc/amlogic/amlogic,meson-gx-hhi-sysctrl.yaml#
arch/arm64/boot/dts/amlogic/meson-g12b-bananapi-cm4-mnt-reform2.dtb: sound: Unevaluated properties are not allowed ('assigned-clock-parents', 'assigned-clock-rates', 'assigned-clocks' were unexpected)
from schema $id: http://devicetree.org/schemas/sound/amlogic,axg-sound-card.yaml#
arch/arm64/boot/dts/amlogic/meson-g12b-bananapi-cm4-mnt-reform2.dtb: sound: 'anyOf' conditional failed, one must be fixed:
'clocks' is a required property
'#clock-cells' is a required property
from schema $id: http://devicetree.org/schemas/clock/clock.yaml#
On Wed 03 Apr 2024 at 09:46, Neil Armstrong <[email protected]> wrote:
> The VCLK and VCLK_DIV clocks have supplementary bits.
>
> The VCLK gate has a "SOFT RESET" bit to toggle after the whole
> VCLK sub-tree rate has been set, this is implemented in
> the gate enable callback.
>
> The VCLK_DIV clocks as enable and reset bits used to disable
> and reset the divider, associated with CLK_SET_RATE_GATE it ensures
> the rate is set while the divider is disabled and in reset mode.
>
> The VCLK_DIV enable bit isn't implemented as a gate since it's part
> of the divider logic and vendor does this exact sequence to ensure
> the divider is correctly set.
The checkpatch warning is still there. Is it a choice or a mistake ?
Documentation says "GPL v2" exists for historic reason which seems to
hint "GPL" is preferred, and I suppose this is why checkpatch warns for
it.
>
> Signed-off-by: Neil Armstrong <[email protected]>
> ---
> drivers/clk/meson/Kconfig | 4 ++
> drivers/clk/meson/Makefile | 1 +
> drivers/clk/meson/vclk.c | 141 +++++++++++++++++++++++++++++++++++++++++++++
> drivers/clk/meson/vclk.h | 51 ++++++++++++++++
> 4 files changed, 197 insertions(+)
>
> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
> index 29ffd14d267b..8a9823789fa3 100644
> --- a/drivers/clk/meson/Kconfig
> +++ b/drivers/clk/meson/Kconfig
> @@ -30,6 +30,10 @@ config COMMON_CLK_MESON_VID_PLL_DIV
> tristate
> select COMMON_CLK_MESON_REGMAP
>
> +config COMMON_CLK_MESON_VCLK
> + tristate
> + select COMMON_CLK_MESON_REGMAP
> +
> config COMMON_CLK_MESON_CLKC_UTILS
> tristate
>
> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
> index 9ee4b954c896..9ba43fe7a07a 100644
> --- a/drivers/clk/meson/Makefile
> +++ b/drivers/clk/meson/Makefile
> @@ -12,6 +12,7 @@ obj-$(CONFIG_COMMON_CLK_MESON_PLL) += clk-pll.o
> obj-$(CONFIG_COMMON_CLK_MESON_REGMAP) += clk-regmap.o
> obj-$(CONFIG_COMMON_CLK_MESON_SCLK_DIV) += sclk-div.o
> obj-$(CONFIG_COMMON_CLK_MESON_VID_PLL_DIV) += vid-pll-div.o
> +obj-$(CONFIG_COMMON_CLK_MESON_VCLK) += vclk.o
>
> # Amlogic Clock controllers
>
> diff --git a/drivers/clk/meson/vclk.c b/drivers/clk/meson/vclk.c
> new file mode 100644
> index 000000000000..45dc216941ea
> --- /dev/null
> +++ b/drivers/clk/meson/vclk.c
> @@ -0,0 +1,141 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2024 Neil Armstrong <[email protected]>
> + */
> +
> +#include <linux/module.h>
> +#include "vclk.h"
> +
> +/* The VCLK gate has a supplementary reset bit to pulse after ungating */
> +
> +static inline struct meson_vclk_gate_data *
> +clk_get_meson_vclk_gate_data(struct clk_regmap *clk)
> +{
> + return (struct meson_vclk_gate_data *)clk->data;
> +}
> +
> +static int meson_vclk_gate_enable(struct clk_hw *hw)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct meson_vclk_gate_data *vclk = clk_get_meson_vclk_gate_data(clk);
> +
> + meson_parm_write(clk->map, &vclk->enable, 1);
> +
> + /* Do a reset pulse */
> + meson_parm_write(clk->map, &vclk->reset, 1);
> + meson_parm_write(clk->map, &vclk->reset, 0);
> +
> + return 0;
> +}
> +
> +static void meson_vclk_gate_disable(struct clk_hw *hw)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct meson_vclk_gate_data *vclk = clk_get_meson_vclk_gate_data(clk);
> +
> + meson_parm_write(clk->map, &vclk->enable, 0);
> +}
> +
> +static int meson_vclk_gate_is_enabled(struct clk_hw *hw)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct meson_vclk_gate_data *vclk = clk_get_meson_vclk_gate_data(clk);
> +
> + return meson_parm_read(clk->map, &vclk->enable);
> +}
> +
> +const struct clk_ops meson_vclk_gate_ops = {
> + .enable = meson_vclk_gate_enable,
> + .disable = meson_vclk_gate_disable,
> + .is_enabled = meson_vclk_gate_is_enabled,
> +};
> +EXPORT_SYMBOL_GPL(meson_vclk_gate_ops);
> +
> +/* The VCLK Divider has supplementary reset & enable bits */
> +
> +static inline struct meson_vclk_div_data *
> +clk_get_meson_vclk_div_data(struct clk_regmap *clk)
> +{
> + return (struct meson_vclk_div_data *)clk->data;
> +}
> +
> +static unsigned long meson_vclk_div_recalc_rate(struct clk_hw *hw,
> + unsigned long prate)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
> +
> + return divider_recalc_rate(hw, prate, meson_parm_read(clk->map, &vclk->div),
> + vclk->table, vclk->flags, vclk->div.width);
> +}
> +
> +static int meson_vclk_div_determine_rate(struct clk_hw *hw,
> + struct clk_rate_request *req)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
> +
> + return divider_determine_rate(hw, req, vclk->table, vclk->div.width,
> + vclk->flags);
> +}
> +
> +static int meson_vclk_div_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
> + int ret;
> +
> + ret = divider_get_val(rate, parent_rate, vclk->table, vclk->div.width,
> + vclk->flags);
> + if (ret < 0)
> + return ret;
> +
> + meson_parm_write(clk->map, &vclk->div, ret);
> +
> + return 0;
> +};
> +
> +static int meson_vclk_div_enable(struct clk_hw *hw)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
> +
> + /* Unreset the divider when ungating */
> + meson_parm_write(clk->map, &vclk->reset, 0);
> + meson_parm_write(clk->map, &vclk->enable, 1);
> +
> + return 0;
> +}
> +
> +static void meson_vclk_div_disable(struct clk_hw *hw)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
> +
> + /* Reset the divider when gating */
> + meson_parm_write(clk->map, &vclk->enable, 0);
> + meson_parm_write(clk->map, &vclk->reset, 1);
> +}
> +
> +static int meson_vclk_div_is_enabled(struct clk_hw *hw)
> +{
> + struct clk_regmap *clk = to_clk_regmap(hw);
> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
> +
> + return meson_parm_read(clk->map, &vclk->enable);
> +}
> +
> +const struct clk_ops meson_vclk_div_ops = {
> + .recalc_rate = meson_vclk_div_recalc_rate,
> + .determine_rate = meson_vclk_div_determine_rate,
> + .set_rate = meson_vclk_div_set_rate,
> + .enable = meson_vclk_div_enable,
> + .disable = meson_vclk_div_disable,
> + .is_enabled = meson_vclk_div_is_enabled,
> +};
> +EXPORT_SYMBOL_GPL(meson_vclk_div_ops);
> +
> +MODULE_DESCRIPTION("Amlogic vclk clock driver");
> +MODULE_AUTHOR("Neil Armstrong <[email protected]>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/clk/meson/vclk.h b/drivers/clk/meson/vclk.h
> new file mode 100644
> index 000000000000..20b0b181db09
> --- /dev/null
> +++ b/drivers/clk/meson/vclk.h
> @@ -0,0 +1,51 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2024 Neil Armstrong <[email protected]>
> + */
> +
> +#ifndef __VCLK_H
> +#define __VCLK_H
> +
> +#include "clk-regmap.h"
> +#include "parm.h"
> +
> +/**
> + * struct meson_vclk_gate_data - vclk_gate regmap backed specific data
> + *
> + * @enable: vclk enable field
> + * @reset: vclk reset field
> + * @flags: hardware-specific flags
> + *
> + * Flags:
> + * Same as clk_gate except CLK_GATE_HIWORD_MASK which is ignored
> + */
> +struct meson_vclk_gate_data {
> + struct parm enable;
> + struct parm reset;
> + u8 flags;
> +};
> +
> +extern const struct clk_ops meson_vclk_gate_ops;
> +
> +/**
> + * struct meson_vclk_div_data - vclk_div regmap back specific data
> + *
> + * @div: divider field
> + * @enable: vclk divider enable field
> + * @reset: vclk divider reset field
> + * @table: array of value/divider pairs, last entry should have div = 0
> + *
> + * Flags:
> + * Same as clk_divider except CLK_DIVIDER_HIWORD_MASK which is ignored
> + */
> +struct meson_vclk_div_data {
> + struct parm div;
> + struct parm enable;
> + struct parm reset;
> + const struct clk_div_table *table;
> + u8 flags;
> +};
> +
> +extern const struct clk_ops meson_vclk_div_ops;
> +
> +#endif /* __VCLK_H */
--
Jerome
On 04/04/2024 10:13, Jerome Brunet wrote:
>
> On Wed 03 Apr 2024 at 09:46, Neil Armstrong <[email protected]> wrote:
>
>> The VCLK and VCLK_DIV clocks have supplementary bits.
>>
>> The VCLK gate has a "SOFT RESET" bit to toggle after the whole
>> VCLK sub-tree rate has been set, this is implemented in
>> the gate enable callback.
>>
>> The VCLK_DIV clocks as enable and reset bits used to disable
>> and reset the divider, associated with CLK_SET_RATE_GATE it ensures
>> the rate is set while the divider is disabled and in reset mode.
>>
>> The VCLK_DIV enable bit isn't implemented as a gate since it's part
>> of the divider logic and vendor does this exact sequence to ensure
>> the divider is correctly set.
>
> The checkpatch warning is still there. Is it a choice or a mistake ?
>
> Documentation says "GPL v2" exists for historic reason which seems to
> hint "GPL" is preferred, and I suppose this is why checkpatch warns for
> it.
Well I didn't see this warning, this is what I fixed:
$ scripts/checkpatch.pl --strict drivers/clk/meson/vclk.c
CHECK: Alignment should match open parenthesis
#63: FILE: drivers/clk/meson/vclk.c:63:
+static unsigned long meson_vclk_div_recalc_rate(struct clk_hw *hw,
+ unsigned long prate)
CHECK: Alignment should match open parenthesis
#73: FILE: drivers/clk/meson/vclk.c:73:
+static int meson_vclk_div_determine_rate(struct clk_hw *hw,
+ struct clk_rate_request *req)
CHECK: Alignment should match open parenthesis
#83: FILE: drivers/clk/meson/vclk.c:83:
+static int meson_vclk_div_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
<snip>
It seems that checking a commit triggers an extra check....
$ scripts/checkpatch.pl --strict -G 1bac9f6aa3c3
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#58:
new file mode 100644
<snip>
WARNING: Prefer "GPL" over "GPL v2" - see commit bf7fbeeae6db ("module: Cure the MODULE_LICENSE "GPL" vs. "GPL v2" bogosity")
#203: FILE: drivers/clk/meson/vclk.c:141:
+MODULE_LICENSE("GPL v2");
<snip>
Neil
>
>>
>> Signed-off-by: Neil Armstrong <[email protected]>
>> ---
>> drivers/clk/meson/Kconfig | 4 ++
>> drivers/clk/meson/Makefile | 1 +
>> drivers/clk/meson/vclk.c | 141 +++++++++++++++++++++++++++++++++++++++++++++
>> drivers/clk/meson/vclk.h | 51 ++++++++++++++++
>> 4 files changed, 197 insertions(+)
>>
>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
>> index 29ffd14d267b..8a9823789fa3 100644
>> --- a/drivers/clk/meson/Kconfig
>> +++ b/drivers/clk/meson/Kconfig
>> @@ -30,6 +30,10 @@ config COMMON_CLK_MESON_VID_PLL_DIV
>> tristate
>> select COMMON_CLK_MESON_REGMAP
>>
>> +config COMMON_CLK_MESON_VCLK
>> + tristate
>> + select COMMON_CLK_MESON_REGMAP
>> +
>> config COMMON_CLK_MESON_CLKC_UTILS
>> tristate
>>
>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>> index 9ee4b954c896..9ba43fe7a07a 100644
>> --- a/drivers/clk/meson/Makefile
>> +++ b/drivers/clk/meson/Makefile
>> @@ -12,6 +12,7 @@ obj-$(CONFIG_COMMON_CLK_MESON_PLL) += clk-pll.o
>> obj-$(CONFIG_COMMON_CLK_MESON_REGMAP) += clk-regmap.o
>> obj-$(CONFIG_COMMON_CLK_MESON_SCLK_DIV) += sclk-div.o
>> obj-$(CONFIG_COMMON_CLK_MESON_VID_PLL_DIV) += vid-pll-div.o
>> +obj-$(CONFIG_COMMON_CLK_MESON_VCLK) += vclk.o
>>
>> # Amlogic Clock controllers
>>
>> diff --git a/drivers/clk/meson/vclk.c b/drivers/clk/meson/vclk.c
>> new file mode 100644
>> index 000000000000..45dc216941ea
>> --- /dev/null
>> +++ b/drivers/clk/meson/vclk.c
>> @@ -0,0 +1,141 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2024 Neil Armstrong <[email protected]>
>> + */
>> +
>> +#include <linux/module.h>
>> +#include "vclk.h"
>> +
>> +/* The VCLK gate has a supplementary reset bit to pulse after ungating */
>> +
>> +static inline struct meson_vclk_gate_data *
>> +clk_get_meson_vclk_gate_data(struct clk_regmap *clk)
>> +{
>> + return (struct meson_vclk_gate_data *)clk->data;
>> +}
>> +
>> +static int meson_vclk_gate_enable(struct clk_hw *hw)
>> +{
>> + struct clk_regmap *clk = to_clk_regmap(hw);
>> + struct meson_vclk_gate_data *vclk = clk_get_meson_vclk_gate_data(clk);
>> +
>> + meson_parm_write(clk->map, &vclk->enable, 1);
>> +
>> + /* Do a reset pulse */
>> + meson_parm_write(clk->map, &vclk->reset, 1);
>> + meson_parm_write(clk->map, &vclk->reset, 0);
>> +
>> + return 0;
>> +}
>> +
>> +static void meson_vclk_gate_disable(struct clk_hw *hw)
>> +{
>> + struct clk_regmap *clk = to_clk_regmap(hw);
>> + struct meson_vclk_gate_data *vclk = clk_get_meson_vclk_gate_data(clk);
>> +
>> + meson_parm_write(clk->map, &vclk->enable, 0);
>> +}
>> +
>> +static int meson_vclk_gate_is_enabled(struct clk_hw *hw)
>> +{
>> + struct clk_regmap *clk = to_clk_regmap(hw);
>> + struct meson_vclk_gate_data *vclk = clk_get_meson_vclk_gate_data(clk);
>> +
>> + return meson_parm_read(clk->map, &vclk->enable);
>> +}
>> +
>> +const struct clk_ops meson_vclk_gate_ops = {
>> + .enable = meson_vclk_gate_enable,
>> + .disable = meson_vclk_gate_disable,
>> + .is_enabled = meson_vclk_gate_is_enabled,
>> +};
>> +EXPORT_SYMBOL_GPL(meson_vclk_gate_ops);
>> +
>> +/* The VCLK Divider has supplementary reset & enable bits */
>> +
>> +static inline struct meson_vclk_div_data *
>> +clk_get_meson_vclk_div_data(struct clk_regmap *clk)
>> +{
>> + return (struct meson_vclk_div_data *)clk->data;
>> +}
>> +
>> +static unsigned long meson_vclk_div_recalc_rate(struct clk_hw *hw,
>> + unsigned long prate)
>> +{
>> + struct clk_regmap *clk = to_clk_regmap(hw);
>> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
>> +
>> + return divider_recalc_rate(hw, prate, meson_parm_read(clk->map, &vclk->div),
>> + vclk->table, vclk->flags, vclk->div.width);
>> +}
>> +
>> +static int meson_vclk_div_determine_rate(struct clk_hw *hw,
>> + struct clk_rate_request *req)
>> +{
>> + struct clk_regmap *clk = to_clk_regmap(hw);
>> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
>> +
>> + return divider_determine_rate(hw, req, vclk->table, vclk->div.width,
>> + vclk->flags);
>> +}
>> +
>> +static int meson_vclk_div_set_rate(struct clk_hw *hw, unsigned long rate,
>> + unsigned long parent_rate)
>> +{
>> + struct clk_regmap *clk = to_clk_regmap(hw);
>> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
>> + int ret;
>> +
>> + ret = divider_get_val(rate, parent_rate, vclk->table, vclk->div.width,
>> + vclk->flags);
>> + if (ret < 0)
>> + return ret;
>> +
>> + meson_parm_write(clk->map, &vclk->div, ret);
>> +
>> + return 0;
>> +};
>> +
>> +static int meson_vclk_div_enable(struct clk_hw *hw)
>> +{
>> + struct clk_regmap *clk = to_clk_regmap(hw);
>> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
>> +
>> + /* Unreset the divider when ungating */
>> + meson_parm_write(clk->map, &vclk->reset, 0);
>> + meson_parm_write(clk->map, &vclk->enable, 1);
>> +
>> + return 0;
>> +}
>> +
>> +static void meson_vclk_div_disable(struct clk_hw *hw)
>> +{
>> + struct clk_regmap *clk = to_clk_regmap(hw);
>> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
>> +
>> + /* Reset the divider when gating */
>> + meson_parm_write(clk->map, &vclk->enable, 0);
>> + meson_parm_write(clk->map, &vclk->reset, 1);
>> +}
>> +
>> +static int meson_vclk_div_is_enabled(struct clk_hw *hw)
>> +{
>> + struct clk_regmap *clk = to_clk_regmap(hw);
>> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
>> +
>> + return meson_parm_read(clk->map, &vclk->enable);
>> +}
>> +
>> +const struct clk_ops meson_vclk_div_ops = {
>> + .recalc_rate = meson_vclk_div_recalc_rate,
>> + .determine_rate = meson_vclk_div_determine_rate,
>> + .set_rate = meson_vclk_div_set_rate,
>> + .enable = meson_vclk_div_enable,
>> + .disable = meson_vclk_div_disable,
>> + .is_enabled = meson_vclk_div_is_enabled,
>> +};
>> +EXPORT_SYMBOL_GPL(meson_vclk_div_ops);
>> +
>> +MODULE_DESCRIPTION("Amlogic vclk clock driver");
>> +MODULE_AUTHOR("Neil Armstrong <[email protected]>");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/clk/meson/vclk.h b/drivers/clk/meson/vclk.h
>> new file mode 100644
>> index 000000000000..20b0b181db09
>> --- /dev/null
>> +++ b/drivers/clk/meson/vclk.h
>> @@ -0,0 +1,51 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Copyright (c) 2024 Neil Armstrong <[email protected]>
>> + */
>> +
>> +#ifndef __VCLK_H
>> +#define __VCLK_H
>> +
>> +#include "clk-regmap.h"
>> +#include "parm.h"
>> +
>> +/**
>> + * struct meson_vclk_gate_data - vclk_gate regmap backed specific data
>> + *
>> + * @enable: vclk enable field
>> + * @reset: vclk reset field
>> + * @flags: hardware-specific flags
>> + *
>> + * Flags:
>> + * Same as clk_gate except CLK_GATE_HIWORD_MASK which is ignored
>> + */
>> +struct meson_vclk_gate_data {
>> + struct parm enable;
>> + struct parm reset;
>> + u8 flags;
>> +};
>> +
>> +extern const struct clk_ops meson_vclk_gate_ops;
>> +
>> +/**
>> + * struct meson_vclk_div_data - vclk_div regmap back specific data
>> + *
>> + * @div: divider field
>> + * @enable: vclk divider enable field
>> + * @reset: vclk divider reset field
>> + * @table: array of value/divider pairs, last entry should have div = 0
>> + *
>> + * Flags:
>> + * Same as clk_divider except CLK_DIVIDER_HIWORD_MASK which is ignored
>> + */
>> +struct meson_vclk_div_data {
>> + struct parm div;
>> + struct parm enable;
>> + struct parm reset;
>> + const struct clk_div_table *table;
>> + u8 flags;
>> +};
>> +
>> +extern const struct clk_ops meson_vclk_div_ops;
>> +
>> +#endif /* __VCLK_H */
>
>
On Thu 04 Apr 2024 at 18:59, Neil Armstrong <[email protected]> wrote:
> On 04/04/2024 10:13, Jerome Brunet wrote:
>> On Wed 03 Apr 2024 at 09:46, Neil Armstrong <[email protected]>
>> wrote:
>>
>>> The VCLK and VCLK_DIV clocks have supplementary bits.
>>>
>>> The VCLK gate has a "SOFT RESET" bit to toggle after the whole
>>> VCLK sub-tree rate has been set, this is implemented in
>>> the gate enable callback.
>>>
>>> The VCLK_DIV clocks as enable and reset bits used to disable
>>> and reset the divider, associated with CLK_SET_RATE_GATE it ensures
>>> the rate is set while the divider is disabled and in reset mode.
>>>
>>> The VCLK_DIV enable bit isn't implemented as a gate since it's part
>>> of the divider logic and vendor does this exact sequence to ensure
>>> the divider is correctly set.
>> The checkpatch warning is still there. Is it a choice or a mistake ?
>> Documentation says "GPL v2" exists for historic reason which seems to
>> hint "GPL" is preferred, and I suppose this is why checkpatch warns for
>> it.
>
> Well I didn't see this warning, this is what I fixed:
>
> $ scripts/checkpatch.pl --strict drivers/clk/meson/vclk.c
> CHECK: Alignment should match open parenthesis
> #63: FILE: drivers/clk/meson/vclk.c:63:
> +static unsigned long meson_vclk_div_recalc_rate(struct clk_hw *hw,
> + unsigned long prate)
>
> CHECK: Alignment should match open parenthesis
> #73: FILE: drivers/clk/meson/vclk.c:73:
> +static int meson_vclk_div_determine_rate(struct clk_hw *hw,
> + struct clk_rate_request *req)
>
> CHECK: Alignment should match open parenthesis
> #83: FILE: drivers/clk/meson/vclk.c:83:
> +static int meson_vclk_div_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
>
I would not ask a respin solely for this. It's nice to fix it but I was
mostly after the warning TBH.
> <snip>
>
> It seems that checking a commit triggers an extra check....
>
> $ scripts/checkpatch.pl --strict -G 1bac9f6aa3c3
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #58:
> new file mode 100644
>
> <snip>
>
> WARNING: Prefer "GPL" over "GPL v2" - see commit bf7fbeeae6db ("module: Cure the MODULE_LICENSE "GPL" vs. "GPL v2" bogosity")
> #203: FILE: drivers/clk/meson/vclk.c:141:
> +MODULE_LICENSE("GPL v2");
Hum, I'm running checkpatch against the mail itself, not the commit. I
still get the warning
>
> <snip>
>
> Neil
>
>>
>>>
>>> Signed-off-by: Neil Armstrong <[email protected]>
>>> ---
>>> drivers/clk/meson/Kconfig | 4 ++
>>> drivers/clk/meson/Makefile | 1 +
>>> drivers/clk/meson/vclk.c | 141 +++++++++++++++++++++++++++++++++++++++++++++
>>> drivers/clk/meson/vclk.h | 51 ++++++++++++++++
>>> 4 files changed, 197 insertions(+)
>>>
>>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
>>> index 29ffd14d267b..8a9823789fa3 100644
>>> --- a/drivers/clk/meson/Kconfig
>>> +++ b/drivers/clk/meson/Kconfig
>>> @@ -30,6 +30,10 @@ config COMMON_CLK_MESON_VID_PLL_DIV
>>> tristate
>>> select COMMON_CLK_MESON_REGMAP
>>> +config COMMON_CLK_MESON_VCLK
>>> + tristate
>>> + select COMMON_CLK_MESON_REGMAP
>>> +
>>> config COMMON_CLK_MESON_CLKC_UTILS
>>> tristate
>>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>>> index 9ee4b954c896..9ba43fe7a07a 100644
>>> --- a/drivers/clk/meson/Makefile
>>> +++ b/drivers/clk/meson/Makefile
>>> @@ -12,6 +12,7 @@ obj-$(CONFIG_COMMON_CLK_MESON_PLL) += clk-pll.o
>>> obj-$(CONFIG_COMMON_CLK_MESON_REGMAP) += clk-regmap.o
>>> obj-$(CONFIG_COMMON_CLK_MESON_SCLK_DIV) += sclk-div.o
>>> obj-$(CONFIG_COMMON_CLK_MESON_VID_PLL_DIV) += vid-pll-div.o
>>> +obj-$(CONFIG_COMMON_CLK_MESON_VCLK) += vclk.o
>>> # Amlogic Clock controllers
>>> diff --git a/drivers/clk/meson/vclk.c b/drivers/clk/meson/vclk.c
>>> new file mode 100644
>>> index 000000000000..45dc216941ea
>>> --- /dev/null
>>> +++ b/drivers/clk/meson/vclk.c
>>> @@ -0,0 +1,141 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright (c) 2024 Neil Armstrong <[email protected]>
>>> + */
>>> +
>>> +#include <linux/module.h>
>>> +#include "vclk.h"
>>> +
>>> +/* The VCLK gate has a supplementary reset bit to pulse after ungating */
>>> +
>>> +static inline struct meson_vclk_gate_data *
>>> +clk_get_meson_vclk_gate_data(struct clk_regmap *clk)
>>> +{
>>> + return (struct meson_vclk_gate_data *)clk->data;
>>> +}
>>> +
>>> +static int meson_vclk_gate_enable(struct clk_hw *hw)
>>> +{
>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>> + struct meson_vclk_gate_data *vclk = clk_get_meson_vclk_gate_data(clk);
>>> +
>>> + meson_parm_write(clk->map, &vclk->enable, 1);
>>> +
>>> + /* Do a reset pulse */
>>> + meson_parm_write(clk->map, &vclk->reset, 1);
>>> + meson_parm_write(clk->map, &vclk->reset, 0);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void meson_vclk_gate_disable(struct clk_hw *hw)
>>> +{
>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>> + struct meson_vclk_gate_data *vclk = clk_get_meson_vclk_gate_data(clk);
>>> +
>>> + meson_parm_write(clk->map, &vclk->enable, 0);
>>> +}
>>> +
>>> +static int meson_vclk_gate_is_enabled(struct clk_hw *hw)
>>> +{
>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>> + struct meson_vclk_gate_data *vclk = clk_get_meson_vclk_gate_data(clk);
>>> +
>>> + return meson_parm_read(clk->map, &vclk->enable);
>>> +}
>>> +
>>> +const struct clk_ops meson_vclk_gate_ops = {
>>> + .enable = meson_vclk_gate_enable,
>>> + .disable = meson_vclk_gate_disable,
>>> + .is_enabled = meson_vclk_gate_is_enabled,
>>> +};
>>> +EXPORT_SYMBOL_GPL(meson_vclk_gate_ops);
>>> +
>>> +/* The VCLK Divider has supplementary reset & enable bits */
>>> +
>>> +static inline struct meson_vclk_div_data *
>>> +clk_get_meson_vclk_div_data(struct clk_regmap *clk)
>>> +{
>>> + return (struct meson_vclk_div_data *)clk->data;
>>> +}
>>> +
>>> +static unsigned long meson_vclk_div_recalc_rate(struct clk_hw *hw,
>>> + unsigned long prate)
>>> +{
>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
>>> +
>>> + return divider_recalc_rate(hw, prate, meson_parm_read(clk->map, &vclk->div),
>>> + vclk->table, vclk->flags, vclk->div.width);
>>> +}
>>> +
>>> +static int meson_vclk_div_determine_rate(struct clk_hw *hw,
>>> + struct clk_rate_request *req)
>>> +{
>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
>>> +
>>> + return divider_determine_rate(hw, req, vclk->table, vclk->div.width,
>>> + vclk->flags);
>>> +}
>>> +
>>> +static int meson_vclk_div_set_rate(struct clk_hw *hw, unsigned long rate,
>>> + unsigned long parent_rate)
>>> +{
>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
>>> + int ret;
>>> +
>>> + ret = divider_get_val(rate, parent_rate, vclk->table, vclk->div.width,
>>> + vclk->flags);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + meson_parm_write(clk->map, &vclk->div, ret);
>>> +
>>> + return 0;
>>> +};
>>> +
>>> +static int meson_vclk_div_enable(struct clk_hw *hw)
>>> +{
>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
>>> +
>>> + /* Unreset the divider when ungating */
>>> + meson_parm_write(clk->map, &vclk->reset, 0);
>>> + meson_parm_write(clk->map, &vclk->enable, 1);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void meson_vclk_div_disable(struct clk_hw *hw)
>>> +{
>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
>>> +
>>> + /* Reset the divider when gating */
>>> + meson_parm_write(clk->map, &vclk->enable, 0);
>>> + meson_parm_write(clk->map, &vclk->reset, 1);
>>> +}
>>> +
>>> +static int meson_vclk_div_is_enabled(struct clk_hw *hw)
>>> +{
>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
>>> +
>>> + return meson_parm_read(clk->map, &vclk->enable);
>>> +}
>>> +
>>> +const struct clk_ops meson_vclk_div_ops = {
>>> + .recalc_rate = meson_vclk_div_recalc_rate,
>>> + .determine_rate = meson_vclk_div_determine_rate,
>>> + .set_rate = meson_vclk_div_set_rate,
>>> + .enable = meson_vclk_div_enable,
>>> + .disable = meson_vclk_div_disable,
>>> + .is_enabled = meson_vclk_div_is_enabled,
>>> +};
>>> +EXPORT_SYMBOL_GPL(meson_vclk_div_ops);
>>> +
>>> +MODULE_DESCRIPTION("Amlogic vclk clock driver");
>>> +MODULE_AUTHOR("Neil Armstrong <[email protected]>");
>>> +MODULE_LICENSE("GPL v2");
>>> diff --git a/drivers/clk/meson/vclk.h b/drivers/clk/meson/vclk.h
>>> new file mode 100644
>>> index 000000000000..20b0b181db09
>>> --- /dev/null
>>> +++ b/drivers/clk/meson/vclk.h
>>> @@ -0,0 +1,51 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +/*
>>> + * Copyright (c) 2024 Neil Armstrong <[email protected]>
>>> + */
>>> +
>>> +#ifndef __VCLK_H
>>> +#define __VCLK_H
>>> +
>>> +#include "clk-regmap.h"
>>> +#include "parm.h"
>>> +
>>> +/**
>>> + * struct meson_vclk_gate_data - vclk_gate regmap backed specific data
>>> + *
>>> + * @enable: vclk enable field
>>> + * @reset: vclk reset field
>>> + * @flags: hardware-specific flags
>>> + *
>>> + * Flags:
>>> + * Same as clk_gate except CLK_GATE_HIWORD_MASK which is ignored
>>> + */
>>> +struct meson_vclk_gate_data {
>>> + struct parm enable;
>>> + struct parm reset;
>>> + u8 flags;
>>> +};
>>> +
>>> +extern const struct clk_ops meson_vclk_gate_ops;
>>> +
>>> +/**
>>> + * struct meson_vclk_div_data - vclk_div regmap back specific data
>>> + *
>>> + * @div: divider field
>>> + * @enable: vclk divider enable field
>>> + * @reset: vclk divider reset field
>>> + * @table: array of value/divider pairs, last entry should have div = 0
>>> + *
>>> + * Flags:
>>> + * Same as clk_divider except CLK_DIVIDER_HIWORD_MASK which is ignored
>>> + */
>>> +struct meson_vclk_div_data {
>>> + struct parm div;
>>> + struct parm enable;
>>> + struct parm reset;
>>> + const struct clk_div_table *table;
>>> + u8 flags;
>>> +};
>>> +
>>> +extern const struct clk_ops meson_vclk_div_ops;
>>> +
>>> +#endif /* __VCLK_H */
>>
--
Jerome
On 05/04/2024 09:00, Jerome Brunet wrote:
>
> On Thu 04 Apr 2024 at 18:59, Neil Armstrong <[email protected]> wrote:
>
>> On 04/04/2024 10:13, Jerome Brunet wrote:
>>> On Wed 03 Apr 2024 at 09:46, Neil Armstrong <[email protected]>
>>> wrote:
>>>
>>>> The VCLK and VCLK_DIV clocks have supplementary bits.
>>>>
>>>> The VCLK gate has a "SOFT RESET" bit to toggle after the whole
>>>> VCLK sub-tree rate has been set, this is implemented in
>>>> the gate enable callback.
>>>>
>>>> The VCLK_DIV clocks as enable and reset bits used to disable
>>>> and reset the divider, associated with CLK_SET_RATE_GATE it ensures
>>>> the rate is set while the divider is disabled and in reset mode.
>>>>
>>>> The VCLK_DIV enable bit isn't implemented as a gate since it's part
>>>> of the divider logic and vendor does this exact sequence to ensure
>>>> the divider is correctly set.
>>> The checkpatch warning is still there. Is it a choice or a mistake ?
>>> Documentation says "GPL v2" exists for historic reason which seems to
>>> hint "GPL" is preferred, and I suppose this is why checkpatch warns for
>>> it.
>>
>> Well I didn't see this warning, this is what I fixed:
>>
>> $ scripts/checkpatch.pl --strict drivers/clk/meson/vclk.c
>> CHECK: Alignment should match open parenthesis
>> #63: FILE: drivers/clk/meson/vclk.c:63:
>> +static unsigned long meson_vclk_div_recalc_rate(struct clk_hw *hw,
>> + unsigned long prate)
>>
>> CHECK: Alignment should match open parenthesis
>> #73: FILE: drivers/clk/meson/vclk.c:73:
>> +static int meson_vclk_div_determine_rate(struct clk_hw *hw,
>> + struct clk_rate_request *req)
>>
>> CHECK: Alignment should match open parenthesis
>> #83: FILE: drivers/clk/meson/vclk.c:83:
>> +static int meson_vclk_div_set_rate(struct clk_hw *hw, unsigned long rate,
>> + unsigned long parent_rate)
>>
>
> I would not ask a respin solely for this. It's nice to fix it but I was
> mostly after the warning TBH.
>
>> <snip>
>>
>> It seems that checking a commit triggers an extra check....
>>
>> $ scripts/checkpatch.pl --strict -G 1bac9f6aa3c3
>> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
>> #58:
>> new file mode 100644
>>
>> <snip>
>>
>> WARNING: Prefer "GPL" over "GPL v2" - see commit bf7fbeeae6db ("module: Cure the MODULE_LICENSE "GPL" vs. "GPL v2" bogosity")
>> #203: FILE: drivers/clk/meson/vclk.c:141:
>> +MODULE_LICENSE("GPL v2");
>
> Hum, I'm running checkpatch against the mail itself, not the commit. I
> still get the warning
Patch or commit seems to trigger more tests than a file directly, anyway I sent a follow-up patch:
https://lore.kernel.org/all/20240408-amlogic-v6-9-upstream-fix-clk-module-license-v1-1-366ddc0f3db9@linaro.org/
Thanks,
Neil
>
>>
>> <snip>
>>
>> Neil
>>
>>>
>>>>
>>>> Signed-off-by: Neil Armstrong <[email protected]>
>>>> ---
>>>> drivers/clk/meson/Kconfig | 4 ++
>>>> drivers/clk/meson/Makefile | 1 +
>>>> drivers/clk/meson/vclk.c | 141 +++++++++++++++++++++++++++++++++++++++++++++
>>>> drivers/clk/meson/vclk.h | 51 ++++++++++++++++
>>>> 4 files changed, 197 insertions(+)
>>>>
>>>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
>>>> index 29ffd14d267b..8a9823789fa3 100644
>>>> --- a/drivers/clk/meson/Kconfig
>>>> +++ b/drivers/clk/meson/Kconfig
>>>> @@ -30,6 +30,10 @@ config COMMON_CLK_MESON_VID_PLL_DIV
>>>> tristate
>>>> select COMMON_CLK_MESON_REGMAP
>>>> +config COMMON_CLK_MESON_VCLK
>>>> + tristate
>>>> + select COMMON_CLK_MESON_REGMAP
>>>> +
>>>> config COMMON_CLK_MESON_CLKC_UTILS
>>>> tristate
>>>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>>>> index 9ee4b954c896..9ba43fe7a07a 100644
>>>> --- a/drivers/clk/meson/Makefile
>>>> +++ b/drivers/clk/meson/Makefile
>>>> @@ -12,6 +12,7 @@ obj-$(CONFIG_COMMON_CLK_MESON_PLL) += clk-pll.o
>>>> obj-$(CONFIG_COMMON_CLK_MESON_REGMAP) += clk-regmap.o
>>>> obj-$(CONFIG_COMMON_CLK_MESON_SCLK_DIV) += sclk-div.o
>>>> obj-$(CONFIG_COMMON_CLK_MESON_VID_PLL_DIV) += vid-pll-div.o
>>>> +obj-$(CONFIG_COMMON_CLK_MESON_VCLK) += vclk.o
>>>> # Amlogic Clock controllers
>>>> diff --git a/drivers/clk/meson/vclk.c b/drivers/clk/meson/vclk.c
>>>> new file mode 100644
>>>> index 000000000000..45dc216941ea
>>>> --- /dev/null
>>>> +++ b/drivers/clk/meson/vclk.c
>>>> @@ -0,0 +1,141 @@
>>>> +// SPDX-License-Identifier: GPL-2.0
>>>> +/*
>>>> + * Copyright (c) 2024 Neil Armstrong <[email protected]>
>>>> + */
>>>> +
>>>> +#include <linux/module.h>
>>>> +#include "vclk.h"
>>>> +
>>>> +/* The VCLK gate has a supplementary reset bit to pulse after ungating */
>>>> +
>>>> +static inline struct meson_vclk_gate_data *
>>>> +clk_get_meson_vclk_gate_data(struct clk_regmap *clk)
>>>> +{
>>>> + return (struct meson_vclk_gate_data *)clk->data;
>>>> +}
>>>> +
>>>> +static int meson_vclk_gate_enable(struct clk_hw *hw)
>>>> +{
>>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>>> + struct meson_vclk_gate_data *vclk = clk_get_meson_vclk_gate_data(clk);
>>>> +
>>>> + meson_parm_write(clk->map, &vclk->enable, 1);
>>>> +
>>>> + /* Do a reset pulse */
>>>> + meson_parm_write(clk->map, &vclk->reset, 1);
>>>> + meson_parm_write(clk->map, &vclk->reset, 0);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static void meson_vclk_gate_disable(struct clk_hw *hw)
>>>> +{
>>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>>> + struct meson_vclk_gate_data *vclk = clk_get_meson_vclk_gate_data(clk);
>>>> +
>>>> + meson_parm_write(clk->map, &vclk->enable, 0);
>>>> +}
>>>> +
>>>> +static int meson_vclk_gate_is_enabled(struct clk_hw *hw)
>>>> +{
>>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>>> + struct meson_vclk_gate_data *vclk = clk_get_meson_vclk_gate_data(clk);
>>>> +
>>>> + return meson_parm_read(clk->map, &vclk->enable);
>>>> +}
>>>> +
>>>> +const struct clk_ops meson_vclk_gate_ops = {
>>>> + .enable = meson_vclk_gate_enable,
>>>> + .disable = meson_vclk_gate_disable,
>>>> + .is_enabled = meson_vclk_gate_is_enabled,
>>>> +};
>>>> +EXPORT_SYMBOL_GPL(meson_vclk_gate_ops);
>>>> +
>>>> +/* The VCLK Divider has supplementary reset & enable bits */
>>>> +
>>>> +static inline struct meson_vclk_div_data *
>>>> +clk_get_meson_vclk_div_data(struct clk_regmap *clk)
>>>> +{
>>>> + return (struct meson_vclk_div_data *)clk->data;
>>>> +}
>>>> +
>>>> +static unsigned long meson_vclk_div_recalc_rate(struct clk_hw *hw,
>>>> + unsigned long prate)
>>>> +{
>>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>>> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
>>>> +
>>>> + return divider_recalc_rate(hw, prate, meson_parm_read(clk->map, &vclk->div),
>>>> + vclk->table, vclk->flags, vclk->div.width);
>>>> +}
>>>> +
>>>> +static int meson_vclk_div_determine_rate(struct clk_hw *hw,
>>>> + struct clk_rate_request *req)
>>>> +{
>>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>>> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
>>>> +
>>>> + return divider_determine_rate(hw, req, vclk->table, vclk->div.width,
>>>> + vclk->flags);
>>>> +}
>>>> +
>>>> +static int meson_vclk_div_set_rate(struct clk_hw *hw, unsigned long rate,
>>>> + unsigned long parent_rate)
>>>> +{
>>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>>> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
>>>> + int ret;
>>>> +
>>>> + ret = divider_get_val(rate, parent_rate, vclk->table, vclk->div.width,
>>>> + vclk->flags);
>>>> + if (ret < 0)
>>>> + return ret;
>>>> +
>>>> + meson_parm_write(clk->map, &vclk->div, ret);
>>>> +
>>>> + return 0;
>>>> +};
>>>> +
>>>> +static int meson_vclk_div_enable(struct clk_hw *hw)
>>>> +{
>>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>>> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
>>>> +
>>>> + /* Unreset the divider when ungating */
>>>> + meson_parm_write(clk->map, &vclk->reset, 0);
>>>> + meson_parm_write(clk->map, &vclk->enable, 1);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static void meson_vclk_div_disable(struct clk_hw *hw)
>>>> +{
>>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>>> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
>>>> +
>>>> + /* Reset the divider when gating */
>>>> + meson_parm_write(clk->map, &vclk->enable, 0);
>>>> + meson_parm_write(clk->map, &vclk->reset, 1);
>>>> +}
>>>> +
>>>> +static int meson_vclk_div_is_enabled(struct clk_hw *hw)
>>>> +{
>>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>>> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
>>>> +
>>>> + return meson_parm_read(clk->map, &vclk->enable);
>>>> +}
>>>> +
>>>> +const struct clk_ops meson_vclk_div_ops = {
>>>> + .recalc_rate = meson_vclk_div_recalc_rate,
>>>> + .determine_rate = meson_vclk_div_determine_rate,
>>>> + .set_rate = meson_vclk_div_set_rate,
>>>> + .enable = meson_vclk_div_enable,
>>>> + .disable = meson_vclk_div_disable,
>>>> + .is_enabled = meson_vclk_div_is_enabled,
>>>> +};
>>>> +EXPORT_SYMBOL_GPL(meson_vclk_div_ops);
>>>> +
>>>> +MODULE_DESCRIPTION("Amlogic vclk clock driver");
>>>> +MODULE_AUTHOR("Neil Armstrong <[email protected]>");
>>>> +MODULE_LICENSE("GPL v2");
>>>> diff --git a/drivers/clk/meson/vclk.h b/drivers/clk/meson/vclk.h
>>>> new file mode 100644
>>>> index 000000000000..20b0b181db09
>>>> --- /dev/null
>>>> +++ b/drivers/clk/meson/vclk.h
>>>> @@ -0,0 +1,51 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>>> +/*
>>>> + * Copyright (c) 2024 Neil Armstrong <[email protected]>
>>>> + */
>>>> +
>>>> +#ifndef __VCLK_H
>>>> +#define __VCLK_H
>>>> +
>>>> +#include "clk-regmap.h"
>>>> +#include "parm.h"
>>>> +
>>>> +/**
>>>> + * struct meson_vclk_gate_data - vclk_gate regmap backed specific data
>>>> + *
>>>> + * @enable: vclk enable field
>>>> + * @reset: vclk reset field
>>>> + * @flags: hardware-specific flags
>>>> + *
>>>> + * Flags:
>>>> + * Same as clk_gate except CLK_GATE_HIWORD_MASK which is ignored
>>>> + */
>>>> +struct meson_vclk_gate_data {
>>>> + struct parm enable;
>>>> + struct parm reset;
>>>> + u8 flags;
>>>> +};
>>>> +
>>>> +extern const struct clk_ops meson_vclk_gate_ops;
>>>> +
>>>> +/**
>>>> + * struct meson_vclk_div_data - vclk_div regmap back specific data
>>>> + *
>>>> + * @div: divider field
>>>> + * @enable: vclk divider enable field
>>>> + * @reset: vclk divider reset field
>>>> + * @table: array of value/divider pairs, last entry should have div = 0
>>>> + *
>>>> + * Flags:
>>>> + * Same as clk_divider except CLK_DIVIDER_HIWORD_MASK which is ignored
>>>> + */
>>>> +struct meson_vclk_div_data {
>>>> + struct parm div;
>>>> + struct parm enable;
>>>> + struct parm reset;
>>>> + const struct clk_div_table *table;
>>>> + u8 flags;
>>>> +};
>>>> +
>>>> +extern const struct clk_ops meson_vclk_div_ops;
>>>> +
>>>> +#endif /* __VCLK_H */
>>>
>
>
Applied to clk-meson (v6.10/drivers), thanks!
[2/7] clk: meson: add vclk driver
https://github.com/BayLibre/clk-meson/commit/bb5aa08572b5
[3/7] clk: meson: g12a: make VCLK2 and ENCL clock path configurable by CCF
https://github.com/BayLibre/clk-meson/commit/b70cb1a21a54
Best regards,
--
Jerome
Hi,
On Wed, 03 Apr 2024 09:46:31 +0200, Neil Armstrong wrote:
> The Amlogic G12A, G12B & SM1 SoCs embeds a Synopsys DW-MIPI-DSI transceiver (ver 1.21a),
> with a custom glue managing the IP resets, clock and data input similar to the DW-HDMI
> glue on the same Amlogic SoCs.
>
> This is a follow-up of v5 now the DRM patches are applied, the clk & DT changes
> remains for a full DSI support on G12A & SM1 platforms.
>
> [...]
Thanks, Applied to https://git.kernel.org/pub/scm/linux/kernel/git/amlogic/linux.git (v6.10/arm64-dt)
[1/7] dt-bindings: arm: amlogic: Document the MNT Reform 2 CM4 adapter with a BPI-CM4 Module
https://git.kernel.org/amlogic/c/ef5a84d716042871599ff7c8ff571a6390b99718
[5/7] arm64: meson: g12-common: add the MIPI DSI nodes
https://git.kernel.org/amlogic/c/6f1c2a12ed1138c3e680935718672d361afee372
[6/7] arm64: meson: khadas-vim3l: add TS050 DSI panel overlay
https://git.kernel.org/amlogic/c/2a885bad5ba4d553758d3f1689000cee8e6dae87
[7/7] arm64: dts: amlogic: meson-g12b-bananapi-cm4: add support for MNT Reform2 with CM4 adaper
https://git.kernel.org/amlogic/c/fde2d69c1626bebb3a8851909c912e582db1ca95
These changes has been applied on the intermediate git tree [1].
The v6.10/arm64-dt branch will then be sent via a formal Pull Request to the Linux SoC maintainers
for inclusion in their intermediate git branches in order to be sent to Linus during
the next merge window, or sooner if it's a set of fixes.
In the cases of fixes, those will be merged in the current release candidate
kernel and as soon they appear on the Linux master branch they will be
backported to the previous Stable and Long-Stable kernels [2].
The intermediate git branches are merged daily in the linux-next tree [3],
people are encouraged testing these pre-release kernels and report issues on the
relevant mailing-lists.
If problems are discovered on those changes, please submit a signed-off-by revert
patch followed by a corrective changeset.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/amlogic/linux.git
[2] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
[3] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
--
Neil
Le mer. 3 avr. 2024 à 09:46, Neil Armstrong
<[email protected]> a écrit :
>
> Disable the px_clk when setting the rate to recover a fully
> configured and correctly reset VCLK clock tree after the rate
> is set.
>
> Fixes: 77d9e1e6b846 ("drm/meson: add support for MIPI-DSI transceiver")
> Signed-off-by: Neil Armstrong <[email protected]>
> ---
> drivers/gpu/drm/meson/meson_dw_mipi_dsi.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c b/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c
> index a6bc1bdb3d0d..a10cff3ca1fe 100644
> --- a/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c
> +++ b/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c
> @@ -95,6 +95,7 @@ static int dw_mipi_dsi_phy_init(void *priv_data)
> return ret;
> }
>
> + clk_disable_unprepare(mipi_dsi->px_clk);
> ret = clk_set_rate(mipi_dsi->px_clk, mipi_dsi->mode->clock * 1000);
>
> if (ret) {
> @@ -103,6 +104,12 @@ static int dw_mipi_dsi_phy_init(void *priv_data)
> return ret;
> }
>
> + ret = clk_prepare_enable(mipi_dsi->px_clk);
> + if (ret) {
> + dev_err(mipi_dsi->dev, "Failed to enable DSI Pixel clock (ret %d)\n", ret);
> + return ret;
> + }
> +
> switch (mipi_dsi->dsi_device->format) {
> case MIPI_DSI_FMT_RGB888:
> dpi_data_format = DPI_COLOR_24BIT;
>
> --
> 2.34.1
>
Looks good to me
Reviewed-by: Nicolas Belin <[email protected]>
Hi Neil,
On Wed, Apr 3, 2024 at 9:46 AM Neil Armstrong <[email protected]> wrote:
>
> Disable the px_clk when setting the rate to recover a fully
> configured and correctly reset VCLK clock tree after the rate
> is set.
>
> Fixes: 77d9e1e6b846 ("drm/meson: add support for MIPI-DSI transceiver")
> Signed-off-by: Neil Armstrong <[email protected]>
> ---
> drivers/gpu/drm/meson/meson_dw_mipi_dsi.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c b/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c
> index a6bc1bdb3d0d..a10cff3ca1fe 100644
> --- a/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c
> +++ b/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c
> @@ -95,6 +95,7 @@ static int dw_mipi_dsi_phy_init(void *priv_data)
> return ret;
> }
>
> + clk_disable_unprepare(mipi_dsi->px_clk);
nit-pick: clk_disable(mipi_dsi->px_clk); should be enough here as my
understanding is that we only need to {un,}prepare a clock once.
> ret = clk_set_rate(mipi_dsi->px_clk, mipi_dsi->mode->clock * 1000);
>
> if (ret) {
> @@ -103,6 +104,12 @@ static int dw_mipi_dsi_phy_init(void *priv_data)
> return ret;
> }
>
> + ret = clk_prepare_enable(mipi_dsi->px_clk);
> + if (ret) {
> + dev_err(mipi_dsi->dev, "Failed to enable DSI Pixel clock (ret %d)\n", ret);
> + return ret;
If we ever hit this error case then there will be a lot of additional
errors in the kernel log:
- initially the clock is prepared and enabled in
meson_dw_mipi_dsi_probe() by calling devm_clk_get_enabled()
- we then disable the clock above (generally disabling a clock is
expected to always succeed)
- if the clock can NOT be re-enabled here we just log the error
- in case a user tries to rmmod the driver (to modprobe it again) to
try and recover from an error the automatic disabling of the pix_clk
(based on devm_clk_get_enabled() where it was enabled initially) there
will be a splat because the clock is already disabled (and enabled
count is zero, so it cannot be disabled any further)
For the 32-bit SoC video clocks I keep track of them being enabled or
disabled, see [0], [1] and [2].
In my case this is important because we can run into cases where the
PLL doesn't lock (I am not sure how likely this is for your case).
It *seems* like we need to do something similar as
dw_mipi_dsi_phy_init() can be called when changing the display
resolution (or whenever drm_bridge_funcs.atomic_pre_enable) is called.
To illustrate what I have in mind I attached a diff (it's based on
this patch) - it's compile tested only as I have no DSI hardware.
In case dw_mipi_dsi_phy_init() is called only once per device
lifecycle things may get easier.
PS: I'm so happy that we don't need any clock notifiers for this!
So: good work with the clock driver bits.
Let me know what you think,
Martin
[0] https://github.com/xdarklight/linux/blob/meson-mx-integration-6.9-20240323/drivers/gpu/drm/meson/meson_vclk.c#L1177-L1179
[1] https://github.com/xdarklight/linux/blob/meson-mx-integration-6.9-20240323/drivers/gpu/drm/meson/meson_vclk.c#L1077
[2] https://github.com/xdarklight/linux/blob/meson-mx-integration-6.9-20240323/drivers/gpu/drm/meson/meson_vclk.c#L1053
On 10/04/2024 21:34, Martin Blumenstingl wrote:
> Hi Neil,
>
> On Wed, Apr 3, 2024 at 9:46 AM Neil Armstrong <[email protected]> wrote:
>>
>> Disable the px_clk when setting the rate to recover a fully
>> configured and correctly reset VCLK clock tree after the rate
>> is set.
>>
>> Fixes: 77d9e1e6b846 ("drm/meson: add support for MIPI-DSI transceiver")
>> Signed-off-by: Neil Armstrong <[email protected]>
>> ---
>> drivers/gpu/drm/meson/meson_dw_mipi_dsi.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c b/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c
>> index a6bc1bdb3d0d..a10cff3ca1fe 100644
>> --- a/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c
>> +++ b/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c
>> @@ -95,6 +95,7 @@ static int dw_mipi_dsi_phy_init(void *priv_data)
>> return ret;
>> }
>>
>> + clk_disable_unprepare(mipi_dsi->px_clk);
> nit-pick: clk_disable(mipi_dsi->px_clk); should be enough here as my
> understanding is that we only need to {un,}prepare a clock once.
>
>> ret = clk_set_rate(mipi_dsi->px_clk, mipi_dsi->mode->clock * 1000);
>>
>> if (ret) {
>> @@ -103,6 +104,12 @@ static int dw_mipi_dsi_phy_init(void *priv_data)
>> return ret;
>> }
>>
>> + ret = clk_prepare_enable(mipi_dsi->px_clk);
>> + if (ret) {
>> + dev_err(mipi_dsi->dev, "Failed to enable DSI Pixel clock (ret %d)\n", ret);
>> + return ret;
> If we ever hit this error case then there will be a lot of additional
> errors in the kernel log:
> - initially the clock is prepared and enabled in
> meson_dw_mipi_dsi_probe() by calling devm_clk_get_enabled()
> - we then disable the clock above (generally disabling a clock is
> expected to always succeed)
> - if the clock can NOT be re-enabled here we just log the error
> - in case a user tries to rmmod the driver (to modprobe it again) to
> try and recover from an error the automatic disabling of the pix_clk
> (based on devm_clk_get_enabled() where it was enabled initially) there
> will be a splat because the clock is already disabled (and enabled
> count is zero, so it cannot be disabled any further)
>
> For the 32-bit SoC video clocks I keep track of them being enabled or
> disabled, see [0], [1] and [2].
> In my case this is important because we can run into cases where the
> PLL doesn't lock (I am not sure how likely this is for your case).
>
> It *seems* like we need to do something similar as
> dw_mipi_dsi_phy_init() can be called when changing the display
> resolution (or whenever drm_bridge_funcs.atomic_pre_enable) is called.
> To illustrate what I have in mind I attached a diff (it's based on
> this patch) - it's compile tested only as I have no DSI hardware.
> In case dw_mipi_dsi_phy_init() is called only once per device
> lifecycle things may get easier.
Indeed your scheme looks good, I'll try it since indeed we only need
to prepare it once in the lifetime of the driver.
>
> PS: I'm so happy that we don't need any clock notifiers for this!
> So: good work with the clock driver bits.
Thx !
>
>
> Let me know what you think,
> Martin
>
>
> [0] https://github.com/xdarklight/linux/blob/meson-mx-integration-6.9-20240323/drivers/gpu/drm/meson/meson_vclk.c#L1177-L1179
> [1] https://github.com/xdarklight/linux/blob/meson-mx-integration-6.9-20240323/drivers/gpu/drm/meson/meson_vclk.c#L1077
> [2] https://github.com/xdarklight/linux/blob/meson-mx-integration-6.9-20240323/drivers/gpu/drm/meson/meson_vclk.c#L1053
Hi Martin,
On 10/04/2024 21:34, Martin Blumenstingl wrote:
> Hi Neil,
>
> On Wed, Apr 3, 2024 at 9:46 AM Neil Armstrong <[email protected]> wrote:
>>
>> Disable the px_clk when setting the rate to recover a fully
>> configured and correctly reset VCLK clock tree after the rate
>> is set.
>>
>> Fixes: 77d9e1e6b846 ("drm/meson: add support for MIPI-DSI transceiver")
>> Signed-off-by: Neil Armstrong <[email protected]>
>> ---
>> drivers/gpu/drm/meson/meson_dw_mipi_dsi.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c b/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c
>> index a6bc1bdb3d0d..a10cff3ca1fe 100644
>> --- a/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c
>> +++ b/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c
>> @@ -95,6 +95,7 @@ static int dw_mipi_dsi_phy_init(void *priv_data)
>> return ret;
>> }
>>
>> + clk_disable_unprepare(mipi_dsi->px_clk);
> nit-pick: clk_disable(mipi_dsi->px_clk); should be enough here as my
> understanding is that we only need to {un,}prepare a clock once.
>
>> ret = clk_set_rate(mipi_dsi->px_clk, mipi_dsi->mode->clock * 1000);
>>
>> if (ret) {
>> @@ -103,6 +104,12 @@ static int dw_mipi_dsi_phy_init(void *priv_data)
>> return ret;
>> }
>>
>> + ret = clk_prepare_enable(mipi_dsi->px_clk);
>> + if (ret) {
>> + dev_err(mipi_dsi->dev, "Failed to enable DSI Pixel clock (ret %d)\n", ret);
>> + return ret;
> If we ever hit this error case then there will be a lot of additional
> errors in the kernel log:
> - initially the clock is prepared and enabled in
> meson_dw_mipi_dsi_probe() by calling devm_clk_get_enabled()
> - we then disable the clock above (generally disabling a clock is
> expected to always succeed)
> - if the clock can NOT be re-enabled here we just log the error
> - in case a user tries to rmmod the driver (to modprobe it again) to
> try and recover from an error the automatic disabling of the pix_clk
> (based on devm_clk_get_enabled() where it was enabled initially) there
> will be a splat because the clock is already disabled (and enabled
> count is zero, so it cannot be disabled any further)
>
> For the 32-bit SoC video clocks I keep track of them being enabled or
> disabled, see [0], [1] and [2].
> In my case this is important because we can run into cases where the
> PLL doesn't lock (I am not sure how likely this is for your case).
>
> It *seems* like we need to do something similar as
> dw_mipi_dsi_phy_init() can be called when changing the display
> resolution (or whenever drm_bridge_funcs.atomic_pre_enable) is called.
> To illustrate what I have in mind I attached a diff (it's based on
> this patch) - it's compile tested only as I have no DSI hardware.
> In case dw_mipi_dsi_phy_init() is called only once per device
> lifecycle things may get easier.
>
> PS: I'm so happy that we don't need any clock notifiers for this!
> So: good work with the clock driver bits.
I checked and tested your patches and it doesn't work because the pc_clk
needs to be disabled & prepared in order to correctly reset and setup again
the video clock tree.
dw_mipi_dsi_phy_init() is called at each DSI mode change, but it requires a
full clock tree recalc and reset, so it's safer to keep the current design.
I'll try to send a change to better handle the disable_unprepare() failure, but
definitely only calling clk_disable() wasn't enough.
Thanks,
Neil
>
>
> Let me know what you think,
> Martin
>
>
> [0] https://github.com/xdarklight/linux/blob/meson-mx-integration-6.9-20240323/drivers/gpu/drm/meson/meson_vclk.c#L1177-L1179
> [1] https://github.com/xdarklight/linux/blob/meson-mx-integration-6.9-20240323/drivers/gpu/drm/meson/meson_vclk.c#L1077
> [2] https://github.com/xdarklight/linux/blob/meson-mx-integration-6.9-20240323/drivers/gpu/drm/meson/meson_vclk.c#L1053
Hi,
On Wed, 03 Apr 2024 09:46:31 +0200, Neil Armstrong wrote:
> The Amlogic G12A, G12B & SM1 SoCs embeds a Synopsys DW-MIPI-DSI transceiver (ver 1.21a),
> with a custom glue managing the IP resets, clock and data input similar to the DW-HDMI
> glue on the same Amlogic SoCs.
>
> This is a follow-up of v5 now the DRM patches are applied, the clk & DT changes
> remains for a full DSI support on G12A & SM1 platforms.
>
> [...]
Thanks, Applied to https://gitlab.freedesktop.org/drm/misc/kernel.git (drm-misc-next)
[4/7] drm/meson: gate px_clk when setting rate
https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/5c9837374ecf55a1fa3b7622d365a0456960270f
--
Neil