From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757466AbaGWQAa (ORCPT ); Wed, 23 Jul 2014 12:00:30 -0400 Received: from gloria.sntech.de ([95.129.55.99]:48723 "EHLO gloria.sntech.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754151AbaGWQA1 (ORCPT ); Wed, 23 Jul 2014 12:00:27 -0400 From: Heiko =?ISO-8859-1?Q?St=FCbner?= To: Caesar Wang Cc: thierry.reding@gmail.com, b.galvani@gmail.com, linux-arm-kernel@lists.infradead.org, linux-pwm@vger.kernel.org, linux-kernel@vger.kernel.org, cf@rock-chips.com, huangtao@rock-chips.com, hj@rock-chips.com, xjq@rock-chips.com, addy.ke@rock-chips.com Subject: Re: [PATCH v3 2/2] pwm: rockchip: Added to support for RK3288 SoC Date: Wed, 23 Jul 2014 18:01:46 +0200 Message-ID: <1648580.P46ct6rREY@diego> User-Agent: KMail/4.11.5 (Linux/3.13-1-amd64; KDE/4.11.3; x86_64; ; ) In-Reply-To: <1406097521-6457-3-git-send-email-caesar.wang@rock-chips.com> References: <1406097521-6457-1-git-send-email-caesar.wang@rock-chips.com> <1406097521-6457-3-git-send-email-caesar.wang@rock-chips.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Caesar. Am Mittwoch, 23. Juli 2014, 14:38:41 schrieb Caesar Wang: > This patch added to support the PWM controller found on > RK3288 SoC. > > Signed-off-by: Caesar Wang > --- > drivers/pwm/pwm-rockchip.c | 141 > +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 122 > insertions(+), 19 deletions(-) > > diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c > index eec2145..8d72a98 100644 > --- a/drivers/pwm/pwm-rockchip.c > +++ b/drivers/pwm/pwm-rockchip.c > @@ -2,6 +2,7 @@ > * PWM driver for Rockchip SoCs > * > * Copyright (C) 2014 Beniamino Galvani > + * Copyright (C) 2014 Caesar Wang you might want to check who actually holds the copyright for your contributions, I guess a "Copyright (C) 2014 Rockchip"-something would be more appropriate? > * > * This program is free software; you can redistribute it and/or > * modify it under the terms of the GNU General Public License > @@ -12,6 +13,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -25,17 +27,89 @@ > > #define PRESCALER 2 > > +#define PWM_ENABLE (1 << 0) > +#define PWM_CONTINUOUS (1 << 1) > +#define PWM_DUTY_POSITIVE (1 << 3) > +#define PWM_INACTIVE_NEGATIVE (0 << 4) > +#define PWM_OUTPUT_LEFT (0 << 5) > +#define PWM_LP_DISABLE (0 << 8) > + > struct rockchip_pwm_chip { > struct pwm_chip chip; > struct clk *clk; > + const struct rockchip_pwm_data *data; > void __iomem *base; > }; > > +struct rockchip_pwm_regs { > + unsigned long duty; > + unsigned long period; > + unsigned long cntr; > + unsigned long ctrl; > +}; > + > +struct rockchip_pwm_data { > + struct rockchip_pwm_regs regs; > + unsigned int prescaler; > + > + void (*set_enable)(struct pwm_chip *chip, bool enable); > +}; > + > static inline struct rockchip_pwm_chip *to_rockchip_pwm_chip(struct > pwm_chip *c) { > return container_of(c, struct rockchip_pwm_chip, chip); > } > > +static void rockchip_pwm_set_enable_v1(struct pwm_chip *chip, bool enable) > +{ > + struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip); > + u32 val = 0; > + u32 enable_conf = PWM_CTRL_OUTPUT_EN | PWM_CTRL_TIMER_EN; > + > + val = readl_relaxed(pc->base + pc->data->regs.ctrl); > + > + if (enable) > + val |= enable_conf; > + else > + val &= ~enable_conf; > + > + writel_relaxed(val, pc->base + pc->data->regs.ctrl); > +} > + > +static void rockchip_pwm_set_enable_v2(struct pwm_chip *chip, bool enable) > +{ > + struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip); > + u32 val = 0; > + u32 enable_conf = PWM_OUTPUT_LEFT | PWM_LP_DISABLE | PWM_ENABLE | > + PWM_CONTINUOUS | PWM_DUTY_POSITIVE | PWM_INACTIVE_NEGATIVE; > + > + val = readl_relaxed(pc->base + pc->data->regs.ctrl); > + > + if (enable) > + val |= enable_conf; > + else > + val &= ~enable_conf; > + > + writel_relaxed(val, pc->base + pc->data->regs.ctrl); > +} > + > +static void rockchip_pwm_set_enable_vop(struct pwm_chip *chip, bool enable) > +{ > + struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip); > + u32 val = 0; > + u32 enable_conf = PWM_OUTPUT_LEFT | PWM_LP_DISABLE | PWM_ENABLE | > + PWM_CONTINUOUS | PWM_DUTY_POSITIVE | PWM_INACTIVE_NEGATIVE; > + > + val = readl_relaxed(pc->base + pc->data->regs.ctrl); > + > + if (enable) > + val |= enable_conf; > + else > + val &= ~enable_conf; > + > + writel_relaxed(val, pc->base + pc->data->regs.ctrl); > +} not sure if I'm just blind ... do rockchip_pwm_set_enable_v2 and rockchip_pwm_set_enable_vop differ at all? If they don't differ, I guess pwm_data_vop should just use rockchip_pwm_set_enable_v2 instead of duplicating it. Heiko > + > static int rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device > *pwm, int duty_ns, int period_ns) > { > @@ -52,20 +126,20 @@ static int rockchip_pwm_config(struct pwm_chip *chip, > struct pwm_device *pwm, * default prescaler value for all practical clock > rate values. > */ > div = clk_rate * period_ns; > - do_div(div, PRESCALER * NSEC_PER_SEC); > + do_div(div, pc->data->prescaler * NSEC_PER_SEC); > period = div; > > div = clk_rate * duty_ns; > - do_div(div, PRESCALER * NSEC_PER_SEC); > + do_div(div, pc->data->prescaler * NSEC_PER_SEC); > duty = div; > > ret = clk_enable(pc->clk); > if (ret) > return ret; > > - writel(period, pc->base + PWM_LRC); > - writel(duty, pc->base + PWM_HRC); > - writel(0, pc->base + PWM_CNTR); > + writel(period, pc->base + pc->data->regs.period); > + writel(duty, pc->base + pc->data->regs.duty); > + writel(0, pc->base + pc->data->regs.cntr); > > clk_disable(pc->clk); > > @@ -76,15 +150,12 @@ static int rockchip_pwm_enable(struct pwm_chip *chip, > struct pwm_device *pwm) { > struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip); > int ret; > - u32 val; > > ret = clk_enable(pc->clk); > if (ret) > return ret; > > - val = readl_relaxed(pc->base + PWM_CTRL); > - val |= PWM_CTRL_OUTPUT_EN | PWM_CTRL_TIMER_EN; > - writel_relaxed(val, pc->base + PWM_CTRL); > + pc->data->set_enable(chip, true); > > return 0; > } > @@ -92,11 +163,8 @@ static int rockchip_pwm_enable(struct pwm_chip *chip, > struct pwm_device *pwm) static void rockchip_pwm_disable(struct pwm_chip > *chip, struct pwm_device *pwm) { > struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip); > - u32 val; > > - val = readl_relaxed(pc->base + PWM_CTRL); > - val &= ~(PWM_CTRL_OUTPUT_EN | PWM_CTRL_TIMER_EN); > - writel_relaxed(val, pc->base + PWM_CTRL); > + pc->data->set_enable(chip, false); > > clk_disable(pc->clk); > } > @@ -108,12 +176,52 @@ static const struct pwm_ops rockchip_pwm_ops = { > .owner = THIS_MODULE, > }; > > +static const struct rockchip_pwm_data pwm_data_v1 = { > + .regs.duty = PWM_HRC, > + .regs.period = PWM_LRC, > + .regs.cntr = PWM_CNTR, > + .regs.ctrl = PWM_CTRL, > + .prescaler = PRESCALER, > + .set_enable = rockchip_pwm_set_enable_v1, > +}; > + > +static const struct rockchip_pwm_data pwm_data_v2 = { > + .regs.duty = PWM_LRC, > + .regs.period = PWM_HRC, > + .regs.cntr = PWM_CNTR, > + .regs.ctrl = PWM_CTRL, > + .prescaler = PRESCALER-1, > + .set_enable = rockchip_pwm_set_enable_v2, > +}; > + > +static const struct rockchip_pwm_data pwm_data_vop = { > + .regs.duty = PWM_LRC, > + .regs.period = PWM_HRC, > + .regs.cntr = PWM_CTRL, > + .regs.ctrl = PWM_CNTR, > + .prescaler = PRESCALER-1, > + .set_enable = rockchip_pwm_set_enable_vop, > +}; > + > +static const struct of_device_id rockchip_pwm_dt_ids[] = { > + { .compatible = "rockchip,rk2928-pwm", .data = &pwm_data_v1}, > + { .compatible = "rockchip,rk3288-pwm", .data = &pwm_data_v2}, > + { .compatible = "rockchip,vop-pwm", .data = &pwm_data_vop}, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, rockchip_pwm_dt_ids); > + > static int rockchip_pwm_probe(struct platform_device *pdev) > { > + const struct of_device_id *id; > struct rockchip_pwm_chip *pc; > struct resource *r; > int ret; > > + id = of_match_device(rockchip_pwm_dt_ids, &pdev->dev); > + if (!id) > + return -EINVAL; > + > pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL); > if (!pc) > return -ENOMEM; > @@ -133,6 +241,7 @@ static int rockchip_pwm_probe(struct platform_device > *pdev) > > platform_set_drvdata(pdev, pc); > > + pc->data = id->data; > pc->chip.dev = &pdev->dev; > pc->chip.ops = &rockchip_pwm_ops; > pc->chip.base = -1; > @@ -156,12 +265,6 @@ static int rockchip_pwm_remove(struct platform_device > *pdev) return pwmchip_remove(&pc->chip); > } > > -static const struct of_device_id rockchip_pwm_dt_ids[] = { > - { .compatible = "rockchip,rk2928-pwm" }, > - { /* sentinel */ } > -}; > -MODULE_DEVICE_TABLE(of, rockchip_pwm_dt_ids); > - > static struct platform_driver rockchip_pwm_driver = { > .driver = { > .name = "rockchip-pwm", From mboxrd@z Thu Jan 1 00:00:00 1970 From: heiko@sntech.de (Heiko =?ISO-8859-1?Q?St=FCbner?=) Date: Wed, 23 Jul 2014 18:01:46 +0200 Subject: [PATCH v3 2/2] pwm: rockchip: Added to support for RK3288 SoC In-Reply-To: <1406097521-6457-3-git-send-email-caesar.wang@rock-chips.com> References: <1406097521-6457-1-git-send-email-caesar.wang@rock-chips.com> <1406097521-6457-3-git-send-email-caesar.wang@rock-chips.com> Message-ID: <1648580.P46ct6rREY@diego> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Caesar. Am Mittwoch, 23. Juli 2014, 14:38:41 schrieb Caesar Wang: > This patch added to support the PWM controller found on > RK3288 SoC. > > Signed-off-by: Caesar Wang > --- > drivers/pwm/pwm-rockchip.c | 141 > +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 122 > insertions(+), 19 deletions(-) > > diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c > index eec2145..8d72a98 100644 > --- a/drivers/pwm/pwm-rockchip.c > +++ b/drivers/pwm/pwm-rockchip.c > @@ -2,6 +2,7 @@ > * PWM driver for Rockchip SoCs > * > * Copyright (C) 2014 Beniamino Galvani > + * Copyright (C) 2014 Caesar Wang you might want to check who actually holds the copyright for your contributions, I guess a "Copyright (C) 2014 Rockchip"-something would be more appropriate? > * > * This program is free software; you can redistribute it and/or > * modify it under the terms of the GNU General Public License > @@ -12,6 +13,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -25,17 +27,89 @@ > > #define PRESCALER 2 > > +#define PWM_ENABLE (1 << 0) > +#define PWM_CONTINUOUS (1 << 1) > +#define PWM_DUTY_POSITIVE (1 << 3) > +#define PWM_INACTIVE_NEGATIVE (0 << 4) > +#define PWM_OUTPUT_LEFT (0 << 5) > +#define PWM_LP_DISABLE (0 << 8) > + > struct rockchip_pwm_chip { > struct pwm_chip chip; > struct clk *clk; > + const struct rockchip_pwm_data *data; > void __iomem *base; > }; > > +struct rockchip_pwm_regs { > + unsigned long duty; > + unsigned long period; > + unsigned long cntr; > + unsigned long ctrl; > +}; > + > +struct rockchip_pwm_data { > + struct rockchip_pwm_regs regs; > + unsigned int prescaler; > + > + void (*set_enable)(struct pwm_chip *chip, bool enable); > +}; > + > static inline struct rockchip_pwm_chip *to_rockchip_pwm_chip(struct > pwm_chip *c) { > return container_of(c, struct rockchip_pwm_chip, chip); > } > > +static void rockchip_pwm_set_enable_v1(struct pwm_chip *chip, bool enable) > +{ > + struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip); > + u32 val = 0; > + u32 enable_conf = PWM_CTRL_OUTPUT_EN | PWM_CTRL_TIMER_EN; > + > + val = readl_relaxed(pc->base + pc->data->regs.ctrl); > + > + if (enable) > + val |= enable_conf; > + else > + val &= ~enable_conf; > + > + writel_relaxed(val, pc->base + pc->data->regs.ctrl); > +} > + > +static void rockchip_pwm_set_enable_v2(struct pwm_chip *chip, bool enable) > +{ > + struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip); > + u32 val = 0; > + u32 enable_conf = PWM_OUTPUT_LEFT | PWM_LP_DISABLE | PWM_ENABLE | > + PWM_CONTINUOUS | PWM_DUTY_POSITIVE | PWM_INACTIVE_NEGATIVE; > + > + val = readl_relaxed(pc->base + pc->data->regs.ctrl); > + > + if (enable) > + val |= enable_conf; > + else > + val &= ~enable_conf; > + > + writel_relaxed(val, pc->base + pc->data->regs.ctrl); > +} > + > +static void rockchip_pwm_set_enable_vop(struct pwm_chip *chip, bool enable) > +{ > + struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip); > + u32 val = 0; > + u32 enable_conf = PWM_OUTPUT_LEFT | PWM_LP_DISABLE | PWM_ENABLE | > + PWM_CONTINUOUS | PWM_DUTY_POSITIVE | PWM_INACTIVE_NEGATIVE; > + > + val = readl_relaxed(pc->base + pc->data->regs.ctrl); > + > + if (enable) > + val |= enable_conf; > + else > + val &= ~enable_conf; > + > + writel_relaxed(val, pc->base + pc->data->regs.ctrl); > +} not sure if I'm just blind ... do rockchip_pwm_set_enable_v2 and rockchip_pwm_set_enable_vop differ at all? If they don't differ, I guess pwm_data_vop should just use rockchip_pwm_set_enable_v2 instead of duplicating it. Heiko > + > static int rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device > *pwm, int duty_ns, int period_ns) > { > @@ -52,20 +126,20 @@ static int rockchip_pwm_config(struct pwm_chip *chip, > struct pwm_device *pwm, * default prescaler value for all practical clock > rate values. > */ > div = clk_rate * period_ns; > - do_div(div, PRESCALER * NSEC_PER_SEC); > + do_div(div, pc->data->prescaler * NSEC_PER_SEC); > period = div; > > div = clk_rate * duty_ns; > - do_div(div, PRESCALER * NSEC_PER_SEC); > + do_div(div, pc->data->prescaler * NSEC_PER_SEC); > duty = div; > > ret = clk_enable(pc->clk); > if (ret) > return ret; > > - writel(period, pc->base + PWM_LRC); > - writel(duty, pc->base + PWM_HRC); > - writel(0, pc->base + PWM_CNTR); > + writel(period, pc->base + pc->data->regs.period); > + writel(duty, pc->base + pc->data->regs.duty); > + writel(0, pc->base + pc->data->regs.cntr); > > clk_disable(pc->clk); > > @@ -76,15 +150,12 @@ static int rockchip_pwm_enable(struct pwm_chip *chip, > struct pwm_device *pwm) { > struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip); > int ret; > - u32 val; > > ret = clk_enable(pc->clk); > if (ret) > return ret; > > - val = readl_relaxed(pc->base + PWM_CTRL); > - val |= PWM_CTRL_OUTPUT_EN | PWM_CTRL_TIMER_EN; > - writel_relaxed(val, pc->base + PWM_CTRL); > + pc->data->set_enable(chip, true); > > return 0; > } > @@ -92,11 +163,8 @@ static int rockchip_pwm_enable(struct pwm_chip *chip, > struct pwm_device *pwm) static void rockchip_pwm_disable(struct pwm_chip > *chip, struct pwm_device *pwm) { > struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip); > - u32 val; > > - val = readl_relaxed(pc->base + PWM_CTRL); > - val &= ~(PWM_CTRL_OUTPUT_EN | PWM_CTRL_TIMER_EN); > - writel_relaxed(val, pc->base + PWM_CTRL); > + pc->data->set_enable(chip, false); > > clk_disable(pc->clk); > } > @@ -108,12 +176,52 @@ static const struct pwm_ops rockchip_pwm_ops = { > .owner = THIS_MODULE, > }; > > +static const struct rockchip_pwm_data pwm_data_v1 = { > + .regs.duty = PWM_HRC, > + .regs.period = PWM_LRC, > + .regs.cntr = PWM_CNTR, > + .regs.ctrl = PWM_CTRL, > + .prescaler = PRESCALER, > + .set_enable = rockchip_pwm_set_enable_v1, > +}; > + > +static const struct rockchip_pwm_data pwm_data_v2 = { > + .regs.duty = PWM_LRC, > + .regs.period = PWM_HRC, > + .regs.cntr = PWM_CNTR, > + .regs.ctrl = PWM_CTRL, > + .prescaler = PRESCALER-1, > + .set_enable = rockchip_pwm_set_enable_v2, > +}; > + > +static const struct rockchip_pwm_data pwm_data_vop = { > + .regs.duty = PWM_LRC, > + .regs.period = PWM_HRC, > + .regs.cntr = PWM_CTRL, > + .regs.ctrl = PWM_CNTR, > + .prescaler = PRESCALER-1, > + .set_enable = rockchip_pwm_set_enable_vop, > +}; > + > +static const struct of_device_id rockchip_pwm_dt_ids[] = { > + { .compatible = "rockchip,rk2928-pwm", .data = &pwm_data_v1}, > + { .compatible = "rockchip,rk3288-pwm", .data = &pwm_data_v2}, > + { .compatible = "rockchip,vop-pwm", .data = &pwm_data_vop}, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, rockchip_pwm_dt_ids); > + > static int rockchip_pwm_probe(struct platform_device *pdev) > { > + const struct of_device_id *id; > struct rockchip_pwm_chip *pc; > struct resource *r; > int ret; > > + id = of_match_device(rockchip_pwm_dt_ids, &pdev->dev); > + if (!id) > + return -EINVAL; > + > pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL); > if (!pc) > return -ENOMEM; > @@ -133,6 +241,7 @@ static int rockchip_pwm_probe(struct platform_device > *pdev) > > platform_set_drvdata(pdev, pc); > > + pc->data = id->data; > pc->chip.dev = &pdev->dev; > pc->chip.ops = &rockchip_pwm_ops; > pc->chip.base = -1; > @@ -156,12 +265,6 @@ static int rockchip_pwm_remove(struct platform_device > *pdev) return pwmchip_remove(&pc->chip); > } > > -static const struct of_device_id rockchip_pwm_dt_ids[] = { > - { .compatible = "rockchip,rk2928-pwm" }, > - { /* sentinel */ } > -}; > -MODULE_DEVICE_TABLE(of, rockchip_pwm_dt_ids); > - > static struct platform_driver rockchip_pwm_driver = { > .driver = { > .name = "rockchip-pwm",