| Message ID | 1405028569-14253-2-git-send-email-ttynkkynen@nvidia.com (mailing list archive) |
|---|---|
| State | New, 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?
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.
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.
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