| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20190111205403.e4bgszqbrq65jsgh@pengutronix.de>
Date: Fri, 11 Jan 2019 21:54:03 +0100
From: Uwe Kleine-König
<u.kleine-koenig@...gutronix.de>
To: Sheetal Tigadoli <sheetal.tigadoli@...adcom.com>
Cc: Thierry Reding <thierry.reding@...il.com>,
Rob Herring <robh+dt@...nel.org>,
Florian Fainelli <f.fainelli@...il.com>,
Ray Jui <rjui@...adcom.com>,
Scott Branden <sbranden@...adcom.com>,
bcm-kernel-feedback-list@...adcom.com, linux-pwm@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
Praveen Kumar B <praveen.b@...adcom.com>
Subject: Re: [PATCH 2/3] drivers: pwm: pwm-bcm-kona: Add pwm-kona-v2 support
Hello,
On Fri, Jan 11, 2019 at 10:51:15AM +0530, Sheetal Tigadoli wrote:
> From: Praveen Kumar B <praveen.b@...adcom.com>
>
> Add support for new version of pwm-kona.
> Add support to make PWM changes configured and stable.
>
> Signed-off-by: Praveen Kumar B <praveen.b@...adcom.com>
> Reviewed-by: Ray Jui <ray.jui@...adcom.com>
> Reviewed-by: Scott Branden <scott.branden@...adcom.com>
> Signed-off-by: Sheetal Tigadoli <sheetal.tigadoli@...adcom.com>
> ---
> drivers/pwm/pwm-bcm-kona.c | 128 ++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 98 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/pwm/pwm-bcm-kona.c b/drivers/pwm/pwm-bcm-kona.c
> index 09a95ae..2b44ad8 100644
> --- a/drivers/pwm/pwm-bcm-kona.c
> +++ b/drivers/pwm/pwm-bcm-kona.c
> @@ -45,30 +45,39 @@
> * high or low depending on its state at that exact instant.
> */
>
> -#define PWM_CONTROL_OFFSET (0x00000000)
> +#define PWM_CONTROL_OFFSET 0x00000000
> #define PWM_CONTROL_SMOOTH_SHIFT(chan) (24 + (chan))
> #define PWM_CONTROL_TYPE_SHIFT(chan) (16 + (chan))
> #define PWM_CONTROL_POLARITY_SHIFT(chan) (8 + (chan))
> #define PWM_CONTROL_TRIGGER_SHIFT(chan) (chan)
>
> -#define PRESCALE_OFFSET (0x00000004)
> +#define PRESCALE_OFFSET 0x00000004
> #define PRESCALE_SHIFT(chan) ((chan) << 2)
> #define PRESCALE_MASK(chan) (0x7 << PRESCALE_SHIFT(chan))
> -#define PRESCALE_MIN (0x00000000)
> -#define PRESCALE_MAX (0x00000007)
> +#define PRESCALE_MIN 0x00000000
> +#define PRESCALE_MAX 0x00000007
>
> #define PERIOD_COUNT_OFFSET(chan) (0x00000008 + ((chan) << 3))
> -#define PERIOD_COUNT_MIN (0x00000002)
> -#define PERIOD_COUNT_MAX (0x00ffffff)
> +#define PERIOD_COUNT_MIN 0x00000002
> +#define PERIOD_COUNT_MAX 0x00ffffff
>
> #define DUTY_CYCLE_HIGH_OFFSET(chan) (0x0000000c + ((chan) << 3))
> -#define DUTY_CYCLE_HIGH_MIN (0x00000000)
> -#define DUTY_CYCLE_HIGH_MAX (0x00ffffff)
> +#define DUTY_CYCLE_HIGH_MIN 0x00000000
> +#define DUTY_CYCLE_HIGH_MAX 0x00ffffff
All these are unrelated changes.
> +#define PWM_MONITOR_OFFSET 0xb0
> +#define PWM_MONITOR_TIMEOUT_US 5
> +
> +enum kona_pwmc_ver {
> + KONA_PWM_V1 = 1,
> + KONA_PWM_V2
> +};
>
> struct kona_pwmc {
> struct pwm_chip chip;
> void __iomem *base;
> struct clk *clk;
> + enum kona_pwmc_ver version;
> };
>
> static inline struct kona_pwmc *to_kona_pwmc(struct pwm_chip *_chip)
> @@ -76,11 +85,40 @@ static inline struct kona_pwmc *to_kona_pwmc(struct pwm_chip *_chip)
> return container_of(_chip, struct kona_pwmc, chip);
> }
>
> +static int kona_pwmc_wait_stable(struct pwm_chip *chip, unsigned int chan,
> + unsigned int kona_ver)
> +{
> + struct kona_pwmc *kp = to_kona_pwmc(chip);
> + unsigned int value;
> + unsigned int count = PWM_MONITOR_TIMEOUT_US * 1000;
> +
> + switch (kona_ver) {
> + case KONA_PWM_V1:
> + /*
> + * There must be a min 400ns delay between clearing trigger and
> + * settingit. Failing to do this may result in no PWM signal.
Space missing.
> + */
> + ndelay(400);
> + return 0;
> + case KONA_PWM_V2:
> + do {
> + value = readl(kp->base + PWM_MONITOR_OFFSET);
> + if (!(value & (BIT(chan))))
> + return 0;
> + ndelay(1);
> + } while (count--);
> +
> + return -ETIMEDOUT;
> + default:
> + return -ENODEV;
> + }
I wonder if v1 and v2 are different enough to justify a separate driver
for v2.
> +}
> +
> /*
> * Clear trigger bit but set smooth bit to maintain old output.
> */
> -static void kona_pwmc_prepare_for_settings(struct kona_pwmc *kp,
> - unsigned int chan)
> +static int kona_pwmc_prepare_for_settings(struct kona_pwmc *kp,
> + unsigned int chan)
> {
> unsigned int value = readl(kp->base + PWM_CONTROL_OFFSET);
>
> @@ -88,14 +126,10 @@ static void kona_pwmc_prepare_for_settings(struct kona_pwmc *kp,
> value &= ~(1 << PWM_CONTROL_TRIGGER_SHIFT(chan));
> writel(value, kp->base + PWM_CONTROL_OFFSET);
>
> - /*
> - * There must be a min 400ns delay between clearing trigger and setting
> - * it. Failing to do this may result in no PWM signal.
> - */
> - ndelay(400);
> + return kona_pwmc_wait_stable(&kp->chip, chan, kp->version);
> }
>
> -static void kona_pwmc_apply_settings(struct kona_pwmc *kp, unsigned int chan)
> +static int kona_pwmc_apply_settings(struct kona_pwmc *kp, unsigned int chan)
> {
> unsigned int value = readl(kp->base + PWM_CONTROL_OFFSET);
>
> @@ -104,8 +138,7 @@ static void kona_pwmc_apply_settings(struct kona_pwmc *kp, unsigned int chan)
> value |= 1 << PWM_CONTROL_TRIGGER_SHIFT(chan);
> writel(value, kp->base + PWM_CONTROL_OFFSET);
>
> - /* Trigger bit must be held high for at least 400 ns. */
> - ndelay(400);
> + return kona_pwmc_wait_stable(&kp->chip, chan, kp->version);
> }
>
> static int kona_pwmc_config(struct pwm_chip *chip, struct pwm_device *pwm,
> @@ -115,6 +148,7 @@ static int kona_pwmc_config(struct pwm_chip *chip, struct pwm_device *pwm,
It would be great if you converted the driver to new-style (that is
implementing .apply instead of .config, .enable, .disable and
.set_polarity). This usually simplifies the driver and maybe adding
support for v2 would be easier then, too.
> u64 val, div, rate;
> unsigned long prescale = PRESCALE_MIN, pc, dc;
> unsigned int value, chan = pwm->hwpwm;
> + int ret;
>
> /*
> * Find period count, duty count and prescale to suit duty_ns and
> @@ -156,7 +190,12 @@ static int kona_pwmc_config(struct pwm_chip *chip, struct pwm_device *pwm,
> * validated immediately instead of on enable.
> */
> if (pwm_is_enabled(pwm)) {
> - kona_pwmc_prepare_for_settings(kp, chan);
> + ret = kona_pwmc_prepare_for_settings(kp, chan);
> + if (ret < 0) {
> + dev_err(chip->dev, "failed to prepare pwm settings: %d\n",
> + ret);
> + return ret;
> + }
>
> value = readl(kp->base + PRESCALE_OFFSET);
> value &= ~PRESCALE_MASK(chan);
> @@ -167,7 +206,12 @@ static int kona_pwmc_config(struct pwm_chip *chip, struct pwm_device *pwm,
>
> writel(dc, kp->base + DUTY_CYCLE_HIGH_OFFSET(chan));
>
> - kona_pwmc_apply_settings(kp, chan);
> + ret = kona_pwmc_apply_settings(kp, chan);
> + if (ret < 0) {
> + dev_err(chip->dev, "failed to apply settings: %d\n",
> + ret);
> + return ret;
> + }
> }
>
> return 0;
> @@ -187,7 +231,11 @@ static int kona_pwmc_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm,
> return ret;
> }
>
> - kona_pwmc_prepare_for_settings(kp, chan);
> + ret = kona_pwmc_prepare_for_settings(kp, chan);
> + if (ret < 0) {
> + dev_err(chip->dev, "failed to prepare pwm settings: %d\n", ret);
> + return ret;
> + }
>
> value = readl(kp->base + PWM_CONTROL_OFFSET);
>
> @@ -198,7 +246,11 @@ static int kona_pwmc_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm,
>
> writel(value, kp->base + PWM_CONTROL_OFFSET);
>
> - kona_pwmc_apply_settings(kp, chan);
> + ret = kona_pwmc_apply_settings(kp, chan);
> + if (ret < 0) {
> + dev_err(chip->dev, "failed to apply pwm settings: %d\n", ret);
> + return ret;
> + }
>
> clk_disable_unprepare(kp->clk);
>
> @@ -231,8 +283,13 @@ static void kona_pwmc_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> struct kona_pwmc *kp = to_kona_pwmc(chip);
> unsigned int chan = pwm->hwpwm;
> unsigned int value;
> + int ret;
>
> - kona_pwmc_prepare_for_settings(kp, chan);
> + ret = kona_pwmc_prepare_for_settings(kp, chan);
> + if (ret < 0) {
> + dev_err(chip->dev, "failed to prepare pwm settings: %d\n", ret);
> + return;
> + }
>
> /* Simulate a disable by configuring for zero duty */
> writel(0, kp->base + DUTY_CYCLE_HIGH_OFFSET(chan));
> @@ -243,7 +300,11 @@ static void kona_pwmc_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> value &= ~PRESCALE_MASK(chan);
> writel(value, kp->base + PRESCALE_OFFSET);
>
> - kona_pwmc_apply_settings(kp, chan);
> + ret = kona_pwmc_apply_settings(kp, chan);
> + if (ret < 0) {
> + dev_err(chip->dev, "failed to prepare pwm settings: %d\n", ret);
> + return;
> + }
>
> clk_disable_unprepare(kp->clk);
> }
> @@ -256,14 +317,26 @@ static void kona_pwmc_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> .owner = THIS_MODULE,
> };
>
> +static const struct of_device_id bcm_kona_pwmc_dt[] = {
> + { .compatible = "brcm,kona-pwm", .data = (void *)KONA_PWM_V1},
> + { .compatible = "brcm,kona-pwm-v2", .data = (void *)KONA_PWM_V2},
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, bcm_kona_pwmc_dt);
> +
> static int kona_pwmc_probe(struct platform_device *pdev)
> {
> struct kona_pwmc *kp;
> struct resource *res;
> + const struct of_device_id *of_id;
> unsigned int chan;
> unsigned int value = 0;
> int ret = 0;
>
> + of_id = of_match_node(bcm_kona_pwmc_dt, pdev->dev.of_node);
> + if (!of_id)
> + return -ENODEV;
> +
> kp = devm_kzalloc(&pdev->dev, sizeof(*kp), GFP_KERNEL);
> if (kp == NULL)
> return -ENOMEM;
> @@ -276,6 +349,7 @@ static int kona_pwmc_probe(struct platform_device *pdev)
> kp->chip.npwm = 6;
> kp->chip.of_xlate = of_pwm_xlate_with_flags;
> kp->chip.of_pwm_n_cells = 3;
> + kp->version = (enum kona_pwmc_ver)of_id->data;
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> kp->base = devm_ioremap_resource(&pdev->dev, res);
> @@ -322,12 +396,6 @@ static int kona_pwmc_remove(struct platform_device *pdev)
> return pwmchip_remove(&kp->chip);
> }
>
> -static const struct of_device_id bcm_kona_pwmc_dt[] = {
> - { .compatible = "brcm,kona-pwm" },
> - { },
> -};
> -MODULE_DEVICE_TABLE(of, bcm_kona_pwmc_dt);
> -
> static struct platform_driver kona_pwmc_driver = {
> .driver = {
> .name = "bcm-kona-pwm",
> --
> 1.9.1
>
>
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Powered by blists - more mailing lists