On Thu, Sep 29, 2016 at 12:34:39PM -0400, Olimpiu Dejeu wrote: > Resubmition of arcxcnn backliught driver adding definitions of the > internal registers of the chip. s/backliught/backlight/ [...] > +#ifdef CONFIG_OF > +static int arcxcnn_parse_dt(struct arcxcnn *lp) > +{ > + struct device *dev = lp->dev; > + struct device_node *node = dev->of_node; > + u32 prog_length, prog_val, num_entry, sources[6]; > + int ret; > + > + if (!node) { > + dev_err(dev, "no platform data.\n"); > + return -EINVAL; > + } > + of_property_read_string(node, "label", &lp->pdata->name); > + of_property_read_u32(node, "default-brightness", &prog_val); > + lp->pdata->initial_brightness = prog_val; You must check the return value of the of_ accessors. If there's no default-brightness property in the DT, prog_val will be uninitialised here. > + of_property_read_u32(node, "pwm-period", &lp->pdata->period_ns); This was not in the binding document. > + /* Fill program from platform data if defined > + */ > + lp->pdata->prog_entries = 0; > + prog_length = of_get_child_count(node); > + if (prog_length > 0) { > + struct device_node *child; > + u32 i = 0; > + > + for_each_child_of_node(node, child) { > + if (i >= ARCXCNN_MAX_PROGENTRIES) > + break; > + of_property_read_u32(child, "reg", &prog_val); > + lp->pdata->prog_addr[i] = (u8)prog_val; > + of_property_read_u32(child, "val", &prog_val); > + lp->pdata->prog_data[i] = (u8)prog_val; > + i++; > + } > + lp->pdata->prog_entries = i; > + } This was also not documented. What is this? What is it used for? Why is it in the DT? > + /* playback program now that its all installed */ > + for (i = 0; i < lp->pdata->prog_entries; i++) { > + arcxcnn_write_byte(lp, lp->pdata->prog_addr[i], > + lp->pdata->prog_data[i]); > + /* > + printk(KERN_ALERT "%02X, %02X\n", > + lp->pdata->prog_addr[i], lp->pdata->prog_data[i]); > + */ > + } Why is it necessary to have an arbitrary register programming sequence? What exactly varies between parts? I don't think this is the right way to solve your problem. Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html