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 > + 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. > + case PWM_CONFIG_START: > + if (!hrtimer_active(&gp->t)) { > + gpio_pwm_start(p); > + } dont really need those braces > + struct gpio_pwm *gp = container_of(p->pwm, struct gpio_pwm, pwm); helper macro for this ? > +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 > +static int __init > +gpio_pwm_probe(struct platform_device *pdev) __devinit > +static struct platform_driver gpio_pwm_driver = { > + .driver = { > + .name = "gpio_pwm", > + .owner = THIS_MODULE, > + }, dont need the explicit .owner ? > +static void gpio_pwm_exit(void) __exit > +MODULE_DESCRIPTION("PWM output using GPIO and an itimer"); itimer -> hrtimer ? -mike -- 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