On 04/28/14 17:22, Sascha Hauer wrote:
On Mon, Apr 28, 2014 at 04:55:31PM +0800, Sean Cross wrote:
The IMX PWM block supports using both the system clock and a 32 kHz
clock for driving PWM events. For very long period lengths, use the
32 kHz clock instead of the high-speed clock.
Signed-off-by: Sean Cross <xobs@xxxxxxxxxx>
---
drivers/pwm/pwm-imx.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
index cc47733..8410455 100644
--- a/drivers/pwm/pwm-imx.c
+++ b/drivers/pwm/pwm-imx.c
@@ -36,9 +36,11 @@
#define MX3_PWMCR_DOZEEN (1 << 24)
#define MX3_PWMCR_WAITEN (1 << 23)
#define MX3_PWMCR_DBGEN (1 << 22)
+#define MX3_PWMCR_CLKSRC_IPG_32K (3 << 16)
#define MX3_PWMCR_CLKSRC_IPG_HIGH (2 << 16)
#define MX3_PWMCR_CLKSRC_IPG (1 << 16)
#define MX3_PWMCR_EN (1 << 0)
+#define MX3_SLOW_THRESHOLD_NS 100000
struct imx_chip {
struct clk *clk_per;
@@ -107,7 +109,13 @@ static int imx_pwm_config_v2(struct pwm_chip *chip,
unsigned long period_cycles, duty_cycles, prescale;
u32 cr;
- c = clk_get_rate(imx->clk_per);
+ if (duty_ns > MX3_SLOW_THRESHOLD_NS) {
If anything you have to check the period_cycles, not the duty_cycles.
100000 seems to be some arbitrary value suitable for some unspecified
ipg clock rate. I think you should use it when the period_cycles
register overflows.
In this case, it's for a backlight controller that requires a 100 Hz
clock. But you're right, hard-coding the values is a bad idea.
With the prescaler and the period_cycles register, it might not actually
overflow, but instead the resolution might become too poor to be
useful. The prescaler supports dividing by up to 4096, but it gets
difficult to hit a particular value.
So with my target of a 100 Hz clock, it appears as though no values
overflow yet I can't use IPG_HIGH.
What is the best way to handle this?
+ cr = MX3_PWMCR_CLKSRC_IPG_32K;
+ c = 32768;
How about providing a proper clock instead of hardcoding this?
That seems like a lot of work, and I don't see any other PWM devices
that do this. If I understand the clock system, I'd have to add the
following to pwm-imx.c:
- Each PWM would get three new clocks added to its struct imx_chip
-- a clk_ipg_mux, a clk_ipg_32k, and a clk_ipg_high
- The struct imx_chip would also gain a clk_ckil entry for the
32768 Hz clock
- These three clocks would need to be created on _probe() and
destroyed on _remove()
- clk_ipg_32k would get reparented to ckil (or whatever the 32768
Hz clock is on that platform)
- clk_ipg_high would get reparented to imx->clk_per
- The recalc_rate(), round_rate(), and set_rate() would need to be
implemented (and calculated) for clk_ipg_32k and clk_ipg_high
Additionally, "ckil" would need to be added to the pwm entry for every
dts file.
Is it worth it to add that much complexity? What does it get us to have
a proper clock? Or have I overestimated the amount of rework involved?
I'm happy to do it if you think it's necessary.
Sean
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
