2014-04-04 14:53:46

by Tomasz Stanislawski

[permalink] [raw]
Subject: [PATCH 0/4] Update to Exynos clocks

This patchset adds some updates to clocks for Exynos4 platform and to the clock
core. The patches are rebased on linux/next.

An interesting part might be 'propagation of clk_set_parent()'. This feature
simplifies configuration of complex topologyof clocks by drivers. Such a
situation happens for Exynos HDMI driver.

The HDMI device is clocked by sclk_hdmi clock. In older versions of SoC and its
driver, the clock had two parent clocks: sclk_hdmiphy and sclk_pixel.
The sclk_hdmi was a gated clock.

In the recent version of Exynos clock provider this topology was slightly
changed. A new clock named mout_hdmi was introduced. This new clock represents
the output of multiplexer that selects between sclk_hdmiphy and sclk_pixel.

After the change the sclk_hdmi is still gated but has a single parent -
mout_hdmi.

This change caused interesting situation in Exynos HDMI driver because this
driver used only sclk_hdmi. Now clk_set_parent(sclk_hdmi, ...) fails because
sclk_hdmi has a single parent now. Using mout_hdmi instead of sclk_hdmi will
not work because clk_enable(mout_hdmi) is not propagated to sclk_hdmi.

To solve this problem, the setup of mout_hdmi was added to Exynos HDMI in the
patch: "drm/exynos: add mout_hdmi clock in hdmi driver to change parent"

IMO, this change breaks abstraction of clock API reveling too detailed
information about clock topology to the driver.

Moreover, the driver no longer works with old DT bindings because they provide
no mout_hdmi clock.

The clock set_parent propagation can be used to fix this. The clk_set_parent()
is propagated to a parent as long as the current clock has a single parent and
has the flag CLK_SET_PARENT_PARENT set.

Now Exynos HDMI can use only sclk_hdmi clock. The clk_enable() gates this clock
directly and clk_set_parent() is propagated to mout_hdmi.

This new behaviour does not break DT bindings because mout_hdmi would be simply
ignored.

After this change the 'add mout_hdmi clock' is no longer needed.

Regards,
Tomasz Stanislawski

Tomasz Stanislawski (4):
clk: propagate parent change up one level
clk: exynos4: export sclk_hdmiphy clock
clk: exynos4: enable clk_set_parent() propagation for sclk_hdmi and
sclk_mixer clocks
Revert "drm/exynos: add mout_hdmi clock in hdmi driver to change
parent"

drivers/clk/clk.c | 6 ++++++
drivers/clk/samsung/clk-exynos4.c | 8 +++++---
drivers/gpu/drm/exynos/exynos_hdmi.c | 14 ++++----------
include/dt-bindings/clock/exynos4.h | 1 +
include/linux/clk-provider.h | 1 +
5 files changed, 17 insertions(+), 13 deletions(-)

--
1.7.9.5


2014-04-04 14:54:17

by Tomasz Stanislawski

[permalink] [raw]
Subject: [PATCH 2/4] clk: exynos4: export sclk_hdmiphy clock

Export sclk_hdmiphy clock to be usable from DT.

Signed-off-by: Tomasz Stanislawski <[email protected]>
---
drivers/clk/samsung/clk-exynos4.c | 2 +-
include/dt-bindings/clock/exynos4.h | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
index b4f9672..528f8eb 100644
--- a/drivers/clk/samsung/clk-exynos4.c
+++ b/drivers/clk/samsung/clk-exynos4.c
@@ -428,7 +428,7 @@ static struct samsung_fixed_rate_clock exynos4_fixed_rate_ext_clks[] __initdata
/* fixed rate clocks generated inside the soc */
static struct samsung_fixed_rate_clock exynos4_fixed_rate_clks[] __initdata = {
FRATE(0, "sclk_hdmi24m", NULL, CLK_IS_ROOT, 24000000),
- FRATE(0, "sclk_hdmiphy", NULL, CLK_IS_ROOT, 27000000),
+ FRATE(CLK_SCLK_HDMIPHY, "sclk_hdmiphy", NULL, CLK_IS_ROOT, 27000000),
FRATE(0, "sclk_usbphy0", NULL, CLK_IS_ROOT, 48000000),
};

diff --git a/include/dt-bindings/clock/exynos4.h b/include/dt-bindings/clock/exynos4.h
index 75aff33..0e245eb 100644
--- a/include/dt-bindings/clock/exynos4.h
+++ b/include/dt-bindings/clock/exynos4.h
@@ -33,6 +33,7 @@
#define CLK_MOUT_MPLL_USER_C 18 /* Exynos4x12 only */
#define CLK_MOUT_CORE 19
#define CLK_MOUT_APLL 20
+#define CLK_SCLK_HDMIPHY 22

/* gate for special clocks (sclk) */
#define CLK_SCLK_FIMC0 128
--
1.7.9.5

2014-04-04 14:54:40

by Tomasz Stanislawski

[permalink] [raw]
Subject: [PATCH 3/4] clk: exynos4: enable clk_set_parent() propagation for sclk_hdmi and sclk_mixer clocks

This patch enables clk_set_parent() propagation for clocks used
by s5p-tv and exynos-drm drivers.

Signed-off-by: Tomasz Stanislawski <[email protected]>
---
drivers/clk/samsung/clk-exynos4.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
index 528f8eb..87b8264 100644
--- a/drivers/clk/samsung/clk-exynos4.c
+++ b/drivers/clk/samsung/clk-exynos4.c
@@ -680,7 +680,8 @@ static struct samsung_gate_clock exynos4_gate_clks[] __initdata = {
* the device name and clock alias names specified below for some
* of the clocks can be removed.
*/
- GATE(CLK_SCLK_HDMI, "sclk_hdmi", "mout_hdmi", SRC_MASK_TV, 0, 0, 0),
+ GATE(CLK_SCLK_HDMI, "sclk_hdmi", "mout_hdmi", SRC_MASK_TV, 0,
+ CLK_SET_PARENT_PARENT, 0),
GATE(CLK_SCLK_SPDIF, "sclk_spdif", "mout_spdif", SRC_MASK_PERIL1, 8, 0,
0),
GATE(CLK_JPEG, "jpeg", "aclk160", GATE_IP_CAM, 6, 0, 0),
@@ -880,7 +881,8 @@ static struct samsung_gate_clock exynos4210_gate_clks[] __initdata = {
E4210_SRC_MASK_LCD1, 12, CLK_SET_RATE_PARENT, 0),
GATE(CLK_SCLK_SATA, "sclk_sata", "div_sata",
SRC_MASK_FSYS, 24, CLK_SET_RATE_PARENT, 0),
- GATE(CLK_SCLK_MIXER, "sclk_mixer", "mout_mixer", SRC_MASK_TV, 4, 0, 0),
+ GATE(CLK_SCLK_MIXER, "sclk_mixer", "mout_mixer", SRC_MASK_TV, 4,
+ CLK_SET_PARENT_PARENT, 0),
GATE(CLK_SCLK_DAC, "sclk_dac", "mout_dac", SRC_MASK_TV, 8, 0, 0),
GATE(CLK_TSADC, "tsadc", "aclk100", GATE_IP_PERIL, 15,
0, 0),
--
1.7.9.5

2014-04-04 14:55:01

by Tomasz Stanislawski

[permalink] [raw]
Subject: [PATCH 4/4] Revert "drm/exynos: add mout_hdmi clock in hdmi driver to change parent"

This reverts commit 59956d35a8618235ea715280b49447bb36f2c975.

Signed-off-by: Tomasz Stanislawski <[email protected]>
---
drivers/gpu/drm/exynos/exynos_hdmi.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 9a6d652..8ebb4bf 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -75,7 +75,6 @@ struct hdmi_resources {
struct clk *sclk_pixel;
struct clk *sclk_hdmiphy;
struct clk *hdmiphy;
- struct clk *mout_hdmi;
struct regulator_bulk_data *regul_bulk;
int regul_count;
};
@@ -1282,7 +1281,7 @@ static void hdmi_v13_mode_apply(struct hdmi_context *hdata)
}

clk_disable_unprepare(hdata->res.sclk_hdmi);
- clk_set_parent(hdata->res.mout_hdmi, hdata->res.sclk_hdmiphy);
+ clk_set_parent(hdata->res.sclk_hdmi, hdata->res.sclk_hdmiphy);
clk_prepare_enable(hdata->res.sclk_hdmi);

/* enable HDMI and timing generator */
@@ -1449,7 +1448,7 @@ static void hdmi_v14_mode_apply(struct hdmi_context *hdata)
}

clk_disable_unprepare(hdata->res.sclk_hdmi);
- clk_set_parent(hdata->res.mout_hdmi, hdata->res.sclk_hdmiphy);
+ clk_set_parent(hdata->res.sclk_hdmi, hdata->res.sclk_hdmiphy);
clk_prepare_enable(hdata->res.sclk_hdmi);

/* enable HDMI and timing generator */
@@ -1475,7 +1474,7 @@ static void hdmiphy_conf_reset(struct hdmi_context *hdata)
u32 reg;

clk_disable_unprepare(hdata->res.sclk_hdmi);
- clk_set_parent(hdata->res.mout_hdmi, hdata->res.sclk_pixel);
+ clk_set_parent(hdata->res.sclk_hdmi, hdata->res.sclk_pixel);
clk_prepare_enable(hdata->res.sclk_hdmi);

/* operation mode */
@@ -1982,13 +1981,8 @@ static int hdmi_resources_init(struct hdmi_context *hdata)
DRM_ERROR("failed to get clock 'hdmiphy'\n");
goto fail;
}
- res->mout_hdmi = devm_clk_get(dev, "mout_hdmi");
- if (IS_ERR(res->mout_hdmi)) {
- DRM_ERROR("failed to get clock 'mout_hdmi'\n");
- goto fail;
- }

- clk_set_parent(res->mout_hdmi, res->sclk_pixel);
+ clk_set_parent(res->sclk_hdmi, res->sclk_pixel);

res->regul_bulk = devm_kzalloc(dev, ARRAY_SIZE(supply) *
sizeof(res->regul_bulk[0]), GFP_KERNEL);
--
1.7.9.5

2014-04-04 14:53:59

by Tomasz Stanislawski

[permalink] [raw]
Subject: [PATCH 1/4] clk: propagate parent change up one level

This patch adds support for propagation of setup of clock's parent one level
up.

This feature is helpful when a driver changes topology of its clocks using
clk_set_parent(). The problem occurs when on one platform/SoC driver's clock
is located at MUX output but on the other platform/SoC there is a gated proxy
clock between the MUX and driver's clock. In such a case, driver's code has to
be modified to use one clock for enabling and the other clock for setup of a
parent.

The code updates are avoided by propagating setup of a parent up one level.

Additionally, this patch adds CLK_SET_PARENT_PARENT (sorry for naming) flag to
inform clk-core that clk_set_parent() should be propagated.

Signed-off-by: Tomasz Stanislawski <[email protected]>
---
drivers/clk/clk.c | 6 ++++++
include/linux/clk-provider.h | 1 +
2 files changed, 7 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index dff0373..53bbfda 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1737,6 +1737,12 @@ int clk_set_parent(struct clk *clk, struct clk *parent)

/* try finding the new parent index */
if (parent) {
+ if ((clk->flags & CLK_SET_PARENT_PARENT)
+ && clk->num_parents == 1) {
+ ret = clk_set_parent(clk->parent, parent);
+ goto out;
+ }
+
p_index = clk_fetch_parent_index(clk, parent);
p_rate = parent->rate;
if (p_index < 0) {
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 5119174..daa0b03 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -30,6 +30,7 @@
#define CLK_GET_RATE_NOCACHE BIT(6) /* do not use the cached clk rate */
#define CLK_SET_RATE_NO_REPARENT BIT(7) /* don't re-parent on rate change */
#define CLK_GET_ACCURACY_NOCACHE BIT(8) /* do not use the cached clk accuracy */
+#define CLK_SET_PARENT_PARENT BIT(9) /* propagate parent change up one level */

struct clk_hw;
struct dentry;
--
1.7.9.5

2014-04-08 15:45:24

by Tomasz Figa

[permalink] [raw]
Subject: Re: [PATCH 1/4] clk: propagate parent change up one level

Hi,

On 04.04.2014 16:53, Tomasz Stanislawski wrote:
> This patch adds support for propagation of setup of clock's parent one level
> up.
>
> This feature is helpful when a driver changes topology of its clocks using
> clk_set_parent(). The problem occurs when on one platform/SoC driver's clock
> is located at MUX output but on the other platform/SoC there is a gated proxy
> clock between the MUX and driver's clock. In such a case, driver's code has to
> be modified to use one clock for enabling and the other clock for setup of a
> parent.
>
> The code updates are avoided by propagating setup of a parent up one level.
>
> Additionally, this patch adds CLK_SET_PARENT_PARENT (sorry for naming) flag to
> inform clk-core that clk_set_parent() should be propagated.
>
> Signed-off-by: Tomasz Stanislawski <[email protected]>
> ---
> drivers/clk/clk.c | 6 ++++++
> include/linux/clk-provider.h | 1 +
> 2 files changed, 7 insertions(+)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index dff0373..53bbfda 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -1737,6 +1737,12 @@ int clk_set_parent(struct clk *clk, struct clk *parent)
>
> /* try finding the new parent index */
> if (parent) {
> + if ((clk->flags & CLK_SET_PARENT_PARENT)
> + && clk->num_parents == 1) {
> + ret = clk_set_parent(clk->parent, parent);
> + goto out;
> + }
> +
> p_index = clk_fetch_parent_index(clk, parent);
> p_rate = parent->rate;
> if (p_index < 0) {
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index 5119174..daa0b03 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -30,6 +30,7 @@
> #define CLK_GET_RATE_NOCACHE BIT(6) /* do not use the cached clk rate */
> #define CLK_SET_RATE_NO_REPARENT BIT(7) /* don't re-parent on rate change */
> #define CLK_GET_ACCURACY_NOCACHE BIT(8) /* do not use the cached clk accuracy */
> +#define CLK_SET_PARENT_PARENT BIT(9) /* propagate parent change up one level */
>
> struct clk_hw;
> struct dentry;
>

This would be very useful, at least on Exynos platforms, with
mux-div-gate clock paths. PARENT_PARENT sounds a bit funny, though.

Reviewed-by: Tomasz Figa <[email protected]>

Best regards,
Tomasz

2014-04-08 15:48:30

by Tomasz Figa

[permalink] [raw]
Subject: Re: [PATCH 2/4] clk: exynos4: export sclk_hdmiphy clock

Hi Tomasz,

On 04.04.2014 16:53, Tomasz Stanislawski wrote:
> Export sclk_hdmiphy clock to be usable from DT.
>
> Signed-off-by: Tomasz Stanislawski <[email protected]>
> ---
> drivers/clk/samsung/clk-exynos4.c | 2 +-
> include/dt-bindings/clock/exynos4.h | 1 +
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
> index b4f9672..528f8eb 100644
> --- a/drivers/clk/samsung/clk-exynos4.c
> +++ b/drivers/clk/samsung/clk-exynos4.c
> @@ -428,7 +428,7 @@ static struct samsung_fixed_rate_clock exynos4_fixed_rate_ext_clks[] __initdata
> /* fixed rate clocks generated inside the soc */
> static struct samsung_fixed_rate_clock exynos4_fixed_rate_clks[] __initdata = {
> FRATE(0, "sclk_hdmi24m", NULL, CLK_IS_ROOT, 24000000),
> - FRATE(0, "sclk_hdmiphy", NULL, CLK_IS_ROOT, 27000000),
> + FRATE(CLK_SCLK_HDMIPHY, "sclk_hdmiphy", NULL, CLK_IS_ROOT, 27000000),
> FRATE(0, "sclk_usbphy0", NULL, CLK_IS_ROOT, 48000000),
> };
>
> diff --git a/include/dt-bindings/clock/exynos4.h b/include/dt-bindings/clock/exynos4.h
> index 75aff33..0e245eb 100644
> --- a/include/dt-bindings/clock/exynos4.h
> +++ b/include/dt-bindings/clock/exynos4.h
> @@ -33,6 +33,7 @@
> #define CLK_MOUT_MPLL_USER_C 18 /* Exynos4x12 only */
> #define CLK_MOUT_CORE 19
> #define CLK_MOUT_APLL 20
> +#define CLK_SCLK_HDMIPHY 22
>
> /* gate for special clocks (sclk) */
> #define CLK_SCLK_FIMC0 128
>

I believe this clock should be properly abstracted as an output of HDMI
PHY, but I don't see any way to do this with existing infrastructure, so
probably for now such workaround is fine.

Will apply, if nobody opposes.

Best regards,
Tomasz

2014-04-08 15:49:49

by Tomasz Figa

[permalink] [raw]
Subject: Re: [PATCH 3/4] clk: exynos4: enable clk_set_parent() propagation for sclk_hdmi and sclk_mixer clocks

On 04.04.2014 16:53, Tomasz Stanislawski wrote:
> This patch enables clk_set_parent() propagation for clocks used
> by s5p-tv and exynos-drm drivers.
>
> Signed-off-by: Tomasz Stanislawski <[email protected]>
> ---
> drivers/clk/samsung/clk-exynos4.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
> index 528f8eb..87b8264 100644
> --- a/drivers/clk/samsung/clk-exynos4.c
> +++ b/drivers/clk/samsung/clk-exynos4.c
> @@ -680,7 +680,8 @@ static struct samsung_gate_clock exynos4_gate_clks[] __initdata = {
> * the device name and clock alias names specified below for some
> * of the clocks can be removed.
> */
> - GATE(CLK_SCLK_HDMI, "sclk_hdmi", "mout_hdmi", SRC_MASK_TV, 0, 0, 0),
> + GATE(CLK_SCLK_HDMI, "sclk_hdmi", "mout_hdmi", SRC_MASK_TV, 0,
> + CLK_SET_PARENT_PARENT, 0),
> GATE(CLK_SCLK_SPDIF, "sclk_spdif", "mout_spdif", SRC_MASK_PERIL1, 8, 0,
> 0),
> GATE(CLK_JPEG, "jpeg", "aclk160", GATE_IP_CAM, 6, 0, 0),
> @@ -880,7 +881,8 @@ static struct samsung_gate_clock exynos4210_gate_clks[] __initdata = {
> E4210_SRC_MASK_LCD1, 12, CLK_SET_RATE_PARENT, 0),
> GATE(CLK_SCLK_SATA, "sclk_sata", "div_sata",
> SRC_MASK_FSYS, 24, CLK_SET_RATE_PARENT, 0),
> - GATE(CLK_SCLK_MIXER, "sclk_mixer", "mout_mixer", SRC_MASK_TV, 4, 0, 0),
> + GATE(CLK_SCLK_MIXER, "sclk_mixer", "mout_mixer", SRC_MASK_TV, 4,
> + CLK_SET_PARENT_PARENT, 0),
> GATE(CLK_SCLK_DAC, "sclk_dac", "mout_dac", SRC_MASK_TV, 8, 0, 0),
> GATE(CLK_TSADC, "tsadc", "aclk100", GATE_IP_PERIL, 15,
> 0, 0),
>

Looks fine, but since it depends on patch 1/4, which needs to be
discussed a bit, I'll hold on with applying it for now.

Best regards,
Tomasz

2014-04-08 15:52:19

by Tomasz Figa

[permalink] [raw]
Subject: Re: [PATCH 4/4] Revert "drm/exynos: add mout_hdmi clock in hdmi driver to change parent"

Hi Tomasz,

On 04.04.2014 16:53, Tomasz Stanislawski wrote:
> This reverts commit 59956d35a8618235ea715280b49447bb36f2c975.
>

Probably a reason why this commit is being reverted would be a good idea.

> Signed-off-by: Tomasz Stanislawski <[email protected]>
> ---
> drivers/gpu/drm/exynos/exynos_hdmi.c | 14 ++++----------
> 1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index 9a6d652..8ebb4bf 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -75,7 +75,6 @@ struct hdmi_resources {
> struct clk *sclk_pixel;
> struct clk *sclk_hdmiphy;
> struct clk *hdmiphy;
> - struct clk *mout_hdmi;
> struct regulator_bulk_data *regul_bulk;
> int regul_count;
> };
> @@ -1282,7 +1281,7 @@ static void hdmi_v13_mode_apply(struct hdmi_context *hdata)
> }
>
> clk_disable_unprepare(hdata->res.sclk_hdmi);
> - clk_set_parent(hdata->res.mout_hdmi, hdata->res.sclk_hdmiphy);
> + clk_set_parent(hdata->res.sclk_hdmi, hdata->res.sclk_hdmiphy);
> clk_prepare_enable(hdata->res.sclk_hdmi);
>
> /* enable HDMI and timing generator */
> @@ -1449,7 +1448,7 @@ static void hdmi_v14_mode_apply(struct hdmi_context *hdata)
> }
>
> clk_disable_unprepare(hdata->res.sclk_hdmi);
> - clk_set_parent(hdata->res.mout_hdmi, hdata->res.sclk_hdmiphy);
> + clk_set_parent(hdata->res.sclk_hdmi, hdata->res.sclk_hdmiphy);
> clk_prepare_enable(hdata->res.sclk_hdmi);
>
> /* enable HDMI and timing generator */
> @@ -1475,7 +1474,7 @@ static void hdmiphy_conf_reset(struct hdmi_context *hdata)
> u32 reg;
>
> clk_disable_unprepare(hdata->res.sclk_hdmi);
> - clk_set_parent(hdata->res.mout_hdmi, hdata->res.sclk_pixel);
> + clk_set_parent(hdata->res.sclk_hdmi, hdata->res.sclk_pixel);
> clk_prepare_enable(hdata->res.sclk_hdmi);
>
> /* operation mode */
> @@ -1982,13 +1981,8 @@ static int hdmi_resources_init(struct hdmi_context *hdata)
> DRM_ERROR("failed to get clock 'hdmiphy'\n");
> goto fail;
> }
> - res->mout_hdmi = devm_clk_get(dev, "mout_hdmi");
> - if (IS_ERR(res->mout_hdmi)) {
> - DRM_ERROR("failed to get clock 'mout_hdmi'\n");
> - goto fail;
> - }
>
> - clk_set_parent(res->mout_hdmi, res->sclk_pixel);
> + clk_set_parent(res->sclk_hdmi, res->sclk_pixel);
>
> res->regul_bulk = devm_kzalloc(dev, ARRAY_SIZE(supply) *
> sizeof(res->regul_bulk[0]), GFP_KERNEL);
>

Otherwise looks fine and will apply after all its dependencies are
agreed upon.

Best regards,
Tomasz

2014-04-30 22:19:47

by Tomasz Figa

[permalink] [raw]
Subject: Re: [PATCH 1/4] clk: propagate parent change up one level

Mike,

On 08.04.2014 17:45, Tomasz Figa wrote:
> Hi,
>
> On 04.04.2014 16:53, Tomasz Stanislawski wrote:
>> This patch adds support for propagation of setup of clock's parent one
>> level
>> up.
>>
>> This feature is helpful when a driver changes topology of its clocks
>> using
>> clk_set_parent(). The problem occurs when on one platform/SoC
>> driver's clock
>> is located at MUX output but on the other platform/SoC there is a
>> gated proxy
>> clock between the MUX and driver's clock. In such a case, driver's
>> code has to
>> be modified to use one clock for enabling and the other clock for
>> setup of a
>> parent.
>>
>> The code updates are avoided by propagating setup of a parent up one
>> level.
>>
>> Additionally, this patch adds CLK_SET_PARENT_PARENT (sorry for naming)
>> flag to
>> inform clk-core that clk_set_parent() should be propagated.
>>
>> Signed-off-by: Tomasz Stanislawski <[email protected]>
>> ---
>> drivers/clk/clk.c | 6 ++++++
>> include/linux/clk-provider.h | 1 +
>> 2 files changed, 7 insertions(+)
>>
>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>> index dff0373..53bbfda 100644
>> --- a/drivers/clk/clk.c
>> +++ b/drivers/clk/clk.c
>> @@ -1737,6 +1737,12 @@ int clk_set_parent(struct clk *clk, struct clk
>> *parent)
>>
>> /* try finding the new parent index */
>> if (parent) {
>> + if ((clk->flags & CLK_SET_PARENT_PARENT)
>> + && clk->num_parents == 1) {
>> + ret = clk_set_parent(clk->parent, parent);
>> + goto out;
>> + }
>> +
>> p_index = clk_fetch_parent_index(clk, parent);
>> p_rate = parent->rate;
>> if (p_index < 0) {
>> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
>> index 5119174..daa0b03 100644
>> --- a/include/linux/clk-provider.h
>> +++ b/include/linux/clk-provider.h
>> @@ -30,6 +30,7 @@
>> #define CLK_GET_RATE_NOCACHE BIT(6) /* do not use the cached clk
>> rate */
>> #define CLK_SET_RATE_NO_REPARENT BIT(7) /* don't re-parent on rate
>> change */
>> #define CLK_GET_ACCURACY_NOCACHE BIT(8) /* do not use the cached clk
>> accuracy */
>> +#define CLK_SET_PARENT_PARENT BIT(9) /* propagate parent change up
>> one level */
>>
>> struct clk_hw;
>> struct dentry;
>>
>
> This would be very useful, at least on Exynos platforms, with
> mux-div-gate clock paths. PARENT_PARENT sounds a bit funny, though.
>
> Reviewed-by: Tomasz Figa <[email protected]>

Your opinion on this would be greatly appreciated.

Best regards,
Tomasz

2014-04-30 22:49:33

by Tomasz Figa

[permalink] [raw]
Subject: Re: [PATCH 2/4] clk: exynos4: export sclk_hdmiphy clock



On 04.04.2014 16:53, Tomasz Stanislawski wrote:
> Export sclk_hdmiphy clock to be usable from DT.
>
> Signed-off-by: Tomasz Stanislawski <[email protected]>
> ---
> drivers/clk/samsung/clk-exynos4.c | 2 +-
> include/dt-bindings/clock/exynos4.h | 1 +
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
> index b4f9672..528f8eb 100644
> --- a/drivers/clk/samsung/clk-exynos4.c
> +++ b/drivers/clk/samsung/clk-exynos4.c
> @@ -428,7 +428,7 @@ static struct samsung_fixed_rate_clock exynos4_fixed_rate_ext_clks[] __initdata
> /* fixed rate clocks generated inside the soc */
> static struct samsung_fixed_rate_clock exynos4_fixed_rate_clks[] __initdata = {
> FRATE(0, "sclk_hdmi24m", NULL, CLK_IS_ROOT, 24000000),
> - FRATE(0, "sclk_hdmiphy", NULL, CLK_IS_ROOT, 27000000),
> + FRATE(CLK_SCLK_HDMIPHY, "sclk_hdmiphy", NULL, CLK_IS_ROOT, 27000000),
> FRATE(0, "sclk_usbphy0", NULL, CLK_IS_ROOT, 48000000),
> };
>
> diff --git a/include/dt-bindings/clock/exynos4.h b/include/dt-bindings/clock/exynos4.h
> index 75aff33..0e245eb 100644
> --- a/include/dt-bindings/clock/exynos4.h
> +++ b/include/dt-bindings/clock/exynos4.h
> @@ -33,6 +33,7 @@
> #define CLK_MOUT_MPLL_USER_C 18 /* Exynos4x12 only */
> #define CLK_MOUT_CORE 19
> #define CLK_MOUT_APLL 20
> +#define CLK_SCLK_HDMIPHY 22
>
> /* gate for special clocks (sclk) */
> #define CLK_SCLK_FIMC0 128
>

Applied.

Best regards,
Tomasz

2014-06-18 10:12:03

by Tomasz Stanislawski

[permalink] [raw]
Subject: Re: [PATCH 1/4] clk: propagate parent change up one level

Hi Mike,
Do you have any comments about this patch?
The patch is needed to provide a clean fix for recently
broken support for HDMI on Exynos4210 SoC in mainline.

Regards,
Tomasz Stanislawski


On 05/01/2014 12:19 AM, Tomasz Figa wrote:
> Mike,
>
> On 08.04.2014 17:45, Tomasz Figa wrote:
>> Hi,
>>
>> On 04.04.2014 16:53, Tomasz Stanislawski wrote:
>>> This patch adds support for propagation of setup of clock's parent one
>>> level
>>> up.
>>>
>>> This feature is helpful when a driver changes topology of its clocks
>>> using
>>> clk_set_parent(). The problem occurs when on one platform/SoC
>>> driver's clock
>>> is located at MUX output but on the other platform/SoC there is a
>>> gated proxy
>>> clock between the MUX and driver's clock. In such a case, driver's
>>> code has to
>>> be modified to use one clock for enabling and the other clock for
>>> setup of a
>>> parent.
>>>
>>> The code updates are avoided by propagating setup of a parent up one
>>> level.
>>>
>>> Additionally, this patch adds CLK_SET_PARENT_PARENT (sorry for naming)
>>> flag to
>>> inform clk-core that clk_set_parent() should be propagated.
>>>
>>> Signed-off-by: Tomasz Stanislawski <[email protected]>
>>> ---
>>> drivers/clk/clk.c | 6 ++++++
>>> include/linux/clk-provider.h | 1 +
>>> 2 files changed, 7 insertions(+)
>>>
>>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>>> index dff0373..53bbfda 100644
>>> --- a/drivers/clk/clk.c
>>> +++ b/drivers/clk/clk.c
>>> @@ -1737,6 +1737,12 @@ int clk_set_parent(struct clk *clk, struct clk
>>> *parent)
>>>
>>> /* try finding the new parent index */
>>> if (parent) {
>>> + if ((clk->flags & CLK_SET_PARENT_PARENT)
>>> + && clk->num_parents == 1) {
>>> + ret = clk_set_parent(clk->parent, parent);
>>> + goto out;
>>> + }
>>> +
>>> p_index = clk_fetch_parent_index(clk, parent);
>>> p_rate = parent->rate;
>>> if (p_index < 0) {
>>> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
>>> index 5119174..daa0b03 100644
>>> --- a/include/linux/clk-provider.h
>>> +++ b/include/linux/clk-provider.h
>>> @@ -30,6 +30,7 @@
>>> #define CLK_GET_RATE_NOCACHE BIT(6) /* do not use the cached clk
>>> rate */
>>> #define CLK_SET_RATE_NO_REPARENT BIT(7) /* don't re-parent on rate
>>> change */
>>> #define CLK_GET_ACCURACY_NOCACHE BIT(8) /* do not use the cached clk
>>> accuracy */
>>> +#define CLK_SET_PARENT_PARENT BIT(9) /* propagate parent change up
>>> one level */
>>>
>>> struct clk_hw;
>>> struct dentry;
>>>
>>
>> This would be very useful, at least on Exynos platforms, with
>> mux-div-gate clock paths. PARENT_PARENT sounds a bit funny, though.
>>
>> Reviewed-by: Tomasz Figa <[email protected]>
>
> Your opinion on this would be greatly appreciated.
>
> Best regards,
> Tomasz