* [PATCH 3/5] mx51: Add mx51 processor to the pwm driver
@ 2010-11-16 18:36 Fabio Estevam
2010-11-16 19:50 ` Uwe Kleine-König
0 siblings, 1 reply; 4+ messages in thread
From: Fabio Estevam @ 2010-11-16 18:36 UTC (permalink / raw)
To: linux-arm-kernel
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
arch/arm/plat-mxc/pwm.c |? ? 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/arm/plat-mxc/pwm.c b/arch/arm/plat-mxc/pwm.c
index c36f263..c6cdced 100644
--- a/arch/arm/plat-mxc/pwm.c
+++ b/arch/arm/plat-mxc/pwm.c
@@ -26,7 +26,7 @@
#define MX1_PWMP? ? 0x08???/* PWM Period Register */
-/* i.MX27, i.MX31, i.MX35 share the same PWM function block: */
+/* i.MX25/27/31/35/51 share the same PWM function block: */
#define MX3_PWMCR? ? ? ? ? ? ? ???0x00? ? /* PWM Control Register */
#define MX3_PWMSAR? ? ? ? ? ? ? ? 0x0C? ? /* PWM Sample Register */
@@ -57,7 +57,7 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
??? if (pwm == NULL || period_ns == 0 || duty_ns > period_ns)
??? ??? return -EINVAL;
-??? if (cpu_is_mx27() || cpu_is_mx3() || cpu_is_mx25()) {
+??? if (cpu_is_mx27() || cpu_is_mx3() || cpu_is_mx25() || cpu_is_mx51()) {
??? ??? unsigned long long c;
??? ??? unsigned long period_cycles, duty_cycles, prescale;
??? ??? u32 cr;
@@ -79,7 +79,7 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
??? ??? cr = MX3_PWMCR_PRESCALER(prescale) | MX3_PWMCR_EN;
-??? ??? if (cpu_is_mx25())
+??? ??? if (cpu_is_mx25() || cpu_is_mx51())
??? ??? ??? cr |= MX3_PWMCR_CLKSRC_IPG;
??? ??? else
??? ??? ??? cr |= MX3_PWMCR_CLKSRC_IPG_HIGH;
@@ -198,6 +198,7 @@ static int __devinit mxc_pwm_probe(struct platform_device *pdev)
??? if (IS_ERR(pwm->clk)) {
??? ??? ret = PTR_ERR(pwm->clk);
+??? ??? dev_err(&pdev->dev, "can't get PWM clock\n");
??? ??? goto err_free;
??? }
@@ -233,6 +234,7 @@ static int __devinit mxc_pwm_probe(struct platform_device *pdev)
??? mutex_unlock(&pwm_lock);
??? platform_set_drvdata(pdev, pwm);
+??? dev_info(&pdev->dev, "i.MX PWM Driver Enabled\n");
??? return 0;
err_free_mem:
--
1.6.0.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 3/5] mx51: Add mx51 processor to the pwm driver
2010-11-16 18:36 [PATCH 3/5] mx51: Add mx51 processor to the pwm driver Fabio Estevam
@ 2010-11-16 19:50 ` Uwe Kleine-König
2010-11-18 10:15 ` Sascha Hauer
0 siblings, 1 reply; 4+ messages in thread
From: Uwe Kleine-König @ 2010-11-16 19:50 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Nov 16, 2010 at 10:36:35AM -0800, Fabio Estevam wrote:
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> arch/arm/plat-mxc/pwm.c |? ? 8 +++++---
> 1 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/plat-mxc/pwm.c b/arch/arm/plat-mxc/pwm.c
> index c36f263..c6cdced 100644
> --- a/arch/arm/plat-mxc/pwm.c
> +++ b/arch/arm/plat-mxc/pwm.c
> @@ -26,7 +26,7 @@
> #define MX1_PWMP? ? 0x08???/* PWM Period Register */
>
>
> -/* i.MX27, i.MX31, i.MX35 share the same PWM function block: */
> +/* i.MX25/27/31/35/51 share the same PWM function block: */
If this is correct, ...
>
> #define MX3_PWMCR? ? ? ? ? ? ? ???0x00? ? /* PWM Control Register */
> #define MX3_PWMSAR? ? ? ? ? ? ? ? 0x0C? ? /* PWM Sample Register */
> @@ -57,7 +57,7 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
> ??? if (pwm == NULL || period_ns == 0 || duty_ns > period_ns)
> ??? ??? return -EINVAL;
>
> -??? if (cpu_is_mx27() || cpu_is_mx3() || cpu_is_mx25()) {
> +??? if (cpu_is_mx27() || cpu_is_mx3() || cpu_is_mx25() || cpu_is_mx51()) {
> ??? ??? unsigned long long c;
> ??? ??? unsigned long period_cycles, duty_cycles, prescale;
> ??? ??? u32 cr;
> @@ -79,7 +79,7 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
>
> ??? ??? cr = MX3_PWMCR_PRESCALER(prescale) | MX3_PWMCR_EN;
>
> -??? ??? if (cpu_is_mx25())
> +??? ??? if (cpu_is_mx25() || cpu_is_mx51())
I wonder why mx27 and mx3 are missing here.
> ??? ??? ??? cr |= MX3_PWMCR_CLKSRC_IPG;
> ??? ??? else
> ??? ??? ??? cr |= MX3_PWMCR_CLKSRC_IPG_HIGH;
> @@ -198,6 +198,7 @@ static int __devinit mxc_pwm_probe(struct platform_device *pdev)
>
> ??? if (IS_ERR(pwm->clk)) {
> ??? ??? ret = PTR_ERR(pwm->clk);
> +??? ??? dev_err(&pdev->dev, "can't get PWM clock\n");
> ??? ??? goto err_free;
> ??? }
>
> @@ -233,6 +234,7 @@ static int __devinit mxc_pwm_probe(struct platform_device *pdev)
> ??? mutex_unlock(&pwm_lock);
>
> ??? platform_set_drvdata(pdev, pwm);
> +??? dev_info(&pdev->dev, "i.MX PWM Driver Enabled\n");
> ??? return 0;
>
> err_free_mem:
Some general comments for this driver:
- should move to drivers/something
- IMHO should use platform ids to get rid of the cpu_is_ stuff
(And BTW, you forgot to use --thread making your mails being spread in
(at least my) lakml folder.)
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH 3/5] mx51: Add mx51 processor to the pwm driver
2010-11-16 19:50 ` Uwe Kleine-König
@ 2010-11-18 10:15 ` Sascha Hauer
2010-11-18 12:04 ` Uwe Kleine-König
0 siblings, 1 reply; 4+ messages in thread
From: Sascha Hauer @ 2010-11-18 10:15 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Nov 16, 2010 at 08:50:32PM +0100, Uwe Kleine-K?nig wrote:
> On Tue, Nov 16, 2010 at 10:36:35AM -0800, Fabio Estevam wrote:
> > Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> > ---
> > arch/arm/plat-mxc/pwm.c |? ? 8 +++++---
> > 1 files changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm/plat-mxc/pwm.c b/arch/arm/plat-mxc/pwm.c
> > index c36f263..c6cdced 100644
> > --- a/arch/arm/plat-mxc/pwm.c
> > +++ b/arch/arm/plat-mxc/pwm.c
> > @@ -26,7 +26,7 @@
> > #define MX1_PWMP? ? 0x08???/* PWM Period Register */
> >
> >
> > -/* i.MX27, i.MX31, i.MX35 share the same PWM function block: */
> > +/* i.MX25/27/31/35/51 share the same PWM function block: */
> If this is correct, ...
> >
> > #define MX3_PWMCR? ? ? ? ? ? ? ???0x00? ? /* PWM Control Register */
> > #define MX3_PWMSAR? ? ? ? ? ? ? ? 0x0C? ? /* PWM Sample Register */
> > @@ -57,7 +57,7 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
> > ??? if (pwm == NULL || period_ns == 0 || duty_ns > period_ns)
> > ??? ??? return -EINVAL;
> >
> > -??? if (cpu_is_mx27() || cpu_is_mx3() || cpu_is_mx25()) {
> > +??? if (cpu_is_mx27() || cpu_is_mx3() || cpu_is_mx25() || cpu_is_mx51()) {
> > ??? ??? unsigned long long c;
> > ??? ??? unsigned long period_cycles, duty_cycles, prescale;
> > ??? ??? u32 cr;
> > @@ -79,7 +79,7 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
> >
> > ??? ??? cr = MX3_PWMCR_PRESCALER(prescale) | MX3_PWMCR_EN;
> >
> > -??? ??? if (cpu_is_mx25())
> > +??? ??? if (cpu_is_mx25() || cpu_is_mx51())
> I wonder why mx27 and mx3 are missing here.
Look at the code. We are already inside a mx27/3/25 block and we have to
do this only on mx25.
>
> > ??? ??? ??? cr |= MX3_PWMCR_CLKSRC_IPG;
> > ??? ??? else
> > ??? ??? ??? cr |= MX3_PWMCR_CLKSRC_IPG_HIGH;
> > @@ -198,6 +198,7 @@ static int __devinit mxc_pwm_probe(struct platform_device *pdev)
> >
> > ??? if (IS_ERR(pwm->clk)) {
> > ??? ??? ret = PTR_ERR(pwm->clk);
> > +??? ??? dev_err(&pdev->dev, "can't get PWM clock\n");
> > ??? ??? goto err_free;
> > ??? }
> >
> > @@ -233,6 +234,7 @@ static int __devinit mxc_pwm_probe(struct platform_device *pdev)
> > ??? mutex_unlock(&pwm_lock);
> >
> > ??? platform_set_drvdata(pdev, pwm);
> > +??? dev_info(&pdev->dev, "i.MX PWM Driver Enabled\n");
> > ??? return 0;
> >
> > err_free_mem:
> Some general comments for this driver:
>
> - should move to drivers/something
> - IMHO should use platform ids to get rid of the cpu_is_ stuff
Agreed, but this of course should be the scope of another patch.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH 3/5] mx51: Add mx51 processor to the pwm driver
2010-11-18 10:15 ` Sascha Hauer
@ 2010-11-18 12:04 ` Uwe Kleine-König
0 siblings, 0 replies; 4+ messages in thread
From: Uwe Kleine-König @ 2010-11-18 12:04 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Nov 18, 2010 at 11:15:41AM +0100, Sascha Hauer wrote:
> On Tue, Nov 16, 2010 at 08:50:32PM +0100, Uwe Kleine-K?nig wrote:
> > On Tue, Nov 16, 2010 at 10:36:35AM -0800, Fabio Estevam wrote:
> > > Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> > > ---
> > > arch/arm/plat-mxc/pwm.c |? ? 8 +++++---
> > > 1 files changed, 5 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/arch/arm/plat-mxc/pwm.c b/arch/arm/plat-mxc/pwm.c
> > > index c36f263..c6cdced 100644
> > > --- a/arch/arm/plat-mxc/pwm.c
> > > +++ b/arch/arm/plat-mxc/pwm.c
> > > @@ -26,7 +26,7 @@
> > > #define MX1_PWMP? ? 0x08???/* PWM Period Register */
> > >
> > >
> > > -/* i.MX27, i.MX31, i.MX35 share the same PWM function block: */
> > > +/* i.MX25/27/31/35/51 share the same PWM function block: */
> > If this is correct, ...
> > >
> > > #define MX3_PWMCR? ? ? ? ? ? ? ???0x00? ? /* PWM Control Register */
> > > #define MX3_PWMSAR? ? ? ? ? ? ? ? 0x0C? ? /* PWM Sample Register */
> > > @@ -57,7 +57,7 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
> > > ??? if (pwm == NULL || period_ns == 0 || duty_ns > period_ns)
> > > ??? ??? return -EINVAL;
> > >
> > > -??? if (cpu_is_mx27() || cpu_is_mx3() || cpu_is_mx25()) {
> > > +??? if (cpu_is_mx27() || cpu_is_mx3() || cpu_is_mx25() || cpu_is_mx51()) {
> > > ??? ??? unsigned long long c;
> > > ??? ??? unsigned long period_cycles, duty_cycles, prescale;
> > > ??? ??? u32 cr;
> > > @@ -79,7 +79,7 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
> > >
> > > ??? ??? cr = MX3_PWMCR_PRESCALER(prescale) | MX3_PWMCR_EN;
> > >
> > > -??? ??? if (cpu_is_mx25())
> > > +??? ??? if (cpu_is_mx25() || cpu_is_mx51())
> > I wonder why mx27 and mx3 are missing here.
>
> Look at the code. We are already inside a mx27/3/25 block and we have to
> do this only on mx25.
My intention was to point out, that the statement above
"i.MX25/27/31/35/51 share the same PWM function block" isn't correct.
> >
> > > ??? ??? ??? cr |= MX3_PWMCR_CLKSRC_IPG;
> > > ??? ??? else
> > > ??? ??? ??? cr |= MX3_PWMCR_CLKSRC_IPG_HIGH;
> > > @@ -198,6 +198,7 @@ static int __devinit mxc_pwm_probe(struct platform_device *pdev)
> > >
> > > ??? if (IS_ERR(pwm->clk)) {
> > > ??? ??? ret = PTR_ERR(pwm->clk);
> > > +??? ??? dev_err(&pdev->dev, "can't get PWM clock\n");
> > > ??? ??? goto err_free;
> > > ??? }
> > >
> > > @@ -233,6 +234,7 @@ static int __devinit mxc_pwm_probe(struct platform_device *pdev)
> > > ??? mutex_unlock(&pwm_lock);
> > >
> > > ??? platform_set_drvdata(pdev, pwm);
> > > +??? dev_info(&pdev->dev, "i.MX PWM Driver Enabled\n");
> > > ??? return 0;
> > >
> > > err_free_mem:
> > Some general comments for this driver:
> >
> > - should move to drivers/something
> > - IMHO should use platform ids to get rid of the cpu_is_ stuff
>
> Agreed, but this of course should be the scope of another patch.
Ack, that's why I called it "general comments for this driver" :-)
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-11-18 12:04 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-16 18:36 [PATCH 3/5] mx51: Add mx51 processor to the pwm driver Fabio Estevam
2010-11-16 19:50 ` Uwe Kleine-König
2010-11-18 10:15 ` Sascha Hauer
2010-11-18 12:04 ` Uwe Kleine-König
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.