diff mbox series

[v3] arm64: dts: mediatek: Add mt8192 power domains controller

Message ID 1605782884-19741-1-git-send-email-weiyi.lu@mediatek.com (mailing list archive)
State New
Headers show
Series [v3] arm64: dts: mediatek: Add mt8192 power domains controller | expand

Commit Message

Weiyi Lu Nov. 19, 2020, 10:48 a.m. UTC
Add power domains controller node for SoC mt8192

Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
---

Change in v3: None, just rebase dts onto v5.10-rc1 and
       V4 of series "Add new driver for SCPSYS power domains controller"[1]

[1] https://patchwork.kernel.org/project/linux-mediatek/list/?series=374013

 arch/arm64/boot/dts/mediatek/mt8192.dtsi | 201 +++++++++++++++++++++++++++++++
 1 file changed, 201 insertions(+)

Comments

Enric Balletbo Serra Nov. 19, 2020, 12:13 p.m. UTC | #1
Hi Weiyi,

Thank you for the patch

Missatge de Weiyi Lu <weiyi.lu@mediatek.com> del dia dj., 19 de nov.
2020 a les 11:48:
>
> Add power domains controller node for SoC mt8192
>
> Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
> ---
>
> Change in v3: None, just rebase dts onto v5.10-rc1 and
>        V4 of series "Add new driver for SCPSYS power domains controller"[1]
>
> [1] https://patchwork.kernel.org/project/linux-mediatek/list/?series=374013
>
>  arch/arm64/boot/dts/mediatek/mt8192.dtsi | 201 +++++++++++++++++++++++++++++++
>  1 file changed, 201 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
> index 69d45c7..08449eb 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
> @@ -9,6 +9,7 @@
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>  #include <dt-bindings/interrupt-controller/irq.h>
>  #include <dt-bindings/pinctrl/mt8192-pinfunc.h>
> +#include <dt-bindings/power/mt8192-power.h>
>
>  / {
>         compatible = "mediatek,mt8192";
> @@ -257,6 +258,206 @@
>                         #interrupt-cells = <2>;
>                 };
>
> +               scpsys: syscon@10006000 {
> +                       compatible = "syscon", "simple-mfd";
> +                       reg = <0 0x10006000 0 0x1000>;
> +                       #power-domain-cells = <1>;
> +
> +                       /* System Power Manager */
> +                       spm: power-controller {
> +                               compatible = "mediatek,mt8192-power-controller";
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;
> +                               #power-domain-cells = <1>;
> +
> +                               /* power domain of the SoC */
> +                               audio@MT8192_POWER_DOMAIN_AUDIO {

If you run the dt_bindings_check it should return some errors, as all
these node names should be 'power-domain@'. Which is a bit annoying
because then you will get a bunch of errors like this:

[    1.969110] debugfs: Directory 'power-domain' with parent
'pm_genpd' already present!
[    1.976997] debugfs: Directory 'power-domain' with parent
'pm_genpd' already present!
[    1.984828] debugfs: Directory 'power-domain' with parent
'pm_genpd' already present!
[    1.992657] debugfs: Directory 'power-domain' with parent
'pm_genpd' already present!
[    2.000685] debugfs: Directory 'power-domain' with parent
'pm_genpd' already present!
[    2.008566] debugfs: Directory 'power-domain' with parent
'pm_genpd' already present!
[    2.016395] debugfs: Directory 'power-domain' with parent
'pm_genpd' already present!
[    2.024221] debugfs: Directory 'power-domain' with parent
'pm_genpd' already present!
[    2.032049] debugfs: Directory 'power-domain' with parent
'pm_genpd' already present!
[    2.039874] debugfs: Directory 'power-domain' with parent
'pm_genpd' already present!
[    2.047699] debugfs: Directory 'power-domain' with parent
'pm_genpd' already present!
[    2.055524] debugfs: Directory 'power-domain' with parent
'pm_genpd' already present!
[    2.063352] debugfs: Directory 'power-domain' with parent
'pm_genpd' already present!
[    2.071176] debugfs: Directory 'power-domain' with parent
'pm_genpd' already present!

But that's another problem that should be handled in debugfs system.

> +                                       reg = <MT8192_POWER_DOMAIN_AUDIO>;
> +                                       clocks = <&topckgen CLK_TOP_AUD_INTBUS_SEL>,
> +                                                <&infracfg CLK_INFRA_AUDIO_26M_B>,
> +                                                <&infracfg CLK_INFRA_AUDIO>;
> +                                       clock-names = "audio", "audio1", "audio2";
> +                                       mediatek,infracfg = <&infracfg>;
> +                                       #power-domain-cells = <0>;
> +                               };
> +
> +                               conn@MT8192_POWER_DOMAIN_CONN {
> +                                       reg = <MT8192_POWER_DOMAIN_CONN>;
> +                                       clocks = <&infracfg CLK_INFRA_PMIC_CONN>;
> +                                       clock-names = "conn";
> +                                       mediatek,infracfg = <&infracfg>;
> +                                       #power-domain-cells = <0>;
> +                               };
> +
> +                               mfg@MT8192_POWER_DOMAIN_MFG0 {
> +                                       reg = <MT8192_POWER_DOMAIN_MFG0>;
> +                                       clocks = <&topckgen CLK_TOP_MFG_PLL_SEL>;
> +                                       clock-names = "mfg";
> +                                       #address-cells = <1>;
> +                                       #size-cells = <0>;
> +                                       #power-domain-cells = <1>;
> +
> +                                       mfg1@MT8192_POWER_DOMAIN_MFG1 {
> +                                               reg = <MT8192_POWER_DOMAIN_MFG1>;
> +                                               mediatek,infracfg = <&infracfg>;
> +                                               #address-cells = <1>;
> +                                               #size-cells = <0>;
> +                                               #power-domain-cells = <1>;
> +
> +                                               mfg2@MT8192_POWER_DOMAIN_MFG2 {
> +                                                       reg = <MT8192_POWER_DOMAIN_MFG2>;
> +                                                       #power-domain-cells = <0>;
> +                                               };
> +
> +                                               mfg3@MT8192_POWER_DOMAIN_MFG3 {
> +                                                       reg = <MT8192_POWER_DOMAIN_MFG3>;
> +                                                       #power-domain-cells = <0>;
> +                                               };
> +
> +                                               mfg4@MT8192_POWER_DOMAIN_MFG4 {
> +                                                       reg = <MT8192_POWER_DOMAIN_MFG4>;
> +                                                       #power-domain-cells = <0>;
> +                                               };
> +
> +                                               mfg5@MT8192_POWER_DOMAIN_MFG5 {
> +                                                       reg = <MT8192_POWER_DOMAIN_MFG5>;
> +                                                       #power-domain-cells = <0>;
> +                                               };
> +
> +                                               mfg6@MT8192_POWER_DOMAIN_MFG6 {
> +                                                       reg = <MT8192_POWER_DOMAIN_MFG6>;
> +                                                       #power-domain-cells = <0>;
> +                                               };
> +                                       };
> +                               };
> +
> +                               disp@MT8192_POWER_DOMAIN_DISP {
> +                                       reg = <MT8192_POWER_DOMAIN_DISP>;
> +                                       clocks = <&topckgen CLK_TOP_DISP_SEL>,
> +                                                <&mmsys CLK_MM_SMI_INFRA>,
> +                                                <&mmsys CLK_MM_SMI_COMMON>,
> +                                                <&mmsys CLK_MM_SMI_GALS>,
> +                                                <&mmsys CLK_MM_SMI_IOMMU>;
> +                                       clock-names = "disp", "disp-0", "disp-1", "disp-2",
> +                                                     "disp-3";
> +                                       mediatek,infracfg = <&infracfg>;
> +                                       #address-cells = <1>;
> +                                       #size-cells = <0>;
> +                                       #power-domain-cells = <1>;
> +
> +                                       ipe@MT8192_POWER_DOMAIN_IPE {
> +                                               reg = <MT8192_POWER_DOMAIN_IPE>;
> +                                               clocks = <&topckgen CLK_TOP_IPE_SEL>,
> +                                                        <&ipesys CLK_IPE_LARB19>,
> +                                                        <&ipesys CLK_IPE_LARB20>,
> +                                                        <&ipesys CLK_IPE_SMI_SUBCOM>,
> +                                                        <&ipesys CLK_IPE_GALS>;
> +                                               clock-names = "ipe", "ipe-0", "ipe-1", "ipe-2",
> +                                                             "ipe-3";
> +                                               mediatek,infracfg = <&infracfg>;
> +                                               #power-domain-cells = <0>;
> +                                       };
> +
> +                                       isp@MT8192_POWER_DOMAIN_ISP {
> +                                               reg = <MT8192_POWER_DOMAIN_ISP>;
> +                                               clocks = <&topckgen CLK_TOP_IMG1_SEL>,
> +                                                        <&imgsys CLK_IMG_LARB9>,
> +                                                        <&imgsys CLK_IMG_GALS>;
> +                                               clock-names = "isp", "isp-0", "isp-1";
> +                                               mediatek,infracfg = <&infracfg>;
> +                                               #power-domain-cells = <0>;
> +                                       };
> +
> +                                       isp2@MT8192_POWER_DOMAIN_ISP2 {
> +                                               reg = <MT8192_POWER_DOMAIN_ISP2>;
> +                                               clocks = <&topckgen CLK_TOP_IMG2_SEL>,
> +                                                        <&imgsys2 CLK_IMG2_LARB11>,
> +                                                        <&imgsys2 CLK_IMG2_GALS>;
> +                                               clock-names = "isp2", "isp2-0", "isp2-1";
> +                                               mediatek,infracfg = <&infracfg>;
> +                                               #power-domain-cells = <0>;
> +                                       };
> +
> +                                       mdp@MT8192_POWER_DOMAIN_MDP {
> +                                               reg = <MT8192_POWER_DOMAIN_MDP>;
> +                                               clocks = <&topckgen CLK_TOP_MDP_SEL>,
> +                                                        <&mdpsys CLK_MDP_SMI0>;
> +                                               clock-names = "mdp", "mdp-0";
> +                                               mediatek,infracfg = <&infracfg>;
> +                                               #power-domain-cells = <0>;
> +                                       };
> +
> +                                       venc@MT8192_POWER_DOMAIN_VENC {
> +                                               reg = <MT8192_POWER_DOMAIN_VENC>;
> +                                               clocks = <&topckgen CLK_TOP_VENC_SEL>,
> +                                                        <&vencsys CLK_VENC_SET1_VENC>;
> +                                               clock-names = "venc", "venc-0";
> +                                               mediatek,infracfg = <&infracfg>;
> +                                               #power-domain-cells = <0>;
> +                                       };
> +
> +                                       vdec@MT8192_POWER_DOMAIN_VDEC {
> +                                               reg = <MT8192_POWER_DOMAIN_VDEC>;
> +                                               clocks = <&topckgen CLK_TOP_VDEC_SEL>,
> +                                                        <&vdecsys_soc CLK_VDEC_SOC_VDEC>,
> +                                                        <&vdecsys_soc CLK_VDEC_SOC_LAT>,
> +                                                        <&vdecsys_soc CLK_VDEC_SOC_LARB1>;
> +                                               clock-names = "vdec", "vdec-0", "vdec-1", "vdec-2";
> +                                               mediatek,infracfg = <&infracfg>;
> +                                               #address-cells = <1>;
> +                                               #size-cells = <0>;
> +                                               #power-domain-cells = <1>;
> +
> +                                               vdec2@MT8192_POWER_DOMAIN_VDEC2 {
> +                                                       reg = <MT8192_POWER_DOMAIN_VDEC2>;
> +                                                       clocks = <&vdecsys CLK_VDEC_VDEC>,
> +                                                                <&vdecsys CLK_VDEC_LAT>,
> +                                                                <&vdecsys CLK_VDEC_LARB1>;
> +                                                       clock-names = "vdec2-0", "vdec2-1",
> +                                                                     "vdec2-2";
> +                                                       #power-domain-cells = <0>;
> +                                               };
> +                                       };
> +
> +                                       cam@MT8192_POWER_DOMAIN_CAM {
> +                                               reg = <MT8192_POWER_DOMAIN_CAM>;
> +                                               clocks = <&topckgen CLK_TOP_CAM_SEL>,
> +                                                        <&camsys CLK_CAM_LARB13>,
> +                                                        <&camsys CLK_CAM_LARB14>,
> +                                                        <&camsys CLK_CAM_CCU_GALS>,
> +                                                        <&camsys CLK_CAM_CAM2MM_GALS>;
> +                                               clock-names = "cam", "cam-0", "cam-1", "cam-2",
> +                                                             "cam-3";
> +                                               mediatek,infracfg = <&infracfg>;
> +                                               #address-cells = <1>;
> +                                               #size-cells = <0>;
> +                                               #power-domain-cells = <1>;
> +
> +                                               cam_rawa@MT8192_POWER_DOMAIN_CAM_RAWA {
> +                                                       reg = <MT8192_POWER_DOMAIN_CAM_RAWA>;
> +                                                       clocks = <&camsys_rawa CLK_CAM_RAWA_LARBX>;
> +                                                       clock-names = "cam_rawa-0";
> +                                                       #power-domain-cells = <0>;
> +                                               };
> +
> +                                               cam_rawb@MT8192_POWER_DOMAIN_CAM_RAWB {
> +                                                       reg = <MT8192_POWER_DOMAIN_CAM_RAWB>;
> +                                                       clocks = <&camsys_rawb CLK_CAM_RAWB_LARBX>;
> +                                                       clock-names = "cam_rawb-0";
> +                                                       #power-domain-cells = <0>;
> +                                               };
> +
> +                                               cam_rawc@MT8192_POWER_DOMAIN_CAM_RAWC {
> +                                                       reg = <MT8192_POWER_DOMAIN_CAM_RAWC>;
> +                                                       clocks = <&camsys_rawc CLK_CAM_RAWC_LARBX>;
> +                                                       clock-names = "cam_rawc-0";
> +                                                       #power-domain-cells = <0>;
> +                                               };
> +                                       };
> +                               };
> +                       };
> +               };
> +
>                 apmixedsys: syscon@1000c000 {
>                         compatible = "mediatek,mt8192-apmixedsys", "syscon";
>                         reg = <0 0x1000c000 0 0x1000>;
> --
> 1.8.1.1.dirty
>
Weiyi Lu Nov. 19, 2020, 1:10 p.m. UTC | #2
On Thu, 2020-11-19 at 13:13 +0100, Enric Balletbo Serra wrote:
> Hi Weiyi,
> 
> Thank you for the patch
> 
> Missatge de Weiyi Lu <weiyi.lu@mediatek.com> del dia dj., 19 de nov.
> 2020 a les 11:48:
> >
> > Add power domains controller node for SoC mt8192
> >
> > Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
> > ---
> >
> > Change in v3: None, just rebase dts onto v5.10-rc1 and
> >        V4 of series "Add new driver for SCPSYS power domains controller"[1]
> >
> > [1] https://patchwork.kernel.org/project/linux-mediatek/list/?series=374013
> >
> >  arch/arm64/boot/dts/mediatek/mt8192.dtsi | 201 +++++++++++++++++++++++++++++++
> >  1 file changed, 201 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
> > index 69d45c7..08449eb 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
> > @@ -9,6 +9,7 @@
> >  #include <dt-bindings/interrupt-controller/arm-gic.h>
> >  #include <dt-bindings/interrupt-controller/irq.h>
> >  #include <dt-bindings/pinctrl/mt8192-pinfunc.h>
> > +#include <dt-bindings/power/mt8192-power.h>
> >
> >  / {
> >         compatible = "mediatek,mt8192";
> > @@ -257,6 +258,206 @@
> >                         #interrupt-cells = <2>;
> >                 };
> >
> > +               scpsys: syscon@10006000 {
> > +                       compatible = "syscon", "simple-mfd";
> > +                       reg = <0 0x10006000 0 0x1000>;
> > +                       #power-domain-cells = <1>;
> > +
> > +                       /* System Power Manager */
> > +                       spm: power-controller {
> > +                               compatible = "mediatek,mt8192-power-controller";
> > +                               #address-cells = <1>;
> > +                               #size-cells = <0>;
> > +                               #power-domain-cells = <1>;
> > +
> > +                               /* power domain of the SoC */
> > +                               audio@MT8192_POWER_DOMAIN_AUDIO {
> 
> If you run the dt_bindings_check it should return some errors, as all
> these node names should be 'power-domain@'. Which is a bit annoying
> because then you will get a bunch of errors like this:
> 
> [    1.969110] debugfs: Directory 'power-domain' with parent
> 'pm_genpd' already present!
> [    1.976997] debugfs: Directory 'power-domain' with parent
> 'pm_genpd' already present!
> [    1.984828] debugfs: Directory 'power-domain' with parent
> 'pm_genpd' already present!
> [    1.992657] debugfs: Directory 'power-domain' with parent
> 'pm_genpd' already present!
> [    2.000685] debugfs: Directory 'power-domain' with parent
> 'pm_genpd' already present!
> [    2.008566] debugfs: Directory 'power-domain' with parent
> 'pm_genpd' already present!
> [    2.016395] debugfs: Directory 'power-domain' with parent
> 'pm_genpd' already present!
> [    2.024221] debugfs: Directory 'power-domain' with parent
> 'pm_genpd' already present!
> [    2.032049] debugfs: Directory 'power-domain' with parent
> 'pm_genpd' already present!
> [    2.039874] debugfs: Directory 'power-domain' with parent
> 'pm_genpd' already present!
> [    2.047699] debugfs: Directory 'power-domain' with parent
> 'pm_genpd' already present!
> [    2.055524] debugfs: Directory 'power-domain' with parent
> 'pm_genpd' already present!
> [    2.063352] debugfs: Directory 'power-domain' with parent
> 'pm_genpd' already present!
> [    2.071176] debugfs: Directory 'power-domain' with parent
> 'pm_genpd' already present!
> 
> But that's another problem that should be handled in debugfs system.
> 

Indeed...so I chose to use different name in dts to avoid problems in
debugfs. It does violate the naming rules.

> > +                                       reg = <MT8192_POWER_DOMAIN_AUDIO>;
> > +                                       clocks = <&topckgen CLK_TOP_AUD_INTBUS_SEL>,
> > +                                                <&infracfg CLK_INFRA_AUDIO_26M_B>,
> > +                                                <&infracfg CLK_INFRA_AUDIO>;
> > +                                       clock-names = "audio", "audio1", "audio2";
> > +                                       mediatek,infracfg = <&infracfg>;
> > +                                       #power-domain-cells = <0>;
> > +                               };
> > +
> > +                               conn@MT8192_POWER_DOMAIN_CONN {
> > +                                       reg = <MT8192_POWER_DOMAIN_CONN>;
> > +                                       clocks = <&infracfg CLK_INFRA_PMIC_CONN>;
> > +                                       clock-names = "conn";
> > +                                       mediatek,infracfg = <&infracfg>;
> > +                                       #power-domain-cells = <0>;
> > +                               };
> > +
> > +                               mfg@MT8192_POWER_DOMAIN_MFG0 {
> > +                                       reg = <MT8192_POWER_DOMAIN_MFG0>;
> > +                                       clocks = <&topckgen CLK_TOP_MFG_PLL_SEL>;
> > +                                       clock-names = "mfg";
> > +                                       #address-cells = <1>;
> > +                                       #size-cells = <0>;
> > +                                       #power-domain-cells = <1>;
> > +
> > +                                       mfg1@MT8192_POWER_DOMAIN_MFG1 {
> > +                                               reg = <MT8192_POWER_DOMAIN_MFG1>;
> > +                                               mediatek,infracfg = <&infracfg>;
> > +                                               #address-cells = <1>;
> > +                                               #size-cells = <0>;
> > +                                               #power-domain-cells = <1>;
> > +
> > +                                               mfg2@MT8192_POWER_DOMAIN_MFG2 {
> > +                                                       reg = <MT8192_POWER_DOMAIN_MFG2>;
> > +                                                       #power-domain-cells = <0>;
> > +                                               };
> > +
> > +                                               mfg3@MT8192_POWER_DOMAIN_MFG3 {
> > +                                                       reg = <MT8192_POWER_DOMAIN_MFG3>;
> > +                                                       #power-domain-cells = <0>;
> > +                                               };
> > +
> > +                                               mfg4@MT8192_POWER_DOMAIN_MFG4 {
> > +                                                       reg = <MT8192_POWER_DOMAIN_MFG4>;
> > +                                                       #power-domain-cells = <0>;
> > +                                               };
> > +
> > +                                               mfg5@MT8192_POWER_DOMAIN_MFG5 {
> > +                                                       reg = <MT8192_POWER_DOMAIN_MFG5>;
> > +                                                       #power-domain-cells = <0>;
> > +                                               };
> > +
> > +                                               mfg6@MT8192_POWER_DOMAIN_MFG6 {
> > +                                                       reg = <MT8192_POWER_DOMAIN_MFG6>;
> > +                                                       #power-domain-cells = <0>;
> > +                                               };
> > +                                       };
> > +                               };
> > +
> > +                               disp@MT8192_POWER_DOMAIN_DISP {
> > +                                       reg = <MT8192_POWER_DOMAIN_DISP>;
> > +                                       clocks = <&topckgen CLK_TOP_DISP_SEL>,
> > +                                                <&mmsys CLK_MM_SMI_INFRA>,
> > +                                                <&mmsys CLK_MM_SMI_COMMON>,
> > +                                                <&mmsys CLK_MM_SMI_GALS>,
> > +                                                <&mmsys CLK_MM_SMI_IOMMU>;
> > +                                       clock-names = "disp", "disp-0", "disp-1", "disp-2",
> > +                                                     "disp-3";
> > +                                       mediatek,infracfg = <&infracfg>;
> > +                                       #address-cells = <1>;
> > +                                       #size-cells = <0>;
> > +                                       #power-domain-cells = <1>;
> > +
> > +                                       ipe@MT8192_POWER_DOMAIN_IPE {
> > +                                               reg = <MT8192_POWER_DOMAIN_IPE>;
> > +                                               clocks = <&topckgen CLK_TOP_IPE_SEL>,
> > +                                                        <&ipesys CLK_IPE_LARB19>,
> > +                                                        <&ipesys CLK_IPE_LARB20>,
> > +                                                        <&ipesys CLK_IPE_SMI_SUBCOM>,
> > +                                                        <&ipesys CLK_IPE_GALS>;
> > +                                               clock-names = "ipe", "ipe-0", "ipe-1", "ipe-2",
> > +                                                             "ipe-3";
> > +                                               mediatek,infracfg = <&infracfg>;
> > +                                               #power-domain-cells = <0>;
> > +                                       };
> > +
> > +                                       isp@MT8192_POWER_DOMAIN_ISP {
> > +                                               reg = <MT8192_POWER_DOMAIN_ISP>;
> > +                                               clocks = <&topckgen CLK_TOP_IMG1_SEL>,
> > +                                                        <&imgsys CLK_IMG_LARB9>,
> > +                                                        <&imgsys CLK_IMG_GALS>;
> > +                                               clock-names = "isp", "isp-0", "isp-1";
> > +                                               mediatek,infracfg = <&infracfg>;
> > +                                               #power-domain-cells = <0>;
> > +                                       };
> > +
> > +                                       isp2@MT8192_POWER_DOMAIN_ISP2 {
> > +                                               reg = <MT8192_POWER_DOMAIN_ISP2>;
> > +                                               clocks = <&topckgen CLK_TOP_IMG2_SEL>,
> > +                                                        <&imgsys2 CLK_IMG2_LARB11>,
> > +                                                        <&imgsys2 CLK_IMG2_GALS>;
> > +                                               clock-names = "isp2", "isp2-0", "isp2-1";
> > +                                               mediatek,infracfg = <&infracfg>;
> > +                                               #power-domain-cells = <0>;
> > +                                       };
> > +
> > +                                       mdp@MT8192_POWER_DOMAIN_MDP {
> > +                                               reg = <MT8192_POWER_DOMAIN_MDP>;
> > +                                               clocks = <&topckgen CLK_TOP_MDP_SEL>,
> > +                                                        <&mdpsys CLK_MDP_SMI0>;
> > +                                               clock-names = "mdp", "mdp-0";
> > +                                               mediatek,infracfg = <&infracfg>;
> > +                                               #power-domain-cells = <0>;
> > +                                       };
> > +
> > +                                       venc@MT8192_POWER_DOMAIN_VENC {
> > +                                               reg = <MT8192_POWER_DOMAIN_VENC>;
> > +                                               clocks = <&topckgen CLK_TOP_VENC_SEL>,
> > +                                                        <&vencsys CLK_VENC_SET1_VENC>;
> > +                                               clock-names = "venc", "venc-0";
> > +                                               mediatek,infracfg = <&infracfg>;
> > +                                               #power-domain-cells = <0>;
> > +                                       };
> > +
> > +                                       vdec@MT8192_POWER_DOMAIN_VDEC {
> > +                                               reg = <MT8192_POWER_DOMAIN_VDEC>;
> > +                                               clocks = <&topckgen CLK_TOP_VDEC_SEL>,
> > +                                                        <&vdecsys_soc CLK_VDEC_SOC_VDEC>,
> > +                                                        <&vdecsys_soc CLK_VDEC_SOC_LAT>,
> > +                                                        <&vdecsys_soc CLK_VDEC_SOC_LARB1>;
> > +                                               clock-names = "vdec", "vdec-0", "vdec-1", "vdec-2";
> > +                                               mediatek,infracfg = <&infracfg>;
> > +                                               #address-cells = <1>;
> > +                                               #size-cells = <0>;
> > +                                               #power-domain-cells = <1>;
> > +
> > +                                               vdec2@MT8192_POWER_DOMAIN_VDEC2 {
> > +                                                       reg = <MT8192_POWER_DOMAIN_VDEC2>;
> > +                                                       clocks = <&vdecsys CLK_VDEC_VDEC>,
> > +                                                                <&vdecsys CLK_VDEC_LAT>,
> > +                                                                <&vdecsys CLK_VDEC_LARB1>;
> > +                                                       clock-names = "vdec2-0", "vdec2-1",
> > +                                                                     "vdec2-2";
> > +                                                       #power-domain-cells = <0>;
> > +                                               };
> > +                                       };
> > +
> > +                                       cam@MT8192_POWER_DOMAIN_CAM {
> > +                                               reg = <MT8192_POWER_DOMAIN_CAM>;
> > +                                               clocks = <&topckgen CLK_TOP_CAM_SEL>,
> > +                                                        <&camsys CLK_CAM_LARB13>,
> > +                                                        <&camsys CLK_CAM_LARB14>,
> > +                                                        <&camsys CLK_CAM_CCU_GALS>,
> > +                                                        <&camsys CLK_CAM_CAM2MM_GALS>;
> > +                                               clock-names = "cam", "cam-0", "cam-1", "cam-2",
> > +                                                             "cam-3";
> > +                                               mediatek,infracfg = <&infracfg>;
> > +                                               #address-cells = <1>;
> > +                                               #size-cells = <0>;
> > +                                               #power-domain-cells = <1>;
> > +
> > +                                               cam_rawa@MT8192_POWER_DOMAIN_CAM_RAWA {
> > +                                                       reg = <MT8192_POWER_DOMAIN_CAM_RAWA>;
> > +                                                       clocks = <&camsys_rawa CLK_CAM_RAWA_LARBX>;
> > +                                                       clock-names = "cam_rawa-0";
> > +                                                       #power-domain-cells = <0>;
> > +                                               };
> > +
> > +                                               cam_rawb@MT8192_POWER_DOMAIN_CAM_RAWB {
> > +                                                       reg = <MT8192_POWER_DOMAIN_CAM_RAWB>;
> > +                                                       clocks = <&camsys_rawb CLK_CAM_RAWB_LARBX>;
> > +                                                       clock-names = "cam_rawb-0";
> > +                                                       #power-domain-cells = <0>;
> > +                                               };
> > +
> > +                                               cam_rawc@MT8192_POWER_DOMAIN_CAM_RAWC {
> > +                                                       reg = <MT8192_POWER_DOMAIN_CAM_RAWC>;
> > +                                                       clocks = <&camsys_rawc CLK_CAM_RAWC_LARBX>;
> > +                                                       clock-names = "cam_rawc-0";
> > +                                                       #power-domain-cells = <0>;
> > +                                               };
> > +                                       };
> > +                               };
> > +                       };
> > +               };
> > +
> >                 apmixedsys: syscon@1000c000 {
> >                         compatible = "mediatek,mt8192-apmixedsys", "syscon";
> >                         reg = <0 0x1000c000 0 0x1000>;
> > --
> > 1.8.1.1.dirty
> >
Enric Balletbo Serra Nov. 19, 2020, 2:13 p.m. UTC | #3
Hi Weiyi,

Missatge de Weiyi Lu <weiyi.lu@mediatek.com> del dia dj., 19 de nov.
2020 a les 14:10:
>
> On Thu, 2020-11-19 at 13:13 +0100, Enric Balletbo Serra wrote:
> > Hi Weiyi,
> >
> > Thank you for the patch
> >
> > Missatge de Weiyi Lu <weiyi.lu@mediatek.com> del dia dj., 19 de nov.
> > 2020 a les 11:48:
> > >
> > > Add power domains controller node for SoC mt8192
> > >
> > > Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
> > > ---
> > >
> > > Change in v3: None, just rebase dts onto v5.10-rc1 and
> > >        V4 of series "Add new driver for SCPSYS power domains controller"[1]
> > >
> > > [1] https://patchwork.kernel.org/project/linux-mediatek/list/?series=374013
> > >
> > >  arch/arm64/boot/dts/mediatek/mt8192.dtsi | 201 +++++++++++++++++++++++++++++++
> > >  1 file changed, 201 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
> > > index 69d45c7..08449eb 100644
> > > --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi
> > > +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
> > > @@ -9,6 +9,7 @@
> > >  #include <dt-bindings/interrupt-controller/arm-gic.h>
> > >  #include <dt-bindings/interrupt-controller/irq.h>
> > >  #include <dt-bindings/pinctrl/mt8192-pinfunc.h>
> > > +#include <dt-bindings/power/mt8192-power.h>
> > >
> > >  / {
> > >         compatible = "mediatek,mt8192";
> > > @@ -257,6 +258,206 @@
> > >                         #interrupt-cells = <2>;
> > >                 };
> > >
> > > +               scpsys: syscon@10006000 {
> > > +                       compatible = "syscon", "simple-mfd";
> > > +                       reg = <0 0x10006000 0 0x1000>;
> > > +                       #power-domain-cells = <1>;
> > > +
> > > +                       /* System Power Manager */
> > > +                       spm: power-controller {
> > > +                               compatible = "mediatek,mt8192-power-controller";
> > > +                               #address-cells = <1>;
> > > +                               #size-cells = <0>;
> > > +                               #power-domain-cells = <1>;
> > > +
> > > +                               /* power domain of the SoC */
> > > +                               audio@MT8192_POWER_DOMAIN_AUDIO {
> >
> > If you run the dt_bindings_check it should return some errors, as all
> > these node names should be 'power-domain@'. Which is a bit annoying
> > because then you will get a bunch of errors like this:
> >
> > [    1.969110] debugfs: Directory 'power-domain' with parent
> > 'pm_genpd' already present!
> > [    1.976997] debugfs: Directory 'power-domain' with parent
> > 'pm_genpd' already present!
> > [    1.984828] debugfs: Directory 'power-domain' with parent
> > 'pm_genpd' already present!
> > [    1.992657] debugfs: Directory 'power-domain' with parent
> > 'pm_genpd' already present!
> > [    2.000685] debugfs: Directory 'power-domain' with parent
> > 'pm_genpd' already present!
> > [    2.008566] debugfs: Directory 'power-domain' with parent
> > 'pm_genpd' already present!
> > [    2.016395] debugfs: Directory 'power-domain' with parent
> > 'pm_genpd' already present!
> > [    2.024221] debugfs: Directory 'power-domain' with parent
> > 'pm_genpd' already present!
> > [    2.032049] debugfs: Directory 'power-domain' with parent
> > 'pm_genpd' already present!
> > [    2.039874] debugfs: Directory 'power-domain' with parent
> > 'pm_genpd' already present!
> > [    2.047699] debugfs: Directory 'power-domain' with parent
> > 'pm_genpd' already present!
> > [    2.055524] debugfs: Directory 'power-domain' with parent
> > 'pm_genpd' already present!
> > [    2.063352] debugfs: Directory 'power-domain' with parent
> > 'pm_genpd' already present!
> > [    2.071176] debugfs: Directory 'power-domain' with parent
> > 'pm_genpd' already present!
> >
> > But that's another problem that should be handled in debugfs system.
> >
>
> Indeed...so I chose to use different name in dts to avoid problems in
> debugfs. It does violate the naming rules.
>

But your binding will not pass (or trigger warnings) the dtb check
then. Rob was clear that names should be generic. Proper fix is fix
debugfs not the binding.

Cheers,
  Enric

> > > +                                       reg = <MT8192_POWER_DOMAIN_AUDIO>;
> > > +                                       clocks = <&topckgen CLK_TOP_AUD_INTBUS_SEL>,
> > > +                                                <&infracfg CLK_INFRA_AUDIO_26M_B>,
> > > +                                                <&infracfg CLK_INFRA_AUDIO>;
> > > +                                       clock-names = "audio", "audio1", "audio2";
> > > +                                       mediatek,infracfg = <&infracfg>;
> > > +                                       #power-domain-cells = <0>;
> > > +                               };
> > > +
> > > +                               conn@MT8192_POWER_DOMAIN_CONN {
> > > +                                       reg = <MT8192_POWER_DOMAIN_CONN>;
> > > +                                       clocks = <&infracfg CLK_INFRA_PMIC_CONN>;
> > > +                                       clock-names = "conn";
> > > +                                       mediatek,infracfg = <&infracfg>;
> > > +                                       #power-domain-cells = <0>;
> > > +                               };
> > > +
> > > +                               mfg@MT8192_POWER_DOMAIN_MFG0 {
> > > +                                       reg = <MT8192_POWER_DOMAIN_MFG0>;
> > > +                                       clocks = <&topckgen CLK_TOP_MFG_PLL_SEL>;
> > > +                                       clock-names = "mfg";
> > > +                                       #address-cells = <1>;
> > > +                                       #size-cells = <0>;
> > > +                                       #power-domain-cells = <1>;
> > > +
> > > +                                       mfg1@MT8192_POWER_DOMAIN_MFG1 {
> > > +                                               reg = <MT8192_POWER_DOMAIN_MFG1>;
> > > +                                               mediatek,infracfg = <&infracfg>;
> > > +                                               #address-cells = <1>;
> > > +                                               #size-cells = <0>;
> > > +                                               #power-domain-cells = <1>;
> > > +
> > > +                                               mfg2@MT8192_POWER_DOMAIN_MFG2 {
> > > +                                                       reg = <MT8192_POWER_DOMAIN_MFG2>;
> > > +                                                       #power-domain-cells = <0>;
> > > +                                               };
> > > +
> > > +                                               mfg3@MT8192_POWER_DOMAIN_MFG3 {
> > > +                                                       reg = <MT8192_POWER_DOMAIN_MFG3>;
> > > +                                                       #power-domain-cells = <0>;
> > > +                                               };
> > > +
> > > +                                               mfg4@MT8192_POWER_DOMAIN_MFG4 {
> > > +                                                       reg = <MT8192_POWER_DOMAIN_MFG4>;
> > > +                                                       #power-domain-cells = <0>;
> > > +                                               };
> > > +
> > > +                                               mfg5@MT8192_POWER_DOMAIN_MFG5 {
> > > +                                                       reg = <MT8192_POWER_DOMAIN_MFG5>;
> > > +                                                       #power-domain-cells = <0>;
> > > +                                               };
> > > +
> > > +                                               mfg6@MT8192_POWER_DOMAIN_MFG6 {
> > > +                                                       reg = <MT8192_POWER_DOMAIN_MFG6>;
> > > +                                                       #power-domain-cells = <0>;
> > > +                                               };
> > > +                                       };
> > > +                               };
> > > +
> > > +                               disp@MT8192_POWER_DOMAIN_DISP {
> > > +                                       reg = <MT8192_POWER_DOMAIN_DISP>;
> > > +                                       clocks = <&topckgen CLK_TOP_DISP_SEL>,
> > > +                                                <&mmsys CLK_MM_SMI_INFRA>,
> > > +                                                <&mmsys CLK_MM_SMI_COMMON>,
> > > +                                                <&mmsys CLK_MM_SMI_GALS>,
> > > +                                                <&mmsys CLK_MM_SMI_IOMMU>;
> > > +                                       clock-names = "disp", "disp-0", "disp-1", "disp-2",
> > > +                                                     "disp-3";
> > > +                                       mediatek,infracfg = <&infracfg>;
> > > +                                       #address-cells = <1>;
> > > +                                       #size-cells = <0>;
> > > +                                       #power-domain-cells = <1>;
> > > +
> > > +                                       ipe@MT8192_POWER_DOMAIN_IPE {
> > > +                                               reg = <MT8192_POWER_DOMAIN_IPE>;
> > > +                                               clocks = <&topckgen CLK_TOP_IPE_SEL>,
> > > +                                                        <&ipesys CLK_IPE_LARB19>,
> > > +                                                        <&ipesys CLK_IPE_LARB20>,
> > > +                                                        <&ipesys CLK_IPE_SMI_SUBCOM>,
> > > +                                                        <&ipesys CLK_IPE_GALS>;
> > > +                                               clock-names = "ipe", "ipe-0", "ipe-1", "ipe-2",
> > > +                                                             "ipe-3";
> > > +                                               mediatek,infracfg = <&infracfg>;
> > > +                                               #power-domain-cells = <0>;
> > > +                                       };
> > > +
> > > +                                       isp@MT8192_POWER_DOMAIN_ISP {
> > > +                                               reg = <MT8192_POWER_DOMAIN_ISP>;
> > > +                                               clocks = <&topckgen CLK_TOP_IMG1_SEL>,
> > > +                                                        <&imgsys CLK_IMG_LARB9>,
> > > +                                                        <&imgsys CLK_IMG_GALS>;
> > > +                                               clock-names = "isp", "isp-0", "isp-1";
> > > +                                               mediatek,infracfg = <&infracfg>;
> > > +                                               #power-domain-cells = <0>;
> > > +                                       };
> > > +
> > > +                                       isp2@MT8192_POWER_DOMAIN_ISP2 {
> > > +                                               reg = <MT8192_POWER_DOMAIN_ISP2>;
> > > +                                               clocks = <&topckgen CLK_TOP_IMG2_SEL>,
> > > +                                                        <&imgsys2 CLK_IMG2_LARB11>,
> > > +                                                        <&imgsys2 CLK_IMG2_GALS>;
> > > +                                               clock-names = "isp2", "isp2-0", "isp2-1";
> > > +                                               mediatek,infracfg = <&infracfg>;
> > > +                                               #power-domain-cells = <0>;
> > > +                                       };
> > > +
> > > +                                       mdp@MT8192_POWER_DOMAIN_MDP {
> > > +                                               reg = <MT8192_POWER_DOMAIN_MDP>;
> > > +                                               clocks = <&topckgen CLK_TOP_MDP_SEL>,
> > > +                                                        <&mdpsys CLK_MDP_SMI0>;
> > > +                                               clock-names = "mdp", "mdp-0";
> > > +                                               mediatek,infracfg = <&infracfg>;
> > > +                                               #power-domain-cells = <0>;
> > > +                                       };
> > > +
> > > +                                       venc@MT8192_POWER_DOMAIN_VENC {
> > > +                                               reg = <MT8192_POWER_DOMAIN_VENC>;
> > > +                                               clocks = <&topckgen CLK_TOP_VENC_SEL>,
> > > +                                                        <&vencsys CLK_VENC_SET1_VENC>;
> > > +                                               clock-names = "venc", "venc-0";
> > > +                                               mediatek,infracfg = <&infracfg>;
> > > +                                               #power-domain-cells = <0>;
> > > +                                       };
> > > +
> > > +                                       vdec@MT8192_POWER_DOMAIN_VDEC {
> > > +                                               reg = <MT8192_POWER_DOMAIN_VDEC>;
> > > +                                               clocks = <&topckgen CLK_TOP_VDEC_SEL>,
> > > +                                                        <&vdecsys_soc CLK_VDEC_SOC_VDEC>,
> > > +                                                        <&vdecsys_soc CLK_VDEC_SOC_LAT>,
> > > +                                                        <&vdecsys_soc CLK_VDEC_SOC_LARB1>;
> > > +                                               clock-names = "vdec", "vdec-0", "vdec-1", "vdec-2";
> > > +                                               mediatek,infracfg = <&infracfg>;
> > > +                                               #address-cells = <1>;
> > > +                                               #size-cells = <0>;
> > > +                                               #power-domain-cells = <1>;
> > > +
> > > +                                               vdec2@MT8192_POWER_DOMAIN_VDEC2 {
> > > +                                                       reg = <MT8192_POWER_DOMAIN_VDEC2>;
> > > +                                                       clocks = <&vdecsys CLK_VDEC_VDEC>,
> > > +                                                                <&vdecsys CLK_VDEC_LAT>,
> > > +                                                                <&vdecsys CLK_VDEC_LARB1>;
> > > +                                                       clock-names = "vdec2-0", "vdec2-1",
> > > +                                                                     "vdec2-2";
> > > +                                                       #power-domain-cells = <0>;
> > > +                                               };
> > > +                                       };
> > > +
> > > +                                       cam@MT8192_POWER_DOMAIN_CAM {
> > > +                                               reg = <MT8192_POWER_DOMAIN_CAM>;
> > > +                                               clocks = <&topckgen CLK_TOP_CAM_SEL>,
> > > +                                                        <&camsys CLK_CAM_LARB13>,
> > > +                                                        <&camsys CLK_CAM_LARB14>,
> > > +                                                        <&camsys CLK_CAM_CCU_GALS>,
> > > +                                                        <&camsys CLK_CAM_CAM2MM_GALS>;
> > > +                                               clock-names = "cam", "cam-0", "cam-1", "cam-2",
> > > +                                                             "cam-3";
> > > +                                               mediatek,infracfg = <&infracfg>;
> > > +                                               #address-cells = <1>;
> > > +                                               #size-cells = <0>;
> > > +                                               #power-domain-cells = <1>;
> > > +
> > > +                                               cam_rawa@MT8192_POWER_DOMAIN_CAM_RAWA {
> > > +                                                       reg = <MT8192_POWER_DOMAIN_CAM_RAWA>;
> > > +                                                       clocks = <&camsys_rawa CLK_CAM_RAWA_LARBX>;
> > > +                                                       clock-names = "cam_rawa-0";
> > > +                                                       #power-domain-cells = <0>;
> > > +                                               };
> > > +
> > > +                                               cam_rawb@MT8192_POWER_DOMAIN_CAM_RAWB {
> > > +                                                       reg = <MT8192_POWER_DOMAIN_CAM_RAWB>;
> > > +                                                       clocks = <&camsys_rawb CLK_CAM_RAWB_LARBX>;
> > > +                                                       clock-names = "cam_rawb-0";
> > > +                                                       #power-domain-cells = <0>;
> > > +                                               };
> > > +
> > > +                                               cam_rawc@MT8192_POWER_DOMAIN_CAM_RAWC {
> > > +                                                       reg = <MT8192_POWER_DOMAIN_CAM_RAWC>;
> > > +                                                       clocks = <&camsys_rawc CLK_CAM_RAWC_LARBX>;
> > > +                                                       clock-names = "cam_rawc-0";
> > > +                                                       #power-domain-cells = <0>;
> > > +                                               };
> > > +                                       };
> > > +                               };
> > > +                       };
> > > +               };
> > > +
> > >                 apmixedsys: syscon@1000c000 {
> > >                         compatible = "mediatek,mt8192-apmixedsys", "syscon";
> > >                         reg = <0 0x1000c000 0 0x1000>;
> > > --
> > > 1.8.1.1.dirty
> > >
>
Matthias Brugger Nov. 27, 2020, 12:42 p.m. UTC | #4
On 19/11/2020 15:13, Enric Balletbo Serra wrote:
> Hi Weiyi,
> 
> Missatge de Weiyi Lu <weiyi.lu@mediatek.com> del dia dj., 19 de nov.
> 2020 a les 14:10:
>>
>> On Thu, 2020-11-19 at 13:13 +0100, Enric Balletbo Serra wrote:
>>> Hi Weiyi,
>>>
>>> Thank you for the patch
>>>
>>> Missatge de Weiyi Lu <weiyi.lu@mediatek.com> del dia dj., 19 de nov.
>>> 2020 a les 11:48:
>>>>
>>>> Add power domains controller node for SoC mt8192
>>>>
>>>> Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
>>>> ---
[...]
>>>> +                       /* System Power Manager */
>>>> +                       spm: power-controller {
>>>> +                               compatible = "mediatek,mt8192-power-controller";
>>>> +                               #address-cells = <1>;
>>>> +                               #size-cells = <0>;
>>>> +                               #power-domain-cells = <1>;
>>>> +
>>>> +                               /* power domain of the SoC */
>>>> +                               audio@MT8192_POWER_DOMAIN_AUDIO {
>>>
>>> If you run the dt_bindings_check it should return some errors, as all
>>> these node names should be 'power-domain@'. Which is a bit annoying
>>> because then you will get a bunch of errors like this:
>>>
>>> [    1.969110] debugfs: Directory 'power-domain' with parent
>>> 'pm_genpd' already present!
>>> [    1.976997] debugfs: Directory 'power-domain' with parent
>>> 'pm_genpd' already present!
>>> [    1.984828] debugfs: Directory 'power-domain' with parent
>>> 'pm_genpd' already present!
>>> [    1.992657] debugfs: Directory 'power-domain' with parent
>>> 'pm_genpd' already present!
>>> [    2.000685] debugfs: Directory 'power-domain' with parent
>>> 'pm_genpd' already present!
>>> [    2.008566] debugfs: Directory 'power-domain' with parent
>>> 'pm_genpd' already present!
>>> [    2.016395] debugfs: Directory 'power-domain' with parent
>>> 'pm_genpd' already present!
>>> [    2.024221] debugfs: Directory 'power-domain' with parent
>>> 'pm_genpd' already present!
>>> [    2.032049] debugfs: Directory 'power-domain' with parent
>>> 'pm_genpd' already present!
>>> [    2.039874] debugfs: Directory 'power-domain' with parent
>>> 'pm_genpd' already present!
>>> [    2.047699] debugfs: Directory 'power-domain' with parent
>>> 'pm_genpd' already present!
>>> [    2.055524] debugfs: Directory 'power-domain' with parent
>>> 'pm_genpd' already present!
>>> [    2.063352] debugfs: Directory 'power-domain' with parent
>>> 'pm_genpd' already present!
>>> [    2.071176] debugfs: Directory 'power-domain' with parent
>>> 'pm_genpd' already present!
>>>
>>> But that's another problem that should be handled in debugfs system.
>>>
>>
>> Indeed...so I chose to use different name in dts to avoid problems in
>> debugfs. It does violate the naming rules.
>>
> 
> But your binding will not pass (or trigger warnings) the dtb check
> then. Rob was clear that names should be generic. Proper fix is fix
> debugfs not the binding.
> 

By the way, is anybody working on this debugfs issue?

Regards,
Matthias
Weiyi Lu Nov. 30, 2020, 11:16 a.m. UTC | #5
On Fri, 2020-11-27 at 13:42 +0100, Matthias Brugger wrote:
> 
> On 19/11/2020 15:13, Enric Balletbo Serra wrote:
> > Hi Weiyi,
> > 
> > Missatge de Weiyi Lu <weiyi.lu@mediatek.com> del dia dj., 19 de nov.
> > 2020 a les 14:10:
> >>
> >> On Thu, 2020-11-19 at 13:13 +0100, Enric Balletbo Serra wrote:
> >>> Hi Weiyi,
> >>>
> >>> Thank you for the patch
> >>>
> >>> Missatge de Weiyi Lu <weiyi.lu@mediatek.com> del dia dj., 19 de nov.
> >>> 2020 a les 11:48:
> >>>>
> >>>> Add power domains controller node for SoC mt8192
> >>>>
> >>>> Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
> >>>> ---
> [...]
> >>>> +                       /* System Power Manager */
> >>>> +                       spm: power-controller {
> >>>> +                               compatible = "mediatek,mt8192-power-controller";
> >>>> +                               #address-cells = <1>;
> >>>> +                               #size-cells = <0>;
> >>>> +                               #power-domain-cells = <1>;
> >>>> +
> >>>> +                               /* power domain of the SoC */
> >>>> +                               audio@MT8192_POWER_DOMAIN_AUDIO {
> >>>
> >>> If you run the dt_bindings_check it should return some errors, as all
> >>> these node names should be 'power-domain@'. Which is a bit annoying
> >>> because then you will get a bunch of errors like this:
> >>>
> >>> [    1.969110] debugfs: Directory 'power-domain' with parent
> >>> 'pm_genpd' already present!
> >>> [    1.976997] debugfs: Directory 'power-domain' with parent
> >>> 'pm_genpd' already present!
> >>> [    1.984828] debugfs: Directory 'power-domain' with parent
> >>> 'pm_genpd' already present!
> >>> [    1.992657] debugfs: Directory 'power-domain' with parent
> >>> 'pm_genpd' already present!
> >>> [    2.000685] debugfs: Directory 'power-domain' with parent
> >>> 'pm_genpd' already present!
> >>> [    2.008566] debugfs: Directory 'power-domain' with parent
> >>> 'pm_genpd' already present!
> >>> [    2.016395] debugfs: Directory 'power-domain' with parent
> >>> 'pm_genpd' already present!
> >>> [    2.024221] debugfs: Directory 'power-domain' with parent
> >>> 'pm_genpd' already present!
> >>> [    2.032049] debugfs: Directory 'power-domain' with parent
> >>> 'pm_genpd' already present!
> >>> [    2.039874] debugfs: Directory 'power-domain' with parent
> >>> 'pm_genpd' already present!
> >>> [    2.047699] debugfs: Directory 'power-domain' with parent
> >>> 'pm_genpd' already present!
> >>> [    2.055524] debugfs: Directory 'power-domain' with parent
> >>> 'pm_genpd' already present!
> >>> [    2.063352] debugfs: Directory 'power-domain' with parent
> >>> 'pm_genpd' already present!
> >>> [    2.071176] debugfs: Directory 'power-domain' with parent
> >>> 'pm_genpd' already present!
> >>>
> >>> But that's another problem that should be handled in debugfs system.
> >>>
> >>
> >> Indeed...so I chose to use different name in dts to avoid problems in
> >> debugfs. It does violate the naming rules.
> >>
> > 
> > But your binding will not pass (or trigger warnings) the dtb check
> > then. Rob was clear that names should be generic. Proper fix is fix
> > debugfs not the binding.
> > 
> 
> By the way, is anybody working on this debugfs issue?
> 

I think we can solve this problem by adding "name" to the struct
scpsys_domain_data and use this domain_data->name as the genpd.name.
This is very simple. But I want to know if you both like it?

> Regards,
> Matthias
Weiyi Lu Feb. 18, 2021, 10:31 a.m. UTC | #6
On Mon, 2020-11-30 at 19:16 +0800, Weiyi Lu wrote:
> On Fri, 2020-11-27 at 13:42 +0100, Matthias Brugger wrote:
> > 
> > On 19/11/2020 15:13, Enric Balletbo Serra wrote:
> > > Hi Weiyi,
> > > 
> > > Missatge de Weiyi Lu <weiyi.lu@mediatek.com> del dia dj., 19 de nov.
> > > 2020 a les 14:10:
> > >>
> > >> On Thu, 2020-11-19 at 13:13 +0100, Enric Balletbo Serra wrote:
> > >>> Hi Weiyi,
> > >>>
> > >>> Thank you for the patch
> > >>>
> > >>> Missatge de Weiyi Lu <weiyi.lu@mediatek.com> del dia dj., 19 de nov.
> > >>> 2020 a les 11:48:
> > >>>>
> > >>>> Add power domains controller node for SoC mt8192
> > >>>>
> > >>>> Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
> > >>>> ---
> > [...]
> > >>>> +                       /* System Power Manager */
> > >>>> +                       spm: power-controller {
> > >>>> +                               compatible = "mediatek,mt8192-power-controller";
> > >>>> +                               #address-cells = <1>;
> > >>>> +                               #size-cells = <0>;
> > >>>> +                               #power-domain-cells = <1>;
> > >>>> +
> > >>>> +                               /* power domain of the SoC */
> > >>>> +                               audio@MT8192_POWER_DOMAIN_AUDIO {
> > >>>
> > >>> If you run the dt_bindings_check it should return some errors, as all
> > >>> these node names should be 'power-domain@'. Which is a bit annoying
> > >>> because then you will get a bunch of errors like this:
> > >>>
> > >>> [    1.969110] debugfs: Directory 'power-domain' with parent
> > >>> 'pm_genpd' already present!
> > >>> [    1.976997] debugfs: Directory 'power-domain' with parent
> > >>> 'pm_genpd' already present!
> > >>> [    1.984828] debugfs: Directory 'power-domain' with parent
> > >>> 'pm_genpd' already present!
> > >>> [    1.992657] debugfs: Directory 'power-domain' with parent
> > >>> 'pm_genpd' already present!
> > >>> [    2.000685] debugfs: Directory 'power-domain' with parent
> > >>> 'pm_genpd' already present!
> > >>> [    2.008566] debugfs: Directory 'power-domain' with parent
> > >>> 'pm_genpd' already present!
> > >>> [    2.016395] debugfs: Directory 'power-domain' with parent
> > >>> 'pm_genpd' already present!
> > >>> [    2.024221] debugfs: Directory 'power-domain' with parent
> > >>> 'pm_genpd' already present!
> > >>> [    2.032049] debugfs: Directory 'power-domain' with parent
> > >>> 'pm_genpd' already present!
> > >>> [    2.039874] debugfs: Directory 'power-domain' with parent
> > >>> 'pm_genpd' already present!
> > >>> [    2.047699] debugfs: Directory 'power-domain' with parent
> > >>> 'pm_genpd' already present!
> > >>> [    2.055524] debugfs: Directory 'power-domain' with parent
> > >>> 'pm_genpd' already present!
> > >>> [    2.063352] debugfs: Directory 'power-domain' with parent
> > >>> 'pm_genpd' already present!
> > >>> [    2.071176] debugfs: Directory 'power-domain' with parent
> > >>> 'pm_genpd' already present!
> > >>>
> > >>> But that's another problem that should be handled in debugfs system.
> > >>>
> > >>
> > >> Indeed...so I chose to use different name in dts to avoid problems in
> > >> debugfs. It does violate the naming rules.
> > >>
> > > 
> > > But your binding will not pass (or trigger warnings) the dtb check
> > > then. Rob was clear that names should be generic. Proper fix is fix
> > > debugfs not the binding.
> > > 
> > 
> > By the way, is anybody working on this debugfs issue?
> > 
> 
> I think we can solve this problem by adding "name" to the struct
> scpsys_domain_data and use this domain_data->name as the genpd.name.
> This is very simple. But I want to know if you both like it?
> 

Hi Enric and Matthias,

May I have your opinions on how you might to fix this issue?
I'll try to give another name to each power domain in the
scpsys_domain_data and register power domain with this name like below

struct scpsys_domain_data {
 	...
+	char *name;
 };


-	pd->genpd.name = node->name;
+	pd->genpd.name = pd->data->name ?: node->name;


Does it violate the naming rules to some extent? or it's acceptable?

> > Regards,
> > Matthias
> 
> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
Enric Balletbo Serra Feb. 18, 2021, 12:07 p.m. UTC | #7
Hi Weiyi,

Missatge de Weiyi Lu <weiyi.lu@mediatek.com> del dia dj., 18 de febr.
2021 a les 11:31:
>
> On Mon, 2020-11-30 at 19:16 +0800, Weiyi Lu wrote:
> > On Fri, 2020-11-27 at 13:42 +0100, Matthias Brugger wrote:
> > >
> > > On 19/11/2020 15:13, Enric Balletbo Serra wrote:
> > > > Hi Weiyi,
> > > >
> > > > Missatge de Weiyi Lu <weiyi.lu@mediatek.com> del dia dj., 19 de nov.
> > > > 2020 a les 14:10:
> > > >>
> > > >> On Thu, 2020-11-19 at 13:13 +0100, Enric Balletbo Serra wrote:
> > > >>> Hi Weiyi,
> > > >>>
> > > >>> Thank you for the patch
> > > >>>
> > > >>> Missatge de Weiyi Lu <weiyi.lu@mediatek.com> del dia dj., 19 de nov.
> > > >>> 2020 a les 11:48:
> > > >>>>
> > > >>>> Add power domains controller node for SoC mt8192
> > > >>>>
> > > >>>> Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com>
> > > >>>> ---
> > > [...]
> > > >>>> +                       /* System Power Manager */
> > > >>>> +                       spm: power-controller {
> > > >>>> +                               compatible = "mediatek,mt8192-power-controller";
> > > >>>> +                               #address-cells = <1>;
> > > >>>> +                               #size-cells = <0>;
> > > >>>> +                               #power-domain-cells = <1>;
> > > >>>> +
> > > >>>> +                               /* power domain of the SoC */
> > > >>>> +                               audio@MT8192_POWER_DOMAIN_AUDIO {
> > > >>>
> > > >>> If you run the dt_bindings_check it should return some errors, as all
> > > >>> these node names should be 'power-domain@'. Which is a bit annoying
> > > >>> because then you will get a bunch of errors like this:
> > > >>>
> > > >>> [    1.969110] debugfs: Directory 'power-domain' with parent
> > > >>> 'pm_genpd' already present!
> > > >>> [    1.976997] debugfs: Directory 'power-domain' with parent
> > > >>> 'pm_genpd' already present!
> > > >>> [    1.984828] debugfs: Directory 'power-domain' with parent
> > > >>> 'pm_genpd' already present!
> > > >>> [    1.992657] debugfs: Directory 'power-domain' with parent
> > > >>> 'pm_genpd' already present!
> > > >>> [    2.000685] debugfs: Directory 'power-domain' with parent
> > > >>> 'pm_genpd' already present!
> > > >>> [    2.008566] debugfs: Directory 'power-domain' with parent
> > > >>> 'pm_genpd' already present!
> > > >>> [    2.016395] debugfs: Directory 'power-domain' with parent
> > > >>> 'pm_genpd' already present!
> > > >>> [    2.024221] debugfs: Directory 'power-domain' with parent
> > > >>> 'pm_genpd' already present!
> > > >>> [    2.032049] debugfs: Directory 'power-domain' with parent
> > > >>> 'pm_genpd' already present!
> > > >>> [    2.039874] debugfs: Directory 'power-domain' with parent
> > > >>> 'pm_genpd' already present!
> > > >>> [    2.047699] debugfs: Directory 'power-domain' with parent
> > > >>> 'pm_genpd' already present!
> > > >>> [    2.055524] debugfs: Directory 'power-domain' with parent
> > > >>> 'pm_genpd' already present!
> > > >>> [    2.063352] debugfs: Directory 'power-domain' with parent
> > > >>> 'pm_genpd' already present!
> > > >>> [    2.071176] debugfs: Directory 'power-domain' with parent
> > > >>> 'pm_genpd' already present!
> > > >>>
> > > >>> But that's another problem that should be handled in debugfs system.
> > > >>>
> > > >>
> > > >> Indeed...so I chose to use different name in dts to avoid problems in
> > > >> debugfs. It does violate the naming rules.
> > > >>
> > > >
> > > > But your binding will not pass (or trigger warnings) the dtb check
> > > > then. Rob was clear that names should be generic. Proper fix is fix
> > > > debugfs not the binding.
> > > >
> > >
> > > By the way, is anybody working on this debugfs issue?
> > >
> >
> > I think we can solve this problem by adding "name" to the struct
> > scpsys_domain_data and use this domain_data->name as the genpd.name.
> > This is very simple. But I want to know if you both like it?
> >
>
> Hi Enric and Matthias,
>
> May I have your opinions on how you might to fix this issue?
> I'll try to give another name to each power domain in the
> scpsys_domain_data and register power domain with this name like below
>
> struct scpsys_domain_data {
>         ...
> +       char *name;
>  };
>
>
> -       pd->genpd.name = node->name;
> +       pd->genpd.name = pd->data->name ?: node->name;
>
>
> Does it violate the naming rules to some extent? or it's acceptable?
>

I think it could be acceptable, I think doesn't violate any rule. My
doubt here is we should name in a generic way in the form
'power-domain@addr' getting the last part of the full node name to
avoid conflicts or if the name should be driver-specific. I think it
makes sense to be driver-specific as is more helpful for debugging
purposes. If we do driver-specific (with data->name) I'd fail if is
not supplied.

Thanks,
  Enric

> > > Regards,
> > > Matthias
> >
> >
> > _______________________________________________
> > Linux-mediatek mailing list
> > Linux-mediatek@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-mediatek
>
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
index 69d45c7..08449eb 100644
--- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi
@@ -9,6 +9,7 @@ 
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/pinctrl/mt8192-pinfunc.h>
+#include <dt-bindings/power/mt8192-power.h>
 
 / {
 	compatible = "mediatek,mt8192";
@@ -257,6 +258,206 @@ 
 			#interrupt-cells = <2>;
 		};
 
+		scpsys: syscon@10006000 {
+			compatible = "syscon", "simple-mfd";
+			reg = <0 0x10006000 0 0x1000>;
+			#power-domain-cells = <1>;
+
+			/* System Power Manager */
+			spm: power-controller {
+				compatible = "mediatek,mt8192-power-controller";
+				#address-cells = <1>;
+				#size-cells = <0>;
+				#power-domain-cells = <1>;
+
+				/* power domain of the SoC */
+				audio@MT8192_POWER_DOMAIN_AUDIO {
+					reg = <MT8192_POWER_DOMAIN_AUDIO>;
+					clocks = <&topckgen CLK_TOP_AUD_INTBUS_SEL>,
+						 <&infracfg CLK_INFRA_AUDIO_26M_B>,
+						 <&infracfg CLK_INFRA_AUDIO>;
+					clock-names = "audio", "audio1", "audio2";
+					mediatek,infracfg = <&infracfg>;
+					#power-domain-cells = <0>;
+				};
+
+				conn@MT8192_POWER_DOMAIN_CONN {
+					reg = <MT8192_POWER_DOMAIN_CONN>;
+					clocks = <&infracfg CLK_INFRA_PMIC_CONN>;
+					clock-names = "conn";
+					mediatek,infracfg = <&infracfg>;
+					#power-domain-cells = <0>;
+				};
+
+				mfg@MT8192_POWER_DOMAIN_MFG0 {
+					reg = <MT8192_POWER_DOMAIN_MFG0>;
+					clocks = <&topckgen CLK_TOP_MFG_PLL_SEL>;
+					clock-names = "mfg";
+					#address-cells = <1>;
+					#size-cells = <0>;
+					#power-domain-cells = <1>;
+
+					mfg1@MT8192_POWER_DOMAIN_MFG1 {
+						reg = <MT8192_POWER_DOMAIN_MFG1>;
+						mediatek,infracfg = <&infracfg>;
+						#address-cells = <1>;
+						#size-cells = <0>;
+						#power-domain-cells = <1>;
+
+						mfg2@MT8192_POWER_DOMAIN_MFG2 {
+							reg = <MT8192_POWER_DOMAIN_MFG2>;
+							#power-domain-cells = <0>;
+						};
+
+						mfg3@MT8192_POWER_DOMAIN_MFG3 {
+							reg = <MT8192_POWER_DOMAIN_MFG3>;
+							#power-domain-cells = <0>;
+						};
+
+						mfg4@MT8192_POWER_DOMAIN_MFG4 {
+							reg = <MT8192_POWER_DOMAIN_MFG4>;
+							#power-domain-cells = <0>;
+						};
+
+						mfg5@MT8192_POWER_DOMAIN_MFG5 {
+							reg = <MT8192_POWER_DOMAIN_MFG5>;
+							#power-domain-cells = <0>;
+						};
+
+						mfg6@MT8192_POWER_DOMAIN_MFG6 {
+							reg = <MT8192_POWER_DOMAIN_MFG6>;
+							#power-domain-cells = <0>;
+						};
+					};
+				};
+
+				disp@MT8192_POWER_DOMAIN_DISP {
+					reg = <MT8192_POWER_DOMAIN_DISP>;
+					clocks = <&topckgen CLK_TOP_DISP_SEL>,
+						 <&mmsys CLK_MM_SMI_INFRA>,
+						 <&mmsys CLK_MM_SMI_COMMON>,
+						 <&mmsys CLK_MM_SMI_GALS>,
+						 <&mmsys CLK_MM_SMI_IOMMU>;
+					clock-names = "disp", "disp-0", "disp-1", "disp-2",
+						      "disp-3";
+					mediatek,infracfg = <&infracfg>;
+					#address-cells = <1>;
+					#size-cells = <0>;
+					#power-domain-cells = <1>;
+
+					ipe@MT8192_POWER_DOMAIN_IPE {
+						reg = <MT8192_POWER_DOMAIN_IPE>;
+						clocks = <&topckgen CLK_TOP_IPE_SEL>,
+							 <&ipesys CLK_IPE_LARB19>,
+							 <&ipesys CLK_IPE_LARB20>,
+							 <&ipesys CLK_IPE_SMI_SUBCOM>,
+							 <&ipesys CLK_IPE_GALS>;
+						clock-names = "ipe", "ipe-0", "ipe-1", "ipe-2",
+							      "ipe-3";
+						mediatek,infracfg = <&infracfg>;
+						#power-domain-cells = <0>;
+					};
+
+					isp@MT8192_POWER_DOMAIN_ISP {
+						reg = <MT8192_POWER_DOMAIN_ISP>;
+						clocks = <&topckgen CLK_TOP_IMG1_SEL>,
+							 <&imgsys CLK_IMG_LARB9>,
+							 <&imgsys CLK_IMG_GALS>;
+						clock-names = "isp", "isp-0", "isp-1";
+						mediatek,infracfg = <&infracfg>;
+						#power-domain-cells = <0>;
+					};
+
+					isp2@MT8192_POWER_DOMAIN_ISP2 {
+						reg = <MT8192_POWER_DOMAIN_ISP2>;
+						clocks = <&topckgen CLK_TOP_IMG2_SEL>,
+							 <&imgsys2 CLK_IMG2_LARB11>,
+							 <&imgsys2 CLK_IMG2_GALS>;
+						clock-names = "isp2", "isp2-0", "isp2-1";
+						mediatek,infracfg = <&infracfg>;
+						#power-domain-cells = <0>;
+					};
+
+					mdp@MT8192_POWER_DOMAIN_MDP {
+						reg = <MT8192_POWER_DOMAIN_MDP>;
+						clocks = <&topckgen CLK_TOP_MDP_SEL>,
+							 <&mdpsys CLK_MDP_SMI0>;
+						clock-names = "mdp", "mdp-0";
+						mediatek,infracfg = <&infracfg>;
+						#power-domain-cells = <0>;
+					};
+
+					venc@MT8192_POWER_DOMAIN_VENC {
+						reg = <MT8192_POWER_DOMAIN_VENC>;
+						clocks = <&topckgen CLK_TOP_VENC_SEL>,
+							 <&vencsys CLK_VENC_SET1_VENC>;
+						clock-names = "venc", "venc-0";
+						mediatek,infracfg = <&infracfg>;
+						#power-domain-cells = <0>;
+					};
+
+					vdec@MT8192_POWER_DOMAIN_VDEC {
+						reg = <MT8192_POWER_DOMAIN_VDEC>;
+						clocks = <&topckgen CLK_TOP_VDEC_SEL>,
+							 <&vdecsys_soc CLK_VDEC_SOC_VDEC>,
+							 <&vdecsys_soc CLK_VDEC_SOC_LAT>,
+							 <&vdecsys_soc CLK_VDEC_SOC_LARB1>;
+						clock-names = "vdec", "vdec-0", "vdec-1", "vdec-2";
+						mediatek,infracfg = <&infracfg>;
+						#address-cells = <1>;
+						#size-cells = <0>;
+						#power-domain-cells = <1>;
+
+						vdec2@MT8192_POWER_DOMAIN_VDEC2 {
+							reg = <MT8192_POWER_DOMAIN_VDEC2>;
+							clocks = <&vdecsys CLK_VDEC_VDEC>,
+								 <&vdecsys CLK_VDEC_LAT>,
+								 <&vdecsys CLK_VDEC_LARB1>;
+							clock-names = "vdec2-0", "vdec2-1",
+								      "vdec2-2";
+							#power-domain-cells = <0>;
+						};
+					};
+
+					cam@MT8192_POWER_DOMAIN_CAM {
+						reg = <MT8192_POWER_DOMAIN_CAM>;
+						clocks = <&topckgen CLK_TOP_CAM_SEL>,
+							 <&camsys CLK_CAM_LARB13>,
+							 <&camsys CLK_CAM_LARB14>,
+							 <&camsys CLK_CAM_CCU_GALS>,
+							 <&camsys CLK_CAM_CAM2MM_GALS>;
+						clock-names = "cam", "cam-0", "cam-1", "cam-2",
+							      "cam-3";
+						mediatek,infracfg = <&infracfg>;
+						#address-cells = <1>;
+						#size-cells = <0>;
+						#power-domain-cells = <1>;
+
+						cam_rawa@MT8192_POWER_DOMAIN_CAM_RAWA {
+							reg = <MT8192_POWER_DOMAIN_CAM_RAWA>;
+							clocks = <&camsys_rawa CLK_CAM_RAWA_LARBX>;
+							clock-names = "cam_rawa-0";
+							#power-domain-cells = <0>;
+						};
+
+						cam_rawb@MT8192_POWER_DOMAIN_CAM_RAWB {
+							reg = <MT8192_POWER_DOMAIN_CAM_RAWB>;
+							clocks = <&camsys_rawb CLK_CAM_RAWB_LARBX>;
+							clock-names = "cam_rawb-0";
+							#power-domain-cells = <0>;
+						};
+
+						cam_rawc@MT8192_POWER_DOMAIN_CAM_RAWC {
+							reg = <MT8192_POWER_DOMAIN_CAM_RAWC>;
+							clocks = <&camsys_rawc CLK_CAM_RAWC_LARBX>;
+							clock-names = "cam_rawc-0";
+							#power-domain-cells = <0>;
+						};
+					};
+				};
+			};
+		};
+
 		apmixedsys: syscon@1000c000 {
 			compatible = "mediatek,mt8192-apmixedsys", "syscon";
 			reg = <0 0x1000c000 0 0x1000>;