[PATCH 0/6] rockchip: rk3328 fixes in preparation for usb3-phy

[PATCH 0/6] rockchip: rk3328 fixes in preparation for usb3-phy

[Peter Geis]

This is a series of fixes I uncovered during my work on the next
generation rk3328 usb3 phy driver.

The first patch fixes the error handling of the pm-domain driver. I
don't expect this to break anything, but it is entirely possible some
driver code makes some bad assumptions on the fact that this has been
broken from the very beginning.

The second patch fixes the ref_usb3otg clock parent. This was preventing
correct reclocking of the usb3 phy.

The third patch fixes the ethernet alias that was accidentlly readded
during the rk3328-roc dtsi conversion.

The fourth patch fixes a race condition between power domains and clocks
being shut off during boot, which would cause an ugly splat on rk3328
during boot on recent kernels.

The fifth patch corrects the rk3328-roc fixed regulators and power input
map. It also cleans up the fixed regulator flags to be consistent.

The sixth patch removes address aligned beats and the redundant rxpbl
and txpbl flags from the rk3328-roc, which are unnecessary now.

Please examine and test these as necessary, especially the pm-domain fix
patch.

Very Respectfully,
Peter Geis


Peter Geis (6):
  pmdomain: rockchip: fix rockchip_pd_power error handling
  clk: rockchip: fix wrong clk_ref_usb3otg parent for rk3328
  arm64: dts: rockchip: remove ethernet alias from rk3328-roc
  arm64: dts: rockchip: add hevc power domain clock to rk3328
  arm64: dts: rockchip: correct rk3328-roc regulator map
  arm64: dts: rockchip: Remove address aligned beats from rk3328-roc

 arch/arm64/boot/dts/rockchip/rk3328-roc.dtsi | 27 +++++++++++---------
 arch/arm64/boot/dts/rockchip/rk3328.dtsi     |  1 +
 drivers/clk/rockchip/clk-rk3328.c            |  2 +-
 drivers/pmdomain/rockchip/pm-domains.c       |  8 ++++--
 4 files changed, 23 insertions(+), 15 deletions(-)

-- 
2.39.5

[PATCH 2/6] clk: rockchip: fix wrong clk_ref_usb3otg parent for rk3328

[Peter Geis]

Correct the clk_ref_usb3otg parent to fix clock control for the usb3
controller on rk3328. Verified against the rk3328 trm and usb3 clock tree
documentation.

Fixes: fe3511ad8a1c ("clk: rockchip: add clock controller for rk3328")
Signed-off-by: Peter Geis <pgwipeout@gmail.com>
---

 drivers/clk/rockchip/clk-rk3328.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/rockchip/clk-rk3328.c b/drivers/clk/rockchip/clk-rk3328.c
index 3bb87b27b662..cf60fcf2fa5c 100644
--- a/drivers/clk/rockchip/clk-rk3328.c
+++ b/drivers/clk/rockchip/clk-rk3328.c
@@ -201,7 +201,7 @@ PNAME(mux_aclk_peri_pre_p)	= { "cpll_peri",
 				    "gpll_peri",
 				    "hdmiphy_peri" };
 PNAME(mux_ref_usb3otg_src_p)	= { "xin24m",
-				    "clk_usb3otg_ref" };
+				    "clk_ref_usb3otg_src" };
 PNAME(mux_xin24m_32k_p)		= { "xin24m",
 				    "clk_rtc32k" };
 PNAME(mux_mac2io_src_p)		= { "clk_mac2io_src",
-- 
2.39.5

Re: [PATCH 2/6] clk: rockchip: fix wrong clk_ref_usb3otg parent for rk3328

[Dragan Simic]

Hello Peter,

On 2024-12-10 02:30, Peter Geis wrote:
> Correct the clk_ref_usb3otg parent to fix clock control for the usb3
> controller on rk3328. Verified against the rk3328 trm and usb3 clock 
> tree
> documentation.
> 
> Fixes: fe3511ad8a1c ("clk: rockchip: add clock controller for rk3328")
> Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> ---
> 
>  drivers/clk/rockchip/clk-rk3328.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/rockchip/clk-rk3328.c
> b/drivers/clk/rockchip/clk-rk3328.c
> index 3bb87b27b662..cf60fcf2fa5c 100644
> --- a/drivers/clk/rockchip/clk-rk3328.c
> +++ b/drivers/clk/rockchip/clk-rk3328.c
> @@ -201,7 +201,7 @@ PNAME(mux_aclk_peri_pre_p)	= { "cpll_peri",
>  				    "gpll_peri",
>  				    "hdmiphy_peri" };
>  PNAME(mux_ref_usb3otg_src_p)	= { "xin24m",
> -				    "clk_usb3otg_ref" };
> +				    "clk_ref_usb3otg_src" };
>  PNAME(mux_xin24m_32k_p)		= { "xin24m",
>  				    "clk_rtc32k" };
>  PNAME(mux_mac2io_src_p)		= { "clk_mac2io_src",

Sorry, but I was unable to verify this in the part 1 of the
RK3328 TRM, in both versions 1.1 and 1.2, which is all I have
when it comes to the RK3328 TRM.  Is that maybe described in
the part 2, which I've been unable to locate for years?

Moreover, the downstream kernel source from Rockchip does it
the way [1] it's currently done in the mainline kernel, which
makes me confused a bit?  Could you, please, provide more
details about the two references you mentioned in the patch
description, or maybe even you could provide the links to
those two references?

[1] 
https://raw.githubusercontent.com/rockchip-linux/kernel/refs/heads/develop-4.4/drivers/clk/rockchip/clk-rk3328.c

Re: [PATCH 2/6] clk: rockchip: fix wrong clk_ref_usb3otg parent for rk3328

[Peter Geis]

On Tue, Dec 10, 2024 at 4:44 AM Dragan Simic <dsimic@manjaro.org> wrote:
>
> Hello Peter,
>
> On 2024-12-10 02:30, Peter Geis wrote:
> > Correct the clk_ref_usb3otg parent to fix clock control for the usb3
> > controller on rk3328. Verified against the rk3328 trm and usb3 clock
> > tree
> > documentation.
> >
> > Fixes: fe3511ad8a1c ("clk: rockchip: add clock controller for rk3328")
> > Signed-off-by: Peter Geis <pgwipeout@gmail.com>
> > ---
> >
> >  drivers/clk/rockchip/clk-rk3328.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/rockchip/clk-rk3328.c
> > b/drivers/clk/rockchip/clk-rk3328.c
> > index 3bb87b27b662..cf60fcf2fa5c 100644
> > --- a/drivers/clk/rockchip/clk-rk3328.c
> > +++ b/drivers/clk/rockchip/clk-rk3328.c
> > @@ -201,7 +201,7 @@ PNAME(mux_aclk_peri_pre_p)        = { "cpll_peri",
> >                                   "gpll_peri",
> >                                   "hdmiphy_peri" };
> >  PNAME(mux_ref_usb3otg_src_p) = { "xin24m",
> > -                                 "clk_usb3otg_ref" };
> > +                                 "clk_ref_usb3otg_src" };
> >  PNAME(mux_xin24m_32k_p)              = { "xin24m",
> >                                   "clk_rtc32k" };
> >  PNAME(mux_mac2io_src_p)              = { "clk_mac2io_src",
>
> Sorry, but I was unable to verify this in the part 1 of the
> RK3328 TRM, in both versions 1.1 and 1.2, which is all I have
> when it comes to the RK3328 TRM.  Is that maybe described in
> the part 2, which I've been unable to locate for years?
>
> Moreover, the downstream kernel source from Rockchip does it
> the way [1] it's currently done in the mainline kernel, which
> makes me confused a bit?  Could you, please, provide more
> details about the two references you mentioned in the patch
> description, or maybe even you could provide the links to
> those two references?
>
> [1]
> https://raw.githubusercontent.com/rockchip-linux/kernel/refs/heads/develop-4.4/drivers/clk/rockchip/clk-rk3328.c

It is unfortunate the TRM doesn't include the clock maps, because they
are extremely helpful when one can acquire them. It also doesn't help
that the TRM register definition only referred to this as "pll". I was
sent specifically the usb3 phy clock map for my work on the driver,
which had the location of each switch and divider along with the
register and bit that controlled it. That combined with the TRM
register map allowed me to find this error.

Thanks!
Peter

Re: [PATCH 2/6] clk: rockchip: fix wrong clk_ref_usb3otg parent for rk3328

[Dragan Simic]

Hello Peter,

On 2024-12-10 14:27, Peter Geis wrote:
> On Tue, Dec 10, 2024 at 4:44 AM Dragan Simic <dsimic@manjaro.org> 
> wrote:
>> On 2024-12-10 02:30, Peter Geis wrote:
>> > Correct the clk_ref_usb3otg parent to fix clock control for the usb3
>> > controller on rk3328. Verified against the rk3328 trm and usb3 clock
>> > tree
>> > documentation.
>> >
>> > Fixes: fe3511ad8a1c ("clk: rockchip: add clock controller for rk3328")
>> > Signed-off-by: Peter Geis <pgwipeout@gmail.com>
>> > ---
>> >
>> >  drivers/clk/rockchip/clk-rk3328.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/clk/rockchip/clk-rk3328.c
>> > b/drivers/clk/rockchip/clk-rk3328.c
>> > index 3bb87b27b662..cf60fcf2fa5c 100644
>> > --- a/drivers/clk/rockchip/clk-rk3328.c
>> > +++ b/drivers/clk/rockchip/clk-rk3328.c
>> > @@ -201,7 +201,7 @@ PNAME(mux_aclk_peri_pre_p)        = { "cpll_peri",
>> >                                   "gpll_peri",
>> >                                   "hdmiphy_peri" };
>> >  PNAME(mux_ref_usb3otg_src_p) = { "xin24m",
>> > -                                 "clk_usb3otg_ref" };
>> > +                                 "clk_ref_usb3otg_src" };
>> >  PNAME(mux_xin24m_32k_p)              = { "xin24m",
>> >                                   "clk_rtc32k" };
>> >  PNAME(mux_mac2io_src_p)              = { "clk_mac2io_src",
>> 
>> Sorry, but I was unable to verify this in the part 1 of the
>> RK3328 TRM, in both versions 1.1 and 1.2, which is all I have
>> when it comes to the RK3328 TRM.  Is that maybe described in
>> the part 2, which I've been unable to locate for years?
>> 
>> Moreover, the downstream kernel source from Rockchip does it
>> the way [1] it's currently done in the mainline kernel, which
>> makes me confused a bit?  Could you, please, provide more
>> details about the two references you mentioned in the patch
>> description, or maybe even you could provide the links to
>> those two references?
>> 
>> [1] 
>> https://raw.githubusercontent.com/rockchip-linux/kernel/refs/heads/develop-4.4/drivers/clk/rockchip/clk-rk3328.c
> 
> It is unfortunate the TRM doesn't include the clock maps, because they
> are extremely helpful when one can acquire them. It also doesn't help
> that the TRM register definition only referred to this as "pll". I was
> sent specifically the usb3 phy clock map for my work on the driver,
> which had the location of each switch and divider along with the
> register and bit that controlled it. That combined with the TRM
> register map allowed me to find this error.

I see, thanks for the clarification.  I'd assume that you aren't allowed
to share the additional documentation you've got, which is unfortunate,
but it is what it is.  We should be happy that at least you got it, and
were able to put it into good use.

Thanks for fixing this!

Re: [PATCH 2/6] clk: rockchip: fix wrong clk_ref_usb3otg parent for rk3328

[Jonas Karlman]

On 2024-12-10 14:27, Peter Geis wrote:
> On Tue, Dec 10, 2024 at 4:44 AM Dragan Simic <dsimic@manjaro.org> wrote:
>>
>> Hello Peter,
>>
>> On 2024-12-10 02:30, Peter Geis wrote:
>>> Correct the clk_ref_usb3otg parent to fix clock control for the usb3
>>> controller on rk3328. Verified against the rk3328 trm and usb3 clock
>>> tree
>>> documentation.
>>>
>>> Fixes: fe3511ad8a1c ("clk: rockchip: add clock controller for rk3328")
>>> Signed-off-by: Peter Geis <pgwipeout@gmail.com>
>>> ---
>>>
>>>  drivers/clk/rockchip/clk-rk3328.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/clk/rockchip/clk-rk3328.c
>>> b/drivers/clk/rockchip/clk-rk3328.c
>>> index 3bb87b27b662..cf60fcf2fa5c 100644
>>> --- a/drivers/clk/rockchip/clk-rk3328.c
>>> +++ b/drivers/clk/rockchip/clk-rk3328.c
>>> @@ -201,7 +201,7 @@ PNAME(mux_aclk_peri_pre_p)        = { "cpll_peri",
>>>                                   "gpll_peri",
>>>                                   "hdmiphy_peri" };
>>>  PNAME(mux_ref_usb3otg_src_p) = { "xin24m",
>>> -                                 "clk_usb3otg_ref" };
>>> +                                 "clk_ref_usb3otg_src" };
>>>  PNAME(mux_xin24m_32k_p)              = { "xin24m",
>>>                                   "clk_rtc32k" };
>>>  PNAME(mux_mac2io_src_p)              = { "clk_mac2io_src",
>>
>> Sorry, but I was unable to verify this in the part 1 of the
>> RK3328 TRM, in both versions 1.1 and 1.2, which is all I have
>> when it comes to the RK3328 TRM.  Is that maybe described in
>> the part 2, which I've been unable to locate for years?
>>
>> Moreover, the downstream kernel source from Rockchip does it
>> the way [1] it's currently done in the mainline kernel, which
>> makes me confused a bit?  Could you, please, provide more
>> details about the two references you mentioned in the patch
>> description, or maybe even you could provide the links to
>> those two references?
>>
>> [1]
>> https://raw.githubusercontent.com/rockchip-linux/kernel/refs/heads/develop-4.4/drivers/clk/rockchip/clk-rk3328.c
> 
> It is unfortunate the TRM doesn't include the clock maps, because they
> are extremely helpful when one can acquire them. It also doesn't help
> that the TRM register definition only referred to this as "pll". I was
> sent specifically the usb3 phy clock map for my work on the driver,
> which had the location of each switch and divider along with the
> register and bit that controlled it. That combined with the TRM
> register map allowed me to find this error.

I can also confirm that the changes in this patch matches Fig. 3-8
RK3228H Clock Architecture Diagram 7 for the USB3OTG block.

                                    XIN24M -\
                                             S45_8 - ref_clk_usb3otg
S45_7 (2PLL) / G4_9 / S45_0 (DivFree 1~64) -/

Regards,
Jonas

> 
> Thanks!
> Peter
>