Re: [PATCH v3 2/2] PM / devfreq: Add devfreq driver for interconnect bandwidth voting

From: skannan
Date: Tue Aug 07 2018 - 15:31:51 EST

On 2018-08-07 09:51, Rob Herring wrote:
On Wed, Aug 01, 2018 at 05:57:42PM -0700, Saravana Kannan wrote:
This driver registers itself as a devfreq device that allows devfreq
governors to make bandwidth votes for an interconnect path. This allows
applying various policies for different interconnect paths using devfreq

Example uses:
* Use the devfreq performance governor to set the CPU to DDR interconnect
path for maximum performance.
* Use the devfreq performance governor to set the GPU to DDR interconnect
path for maximum performance.
* Use the CPU frequency to device frequency mapping governor to scale the
DDR frequency based on the needs of the CPUs' current frequency.

Signed-off-by: Saravana Kannan <skannan@xxxxxxxxxxxxxx>
Documentation/devicetree/bindings/devfreq/icbw.txt | 21 ++++

Please make bindings separate a patch.

Yeah, I was aware of that. I just wanted to give some context in the v1 of this patch (I wasn't expecting this to merge as is).

drivers/devfreq/Kconfig | 13 +++
drivers/devfreq/Makefile | 1 +
drivers/devfreq/devfreq_icbw.c | 116 +++++++++++++++++++++
4 files changed, 151 insertions(+)
create mode 100644 Documentation/devicetree/bindings/devfreq/icbw.txt
create mode 100644 drivers/devfreq/devfreq_icbw.c

diff --git a/Documentation/devicetree/bindings/devfreq/icbw.txt b/Documentation/devicetree/bindings/devfreq/icbw.txt
new file mode 100644
index 0000000..36cf045
--- /dev/null
+++ b/Documentation/devicetree/bindings/devfreq/icbw.txt
@@ -0,0 +1,21 @@
+Interconnect bandwidth device
+icbw is a device that represents an interconnect path that connects two
+devices. This device is typically used to vote for BW requirements between
+two devices. Eg: CPU to DDR, GPU to DDR, etc

I'm pretty sure this doesn't represent a h/w device. This usage doesn't
encourage me to accept the interconnects binding either.

Hasn't the DT rules moved past "only HW devices" in DT? Logical devices are still allowed in Linux DT bindings?

Having said that, this is explicitly representing a real HW path and the ability to control its performance.

+Required properties:
+- compatible: Must be "devfreq-icbw"
+- interconnects: Pairs of phandles and interconnect provider specifier
+ to denote the edge source and destination ports of
+ the interconnect path. See also:
+ Documentation/devicetree/bindings/interconnect/interconnect.txt
+- interconnect-names: Must have one entry with the name "path".

That's pretty useless...

True. But the current DT binding for interconnect consumer bindings needs a interconnect name to use the of_* API. I'm open to switching to an index based API if one is provided.

+ qcom,cpubw {

Someone in QCom please broadcast to stop using qcom,foo for node names.
It is amazing how consistent you all are. If only folks were as
consistent in reading

Sorry :(