On Wed, Jul 26, 2017 at 8:54 PM, Mykola Kostenok <c_mykolak@xxxxxxxxxxxx> wrote: > Add support in aspeed-pwm-tacho driver for cooling device creation. > This cooling device could be bound to a thermal zone > for the thermal control. Device will appear in /sys/class/thermal > folder as cooling_deviceX. Then it could be bound to particular > thermal zones. Allow specification of the cooling levels > vector - PWM duty cycle values in a range from 0 to 255 > which correspond to thermal cooling states. > > Signed-off-by: Mykola Kostenok <c_mykolak@xxxxxxxxxxxx> > > v1 -> v2: > - Remove device tree binding file from the patch. > - Move of_node_put out of cycle because of_get_next_child > already do of_put_node on previous child. > > v2 -> v3: > Pointed out by Rob Herring: > - Put cooling-levels under fan subnodes. > > v3 -> v4: > Pointed out by Joel Stanley: > - Move patch history after Signoff. > - Remove unnecessary type cast. > - Use array instead of pointers for colling_levels. > - Rename num_level to num_levels. > - Use local variable to make function easier to read. > - Drop unnesesary sizeof(u8). > - Use IS_ERR instead of PTR_ERR_OR_ZERO. Changes look good! > +static int aspeed_create_pwm_cooling(struct device *dev, > + struct device_node *child, > + struct aspeed_pwm_tacho_data *priv, > + u32 pwm_port, u8 num_levels) > +{ > + int ret; > + struct aspeed_cooling_device *cdev; > + > + cdev = devm_kzalloc(dev, sizeof(*cdev), GFP_KERNEL); > + > + if (!cdev) > + return -ENOMEM; > + > + cdev->cooling_levels = devm_kzalloc(dev, num_levels, GFP_KERNEL); > + if (!cdev->cooling_levels) > + return -ENOMEM; > + > + cdev->max_state = num_levels - 1; > + ret = of_property_read_u8_array(child, "cooling-levels", > + cdev->cooling_levels, > + num_levels); > + if (ret) { > + dev_err(dev, "Property 'cooling-levels' cannot be read.\n"); > + return ret; > + } > + sprintf(cdev->name, "%s%d", child->name, pwm_port); Oops, I missed this one. It looks like if child->name is too long then this will overflow name[16]. You could dynamically allocate cdev->name[16], or use snprintf. Once you've fixed that I will add my reviewed-by. Cheers, Joel -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html