[U-Boot] [PATCH v2] armv7: s5p: pwm driver support

[Minkyu Kang]

Dear Donghwa Lee,

On 2 March 2011 11:23, Donghwa Lee <dh09.lee at samsung.com> wrote:
> This is common pwm driver version2 of S5P.
>
> Signed-off-by: Donghwa Lee <dh09.lee at samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park at samsung.com>
> ---

Please add change log here.
http://www.denx.de/wiki/view/U-Boot/Patches#Sending_updated_patch_versions

>  arch/arm/cpu/armv7/s5p-common/Makefile  |    1 +
>  arch/arm/cpu/armv7/s5p-common/pwm.c     |  218 +++++++++++++++++++++++++++++++
>  arch/arm/include/asm/arch-s5pc1xx/pwm.h |   14 ++
>  arch/arm/include/asm/arch-s5pc2xx/pwm.h |   14 ++
>  include/pwm.h                           |   32 +++++
>  5 files changed, 279 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/cpu/armv7/s5p-common/pwm.c
>  create mode 100644 include/pwm.h
>
> diff --git a/arch/arm/cpu/armv7/s5p-common/Makefile b/arch/arm/cpu/armv7/s5p-common/Makefile
> index 922cd95..da5623d 100644
> --- a/arch/arm/cpu/armv7/s5p-common/Makefile
> +++ b/arch/arm/cpu/armv7/s5p-common/Makefile
> @@ -28,6 +28,7 @@ LIB   = $(obj)libs5p-common.o
>  COBJS-y                += cpu_info.o
>  COBJS-y                += timer.o

remove this space

> +COBJS-$(CONFIG_PWM)            += pwm.o

need space here

>  SRCS   := $(SOBJS:.o=.S) $(COBJS:.o=.c)
>  OBJS   := $(addprefix $(obj),$(COBJS-y) $(SOBJS))
>
> diff --git a/arch/arm/cpu/armv7/s5p-common/pwm.c b/arch/arm/cpu/armv7/s5p-common/pwm.c
> new file mode 100644
> index 0000000..2e0601a
> --- /dev/null
> +++ b/arch/arm/cpu/armv7/s5p-common/pwm.c
> +void pwm_disable(int pwm_id)
> +{
> +       const struct s5p_timer *pwm = (struct s5p_timer *)samsung_get_base_timer();
> +       unsigned long tcon;
> +
> +       tcon = readl(&pwm->tcon);
> +       if (pwm_id == 0)
> +               tcon &= ~TCON0_START;
> +       else
> +               tcon &= ~TCON_START(pwm_id);
> +
> +       writel(tcon, &pwm->tcon);
> +

remove this space.

> +}
> +

> diff --git a/arch/arm/include/asm/arch-s5pc1xx/pwm.h b/arch/arm/include/asm/arch-s5pc1xx/pwm.h
> index 0369968..843d66d 100644
> --- a/arch/arm/include/asm/arch-s5pc1xx/pwm.h
> +++ b/arch/arm/include/asm/arch-s5pc1xx/pwm.h
> @@ -29,6 +29,20 @@
>  /* start bit of PWM Timer 4 */
>  #define TCON4_START            (1 << 20)
>
> +/* Interval mode(Auto Reload) of PWM Timer 0 */
> +#define TCON0_AUTO_RELOAD      (1 << 3)
> +/* Inverter On/Off */
> +#define TCON0_INVERTER         (1 << 2)
> +/* Update TCNTB0 */
> +#define TCON0_UPDATE           (1 << 1)
> +/* start bit of PWM Timer 0 */
> +#define TCON0_START            (1 << 0)
> +
> +#define TCON_AUTO_RELOAD(x)    (1 << (((x + 1) * 4) + 3))
> +#define TCON_INVERTER(x)       (1 << (((x + 1) * 4) + 2))
> +#define TCON_UPDATE(x)         (1 << (((x + 1) * 4) + 1))
> +#define TCON_START(x)          (1 << (((x + 1) * 4)))

I think we can combine these macros.
please check bellow macros.

#define TCON_OFFSET(x)		((x + 1) * (!!x) << 2)

#define TCON_START(x)		(1 << TCON_OFFSET(x))
#define TCON_UPDATE(x)		(1 << TCON_OFFSET(x) + 1)
#define TCON_INVERTER(x)	(1 << TCON_OFFSET(x) + 2)
#define TCON_AUTO_RELOAD(x)	(1 << TCON_OFFSET(x) + 3)

test result.

TCON 0 = 0
TCON 1 = 8
TCON 2 = 12
TCON 3 = 16
TCON 4 = 20

How you think?
then, we can remove all of "if (pwm_id == 0)".

> +
>  #ifndef __ASSEMBLY__
>  struct s5p_timer {
>        unsigned int    tcfg0;
> diff --git a/arch/arm/include/asm/arch-s5pc2xx/pwm.h b/arch/arm/include/asm/arch-s5pc2xx/pwm.h
> index 0369968..843d66d 100644
> --- a/arch/arm/include/asm/arch-s5pc2xx/pwm.h
> +++ b/arch/arm/include/asm/arch-s5pc2xx/pwm.h
> @@ -29,6 +29,20 @@
>  /* start bit of PWM Timer 4 */
>  #define TCON4_START            (1 << 20)
>
> +/* Interval mode(Auto Reload) of PWM Timer 0 */
> +#define TCON0_AUTO_RELOAD      (1 << 3)
> +/* Inverter On/Off */
> +#define TCON0_INVERTER         (1 << 2)
> +/* Update TCNTB0 */
> +#define TCON0_UPDATE           (1 << 1)
> +/* start bit of PWM Timer 0 */
> +#define TCON0_START            (1 << 0)
> +
> +#define TCON_AUTO_RELOAD(x)    (1 << (((x + 1) * 4) + 3))
> +#define TCON_INVERTER(x)       (1 << (((x + 1) * 4) + 2))
> +#define TCON_UPDATE(x)         (1 << (((x + 1) * 4) + 1))
> +#define TCON_START(x)          (1 << (((x + 1) * 4)))

ditto.

> +
>  #ifndef __ASSEMBLY__
>  struct s5p_timer {
>        unsigned int    tcfg0;
> diff --git a/include/pwm.h b/include/pwm.h
> new file mode 100644
> index 0000000..714a237
> --- /dev/null
> +++ b/include/pwm.h
> +
> +int    pwm_init                (int pwm_id, int div, int invert);
> +int    pwm_config              (int pwm_id, int duty_ns, int period_ns);
> +int    pwm_enable              (int pwm_id);
> +void   pwm_disable             (int pwm_id);
> +
> +#endif /* _pwm_h_ */
> +

please remove this space too.

> --

Thanks
Minkyu Kang
-- 
from. prom.
www.promsoft.net