On Sat, Feb 19, 2011 at 23:10, Bill Gatliff wrote:
> +void pwm_release(struct pwm_device *p)
> +{
> + mutex_lock(&device_list_mutex);
> +
> + if (!test_and_clear_bit(FLAG_REQUESTED, &p->flags)) {
> + BUG();
> + goto done;
> + }
shouldnt that BUG be a WARN ?
> +int pwm_set(struct pwm_device *p, unsigned long period_ns,
> + unsigned long duty_ns, int active_high)
> +{
> + struct pwm_config c = {
> + .config_mask = (BIT(PWM_CONFIG_PERIOD_TICKS)
> + | BIT(PWM_CONFIG_DUTY_TICKS)
> + | BIT(PWM_CONFIG_POLARITY)),
> + .period_ticks = pwm_ns_to_ticks(p, period_ns),
> + .duty_ticks = pwm_ns_to_ticks(p, duty_ns),
> + .polarity = active_high};
i think that brace needs to be uncuddled
> +static const struct attribute *pwm_attrs[] =
> +{
> + &dev_attr_tick_hz.attr,
cuddle up that brace
> +static const struct attribute_group pwm_device_attr_group = {
> + .attrs = (struct attribute **) pwm_attrs,
> +};
should attribute_group have its attrs member constified ?
> +static void __exit pwm_exit(void)
> +{
> + destroy_workqueue(pwm_handler_workqueue);
> + class_unregister(&pwm_class);
> +}
> +
> +#ifdef MODULE
> +module_init(pwm_init);
> +module_exit(pwm_exit);
> +MODULE_LICENSE("GPL");
> +#else
> +postcore_initcall(pwm_init);
> +#endif
i dont think you need this MODULE trickery. common code already takes
care of this for you, and it'd let you avoid a warning about pwm_exit
being unused.
postcore_initcall(pwm_init);
module_exit(pwm_exit);
MODULE_LICENSE("GPL");
also, no MODULE_{INFO,AUTHOR} ?
> +struct pwm_device {
> + struct pwm_device_ops *ops;
const ?
> +void pwm_callback(struct pwm_device *pwm);
> +int gpio_pwm_destroy(struct pwm_device *p);
seems a bit inconsistent ... sometimes you use "p", sometimes you use "pwm" ...
-mike
--
To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
