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

Rajendra Nayak
2011-09-16T07:25:14Z

On Friday 16 September 2011 03:48 AM, Grant Likely wrote:
> 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.

yeah, I did this hackery to just get around the drivers per-regulator
lookup table for which it uses the pdev->id as the index.
I will need to do this lookup based on compatible instead I guess.

>
>> 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.

We have multiple chips in the twl family like twl6030/twl6040/twl6025
but just one driver which handles all variants.

>
> 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
>>