Re: [PATCH 1/2] PWM: add pwm framework support

[Kurt Van Dijck <kurt.van.dijck@eia.be>]

On Tue, Jun 28, 2011 at 12:02:47PM +0200, Sascha Hauer wrote:
> This patch adds framework support for PWM (pulse width modulation) devices.
> 
> The is a barebone PWM API already in the kernel under include/linux/pwm.h,
The ?? is ...
> +
[...]
> +Locking
> +-------
> +
> +The PWM core list manipulations are protected by a mutex, so pwm_request()
> +and pwm_free() may not be called from an atomic context. Currently the
> +PWM core does not enforce any locking to pwm_enable(), pwm_disable() and
> +pwm_config(), so the calling context is currently driver specific. This
> +is an issue derived from the former barebone API and should be fixed soon.
[...]
> +
> +struct pwm_device {
> +	struct			pwm_chip *chip;
> +	const char		*label;
> +	int			enabled;
.enabled is never used, in favor of FLAG_ENABLED
> +	unsigned long		flags;
> +#define FLAG_REQUESTED	0
> +#define FLAG_ENABLED	1
> +	struct list_head	node;
> +};
> +
[...]
> +
> +static struct pwm_device *find_pwm(int pwm_id)
s/find_pwm/_find_pwm/ to indicate it needs pwm_lock?

> +/**
> + * pwmchip_add() - register a new pwm
> + * @chip: the pwm
> + *
> + * register a new pwm. pwm->pwm_id must be initialized. if pwm_id < 0 then
> + * a dynamically assigned id will be used, otherwise the id specified,
> + */
> +int pwmchip_add(struct pwm_chip *chip)
> +{
> +	struct pwm_device *pwm;
> +	int ret = 0;
> +
> +	pwm = kzalloc(sizeof(*pwm), GFP_KERNEL);
> +	if (!pwm)
> +		return -ENOMEM;
> +
> +	pwm->chip = chip;
> +
> +	mutex_lock(&pwm_lock);
> +
> +	if (chip->pwm_id >= 0 && find_pwm(chip->pwm_id)) {
> +		ret = -EBUSY;
> +		goto out;
> +	}
> +
> +	if (chip->pwm_id < 0)
> +		chip->pwm_id = next_pwm_id++;
> +
> +	list_add_tail(&pwm->node, &pwm_list);
> +out:
> +	mutex_unlock(&pwm_lock);
> +
+	if (ret) ??
> +	kfree(pwm);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(pwmchip_add);
> +

Kind regards,
Kurt