Hi Jamie

> +static int gpio_fan_get_of_pdata(struct device *dev,
> +                         struct gpio_fan_platform_data *pdata)
> +{
> +     struct device_node *node;
> +     struct gpio_fan_speed  *speed;
> +     unsigned *ctrl;
> +     unsigned i;
> +     u32 u;
> +     struct property *prop;
> +     const __be32 *p;
> +
> +     node = dev->of_node;
> +
> +     /* Fill GPIO pin array */
> +     pdata->num_ctrl = of_gpio_count(node);
> +     ctrl = devm_kzalloc(dev, pdata->num_ctrl * sizeof(unsigned),
> +                             GFP_KERNEL);
> +     if (!ctrl)
> +             return -ENOMEM;
> +     for (i = 0; i < of_gpio_count(node); i++) {


It seems unlikely the number of gpio pins is going to change while
inside this loop. So just use pdata->num_ctrl instead of counting them
every iteration.

> +             int val;
> +
> +             val = of_get_gpio(node, i);
> +             if (val >= 0)
> +                     ctrl[i] = val;
> +             else
> +                     return -EINVAL;
> +     }
> +     pdata->ctrl = ctrl;
> +
> +     /* Get speed map array size */
> +     i = 0;
> +     of_property_for_each_u32(node, "gpio-fan,speed-map", prop, p, u)
> +             i++;
> +     if (i & 1) {
> +             dev_err(dev, "gpio-fan,speed-map contains odd number of 
> entries");
> +             return -ENODEV;
> +     }

This is not so clear on the first reading. i is the number of numbers
in the speed-map, and each entry needs two numbers, hence the (i & 1).
Maybe a comment to explain this? Or see if there is an of_ function
which returns records instead of individual properties?

> +static const struct of_device_id of_gpio_fan_match[] = {
> +     { .compatible = "gpio-fan", },
> +     {},
> +};

This should probably have an __devinitdata attribute.

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

Reply via email to