[PATCH 1/2] hwmon: Add devicetree bindings to gpio-fan
Andrew Lunn
andrew at lunn.ch
Sat Sep 8 17:32:23 EST 2012
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
More information about the devicetree-discuss
mailing list