Messages in this thread | | | Date | Tue, 07 Oct 2014 16:14:41 +0200 | From | Marc Kleine-Budde <> | Subject | Re: [PATCH v6]pwm: add BCM2835 PWM driver |
| |
On 10/07/2014 04:04 PM, Bart Tanghe wrote: > Add pwm driver for Broadcom BCM2835 processor (Raspberry Pi)
> --- /dev/null > +++ b/drivers/pwm/pwm-bcm2835.c > @@ -0,0 +1,201 @@ > +/* > + * Copyright 2014 Bart Tanghe <bart.tanghe@thomasmore.be> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; version 2. > + */ > + > +#include <linux/clk.h> > +#include <linux/err.h> > +#include <linux/io.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/platform_device.h> > +#include <linux/pwm.h> > + > +#define DUTY 0x14 > +#define PERIOD 0x10 > +#define CHANNEL 0x10 > + > +#define PWM_ENABLE (1 << 0) > +#define PWM_POLARITY (1 << 4) > + > +#define PWM_CONTROL_MASK 0xff > +#define PWM_MODE 0x80 /* set timer in pwm mode */ > +#define DEFAULT 0xff /* set timer in default mode */ > +#define PWM_CONTROL_STRIDE 8 > +#define MIN_PERIOD 108 /* 9.2Mhz max pwm clock */ > + > +struct bcm2835_pwm { > + struct pwm_chip chip; > + struct device *dev; > + int channel; > + unsigned long scaler; > + void __iomem *base; > + struct clk *clk; > +}; > + > +static inline struct bcm2835_pwm *to_bcm2835_pwm(struct pwm_chip *chip) ^^ nitpick: on space is enough
> +{ > + return container_of(chip, struct bcm2835_pwm, chip); > +} > +
[...]
> +static int bcm2835_pwm_probe(struct platform_device *pdev) > +{ > + struct bcm2835_pwm *pwm; > + int ret; > + struct resource *r; > + struct clk *clk; > + > + pwm = devm_kzalloc(&pdev->dev, sizeof(*pwm), GFP_KERNEL); > + if (!pwm) > + return -ENOMEM; > + > + pwm->dev = &pdev->dev; > + > + clk = devm_clk_get(&pdev->dev, NULL); > + if (IS_ERR(clk)) { > + dev_err(&pdev->dev, "no clock found: %ld\n", PTR_ERR(clk)); > + return PTR_ERR(clk); > + } > + > + pwm->clk = clk; > + ret = clk_prepare_enable(pwm->clk); > + if (ret) { > + clk_disable_unprepare(pwm->clk);
If enabling of the clock fails, don't disable it.
> + return ret; > + } > + > + pwm->scaler = NSEC_PER_SEC / clk_get_rate(clk); > + > + r = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + pwm->base = devm_ioremap_resource(&pdev->dev, r); > + if (IS_ERR(pwm->base)) > + return PTR_ERR(pwm->base);
Here you should disable the clock instead.
> + > + pwm->chip.dev = &pdev->dev; > + pwm->chip.ops = &bcm2835_pwm_ops; > + pwm->chip.npwm = 2; > + > + platform_set_drvdata(pdev, pwm); > + > + ret = pwmchip_add(&pwm->chip); > + if (ret < 0) { > + dev_err(&pdev->dev, "pwmchip_add() failed: %d\n", ret); > + return ret;
and here....or better introduce a disable_clock jump-label at end of this function.
> + } > + return 0; > +}
Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[unhandled content-type:application/pgp-signature] | |