2021-05-21 20:13:05

by Peng Fan (OSS)

[permalink] [raw]
Subject: [PATCH V5 0/4] soc: imx: add i.MX BLK-CTL support

From: Peng Fan <[email protected]>

V5:
Rework the blk-ctl driver to let sub-PGC use blk-ctl as parent power
domain to fix the potential handshake issue.
I still keep R-b/A-b tag for Patch 1,2,4, since very minor changes
I only drop R-b tag for Patch 3, since it has big change.
An example, the pgc_mipi not take pgc_dispmix as parent:

pgc_dispmix: power-domain@10 {
#power-domain-cells = <0>;
reg = <IMX8MM_POWER_DOMAIN_DISPMIX>;
clocks = <&clk IMX8MM_CLK_DISP_ROOT>,
<&clk IMX8MM_CLK_DISP_AXI_ROOT>,
<&clk IMX8MM_CLK_DISP_APB_ROOT>;
};

pgc_mipi: power-domain@11 {
#power-domain-cells = <0>;
reg = <IMX8MM_POWER_DOMAIN_MIPI>;
power-domains = <&dispmix_blk_ctl IMX8MM_BLK_CTL_PD_DISPMIX_BUS>;
};

dispmix_blk_ctl: clock-controller@32e28000 {
compatible = "fsl,imx8mm-dispmix-blk-ctl", "syscon";
reg = <0x32e28000 0x100>;
#power-domain-cells = <1>;
power-domains = <&pgc_dispmix>, <&pgc_mipi>;
power-domain-names = "dispmix", "mipi";
clocks = <&clk IMX8MM_CLK_DISP_ROOT>, <&clk IMX8MM_CLK_DISP_AXI_ROOT>,
<&clk IMX8MM_CLK_DISP_APB_ROOT>;
};

V4:
Add R-b tag
Typo fix
Update the power domain macro names Per Abel and Frieder

V3:
Add explaination for not listing items in patch 2 commit log Per Rob.
Addressed comments from Lucas and Frieder on patch [3,4].
A few comments from Jacky was ignored, because following gpcv2
coding style.

V2:
Fix yaml check failure.

Previously there is an effort from Abel that take BLK-CTL as clock
provider, but it turns out that there is A/B lock issue and we are
not able resolve that.

Per discuss with Lucas and Jacky, we made an agreement that take BLK-CTL
as a power domain provider and use GPC's domain as parent, the consumer
node take BLK-CTL as power domain input.

This patchset has been tested on i.MX8MM EVK board, but one hack
is not included in the patchset is that the DISPMIX BLK-CTL
MIPI_M/S_RESET not implemented. Per Lucas, we will finally have a MIPI
DPHY driver, so fine to leave it.

Thanks for Lucas's suggestion, Frieder Schrempf for collecting
all the patches, Abel's previous BLK-CTL work, Jacky Bai on help
debug issues.


Peng Fan (4):
dt-bindings: power: Add defines for i.MX8MM BLK-CTL power domains
Documentation: bindings: clk: Add bindings for i.MX BLK_CTL
soc: imx: Add generic blk-ctl driver
soc: imx: Add blk-ctl driver for i.MX8MM

.../bindings/soc/imx/fsl,imx-blk-ctl.yaml | 66 ++++
drivers/soc/imx/Makefile | 2 +-
drivers/soc/imx/blk-ctl-imx8mm.c | 139 ++++++++
drivers/soc/imx/blk-ctl.c | 311 ++++++++++++++++++
drivers/soc/imx/blk-ctl.h | 85 +++++
include/dt-bindings/power/imx8mm-power.h | 13 +
6 files changed, 615 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/soc/imx/fsl,imx-blk-ctl.yaml
create mode 100644 drivers/soc/imx/blk-ctl-imx8mm.c
create mode 100644 drivers/soc/imx/blk-ctl.c
create mode 100644 drivers/soc/imx/blk-ctl.h

--
2.30.0


2021-05-21 20:13:16

by Peng Fan (OSS)

[permalink] [raw]
Subject: [PATCH V5 4/4] soc: imx: Add blk-ctl driver for i.MX8MM

From: Peng Fan <[email protected]>

The i.MX8MM SoC has dispmix BLK-CTL and vpumix BLK-CTL, so we add
that support in this driver.

Reviewed-by: Abel Vesa <[email protected]>
Signed-off-by: Peng Fan <[email protected]>
---
drivers/soc/imx/Makefile | 2 +-
drivers/soc/imx/blk-ctl-imx8mm.c | 139 +++++++++++++++++++++++++++++++
2 files changed, 140 insertions(+), 1 deletion(-)
create mode 100644 drivers/soc/imx/blk-ctl-imx8mm.c

diff --git a/drivers/soc/imx/Makefile b/drivers/soc/imx/Makefile
index d3d2b49a386c..c260b962f495 100644
--- a/drivers/soc/imx/Makefile
+++ b/drivers/soc/imx/Makefile
@@ -4,4 +4,4 @@ obj-$(CONFIG_ARCH_MXC) += soc-imx.o
endif
obj-$(CONFIG_HAVE_IMX_GPC) += gpc.o
obj-$(CONFIG_IMX_GPCV2_PM_DOMAINS) += gpcv2.o
-obj-$(CONFIG_SOC_IMX8M) += soc-imx8m.o blk-ctl.o
+obj-$(CONFIG_SOC_IMX8M) += soc-imx8m.o blk-ctl.o blk-ctl-imx8mm.o
diff --git a/drivers/soc/imx/blk-ctl-imx8mm.c b/drivers/soc/imx/blk-ctl-imx8mm.c
new file mode 100644
index 000000000000..59443588f892
--- /dev/null
+++ b/drivers/soc/imx/blk-ctl-imx8mm.c
@@ -0,0 +1,139 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2021 NXP
+ */
+
+#include <dt-bindings/clock/imx8mm-clock.h>
+#include <dt-bindings/power/imx8mm-power.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/pm_domain.h>
+#include <linux/regmap.h>
+
+#include "blk-ctl.h"
+
+#define MEDIA_BLK_BUS_RSTN_BLK_SYNC_SFT_EN BIT(6)
+#define MEDIA_BLK_MIPI_DSI_I_PRESETN_SFT_EN BIT(5)
+#define MEDIA_BLK_MIPI_CSI_I_PRESETN_SFT_EN BIT(4)
+#define MEDIA_BLK_CAMERA_PIXEL_RESET_N_SFT_EN BIT(3)
+#define MEDIA_BLK_CSI_BRIDGE_SFT_EN GENMASK(2, 0)
+
+#define MEDIA_BLK_BUS_PD_MASK BIT(12)
+#define MEDIA_BLK_MIPI_CSI_PD_MASK GENMASK(11, 10)
+#define MEDIA_BLK_MIPI_DSI_PD_MASK GENMASK(9, 8)
+#define MEDIA_BLK_LCDIF_PD_MASK GENMASK(7, 6)
+#define MEDIA_BLK_CSI_BRIDGE_PD_MASK GENMASK(5, 0)
+
+static struct imx_blk_ctl_hw imx8mm_dispmix_blk_ctl_pds[] = {
+ IMX_BLK_CTL_PD("CSI_BRIDGE", NULL, IMX8MM_BLK_CTL_PD_DISPMIX_CSI_BRIDGE, 0x4,
+ MEDIA_BLK_CSI_BRIDGE_PD_MASK, 0, MEDIA_BLK_CSI_BRIDGE_SFT_EN,
+ IMX_BLK_CTL_PD_RESET),
+ IMX_BLK_CTL_PD("LCDIF", NULL, IMX8MM_BLK_CTL_PD_DISPMIX_LCDIF, 0x4,
+ MEDIA_BLK_LCDIF_PD_MASK, -1, -1, 0),
+ IMX_BLK_CTL_PD("MIPI_DSI", "mipi", IMX8MM_BLK_CTL_PD_DISPMIX_MIPI_DSI, 0x4,
+ MEDIA_BLK_MIPI_DSI_PD_MASK, 0, MEDIA_BLK_MIPI_DSI_I_PRESETN_SFT_EN,
+ IMX_BLK_CTL_PD_RESET),
+ IMX_BLK_CTL_PD("MIPI_CSI", "mipi", IMX8MM_BLK_CTL_PD_DISPMIX_MIPI_CSI, 0x4,
+ MEDIA_BLK_MIPI_CSI_PD_MASK, 0,
+ MEDIA_BLK_MIPI_CSI_I_PRESETN_SFT_EN | MEDIA_BLK_CAMERA_PIXEL_RESET_N_SFT_EN,
+ IMX_BLK_CTL_PD_RESET),
+ IMX_BLK_CTL_PD("DISPMIX_BUS", "dispmix", IMX8MM_BLK_CTL_PD_DISPMIX_BUS, 0x4,
+ MEDIA_BLK_BUS_PD_MASK, 0, MEDIA_BLK_BUS_RSTN_BLK_SYNC_SFT_EN,
+ IMX_BLK_CTL_PD_HANDSHAKE | IMX_BLK_CTL_PD_RESET)
+};
+
+static struct imx_blk_ctl_hw imx8mm_vpumix_blk_ctl_pds[] = {
+ IMX_BLK_CTL_PD("VPU_BLK_CTL_G2", "vpu-g2", IMX8MM_BLK_CTL_PD_VPU_G2, 0x4,
+ BIT(0), 0, BIT(0), IMX_BLK_CTL_PD_RESET),
+ IMX_BLK_CTL_PD("VPU_BLK_CTL_G1", "vpu-g1", IMX8MM_BLK_CTL_PD_VPU_G1, 0x4,
+ BIT(1), 0, BIT(1), IMX_BLK_CTL_PD_RESET),
+ IMX_BLK_CTL_PD("VPU_BLK_CTL_H1", "vpu-h1", IMX8MM_BLK_CTL_PD_VPU_H1, 0x4,
+ BIT(2), 0, BIT(2), IMX_BLK_CTL_PD_RESET),
+ IMX_BLK_CTL_PD("VPU_BLK_CTL_BUS", "vpumix", IMX8MM_BLK_CTL_PD_VPU_BUS, 0x4,
+ BIT(2), 0, BIT(2), IMX_BLK_CTL_PD_HANDSHAKE | IMX_BLK_CTL_PD_RESET)
+};
+
+static const struct regmap_config imx8mm_blk_ctl_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .max_register = 0x30,
+ .fast_io = true,
+};
+
+static const struct imx_blk_ctl_dev_data imx8mm_vpumix_blk_ctl_dev_data = {
+ .pds = imx8mm_vpumix_blk_ctl_pds,
+ .pds_num = ARRAY_SIZE(imx8mm_vpumix_blk_ctl_pds),
+ .max_num = IMX8MM_BLK_CTL_PD_VPU_MAX,
+ .hw_hsk = &imx8mm_vpumix_blk_ctl_pds[3],
+ .config = imx8mm_blk_ctl_regmap_config,
+ .name = "imx-vpumix-blk-ctl",
+};
+
+static const struct imx_blk_ctl_dev_data imx8mm_dispmix_blk_ctl_dev_data = {
+ .pds = imx8mm_dispmix_blk_ctl_pds,
+ .pds_num = ARRAY_SIZE(imx8mm_dispmix_blk_ctl_pds),
+ .max_num = IMX8MM_BLK_CTL_PD_DISPMIX_MAX,
+ .hw_hsk = &imx8mm_dispmix_blk_ctl_pds[4],
+ .config = imx8mm_blk_ctl_regmap_config,
+ .name = "imx-dispmix-blk-ctl",
+};
+
+static int imx8mm_blk_ctl_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ const struct imx_blk_ctl_dev_data *dev_data = of_device_get_match_data(dev);
+ struct regmap *regmap;
+ struct imx_blk_ctl *ctl;
+ void __iomem *base;
+
+ ctl = devm_kzalloc(dev, sizeof(*ctl), GFP_KERNEL);
+ if (!ctl)
+ return -ENOMEM;
+
+ base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+
+ regmap = devm_regmap_init_mmio(dev, base, &dev_data->config);
+ if (IS_ERR(regmap))
+ return PTR_ERR(regmap);
+
+ ctl->regmap = regmap;
+ ctl->dev = dev;
+ mutex_init(&ctl->lock);
+
+ ctl->num_clks = devm_clk_bulk_get_all(dev, &ctl->clks);
+ if (ctl->num_clks < 0)
+ return ctl->num_clks;
+
+ dev_set_drvdata(dev, ctl);
+ ctl->dev_data = dev_data;
+
+ return imx_blk_ctl_register(dev);
+}
+
+static const struct of_device_id imx_blk_ctl_of_match[] = {
+ { .compatible = "fsl,imx8mm-vpumix-blk-ctl", .data = &imx8mm_vpumix_blk_ctl_dev_data },
+ { .compatible = "fsl,imx8mm-dispmix-blk-ctl", .data = &imx8mm_dispmix_blk_ctl_dev_data },
+ { /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, imx_blk_ctl_of_match);
+
+static struct platform_driver imx_blk_ctl_driver = {
+ .probe = imx8mm_blk_ctl_probe,
+ .driver = {
+ .name = "imx8mm-blk-ctl",
+ .of_match_table = of_match_ptr(imx_blk_ctl_of_match),
+ .pm = &imx_blk_ctl_pm_ops,
+ },
+};
+module_platform_driver(imx_blk_ctl_driver);
--
2.30.0

2021-05-21 20:21:12

by Adam Ford

[permalink] [raw]
Subject: Re: [PATCH V5 0/4] soc: imx: add i.MX BLK-CTL support

On Fri, May 21, 2021 at 5:27 AM Peng Fan (OSS) <[email protected]> wrote:
>
> From: Peng Fan <[email protected]>
>
> V5:
> Rework the blk-ctl driver to let sub-PGC use blk-ctl as parent power
> domain to fix the potential handshake issue.
> I still keep R-b/A-b tag for Patch 1,2,4, since very minor changes
> I only drop R-b tag for Patch 3, since it has big change.
> An example, the pgc_mipi not take pgc_dispmix as parent:
>
> pgc_dispmix: power-domain@10 {
> #power-domain-cells = <0>;
> reg = <IMX8MM_POWER_DOMAIN_DISPMIX>;
> clocks = <&clk IMX8MM_CLK_DISP_ROOT>,
> <&clk IMX8MM_CLK_DISP_AXI_ROOT>,
> <&clk IMX8MM_CLK_DISP_APB_ROOT>;
> };
>
> pgc_mipi: power-domain@11 {
> #power-domain-cells = <0>;
> reg = <IMX8MM_POWER_DOMAIN_MIPI>;
> power-domains = <&dispmix_blk_ctl IMX8MM_BLK_CTL_PD_DISPMIX_BUS>;

With this change, I get a bunch of errors on boot. The list of
power-domains appear correct on the surface, but it also has trouble
waking from sleep.

[ 0.695947] imx8mm-blk-ctl imx-dispmix-blk-ctl.0: invalid resource
[ 0.702849] imx8mm-blk-ctl: probe of imx-dispmix-blk-ctl.0 failed
with error -22
[ 0.711259] imx8mm-blk-ctl imx-dispmix-blk-ctl.1: invalid resource
[ 0.716451] imx8mm-blk-ctl: probe of imx-dispmix-blk-ctl.1 failed
with error -22
[ 0.724856] imx8mm-blk-ctl imx-dispmix-blk-ctl.2: invalid resource
[ 0.730097] imx8mm-blk-ctl: probe of imx-dispmix-blk-ctl.2 failed
with error -22
[ 0.738398] imx8mm-blk-ctl imx-dispmix-blk-ctl.3: invalid resource
[ 0.743747] imx8mm-blk-ctl: probe of imx-dispmix-blk-ctl.3 failed
with error -22

If I have a wrong device tree configuration, can you please post an
updated device tree? I don't think an official patch for original
pgc's were pushed as part of either series. I used this e-mail as the
patch to enable the blk-ctl.

thanks,

adam

> };
>
> dispmix_blk_ctl: clock-controller@32e28000 {
> compatible = "fsl,imx8mm-dispmix-blk-ctl", "syscon";
> reg = <0x32e28000 0x100>;
> #power-domain-cells = <1>;
> power-domains = <&pgc_dispmix>, <&pgc_mipi>;
> power-domain-names = "dispmix", "mipi";
> clocks = <&clk IMX8MM_CLK_DISP_ROOT>, <&clk IMX8MM_CLK_DISP_AXI_ROOT>,
> <&clk IMX8MM_CLK_DISP_APB_ROOT>;
> };
>
> V4:
> Add R-b tag
> Typo fix
> Update the power domain macro names Per Abel and Frieder
>
> V3:
> Add explaination for not listing items in patch 2 commit log Per Rob.
> Addressed comments from Lucas and Frieder on patch [3,4].
> A few comments from Jacky was ignored, because following gpcv2
> coding style.
>
> V2:
> Fix yaml check failure.
>
> Previously there is an effort from Abel that take BLK-CTL as clock
> provider, but it turns out that there is A/B lock issue and we are
> not able resolve that.
>
> Per discuss with Lucas and Jacky, we made an agreement that take BLK-CTL
> as a power domain provider and use GPC's domain as parent, the consumer
> node take BLK-CTL as power domain input.
>
> This patchset has been tested on i.MX8MM EVK board, but one hack
> is not included in the patchset is that the DISPMIX BLK-CTL
> MIPI_M/S_RESET not implemented. Per Lucas, we will finally have a MIPI
> DPHY driver, so fine to leave it.
>
> Thanks for Lucas's suggestion, Frieder Schrempf for collecting
> all the patches, Abel's previous BLK-CTL work, Jacky Bai on help
> debug issues.
>
>
> Peng Fan (4):
> dt-bindings: power: Add defines for i.MX8MM BLK-CTL power domains
> Documentation: bindings: clk: Add bindings for i.MX BLK_CTL
> soc: imx: Add generic blk-ctl driver
> soc: imx: Add blk-ctl driver for i.MX8MM
>
> .../bindings/soc/imx/fsl,imx-blk-ctl.yaml | 66 ++++
> drivers/soc/imx/Makefile | 2 +-
> drivers/soc/imx/blk-ctl-imx8mm.c | 139 ++++++++
> drivers/soc/imx/blk-ctl.c | 311 ++++++++++++++++++
> drivers/soc/imx/blk-ctl.h | 85 +++++
> include/dt-bindings/power/imx8mm-power.h | 13 +
> 6 files changed, 615 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/devicetree/bindings/soc/imx/fsl,imx-blk-ctl.yaml
> create mode 100644 drivers/soc/imx/blk-ctl-imx8mm.c
> create mode 100644 drivers/soc/imx/blk-ctl.c
> create mode 100644 drivers/soc/imx/blk-ctl.h
>
> --
> 2.30.0
>

2021-05-22 00:56:14

by Peng Fan

[permalink] [raw]
Subject: RE: [PATCH V5 0/4] soc: imx: add i.MX BLK-CTL support

> Subject: Re: [PATCH V5 0/4] soc: imx: add i.MX BLK-CTL support
>
> On Fri, May 21, 2021 at 5:27 AM Peng Fan (OSS) <[email protected]>
> wrote:
> >
> > From: Peng Fan <[email protected]>
> >
> > V5:
> > Rework the blk-ctl driver to let sub-PGC use blk-ctl as parent power
> > domain to fix the potential handshake issue.
> > I still keep R-b/A-b tag for Patch 1,2,4, since very minor changes I
> > only drop R-b tag for Patch 3, since it has big change.
> > An example, the pgc_mipi not take pgc_dispmix as parent:
> >
> > pgc_dispmix: power-domain@10 {
> > #power-domain-cells = <0>;
> > reg = <IMX8MM_POWER_DOMAIN_DISPMIX>;
> > clocks = <&clk IMX8MM_CLK_DISP_ROOT>,
> > <&clk IMX8MM_CLK_DISP_AXI_ROOT>,
> > <&clk IMX8MM_CLK_DISP_APB_ROOT>;
> > };
> >
> > pgc_mipi: power-domain@11 {
> > #power-domain-cells = <0>;
> > reg = <IMX8MM_POWER_DOMAIN_MIPI>;
> > power-domains = <&dispmix_blk_ctl
> > IMX8MM_BLK_CTL_PD_DISPMIX_BUS>;
>
> With this change, I get a bunch of errors on boot. The list of power-domains
> appear correct on the surface, but it also has trouble waking from sleep.
>
> [ 0.695947] imx8mm-blk-ctl imx-dispmix-blk-ctl.0: invalid resource
> [ 0.702849] imx8mm-blk-ctl: probe of imx-dispmix-blk-ctl.0 failed
> with error -22
> [ 0.711259] imx8mm-blk-ctl imx-dispmix-blk-ctl.1: invalid resource
> [ 0.716451] imx8mm-blk-ctl: probe of imx-dispmix-blk-ctl.1 failed
> with error -22
> [ 0.724856] imx8mm-blk-ctl imx-dispmix-blk-ctl.2: invalid resource
> [ 0.730097] imx8mm-blk-ctl: probe of imx-dispmix-blk-ctl.2 failed
> with error -22
> [ 0.738398] imx8mm-blk-ctl imx-dispmix-blk-ctl.3: invalid resource
> [ 0.743747] imx8mm-blk-ctl: probe of imx-dispmix-blk-ctl.3 failed
> with error -22

It is just the imx8mm-blk-ctl driver matches with the new created
child device, because the child device points the of_node of the parent
device.
But this error will not affect functionality.
I'll resolve this issue and send out v6.
>
> If I have a wrong device tree configuration, can you please post an updated
> device tree? I don't think an official patch for original pgc's were pushed as
> part of either series. I used this e-mail as the patch to enable the blk-ctl.

Do you have an device tree, I could give a look.

Regards,
Peng.

>
> thanks,
>
> adam
>
> > };
> >
> > dispmix_blk_ctl: clock-controller@32e28000 {
> > compatible = "fsl,imx8mm-dispmix-blk-ctl", "syscon";
> > reg = <0x32e28000 0x100>;
> > #power-domain-cells = <1>;
> > power-domains = <&pgc_dispmix>, <&pgc_mipi>;
> > power-domain-names = "dispmix", "mipi";
> > clocks = <&clk IMX8MM_CLK_DISP_ROOT>, <&clk
> IMX8MM_CLK_DISP_AXI_ROOT>,
> > <&clk IMX8MM_CLK_DISP_APB_ROOT>;
> > };
> >
> > V4:
> > Add R-b tag
> > Typo fix
> > Update the power domain macro names Per Abel and Frieder
> >
> > V3:
> > Add explaination for not listing items in patch 2 commit log Per Rob.
> > Addressed comments from Lucas and Frieder on patch [3,4].
> > A few comments from Jacky was ignored, because following gpcv2
> > coding style.
> >
> > V2:
> > Fix yaml check failure.
> >
> > Previously there is an effort from Abel that take BLK-CTL as clock
> > provider, but it turns out that there is A/B lock issue and we are not
> > able resolve that.
> >
> > Per discuss with Lucas and Jacky, we made an agreement that take
> > BLK-CTL as a power domain provider and use GPC's domain as parent, the
> > consumer node take BLK-CTL as power domain input.
> >
> > This patchset has been tested on i.MX8MM EVK board, but one hack is
> > not included in the patchset is that the DISPMIX BLK-CTL
> > MIPI_M/S_RESET not implemented. Per Lucas, we will finally have a MIPI
> > DPHY driver, so fine to leave it.
> >
> > Thanks for Lucas's suggestion, Frieder Schrempf for collecting all the
> > patches, Abel's previous BLK-CTL work, Jacky Bai on help debug issues.
> >
> >
> > Peng Fan (4):
> > dt-bindings: power: Add defines for i.MX8MM BLK-CTL power domains
> > Documentation: bindings: clk: Add bindings for i.MX BLK_CTL
> > soc: imx: Add generic blk-ctl driver
> > soc: imx: Add blk-ctl driver for i.MX8MM
> >
> > .../bindings/soc/imx/fsl,imx-blk-ctl.yaml | 66 ++++
> > drivers/soc/imx/Makefile | 2 +-
> > drivers/soc/imx/blk-ctl-imx8mm.c | 139 ++++++++
> > drivers/soc/imx/blk-ctl.c | 311
> ++++++++++++++++++
> > drivers/soc/imx/blk-ctl.h | 85 +++++
> > include/dt-bindings/power/imx8mm-power.h | 13 +
> > 6 files changed, 615 insertions(+), 1 deletion(-) create mode 100644
> > Documentation/devicetree/bindings/soc/imx/fsl,imx-blk-ctl.yaml
> > create mode 100644 drivers/soc/imx/blk-ctl-imx8mm.c create mode
> > 100644 drivers/soc/imx/blk-ctl.c create mode 100644
> > drivers/soc/imx/blk-ctl.h
> >
> > --
> > 2.30.0
> >

2021-05-22 02:35:15

by Adam Ford

[permalink] [raw]
Subject: Re: [PATCH V5 0/4] soc: imx: add i.MX BLK-CTL support

On Fri, May 21, 2021 at 7:54 PM Peng Fan <[email protected]> wrote:
>
> > Subject: Re: [PATCH V5 0/4] soc: imx: add i.MX BLK-CTL support
> >
> > On Fri, May 21, 2021 at 5:27 AM Peng Fan (OSS) <[email protected]>
> > wrote:
> > >
> > > From: Peng Fan <[email protected]>
> > >
> > > V5:
> > > Rework the blk-ctl driver to let sub-PGC use blk-ctl as parent power
> > > domain to fix the potential handshake issue.
> > > I still keep R-b/A-b tag for Patch 1,2,4, since very minor changes I
> > > only drop R-b tag for Patch 3, since it has big change.
> > > An example, the pgc_mipi not take pgc_dispmix as parent:
> > >
> > > pgc_dispmix: power-domain@10 {
> > > #power-domain-cells = <0>;
> > > reg = <IMX8MM_POWER_DOMAIN_DISPMIX>;
> > > clocks = <&clk IMX8MM_CLK_DISP_ROOT>,
> > > <&clk IMX8MM_CLK_DISP_AXI_ROOT>,
> > > <&clk IMX8MM_CLK_DISP_APB_ROOT>;
> > > };
> > >
> > > pgc_mipi: power-domain@11 {
> > > #power-domain-cells = <0>;
> > > reg = <IMX8MM_POWER_DOMAIN_MIPI>;
> > > power-domains = <&dispmix_blk_ctl
> > > IMX8MM_BLK_CTL_PD_DISPMIX_BUS>;
> >
> > With this change, I get a bunch of errors on boot. The list of power-domains
> > appear correct on the surface, but it also has trouble waking from sleep.
> >
> > [ 0.695947] imx8mm-blk-ctl imx-dispmix-blk-ctl.0: invalid resource
> > [ 0.702849] imx8mm-blk-ctl: probe of imx-dispmix-blk-ctl.0 failed
> > with error -22
> > [ 0.711259] imx8mm-blk-ctl imx-dispmix-blk-ctl.1: invalid resource
> > [ 0.716451] imx8mm-blk-ctl: probe of imx-dispmix-blk-ctl.1 failed
> > with error -22
> > [ 0.724856] imx8mm-blk-ctl imx-dispmix-blk-ctl.2: invalid resource
> > [ 0.730097] imx8mm-blk-ctl: probe of imx-dispmix-blk-ctl.2 failed
> > with error -22
> > [ 0.738398] imx8mm-blk-ctl imx-dispmix-blk-ctl.3: invalid resource
> > [ 0.743747] imx8mm-blk-ctl: probe of imx-dispmix-blk-ctl.3 failed
> > with error -22
>
> It is just the imx8mm-blk-ctl driver matches with the new created
> child device, because the child device points the of_node of the parent
> device.
> But this error will not affect functionality.
> I'll resolve this issue and send out v6.
> >
> > If I have a wrong device tree configuration, can you please post an updated
> > device tree? I don't think an official patch for original pgc's were pushed as
> > part of either series. I used this e-mail as the patch to enable the blk-ctl.
>
> Do you have an device tree, I could give a look.

I have a git repo where I've been collecting the various power domain
patches. I have updated the imx8mn blk-ctl and device trees as well
in that same repo.

https://github.com/aford173/linux/blob/linux-5.13.y-aford/arch/arm64/boot/dts/freescale/imx8mm.dtsi

thanks for looking at this.

adam
>
> Regards,
> Peng.
>
> >
> > thanks,
> >
> > adam
> >
> > > };
> > >
> > > dispmix_blk_ctl: clock-controller@32e28000 {
> > > compatible = "fsl,imx8mm-dispmix-blk-ctl", "syscon";
> > > reg = <0x32e28000 0x100>;
> > > #power-domain-cells = <1>;
> > > power-domains = <&pgc_dispmix>, <&pgc_mipi>;
> > > power-domain-names = "dispmix", "mipi";
> > > clocks = <&clk IMX8MM_CLK_DISP_ROOT>, <&clk
> > IMX8MM_CLK_DISP_AXI_ROOT>,
> > > <&clk IMX8MM_CLK_DISP_APB_ROOT>;
> > > };
> > >
> > > V4:
> > > Add R-b tag
> > > Typo fix
> > > Update the power domain macro names Per Abel and Frieder
> > >
> > > V3:
> > > Add explaination for not listing items in patch 2 commit log Per Rob.
> > > Addressed comments from Lucas and Frieder on patch [3,4].
> > > A few comments from Jacky was ignored, because following gpcv2
> > > coding style.
> > >
> > > V2:
> > > Fix yaml check failure.
> > >
> > > Previously there is an effort from Abel that take BLK-CTL as clock
> > > provider, but it turns out that there is A/B lock issue and we are not
> > > able resolve that.
> > >
> > > Per discuss with Lucas and Jacky, we made an agreement that take
> > > BLK-CTL as a power domain provider and use GPC's domain as parent, the
> > > consumer node take BLK-CTL as power domain input.
> > >
> > > This patchset has been tested on i.MX8MM EVK board, but one hack is
> > > not included in the patchset is that the DISPMIX BLK-CTL
> > > MIPI_M/S_RESET not implemented. Per Lucas, we will finally have a MIPI
> > > DPHY driver, so fine to leave it.
> > >
> > > Thanks for Lucas's suggestion, Frieder Schrempf for collecting all the
> > > patches, Abel's previous BLK-CTL work, Jacky Bai on help debug issues.
> > >
> > >
> > > Peng Fan (4):
> > > dt-bindings: power: Add defines for i.MX8MM BLK-CTL power domains
> > > Documentation: bindings: clk: Add bindings for i.MX BLK_CTL
> > > soc: imx: Add generic blk-ctl driver
> > > soc: imx: Add blk-ctl driver for i.MX8MM
> > >
> > > .../bindings/soc/imx/fsl,imx-blk-ctl.yaml | 66 ++++
> > > drivers/soc/imx/Makefile | 2 +-
> > > drivers/soc/imx/blk-ctl-imx8mm.c | 139 ++++++++
> > > drivers/soc/imx/blk-ctl.c | 311
> > ++++++++++++++++++
> > > drivers/soc/imx/blk-ctl.h | 85 +++++
> > > include/dt-bindings/power/imx8mm-power.h | 13 +
> > > 6 files changed, 615 insertions(+), 1 deletion(-) create mode 100644
> > > Documentation/devicetree/bindings/soc/imx/fsl,imx-blk-ctl.yaml
> > > create mode 100644 drivers/soc/imx/blk-ctl-imx8mm.c create mode
> > > 100644 drivers/soc/imx/blk-ctl.c create mode 100644
> > > drivers/soc/imx/blk-ctl.h
> > >
> > > --
> > > 2.30.0
> > >

2021-05-22 12:32:15

by Peng Fan (OSS)

[permalink] [raw]
Subject: RE: [PATCH V5 0/4] soc: imx: add i.MX BLK-CTL support

> Subject: Re: [PATCH V5 0/4] soc: imx: add i.MX BLK-CTL support
>
> On Fri, May 21, 2021 at 7:54 PM Peng Fan <[email protected]> wrote:
> >
> > > Subject: Re: [PATCH V5 0/4] soc: imx: add i.MX BLK-CTL support
> > >
> > > On Fri, May 21, 2021 at 5:27 AM Peng Fan (OSS)
> > > <[email protected]>
> > > wrote:
> > > >
> > > > From: Peng Fan <[email protected]>
> > > >
> > > > V5:
> > > > Rework the blk-ctl driver to let sub-PGC use blk-ctl as parent
> > > > power domain to fix the potential handshake issue.
> > > > I still keep R-b/A-b tag for Patch 1,2,4, since very minor
> > > > changes I only drop R-b tag for Patch 3, since it has big change.
> > > > An example, the pgc_mipi not take pgc_dispmix as parent:
> > > >
> > > > pgc_dispmix: power-domain@10 {
> > > > #power-domain-cells = <0>;
> > > > reg = <IMX8MM_POWER_DOMAIN_DISPMIX>;
> > > > clocks = <&clk IMX8MM_CLK_DISP_ROOT>,
> > > > <&clk IMX8MM_CLK_DISP_AXI_ROOT>,
> > > > <&clk
> IMX8MM_CLK_DISP_APB_ROOT>;
> > > > };
> > > >
> > > > pgc_mipi: power-domain@11 {
> > > > #power-domain-cells = <0>;
> > > > reg = <IMX8MM_POWER_DOMAIN_MIPI>;
> > > > power-domains = <&dispmix_blk_ctl
> > > > IMX8MM_BLK_CTL_PD_DISPMIX_BUS>;
> > >
> > > With this change, I get a bunch of errors on boot. The list of
> > > power-domains appear correct on the surface, but it also has trouble
> waking from sleep.
> > >
> > > [ 0.695947] imx8mm-blk-ctl imx-dispmix-blk-ctl.0: invalid resource
> > > [ 0.702849] imx8mm-blk-ctl: probe of imx-dispmix-blk-ctl.0 failed
> > > with error -22
> > > [ 0.711259] imx8mm-blk-ctl imx-dispmix-blk-ctl.1: invalid resource
> > > [ 0.716451] imx8mm-blk-ctl: probe of imx-dispmix-blk-ctl.1 failed
> > > with error -22
> > > [ 0.724856] imx8mm-blk-ctl imx-dispmix-blk-ctl.2: invalid resource
> > > [ 0.730097] imx8mm-blk-ctl: probe of imx-dispmix-blk-ctl.2 failed
> > > with error -22
> > > [ 0.738398] imx8mm-blk-ctl imx-dispmix-blk-ctl.3: invalid resource
> > > [ 0.743747] imx8mm-blk-ctl: probe of imx-dispmix-blk-ctl.3 failed
> > > with error -22
> >
> > It is just the imx8mm-blk-ctl driver matches with the new created
> > child device, because the child device points the of_node of the
> > parent device.
> > But this error will not affect functionality.
> > I'll resolve this issue and send out v6.
> > >
> > > If I have a wrong device tree configuration, can you please post an
> > > updated device tree? I don't think an official patch for original
> > > pgc's were pushed as part of either series. I used this e-mail as the patch
> to enable the blk-ctl.
> >
> > Do you have an device tree, I could give a look.
>
> I have a git repo where I've been collecting the various power domain patches.
> I have updated the imx8mn blk-ctl and device trees as well in that same repo.

I put my dts diff here:
https://gist.github.com/MrVan/d73888d8273c43ea4a3b28fa668ca1d0
Not include vpu, lcdif, mipi, just gpc and blk-ctl.

Your dts has error, the vpu-g1/g2/h1 should use vpu-blk as parent power domain.
And the vpu-blk-ctl power domain changes as below:
power-domain-names = "vpumix", "vpu-g1", "vpu-g2", "vpu-h1";

Regards,
Peng.

>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.
> com%2Faford173%2Flinux%2Fblob%2Flinux-5.13.y-aford%2Farch%2Farm64%
> 2Fboot%2Fdts%2Ffreescale%2Fimx8mm.dtsi&amp;data=04%7C01%7Cpeng.f
> an%40nxp.com%7Ca5c04b8b13514290944008d91cc9d95f%7C686ea1d3bc2
> b4c6fa92cd99c5c301635%7C0%7C0%7C637572475532314749%7CUnknown
> %7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1ha
> WwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=NjxeojlrUc5t1tmrpZrhmYL1jaGB
> QNHv8QVh2OcgMpA%3D&amp;reserved=0
>
> thanks for looking at this.
>
> adam
> >
> > Regards,
> > Peng.
> >
> > >
> > > thanks,
> > >
> > > adam
> > >
> > > > };
> > > >
> > > > dispmix_blk_ctl: clock-controller@32e28000 {
> > > > compatible = "fsl,imx8mm-dispmix-blk-ctl",
> "syscon";
> > > > reg = <0x32e28000 0x100>;
> > > > #power-domain-cells = <1>;
> > > > power-domains = <&pgc_dispmix>, <&pgc_mipi>;
> > > > power-domain-names = "dispmix", "mipi";
> > > > clocks = <&clk IMX8MM_CLK_DISP_ROOT>, <&clk
> > > IMX8MM_CLK_DISP_AXI_ROOT>,
> > > > <&clk
> IMX8MM_CLK_DISP_APB_ROOT>;
> > > > };
> > > >
> > > > V4:
> > > > Add R-b tag
> > > > Typo fix
> > > > Update the power domain macro names Per Abel and Frieder
> > > >
> > > > V3:
> > > > Add explaination for not listing items in patch 2 commit log Per Rob.
> > > > Addressed comments from Lucas and Frieder on patch [3,4].
> > > > A few comments from Jacky was ignored, because following gpcv2
> > > > coding style.
> > > >
> > > > V2:
> > > > Fix yaml check failure.
> > > >
> > > > Previously there is an effort from Abel that take BLK-CTL as clock
> > > > provider, but it turns out that there is A/B lock issue and we are
> > > > not able resolve that.
> > > >
> > > > Per discuss with Lucas and Jacky, we made an agreement that take
> > > > BLK-CTL as a power domain provider and use GPC's domain as parent,
> > > > the consumer node take BLK-CTL as power domain input.
> > > >
> > > > This patchset has been tested on i.MX8MM EVK board, but one hack
> > > > is not included in the patchset is that the DISPMIX BLK-CTL
> > > > MIPI_M/S_RESET not implemented. Per Lucas, we will finally have a
> > > > MIPI DPHY driver, so fine to leave it.
> > > >
> > > > Thanks for Lucas's suggestion, Frieder Schrempf for collecting all
> > > > the patches, Abel's previous BLK-CTL work, Jacky Bai on help debug
> issues.
> > > >
> > > >
> > > > Peng Fan (4):
> > > > dt-bindings: power: Add defines for i.MX8MM BLK-CTL power
> domains
> > > > Documentation: bindings: clk: Add bindings for i.MX BLK_CTL
> > > > soc: imx: Add generic blk-ctl driver
> > > > soc: imx: Add blk-ctl driver for i.MX8MM
> > > >
> > > > .../bindings/soc/imx/fsl,imx-blk-ctl.yaml | 66 ++++
> > > > drivers/soc/imx/Makefile | 2 +-
> > > > drivers/soc/imx/blk-ctl-imx8mm.c | 139 ++++++++
> > > > drivers/soc/imx/blk-ctl.c | 311
> > > ++++++++++++++++++
> > > > drivers/soc/imx/blk-ctl.h | 85 +++++
> > > > include/dt-bindings/power/imx8mm-power.h | 13 +
> > > > 6 files changed, 615 insertions(+), 1 deletion(-) create mode
> > > > 100644
> > > > Documentation/devicetree/bindings/soc/imx/fsl,imx-blk-ctl.yaml
> > > > create mode 100644 drivers/soc/imx/blk-ctl-imx8mm.c create
> mode
> > > > 100644 drivers/soc/imx/blk-ctl.c create mode 100644
> > > > drivers/soc/imx/blk-ctl.h
> > > >
> > > > --
> > > > 2.30.0
> > > >