Re: [RFC PATCH 05/11] TWL: regulator: Make twl-regulator driver extract data from DT

Grant Likely
2011-09-15T22:18:16Z

On Thu, Sep 15, 2011 at 04:52:01PM +0530, Rajendra Nayak wrote:
> Modify the twl regulator driver to extract the regulator_init_data from
> device tree when passed, instead of getting it through platform_data
> structures (on non-DT builds)
>
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> ---
> drivers/regulator/twl-regulator.c | 28 +++++++++++++++++++++++++---
> 1 files changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/regulator/twl-regulator.c b/drivers/regulator/twl-regulator.c
> index ee8747f..df1b95a 100644
> --- a/drivers/regulator/twl-regulator.c
> +++ b/drivers/regulator/twl-regulator.c
> @@ -17,6 +17,8 @@
> #include <linux/regulator/driver.h>
> #include <linux/regulator/machine.h>
> #include <linux/i2c/twl.h>
> +#include <linux/of.h>
> +#include <linux/of_regulator.h>
>
>
> /*
> @@ -1011,6 +1013,9 @@ static int __devinit twlreg_probe(struct platform_device *pdev)
> struct regulation_constraints *c;
> struct regulator_dev *rdev;
>
> + if (pdev->dev.of_node)
> + of_property_read_u32(pdev->dev.of_node, "ti,reg-id", &pdev->id);
> +

Don't do this. As much as possible, don't reply on plaform_device->id
when using DT. Plus it is illegal to modify pdev->id after the device
is registered.

> for (i = 0, info = NULL; i < ARRAY_SIZE(twl_regs); i++) {
> if (twl_regs[i].desc.id != pdev->id)
> continue;
> @@ -1020,7 +1025,11 @@ static int __devinit twlreg_probe(struct platform_device *pdev)
> if (!info)
> return -ENODEV;
>
> - initdata = pdev->dev.platform_data;
> + if (pdev->dev.of_node)
> + initdata = of_get_regulator_init_data(pdev->dev.of_node);
> + else
> + initdata = pdev->dev.platform_data;
> +
> if (!initdata)
> return -EINVAL;
>
> @@ -1101,14 +1110,27 @@ static int __devexit twlreg_remove(struct platform_device *pdev)
>
> MODULE_ALIAS("platform:twl_reg");
>
> +#if defined(CONFIG_OF)
> +static const struct of_device_id twl_of_match[] __devinitconst = {
> + { .compatible = "ti,twl-reg", },

This looks rather generic. Is this a specific chip? It should be.

g.

> + {},
> +};
> +MODULE_DEVICE_TABLE(of, twl_of_match);
> +#else
> +#define twl_of_match NULL
> +#endif
> +
> static struct platform_driver twlreg_driver = {
> .probe = twlreg_probe,
> .remove = __devexit_p(twlreg_remove),
> /* NOTE: short name, to work around driver model truncation of
> * "twl_regulator.12" (and friends) to "twl_regulator.1".
> */
> - .driver.name = "twl_reg",
> - .driver.owner = THIS_MODULE,
> + .driver = {
> + .name = "twl_reg",
> + .owner = THIS_MODULE,
> + .of_match_table = twl_of_match,
> + },
> };
>
> static int __init twlreg_init(void)
> --
> 1.7.1
>