All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] arm64: dts: mediatek: mt8186: fix clock names for power domains
@ 2023-10-05 10:30 ` Eugen Hristev
  0 siblings, 0 replies; 10+ messages in thread
From: Eugen Hristev @ 2023-10-05 10:30 UTC (permalink / raw)
  To: matthias.bgg, angelogioacchino.delregno
  Cc: allen-kh.cheng, devicetree, linux-kernel, linux-arm-kernel,
	linux-mediatek, Eugen Hristev

Clocks for each power domain are split into big categories: pd clocks
and subsys clocks.
According to the binding, all clocks which have a dash '-' in their name
are treated as subsys clocks, and must be placed at the end of the list.
The other clocks which are pd clocks must come first.
Fixed the naming and the placing of all clocks in the power domains.
For the avoidance of doubt, prefixed all subsys clocks with the 'subsys'
prefix. The binding does not enforce strict clock names, the driver
uses them in bulk, only making a difference for pd clocks vs subsys clocks.

The above problem appears to be trivial, however, it leads to incorrect
power up and power down sequence of the power domains, because some
clocks will be mistakenly taken for subsys clocks and viceversa.
One consequence is the fact that if the DIS power domain goes power down
and power back up during the boot process, when it comes back up, there
are still transactions left on the bus which makes the display inoperable.

Some of the clocks for the DIS power domain were wrongly using '_' instead
of '-', which again made these clocks being treated as pd clocks instead of
subsys clocks.

Fixes: d9e43c1e7a38 ("arm64: dts: mt8186: Add power domains controller")
Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>
---
 arch/arm64/boot/dts/mediatek/mt8186.dtsi | 42 +++++++++++++++---------
 1 file changed, 27 insertions(+), 15 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8186.dtsi b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
index af6f6687de35..7121d4312bee 100644
--- a/arch/arm64/boot/dts/mediatek/mt8186.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
@@ -924,7 +924,8 @@ power-domain@MT8186_POWER_DOMAIN_CSIRX_TOP {
 					reg = <MT8186_POWER_DOMAIN_CSIRX_TOP>;
 					clocks = <&topckgen CLK_TOP_SENINF>,
 						 <&topckgen CLK_TOP_SENINF1>;
-					clock-names = "csirx_top0", "csirx_top1";
+					clock-names = "subsys-csirx-top0",
+						      "subsys-csirx-top1";
 					#power-domain-cells = <0>;
 				};
 
@@ -942,7 +943,8 @@ power-domain@MT8186_POWER_DOMAIN_ADSP_AO {
 					reg = <MT8186_POWER_DOMAIN_ADSP_AO>;
 					clocks = <&topckgen CLK_TOP_AUDIODSP>,
 						 <&topckgen CLK_TOP_ADSP_BUS>;
-					clock-names = "audioadsp", "adsp_bus";
+					clock-names = "audioadsp",
+						      "subsys-adsp-bus";
 					#address-cells = <1>;
 					#size-cells = <0>;
 					#power-domain-cells = <1>;
@@ -975,8 +977,11 @@ power-domain@MT8186_POWER_DOMAIN_DIS {
 						 <&mmsys CLK_MM_SMI_COMMON>,
 						 <&mmsys CLK_MM_SMI_GALS>,
 						 <&mmsys CLK_MM_SMI_IOMMU>;
-					clock-names = "disp", "mdp", "smi_infra", "smi_common",
-						     "smi_gals", "smi_iommu";
+					clock-names = "disp", "mdp",
+						      "subsys-smi-infra",
+						      "subsys-smi-common",
+						      "subsys-smi-gals",
+						      "subsys-smi-iommu";
 					mediatek,infracfg = <&infracfg_ao>;
 					#address-cells = <1>;
 					#size-cells = <0>;
@@ -993,15 +998,17 @@ power-domain@MT8186_POWER_DOMAIN_VDEC {
 
 					power-domain@MT8186_POWER_DOMAIN_CAM {
 						reg = <MT8186_POWER_DOMAIN_CAM>;
-						clocks = <&topckgen CLK_TOP_CAM>,
-							 <&topckgen CLK_TOP_SENINF>,
+						clocks = <&topckgen CLK_TOP_SENINF>,
 							 <&topckgen CLK_TOP_SENINF1>,
 							 <&topckgen CLK_TOP_SENINF2>,
 							 <&topckgen CLK_TOP_SENINF3>,
+							 <&camsys CLK_CAM2MM_GALS>,
 							 <&topckgen CLK_TOP_CAMTM>,
-							 <&camsys CLK_CAM2MM_GALS>;
-						clock-names = "cam-top", "cam0", "cam1", "cam2",
-							     "cam3", "cam-tm", "gals";
+							 <&topckgen CLK_TOP_CAM>;
+						clock-names = "cam0", "cam1", "cam2",
+							      "cam3", "gals",
+							      "subsys-cam-tm",
+							      "subsys-cam-top";
 						mediatek,infracfg = <&infracfg_ao>;
 						#address-cells = <1>;
 						#size-cells = <0>;
@@ -1020,9 +1027,9 @@ power-domain@MT8186_POWER_DOMAIN_CAM_RAWA {
 
 					power-domain@MT8186_POWER_DOMAIN_IMG {
 						reg = <MT8186_POWER_DOMAIN_IMG>;
-						clocks = <&topckgen CLK_TOP_IMG1>,
-							 <&imgsys1 CLK_IMG1_GALS_IMG1>;
-						clock-names = "img-top", "gals";
+						clocks = <&imgsys1 CLK_IMG1_GALS_IMG1>,
+							 <&topckgen CLK_TOP_IMG1>;
+						clock-names = "gals", "subsys-img-top";
 						mediatek,infracfg = <&infracfg_ao>;
 						#address-cells = <1>;
 						#size-cells = <0>;
@@ -1041,8 +1048,11 @@ power-domain@MT8186_POWER_DOMAIN_IPE {
 							 <&ipesys CLK_IPE_LARB20>,
 							 <&ipesys CLK_IPE_SMI_SUBCOM>,
 							 <&ipesys CLK_IPE_GALS_IPE>;
-						clock-names = "ipe-top", "ipe-larb0", "ipe-larb1",
-							      "ipe-smi", "ipe-gals";
+						clock-names = "subsys-ipe-top",
+							      "subsys-ipe-larb0",
+							      "subsys-ipe-larb1",
+							      "subsys-ipe-smi",
+							      "subsys-ipe-gals";
 						mediatek,infracfg = <&infracfg_ao>;
 						#power-domain-cells = <0>;
 					};
@@ -1061,7 +1071,9 @@ power-domain@MT8186_POWER_DOMAIN_WPE {
 						clocks = <&topckgen CLK_TOP_WPE>,
 							 <&wpesys CLK_WPE_SMI_LARB8_CK_EN>,
 							 <&wpesys CLK_WPE_SMI_LARB8_PCLK_EN>;
-						clock-names = "wpe0", "larb-ck", "larb-pclk";
+						clock-names = "wpe0",
+							      "subsys-larb-ck",
+							      "subsys-larb-pclk";
 						mediatek,infracfg = <&infracfg_ao>;
 						#power-domain-cells = <0>;
 					};
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH] arm64: dts: mediatek: mt8186: fix clock names for power domains
@ 2023-10-05 10:30 ` Eugen Hristev
  0 siblings, 0 replies; 10+ messages in thread
From: Eugen Hristev @ 2023-10-05 10:30 UTC (permalink / raw)
  To: matthias.bgg, angelogioacchino.delregno
  Cc: allen-kh.cheng, devicetree, linux-kernel, linux-arm-kernel,
	linux-mediatek, Eugen Hristev

Clocks for each power domain are split into big categories: pd clocks
and subsys clocks.
According to the binding, all clocks which have a dash '-' in their name
are treated as subsys clocks, and must be placed at the end of the list.
The other clocks which are pd clocks must come first.
Fixed the naming and the placing of all clocks in the power domains.
For the avoidance of doubt, prefixed all subsys clocks with the 'subsys'
prefix. The binding does not enforce strict clock names, the driver
uses them in bulk, only making a difference for pd clocks vs subsys clocks.

The above problem appears to be trivial, however, it leads to incorrect
power up and power down sequence of the power domains, because some
clocks will be mistakenly taken for subsys clocks and viceversa.
One consequence is the fact that if the DIS power domain goes power down
and power back up during the boot process, when it comes back up, there
are still transactions left on the bus which makes the display inoperable.

Some of the clocks for the DIS power domain were wrongly using '_' instead
of '-', which again made these clocks being treated as pd clocks instead of
subsys clocks.

Fixes: d9e43c1e7a38 ("arm64: dts: mt8186: Add power domains controller")
Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>
---
 arch/arm64/boot/dts/mediatek/mt8186.dtsi | 42 +++++++++++++++---------
 1 file changed, 27 insertions(+), 15 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8186.dtsi b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
index af6f6687de35..7121d4312bee 100644
--- a/arch/arm64/boot/dts/mediatek/mt8186.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
@@ -924,7 +924,8 @@ power-domain@MT8186_POWER_DOMAIN_CSIRX_TOP {
 					reg = <MT8186_POWER_DOMAIN_CSIRX_TOP>;
 					clocks = <&topckgen CLK_TOP_SENINF>,
 						 <&topckgen CLK_TOP_SENINF1>;
-					clock-names = "csirx_top0", "csirx_top1";
+					clock-names = "subsys-csirx-top0",
+						      "subsys-csirx-top1";
 					#power-domain-cells = <0>;
 				};
 
@@ -942,7 +943,8 @@ power-domain@MT8186_POWER_DOMAIN_ADSP_AO {
 					reg = <MT8186_POWER_DOMAIN_ADSP_AO>;
 					clocks = <&topckgen CLK_TOP_AUDIODSP>,
 						 <&topckgen CLK_TOP_ADSP_BUS>;
-					clock-names = "audioadsp", "adsp_bus";
+					clock-names = "audioadsp",
+						      "subsys-adsp-bus";
 					#address-cells = <1>;
 					#size-cells = <0>;
 					#power-domain-cells = <1>;
@@ -975,8 +977,11 @@ power-domain@MT8186_POWER_DOMAIN_DIS {
 						 <&mmsys CLK_MM_SMI_COMMON>,
 						 <&mmsys CLK_MM_SMI_GALS>,
 						 <&mmsys CLK_MM_SMI_IOMMU>;
-					clock-names = "disp", "mdp", "smi_infra", "smi_common",
-						     "smi_gals", "smi_iommu";
+					clock-names = "disp", "mdp",
+						      "subsys-smi-infra",
+						      "subsys-smi-common",
+						      "subsys-smi-gals",
+						      "subsys-smi-iommu";
 					mediatek,infracfg = <&infracfg_ao>;
 					#address-cells = <1>;
 					#size-cells = <0>;
@@ -993,15 +998,17 @@ power-domain@MT8186_POWER_DOMAIN_VDEC {
 
 					power-domain@MT8186_POWER_DOMAIN_CAM {
 						reg = <MT8186_POWER_DOMAIN_CAM>;
-						clocks = <&topckgen CLK_TOP_CAM>,
-							 <&topckgen CLK_TOP_SENINF>,
+						clocks = <&topckgen CLK_TOP_SENINF>,
 							 <&topckgen CLK_TOP_SENINF1>,
 							 <&topckgen CLK_TOP_SENINF2>,
 							 <&topckgen CLK_TOP_SENINF3>,
+							 <&camsys CLK_CAM2MM_GALS>,
 							 <&topckgen CLK_TOP_CAMTM>,
-							 <&camsys CLK_CAM2MM_GALS>;
-						clock-names = "cam-top", "cam0", "cam1", "cam2",
-							     "cam3", "cam-tm", "gals";
+							 <&topckgen CLK_TOP_CAM>;
+						clock-names = "cam0", "cam1", "cam2",
+							      "cam3", "gals",
+							      "subsys-cam-tm",
+							      "subsys-cam-top";
 						mediatek,infracfg = <&infracfg_ao>;
 						#address-cells = <1>;
 						#size-cells = <0>;
@@ -1020,9 +1027,9 @@ power-domain@MT8186_POWER_DOMAIN_CAM_RAWA {
 
 					power-domain@MT8186_POWER_DOMAIN_IMG {
 						reg = <MT8186_POWER_DOMAIN_IMG>;
-						clocks = <&topckgen CLK_TOP_IMG1>,
-							 <&imgsys1 CLK_IMG1_GALS_IMG1>;
-						clock-names = "img-top", "gals";
+						clocks = <&imgsys1 CLK_IMG1_GALS_IMG1>,
+							 <&topckgen CLK_TOP_IMG1>;
+						clock-names = "gals", "subsys-img-top";
 						mediatek,infracfg = <&infracfg_ao>;
 						#address-cells = <1>;
 						#size-cells = <0>;
@@ -1041,8 +1048,11 @@ power-domain@MT8186_POWER_DOMAIN_IPE {
 							 <&ipesys CLK_IPE_LARB20>,
 							 <&ipesys CLK_IPE_SMI_SUBCOM>,
 							 <&ipesys CLK_IPE_GALS_IPE>;
-						clock-names = "ipe-top", "ipe-larb0", "ipe-larb1",
-							      "ipe-smi", "ipe-gals";
+						clock-names = "subsys-ipe-top",
+							      "subsys-ipe-larb0",
+							      "subsys-ipe-larb1",
+							      "subsys-ipe-smi",
+							      "subsys-ipe-gals";
 						mediatek,infracfg = <&infracfg_ao>;
 						#power-domain-cells = <0>;
 					};
@@ -1061,7 +1071,9 @@ power-domain@MT8186_POWER_DOMAIN_WPE {
 						clocks = <&topckgen CLK_TOP_WPE>,
 							 <&wpesys CLK_WPE_SMI_LARB8_CK_EN>,
 							 <&wpesys CLK_WPE_SMI_LARB8_PCLK_EN>;
-						clock-names = "wpe0", "larb-ck", "larb-pclk";
+						clock-names = "wpe0",
+							      "subsys-larb-ck",
+							      "subsys-larb-pclk";
 						mediatek,infracfg = <&infracfg_ao>;
 						#power-domain-cells = <0>;
 					};
-- 
2.34.1


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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] arm64: dts: mediatek: mt8186: fix clock names for power domains
  2023-10-05 10:30 ` Eugen Hristev
@ 2023-10-05 10:42   ` Chen-Yu Tsai
  -1 siblings, 0 replies; 10+ messages in thread
From: Chen-Yu Tsai @ 2023-10-05 10:42 UTC (permalink / raw)
  To: Eugen Hristev
  Cc: matthias.bgg, angelogioacchino.delregno, allen-kh.cheng,
	devicetree, linux-kernel, linux-arm-kernel, linux-mediatek

On Thu, Oct 5, 2023 at 6:31 PM Eugen Hristev
<eugen.hristev@collabora.com> wrote:
>
> Clocks for each power domain are split into big categories: pd clocks
> and subsys clocks.
> According to the binding, all clocks which have a dash '-' in their name
> are treated as subsys clocks, and must be placed at the end of the list.
> The other clocks which are pd clocks must come first.
> Fixed the naming and the placing of all clocks in the power domains.
> For the avoidance of doubt, prefixed all subsys clocks with the 'subsys'
> prefix. The binding does not enforce strict clock names, the driver
> uses them in bulk, only making a difference for pd clocks vs subsys clocks.
>
> The above problem appears to be trivial, however, it leads to incorrect
> power up and power down sequence of the power domains, because some
> clocks will be mistakenly taken for subsys clocks and viceversa.
> One consequence is the fact that if the DIS power domain goes power down
> and power back up during the boot process, when it comes back up, there
> are still transactions left on the bus which makes the display inoperable.
>
> Some of the clocks for the DIS power domain were wrongly using '_' instead
> of '-', which again made these clocks being treated as pd clocks instead of
> subsys clocks.
>
> Fixes: d9e43c1e7a38 ("arm64: dts: mt8186: Add power domains controller")
> Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>

Tested-by: Chen-Yu Tsai <wenst@chromium.org>

This brings the display back to life on my MT8186 device. :D
Thank you for tracking down the issue!

> ---
>  arch/arm64/boot/dts/mediatek/mt8186.dtsi | 42 +++++++++++++++---------
>  1 file changed, 27 insertions(+), 15 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8186.dtsi b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
> index af6f6687de35..7121d4312bee 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8186.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
> @@ -924,7 +924,8 @@ power-domain@MT8186_POWER_DOMAIN_CSIRX_TOP {
>                                         reg = <MT8186_POWER_DOMAIN_CSIRX_TOP>;
>                                         clocks = <&topckgen CLK_TOP_SENINF>,
>                                                  <&topckgen CLK_TOP_SENINF1>;
> -                                       clock-names = "csirx_top0", "csirx_top1";
> +                                       clock-names = "subsys-csirx-top0",
> +                                                     "subsys-csirx-top1";
>                                         #power-domain-cells = <0>;
>                                 };
>
> @@ -942,7 +943,8 @@ power-domain@MT8186_POWER_DOMAIN_ADSP_AO {
>                                         reg = <MT8186_POWER_DOMAIN_ADSP_AO>;
>                                         clocks = <&topckgen CLK_TOP_AUDIODSP>,
>                                                  <&topckgen CLK_TOP_ADSP_BUS>;
> -                                       clock-names = "audioadsp", "adsp_bus";
> +                                       clock-names = "audioadsp",
> +                                                     "subsys-adsp-bus";
>                                         #address-cells = <1>;
>                                         #size-cells = <0>;
>                                         #power-domain-cells = <1>;
> @@ -975,8 +977,11 @@ power-domain@MT8186_POWER_DOMAIN_DIS {
>                                                  <&mmsys CLK_MM_SMI_COMMON>,
>                                                  <&mmsys CLK_MM_SMI_GALS>,
>                                                  <&mmsys CLK_MM_SMI_IOMMU>;
> -                                       clock-names = "disp", "mdp", "smi_infra", "smi_common",
> -                                                    "smi_gals", "smi_iommu";
> +                                       clock-names = "disp", "mdp",
> +                                                     "subsys-smi-infra",
> +                                                     "subsys-smi-common",
> +                                                     "subsys-smi-gals",
> +                                                     "subsys-smi-iommu";
>                                         mediatek,infracfg = <&infracfg_ao>;
>                                         #address-cells = <1>;
>                                         #size-cells = <0>;
> @@ -993,15 +998,17 @@ power-domain@MT8186_POWER_DOMAIN_VDEC {
>
>                                         power-domain@MT8186_POWER_DOMAIN_CAM {
>                                                 reg = <MT8186_POWER_DOMAIN_CAM>;
> -                                               clocks = <&topckgen CLK_TOP_CAM>,
> -                                                        <&topckgen CLK_TOP_SENINF>,
> +                                               clocks = <&topckgen CLK_TOP_SENINF>,
>                                                          <&topckgen CLK_TOP_SENINF1>,
>                                                          <&topckgen CLK_TOP_SENINF2>,
>                                                          <&topckgen CLK_TOP_SENINF3>,
> +                                                        <&camsys CLK_CAM2MM_GALS>,
>                                                          <&topckgen CLK_TOP_CAMTM>,
> -                                                        <&camsys CLK_CAM2MM_GALS>;
> -                                               clock-names = "cam-top", "cam0", "cam1", "cam2",
> -                                                            "cam3", "cam-tm", "gals";
> +                                                        <&topckgen CLK_TOP_CAM>;
> +                                               clock-names = "cam0", "cam1", "cam2",
> +                                                             "cam3", "gals",
> +                                                             "subsys-cam-tm",
> +                                                             "subsys-cam-top";
>                                                 mediatek,infracfg = <&infracfg_ao>;
>                                                 #address-cells = <1>;
>                                                 #size-cells = <0>;
> @@ -1020,9 +1027,9 @@ power-domain@MT8186_POWER_DOMAIN_CAM_RAWA {
>
>                                         power-domain@MT8186_POWER_DOMAIN_IMG {
>                                                 reg = <MT8186_POWER_DOMAIN_IMG>;
> -                                               clocks = <&topckgen CLK_TOP_IMG1>,
> -                                                        <&imgsys1 CLK_IMG1_GALS_IMG1>;
> -                                               clock-names = "img-top", "gals";
> +                                               clocks = <&imgsys1 CLK_IMG1_GALS_IMG1>,
> +                                                        <&topckgen CLK_TOP_IMG1>;
> +                                               clock-names = "gals", "subsys-img-top";
>                                                 mediatek,infracfg = <&infracfg_ao>;
>                                                 #address-cells = <1>;
>                                                 #size-cells = <0>;
> @@ -1041,8 +1048,11 @@ power-domain@MT8186_POWER_DOMAIN_IPE {
>                                                          <&ipesys CLK_IPE_LARB20>,
>                                                          <&ipesys CLK_IPE_SMI_SUBCOM>,
>                                                          <&ipesys CLK_IPE_GALS_IPE>;
> -                                               clock-names = "ipe-top", "ipe-larb0", "ipe-larb1",
> -                                                             "ipe-smi", "ipe-gals";
> +                                               clock-names = "subsys-ipe-top",
> +                                                             "subsys-ipe-larb0",
> +                                                             "subsys-ipe-larb1",
> +                                                             "subsys-ipe-smi",
> +                                                             "subsys-ipe-gals";
>                                                 mediatek,infracfg = <&infracfg_ao>;
>                                                 #power-domain-cells = <0>;
>                                         };
> @@ -1061,7 +1071,9 @@ power-domain@MT8186_POWER_DOMAIN_WPE {
>                                                 clocks = <&topckgen CLK_TOP_WPE>,
>                                                          <&wpesys CLK_WPE_SMI_LARB8_CK_EN>,
>                                                          <&wpesys CLK_WPE_SMI_LARB8_PCLK_EN>;
> -                                               clock-names = "wpe0", "larb-ck", "larb-pclk";
> +                                               clock-names = "wpe0",
> +                                                             "subsys-larb-ck",
> +                                                             "subsys-larb-pclk";
>                                                 mediatek,infracfg = <&infracfg_ao>;
>                                                 #power-domain-cells = <0>;
>                                         };
> --
> 2.34.1
>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] arm64: dts: mediatek: mt8186: fix clock names for power domains
@ 2023-10-05 10:42   ` Chen-Yu Tsai
  0 siblings, 0 replies; 10+ messages in thread
From: Chen-Yu Tsai @ 2023-10-05 10:42 UTC (permalink / raw)
  To: Eugen Hristev
  Cc: matthias.bgg, angelogioacchino.delregno, allen-kh.cheng,
	devicetree, linux-kernel, linux-arm-kernel, linux-mediatek

On Thu, Oct 5, 2023 at 6:31 PM Eugen Hristev
<eugen.hristev@collabora.com> wrote:
>
> Clocks for each power domain are split into big categories: pd clocks
> and subsys clocks.
> According to the binding, all clocks which have a dash '-' in their name
> are treated as subsys clocks, and must be placed at the end of the list.
> The other clocks which are pd clocks must come first.
> Fixed the naming and the placing of all clocks in the power domains.
> For the avoidance of doubt, prefixed all subsys clocks with the 'subsys'
> prefix. The binding does not enforce strict clock names, the driver
> uses them in bulk, only making a difference for pd clocks vs subsys clocks.
>
> The above problem appears to be trivial, however, it leads to incorrect
> power up and power down sequence of the power domains, because some
> clocks will be mistakenly taken for subsys clocks and viceversa.
> One consequence is the fact that if the DIS power domain goes power down
> and power back up during the boot process, when it comes back up, there
> are still transactions left on the bus which makes the display inoperable.
>
> Some of the clocks for the DIS power domain were wrongly using '_' instead
> of '-', which again made these clocks being treated as pd clocks instead of
> subsys clocks.
>
> Fixes: d9e43c1e7a38 ("arm64: dts: mt8186: Add power domains controller")
> Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>

Tested-by: Chen-Yu Tsai <wenst@chromium.org>

This brings the display back to life on my MT8186 device. :D
Thank you for tracking down the issue!

> ---
>  arch/arm64/boot/dts/mediatek/mt8186.dtsi | 42 +++++++++++++++---------
>  1 file changed, 27 insertions(+), 15 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8186.dtsi b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
> index af6f6687de35..7121d4312bee 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8186.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8186.dtsi
> @@ -924,7 +924,8 @@ power-domain@MT8186_POWER_DOMAIN_CSIRX_TOP {
>                                         reg = <MT8186_POWER_DOMAIN_CSIRX_TOP>;
>                                         clocks = <&topckgen CLK_TOP_SENINF>,
>                                                  <&topckgen CLK_TOP_SENINF1>;
> -                                       clock-names = "csirx_top0", "csirx_top1";
> +                                       clock-names = "subsys-csirx-top0",
> +                                                     "subsys-csirx-top1";
>                                         #power-domain-cells = <0>;
>                                 };
>
> @@ -942,7 +943,8 @@ power-domain@MT8186_POWER_DOMAIN_ADSP_AO {
>                                         reg = <MT8186_POWER_DOMAIN_ADSP_AO>;
>                                         clocks = <&topckgen CLK_TOP_AUDIODSP>,
>                                                  <&topckgen CLK_TOP_ADSP_BUS>;
> -                                       clock-names = "audioadsp", "adsp_bus";
> +                                       clock-names = "audioadsp",
> +                                                     "subsys-adsp-bus";
>                                         #address-cells = <1>;
>                                         #size-cells = <0>;
>                                         #power-domain-cells = <1>;
> @@ -975,8 +977,11 @@ power-domain@MT8186_POWER_DOMAIN_DIS {
>                                                  <&mmsys CLK_MM_SMI_COMMON>,
>                                                  <&mmsys CLK_MM_SMI_GALS>,
>                                                  <&mmsys CLK_MM_SMI_IOMMU>;
> -                                       clock-names = "disp", "mdp", "smi_infra", "smi_common",
> -                                                    "smi_gals", "smi_iommu";
> +                                       clock-names = "disp", "mdp",
> +                                                     "subsys-smi-infra",
> +                                                     "subsys-smi-common",
> +                                                     "subsys-smi-gals",
> +                                                     "subsys-smi-iommu";
>                                         mediatek,infracfg = <&infracfg_ao>;
>                                         #address-cells = <1>;
>                                         #size-cells = <0>;
> @@ -993,15 +998,17 @@ power-domain@MT8186_POWER_DOMAIN_VDEC {
>
>                                         power-domain@MT8186_POWER_DOMAIN_CAM {
>                                                 reg = <MT8186_POWER_DOMAIN_CAM>;
> -                                               clocks = <&topckgen CLK_TOP_CAM>,
> -                                                        <&topckgen CLK_TOP_SENINF>,
> +                                               clocks = <&topckgen CLK_TOP_SENINF>,
>                                                          <&topckgen CLK_TOP_SENINF1>,
>                                                          <&topckgen CLK_TOP_SENINF2>,
>                                                          <&topckgen CLK_TOP_SENINF3>,
> +                                                        <&camsys CLK_CAM2MM_GALS>,
>                                                          <&topckgen CLK_TOP_CAMTM>,
> -                                                        <&camsys CLK_CAM2MM_GALS>;
> -                                               clock-names = "cam-top", "cam0", "cam1", "cam2",
> -                                                            "cam3", "cam-tm", "gals";
> +                                                        <&topckgen CLK_TOP_CAM>;
> +                                               clock-names = "cam0", "cam1", "cam2",
> +                                                             "cam3", "gals",
> +                                                             "subsys-cam-tm",
> +                                                             "subsys-cam-top";
>                                                 mediatek,infracfg = <&infracfg_ao>;
>                                                 #address-cells = <1>;
>                                                 #size-cells = <0>;
> @@ -1020,9 +1027,9 @@ power-domain@MT8186_POWER_DOMAIN_CAM_RAWA {
>
>                                         power-domain@MT8186_POWER_DOMAIN_IMG {
>                                                 reg = <MT8186_POWER_DOMAIN_IMG>;
> -                                               clocks = <&topckgen CLK_TOP_IMG1>,
> -                                                        <&imgsys1 CLK_IMG1_GALS_IMG1>;
> -                                               clock-names = "img-top", "gals";
> +                                               clocks = <&imgsys1 CLK_IMG1_GALS_IMG1>,
> +                                                        <&topckgen CLK_TOP_IMG1>;
> +                                               clock-names = "gals", "subsys-img-top";
>                                                 mediatek,infracfg = <&infracfg_ao>;
>                                                 #address-cells = <1>;
>                                                 #size-cells = <0>;
> @@ -1041,8 +1048,11 @@ power-domain@MT8186_POWER_DOMAIN_IPE {
>                                                          <&ipesys CLK_IPE_LARB20>,
>                                                          <&ipesys CLK_IPE_SMI_SUBCOM>,
>                                                          <&ipesys CLK_IPE_GALS_IPE>;
> -                                               clock-names = "ipe-top", "ipe-larb0", "ipe-larb1",
> -                                                             "ipe-smi", "ipe-gals";
> +                                               clock-names = "subsys-ipe-top",
> +                                                             "subsys-ipe-larb0",
> +                                                             "subsys-ipe-larb1",
> +                                                             "subsys-ipe-smi",
> +                                                             "subsys-ipe-gals";
>                                                 mediatek,infracfg = <&infracfg_ao>;
>                                                 #power-domain-cells = <0>;
>                                         };
> @@ -1061,7 +1071,9 @@ power-domain@MT8186_POWER_DOMAIN_WPE {
>                                                 clocks = <&topckgen CLK_TOP_WPE>,
>                                                          <&wpesys CLK_WPE_SMI_LARB8_CK_EN>,
>                                                          <&wpesys CLK_WPE_SMI_LARB8_PCLK_EN>;
> -                                               clock-names = "wpe0", "larb-ck", "larb-pclk";
> +                                               clock-names = "wpe0",
> +                                                             "subsys-larb-ck",
> +                                                             "subsys-larb-pclk";
>                                                 mediatek,infracfg = <&infracfg_ao>;
>                                                 #power-domain-cells = <0>;
>                                         };
> --
> 2.34.1
>
>

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] arm64: dts: mediatek: mt8186: fix clock names for power domains
  2023-10-05 10:30 ` Eugen Hristev
@ 2023-10-05 12:41   ` AngeloGioacchino Del Regno
  -1 siblings, 0 replies; 10+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-10-05 12:41 UTC (permalink / raw)
  To: Eugen Hristev, matthias.bgg
  Cc: allen-kh.cheng, devicetree, linux-kernel, linux-arm-kernel,
	linux-mediatek

Il 05/10/23 12:30, Eugen Hristev ha scritto:
> Clocks for each power domain are split into big categories: pd clocks
> and subsys clocks.
> According to the binding, all clocks which have a dash '-' in their name
> are treated as subsys clocks, and must be placed at the end of the list.
> The other clocks which are pd clocks must come first.
> Fixed the naming and the placing of all clocks in the power domains.
> For the avoidance of doubt, prefixed all subsys clocks with the 'subsys'
> prefix. The binding does not enforce strict clock names, the driver
> uses them in bulk, only making a difference for pd clocks vs subsys clocks.
> 
> The above problem appears to be trivial, however, it leads to incorrect
> power up and power down sequence of the power domains, because some
> clocks will be mistakenly taken for subsys clocks and viceversa.
> One consequence is the fact that if the DIS power domain goes power down
> and power back up during the boot process, when it comes back up, there
> are still transactions left on the bus which makes the display inoperable.
> 
> Some of the clocks for the DIS power domain were wrongly using '_' instead
> of '-', which again made these clocks being treated as pd clocks instead of
> subsys clocks.
> 
> Fixes: d9e43c1e7a38 ("arm64: dts: mt8186: Add power domains controller")
> Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Tested-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] arm64: dts: mediatek: mt8186: fix clock names for power domains
@ 2023-10-05 12:41   ` AngeloGioacchino Del Regno
  0 siblings, 0 replies; 10+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-10-05 12:41 UTC (permalink / raw)
  To: Eugen Hristev, matthias.bgg
  Cc: allen-kh.cheng, devicetree, linux-kernel, linux-arm-kernel,
	linux-mediatek

Il 05/10/23 12:30, Eugen Hristev ha scritto:
> Clocks for each power domain are split into big categories: pd clocks
> and subsys clocks.
> According to the binding, all clocks which have a dash '-' in their name
> are treated as subsys clocks, and must be placed at the end of the list.
> The other clocks which are pd clocks must come first.
> Fixed the naming and the placing of all clocks in the power domains.
> For the avoidance of doubt, prefixed all subsys clocks with the 'subsys'
> prefix. The binding does not enforce strict clock names, the driver
> uses them in bulk, only making a difference for pd clocks vs subsys clocks.
> 
> The above problem appears to be trivial, however, it leads to incorrect
> power up and power down sequence of the power domains, because some
> clocks will be mistakenly taken for subsys clocks and viceversa.
> One consequence is the fact that if the DIS power domain goes power down
> and power back up during the boot process, when it comes back up, there
> are still transactions left on the bus which makes the display inoperable.
> 
> Some of the clocks for the DIS power domain were wrongly using '_' instead
> of '-', which again made these clocks being treated as pd clocks instead of
> subsys clocks.
> 
> Fixes: d9e43c1e7a38 ("arm64: dts: mt8186: Add power domains controller")
> Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Tested-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] arm64: dts: mediatek: mt8186: fix clock names for power domains
  2023-10-05 10:30 ` Eugen Hristev
@ 2023-10-10 12:31   ` Alexandre Mergnat
  -1 siblings, 0 replies; 10+ messages in thread
From: Alexandre Mergnat @ 2023-10-10 12:31 UTC (permalink / raw)
  To: Eugen Hristev, matthias.bgg, angelogioacchino.delregno
  Cc: allen-kh.cheng, devicetree, linux-kernel, linux-arm-kernel,
	linux-mediatek

Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>

On 05/10/2023 12:30, Eugen Hristev wrote:
> Clocks for each power domain are split into big categories: pd clocks 
> and subsys clocks. According to the binding, all clocks which have a 
> dash '-' in their name are treated as subsys clocks, and must be placed 
> at the end of the list. The other clocks which are pd clocks must come 
> first. Fixed the naming and the placing of all clocks in the power 
> domains. For the avoidance of doubt, prefixed all subsys clocks with the 
> 'subsys' prefix. The binding does not enforce strict clock names, the 
> driver uses them in bulk, only making a difference for pd clocks vs 
> subsys clocks. The above problem appears to be trivial, however, it 
> leads to incorrect power up and power down sequence of the power 
> domains, because some clocks will be mistakenly taken for subsys clocks 
> and viceversa. One consequence is the fact that if the DIS power domain 
> goes power down and power back up during the boot process, when it comes 
> back up, there are still transactions left on the bus which makes the 
> display inoperable. Some of the clocks for the DIS power domain were 
> wrongly using '_' instead of '-', which again made these clocks being 
> treated as pd clocks instead of subsys clocks.

-- 
Regards,
Alexandre

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] arm64: dts: mediatek: mt8186: fix clock names for power domains
@ 2023-10-10 12:31   ` Alexandre Mergnat
  0 siblings, 0 replies; 10+ messages in thread
From: Alexandre Mergnat @ 2023-10-10 12:31 UTC (permalink / raw)
  To: Eugen Hristev, matthias.bgg, angelogioacchino.delregno
  Cc: allen-kh.cheng, devicetree, linux-kernel, linux-arm-kernel,
	linux-mediatek

Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>

On 05/10/2023 12:30, Eugen Hristev wrote:
> Clocks for each power domain are split into big categories: pd clocks 
> and subsys clocks. According to the binding, all clocks which have a 
> dash '-' in their name are treated as subsys clocks, and must be placed 
> at the end of the list. The other clocks which are pd clocks must come 
> first. Fixed the naming and the placing of all clocks in the power 
> domains. For the avoidance of doubt, prefixed all subsys clocks with the 
> 'subsys' prefix. The binding does not enforce strict clock names, the 
> driver uses them in bulk, only making a difference for pd clocks vs 
> subsys clocks. The above problem appears to be trivial, however, it 
> leads to incorrect power up and power down sequence of the power 
> domains, because some clocks will be mistakenly taken for subsys clocks 
> and viceversa. One consequence is the fact that if the DIS power domain 
> goes power down and power back up during the boot process, when it comes 
> back up, there are still transactions left on the bus which makes the 
> display inoperable. Some of the clocks for the DIS power domain were 
> wrongly using '_' instead of '-', which again made these clocks being 
> treated as pd clocks instead of subsys clocks.

-- 
Regards,
Alexandre

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] arm64: dts: mediatek: mt8186: fix clock names for power domains
  2023-10-05 10:30 ` Eugen Hristev
@ 2023-11-29 13:27   ` AngeloGioacchino Del Regno
  -1 siblings, 0 replies; 10+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-11-29 13:27 UTC (permalink / raw)
  To: linux-mediatek, Eugen Hristev, matthias.bgg
  Cc: AngeloGioacchino Del Regno, krzysztof.kozlowski+dt,
	linux-arm-kernel, linux-kernel, devicetree, kernel,
	allen-kh.cheng


On Thu, 05 Oct 2023 13:30:41 +0300, Eugen Hristev wrote:
> Clocks for each power domain are split into big categories: pd clocks
> and subsys clocks.
> According to the binding, all clocks which have a dash '-' in their name
> are treated as subsys clocks, and must be placed at the end of the list.
> The other clocks which are pd clocks must come first.
> Fixed the naming and the placing of all clocks in the power domains.
> For the avoidance of doubt, prefixed all subsys clocks with the 'subsys'
> prefix. The binding does not enforce strict clock names, the driver
> uses them in bulk, only making a difference for pd clocks vs subsys clocks.
> 
> [...]

Applied, thanks!

[1/1] arm64: dts: mediatek: mt8186: fix clock names for power domains
      (no commit info)

Best regards,
-- 
AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] arm64: dts: mediatek: mt8186: fix clock names for power domains
@ 2023-11-29 13:27   ` AngeloGioacchino Del Regno
  0 siblings, 0 replies; 10+ messages in thread
From: AngeloGioacchino Del Regno @ 2023-11-29 13:27 UTC (permalink / raw)
  To: linux-mediatek, Eugen Hristev, matthias.bgg
  Cc: AngeloGioacchino Del Regno, krzysztof.kozlowski+dt,
	linux-arm-kernel, linux-kernel, devicetree, kernel,
	allen-kh.cheng


On Thu, 05 Oct 2023 13:30:41 +0300, Eugen Hristev wrote:
> Clocks for each power domain are split into big categories: pd clocks
> and subsys clocks.
> According to the binding, all clocks which have a dash '-' in their name
> are treated as subsys clocks, and must be placed at the end of the list.
> The other clocks which are pd clocks must come first.
> Fixed the naming and the placing of all clocks in the power domains.
> For the avoidance of doubt, prefixed all subsys clocks with the 'subsys'
> prefix. The binding does not enforce strict clock names, the driver
> uses them in bulk, only making a difference for pd clocks vs subsys clocks.
> 
> [...]

Applied, thanks!

[1/1] arm64: dts: mediatek: mt8186: fix clock names for power domains
      (no commit info)

Best regards,
-- 
AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-11-29 13:29 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-05 10:30 [PATCH] arm64: dts: mediatek: mt8186: fix clock names for power domains Eugen Hristev
2023-10-05 10:30 ` Eugen Hristev
2023-10-05 10:42 ` Chen-Yu Tsai
2023-10-05 10:42   ` Chen-Yu Tsai
2023-10-05 12:41 ` AngeloGioacchino Del Regno
2023-10-05 12:41   ` AngeloGioacchino Del Regno
2023-10-10 12:31 ` Alexandre Mergnat
2023-10-10 12:31   ` Alexandre Mergnat
2023-11-29 13:27 ` AngeloGioacchino Del Regno
2023-11-29 13:27   ` AngeloGioacchino Del Regno

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.