| Message ID | 20230419035646.43702-2-changhuang.liang@starfivetech.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Conor Dooley |
| Headers | show |
| Series | [RESEND,v2,1/6] dt-bindings: power: Add JH7110 AON PMU support | expand |
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 >
On 19/04/2023 05:56, 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. > > 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 +++ I have impression I just reviewed your v2... Apply the comments, reviews/acks etc from that email. Best regards, Krzysztof
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 >>
On 2023/4/20 3:46, Krzysztof Kozlowski wrote: > On 19/04/2023 05:56, 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. >> >> 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 +++ > > I have impression I just reviewed your v2... Apply the comments, > reviews/acks etc from that email. > > Best regards, > Krzysztof > Yes, thanks for you support!
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 > >>
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: >>>>
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.
>>>>>> >>>>>> 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.
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
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.
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.
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?
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.
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/
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
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
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
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
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
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
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
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
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
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.
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.
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.
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
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
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.
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
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
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.
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"
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
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. 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(-)