| Message ID | 3f27038292c09c8bf07a086eac759132c100aedb.1566570260.git.leonard.crestez@nxp.com (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers | show |
| Series | interconnect: Add imx support via devfreq | expand |
On Fri, Aug 23, 2019 at 05:36:56PM +0300, Leonard Crestez wrote: > The interconnect-node-id property is parsed by the imx interconnect > driver to find nodes on which frequencies can be adjusted. > > Add #interconnect-cells so that device drivers can request paths from > bus nodes instead of requiring a separate "virtual" node to represent > the interconnect itself. > > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> > --- > Documentation/devicetree/bindings/devfreq/imx-ddrc.yaml | 5 +++++ > Documentation/devicetree/bindings/devfreq/imx.yaml | 5 +++++ > 2 files changed, 10 insertions(+) Please combine this with the other series for devfreq support. Rob
On 17.09.2019 23:20, Rob Herring wrote: > On Fri, Aug 23, 2019 at 05:36:56PM +0300, Leonard Crestez wrote: >> The interconnect-node-id property is parsed by the imx interconnect >> driver to find nodes on which frequencies can be adjusted. >> >> Add #interconnect-cells so that device drivers can request paths from >> bus nodes instead of requiring a separate "virtual" node to represent >> the interconnect itself. >> >> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> >> --- >> Documentation/devicetree/bindings/devfreq/imx-ddrc.yaml | 5 +++++ >> Documentation/devicetree/bindings/devfreq/imx.yaml | 5 +++++ >> 2 files changed, 10 insertions(+) > > Please combine this with the other series for devfreq support. I understand that having two series which add to the same bindings file is odd but the devfreq and interconnect parts are independent to a very large degree and devfreq can be useful on it's own. The only reason devfreq bindings are updated is to avoid adding a "virtual" node for interconnect. Since DT is a big source of confusion here can you read https://patchwork.kernel.org/cover/11111865/#22883457 and maybe offer some advice? -- Regards, Leonard
On Mon, Sep 23, 2019 at 12:42 PM Leonard Crestez <leonard.crestez@nxp.com> wrote: > > On 17.09.2019 23:20, Rob Herring wrote: > > On Fri, Aug 23, 2019 at 05:36:56PM +0300, Leonard Crestez wrote: > >> The interconnect-node-id property is parsed by the imx interconnect > >> driver to find nodes on which frequencies can be adjusted. > >> > >> Add #interconnect-cells so that device drivers can request paths from > >> bus nodes instead of requiring a separate "virtual" node to represent > >> the interconnect itself. > >> > >> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> > >> --- > >> Documentation/devicetree/bindings/devfreq/imx-ddrc.yaml | 5 +++++ > >> Documentation/devicetree/bindings/devfreq/imx.yaml | 5 +++++ > >> 2 files changed, 10 insertions(+) > > > > Please combine this with the other series for devfreq support. > > I understand that having two series which add to the same bindings file > is odd but the devfreq and interconnect parts are independent to a very > large degree and devfreq can be useful on it's own. To start with, I'm suspicious of any 'devfreq' binding because that's a Linux thing. I somewhat expect that the interconnect binding should replace the devfreq binding, but I haven't been able to investigate. > The only reason devfreq bindings are updated is to avoid adding a > "virtual" node for interconnect. Since DT is a big source of confusion > here can you read https://patchwork.kernel.org/cover/11111865/#22883457 > and maybe offer some advice? Design something that matches the structure of the h/w not how Linux drivers happen to be structured. I can't tell what that is without any context around adding a couple of properties. Nor do I have the time to dig into each SoC vendor's bus structure if it's even documented publicly. I also don't follow why you need 'interconnect-node-id' and if you do, it should be a common property. Rob
On 24.09.2019 00:04, Rob Herring wrote: > On Mon, Sep 23, 2019 at 12:42 PM Leonard Crestez > <leonard.crestez@nxp.com> wrote: >> >> On 17.09.2019 23:20, Rob Herring wrote: >>> On Fri, Aug 23, 2019 at 05:36:56PM +0300, Leonard Crestez wrote: >>>> The interconnect-node-id property is parsed by the imx interconnect >>>> driver to find nodes on which frequencies can be adjusted. >>>> >>>> Add #interconnect-cells so that device drivers can request paths from >>>> bus nodes instead of requiring a separate "virtual" node to represent >>>> the interconnect itself. >>>> >>>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> >>>> --- >>>> Documentation/devicetree/bindings/devfreq/imx-ddrc.yaml | 5 +++++ >>>> Documentation/devicetree/bindings/devfreq/imx.yaml | 5 +++++ >>>> 2 files changed, 10 insertions(+) >>> >>> Please combine this with the other series for devfreq support. >> >> I understand that having two series which add to the same bindings file >> is odd but the devfreq and interconnect parts are independent to a very >> large degree and devfreq can be useful on it's own. > > To start with, I'm suspicious of any 'devfreq' binding because that's > a Linux thing. I somewhat expect that the interconnect binding should > replace the devfreq binding, but I haven't been able to investigate. > > Design something that matches the structure of the h/w not how Linux > drivers happen to be structured. I can't tell what that is without any > context around adding a couple of properties. Nor do I have the time > to dig into each SoC vendor's bus structure if it's even documented > publicly. Device tree binding files are organized based on linux subsystems but otherwise there's little particularly specific to "devfreq" in this new binding. An imx NIC or NOC is a physical component of a SOC which can run at a variable clock speed. The device binding uses standard clk and opp tables in meaningful ways so that first part is reasonable. I also want to implement an interconnect provider but an "interconnect" is not a single device but rather a graph of discrete buses. Some options: * Add a custom virtual device, easy but not upstreamable. * Have a single icc provider device use the individual buses as "proxies" for OF parting. Implemented in this series but not very pretty. * Pick the "main noc" as the single interconnect provider? Alternatively the approach of defining the graph in the driver could be dropped and everything could be described in DT (quite verbose). This seems to be what Samsung is doing: https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=176291 It might even possible to use of_graph; this is complicated but the NICs and NOCs do in fact have discrete ports with assignable properties. > I also don't follow why you need 'interconnect-node-id' and if you do, > it should be a common property. The "interconnect-node-id" property in devicetree identifies nodes from the interconnect graph; for example the DDRC node identifies itself as IMX8MM_ICS_DRAM. -- Regards, Leonard
diff --git a/Documentation/devicetree/bindings/devfreq/imx-ddrc.yaml b/Documentation/devicetree/bindings/devfreq/imx-ddrc.yaml index 31db204e6845..014449a9dd01 100644 --- a/Documentation/devicetree/bindings/devfreq/imx-ddrc.yaml +++ b/Documentation/devicetree/bindings/devfreq/imx-ddrc.yaml @@ -31,10 +31,15 @@ properties: - const: dram_alt - const: dram_apb operating-points-v2: true + interconnect-node-id: + $ref: /schemas/types.yaml#/definitions/uint32 + '#interconnect-cells': + const: 1 + devfreq-events: description: Phandle of PMU node $ref: "/schemas/types.yaml#/definitions/phandle" required: diff --git a/Documentation/devicetree/bindings/devfreq/imx.yaml b/Documentation/devicetree/bindings/devfreq/imx.yaml index 634870496d5e..f2f9b76c752f 100644 --- a/Documentation/devicetree/bindings/devfreq/imx.yaml +++ b/Documentation/devicetree/bindings/devfreq/imx.yaml @@ -43,10 +43,15 @@ properties: clocks: maxItems: 1 operating-points-v2: true + interconnect-node-id: + $ref: /schemas/types.yaml#/definitions/uint32 + '#interconnect-cells': + const: 1 + devfreq: description: | Phandle to another devfreq device to match OPPs with by using the passive governor. $ref: "/schemas/types.yaml#/definitions/phandle"
The interconnect-node-id property is parsed by the imx interconnect driver to find nodes on which frequencies can be adjusted. Add #interconnect-cells so that device drivers can request paths from bus nodes instead of requiring a separate "virtual" node to represent the interconnect itself. Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> --- Documentation/devicetree/bindings/devfreq/imx-ddrc.yaml | 5 +++++ Documentation/devicetree/bindings/devfreq/imx.yaml | 5 +++++ 2 files changed, 10 insertions(+)