| 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
| ||
|
Date: Mon, 21 Jan 2019 19:53:53 +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>,
Mark Rutland <mark.rutland@....com>,
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 V2 3/4] drivers: pwm: pwm-bcm-kona: Add cygnus-pwm support
Hello,
On Thu, Jan 17, 2019 at 01:45:15AM +0530, Sheetal Tigadoli wrote:
> From: Praveen Kumar B <praveen.b@...adcom.com>
>
> Add support for new version of pwm-cygnus.
> 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 | 95 ++++++++++++++++++++++++++++++++++++----------
> 1 file changed, 76 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/pwm/pwm-bcm-kona.c b/drivers/pwm/pwm-bcm-kona.c
> index fe63289..143843f 100644
> --- a/drivers/pwm/pwm-bcm-kona.c
> +++ b/drivers/pwm/pwm-bcm-kona.c
> @@ -15,6 +15,7 @@
> #include <linux/delay.h>
> #include <linux/err.h>
> #include <linux/io.h>
> +#include <linux/iopoll.h>
> #include <linux/ioport.h>
> #include <linux/math64.h>
> #include <linux/module.h>
> @@ -65,10 +66,19 @@
> #define DUTY_CYCLE_HIGH_MIN (0x00000000)
> #define DUTY_CYCLE_HIGH_MAX (0x00ffffff)
>
> +#define PWM_MONITOR_OFFSET 0xb0
> +#define PWM_MONITOR_TIMEOUT_US 5
> +
> +enum kona_pwmc_ver {
> + KONA_PWM = 1,
> + CYGNUS_PWM
> +};
> +
> 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,10 +86,36 @@ 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;
> +
> + switch (kona_ver) {
> + case KONA_PWM:
> + /*
> + * There must be a min 400ns delay between clearing trigger and
> + * settingit. Failing to do this may result in no PWM signal.
> + */
> + ndelay(400);
> + return 0;
> +
> + case CYGNUS_PWM:
> + return readl_poll_timeout(kp->base + PWM_MONITOR_OFFSET, value,
> + !(value & (BIT(chan))), 0,
> + PWM_MONITOR_TIMEOUT_US);
> +
> + default:
> + return -ENODEV;
> +
> + }
> +}
This function is the only difference between these two otherwise similar
implementations. If you do the following instead:
static int kona_pwmc_wait_stable(struct pwm_chip *chip, unsigned int chan)
{
ndelay(400);
return 0;
}
static int cygnus_pwmc_wait_stable(struct pwm_chip *chip, unsigned int chan)
{
return readl_poll_timeout(...);
}
and then maintain a
int (*wait_stable)(struct pwm_chip *, unsigned int);
in struct kona_pwmc that is initialized in .probe you save quite some
checks for the version.
> +
> /*
> * Clear trigger bit but set smooth bit to maintain old output.
> */
> -static void kona_pwmc_prepare_for_settings(struct kona_pwmc *kp,
> +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 +124,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 +136,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 void kona_pwmc_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> @@ -113,8 +144,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));
> @@ -125,7 +161,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 apply pwm settings: %d\n", ret);
> + return;
I think it would make sense to change kona_pwmc_disable to return an
error code and then use that in the callers.
> + }
>
> clk_disable_unprepare(kp->clk);
> }
> @@ -188,7 +228,12 @@ static int kona_pwmc_apply(struct pwm_chip *chip, struct pwm_device *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);
> @@ -208,7 +253,12 @@ static int kona_pwmc_apply(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 prepare pwm settings: %d\n", ret);
> + return ret;
> + }
> } else if (cstate.enabled) {
> kona_pwmc_disable(chip, pwm);
> }
> @@ -221,14 +271,26 @@ static int kona_pwmc_apply(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},
> + { .compatible = "brcm,cygnus-pwm", .data = (void *)CYGNUS_PWM},
> + { },
> +};
> +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;
> @@ -241,6 +303,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);
> @@ -287,12 +350,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",
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Powered by blists - more mailing lists