| Message ID | 1375874709-10438-2-git-send-email-markz@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 08/07/2013 05:25 AM, Mark Zhang wrote: > From: Peter De Schrijver <pdeschrijver@nvidia.com> > > COP is a reset only clock. So this patch adds NO_CLK support > then adds the COP clock. Do we actually need this clock upstream yet? IIRC, Prashant was working on implementing the common reset API, so I'd prefer not to add any reset-only "clocks" to the clock driver, but rather to simply make sure that the new reset driver exposes them. > diff --git a/drivers/clk/tegra/clk-tegra114.c b/drivers/clk/tegra/clk-tegra114.c > enum tegra114_clk { > - rtc = 4, timer = 5, uarta = 6, sdmmc2 = 9, i2s1 = 11, i2c1 = 12, > + cop = 1, rtc = 4, timer = 5, uarta = 6, sdmmc2 = 9, i2s1 = 11, i2c1 = 12, To make this change, you would also need to edit include/dt-bindings/clock/tegra114-car.h. BTW, why doesn't the Tegra clock driver include that file rather than defining its own enum?
Okay, I don't know these background infos. If so, there is no reason to upstream this kind of patches. On 08/08/2013 12:58 AM, Stephen Warren wrote: > On 08/07/2013 05:25 AM, Mark Zhang wrote: >> From: Peter De Schrijver <pdeschrijver@nvidia.com> >> >> COP is a reset only clock. So this patch adds NO_CLK support >> then adds the COP clock. > > Do we actually need this clock upstream yet? > > IIRC, Prashant was working on implementing the common reset API, so I'd > prefer not to add any reset-only "clocks" to the clock driver, but > rather to simply make sure that the new reset driver exposes them. > >> diff --git a/drivers/clk/tegra/clk-tegra114.c b/drivers/clk/tegra/clk-tegra114.c > >> enum tegra114_clk { >> - rtc = 4, timer = 5, uarta = 6, sdmmc2 = 9, i2s1 = 11, i2c1 = 12, >> + cop = 1, rtc = 4, timer = 5, uarta = 6, sdmmc2 = 9, i2s1 = 11, i2c1 = 12, > > To make this change, you would also need to edit > include/dt-bindings/clock/tegra114-car.h. BTW, why doesn't the Tegra > clock driver include that file rather than defining its own enum? >
On Wed, Aug 07, 2013 at 06:58:03PM +0200, Stephen Warren wrote: > On 08/07/2013 05:25 AM, Mark Zhang wrote: > > From: Peter De Schrijver <pdeschrijver@nvidia.com> > > > > COP is a reset only clock. So this patch adds NO_CLK support > > then adds the COP clock. > > Do we actually need this clock upstream yet? > > IIRC, Prashant was working on implementing the common reset API, so I'd > prefer not to add any reset-only "clocks" to the clock driver, but > rather to simply make sure that the new reset driver exposes them. > It would be better if we can switch to the reset API. > > diff --git a/drivers/clk/tegra/clk-tegra114.c b/drivers/clk/tegra/clk-tegra114.c > > > enum tegra114_clk { > > - rtc = 4, timer = 5, uarta = 6, sdmmc2 = 9, i2s1 = 11, i2c1 = 12, > > + cop = 1, rtc = 4, timer = 5, uarta = 6, sdmmc2 = 9, i2s1 = 11, i2c1 = 12, > > To make this change, you would also need to edit > include/dt-bindings/clock/tegra114-car.h. BTW, why doesn't the Tegra > clock driver include that file rather than defining its own enum? History. include/dt-bindings/clock/tegra114-car.h didn't exist when clk-tegra114.c was written. Cheers, Peter.
On 08/19/2013 08:53 AM, Peter De Schrijver wrote: > On Wed, Aug 07, 2013 at 06:58:03PM +0200, Stephen Warren wrote: >> On 08/07/2013 05:25 AM, Mark Zhang wrote: >>> From: Peter De Schrijver <pdeschrijver@nvidia.com> >>> >>> COP is a reset only clock. So this patch adds NO_CLK support >>> then adds the COP clock. >> >> Do we actually need this clock upstream yet? >> >> IIRC, Prashant was working on implementing the common reset API, so I'd >> prefer not to add any reset-only "clocks" to the clock driver, but >> rather to simply make sure that the new reset driver exposes them. >> > > It would be better if we can switch to the reset API. > >>> diff --git a/drivers/clk/tegra/clk-tegra114.c b/drivers/clk/tegra/clk-tegra114.c >> >>> enum tegra114_clk { >>> - rtc = 4, timer = 5, uarta = 6, sdmmc2 = 9, i2s1 = 11, i2c1 = 12, >>> + cop = 1, rtc = 4, timer = 5, uarta = 6, sdmmc2 = 9, i2s1 = 11, i2c1 = 12, >> >> To make this change, you would also need to edit >> include/dt-bindings/clock/tegra114-car.h. BTW, why doesn't the Tegra >> clock driver include that file rather than defining its own enum? > > History. include/dt-bindings/clock/tegra114-car.h didn't exist when > clk-tegra114.c was written. Right, but then someone created tegra114-car.h, but didn't bother to follow through and update the clock driver:-(
diff --git a/drivers/clk/tegra/clk-periph-gate.c b/drivers/clk/tegra/clk-periph-gate.c index bafee98..092f256 100644 --- a/drivers/clk/tegra/clk-periph-gate.c +++ b/drivers/clk/tegra/clk-periph-gate.c @@ -51,6 +51,11 @@ static int clk_periph_is_enabled(struct clk_hw *hw) struct tegra_clk_periph_gate *gate = to_clk_periph_gate(hw); int state = 1; + if (gate->flags & TEGRA_PERIPH_NO_CLK) { + WARN_ON(1); + return 0; + } + if (!(read_enb(gate) & periph_clk_to_bit(gate))) state = 0; @@ -66,6 +71,11 @@ static int clk_periph_enable(struct clk_hw *hw) struct tegra_clk_periph_gate *gate = to_clk_periph_gate(hw); unsigned long flags = 0; + if (gate->flags & TEGRA_PERIPH_NO_CLK) { + WARN_ON(1); + return -EINVAL; + } + spin_lock_irqsave(&periph_ref_lock, flags); gate->enable_refcnt[gate->clk_num]++; @@ -102,6 +112,11 @@ static void clk_periph_disable(struct clk_hw *hw) struct tegra_clk_periph_gate *gate = to_clk_periph_gate(hw); unsigned long flags = 0; + if (gate->flags & TEGRA_PERIPH_NO_CLK) { + WARN_ON(1); + return; + } + spin_lock_irqsave(&periph_ref_lock, flags); gate->enable_refcnt[gate->clk_num]--; diff --git a/drivers/clk/tegra/clk-tegra114.c b/drivers/clk/tegra/clk-tegra114.c index 71db736..7172faf 100644 --- a/drivers/clk/tegra/clk-tegra114.c +++ b/drivers/clk/tegra/clk-tegra114.c @@ -863,7 +863,7 @@ static unsigned long tegra114_input_freq[] = { mux_d_audio_clk_idx, 0) enum tegra114_clk { - rtc = 4, timer = 5, uarta = 6, sdmmc2 = 9, i2s1 = 11, i2c1 = 12, + cop = 1, rtc = 4, timer = 5, uarta = 6, sdmmc2 = 9, i2s1 = 11, i2c1 = 12, ndflash = 13, sdmmc1 = 14, sdmmc4 = 15, pwm = 17, i2s2 = 18, epp = 19, gr_2d = 21, usbd = 22, isp = 23, gr_3d = 24, disp2 = 26, disp1 = 27, host1x = 28, vcp = 29, i2s0 = 30, apbdma = 34, kbc = 36, kfuse = 40, @@ -1921,6 +1921,13 @@ static __init void tegra114_periph_clk_init(void __iomem *clk_base) int i; u32 val; + /* cop */ + clk = tegra_clk_register_periph_gate("cop", NULL, TEGRA_PERIPH_NO_CLK, + clk_base, CLK_IGNORE_UNUSED, 1, + &periph_l_regs, + periph_clk_enb_refcnt); + clks[cop] = clk; + /* apbdma */ clk = tegra_clk_register_periph_gate("apbdma", "clk_m", 0, clk_base, 0, 34, &periph_h_regs, diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h index 07cfacd..0124e11 100644 --- a/drivers/clk/tegra/clk.h +++ b/drivers/clk/tegra/clk.h @@ -375,6 +375,7 @@ struct tegra_clk_periph_regs { * bus to flush the write operation in apb bus. This flag indicates * that this peripheral is in apb bus. * TEGRA_PERIPH_WAR_1005168 - Apply workaround for Tegra114 MSENC bug + * TEGRA_PERIPH_NO_CLK - Reset only clock node */ struct tegra_clk_periph_gate { u32 magic; @@ -395,6 +396,7 @@ struct tegra_clk_periph_gate { #define TEGRA_PERIPH_MANUAL_RESET BIT(1) #define TEGRA_PERIPH_ON_APB BIT(2) #define TEGRA_PERIPH_WAR_1005168 BIT(3) +#define TEGRA_PERIPH_NO_CLK BIT(4) void tegra_periph_reset(struct tegra_clk_periph_gate *gate, bool assert); extern const struct clk_ops tegra_clk_periph_gate_ops;