Re: [PATCH RFC v6 0/9] interconnect: Add imx support via devfreq

[Leonard Crestez <leonard.crestez@nxp.com>]

On 14.11.2019 22:10, Leonard Crestez wrote:
> This series adds interconnect scaling support for imx8m series chips. It uses a
> per-SOC interconnect provider layered on top of multiple instances of devfreq
> for scalable nodes along the interconnect.
> 
> Existing qcom interconnect providers mostly translate bandwidth requests into
> firmware calls but equivalent firmware on imx8m is much thinner. Scaling
> support for individual nodes is implemented as distinct devfreq drivers
> instead.
> 
> The imx interconnect provider doesn't communicate with devfreq directly
> but rather computes "minimum frequencies" for nodes along the path and
> creates dev_pm_qos requests.
> 
> Since there is no single devicetree node that can represent the
> "interconnect" the main NOC is picked as the "interconnect provider" and
> will probe the interconnect platform device if #interconnect-cells is
> present. This avoids introducing "virtual" devices but it means that DT
> bindings of main NOC includes properties for both devfreq and
> interconnect.
> 
> This depends on other series for devfreq pm/qos and ddrc:

It took a while but all runtime dependencies are included in 
next-20191210 and the compile-time dependency on 
DEV_PM_QOS_MIN_FREQUENCY is also included in v5.5-rc1. This series is a 
bit old but still applies usefully so I thought I'd ask for additional 
comments instead of a blank resend.

Georgi: can you please take a look at this series and suggest what would 
make it suitable for inclusion?

Some dubious aspects:

* Existing drivers have multiple providers per SOC (for each internal 
NOC?) but this is a single driver for the entire SOC topology. Not clear 
what would be gain from this.
* NOC DT node implements devfreq (because it can be scaled) and 
interconnect (to avoid virtual DT nodes). Maybe the devfreq NOC scaling 
implementation could be moved into drivers/interconnect/imx?

It is quite different from qcom providers but hardware/firmware is also 
very different.

> Changes since RFCv5:
> * Replace scanning for interconnect-node-id with explicit
> scalable-nodes/scalable-node-ids property on NoC.
> * Now passes make `dtbs_check`
> * Remove struct imx_icc_provider
> * Switch to of_icc_xlate_onecell
> * Use of_find_device_by_node to fetch QoS target, this causes fewer probe
> deferrals, removes dependency on devfreq API and even allows reloading ddrc
> module at runtime
> * Add imx_icc_node_destroy helper
> * Remove 0/1 on DEFINE_BUS_SLAVE/MASTER which created spurious links
> Link: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.kernel.org%2Fcover%2F11222015%2F&data=02%7C01%7Cleonard.crestez%40nxp.com%7Cff0127cbde3e4e6f6c8508d7693eb8d5%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637093590418229667&sdata=0EN1w8RtXHgyvXpRnOovEkWILoVlQ%2FEXJ0zdOOHI%2FgM%3D&reserved=0
> 
> Changes since RFCv4:
> * Drop icc proxy nonsense
> * Make devfreq driver for NOC probe the ICC driver if
> #interconnect-cells is present
> * Move NOC support to interconnect series and rename the node in DT
> * Add support for all chips at once, differences are not intereseting
> and there is more community interest for 8mq than 8mm.
> Link: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.kernel.org%2Fcover%2F11111865%2F&data=02%7C01%7Cleonard.crestez%40nxp.com%7Cff0127cbde3e4e6f6c8508d7693eb8d5%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637093590418229667&sdata=bsJqXPVJSszKJf%2BXdqFT2E8wriuKQU00sWwkRqZYmn0%3D&reserved=0
> 
> Changes since RFCv3:
> * Remove the virtual "icc" node and add devfreq nodes as proxy providers
> * Fix build on 32-bit arm (reported by kbuilt test robot)
> * Remove ARCH_MXC_ARM64 (never existed in upstream)
> * Remove _numlinks, calculate instead
> * Replace __BUSFREQ_H header guard
> * Improve commit message and comment spelling
> * Fix checkpatch issues
> Link to RFCv3: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.kernel.org%2Fcover%2F11078671%2F&data=02%7C01%7Cleonard.crestez%40nxp.com%7Cff0127cbde3e4e6f6c8508d7693eb8d5%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637093590418229667&sdata=c9xKEw9Fjlv%2FqDiWKr7zOUY5zZMWXjNyApBd94Nidnc%3D&reserved=0
> 
> Changes since RFCv2 and initial work by Alexandre Bailon:
> * Relying on devfreq and dev_pm_qos instead of CLK
> * No more "platform opp" stuff
> * No more special suspend handling: use suspend-opp on devfreq instead
> * Replace all mentions of "busfreq" with "interconnect"
> Link to v2: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.kernel.org%2Fcover%2F11021563%2F&data=02%7C01%7Cleonard.crestez%40nxp.com%7Cff0127cbde3e4e6f6c8508d7693eb8d5%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637093590418229667&sdata=1tEQS9BaHRTjdUYvZJ9%2FJG2BQQGl6hkbPWxiIZ811FY%3D&reserved=0
> 
> Leonard Crestez (9):
>    dt-bindings: interconnect: Add bindings for imx8m noc
>    PM / devfreq: Add generic imx bus scaling driver
>    PM / devfreq: imx: Register interconnect device
>    interconnect: Add imx core driver
>    interconnect: imx: Add platform driver for imx8mm
>    interconnect: imx: Add platform driver for imx8mq
>    interconnect: imx: Add platform driver for imx8mn
>    arm64: dts: imx8m: Add NOC nodes
>    arm64: dts: imx8m: Add interconnect provider properties
> 
>   .../bindings/interconnect/fsl,imx8m-noc.yaml  | 104 ++++++
>   arch/arm64/boot/dts/freescale/imx8mm.dtsi     |  28 ++
>   arch/arm64/boot/dts/freescale/imx8mn.dtsi     |  28 ++
>   arch/arm64/boot/dts/freescale/imx8mq.dtsi     |  28 ++
>   drivers/devfreq/Kconfig                       |   9 +
>   drivers/devfreq/Makefile                      |   1 +
>   drivers/devfreq/imx-devfreq.c                 | 187 +++++++++++
>   drivers/interconnect/Kconfig                  |   1 +
>   drivers/interconnect/Makefile                 |   1 +
>   drivers/interconnect/imx/Kconfig              |  17 +
>   drivers/interconnect/imx/Makefile             |   4 +
>   drivers/interconnect/imx/imx.c                | 301 ++++++++++++++++++
>   drivers/interconnect/imx/imx.h                |  60 ++++
>   drivers/interconnect/imx/imx8mm.c             | 105 ++++++
>   drivers/interconnect/imx/imx8mn.c             |  94 ++++++
>   drivers/interconnect/imx/imx8mq.c             | 103 ++++++
>   include/dt-bindings/interconnect/imx8mm.h     |  49 +++
>   include/dt-bindings/interconnect/imx8mn.h     |  41 +++
>   include/dt-bindings/interconnect/imx8mq.h     |  48 +++
>   19 files changed, 1209 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
>   create mode 100644 drivers/devfreq/imx-devfreq.c
>   create mode 100644 drivers/interconnect/imx/Kconfig
>   create mode 100644 drivers/interconnect/imx/Makefile
>   create mode 100644 drivers/interconnect/imx/imx.c
>   create mode 100644 drivers/interconnect/imx/imx.h
>   create mode 100644 drivers/interconnect/imx/imx8mm.c
>   create mode 100644 drivers/interconnect/imx/imx8mn.c
>   create mode 100644 drivers/interconnect/imx/imx8mq.c
>   create mode 100644 include/dt-bindings/interconnect/imx8mm.h
>   create mode 100644 include/dt-bindings/interconnect/imx8mn.h
>   create mode 100644 include/dt-bindings/interconnect/imx8mq.h
>