Because that way adding that theoretical 2nd fan in the future won't
cause too much trouble in the dt-binding.
> I don't care either way, it just seems odd. Either case,
>
> Acked-by: Guenter Roeck <linux@xxxxxxxxxxxx>
>
> > + if (!fwnode)
> > + return 0;
> > +
> > + /* if we found the fan-node, we're keeping it until device-unbind */
> > + hwm->fan_node = fwnode;
> > + ret = devm_add_action_or_reset(dev, devm_fan_node_release, hwm);
> > + if (ret)
> > + return ret;
> > +
> > + if (!fwnode_property_present(fwnode, "cooling-levels"))
> > + return 0;
> > +
>
> Side note: One could argue that a sub-node with no content does not really
> make sense and should be invalid.
I remember thinking about that, but didn't come to a real decision on my
own, hence left it as it was. So will follow your suggestion :-)
> > + ret = fwnode_property_count_u32(fwnode, "cooling-levels");
> > + if (ret <= 0) {
> > + dev_err(dev, "Failed to count elements in 'cooling-levels'\n");
> > + return ret ? : -EINVAL;
> > + }
> > +
> > + num = ret;
>
> Another side note: Using ret here isn't necessary. I'd just have used
> 'num' directly.
will do
>
> > + hwm->fan_cooling_levels = devm_kcalloc(dev, num, sizeof(u32),
> > + GFP_KERNEL);
> > + if (!hwm->fan_cooling_levels)
> > + return -ENOMEM;
> > +
> > + ret = fwnode_property_read_u32_array(fwnode, "cooling-levels",
> > + hwm->fan_cooling_levels, num);
> > + if (ret) {
> > + dev_err(dev, "Failed to read 'cooling-levels'\n");
> > + return ret;
> > + }
> > +
> > + for (i = 0; i < num; i++) {
> > + if (hwm->fan_cooling_levels[i] > hwm->pwm_max) {
> > + dev_err(dev, "fan state[%d]:%d > %d\n", i,
> > + hwm->fan_cooling_levels[i], hwm->pwm_max);
> > + return -EINVAL;
>
> In case you send another version, you might want to consider using dev_err_probe().
ok will do.
I was probably way overthinking if I should not use dev_err_probe in a
function that is not a probe function (though of course part of the probe
process).
Thanks a lot for looking over the code once again
Heiko
