The ldb_di[0/1]_ipu_div clock dividers in the CSCMR2 register
of i.MX53, i.MX6Q and i.MX6DL SoCs can be configured to a 1/3.5
drivider or a 1/7 divider. The common clock framework cannot
deal with the two dividers directly even with the divider table
which only supports integral dividers. So, the idea is to take
the 1/3.5 and 1/7 dividers as separate fixed factor dividers and
introduce a new multiplexer clock to be derived from the them.
Then, the ldb display clock trees can be setup correctly.
This series contains the necessary clock driver changes, dts code
changes and imx-drm/ldb driver changes to fullfill the task.
Liu Ying (3):
ARM: imx6q: refactor some ldb related clocks
ARM: dts: imx6q/imx6dl: add necessary clocks for ldb node
staging: drm/imx: ldb: correct the ldb di clock trees
.../devicetree/bindings/clock/imx6q-clock.txt | 6 ++--
arch/arm/boot/dts/imx6dl.dtsi | 8 +++--
arch/arm/boot/dts/imx6q.dtsi | 8 +++--
arch/arm/mach-imx/clk-imx6q.c | 25 +++++++------
drivers/staging/imx-drm/imx-ldb.c | 38 +++++++++++++++-----
5 files changed, 61 insertions(+), 24 deletions(-)
--
1.7.9.5
The ldb_di[0/1]_ipu_div dividers may divide their parent clock
frequencies by either 3.5 or 7. The non-integral dividers cannot
be dealt with the common clock framework directly, so they cannot
be registered as common clock dividers. So this patch adds a fixed
factor clock of 1/7 and introduces ldb_di[0/1]_div_sel multiplexers
so that the fixed factor clocks of 1/3.5 and 1/7 can be set to be
the parents of ldb_di[0/1]_div_sel multiplexers. The ldb_di[0/1]_podf
dividers are no longer used then.
Signed-off-by: Liu Ying <[email protected]>
---
.../devicetree/bindings/clock/imx6q-clock.txt | 6 +++--
arch/arm/mach-imx/clk-imx6q.c | 25 ++++++++++++--------
2 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/Documentation/devicetree/bindings/clock/imx6q-clock.txt b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
index 5a90a72..90e923e 100644
--- a/Documentation/devicetree/bindings/clock/imx6q-clock.txt
+++ b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
@@ -89,8 +89,6 @@ clocks and IDs.
gpu3d_shader 74
ipu1_podf 75
ipu2_podf 76
- ldb_di0_podf 77
- ldb_di1_podf 78
ipu1_di0_pre 79
ipu1_di1_pre 80
ipu2_di0_pre 81
@@ -215,6 +213,10 @@ clocks and IDs.
cko2 200
cko 201
vdoa 202
+ ldb_di0_div_7 203
+ ldb_di1_div_7 204
+ ldb_di0_div_sel 205
+ ldb_di1_div_sel 206
Examples:
diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
index 9181a24..2b5be96 100644
--- a/arch/arm/mach-imx/clk-imx6q.c
+++ b/arch/arm/mach-imx/clk-imx6q.c
@@ -189,6 +189,8 @@ static const char *gpu3d_core_sels[] = { "mmdc_ch0_axi", "pll3_usb_otg", "pll2_p
static const char *gpu3d_shader_sels[] = { "mmdc_ch0_axi", "pll3_usb_otg", "pll2_pfd1_594m", "pll3_pfd0_720m", };
static const char *ipu_sels[] = { "mmdc_ch0_axi", "pll2_pfd2_396m", "pll3_120m", "pll3_pfd1_540m", };
static const char *ldb_di_sels[] = { "pll5_video_div", "pll2_pfd0_352m", "pll2_pfd2_396m", "mmdc_ch1_axi", "pll3_usb_otg", };
+static const char *ldb_di0_div_sels[] = { "ldb_di0_div_3_5", "ldb_di0_div_7", };
+static const char *ldb_di1_div_sels[] = { "ldb_di1_div_3_5", "ldb_di1_div_7", };
static const char *ipu_di_pre_sels[] = { "mmdc_ch0_axi", "pll3_usb_otg", "pll5_video_div", "pll2_pfd0_352m", "pll2_pfd2_396m", "pll3_pfd1_540m", };
static const char *ipu1_di0_sels[] = { "ipu1_di0_pre", "dummy", "dummy", "ldb_di0", "ldb_di1", };
static const char *ipu1_di1_sels[] = { "ipu1_di1_pre", "dummy", "dummy", "ldb_di0", "ldb_di1", };
@@ -233,11 +235,11 @@ enum mx6q_clks {
periph_clk2, periph2_clk2, ipg, ipg_per, esai_pred, esai_podf,
asrc_pred, asrc_podf, spdif_pred, spdif_podf, can_root, ecspi_root,
gpu2d_core_podf, gpu3d_core_podf, gpu3d_shader, ipu1_podf, ipu2_podf,
- ldb_di0_podf, ldb_di1_podf, ipu1_di0_pre, ipu1_di1_pre, ipu2_di0_pre,
- ipu2_di1_pre, hsi_tx_podf, ssi1_pred, ssi1_podf, ssi2_pred, ssi2_podf,
- ssi3_pred, ssi3_podf, uart_serial_podf, usdhc1_podf, usdhc2_podf,
- usdhc3_podf, usdhc4_podf, enfc_pred, enfc_podf, emi_podf,
- emi_slow_podf, vpu_axi_podf, cko1_podf, axi, mmdc_ch0_axi_podf,
+ ldb_di0_podf_unused, ldb_di1_podf_unused, ipu1_di0_pre, ipu1_di1_pre,
+ ipu2_di0_pre, ipu2_di1_pre, hsi_tx_podf, ssi1_pred, ssi1_podf,
+ ssi2_pred, ssi2_podf, ssi3_pred, ssi3_podf, uart_serial_podf,
+ usdhc1_podf, usdhc2_podf, usdhc3_podf, usdhc4_podf, enfc_pred, enfc_podf,
+ emi_podf, emi_slow_podf, vpu_axi_podf, cko1_podf, axi, mmdc_ch0_axi_podf,
mmdc_ch1_axi_podf, arm, ahb, apbh_dma, asrc, can1_ipg, can1_serial,
can2_ipg, can2_serial, ecspi1, ecspi2, ecspi3, ecspi4, ecspi5, enet,
esai, gpt_ipg, gpt_ipg_per, gpu2d_core, gpu3d_core, hdmi_iahb,
@@ -251,7 +253,8 @@ enum mx6q_clks {
ssi2_ipg, ssi3_ipg, rom, usbphy1, usbphy2, ldb_di0_div_3_5, ldb_di1_div_3_5,
sata_ref, sata_ref_100m, pcie_ref, pcie_ref_125m, enet_ref, usbphy1_gate,
usbphy2_gate, pll4_post_div, pll5_post_div, pll5_video_div, eim_slow,
- spdif, cko2_sel, cko2_podf, cko2, cko, vdoa, clk_max
+ spdif, cko2_sel, cko2_podf, cko2, cko, vdoa, ldb_di0_div_7, ldb_di1_div_7,
+ ldb_di0_div_sel, ldb_di1_div_sel, clk_max
};
static struct clk *clk[clk_max];
@@ -387,6 +390,8 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
clk[ipu2_sel] = imx_clk_mux("ipu2_sel", base + 0x3c, 14, 2, ipu_sels, ARRAY_SIZE(ipu_sels));
clk[ldb_di0_sel] = imx_clk_mux_flags("ldb_di0_sel", base + 0x2c, 9, 3, ldb_di_sels, ARRAY_SIZE(ldb_di_sels), CLK_SET_RATE_PARENT);
clk[ldb_di1_sel] = imx_clk_mux_flags("ldb_di1_sel", base + 0x2c, 12, 3, ldb_di_sels, ARRAY_SIZE(ldb_di_sels), CLK_SET_RATE_PARENT);
+ clk[ldb_di0_div_sel] = imx_clk_mux_flags("ldb_di0_div_sel", base + 0x20, 10, 1, ldb_di0_div_sels, ARRAY_SIZE(ldb_di0_div_sels), CLK_SET_RATE_PARENT);
+ clk[ldb_di1_div_sel] = imx_clk_mux_flags("ldb_di1_div_sel", base + 0x20, 11, 1, ldb_di1_div_sels, ARRAY_SIZE(ldb_di1_div_sels), CLK_SET_RATE_PARENT);
clk[ipu1_di0_pre_sel] = imx_clk_mux("ipu1_di0_pre_sel", base + 0x34, 6, 3, ipu_di_pre_sels, ARRAY_SIZE(ipu_di_pre_sels));
clk[ipu1_di1_pre_sel] = imx_clk_mux("ipu1_di1_pre_sel", base + 0x34, 15, 3, ipu_di_pre_sels, ARRAY_SIZE(ipu_di_pre_sels));
clk[ipu2_di0_pre_sel] = imx_clk_mux("ipu2_di0_pre_sel", base + 0x38, 6, 3, ipu_di_pre_sels, ARRAY_SIZE(ipu_di_pre_sels));
@@ -436,9 +441,9 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
clk[ipu1_podf] = imx_clk_divider("ipu1_podf", "ipu1_sel", base + 0x3c, 11, 3);
clk[ipu2_podf] = imx_clk_divider("ipu2_podf", "ipu2_sel", base + 0x3c, 16, 3);
clk[ldb_di0_div_3_5] = imx_clk_fixed_factor("ldb_di0_div_3_5", "ldb_di0_sel", 2, 7);
- clk[ldb_di0_podf] = imx_clk_divider_flags("ldb_di0_podf", "ldb_di0_div_3_5", base + 0x20, 10, 1, 0);
+ clk[ldb_di0_div_7] = imx_clk_fixed_factor("ldb_di0_div_7", "ldb_di0_sel", 1, 7);
clk[ldb_di1_div_3_5] = imx_clk_fixed_factor("ldb_di1_div_3_5", "ldb_di1_sel", 2, 7);
- clk[ldb_di1_podf] = imx_clk_divider_flags("ldb_di1_podf", "ldb_di1_div_3_5", base + 0x20, 11, 1, 0);
+ clk[ldb_di1_div_7] = imx_clk_fixed_factor("ldb_di1_div_7", "ldb_di1_sel", 1, 7);
clk[ipu1_di0_pre] = imx_clk_divider("ipu1_di0_pre", "ipu1_di0_pre_sel", base + 0x34, 3, 3);
clk[ipu1_di1_pre] = imx_clk_divider("ipu1_di1_pre", "ipu1_di1_pre_sel", base + 0x34, 12, 3);
clk[ipu2_di0_pre] = imx_clk_divider("ipu2_di0_pre", "ipu2_di0_pre_sel", base + 0x38, 3, 3);
@@ -508,8 +513,8 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
clk[ipu1_di1] = imx_clk_gate2("ipu1_di1", "ipu1_di1_sel", base + 0x74, 4);
clk[ipu2] = imx_clk_gate2("ipu2", "ipu2_podf", base + 0x74, 6);
clk[ipu2_di0] = imx_clk_gate2("ipu2_di0", "ipu2_di0_sel", base + 0x74, 8);
- clk[ldb_di0] = imx_clk_gate2("ldb_di0", "ldb_di0_podf", base + 0x74, 12);
- clk[ldb_di1] = imx_clk_gate2("ldb_di1", "ldb_di1_podf", base + 0x74, 14);
+ clk[ldb_di0] = imx_clk_gate2("ldb_di0", "ldb_di0_div_sel", base + 0x74, 12);
+ clk[ldb_di1] = imx_clk_gate2("ldb_di1", "ldb_di1_div_sel", base + 0x74, 14);
clk[ipu2_di1] = imx_clk_gate2("ipu2_di1", "ipu2_di1_sel", base + 0x74, 10);
clk[hsi_tx] = imx_clk_gate2("hsi_tx", "hsi_tx_podf", base + 0x74, 16);
if (cpu_is_imx6dl())
--
1.7.9.5
This patch adds di[0/1]_div_3_5, di[0/1]_div_7 and di[0/1]_div_sel
clocks to the ldb nodes so that the ldb driver may use them to
setup the display clock trees.
Signed-off-by: Liu Ying <[email protected]>
---
arch/arm/boot/dts/imx6dl.dtsi | 8 ++++++--
arch/arm/boot/dts/imx6q.dtsi | 8 ++++++--
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/imx6dl.dtsi b/arch/arm/boot/dts/imx6dl.dtsi
index 9e8ae11..ff0d743 100644
--- a/arch/arm/boot/dts/imx6dl.dtsi
+++ b/arch/arm/boot/dts/imx6dl.dtsi
@@ -75,10 +75,14 @@
&ldb {
clocks = <&clks 33>, <&clks 34>,
<&clks 39>, <&clks 40>,
- <&clks 135>, <&clks 136>;
+ <&clks 135>, <&clks 136>,
+ <&clks 184>, <&clks 185>, <&clks 203>, <&clks 204>,
+ <&clks 205>, <&clks 206>;
clock-names = "di0_pll", "di1_pll",
"di0_sel", "di1_sel",
- "di0", "di1";
+ "di0", "di1",
+ "di0_div_3_5", "di1_div_3_5", "di0_div_7", "di1_div_7",
+ "di0_div_sel", "di1_div_sel";
lvds-channel@0 {
crtcs = <&ipu1 0>, <&ipu1 1>;
diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index f024ef2..b078a42 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -146,10 +146,14 @@
&ldb {
clocks = <&clks 33>, <&clks 34>,
<&clks 39>, <&clks 40>, <&clks 41>, <&clks 42>,
- <&clks 135>, <&clks 136>;
+ <&clks 135>, <&clks 136>,
+ <&clks 184>, <&clks 185>, <&clks 203>, <&clks 204>,
+ <&clks 205>, <&clks 206>;
clock-names = "di0_pll", "di1_pll",
"di0_sel", "di1_sel", "di2_sel", "di3_sel",
- "di0", "di1";
+ "di0", "di1",
+ "di0_div_3_5", "di1_div_3_5", "di0_div_7", "di1_div_7",
+ "di0_div_sel", "di1_div_sel";
lvds-channel@0 {
crtcs = <&ipu1 0>, <&ipu1 1>, <&ipu2 0>, <&ipu2 1>;
--
1.7.9.5
In ldb split mode, the ldb_di[0/1]_ipu_div dividers should
be configured as clock dividers of 1/3.5, while in others
ldb modes of 1/7. This patch gets the di[0/1]_div_3_5,
di[0/1]_div_7 and di[0/1]_div_sel clocks and sets the
di[0/1]_div_3_5 or di[0/1]_div_7 clocks to be the parents of
di[0/1]_div_sel clocks according to the ldb mode. The real
dividers are the two fixed factors bewteen the ldb_di[0/1] and
the pll clocks, so it's unnecessary to set the frequency for
the ldb_di[0/1] clocks again after pll clock frequency is set.
This patch removes the redundant clock frequency setting code
as well.
Signed-off-by: Liu Ying <[email protected]>
---
drivers/staging/imx-drm/imx-ldb.c | 38 +++++++++++++++++++++++++++++--------
1 file changed, 30 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/imx-drm/imx-ldb.c b/drivers/staging/imx-drm/imx-ldb.c
index 8af7f3b..7c553b8 100644
--- a/drivers/staging/imx-drm/imx-ldb.c
+++ b/drivers/staging/imx-drm/imx-ldb.c
@@ -81,6 +81,9 @@ struct imx_ldb {
struct clk *clk[2]; /* our own clock */
struct clk *clk_sel[4]; /* parent of display clock */
struct clk *clk_pll[2]; /* upstream clock we can adjust */
+ struct clk *clk_div_3_5[2]; /* fixed factor of 1/3.5 */
+ struct clk *clk_div_7[2]; /* fixed factor of 1/7 */
+ struct clk *clk_div_sel[2]; /* 1/3.5 or 1/7 */
u32 ldb_ctrl;
const struct bus_mux *lvds_mux;
};
@@ -150,6 +153,18 @@ static void imx_ldb_set_clock(struct imx_ldb *ldb, int mux, int chno,
{
int ret;
+ if (ldb->ldb_ctrl & LDB_SPLIT_MODE_EN) {
+ ret = clk_set_parent(ldb->clk_div_sel[chno], ldb->clk_div_3_5[chno]);
+ if (ret)
+ dev_err(ldb->dev, "unable to set di%d_div_sel parent clock "
+ "to di%d_div_3_5\n", chno, chno);
+ } else {
+ ret = clk_set_parent(ldb->clk_div_sel[chno], ldb->clk_div_7[chno]);
+ if (ret)
+ dev_err(ldb->dev, "unable to set di%d_div_sel parent clock "
+ "to di%d_div_7\n", chno, chno);
+ }
+
dev_dbg(ldb->dev, "%s: now: %ld want: %ld\n", __func__,
clk_get_rate(ldb->clk_pll[chno]), serial_clk);
clk_set_rate(ldb->clk_pll[chno], serial_clk);
@@ -157,14 +172,6 @@ static void imx_ldb_set_clock(struct imx_ldb *ldb, int mux, int chno,
dev_dbg(ldb->dev, "%s after: %ld\n", __func__,
clk_get_rate(ldb->clk_pll[chno]));
- dev_dbg(ldb->dev, "%s: now: %ld want: %ld\n", __func__,
- clk_get_rate(ldb->clk[chno]),
- (long int)di_clk);
- clk_set_rate(ldb->clk[chno], di_clk);
-
- dev_dbg(ldb->dev, "%s after: %ld\n", __func__,
- clk_get_rate(ldb->clk[chno]));
-
/* set display clock mux to LDB input clock */
ret = clk_set_parent(ldb->clk_sel[mux], ldb->clk[chno]);
if (ret) {
@@ -362,6 +369,21 @@ static int imx_ldb_get_clk(struct imx_ldb *ldb, int chno)
if (IS_ERR(ldb->clk_pll[chno]))
return PTR_ERR(ldb->clk_pll[chno]);
+ sprintf(clkname, "di%d_div_3_5", chno);
+ ldb->clk_div_3_5[chno] = devm_clk_get(ldb->dev, clkname);
+ if (IS_ERR(ldb->clk_div_3_5[chno]))
+ return PTR_ERR(ldb->clk_div_3_5[chno]);
+
+ sprintf(clkname, "di%d_div_7", chno);
+ ldb->clk_div_7[chno] = devm_clk_get(ldb->dev, clkname);
+ if (IS_ERR(ldb->clk_div_7[chno]))
+ return PTR_ERR(ldb->clk_div_7[chno]);
+
+ sprintf(clkname, "di%d_div_sel", chno);
+ ldb->clk_div_sel[chno] = devm_clk_get(ldb->dev, clkname);
+ if (IS_ERR(ldb->clk_div_sel[chno]))
+ return PTR_ERR(ldb->clk_div_sel[chno]);
+
return 0;
}
--
1.7.9.5
Am Dienstag, den 20.08.2013, 16:38 +0800 schrieb Liu Ying:
> The ldb_di[0/1]_ipu_div clock dividers in the CSCMR2 register
> of i.MX53, i.MX6Q and i.MX6DL SoCs can be configured to a 1/3.5
> drivider or a 1/7 divider. The common clock framework cannot
> deal with the two dividers directly even with the divider table
> which only supports integral dividers. So, the idea is to take
> the 1/3.5 and 1/7 dividers as separate fixed factor dividers and
> introduce a new multiplexer clock to be derived from the them.
> Then, the ldb display clock trees can be setup correctly.
> This series contains the necessary clock driver changes, dts code
> changes and imx-drm/ldb driver changes to fullfill the task.
I don't see how this improves the situation. Does this solve any real
problem?
While I admit to having introduced the combination of 1/3.5 fixed
divider and configurable 1/1,1/2 divder clocks to describe this
fractional divider for the reasons you state, I think the correct
solution would be to improve the table divider to support fractional
values and get rid of the virtual ldb_di<n>_div_3_5 clocks, not
introduce more virtual clocks.
regards
Philipp
On 08/20/2013 05:43 PM, Philipp Zabel wrote:
> Am Dienstag, den 20.08.2013, 16:38 +0800 schrieb Liu Ying:
>> The ldb_di[0/1]_ipu_div clock dividers in the CSCMR2 register
>> of i.MX53, i.MX6Q and i.MX6DL SoCs can be configured to a 1/3.5
>> drivider or a 1/7 divider. The common clock framework cannot
>> deal with the two dividers directly even with the divider table
>> which only supports integral dividers. So, the idea is to take
>> the 1/3.5 and 1/7 dividers as separate fixed factor dividers and
>> introduce a new multiplexer clock to be derived from the them.
>> Then, the ldb display clock trees can be setup correctly.
>> This series contains the necessary clock driver changes, dts code
>> changes and imx-drm/ldb driver changes to fullfill the task.
>
> I don't see how this improves the situation. Does this solve any real
> problem?
>
I don't see any functional problem without this series.
But, it may correct ldb_di[n] clock frequency returned from clk_get_rate() when using 1/7 divider.
Furthermore, since this series makes the ldb related clocks from pll to ldb_di[0/1] have the CLK_SET_RATE_PARENT flag set, the imx-drm/ldb driver may set the clocks' frequency more flexibly, i.e.,
only calling clk_set_rate() for ldb_di[n] clock would be an alternative.
> While I admit to having introduced the combination of 1/3.5 fixed
> divider and configurable 1/1,1/2 divder clocks to describe this
> fractional divider for the reasons you state, I think the correct
> solution would be to improve the table divider to support fractional
> values and get rid of the virtual ldb_di<n>_div_3_5 clocks, not
> introduce more virtual clocks.
Yes, it's good to support fractional values for the table divider(not sure if there is any plan for this).
I see there is something similar in 'include/linux/sh_clk.h'.
>
> regards
> Philipp
>
>
Regards,
Liu Ying
On Tue, Aug 20, 2013 at 5:38 AM, Liu Ying <[email protected]> wrote:
> diff --git a/Documentation/devicetree/bindings/clock/imx6q-clock.txt b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
> index 5a90a72..90e923e 100644
> --- a/Documentation/devicetree/bindings/clock/imx6q-clock.txt
> +++ b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
> @@ -89,8 +89,6 @@ clocks and IDs.
> gpu3d_shader 74
> ipu1_podf 75
> ipu2_podf 76
> - ldb_di0_podf 77
> - ldb_di1_podf 78
> ipu1_di0_pre 79
> ipu1_di1_pre 80
> ipu2_di0_pre 81
This causes a 'hole' in the clock numbering scheme: 74, 75, 76, 79, 80, etc
Quoting Fabio Estevam (2013-08-20 08:40:52)
> On Tue, Aug 20, 2013 at 5:38 AM, Liu Ying <[email protected]> wrote:
>
> > diff --git a/Documentation/devicetree/bindings/clock/imx6q-clock.txt b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
> > index 5a90a72..90e923e 100644
> > --- a/Documentation/devicetree/bindings/clock/imx6q-clock.txt
> > +++ b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
> > @@ -89,8 +89,6 @@ clocks and IDs.
> > gpu3d_shader 74
> > ipu1_podf 75
> > ipu2_podf 76
> > - ldb_di0_podf 77
> > - ldb_di1_podf 78
> > ipu1_di0_pre 79
> > ipu1_di1_pre 80
> > ipu2_di0_pre 81
>
> This causes a 'hole' in the clock numbering scheme: 74, 75, 76, 79, 80, etc
How does this fit in with the idea of having a stable binding/ABI? Seems
like changing this would be a bad idea for devices in the field that
have older DTBs.
Regards,
Mike
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Tue, Aug 20, 2013 at 02:18:27PM -0700, Mike Turquette wrote:
> Quoting Fabio Estevam (2013-08-20 08:40:52)
> > On Tue, Aug 20, 2013 at 5:38 AM, Liu Ying <[email protected]> wrote:
> >
> > > diff --git a/Documentation/devicetree/bindings/clock/imx6q-clock.txt b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
> > > index 5a90a72..90e923e 100644
> > > --- a/Documentation/devicetree/bindings/clock/imx6q-clock.txt
> > > +++ b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
> > > @@ -89,8 +89,6 @@ clocks and IDs.
> > > gpu3d_shader 74
> > > ipu1_podf 75
> > > ipu2_podf 76
> > > - ldb_di0_podf 77
> > > - ldb_di1_podf 78
> > > ipu1_di0_pre 79
> > > ipu1_di1_pre 80
> > > ipu2_di0_pre 81
> >
> > This causes a 'hole' in the clock numbering scheme: 74, 75, 76, 79, 80, etc
>
> How does this fit in with the idea of having a stable binding/ABI? Seems
> like changing this would be a bad idea for devices in the field that
> have older DTBs.
We should be safe since none of existing DTBs refers to the clocks (they
are not leaf clocks in the whole clock tree but some interconnection
ones).
Shawn
Hi Ying,
On Tue, Aug 20, 2013 at 06:08:48PM +0800, Liu Ying wrote:
> > While I admit to having introduced the combination of 1/3.5 fixed
> > divider and configurable 1/1,1/2 divder clocks to describe this
> > fractional divider for the reasons you state, I think the correct
> > solution would be to improve the table divider to support fractional
> > values and get rid of the virtual ldb_di<n>_div_3_5 clocks, not
> > introduce more virtual clocks.
>
> Yes, it's good to support fractional values for the table divider(not sure if there is any plan for this).
> I see there is something similar in 'include/linux/sh_clk.h'.
Yeah, I agree with Philipp on improving table divider to support
fractional values. Would you like to work on that?
Shawn
On 08/21/2013 09:59 AM, Shawn Guo wrote:
> Hi Ying,
>
> On Tue, Aug 20, 2013 at 06:08:48PM +0800, Liu Ying wrote:
>>> While I admit to having introduced the combination of 1/3.5 fixed
>>> divider and configurable 1/1,1/2 divder clocks to describe this
>>> fractional divider for the reasons you state, I think the correct
>>> solution would be to improve the table divider to support fractional
>>> values and get rid of the virtual ldb_di<n>_div_3_5 clocks, not
>>> introduce more virtual clocks.
>>
>> Yes, it's good to support fractional values for the table divider(not sure if there is any plan for this).
>> I see there is something similar in 'include/linux/sh_clk.h'.
>
> Yeah, I agree with Philipp on improving table divider to support
> fractional values. Would you like to work on that?
>
> Shawn
>
I don't have a plan to work on that now.
Liu Ying
On 08/20/2013 11:40 PM, Fabio Estevam wrote:
> On Tue, Aug 20, 2013 at 5:38 AM, Liu Ying <[email protected]> wrote:
>
>> diff --git a/Documentation/devicetree/bindings/clock/imx6q-clock.txt b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
>> index 5a90a72..90e923e 100644
>> --- a/Documentation/devicetree/bindings/clock/imx6q-clock.txt
>> +++ b/Documentation/devicetree/bindings/clock/imx6q-clock.txt
>> @@ -89,8 +89,6 @@ clocks and IDs.
>> gpu3d_shader 74
>> ipu1_podf 75
>> ipu2_podf 76
>> - ldb_di0_podf 77
>> - ldb_di1_podf 78
>> ipu1_di0_pre 79
>> ipu1_di1_pre 80
>> ipu2_di0_pre 81
>
> This causes a 'hole' in the clock numbering scheme: 74, 75, 76, 79, 80, etc
>
I find there is a 'hole' in Documentation/devicetree/bindings/clock/imx5-clock.txt as well.
The 'hole' was taken by tve_di(26) clock before.
Is this more acceptable?
ldb_di0_podf_unused 77
ldb_di1_podf_unused 78
Liu Ying