| Message ID | 1405028569-14253-2-git-send-email-ttynkkynen@nvidia.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Thu, Jul 10, 2014 at 2:42 PM, Tuomas Tynkkynen <ttynkkynen@nvidia.com> wrote: > The DFLL is the main clocksource for the fast CPU cluster on Tegra124 > and also provides automatic CPU rail voltage scaling as well. The DFLL > is a separate IP block from the usual Tegra124 clock-and-reset > controller, so it gets its own node in the device tree. > diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt > +- nvidia,pmic-voltage-table: Array of 2-tuples. Each entry should have the > + form <register-value voltage-in-uV>, indicating the register value that > + needs to be programmed to the PMIC for changing the VDD_CPU voltage to > + the specified voltage. The table must be in ascending order by the voltage. Instead of listing the register values for each voltage in the DT, can't you use regulator_list_voltage() to create this map? -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/07/14 19:28, Andrew Bresticker wrote: > On Thu, Jul 10, 2014 at 2:42 PM, Tuomas Tynkkynen <ttynkkynen@nvidia.com> wrote: >> The DFLL is the main clocksource for the fast CPU cluster on Tegra124 >> and also provides automatic CPU rail voltage scaling as well. The DFLL >> is a separate IP block from the usual Tegra124 clock-and-reset >> controller, so it gets its own node in the device tree. > >> diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt > >> +- nvidia,pmic-voltage-table: Array of 2-tuples. Each entry should have the >> + form <register-value voltage-in-uV>, indicating the register value that >> + needs to be programmed to the PMIC for changing the VDD_CPU voltage to >> + the specified voltage. The table must be in ascending order by the voltage. > > Instead of listing the register values for each voltage in the DT, > can't you use regulator_list_voltage() to create this map? > I don't see a way to get the register values that way, unless we assume that the mapping is linear and doesn't have holes.
On Fri, Jul 11, 2014 at 9:48 AM, Tuomas Tynkkynen <ttynkkynen@nvidia.com> wrote: > > > On 11/07/14 19:28, Andrew Bresticker wrote: >> >> On Thu, Jul 10, 2014 at 2:42 PM, Tuomas Tynkkynen <ttynkkynen@nvidia.com> >> wrote: >>> >>> The DFLL is the main clocksource for the fast CPU cluster on Tegra124 >>> and also provides automatic CPU rail voltage scaling as well. The DFLL >>> is a separate IP block from the usual Tegra124 clock-and-reset >>> controller, so it gets its own node in the device tree. >> >> >>> diff --git >>> a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt >>> b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt >> >> >>> +- nvidia,pmic-voltage-table: Array of 2-tuples. Each entry should have >>> the >>> + form <register-value voltage-in-uV>, indicating the register value >>> that >>> + needs to be programmed to the PMIC for changing the VDD_CPU voltage to >>> + the specified voltage. The table must be in ascending order by the >>> voltage. >> >> >> Instead of listing the register values for each voltage in the DT, >> can't you use regulator_list_voltage() to create this map? >> > > I don't see a way to get the register values that way, unless we assume that > the mapping is linear and doesn't have holes. Hmm... I guess if you don't assume it's linear and continuous you'd have to iterate over all 256 selectors. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/07/14 20:08, Andrew Bresticker wrote: > On Fri, Jul 11, 2014 at 9:48 AM, Tuomas Tynkkynen <ttynkkynen@nvidia.com> wrote: >> >> >> On 11/07/14 19:28, Andrew Bresticker wrote: >>> >>> On Thu, Jul 10, 2014 at 2:42 PM, Tuomas Tynkkynen <ttynkkynen@nvidia.com> >>> wrote: >>>> >>>> The DFLL is the main clocksource for the fast CPU cluster on Tegra124 >>>> and also provides automatic CPU rail voltage scaling as well. The DFLL >>>> is a separate IP block from the usual Tegra124 clock-and-reset >>>> controller, so it gets its own node in the device tree. >>> >>> >>>> diff --git >>>> a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt >>>> b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt >>> >>> >>>> +- nvidia,pmic-voltage-table: Array of 2-tuples. Each entry should have >>>> the >>>> + form <register-value voltage-in-uV>, indicating the register value >>>> that >>>> + needs to be programmed to the PMIC for changing the VDD_CPU voltage to >>>> + the specified voltage. The table must be in ascending order by the >>>> voltage. >>> >>> >>> Instead of listing the register values for each voltage in the DT, >>> can't you use regulator_list_voltage() to create this map? >>> >> >> I don't see a way to get the register values that way, unless we assume that >> the mapping is linear and doesn't have holes. > > Hmm... I guess if you don't assume it's linear and continuous you'd > have to iterate over all 256 selectors. > I don't think we can assume that each selector maps to a concrete register value, though I'm not sure. include/linux/regulator/driver.h documents for @list_voltage "Selectors range from zero to one less regulator_desc.n_voltages." but maybe the consumer API could take different values.
On Fri, Jul 11, 2014 at 08:21:27PM +0300, Tuomas Tynkkynen wrote: > > > On 11/07/14 20:08, Andrew Bresticker wrote: > >On Fri, Jul 11, 2014 at 9:48 AM, Tuomas Tynkkynen <ttynkkynen@nvidia.com> wrote: > >> > >> > >>On 11/07/14 19:28, Andrew Bresticker wrote: > >>> > >>>On Thu, Jul 10, 2014 at 2:42 PM, Tuomas Tynkkynen <ttynkkynen@nvidia.com> > >>>wrote: > >>>> > >>>>The DFLL is the main clocksource for the fast CPU cluster on Tegra124 > >>>>and also provides automatic CPU rail voltage scaling as well. The DFLL > >>>>is a separate IP block from the usual Tegra124 clock-and-reset > >>>>controller, so it gets its own node in the device tree. > >>> > >>> > >>>>diff --git > >>>>a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt > >>>>b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt > >>> > >>> > >>>>+- nvidia,pmic-voltage-table: Array of 2-tuples. Each entry should have > >>>>the > >>>>+ form <register-value voltage-in-uV>, indicating the register value > >>>>that > >>>>+ needs to be programmed to the PMIC for changing the VDD_CPU voltage to > >>>>+ the specified voltage. The table must be in ascending order by the > >>>>voltage. > >>> > >>> > >>>Instead of listing the register values for each voltage in the DT, > >>>can't you use regulator_list_voltage() to create this map? > >>> > >> > >>I don't see a way to get the register values that way, unless we assume that > >>the mapping is linear and doesn't have holes. > > > >Hmm... I guess if you don't assume it's linear and continuous you'd > >have to iterate over all 256 selectors. > > > > I don't think we can assume that each selector maps to a concrete register > value, though I'm not sure. include/linux/regulator/driver.h documents for > @list_voltage "Selectors range from zero to one less > regulator_desc.n_voltages." but maybe the consumer API could take different > values. I don't think the regulator API makes any guarantees that the selector corresponds to a register value. Adding Mark Brown, maybe he can help figure out the best way to do this. Thierry
On Mon, Jul 14, 2014 at 10:38:56AM +0200, Thierry Reding wrote: > On Fri, Jul 11, 2014 at 08:21:27PM +0300, Tuomas Tynkkynen wrote: > > I don't think we can assume that each selector maps to a concrete register > > value, though I'm not sure. include/linux/regulator/driver.h documents for > > @list_voltage "Selectors range from zero to one less > > regulator_desc.n_voltages." but maybe the consumer API could take different > > values. > I don't think the regulator API makes any guarantees that the selector > corresponds to a register value. Adding Mark Brown, maybe he can help > figure out the best way to do this. The selector value is opaque, it's entirely up to the driver to define it. If you could tell me what "this" is I might be able to advise on how to do it.
On Mon, Jul 14, 2014 at 10:12:33AM +0100, Mark Brown wrote: > On Mon, Jul 14, 2014 at 10:38:56AM +0200, Thierry Reding wrote: > > On Fri, Jul 11, 2014 at 08:21:27PM +0300, Tuomas Tynkkynen wrote: > > > > I don't think we can assume that each selector maps to a concrete register > > > value, though I'm not sure. include/linux/regulator/driver.h documents for > > > @list_voltage "Selectors range from zero to one less > > > regulator_desc.n_voltages." but maybe the consumer API could take different > > > values. > > > I don't think the regulator API makes any guarantees that the selector > > corresponds to a register value. Adding Mark Brown, maybe he can help > > figure out the best way to do this. > > The selector value is opaque, it's entirely up to the driver to define > it. If you could tell me what "this" is I might be able to advise on > how to do it. Tegra124 (and later, also some earlier variants) have this DFLL clock that can program a PMIC automatically depending on the CPU frequency. This DT binding did propose putting this into device tree as a table of <frequency value> pairs where the frequency corresponds to the CPU frequency and the value is the register value to be programmed into the PMIC by the DFLL hardware (there are two additional properties to define the slave address and the register offset). Andrew proposed that this table could instead be built by using regulator_list_voltage() instead. However, due to the fact that the DFLL hardware needs to know the immediate value to write into a register, the requirement would be for a 1:1 mapping between selector and register value. Given that the API needs to cover the general case I don't see how it could practically ensure this. Thierry
On Mon, Jul 14, 2014 at 11:24:35AM +0200, Thierry Reding wrote: > On Mon, Jul 14, 2014 at 10:12:33AM +0100, Mark Brown wrote: > > The selector value is opaque, it's entirely up to the driver to define > > it. If you could tell me what "this" is I might be able to advise on > > how to do it. > Tegra124 (and later, also some earlier variants) have this DFLL clock > that can program a PMIC automatically depending on the CPU frequency. > This DT binding did propose putting this into device tree as a table of > <frequency value> pairs where the frequency corresponds to the CPU > frequency and the value is the register value to be programmed into the > PMIC by the DFLL hardware (there are two additional properties to define > the slave address and the register offset). > Andrew proposed that this table could instead be built by using > regulator_list_voltage() instead. However, due to the fact that the DFLL > hardware needs to know the immediate value to write into a register, the > requirement would be for a 1:1 mapping between selector and register > value. Given that the API needs to cover the general case I don't see > how it could practically ensure this. Well, if you're going to do that you've already created a private API between the regulator driver and the device since you're assuming that the device is controlled only by register writes to a single register bitfield which isn't always the case. As with all these things it would also be better to extend the regulator API so that users like this can discover the register address and so on too rather than having to replicate that information in the device tree. No sense in having to specify this information multiple times.
On Tue, Jul 15, 2014 at 11:23:39PM +0300, Tuomas Tynkkynen wrote: > That sounds indeed useful for this case. How'd the following interface > sound for the register offset / selector-to-register-value conversion? > The I2C address would be a bit trickier to get as it would touch the > regmap stuff as well, but perhaps it would be a good idea to have a > phandle to the I2C device itself, and then parse the reg field for > the address. This looks fine, can you submit properly please? For the I2C address why not just have an interface to get the regmap and then provide a way to get the underlying device back from the regmap?
On Tue, Jul 15, 2014 at 11:52:52PM +0100, Mark Brown wrote: > On Tue, Jul 15, 2014 at 11:23:39PM +0300, Tuomas Tynkkynen wrote: > > > That sounds indeed useful for this case. How'd the following interface > > sound for the register offset / selector-to-register-value conversion? > > The I2C address would be a bit trickier to get as it would touch the > > regmap stuff as well, but perhaps it would be a good idea to have a > > phandle to the I2C device itself, and then parse the reg field for > > the address. > > This looks fine, can you submit properly please? For the I2C address > why not just have an interface to get the regmap and then provide a way > to get the underlying device back from the regmap? Is it mandatory for regulators to use regmap? Also I'm not sure how this will work with MFDs, since the device may not be the actual bus device, but rather a child of the MFD (and what we want access to is the MFD). Perhaps for regmaps that would work in most cases since MFDs seem to aften share the regmap with their children. However, the code in regulator_register() at least indicates that even then it's possible to have a regmap in children that's different from the top-level MFD. Thierry
On Wed, Jul 16, 2014 at 10:01:34AM +0200, Thierry Reding wrote: > On Tue, Jul 15, 2014 at 11:52:52PM +0100, Mark Brown wrote: > > This looks fine, can you submit properly please? For the I2C address > > why not just have an interface to get the regmap and then provide a way > > to get the underlying device back from the regmap? > Is it mandatory for regulators to use regmap? Also I'm not sure how this No, but this is for a limited subset of devices that the hardware knows how to write to directly which means that they're restricted to things that can be supported with regmap (or already know how to interact with the hardware and don't need this interface at all). The proposed patch already relies on regmap - it's passing back the information the regmap helpers use. > will work with MFDs, since the device may not be the actual bus device, > but rather a child of the MFD (and what we want access to is the MFD). > Perhaps for regmaps that would work in most cases since MFDs seem to > aften share the regmap with their children. However, the code in > regulator_register() at least indicates that even then it's possible to > have a regmap in children that's different from the top-level MFD. Right, drivers can say exactly which regmap to use.
diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt new file mode 100644 index 0000000..cf89802 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt @@ -0,0 +1,86 @@ +NVIDIA Tegra124 DFLL FCPU clocksource + +This binding uses the common clock binding: +Documentation/devicetree/bindings/clock/clock-bindings.txt + +The DFLL IP block on Tegra is a root clocksource designed for clocking +the fast CPU cluster. It consists of a free-running voltage controlled +oscillator connected to the CPU voltage rail (VDD_CPU), and a closed loop +control module that will automatically adjust the VDD_CPU voltage by +communicating with an off-chip PMIC either via an I2C bus or via PWM signals. + +Required properties: +- compatible : should be "nvidia,tegra124-dfll-fcpu" +- reg : Defines the following set of registers, in the order listed: + - registers for the DFLL control logic. + - registers for the I2C output logic. + - registers for the integrated I2C master controller. + - look-up table RAM for voltage register values. +- interrupts: Should contain the DFLL block interrupt. +- clocks: Must contain an entry for each entry in clock-names. + See ../clocks/clock-bindings.txt for details. +- clock-names: Must include the following entries: + - soc: Clock source for the DFLL control logic. + - ref: The closed loop reference clock + - i2c: Clock source for the integrated I2C master. +- #clock-cells: Must be 0. +- clock-output-names: Name of the clock output. +- vdd_cpu-supply: Regulator for the CPU voltage rail that the DFLL + hardware will start controlling. + +Required properties for the control loop parameters: +- nvidia,sample-rate: Sample rate of the DFLL control loop. +- nvidia,droop-ctrl: See the register CL_DVFS_DROOP_CTRL in the TRM. +- nvidia,force-mode: See the field DFLL_PARAMS_FORCE_MODE in the TRM. +- nvidia,cf: Numeric value, see the field DFLL_PARAMS_CF_PARAM in the TRM. +- nvidia,ci: Numeric value, see the field DFLL_PARAMS_CI_PARAM in the TRM. +- nvidia,cg: Numeric value, see the field DFLL_PARAMS_CG_PARAM in the TRM. +- nvidia,cg-scale: Boolean value, see the field DFLL_PARAMS_CG_SCALE in the TRM. + +Required properties for I2C mode: +- nvidia,pmic-i2c-address: I2C address of the PMIC that controls the VDD_CPU + voltage. +- nvidia,i2c-10-bit-addresses: Boolean, whether to use 10-bit I2C addressing. +- nvidia,pmic-i2c-voltage-register: Register of the PMIC that controls the + VDD_CPU voltage. +- nvidia,i2c-fs-rate: I2C transfer rate, if using FS mode. +- nvidia,pmic-voltage-table: Array of 2-tuples. Each entry should have the + form <register-value voltage-in-uV>, indicating the register value that + needs to be programmed to the PMIC for changing the VDD_CPU voltage to + the specified voltage. The table must be in ascending order by the voltage. + +Example: + +dfll@0,70110000 { + compatible = "nvidia,tegra124-dfll"; + reg = <0 0x70110000 0 0x100>, /* DFLL control */ + <0 0x70110000 0 0x100>, /* I2C output control */ + <0 0x70110100 0 0x100>, /* Integrated I2C controller */ + <0 0x70110200 0 0x100>; /* Look-up table RAM */ + interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&tegra_car TEGRA124_CLK_DFLL_SOC>, + <&tegra_car TEGRA124_CLK_DFLL_REF>, + <&tegra_car TEGRA124_CLK_I2C5>; + clock-names = "soc", "ref", "i2c"; + #clock-cells = <0>; + clock-output-names = "dfllCPU_out"; + vdd_cpu-supply = <&vdd_cpu>; + status = "okay"; + + nvidia,sample-rate = <12500>; + nvidia,droop-ctrl = <0x00000f00>; + nvidia,force-mode = <1>; + nvidia,cf = <10>; + nvidia,ci = <0>; + nvidia,cg = <2>; + + nvidia,i2c-fs-rate = <400000>; + nvidia,pmic-i2c-address = <0x40>; + nvidia,pmic-i2c-voltage-register = <0x00>; + nvidia,pmic-voltage-table = + <0x1e 700000>, + <0x1f 710000>, + /* etc... */ + <0x63 1390000>, + <0x64 1400000>; +};
The DFLL is the main clocksource for the fast CPU cluster on Tegra124 and also provides automatic CPU rail voltage scaling as well. The DFLL is a separate IP block from the usual Tegra124 clock-and-reset controller, so it gets its own node in the device tree. Signed-off-by: Tuomas Tynkkynen <ttynkkynen@nvidia.com> --- .../bindings/clock/nvidia,tegra124-dfll.txt | 86 ++++++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt