On Wednesday 22 February 2012, Thierry Reding wrote:

>  #include <linux/module.h>
> +#include <linux/of_pwm.h>
>  #include <linux/pwm.h>

You should probably reorder the patches for bisectability, or move the
of_* related changes out of this patch into patch 3. At the point
where patch 2 is applied, linux/of_pwm.h does not exist yet.

>  
> +/**
> + * pwmchip_find() - iterator for locating a specific pwm_chip
> + * @data: data to pass to match function
> + * @match: callback function to check pwm_chip
> + */
> +struct pwm_chip *pwmchip_find(void *data, int (*match)(struct pwm_chip *chip,
> +                                                    void *data))
> +{
> +     struct pwm_chip *ret = NULL;
> +     struct pwm_chip *chip;
> +
> +     mutex_lock(&pwm_lock);
> +
> +     list_for_each_entry(chip, &pwm_chips, list) {
> +             if (match(chip, data)) {
> +                     ret = chip;
> +                     break;
> +             }
> +     }
> +
> +     mutex_unlock(&pwm_lock);
> +
> +     return ret;
> +}
> +EXPORT_SYMBOL_GPL(pwmchip_find);

Is this only used for the device tree functions? If so, I would recommend
making it less generic and always search for a device node.

> +static int pwm_show(struct seq_file *s, void *unused)
> +{
> +     const char *prefix = "";
> +     struct pwm_chip *chip;
> +
> +     list_for_each_entry(chip, &pwm_chips, list) {
> +             struct device *dev = chip->dev;
> +
> +             seq_printf(s, "%s%s/%s, %d PWM devices\n", prefix,
> +                        dev->bus ? dev->bus->name : "no-bus",
> +                        dev_name(dev), chip->npwm);
> +
> +             if (chip->ops->dbg_show)
> +                     chip->ops->dbg_show(chip, s);
> +             else
> +                     pwm_dbg_show(chip, s);
> +
> +             prefix = "\n";
> +     }
> +
> +     return 0;
> +}
> +
> +static int pwm_open(struct inode *inode, struct file *file)
> +{
> +     return single_open(file, pwm_show, NULL);
> +}

When you have a seq_file with a (possibly long) list of entries, better
use seq_open instead of single_open and print each item in the
->next() callback function.

        Arnd
_______________________________________________
devicetree-discuss mailing list
[email protected]
https://lists.ozlabs.org/listinfo/devicetree-discuss

Reply via email to