[PATCH 0/8] soc: mediatek: MT8365 power support

[PATCH 0/8] soc: mediatek: MT8365 power support

[Markus Schneider-Pargmann]

Hi,

sorry for the long delay, here is an update to the mt8365 power domain
series. Thanks for all your feedback, I hope I addressed everything. As
Matthias brought up some good points, I got back to try to make the
WAY_EN feature a bit more understandable with less code duplication. I
added a few patches to prepare the driver for the WAY_EN patch. I hope
it is better to understand now.

Thank you!

Best,
Markus

Changes in v4:
- Redesigned WAY_EN patch and split it up in smaller patches.
- Added two documentation patches.
- Added mediatek,infracfg-nao field to the binding.

Changes in v3:
- Mainly redesigned WAY_EN patch to be easier to understand
- Rebased onto v6.0-rc1
- Several other stuff that is described in the individual patches

Changes in v2:
- Updated error handling path for scpsys_power_on()
- Minor updates described in each patch

Previous versions:
v1 - https://lore.kernel.org/linux-mediatek/20220530204214.913251-1-fparent@baylibre.com/
v2 - https://lore.kernel.org/linux-mediatek/20220725081853.1636444-1-msp@baylibre.com/
v3 - https://lore.kernel.org/linux-mediatek/20220822144303.3438467-1-msp@baylibre.com/

Alexandre Bailon (2):
  soc: mediatek: Add support for WAY_EN operations
  soc: mediatek: Add support for MTK_SCPD_STRICT_BUS_PROTECTION cap

Fabien Parent (2):
  dt-bindings: power: Add MT8365 power domains
  soc: mediatek: pm-domains: Add support for MT8365

Markus Schneider-Pargmann (4):
  soc: mediatek: pm-domains: Split bus_prot_mask
  soc: mediatek: pm-domains: Create bus protection operation functions
  soc: mediatek: pm-domains: Document scpsys_bus_prot_data
  soc: mediatek: pm-domains: Fix caps field documentation

 .../power/mediatek,power-controller.yaml      |   6 +
 drivers/soc/mediatek/mt8365-pm-domains.h      | 147 ++++++++++++++++++
 drivers/soc/mediatek/mtk-pm-domains.c         | 135 +++++++++++-----
 drivers/soc/mediatek/mtk-pm-domains.h         |  37 ++++-
 .../dt-bindings/power/mediatek,mt8365-power.h |  19 +++
 5 files changed, 302 insertions(+), 42 deletions(-)
 create mode 100644 drivers/soc/mediatek/mt8365-pm-domains.h
 create mode 100644 include/dt-bindings/power/mediatek,mt8365-power.h

-- 
2.39.0

[PATCH 0/8] soc: mediatek: MT8365 power support

[Markus Schneider-Pargmann]

Hi,

sorry for the long delay, here is an update to the mt8365 power domain
series. Thanks for all your feedback, I hope I addressed everything. As
Matthias brought up some good points, I got back to try to make the
WAY_EN feature a bit more understandable with less code duplication. I
added a few patches to prepare the driver for the WAY_EN patch. I hope
it is better to understand now.

Thank you!

Best,
Markus

Changes in v4:
- Redesigned WAY_EN patch and split it up in smaller patches.
- Added two documentation patches.
- Added mediatek,infracfg-nao field to the binding.

Changes in v3:
- Mainly redesigned WAY_EN patch to be easier to understand
- Rebased onto v6.0-rc1
- Several other stuff that is described in the individual patches

Changes in v2:
- Updated error handling path for scpsys_power_on()
- Minor updates described in each patch

Previous versions:
v1 - https://lore.kernel.org/linux-mediatek/20220530204214.913251-1-fparent@baylibre.com/
v2 - https://lore.kernel.org/linux-mediatek/20220725081853.1636444-1-msp@baylibre.com/
v3 - https://lore.kernel.org/linux-mediatek/20220822144303.3438467-1-msp@baylibre.com/

Alexandre Bailon (2):
  soc: mediatek: Add support for WAY_EN operations
  soc: mediatek: Add support for MTK_SCPD_STRICT_BUS_PROTECTION cap

Fabien Parent (2):
  dt-bindings: power: Add MT8365 power domains
  soc: mediatek: pm-domains: Add support for MT8365

Markus Schneider-Pargmann (4):
  soc: mediatek: pm-domains: Split bus_prot_mask
  soc: mediatek: pm-domains: Create bus protection operation functions
  soc: mediatek: pm-domains: Document scpsys_bus_prot_data
  soc: mediatek: pm-domains: Fix caps field documentation

 .../power/mediatek,power-controller.yaml      |   6 +
 drivers/soc/mediatek/mt8365-pm-domains.h      | 147 ++++++++++++++++++
 drivers/soc/mediatek/mtk-pm-domains.c         | 135 +++++++++++-----
 drivers/soc/mediatek/mtk-pm-domains.h         |  37 ++++-
 .../dt-bindings/power/mediatek,mt8365-power.h |  19 +++
 5 files changed, 302 insertions(+), 42 deletions(-)
 create mode 100644 drivers/soc/mediatek/mt8365-pm-domains.h
 create mode 100644 include/dt-bindings/power/mediatek,mt8365-power.h

-- 
2.39.0


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

[PATCH 1/8] dt-bindings: power: Add MT8365 power domains

[Markus Schneider-Pargmann]

From: Fabien Parent <fparent@baylibre.com>

Add power domains dt-bindings for MT8365.

Signed-off-by: Fabien Parent <fparent@baylibre.com>
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---

Notes:
    Changes in v4:
    - Add infracfg_nao as it is used by mt8365
    
    Changes in v3:
    - Renamed mt8365-power.h to mediatek,mt8365-power.h
    
    Changes in v2:
    - Made include/dt-bindings/power/mt8365-power.h dual-license.

 .../power/mediatek,power-controller.yaml      |  6 ++++++
 .../dt-bindings/power/mediatek,mt8365-power.h | 19 +++++++++++++++++++
 2 files changed, 25 insertions(+)
 create mode 100644 include/dt-bindings/power/mediatek,mt8365-power.h

diff --git a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
index 605ec7ab5f63..a496c43cfa16 100644
--- a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
+++ b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
@@ -30,6 +30,7 @@ properties:
       - mediatek,mt8186-power-controller
       - mediatek,mt8192-power-controller
       - mediatek,mt8195-power-controller
+      - mediatek,mt8365-power-controller
 
   '#power-domain-cells':
     const: 1
@@ -86,6 +87,7 @@ $defs:
               "include/dt-bindings/power/mt8183-power.h" - for MT8183 type power domain.
               "include/dt-bindings/power/mt8192-power.h" - for MT8192 type power domain.
               "include/dt-bindings/power/mt8195-power.h" - for MT8195 type power domain.
+              "include/dt-bindings/power/mediatek,mt8365-power.h" - for MT8365 type power domain.
         maxItems: 1
 
       clocks:
@@ -113,6 +115,10 @@ $defs:
         $ref: /schemas/types.yaml#/definitions/phandle
         description: phandle to the device containing the INFRACFG register range.
 
+      mediatek,infracfg-nao:
+        $ref: /schemas/types.yaml#/definitions/phandle
+        description: phandle to the device containing the INFRACFG-NAO register range.
+
       mediatek,smi:
         $ref: /schemas/types.yaml#/definitions/phandle
         description: phandle to the device containing the SMI register range.
diff --git a/include/dt-bindings/power/mediatek,mt8365-power.h b/include/dt-bindings/power/mediatek,mt8365-power.h
new file mode 100644
index 000000000000..e6cfd0ec7871
--- /dev/null
+++ b/include/dt-bindings/power/mediatek,mt8365-power.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
+/*
+ * Copyright (c) 2022 MediaTek Inc.
+ */
+
+#ifndef _DT_BINDINGS_POWER_MT8365_POWER_H
+#define _DT_BINDINGS_POWER_MT8365_POWER_H
+
+#define MT8365_POWER_DOMAIN_MM		0
+#define MT8365_POWER_DOMAIN_CONN	1
+#define MT8365_POWER_DOMAIN_MFG		2
+#define MT8365_POWER_DOMAIN_AUDIO	3
+#define MT8365_POWER_DOMAIN_CAM		4
+#define MT8365_POWER_DOMAIN_DSP		5
+#define MT8365_POWER_DOMAIN_VDEC	6
+#define MT8365_POWER_DOMAIN_VENC	7
+#define MT8365_POWER_DOMAIN_APU		8
+
+#endif /* _DT_BINDINGS_POWER_MT8365_POWER_H */
-- 
2.39.0

[PATCH 1/8] dt-bindings: power: Add MT8365 power domains

[Markus Schneider-Pargmann]

From: Fabien Parent <fparent@baylibre.com>

Add power domains dt-bindings for MT8365.

Signed-off-by: Fabien Parent <fparent@baylibre.com>
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---

Notes:
    Changes in v4:
    - Add infracfg_nao as it is used by mt8365
    
    Changes in v3:
    - Renamed mt8365-power.h to mediatek,mt8365-power.h
    
    Changes in v2:
    - Made include/dt-bindings/power/mt8365-power.h dual-license.

 .../power/mediatek,power-controller.yaml      |  6 ++++++
 .../dt-bindings/power/mediatek,mt8365-power.h | 19 +++++++++++++++++++
 2 files changed, 25 insertions(+)
 create mode 100644 include/dt-bindings/power/mediatek,mt8365-power.h

diff --git a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
index 605ec7ab5f63..a496c43cfa16 100644
--- a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
+++ b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
@@ -30,6 +30,7 @@ properties:
       - mediatek,mt8186-power-controller
       - mediatek,mt8192-power-controller
       - mediatek,mt8195-power-controller
+      - mediatek,mt8365-power-controller
 
   '#power-domain-cells':
     const: 1
@@ -86,6 +87,7 @@ $defs:
               "include/dt-bindings/power/mt8183-power.h" - for MT8183 type power domain.
               "include/dt-bindings/power/mt8192-power.h" - for MT8192 type power domain.
               "include/dt-bindings/power/mt8195-power.h" - for MT8195 type power domain.
+              "include/dt-bindings/power/mediatek,mt8365-power.h" - for MT8365 type power domain.
         maxItems: 1
 
       clocks:
@@ -113,6 +115,10 @@ $defs:
         $ref: /schemas/types.yaml#/definitions/phandle
         description: phandle to the device containing the INFRACFG register range.
 
+      mediatek,infracfg-nao:
+        $ref: /schemas/types.yaml#/definitions/phandle
+        description: phandle to the device containing the INFRACFG-NAO register range.
+
       mediatek,smi:
         $ref: /schemas/types.yaml#/definitions/phandle
         description: phandle to the device containing the SMI register range.
diff --git a/include/dt-bindings/power/mediatek,mt8365-power.h b/include/dt-bindings/power/mediatek,mt8365-power.h
new file mode 100644
index 000000000000..e6cfd0ec7871
--- /dev/null
+++ b/include/dt-bindings/power/mediatek,mt8365-power.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
+/*
+ * Copyright (c) 2022 MediaTek Inc.
+ */
+
+#ifndef _DT_BINDINGS_POWER_MT8365_POWER_H
+#define _DT_BINDINGS_POWER_MT8365_POWER_H
+
+#define MT8365_POWER_DOMAIN_MM		0
+#define MT8365_POWER_DOMAIN_CONN	1
+#define MT8365_POWER_DOMAIN_MFG		2
+#define MT8365_POWER_DOMAIN_AUDIO	3
+#define MT8365_POWER_DOMAIN_CAM		4
+#define MT8365_POWER_DOMAIN_DSP		5
+#define MT8365_POWER_DOMAIN_VDEC	6
+#define MT8365_POWER_DOMAIN_VENC	7
+#define MT8365_POWER_DOMAIN_APU		8
+
+#endif /* _DT_BINDINGS_POWER_MT8365_POWER_H */
-- 
2.39.0


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

Re: [PATCH 1/8] dt-bindings: power: Add MT8365 power domains

[Rob Herring]

On Thu, 05 Jan 2023 18:07:28 +0100, Markus Schneider-Pargmann wrote:
> From: Fabien Parent <fparent@baylibre.com>
> 
> Add power domains dt-bindings for MT8365.
> 
> Signed-off-by: Fabien Parent <fparent@baylibre.com>
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
> 
> Notes:
>     Changes in v4:
>     - Add infracfg_nao as it is used by mt8365
> 
>     Changes in v3:
>     - Renamed mt8365-power.h to mediatek,mt8365-power.h
> 
>     Changes in v2:
>     - Made include/dt-bindings/power/mt8365-power.h dual-license.
> 
>  .../power/mediatek,power-controller.yaml      |  6 ++++++
>  .../dt-bindings/power/mediatek,mt8365-power.h | 19 +++++++++++++++++++
>  2 files changed, 25 insertions(+)
>  create mode 100644 include/dt-bindings/power/mediatek,mt8365-power.h
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH 1/8] dt-bindings: power: Add MT8365 power domains

[Rob Herring]

On Thu, 05 Jan 2023 18:07:28 +0100, Markus Schneider-Pargmann wrote:
> From: Fabien Parent <fparent@baylibre.com>
> 
> Add power domains dt-bindings for MT8365.
> 
> Signed-off-by: Fabien Parent <fparent@baylibre.com>
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
> 
> Notes:
>     Changes in v4:
>     - Add infracfg_nao as it is used by mt8365
> 
>     Changes in v3:
>     - Renamed mt8365-power.h to mediatek,mt8365-power.h
> 
>     Changes in v2:
>     - Made include/dt-bindings/power/mt8365-power.h dual-license.
> 
>  .../power/mediatek,power-controller.yaml      |  6 ++++++
>  .../dt-bindings/power/mediatek,mt8365-power.h | 19 +++++++++++++++++++
>  2 files changed, 25 insertions(+)
>  create mode 100644 include/dt-bindings/power/mediatek,mt8365-power.h
> 

Reviewed-by: Rob Herring <robh@kernel.org>

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

Re: [PATCH 1/8] dt-bindings: power: Add MT8365 power domains

[Matthias Brugger]

On 05/01/2023 18:07, Markus Schneider-Pargmann wrote:
> From: Fabien Parent <fparent@baylibre.com>
> 
> Add power domains dt-bindings for MT8365.
> 
> Signed-off-by: Fabien Parent <fparent@baylibre.com>
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
> 
> Notes:
>      Changes in v4:
>      - Add infracfg_nao as it is used by mt8365
>      
>      Changes in v3:
>      - Renamed mt8365-power.h to mediatek,mt8365-power.h
>      
>      Changes in v2:
>      - Made include/dt-bindings/power/mt8365-power.h dual-license.
> 
>   .../power/mediatek,power-controller.yaml      |  6 ++++++
>   .../dt-bindings/power/mediatek,mt8365-power.h | 19 +++++++++++++++++++
>   2 files changed, 25 insertions(+)
>   create mode 100644 include/dt-bindings/power/mediatek,mt8365-power.h
> 
> diff --git a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> index 605ec7ab5f63..a496c43cfa16 100644
> --- a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> +++ b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> @@ -30,6 +30,7 @@ properties:
>         - mediatek,mt8186-power-controller
>         - mediatek,mt8192-power-controller
>         - mediatek,mt8195-power-controller
> +      - mediatek,mt8365-power-controller
>   
>     '#power-domain-cells':
>       const: 1
> @@ -86,6 +87,7 @@ $defs:
>                 "include/dt-bindings/power/mt8183-power.h" - for MT8183 type power domain.
>                 "include/dt-bindings/power/mt8192-power.h" - for MT8192 type power domain.
>                 "include/dt-bindings/power/mt8195-power.h" - for MT8195 type power domain.
> +              "include/dt-bindings/power/mediatek,mt8365-power.h" - for MT8365 type power domain.
>           maxItems: 1
>   
>         clocks:
> @@ -113,6 +115,10 @@ $defs:
>           $ref: /schemas/types.yaml#/definitions/phandle
>           description: phandle to the device containing the INFRACFG register range.
>   
> +      mediatek,infracfg-nao:
> +        $ref: /schemas/types.yaml#/definitions/phandle
> +        description: phandle to the device containing the INFRACFG-NAO register range.
> +


Hm, so we have mediatek,mt8365-infracfg defined in mt8365-sys-clock. Also the 
description talks about infracfg_ao. You now introduce INFRACFG-NAO. Is that 
just another system clock block, or is this covered with the 
mediatek,mt8365-infracfg binding?

We would need to clean that up.

Regards,
Matthias

>         mediatek,smi:
>           $ref: /schemas/types.yaml#/definitions/phandle
>           description: phandle to the device containing the SMI register range.
> diff --git a/include/dt-bindings/power/mediatek,mt8365-power.h b/include/dt-bindings/power/mediatek,mt8365-power.h
> new file mode 100644
> index 000000000000..e6cfd0ec7871
> --- /dev/null
> +++ b/include/dt-bindings/power/mediatek,mt8365-power.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
> +/*
> + * Copyright (c) 2022 MediaTek Inc.
> + */
> +
> +#ifndef _DT_BINDINGS_POWER_MT8365_POWER_H
> +#define _DT_BINDINGS_POWER_MT8365_POWER_H
> +
> +#define MT8365_POWER_DOMAIN_MM		0
> +#define MT8365_POWER_DOMAIN_CONN	1
> +#define MT8365_POWER_DOMAIN_MFG		2
> +#define MT8365_POWER_DOMAIN_AUDIO	3
> +#define MT8365_POWER_DOMAIN_CAM		4
> +#define MT8365_POWER_DOMAIN_DSP		5
> +#define MT8365_POWER_DOMAIN_VDEC	6
> +#define MT8365_POWER_DOMAIN_VENC	7
> +#define MT8365_POWER_DOMAIN_APU		8
> +
> +#endif /* _DT_BINDINGS_POWER_MT8365_POWER_H */

Re: [PATCH 1/8] dt-bindings: power: Add MT8365 power domains

[Matthias Brugger]

On 05/01/2023 18:07, Markus Schneider-Pargmann wrote:
> From: Fabien Parent <fparent@baylibre.com>
> 
> Add power domains dt-bindings for MT8365.
> 
> Signed-off-by: Fabien Parent <fparent@baylibre.com>
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
> 
> Notes:
>      Changes in v4:
>      - Add infracfg_nao as it is used by mt8365
>      
>      Changes in v3:
>      - Renamed mt8365-power.h to mediatek,mt8365-power.h
>      
>      Changes in v2:
>      - Made include/dt-bindings/power/mt8365-power.h dual-license.
> 
>   .../power/mediatek,power-controller.yaml      |  6 ++++++
>   .../dt-bindings/power/mediatek,mt8365-power.h | 19 +++++++++++++++++++
>   2 files changed, 25 insertions(+)
>   create mode 100644 include/dt-bindings/power/mediatek,mt8365-power.h
> 
> diff --git a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> index 605ec7ab5f63..a496c43cfa16 100644
> --- a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> +++ b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> @@ -30,6 +30,7 @@ properties:
>         - mediatek,mt8186-power-controller
>         - mediatek,mt8192-power-controller
>         - mediatek,mt8195-power-controller
> +      - mediatek,mt8365-power-controller
>   
>     '#power-domain-cells':
>       const: 1
> @@ -86,6 +87,7 @@ $defs:
>                 "include/dt-bindings/power/mt8183-power.h" - for MT8183 type power domain.
>                 "include/dt-bindings/power/mt8192-power.h" - for MT8192 type power domain.
>                 "include/dt-bindings/power/mt8195-power.h" - for MT8195 type power domain.
> +              "include/dt-bindings/power/mediatek,mt8365-power.h" - for MT8365 type power domain.
>           maxItems: 1
>   
>         clocks:
> @@ -113,6 +115,10 @@ $defs:
>           $ref: /schemas/types.yaml#/definitions/phandle
>           description: phandle to the device containing the INFRACFG register range.
>   
> +      mediatek,infracfg-nao:
> +        $ref: /schemas/types.yaml#/definitions/phandle
> +        description: phandle to the device containing the INFRACFG-NAO register range.
> +


Hm, so we have mediatek,mt8365-infracfg defined in mt8365-sys-clock. Also the 
description talks about infracfg_ao. You now introduce INFRACFG-NAO. Is that 
just another system clock block, or is this covered with the 
mediatek,mt8365-infracfg binding?

We would need to clean that up.

Regards,
Matthias

>         mediatek,smi:
>           $ref: /schemas/types.yaml#/definitions/phandle
>           description: phandle to the device containing the SMI register range.
> diff --git a/include/dt-bindings/power/mediatek,mt8365-power.h b/include/dt-bindings/power/mediatek,mt8365-power.h
> new file mode 100644
> index 000000000000..e6cfd0ec7871
> --- /dev/null
> +++ b/include/dt-bindings/power/mediatek,mt8365-power.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
> +/*
> + * Copyright (c) 2022 MediaTek Inc.
> + */
> +
> +#ifndef _DT_BINDINGS_POWER_MT8365_POWER_H
> +#define _DT_BINDINGS_POWER_MT8365_POWER_H
> +
> +#define MT8365_POWER_DOMAIN_MM		0
> +#define MT8365_POWER_DOMAIN_CONN	1
> +#define MT8365_POWER_DOMAIN_MFG		2
> +#define MT8365_POWER_DOMAIN_AUDIO	3
> +#define MT8365_POWER_DOMAIN_CAM		4
> +#define MT8365_POWER_DOMAIN_DSP		5
> +#define MT8365_POWER_DOMAIN_VDEC	6
> +#define MT8365_POWER_DOMAIN_VENC	7
> +#define MT8365_POWER_DOMAIN_APU		8
> +
> +#endif /* _DT_BINDINGS_POWER_MT8365_POWER_H */

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

Re: [PATCH 1/8] dt-bindings: power: Add MT8365 power domains

[Markus Schneider-Pargmann]

Hi Matthias,

On Fri, Feb 03, 2023 at 01:31:58PM +0100, Matthias Brugger wrote:
> 
> 
> On 05/01/2023 18:07, Markus Schneider-Pargmann wrote:
> > From: Fabien Parent <fparent@baylibre.com>
> > 
> > Add power domains dt-bindings for MT8365.
> > 
> > Signed-off-by: Fabien Parent <fparent@baylibre.com>
> > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> > ---
> > 
> > Notes:
> >      Changes in v4:
> >      - Add infracfg_nao as it is used by mt8365
> >      Changes in v3:
> >      - Renamed mt8365-power.h to mediatek,mt8365-power.h
> >      Changes in v2:
> >      - Made include/dt-bindings/power/mt8365-power.h dual-license.
> > 
> >   .../power/mediatek,power-controller.yaml      |  6 ++++++
> >   .../dt-bindings/power/mediatek,mt8365-power.h | 19 +++++++++++++++++++
> >   2 files changed, 25 insertions(+)
> >   create mode 100644 include/dt-bindings/power/mediatek,mt8365-power.h
> > 
> > diff --git a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> > index 605ec7ab5f63..a496c43cfa16 100644
> > --- a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> > +++ b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> > @@ -30,6 +30,7 @@ properties:
> >         - mediatek,mt8186-power-controller
> >         - mediatek,mt8192-power-controller
> >         - mediatek,mt8195-power-controller
> > +      - mediatek,mt8365-power-controller
> >     '#power-domain-cells':
> >       const: 1
> > @@ -86,6 +87,7 @@ $defs:
> >                 "include/dt-bindings/power/mt8183-power.h" - for MT8183 type power domain.
> >                 "include/dt-bindings/power/mt8192-power.h" - for MT8192 type power domain.
> >                 "include/dt-bindings/power/mt8195-power.h" - for MT8195 type power domain.
> > +              "include/dt-bindings/power/mediatek,mt8365-power.h" - for MT8365 type power domain.
> >           maxItems: 1
> >         clocks:
> > @@ -113,6 +115,10 @@ $defs:
> >           $ref: /schemas/types.yaml#/definitions/phandle
> >           description: phandle to the device containing the INFRACFG register range.
> > +      mediatek,infracfg-nao:
> > +        $ref: /schemas/types.yaml#/definitions/phandle
> > +        description: phandle to the device containing the INFRACFG-NAO register range.
> > +
> 
> 
> Hm, so we have mediatek,mt8365-infracfg defined in mt8365-sys-clock. Also
> the description talks about infracfg_ao. You now introduce INFRACFG-NAO. Is
> that just another system clock block, or is this covered with the
> mediatek,mt8365-infracfg binding?
> 
> We would need to clean that up.

Looking through the datasheet I don't see any clock related registers
for infracfg-nao. So I don't think it is another system clock block. It
seems to be a lot of status registers and debug registers, but not all
of them. It really doesn't seem to have a specific topic here. So from
my point of view it is just a syscon block. Should I add a
mediatek,mt8365-infracfg-nao compatible to the syscon binding
documentation?

Thanks,
Markus

> 
> Regards,
> Matthias
> 
> >         mediatek,smi:
> >           $ref: /schemas/types.yaml#/definitions/phandle
> >           description: phandle to the device containing the SMI register range.
> > diff --git a/include/dt-bindings/power/mediatek,mt8365-power.h b/include/dt-bindings/power/mediatek,mt8365-power.h
> > new file mode 100644
> > index 000000000000..e6cfd0ec7871
> > --- /dev/null
> > +++ b/include/dt-bindings/power/mediatek,mt8365-power.h
> > @@ -0,0 +1,19 @@
> > +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
> > +/*
> > + * Copyright (c) 2022 MediaTek Inc.
> > + */
> > +
> > +#ifndef _DT_BINDINGS_POWER_MT8365_POWER_H
> > +#define _DT_BINDINGS_POWER_MT8365_POWER_H
> > +
> > +#define MT8365_POWER_DOMAIN_MM		0
> > +#define MT8365_POWER_DOMAIN_CONN	1
> > +#define MT8365_POWER_DOMAIN_MFG		2
> > +#define MT8365_POWER_DOMAIN_AUDIO	3
> > +#define MT8365_POWER_DOMAIN_CAM		4
> > +#define MT8365_POWER_DOMAIN_DSP		5
> > +#define MT8365_POWER_DOMAIN_VDEC	6
> > +#define MT8365_POWER_DOMAIN_VENC	7
> > +#define MT8365_POWER_DOMAIN_APU		8
> > +
> > +#endif /* _DT_BINDINGS_POWER_MT8365_POWER_H */

Re: [PATCH 1/8] dt-bindings: power: Add MT8365 power domains

[Markus Schneider-Pargmann]

Hi Matthias,

On Fri, Feb 03, 2023 at 01:31:58PM +0100, Matthias Brugger wrote:
> 
> 
> On 05/01/2023 18:07, Markus Schneider-Pargmann wrote:
> > From: Fabien Parent <fparent@baylibre.com>
> > 
> > Add power domains dt-bindings for MT8365.
> > 
> > Signed-off-by: Fabien Parent <fparent@baylibre.com>
> > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> > ---
> > 
> > Notes:
> >      Changes in v4:
> >      - Add infracfg_nao as it is used by mt8365
> >      Changes in v3:
> >      - Renamed mt8365-power.h to mediatek,mt8365-power.h
> >      Changes in v2:
> >      - Made include/dt-bindings/power/mt8365-power.h dual-license.
> > 
> >   .../power/mediatek,power-controller.yaml      |  6 ++++++
> >   .../dt-bindings/power/mediatek,mt8365-power.h | 19 +++++++++++++++++++
> >   2 files changed, 25 insertions(+)
> >   create mode 100644 include/dt-bindings/power/mediatek,mt8365-power.h
> > 
> > diff --git a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> > index 605ec7ab5f63..a496c43cfa16 100644
> > --- a/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> > +++ b/Documentation/devicetree/bindings/power/mediatek,power-controller.yaml
> > @@ -30,6 +30,7 @@ properties:
> >         - mediatek,mt8186-power-controller
> >         - mediatek,mt8192-power-controller
> >         - mediatek,mt8195-power-controller
> > +      - mediatek,mt8365-power-controller
> >     '#power-domain-cells':
> >       const: 1
> > @@ -86,6 +87,7 @@ $defs:
> >                 "include/dt-bindings/power/mt8183-power.h" - for MT8183 type power domain.
> >                 "include/dt-bindings/power/mt8192-power.h" - for MT8192 type power domain.
> >                 "include/dt-bindings/power/mt8195-power.h" - for MT8195 type power domain.
> > +              "include/dt-bindings/power/mediatek,mt8365-power.h" - for MT8365 type power domain.
> >           maxItems: 1
> >         clocks:
> > @@ -113,6 +115,10 @@ $defs:
> >           $ref: /schemas/types.yaml#/definitions/phandle
> >           description: phandle to the device containing the INFRACFG register range.
> > +      mediatek,infracfg-nao:
> > +        $ref: /schemas/types.yaml#/definitions/phandle
> > +        description: phandle to the device containing the INFRACFG-NAO register range.
> > +
> 
> 
> Hm, so we have mediatek,mt8365-infracfg defined in mt8365-sys-clock. Also
> the description talks about infracfg_ao. You now introduce INFRACFG-NAO. Is
> that just another system clock block, or is this covered with the
> mediatek,mt8365-infracfg binding?
> 
> We would need to clean that up.

Looking through the datasheet I don't see any clock related registers
for infracfg-nao. So I don't think it is another system clock block. It
seems to be a lot of status registers and debug registers, but not all
of them. It really doesn't seem to have a specific topic here. So from
my point of view it is just a syscon block. Should I add a
mediatek,mt8365-infracfg-nao compatible to the syscon binding
documentation?

Thanks,
Markus

> 
> Regards,
> Matthias
> 
> >         mediatek,smi:
> >           $ref: /schemas/types.yaml#/definitions/phandle
> >           description: phandle to the device containing the SMI register range.
> > diff --git a/include/dt-bindings/power/mediatek,mt8365-power.h b/include/dt-bindings/power/mediatek,mt8365-power.h
> > new file mode 100644
> > index 000000000000..e6cfd0ec7871
> > --- /dev/null
> > +++ b/include/dt-bindings/power/mediatek,mt8365-power.h
> > @@ -0,0 +1,19 @@
> > +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
> > +/*
> > + * Copyright (c) 2022 MediaTek Inc.
> > + */
> > +
> > +#ifndef _DT_BINDINGS_POWER_MT8365_POWER_H
> > +#define _DT_BINDINGS_POWER_MT8365_POWER_H
> > +
> > +#define MT8365_POWER_DOMAIN_MM		0
> > +#define MT8365_POWER_DOMAIN_CONN	1
> > +#define MT8365_POWER_DOMAIN_MFG		2
> > +#define MT8365_POWER_DOMAIN_AUDIO	3
> > +#define MT8365_POWER_DOMAIN_CAM		4
> > +#define MT8365_POWER_DOMAIN_DSP		5
> > +#define MT8365_POWER_DOMAIN_VDEC	6
> > +#define MT8365_POWER_DOMAIN_VENC	7
> > +#define MT8365_POWER_DOMAIN_APU		8
> > +
> > +#endif /* _DT_BINDINGS_POWER_MT8365_POWER_H */

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

[PATCH 2/8] soc: mediatek: pm-domains: Split bus_prot_mask

[Markus Schneider-Pargmann]

bus_prot_mask is used for all operations, set clear and acknowledge. In
preparation of m8365 power domain support split this one mask into two,
one mask for set and clear, another one for acknowledge.

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 drivers/soc/mediatek/mtk-pm-domains.c | 24 ++++++++++++++----------
 drivers/soc/mediatek/mtk-pm-domains.h | 14 ++++++++------
 2 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
index 474b272f9b02..4333cd297405 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.c
+++ b/drivers/soc/mediatek/mtk-pm-domains.c
@@ -122,18 +122,20 @@ static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd, st
 	int i, ret;
 
 	for (i = 0; i < SPM_MAX_BUS_PROT_DATA; i++) {
-		u32 val, mask = bpd[i].bus_prot_mask;
+		u32 val;
+		u32 set_clr_mask = bpd[i].bus_prot_set_clr_mask;
+		u32 sta_mask = bpd[i].bus_prot_sta_mask;
 
-		if (!mask)
+		if (!set_clr_mask)
 			break;
 
 		if (bpd[i].bus_prot_reg_update)
-			regmap_set_bits(regmap, bpd[i].bus_prot_set, mask);
+			regmap_set_bits(regmap, bpd[i].bus_prot_set, set_clr_mask);
 		else
-			regmap_write(regmap, bpd[i].bus_prot_set, mask);
+			regmap_write(regmap, bpd[i].bus_prot_set, set_clr_mask);
 
 		ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
-					       val, (val & mask) == mask,
+					       val, (val & sta_mask) == sta_mask,
 					       MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
 		if (ret)
 			return ret;
@@ -159,21 +161,23 @@ static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
 	int i, ret;
 
 	for (i = SPM_MAX_BUS_PROT_DATA - 1; i >= 0; i--) {
-		u32 val, mask = bpd[i].bus_prot_mask;
+		u32 val;
+		u32 set_clr_mask = bpd[i].bus_prot_set_clr_mask;
+		u32 sta_mask = bpd[i].bus_prot_sta_mask;
 
-		if (!mask)
+		if (!set_clr_mask)
 			continue;
 
 		if (bpd[i].bus_prot_reg_update)
-			regmap_clear_bits(regmap, bpd[i].bus_prot_clr, mask);
+			regmap_clear_bits(regmap, bpd[i].bus_prot_clr, set_clr_mask);
 		else
-			regmap_write(regmap, bpd[i].bus_prot_clr, mask);
+			regmap_write(regmap, bpd[i].bus_prot_clr, set_clr_mask);
 
 		if (bpd[i].ignore_clr_ack)
 			continue;
 
 		ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
-					       val, !(val & mask),
+					       val, !(val & sta_mask),
 					       MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
 		if (ret)
 			return ret;
diff --git a/drivers/soc/mediatek/mtk-pm-domains.h b/drivers/soc/mediatek/mtk-pm-domains.h
index 7d3c0c36316c..8aaed1c939d7 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.h
+++ b/drivers/soc/mediatek/mtk-pm-domains.h
@@ -41,23 +41,24 @@
 
 #define SPM_MAX_BUS_PROT_DATA		6
 
-#define _BUS_PROT(_mask, _set, _clr, _sta, _update, _ignore) {	\
-		.bus_prot_mask = (_mask),			\
+#define _BUS_PROT(_set_clr_mask, _set, _clr, _sta_mask, _sta, _update, _ignore) {	\
+		.bus_prot_set_clr_mask = (_set_clr_mask),	\
 		.bus_prot_set = _set,				\
 		.bus_prot_clr = _clr,				\
+		.bus_prot_sta_mask = (_sta_mask),		\
 		.bus_prot_sta = _sta,				\
 		.bus_prot_reg_update = _update,			\
 		.ignore_clr_ack = _ignore,			\
 	}
 
 #define BUS_PROT_WR(_mask, _set, _clr, _sta)			\
-		_BUS_PROT(_mask, _set, _clr, _sta, false, false)
+		_BUS_PROT(_mask, _set, _clr, _mask, _sta, false, false)
 
 #define BUS_PROT_WR_IGN(_mask, _set, _clr, _sta)		\
-		_BUS_PROT(_mask, _set, _clr, _sta, false, true)
+		_BUS_PROT(_mask, _set, _clr, _mask, _sta, false, true)
 
 #define BUS_PROT_UPDATE(_mask, _set, _clr, _sta)		\
-		_BUS_PROT(_mask, _set, _clr, _sta, true, false)
+		_BUS_PROT(_mask, _set, _clr, _mask, _sta, true, false)
 
 #define BUS_PROT_UPDATE_TOPAXI(_mask)				\
 		BUS_PROT_UPDATE(_mask,				\
@@ -66,9 +67,10 @@
 				INFRA_TOPAXI_PROTECTSTA1)
 
 struct scpsys_bus_prot_data {
-	u32 bus_prot_mask;
+	u32 bus_prot_set_clr_mask;
 	u32 bus_prot_set;
 	u32 bus_prot_clr;
+	u32 bus_prot_sta_mask;
 	u32 bus_prot_sta;
 	bool bus_prot_reg_update;
 	bool ignore_clr_ack;
-- 
2.39.0

[PATCH 2/8] soc: mediatek: pm-domains: Split bus_prot_mask

[Markus Schneider-Pargmann]

bus_prot_mask is used for all operations, set clear and acknowledge. In
preparation of m8365 power domain support split this one mask into two,
one mask for set and clear, another one for acknowledge.

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 drivers/soc/mediatek/mtk-pm-domains.c | 24 ++++++++++++++----------
 drivers/soc/mediatek/mtk-pm-domains.h | 14 ++++++++------
 2 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
index 474b272f9b02..4333cd297405 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.c
+++ b/drivers/soc/mediatek/mtk-pm-domains.c
@@ -122,18 +122,20 @@ static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd, st
 	int i, ret;
 
 	for (i = 0; i < SPM_MAX_BUS_PROT_DATA; i++) {
-		u32 val, mask = bpd[i].bus_prot_mask;
+		u32 val;
+		u32 set_clr_mask = bpd[i].bus_prot_set_clr_mask;
+		u32 sta_mask = bpd[i].bus_prot_sta_mask;
 
-		if (!mask)
+		if (!set_clr_mask)
 			break;
 
 		if (bpd[i].bus_prot_reg_update)
-			regmap_set_bits(regmap, bpd[i].bus_prot_set, mask);
+			regmap_set_bits(regmap, bpd[i].bus_prot_set, set_clr_mask);
 		else
-			regmap_write(regmap, bpd[i].bus_prot_set, mask);
+			regmap_write(regmap, bpd[i].bus_prot_set, set_clr_mask);
 
 		ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
-					       val, (val & mask) == mask,
+					       val, (val & sta_mask) == sta_mask,
 					       MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
 		if (ret)
 			return ret;
@@ -159,21 +161,23 @@ static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
 	int i, ret;
 
 	for (i = SPM_MAX_BUS_PROT_DATA - 1; i >= 0; i--) {
-		u32 val, mask = bpd[i].bus_prot_mask;
+		u32 val;
+		u32 set_clr_mask = bpd[i].bus_prot_set_clr_mask;
+		u32 sta_mask = bpd[i].bus_prot_sta_mask;
 
-		if (!mask)
+		if (!set_clr_mask)
 			continue;
 
 		if (bpd[i].bus_prot_reg_update)
-			regmap_clear_bits(regmap, bpd[i].bus_prot_clr, mask);
+			regmap_clear_bits(regmap, bpd[i].bus_prot_clr, set_clr_mask);
 		else
-			regmap_write(regmap, bpd[i].bus_prot_clr, mask);
+			regmap_write(regmap, bpd[i].bus_prot_clr, set_clr_mask);
 
 		if (bpd[i].ignore_clr_ack)
 			continue;
 
 		ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
-					       val, !(val & mask),
+					       val, !(val & sta_mask),
 					       MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
 		if (ret)
 			return ret;
diff --git a/drivers/soc/mediatek/mtk-pm-domains.h b/drivers/soc/mediatek/mtk-pm-domains.h
index 7d3c0c36316c..8aaed1c939d7 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.h
+++ b/drivers/soc/mediatek/mtk-pm-domains.h
@@ -41,23 +41,24 @@
 
 #define SPM_MAX_BUS_PROT_DATA		6
 
-#define _BUS_PROT(_mask, _set, _clr, _sta, _update, _ignore) {	\
-		.bus_prot_mask = (_mask),			\
+#define _BUS_PROT(_set_clr_mask, _set, _clr, _sta_mask, _sta, _update, _ignore) {	\
+		.bus_prot_set_clr_mask = (_set_clr_mask),	\
 		.bus_prot_set = _set,				\
 		.bus_prot_clr = _clr,				\
+		.bus_prot_sta_mask = (_sta_mask),		\
 		.bus_prot_sta = _sta,				\
 		.bus_prot_reg_update = _update,			\
 		.ignore_clr_ack = _ignore,			\
 	}
 
 #define BUS_PROT_WR(_mask, _set, _clr, _sta)			\
-		_BUS_PROT(_mask, _set, _clr, _sta, false, false)
+		_BUS_PROT(_mask, _set, _clr, _mask, _sta, false, false)
 
 #define BUS_PROT_WR_IGN(_mask, _set, _clr, _sta)		\
-		_BUS_PROT(_mask, _set, _clr, _sta, false, true)
+		_BUS_PROT(_mask, _set, _clr, _mask, _sta, false, true)
 
 #define BUS_PROT_UPDATE(_mask, _set, _clr, _sta)		\
-		_BUS_PROT(_mask, _set, _clr, _sta, true, false)
+		_BUS_PROT(_mask, _set, _clr, _mask, _sta, true, false)
 
 #define BUS_PROT_UPDATE_TOPAXI(_mask)				\
 		BUS_PROT_UPDATE(_mask,				\
@@ -66,9 +67,10 @@
 				INFRA_TOPAXI_PROTECTSTA1)
 
 struct scpsys_bus_prot_data {
-	u32 bus_prot_mask;
+	u32 bus_prot_set_clr_mask;
 	u32 bus_prot_set;
 	u32 bus_prot_clr;
+	u32 bus_prot_sta_mask;
 	u32 bus_prot_sta;
 	bool bus_prot_reg_update;
 	bool ignore_clr_ack;
-- 
2.39.0


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

[PATCH 3/8] soc: mediatek: pm-domains: Create bus protection operation functions

[Markus Schneider-Pargmann]

Separate the register access used for bus protection enable/disable into
their own functions. These will be used later for WAY_EN bits.

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 drivers/soc/mediatek/mtk-pm-domains.c | 68 +++++++++++++++------------
 1 file changed, 39 insertions(+), 29 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
index 4333cd297405..999e1f6c86b0 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.c
+++ b/drivers/soc/mediatek/mtk-pm-domains.c
@@ -117,26 +117,50 @@ static int scpsys_sram_disable(struct scpsys_domain *pd)
 					MTK_POLL_TIMEOUT);
 }
 
+static int scpsys_bus_protect_clear(const struct scpsys_bus_prot_data *bpd,
+				    struct regmap *regmap)
+{
+	u32 val;
+	u32 sta_mask = bpd->bus_prot_sta_mask;
+
+	if (bpd->bus_prot_reg_update)
+		regmap_clear_bits(regmap, bpd->bus_prot_clr, bpd->bus_prot_set_clr_mask);
+	else
+		regmap_write(regmap, bpd->bus_prot_clr, bpd->bus_prot_set_clr_mask);
+
+	if (bpd->ignore_clr_ack)
+		return 0;
+
+	return regmap_read_poll_timeout(regmap, bpd->bus_prot_sta,
+					val, !(val & sta_mask),
+					MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
+}
+
+static int scpsys_bus_protect_set(const struct scpsys_bus_prot_data *bpd,
+				  struct regmap *regmap)
+{
+	u32 val;
+	u32 sta_mask = bpd->bus_prot_sta_mask;
+
+	if (bpd->bus_prot_reg_update)
+		regmap_set_bits(regmap, bpd->bus_prot_set, bpd->bus_prot_set_clr_mask);
+	else
+		regmap_write(regmap, bpd->bus_prot_set, bpd->bus_prot_set_clr_mask);
+
+	return regmap_read_poll_timeout(regmap, bpd->bus_prot_sta,
+					val, (val & sta_mask) == sta_mask,
+					MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
+}
+
 static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd, struct regmap *regmap)
 {
 	int i, ret;
 
 	for (i = 0; i < SPM_MAX_BUS_PROT_DATA; i++) {
-		u32 val;
-		u32 set_clr_mask = bpd[i].bus_prot_set_clr_mask;
-		u32 sta_mask = bpd[i].bus_prot_sta_mask;
-
-		if (!set_clr_mask)
+		if (!bpd[i].bus_prot_set_clr_mask)
 			break;
 
-		if (bpd[i].bus_prot_reg_update)
-			regmap_set_bits(regmap, bpd[i].bus_prot_set, set_clr_mask);
-		else
-			regmap_write(regmap, bpd[i].bus_prot_set, set_clr_mask);
-
-		ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
-					       val, (val & sta_mask) == sta_mask,
-					       MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
+		ret = scpsys_bus_protect_set(&bpd[i], regmap);
 		if (ret)
 			return ret;
 	}
@@ -161,24 +185,10 @@ static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
 	int i, ret;
 
 	for (i = SPM_MAX_BUS_PROT_DATA - 1; i >= 0; i--) {
-		u32 val;
-		u32 set_clr_mask = bpd[i].bus_prot_set_clr_mask;
-		u32 sta_mask = bpd[i].bus_prot_sta_mask;
-
-		if (!set_clr_mask)
-			continue;
-
-		if (bpd[i].bus_prot_reg_update)
-			regmap_clear_bits(regmap, bpd[i].bus_prot_clr, set_clr_mask);
-		else
-			regmap_write(regmap, bpd[i].bus_prot_clr, set_clr_mask);
-
-		if (bpd[i].ignore_clr_ack)
+		if (!bpd[i].bus_prot_set_clr_mask)
 			continue;
 
-		ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
-					       val, !(val & sta_mask),
-					       MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
+		ret = scpsys_bus_protect_clear(&bpd[i], regmap);
 		if (ret)
 			return ret;
 	}
-- 
2.39.0

[PATCH 3/8] soc: mediatek: pm-domains: Create bus protection operation functions

[Markus Schneider-Pargmann]

Separate the register access used for bus protection enable/disable into
their own functions. These will be used later for WAY_EN bits.

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 drivers/soc/mediatek/mtk-pm-domains.c | 68 +++++++++++++++------------
 1 file changed, 39 insertions(+), 29 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
index 4333cd297405..999e1f6c86b0 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.c
+++ b/drivers/soc/mediatek/mtk-pm-domains.c
@@ -117,26 +117,50 @@ static int scpsys_sram_disable(struct scpsys_domain *pd)
 					MTK_POLL_TIMEOUT);
 }
 
+static int scpsys_bus_protect_clear(const struct scpsys_bus_prot_data *bpd,
+				    struct regmap *regmap)
+{
+	u32 val;
+	u32 sta_mask = bpd->bus_prot_sta_mask;
+
+	if (bpd->bus_prot_reg_update)
+		regmap_clear_bits(regmap, bpd->bus_prot_clr, bpd->bus_prot_set_clr_mask);
+	else
+		regmap_write(regmap, bpd->bus_prot_clr, bpd->bus_prot_set_clr_mask);
+
+	if (bpd->ignore_clr_ack)
+		return 0;
+
+	return regmap_read_poll_timeout(regmap, bpd->bus_prot_sta,
+					val, !(val & sta_mask),
+					MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
+}
+
+static int scpsys_bus_protect_set(const struct scpsys_bus_prot_data *bpd,
+				  struct regmap *regmap)
+{
+	u32 val;
+	u32 sta_mask = bpd->bus_prot_sta_mask;
+
+	if (bpd->bus_prot_reg_update)
+		regmap_set_bits(regmap, bpd->bus_prot_set, bpd->bus_prot_set_clr_mask);
+	else
+		regmap_write(regmap, bpd->bus_prot_set, bpd->bus_prot_set_clr_mask);
+
+	return regmap_read_poll_timeout(regmap, bpd->bus_prot_sta,
+					val, (val & sta_mask) == sta_mask,
+					MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
+}
+
 static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd, struct regmap *regmap)
 {
 	int i, ret;
 
 	for (i = 0; i < SPM_MAX_BUS_PROT_DATA; i++) {
-		u32 val;
-		u32 set_clr_mask = bpd[i].bus_prot_set_clr_mask;
-		u32 sta_mask = bpd[i].bus_prot_sta_mask;
-
-		if (!set_clr_mask)
+		if (!bpd[i].bus_prot_set_clr_mask)
 			break;
 
-		if (bpd[i].bus_prot_reg_update)
-			regmap_set_bits(regmap, bpd[i].bus_prot_set, set_clr_mask);
-		else
-			regmap_write(regmap, bpd[i].bus_prot_set, set_clr_mask);
-
-		ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
-					       val, (val & sta_mask) == sta_mask,
-					       MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
+		ret = scpsys_bus_protect_set(&bpd[i], regmap);
 		if (ret)
 			return ret;
 	}
@@ -161,24 +185,10 @@ static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
 	int i, ret;
 
 	for (i = SPM_MAX_BUS_PROT_DATA - 1; i >= 0; i--) {
-		u32 val;
-		u32 set_clr_mask = bpd[i].bus_prot_set_clr_mask;
-		u32 sta_mask = bpd[i].bus_prot_sta_mask;
-
-		if (!set_clr_mask)
-			continue;
-
-		if (bpd[i].bus_prot_reg_update)
-			regmap_clear_bits(regmap, bpd[i].bus_prot_clr, set_clr_mask);
-		else
-			regmap_write(regmap, bpd[i].bus_prot_clr, set_clr_mask);
-
-		if (bpd[i].ignore_clr_ack)
+		if (!bpd[i].bus_prot_set_clr_mask)
 			continue;
 
-		ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
-					       val, !(val & sta_mask),
-					       MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
+		ret = scpsys_bus_protect_clear(&bpd[i], regmap);
 		if (ret)
 			return ret;
 	}
-- 
2.39.0


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

Re: [PATCH 3/8] soc: mediatek: pm-domains: Create bus protection operation functions

[Matthias Brugger]

On 05/01/2023 18:07, Markus Schneider-Pargmann wrote:
> Separate the register access used for bus protection enable/disable into
> their own functions. These will be used later for WAY_EN bits.
> 
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
>   drivers/soc/mediatek/mtk-pm-domains.c | 68 +++++++++++++++------------
>   1 file changed, 39 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
> index 4333cd297405..999e1f6c86b0 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.c
> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
> @@ -117,26 +117,50 @@ static int scpsys_sram_disable(struct scpsys_domain *pd)
>   					MTK_POLL_TIMEOUT);
>   }
>   
> +static int scpsys_bus_protect_clear(const struct scpsys_bus_prot_data *bpd,
> +				    struct regmap *regmap)
> +{
> +	u32 val;
> +	u32 sta_mask = bpd->bus_prot_sta_mask;
> +
> +	if (bpd->bus_prot_reg_update)
> +		regmap_clear_bits(regmap, bpd->bus_prot_clr, bpd->bus_prot_set_clr_mask);
> +	else
> +		regmap_write(regmap, bpd->bus_prot_clr, bpd->bus_prot_set_clr_mask);
> +
> +	if (bpd->ignore_clr_ack)
> +		return 0;
> +
> +	return regmap_read_poll_timeout(regmap, bpd->bus_prot_sta,
> +					val, !(val & sta_mask),
> +					MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> +}
> +

NIT: please move this function below
static int scpsys_bus_protect_enable(struct scpsys_domain *pd) {...}

This allows for better readability.

> +static int scpsys_bus_protect_set(const struct scpsys_bus_prot_data *bpd,
> +				  struct regmap *regmap)
> +{
> +	u32 val;
> +	u32 sta_mask = bpd->bus_prot_sta_mask;
> +
> +	if (bpd->bus_prot_reg_update)
> +		regmap_set_bits(regmap, bpd->bus_prot_set, bpd->bus_prot_set_clr_mask);
> +	else
> +		regmap_write(regmap, bpd->bus_prot_set, bpd->bus_prot_set_clr_mask);
> +
> +	return regmap_read_poll_timeout(regmap, bpd->bus_prot_sta,
> +					val, (val & sta_mask) == sta_mask,
> +					MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> +}
> +
>   static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd, struct regmap *regmap)
>   {
>   	int i, ret;
>   
>   	for (i = 0; i < SPM_MAX_BUS_PROT_DATA; i++) {
> -		u32 val;
> -		u32 set_clr_mask = bpd[i].bus_prot_set_clr_mask;
> -		u32 sta_mask = bpd[i].bus_prot_sta_mask;
> -
> -		if (!set_clr_mask)
> +		if (!bpd[i].bus_prot_set_clr_mask)
>   			break;
>   
> -		if (bpd[i].bus_prot_reg_update)
> -			regmap_set_bits(regmap, bpd[i].bus_prot_set, set_clr_mask);
> -		else
> -			regmap_write(regmap, bpd[i].bus_prot_set, set_clr_mask);
> -
> -		ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
> -					       val, (val & sta_mask) == sta_mask,
> -					       MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> +		ret = scpsys_bus_protect_set(&bpd[i], regmap);
>   		if (ret)
>   			return ret;
>   	}
> @@ -161,24 +185,10 @@ static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
>   	int i, ret;
>   
>   	for (i = SPM_MAX_BUS_PROT_DATA - 1; i >= 0; i--) {
> -		u32 val;
> -		u32 set_clr_mask = bpd[i].bus_prot_set_clr_mask;
> -		u32 sta_mask = bpd[i].bus_prot_sta_mask;
> -
> -		if (!set_clr_mask)
> -			continue;
> -
> -		if (bpd[i].bus_prot_reg_update)
> -			regmap_clear_bits(regmap, bpd[i].bus_prot_clr, set_clr_mask);
> -		else
> -			regmap_write(regmap, bpd[i].bus_prot_clr, set_clr_mask);
> -
> -		if (bpd[i].ignore_clr_ack)
> +		if (!bpd[i].bus_prot_set_clr_mask)
>   			continue;
>   
> -		ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
> -					       val, !(val & sta_mask),
> -					       MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> +		ret = scpsys_bus_protect_clear(&bpd[i], regmap);
>   		if (ret)
>   			return ret;
>   	}

Re: [PATCH 3/8] soc: mediatek: pm-domains: Create bus protection operation functions

[Matthias Brugger]

On 05/01/2023 18:07, Markus Schneider-Pargmann wrote:
> Separate the register access used for bus protection enable/disable into
> their own functions. These will be used later for WAY_EN bits.
> 
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
>   drivers/soc/mediatek/mtk-pm-domains.c | 68 +++++++++++++++------------
>   1 file changed, 39 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
> index 4333cd297405..999e1f6c86b0 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.c
> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
> @@ -117,26 +117,50 @@ static int scpsys_sram_disable(struct scpsys_domain *pd)
>   					MTK_POLL_TIMEOUT);
>   }
>   
> +static int scpsys_bus_protect_clear(const struct scpsys_bus_prot_data *bpd,
> +				    struct regmap *regmap)
> +{
> +	u32 val;
> +	u32 sta_mask = bpd->bus_prot_sta_mask;
> +
> +	if (bpd->bus_prot_reg_update)
> +		regmap_clear_bits(regmap, bpd->bus_prot_clr, bpd->bus_prot_set_clr_mask);
> +	else
> +		regmap_write(regmap, bpd->bus_prot_clr, bpd->bus_prot_set_clr_mask);
> +
> +	if (bpd->ignore_clr_ack)
> +		return 0;
> +
> +	return regmap_read_poll_timeout(regmap, bpd->bus_prot_sta,
> +					val, !(val & sta_mask),
> +					MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> +}
> +

NIT: please move this function below
static int scpsys_bus_protect_enable(struct scpsys_domain *pd) {...}

This allows for better readability.

> +static int scpsys_bus_protect_set(const struct scpsys_bus_prot_data *bpd,
> +				  struct regmap *regmap)
> +{
> +	u32 val;
> +	u32 sta_mask = bpd->bus_prot_sta_mask;
> +
> +	if (bpd->bus_prot_reg_update)
> +		regmap_set_bits(regmap, bpd->bus_prot_set, bpd->bus_prot_set_clr_mask);
> +	else
> +		regmap_write(regmap, bpd->bus_prot_set, bpd->bus_prot_set_clr_mask);
> +
> +	return regmap_read_poll_timeout(regmap, bpd->bus_prot_sta,
> +					val, (val & sta_mask) == sta_mask,
> +					MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> +}
> +
>   static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd, struct regmap *regmap)
>   {
>   	int i, ret;
>   
>   	for (i = 0; i < SPM_MAX_BUS_PROT_DATA; i++) {
> -		u32 val;
> -		u32 set_clr_mask = bpd[i].bus_prot_set_clr_mask;
> -		u32 sta_mask = bpd[i].bus_prot_sta_mask;
> -
> -		if (!set_clr_mask)
> +		if (!bpd[i].bus_prot_set_clr_mask)
>   			break;
>   
> -		if (bpd[i].bus_prot_reg_update)
> -			regmap_set_bits(regmap, bpd[i].bus_prot_set, set_clr_mask);
> -		else
> -			regmap_write(regmap, bpd[i].bus_prot_set, set_clr_mask);
> -
> -		ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
> -					       val, (val & sta_mask) == sta_mask,
> -					       MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> +		ret = scpsys_bus_protect_set(&bpd[i], regmap);
>   		if (ret)
>   			return ret;
>   	}
> @@ -161,24 +185,10 @@ static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
>   	int i, ret;
>   
>   	for (i = SPM_MAX_BUS_PROT_DATA - 1; i >= 0; i--) {
> -		u32 val;
> -		u32 set_clr_mask = bpd[i].bus_prot_set_clr_mask;
> -		u32 sta_mask = bpd[i].bus_prot_sta_mask;
> -
> -		if (!set_clr_mask)
> -			continue;
> -
> -		if (bpd[i].bus_prot_reg_update)
> -			regmap_clear_bits(regmap, bpd[i].bus_prot_clr, set_clr_mask);
> -		else
> -			regmap_write(regmap, bpd[i].bus_prot_clr, set_clr_mask);
> -
> -		if (bpd[i].ignore_clr_ack)
> +		if (!bpd[i].bus_prot_set_clr_mask)
>   			continue;
>   
> -		ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
> -					       val, !(val & sta_mask),
> -					       MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> +		ret = scpsys_bus_protect_clear(&bpd[i], regmap);
>   		if (ret)
>   			return ret;
>   	}

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

Re: [PATCH 3/8] soc: mediatek: pm-domains: Create bus protection operation functions

[Markus Schneider-Pargmann]

Hi Matthias,

On Fri, Feb 03, 2023 at 01:32:11PM +0100, Matthias Brugger wrote:
> 
> 
> On 05/01/2023 18:07, Markus Schneider-Pargmann wrote:
> > Separate the register access used for bus protection enable/disable into
> > their own functions. These will be used later for WAY_EN bits.
> > 
> > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> > ---
> >   drivers/soc/mediatek/mtk-pm-domains.c | 68 +++++++++++++++------------
> >   1 file changed, 39 insertions(+), 29 deletions(-)
> > 
> > diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
> > index 4333cd297405..999e1f6c86b0 100644
> > --- a/drivers/soc/mediatek/mtk-pm-domains.c
> > +++ b/drivers/soc/mediatek/mtk-pm-domains.c
> > @@ -117,26 +117,50 @@ static int scpsys_sram_disable(struct scpsys_domain *pd)
> >   					MTK_POLL_TIMEOUT);
> >   }
> > +static int scpsys_bus_protect_clear(const struct scpsys_bus_prot_data *bpd,
> > +				    struct regmap *regmap)
> > +{
> > +	u32 val;
> > +	u32 sta_mask = bpd->bus_prot_sta_mask;
> > +
> > +	if (bpd->bus_prot_reg_update)
> > +		regmap_clear_bits(regmap, bpd->bus_prot_clr, bpd->bus_prot_set_clr_mask);
> > +	else
> > +		regmap_write(regmap, bpd->bus_prot_clr, bpd->bus_prot_set_clr_mask);
> > +
> > +	if (bpd->ignore_clr_ack)
> > +		return 0;
> > +
> > +	return regmap_read_poll_timeout(regmap, bpd->bus_prot_sta,
> > +					val, !(val & sta_mask),
> > +					MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> > +}
> > +
> 
> NIT: please move this function below
> static int scpsys_bus_protect_enable(struct scpsys_domain *pd) {...}
> 
> This allows for better readability.

With the WAY_EN patch later in this series scpsys_bus_protect_enable()
is going to use scpsys_bus_protect_clear(). So I would need to add a
signature in that patch for scpsys_bus_protect_clear() or move it back
up. I would prefer to avoid adding a signature or moving so I added it
above these functions here. I could swap scpsys_bus_protect_set() and
scpscpsys_bus_protect_clear() if that helps readability.

Thank you,
Markus

> 
> > +static int scpsys_bus_protect_set(const struct scpsys_bus_prot_data *bpd,
> > +				  struct regmap *regmap)
> > +{
> > +	u32 val;
> > +	u32 sta_mask = bpd->bus_prot_sta_mask;
> > +
> > +	if (bpd->bus_prot_reg_update)
> > +		regmap_set_bits(regmap, bpd->bus_prot_set, bpd->bus_prot_set_clr_mask);
> > +	else
> > +		regmap_write(regmap, bpd->bus_prot_set, bpd->bus_prot_set_clr_mask);
> > +
> > +	return regmap_read_poll_timeout(regmap, bpd->bus_prot_sta,
> > +					val, (val & sta_mask) == sta_mask,
> > +					MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> > +}
> > +
> >   static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd, struct regmap *regmap)
> >   {
> >   	int i, ret;
> >   	for (i = 0; i < SPM_MAX_BUS_PROT_DATA; i++) {
> > -		u32 val;
> > -		u32 set_clr_mask = bpd[i].bus_prot_set_clr_mask;
> > -		u32 sta_mask = bpd[i].bus_prot_sta_mask;
> > -
> > -		if (!set_clr_mask)
> > +		if (!bpd[i].bus_prot_set_clr_mask)
> >   			break;
> > -		if (bpd[i].bus_prot_reg_update)
> > -			regmap_set_bits(regmap, bpd[i].bus_prot_set, set_clr_mask);
> > -		else
> > -			regmap_write(regmap, bpd[i].bus_prot_set, set_clr_mask);
> > -
> > -		ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
> > -					       val, (val & sta_mask) == sta_mask,
> > -					       MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> > +		ret = scpsys_bus_protect_set(&bpd[i], regmap);
> >   		if (ret)
> >   			return ret;
> >   	}
> > @@ -161,24 +185,10 @@ static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
> >   	int i, ret;
> >   	for (i = SPM_MAX_BUS_PROT_DATA - 1; i >= 0; i--) {
> > -		u32 val;
> > -		u32 set_clr_mask = bpd[i].bus_prot_set_clr_mask;
> > -		u32 sta_mask = bpd[i].bus_prot_sta_mask;
> > -
> > -		if (!set_clr_mask)
> > -			continue;
> > -
> > -		if (bpd[i].bus_prot_reg_update)
> > -			regmap_clear_bits(regmap, bpd[i].bus_prot_clr, set_clr_mask);
> > -		else
> > -			regmap_write(regmap, bpd[i].bus_prot_clr, set_clr_mask);
> > -
> > -		if (bpd[i].ignore_clr_ack)
> > +		if (!bpd[i].bus_prot_set_clr_mask)
> >   			continue;
> > -		ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
> > -					       val, !(val & sta_mask),
> > -					       MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> > +		ret = scpsys_bus_protect_clear(&bpd[i], regmap);
> >   		if (ret)
> >   			return ret;
> >   	}

Re: [PATCH 3/8] soc: mediatek: pm-domains: Create bus protection operation functions

[Markus Schneider-Pargmann]

Hi Matthias,

On Fri, Feb 03, 2023 at 01:32:11PM +0100, Matthias Brugger wrote:
> 
> 
> On 05/01/2023 18:07, Markus Schneider-Pargmann wrote:
> > Separate the register access used for bus protection enable/disable into
> > their own functions. These will be used later for WAY_EN bits.
> > 
> > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> > ---
> >   drivers/soc/mediatek/mtk-pm-domains.c | 68 +++++++++++++++------------
> >   1 file changed, 39 insertions(+), 29 deletions(-)
> > 
> > diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
> > index 4333cd297405..999e1f6c86b0 100644
> > --- a/drivers/soc/mediatek/mtk-pm-domains.c
> > +++ b/drivers/soc/mediatek/mtk-pm-domains.c
> > @@ -117,26 +117,50 @@ static int scpsys_sram_disable(struct scpsys_domain *pd)
> >   					MTK_POLL_TIMEOUT);
> >   }
> > +static int scpsys_bus_protect_clear(const struct scpsys_bus_prot_data *bpd,
> > +				    struct regmap *regmap)
> > +{
> > +	u32 val;
> > +	u32 sta_mask = bpd->bus_prot_sta_mask;
> > +
> > +	if (bpd->bus_prot_reg_update)
> > +		regmap_clear_bits(regmap, bpd->bus_prot_clr, bpd->bus_prot_set_clr_mask);
> > +	else
> > +		regmap_write(regmap, bpd->bus_prot_clr, bpd->bus_prot_set_clr_mask);
> > +
> > +	if (bpd->ignore_clr_ack)
> > +		return 0;
> > +
> > +	return regmap_read_poll_timeout(regmap, bpd->bus_prot_sta,
> > +					val, !(val & sta_mask),
> > +					MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> > +}
> > +
> 
> NIT: please move this function below
> static int scpsys_bus_protect_enable(struct scpsys_domain *pd) {...}
> 
> This allows for better readability.

With the WAY_EN patch later in this series scpsys_bus_protect_enable()
is going to use scpsys_bus_protect_clear(). So I would need to add a
signature in that patch for scpsys_bus_protect_clear() or move it back
up. I would prefer to avoid adding a signature or moving so I added it
above these functions here. I could swap scpsys_bus_protect_set() and
scpscpsys_bus_protect_clear() if that helps readability.

Thank you,
Markus

> 
> > +static int scpsys_bus_protect_set(const struct scpsys_bus_prot_data *bpd,
> > +				  struct regmap *regmap)
> > +{
> > +	u32 val;
> > +	u32 sta_mask = bpd->bus_prot_sta_mask;
> > +
> > +	if (bpd->bus_prot_reg_update)
> > +		regmap_set_bits(regmap, bpd->bus_prot_set, bpd->bus_prot_set_clr_mask);
> > +	else
> > +		regmap_write(regmap, bpd->bus_prot_set, bpd->bus_prot_set_clr_mask);
> > +
> > +	return regmap_read_poll_timeout(regmap, bpd->bus_prot_sta,
> > +					val, (val & sta_mask) == sta_mask,
> > +					MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> > +}
> > +
> >   static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd, struct regmap *regmap)
> >   {
> >   	int i, ret;
> >   	for (i = 0; i < SPM_MAX_BUS_PROT_DATA; i++) {
> > -		u32 val;
> > -		u32 set_clr_mask = bpd[i].bus_prot_set_clr_mask;
> > -		u32 sta_mask = bpd[i].bus_prot_sta_mask;
> > -
> > -		if (!set_clr_mask)
> > +		if (!bpd[i].bus_prot_set_clr_mask)
> >   			break;
> > -		if (bpd[i].bus_prot_reg_update)
> > -			regmap_set_bits(regmap, bpd[i].bus_prot_set, set_clr_mask);
> > -		else
> > -			regmap_write(regmap, bpd[i].bus_prot_set, set_clr_mask);
> > -
> > -		ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
> > -					       val, (val & sta_mask) == sta_mask,
> > -					       MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> > +		ret = scpsys_bus_protect_set(&bpd[i], regmap);
> >   		if (ret)
> >   			return ret;
> >   	}
> > @@ -161,24 +185,10 @@ static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
> >   	int i, ret;
> >   	for (i = SPM_MAX_BUS_PROT_DATA - 1; i >= 0; i--) {
> > -		u32 val;
> > -		u32 set_clr_mask = bpd[i].bus_prot_set_clr_mask;
> > -		u32 sta_mask = bpd[i].bus_prot_sta_mask;
> > -
> > -		if (!set_clr_mask)
> > -			continue;
> > -
> > -		if (bpd[i].bus_prot_reg_update)
> > -			regmap_clear_bits(regmap, bpd[i].bus_prot_clr, set_clr_mask);
> > -		else
> > -			regmap_write(regmap, bpd[i].bus_prot_clr, set_clr_mask);
> > -
> > -		if (bpd[i].ignore_clr_ack)
> > +		if (!bpd[i].bus_prot_set_clr_mask)
> >   			continue;
> > -		ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
> > -					       val, !(val & sta_mask),
> > -					       MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> > +		ret = scpsys_bus_protect_clear(&bpd[i], regmap);
> >   		if (ret)
> >   			return ret;
> >   	}

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

[PATCH 4/8] soc: mediatek: pm-domains: Document scpsys_bus_prot_data

[Markus Schneider-Pargmann]

Add a short documentation for the fields in struct scpsys_bus_prot_data.

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 drivers/soc/mediatek/mtk-pm-domains.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/soc/mediatek/mtk-pm-domains.h b/drivers/soc/mediatek/mtk-pm-domains.h
index 8aaed1c939d7..da827e91d462 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.h
+++ b/drivers/soc/mediatek/mtk-pm-domains.h
@@ -66,6 +66,18 @@
 				INFRA_TOPAXI_PROTECTEN,		\
 				INFRA_TOPAXI_PROTECTSTA1)
 
+/**
+ * struct scpsys_bus_prot_data - Bus protection setting
+ * @bus_prot_set_clr_mask: Bitmask used for the set and clear registers.
+ * @bus_prot_set: infracfg set register.
+ * @bus_prot_clr: infracfg clear register.
+ * @bus_prot_sta_mask: Bitmask used for the status register.
+ * @bus_prot_sta: infracfg status register.
+ * @bus_prot_reg_update: Only update the register bits given in the mask, do not
+ *			 write the whole register.
+ * @ignore_clk_ack: Ignore the result in the status register for clear
+ *		    operations.
+ */
 struct scpsys_bus_prot_data {
 	u32 bus_prot_set_clr_mask;
 	u32 bus_prot_set;
-- 
2.39.0

[PATCH 4/8] soc: mediatek: pm-domains: Document scpsys_bus_prot_data

[Markus Schneider-Pargmann]

Add a short documentation for the fields in struct scpsys_bus_prot_data.

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 drivers/soc/mediatek/mtk-pm-domains.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/soc/mediatek/mtk-pm-domains.h b/drivers/soc/mediatek/mtk-pm-domains.h
index 8aaed1c939d7..da827e91d462 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.h
+++ b/drivers/soc/mediatek/mtk-pm-domains.h
@@ -66,6 +66,18 @@
 				INFRA_TOPAXI_PROTECTEN,		\
 				INFRA_TOPAXI_PROTECTSTA1)
 
+/**
+ * struct scpsys_bus_prot_data - Bus protection setting
+ * @bus_prot_set_clr_mask: Bitmask used for the set and clear registers.
+ * @bus_prot_set: infracfg set register.
+ * @bus_prot_clr: infracfg clear register.
+ * @bus_prot_sta_mask: Bitmask used for the status register.
+ * @bus_prot_sta: infracfg status register.
+ * @bus_prot_reg_update: Only update the register bits given in the mask, do not
+ *			 write the whole register.
+ * @ignore_clk_ack: Ignore the result in the status register for clear
+ *		    operations.
+ */
 struct scpsys_bus_prot_data {
 	u32 bus_prot_set_clr_mask;
 	u32 bus_prot_set;
-- 
2.39.0


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

[PATCH 5/8] soc: mediatek: pm-domains: Fix caps field documentation

[Markus Schneider-Pargmann]

This field is not for ACTIVE_WAKEUP exclusively. There are a lot of
other flags defined as well.

Fixes: 59b644b01cf4 ("soc: mediatek: Add MediaTek SCPSYS power domains")
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 drivers/soc/mediatek/mtk-pm-domains.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/mediatek/mtk-pm-domains.h b/drivers/soc/mediatek/mtk-pm-domains.h
index da827e91d462..34642a279213 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.h
+++ b/drivers/soc/mediatek/mtk-pm-domains.h
@@ -95,7 +95,7 @@ struct scpsys_bus_prot_data {
  * @ctl_offs: The offset for main power control register.
  * @sram_pdn_bits: The mask for sram power control bits.
  * @sram_pdn_ack_bits: The mask for sram power control acked bits.
- * @caps: The flag for active wake-up action.
+ * @caps: MTK_SCPD_* capability flags.
  * @bp_infracfg: bus protection for infracfg subsystem
  * @bp_smi: bus protection for smi subsystem
  */
-- 
2.39.0

[PATCH 5/8] soc: mediatek: pm-domains: Fix caps field documentation

[Markus Schneider-Pargmann]

This field is not for ACTIVE_WAKEUP exclusively. There are a lot of
other flags defined as well.

Fixes: 59b644b01cf4 ("soc: mediatek: Add MediaTek SCPSYS power domains")
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 drivers/soc/mediatek/mtk-pm-domains.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/mediatek/mtk-pm-domains.h b/drivers/soc/mediatek/mtk-pm-domains.h
index da827e91d462..34642a279213 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.h
+++ b/drivers/soc/mediatek/mtk-pm-domains.h
@@ -95,7 +95,7 @@ struct scpsys_bus_prot_data {
  * @ctl_offs: The offset for main power control register.
  * @sram_pdn_bits: The mask for sram power control bits.
  * @sram_pdn_ack_bits: The mask for sram power control acked bits.
- * @caps: The flag for active wake-up action.
+ * @caps: MTK_SCPD_* capability flags.
  * @bp_infracfg: bus protection for infracfg subsystem
  * @bp_smi: bus protection for smi subsystem
  */
-- 
2.39.0


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

Re: [PATCH 5/8] soc: mediatek: pm-domains: Fix caps field documentation

[Matthias Brugger]

On 05/01/2023 18:07, Markus Schneider-Pargmann wrote:
> This field is not for ACTIVE_WAKEUP exclusively. There are a lot of
> other flags defined as well.
> 
> Fixes: 59b644b01cf4 ("soc: mediatek: Add MediaTek SCPSYS power domains")

No bug here, nothing that needs to be fixed in the stable kernels. We can drop 
the Fixes tag here.

> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
>   drivers/soc/mediatek/mtk-pm-domains.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.h b/drivers/soc/mediatek/mtk-pm-domains.h
> index da827e91d462..34642a279213 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.h
> +++ b/drivers/soc/mediatek/mtk-pm-domains.h
> @@ -95,7 +95,7 @@ struct scpsys_bus_prot_data {
>    * @ctl_offs: The offset for main power control register.
>    * @sram_pdn_bits: The mask for sram power control bits.
>    * @sram_pdn_ack_bits: The mask for sram power control acked bits.
> - * @caps: The flag for active wake-up action.
> + * @caps: MTK_SCPD_* capability flags.
>    * @bp_infracfg: bus protection for infracfg subsystem
>    * @bp_smi: bus protection for smi subsystem
>    */

Re: [PATCH 5/8] soc: mediatek: pm-domains: Fix caps field documentation

[Matthias Brugger]

On 05/01/2023 18:07, Markus Schneider-Pargmann wrote:
> This field is not for ACTIVE_WAKEUP exclusively. There are a lot of
> other flags defined as well.
> 
> Fixes: 59b644b01cf4 ("soc: mediatek: Add MediaTek SCPSYS power domains")

No bug here, nothing that needs to be fixed in the stable kernels. We can drop 
the Fixes tag here.

> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
>   drivers/soc/mediatek/mtk-pm-domains.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.h b/drivers/soc/mediatek/mtk-pm-domains.h
> index da827e91d462..34642a279213 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.h
> +++ b/drivers/soc/mediatek/mtk-pm-domains.h
> @@ -95,7 +95,7 @@ struct scpsys_bus_prot_data {
>    * @ctl_offs: The offset for main power control register.
>    * @sram_pdn_bits: The mask for sram power control bits.
>    * @sram_pdn_ack_bits: The mask for sram power control acked bits.
> - * @caps: The flag for active wake-up action.
> + * @caps: MTK_SCPD_* capability flags.
>    * @bp_infracfg: bus protection for infracfg subsystem
>    * @bp_smi: bus protection for smi subsystem
>    */

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

[PATCH 6/8] soc: mediatek: Add support for WAY_EN operations

[Markus Schneider-Pargmann]

From: Alexandre Bailon <abailon@baylibre.com>

This updates the power domain to support WAY_EN operations. These
operations enable a path between different units of the chip and are
labeled as 'way_en' in the register descriptions.

This operation is required by the mt8365 for the MM power domain.

Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
Signed-off-by: Fabien Parent <fparent@baylibre.com>
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---

Notes:
    Changes in v4:
    - Redesigned the patch to be better to understand and have less code
      duplication
    
    Changes in v3:
    - Separated the way_en functions for clarity
    - Added some checks for infracfg_nao
    
    Changes in v2:
    - some minor style fixes.
    - Renamed 'wayen' to 'way_en' to clarify the meaning
    - Updated commit message

 drivers/soc/mediatek/mtk-pm-domains.c | 57 ++++++++++++++++++++-------
 drivers/soc/mediatek/mtk-pm-domains.h | 16 ++++++--
 2 files changed, 54 insertions(+), 19 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
index 999e1f6c86b0..d53309f050ee 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.c
+++ b/drivers/soc/mediatek/mtk-pm-domains.c
@@ -43,6 +43,7 @@ struct scpsys_domain {
 	struct clk_bulk_data *clks;
 	int num_subsys_clks;
 	struct clk_bulk_data *subsys_clks;
+	struct regmap *infracfg_nao;
 	struct regmap *infracfg;
 	struct regmap *smi;
 	struct regulator *supply;
@@ -118,10 +119,13 @@ static int scpsys_sram_disable(struct scpsys_domain *pd)
 }
 
 static int scpsys_bus_protect_clear(const struct scpsys_bus_prot_data *bpd,
-				    struct regmap *regmap)
+				    struct regmap *regmap,
+				    struct regmap *sta_regmap)
 {
 	u32 val;
 	u32 sta_mask = bpd->bus_prot_sta_mask;
+	/* way_en acknowledges successful clear with the bit being set */
+	u32 expected_ack = (bpd->way_en ? sta_mask : 0);
 
 	if (bpd->bus_prot_reg_update)
 		regmap_clear_bits(regmap, bpd->bus_prot_clr, bpd->bus_prot_set_clr_mask);
@@ -131,13 +135,14 @@ static int scpsys_bus_protect_clear(const struct scpsys_bus_prot_data *bpd,
 	if (bpd->ignore_clr_ack)
 		return 0;
 
-	return regmap_read_poll_timeout(regmap, bpd->bus_prot_sta,
-					val, !(val & sta_mask),
+	return regmap_read_poll_timeout(sta_regmap, bpd->bus_prot_sta,
+					val, (val & sta_mask) == expected_ack,
 					MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
 }
 
 static int scpsys_bus_protect_set(const struct scpsys_bus_prot_data *bpd,
-				  struct regmap *regmap)
+				  struct regmap *regmap,
+				  struct regmap *sta_regmap)
 {
 	u32 val;
 	u32 sta_mask = bpd->bus_prot_sta_mask;
@@ -147,12 +152,13 @@ static int scpsys_bus_protect_set(const struct scpsys_bus_prot_data *bpd,
 	else
 		regmap_write(regmap, bpd->bus_prot_set, bpd->bus_prot_set_clr_mask);
 
-	return regmap_read_poll_timeout(regmap, bpd->bus_prot_sta,
+	return regmap_read_poll_timeout(sta_regmap, bpd->bus_prot_sta,
 					val, (val & sta_mask) == sta_mask,
 					MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
 }
 
-static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd, struct regmap *regmap)
+static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd,
+				      struct regmap *regmap, struct regmap *infracfg_nao)
 {
 	int i, ret;
 
@@ -160,9 +166,14 @@ static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd, st
 		if (!bpd[i].bus_prot_set_clr_mask)
 			break;
 
-		ret = scpsys_bus_protect_set(&bpd[i], regmap);
-		if (ret)
+		if (bpd[i].way_en)
+			ret = scpsys_bus_protect_clear(&bpd[i], regmap, infracfg_nao);
+		else
+			ret = scpsys_bus_protect_set(&bpd[i], regmap, regmap);
+		if (ret) {
+			pr_err("%s %d %d\n", __PRETTY_FUNCTION__, __LINE__, ret);
 			return ret;
+		}
 	}
 
 	return 0;
@@ -172,15 +183,17 @@ static int scpsys_bus_protect_enable(struct scpsys_domain *pd)
 {
 	int ret;
 
-	ret = _scpsys_bus_protect_enable(pd->data->bp_infracfg, pd->infracfg);
+	ret = _scpsys_bus_protect_enable(pd->data->bp_infracfg,
+					 pd->infracfg, pd->infracfg_nao);
 	if (ret)
 		return ret;
 
-	return _scpsys_bus_protect_enable(pd->data->bp_smi, pd->smi);
+	return _scpsys_bus_protect_enable(pd->data->bp_smi, pd->smi, NULL);
 }
 
 static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
-				       struct regmap *regmap)
+				       struct regmap *regmap,
+				       struct regmap *infracfg_nao)
 {
 	int i, ret;
 
@@ -188,9 +201,14 @@ static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
 		if (!bpd[i].bus_prot_set_clr_mask)
 			continue;
 
-		ret = scpsys_bus_protect_clear(&bpd[i], regmap);
-		if (ret)
+		if (bpd[i].way_en)
+			ret = scpsys_bus_protect_set(&bpd[i], regmap, infracfg_nao);
+		else
+			ret = scpsys_bus_protect_clear(&bpd[i], regmap, regmap);
+		if (ret) {
+			pr_err("%s %d %d\n", __PRETTY_FUNCTION__, __LINE__, ret);
 			return ret;
+		}
 	}
 
 	return 0;
@@ -200,11 +218,12 @@ static int scpsys_bus_protect_disable(struct scpsys_domain *pd)
 {
 	int ret;
 
-	ret = _scpsys_bus_protect_disable(pd->data->bp_smi, pd->smi);
+	ret = _scpsys_bus_protect_disable(pd->data->bp_smi, pd->smi, NULL);
 	if (ret)
 		return ret;
 
-	return _scpsys_bus_protect_disable(pd->data->bp_infracfg, pd->infracfg);
+	return _scpsys_bus_protect_disable(pd->data->bp_infracfg,
+			pd->infracfg, pd->infracfg_nao);
 }
 
 static int scpsys_regulator_enable(struct regulator *supply)
@@ -378,6 +397,14 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
 			return ERR_CAST(pd->smi);
 	}
 
+	pd->infracfg_nao = syscon_regmap_lookup_by_phandle(node, "mediatek,infracfg-nao");
+	if (IS_ERR(pd->infracfg_nao)) {
+		if (MTK_SCPD_CAPS(pd, MTK_SCPD_HAS_WAY_EN))
+			return ERR_CAST(pd->infracfg_nao);
+
+		pd->infracfg_nao = NULL;
+	}
+
 	num_clks = of_clk_get_parent_count(node);
 	if (num_clks > 0) {
 		/* Calculate number of subsys_clks */
diff --git a/drivers/soc/mediatek/mtk-pm-domains.h b/drivers/soc/mediatek/mtk-pm-domains.h
index 34642a279213..1fa634509db1 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.h
+++ b/drivers/soc/mediatek/mtk-pm-domains.h
@@ -10,6 +10,7 @@
 #define MTK_SCPD_DOMAIN_SUPPLY		BIT(4)
 /* can't set MTK_SCPD_KEEP_DEFAULT_OFF at the same time */
 #define MTK_SCPD_ALWAYS_ON		BIT(5)
+#define MTK_SCPD_HAS_WAY_EN		BIT(6)
 #define MTK_SCPD_CAPS(_scpd, _x)	((_scpd)->data->caps & (_x))
 
 #define SPM_VDE_PWR_CON			0x0210
@@ -41,7 +42,7 @@
 
 #define SPM_MAX_BUS_PROT_DATA		6
 
-#define _BUS_PROT(_set_clr_mask, _set, _clr, _sta_mask, _sta, _update, _ignore) {	\
+#define _BUS_PROT(_set_clr_mask, _set, _clr, _sta_mask, _sta, _update, _ignore, _way_en) {	\
 		.bus_prot_set_clr_mask = (_set_clr_mask),	\
 		.bus_prot_set = _set,				\
 		.bus_prot_clr = _clr,				\
@@ -49,16 +50,20 @@
 		.bus_prot_sta = _sta,				\
 		.bus_prot_reg_update = _update,			\
 		.ignore_clr_ack = _ignore,			\
+		.way_en = _way_en,				\
 	}
 
 #define BUS_PROT_WR(_mask, _set, _clr, _sta)			\
-		_BUS_PROT(_mask, _set, _clr, _mask, _sta, false, false)
+		_BUS_PROT(_mask, _set, _clr, _mask, _sta, false, false, false)
 
 #define BUS_PROT_WR_IGN(_mask, _set, _clr, _sta)		\
-		_BUS_PROT(_mask, _set, _clr, _mask, _sta, false, true)
+		_BUS_PROT(_mask, _set, _clr, _mask, _sta, false, true, false)
 
 #define BUS_PROT_UPDATE(_mask, _set, _clr, _sta)		\
-		_BUS_PROT(_mask, _set, _clr, _mask, _sta, true, false)
+		_BUS_PROT(_mask, _set, _clr, _mask, _sta, true, false, false)
+
+#define BUS_PROT_WAY_EN(_set_mask, _set, _sta_mask, _sta)	\
+		_BUS_PROT(_set_mask, _set, _set, _sta_mask, _sta, true, false, true)
 
 #define BUS_PROT_UPDATE_TOPAXI(_mask)				\
 		BUS_PROT_UPDATE(_mask,				\
@@ -77,6 +82,8 @@
  *			 write the whole register.
  * @ignore_clk_ack: Ignore the result in the status register for clear
  *		    operations.
+ * @way_en: Enable a way in the SoC. For this type the status bit to acknowledge
+ *	    the change is always 1 on successful change.
  */
 struct scpsys_bus_prot_data {
 	u32 bus_prot_set_clr_mask;
@@ -86,6 +93,7 @@ struct scpsys_bus_prot_data {
 	u32 bus_prot_sta;
 	bool bus_prot_reg_update;
 	bool ignore_clr_ack;
+	bool way_en;
 };
 
 /**
-- 
2.39.0

[PATCH 6/8] soc: mediatek: Add support for WAY_EN operations

[Markus Schneider-Pargmann]

From: Alexandre Bailon <abailon@baylibre.com>

This updates the power domain to support WAY_EN operations. These
operations enable a path between different units of the chip and are
labeled as 'way_en' in the register descriptions.

This operation is required by the mt8365 for the MM power domain.

Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
Signed-off-by: Fabien Parent <fparent@baylibre.com>
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---

Notes:
    Changes in v4:
    - Redesigned the patch to be better to understand and have less code
      duplication
    
    Changes in v3:
    - Separated the way_en functions for clarity
    - Added some checks for infracfg_nao
    
    Changes in v2:
    - some minor style fixes.
    - Renamed 'wayen' to 'way_en' to clarify the meaning
    - Updated commit message

 drivers/soc/mediatek/mtk-pm-domains.c | 57 ++++++++++++++++++++-------
 drivers/soc/mediatek/mtk-pm-domains.h | 16 ++++++--
 2 files changed, 54 insertions(+), 19 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
index 999e1f6c86b0..d53309f050ee 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.c
+++ b/drivers/soc/mediatek/mtk-pm-domains.c
@@ -43,6 +43,7 @@ struct scpsys_domain {
 	struct clk_bulk_data *clks;
 	int num_subsys_clks;
 	struct clk_bulk_data *subsys_clks;
+	struct regmap *infracfg_nao;
 	struct regmap *infracfg;
 	struct regmap *smi;
 	struct regulator *supply;
@@ -118,10 +119,13 @@ static int scpsys_sram_disable(struct scpsys_domain *pd)
 }
 
 static int scpsys_bus_protect_clear(const struct scpsys_bus_prot_data *bpd,
-				    struct regmap *regmap)
+				    struct regmap *regmap,
+				    struct regmap *sta_regmap)
 {
 	u32 val;
 	u32 sta_mask = bpd->bus_prot_sta_mask;
+	/* way_en acknowledges successful clear with the bit being set */
+	u32 expected_ack = (bpd->way_en ? sta_mask : 0);
 
 	if (bpd->bus_prot_reg_update)
 		regmap_clear_bits(regmap, bpd->bus_prot_clr, bpd->bus_prot_set_clr_mask);
@@ -131,13 +135,14 @@ static int scpsys_bus_protect_clear(const struct scpsys_bus_prot_data *bpd,
 	if (bpd->ignore_clr_ack)
 		return 0;
 
-	return regmap_read_poll_timeout(regmap, bpd->bus_prot_sta,
-					val, !(val & sta_mask),
+	return regmap_read_poll_timeout(sta_regmap, bpd->bus_prot_sta,
+					val, (val & sta_mask) == expected_ack,
 					MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
 }
 
 static int scpsys_bus_protect_set(const struct scpsys_bus_prot_data *bpd,
-				  struct regmap *regmap)
+				  struct regmap *regmap,
+				  struct regmap *sta_regmap)
 {
 	u32 val;
 	u32 sta_mask = bpd->bus_prot_sta_mask;
@@ -147,12 +152,13 @@ static int scpsys_bus_protect_set(const struct scpsys_bus_prot_data *bpd,
 	else
 		regmap_write(regmap, bpd->bus_prot_set, bpd->bus_prot_set_clr_mask);
 
-	return regmap_read_poll_timeout(regmap, bpd->bus_prot_sta,
+	return regmap_read_poll_timeout(sta_regmap, bpd->bus_prot_sta,
 					val, (val & sta_mask) == sta_mask,
 					MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
 }
 
-static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd, struct regmap *regmap)
+static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd,
+				      struct regmap *regmap, struct regmap *infracfg_nao)
 {
 	int i, ret;
 
@@ -160,9 +166,14 @@ static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd, st
 		if (!bpd[i].bus_prot_set_clr_mask)
 			break;
 
-		ret = scpsys_bus_protect_set(&bpd[i], regmap);
-		if (ret)
+		if (bpd[i].way_en)
+			ret = scpsys_bus_protect_clear(&bpd[i], regmap, infracfg_nao);
+		else
+			ret = scpsys_bus_protect_set(&bpd[i], regmap, regmap);
+		if (ret) {
+			pr_err("%s %d %d\n", __PRETTY_FUNCTION__, __LINE__, ret);
 			return ret;
+		}
 	}
 
 	return 0;
@@ -172,15 +183,17 @@ static int scpsys_bus_protect_enable(struct scpsys_domain *pd)
 {
 	int ret;
 
-	ret = _scpsys_bus_protect_enable(pd->data->bp_infracfg, pd->infracfg);
+	ret = _scpsys_bus_protect_enable(pd->data->bp_infracfg,
+					 pd->infracfg, pd->infracfg_nao);
 	if (ret)
 		return ret;
 
-	return _scpsys_bus_protect_enable(pd->data->bp_smi, pd->smi);
+	return _scpsys_bus_protect_enable(pd->data->bp_smi, pd->smi, NULL);
 }
 
 static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
-				       struct regmap *regmap)
+				       struct regmap *regmap,
+				       struct regmap *infracfg_nao)
 {
 	int i, ret;
 
@@ -188,9 +201,14 @@ static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
 		if (!bpd[i].bus_prot_set_clr_mask)
 			continue;
 
-		ret = scpsys_bus_protect_clear(&bpd[i], regmap);
-		if (ret)
+		if (bpd[i].way_en)
+			ret = scpsys_bus_protect_set(&bpd[i], regmap, infracfg_nao);
+		else
+			ret = scpsys_bus_protect_clear(&bpd[i], regmap, regmap);
+		if (ret) {
+			pr_err("%s %d %d\n", __PRETTY_FUNCTION__, __LINE__, ret);
 			return ret;
+		}
 	}
 
 	return 0;
@@ -200,11 +218,12 @@ static int scpsys_bus_protect_disable(struct scpsys_domain *pd)
 {
 	int ret;
 
-	ret = _scpsys_bus_protect_disable(pd->data->bp_smi, pd->smi);
+	ret = _scpsys_bus_protect_disable(pd->data->bp_smi, pd->smi, NULL);
 	if (ret)
 		return ret;
 
-	return _scpsys_bus_protect_disable(pd->data->bp_infracfg, pd->infracfg);
+	return _scpsys_bus_protect_disable(pd->data->bp_infracfg,
+			pd->infracfg, pd->infracfg_nao);
 }
 
 static int scpsys_regulator_enable(struct regulator *supply)
@@ -378,6 +397,14 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
 			return ERR_CAST(pd->smi);
 	}
 
+	pd->infracfg_nao = syscon_regmap_lookup_by_phandle(node, "mediatek,infracfg-nao");
+	if (IS_ERR(pd->infracfg_nao)) {
+		if (MTK_SCPD_CAPS(pd, MTK_SCPD_HAS_WAY_EN))
+			return ERR_CAST(pd->infracfg_nao);
+
+		pd->infracfg_nao = NULL;
+	}
+
 	num_clks = of_clk_get_parent_count(node);
 	if (num_clks > 0) {
 		/* Calculate number of subsys_clks */
diff --git a/drivers/soc/mediatek/mtk-pm-domains.h b/drivers/soc/mediatek/mtk-pm-domains.h
index 34642a279213..1fa634509db1 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.h
+++ b/drivers/soc/mediatek/mtk-pm-domains.h
@@ -10,6 +10,7 @@
 #define MTK_SCPD_DOMAIN_SUPPLY		BIT(4)
 /* can't set MTK_SCPD_KEEP_DEFAULT_OFF at the same time */
 #define MTK_SCPD_ALWAYS_ON		BIT(5)
+#define MTK_SCPD_HAS_WAY_EN		BIT(6)
 #define MTK_SCPD_CAPS(_scpd, _x)	((_scpd)->data->caps & (_x))
 
 #define SPM_VDE_PWR_CON			0x0210
@@ -41,7 +42,7 @@
 
 #define SPM_MAX_BUS_PROT_DATA		6
 
-#define _BUS_PROT(_set_clr_mask, _set, _clr, _sta_mask, _sta, _update, _ignore) {	\
+#define _BUS_PROT(_set_clr_mask, _set, _clr, _sta_mask, _sta, _update, _ignore, _way_en) {	\
 		.bus_prot_set_clr_mask = (_set_clr_mask),	\
 		.bus_prot_set = _set,				\
 		.bus_prot_clr = _clr,				\
@@ -49,16 +50,20 @@
 		.bus_prot_sta = _sta,				\
 		.bus_prot_reg_update = _update,			\
 		.ignore_clr_ack = _ignore,			\
+		.way_en = _way_en,				\
 	}
 
 #define BUS_PROT_WR(_mask, _set, _clr, _sta)			\
-		_BUS_PROT(_mask, _set, _clr, _mask, _sta, false, false)
+		_BUS_PROT(_mask, _set, _clr, _mask, _sta, false, false, false)
 
 #define BUS_PROT_WR_IGN(_mask, _set, _clr, _sta)		\
-		_BUS_PROT(_mask, _set, _clr, _mask, _sta, false, true)
+		_BUS_PROT(_mask, _set, _clr, _mask, _sta, false, true, false)
 
 #define BUS_PROT_UPDATE(_mask, _set, _clr, _sta)		\
-		_BUS_PROT(_mask, _set, _clr, _mask, _sta, true, false)
+		_BUS_PROT(_mask, _set, _clr, _mask, _sta, true, false, false)
+
+#define BUS_PROT_WAY_EN(_set_mask, _set, _sta_mask, _sta)	\
+		_BUS_PROT(_set_mask, _set, _set, _sta_mask, _sta, true, false, true)
 
 #define BUS_PROT_UPDATE_TOPAXI(_mask)				\
 		BUS_PROT_UPDATE(_mask,				\
@@ -77,6 +82,8 @@
  *			 write the whole register.
  * @ignore_clk_ack: Ignore the result in the status register for clear
  *		    operations.
+ * @way_en: Enable a way in the SoC. For this type the status bit to acknowledge
+ *	    the change is always 1 on successful change.
  */
 struct scpsys_bus_prot_data {
 	u32 bus_prot_set_clr_mask;
@@ -86,6 +93,7 @@ struct scpsys_bus_prot_data {
 	u32 bus_prot_sta;
 	bool bus_prot_reg_update;
 	bool ignore_clr_ack;
+	bool way_en;
 };
 
 /**
-- 
2.39.0


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

Re: [PATCH 6/8] soc: mediatek: Add support for WAY_EN operations

[Matthias Brugger]

On 05/01/2023 18:07, Markus Schneider-Pargmann wrote:
> From: Alexandre Bailon <abailon@baylibre.com>
> 
> This updates the power domain to support WAY_EN operations. These
> operations enable a path between different units of the chip and are
> labeled as 'way_en' in the register descriptions.
> 
> This operation is required by the mt8365 for the MM power domain.
> 
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> Signed-off-by: Fabien Parent <fparent@baylibre.com>
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
> 
> Notes:
>      Changes in v4:
>      - Redesigned the patch to be better to understand and have less code
>        duplication
>      
>      Changes in v3:
>      - Separated the way_en functions for clarity
>      - Added some checks for infracfg_nao
>      
>      Changes in v2:
>      - some minor style fixes.
>      - Renamed 'wayen' to 'way_en' to clarify the meaning
>      - Updated commit message
> 
>   drivers/soc/mediatek/mtk-pm-domains.c | 57 ++++++++++++++++++++-------
>   drivers/soc/mediatek/mtk-pm-domains.h | 16 ++++++--
>   2 files changed, 54 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
> index 999e1f6c86b0..d53309f050ee 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.c
> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
> @@ -43,6 +43,7 @@ struct scpsys_domain {
>   	struct clk_bulk_data *clks;
>   	int num_subsys_clks;
>   	struct clk_bulk_data *subsys_clks;
> +	struct regmap *infracfg_nao;
>   	struct regmap *infracfg;
>   	struct regmap *smi;
>   	struct regulator *supply;
> @@ -118,10 +119,13 @@ static int scpsys_sram_disable(struct scpsys_domain *pd)
>   }
>   
>   static int scpsys_bus_protect_clear(const struct scpsys_bus_prot_data *bpd,
> -				    struct regmap *regmap)
> +				    struct regmap *regmap,
> +				    struct regmap *sta_regmap)

I had a look at this and I don't like the idea to add another regmap that's used 
based on flag in scpsys_bus_prot_data.

Up to now the assumption was that the different actions to enable/disable bus 
protection were done using one regmap. For this reason the regmap was defined 
once in scpsys_domain. It's not a good design to just pass more and more regmaps 
or other members to the functions. We should try to find a better way to 
structure the data so that it fulfills our needs.

Possible solutions I can think of right now:
a) Pass a struct of two regmaps to _scpsys_bus_protect_[disable,enable]()
We can pass the second regmap only if pd->way_en is set, otherwise it will be 
ignored anyway.
It's less flexible if in the future we need another regmap. We will have two a 
flag to mark we use way_en twice, in struct scpsys_domain and in struct 
scpsys_domain_data. Please see my comment below. It's something else to think about.


b) Add a flag field to scpsys_bus_prot_data to say explicitely if we want to use 
infracfg, smi with/without way_en. Then we can pass all three regmaps in a 
struct, it will be easy to add new regmaps in the future.
Downsides:
1) We will overload even more the _BUS_PROT macros, which get's hard to read and 
understand already. Maybe that can somehow be fixed. An idea would be by adding 
a bit field to scpsys_bus_prot_data.
2) We will need to rework all the scpsys_domain_data structures for all SoCs 
already supported. It's doable, but it's some extra work.

Any other ideas?

>   {
>   	u32 val;
>   	u32 sta_mask = bpd->bus_prot_sta_mask;
> +	/* way_en acknowledges successful clear with the bit being set */
> +	u32 expected_ack = (bpd->way_en ? sta_mask : 0);
>   
>   	if (bpd->bus_prot_reg_update)
>   		regmap_clear_bits(regmap, bpd->bus_prot_clr, bpd->bus_prot_set_clr_mask);
> @@ -131,13 +135,14 @@ static int scpsys_bus_protect_clear(const struct scpsys_bus_prot_data *bpd,
>   	if (bpd->ignore_clr_ack)
>   		return 0;
>   
> -	return regmap_read_poll_timeout(regmap, bpd->bus_prot_sta,
> -					val, !(val & sta_mask),
> +	return regmap_read_poll_timeout(sta_regmap, bpd->bus_prot_sta,
> +					val, (val & sta_mask) == expected_ack,
>   					MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
>   }
>   
>   static int scpsys_bus_protect_set(const struct scpsys_bus_prot_data *bpd,
> -				  struct regmap *regmap)
> +				  struct regmap *regmap,
> +				  struct regmap *sta_regmap)
>   {
>   	u32 val;
>   	u32 sta_mask = bpd->bus_prot_sta_mask;
> @@ -147,12 +152,13 @@ static int scpsys_bus_protect_set(const struct scpsys_bus_prot_data *bpd,
>   	else
>   		regmap_write(regmap, bpd->bus_prot_set, bpd->bus_prot_set_clr_mask);
>   
> -	return regmap_read_poll_timeout(regmap, bpd->bus_prot_sta,
> +	return regmap_read_poll_timeout(sta_regmap, bpd->bus_prot_sta,
>   					val, (val & sta_mask) == sta_mask,
>   					MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
>   }
>   
> -static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd, struct regmap *regmap)
> +static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd,
> +				      struct regmap *regmap, struct regmap *infracfg_nao)
>   {
>   	int i, ret;
>   
> @@ -160,9 +166,14 @@ static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd, st
>   		if (!bpd[i].bus_prot_set_clr_mask)
>   			break;
>   
> -		ret = scpsys_bus_protect_set(&bpd[i], regmap);
> -		if (ret)
> +		if (bpd[i].way_en)
> +			ret = scpsys_bus_protect_clear(&bpd[i], regmap, infracfg_nao);
> +		else
> +			ret = scpsys_bus_protect_set(&bpd[i], regmap, regmap);
> +		if (ret) {
> +			pr_err("%s %d %d\n", __PRETTY_FUNCTION__, __LINE__, ret);
>   			return ret;
> +		}
>   	}
>   
>   	return 0;
> @@ -172,15 +183,17 @@ static int scpsys_bus_protect_enable(struct scpsys_domain *pd)
>   {
>   	int ret;
>   
> -	ret = _scpsys_bus_protect_enable(pd->data->bp_infracfg, pd->infracfg);
> +	ret = _scpsys_bus_protect_enable(pd->data->bp_infracfg,
> +					 pd->infracfg, pd->infracfg_nao);
>   	if (ret)
>   		return ret;
>   
> -	return _scpsys_bus_protect_enable(pd->data->bp_smi, pd->smi);
> +	return _scpsys_bus_protect_enable(pd->data->bp_smi, pd->smi, NULL);
>   }
>   
>   static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
> -				       struct regmap *regmap)
> +				       struct regmap *regmap,
> +				       struct regmap *infracfg_nao)
>   {
>   	int i, ret;
>   
> @@ -188,9 +201,14 @@ static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
>   		if (!bpd[i].bus_prot_set_clr_mask)
>   			continue;
>   
> -		ret = scpsys_bus_protect_clear(&bpd[i], regmap);
> -		if (ret)
> +		if (bpd[i].way_en)
> +			ret = scpsys_bus_protect_set(&bpd[i], regmap, infracfg_nao);
> +		else
> +			ret = scpsys_bus_protect_clear(&bpd[i], regmap, regmap);
> +		if (ret) {
> +			pr_err("%s %d %d\n", __PRETTY_FUNCTION__, __LINE__, ret);
>   			return ret;
> +		}
>   	}
>   
>   	return 0;
> @@ -200,11 +218,12 @@ static int scpsys_bus_protect_disable(struct scpsys_domain *pd)
>   {
>   	int ret;
>   
> -	ret = _scpsys_bus_protect_disable(pd->data->bp_smi, pd->smi);
> +	ret = _scpsys_bus_protect_disable(pd->data->bp_smi, pd->smi, NULL);
>   	if (ret)
>   		return ret;
>   
> -	return _scpsys_bus_protect_disable(pd->data->bp_infracfg, pd->infracfg);
> +	return _scpsys_bus_protect_disable(pd->data->bp_infracfg,
> +			pd->infracfg, pd->infracfg_nao);
>   }
>   
>   static int scpsys_regulator_enable(struct regulator *supply)
> @@ -378,6 +397,14 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
>   			return ERR_CAST(pd->smi);
>   	}
>   
> +	pd->infracfg_nao = syscon_regmap_lookup_by_phandle(node, "mediatek,infracfg-nao");
> +	if (IS_ERR(pd->infracfg_nao)) {
> +		if (MTK_SCPD_CAPS(pd, MTK_SCPD_HAS_WAY_EN))
> +			return ERR_CAST(pd->infracfg_nao);

I don't like the fact that we have a caps to show we have WAY_EN and then we 
have a bool in scpsys_bus_prot_data to flag when we need to use the second 
regmap and the expected_ack mask.
Can't we just make the lookup optional and let it all blow up if DTS is wrong 
because no infracfg-nao was defined while there is at least one domain using way_en?

Regards,
Matthias

> +
> +		pd->infracfg_nao = NULL;
> +	}
> +
>   	num_clks = of_clk_get_parent_count(node);
>   	if (num_clks > 0) {
>   		/* Calculate number of subsys_clks */
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.h b/drivers/soc/mediatek/mtk-pm-domains.h
> index 34642a279213..1fa634509db1 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.h
> +++ b/drivers/soc/mediatek/mtk-pm-domains.h
> @@ -10,6 +10,7 @@
>   #define MTK_SCPD_DOMAIN_SUPPLY		BIT(4)
>   /* can't set MTK_SCPD_KEEP_DEFAULT_OFF at the same time */
>   #define MTK_SCPD_ALWAYS_ON		BIT(5)
> +#define MTK_SCPD_HAS_WAY_EN		BIT(6)
>   #define MTK_SCPD_CAPS(_scpd, _x)	((_scpd)->data->caps & (_x))
>   
>   #define SPM_VDE_PWR_CON			0x0210
> @@ -41,7 +42,7 @@
>   
>   #define SPM_MAX_BUS_PROT_DATA		6
>   
> -#define _BUS_PROT(_set_clr_mask, _set, _clr, _sta_mask, _sta, _update, _ignore) {	\
> +#define _BUS_PROT(_set_clr_mask, _set, _clr, _sta_mask, _sta, _update, _ignore, _way_en) {	\
>   		.bus_prot_set_clr_mask = (_set_clr_mask),	\
>   		.bus_prot_set = _set,				\
>   		.bus_prot_clr = _clr,				\
> @@ -49,16 +50,20 @@
>   		.bus_prot_sta = _sta,				\
>   		.bus_prot_reg_update = _update,			\
>   		.ignore_clr_ack = _ignore,			\
> +		.way_en = _way_en,				\
>   	}
>   
>   #define BUS_PROT_WR(_mask, _set, _clr, _sta)			\
> -		_BUS_PROT(_mask, _set, _clr, _mask, _sta, false, false)
> +		_BUS_PROT(_mask, _set, _clr, _mask, _sta, false, false, false)
>   
>   #define BUS_PROT_WR_IGN(_mask, _set, _clr, _sta)		\
> -		_BUS_PROT(_mask, _set, _clr, _mask, _sta, false, true)
> +		_BUS_PROT(_mask, _set, _clr, _mask, _sta, false, true, false)
>   
>   #define BUS_PROT_UPDATE(_mask, _set, _clr, _sta)		\
> -		_BUS_PROT(_mask, _set, _clr, _mask, _sta, true, false)
> +		_BUS_PROT(_mask, _set, _clr, _mask, _sta, true, false, false)
> +
> +#define BUS_PROT_WAY_EN(_set_mask, _set, _sta_mask, _sta)	\
> +		_BUS_PROT(_set_mask, _set, _set, _sta_mask, _sta, true, false, true)
>   
>   #define BUS_PROT_UPDATE_TOPAXI(_mask)				\
>   		BUS_PROT_UPDATE(_mask,				\
> @@ -77,6 +82,8 @@
>    *			 write the whole register.
>    * @ignore_clk_ack: Ignore the result in the status register for clear
>    *		    operations.
> + * @way_en: Enable a way in the SoC. For this type the status bit to acknowledge
> + *	    the change is always 1 on successful change.
>    */
>   struct scpsys_bus_prot_data {
>   	u32 bus_prot_set_clr_mask;
> @@ -86,6 +93,7 @@ struct scpsys_bus_prot_data {
>   	u32 bus_prot_sta;
>   	bool bus_prot_reg_update;
>   	bool ignore_clr_ack;
> +	bool way_en;
>   };
>   
>   /**

Re: [PATCH 6/8] soc: mediatek: Add support for WAY_EN operations

[Matthias Brugger]

On 05/01/2023 18:07, Markus Schneider-Pargmann wrote:
> From: Alexandre Bailon <abailon@baylibre.com>
> 
> This updates the power domain to support WAY_EN operations. These
> operations enable a path between different units of the chip and are
> labeled as 'way_en' in the register descriptions.
> 
> This operation is required by the mt8365 for the MM power domain.
> 
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> Signed-off-by: Fabien Parent <fparent@baylibre.com>
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
> 
> Notes:
>      Changes in v4:
>      - Redesigned the patch to be better to understand and have less code
>        duplication
>      
>      Changes in v3:
>      - Separated the way_en functions for clarity
>      - Added some checks for infracfg_nao
>      
>      Changes in v2:
>      - some minor style fixes.
>      - Renamed 'wayen' to 'way_en' to clarify the meaning
>      - Updated commit message
> 
>   drivers/soc/mediatek/mtk-pm-domains.c | 57 ++++++++++++++++++++-------
>   drivers/soc/mediatek/mtk-pm-domains.h | 16 ++++++--
>   2 files changed, 54 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
> index 999e1f6c86b0..d53309f050ee 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.c
> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
> @@ -43,6 +43,7 @@ struct scpsys_domain {
>   	struct clk_bulk_data *clks;
>   	int num_subsys_clks;
>   	struct clk_bulk_data *subsys_clks;
> +	struct regmap *infracfg_nao;
>   	struct regmap *infracfg;
>   	struct regmap *smi;
>   	struct regulator *supply;
> @@ -118,10 +119,13 @@ static int scpsys_sram_disable(struct scpsys_domain *pd)
>   }
>   
>   static int scpsys_bus_protect_clear(const struct scpsys_bus_prot_data *bpd,
> -				    struct regmap *regmap)
> +				    struct regmap *regmap,
> +				    struct regmap *sta_regmap)

I had a look at this and I don't like the idea to add another regmap that's used 
based on flag in scpsys_bus_prot_data.

Up to now the assumption was that the different actions to enable/disable bus 
protection were done using one regmap. For this reason the regmap was defined 
once in scpsys_domain. It's not a good design to just pass more and more regmaps 
or other members to the functions. We should try to find a better way to 
structure the data so that it fulfills our needs.

Possible solutions I can think of right now:
a) Pass a struct of two regmaps to _scpsys_bus_protect_[disable,enable]()
We can pass the second regmap only if pd->way_en is set, otherwise it will be 
ignored anyway.
It's less flexible if in the future we need another regmap. We will have two a 
flag to mark we use way_en twice, in struct scpsys_domain and in struct 
scpsys_domain_data. Please see my comment below. It's something else to think about.


b) Add a flag field to scpsys_bus_prot_data to say explicitely if we want to use 
infracfg, smi with/without way_en. Then we can pass all three regmaps in a 
struct, it will be easy to add new regmaps in the future.
Downsides:
1) We will overload even more the _BUS_PROT macros, which get's hard to read and 
understand already. Maybe that can somehow be fixed. An idea would be by adding 
a bit field to scpsys_bus_prot_data.
2) We will need to rework all the scpsys_domain_data structures for all SoCs 
already supported. It's doable, but it's some extra work.

Any other ideas?

>   {
>   	u32 val;
>   	u32 sta_mask = bpd->bus_prot_sta_mask;
> +	/* way_en acknowledges successful clear with the bit being set */
> +	u32 expected_ack = (bpd->way_en ? sta_mask : 0);
>   
>   	if (bpd->bus_prot_reg_update)
>   		regmap_clear_bits(regmap, bpd->bus_prot_clr, bpd->bus_prot_set_clr_mask);
> @@ -131,13 +135,14 @@ static int scpsys_bus_protect_clear(const struct scpsys_bus_prot_data *bpd,
>   	if (bpd->ignore_clr_ack)
>   		return 0;
>   
> -	return regmap_read_poll_timeout(regmap, bpd->bus_prot_sta,
> -					val, !(val & sta_mask),
> +	return regmap_read_poll_timeout(sta_regmap, bpd->bus_prot_sta,
> +					val, (val & sta_mask) == expected_ack,
>   					MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
>   }
>   
>   static int scpsys_bus_protect_set(const struct scpsys_bus_prot_data *bpd,
> -				  struct regmap *regmap)
> +				  struct regmap *regmap,
> +				  struct regmap *sta_regmap)
>   {
>   	u32 val;
>   	u32 sta_mask = bpd->bus_prot_sta_mask;
> @@ -147,12 +152,13 @@ static int scpsys_bus_protect_set(const struct scpsys_bus_prot_data *bpd,
>   	else
>   		regmap_write(regmap, bpd->bus_prot_set, bpd->bus_prot_set_clr_mask);
>   
> -	return regmap_read_poll_timeout(regmap, bpd->bus_prot_sta,
> +	return regmap_read_poll_timeout(sta_regmap, bpd->bus_prot_sta,
>   					val, (val & sta_mask) == sta_mask,
>   					MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
>   }
>   
> -static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd, struct regmap *regmap)
> +static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd,
> +				      struct regmap *regmap, struct regmap *infracfg_nao)
>   {
>   	int i, ret;
>   
> @@ -160,9 +166,14 @@ static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd, st
>   		if (!bpd[i].bus_prot_set_clr_mask)
>   			break;
>   
> -		ret = scpsys_bus_protect_set(&bpd[i], regmap);
> -		if (ret)
> +		if (bpd[i].way_en)
> +			ret = scpsys_bus_protect_clear(&bpd[i], regmap, infracfg_nao);
> +		else
> +			ret = scpsys_bus_protect_set(&bpd[i], regmap, regmap);
> +		if (ret) {
> +			pr_err("%s %d %d\n", __PRETTY_FUNCTION__, __LINE__, ret);
>   			return ret;
> +		}
>   	}
>   
>   	return 0;
> @@ -172,15 +183,17 @@ static int scpsys_bus_protect_enable(struct scpsys_domain *pd)
>   {
>   	int ret;
>   
> -	ret = _scpsys_bus_protect_enable(pd->data->bp_infracfg, pd->infracfg);
> +	ret = _scpsys_bus_protect_enable(pd->data->bp_infracfg,
> +					 pd->infracfg, pd->infracfg_nao);
>   	if (ret)
>   		return ret;
>   
> -	return _scpsys_bus_protect_enable(pd->data->bp_smi, pd->smi);
> +	return _scpsys_bus_protect_enable(pd->data->bp_smi, pd->smi, NULL);
>   }
>   
>   static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
> -				       struct regmap *regmap)
> +				       struct regmap *regmap,
> +				       struct regmap *infracfg_nao)
>   {
>   	int i, ret;
>   
> @@ -188,9 +201,14 @@ static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
>   		if (!bpd[i].bus_prot_set_clr_mask)
>   			continue;
>   
> -		ret = scpsys_bus_protect_clear(&bpd[i], regmap);
> -		if (ret)
> +		if (bpd[i].way_en)
> +			ret = scpsys_bus_protect_set(&bpd[i], regmap, infracfg_nao);
> +		else
> +			ret = scpsys_bus_protect_clear(&bpd[i], regmap, regmap);
> +		if (ret) {
> +			pr_err("%s %d %d\n", __PRETTY_FUNCTION__, __LINE__, ret);
>   			return ret;
> +		}
>   	}
>   
>   	return 0;
> @@ -200,11 +218,12 @@ static int scpsys_bus_protect_disable(struct scpsys_domain *pd)
>   {
>   	int ret;
>   
> -	ret = _scpsys_bus_protect_disable(pd->data->bp_smi, pd->smi);
> +	ret = _scpsys_bus_protect_disable(pd->data->bp_smi, pd->smi, NULL);
>   	if (ret)
>   		return ret;
>   
> -	return _scpsys_bus_protect_disable(pd->data->bp_infracfg, pd->infracfg);
> +	return _scpsys_bus_protect_disable(pd->data->bp_infracfg,
> +			pd->infracfg, pd->infracfg_nao);
>   }
>   
>   static int scpsys_regulator_enable(struct regulator *supply)
> @@ -378,6 +397,14 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
>   			return ERR_CAST(pd->smi);
>   	}
>   
> +	pd->infracfg_nao = syscon_regmap_lookup_by_phandle(node, "mediatek,infracfg-nao");
> +	if (IS_ERR(pd->infracfg_nao)) {
> +		if (MTK_SCPD_CAPS(pd, MTK_SCPD_HAS_WAY_EN))
> +			return ERR_CAST(pd->infracfg_nao);

I don't like the fact that we have a caps to show we have WAY_EN and then we 
have a bool in scpsys_bus_prot_data to flag when we need to use the second 
regmap and the expected_ack mask.
Can't we just make the lookup optional and let it all blow up if DTS is wrong 
because no infracfg-nao was defined while there is at least one domain using way_en?

Regards,
Matthias

> +
> +		pd->infracfg_nao = NULL;
> +	}
> +
>   	num_clks = of_clk_get_parent_count(node);
>   	if (num_clks > 0) {
>   		/* Calculate number of subsys_clks */
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.h b/drivers/soc/mediatek/mtk-pm-domains.h
> index 34642a279213..1fa634509db1 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.h
> +++ b/drivers/soc/mediatek/mtk-pm-domains.h
> @@ -10,6 +10,7 @@
>   #define MTK_SCPD_DOMAIN_SUPPLY		BIT(4)
>   /* can't set MTK_SCPD_KEEP_DEFAULT_OFF at the same time */
>   #define MTK_SCPD_ALWAYS_ON		BIT(5)
> +#define MTK_SCPD_HAS_WAY_EN		BIT(6)
>   #define MTK_SCPD_CAPS(_scpd, _x)	((_scpd)->data->caps & (_x))
>   
>   #define SPM_VDE_PWR_CON			0x0210
> @@ -41,7 +42,7 @@
>   
>   #define SPM_MAX_BUS_PROT_DATA		6
>   
> -#define _BUS_PROT(_set_clr_mask, _set, _clr, _sta_mask, _sta, _update, _ignore) {	\
> +#define _BUS_PROT(_set_clr_mask, _set, _clr, _sta_mask, _sta, _update, _ignore, _way_en) {	\
>   		.bus_prot_set_clr_mask = (_set_clr_mask),	\
>   		.bus_prot_set = _set,				\
>   		.bus_prot_clr = _clr,				\
> @@ -49,16 +50,20 @@
>   		.bus_prot_sta = _sta,				\
>   		.bus_prot_reg_update = _update,			\
>   		.ignore_clr_ack = _ignore,			\
> +		.way_en = _way_en,				\
>   	}
>   
>   #define BUS_PROT_WR(_mask, _set, _clr, _sta)			\
> -		_BUS_PROT(_mask, _set, _clr, _mask, _sta, false, false)
> +		_BUS_PROT(_mask, _set, _clr, _mask, _sta, false, false, false)
>   
>   #define BUS_PROT_WR_IGN(_mask, _set, _clr, _sta)		\
> -		_BUS_PROT(_mask, _set, _clr, _mask, _sta, false, true)
> +		_BUS_PROT(_mask, _set, _clr, _mask, _sta, false, true, false)
>   
>   #define BUS_PROT_UPDATE(_mask, _set, _clr, _sta)		\
> -		_BUS_PROT(_mask, _set, _clr, _mask, _sta, true, false)
> +		_BUS_PROT(_mask, _set, _clr, _mask, _sta, true, false, false)
> +
> +#define BUS_PROT_WAY_EN(_set_mask, _set, _sta_mask, _sta)	\
> +		_BUS_PROT(_set_mask, _set, _set, _sta_mask, _sta, true, false, true)
>   
>   #define BUS_PROT_UPDATE_TOPAXI(_mask)				\
>   		BUS_PROT_UPDATE(_mask,				\
> @@ -77,6 +82,8 @@
>    *			 write the whole register.
>    * @ignore_clk_ack: Ignore the result in the status register for clear
>    *		    operations.
> + * @way_en: Enable a way in the SoC. For this type the status bit to acknowledge
> + *	    the change is always 1 on successful change.
>    */
>   struct scpsys_bus_prot_data {
>   	u32 bus_prot_set_clr_mask;
> @@ -86,6 +93,7 @@ struct scpsys_bus_prot_data {
>   	u32 bus_prot_sta;
>   	bool bus_prot_reg_update;
>   	bool ignore_clr_ack;
> +	bool way_en;
>   };
>   
>   /**

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

[PATCH 7/8] soc: mediatek: Add support for MTK_SCPD_STRICT_BUS_PROTECTION cap

[Markus Schneider-Pargmann]

From: Alexandre Bailon <abailon@baylibre.com>

This adds support for MTK_SCPD_STRICT_BUS_PROTECTION capability. It is a
strict bus protection policy that requires the bus protection to be
disabled before accessing the bus.
This is required by the mt8365, for the MM power domain.

Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
Signed-off-by: Fabien Parent <fparent@baylibre.com>
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---

Notes:
    Changes in v4:
    - Change name in title and commit message
    
    Changes in v3:
    - Rename MTK_SCPD_STRICT_BUSP to MTK_SCPD_STRICT_BUS_PROTECTION
    - Remove extra bool variable reflecting the capability
    
    Changes in v2:
    - Fixup error handling path.

 drivers/soc/mediatek/mtk-pm-domains.c | 27 +++++++++++++++++++++++----
 drivers/soc/mediatek/mtk-pm-domains.h |  1 +
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
index d53309f050ee..29a9028dd9b3 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.c
+++ b/drivers/soc/mediatek/mtk-pm-domains.c
@@ -265,9 +265,17 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
 	regmap_clear_bits(scpsys->base, pd->data->ctl_offs, PWR_ISO_BIT);
 	regmap_set_bits(scpsys->base, pd->data->ctl_offs, PWR_RST_B_BIT);
 
-	ret = clk_bulk_prepare_enable(pd->num_subsys_clks, pd->subsys_clks);
-	if (ret)
-		goto err_pwr_ack;
+	/*
+	 * In few Mediatek platforms(e.g. MT6779), the bus protect policy is
+	 * stricter, which leads to bus protect release must be prior to bus
+	 * access.
+	 */
+	if (!MTK_SCPD_CAPS(pd, MTK_SCPD_STRICT_BUS_PROTECTION)) {
+		ret = clk_bulk_prepare_enable(pd->num_subsys_clks,
+					      pd->subsys_clks);
+		if (ret)
+			goto err_pwr_ack;
+	}
 
 	ret = scpsys_sram_enable(pd);
 	if (ret < 0)
@@ -277,12 +285,23 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
 	if (ret < 0)
 		goto err_disable_sram;
 
+	if (MTK_SCPD_CAPS(pd, MTK_SCPD_STRICT_BUS_PROTECTION)) {
+		ret = clk_bulk_prepare_enable(pd->num_subsys_clks,
+					      pd->subsys_clks);
+		if (ret)
+			goto err_enable_bus_protect;
+	}
+
 	return 0;
 
+err_enable_bus_protect:
+	scpsys_bus_protect_enable(pd);
 err_disable_sram:
 	scpsys_sram_disable(pd);
 err_disable_subsys_clks:
-	clk_bulk_disable_unprepare(pd->num_subsys_clks, pd->subsys_clks);
+	if (!MTK_SCPD_CAPS(pd, MTK_SCPD_STRICT_BUS_PROTECTION))
+		clk_bulk_disable_unprepare(pd->num_subsys_clks,
+					   pd->subsys_clks);
 err_pwr_ack:
 	clk_bulk_disable_unprepare(pd->num_clks, pd->clks);
 err_reg:
diff --git a/drivers/soc/mediatek/mtk-pm-domains.h b/drivers/soc/mediatek/mtk-pm-domains.h
index 1fa634509db1..e5aa2348a9db 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.h
+++ b/drivers/soc/mediatek/mtk-pm-domains.h
@@ -11,6 +11,7 @@
 /* can't set MTK_SCPD_KEEP_DEFAULT_OFF at the same time */
 #define MTK_SCPD_ALWAYS_ON		BIT(5)
 #define MTK_SCPD_HAS_WAY_EN		BIT(6)
+#define MTK_SCPD_STRICT_BUS_PROTECTION	BIT(7)
 #define MTK_SCPD_CAPS(_scpd, _x)	((_scpd)->data->caps & (_x))
 
 #define SPM_VDE_PWR_CON			0x0210
-- 
2.39.0

[PATCH 7/8] soc: mediatek: Add support for MTK_SCPD_STRICT_BUS_PROTECTION cap

[Markus Schneider-Pargmann]

From: Alexandre Bailon <abailon@baylibre.com>

This adds support for MTK_SCPD_STRICT_BUS_PROTECTION capability. It is a
strict bus protection policy that requires the bus protection to be
disabled before accessing the bus.
This is required by the mt8365, for the MM power domain.

Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
Signed-off-by: Fabien Parent <fparent@baylibre.com>
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---

Notes:
    Changes in v4:
    - Change name in title and commit message
    
    Changes in v3:
    - Rename MTK_SCPD_STRICT_BUSP to MTK_SCPD_STRICT_BUS_PROTECTION
    - Remove extra bool variable reflecting the capability
    
    Changes in v2:
    - Fixup error handling path.

 drivers/soc/mediatek/mtk-pm-domains.c | 27 +++++++++++++++++++++++----
 drivers/soc/mediatek/mtk-pm-domains.h |  1 +
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
index d53309f050ee..29a9028dd9b3 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.c
+++ b/drivers/soc/mediatek/mtk-pm-domains.c
@@ -265,9 +265,17 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
 	regmap_clear_bits(scpsys->base, pd->data->ctl_offs, PWR_ISO_BIT);
 	regmap_set_bits(scpsys->base, pd->data->ctl_offs, PWR_RST_B_BIT);
 
-	ret = clk_bulk_prepare_enable(pd->num_subsys_clks, pd->subsys_clks);
-	if (ret)
-		goto err_pwr_ack;
+	/*
+	 * In few Mediatek platforms(e.g. MT6779), the bus protect policy is
+	 * stricter, which leads to bus protect release must be prior to bus
+	 * access.
+	 */
+	if (!MTK_SCPD_CAPS(pd, MTK_SCPD_STRICT_BUS_PROTECTION)) {
+		ret = clk_bulk_prepare_enable(pd->num_subsys_clks,
+					      pd->subsys_clks);
+		if (ret)
+			goto err_pwr_ack;
+	}
 
 	ret = scpsys_sram_enable(pd);
 	if (ret < 0)
@@ -277,12 +285,23 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
 	if (ret < 0)
 		goto err_disable_sram;
 
+	if (MTK_SCPD_CAPS(pd, MTK_SCPD_STRICT_BUS_PROTECTION)) {
+		ret = clk_bulk_prepare_enable(pd->num_subsys_clks,
+					      pd->subsys_clks);
+		if (ret)
+			goto err_enable_bus_protect;
+	}
+
 	return 0;
 
+err_enable_bus_protect:
+	scpsys_bus_protect_enable(pd);
 err_disable_sram:
 	scpsys_sram_disable(pd);
 err_disable_subsys_clks:
-	clk_bulk_disable_unprepare(pd->num_subsys_clks, pd->subsys_clks);
+	if (!MTK_SCPD_CAPS(pd, MTK_SCPD_STRICT_BUS_PROTECTION))
+		clk_bulk_disable_unprepare(pd->num_subsys_clks,
+					   pd->subsys_clks);
 err_pwr_ack:
 	clk_bulk_disable_unprepare(pd->num_clks, pd->clks);
 err_reg:
diff --git a/drivers/soc/mediatek/mtk-pm-domains.h b/drivers/soc/mediatek/mtk-pm-domains.h
index 1fa634509db1..e5aa2348a9db 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.h
+++ b/drivers/soc/mediatek/mtk-pm-domains.h
@@ -11,6 +11,7 @@
 /* can't set MTK_SCPD_KEEP_DEFAULT_OFF at the same time */
 #define MTK_SCPD_ALWAYS_ON		BIT(5)
 #define MTK_SCPD_HAS_WAY_EN		BIT(6)
+#define MTK_SCPD_STRICT_BUS_PROTECTION	BIT(7)
 #define MTK_SCPD_CAPS(_scpd, _x)	((_scpd)->data->caps & (_x))
 
 #define SPM_VDE_PWR_CON			0x0210
-- 
2.39.0


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

[PATCH 8/8] soc: mediatek: pm-domains: Add support for MT8365

[Markus Schneider-Pargmann]

From: Fabien Parent <fparent@baylibre.com>

Add the needed board data to support MT8365 SoC.

Signed-off-by: Fabien Parent <fparent@baylibre.com>
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 drivers/soc/mediatek/mt8365-pm-domains.h | 147 +++++++++++++++++++++++
 drivers/soc/mediatek/mtk-pm-domains.c    |   5 +
 2 files changed, 152 insertions(+)
 create mode 100644 drivers/soc/mediatek/mt8365-pm-domains.h

diff --git a/drivers/soc/mediatek/mt8365-pm-domains.h b/drivers/soc/mediatek/mt8365-pm-domains.h
new file mode 100644
index 000000000000..8735e833b15b
--- /dev/null
+++ b/drivers/soc/mediatek/mt8365-pm-domains.h
@@ -0,0 +1,147 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __SOC_MEDIATEK_MT8365_PM_DOMAINS_H
+#define __SOC_MEDIATEK_MT8365_PM_DOMAINS_H
+
+#include "mtk-pm-domains.h"
+#include <dt-bindings/power/mediatek,mt8365-power.h>
+
+/*
+ * MT8365 power domain support
+ */
+
+static const struct scpsys_domain_data scpsys_domain_data_mt8365[] = {
+	[MT8365_POWER_DOMAIN_MM] = {
+		.name = "mm",
+		.sta_mask = PWR_STATUS_DISP,
+		.ctl_offs = 0x30c,
+		.pwr_sta_offs = 0x0180,
+		.pwr_sta2nd_offs = 0x0184,
+		.sram_pdn_bits = GENMASK(8, 8),
+		.sram_pdn_ack_bits = GENMASK(12, 12),
+		.caps = MTK_SCPD_STRICT_BUS_PROTECTION | MTK_SCPD_HAS_WAY_EN,
+		.bp_infracfg = {
+			BUS_PROT_WR(BIT(16) | BIT(17), 0x2a8, 0x2ac, 0x258),
+			BUS_PROT_WR(BIT(1) | BIT(2) | BIT(10) | BIT(11), 0x2a0, 0x2a4, 0x228),
+			BUS_PROT_WAY_EN(BIT(6), 0x200, BIT(24), 0x0),
+			BUS_PROT_WAY_EN(BIT(5), 0x234, BIT(14), 0x28),
+			BUS_PROT_WR(BIT(6), 0x2a0, 0x2a4, 0x228),
+		},
+	},
+	[MT8365_POWER_DOMAIN_VENC] = {
+		.name = "venc",
+		.sta_mask = PWR_STATUS_VENC,
+		.ctl_offs = 0x0304,
+		.pwr_sta_offs = 0x0180,
+		.pwr_sta2nd_offs = 0x0184,
+		.sram_pdn_bits = GENMASK(8, 8),
+		.sram_pdn_ack_bits = GENMASK(12, 12),
+		.bp_smi = {
+			BUS_PROT_WR(BIT(1), 0x3c4, 0x3c8, 0x3c0),
+		},
+	},
+	[MT8365_POWER_DOMAIN_AUDIO] = {
+		.name = "audio",
+		.sta_mask = PWR_STATUS_AUDIO,
+		.ctl_offs = 0x0314,
+		.pwr_sta_offs = 0x0180,
+		.pwr_sta2nd_offs = 0x0184,
+		.sram_pdn_bits = GENMASK(12, 8),
+		.sram_pdn_ack_bits = GENMASK(17, 13),
+		.bp_infracfg = {
+			BUS_PROT_WR(BIT(27) | BIT(28), 0x2a8, 0x2ac, 0x258),
+		},
+		.caps = MTK_SCPD_ACTIVE_WAKEUP,
+	},
+	[MT8365_POWER_DOMAIN_CONN] = {
+		.name = "conn",
+		.sta_mask = PWR_STATUS_CONN,
+		.ctl_offs = 0x032c,
+		.pwr_sta_offs = 0x0180,
+		.pwr_sta2nd_offs = 0x0184,
+		.sram_pdn_bits = 0,
+		.sram_pdn_ack_bits = 0,
+		.bp_infracfg = {
+			BUS_PROT_WR(BIT(13), 0x2a0, 0x2a4, 0x228),
+			BUS_PROT_WR(BIT(18), 0x2a8, 0x2ac, 0x258),
+			BUS_PROT_WR(BIT(14), 0x2a0, 0x2a4, 0x228),
+			BUS_PROT_WR(BIT(21), 0x2a8, 0x2ac, 0x258),
+		},
+		.caps = MTK_SCPD_ACTIVE_WAKEUP | MTK_SCPD_KEEP_DEFAULT_OFF,
+	},
+	[MT8365_POWER_DOMAIN_MFG] = {
+		.name = "mfg",
+		.sta_mask = PWR_STATUS_MFG,
+		.ctl_offs = 0x0338,
+		.pwr_sta_offs = 0x0180,
+		.pwr_sta2nd_offs = 0x0184,
+		.sram_pdn_bits = GENMASK(9, 8),
+		.sram_pdn_ack_bits = GENMASK(13, 12),
+		.bp_infracfg = {
+			BUS_PROT_WR(BIT(25), 0x2a0, 0x2a4, 0x228),
+			BUS_PROT_WR(BIT(21) | BIT(22), 0x2a0, 0x2a4, 0x228),
+		},
+	},
+	[MT8365_POWER_DOMAIN_CAM] = {
+		.name = "cam",
+		.sta_mask = BIT(25),
+		.ctl_offs = 0x0344,
+		.pwr_sta_offs = 0x0180,
+		.pwr_sta2nd_offs = 0x0184,
+		.sram_pdn_bits = GENMASK(9, 8),
+		.sram_pdn_ack_bits = GENMASK(13, 12),
+		.bp_infracfg = {
+			BUS_PROT_WR(BIT(19), 0x2a8, 0x2ac, 0x258),
+		},
+		.bp_smi = {
+			BUS_PROT_WR(BIT(2), 0x3c4, 0x3c8, 0x3c0),
+		},
+	},
+	[MT8365_POWER_DOMAIN_VDEC] = {
+		.name = "vdec",
+		.sta_mask = BIT(31),
+		.ctl_offs = 0x0370,
+		.pwr_sta_offs = 0x0180,
+		.pwr_sta2nd_offs = 0x0184,
+		.sram_pdn_bits = GENMASK(8, 8),
+		.sram_pdn_ack_bits = GENMASK(12, 12),
+		.bp_smi = {
+			BUS_PROT_WR(BIT(3), 0x3c4, 0x3c8, 0x3c0),
+		},
+	},
+	[MT8365_POWER_DOMAIN_APU] = {
+		.name = "apu",
+		.sta_mask = BIT(16),
+		.ctl_offs = 0x0378,
+		.pwr_sta_offs = 0x0180,
+		.pwr_sta2nd_offs = 0x0184,
+		.sram_pdn_bits = GENMASK(14, 8),
+		.sram_pdn_ack_bits = GENMASK(21, 15),
+		.bp_infracfg = {
+			BUS_PROT_WR(BIT(2) | BIT(20), 0x2a8, 0x2ac, 0x258),
+		},
+		.bp_smi = {
+			BUS_PROT_WR(BIT(4), 0x3c4, 0x3c8, 0x3c0),
+		},
+	},
+	[MT8365_POWER_DOMAIN_DSP] = {
+		.name = "dsp",
+		.sta_mask = BIT(17),
+		.ctl_offs = 0x037C,
+		.pwr_sta_offs = 0x0180,
+		.pwr_sta2nd_offs = 0x0184,
+		.sram_pdn_bits = GENMASK(11, 8),
+		.sram_pdn_ack_bits = GENMASK(15, 12),
+		.bp_infracfg = {
+			BUS_PROT_WR(BIT(24) | BIT(30) | BIT(31), 0x2a8, 0x2ac, 0x258),
+		},
+		.caps = MTK_SCPD_ACTIVE_WAKEUP,
+	},
+};
+
+static const struct scpsys_soc_data mt8365_scpsys_data = {
+	.domains_data = scpsys_domain_data_mt8365,
+	.num_domains = ARRAY_SIZE(scpsys_domain_data_mt8365),
+};
+
+#endif /* __SOC_MEDIATEK_MT8365_PM_DOMAINS_H */
diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
index 29a9028dd9b3..adb7716df0a8 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.c
+++ b/drivers/soc/mediatek/mtk-pm-domains.c
@@ -23,6 +23,7 @@
 #include "mt8186-pm-domains.h"
 #include "mt8192-pm-domains.h"
 #include "mt8195-pm-domains.h"
+#include "mt8365-pm-domains.h"
 
 #define MTK_POLL_DELAY_US		10
 #define MTK_POLL_TIMEOUT		USEC_PER_SEC
@@ -647,6 +648,10 @@ static const struct of_device_id scpsys_of_match[] = {
 		.compatible = "mediatek,mt8195-power-controller",
 		.data = &mt8195_scpsys_data,
 	},
+	{
+		.compatible = "mediatek,mt8365-power-controller",
+		.data = &mt8365_scpsys_data,
+	},
 	{ }
 };
 
-- 
2.39.0

[PATCH 8/8] soc: mediatek: pm-domains: Add support for MT8365

[Markus Schneider-Pargmann]

From: Fabien Parent <fparent@baylibre.com>

Add the needed board data to support MT8365 SoC.

Signed-off-by: Fabien Parent <fparent@baylibre.com>
Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 drivers/soc/mediatek/mt8365-pm-domains.h | 147 +++++++++++++++++++++++
 drivers/soc/mediatek/mtk-pm-domains.c    |   5 +
 2 files changed, 152 insertions(+)
 create mode 100644 drivers/soc/mediatek/mt8365-pm-domains.h

diff --git a/drivers/soc/mediatek/mt8365-pm-domains.h b/drivers/soc/mediatek/mt8365-pm-domains.h
new file mode 100644
index 000000000000..8735e833b15b
--- /dev/null
+++ b/drivers/soc/mediatek/mt8365-pm-domains.h
@@ -0,0 +1,147 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __SOC_MEDIATEK_MT8365_PM_DOMAINS_H
+#define __SOC_MEDIATEK_MT8365_PM_DOMAINS_H
+
+#include "mtk-pm-domains.h"
+#include <dt-bindings/power/mediatek,mt8365-power.h>
+
+/*
+ * MT8365 power domain support
+ */
+
+static const struct scpsys_domain_data scpsys_domain_data_mt8365[] = {
+	[MT8365_POWER_DOMAIN_MM] = {
+		.name = "mm",
+		.sta_mask = PWR_STATUS_DISP,
+		.ctl_offs = 0x30c,
+		.pwr_sta_offs = 0x0180,
+		.pwr_sta2nd_offs = 0x0184,
+		.sram_pdn_bits = GENMASK(8, 8),
+		.sram_pdn_ack_bits = GENMASK(12, 12),
+		.caps = MTK_SCPD_STRICT_BUS_PROTECTION | MTK_SCPD_HAS_WAY_EN,
+		.bp_infracfg = {
+			BUS_PROT_WR(BIT(16) | BIT(17), 0x2a8, 0x2ac, 0x258),
+			BUS_PROT_WR(BIT(1) | BIT(2) | BIT(10) | BIT(11), 0x2a0, 0x2a4, 0x228),
+			BUS_PROT_WAY_EN(BIT(6), 0x200, BIT(24), 0x0),
+			BUS_PROT_WAY_EN(BIT(5), 0x234, BIT(14), 0x28),
+			BUS_PROT_WR(BIT(6), 0x2a0, 0x2a4, 0x228),
+		},
+	},
+	[MT8365_POWER_DOMAIN_VENC] = {
+		.name = "venc",
+		.sta_mask = PWR_STATUS_VENC,
+		.ctl_offs = 0x0304,
+		.pwr_sta_offs = 0x0180,
+		.pwr_sta2nd_offs = 0x0184,
+		.sram_pdn_bits = GENMASK(8, 8),
+		.sram_pdn_ack_bits = GENMASK(12, 12),
+		.bp_smi = {
+			BUS_PROT_WR(BIT(1), 0x3c4, 0x3c8, 0x3c0),
+		},
+	},
+	[MT8365_POWER_DOMAIN_AUDIO] = {
+		.name = "audio",
+		.sta_mask = PWR_STATUS_AUDIO,
+		.ctl_offs = 0x0314,
+		.pwr_sta_offs = 0x0180,
+		.pwr_sta2nd_offs = 0x0184,
+		.sram_pdn_bits = GENMASK(12, 8),
+		.sram_pdn_ack_bits = GENMASK(17, 13),
+		.bp_infracfg = {
+			BUS_PROT_WR(BIT(27) | BIT(28), 0x2a8, 0x2ac, 0x258),
+		},
+		.caps = MTK_SCPD_ACTIVE_WAKEUP,
+	},
+	[MT8365_POWER_DOMAIN_CONN] = {
+		.name = "conn",
+		.sta_mask = PWR_STATUS_CONN,
+		.ctl_offs = 0x032c,
+		.pwr_sta_offs = 0x0180,
+		.pwr_sta2nd_offs = 0x0184,
+		.sram_pdn_bits = 0,
+		.sram_pdn_ack_bits = 0,
+		.bp_infracfg = {
+			BUS_PROT_WR(BIT(13), 0x2a0, 0x2a4, 0x228),
+			BUS_PROT_WR(BIT(18), 0x2a8, 0x2ac, 0x258),
+			BUS_PROT_WR(BIT(14), 0x2a0, 0x2a4, 0x228),
+			BUS_PROT_WR(BIT(21), 0x2a8, 0x2ac, 0x258),
+		},
+		.caps = MTK_SCPD_ACTIVE_WAKEUP | MTK_SCPD_KEEP_DEFAULT_OFF,
+	},
+	[MT8365_POWER_DOMAIN_MFG] = {
+		.name = "mfg",
+		.sta_mask = PWR_STATUS_MFG,
+		.ctl_offs = 0x0338,
+		.pwr_sta_offs = 0x0180,
+		.pwr_sta2nd_offs = 0x0184,
+		.sram_pdn_bits = GENMASK(9, 8),
+		.sram_pdn_ack_bits = GENMASK(13, 12),
+		.bp_infracfg = {
+			BUS_PROT_WR(BIT(25), 0x2a0, 0x2a4, 0x228),
+			BUS_PROT_WR(BIT(21) | BIT(22), 0x2a0, 0x2a4, 0x228),
+		},
+	},
+	[MT8365_POWER_DOMAIN_CAM] = {
+		.name = "cam",
+		.sta_mask = BIT(25),
+		.ctl_offs = 0x0344,
+		.pwr_sta_offs = 0x0180,
+		.pwr_sta2nd_offs = 0x0184,
+		.sram_pdn_bits = GENMASK(9, 8),
+		.sram_pdn_ack_bits = GENMASK(13, 12),
+		.bp_infracfg = {
+			BUS_PROT_WR(BIT(19), 0x2a8, 0x2ac, 0x258),
+		},
+		.bp_smi = {
+			BUS_PROT_WR(BIT(2), 0x3c4, 0x3c8, 0x3c0),
+		},
+	},
+	[MT8365_POWER_DOMAIN_VDEC] = {
+		.name = "vdec",
+		.sta_mask = BIT(31),
+		.ctl_offs = 0x0370,
+		.pwr_sta_offs = 0x0180,
+		.pwr_sta2nd_offs = 0x0184,
+		.sram_pdn_bits = GENMASK(8, 8),
+		.sram_pdn_ack_bits = GENMASK(12, 12),
+		.bp_smi = {
+			BUS_PROT_WR(BIT(3), 0x3c4, 0x3c8, 0x3c0),
+		},
+	},
+	[MT8365_POWER_DOMAIN_APU] = {
+		.name = "apu",
+		.sta_mask = BIT(16),
+		.ctl_offs = 0x0378,
+		.pwr_sta_offs = 0x0180,
+		.pwr_sta2nd_offs = 0x0184,
+		.sram_pdn_bits = GENMASK(14, 8),
+		.sram_pdn_ack_bits = GENMASK(21, 15),
+		.bp_infracfg = {
+			BUS_PROT_WR(BIT(2) | BIT(20), 0x2a8, 0x2ac, 0x258),
+		},
+		.bp_smi = {
+			BUS_PROT_WR(BIT(4), 0x3c4, 0x3c8, 0x3c0),
+		},
+	},
+	[MT8365_POWER_DOMAIN_DSP] = {
+		.name = "dsp",
+		.sta_mask = BIT(17),
+		.ctl_offs = 0x037C,
+		.pwr_sta_offs = 0x0180,
+		.pwr_sta2nd_offs = 0x0184,
+		.sram_pdn_bits = GENMASK(11, 8),
+		.sram_pdn_ack_bits = GENMASK(15, 12),
+		.bp_infracfg = {
+			BUS_PROT_WR(BIT(24) | BIT(30) | BIT(31), 0x2a8, 0x2ac, 0x258),
+		},
+		.caps = MTK_SCPD_ACTIVE_WAKEUP,
+	},
+};
+
+static const struct scpsys_soc_data mt8365_scpsys_data = {
+	.domains_data = scpsys_domain_data_mt8365,
+	.num_domains = ARRAY_SIZE(scpsys_domain_data_mt8365),
+};
+
+#endif /* __SOC_MEDIATEK_MT8365_PM_DOMAINS_H */
diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
index 29a9028dd9b3..adb7716df0a8 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.c
+++ b/drivers/soc/mediatek/mtk-pm-domains.c
@@ -23,6 +23,7 @@
 #include "mt8186-pm-domains.h"
 #include "mt8192-pm-domains.h"
 #include "mt8195-pm-domains.h"
+#include "mt8365-pm-domains.h"
 
 #define MTK_POLL_DELAY_US		10
 #define MTK_POLL_TIMEOUT		USEC_PER_SEC
@@ -647,6 +648,10 @@ static const struct of_device_id scpsys_of_match[] = {
 		.compatible = "mediatek,mt8195-power-controller",
 		.data = &mt8195_scpsys_data,
 	},
+	{
+		.compatible = "mediatek,mt8365-power-controller",
+		.data = &mt8365_scpsys_data,
+	},
 	{ }
 };
 
-- 
2.39.0


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

Re: [PATCH 8/8] soc: mediatek: pm-domains: Add support for MT8365

[Matthias Brugger]

On 05/01/2023 18:07, Markus Schneider-Pargmann wrote:
> From: Fabien Parent <fparent@baylibre.com>
> 
> Add the needed board data to support MT8365 SoC.
> 
> Signed-off-by: Fabien Parent <fparent@baylibre.com>
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
>   drivers/soc/mediatek/mt8365-pm-domains.h | 147 +++++++++++++++++++++++
>   drivers/soc/mediatek/mtk-pm-domains.c    |   5 +
>   2 files changed, 152 insertions(+)
>   create mode 100644 drivers/soc/mediatek/mt8365-pm-domains.h
> 
> diff --git a/drivers/soc/mediatek/mt8365-pm-domains.h b/drivers/soc/mediatek/mt8365-pm-domains.h
> new file mode 100644
> index 000000000000..8735e833b15b
> --- /dev/null
> +++ b/drivers/soc/mediatek/mt8365-pm-domains.h
> @@ -0,0 +1,147 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef __SOC_MEDIATEK_MT8365_PM_DOMAINS_H
> +#define __SOC_MEDIATEK_MT8365_PM_DOMAINS_H
> +
> +#include "mtk-pm-domains.h"
> +#include <dt-bindings/power/mediatek,mt8365-power.h>
> +
> +/*
> + * MT8365 power domain support
> + */
> +
> +static const struct scpsys_domain_data scpsys_domain_data_mt8365[] = {
> +	[MT8365_POWER_DOMAIN_MM] = {
> +		.name = "mm",
> +		.sta_mask = PWR_STATUS_DISP,
> +		.ctl_offs = 0x30c,
> +		.pwr_sta_offs = 0x0180,
> +		.pwr_sta2nd_offs = 0x0184,
> +		.sram_pdn_bits = GENMASK(8, 8),
> +		.sram_pdn_ack_bits = GENMASK(12, 12),
> +		.caps = MTK_SCPD_STRICT_BUS_PROTECTION | MTK_SCPD_HAS_WAY_EN,
> +		.bp_infracfg = {
> +			BUS_PROT_WR(BIT(16) | BIT(17), 0x2a8, 0x2ac, 0x258),
> +			BUS_PROT_WR(BIT(1) | BIT(2) | BIT(10) | BIT(11), 0x2a0, 0x2a4, 0x228),
> +			BUS_PROT_WAY_EN(BIT(6), 0x200, BIT(24), 0x0),
> +			BUS_PROT_WAY_EN(BIT(5), 0x234, BIT(14), 0x28),
> +			BUS_PROT_WR(BIT(6), 0x2a0, 0x2a4, 0x228),


BUS_PROT_WR(BIT(6), 0x2a0, 0x2a4, 0x228) repeates several times in the 
definition. Would it make sense to create a new define like we did with 
BUS_PROT_UPDATE_TOPAXI()? Are this offests are used in other SoCs.

In any case instead of magic numbers the values should be defined in 
include/linux/soc/mediatek/infracfg.h or appropiate header files.

Regards,
Matthias

> +		},
> +	},
> +	[MT8365_POWER_DOMAIN_VENC] = {
> +		.name = "venc",
> +		.sta_mask = PWR_STATUS_VENC,
> +		.ctl_offs = 0x0304,
> +		.pwr_sta_offs = 0x0180,
> +		.pwr_sta2nd_offs = 0x0184,
> +		.sram_pdn_bits = GENMASK(8, 8),
> +		.sram_pdn_ack_bits = GENMASK(12, 12),
> +		.bp_smi = {
> +			BUS_PROT_WR(BIT(1), 0x3c4, 0x3c8, 0x3c0),
> +		},
> +	},
> +	[MT8365_POWER_DOMAIN_AUDIO] = {
> +		.name = "audio",
> +		.sta_mask = PWR_STATUS_AUDIO,
> +		.ctl_offs = 0x0314,
> +		.pwr_sta_offs = 0x0180,
> +		.pwr_sta2nd_offs = 0x0184,
> +		.sram_pdn_bits = GENMASK(12, 8),
> +		.sram_pdn_ack_bits = GENMASK(17, 13),
> +		.bp_infracfg = {
> +			BUS_PROT_WR(BIT(27) | BIT(28), 0x2a8, 0x2ac, 0x258),
> +		},
> +		.caps = MTK_SCPD_ACTIVE_WAKEUP,
> +	},
> +	[MT8365_POWER_DOMAIN_CONN] = {
> +		.name = "conn",
> +		.sta_mask = PWR_STATUS_CONN,
> +		.ctl_offs = 0x032c,
> +		.pwr_sta_offs = 0x0180,
> +		.pwr_sta2nd_offs = 0x0184,
> +		.sram_pdn_bits = 0,
> +		.sram_pdn_ack_bits = 0,
> +		.bp_infracfg = {
> +			BUS_PROT_WR(BIT(13), 0x2a0, 0x2a4, 0x228),
> +			BUS_PROT_WR(BIT(18), 0x2a8, 0x2ac, 0x258),
> +			BUS_PROT_WR(BIT(14), 0x2a0, 0x2a4, 0x228),
> +			BUS_PROT_WR(BIT(21), 0x2a8, 0x2ac, 0x258),
> +		},
> +		.caps = MTK_SCPD_ACTIVE_WAKEUP | MTK_SCPD_KEEP_DEFAULT_OFF,
> +	},
> +	[MT8365_POWER_DOMAIN_MFG] = {
> +		.name = "mfg",
> +		.sta_mask = PWR_STATUS_MFG,
> +		.ctl_offs = 0x0338,
> +		.pwr_sta_offs = 0x0180,
> +		.pwr_sta2nd_offs = 0x0184,
> +		.sram_pdn_bits = GENMASK(9, 8),
> +		.sram_pdn_ack_bits = GENMASK(13, 12),
> +		.bp_infracfg = {
> +			BUS_PROT_WR(BIT(25), 0x2a0, 0x2a4, 0x228),
> +			BUS_PROT_WR(BIT(21) | BIT(22), 0x2a0, 0x2a4, 0x228),
> +		},
> +	},
> +	[MT8365_POWER_DOMAIN_CAM] = {
> +		.name = "cam",
> +		.sta_mask = BIT(25),
> +		.ctl_offs = 0x0344,
> +		.pwr_sta_offs = 0x0180,
> +		.pwr_sta2nd_offs = 0x0184,
> +		.sram_pdn_bits = GENMASK(9, 8),
> +		.sram_pdn_ack_bits = GENMASK(13, 12),
> +		.bp_infracfg = {
> +			BUS_PROT_WR(BIT(19), 0x2a8, 0x2ac, 0x258),
> +		},
> +		.bp_smi = {
> +			BUS_PROT_WR(BIT(2), 0x3c4, 0x3c8, 0x3c0),
> +		},
> +	},
> +	[MT8365_POWER_DOMAIN_VDEC] = {
> +		.name = "vdec",
> +		.sta_mask = BIT(31),
> +		.ctl_offs = 0x0370,
> +		.pwr_sta_offs = 0x0180,
> +		.pwr_sta2nd_offs = 0x0184,
> +		.sram_pdn_bits = GENMASK(8, 8),
> +		.sram_pdn_ack_bits = GENMASK(12, 12),
> +		.bp_smi = {
> +			BUS_PROT_WR(BIT(3), 0x3c4, 0x3c8, 0x3c0),
> +		},
> +	},
> +	[MT8365_POWER_DOMAIN_APU] = {
> +		.name = "apu",
> +		.sta_mask = BIT(16),
> +		.ctl_offs = 0x0378,
> +		.pwr_sta_offs = 0x0180,
> +		.pwr_sta2nd_offs = 0x0184,
> +		.sram_pdn_bits = GENMASK(14, 8),
> +		.sram_pdn_ack_bits = GENMASK(21, 15),
> +		.bp_infracfg = {
> +			BUS_PROT_WR(BIT(2) | BIT(20), 0x2a8, 0x2ac, 0x258),
> +		},
> +		.bp_smi = {
> +			BUS_PROT_WR(BIT(4), 0x3c4, 0x3c8, 0x3c0),
> +		},
> +	},
> +	[MT8365_POWER_DOMAIN_DSP] = {
> +		.name = "dsp",
> +		.sta_mask = BIT(17),
> +		.ctl_offs = 0x037C,
> +		.pwr_sta_offs = 0x0180,
> +		.pwr_sta2nd_offs = 0x0184,
> +		.sram_pdn_bits = GENMASK(11, 8),
> +		.sram_pdn_ack_bits = GENMASK(15, 12),
> +		.bp_infracfg = {
> +			BUS_PROT_WR(BIT(24) | BIT(30) | BIT(31), 0x2a8, 0x2ac, 0x258),
> +		},
> +		.caps = MTK_SCPD_ACTIVE_WAKEUP,
> +	},
> +};
> +
> +static const struct scpsys_soc_data mt8365_scpsys_data = {
> +	.domains_data = scpsys_domain_data_mt8365,
> +	.num_domains = ARRAY_SIZE(scpsys_domain_data_mt8365),
> +};
> +
> +#endif /* __SOC_MEDIATEK_MT8365_PM_DOMAINS_H */
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
> index 29a9028dd9b3..adb7716df0a8 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.c
> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
> @@ -23,6 +23,7 @@
>   #include "mt8186-pm-domains.h"
>   #include "mt8192-pm-domains.h"
>   #include "mt8195-pm-domains.h"
> +#include "mt8365-pm-domains.h"
>   
>   #define MTK_POLL_DELAY_US		10
>   #define MTK_POLL_TIMEOUT		USEC_PER_SEC
> @@ -647,6 +648,10 @@ static const struct of_device_id scpsys_of_match[] = {
>   		.compatible = "mediatek,mt8195-power-controller",
>   		.data = &mt8195_scpsys_data,
>   	},
> +	{
> +		.compatible = "mediatek,mt8365-power-controller",
> +		.data = &mt8365_scpsys_data,
> +	},
>   	{ }
>   };
>   

Re: [PATCH 8/8] soc: mediatek: pm-domains: Add support for MT8365

[Matthias Brugger]

On 05/01/2023 18:07, Markus Schneider-Pargmann wrote:
> From: Fabien Parent <fparent@baylibre.com>
> 
> Add the needed board data to support MT8365 SoC.
> 
> Signed-off-by: Fabien Parent <fparent@baylibre.com>
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
>   drivers/soc/mediatek/mt8365-pm-domains.h | 147 +++++++++++++++++++++++
>   drivers/soc/mediatek/mtk-pm-domains.c    |   5 +
>   2 files changed, 152 insertions(+)
>   create mode 100644 drivers/soc/mediatek/mt8365-pm-domains.h
> 
> diff --git a/drivers/soc/mediatek/mt8365-pm-domains.h b/drivers/soc/mediatek/mt8365-pm-domains.h
> new file mode 100644
> index 000000000000..8735e833b15b
> --- /dev/null
> +++ b/drivers/soc/mediatek/mt8365-pm-domains.h
> @@ -0,0 +1,147 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef __SOC_MEDIATEK_MT8365_PM_DOMAINS_H
> +#define __SOC_MEDIATEK_MT8365_PM_DOMAINS_H
> +
> +#include "mtk-pm-domains.h"
> +#include <dt-bindings/power/mediatek,mt8365-power.h>
> +
> +/*
> + * MT8365 power domain support
> + */
> +
> +static const struct scpsys_domain_data scpsys_domain_data_mt8365[] = {
> +	[MT8365_POWER_DOMAIN_MM] = {
> +		.name = "mm",
> +		.sta_mask = PWR_STATUS_DISP,
> +		.ctl_offs = 0x30c,
> +		.pwr_sta_offs = 0x0180,
> +		.pwr_sta2nd_offs = 0x0184,
> +		.sram_pdn_bits = GENMASK(8, 8),
> +		.sram_pdn_ack_bits = GENMASK(12, 12),
> +		.caps = MTK_SCPD_STRICT_BUS_PROTECTION | MTK_SCPD_HAS_WAY_EN,
> +		.bp_infracfg = {
> +			BUS_PROT_WR(BIT(16) | BIT(17), 0x2a8, 0x2ac, 0x258),
> +			BUS_PROT_WR(BIT(1) | BIT(2) | BIT(10) | BIT(11), 0x2a0, 0x2a4, 0x228),
> +			BUS_PROT_WAY_EN(BIT(6), 0x200, BIT(24), 0x0),
> +			BUS_PROT_WAY_EN(BIT(5), 0x234, BIT(14), 0x28),
> +			BUS_PROT_WR(BIT(6), 0x2a0, 0x2a4, 0x228),


BUS_PROT_WR(BIT(6), 0x2a0, 0x2a4, 0x228) repeates several times in the 
definition. Would it make sense to create a new define like we did with 
BUS_PROT_UPDATE_TOPAXI()? Are this offests are used in other SoCs.

In any case instead of magic numbers the values should be defined in 
include/linux/soc/mediatek/infracfg.h or appropiate header files.

Regards,
Matthias

> +		},
> +	},
> +	[MT8365_POWER_DOMAIN_VENC] = {
> +		.name = "venc",
> +		.sta_mask = PWR_STATUS_VENC,
> +		.ctl_offs = 0x0304,
> +		.pwr_sta_offs = 0x0180,
> +		.pwr_sta2nd_offs = 0x0184,
> +		.sram_pdn_bits = GENMASK(8, 8),
> +		.sram_pdn_ack_bits = GENMASK(12, 12),
> +		.bp_smi = {
> +			BUS_PROT_WR(BIT(1), 0x3c4, 0x3c8, 0x3c0),
> +		},
> +	},
> +	[MT8365_POWER_DOMAIN_AUDIO] = {
> +		.name = "audio",
> +		.sta_mask = PWR_STATUS_AUDIO,
> +		.ctl_offs = 0x0314,
> +		.pwr_sta_offs = 0x0180,
> +		.pwr_sta2nd_offs = 0x0184,
> +		.sram_pdn_bits = GENMASK(12, 8),
> +		.sram_pdn_ack_bits = GENMASK(17, 13),
> +		.bp_infracfg = {
> +			BUS_PROT_WR(BIT(27) | BIT(28), 0x2a8, 0x2ac, 0x258),
> +		},
> +		.caps = MTK_SCPD_ACTIVE_WAKEUP,
> +	},
> +	[MT8365_POWER_DOMAIN_CONN] = {
> +		.name = "conn",
> +		.sta_mask = PWR_STATUS_CONN,
> +		.ctl_offs = 0x032c,
> +		.pwr_sta_offs = 0x0180,
> +		.pwr_sta2nd_offs = 0x0184,
> +		.sram_pdn_bits = 0,
> +		.sram_pdn_ack_bits = 0,
> +		.bp_infracfg = {
> +			BUS_PROT_WR(BIT(13), 0x2a0, 0x2a4, 0x228),
> +			BUS_PROT_WR(BIT(18), 0x2a8, 0x2ac, 0x258),
> +			BUS_PROT_WR(BIT(14), 0x2a0, 0x2a4, 0x228),
> +			BUS_PROT_WR(BIT(21), 0x2a8, 0x2ac, 0x258),
> +		},
> +		.caps = MTK_SCPD_ACTIVE_WAKEUP | MTK_SCPD_KEEP_DEFAULT_OFF,
> +	},
> +	[MT8365_POWER_DOMAIN_MFG] = {
> +		.name = "mfg",
> +		.sta_mask = PWR_STATUS_MFG,
> +		.ctl_offs = 0x0338,
> +		.pwr_sta_offs = 0x0180,
> +		.pwr_sta2nd_offs = 0x0184,
> +		.sram_pdn_bits = GENMASK(9, 8),
> +		.sram_pdn_ack_bits = GENMASK(13, 12),
> +		.bp_infracfg = {
> +			BUS_PROT_WR(BIT(25), 0x2a0, 0x2a4, 0x228),
> +			BUS_PROT_WR(BIT(21) | BIT(22), 0x2a0, 0x2a4, 0x228),
> +		},
> +	},
> +	[MT8365_POWER_DOMAIN_CAM] = {
> +		.name = "cam",
> +		.sta_mask = BIT(25),
> +		.ctl_offs = 0x0344,
> +		.pwr_sta_offs = 0x0180,
> +		.pwr_sta2nd_offs = 0x0184,
> +		.sram_pdn_bits = GENMASK(9, 8),
> +		.sram_pdn_ack_bits = GENMASK(13, 12),
> +		.bp_infracfg = {
> +			BUS_PROT_WR(BIT(19), 0x2a8, 0x2ac, 0x258),
> +		},
> +		.bp_smi = {
> +			BUS_PROT_WR(BIT(2), 0x3c4, 0x3c8, 0x3c0),
> +		},
> +	},
> +	[MT8365_POWER_DOMAIN_VDEC] = {
> +		.name = "vdec",
> +		.sta_mask = BIT(31),
> +		.ctl_offs = 0x0370,
> +		.pwr_sta_offs = 0x0180,
> +		.pwr_sta2nd_offs = 0x0184,
> +		.sram_pdn_bits = GENMASK(8, 8),
> +		.sram_pdn_ack_bits = GENMASK(12, 12),
> +		.bp_smi = {
> +			BUS_PROT_WR(BIT(3), 0x3c4, 0x3c8, 0x3c0),
> +		},
> +	},
> +	[MT8365_POWER_DOMAIN_APU] = {
> +		.name = "apu",
> +		.sta_mask = BIT(16),
> +		.ctl_offs = 0x0378,
> +		.pwr_sta_offs = 0x0180,
> +		.pwr_sta2nd_offs = 0x0184,
> +		.sram_pdn_bits = GENMASK(14, 8),
> +		.sram_pdn_ack_bits = GENMASK(21, 15),
> +		.bp_infracfg = {
> +			BUS_PROT_WR(BIT(2) | BIT(20), 0x2a8, 0x2ac, 0x258),
> +		},
> +		.bp_smi = {
> +			BUS_PROT_WR(BIT(4), 0x3c4, 0x3c8, 0x3c0),
> +		},
> +	},
> +	[MT8365_POWER_DOMAIN_DSP] = {
> +		.name = "dsp",
> +		.sta_mask = BIT(17),
> +		.ctl_offs = 0x037C,
> +		.pwr_sta_offs = 0x0180,
> +		.pwr_sta2nd_offs = 0x0184,
> +		.sram_pdn_bits = GENMASK(11, 8),
> +		.sram_pdn_ack_bits = GENMASK(15, 12),
> +		.bp_infracfg = {
> +			BUS_PROT_WR(BIT(24) | BIT(30) | BIT(31), 0x2a8, 0x2ac, 0x258),
> +		},
> +		.caps = MTK_SCPD_ACTIVE_WAKEUP,
> +	},
> +};
> +
> +static const struct scpsys_soc_data mt8365_scpsys_data = {
> +	.domains_data = scpsys_domain_data_mt8365,
> +	.num_domains = ARRAY_SIZE(scpsys_domain_data_mt8365),
> +};
> +
> +#endif /* __SOC_MEDIATEK_MT8365_PM_DOMAINS_H */
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
> index 29a9028dd9b3..adb7716df0a8 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.c
> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
> @@ -23,6 +23,7 @@
>   #include "mt8186-pm-domains.h"
>   #include "mt8192-pm-domains.h"
>   #include "mt8195-pm-domains.h"
> +#include "mt8365-pm-domains.h"
>   
>   #define MTK_POLL_DELAY_US		10
>   #define MTK_POLL_TIMEOUT		USEC_PER_SEC
> @@ -647,6 +648,10 @@ static const struct of_device_id scpsys_of_match[] = {
>   		.compatible = "mediatek,mt8195-power-controller",
>   		.data = &mt8195_scpsys_data,
>   	},
> +	{
> +		.compatible = "mediatek,mt8365-power-controller",
> +		.data = &mt8365_scpsys_data,
> +	},
>   	{ }
>   };
>   

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

Re: [PATCH 8/8] soc: mediatek: pm-domains: Add support for MT8365

[Markus Schneider-Pargmann]

Hi Matthias,

On Fri, Feb 03, 2023 at 01:22:38PM +0100, Matthias Brugger wrote:
> 
> 
> On 05/01/2023 18:07, Markus Schneider-Pargmann wrote:
> > From: Fabien Parent <fparent@baylibre.com>
> > 
> > Add the needed board data to support MT8365 SoC.
> > 
> > Signed-off-by: Fabien Parent <fparent@baylibre.com>
> > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> > ---
> >   drivers/soc/mediatek/mt8365-pm-domains.h | 147 +++++++++++++++++++++++
> >   drivers/soc/mediatek/mtk-pm-domains.c    |   5 +
> >   2 files changed, 152 insertions(+)
> >   create mode 100644 drivers/soc/mediatek/mt8365-pm-domains.h
> > 
> > diff --git a/drivers/soc/mediatek/mt8365-pm-domains.h b/drivers/soc/mediatek/mt8365-pm-domains.h
> > new file mode 100644
> > index 000000000000..8735e833b15b
> > --- /dev/null
> > +++ b/drivers/soc/mediatek/mt8365-pm-domains.h
> > @@ -0,0 +1,147 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +
> > +#ifndef __SOC_MEDIATEK_MT8365_PM_DOMAINS_H
> > +#define __SOC_MEDIATEK_MT8365_PM_DOMAINS_H
> > +
> > +#include "mtk-pm-domains.h"
> > +#include <dt-bindings/power/mediatek,mt8365-power.h>
> > +
> > +/*
> > + * MT8365 power domain support
> > + */
> > +
> > +static const struct scpsys_domain_data scpsys_domain_data_mt8365[] = {
> > +	[MT8365_POWER_DOMAIN_MM] = {
> > +		.name = "mm",
> > +		.sta_mask = PWR_STATUS_DISP,
> > +		.ctl_offs = 0x30c,
> > +		.pwr_sta_offs = 0x0180,
> > +		.pwr_sta2nd_offs = 0x0184,
> > +		.sram_pdn_bits = GENMASK(8, 8),
> > +		.sram_pdn_ack_bits = GENMASK(12, 12),
> > +		.caps = MTK_SCPD_STRICT_BUS_PROTECTION | MTK_SCPD_HAS_WAY_EN,
> > +		.bp_infracfg = {
> > +			BUS_PROT_WR(BIT(16) | BIT(17), 0x2a8, 0x2ac, 0x258),
> > +			BUS_PROT_WR(BIT(1) | BIT(2) | BIT(10) | BIT(11), 0x2a0, 0x2a4, 0x228),
> > +			BUS_PROT_WAY_EN(BIT(6), 0x200, BIT(24), 0x0),
> > +			BUS_PROT_WAY_EN(BIT(5), 0x234, BIT(14), 0x28),
> > +			BUS_PROT_WR(BIT(6), 0x2a0, 0x2a4, 0x228),
> 
> 
> BUS_PROT_WR(BIT(6), 0x2a0, 0x2a4, 0x228) repeates several times in the
> definition. Would it make sense to create a new define like we did with
> BUS_PROT_UPDATE_TOPAXI()? Are this offests are used in other SoCs.
> 
> In any case instead of magic numbers the values should be defined in
> include/linux/soc/mediatek/infracfg.h or appropiate header files.

Thanks, you are right, I got rid of all the magic numbers and introduced
some helper defines as well.

Thank you,
Markus

Re: [PATCH 8/8] soc: mediatek: pm-domains: Add support for MT8365

[Markus Schneider-Pargmann]

Hi Matthias,

On Fri, Feb 03, 2023 at 01:22:38PM +0100, Matthias Brugger wrote:
> 
> 
> On 05/01/2023 18:07, Markus Schneider-Pargmann wrote:
> > From: Fabien Parent <fparent@baylibre.com>
> > 
> > Add the needed board data to support MT8365 SoC.
> > 
> > Signed-off-by: Fabien Parent <fparent@baylibre.com>
> > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> > ---
> >   drivers/soc/mediatek/mt8365-pm-domains.h | 147 +++++++++++++++++++++++
> >   drivers/soc/mediatek/mtk-pm-domains.c    |   5 +
> >   2 files changed, 152 insertions(+)
> >   create mode 100644 drivers/soc/mediatek/mt8365-pm-domains.h
> > 
> > diff --git a/drivers/soc/mediatek/mt8365-pm-domains.h b/drivers/soc/mediatek/mt8365-pm-domains.h
> > new file mode 100644
> > index 000000000000..8735e833b15b
> > --- /dev/null
> > +++ b/drivers/soc/mediatek/mt8365-pm-domains.h
> > @@ -0,0 +1,147 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +
> > +#ifndef __SOC_MEDIATEK_MT8365_PM_DOMAINS_H
> > +#define __SOC_MEDIATEK_MT8365_PM_DOMAINS_H
> > +
> > +#include "mtk-pm-domains.h"
> > +#include <dt-bindings/power/mediatek,mt8365-power.h>
> > +
> > +/*
> > + * MT8365 power domain support
> > + */
> > +
> > +static const struct scpsys_domain_data scpsys_domain_data_mt8365[] = {
> > +	[MT8365_POWER_DOMAIN_MM] = {
> > +		.name = "mm",
> > +		.sta_mask = PWR_STATUS_DISP,
> > +		.ctl_offs = 0x30c,
> > +		.pwr_sta_offs = 0x0180,
> > +		.pwr_sta2nd_offs = 0x0184,
> > +		.sram_pdn_bits = GENMASK(8, 8),
> > +		.sram_pdn_ack_bits = GENMASK(12, 12),
> > +		.caps = MTK_SCPD_STRICT_BUS_PROTECTION | MTK_SCPD_HAS_WAY_EN,
> > +		.bp_infracfg = {
> > +			BUS_PROT_WR(BIT(16) | BIT(17), 0x2a8, 0x2ac, 0x258),
> > +			BUS_PROT_WR(BIT(1) | BIT(2) | BIT(10) | BIT(11), 0x2a0, 0x2a4, 0x228),
> > +			BUS_PROT_WAY_EN(BIT(6), 0x200, BIT(24), 0x0),
> > +			BUS_PROT_WAY_EN(BIT(5), 0x234, BIT(14), 0x28),
> > +			BUS_PROT_WR(BIT(6), 0x2a0, 0x2a4, 0x228),
> 
> 
> BUS_PROT_WR(BIT(6), 0x2a0, 0x2a4, 0x228) repeates several times in the
> definition. Would it make sense to create a new define like we did with
> BUS_PROT_UPDATE_TOPAXI()? Are this offests are used in other SoCs.
> 
> In any case instead of magic numbers the values should be defined in
> include/linux/soc/mediatek/infracfg.h or appropiate header files.

Thanks, you are right, I got rid of all the magic numbers and introduced
some helper defines as well.

Thank you,
Markus

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