[PATCH v1 0/7] Add JH7110 DPHY PMU support

[PATCH v1 0/7] Add JH7110 DPHY PMU support

[Changhuang Liang]

This patchset adds mipi dphy power domain driver for the StarFive JH7110
SoC. It is used to turn on dphy power switch. The series has been tested
on the VisionFive 2 board.

This patchset should be applied after the patchset [1]:
[1] https://lore.kernel.org/all/20230406103308.1280860-1-william.qiu@starfivetech.com/

Changhuang Liang (7):
  dt-bindings: power: Constrain properties for JH7110 PMU
  soc: starfive: Replace SOC_STARFIVE with ARCH_SATRFIVE
  soc: starfive: Modify ioremap to regmap
  soc: starfive: Add pmu type operation
  soc: starfive: Use call back to parse device tree resources
  soc: starfive: Add dphy pmu support
  riscv: dts: starfive: Add dphy rx pmu node

 .../bindings/power/starfive,jh7110-pmu.yaml   |  14 +-
 MAINTAINERS                                   |   1 +
 arch/riscv/boot/dts/starfive/jh7110.dtsi      |   5 +
 drivers/soc/starfive/Kconfig                  |   4 +-
 drivers/soc/starfive/jh71xx_pmu.c             | 213 ++++++++++++++----
 .../dt-bindings/power/starfive,jh7110-pmu.h   |   3 +
 6 files changed, 187 insertions(+), 53 deletions(-)


base-commit: 197b6b60ae7bc51dd0814953c562833143b292aa
prerequisite-patch-id: 388b8adbb0fe2daf4d07a21eafd4f1bd50ce2403
prerequisite-patch-id: 1117ecaa40a353c667b71802ab34ecf9568d8bb2
prerequisite-patch-id: b00c6b21fbd0353d88b7c9b09093ba30b765f45b
prerequisite-patch-id: 08ec9027e8a5c6fdf201726833168c7464a9b94d
prerequisite-patch-id: fb5120248e48fe1faf053ae0b490c92507ec2b44
prerequisite-patch-id: 4b93d8d590b0a2abe7b4be5287232c494c35be4a
prerequisite-patch-id: 89f049f951e5acf75aab92541992f816fd0acc0d
prerequisite-patch-id: c09c4c68af017b8e5c97b515cb50b70c18a2e705
prerequisite-patch-id: 0df8ccb0e848c2df4c2da95026494bebecede92d
prerequisite-patch-id: 315303931e4b6499de7127a88113763f86e97e16
prerequisite-patch-id: 40cb8212ddb024c20593f73d8b87d9894877e172
prerequisite-patch-id: a1673a9e9f19d6fab5a51abb721e54e36636f067
prerequisite-patch-id: d57cc467fb036241b9276320ff076c4a30d376d6
prerequisite-patch-id: 6e563d68bc5dbf951d4ced17897f9cc4d56169fe
prerequisite-patch-id: 61ec2caa21fd0fc60e57977f7d16d3f72b135745
prerequisite-patch-id: 1387a7e87b446329dfc21f3e575ceae7ebcf954c
prerequisite-patch-id: 258ea5f9b8bf41b6981345dcc81795f25865d38f
prerequisite-patch-id: 8b6f2c9660c0ac0ee4e73e4c21aca8e6b75e81b9
prerequisite-patch-id: dbb0c0151b8bdf093e6ce79fd2fe3f60791a6e0b
prerequisite-patch-id: 9007c8610fdcd387592475949864edde874c20a2
prerequisite-patch-id: d57e95d31686772abc4c4d5aa1cadc344dc293cd
prerequisite-patch-id: 0a0ac5a8a90655b415f6b62e324f3db083cdaaee
prerequisite-patch-id: 7ff6864ac74df5392c8646fe756cadd584fcc813
prerequisite-patch-id: 284b5d1b95c6d68bca08db1e82ed14930c98b777
--
2.25.1

[PATCH v1 1/7] dt-bindings: power: Constrain properties for JH7110 PMU

[Changhuang Liang]

When use "starfive,jh7110-pmu-dphy" compatible, do not need the reg and
interrupts properties.

Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
 .../bindings/power/starfive,jh7110-pmu.yaml        | 14 ++++++++++++--
 include/dt-bindings/power/starfive,jh7110-pmu.h    |  3 +++
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/power/starfive,jh7110-pmu.yaml b/Documentation/devicetree/bindings/power/starfive,jh7110-pmu.yaml
index 98eb8b4110e7..ffb4406c2e56 100644
--- a/Documentation/devicetree/bindings/power/starfive,jh7110-pmu.yaml
+++ b/Documentation/devicetree/bindings/power/starfive,jh7110-pmu.yaml
@@ -8,6 +8,7 @@ title: StarFive JH7110 Power Management Unit
 
 maintainers:
   - Walker Chen <walker.chen@starfivetech.com>
+  - Changhuang Liang <changhuang.liang@starfivetech.com>
 
 description: |
   StarFive JH7110 SoC includes support for multiple power domains which can be
@@ -17,6 +18,7 @@ properties:
   compatible:
     enum:
       - starfive,jh7110-pmu
+      - starfive,jh7110-pmu-dphy
 
   reg:
     maxItems: 1
@@ -29,10 +31,18 @@ properties:
 
 required:
   - compatible
-  - reg
-  - interrupts
   - "#power-domain-cells"
 
+if:
+  properties:
+    compatible:
+      contains:
+        const: starfive,jh7110-pmu
+then:
+  required:
+    - reg
+    - interrupts
+
 additionalProperties: false
 
 examples:
diff --git a/include/dt-bindings/power/starfive,jh7110-pmu.h b/include/dt-bindings/power/starfive,jh7110-pmu.h
index 132bfe401fc8..0bfd6700c144 100644
--- a/include/dt-bindings/power/starfive,jh7110-pmu.h
+++ b/include/dt-bindings/power/starfive,jh7110-pmu.h
@@ -14,4 +14,7 @@
 #define JH7110_PD_ISP		5
 #define JH7110_PD_VENC		6
 
+#define JH7110_PD_DPHY_TX	0
+#define JH7110_PD_DPHY_RX	1
+
 #endif
-- 
2.25.1

Re: [PATCH v1 1/7] dt-bindings: power: Constrain properties for JH7110 PMU

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2395 bytes --]

On Mon, Apr 10, 2023 at 11:47:37PM -0700, Changhuang Liang wrote:
> When use "starfive,jh7110-pmu-dphy" compatible, do not need the reg and
> interrupts properties.

Please write a commit message explaining why this is needed.
The commit message as-is is insufficient, but also IMO wrong incorrect.
I think it would more accurately be "...: add jh7110 dphy pmu support" or
similar & the body should explain why this particular PMU has no
reg/interrupts.

Cheers,
Conor.

> 
> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> ---
>  .../bindings/power/starfive,jh7110-pmu.yaml        | 14 ++++++++++++--
>  include/dt-bindings/power/starfive,jh7110-pmu.h    |  3 +++
>  2 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/power/starfive,jh7110-pmu.yaml b/Documentation/devicetree/bindings/power/starfive,jh7110-pmu.yaml
> index 98eb8b4110e7..ffb4406c2e56 100644
> --- a/Documentation/devicetree/bindings/power/starfive,jh7110-pmu.yaml
> +++ b/Documentation/devicetree/bindings/power/starfive,jh7110-pmu.yaml
> @@ -8,6 +8,7 @@ title: StarFive JH7110 Power Management Unit
>  
>  maintainers:
>    - Walker Chen <walker.chen@starfivetech.com>
> +  - Changhuang Liang <changhuang.liang@starfivetech.com>
>  
>  description: |
>    StarFive JH7110 SoC includes support for multiple power domains which can be
> @@ -17,6 +18,7 @@ properties:
>    compatible:
>      enum:
>        - starfive,jh7110-pmu
> +      - starfive,jh7110-pmu-dphy
>  
>    reg:
>      maxItems: 1
> @@ -29,10 +31,18 @@ properties:
>  
>  required:
>    - compatible
> -  - reg
> -  - interrupts
>    - "#power-domain-cells"
>  
> +if:
> +  properties:
> +    compatible:
> +      contains:
> +        const: starfive,jh7110-pmu
> +then:
> +  required:
> +    - reg
> +    - interrupts
> +
>  additionalProperties: false
>  
>  examples:
> diff --git a/include/dt-bindings/power/starfive,jh7110-pmu.h b/include/dt-bindings/power/starfive,jh7110-pmu.h
> index 132bfe401fc8..0bfd6700c144 100644
> --- a/include/dt-bindings/power/starfive,jh7110-pmu.h
> +++ b/include/dt-bindings/power/starfive,jh7110-pmu.h
> @@ -14,4 +14,7 @@
>  #define JH7110_PD_ISP		5
>  #define JH7110_PD_VENC		6
>  
> +#define JH7110_PD_DPHY_TX	0
> +#define JH7110_PD_DPHY_RX	1
> +
>  #endif
> -- 
> 2.25.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1 1/7] dt-bindings: power: Constrain properties for JH7110 PMU

[Changhuang Liang]

On 2023/4/12 4:13, Conor Dooley wrote:
> On Mon, Apr 10, 2023 at 11:47:37PM -0700, Changhuang Liang wrote:
>> When use "starfive,jh7110-pmu-dphy" compatible, do not need the reg and
>> interrupts properties.
> 
> Please write a commit message explaining why this is needed.
> The commit message as-is is insufficient, but also IMO wrong incorrect.
> I think it would more accurately be "...: add jh7110 dphy pmu support" or
> similar & the body should explain why this particular PMU has no
> reg/interrupts.
> 
> Cheers,
> Conor.
> 

OK, Thanks for your comments, I will reorganize the commit message more clarity.

>>
>> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
>> ---
>>  .../bindings/power/starfive,jh7110-pmu.yaml        | 14 ++++++++++++--
>>  include/dt-bindings/power/starfive,jh7110-pmu.h    |  3 +++
[...]
>>

Re: [PATCH v1 1/7] dt-bindings: power: Constrain properties for JH7110 PMU

[Krzysztof Kozlowski]

On 11/04/2023 08:47, Changhuang Liang wrote:
> When use "starfive,jh7110-pmu-dphy" compatible, do not need the reg and
> interrupts properties.
> 
> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> ---
>  .../bindings/power/starfive,jh7110-pmu.yaml        | 14 ++++++++++++--
>  include/dt-bindings/power/starfive,jh7110-pmu.h    |  3 +++
>  2 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/power/starfive,jh7110-pmu.yaml b/Documentation/devicetree/bindings/power/starfive,jh7110-pmu.yaml
> index 98eb8b4110e7..ffb4406c2e56 100644
> --- a/Documentation/devicetree/bindings/power/starfive,jh7110-pmu.yaml
> +++ b/Documentation/devicetree/bindings/power/starfive,jh7110-pmu.yaml
> @@ -8,6 +8,7 @@ title: StarFive JH7110 Power Management Unit
>  
>  maintainers:
>    - Walker Chen <walker.chen@starfivetech.com>
> +  - Changhuang Liang <changhuang.liang@starfivetech.com>
>  
>  description: |
>    StarFive JH7110 SoC includes support for multiple power domains which can be
> @@ -17,6 +18,7 @@ properties:
>    compatible:
>      enum:
>        - starfive,jh7110-pmu
> +      - starfive,jh7110-pmu-dphy

You do here much more than commit msg says.

Isn'y DPHY a phy? Why is it in power?

>  
>    reg:
>      maxItems: 1
> @@ -29,10 +31,18 @@ properties:
>  
>  required:
>    - compatible
> -  - reg
> -  - interrupts
>    - "#power-domain-cells"
>  
> +if:

Put it under allOf (in this place). Will save you one re-indentation later.

> +  properties:
> +    compatible:
> +      contains:
> +        const: starfive,jh7110-pmu
> +then:
> +  required:
> +    - reg
> +    - interrupts
> +
>  additionalProperties: false
>  
>  examples:
> diff --git a/include/dt-bindings/power/starfive,jh7110-pmu.h b/include/dt-bindings/power/starfive,jh7110-pmu.h
> index 132bfe401fc8..0bfd6700c144 100644
> --- a/include/dt-bindings/power/starfive,jh7110-pmu.h
> +++ b/include/dt-bindings/power/starfive,jh7110-pmu.h
> @@ -14,4 +14,7 @@
>  #define JH7110_PD_ISP		5
>  #define JH7110_PD_VENC		6
>  
> +#define JH7110_PD_DPHY_TX	0
> +#define JH7110_PD_DPHY_RX	1
> +
>  #endif

Best regards,
Krzysztof

Re: [PATCH v1 1/7] dt-bindings: power: Constrain properties for JH7110 PMU

[Changhuang Liang]

On 2023/4/12 16:35, Krzysztof Kozlowski wrote:
> On 11/04/2023 08:47, Changhuang Liang wrote:
>> When use "starfive,jh7110-pmu-dphy" compatible, do not need the reg and
>> interrupts properties.
[...]
>>  
>>  description: |
>>    StarFive JH7110 SoC includes support for multiple power domains which can be
>> @@ -17,6 +18,7 @@ properties:
>>    compatible:
>>      enum:
>>        - starfive,jh7110-pmu
>> +      - starfive,jh7110-pmu-dphy
> 
> You do here much more than commit msg says.
> 
> Isn'y DPHY a phy? Why is it in power?
> 

OK, I will add more description. This is a power framework used to turn on/off 
DPHY. So it in power, not a phy.

>>  
>>    reg:
>>      maxItems: 1
>> @@ -29,10 +31,18 @@ properties:
>>  
>>  required:
>>    - compatible
>> -  - reg
>> -  - interrupts
>>    - "#power-domain-cells"
>>  
>> +if:
> 
> Put it under allOf (in this place). Will save you one re-indentation later.
> 

OK, will fix it.

>> +  properties:
>> +    compatible:
>> +      contains:
>> +        const: starfive,jh7110-pmu
>> +then:
>> +  required:
>> +    - reg
>> +    - interrupts
>> +
>>  additionalProperties: false
>>  
>>  examples:
>> diff --git a/include/dt-bindings/power/starfive,jh7110-pmu.h b/include/dt-bindings/power/starfive,jh7110-pmu.h
>> index 132bfe401fc8..0bfd6700c144 100644
>> --- a/include/dt-bindings/power/starfive,jh7110-pmu.h
>> +++ b/include/dt-bindings/power/starfive,jh7110-pmu.h
>> @@ -14,4 +14,7 @@
>>  #define JH7110_PD_ISP		5
>>  #define JH7110_PD_VENC		6
>>  
>> +#define JH7110_PD_DPHY_TX	0
>> +#define JH7110_PD_DPHY_RX	1
>> +
>>  #endif
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v1 1/7] dt-bindings: power: Constrain properties for JH7110 PMU

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1135 bytes --]

On Wed, Apr 12, 2023 at 04:51:16PM +0800, Changhuang Liang wrote:
> 
> 
> On 2023/4/12 16:35, Krzysztof Kozlowski wrote:
> > On 11/04/2023 08:47, Changhuang Liang wrote:
> >> When use "starfive,jh7110-pmu-dphy" compatible, do not need the reg and
> >> interrupts properties.
> [...]
> >>  
> >>  description: |
> >>    StarFive JH7110 SoC includes support for multiple power domains which can be
> >> @@ -17,6 +18,7 @@ properties:
> >>    compatible:
> >>      enum:
> >>        - starfive,jh7110-pmu
> >> +      - starfive,jh7110-pmu-dphy
> > 
> > You do here much more than commit msg says.
> > 
> > Isn'y DPHY a phy? Why is it in power?
> > 
> 
> OK, I will add more description. This is a power framework used to turn on/off 
> DPHY. So it in power, not a phy.

Perhaps tie it less to its role w/ the phy, and more to do with its
location, say "jh7110-aon-pmu"?
There's already "aon"/"sys"/"stg" stuff used in clock-controller and
syscon compatibles etc.

Krzysztof, what do you think of that? (if you remember the whole
discussion we previously had about using those identifiers a few weeks
ago).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1 1/7] dt-bindings: power: Constrain properties for JH7110 PMU

[Krzysztof Kozlowski]

On 12/04/2023 11:42, Conor Dooley wrote:
> On Wed, Apr 12, 2023 at 04:51:16PM +0800, Changhuang Liang wrote:
>>
>>
>> On 2023/4/12 16:35, Krzysztof Kozlowski wrote:
>>> On 11/04/2023 08:47, Changhuang Liang wrote:
>>>> When use "starfive,jh7110-pmu-dphy" compatible, do not need the reg and
>>>> interrupts properties.
>> [...]
>>>>  
>>>>  description: |
>>>>    StarFive JH7110 SoC includes support for multiple power domains which can be
>>>> @@ -17,6 +18,7 @@ properties:
>>>>    compatible:
>>>>      enum:
>>>>        - starfive,jh7110-pmu
>>>> +      - starfive,jh7110-pmu-dphy
>>>
>>> You do here much more than commit msg says.
>>>
>>> Isn'y DPHY a phy? Why is it in power?
>>>
>>
>> OK, I will add more description. This is a power framework used to turn on/off 
>> DPHY. So it in power, not a phy.
> 
> Perhaps tie it less to its role w/ the phy, and more to do with its
> location, say "jh7110-aon-pmu"?
> There's already "aon"/"sys"/"stg" stuff used in clock-controller and
> syscon compatibles etc.
> 
> Krzysztof, what do you think of that? (if you remember the whole
> discussion we previously had about using those identifiers a few weeks
> ago).

Depends whether this is the same case or not. AFAIR, for AON/SYS/STG
these were blocks with few features, not only clock controller.

This sounds like just phy. Powering on/off phy is still a job of phy
controller... unless it is a power domain controller.
Best regards,
Krzysztof

Re: [PATCH v1 1/7] dt-bindings: power: Constrain properties for JH7110 PMU

[Changhuang Liang]

On 2023/4/12 19:29, Krzysztof Kozlowski wrote:
> On 12/04/2023 11:42, Conor Dooley wrote:
>> On Wed, Apr 12, 2023 at 04:51:16PM +0800, Changhuang Liang wrote:
>>>
>>>
>>> On 2023/4/12 16:35, Krzysztof Kozlowski wrote:
>>>> On 11/04/2023 08:47, Changhuang Liang wrote:
>>>>> When use "starfive,jh7110-pmu-dphy" compatible, do not need the reg and
>>>>> interrupts properties.
>>> [...]
>>>>>  
>>>>>  description: |
>>>>>    StarFive JH7110 SoC includes support for multiple power domains which can be
>>>>> @@ -17,6 +18,7 @@ properties:
>>>>>    compatible:
>>>>>      enum:
>>>>>        - starfive,jh7110-pmu
>>>>> +      - starfive,jh7110-pmu-dphy
>>>>
>>>> You do here much more than commit msg says.
>>>>
>>>> Isn'y DPHY a phy? Why is it in power?
>>>>
>>>
>>> OK, I will add more description. This is a power framework used to turn on/off 
>>> DPHY. So it in power, not a phy.
>>
>> Perhaps tie it less to its role w/ the phy, and more to do with its
>> location, say "jh7110-aon-pmu"?
>> There's already "aon"/"sys"/"stg" stuff used in clock-controller and
>> syscon compatibles etc.
>>
>> Krzysztof, what do you think of that? (if you remember the whole
>> discussion we previously had about using those identifiers a few weeks
>> ago).
> 
> Depends whether this is the same case or not. AFAIR, for AON/SYS/STG
> these were blocks with few features, not only clock controller.
> 
> This sounds like just phy. Powering on/off phy is still a job of phy
> controller... unless it is a power domain controller.
> Best regards,
> Krzysztof
> 

Hi, Coner and Krzysztof,

Next version I will change commit message:

dt-bindings: power: Add JH7110 DPHY PMU support.

Add DPHY PMU for StarFive JH7110 SoC, it can be used to turn on/off DPHY rx/tx
power switch, and it don't need the reg and interrupt properties.

I think this commit message will helpful for you to understand it.

Best regards,
Changhuang

Re: [PATCH v1 1/7] dt-bindings: power: Constrain properties for JH7110 PMU

[Changhuang Liang]

On 2023/4/12 19:29, Krzysztof Kozlowski wrote:
> On 12/04/2023 11:42, Conor Dooley wrote:
>> On Wed, Apr 12, 2023 at 04:51:16PM +0800, Changhuang Liang wrote:
>>>
>>>
>>> On 2023/4/12 16:35, Krzysztof Kozlowski wrote:
>>>> On 11/04/2023 08:47, Changhuang Liang wrote:
>>>>> When use "starfive,jh7110-pmu-dphy" compatible, do not need the reg and
>>>>> interrupts properties.
>>> [...]
>>>>>  
>>>>>  description: |
>>>>>    StarFive JH7110 SoC includes support for multiple power domains which can be
>>>>> @@ -17,6 +18,7 @@ properties:
>>>>>    compatible:
>>>>>      enum:
>>>>>        - starfive,jh7110-pmu
>>>>> +      - starfive,jh7110-pmu-dphy
>>>>
>>>> You do here much more than commit msg says.
>>>>
>>>> Isn'y DPHY a phy? Why is it in power?
>>>>
>>>
>>> OK, I will add more description. This is a power framework used to turn on/off 
>>> DPHY. So it in power, not a phy.

I found something wrong with my description here, not turn on/off DPHY,
is turn on/off DPHY power switch. 

>>
>> Perhaps tie it less to its role w/ the phy, and more to do with its
>> location, say "jh7110-aon-pmu"?
>> There's already "aon"/"sys"/"stg" stuff used in clock-controller and
>> syscon compatibles etc.
>>
>> Krzysztof, what do you think of that? (if you remember the whole
>> discussion we previously had about using those identifiers a few weeks
>> ago).
> 
> Depends whether this is the same case or not. AFAIR, for AON/SYS/STG
> these were blocks with few features, not only clock controller.
> 
> This sounds like just phy. Powering on/off phy is still a job of phy
> controller... unless it is a power domain controller.
> Best regards,
> Krzysztof
> 

So, next version the compatible can be changed to "jh7110-aon-pmu"?

Re: [PATCH v1 1/7] dt-bindings: power: Constrain properties for JH7110 PMU

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2228 bytes --]

On Fri, Apr 14, 2023 at 10:20:31AM +0800, Changhuang Liang wrote:
> 
> 
> On 2023/4/12 19:29, Krzysztof Kozlowski wrote:
> > On 12/04/2023 11:42, Conor Dooley wrote:
> >> On Wed, Apr 12, 2023 at 04:51:16PM +0800, Changhuang Liang wrote:
> >>>
> >>>
> >>> On 2023/4/12 16:35, Krzysztof Kozlowski wrote:
> >>>> On 11/04/2023 08:47, Changhuang Liang wrote:
> >>>>> When use "starfive,jh7110-pmu-dphy" compatible, do not need the reg and
> >>>>> interrupts properties.
> >>> [...]
> >>>>>  
> >>>>>  description: |
> >>>>>    StarFive JH7110 SoC includes support for multiple power domains which can be
> >>>>> @@ -17,6 +18,7 @@ properties:
> >>>>>    compatible:
> >>>>>      enum:
> >>>>>        - starfive,jh7110-pmu
> >>>>> +      - starfive,jh7110-pmu-dphy
> >>>>
> >>>> You do here much more than commit msg says.
> >>>>
> >>>> Isn'y DPHY a phy? Why is it in power?
> >>>>
> >>>
> >>> OK, I will add more description. This is a power framework used to turn on/off 
> >>> DPHY. So it in power, not a phy.
> 
> I found something wrong with my description here, not turn on/off DPHY,
> is turn on/off DPHY power switch. 
> 
> >>
> >> Perhaps tie it less to its role w/ the phy, and more to do with its
> >> location, say "jh7110-aon-pmu"?
> >> There's already "aon"/"sys"/"stg" stuff used in clock-controller and
> >> syscon compatibles etc.
> >>
> >> Krzysztof, what do you think of that? (if you remember the whole
> >> discussion we previously had about using those identifiers a few weeks
> >> ago).
> > 
> > Depends whether this is the same case or not. AFAIR, for AON/SYS/STG
> > these were blocks with few features, not only clock controller.
> > 
> > This sounds like just phy. Powering on/off phy is still a job of phy
> > controller... unless it is a power domain controller.
> > Best regards,
> > Krzysztof
> > 
> 
> So, next version the compatible can be changed to "jh7110-aon-pmu"?

Hmm, is the dphy the only thing that's power is controlled by registers
in the aon syscon? I tried looking in the "preliminary" TRM that I have,
but it's not really got a proper register map so I could not tell.

If there are, it'd help your case I think Changhuang Liang.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1 1/7] dt-bindings: power: Constrain properties for JH7110 PMU

[Changhuang Liang]

On 2023/4/18 2:55, Conor Dooley wrote:
> On Fri, Apr 14, 2023 at 10:20:31AM +0800, Changhuang Liang wrote:
>>
>>
>> On 2023/4/12 19:29, Krzysztof Kozlowski wrote:
>>> On 12/04/2023 11:42, Conor Dooley wrote:
>>>> On Wed, Apr 12, 2023 at 04:51:16PM +0800, Changhuang Liang wrote:
>>>>>
>>>>>
>>>>> On 2023/4/12 16:35, Krzysztof Kozlowski wrote:
>>>>>> On 11/04/2023 08:47, Changhuang Liang wrote:
>>>>>>> When use "starfive,jh7110-pmu-dphy" compatible, do not need the reg and
>>>>>>> interrupts properties.
>>>>> [...]
>>>>>>>  
>>>>>>>  description: |
>>>>>>>    StarFive JH7110 SoC includes support for multiple power domains which can be
>>>>>>> @@ -17,6 +18,7 @@ properties:
>>>>>>>    compatible:
>>>>>>>      enum:
>>>>>>>        - starfive,jh7110-pmu
>>>>>>> +      - starfive,jh7110-pmu-dphy
>>>>>>
>>>>>> You do here much more than commit msg says.
>>>>>>
>>>>>> Isn'y DPHY a phy? Why is it in power?
>>>>>>
>>>>>
>>>>> OK, I will add more description. This is a power framework used to turn on/off 
>>>>> DPHY. So it in power, not a phy.
>>
>> I found something wrong with my description here, not turn on/off DPHY,
>> is turn on/off DPHY power switch. 
>>
>>>>
>>>> Perhaps tie it less to its role w/ the phy, and more to do with its
>>>> location, say "jh7110-aon-pmu"?
>>>> There's already "aon"/"sys"/"stg" stuff used in clock-controller and
>>>> syscon compatibles etc.
>>>>
>>>> Krzysztof, what do you think of that? (if you remember the whole
>>>> discussion we previously had about using those identifiers a few weeks
>>>> ago).
>>>
>>> Depends whether this is the same case or not. AFAIR, for AON/SYS/STG
>>> these were blocks with few features, not only clock controller.
>>>
>>> This sounds like just phy. Powering on/off phy is still a job of phy
>>> controller... unless it is a power domain controller.
>>> Best regards,
>>> Krzysztof
>>>
>>
>> So, next version the compatible can be changed to "jh7110-aon-pmu"?
> 
> Hmm, is the dphy the only thing that's power is controlled by registers
> in the aon syscon? I tried looking in the "preliminary" TRM that I have,
> but it's not really got a proper register map so I could not tell.
> 
> If there are, it'd help your case I think Changhuang Liang.

I made a discussion with Walker, We don't use other bit on the visionfive2 
board. And I first naming by function. So I will change to "jh7110-aon-pmu"
next version.

Re: [PATCH v1 1/7] dt-bindings: power: Constrain properties for JH7110 PMU

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1628 bytes --]

On Wed, Apr 12, 2023 at 01:29:57PM +0200, Krzysztof Kozlowski wrote:
> On 12/04/2023 11:42, Conor Dooley wrote:
> > On Wed, Apr 12, 2023 at 04:51:16PM +0800, Changhuang Liang wrote:
> >>
> >>
> >> On 2023/4/12 16:35, Krzysztof Kozlowski wrote:
> >>> On 11/04/2023 08:47, Changhuang Liang wrote:
> >>>> When use "starfive,jh7110-pmu-dphy" compatible, do not need the reg and
> >>>> interrupts properties.
> >> [...]
> >>>>  
> >>>>  description: |
> >>>>    StarFive JH7110 SoC includes support for multiple power domains which can be
> >>>> @@ -17,6 +18,7 @@ properties:
> >>>>    compatible:
> >>>>      enum:
> >>>>        - starfive,jh7110-pmu
> >>>> +      - starfive,jh7110-pmu-dphy
> >>>
> >>> You do here much more than commit msg says.
> >>>
> >>> Isn'y DPHY a phy? Why is it in power?
> >>>
> >>
> >> OK, I will add more description. This is a power framework used to turn on/off 
> >> DPHY. So it in power, not a phy.
> > 
> > Perhaps tie it less to its role w/ the phy, and more to do with its
> > location, say "jh7110-aon-pmu"?
> > There's already "aon"/"sys"/"stg" stuff used in clock-controller and
> > syscon compatibles etc.
> > 
> > Krzysztof, what do you think of that? (if you remember the whole
> > discussion we previously had about using those identifiers a few weeks
> > ago).
> 
> Depends whether this is the same case or not.

> AFAIR, for AON/SYS/STG
> these were blocks with few features, not only clock controller.

Correct, yes. In the dts, this "pmu-dphy" node is a child node of the
aon syscon, so this pmu stuff would be one of the several features.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v1 2/7] soc: starfive: Replace SOC_STARFIVE with ARCH_SATRFIVE

[Changhuang Liang]

Using ARCH_FOO symbol is preferred than SOC_FOO.

Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
 drivers/soc/starfive/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/starfive/Kconfig b/drivers/soc/starfive/Kconfig
index bdb96dc4c989..1e9b0c414fec 100644
--- a/drivers/soc/starfive/Kconfig
+++ b/drivers/soc/starfive/Kconfig
@@ -3,8 +3,8 @@
 config JH71XX_PMU
 	bool "Support PMU for StarFive JH71XX Soc"
 	depends on PM
-	depends on SOC_STARFIVE || COMPILE_TEST
-	default SOC_STARFIVE
+	depends on ARCH_STARFIVE || COMPILE_TEST
+	default ARCH_STARFIVE
 	select PM_GENERIC_DOMAINS
 	help
 	  Say 'y' here to enable support power domain support.
-- 
2.25.1

Re: [PATCH v1 2/7] soc: starfive: Replace SOC_STARFIVE with ARCH_SATRFIVE

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 928 bytes --]

On Mon, Apr 10, 2023 at 11:47:38PM -0700, Changhuang Liang wrote:
> Using ARCH_FOO symbol is preferred than SOC_FOO.
> 

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> ---
>  drivers/soc/starfive/Kconfig | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soc/starfive/Kconfig b/drivers/soc/starfive/Kconfig
> index bdb96dc4c989..1e9b0c414fec 100644
> --- a/drivers/soc/starfive/Kconfig
> +++ b/drivers/soc/starfive/Kconfig
> @@ -3,8 +3,8 @@
>  config JH71XX_PMU
>  	bool "Support PMU for StarFive JH71XX Soc"
>  	depends on PM
> -	depends on SOC_STARFIVE || COMPILE_TEST
> -	default SOC_STARFIVE
> +	depends on ARCH_STARFIVE || COMPILE_TEST
> +	default ARCH_STARFIVE
>  	select PM_GENERIC_DOMAINS
>  	help
>  	  Say 'y' here to enable support power domain support.
> -- 
> 2.25.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1 2/7] soc: starfive: Replace SOC_STARFIVE with ARCH_SATRFIVE

[Walker Chen]

On 2023/4/11 14:47, Changhuang Liang wrote:
> Using ARCH_FOO symbol is preferred than SOC_FOO.
> 
> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> ---
>  drivers/soc/starfive/Kconfig | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soc/starfive/Kconfig b/drivers/soc/starfive/Kconfig
> index bdb96dc4c989..1e9b0c414fec 100644
> --- a/drivers/soc/starfive/Kconfig
> +++ b/drivers/soc/starfive/Kconfig
> @@ -3,8 +3,8 @@
>  config JH71XX_PMU
>  	bool "Support PMU for StarFive JH71XX Soc"
>  	depends on PM
> -	depends on SOC_STARFIVE || COMPILE_TEST
> -	default SOC_STARFIVE
> +	depends on ARCH_STARFIVE || COMPILE_TEST
> +	default ARCH_STARFIVE
>  	select PM_GENERIC_DOMAINS
>  	help
>  	  Say 'y' here to enable support power domain support.

The subject 'ARCH_SATRFIVE' spelling error.

Best regards,
Walker

Re: [PATCH v1 2/7] soc: starfive: Replace SOC_STARFIVE with ARCH_SATRFIVE

[Changhuang Liang]

On 2023/4/12 10:11, Walker Chen wrote:
> 
> 
> On 2023/4/11 14:47, Changhuang Liang wrote:
>> Using ARCH_FOO symbol is preferred than SOC_FOO.
>>
>> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
>> ---
>>  drivers/soc/starfive/Kconfig | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/soc/starfive/Kconfig b/drivers/soc/starfive/Kconfig
>> index bdb96dc4c989..1e9b0c414fec 100644
>> --- a/drivers/soc/starfive/Kconfig
>> +++ b/drivers/soc/starfive/Kconfig
>> @@ -3,8 +3,8 @@
>>  config JH71XX_PMU
>>  	bool "Support PMU for StarFive JH71XX Soc"
>>  	depends on PM
>> -	depends on SOC_STARFIVE || COMPILE_TEST
>> -	default SOC_STARFIVE
>> +	depends on ARCH_STARFIVE || COMPILE_TEST
>> +	default ARCH_STARFIVE
>>  	select PM_GENERIC_DOMAINS
>>  	help
>>  	  Say 'y' here to enable support power domain support.
> 
> The subject 'ARCH_SATRFIVE' spelling error.
> 

OK, will fix it.

> Best regards,
> Walker

[PATCH v1 3/7] soc: starfive: Modify ioremap to regmap

[Changhuang Liang]

Modify ioremap to regmap, easy to simplify code.

Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
 drivers/soc/starfive/jh71xx_pmu.c | 43 +++++++++++++++++--------------
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/drivers/soc/starfive/jh71xx_pmu.c b/drivers/soc/starfive/jh71xx_pmu.c
index 7d5f50d71c0d..306218c83691 100644
--- a/drivers/soc/starfive/jh71xx_pmu.c
+++ b/drivers/soc/starfive/jh71xx_pmu.c
@@ -6,13 +6,13 @@
  */
 
 #include <linux/interrupt.h>
-#include <linux/io.h>
-#include <linux/iopoll.h>
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/pm_domain.h>
+#include <linux/regmap.h>
 #include <dt-bindings/power/starfive,jh7110-pmu.h>
 
 /* register offset */
@@ -59,7 +59,7 @@ struct jh71xx_pmu_match_data {
 struct jh71xx_pmu {
 	struct device *dev;
 	const struct jh71xx_pmu_match_data *match_data;
-	void __iomem *base;
+	struct regmap *base;
 	struct generic_pm_domain **genpd;
 	struct genpd_onecell_data genpd_data;
 	int irq;
@@ -75,11 +75,14 @@ struct jh71xx_pmu_dev {
 static int jh71xx_pmu_get_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool *is_on)
 {
 	struct jh71xx_pmu *pmu = pmd->pmu;
+	unsigned int val;
 
 	if (!mask)
 		return -EINVAL;
 
-	*is_on = readl(pmu->base + JH71XX_PMU_CURR_POWER_MODE) & mask;
+	regmap_read(pmu->base, JH71XX_PMU_CURR_POWER_MODE, &val);
+
+	*is_on = val & mask;
 
 	return 0;
 }
@@ -130,7 +133,7 @@ static int jh71xx_pmu_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
 		encourage_hi = JH71XX_PMU_SW_ENCOURAGE_DIS_HI;
 	}
 
-	writel(mask, pmu->base + mode);
+	regmap_write(pmu->base, mode, mask);
 
 	/*
 	 * 2.Write SW encourage command sequence to the Software Encourage Reg (offset 0x44)
@@ -140,21 +143,21 @@ static int jh71xx_pmu_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
 	 *   Then write the lower bits of the command sequence, followed by the upper
 	 *   bits. The sequence differs between powering on & off a domain.
 	 */
-	writel(JH71XX_PMU_SW_ENCOURAGE_ON, pmu->base + JH71XX_PMU_SW_ENCOURAGE);
-	writel(encourage_lo, pmu->base + JH71XX_PMU_SW_ENCOURAGE);
-	writel(encourage_hi, pmu->base + JH71XX_PMU_SW_ENCOURAGE);
+	regmap_write(pmu->base, JH71XX_PMU_SW_ENCOURAGE, JH71XX_PMU_SW_ENCOURAGE_ON);
+	regmap_write(pmu->base, JH71XX_PMU_SW_ENCOURAGE, encourage_lo);
+	regmap_write(pmu->base, JH71XX_PMU_SW_ENCOURAGE, encourage_hi);
 
 	spin_unlock_irqrestore(&pmu->lock, flags);
 
 	/* Wait for the power domain bit to be enabled / disabled */
 	if (on) {
-		ret = readl_poll_timeout_atomic(pmu->base + JH71XX_PMU_CURR_POWER_MODE,
-						val, val & mask,
-						1, JH71XX_PMU_TIMEOUT_US);
+		ret = regmap_read_poll_timeout_atomic(pmu->base, JH71XX_PMU_CURR_POWER_MODE,
+						      val, val & mask,
+						      1, JH71XX_PMU_TIMEOUT_US);
 	} else {
-		ret = readl_poll_timeout_atomic(pmu->base + JH71XX_PMU_CURR_POWER_MODE,
-						val, !(val & mask),
-						1, JH71XX_PMU_TIMEOUT_US);
+		ret = regmap_read_poll_timeout_atomic(pmu->base, JH71XX_PMU_CURR_POWER_MODE,
+						      val, !(val & mask),
+						      1, JH71XX_PMU_TIMEOUT_US);
 	}
 
 	if (ret) {
@@ -190,14 +193,14 @@ static void jh71xx_pmu_int_enable(struct jh71xx_pmu *pmu, u32 mask, bool enable)
 	unsigned long flags;
 
 	spin_lock_irqsave(&pmu->lock, flags);
-	val = readl(pmu->base + JH71XX_PMU_TIMER_INT_MASK);
+	regmap_read(pmu->base, JH71XX_PMU_TIMER_INT_MASK, &val);
 
 	if (enable)
 		val &= ~mask;
 	else
 		val |= mask;
 
-	writel(val, pmu->base + JH71XX_PMU_TIMER_INT_MASK);
+	regmap_write(pmu->base, JH71XX_PMU_TIMER_INT_MASK, val);
 	spin_unlock_irqrestore(&pmu->lock, flags);
 }
 
@@ -206,7 +209,7 @@ static irqreturn_t jh71xx_pmu_interrupt(int irq, void *data)
 	struct jh71xx_pmu *pmu = data;
 	u32 val;
 
-	val = readl(pmu->base + JH71XX_PMU_INT_STATUS);
+	regmap_read(pmu->base, JH71XX_PMU_INT_STATUS, &val);
 
 	if (val & JH71XX_PMU_INT_SEQ_DONE)
 		dev_dbg(pmu->dev, "sequence done.\n");
@@ -220,8 +223,8 @@ static irqreturn_t jh71xx_pmu_interrupt(int irq, void *data)
 		dev_err(pmu->dev, "p-channel fail event.\n");
 
 	/* clear interrupts */
-	writel(val, pmu->base + JH71XX_PMU_INT_STATUS);
-	writel(val, pmu->base + JH71XX_PMU_EVENT_STATUS);
+	regmap_write(pmu->base, JH71XX_PMU_INT_STATUS, val);
+	regmap_write(pmu->base, JH71XX_PMU_EVENT_STATUS, val);
 
 	return IRQ_HANDLED;
 }
@@ -271,7 +274,7 @@ static int jh71xx_pmu_probe(struct platform_device *pdev)
 	if (!pmu)
 		return -ENOMEM;
 
-	pmu->base = devm_platform_ioremap_resource(pdev, 0);
+	pmu->base = device_node_to_regmap(np);
 	if (IS_ERR(pmu->base))
 		return PTR_ERR(pmu->base);
 
-- 
2.25.1

Re: [PATCH v1 3/7] soc: starfive: Modify ioremap to regmap

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 5491 bytes --]

On Mon, Apr 10, 2023 at 11:47:39PM -0700, Changhuang Liang wrote:
> Modify ioremap to regmap, easy to simplify code.

This doesn't simplify anything, adding regmap to the mix actually makes
it less obvious what is going on here & it's not even fewer LoC:
1 file changed, 23 insertions(+), 20 deletions(-)

Please write a commit message that explains the real motivation for
this change.

Thanks,
Conor.

> 
> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> ---
>  drivers/soc/starfive/jh71xx_pmu.c | 43 +++++++++++++++++--------------
>  1 file changed, 23 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/soc/starfive/jh71xx_pmu.c b/drivers/soc/starfive/jh71xx_pmu.c
> index 7d5f50d71c0d..306218c83691 100644
> --- a/drivers/soc/starfive/jh71xx_pmu.c
> +++ b/drivers/soc/starfive/jh71xx_pmu.c
> @@ -6,13 +6,13 @@
>   */
>  
>  #include <linux/interrupt.h>
> -#include <linux/io.h>
> -#include <linux/iopoll.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_domain.h>
> +#include <linux/regmap.h>
>  #include <dt-bindings/power/starfive,jh7110-pmu.h>
>  
>  /* register offset */
> @@ -59,7 +59,7 @@ struct jh71xx_pmu_match_data {
>  struct jh71xx_pmu {
>  	struct device *dev;
>  	const struct jh71xx_pmu_match_data *match_data;
> -	void __iomem *base;
> +	struct regmap *base;
>  	struct generic_pm_domain **genpd;
>  	struct genpd_onecell_data genpd_data;
>  	int irq;
> @@ -75,11 +75,14 @@ struct jh71xx_pmu_dev {
>  static int jh71xx_pmu_get_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool *is_on)
>  {
>  	struct jh71xx_pmu *pmu = pmd->pmu;
> +	unsigned int val;
>  
>  	if (!mask)
>  		return -EINVAL;
>  
> -	*is_on = readl(pmu->base + JH71XX_PMU_CURR_POWER_MODE) & mask;
> +	regmap_read(pmu->base, JH71XX_PMU_CURR_POWER_MODE, &val);
> +
> +	*is_on = val & mask;
>  
>  	return 0;
>  }
> @@ -130,7 +133,7 @@ static int jh71xx_pmu_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
>  		encourage_hi = JH71XX_PMU_SW_ENCOURAGE_DIS_HI;
>  	}
>  
> -	writel(mask, pmu->base + mode);
> +	regmap_write(pmu->base, mode, mask);
>  
>  	/*
>  	 * 2.Write SW encourage command sequence to the Software Encourage Reg (offset 0x44)
> @@ -140,21 +143,21 @@ static int jh71xx_pmu_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
>  	 *   Then write the lower bits of the command sequence, followed by the upper
>  	 *   bits. The sequence differs between powering on & off a domain.
>  	 */
> -	writel(JH71XX_PMU_SW_ENCOURAGE_ON, pmu->base + JH71XX_PMU_SW_ENCOURAGE);
> -	writel(encourage_lo, pmu->base + JH71XX_PMU_SW_ENCOURAGE);
> -	writel(encourage_hi, pmu->base + JH71XX_PMU_SW_ENCOURAGE);
> +	regmap_write(pmu->base, JH71XX_PMU_SW_ENCOURAGE, JH71XX_PMU_SW_ENCOURAGE_ON);
> +	regmap_write(pmu->base, JH71XX_PMU_SW_ENCOURAGE, encourage_lo);
> +	regmap_write(pmu->base, JH71XX_PMU_SW_ENCOURAGE, encourage_hi);
>  
>  	spin_unlock_irqrestore(&pmu->lock, flags);
>  
>  	/* Wait for the power domain bit to be enabled / disabled */
>  	if (on) {
> -		ret = readl_poll_timeout_atomic(pmu->base + JH71XX_PMU_CURR_POWER_MODE,
> -						val, val & mask,
> -						1, JH71XX_PMU_TIMEOUT_US);
> +		ret = regmap_read_poll_timeout_atomic(pmu->base, JH71XX_PMU_CURR_POWER_MODE,
> +						      val, val & mask,
> +						      1, JH71XX_PMU_TIMEOUT_US);
>  	} else {
> -		ret = readl_poll_timeout_atomic(pmu->base + JH71XX_PMU_CURR_POWER_MODE,
> -						val, !(val & mask),
> -						1, JH71XX_PMU_TIMEOUT_US);
> +		ret = regmap_read_poll_timeout_atomic(pmu->base, JH71XX_PMU_CURR_POWER_MODE,
> +						      val, !(val & mask),
> +						      1, JH71XX_PMU_TIMEOUT_US);
>  	}
>  
>  	if (ret) {
> @@ -190,14 +193,14 @@ static void jh71xx_pmu_int_enable(struct jh71xx_pmu *pmu, u32 mask, bool enable)
>  	unsigned long flags;
>  
>  	spin_lock_irqsave(&pmu->lock, flags);
> -	val = readl(pmu->base + JH71XX_PMU_TIMER_INT_MASK);
> +	regmap_read(pmu->base, JH71XX_PMU_TIMER_INT_MASK, &val);
>  
>  	if (enable)
>  		val &= ~mask;
>  	else
>  		val |= mask;
>  
> -	writel(val, pmu->base + JH71XX_PMU_TIMER_INT_MASK);
> +	regmap_write(pmu->base, JH71XX_PMU_TIMER_INT_MASK, val);
>  	spin_unlock_irqrestore(&pmu->lock, flags);
>  }
>  
> @@ -206,7 +209,7 @@ static irqreturn_t jh71xx_pmu_interrupt(int irq, void *data)
>  	struct jh71xx_pmu *pmu = data;
>  	u32 val;
>  
> -	val = readl(pmu->base + JH71XX_PMU_INT_STATUS);
> +	regmap_read(pmu->base, JH71XX_PMU_INT_STATUS, &val);
>  
>  	if (val & JH71XX_PMU_INT_SEQ_DONE)
>  		dev_dbg(pmu->dev, "sequence done.\n");
> @@ -220,8 +223,8 @@ static irqreturn_t jh71xx_pmu_interrupt(int irq, void *data)
>  		dev_err(pmu->dev, "p-channel fail event.\n");
>  
>  	/* clear interrupts */
> -	writel(val, pmu->base + JH71XX_PMU_INT_STATUS);
> -	writel(val, pmu->base + JH71XX_PMU_EVENT_STATUS);
> +	regmap_write(pmu->base, JH71XX_PMU_INT_STATUS, val);
> +	regmap_write(pmu->base, JH71XX_PMU_EVENT_STATUS, val);
>  
>  	return IRQ_HANDLED;
>  }
> @@ -271,7 +274,7 @@ static int jh71xx_pmu_probe(struct platform_device *pdev)
>  	if (!pmu)
>  		return -ENOMEM;
>  
> -	pmu->base = devm_platform_ioremap_resource(pdev, 0);
> +	pmu->base = device_node_to_regmap(np);
>  	if (IS_ERR(pmu->base))
>  		return PTR_ERR(pmu->base);
>  
> -- 
> 2.25.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1 3/7] soc: starfive: Modify ioremap to regmap

[Changhuang Liang]

On 2023/4/12 4:26, Conor Dooley wrote:
> On Mon, Apr 10, 2023 at 11:47:39PM -0700, Changhuang Liang wrote:
>> Modify ioremap to regmap, easy to simplify code.
> 
> This doesn't simplify anything, adding regmap to the mix actually makes
> it less obvious what is going on here & it's not even fewer LoC:
> 1 file changed, 23 insertions(+), 20 deletions(-)
> 
> Please write a commit message that explains the real motivation for
> this change.
> 
> Thanks,
> Conor.
> 

When a new pmu is introduced later, they can use the same member 
"struct regmap *base" in "struct jh71xx_pmu". I will add more
commit message later.

>>
>> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
>> ---
>>  drivers/soc/starfive/jh71xx_pmu.c | 43 +++++++++++++++++--------------
>>  1 file changed, 23 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/soc/starfive/jh71xx_pmu.c b/drivers/soc/starfive/jh71xx_pmu.c
>> index 7d5f50d71c0d..306218c83691 100644
>> --- a/drivers/soc/starfive/jh71xx_pmu.c
>> +++ b/drivers/soc/starfive/jh71xx_pmu.c
>> @@ -6,13 +6,13 @@
>>   */
>>  
>>  #include <linux/interrupt.h>
>> -#include <linux/io.h>
>> -#include <linux/iopoll.h>
>> +#include <linux/mfd/syscon.h>
>>  #include <linux/module.h>
>>  #include <linux/of.h>
>>  #include <linux/of_device.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/pm_domain.h>
>> +#include <linux/regmap.h>
>>  #include <dt-bindings/power/starfive,jh7110-pmu.h>
>>  
>>  /* register offset */
>> @@ -59,7 +59,7 @@ struct jh71xx_pmu_match_data {
>>  struct jh71xx_pmu {
>>  	struct device *dev;
>>  	const struct jh71xx_pmu_match_data *match_data;
>> -	void __iomem *base;
>> +	struct regmap *base;
>>  	struct generic_pm_domain **genpd;
>>  	struct genpd_onecell_data genpd_data;
>>  	int irq;
>> @@ -75,11 +75,14 @@ struct jh71xx_pmu_dev {
>>  static int jh71xx_pmu_get_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool *is_on)
>>  {
>>  	struct jh71xx_pmu *pmu = pmd->pmu;
>> +	unsigned int val;
>>  
>>  	if (!mask)
>>  		return -EINVAL;
>>  
>> -	*is_on = readl(pmu->base + JH71XX_PMU_CURR_POWER_MODE) & mask;
>> +	regmap_read(pmu->base, JH71XX_PMU_CURR_POWER_MODE, &val);
>> +
>> +	*is_on = val & mask;
>>  
>>  	return 0;
>>  }
>> @@ -130,7 +133,7 @@ static int jh71xx_pmu_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
>>  		encourage_hi = JH71XX_PMU_SW_ENCOURAGE_DIS_HI;
>>  	}
>>  
>> -	writel(mask, pmu->base + mode);
>> +	regmap_write(pmu->base, mode, mask);
>>  
>>  	/*
>>  	 * 2.Write SW encourage command sequence to the Software Encourage Reg (offset 0x44)
>> @@ -140,21 +143,21 @@ static int jh71xx_pmu_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
>>  	 *   Then write the lower bits of the command sequence, followed by the upper
>>  	 *   bits. The sequence differs between powering on & off a domain.
>>  	 */
>> -	writel(JH71XX_PMU_SW_ENCOURAGE_ON, pmu->base + JH71XX_PMU_SW_ENCOURAGE);
>> -	writel(encourage_lo, pmu->base + JH71XX_PMU_SW_ENCOURAGE);
>> -	writel(encourage_hi, pmu->base + JH71XX_PMU_SW_ENCOURAGE);
>> +	regmap_write(pmu->base, JH71XX_PMU_SW_ENCOURAGE, JH71XX_PMU_SW_ENCOURAGE_ON);
>> +	regmap_write(pmu->base, JH71XX_PMU_SW_ENCOURAGE, encourage_lo);
>> +	regmap_write(pmu->base, JH71XX_PMU_SW_ENCOURAGE, encourage_hi);
>>  
>>  	spin_unlock_irqrestore(&pmu->lock, flags);
>>  
>>  	/* Wait for the power domain bit to be enabled / disabled */
>>  	if (on) {
>> -		ret = readl_poll_timeout_atomic(pmu->base + JH71XX_PMU_CURR_POWER_MODE,
>> -						val, val & mask,
>> -						1, JH71XX_PMU_TIMEOUT_US);
>> +		ret = regmap_read_poll_timeout_atomic(pmu->base, JH71XX_PMU_CURR_POWER_MODE,
>> +						      val, val & mask,
>> +						      1, JH71XX_PMU_TIMEOUT_US);
>>  	} else {
>> -		ret = readl_poll_timeout_atomic(pmu->base + JH71XX_PMU_CURR_POWER_MODE,
>> -						val, !(val & mask),
>> -						1, JH71XX_PMU_TIMEOUT_US);
>> +		ret = regmap_read_poll_timeout_atomic(pmu->base, JH71XX_PMU_CURR_POWER_MODE,
>> +						      val, !(val & mask),
>> +						      1, JH71XX_PMU_TIMEOUT_US);
>>  	}
>>  
>>  	if (ret) {
>> @@ -190,14 +193,14 @@ static void jh71xx_pmu_int_enable(struct jh71xx_pmu *pmu, u32 mask, bool enable)
>>  	unsigned long flags;
>>  
>>  	spin_lock_irqsave(&pmu->lock, flags);
>> -	val = readl(pmu->base + JH71XX_PMU_TIMER_INT_MASK);
>> +	regmap_read(pmu->base, JH71XX_PMU_TIMER_INT_MASK, &val);
>>  
>>  	if (enable)
>>  		val &= ~mask;
>>  	else
>>  		val |= mask;
>>  
>> -	writel(val, pmu->base + JH71XX_PMU_TIMER_INT_MASK);
>> +	regmap_write(pmu->base, JH71XX_PMU_TIMER_INT_MASK, val);
>>  	spin_unlock_irqrestore(&pmu->lock, flags);
>>  }
>>  
>> @@ -206,7 +209,7 @@ static irqreturn_t jh71xx_pmu_interrupt(int irq, void *data)
>>  	struct jh71xx_pmu *pmu = data;
>>  	u32 val;
>>  
>> -	val = readl(pmu->base + JH71XX_PMU_INT_STATUS);
>> +	regmap_read(pmu->base, JH71XX_PMU_INT_STATUS, &val);
>>  
>>  	if (val & JH71XX_PMU_INT_SEQ_DONE)
>>  		dev_dbg(pmu->dev, "sequence done.\n");
>> @@ -220,8 +223,8 @@ static irqreturn_t jh71xx_pmu_interrupt(int irq, void *data)
>>  		dev_err(pmu->dev, "p-channel fail event.\n");
>>  
>>  	/* clear interrupts */
>> -	writel(val, pmu->base + JH71XX_PMU_INT_STATUS);
>> -	writel(val, pmu->base + JH71XX_PMU_EVENT_STATUS);
>> +	regmap_write(pmu->base, JH71XX_PMU_INT_STATUS, val);
>> +	regmap_write(pmu->base, JH71XX_PMU_EVENT_STATUS, val);
>>  
>>  	return IRQ_HANDLED;
>>  }
>> @@ -271,7 +274,7 @@ static int jh71xx_pmu_probe(struct platform_device *pdev)
>>  	if (!pmu)
>>  		return -ENOMEM;
>>  
>> -	pmu->base = devm_platform_ioremap_resource(pdev, 0);
>> +	pmu->base = device_node_to_regmap(np);
>>  	if (IS_ERR(pmu->base))
>>  		return PTR_ERR(pmu->base);
>>  
>> -- 
>> 2.25.1
>>

[PATCH v1 4/7] soc: starfive: Add pmu type operation

[Changhuang Liang]

Add pmu type, make a distinction between different PMU.

Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
 drivers/soc/starfive/jh71xx_pmu.c | 55 ++++++++++++++++++++++---------
 1 file changed, 39 insertions(+), 16 deletions(-)

diff --git a/drivers/soc/starfive/jh71xx_pmu.c b/drivers/soc/starfive/jh71xx_pmu.c
index 306218c83691..98f6849d61de 100644
--- a/drivers/soc/starfive/jh71xx_pmu.c
+++ b/drivers/soc/starfive/jh71xx_pmu.c
@@ -45,6 +45,12 @@
  */
 #define JH71XX_PMU_TIMEOUT_US		100
 
+/* pmu type */
+enum pmu_type {
+	JH71XX_PMU_GENERAL,
+	JH71XX_PMU_DPHY,
+};
+
 struct jh71xx_domain_info {
 	const char * const name;
 	unsigned int flags;
@@ -54,6 +60,7 @@ struct jh71xx_domain_info {
 struct jh71xx_pmu_match_data {
 	const struct jh71xx_domain_info *domain_info;
 	int num_domains;
+	u8 pmu_type;
 };
 
 struct jh71xx_pmu {
@@ -75,19 +82,23 @@ struct jh71xx_pmu_dev {
 static int jh71xx_pmu_get_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool *is_on)
 {
 	struct jh71xx_pmu *pmu = pmd->pmu;
+	unsigned int offset = 0;
 	unsigned int val;
 
 	if (!mask)
 		return -EINVAL;
 
-	regmap_read(pmu->base, JH71XX_PMU_CURR_POWER_MODE, &val);
+	if (pmu->match_data->pmu_type == JH71XX_PMU_GENERAL)
+		offset = JH71XX_PMU_CURR_POWER_MODE;
+
+	regmap_read(pmu->base, offset, &val);
 
 	*is_on = val & mask;
 
 	return 0;
 }
 
-static int jh71xx_pmu_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
+static int jh71xx_pmu_general_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
 {
 	struct jh71xx_pmu *pmu = pmd->pmu;
 	unsigned long flags;
@@ -95,22 +106,8 @@ static int jh71xx_pmu_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
 	u32 mode;
 	u32 encourage_lo;
 	u32 encourage_hi;
-	bool is_on;
 	int ret;
 
-	ret = jh71xx_pmu_get_state(pmd, mask, &is_on);
-	if (ret) {
-		dev_dbg(pmu->dev, "unable to get current state for %s\n",
-			pmd->genpd.name);
-		return ret;
-	}
-
-	if (is_on == on) {
-		dev_dbg(pmu->dev, "pm domain [%s] is already %sable status.\n",
-			pmd->genpd.name, on ? "en" : "dis");
-		return 0;
-	}
-
 	spin_lock_irqsave(&pmu->lock, flags);
 
 	/*
@@ -169,6 +166,31 @@ static int jh71xx_pmu_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
 	return 0;
 }
 
+static int jh71xx_pmu_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
+{
+	struct jh71xx_pmu *pmu = pmd->pmu;
+	bool is_on;
+	int ret = 0;
+
+	ret = jh71xx_pmu_get_state(pmd, mask, &is_on);
+	if (ret) {
+		dev_dbg(pmu->dev, "unable to get current state for %s\n",
+			pmd->genpd.name);
+		return ret;
+	}
+
+	if (is_on == on) {
+		dev_dbg(pmu->dev, "pm domain [%s] is already %sable status.\n",
+			pmd->genpd.name, on ? "en" : "dis");
+		return 0;
+	}
+
+	if (pmu->match_data->pmu_type == JH71XX_PMU_GENERAL)
+		ret = jh71xx_pmu_general_set_state(pmd, mask, on);
+
+	return ret;
+}
+
 static int jh71xx_pmu_on(struct generic_pm_domain *genpd)
 {
 	struct jh71xx_pmu_dev *pmd = container_of(genpd,
@@ -360,6 +382,7 @@ static const struct jh71xx_domain_info jh7110_power_domains[] = {
 static const struct jh71xx_pmu_match_data jh7110_pmu = {
 	.num_domains = ARRAY_SIZE(jh7110_power_domains),
 	.domain_info = jh7110_power_domains,
+	.pmu_type = JH71XX_PMU_GENERAL,
 };
 
 static const struct of_device_id jh71xx_pmu_of_match[] = {
-- 
2.25.1

Re: [PATCH v1 4/7] soc: starfive: Add pmu type operation

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1253 bytes --]

Hey Changhuang Liang,

On Mon, Apr 10, 2023 at 11:47:40PM -0700, Changhuang Liang wrote:
> Add pmu type, make a distinction between different PMU.

Please write more detailed commit messages, thanks.

> 
> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> ---
>  drivers/soc/starfive/jh71xx_pmu.c | 55 ++++++++++++++++++++++---------
>  1 file changed, 39 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/soc/starfive/jh71xx_pmu.c b/drivers/soc/starfive/jh71xx_pmu.c
> index 306218c83691..98f6849d61de 100644
> --- a/drivers/soc/starfive/jh71xx_pmu.c
> +++ b/drivers/soc/starfive/jh71xx_pmu.c
> @@ -45,6 +45,12 @@
>   */
>  #define JH71XX_PMU_TIMEOUT_US		100
>  
> +/* pmu type */

Delete this comment, it's obvious.

> +enum pmu_type {
> +	JH71XX_PMU_GENERAL,

I'm really not sold on GENERAL as a name.
Why not name these after the compatibles?

> +	JH71XX_PMU_DPHY,
> +};
> +
>  struct jh71xx_domain_info {
>  	const char * const name;
>  	unsigned int flags;
> @@ -54,6 +60,7 @@ struct jh71xx_domain_info {
>  struct jh71xx_pmu_match_data {
>  	const struct jh71xx_domain_info *domain_info;
>  	int num_domains;
> +	u8 pmu_type;

This is an enum, not a u8?

Thanks,
Conor.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1 4/7] soc: starfive: Add pmu type operation

[Changhuang Liang]

On 2023/4/12 4:52, Conor Dooley wrote:
> Hey Changhuang Liang,
> 
> On Mon, Apr 10, 2023 at 11:47:40PM -0700, Changhuang Liang wrote:
>> Add pmu type, make a distinction between different PMU.
> 
> Please write more detailed commit messages, thanks.
> 

OK, will write more detail for it.

>>
>> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
>> ---
>>  drivers/soc/starfive/jh71xx_pmu.c | 55 ++++++++++++++++++++++---------
>>  1 file changed, 39 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/soc/starfive/jh71xx_pmu.c b/drivers/soc/starfive/jh71xx_pmu.c
>> index 306218c83691..98f6849d61de 100644
>> --- a/drivers/soc/starfive/jh71xx_pmu.c
>> +++ b/drivers/soc/starfive/jh71xx_pmu.c
>> @@ -45,6 +45,12 @@
>>   */
>>  #define JH71XX_PMU_TIMEOUT_US		100
>>  
>> +/* pmu type */
> 
> Delete this comment, it's obvious.
> 

OK, will delete this line.

>> +enum pmu_type {
>> +	JH71XX_PMU_GENERAL,
> 
> I'm really not sold on GENERAL as a name.
> Why not name these after the compatibles?
>

OK, will change to "JH71XX_PMU".
 
>> +	JH71XX_PMU_DPHY,
>> +};
>> +
>>  struct jh71xx_domain_info {
>>  	const char * const name;
>>  	unsigned int flags;
>> @@ -54,6 +60,7 @@ struct jh71xx_domain_info {
>>  struct jh71xx_pmu_match_data {
>>  	const struct jh71xx_domain_info *domain_info;
>>  	int num_domains;
>> +	u8 pmu_type;
> 
> This is an enum, not a u8?
> 

OK, will fix it.

> Thanks,
> Conor.
> 

[PATCH v1 5/7] soc: starfive: Use call back to parse device tree resources

[Changhuang Liang]

Different compatible parse device tree resources work in different ways.

Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
 drivers/soc/starfive/jh71xx_pmu.c | 54 ++++++++++++++++++++++---------
 1 file changed, 39 insertions(+), 15 deletions(-)

diff --git a/drivers/soc/starfive/jh71xx_pmu.c b/drivers/soc/starfive/jh71xx_pmu.c
index 98f6849d61de..990db6735c48 100644
--- a/drivers/soc/starfive/jh71xx_pmu.c
+++ b/drivers/soc/starfive/jh71xx_pmu.c
@@ -57,10 +57,14 @@ struct jh71xx_domain_info {
 	u8 bit;
 };
 
+struct jh71xx_pmu;
+
 struct jh71xx_pmu_match_data {
 	const struct jh71xx_domain_info *domain_info;
 	int num_domains;
 	u8 pmu_type;
+	int (*pmu_parse_dt)(struct platform_device *pdev,
+			    struct jh71xx_pmu *pmu);
 };
 
 struct jh71xx_pmu {
@@ -251,6 +255,31 @@ static irqreturn_t jh71xx_pmu_interrupt(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+static int jh7110_pmu_general_parse_dt(struct platform_device *pdev,
+				       struct jh71xx_pmu *pmu)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	int ret;
+
+	pmu->base = device_node_to_regmap(np);
+	if (IS_ERR(pmu->base))
+		return PTR_ERR(pmu->base);
+
+	pmu->irq = platform_get_irq(pdev, 0);
+	if (pmu->irq < 0)
+		return pmu->irq;
+
+	ret = devm_request_irq(dev, pmu->irq, jh71xx_pmu_interrupt,
+			       0, pdev->name, pmu);
+	if (ret)
+		dev_err(dev, "failed to request irq\n");
+
+	jh71xx_pmu_int_enable(pmu, JH71XX_PMU_INT_ALL_MASK & ~JH71XX_PMU_INT_PCH_FAIL, true);
+
+	return 0;
+}
+
 static int jh71xx_pmu_init_domain(struct jh71xx_pmu *pmu, int index)
 {
 	struct jh71xx_pmu_dev *pmd;
@@ -296,23 +325,20 @@ static int jh71xx_pmu_probe(struct platform_device *pdev)
 	if (!pmu)
 		return -ENOMEM;
 
-	pmu->base = device_node_to_regmap(np);
-	if (IS_ERR(pmu->base))
-		return PTR_ERR(pmu->base);
-
-	pmu->irq = platform_get_irq(pdev, 0);
-	if (pmu->irq < 0)
-		return pmu->irq;
-
-	ret = devm_request_irq(dev, pmu->irq, jh71xx_pmu_interrupt,
-			       0, pdev->name, pmu);
-	if (ret)
-		dev_err(dev, "failed to request irq\n");
+	spin_lock_init(&pmu->lock);
 
 	match_data = of_device_get_match_data(dev);
 	if (!match_data)
 		return -EINVAL;
 
+	if (match_data->pmu_parse_dt) {
+		ret = match_data->pmu_parse_dt(pdev, pmu);
+		if (ret) {
+			dev_err(dev, "failed to parse dt\n");
+			return ret;
+		}
+	}
+
 	pmu->genpd = devm_kcalloc(dev, match_data->num_domains,
 				  sizeof(struct generic_pm_domain *),
 				  GFP_KERNEL);
@@ -332,9 +358,6 @@ static int jh71xx_pmu_probe(struct platform_device *pdev)
 		}
 	}
 
-	spin_lock_init(&pmu->lock);
-	jh71xx_pmu_int_enable(pmu, JH71XX_PMU_INT_ALL_MASK & ~JH71XX_PMU_INT_PCH_FAIL, true);
-
 	ret = of_genpd_add_provider_onecell(np, &pmu->genpd_data);
 	if (ret) {
 		dev_err(dev, "failed to register genpd driver: %d\n", ret);
@@ -383,6 +406,7 @@ static const struct jh71xx_pmu_match_data jh7110_pmu = {
 	.num_domains = ARRAY_SIZE(jh7110_power_domains),
 	.domain_info = jh7110_power_domains,
 	.pmu_type = JH71XX_PMU_GENERAL,
+	.pmu_parse_dt = jh7110_pmu_general_parse_dt,
 };
 
 static const struct of_device_id jh71xx_pmu_of_match[] = {
-- 
2.25.1

Re: [PATCH v1 5/7] soc: starfive: Use call back to parse device tree resources

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1177 bytes --]

On Mon, Apr 10, 2023 at 11:47:41PM -0700, Changhuang Liang wrote:
> Different compatible parse device tree resources work in different ways.

Right now there is only one compatible, so this commit message needs to
be expanded on to provide more information on your motivation.

> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>

>  static int jh71xx_pmu_init_domain(struct jh71xx_pmu *pmu, int index)
>  {
>  	struct jh71xx_pmu_dev *pmd;
> @@ -296,23 +325,20 @@ static int jh71xx_pmu_probe(struct platform_device *pdev)
>  	if (!pmu)
>  		return -ENOMEM;
>  
> -	pmu->base = device_node_to_regmap(np);
> -	if (IS_ERR(pmu->base))
> -		return PTR_ERR(pmu->base);
> -
> -	pmu->irq = platform_get_irq(pdev, 0);
> -	if (pmu->irq < 0)
> -		return pmu->irq;
> -
> -	ret = devm_request_irq(dev, pmu->irq, jh71xx_pmu_interrupt,
> -			       0, pdev->name, pmu);
> -	if (ret)
> -		dev_err(dev, "failed to request irq\n");
> +	spin_lock_init(&pmu->lock);
>  
>  	match_data = of_device_get_match_data(dev);
>  	if (!match_data)
>  		return -EINVAL;
>  
> +	if (match_data->pmu_parse_dt) {

How can this be false?

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1 5/7] soc: starfive: Use call back to parse device tree resources

[Changhuang Liang]

On 2023/4/12 5:06, Conor Dooley wrote:
> On Mon, Apr 10, 2023 at 11:47:41PM -0700, Changhuang Liang wrote:
>> Different compatible parse device tree resources work in different ways.
> 
> Right now there is only one compatible, so this commit message needs to
> be expanded on to provide more information on your motivation.
> 

OK, will add more commit message.

>> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> 
>>  static int jh71xx_pmu_init_domain(struct jh71xx_pmu *pmu, int index)
>>  {
[...]
>>  
>>  	match_data = of_device_get_match_data(dev);
>>  	if (!match_data)
>>  		return -EINVAL;
>>  
>> +	if (match_data->pmu_parse_dt) {
> 
> How can this be false?
> 
> Cheers,
> Conor.

Yes, it will not be false, I will delete this if condition

Re: [PATCH v1 5/7] soc: starfive: Use call back to parse device tree resources

[Walker Chen]

On 2023/4/11 14:47, Changhuang Liang wrote:
> Different compatible parse device tree resources work in different ways.
> 
> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>

I don't think it's necessary to submit multiple patches separately for the same .c file
unless it is very necessary. Because the disadvantage of separating multiple patches 
is that some information lacks completeness and coherence.

> ---
>  drivers/soc/starfive/jh71xx_pmu.c | 54 ++++++++++++++++++++++---------
>  1 file changed, 39 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/soc/starfive/jh71xx_pmu.c b/drivers/soc/starfive/jh71xx_pmu.c
> index 98f6849d61de..990db6735c48 100644
> --- a/drivers/soc/starfive/jh71xx_pmu.c
> +++ b/drivers/soc/starfive/jh71xx_pmu.c
> @@ -57,10 +57,14 @@ struct jh71xx_domain_info {
>  	u8 bit;
>  };
>  
> +struct jh71xx_pmu;
> +
>  struct jh71xx_pmu_match_data {
>  	const struct jh71xx_domain_info *domain_info;
>  	int num_domains;
>  	u8 pmu_type;
> +	int (*pmu_parse_dt)(struct platform_device *pdev,
> +			    struct jh71xx_pmu *pmu);
>  };
>  
>  struct jh71xx_pmu {
> @@ -251,6 +255,31 @@ static irqreturn_t jh71xx_pmu_interrupt(int irq, void *data)
>  	return IRQ_HANDLED;
>  }
>  
> +static int jh7110_pmu_general_parse_dt(struct platform_device *pdev,
> +				       struct jh71xx_pmu *pmu)
> +{

Re: [PATCH v1 5/7] soc: starfive: Use call back to parse device tree resources

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 888 bytes --]

On Wed, Apr 12, 2023 at 02:07:52PM +0800, Walker Chen wrote:
> 
> 
> On 2023/4/11 14:47, Changhuang Liang wrote:
> > Different compatible parse device tree resources work in different ways.
> > 
> > Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> 
> I don't think it's necessary to submit multiple patches separately for the same .c file
> unless it is very necessary. Because the disadvantage of separating multiple patches 
> is that some information lacks completeness and coherence.

Other than patches 4 & 6, which could be squashed, the breakdown here is
fine IMO. Doing one thing per patch makes it obvious to the reader
*what* is happening.

There's just some missing boilerplate in the commit messages across the
series that the DPHY PMU does not have a reg nor interrupts, and this
work is being done to support that.

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v1 6/7] soc: starfive: Add dphy pmu support

[Changhuang Liang]

Add dphy pmu to turn on/off the dphy power switch.

Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
 MAINTAINERS                       |  1 +
 drivers/soc/starfive/jh71xx_pmu.c | 65 +++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 0b2170e1e4ff..4d958f02403e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19944,6 +19944,7 @@ F:	include/dt-bindings/reset/starfive?jh71*.h
 
 STARFIVE JH71XX PMU CONTROLLER DRIVER
 M:	Walker Chen <walker.chen@starfivetech.com>
+M:	Changhuang Liang <changhuang.liang@starfivetech.com>
 S:	Supported
 F:	Documentation/devicetree/bindings/power/starfive*
 F:	drivers/soc/starfive/jh71xx_pmu.c
diff --git a/drivers/soc/starfive/jh71xx_pmu.c b/drivers/soc/starfive/jh71xx_pmu.c
index 990db6735c48..d4092ca4dccf 100644
--- a/drivers/soc/starfive/jh71xx_pmu.c
+++ b/drivers/soc/starfive/jh71xx_pmu.c
@@ -24,6 +24,9 @@
 #define JH71XX_PMU_EVENT_STATUS		0x88
 #define JH71XX_PMU_INT_STATUS		0x8C
 
+/* DPHY pmu register offset */
+#define JH71XX_PMU_DPHY_SWITCH		0x00
+
 /* sw encourage cfg */
 #define JH71XX_PMU_SW_ENCOURAGE_EN_LO	0x05
 #define JH71XX_PMU_SW_ENCOURAGE_EN_HI	0x50
@@ -94,6 +97,8 @@ static int jh71xx_pmu_get_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool *is_o
 
 	if (pmu->match_data->pmu_type == JH71XX_PMU_GENERAL)
 		offset = JH71XX_PMU_CURR_POWER_MODE;
+	else if (pmu->match_data->pmu_type == JH71XX_PMU_DPHY)
+		offset = JH71XX_PMU_DPHY_SWITCH;
 
 	regmap_read(pmu->base, offset, &val);
 
@@ -170,6 +175,23 @@ static int jh71xx_pmu_general_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bo
 	return 0;
 }
 
+static int jh71xx_pmu_dphy_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
+{
+	struct jh71xx_pmu *pmu = pmd->pmu;
+	unsigned long flags;
+
+	spin_lock_irqsave(&pmu->lock, flags);
+
+	if (on)
+		regmap_update_bits(pmu->base, JH71XX_PMU_DPHY_SWITCH, mask, mask);
+	else
+		regmap_update_bits(pmu->base, JH71XX_PMU_DPHY_SWITCH, mask, 0);
+
+	spin_unlock_irqrestore(&pmu->lock, flags);
+
+	return 0;
+}
+
 static int jh71xx_pmu_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
 {
 	struct jh71xx_pmu *pmu = pmd->pmu;
@@ -191,6 +213,8 @@ static int jh71xx_pmu_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
 
 	if (pmu->match_data->pmu_type == JH71XX_PMU_GENERAL)
 		ret = jh71xx_pmu_general_set_state(pmd, mask, on);
+	else if (pmu->match_data->pmu_type == JH71XX_PMU_DPHY)
+		ret = jh71xx_pmu_dphy_set_state(pmd, mask, on);
 
 	return ret;
 }
@@ -280,6 +304,25 @@ static int jh7110_pmu_general_parse_dt(struct platform_device *pdev,
 	return 0;
 }
 
+static int jh7110_pmu_dphy_parse_dt(struct platform_device *pdev,
+				    struct jh71xx_pmu *pmu)
+{
+	struct device *parent;
+	struct device *dev = &pdev->dev;
+
+	parent = pdev->dev.parent;
+	if (!parent) {
+		dev_err(dev, "No parent for syscon pmu\n");
+		return -ENODEV;
+	}
+
+	pmu->base = syscon_node_to_regmap(parent->of_node);
+	if (IS_ERR(pmu->base))
+		return PTR_ERR(pmu->base);
+
+	return 0;
+}
+
 static int jh71xx_pmu_init_domain(struct jh71xx_pmu *pmu, int index)
 {
 	struct jh71xx_pmu_dev *pmd;
@@ -409,10 +452,31 @@ static const struct jh71xx_pmu_match_data jh7110_pmu = {
 	.pmu_parse_dt = jh7110_pmu_general_parse_dt,
 };
 
+static const struct jh71xx_domain_info jh7110_dphy_power_domains[] = {
+	[JH7110_PD_DPHY_TX] = {
+		.name = "DPHY-TX",
+		.bit = 30,
+	},
+	[JH7110_PD_DPHY_RX] = {
+		.name = "DPHY-RX",
+		.bit = 31,
+	},
+};
+
+static const struct jh71xx_pmu_match_data jh7110_pmu_dphy = {
+	.num_domains = ARRAY_SIZE(jh7110_dphy_power_domains),
+	.domain_info = jh7110_dphy_power_domains,
+	.pmu_type = JH71XX_PMU_DPHY,
+	.pmu_parse_dt = jh7110_pmu_dphy_parse_dt,
+};
+
 static const struct of_device_id jh71xx_pmu_of_match[] = {
 	{
 		.compatible = "starfive,jh7110-pmu",
 		.data = (void *)&jh7110_pmu,
+	}, {
+		.compatible = "starfive,jh7110-pmu-dphy",
+		.data = (void *)&jh7110_pmu_dphy,
 	}, {
 		/* sentinel */
 	}
@@ -429,5 +493,6 @@ static struct platform_driver jh71xx_pmu_driver = {
 builtin_platform_driver(jh71xx_pmu_driver);
 
 MODULE_AUTHOR("Walker Chen <walker.chen@starfivetech.com>");
+MODULE_AUTHOR("Changhuang Liang <changhuang.liang@starfivetech.com>");
 MODULE_DESCRIPTION("StarFive JH71XX PMU Driver");
 MODULE_LICENSE("GPL");
-- 
2.25.1

Re: [PATCH v1 6/7] soc: starfive: Add dphy pmu support

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 5390 bytes --]

On Mon, Apr 10, 2023 at 11:47:42PM -0700, Changhuang Liang wrote:
> Add dphy pmu to turn on/off the dphy power switch.
> 
> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> ---
>  MAINTAINERS                       |  1 +
>  drivers/soc/starfive/jh71xx_pmu.c | 65 +++++++++++++++++++++++++++++++
>  2 files changed, 66 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0b2170e1e4ff..4d958f02403e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19944,6 +19944,7 @@ F:	include/dt-bindings/reset/starfive?jh71*.h
>  
>  STARFIVE JH71XX PMU CONTROLLER DRIVER
>  M:	Walker Chen <walker.chen@starfivetech.com>
> +M:	Changhuang Liang <changhuang.liang@starfivetech.com>

Unmentioned in the commit message, plus I would like an R-b or an Ack
from Walker.

>  S:	Supported
>  F:	Documentation/devicetree/bindings/power/starfive*
>  F:	drivers/soc/starfive/jh71xx_pmu.c
> diff --git a/drivers/soc/starfive/jh71xx_pmu.c b/drivers/soc/starfive/jh71xx_pmu.c
> index 990db6735c48..d4092ca4dccf 100644
> --- a/drivers/soc/starfive/jh71xx_pmu.c
> +++ b/drivers/soc/starfive/jh71xx_pmu.c
> @@ -24,6 +24,9 @@
>  #define JH71XX_PMU_EVENT_STATUS		0x88
>  #define JH71XX_PMU_INT_STATUS		0x8C
>  
> +/* DPHY pmu register offset */
> +#define JH71XX_PMU_DPHY_SWITCH		0x00
> +
>  /* sw encourage cfg */
>  #define JH71XX_PMU_SW_ENCOURAGE_EN_LO	0x05
>  #define JH71XX_PMU_SW_ENCOURAGE_EN_HI	0x50
> @@ -94,6 +97,8 @@ static int jh71xx_pmu_get_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool *is_o
>  
>  	if (pmu->match_data->pmu_type == JH71XX_PMU_GENERAL)
>  		offset = JH71XX_PMU_CURR_POWER_MODE;
> +	else if (pmu->match_data->pmu_type == JH71XX_PMU_DPHY)

There are only two options for this "enum", so why `else if`?

> +		offset = JH71XX_PMU_DPHY_SWITCH;
>  
>  	regmap_read(pmu->base, offset, &val);
>  
> @@ -170,6 +175,23 @@ static int jh71xx_pmu_general_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bo
>  	return 0;
>  }
>  
> +static int jh71xx_pmu_dphy_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
> +{
> +	struct jh71xx_pmu *pmu = pmd->pmu;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&pmu->lock, flags);
> +
> +	if (on)
> +		regmap_update_bits(pmu->base, JH71XX_PMU_DPHY_SWITCH, mask, mask);
> +	else
> +		regmap_update_bits(pmu->base, JH71XX_PMU_DPHY_SWITCH, mask, 0);
> +
> +	spin_unlock_irqrestore(&pmu->lock, flags);
> +
> +	return 0;
> +}
> +
>  static int jh71xx_pmu_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
>  {
>  	struct jh71xx_pmu *pmu = pmd->pmu;
> @@ -191,6 +213,8 @@ static int jh71xx_pmu_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
>  
>  	if (pmu->match_data->pmu_type == JH71XX_PMU_GENERAL)
>  		ret = jh71xx_pmu_general_set_state(pmd, mask, on);
> +	else if (pmu->match_data->pmu_type == JH71XX_PMU_DPHY)
> +		ret = jh71xx_pmu_dphy_set_state(pmd, mask, on);

Perhaps I am verging on over-complication, but I dislike this carry on.
Is this the only time we'll see a power domain provider coming out of
a syscon, or are there likely to be more?
Either way, I think having an ops struct w/ both parse_dt() and the
set_state() implementations would be neater than what you have here.

Very much open to dissenting opinions there though. Emil? Walker?

Cheers,
Conor.

>  
>  	return ret;
>  }
> @@ -280,6 +304,25 @@ static int jh7110_pmu_general_parse_dt(struct platform_device *pdev,
>  	return 0;
>  }
>  
> +static int jh7110_pmu_dphy_parse_dt(struct platform_device *pdev,
> +				    struct jh71xx_pmu *pmu)
> +{
> +	struct device *parent;
> +	struct device *dev = &pdev->dev;
> +
> +	parent = pdev->dev.parent;
> +	if (!parent) {
> +		dev_err(dev, "No parent for syscon pmu\n");
> +		return -ENODEV;
> +	}
> +
> +	pmu->base = syscon_node_to_regmap(parent->of_node);
> +	if (IS_ERR(pmu->base))
> +		return PTR_ERR(pmu->base);
> +
> +	return 0;
> +}
> +
>  static int jh71xx_pmu_init_domain(struct jh71xx_pmu *pmu, int index)
>  {
>  	struct jh71xx_pmu_dev *pmd;
> @@ -409,10 +452,31 @@ static const struct jh71xx_pmu_match_data jh7110_pmu = {
>  	.pmu_parse_dt = jh7110_pmu_general_parse_dt,
>  };
>  
> +static const struct jh71xx_domain_info jh7110_dphy_power_domains[] = {
> +	[JH7110_PD_DPHY_TX] = {
> +		.name = "DPHY-TX",
> +		.bit = 30,
> +	},
> +	[JH7110_PD_DPHY_RX] = {
> +		.name = "DPHY-RX",
> +		.bit = 31,
> +	},
> +};
> +
> +static const struct jh71xx_pmu_match_data jh7110_pmu_dphy = {
> +	.num_domains = ARRAY_SIZE(jh7110_dphy_power_domains),
> +	.domain_info = jh7110_dphy_power_domains,
> +	.pmu_type = JH71XX_PMU_DPHY,
> +	.pmu_parse_dt = jh7110_pmu_dphy_parse_dt,
> +};
> +
>  static const struct of_device_id jh71xx_pmu_of_match[] = {
>  	{
>  		.compatible = "starfive,jh7110-pmu",
>  		.data = (void *)&jh7110_pmu,
> +	}, {
> +		.compatible = "starfive,jh7110-pmu-dphy",
> +		.data = (void *)&jh7110_pmu_dphy,
>  	}, {
>  		/* sentinel */
>  	}
> @@ -429,5 +493,6 @@ static struct platform_driver jh71xx_pmu_driver = {
>  builtin_platform_driver(jh71xx_pmu_driver);
>  
>  MODULE_AUTHOR("Walker Chen <walker.chen@starfivetech.com>");
> +MODULE_AUTHOR("Changhuang Liang <changhuang.liang@starfivetech.com>");
>  MODULE_DESCRIPTION("StarFive JH71XX PMU Driver");
>  MODULE_LICENSE("GPL");
> -- 
> 2.25.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1 6/7] soc: starfive: Add dphy pmu support

[Changhuang Liang]

On 2023/4/12 5:15, Conor Dooley wrote:
> On Mon, Apr 10, 2023 at 11:47:42PM -0700, Changhuang Liang wrote:
[...]
>> +++ b/MAINTAINERS
>> @@ -19944,6 +19944,7 @@ F:	include/dt-bindings/reset/starfive?jh71*.h
>>  
>>  STARFIVE JH71XX PMU CONTROLLER DRIVER
>>  M:	Walker Chen <walker.chen@starfivetech.com>
>> +M:	Changhuang Liang <changhuang.liang@starfivetech.com>
> 
> Unmentioned in the commit message, plus I would like an R-b or an Ack
> from Walker.
> 

OK, I will make a discuss with Walker.

>>  S:	Supported
>>  F:	Documentation/devicetree/bindings/power/starfive*
>>  F:	drivers/soc/starfive/jh71xx_pmu.c
>> diff --git a/drivers/soc/starfive/jh71xx_pmu.c b/drivers/soc/starfive/jh71xx_pmu.c
>> index 990db6735c48..d4092ca4dccf 100644
[...]
>> @@ -94,6 +97,8 @@ static int jh71xx_pmu_get_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool *is_o
>>  
>>  	if (pmu->match_data->pmu_type == JH71XX_PMU_GENERAL)
>>  		offset = JH71XX_PMU_CURR_POWER_MODE;
>> +	else if (pmu->match_data->pmu_type == JH71XX_PMU_DPHY)
> 
> There are only two options for this "enum", so why `else if`?
> 

OK, will change to else.

>> +		offset = JH71XX_PMU_DPHY_SWITCH;
>>  
>>  	regmap_read(pmu->base, offset, &val);
>>  
>> @@ -170,6 +175,23 @@ static int jh71xx_pmu_general_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bo
>>  	return 0;
>>  }
>>  
[...]
>>  static int jh71xx_pmu_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
>>  {
>>  	struct jh71xx_pmu *pmu = pmd->pmu;
>> @@ -191,6 +213,8 @@ static int jh71xx_pmu_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
>>  
>>  	if (pmu->match_data->pmu_type == JH71XX_PMU_GENERAL)
>>  		ret = jh71xx_pmu_general_set_state(pmd, mask, on);
>> +	else if (pmu->match_data->pmu_type == JH71XX_PMU_DPHY)
>> +		ret = jh71xx_pmu_dphy_set_state(pmd, mask, on);
> 
> Perhaps I am verging on over-complication, but I dislike this carry on.
> Is this the only time we'll see a power domain provider coming out of
> a syscon, or are there likely to be more?
> Either way, I think having an ops struct w/ both parse_dt() and the
> set_state() implementations would be neater than what you have here.
> 
> Very much open to dissenting opinions there though. Emil? Walker?
> 
> Cheers,
> Conor.
> 

"else if" will change to "else"
As far as I know, there are only two types power domain on the JH7110 SoC.
One is the original, another one is coming out of a syscon.

>>  
>>  	return ret;
>>  }
[...]
>> 2.25.1
>>

Re: [PATCH v1 6/7] soc: starfive: Add dphy pmu support

[Changhuang Liang]

On 2023/4/12 5:15, Conor Dooley wrote:
> On Mon, Apr 10, 2023 at 11:47:42PM -0700, Changhuang Liang wrote:
>> Add dphy pmu to turn on/off the dphy power switch.
[...]
>> +
>>  static int jh71xx_pmu_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
>>  {
>>  	struct jh71xx_pmu *pmu = pmd->pmu;
>> @@ -191,6 +213,8 @@ static int jh71xx_pmu_set_state(struct jh71xx_pmu_dev *pmd, u32 mask, bool on)
>>  
>>  	if (pmu->match_data->pmu_type == JH71XX_PMU_GENERAL)
>>  		ret = jh71xx_pmu_general_set_state(pmd, mask, on);
>> +	else if (pmu->match_data->pmu_type == JH71XX_PMU_DPHY)
>> +		ret = jh71xx_pmu_dphy_set_state(pmd, mask, on);
> 
> Perhaps I am verging on over-complication, but I dislike this carry on.
> Is this the only time we'll see a power domain provider coming out of
> a syscon, or are there likely to be more?
> Either way, I think having an ops struct w/ both parse_dt() and the
> set_state() implementations would be neater than what you have here.
> 

OK, I will use call back make here neater.

> Very much open to dissenting opinions there though. Emil? Walker?
> 
> Cheers,
> Conor.
> 
>>  
>>  	return ret;
>>  }
[...]
>> -- 
>> 2.25.1
>>

[PATCH v1 7/7] riscv: dts: starfive: Add dphy rx pmu node

[Changhuang Liang]

Add dphy rx pmu node to configure dphy power.

Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
Reviewed-by: Walker Chen <walker.chen@starfivetech.com>
---
 arch/riscv/boot/dts/starfive/jh7110.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
index f271c3184d3a..a82374afcc91 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -499,6 +499,11 @@ aoncrg: clock-controller@17000000 {
 		aon_syscon: syscon@17010000 {
 			compatible = "starfive,jh7110-aon-syscon", "syscon", "simple-mfd";
 			reg = <0x0 0x17010000 0x0 0x1000>;
+
+			pwrc_dphy: power-controller {
+				compatible = "starfive,jh7110-pmu-dphy";
+				#power-domain-cells = <1>;
+			};
 		};
 
 		aongpio: pinctrl@17020000 {
-- 
2.25.1

Re: [PATCH v1 0/7] Add JH7110 DPHY PMU support

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2998 bytes --]

Hey Walker,

On Mon, Apr 10, 2023 at 11:47:36PM -0700, Changhuang Liang wrote:
> This patchset adds mipi dphy power domain driver for the StarFive JH7110
> SoC. It is used to turn on dphy power switch. The series has been tested
> on the VisionFive 2 board.

Could you review the driver changes here please.

Thanks,
Conor.

> 
> This patchset should be applied after the patchset [1]:
> [1] https://lore.kernel.org/all/20230406103308.1280860-1-william.qiu@starfivetech.com/
> 
> Changhuang Liang (7):
>   dt-bindings: power: Constrain properties for JH7110 PMU
>   soc: starfive: Replace SOC_STARFIVE with ARCH_SATRFIVE
>   soc: starfive: Modify ioremap to regmap
>   soc: starfive: Add pmu type operation
>   soc: starfive: Use call back to parse device tree resources
>   soc: starfive: Add dphy pmu support
>   riscv: dts: starfive: Add dphy rx pmu node
> 
>  .../bindings/power/starfive,jh7110-pmu.yaml   |  14 +-
>  MAINTAINERS                                   |   1 +
>  arch/riscv/boot/dts/starfive/jh7110.dtsi      |   5 +
>  drivers/soc/starfive/Kconfig                  |   4 +-
>  drivers/soc/starfive/jh71xx_pmu.c             | 213 ++++++++++++++----
>  .../dt-bindings/power/starfive,jh7110-pmu.h   |   3 +
>  6 files changed, 187 insertions(+), 53 deletions(-)
> 
> 
> base-commit: 197b6b60ae7bc51dd0814953c562833143b292aa
> prerequisite-patch-id: 388b8adbb0fe2daf4d07a21eafd4f1bd50ce2403
> prerequisite-patch-id: 1117ecaa40a353c667b71802ab34ecf9568d8bb2
> prerequisite-patch-id: b00c6b21fbd0353d88b7c9b09093ba30b765f45b
> prerequisite-patch-id: 08ec9027e8a5c6fdf201726833168c7464a9b94d
> prerequisite-patch-id: fb5120248e48fe1faf053ae0b490c92507ec2b44
> prerequisite-patch-id: 4b93d8d590b0a2abe7b4be5287232c494c35be4a
> prerequisite-patch-id: 89f049f951e5acf75aab92541992f816fd0acc0d
> prerequisite-patch-id: c09c4c68af017b8e5c97b515cb50b70c18a2e705
> prerequisite-patch-id: 0df8ccb0e848c2df4c2da95026494bebecede92d
> prerequisite-patch-id: 315303931e4b6499de7127a88113763f86e97e16
> prerequisite-patch-id: 40cb8212ddb024c20593f73d8b87d9894877e172
> prerequisite-patch-id: a1673a9e9f19d6fab5a51abb721e54e36636f067
> prerequisite-patch-id: d57cc467fb036241b9276320ff076c4a30d376d6
> prerequisite-patch-id: 6e563d68bc5dbf951d4ced17897f9cc4d56169fe
> prerequisite-patch-id: 61ec2caa21fd0fc60e57977f7d16d3f72b135745
> prerequisite-patch-id: 1387a7e87b446329dfc21f3e575ceae7ebcf954c
> prerequisite-patch-id: 258ea5f9b8bf41b6981345dcc81795f25865d38f
> prerequisite-patch-id: 8b6f2c9660c0ac0ee4e73e4c21aca8e6b75e81b9
> prerequisite-patch-id: dbb0c0151b8bdf093e6ce79fd2fe3f60791a6e0b
> prerequisite-patch-id: 9007c8610fdcd387592475949864edde874c20a2
> prerequisite-patch-id: d57e95d31686772abc4c4d5aa1cadc344dc293cd
> prerequisite-patch-id: 0a0ac5a8a90655b415f6b62e324f3db083cdaaee
> prerequisite-patch-id: 7ff6864ac74df5392c8646fe756cadd584fcc813
> prerequisite-patch-id: 284b5d1b95c6d68bca08db1e82ed14930c98b777
> --
> 2.25.1

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]