[RFC PATCH 03/11] DT: regulator: Helper routine to extract regulator_init_data
Rajendra Nayak
rnayak at ti.com
Fri Sep 16 03:24:18 EDT 2011
On Friday 16 September 2011 03:42 AM, Grant Likely wrote:
> On Thu, Sep 15, 2011 at 04:51:59PM +0530, Rajendra Nayak wrote:
>> The helper routine is meant to be used by regulator drivers
>> to extract the regulator_init_data structure passed from device tree.
>> 'consumer_supplies' which is part of regulator_init_data is not extracted
>> as the regulator consumer mappings are passed through DT differently,
>> implemented in subsequent patches.
>>
>> Also add documentation for regulator bindings to be used to pass
>> regulator_init_data struct information from device tree.
>>
>> Signed-off-by: Rajendra Nayak<rnayak at ti.com>
>> ---
>> .../devicetree/bindings/regulator/regulator.txt | 37 +++++++++
>> drivers/of/Kconfig | 6 ++
>> drivers/of/Makefile | 1 +
>> drivers/of/of_regulator.c | 85 ++++++++++++++++++++
>
> I originally put the DT stuff under drivers/of for i2c and spi because
> there was resistance from driver subsystem maintainers to having code
> for something that was powerpc only. Now that it is no longer the
> case, I recommend putting this code under drivers/regulator.
sure, will move these in drivers/regulator.
>
>> include/linux/of_regulator.h | 23 +++++
>> 5 files changed, 152 insertions(+), 0 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/regulator/regulator.txt
>> create mode 100644 drivers/of/of_regulator.c
>> create mode 100644 include/linux/of_regulator.h
>>
>> diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
>> new file mode 100644
>> index 0000000..001e5ce
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/regulator/regulator.txt
>> @@ -0,0 +1,37 @@
>> +Voltage/Current Regulators
>> +
>> +Required properties:
>> +- compatible: Must be "regulator";
>
> A "regulator" compatible doesn't actually help much. Compatible is
> for specifying the specific device, not for trying to describe what
> kind of device it is. Instead, specific regulator bindings can adopt
> & extend this binding.
yeah, will have a compatible for each specific device.
>
>> +
>> +Optional properties:
>> +- supply-regulator: Name of the parent regulator
>
> If this is a reference to another regulator node then don't use names.
> Use phandles instead. In which case it should probably be named
> something like "regulator-parent" (similar to interrupt parent).
>
> However, I can imagine it possible for a regulator to have multiple
> parents. It may just be better to go with something like the gpio
> scheme of<phandle gpio-specifier> list of entries. Or maybe just a
> list of phandles would be sufficient.
Ok, I can use phandles instead of the name, and have a list to specify
multiple parents.
The linux regulator framework though limits to just one parent I guess.
>
>> +- min-uV: smallest voltage consumers may set
>> +- max-uV: largest voltage consumers may set
>> +- uV-offset: Offset applied to voltages from consumer to compensate for voltage drops
>> +- min-uA: smallest current consumers may set
>> +- max-uA: largest current consumers may set
>> +- mode-fast: boolean, Can handle fast changes in its load
>> +- mode-normal: boolean, Normal regulator power supply mode
>> +- mode-idle: boolean, Can be more efficient during light loads
>> +- mode-standby: boolean, Can be most efficient during light loads
>> +- change-voltage: boolean, Output voltage can be changed by software
>> +- change-current: boolean, Output current can be chnaged by software
>> +- change-mode: boolean, Operating mode can be changed by software
>> +- change-status: boolean, Enable/Disable control for regulator exists
>> +- change-drms: boolean, Dynamic regulator mode switching is enabled
>> +- input-uV: Input voltage for regulator when supplied by another regulator
>> +- initial-mode: Mode to set at startup
>> +- always-on: boolean, regulator should never be disabled
>> +- boot-on: bootloader/firmware enabled regulator
>> +- apply-uV: apply uV constraint if min == max
>
> To avoid collisions, I'd prefix all of these with a common prefix.
> Something like regulator-*.
Ok.
>
> These map 1:1 to how Linux currently implements regulators; which
> isn't exactly bad, but it means that even if Linux changes, we're
> still have to support this binding. Does this represent the best
> layout for high level description of regulators?
I guess, except for some like apply-uV, which like Mark pointed
out are very linux/runtime policy specific.
Mark, what do you think?
>
>> +
>> +Example:
>> +
>> + xyz-regulator: regulator at 0 {
>> + compatible = "regulator";
>> + min-uV =<1000000>;
>> + max-uV =<2500000>;
>> + mode-fast;
>> + change-voltage;
>> + always-on;
>> + };
>> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
>> index b3bfea5..296b96d 100644
>> --- a/drivers/of/Kconfig
>> +++ b/drivers/of/Kconfig
>> @@ -87,4 +87,10 @@ config OF_PCI_IRQ
>> help
>> OpenFirmware PCI IRQ routing helpers
>>
>> +config OF_REGULATOR
>> + def_bool y
>> + depends on REGULATOR
>> + help
>> + OpenFirmware regulator framework helpers
>> +
>> endmenu # OF
>> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
>> index 74b959c..bea2852 100644
>> --- a/drivers/of/Makefile
>> +++ b/drivers/of/Makefile
>> @@ -12,3 +12,4 @@ obj-$(CONFIG_OF_SPI) += of_spi.o
>> obj-$(CONFIG_OF_MDIO) += of_mdio.o
>> obj-$(CONFIG_OF_PCI) += of_pci.o
>> obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o
>> +obj-$(CONFIG_OF_REGULATOR) += of_regulator.o
>> diff --git a/drivers/of/of_regulator.c b/drivers/of/of_regulator.c
>> new file mode 100644
>> index 0000000..f01d275
>> --- /dev/null
>> +++ b/drivers/of/of_regulator.c
>> @@ -0,0 +1,85 @@
>> +/*
>> + * OF helpers for regulator framework
>> + *
>> + * Copyright (C) 2011 Texas Instruments, Inc.
>> + * Rajendra Nayak<rnayak at ti.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + */
>> +
>> +#include<linux/slab.h>
>> +#include<linux/of.h>
>> +#include<linux/regulator/machine.h>
>> +
>> +static void of_get_regulation_constraints(struct device_node *np,
>> + struct regulator_init_data **init_data)
>> +{
>> + unsigned int valid_modes_mask = 0, valid_ops_mask = 0;
>> +
>> + of_property_read_u32(np, "min-uV",&(*init_data)->constraints.min_uV);
>> + of_property_read_u32(np, "max-uV",&(*init_data)->constraints.max_uV);
>> + of_property_read_u32(np, "uV-offset",&(*init_data)->constraints.uV_offset);
>> + of_property_read_u32(np, "min-uA",&(*init_data)->constraints.min_uA);
>> + of_property_read_u32(np, "max-uA",&(*init_data)->constraints.max_uA);
>
> Are all these structure members are int and unsigned int. They need
> to be u32 to be used with of_property_read_u32() (which also tells
> me that the of_property_read_*() api still needs work).
right, will fix that.
>
>> +
>> + /* valid modes mask */
>> + if (of_find_property(np, "mode-fast", NULL))
>> + valid_modes_mask |= REGULATOR_MODE_FAST;
>> + if (of_find_property(np, "mode-normal", NULL))
>> + valid_modes_mask |= REGULATOR_MODE_NORMAL;
>> + if (of_find_property(np, "mode-idle", NULL))
>> + valid_modes_mask |= REGULATOR_MODE_IDLE;
>> + if (of_find_property(np, "mode-standby", NULL))
>> + valid_modes_mask |= REGULATOR_MODE_STANDBY;
>> +
>> + /* valid ops mask */
>> + if (of_find_property(np, "change-voltage", NULL))
>> + valid_ops_mask |= REGULATOR_CHANGE_VOLTAGE;
>> + if (of_find_property(np, "change-current", NULL))
>> + valid_ops_mask |= REGULATOR_CHANGE_CURRENT;
>> + if (of_find_property(np, "change-mode", NULL))
>> + valid_ops_mask |= REGULATOR_CHANGE_MODE;
>> + if (of_find_property(np, "change-status", NULL))
>> + valid_ops_mask |= REGULATOR_CHANGE_STATUS;
>> +
>> + (*init_data)->constraints.valid_modes_mask = valid_modes_mask;
>> + (*init_data)->constraints.valid_ops_mask = valid_ops_mask;
>> +
>> + of_property_read_u32(np, "input-uV",
>> + &(*init_data)->constraints.input_uV);
>> + of_property_read_u32(np, "initial-mode",
>> + &(*init_data)->constraints.initial_mode);
>> +
>> + if (of_find_property(np, "always-on", NULL))
>> + (*init_data)->constraints.always_on = true;
>> + if (of_find_property(np, "boot-on", NULL))
>> + (*init_data)->constraints.boot_on = true;
>> + if (of_find_property(np, "apply-uV", NULL))
>> + (*init_data)->constraints.apply_uV = true;
>> +}
>> +
>> +/**
>> + * of_get_regulator_init_data - extarct regulator_init_data structure info
>> + * @np: Pointer to regulator device tree node
>> + *
>> + * Populates regulator_init_data structure by extracting data from device
>> + * tree node, returns a pointer to the populated struture or NULL if memory
>> + * alloc fails.
>> + */
>> +struct regulator_init_data *of_get_regulator_init_data(struct device_node *np)
>> +{
>> + struct regulator_init_data *init_data;
>> +
>> + init_data = kzalloc(sizeof(struct regulator_init_data), GFP_KERNEL);
>> + if (!init_data)
>> + return NULL; /* Out of memory? */
>> +
>> + init_data->supply_regulator = (char *)of_get_property(np,
>> + "supply-regulator", NULL);
>> + of_get_regulation_constraints(np,&init_data);
>> + return init_data;
>> +}
>> +EXPORT_SYMBOL(of_get_regulator_init_data);
>
> Who will call this? If it is done by proper device drivers, it would
> be better have it do the alloc so that it can do devm_kzalloc(). Or
> maybe have a devm_kzalloc variant.
Yes, its expected to be called always from proper device drivers. So I
could use devm_kzalloc() instead.
>
>> diff --git a/include/linux/of_regulator.h b/include/linux/of_regulator.h
>> new file mode 100644
>> index 0000000..5eb048c
>> --- /dev/null
>> +++ b/include/linux/of_regulator.h
>> @@ -0,0 +1,23 @@
>> +/*
>> + * OpenFirmware regulator support routines
>> + *
>> + */
>> +
>> +#ifndef __LINUX_OF_REG_H
>> +#define __LINUX_OF_REG_H
>> +
>> +#include<linux/regulator/machine.h>
>> +
>> +#if defined(CONFIG_OF_REGULATOR)
>> +extern struct regulator_init_data
>> + *of_get_regulator_init_data(struct device_node *np);
>> +#else
>> +static inline struct regulator_init_data
>> + *of_get_regulator_init_data(struct device_node *np)
>> +{
>> + return NULL;
>> +}
>> +#endif /* CONFIG_OF_REGULATOR */
>> +
>> +#endif /* __LINUX_OF_REG_H */
>> +
>> --
>> 1.7.1
>>
More information about the linux-arm-kernel
mailing list