[PATCH 0/3] drivers: bus: Add Freescale i.MX8qxp pixel link MSI bus support

[PATCH 0/3] drivers: bus: Add Freescale i.MX8qxp pixel link MSI bus support

[Liu Ying]

Hi,

This series aims to add Freescale i.MX8qxp pixel link MSI bus support
by using the existing simple-pm-bus driver. A power domain and two input
clocks need to be enabled before the MSI bus accesses it's child devices,
which matches what a simple power-managed bus is(See simple-pm-bus.yaml).

Patch 1 adds support to populate simple MFD child devices in the
simple-pm-bus driver, since the MSI bus may connect those devices.

Patch 2 enables/disables functional clock(s) as a bulk in the
simple-pm-bus driver when the simple-pm-bus is being power managed,
since the MSI bus takes the two input clocks as functional clocks.

Patch 3 adds dt-bindings for the MSI bus.

Liu Ying (3):
  drivers: bus: simple-pm-bus: Populate simple MFD child devices
  drivers: bus: simple-pm-bus: Use clocks
  dt-bindings: bus: Add Freescale i.MX8qxp pixel link MSI bus binding

 .../bus/fsl,imx8qxp-pixel-link-msi-bus.yaml   | 84 +++++++++++++++++++
 drivers/bus/simple-pm-bus.c                   | 61 +++++++++++++-
 2 files changed, 144 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/bus/fsl,imx8qxp-pixel-link-msi-bus.yaml

-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] drivers: bus: simple-pm-bus: Populate simple MFD child devices

[Liu Ying]

There could be simple MFD device(s) connected to a simple PM bus as child
node(s), like Freescale i.MX8qxp pixel link MSI bus. Add a child match
table as an argument to of_platform_populate() function call to specify
the simple MFD devices so that they can be populated.

Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
 drivers/bus/simple-pm-bus.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/simple-pm-bus.c b/drivers/bus/simple-pm-bus.c
index 6b8d6257ed8a..ff5f8ca5c024 100644
--- a/drivers/bus/simple-pm-bus.c
+++ b/drivers/bus/simple-pm-bus.c
@@ -13,6 +13,11 @@
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 
+static const struct of_device_id simple_pm_bus_child_matches[] = {
+	{ .compatible = "simple-mfd", },
+	{}
+};
+
 static int simple_pm_bus_probe(struct platform_device *pdev)
 {
 	const struct device *dev = &pdev->dev;
@@ -49,7 +54,7 @@ static int simple_pm_bus_probe(struct platform_device *pdev)
 	pm_runtime_enable(&pdev->dev);
 
 	if (np)
-		of_platform_populate(np, NULL, lookup, &pdev->dev);
+		of_platform_populate(np, simple_pm_bus_child_matches, lookup, &pdev->dev);
 
 	return 0;
 }
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] drivers: bus: simple-pm-bus: Use clocks

[Liu Ying]

Simple Power-Managed bus controller may need functional clock(s)
to be enabled before child devices connected to the bus can be
accessed.  Get the clock(s) as a bulk and enable/disable the
clock(s) when the bus is being power managed.

One example is that Freescale i.MX8qxp pixel link MSI bus controller
needs MSI clock and AHB clock to be enabled before accessing child
devices.

Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
 drivers/bus/simple-pm-bus.c | 54 +++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/drivers/bus/simple-pm-bus.c b/drivers/bus/simple-pm-bus.c
index ff5f8ca5c024..876a906724b3 100644
--- a/drivers/bus/simple-pm-bus.c
+++ b/drivers/bus/simple-pm-bus.c
@@ -8,11 +8,17 @@
  * for more details.
  */
 
+#include <linux/clk.h>
 #include <linux/module.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 
+struct simple_pm_bus {
+	struct clk_bulk_data *clks;
+	int num_clks;
+};
+
 static const struct of_device_id simple_pm_bus_child_matches[] = {
 	{ .compatible = "simple-mfd", },
 	{}
@@ -24,6 +30,7 @@ static int simple_pm_bus_probe(struct platform_device *pdev)
 	const struct of_dev_auxdata *lookup = dev_get_platdata(dev);
 	struct device_node *np = dev->of_node;
 	const struct of_device_id *match;
+	struct simple_pm_bus *bus;
 
 	/*
 	 * Allow user to use driver_override to bind this driver to a
@@ -49,6 +56,16 @@ static int simple_pm_bus_probe(struct platform_device *pdev)
 			return -ENODEV;
 	}
 
+	bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
+	if (!bus)
+		return -ENOMEM;
+
+	bus->num_clks = devm_clk_bulk_get_all(&pdev->dev, &bus->clks);
+	if (bus->num_clks < 0)
+		return dev_err_probe(&pdev->dev, bus->num_clks, "failed to get clocks\n");
+
+	dev_set_drvdata(&pdev->dev, bus);
+
 	dev_dbg(&pdev->dev, "%s\n", __func__);
 
 	pm_runtime_enable(&pdev->dev);
@@ -72,6 +89,42 @@ static int simple_pm_bus_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static int simple_pm_bus_runtime_suspend(struct device *dev)
+{
+	struct simple_pm_bus *bus = dev_get_drvdata(dev);
+
+	if (!bus)
+		return 0;
+
+	clk_bulk_disable_unprepare(bus->num_clks, bus->clks);
+
+	return 0;
+}
+
+static int simple_pm_bus_runtime_resume(struct device *dev)
+{
+	struct simple_pm_bus *bus = dev_get_drvdata(dev);
+	int ret;
+
+	if (!bus)
+		return 0;
+
+	ret = clk_bulk_prepare_enable(bus->num_clks, bus->clks);
+	if (ret) {
+		dev_err(dev, "failed to enable clocks: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct dev_pm_ops simple_pm_bus_pm_ops = {
+	SET_RUNTIME_PM_OPS(simple_pm_bus_runtime_suspend,
+			   simple_pm_bus_runtime_resume, NULL)
+	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				      pm_runtime_force_resume)
+};
+
 #define ONLY_BUS	((void *) 1) /* Match if the device is only a bus. */
 
 static const struct of_device_id simple_pm_bus_of_match[] = {
@@ -90,6 +143,7 @@ static struct platform_driver simple_pm_bus_driver = {
 	.driver = {
 		.name = "simple-pm-bus",
 		.of_match_table = simple_pm_bus_of_match,
+		.pm = &simple_pm_bus_pm_ops,
 	},
 };
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/3] drivers: bus: simple-pm-bus: Use clocks

[Krzysztof Kozlowski]

On 02/08/2022 09:13, Liu Ying wrote:
> Simple Power-Managed bus controller may need functional clock(s)
> to be enabled before child devices connected to the bus can be
> accessed.  Get the clock(s) as a bulk and enable/disable the
> clock(s) when the bus is being power managed.
> 
> One example is that Freescale i.MX8qxp pixel link MSI bus controller
> needs MSI clock and AHB clock to be enabled before accessing child
> devices.

No, because it is not simple bus anymore.


Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/3] drivers: bus: simple-pm-bus: Use clocks

[Krzysztof Kozlowski]

On 02/08/2022 12:56, Krzysztof Kozlowski wrote:
> On 02/08/2022 09:13, Liu Ying wrote:
>> Simple Power-Managed bus controller may need functional clock(s)
>> to be enabled before child devices connected to the bus can be
>> accessed.  Get the clock(s) as a bulk and enable/disable the
>> clock(s) when the bus is being power managed.
>>
>> One example is that Freescale i.MX8qxp pixel link MSI bus controller
>> needs MSI clock and AHB clock to be enabled before accessing child
>> devices.
> 
> No, because it is not simple bus anymore.

Hm, actually I take back my no, as this pm-bus was done for such
purpose. Since we allow power domains, we should also allow clocks.

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] dt-bindings: bus: Add Freescale i.MX8qxp pixel link MSI bus binding

[Liu Ying]

Freescale i.MX8qxp pixel link MSI bus is a simple memory-mapped bus.
It is used to access peripherals in i.MX8qm/qxp imaging, LVDS, MIPI
DSI and HDMI TX subsystems, like I2C controller, PWM controller,
MIPI DSI controller and Control and Status Registers (CSR) module.

Reference simple-pm-bus bindings and add Freescale i.MX8qxp pixel
link MSI bus specific bindings.

Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
 .../bus/fsl,imx8qxp-pixel-link-msi-bus.yaml   | 84 +++++++++++++++++++
 1 file changed, 84 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/bus/fsl,imx8qxp-pixel-link-msi-bus.yaml

diff --git a/Documentation/devicetree/bindings/bus/fsl,imx8qxp-pixel-link-msi-bus.yaml b/Documentation/devicetree/bindings/bus/fsl,imx8qxp-pixel-link-msi-bus.yaml
new file mode 100644
index 000000000000..24f50535f5c2
--- /dev/null
+++ b/Documentation/devicetree/bindings/bus/fsl,imx8qxp-pixel-link-msi-bus.yaml
@@ -0,0 +1,84 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bus/fsl,imx8qxp-pixel-link-msi-bus.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale i.MX8qxp Pixel Link Medium Speed Interconnect (MSI) Bus
+
+maintainers:
+  - Liu Ying <victor.liu@nxp.com>
+
+description: |
+  i.MX8qxp pixel link MSI bus is used to control settings of PHYs, I/Os
+  sitting together with the PHYs.  It is not the same as the MSI bus coming
+  from i.MX8 System Controller Unit (SCU) which is used to control power,
+  clock and reset through the i.MX8 Distributed Slave System Controller (DSC).
+
+  i.MX8qxp pixel link MSI bus is a simple memory-mapped bus. Two input clocks,
+  that is, MSI clock and AHB clock, need to be enabled so that peripherals
+  connected to the bus can be accessed. Also, the bus is part of a power
+  domain. The power domain needs to be enabled before the peripherals can
+  be accessed.
+
+  Peripherals in i.MX8qm/qxp imaging, LVDS, MIPI DSI and HDMI TX subsystems,
+  like I2C controller, PWM controller, MIPI DSI controller and Control and
+  Status Registers (CSR) module, are accessed through the bus.
+
+  The i.MX System Controller Firmware (SCFW) owns and uses the i.MX8qm/qxp
+  pixel link MSI bus controller and does not allow SCFW user to control it.
+  So, the controller's registers cannot be accessed by SCFW user. Hence,
+  the interrupts generated by the controller don't make any sense from SCFW
+  user's point of view.
+
+allOf:
+  - $ref: simple-pm-bus.yaml#
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - fsl,imx8qxp-display-pixel-link-msi-bus
+          - fsl,imx8qm-display-pixel-link-msi-bus
+      - {} # simple-pm-bus, but not listed here to avoid false select
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: master gated clock from system
+      - description: AHB clock
+
+  clock-names:
+    items:
+      - const: msi
+      - const: ahb
+
+required:
+  - clocks
+  - clock-names
+  - power-domains
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/imx8-lpcg.h>
+    #include <dt-bindings/firmware/imx/rsrc.h>
+    bus@56200000 {
+        compatible = "fsl,imx8qxp-display-pixel-link-msi-bus", "simple-pm-bus";
+        #address-cells = <1>;
+        #size-cells = <1>;
+        reg = <0x56200000 0x20000>;
+        interrupt-parent = <&dc0_irqsteer>;
+        interrupts = <320>;
+        ranges;
+        clocks = <&dc0_disp_ctrl_link_mst0_lpcg IMX_LPCG_CLK_4>,
+                 <&dc0_disp_ctrl_link_mst0_lpcg IMX_LPCG_CLK_4>;
+        clock-names = "msi", "ahb";
+        power-domains = <&pd IMX_SC_R_DC_0>;
+    };
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] dt-bindings: bus: Add Freescale i.MX8qxp pixel link MSI bus binding

[Krzysztof Kozlowski]

On 02/08/2022 09:13, Liu Ying wrote:
> Freescale i.MX8qxp pixel link MSI bus is a simple memory-mapped bus.
> It is used to access peripherals in i.MX8qm/qxp imaging, LVDS, MIPI
> DSI and HDMI TX subsystems, like I2C controller, PWM controller,
> MIPI DSI controller and Control and Status Registers (CSR) module.
> 
> Reference simple-pm-bus bindings and add Freescale i.MX8qxp pixel
> link MSI bus specific bindings.
> 
> Signed-off-by: Liu Ying <victor.liu@nxp.com>
> ---
>  .../bus/fsl,imx8qxp-pixel-link-msi-bus.yaml   | 84 +++++++++++++++++++
>  1 file changed, 84 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/bus/fsl,imx8qxp-pixel-link-msi-bus.yaml
> 
> diff --git a/Documentation/devicetree/bindings/bus/fsl,imx8qxp-pixel-link-msi-bus.yaml b/Documentation/devicetree/bindings/bus/fsl,imx8qxp-pixel-link-msi-bus.yaml
> new file mode 100644
> index 000000000000..24f50535f5c2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/bus/fsl,imx8qxp-pixel-link-msi-bus.yaml
> @@ -0,0 +1,84 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/bus/fsl,imx8qxp-pixel-link-msi-bus.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale i.MX8qxp Pixel Link Medium Speed Interconnect (MSI) Bus

Shouldn't this be interconnect, not a bus? Not only located in
interconnect directory but actually being proper interconnect? Although
you mentioned that the firmware controls it, so maybe that would explain
this being only a resource provider.

You should be sure of it, because later if you want to add proper
interconnect properties (e.g. bandwidth voting, paths) *you will not be
able*. Ever.

> +
> +maintainers:
> +  - Liu Ying <victor.liu@nxp.com>
> +
> +description: |
> +  i.MX8qxp pixel link MSI bus is used to control settings of PHYs, I/Os
> +  sitting together with the PHYs.  It is not the same as the MSI bus coming
> +  from i.MX8 System Controller Unit (SCU) which is used to control power,
> +  clock and reset through the i.MX8 Distributed Slave System Controller (DSC).
> +
> +  i.MX8qxp pixel link MSI bus is a simple memory-mapped bus. Two input clocks,
> +  that is, MSI clock and AHB clock, need to be enabled so that peripherals
> +  connected to the bus can be accessed. Also, the bus is part of a power
> +  domain. The power domain needs to be enabled before the peripherals can
> +  be accessed.
> +
> +  Peripherals in i.MX8qm/qxp imaging, LVDS, MIPI DSI and HDMI TX subsystems,
> +  like I2C controller, PWM controller, MIPI DSI controller and Control and
> +  Status Registers (CSR) module, are accessed through the bus.
> +
> +  The i.MX System Controller Firmware (SCFW) owns and uses the i.MX8qm/qxp
> +  pixel link MSI bus controller and does not allow SCFW user to control it.
> +  So, the controller's registers cannot be accessed by SCFW user. Hence,
> +  the interrupts generated by the controller don't make any sense from SCFW
> +  user's point of view.
> +
> +allOf:
> +  - $ref: simple-pm-bus.yaml#
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - fsl,imx8qxp-display-pixel-link-msi-bus
> +          - fsl,imx8qm-display-pixel-link-msi-bus
> +      - {} # simple-pm-bus, but not listed here to avoid false select

simple-pm-bus must be here. You need to sort out the select instead,
just like we do it for other devices (e.g. primecell).

> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: master gated clock from system
> +      - description: AHB clock
> +
> +  clock-names:
> +    items:
> +      - const: msi
> +      - const: ahb
> +
> +required:

compatible and reg as well.

> +  - clocks
> +  - clock-names
> +  - power-domains
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/imx8-lpcg.h>
> +    #include <dt-bindings/firmware/imx/rsrc.h>
> +    bus@56200000 {
> +        compatible = "fsl,imx8qxp-display-pixel-link-msi-bus", "simple-pm-bus";
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +        reg = <0x56200000 0x20000>;

put reg just after compatible.

> +        interrupt-parent = <&dc0_irqsteer>;
> +        interrupts = <320>;
> +        ranges;
> +        clocks = <&dc0_disp_ctrl_link_mst0_lpcg IMX_LPCG_CLK_4>,
> +                 <&dc0_disp_ctrl_link_mst0_lpcg IMX_LPCG_CLK_4>;
> +        clock-names = "msi", "ahb";
> +        power-domains = <&pd IMX_SC_R_DC_0>;
> +    };


Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] dt-bindings: bus: Add Freescale i.MX8qxp pixel link MSI bus binding

[Liu Ying]

On Tue, 2022-08-02 at 13:04 +0200, Krzysztof Kozlowski wrote:
> On 02/08/2022 09:13, Liu Ying wrote:
> > Freescale i.MX8qxp pixel link MSI bus is a simple memory-mapped
> > bus.
> > It is used to access peripherals in i.MX8qm/qxp imaging, LVDS, MIPI
> > DSI and HDMI TX subsystems, like I2C controller, PWM controller,
> > MIPI DSI controller and Control and Status Registers (CSR) module.
> > 
> > Reference simple-pm-bus bindings and add Freescale i.MX8qxp pixel
> > link MSI bus specific bindings.
> > 
> > Signed-off-by: Liu Ying <victor.liu@nxp.com>
> > ---
> >  .../bus/fsl,imx8qxp-pixel-link-msi-bus.yaml   | 84
> > +++++++++++++++++++
> >  1 file changed, 84 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/bus/fsl,imx8qxp-pixel-link-msi-
> > bus.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/bus/fsl,imx8qxp-
> > pixel-link-msi-bus.yaml
> > b/Documentation/devicetree/bindings/bus/fsl,imx8qxp-pixel-link-msi-
> > bus.yaml
> > new file mode 100644
> > index 000000000000..24f50535f5c2
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/bus/fsl,imx8qxp-pixel-link-
> > msi-bus.yaml
> > @@ -0,0 +1,84 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: 
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fbus%2Ffsl%2Cimx8qxp-pixel-link-msi-bus.yaml%23&amp;data=05%7C01%7Cvictor.liu%40nxp.com%7C7ee06e5e179b4cb8529308da7476be70%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637950350594434521%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=GsZgqUmEsVm2If6bvx%2FrVOFWnaiJp2zFVERvzWP%2BecM%3D&amp;reserved=0
> > +$schema: 
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=05%7C01%7Cvictor.liu%40nxp.com%7C7ee06e5e179b4cb8529308da7476be70%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637950350594434521%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=v6x0rQDqg%2FrLshXZvKdeM97IdOWtnu9O5o0Dz3%2FQID8%3D&amp;reserved=0
> > +
> > +title: Freescale i.MX8qxp Pixel Link Medium Speed Interconnect
> > (MSI) Bus
> 
> Shouldn't this be interconnect, not a bus? Not only located in
> interconnect directory but actually being proper interconnect?
> Although
> you mentioned that the firmware controls it, so maybe that would
> explain
> this being only a resource provider.

Linux kernel just enables the power domain and the two input clocks
for the MSI bus, then the MSI bus starts to work. All other stuff is
totally out of Linux kernel's control, which means SCFW doesn't expose
any direct or indirect MSI bus control interfaces to it's user. So, it
looks like the MSI bus fits into the simple power-managed bus category.

> 
> You should be sure of it, because later if you want to add proper
> interconnect properties (e.g. bandwidth voting, paths) *you will not
> be
> able*. Ever.

There is really nothing more than the power domain and the two input
clocks that can be controlled for the MSI bus by Linux driver. I don't
expect to add any other properties.

> 
> > +
> > +maintainers:
> > +  - Liu Ying <victor.liu@nxp.com>
> > +
> > +description: |
> > +  i.MX8qxp pixel link MSI bus is used to control settings of PHYs,
> > I/Os
> > +  sitting together with the PHYs.  It is not the same as the MSI
> > bus coming
> > +  from i.MX8 System Controller Unit (SCU) which is used to control
> > power,
> > +  clock and reset through the i.MX8 Distributed Slave System
> > Controller (DSC).
> > +
> > +  i.MX8qxp pixel link MSI bus is a simple memory-mapped bus. Two
> > input clocks,
> > +  that is, MSI clock and AHB clock, need to be enabled so that
> > peripherals
> > +  connected to the bus can be accessed. Also, the bus is part of a
> > power
> > +  domain. The power domain needs to be enabled before the
> > peripherals can
> > +  be accessed.
> > +
> > +  Peripherals in i.MX8qm/qxp imaging, LVDS, MIPI DSI and HDMI TX
> > subsystems,
> > +  like I2C controller, PWM controller, MIPI DSI controller and
> > Control and
> > +  Status Registers (CSR) module, are accessed through the bus.
> > +
> > +  The i.MX System Controller Firmware (SCFW) owns and uses the
> > i.MX8qm/qxp
> > +  pixel link MSI bus controller and does not allow SCFW user to
> > control it.
> > +  So, the controller's registers cannot be accessed by SCFW user.
> > Hence,
> > +  the interrupts generated by the controller don't make any sense
> > from SCFW
> > +  user's point of view.
> > +
> > +allOf:
> > +  - $ref: simple-pm-bus.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - fsl,imx8qxp-display-pixel-link-msi-bus
> > +          - fsl,imx8qm-display-pixel-link-msi-bus
> > +      - {} # simple-pm-bus, but not listed here to avoid false
> > select
> 
> simple-pm-bus must be here. You need to sort out the select instead,
> just like we do it for other devices (e.g. primecell).

Will do.

> 
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: master gated clock from system
> > +      - description: AHB clock
> > +
> > +  clock-names:
> > +    items:
> > +      - const: msi
> > +      - const: ahb
> > +
> > +required:
> 
> compatible and reg as well.

Will do.

> 
> > +  - clocks
> > +  - clock-names
> > +  - power-domains
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/imx8-lpcg.h>
> > +    #include <dt-bindings/firmware/imx/rsrc.h>
> > +    bus@56200000 {
> > +        compatible = "fsl,imx8qxp-display-pixel-link-msi-bus",
> > "simple-pm-bus";
> > +        #address-cells = <1>;
> > +        #size-cells = <1>;
> > +        reg = <0x56200000 0x20000>;
> 
> put reg just after compatible.

Will do.

Thanks for your review.

Liu Ying

> 
> > +        interrupt-parent = <&dc0_irqsteer>;
> > +        interrupts = <320>;
> > +        ranges;
> > +        clocks = <&dc0_disp_ctrl_link_mst0_lpcg IMX_LPCG_CLK_4>,
> > +                 <&dc0_disp_ctrl_link_mst0_lpcg IMX_LPCG_CLK_4>;
> > +        clock-names = "msi", "ahb";
> > +        power-domains = <&pd IMX_SC_R_DC_0>;
> > +    };
> 
> 
> Best regards,
> Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel