Mike Frysinger wrote:
On Mon, Oct 19, 2009 at 16:32, Bill Gatliff wrote:
--- /dev/null
+++ b/drivers/pwm/gpio.c
@@ -0,0 +1,318 @@
+#define DEBUG 99
whoops
Indeed!
+ pr_debug("%s:%d start, %lu ticks\n",
+ dev_name(p->pwm->dev), p->chan, p->duty_ticks);
you already have a struct device, so this is just odd. use dev_dbg()
instead and let it worry about dev_name() stuff. plus you're already
using dev_dbg() in the rest of the code, so i guess you just forgot
about this.
Yea, I think so.
+ case PWM_CONFIG_START:
+ if (!hrtimer_active(&gp->t)) {
+ gpio_pwm_start(p);
+ }
dont really need those braces
Agreed.
+ struct gpio_pwm *gp = container_of(p->pwm, struct gpio_pwm, pwm);
helper macro for this ?
Ditto.
+static int
+gpio_pwm_config(struct pwm_channel *p,
+ struct pwm_channel_config *c)
+{
+ struct gpio_pwm *gp = container_of(p->pwm, struct gpio_pwm, pwm);
+ int was_on = 0;
+
+ if (p->pwm->config_nosleep) {
+ if (!p->pwm->config_nosleep(p, c))
+ return 0;
+ }
+
+ might_sleep();
shouldnt this be above the config_n
Actually, not. In this example I'm hoping that the requested action can
be handled by the config_nosleep code, and if it can then I just
return. Otherwise, I go through the might_sleep and then into the code
that might actually sleep.
But, I think the upper-level code might already do this same thing, so
the above might be entirely redundant. I'll have to check on that.
+static int __init
+gpio_pwm_probe(struct platform_device *pdev)
__devinit
Yep.
+static struct platform_driver gpio_pwm_driver = {
+ .driver = {
+ .name = "gpio_pwm",
+ .owner = THIS_MODULE,
+ },
dont need the explicit .owner ?
Again, not sure.
+static void gpio_pwm_exit(void)
__exit
I think so, yes.
+MODULE_DESCRIPTION("PWM output using GPIO and an itimer");
itimer -> hrtimer ?
Yep.
b.g.
--
Bill Gatliff
bgat@xxxxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
