Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 3498 bytes --]

Hey Changhuang, DT/PHY folks,

On Tue, Apr 18, 2023 at 08:56:41PM -0700, Changhuang Liang wrote:
> Add AON PMU for StarFive JH7110 SoC, it can be used to turn on/off DPHY
> rx/tx power switch, and it don't need the properties of reg and
> interrupts.

Putting this here since the DT guys are more likely to see it this way..
Given how the implementation of the code driving this new
power-controller and the code driving the existing one are rather
different (you've basically re-written the entire driver in this series),
should the dphy driver implement its own power-controller?

I know originally Changuang had tried something along those lines:
https://lore.kernel.org/linux-riscv/5dc4ddc2-9d15-ebb2-38bc-8a544ca67e0d@starfivetech.com/

I see that that was shut down pretty much, partly due to the
non-standard property, hence this series adding the dphy power domain to
the existing driver.

If it was done by looking up the pmu with a
of_find_compatible_node(NULL, "power-controller", "starfive,jh7110-aon-pmu")
type thing, would that make sense? Although, maybe that is not a
question for you, and this series may actually have been better entirely
bundled with the dphy series so the whole thing can be reviewed as a
unit. I've added 

IOW, don't change this patch, or the dts patch, but move all of the
code back into the phy driver..

Sorry for not asking this sooner Changhuang,
Conor.

(hopefully this didn't get sent twice, mutt complained of a bad email
addr during sending the first time)

> 
> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> ---
>  .../bindings/power/starfive,jh7110-pmu.yaml       | 15 +++++++++++++--
>  include/dt-bindings/power/starfive,jh7110-pmu.h   |  3 +++
>  2 files changed, 16 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..c50507c38e14 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-aon-pmu
>  
>    reg:
>      maxItems: 1
> @@ -29,10 +31,19 @@ properties:
>  
>  required:
>    - compatible
> -  - reg
> -  - interrupts
>    - "#power-domain-cells"
>  
> +allOf:
> +  - 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 #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 112 bytes --]

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Changhuang Liang]

On 2023/4/20 2:29, Conor Dooley wrote:
> Hey Changhuang, DT/PHY folks,
> 
> On Tue, Apr 18, 2023 at 08:56:41PM -0700, Changhuang Liang wrote:
>> Add AON PMU for StarFive JH7110 SoC, it can be used to turn on/off DPHY
>> rx/tx power switch, and it don't need the properties of reg and
>> interrupts.
> 
> Putting this here since the DT guys are more likely to see it this way..
> Given how the implementation of the code driving this new
> power-controller and the code driving the existing one are rather
> different (you've basically re-written the entire driver in this series),
> should the dphy driver implement its own power-controller?
> 
> I know originally Changuang had tried something along those lines:
> https://lore.kernel.org/linux-riscv/5dc4ddc2-9d15-ebb2-38bc-8a544ca67e0d@starfivetech.com/
> 
> I see that that was shut down pretty much, partly due to the
> non-standard property, hence this series adding the dphy power domain to
> the existing driver.
> 
> If it was done by looking up the pmu with a
> of_find_compatible_node(NULL, "power-controller", "starfive,jh7110-aon-pmu")
> type thing, would that make sense? Although, maybe that is not a
> question for you, and this series may actually have been better entirely
> bundled with the dphy series so the whole thing can be reviewed as a
> unit. I've added 
> 
> IOW, don't change this patch, or the dts patch, but move all of the
> code back into the phy driver..
> 

Maybe this way can not do that? power domain is binding before driver probe,
if I use "of_find_compatible_node" it phy(DPHY rx) probe. Maybe I can only operate 
this power switch in my phy(DPHY rx) driver, so the all patch of this series isn't 
make sense.

In my opinion, We will also submit DPHY TX module later which use this series.
Maybe this series should independent?

> Sorry for not asking this sooner Changhuang,
> Conor.
> 
> (hopefully this didn't get sent twice, mutt complained of a bad email
> addr during sending the first time)
> 

I'm sorry for that, I will notice later.

>>
>> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
>> ---
>>  .../bindings/power/starfive,jh7110-pmu.yaml       | 15 +++++++++++++--
>>  include/dt-bindings/power/starfive,jh7110-pmu.h   |  3 +++
>>  2 files changed, 16 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..c50507c38e14 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-aon-pmu
>>  
>>    reg:
>>      maxItems: 1
>> @@ -29,10 +31,19 @@ properties:
>>  
>>  required:
>>    - compatible
>> -  - reg
>> -  - interrupts
>>    - "#power-domain-cells"
>>  
>> +allOf:
>> +  - 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
>>

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 5616 bytes --]

Hey Changhuang,

On Thu, Apr 20, 2023 at 03:00:10PM +0800, Changhuang Liang wrote:
> On 2023/4/20 2:29, Conor Dooley wrote:
> > On Tue, Apr 18, 2023 at 08:56:41PM -0700, Changhuang Liang wrote:
> >> Add AON PMU for StarFive JH7110 SoC, it can be used to turn on/off DPHY
> >> rx/tx power switch, and it don't need the properties of reg and
> >> interrupts.
> > 
> > Putting this here since the DT guys are more likely to see it this way..
> > Given how the implementation of the code driving this new
> > power-controller and the code driving the existing one are rather
> > different (you've basically re-written the entire driver in this series),
> > should the dphy driver implement its own power-controller?
> > 
> > I know originally Changuang had tried something along those lines:
> > https://lore.kernel.org/linux-riscv/5dc4ddc2-9d15-ebb2-38bc-8a544ca67e0d@starfivetech.com/
> > 
> > I see that that was shut down pretty much, partly due to the
> > non-standard property, hence this series adding the dphy power domain to
> > the existing driver.
> > 
> > If it was done by looking up the pmu with a
> > of_find_compatible_node(NULL, "power-controller", "starfive,jh7110-aon-pmu")
> > type thing, would that make sense? Although, maybe that is not a
> > question for you, and this series may actually have been better entirely
> > bundled with the dphy series so the whole thing can be reviewed as a
> > unit. I've added 
> > 
> > IOW, don't change this patch, or the dts patch, but move all of the
> > code back into the phy driver..
> > 
> 
> Maybe this way can not do that? power domain is binding before driver probe,
> if I use "of_find_compatible_node" it phy(DPHY rx) probe. Maybe I can only operate 
> this power switch in my phy(DPHY rx) driver, so the all patch of this series isn't 
> make sense.

I'm a wee bit lost here, as I unfortunately know little about how Linux
handles this power-domain stuff. If the DPHY tries to probe and some
pre-requisite does not yet exist, you can return -EPROBE_DEFER right?

But I don't think that's what you are asking, as using
of_find_compatible_node() doesn't depend on there being a driver AFAIU.

> In my opinion, We will also submit DPHY TX module later which use this series.
> Maybe this series should independent?

Is the DPHY tx module a different driver to the rx one?
If yes, does it have a different bit you must set in the syscon?

+CC Walker, do you have a register map for the jh7110? My TRM only says
what the registers are, but not the bits in them. Would make life easier
if I had that info.

I'm fine with taking this code, I just want to make sure that the soc
driver doing this is the right thing to do.
I was kinda hoping that combining with the DPHY-rx series might allow
the PHY folk to spot if you are doing something here with the power
domains that doesn't make sense.

> > Sorry for not asking this sooner Changhuang,
> > Conor.
> > 
> > (hopefully this didn't get sent twice, mutt complained of a bad email
> > addr during sending the first time)
> > 
> 
> I'm sorry for that, I will notice later.

No, this was my mail client doing things that I was unsure of. You
didn't do anything wrong.

> >> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> >> ---
> >>  .../bindings/power/starfive,jh7110-pmu.yaml       | 15 +++++++++++++--
> >>  include/dt-bindings/power/starfive,jh7110-pmu.h   |  3 +++
> >>  2 files changed, 16 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..c50507c38e14 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-aon-pmu

I was speaking to Rob about this over the weekend, he asked:
'Why isn't "starfive,jh7110-aon-syscon" just the power-domain provider
itself?'
Do we actually need to add a new binding for this at all?

Cheers,
Conor.

> >>  
> >>    reg:
> >>      maxItems: 1
> >> @@ -29,10 +31,19 @@ properties:
> >>  
> >>  required:
> >>    - compatible
> >> -  - reg
> >> -  - interrupts
> >>    - "#power-domain-cells"
> >>  
> >> +allOf:
> >> +  - 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 #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 112 bytes --]

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Changhuang Liang]

On 2023/4/25 0:52, Conor Dooley wrote:
> Hey Changhuang,
> 
> On Thu, Apr 20, 2023 at 03:00:10PM +0800, Changhuang Liang wrote:
>> On 2023/4/20 2:29, Conor Dooley wrote:
>>> On Tue, Apr 18, 2023 at 08:56:41PM -0700, Changhuang Liang wrote:
>>>> Add AON PMU for StarFive JH7110 SoC, it can be used to turn on/off DPHY
>>>> rx/tx power switch, and it don't need the properties of reg and
>>>> interrupts.
>>>
>>> Putting this here since the DT guys are more likely to see it this way..
>>> Given how the implementation of the code driving this new
>>> power-controller and the code driving the existing one are rather
>>> different (you've basically re-written the entire driver in this series),
>>> should the dphy driver implement its own power-controller?
>>>
>>> I know originally Changuang had tried something along those lines:
>>> https://lore.kernel.org/linux-riscv/5dc4ddc2-9d15-ebb2-38bc-8a544ca67e0d@starfivetech.com/
>>>
>>> I see that that was shut down pretty much, partly due to the
>>> non-standard property, hence this series adding the dphy power domain to
>>> the existing driver.
>>>
>>> If it was done by looking up the pmu with a
>>> of_find_compatible_node(NULL, "power-controller", "starfive,jh7110-aon-pmu")
>>> type thing, would that make sense? Although, maybe that is not a
>>> question for you, and this series may actually have been better entirely
>>> bundled with the dphy series so the whole thing can be reviewed as a
>>> unit. I've added 
>>>
>>> IOW, don't change this patch, or the dts patch, but move all of the
>>> code back into the phy driver..
>>>
>>
>> Maybe this way can not do that? power domain is binding before driver probe,
>> if I use "of_find_compatible_node" it phy(DPHY rx) probe. Maybe I can only operate 
>> this power switch in my phy(DPHY rx) driver, so the all patch of this series isn't 
>> make sense.
> 
> I'm a wee bit lost here, as I unfortunately know little about how Linux
> handles this power-domain stuff. If the DPHY tries to probe and some
> pre-requisite does not yet exist, you can return -EPROBE_DEFER right?
> 
> But I don't think that's what you are asking, as using
> of_find_compatible_node() doesn't depend on there being a driver AFAIU.
> 
>> In my opinion, We will also submit DPHY TX module later which use this series.
>> Maybe this series should independent?
> 
> Is the DPHY tx module a different driver to the rx one?> If yes, does it have a different bit you must set in the syscon?
> 

Yes, DPHY tx module is a different driver to the DPHY rx. And I have do a
different bit in PATCH 1:

#define JH7110_PD_DPHY_TX	0
#define JH7110_PD_DPHY_RX	1

also in PATCH 5:

static const struct jh71xx_domain_info jh7110_aon_power_domains[] = {
	[JH7110_PD_DPHY_TX] = {
		.name = "DPHY-TX",
		.bit = 30,
	},
	[JH7110_PD_DPHY_RX] = {
		.name = "DPHY-RX",
		.bit = 31,
	},
};

> +CC Walker, do you have a register map for the jh7110? My TRM only says
> what the registers are, but not the bits in them. Would make life easier
> if I had that info.
> 
> I'm fine with taking this code, I just want to make sure that the soc
> driver doing this is the right thing to do.
> I was kinda hoping that combining with the DPHY-rx series might allow
> the PHY folk to spot if you are doing something here with the power
> domains that doesn't make sense.
> 

I asked about our soc colleagues. This syscon register,
offset 0x00:
bit[31] ---> dphy rx power switch
bit[30] ---> dphy tx power switch 
other bits ---> Reserved

>>> Sorry for not asking this sooner Changhuang,
>>> Conor.
>>>
>>> (hopefully this didn't get sent twice, mutt complained of a bad email
>>> addr during sending the first time)
>>>
>>
>> I'm sorry for that, I will notice later.
> 
> No, this was my mail client doing things that I was unsure of. You
> didn't do anything wrong.
> 
[...]
>>>>    - 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-aon-pmu
> 
> I was speaking to Rob about this over the weekend, he asked:
> 'Why isn't "starfive,jh7110-aon-syscon" just the power-domain provider
> itself?'

Maybe not, this syscon only offset "0x00" configure power switch.
other offset configure other functions, maybe not power, so this
"starfive,jh7110-aon-syscon" not the power-domain itself.

> Do we actually need to add a new binding for this at all?
> 
> Cheers,
> Conor.
> 

Maybe this patch do that.
https://lore.kernel.org/all/20230414024157.53203-6-xingyu.wu@starfivetech.com/

>>>>  
>>>>    reg:
>>>>      maxItems: 1
>>>> @@ -29,10 +31,19 @@ properties:
>>>>  

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 5547 bytes --]

On Tue, Apr 25, 2023 at 11:41:38AM +0800, Changhuang Liang wrote:
> On 2023/4/25 0:52, Conor Dooley wrote:
> > On Thu, Apr 20, 2023 at 03:00:10PM +0800, Changhuang Liang wrote:
> >> On 2023/4/20 2:29, Conor Dooley wrote:
> >>> On Tue, Apr 18, 2023 at 08:56:41PM -0700, Changhuang Liang wrote:
> >>>> Add AON PMU for StarFive JH7110 SoC, it can be used to turn on/off DPHY
> >>>> rx/tx power switch, and it don't need the properties of reg and
> >>>> interrupts.
> >>>
> >>> Putting this here since the DT guys are more likely to see it this way..
> >>> Given how the implementation of the code driving this new
> >>> power-controller and the code driving the existing one are rather
> >>> different (you've basically re-written the entire driver in this series),
> >>> should the dphy driver implement its own power-controller?
> >>>
> >>> I know originally Changuang had tried something along those lines:
> >>> https://lore.kernel.org/linux-riscv/5dc4ddc2-9d15-ebb2-38bc-8a544ca67e0d@starfivetech.com/
> >>>
> >>> I see that that was shut down pretty much, partly due to the
> >>> non-standard property, hence this series adding the dphy power domain to
> >>> the existing driver.
> >>>
> >>> If it was done by looking up the pmu with a
> >>> of_find_compatible_node(NULL, "power-controller", "starfive,jh7110-aon-pmu")
> >>> type thing, would that make sense? Although, maybe that is not a
> >>> question for you, and this series may actually have been better entirely
> >>> bundled with the dphy series so the whole thing can be reviewed as a
> >>> unit. I've added 
> >>>
> >>> IOW, don't change this patch, or the dts patch, but move all of the
> >>> code back into the phy driver..
> >>>
> >>
> >> Maybe this way can not do that? power domain is binding before driver probe,
> >> if I use "of_find_compatible_node" it phy(DPHY rx) probe. Maybe I can only operate 
> >> this power switch in my phy(DPHY rx) driver, so the all patch of this series isn't 
> >> make sense.
> > 
> > I'm a wee bit lost here, as I unfortunately know little about how Linux
> > handles this power-domain stuff. If the DPHY tries to probe and some
> > pre-requisite does not yet exist, you can return -EPROBE_DEFER right?
> > 
> > But I don't think that's what you are asking, as using
> > of_find_compatible_node() doesn't depend on there being a driver AFAIU.
> > 
> >> In my opinion, We will also submit DPHY TX module later which use this series.
> >> Maybe this series should independent?
> > 
> > Is the DPHY tx module a different driver to the rx one?> If yes, does it have a different bit you must set in the syscon?
> > 
> 
> Yes, DPHY tx module is a different driver to the DPHY rx. And I have do a
> different bit in PATCH 1:
> 
> #define JH7110_PD_DPHY_TX	0
> #define JH7110_PD_DPHY_RX	1
> 
> also in PATCH 5:
> 
> static const struct jh71xx_domain_info jh7110_aon_power_domains[] = {
> 	[JH7110_PD_DPHY_TX] = {
> 		.name = "DPHY-TX",
> 		.bit = 30,
> 	},
> 	[JH7110_PD_DPHY_RX] = {
> 		.name = "DPHY-RX",
> 		.bit = 31,
> 	},
> };
> 
> > +CC Walker, do you have a register map for the jh7110? My TRM only says
> > what the registers are, but not the bits in them. Would make life easier
> > if I had that info.
> > 
> > I'm fine with taking this code, I just want to make sure that the soc
> > driver doing this is the right thing to do.
> > I was kinda hoping that combining with the DPHY-rx series might allow
> > the PHY folk to spot if you are doing something here with the power
> > domains that doesn't make sense.
> > 
> 
> I asked about our soc colleagues. This syscon register,
> offset 0x00:
> bit[31] ---> dphy rx power switch
> bit[30] ---> dphy tx power switch 
> other bits ---> Reserved

Okay. Unless someone explicitly disagrees, I'm fine with doing this
stand-alone from the DPHY drivers.

> >>> Sorry for not asking this sooner Changhuang,
> >>> Conor.
> >>>
> >>> (hopefully this didn't get sent twice, mutt complained of a bad email
> >>> addr during sending the first time)
> >>>
> >>
> >> I'm sorry for that, I will notice later.
> > 
> > No, this was my mail client doing things that I was unsure of. You
> > didn't do anything wrong.
> > 
> [...]
> >>>>    - 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-aon-pmu
> > 
> > I was speaking to Rob about this over the weekend, he asked:
> > 'Why isn't "starfive,jh7110-aon-syscon" just the power-domain provider
> > itself?'
> 
> Maybe not, this syscon only offset "0x00" configure power switch.
> other offset configure other functions, maybe not power, so this
> "starfive,jh7110-aon-syscon" not the power-domain itself.
> 
> > Do we actually need to add a new binding for this at all?
> > 
> > Cheers,
> > Conor.
> > 
> 
> Maybe this patch do that.
> https://lore.kernel.org/all/20230414024157.53203-6-xingyu.wu@starfivetech.com/

This makes it a child-node right? I think Rob already said no to that in
and earlier revision of this series. What he meant the other day was
making the syscon itself a power domain controller, since the child node
has no meaningful properties (reg, interrupts etc).

Cheers,
Conor.

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

[-- Attachment #2: Type: text/plain, Size: 112 bytes --]

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Changhuang Liang]

>>>>>>  
>>>>>>  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-aon-pmu
>>>
>>> I was speaking to Rob about this over the weekend, he asked:
>>> 'Why isn't "starfive,jh7110-aon-syscon" just the power-domain provider
>>> itself?'
>>
>> Maybe not, this syscon only offset "0x00" configure power switch.
>> other offset configure other functions, maybe not power, so this
>> "starfive,jh7110-aon-syscon" not the power-domain itself.
>>
>>> Do we actually need to add a new binding for this at all?
>>>
>>> Cheers,
>>> Conor.
>>>
>>
>> Maybe this patch do that.
>> https://lore.kernel.org/all/20230414024157.53203-6-xingyu.wu@starfivetech.com/
> 
> This makes it a child-node right? I think Rob already said no to that in
> and earlier revision of this series. What he meant the other day was
> making the syscon itself a power domain controller, since the child node
> has no meaningful properties (reg, interrupts etc).
> 
> Cheers,
> Conor.

Yes, "starfive,jh7110-aon-pmu" is a child-node of "starfive,jh7110-aon-syscon".
In my opinion, "0x17010000" is "aon-syscon" on JH7110 SoC, and this "aon-pmu" is just 
a part of "aon-syscon" function, so I think it is inappropriate to make "aon-syscon"
to a power domain controller. I think using the child-node description is closer to
JH7110 SoC. 

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Krzysztof Kozlowski]

On 25/04/2023 09:57, Changhuang Liang wrote:
>>>>>>>  
>>>>>>>  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-aon-pmu
>>>>
>>>> I was speaking to Rob about this over the weekend, he asked:
>>>> 'Why isn't "starfive,jh7110-aon-syscon" just the power-domain provider
>>>> itself?'
>>>
>>> Maybe not, this syscon only offset "0x00" configure power switch.
>>> other offset configure other functions, maybe not power, so this
>>> "starfive,jh7110-aon-syscon" not the power-domain itself.
>>>
>>>> Do we actually need to add a new binding for this at all?
>>>>
>>>> Cheers,
>>>> Conor.
>>>>
>>>
>>> Maybe this patch do that.
>>> https://lore.kernel.org/all/20230414024157.53203-6-xingyu.wu@starfivetech.com/
>>
>> This makes it a child-node right? I think Rob already said no to that in
>> and earlier revision of this series. What he meant the other day was
>> making the syscon itself a power domain controller, since the child node
>> has no meaningful properties (reg, interrupts etc).
>>
>> Cheers,
>> Conor.
> 
> Yes, "starfive,jh7110-aon-pmu" is a child-node of "starfive,jh7110-aon-syscon".
> In my opinion, "0x17010000" is "aon-syscon" on JH7110 SoC, and this "aon-pmu" is just 
> a part of "aon-syscon" function, so I think it is inappropriate to make "aon-syscon"
> to a power domain controller. I think using the child-node description is closer to
> JH7110 SoC. 

Unfortunately, I do not see the correlation between these, any
connection. Why being a child of syscon block would mean that this
should no be power domain controller? Really, why? These are two
unrelated things.

Best regards,
Krzysztof


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Changhuang Liang]

On 2023/4/25 16:19, Krzysztof Kozlowski wrote:
> On 25/04/2023 09:57, Changhuang Liang wrote:
>>>>>>>>  
>>>>>>>>  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-aon-pmu
>>>>>
>>>>> I was speaking to Rob about this over the weekend, he asked:
>>>>> 'Why isn't "starfive,jh7110-aon-syscon" just the power-domain provider
>>>>> itself?'
>>>>
>>>> Maybe not, this syscon only offset "0x00" configure power switch.
>>>> other offset configure other functions, maybe not power, so this
>>>> "starfive,jh7110-aon-syscon" not the power-domain itself.
>>>>
>>>>> Do we actually need to add a new binding for this at all?
>>>>>
>>>>> Cheers,
>>>>> Conor.
>>>>>
>>>>
>>>> Maybe this patch do that.
>>>> https://lore.kernel.org/all/20230414024157.53203-6-xingyu.wu@starfivetech.com/
>>>
>>> This makes it a child-node right? I think Rob already said no to that in
>>> and earlier revision of this series. What he meant the other day was
>>> making the syscon itself a power domain controller, since the child node
>>> has no meaningful properties (reg, interrupts etc).
>>>
>>> Cheers,
>>> Conor.
>>
>> Yes, "starfive,jh7110-aon-pmu" is a child-node of "starfive,jh7110-aon-syscon".
>> In my opinion, "0x17010000" is "aon-syscon" on JH7110 SoC, and this "aon-pmu" is just 
>> a part of "aon-syscon" function, so I think it is inappropriate to make "aon-syscon"
>> to a power domain controller. I think using the child-node description is closer to
>> JH7110 SoC. 
> 
> Unfortunately, I do not see the correlation between these, any
> connection. Why being a child of syscon block would mean that this
> should no be power domain controller? Really, why? These are two
> unrelated things.
> 
> Best regards,
> Krzysztof
> 

Let me summarize what has been discussed above. 

There has two ways to describe this "starfive,jh7110-aon-syscon"(0x17010000).
1. (0x17010000) is power-controller node:

	aon_pwrc: power-controller@17010000 {
		compatible = "starfive,jh7110-aon-pmu", "syscon";
		reg = <0x0 0x17010000 0x0 0x1000>;
		#power-domain-cells = <1>;
	};


2. (0x17010000) is syscon node, power-controller is child-node of syscon:

	aon_syscon: syscon@17010000 {
		compatible = "starfive,jh7110-aon-syscon", "syscon", "simple-mfd";
		reg = <0x0 0x17010000 0x0 0x1000>;

		aon_pwrc: power-controller {
			compatible = "starfive,jh7110-aon-pmu";
			#power-domain-cells = <1>;
		};
	};

I prefer the way of 2.
This is more in line with the hardware description of JH7110.

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 3115 bytes --]

On Tue, Apr 25, 2023 at 05:18:10PM +0800, Changhuang Liang wrote:
> 
> 
> On 2023/4/25 16:19, Krzysztof Kozlowski wrote:
> > On 25/04/2023 09:57, Changhuang Liang wrote:
> >>>>>>>>  
> >>>>>>>>  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-aon-pmu
> >>>>>
> >>>>> I was speaking to Rob about this over the weekend, he asked:
> >>>>> 'Why isn't "starfive,jh7110-aon-syscon" just the power-domain provider
> >>>>> itself?'
> >>>>
> >>>> Maybe not, this syscon only offset "0x00" configure power switch.
> >>>> other offset configure other functions, maybe not power, so this
> >>>> "starfive,jh7110-aon-syscon" not the power-domain itself.
> >>>>
> >>>>> Do we actually need to add a new binding for this at all?
> >>>>>
> >>>>> Cheers,
> >>>>> Conor.
> >>>>>
> >>>>
> >>>> Maybe this patch do that.
> >>>> https://lore.kernel.org/all/20230414024157.53203-6-xingyu.wu@starfivetech.com/
> >>>
> >>> This makes it a child-node right? I think Rob already said no to that in
> >>> and earlier revision of this series. What he meant the other day was
> >>> making the syscon itself a power domain controller, since the child node
> >>> has no meaningful properties (reg, interrupts etc).
> >>>
> >>> Cheers,
> >>> Conor.
> >>
> >> Yes, "starfive,jh7110-aon-pmu" is a child-node of "starfive,jh7110-aon-syscon".
> >> In my opinion, "0x17010000" is "aon-syscon" on JH7110 SoC, and this "aon-pmu" is just 
> >> a part of "aon-syscon" function, so I think it is inappropriate to make "aon-syscon"
> >> to a power domain controller. I think using the child-node description is closer to
> >> JH7110 SoC. 
> > 
> > Unfortunately, I do not see the correlation between these, any
> > connection. Why being a child of syscon block would mean that this
> > should no be power domain controller? Really, why? These are two
> > unrelated things.
> > 
> > Best regards,
> > Krzysztof
> > 
> 
> Let me summarize what has been discussed above. 
> 
> There has two ways to describe this "starfive,jh7110-aon-syscon"(0x17010000).
> 1. (0x17010000) is power-controller node:
> 
> 	aon_pwrc: power-controller@17010000 {
> 		compatible = "starfive,jh7110-aon-pmu", "syscon";
> 		reg = <0x0 0x17010000 0x0 0x1000>;
> 		#power-domain-cells = <1>;
> 	};
> 
> 
> 2. (0x17010000) is syscon node, power-controller is child-node of syscon:
> 
> 	aon_syscon: syscon@17010000 {
> 		compatible = "starfive,jh7110-aon-syscon", "syscon", "simple-mfd";
> 		reg = <0x0 0x17010000 0x0 0x1000>;
> 
> 		aon_pwrc: power-controller {
> 			compatible = "starfive,jh7110-aon-pmu";
> 			#power-domain-cells = <1>;
> 		};
> 	};

I thought that Rob was suggesting something like this:
	aon_syscon: syscon@17010000 {
		compatible = "starfive,jh7110-aon-syscon", ...
		reg = <0x0 0x17010000 0x0 0x1000>;
		#power-domain-cells = <1>;
	};

Cheers,
Conor.


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

[-- Attachment #2: Type: text/plain, Size: 112 bytes --]

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Changhuang Liang]

On 2023/4/25 17:35, Conor Dooley wrote:
> On Tue, Apr 25, 2023 at 05:18:10PM +0800, Changhuang Liang wrote:
>>
>>
>> On 2023/4/25 16:19, Krzysztof Kozlowski wrote:
>>> On 25/04/2023 09:57, Changhuang Liang wrote:
>>>>>>>>>>  
>>>>>>>>>>  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-aon-pmu
>>>>>>>
>>>>>>> I was speaking to Rob about this over the weekend, he asked:
>>>>>>> 'Why isn't "starfive,jh7110-aon-syscon" just the power-domain provider
>>>>>>> itself?'
>>>>>>
>>>>>> Maybe not, this syscon only offset "0x00" configure power switch.
>>>>>> other offset configure other functions, maybe not power, so this
>>>>>> "starfive,jh7110-aon-syscon" not the power-domain itself.
>>>>>>
>>>>>>> Do we actually need to add a new binding for this at all?
>>>>>>>
>>>>>>> Cheers,
>>>>>>> Conor.
>>>>>>>
>>>>>>
>>>>>> Maybe this patch do that.
>>>>>> https://lore.kernel.org/all/20230414024157.53203-6-xingyu.wu@starfivetech.com/
>>>>>
>>>>> This makes it a child-node right? I think Rob already said no to that in
>>>>> and earlier revision of this series. What he meant the other day was
>>>>> making the syscon itself a power domain controller, since the child node
>>>>> has no meaningful properties (reg, interrupts etc).
>>>>>
>>>>> Cheers,
>>>>> Conor.
>>>>
>>>> Yes, "starfive,jh7110-aon-pmu" is a child-node of "starfive,jh7110-aon-syscon".
>>>> In my opinion, "0x17010000" is "aon-syscon" on JH7110 SoC, and this "aon-pmu" is just 
>>>> a part of "aon-syscon" function, so I think it is inappropriate to make "aon-syscon"
>>>> to a power domain controller. I think using the child-node description is closer to
>>>> JH7110 SoC. 
>>>
>>> Unfortunately, I do not see the correlation between these, any
>>> connection. Why being a child of syscon block would mean that this
>>> should no be power domain controller? Really, why? These are two
>>> unrelated things.
>>>
>>> Best regards,
>>> Krzysztof
>>>
>>
>> Let me summarize what has been discussed above. 
>>
>> There has two ways to describe this "starfive,jh7110-aon-syscon"(0x17010000).
>> 1. (0x17010000) is power-controller node:
>>
>> 	aon_pwrc: power-controller@17010000 {
>> 		compatible = "starfive,jh7110-aon-pmu", "syscon";
>> 		reg = <0x0 0x17010000 0x0 0x1000>;
>> 		#power-domain-cells = <1>;
>> 	};
>>
>>
>> 2. (0x17010000) is syscon node, power-controller is child-node of syscon:
>>
>> 	aon_syscon: syscon@17010000 {
>> 		compatible = "starfive,jh7110-aon-syscon", "syscon", "simple-mfd";
>> 		reg = <0x0 0x17010000 0x0 0x1000>;
>>
>> 		aon_pwrc: power-controller {
>> 			compatible = "starfive,jh7110-aon-pmu";
>> 			#power-domain-cells = <1>;
>> 		};
>> 	};
> 
> I thought that Rob was suggesting something like this:
> 	aon_syscon: syscon@17010000 {
> 		compatible = "starfive,jh7110-aon-syscon", ...
> 		reg = <0x0 0x17010000 0x0 0x1000>;
> 		#power-domain-cells = <1>;
> 	};
> 
> Cheers,
> Conor.
> 

I see the kernel:
https://elixir.bootlin.com/linux/latest/source/arch/arm64/boot/dts/mediatek/mt8167.dtsi
this file line 42:
it's power-controller also has no meaningful properties.
What do you think?


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 2498 bytes --]

On Tue, Apr 25, 2023 at 08:26:35PM +0800, Changhuang Liang wrote:
> On 2023/4/25 17:35, Conor Dooley wrote:
> > On Tue, Apr 25, 2023 at 05:18:10PM +0800, Changhuang Liang wrote:
> >> On 2023/4/25 16:19, Krzysztof Kozlowski wrote:
> >>> On 25/04/2023 09:57, Changhuang Liang wrote:
> >>>> Yes, "starfive,jh7110-aon-pmu" is a child-node of "starfive,jh7110-aon-syscon".
> >>>> In my opinion, "0x17010000" is "aon-syscon" on JH7110 SoC, and this "aon-pmu" is just 
> >>>> a part of "aon-syscon" function, so I think it is inappropriate to make "aon-syscon"
> >>>> to a power domain controller. I think using the child-node description is closer to
> >>>> JH7110 SoC. 
> >>>
> >>> Unfortunately, I do not see the correlation between these, any
> >>> connection. Why being a child of syscon block would mean that this
> >>> should no be power domain controller? Really, why? These are two
> >>> unrelated things.
> >>
> >> Let me summarize what has been discussed above. 
> >>
> >> There has two ways to describe this "starfive,jh7110-aon-syscon"(0x17010000).
> >> 1. (0x17010000) is power-controller node:
> >>
> >> 	aon_pwrc: power-controller@17010000 {
> >> 		compatible = "starfive,jh7110-aon-pmu", "syscon";
> >> 		reg = <0x0 0x17010000 0x0 0x1000>;
> >> 		#power-domain-cells = <1>;
> >> 	};
> >>
> >>
> >> 2. (0x17010000) is syscon node, power-controller is child-node of syscon:
> >>
> >> 	aon_syscon: syscon@17010000 {
> >> 		compatible = "starfive,jh7110-aon-syscon", "syscon", "simple-mfd";
> >> 		reg = <0x0 0x17010000 0x0 0x1000>;
> >>
> >> 		aon_pwrc: power-controller {
> >> 			compatible = "starfive,jh7110-aon-pmu";
> >> 			#power-domain-cells = <1>;
> >> 		};
> >> 	};
> > 
> > I thought that Rob was suggesting something like this:
> > 	aon_syscon: syscon@17010000 {
> > 		compatible = "starfive,jh7110-aon-syscon", ...
> > 		reg = <0x0 0x17010000 0x0 0x1000>;
> > 		#power-domain-cells = <1>;
> > 	};

> I see the kernel:
> https://elixir.bootlin.com/linux/latest/source/arch/arm64/boot/dts/mediatek/mt8167.dtsi
> this file line 42:
> it's power-controller also has no meaningful properties.
> What do you think?

I'm not sure that I follow. It has a bunch of child-nodes does it not,
each of which is a domain?

I didn't see such domains in your dts patch, they're defined directly in
the driver instead AFAIU. Assuming I have understood that correctly,
your situation is different to that mediatek one?

Cheers,
Conor.

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

[-- Attachment #2: Type: text/plain, Size: 112 bytes --]

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Changhuang Liang]

On 2023/4/26 0:56, Conor Dooley wrote:
> On Tue, Apr 25, 2023 at 08:26:35PM +0800, Changhuang Liang wrote:
>> On 2023/4/25 17:35, Conor Dooley wrote:
>>> On Tue, Apr 25, 2023 at 05:18:10PM +0800, Changhuang Liang wrote:
>>>> On 2023/4/25 16:19, Krzysztof Kozlowski wrote:
>>>>> On 25/04/2023 09:57, Changhuang Liang wrote:
>>>>>> Yes, "starfive,jh7110-aon-pmu" is a child-node of "starfive,jh7110-aon-syscon".
>>>>>> In my opinion, "0x17010000" is "aon-syscon" on JH7110 SoC, and this "aon-pmu" is just 
>>>>>> a part of "aon-syscon" function, so I think it is inappropriate to make "aon-syscon"
>>>>>> to a power domain controller. I think using the child-node description is closer to
>>>>>> JH7110 SoC. 
>>>>>
>>>>> Unfortunately, I do not see the correlation between these, any
>>>>> connection. Why being a child of syscon block would mean that this
>>>>> should no be power domain controller? Really, why? These are two
>>>>> unrelated things.
>>>>
>>>> Let me summarize what has been discussed above. 
>>>>
>>>> There has two ways to describe this "starfive,jh7110-aon-syscon"(0x17010000).
>>>> 1. (0x17010000) is power-controller node:
>>>>
>>>> 	aon_pwrc: power-controller@17010000 {
>>>> 		compatible = "starfive,jh7110-aon-pmu", "syscon";
>>>> 		reg = <0x0 0x17010000 0x0 0x1000>;
>>>> 		#power-domain-cells = <1>;
>>>> 	};
>>>>
>>>>
>>>> 2. (0x17010000) is syscon node, power-controller is child-node of syscon:
>>>>
>>>> 	aon_syscon: syscon@17010000 {
>>>> 		compatible = "starfive,jh7110-aon-syscon", "syscon", "simple-mfd";
>>>> 		reg = <0x0 0x17010000 0x0 0x1000>;
>>>>
>>>> 		aon_pwrc: power-controller {
>>>> 			compatible = "starfive,jh7110-aon-pmu";
>>>> 			#power-domain-cells = <1>;
>>>> 		};
>>>> 	};
>>>
>>> I thought that Rob was suggesting something like this:
>>> 	aon_syscon: syscon@17010000 {
>>> 		compatible = "starfive,jh7110-aon-syscon", ...
>>> 		reg = <0x0 0x17010000 0x0 0x1000>;
>>> 		#power-domain-cells = <1>;
>>> 	};
> 
>> I see the kernel:
>> https://elixir.bootlin.com/linux/latest/source/arch/arm64/boot/dts/mediatek/mt8167.dtsi
>> this file line 42:
>> it's power-controller also has no meaningful properties.
>> What do you think?
> 
> I'm not sure that I follow. It has a bunch of child-nodes does it not,
> each of which is a domain?
> 
> I didn't see such domains in your dts patch, they're defined directly in
> the driver instead AFAIU. Assuming I have understood that correctly,
> your situation is different to that mediatek one?
> 
> Cheers,
> Conor.

I think there child-nodes just need to operate some clock signals. Maybe
we don't need to discuss other platforms.

If Rob's method is confirmed. I will try it next version.

Maybe like this:
aon_syscon: syscon@17010000 {
	compatible = "starfive,jh7110-aon-syscon", "syscon", "starfive,jh7110-aon-pmu";
	reg = <0x0 0x17010000 0x0 0x1000>;
	#power-domain-cells = <1>;
};

Rob and krzystof:

And I think patch[1][2] need to change. Right? 

[1] https://lore.kernel.org/all/20230414024157.53203-6-xingyu.wu@starfivetech.com/
[2] https://lore.kernel.org/all/20230414024157.53203-7-xingyu.wu@starfivetech.com/

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Changhuang Liang]

On 2023/4/26 0:56, Conor Dooley wrote:
> On Tue, Apr 25, 2023 at 08:26:35PM +0800, Changhuang Liang wrote:
>> On 2023/4/25 17:35, Conor Dooley wrote:
>>> On Tue, Apr 25, 2023 at 05:18:10PM +0800, Changhuang Liang wrote:
>>>> On 2023/4/25 16:19, Krzysztof Kozlowski wrote:
>>>>> On 25/04/2023 09:57, Changhuang Liang wrote:
>>>>>> Yes, "starfive,jh7110-aon-pmu" is a child-node of "starfive,jh7110-aon-syscon".
>>>>>> In my opinion, "0x17010000" is "aon-syscon" on JH7110 SoC, and this "aon-pmu" is just 
>>>>>> a part of "aon-syscon" function, so I think it is inappropriate to make "aon-syscon"
>>>>>> to a power domain controller. I think using the child-node description is closer to
>>>>>> JH7110 SoC. 
>>>>>
>>>>> Unfortunately, I do not see the correlation between these, any
>>>>> connection. Why being a child of syscon block would mean that this
>>>>> should no be power domain controller? Really, why? These are two
>>>>> unrelated things.
>>>>
>>>> Let me summarize what has been discussed above. 
>>>>
>>>> There has two ways to describe this "starfive,jh7110-aon-syscon"(0x17010000).
>>>> 1. (0x17010000) is power-controller node:
>>>>
>>>> 	aon_pwrc: power-controller@17010000 {
>>>> 		compatible = "starfive,jh7110-aon-pmu", "syscon";
>>>> 		reg = <0x0 0x17010000 0x0 0x1000>;
>>>> 		#power-domain-cells = <1>;
>>>> 	};
>>>>
>>>>
>>>> 2. (0x17010000) is syscon node, power-controller is child-node of syscon:
>>>>
>>>> 	aon_syscon: syscon@17010000 {
>>>> 		compatible = "starfive,jh7110-aon-syscon", "syscon", "simple-mfd";
>>>> 		reg = <0x0 0x17010000 0x0 0x1000>;
>>>>
>>>> 		aon_pwrc: power-controller {
>>>> 			compatible = "starfive,jh7110-aon-pmu";
>>>> 			#power-domain-cells = <1>;
>>>> 		};
>>>> 	};
>>>
>>> I thought that Rob was suggesting something like this:
>>> 	aon_syscon: syscon@17010000 {
>>> 		compatible = "starfive,jh7110-aon-syscon", ...
>>> 		reg = <0x0 0x17010000 0x0 0x1000>;
>>> 		#power-domain-cells = <1>;
>>> 	};
> 
>> I see the kernel:
>> https://elixir.bootlin.com/linux/latest/source/arch/arm64/boot/dts/mediatek/mt8167.dtsi
>> this file line 42:
>> it's power-controller also has no meaningful properties.
>> What do you think?
> 
> I'm not sure that I follow. It has a bunch of child-nodes does it not,
> each of which is a domain?
> 
> I didn't see such domains in your dts patch, they're defined directly in
> the driver instead AFAIU. Assuming I have understood that correctly,
> your situation is different to that mediatek one?
> 
> Cheers,
> Conor.

Conor and Rob, 

How about this way:

aon_syscon: syscon@17010000 {
	compatible = "starfive,jh7110-aon-syscon", "syscon", "simple-mfd";
	reg = <0x0 0x17010000 0x0 0x1000>;
	
	aon_pwrc: power-controller {
		compatible = "starfive,jh7110-aon-pmu";
		regmap = <&aon_syscon>;
		#power-domain-cells = <1>;
	};
};

Add a "regmap" property which is phandle. And it can keep the present child-node
structure. This is more consistent with our soc design.

Best regards,
Changhuang

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Krzysztof Kozlowski]

On 04/05/2023 03:34, Changhuang Liang wrote:
> 
> 
> On 2023/4/26 0:56, Conor Dooley wrote:
>> On Tue, Apr 25, 2023 at 08:26:35PM +0800, Changhuang Liang wrote:
>>> On 2023/4/25 17:35, Conor Dooley wrote:
>>>> On Tue, Apr 25, 2023 at 05:18:10PM +0800, Changhuang Liang wrote:
>>>>> On 2023/4/25 16:19, Krzysztof Kozlowski wrote:
>>>>>> On 25/04/2023 09:57, Changhuang Liang wrote:
>>>>>>> Yes, "starfive,jh7110-aon-pmu" is a child-node of "starfive,jh7110-aon-syscon".
>>>>>>> In my opinion, "0x17010000" is "aon-syscon" on JH7110 SoC, and this "aon-pmu" is just 
>>>>>>> a part of "aon-syscon" function, so I think it is inappropriate to make "aon-syscon"
>>>>>>> to a power domain controller. I think using the child-node description is closer to
>>>>>>> JH7110 SoC. 
>>>>>>
>>>>>> Unfortunately, I do not see the correlation between these, any
>>>>>> connection. Why being a child of syscon block would mean that this
>>>>>> should no be power domain controller? Really, why? These are two
>>>>>> unrelated things.
>>>>>
>>>>> Let me summarize what has been discussed above. 
>>>>>
>>>>> There has two ways to describe this "starfive,jh7110-aon-syscon"(0x17010000).
>>>>> 1. (0x17010000) is power-controller node:
>>>>>
>>>>> 	aon_pwrc: power-controller@17010000 {
>>>>> 		compatible = "starfive,jh7110-aon-pmu", "syscon";
>>>>> 		reg = <0x0 0x17010000 0x0 0x1000>;
>>>>> 		#power-domain-cells = <1>;
>>>>> 	};
>>>>>
>>>>>
>>>>> 2. (0x17010000) is syscon node, power-controller is child-node of syscon:
>>>>>
>>>>> 	aon_syscon: syscon@17010000 {
>>>>> 		compatible = "starfive,jh7110-aon-syscon", "syscon", "simple-mfd";
>>>>> 		reg = <0x0 0x17010000 0x0 0x1000>;
>>>>>
>>>>> 		aon_pwrc: power-controller {
>>>>> 			compatible = "starfive,jh7110-aon-pmu";
>>>>> 			#power-domain-cells = <1>;
>>>>> 		};
>>>>> 	};
>>>>
>>>> I thought that Rob was suggesting something like this:
>>>> 	aon_syscon: syscon@17010000 {
>>>> 		compatible = "starfive,jh7110-aon-syscon", ...
>>>> 		reg = <0x0 0x17010000 0x0 0x1000>;
>>>> 		#power-domain-cells = <1>;
>>>> 	};
>>
>>> I see the kernel:
>>> https://elixir.bootlin.com/linux/latest/source/arch/arm64/boot/dts/mediatek/mt8167.dtsi
>>> this file line 42:
>>> it's power-controller also has no meaningful properties.
>>> What do you think?
>>
>> I'm not sure that I follow. It has a bunch of child-nodes does it not,
>> each of which is a domain?
>>
>> I didn't see such domains in your dts patch, they're defined directly in
>> the driver instead AFAIU. Assuming I have understood that correctly,
>> your situation is different to that mediatek one?
>>
>> Cheers,
>> Conor.
> 
> Conor and Rob, 
> 
> How about this way:
> 
> aon_syscon: syscon@17010000 {
> 	compatible = "starfive,jh7110-aon-syscon", "syscon", "simple-mfd";
> 	reg = <0x0 0x17010000 0x0 0x1000>;
> 	
> 	aon_pwrc: power-controller {
> 		compatible = "starfive,jh7110-aon-pmu";
> 		regmap = <&aon_syscon>;
> 		#power-domain-cells = <1>;
> 	};
> };
> 
> Add a "regmap" property which is phandle. And it can keep the present child-node
> structure. This is more consistent with our soc design.

Adding property from child to parent does not make any sense. Didn't you
already receive comment on this?

Best regards,
Krzysztof


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Changhuang Liang]

On 2023/5/4 14:13, Krzysztof Kozlowski wrote:
> On 04/05/2023 03:34, Changhuang Liang wrote:
>>
>>
>> On 2023/4/26 0:56, Conor Dooley wrote:
>>> On Tue, Apr 25, 2023 at 08:26:35PM +0800, Changhuang Liang wrote:
>>>> On 2023/4/25 17:35, Conor Dooley wrote:
>>>>> On Tue, Apr 25, 2023 at 05:18:10PM +0800, Changhuang Liang wrote:
>>>>>> On 2023/4/25 16:19, Krzysztof Kozlowski wrote:
>>>>>>> On 25/04/2023 09:57, Changhuang Liang wrote:
>>>>>>>> Yes, "starfive,jh7110-aon-pmu" is a child-node of "starfive,jh7110-aon-syscon".
>>>>>>>> In my opinion, "0x17010000" is "aon-syscon" on JH7110 SoC, and this "aon-pmu" is just 
>>>>>>>> a part of "aon-syscon" function, so I think it is inappropriate to make "aon-syscon"
>>>>>>>> to a power domain controller. I think using the child-node description is closer to
>>>>>>>> JH7110 SoC. 
>>>>>>>
>>>>>>> Unfortunately, I do not see the correlation between these, any
>>>>>>> connection. Why being a child of syscon block would mean that this
>>>>>>> should no be power domain controller? Really, why? These are two
>>>>>>> unrelated things.
>>>>>>
>>>>>> Let me summarize what has been discussed above. 
>>>>>>
>>>>>> There has two ways to describe this "starfive,jh7110-aon-syscon"(0x17010000).
>>>>>> 1. (0x17010000) is power-controller node:
>>>>>>
>>>>>> 	aon_pwrc: power-controller@17010000 {
>>>>>> 		compatible = "starfive,jh7110-aon-pmu", "syscon";
>>>>>> 		reg = <0x0 0x17010000 0x0 0x1000>;
>>>>>> 		#power-domain-cells = <1>;
>>>>>> 	};
>>>>>>
>>>>>>
>>>>>> 2. (0x17010000) is syscon node, power-controller is child-node of syscon:
>>>>>>
>>>>>> 	aon_syscon: syscon@17010000 {
>>>>>> 		compatible = "starfive,jh7110-aon-syscon", "syscon", "simple-mfd";
>>>>>> 		reg = <0x0 0x17010000 0x0 0x1000>;
>>>>>>
>>>>>> 		aon_pwrc: power-controller {
>>>>>> 			compatible = "starfive,jh7110-aon-pmu";
>>>>>> 			#power-domain-cells = <1>;
>>>>>> 		};
>>>>>> 	};
>>>>>
>>>>> I thought that Rob was suggesting something like this:
>>>>> 	aon_syscon: syscon@17010000 {
>>>>> 		compatible = "starfive,jh7110-aon-syscon", ...
>>>>> 		reg = <0x0 0x17010000 0x0 0x1000>;
>>>>> 		#power-domain-cells = <1>;
>>>>> 	};
>>>
>>>> I see the kernel:
>>>> https://elixir.bootlin.com/linux/latest/source/arch/arm64/boot/dts/mediatek/mt8167.dtsi
>>>> this file line 42:
>>>> it's power-controller also has no meaningful properties.
>>>> What do you think?
>>>
>>> I'm not sure that I follow. It has a bunch of child-nodes does it not,
>>> each of which is a domain?
>>>
>>> I didn't see such domains in your dts patch, they're defined directly in
>>> the driver instead AFAIU. Assuming I have understood that correctly,
>>> your situation is different to that mediatek one?
>>>
>>> Cheers,
>>> Conor.
>>
>> Conor and Rob, 
>>
>> How about this way:
>>
>> aon_syscon: syscon@17010000 {
>> 	compatible = "starfive,jh7110-aon-syscon", "syscon", "simple-mfd";
>> 	reg = <0x0 0x17010000 0x0 0x1000>;
>> 	
>> 	aon_pwrc: power-controller {
>> 		compatible = "starfive,jh7110-aon-pmu";
>> 		regmap = <&aon_syscon>;
>> 		#power-domain-cells = <1>;
>> 	};
>> };
>>
>> Add a "regmap" property which is phandle. And it can keep the present child-node
>> structure. This is more consistent with our soc design.
> 
> Adding property from child to parent does not make any sense. Didn't you
> already receive comment on this?
> 
> Best regards,
> Krzysztof
> 

Krzysztof,

I am confused about what to do next. How to add this power-controller's
node in device tree?

Best regards,
Changhuang


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Krzysztof Kozlowski]

On 04/05/2023 08:53, Changhuang Liang wrote:
>>> 	};
>>> };
>>>
>>> Add a "regmap" property which is phandle. And it can keep the present child-node
>>> structure. This is more consistent with our soc design.
>>
>> Adding property from child to parent does not make any sense. Didn't you
>> already receive comment on this?
>>
>> Best regards,
>> Krzysztof
>>
> 
> Krzysztof,
> 
> I am confused about what to do next. How to add this power-controller's
> node in device tree?
> 

You just move power-domain-cells up.

Best regards,
Krzysztof


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Changhuang Liang]

On 2023/5/4 15:04, Krzysztof Kozlowski wrote:
> On 04/05/2023 08:53, Changhuang Liang wrote:
>>>> 	};
>>>> };
>>>>
>>>> Add a "regmap" property which is phandle. And it can keep the present child-node
>>>> structure. This is more consistent with our soc design.
>>>
>>> Adding property from child to parent does not make any sense. Didn't you
>>> already receive comment on this?
>>>
>>> Best regards,
>>> Krzysztof
>>>
>>
>> Krzysztof,
>>
>> I am confused about what to do next. How to add this power-controller's
>> node in device tree?
>>
> 
> You just move power-domain-cells up.
> 
> Best regards,
> Krzysztof
> 

Like this? 

aon_syscon: syscon@17010000 {
	compatible = "starfive,jh7110-aon-syscon", "syscon", "starfive,jh7110-aon-pmu";
	reg = <0x0 0x17010000 0x0 0x1000>;
	#power-domain-cells = <1>;
};

If right? I will tell the syscon patch's owner delete the "simple-mfd" in aon_syscon node.

Best regards,
Krzysztof

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Krzysztof Kozlowski]

On 04/05/2023 09:20, Changhuang Liang wrote:
> 
> 
> On 2023/5/4 15:04, Krzysztof Kozlowski wrote:
>> On 04/05/2023 08:53, Changhuang Liang wrote:
>>>>> 	};
>>>>> };
>>>>>
>>>>> Add a "regmap" property which is phandle. And it can keep the present child-node
>>>>> structure. This is more consistent with our soc design.
>>>>
>>>> Adding property from child to parent does not make any sense. Didn't you
>>>> already receive comment on this?
>>>>
>>>> Best regards,
>>>> Krzysztof
>>>>
>>>
>>> Krzysztof,
>>>
>>> I am confused about what to do next. How to add this power-controller's
>>> node in device tree?
>>>
>>
>> You just move power-domain-cells up.
>>
>> Best regards,
>> Krzysztof
>>
> 
> Like this? 
> 
> aon_syscon: syscon@17010000 {
> 	compatible = "starfive,jh7110-aon-syscon", "syscon", "starfive,jh7110-aon-pmu";
> 	reg = <0x0 0x17010000 0x0 0x1000>;
> 	#power-domain-cells = <1>;
> };
> 
> If right? I will tell the syscon patch's owner delete the "simple-mfd" in aon_syscon node.

Yes, but your compatibles are now wrong. Just compatible =
"starfive,jh7110-aon-syscon", "syscon".

Best regards,
Krzysztof


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Changhuang Liang]

On 2023/5/4 15:26, Krzysztof Kozlowski wrote:
> On 04/05/2023 09:20, Changhuang Liang wrote:
>>>>
>>>> Krzysztof,
>>>>
>>>> I am confused about what to do next. How to add this power-controller's
>>>> node in device tree?
>>>>
>>>
>>> You just move power-domain-cells up.
>>>
>>> Best regards,
>>> Krzysztof
>>>
>>
>> Like this? 
>>
>> aon_syscon: syscon@17010000 {
>> 	compatible = "starfive,jh7110-aon-syscon", "syscon", "starfive,jh7110-aon-pmu";
>> 	reg = <0x0 0x17010000 0x0 0x1000>;
>> 	#power-domain-cells = <1>;
>> };
>>
>> If right? I will tell the syscon patch's owner delete the "simple-mfd" in aon_syscon node.
> 
> Yes, but your compatibles are now wrong. Just compatible =
> "starfive,jh7110-aon-syscon", "syscon".
> 

If compatible = "starfive,jh7110-aon-syscon", "syscon". My pmu drivers need use 
"starfive,jh7110-aon-syscon" to match. And my pmu series will add this 
aon_syscon in yaml and device tree, so the syscon patch's owner don't need 
to add the aon_syscon in its yaml and device tree?

Best regards,
Changhuang

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Krzysztof Kozlowski]

On 04/05/2023 10:43, Changhuang Liang wrote:
> 
> 
> On 2023/5/4 15:26, Krzysztof Kozlowski wrote:
>> On 04/05/2023 09:20, Changhuang Liang wrote:
>>>>>
>>>>> Krzysztof,
>>>>>
>>>>> I am confused about what to do next. How to add this power-controller's
>>>>> node in device tree?
>>>>>
>>>>
>>>> You just move power-domain-cells up.
>>>>
>>>> Best regards,
>>>> Krzysztof
>>>>
>>>
>>> Like this? 
>>>
>>> aon_syscon: syscon@17010000 {
>>> 	compatible = "starfive,jh7110-aon-syscon", "syscon", "starfive,jh7110-aon-pmu";
>>> 	reg = <0x0 0x17010000 0x0 0x1000>;
>>> 	#power-domain-cells = <1>;
>>> };
>>>
>>> If right? I will tell the syscon patch's owner delete the "simple-mfd" in aon_syscon node.
>>
>> Yes, but your compatibles are now wrong. Just compatible =
>> "starfive,jh7110-aon-syscon", "syscon".
>>
> 
> If compatible = "starfive,jh7110-aon-syscon", "syscon". My pmu drivers need use 
> "starfive,jh7110-aon-syscon" to match.

And how it would even work with your proposal
"starfive,jh7110-aon-syscon", "syscon", "starfive,jh7110-aon-pmu"?

Try...

>  And my pmu series will add this 
> aon_syscon in yaml and device tree, so the syscon patch's owner don't need 
> to add the aon_syscon in its yaml and device tree?

I don't understand. But if you need to drop syscon, sure, drop it.

Best regards,
Krzysztof


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Changhuang Liang]

On 2023/5/4 17:36, Krzysztof Kozlowski wrote:
> On 04/05/2023 10:43, Changhuang Liang wrote:
>>
>>
>> On 2023/5/4 15:26, Krzysztof Kozlowski wrote:
>>> On 04/05/2023 09:20, Changhuang Liang wrote:
>>>>>>
>>>>>> Krzysztof,
>>>>>>
>>>>>> I am confused about what to do next. How to add this power-controller's
>>>>>> node in device tree?
>>>>>>
>>>>>
>>>>> You just move power-domain-cells up.
>>>>>
>>>>> Best regards,
>>>>> Krzysztof
>>>>>
>>>>
>>>> Like this? 
>>>>
>>>> aon_syscon: syscon@17010000 {
>>>> 	compatible = "starfive,jh7110-aon-syscon", "syscon", "starfive,jh7110-aon-pmu";
>>>> 	reg = <0x0 0x17010000 0x0 0x1000>;
>>>> 	#power-domain-cells = <1>;
>>>> };
>>>>
>>>> If right? I will tell the syscon patch's owner delete the "simple-mfd" in aon_syscon node.
>>>
>>> Yes, but your compatibles are now wrong. Just compatible =
>>> "starfive,jh7110-aon-syscon", "syscon".
>>>
>>
>> If compatible = "starfive,jh7110-aon-syscon", "syscon". My pmu drivers need use 
>> "starfive,jh7110-aon-syscon" to match.
> 
> And how it would even work with your proposal
> "starfive,jh7110-aon-syscon", "syscon", "starfive,jh7110-aon-pmu"?
> 
> Try...
> 
>>  And my pmu series will add this 
>> aon_syscon in yaml and device tree, so the syscon patch's owner don't need 
>> to add the aon_syscon in its yaml and device tree?
> 
> I don't understand. But if you need to drop syscon, sure, drop it.
> 

Yes, I think it can drop aon_syscon node in syscon patch series. And maybe my
compatible = "starfive,jh7110-aon-pmu", "syscon"; is better.

aon_syscon: syscon@17010000 {
	compatible = "starfive,jh7110-aon-pmu", "syscon";
	reg = <0x0 0x17010000 0x0 0x1000>;
	#power-domain-cells = <1>;
};

Best regards,
Krzysztof



-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 1841 bytes --]

On Thu, May 04, 2023 at 05:48:20PM +0800, Changhuang Liang wrote:
> On 2023/5/4 17:36, Krzysztof Kozlowski wrote:
> > On 04/05/2023 10:43, Changhuang Liang wrote:

> >> On 2023/5/4 15:26, Krzysztof Kozlowski wrote:

> >>>> aon_syscon: syscon@17010000 {
> >>>> 	compatible = "starfive,jh7110-aon-syscon", "syscon", "starfive,jh7110-aon-pmu";
> >>>> 	reg = <0x0 0x17010000 0x0 0x1000>;
> >>>> 	#power-domain-cells = <1>;
> >>>> };
> >>>>
> >>>> If right? I will tell the syscon patch's owner delete the "simple-mfd" in aon_syscon node.
> >>>
> >>> Yes, but your compatibles are now wrong. Just compatible =
> >>> "starfive,jh7110-aon-syscon", "syscon".
> >>>
> >>
> >> If compatible = "starfive,jh7110-aon-syscon", "syscon". My pmu drivers need use 
> >> "starfive,jh7110-aon-syscon" to match.
> > 
> > And how it would even work with your proposal
> > "starfive,jh7110-aon-syscon", "syscon", "starfive,jh7110-aon-pmu"?
> > 
> > Try...
> > 
> >>  And my pmu series will add this 
> >> aon_syscon in yaml and device tree, so the syscon patch's owner don't need 
> >> to add the aon_syscon in its yaml and device tree?
> > 
> > I don't understand. But if you need to drop syscon, sure, drop it.
> > 
> 
> Yes, I think it can drop aon_syscon node in syscon patch series. And maybe my
> compatible = "starfive,jh7110-aon-pmu", "syscon"; is better.
> 
> aon_syscon: syscon@17010000 {
> 	compatible = "starfive,jh7110-aon-pmu", "syscon";

I don't really understand why you actually need to have this compatible.
Why not keep "starfive,jh7110-aon-syscon" & register the PMU using a
software mechanism?

> 	reg = <0x0 0x17010000 0x0 0x1000>;
> 	#power-domain-cells = <1>;
> };
> 
> Best regards,
> Krzysztof

^^^^^^^^^^^^^^
btw, your mailer is doing something odd with quotation.

Cheers,
Conor.

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

[-- Attachment #2: Type: text/plain, Size: 112 bytes --]

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Changhuang Liang]

On 2023/5/4 17:57, Conor Dooley wrote:
> On Thu, May 04, 2023 at 05:48:20PM +0800, Changhuang Liang wrote:
>> On 2023/5/4 17:36, Krzysztof Kozlowski wrote:
>>> On 04/05/2023 10:43, Changhuang Liang wrote:
> 
>>>> On 2023/5/4 15:26, Krzysztof Kozlowski wrote:
>>>>
>>>> If compatible = "starfive,jh7110-aon-syscon", "syscon". My pmu drivers need use 
>>>> "starfive,jh7110-aon-syscon" to match.
>>>
>>> And how it would even work with your proposal
>>> "starfive,jh7110-aon-syscon", "syscon", "starfive,jh7110-aon-pmu"?
>>>
>>> Try...
>>>
>>>>  And my pmu series will add this 
>>>> aon_syscon in yaml and device tree, so the syscon patch's owner don't need 
>>>> to add the aon_syscon in its yaml and device tree?
>>>
>>> I don't understand. But if you need to drop syscon, sure, drop it.
>>>
>>
>> Yes, I think it can drop aon_syscon node in syscon patch series. And maybe my
>> compatible = "starfive,jh7110-aon-pmu", "syscon"; is better.
>>
>> aon_syscon: syscon@17010000 {
>> 	compatible = "starfive,jh7110-aon-pmu", "syscon";
> 
> I don't really understand why you actually need to have this compatible.
> Why not keep "starfive,jh7110-aon-syscon" & register the PMU using a
> software mechanism?
> 

But if keep this "starfive,jh7110-aon-syscon" compatible. Which .yaml match to
it? Use this series dt-bindings or syscon series dt-bindings.

>> 	reg = <0x0 0x17010000 0x0 0x1000>;
>> 	#power-domain-cells = <1>;
>> };
>>
>> Best regards,
>> Krzysztof
> 
> ^^^^^^^^^^^^^^
> btw, your mailer is doing something odd with quotation.
> 

OK, will pay attention to it.

> Cheers,
> Conor.

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 586 bytes --]

On Fri, May 05, 2023 at 09:29:15AM +0800, Changhuang Liang wrote:

> But if keep this "starfive,jh7110-aon-syscon" compatible. Which .yaml match to
> it? Use this series dt-bindings or syscon series dt-bindings.

There is no syscon series anymore, it's part of the PLL series now:
https://lore.kernel.org/linux-clk/20230414024157.53203-1-xingyu.wu@starfivetech.com/

I don't really care what you, Walker & Xingyu decide to do, but add the
binding in one series in a complete form. It's far less confusing to
have only have one version of the binding on the go at once.

Thanks,
Conor.


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

[-- Attachment #2: Type: text/plain, Size: 112 bytes --]

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Changhuang Liang]

On 2023/5/5 20:38, Conor Dooley wrote:
> On Fri, May 05, 2023 at 09:29:15AM +0800, Changhuang Liang wrote:
> 
>> But if keep this "starfive,jh7110-aon-syscon" compatible. Which .yaml match to
>> it? Use this series dt-bindings or syscon series dt-bindings.
> 
> There is no syscon series anymore, it's part of the PLL series now:
> https://lore.kernel.org/linux-clk/20230414024157.53203-1-xingyu.wu@starfivetech.com/
> 
> I don't really care what you, Walker & Xingyu decide to do, but add the
> binding in one series in a complete form. It's far less confusing to
> have only have one version of the binding on the go at once.
> 

Hi, Krzysztof and Conor

Due to the current aon pmu needs to be adjusted, it affects the syscon in PLL series.
So It's inevitable to change syscon in PLL series.

My current idea is PLL series don't add the aon_syscon node. I will add it in my
aon pmu series in next version like this:

aon_syscon: syscon@17010000 {
	compatible = "starfive,jh7110-aon-pmu", "syscon";
	reg = <0x0 0x17010000 0x0 0x1000>;
	#power-domain-cells = <1>;
};

In my opinion, the first we add "starfive,jh7110-aon-syscon" because "syscon" can 
not appear alone in the compatible. If we have "starfive,jh7110-aon-pmu", this
"starfive,jh7110-aon-syscon" is not a must-be need.

Do you agree with doing so.

Thanks,
Changhuang

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Krzysztof Kozlowski]

On 06/05/2023 03:45, Changhuang Liang wrote:
> 
> 
> On 2023/5/5 20:38, Conor Dooley wrote:
>> On Fri, May 05, 2023 at 09:29:15AM +0800, Changhuang Liang wrote:
>>
>>> But if keep this "starfive,jh7110-aon-syscon" compatible. Which .yaml match to
>>> it? Use this series dt-bindings or syscon series dt-bindings.
>>
>> There is no syscon series anymore, it's part of the PLL series now:
>> https://lore.kernel.org/linux-clk/20230414024157.53203-1-xingyu.wu@starfivetech.com/
>>
>> I don't really care what you, Walker & Xingyu decide to do, but add the
>> binding in one series in a complete form. It's far less confusing to
>> have only have one version of the binding on the go at once.
>>
> 
> Hi, Krzysztof and Conor
> 
> Due to the current aon pmu needs to be adjusted, it affects the syscon in PLL series.
> So It's inevitable to change syscon in PLL series.
> 
> My current idea is PLL series don't add the aon_syscon node. I will add it in my
> aon pmu series in next version like this:
> 
> aon_syscon: syscon@17010000 {
> 	compatible = "starfive,jh7110-aon-pmu", "syscon";
> 	reg = <0x0 0x17010000 0x0 0x1000>;
> 	#power-domain-cells = <1>;
> };
> 
> In my opinion, the first we add "starfive,jh7110-aon-syscon" because "syscon" can 
> not appear alone in the compatible. If we have "starfive,jh7110-aon-pmu", this
> "starfive,jh7110-aon-syscon" is not a must-be need.
> 
> Do you agree with doing so.

Sorry guys, I don't know what you talk about. I have no clue what are
PLL and aon series. More over I don't understand what is complicated
here... all SoCs follow the same rules and similar way of development.

Best regards,
Krzysztof


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Changhuang Liang]

On 2023/5/6 14:31, Krzysztof Kozlowski wrote:
> On 06/05/2023 03:45, Changhuang Liang wrote:
>>
>> Hi, Krzysztof and Conor
>>
>> Due to the current aon pmu needs to be adjusted, it affects the syscon in PLL series.
>> So It's inevitable to change syscon in PLL series.
>>
>> My current idea is PLL series don't add the aon_syscon node. I will add it in my
>> aon pmu series in next version like this:
>>
>> aon_syscon: syscon@17010000 {
>> 	compatible = "starfive,jh7110-aon-pmu", "syscon";
>> 	reg = <0x0 0x17010000 0x0 0x1000>;
>> 	#power-domain-cells = <1>;
>> };
>>
>> In my opinion, the first we add "starfive,jh7110-aon-syscon" because "syscon" can 
>> not appear alone in the compatible. If we have "starfive,jh7110-aon-pmu", this
>> "starfive,jh7110-aon-syscon" is not a must-be need.
>>
>> Do you agree with doing so.
> 
> Sorry guys, I don't know what you talk about. I have no clue what are
> PLL and aon series. More over I don't understand what is complicated
> here... all SoCs follow the same rules and similar way of development.
> 

In other words, if I use the above approach, 
[1] https://lore.kernel.org/all/20230414024157.53203-6-xingyu.wu@starfivetech.com/
[2] https://lore.kernel.org/all/20230414024157.53203-7-xingyu.wu@starfivetech.com/
Links [1][2] need to be dropped "aon_syscon" node.

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 1736 bytes --]

On Sat, May 06, 2023 at 09:45:07AM +0800, Changhuang Liang wrote:
> 
> 
> On 2023/5/5 20:38, Conor Dooley wrote:
> > On Fri, May 05, 2023 at 09:29:15AM +0800, Changhuang Liang wrote:
> > 
> >> But if keep this "starfive,jh7110-aon-syscon" compatible. Which .yaml match to
> >> it? Use this series dt-bindings or syscon series dt-bindings.
> > 
> > There is no syscon series anymore, it's part of the PLL series now:
> > https://lore.kernel.org/linux-clk/20230414024157.53203-1-xingyu.wu@starfivetech.com/
> > 
> > I don't really care what you, Walker & Xingyu decide to do, but add the
> > binding in one series in a complete form. It's far less confusing to
> > have only have one version of the binding on the go at once.
> > 
> 
> Due to the current aon pmu needs to be adjusted, it affects the syscon in PLL series.
> So It's inevitable to change syscon in PLL series.
> 
> My current idea is PLL series don't add the aon_syscon node. I will add it in my
> aon pmu series in next version

That's fine. Rob was happy with the clock related parts, which was the
original source of confusion there.

> like this:
> 
> aon_syscon: syscon@17010000 {
> 	compatible = "starfive,jh7110-aon-pmu", "syscon";

The syscon does a bunch of things of which one is a pmu. I don't see a
reason to name this other than "starfive,jh100-aon-syscon".

> 	reg = <0x0 0x17010000 0x0 0x1000>;
> 	#power-domain-cells = <1>;
> };
> 
> In my opinion, the first we add "starfive,jh7110-aon-syscon" because "syscon" can 
> not appear alone in the compatible. If we have "starfive,jh7110-aon-pmu", this
> "starfive,jh7110-aon-syscon" is not a must-be need.
> 
> Do you agree with doing so.
> 
> Thanks,
> Changhuang

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

[-- Attachment #2: Type: text/plain, Size: 112 bytes --]

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Changhuang Liang]

On 2023/5/6 18:17, Conor Dooley wrote:
> On Sat, May 06, 2023 at 09:45:07AM +0800, Changhuang Liang wrote:
>>
>>
>> On 2023/5/5 20:38, Conor Dooley wrote:
>>> On Fri, May 05, 2023 at 09:29:15AM +0800, Changhuang Liang wrote:
>>>
>>>> But if keep this "starfive,jh7110-aon-syscon" compatible. Which .yaml match to
>>>> it? Use this series dt-bindings or syscon series dt-bindings.
>>>
>>> There is no syscon series anymore, it's part of the PLL series now:
>>> https://lore.kernel.org/linux-clk/20230414024157.53203-1-xingyu.wu@starfivetech.com/
>>>
>>> I don't really care what you, Walker & Xingyu decide to do, but add the
>>> binding in one series in a complete form. It's far less confusing to
>>> have only have one version of the binding on the go at once.
>>>
>>
>> Due to the current aon pmu needs to be adjusted, it affects the syscon in PLL series.
>> So It's inevitable to change syscon in PLL series.
>>
>> My current idea is PLL series don't add the aon_syscon node. I will add it in my
>> aon pmu series in next version
> 
> That's fine. Rob was happy with the clock related parts, which was the
> original source of confusion there.
> 
>> like this:
>>
>> aon_syscon: syscon@17010000 {
>> 	compatible = "starfive,jh7110-aon-pmu", "syscon";
> 
> The syscon does a bunch of things of which one is a pmu. I don't see a
> reason to name this other than "starfive,jh100-aon-syscon".
> 

OK, will replace "starfive,jh7110-aon-pmu" with "starfive,jh100-aon-syscon" in this series.

Thanks,
Changhuang



-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 292 bytes --]

On Sat, May 06, 2023 at 08:26:01PM +0800, Changhuang Liang wrote:

> OK, will replace "starfive,jh7110-aon-pmu" with "starfive,jh100-aon-syscon" in this series.
                                                            ^^^^^
Just make sure you don't propagate my typo here in the process.


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

[-- Attachment #2: Type: text/plain, Size: 112 bytes --]

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU support

[Changhuang Liang]

On 2023/5/6 20:29, Conor Dooley wrote:
> On Sat, May 06, 2023 at 08:26:01PM +0800, Changhuang Liang wrote:
> 
>> OK, will replace "starfive,jh7110-aon-pmu" with "starfive,jh100-aon-syscon" in this series.
>                                                             ^^^^^
> Just make sure you don't propagate my typo here in the process.
> 

Yes, "starfive,jh7110-aon-syscon"

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy