Messages in this thread | | | Date | Mon, 24 Apr 2023 17:52:23 +0100 | From | Conor Dooley <> | Subject | Re: [RESEND v2 1/6] dt-bindings: power: Add JH7110 AON PMU 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 > >> [unhandled content-type:application/pgp-signature] | |