[PATCH v2 2/2] Enable Stop_enable mode during configuration of pwm.
Shawn Guo
shawn.guo at linaro.org
Mon Aug 27 19:05:13 EDT 2012
On Mon, Aug 27, 2012 at 05:34:32PM +0200, HACHIMI Samir wrote:
> From: Samir Hachimi <shachimi at adeneo-embedded.com>
>
> Enable Stop_enable mode during configuration of pwm.
> Check architecture by looking in driver_data instead of cpu_is_xxx.
It does 2 things, and should be split into 2 patches. Also you should
have a better commit log for the thing you are doing. For example, why
you need to enable STOPEN during configuration, what problem you are
running into without doing so, etc.
>
> Signed-off-by: Samir Hachimi <shachimi at adeneo-embedded.com>
> ---
> drivers/pwm/pwm-imx.c | 59 +++++++++++++++++++++++++++++++++++-------------
> 1 files changed, 43 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
> index 0519bf2..7df919f 100644
> --- a/drivers/pwm/pwm-imx.c
> +++ b/drivers/pwm/pwm-imx.c
> @@ -32,6 +32,7 @@
> #define MX3_PWMSAR 0x0C /* PWM Sample Register */
> #define MX3_PWMPR 0x10 /* PWM Period Register */
> #define MX3_PWMCR_PRESCALER(x) (((x - 1) & 0xFFF) << 4)
> +#define MX3_PWMCR_STOPEN (1 << 25)
> #define MX3_PWMCR_DOZEEN (1 << 24)
> #define MX3_PWMCR_WAITEN (1 << 23)
> #define MX3_PWMCR_DBGEN (1 << 22)
> @@ -39,6 +40,16 @@
> #define MX3_PWMCR_CLKSRC_IPG (1 << 16)
> #define MX3_PWMCR_EN (1 << 0)
>
> +/* Use the platform_id to distinguish different Archs. */
> +#define IS_MX1 0x0
> +#define IS_MX21 0x1
> +#define IS_MX25 0x2
> +#define IS_MX6Q 0x3
I prefer to use enum for these ...
> +#define PWM_IS_MX1(x) ((x)->id_entry->driver_data == IS_MX1)
> +#define PWM_IS_MX21(x) ((x)->id_entry->driver_data == IS_MX21)
> +#define PWM_IS_MX25(x) ((x)->id_entry->driver_data == IS_MX25)
> +#define PWM_IS_MX6Q(x) ((x)->id_entry->driver_data == IS_MX6Q)
> +
... and inline function for these. drivers/mmc/host/sdhci-esdhc-imx.c
could be an example here.
Also, I see PWM_IS_MX6Q used nowhere, which probably means the pwm on
imx6q is compatible with imx25 one, and we should not introduce MX6Q
type at all.
> struct imx_chip {
> struct clk *clk;
>
> @@ -46,6 +57,7 @@ struct imx_chip {
> void __iomem *mmio_base;
>
> struct pwm_chip chip;
> + struct platform_device *pdev;
> };
>
> #define to_imx_chip(chip) container_of(chip, struct imx_chip, chip)
> @@ -65,7 +77,7 @@ static int imx_pwm_config(struct pwm_chip *chip,
> return rc;
> }
>
> - if (!(cpu_is_mx1() || cpu_is_mx21())) {
> + if (!(PWM_IS_MX1(imx->pdev) || PWM_IS_MX21(imx->pdev))) {
> unsigned long long c;
> unsigned long period_cycles, duty_cycles, prescale;
> u32 cr;
> @@ -78,36 +90,33 @@ static int imx_pwm_config(struct pwm_chip *chip,
> prescale = period_cycles / 0x10000 + 1;
>
> period_cycles /= prescale;
> - c = (unsigned long long)period_cycles * duty_ns;
> - do_div(c, period_ns);
> - duty_cycles = c;
>
> - /*
> - * according to imx pwm RM, the real period value should be
> - * PERIOD value in PWMPR plus 2.
> + /* the chip documentation says the counter counts up to
> + * period_cycles + 1 and then is reset to 0, so the
> + * actual period of the PWM wave is period_cycles + 2
> */
> - if (period_cycles > 2)
> - period_cycles -= 2;
> - else
> - period_cycles = 0;
> + c = (unsigned long long)(period_cycles + 2) * duty_ns;
> + do_div(c, period_ns);
> + duty_cycles = c;
>
> writel(duty_cycles, imx->mmio_base + MX3_PWMSAR);
> writel(period_cycles, imx->mmio_base + MX3_PWMPR);
>
> cr = MX3_PWMCR_PRESCALER(prescale) |
> - MX3_PWMCR_DOZEEN | MX3_PWMCR_WAITEN | MX3_PWMCR_DBGEN;
> + MX3_PWMCR_DOZEEN | MX3_PWMCR_WAITEN |
> + MX3_PWMCR_DBGEN | MX3_PWMCR_STOPEN;
>
> /* If the PWM is enabled, keep it so. */
> if (imx->clk_enabled)
> cr |= MX3_PWMCR_EN;
>
> - if (cpu_is_mx25())
> + if (PWM_IS_MX25(imx->pdev))
> cr |= MX3_PWMCR_CLKSRC_IPG;
> else
> cr |= MX3_PWMCR_CLKSRC_IPG_HIGH;
>
> writel(cr, imx->mmio_base + MX3_PWMCR);
> - } else if (cpu_is_mx1() || cpu_is_mx21()) {
> + } else if (PWM_IS_MX1(imx->pdev) || PWM_IS_MX21(imx->pdev)) {
> /* The PWM subsystem allows for exact frequencies. However,
> * I cannot connect a scope on my device to the PWM line and
> * thus cannot provide the program the PWM controller
> @@ -150,7 +159,7 @@ static int imx_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
> if (rc)
> return rc;
>
> - if (!(cpu_is_mx1() || cpu_is_mx21())) {
> + if (!(PWM_IS_MX1(imx->pdev) || PWM_IS_MX21(imx->pdev))) {
> u32 cr = readl(imx->mmio_base + MX3_PWMCR);
> cr |= MX3_PWMCR_EN;
> writel(cr, imx->mmio_base + MX3_PWMCR);
> @@ -167,7 +176,7 @@ static void imx_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> if (!imx->clk_enabled)
> return;
>
> - if (!(cpu_is_mx1() || cpu_is_mx21())) {
> + if (!(PWM_IS_MX1(imx->pdev) || PWM_IS_MX21(imx->pdev))) {
> u32 cr = readl(imx->mmio_base + MX3_PWMCR);
> cr &= ~MX3_PWMCR_EN;
> writel(cr, imx->mmio_base + MX3_PWMCR);
> @@ -207,6 +216,7 @@ static int __devinit imx_pwm_probe(struct platform_device *pdev)
> imx->chip.npwm = 1;
>
> imx->clk_enabled = 0;
> + imx->pdev = pdev;
>
> r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> if (r == NULL) {
> @@ -237,9 +247,26 @@ static int __devexit imx_pwm_remove(struct platform_device *pdev)
> return pwmchip_remove(&imx->chip);
> }
>
> +static const struct platform_device_id pwm_ids[] = {
> + { .name = "imx1-pwm", .driver_data = IS_MX1, },
> + { .name = "imx21-pwm", .driver_data = IS_MX21, },
> + { .name = "imx25-pwm", .driver_data = IS_MX25, },
> + { .name = "imx6q-pwm", .driver_data = IS_MX6Q, },
> + {},
> +};
You are not using the table as .id_table of imx_pwm_driver, so these
PWM_IS_xxx stuff does not work at all. Also, changing to use
platform_device_id will require updates to machine code that registers
platform_device for non-DT case.
> +
> +static const struct of_device_id mxc_pwm_dt_ids[] = {
> + {
> + .compatible = "fsl,imx6q-pwm",
> + .data = (void *)&pwm_ids[IS_MX6Q]
> + },
> + { /* sentinel */ }
> +};
> +
> static struct platform_driver imx_pwm_driver = {
> .driver = {
> .name = "mxc_pwm",
> + .of_match_table = of_match_ptr(mxc_pwm_dt_ids),
> },
> .probe = imx_pwm_probe,
> .remove = __devexit_p(imx_pwm_remove),
> --
> 1.7.1
--
Regards,
Shawn
More information about the linux-arm-kernel
mailing list