Re: [PATCH 2/4] MFD: TPS65218: Add driver for the TPS65218 PMIC

Lee Jones
2013-12-03T11:21:18Z

> >>+ ret = of_platform_populate(client->dev.of_node, NULL, NULL,
> >>+ &client->dev);
> >What are you trying to do here?
> >
> >Register each regulator as a platform device?
>
> Yeah. The probe will be called for every regulator separately.

So there are two schools of thought on this. One is that if you want
to do it that way you should use the MFD framework to do this for you.
There is no need to recreate functionality that already exists. The
other is that you shouldn't be doing this at all with regulators. Mark
likes the idea of having a single regulator controller node which
contains all of these individual regulator sub-nodes and you initiate
a single call to for_each_child_of_node() within the driver in order
to register them all.

<snip>

> >This has a different structure to of_tps65218_match_table, please
> >ensure they're the same. I prefer spaces after '{' and before '}'.
>
> Ok. I will add a space after '{' and before '}'. Sorry I did not
> follow tps65218_id_table being different than of_tps65218_match_table.
>
> One is of the type of_device_id and the other i2c_device_id.

Shouldn't matter, it's just a formatting thing.

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog