> >> On 12/21/2013 05:32 AM, Alexander Shiyan wrote:
> >>> This patch adds devicetree support for the MC13XXX LED driver.
...
> >>> + ret = of_property_read_u32_array(parent, "led-control", ctrls,
> >>> + leds->devtype->num_regs);
> >>> + if (ret)
> >>> + goto out_node_put;
> >>> +
> >>> + for (i = 0; i < leds->devtype->num_regs; i++) {
> >>> + ret = mc13xxx_reg_write(mcdev, leds->devtype->ledctrl_base + i,
> >>> + ctrls[i]);
> >>
> >> Code duplicated from the regular probe.
> >
> > Yes. This was done based on the comments to the previous version:
> > http://www.spinics.net/lists/devicetree/msg14933.html
> >
>
> From what I understand, Tomasz is suggesting exactly the same as
> what I am saying:
>
> "what about making
> mc13xxx_led_setup_of() allocate a platform data structure and fill it
> in with data parsed from DT, so the driver would then proceed normally as
> if the platform data were available? IMHO this should make the code much
> simpler and more readable."
>
> This is not what you have done IMHO, you are cloning the probe
> into a separate DT-only function.
I do not fully understand what you mean. When we had an universal procedure,
I was told to divide them into DT and non-DT. I separated it - now we have
duplicate code.
Merge back?
---