> >> 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? --- ��.n��������+%������w��{.n����z�{��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f