[RFC 2/9] opp-modifier: Add opp-modifier-reg driver
Driver to read from a register and depending on either set bits or a specific known selectively enable or disable OPPs based on DT node. Can support opp-modifier-reg-bit where single bits within the register determine the availability of an OPP or opp-modifier-reg-val where a certain value inside the register or a portion of it determine what the maximum allowed OPP is. The driver expects a device that has already has its OPPs loaded and then will disable the OPPs not matching the criteria specified in the opp-modifier table. Signed-off-by: Dave Gerlach --- .../devicetree/bindings/power/opp-modifier.txt | 111 + drivers/power/opp/Makefile | 1 + drivers/power/opp/opp-modifier-reg.c | 259 + 3 files changed, 371 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/opp-modifier.txt create mode 100644 drivers/power/opp/opp-modifier-reg.c diff --git a/Documentation/devicetree/bindings/power/opp-modifier.txt b/Documentation/devicetree/bindings/power/opp-modifier.txt new file mode 100644 index 000..af8a2e9 --- /dev/null +++ b/Documentation/devicetree/bindings/power/opp-modifier.txt @@ -0,0 +1,111 @@ +* OPP-Modifier - opp modifier to selectively enable operating points + +Many SoCs that have selectively modifiable OPPs can specify +all available OPPs in their operating-points listing and then define +opp_modifiers to enable or disable the OPPs that are actually available +on the specific hardware. + +* OPP Modifier Provider + +For single bits that define the availability of an OPP: +--- +Some SoCs define a bit in a register that indicates whether an OPP is +available. This will disable any OPP with a frequency corresponding to +the bit given if it is not set appropriately. + +properties: +- compatible : Should be "opp-modifier-reg-bit" +- reg : Address and length of the registers needed to identify available + OPPs, here we provide just the register containing OPP data. + +Optional Properties: +- opp,reg-bit-enable-low: Take the complement of register before comparing mask +defined below under opp-modifier. + +Sub-nodes: +Sub-nodes are defined as a container to hold opp modifier table for a +specific device with an operating-points table already defined + +Sub-node properties: +- opp-modifier: A collection of rows consisting of the following entries to + allow specification of available OPPs: + -kHz: The opp to be enabled based on following criteria + -offset: Offset into register where relevant bits are located + -value: Bit that indicates availability of OPP + +Example: + + opp_modifier: opp_modifier@0x44e107fc { + compatible = "opp-modifier-reg-bit"; + reg = <0x44e107fc 0x04>; + + mpu_opp_modifier: mpu_opp_modifier { + opp-modifier = < + /* kHz offset value */ + 100 0 BIT_1 + 72 0 BIT_2 + >; + }; + }; + +For a value that defines the maximum available OPP: +--- +Some SoCs define a value in a register that corresponds to an OPP. If +that value is matched this will disable all OPPs greater than the +associated frequency. + +properties: +- compatible : Should be "opp-modifier-reg-val" +- reg : Address and length of the registers needed to identify available + OPPs, here we provide just the register containing OPP data. + +Optional Properties: +- opp,reg-mask: Only compare the bits masked off by this value. + +Sub-nodes: +Sub-nodes are defined as a container to hold opp modifier table for a +specific device with an operating-points table already defined + +Sub-node properties: +- opp-modifier: A collection of rows consisting of the following entries to + allow specification of available OPPs: + -kHz: The opp to be enabled based on following criteria + -offset: Offset into register where relevant bits are located + -value: Value that indicates maximum available OPP + +Example: + + opp_modifier: opp_modifier@0x44e107fc { + compatible = "opp-modifier-reg-val"; + reg = <0x44e107fc 0x04>; + + mpu_opp_modifier: mpu_opp_modifier { + opp-modifier = < + /* kHz offset value */ + 100 0 VAL_1 + 72 0 VAL_2 + >; + }; + }; + +* OPP Modifier Consumer + +Properties: +- platform-opp-modifier: phandle to the sub-node of the proper opp-modifier + provider that contains the appropriate opp-modifier table + +Example: + +cpu@0 { +compatible = "arm,cortex-a8"; +device_type = "cpu"; + +operating-points = < +/* kHz
Re: [RFC 2/9] opp-modifier: Add opp-modifier-reg driver
On Fri, Mar 14, 2014 at 2:25 PM, Dave Gerlach wrote: > Driver to read from a register and depending on either set bits or > a specific known selectively enable or disable OPPs based on DT node. > > Can support opp-modifier-reg-bit where single bits within the register > determine the availability of an OPP or opp-modifier-reg-val where a > certain value inside the register or a portion of it determine what the > maximum allowed OPP is. > > The driver expects a device that has already has its OPPs loaded > and then will disable the OPPs not matching the criteria specified in > the opp-modifier table. > > Signed-off-by: Dave Gerlach > --- > .../devicetree/bindings/power/opp-modifier.txt | 111 + > drivers/power/opp/Makefile | 1 + > drivers/power/opp/opp-modifier-reg.c | 259 > + > 3 files changed, 371 insertions(+) > create mode 100644 Documentation/devicetree/bindings/power/opp-modifier.txt > create mode 100644 drivers/power/opp/opp-modifier-reg.c > > diff --git a/Documentation/devicetree/bindings/power/opp-modifier.txt > b/Documentation/devicetree/bindings/power/opp-modifier.txt > new file mode 100644 > index 000..af8a2e9 > --- /dev/null > +++ b/Documentation/devicetree/bindings/power/opp-modifier.txt > @@ -0,0 +1,111 @@ > +* OPP-Modifier - opp modifier to selectively enable operating points > + > +Many SoCs that have selectively modifiable OPPs can specify > +all available OPPs in their operating-points listing and then define > +opp_modifiers to enable or disable the OPPs that are actually available > +on the specific hardware. > + > +* OPP Modifier Provider Uggg. Please stop designing around the current OPP binding which has the problem that the OPP table is not extensible to add more data. Define a new OPP binding that solves these problems. This is at least the 3rd OPP related binding addition I've seen recently. But I wouldn't spend a lot of effort on a new OPP binding just to add the functionality you are adding here because I don't like the whole concept in general. This might be a common way to determine valid OPPs on TI chips, but I think it is too low level and I don't want to see bindings for every different possible way. Just add platform code to do the OPP setup you need. Frankly, I prefer the bootloader/firmware fixup the OPP table approach mentioned in the cpufreq-cpu0 thread. Somewhat less desirable, but the kernel could do the fixups as well (via of_update_property). Rob -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 2/9] opp-modifier: Add opp-modifier-reg driver
On 03/14/2014 04:00 PM, Rob Herring wrote: > On Fri, Mar 14, 2014 at 2:25 PM, Dave Gerlach wrote: >> Driver to read from a register and depending on either set bits or >> a specific known selectively enable or disable OPPs based on DT node. >> >> Can support opp-modifier-reg-bit where single bits within the register >> determine the availability of an OPP or opp-modifier-reg-val where a >> certain value inside the register or a portion of it determine what the >> maximum allowed OPP is. >> >> The driver expects a device that has already has its OPPs loaded >> and then will disable the OPPs not matching the criteria specified in >> the opp-modifier table. >> >> Signed-off-by: Dave Gerlach >> --- >> .../devicetree/bindings/power/opp-modifier.txt | 111 + >> drivers/power/opp/Makefile | 1 + >> drivers/power/opp/opp-modifier-reg.c | 259 >> + >> 3 files changed, 371 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/power/opp-modifier.txt >> create mode 100644 drivers/power/opp/opp-modifier-reg.c >> >> diff --git a/Documentation/devicetree/bindings/power/opp-modifier.txt >> b/Documentation/devicetree/bindings/power/opp-modifier.txt >> new file mode 100644 >> index 000..af8a2e9 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/power/opp-modifier.txt >> @@ -0,0 +1,111 @@ >> +* OPP-Modifier - opp modifier to selectively enable operating points >> + >> +Many SoCs that have selectively modifiable OPPs can specify >> +all available OPPs in their operating-points listing and then define >> +opp_modifiers to enable or disable the OPPs that are actually available >> +on the specific hardware. >> + >> +* OPP Modifier Provider > > Uggg. Please stop designing around the current OPP binding which has > the problem that the OPP table is not extensible to add more data. > Define a new OPP binding that solves these problems. This is at least Generically, there are three different issues with current OPP bindings: a) ability to enable disable certain OPPs depending on SoC OTP/Efuse settings. b) ability to reuse OPPs defined for one device node for another (cpu1 to reuse OPP definitions of cpu0) c) ability to add additional information per OPP. we can argue this is a superset of (a), but really, the problems are different. Previous proposals include making each OPP as a phandle, but there does not seem much traction in that direction either. - proposal here has nothing to do with (b) or (c). > the 3rd OPP related binding addition I've seen recently. But I > wouldn't spend a lot of effort on a new OPP binding just to add the > functionality you are adding here because I don't like the whole > concept in general. This might be a common way to determine valid OPPs > on TI chips, but I think it is too low level and I don't want to see Not just TI chips, but iMX, now, Marvell, Xilinx as well. potentially more as well. doing OTP/Efuse based decision on which OPPs are valid on a chip is not a TI specific thing. This was the reason for us to try to define something generic enough to be reused by more SoCs than just TI. > bindings for every different possible way. Just add platform code to > do the OPP setup you need. Errr.. adding platform code means the hardware description goes back to kernel. is'nt that giving up on device tree binding for describing hardware? > > Frankly, I prefer the bootloader/firmware fixup the OPP table approach > mentioned in the cpufreq-cpu0 thread. Somewhat less desirable, but the > kernel could do the fixups as well (via of_update_property). a) Trying to move the hardware definition away from device tree seems to me a major step backwards. b) Allowing for definitions in platform code is a step backwards again for a generic solution that works for more than 1 vendor. c) moving the logic away to bootloader when it can easily be done in kernel again is adding burden to bootloader for data it does need to handle. OPP is a hardware behavior, which OPPs are enabled are described in hardware on certain SoCs. the current proposal is to provide a generic solution for those devices that allow for dynamic definition of OPPs based on SoC efuse definition. -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 2/9] opp-modifier: Add opp-modifier-reg driver
On Mon, Mar 17, 2014 at 9:30 AM, Nishanth Menon wrote: > On 03/14/2014 04:00 PM, Rob Herring wrote: >> On Fri, Mar 14, 2014 at 2:25 PM, Dave Gerlach wrote: >>> Driver to read from a register and depending on either set bits or >>> a specific known selectively enable or disable OPPs based on DT node. >>> >>> Can support opp-modifier-reg-bit where single bits within the register >>> determine the availability of an OPP or opp-modifier-reg-val where a >>> certain value inside the register or a portion of it determine what the >>> maximum allowed OPP is. >>> >>> The driver expects a device that has already has its OPPs loaded >>> and then will disable the OPPs not matching the criteria specified in >>> the opp-modifier table. >>> >>> Signed-off-by: Dave Gerlach >>> --- >>> .../devicetree/bindings/power/opp-modifier.txt | 111 + >>> drivers/power/opp/Makefile | 1 + >>> drivers/power/opp/opp-modifier-reg.c | 259 >>> + >>> 3 files changed, 371 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/power/opp-modifier.txt >>> create mode 100644 drivers/power/opp/opp-modifier-reg.c >>> >>> diff --git a/Documentation/devicetree/bindings/power/opp-modifier.txt >>> b/Documentation/devicetree/bindings/power/opp-modifier.txt >>> new file mode 100644 >>> index 000..af8a2e9 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/power/opp-modifier.txt >>> @@ -0,0 +1,111 @@ >>> +* OPP-Modifier - opp modifier to selectively enable operating points >>> + >>> +Many SoCs that have selectively modifiable OPPs can specify >>> +all available OPPs in their operating-points listing and then define >>> +opp_modifiers to enable or disable the OPPs that are actually available >>> +on the specific hardware. >>> + >>> +* OPP Modifier Provider >> >> Uggg. Please stop designing around the current OPP binding which has >> the problem that the OPP table is not extensible to add more data. >> Define a new OPP binding that solves these problems. This is at least > Generically, there are three different issues with current OPP bindings: > a) ability to enable disable certain OPPs depending on SoC OTP/Efuse > settings. More generically: ...depending on variety of factors. > b) ability to reuse OPPs defined for one device node for another (cpu1 > to reuse OPP definitions of cpu0) > c) ability to add additional information per OPP. we can argue this is > a superset of (a), but really, the problems are different. It is all additional data per OPP. Additional different information is of course for different problems. That doesn't mean we need different solutions. > Previous proposals include making each OPP as a phandle, but there > does not seem much traction in that direction either. - proposal here > has nothing to do with (b) or (c). They may have nothing to do with each other, but they all have to do with the OPP binding. If we're going to change/extend the binding, then all issues need to be taken into account. >> the 3rd OPP related binding addition I've seen recently. But I >> wouldn't spend a lot of effort on a new OPP binding just to add the >> functionality you are adding here because I don't like the whole >> concept in general. This might be a common way to determine valid OPPs >> on TI chips, but I think it is too low level and I don't want to see > > Not just TI chips, but iMX, now, Marvell, Xilinx as well. potentially > more as well. doing OTP/Efuse based decision on which OPPs are valid > on a chip is not a TI specific thing. This was the reason for us to > try to define something generic enough to be reused by more SoCs than > just TI. Agreed, but I'm not convinced how different SOCs determine valid OPPs is common enough. Certainly how to mark an entry disabled is common though. >> bindings for every different possible way. Just add platform code to >> do the OPP setup you need. > Errr.. adding platform code means the hardware description goes back > to kernel. is'nt that giving up on device tree binding for describing > hardware? We're always going to have some platform code. I'm not saying you have to in this case. I'm saying either come up with an OPP binding addressing all these issues or live with the existing one and fix it up in the kernel or bootloader. >> Frankly, I prefer the bootloader/firmware fixup the OPP table approach >> mentioned in the cpufreq-cpu0 thread. Somewhat less desirable, but the >> kernel could do the fixups as well (via of_update_property). > > a) Trying to move the hardware definition away from device tree seems > to me a major step backwards. > b) Allowing for definitions in platform code is a step backwards again > for a generic solution that works for more than 1 vendor. > c) moving the logic away to bootloader when it can easily be done in > kernel again is adding burden to bootloader for data it does need to > handle. The burden has to be somewhere. Maintaining a binding forever i
Re: [RFC 2/9] opp-modifier: Add opp-modifier-reg driver
On 03/17/2014 01:37 PM, Rob Herring wrote: > On Mon, Mar 17, 2014 at 9:30 AM, Nishanth Menon wrote: >> On 03/14/2014 04:00 PM, Rob Herring wrote: >>> On Fri, Mar 14, 2014 at 2:25 PM, Dave Gerlach wrote: Driver to read from a register and depending on either set bits or a specific known selectively enable or disable OPPs based on DT node. Can support opp-modifier-reg-bit where single bits within the register determine the availability of an OPP or opp-modifier-reg-val where a certain value inside the register or a portion of it determine what the maximum allowed OPP is. The driver expects a device that has already has its OPPs loaded and then will disable the OPPs not matching the criteria specified in the opp-modifier table. Signed-off-by: Dave Gerlach --- .../devicetree/bindings/power/opp-modifier.txt | 111 + drivers/power/opp/Makefile | 1 + drivers/power/opp/opp-modifier-reg.c | 259 + 3 files changed, 371 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/opp-modifier.txt create mode 100644 drivers/power/opp/opp-modifier-reg.c diff --git a/Documentation/devicetree/bindings/power/opp-modifier.txt b/Documentation/devicetree/bindings/power/opp-modifier.txt new file mode 100644 index 000..af8a2e9 --- /dev/null +++ b/Documentation/devicetree/bindings/power/opp-modifier.txt @@ -0,0 +1,111 @@ +* OPP-Modifier - opp modifier to selectively enable operating points + +Many SoCs that have selectively modifiable OPPs can specify +all available OPPs in their operating-points listing and then define +opp_modifiers to enable or disable the OPPs that are actually available +on the specific hardware. + +* OPP Modifier Provider >>> >>> Uggg. Please stop designing around the current OPP binding which has >>> the problem that the OPP table is not extensible to add more data. >>> Define a new OPP binding that solves these problems. This is at least >> Generically, there are three different issues with current OPP bindings: >> a) ability to enable disable certain OPPs depending on SoC OTP/Efuse >> settings. > > More generically: ...depending on variety of factors. Agreed. > >> b) ability to reuse OPPs defined for one device node for another (cpu1 >> to reuse OPP definitions of cpu0) >> c) ability to add additional information per OPP. we can argue this is >> a superset of (a), but really, the problems are different. > > It is all additional data per OPP. Additional different information is > of course for different problems. That doesn't mean we need different > solutions. > >> Previous proposals include making each OPP as a phandle, but there >> does not seem much traction in that direction either. - proposal here >> has nothing to do with (b) or (c). > > They may have nothing to do with each other, but they all have to do > with the OPP binding. If we're going to change/extend the binding, > then all issues need to be taken into account. We aren't extending the existing binding in this series. We are just defining how hardware description of which OPPs are valid here. >>> the 3rd OPP related binding addition I've seen recently. But I >>> wouldn't spend a lot of effort on a new OPP binding just to add the >>> functionality you are adding here because I don't like the whole >>> concept in general. This might be a common way to determine valid OPPs >>> on TI chips, but I think it is too low level and I don't want to see >> >> Not just TI chips, but iMX, now, Marvell, Xilinx as well. potentially >> more as well. doing OTP/Efuse based decision on which OPPs are valid >> on a chip is not a TI specific thing. This was the reason for us to >> try to define something generic enough to be reused by more SoCs than >> just TI. > > Agreed, but I'm not convinced how different SOCs determine valid OPPs > is common enough. Certainly how to mark an entry disabled is common > though. Fair enough, without procuring NDA documents for all the SoCs, I cannot comment much either, all we can do is see threads such as http://marc.info/?t=13947079113&r=1&w=2 and propose. This series does include iMx as well which seems to have equivalent challenges. I have given examples here on how the current driver at least tries to make generic the instances of SoCs that we have today, further, the driver in no way constraints us from using opp_modifier_register with proper ops in case we do something weirdly different (example: use non memory mapped operations) - it is just a simple framework. > >>> bindings for every different possible way. Just add platform code to >>> do the OPP setup you need. >> Errr.. adding platform code means the hardware description goes back >> to kernel. is'nt that giving up on device tree binding for describing >> hardwa
Re: [RFC 2/9] opp-modifier: Add opp-modifier-reg driver
On 03/18/2014 10:36 AM, Nishanth Menon wrote: On 03/17/2014 01:37 PM, Rob Herring wrote: On Mon, Mar 17, 2014 at 9:30 AM, Nishanth Menon wrote: On 03/14/2014 04:00 PM, Rob Herring wrote: On Fri, Mar 14, 2014 at 2:25 PM, Dave Gerlach wrote: Driver to read from a register and depending on either set bits or a specific known selectively enable or disable OPPs based on DT node. Can support opp-modifier-reg-bit where single bits within the register determine the availability of an OPP or opp-modifier-reg-val where a certain value inside the register or a portion of it determine what the maximum allowed OPP is. The driver expects a device that has already has its OPPs loaded and then will disable the OPPs not matching the criteria specified in the opp-modifier table. Signed-off-by: Dave Gerlach --- .../devicetree/bindings/power/opp-modifier.txt | 111 + drivers/power/opp/Makefile | 1 + drivers/power/opp/opp-modifier-reg.c | 259 + 3 files changed, 371 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/opp-modifier.txt create mode 100644 drivers/power/opp/opp-modifier-reg.c diff --git a/Documentation/devicetree/bindings/power/opp-modifier.txt b/Documentation/devicetree/bindings/power/opp-modifier.txt new file mode 100644 index 000..af8a2e9 --- /dev/null +++ b/Documentation/devicetree/bindings/power/opp-modifier.txt @@ -0,0 +1,111 @@ +* OPP-Modifier - opp modifier to selectively enable operating points + +Many SoCs that have selectively modifiable OPPs can specify +all available OPPs in their operating-points listing and then define +opp_modifiers to enable or disable the OPPs that are actually available +on the specific hardware. + +* OPP Modifier Provider Uggg. Please stop designing around the current OPP binding which has the problem that the OPP table is not extensible to add more data. Define a new OPP binding that solves these problems. This is at least Generically, there are three different issues with current OPP bindings: a) ability to enable disable certain OPPs depending on SoC OTP/Efuse settings. More generically: ...depending on variety of factors. Agreed. The idea here was not to touch the existing OPP bindings, hence the opp "modifier" name. As Nishanth stated, this is not extending the binding; the opp-modifier just uses the frequency values as an identifier but the driver does not necessarily care where the OPPs already in the table came from, just that they correspond to the same frequencies it describes. And again, just to reiterate, nothing is binding the user to use the opp-modifier-reg child driver, any driver could be written to decide which OPPs to enable or disable. With that said, I do understand that this is far from a perfect solution to the issue of defining which OPPs are available, I meant it as a suggestion for a possible way forward. This could be used as a starting point for something even more generic. It's a common problem on many SoCs even if it is defined in completely different ways so this framework or one like it could give a common point to branch out from. If we don't want to move forward with a generic layer to handle OPP availability, what is the best option? Does anybody else have opinions on this? Regardless of what is decided if everyone can agree on a direction we can all move forward. Regards, Dave b) ability to reuse OPPs defined for one device node for another (cpu1 to reuse OPP definitions of cpu0) c) ability to add additional information per OPP. we can argue this is a superset of (a), but really, the problems are different. It is all additional data per OPP. Additional different information is of course for different problems. That doesn't mean we need different solutions. Previous proposals include making each OPP as a phandle, but there does not seem much traction in that direction either. - proposal here has nothing to do with (b) or (c). They may have nothing to do with each other, but they all have to do with the OPP binding. If we're going to change/extend the binding, then all issues need to be taken into account. We aren't extending the existing binding in this series. We are just defining how hardware description of which OPPs are valid here. the 3rd OPP related binding addition I've seen recently. But I wouldn't spend a lot of effort on a new OPP binding just to add the functionality you are adding here because I don't like the whole concept in general. This might be a common way to determine valid OPPs on TI chips, but I think it is too low level and I don't want to see Not just TI chips, but iMX, now, Marvell, Xilinx as well. potentially more as well. doing OTP/Efuse based decision on which OPPs are valid on a chip is not a TI specific thing. This was the reason for us to try to define something generic enough to be reused by more SoCs than just TI. Agreed, but I'm not convinced