Re: [PATCH v2 3/3] arm64: dts: rockchip: fix the pcie refclock oscillator on Rock 5 ITX

[Dragan Simic <dsimic@xxxxxxxxxxx>]

Hello Anand,

On 2024-07-18 09:26, Anand Moon wrote:
> On Mon, 15 Jul 2024 at 16:35, Heiko Stuebner <heiko@xxxxxxxxx> wrote:
> > The Rock 5 ITX uses two PCIe controllers to drive both a M.2 slot and 
> > its
> > SATA controller with 2 lanes each. The supply for the refclk 
> > oscillator is
> > the same that supplies the M.2 slot, but the SATA controller port is
> > supplied by a different rail.
> >
> > This leads to the effect that if the PCIe30x4 controller for the M.2
> > probes first, everything works normally. But if the PCIe30x2 
> > controller
> > that is connected to the SATA controller probes first, it will hang on
> > the first DBI read as nothing will have enabled the refclock before.
>
> I just checked the rk3588-rock-5-itx.dts in the linux-next.
> You have not enabled sata0 and sata2, which might be the problem
> for the SATA controller not getting initialized.

Rock 5 ITX doesn't use RK5588's built-in SATA interfaces, so that's 
fine.
Please have a look at the board schematic, it uses a separate PCI 
Express
SATA controller for its four SATA ports.

> > Fix this by describing the clock generator with its supplies so that
> > both controllers can reference it as needed.
> >
> > Signed-off-by: Heiko Stuebner <heiko@xxxxxxxxx>
> > ---
> >  .../boot/dts/rockchip/rk3588-rock-5-itx.dts   | 38 
> > ++++++++++++++++++-
> >  1 file changed, 36 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts 
> > b/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts
> > index d0b922b8d67e8..37bc53f2796fc 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5-itx.dts
> > @@ -72,6 +72,15 @@ hdd-led2 {
> >                 };
> >         };
> >
> > +       /* Unnamed voltage oscillator: 100MHz,3.3V,3225 */
> > +       pcie30_port0_refclk: pcie30_port1_refclk: 
> > pcie-voltage-oscillator {
> > +               compatible = "voltage-oscillator";
> > +               #clock-cells = <0>;
> > +               clock-frequency = <100000000>;
> > +               clock-output-names = "pcie30_refclk";
> > +               vdd-supply = <&vcc3v3_pi6c_05>;
> > +       };
> > +
> >         fan0: pwm-fan {
> >                 compatible = "pwm-fan";
> >                 #cooling-cells = <2>;
> > @@ -146,13 +155,14 @@ vcc3v3_lan: vcc3v3_lan_phy2: 
> > regulator-vcc3v3-lan {
> >                 vin-supply = <&vcc_3v3_s3>;
> >         };
> >
> > -       vcc3v3_mkey: regulator-vcc3v3-mkey {
> > +       /* The PCIE30x4_PWREN_H controls two regulators */
> > +       vcc3v3_mkey: vcc3v3_pi6c_05: regulator-vcc3v3-pi6c-05 {
> >                 compatible = "regulator-fixed";
> >                 enable-active-high;
> >                 gpios = <&gpio1 RK_PA4 GPIO_ACTIVE_HIGH>;
> >                 pinctrl-names = "default";
> >                 pinctrl-0 = <&pcie30x4_pwren_h>;
> > -               regulator-name = "vcc3v3_mkey";
> > +               regulator-name = "vcc3v3_pi6c_05";
> >                 regulator-min-microvolt = <3300000>;
> >                 regulator-max-microvolt = <3300000>;
> >                 startup-delay-us = <5000>;
> > @@ -513,6 +523,18 @@ &pcie30phy {
> >
> >  /* ASMedia ASM1164 Sata controller */
> >  &pcie3x2 {
> > +       /*
> > +        * The board has a "pcie_refclk" oscillator that needs 
> > enabling,
> > +        * so add it to the list of clocks.
> > +        */
> > +       clocks = <&cru ACLK_PCIE_2L_MSTR>, <&cru ACLK_PCIE_2L_SLV>,
> > +                <&cru ACLK_PCIE_2L_DBI>, <&cru PCLK_PCIE_2L>,
> > +                <&cru CLK_PCIE_AUX1>, <&cru CLK_PCIE2L_PIPE>,
> > +                <&pcie30_port1_refclk>;
> > +       clock-names = "aclk_mst", "aclk_slv",
> > +                     "aclk_dbi", "pclk",
> > +                     "aux", "pipe",
> > +                     "ref";
> >         pinctrl-names = "default";
> >         pinctrl-0 = <&pcie30x2_perstn_m1_l>;
> >         reset-gpios = <&gpio4 RK_PB0 GPIO_ACTIVE_HIGH>;
> > @@ -522,6 +544,18 @@ &pcie3x2 {
> >
> >  /* M.2 M.key */
> >  &pcie3x4 {
> > +       /*
> > +        * The board has a "pcie_refclk" oscillator that needs 
> > enabling,
> > +        * so add it to the list of clocks.
> > +        */
> > +       clocks = <&cru ACLK_PCIE_4L_MSTR>, <&cru ACLK_PCIE_4L_SLV>,
> > +                <&cru ACLK_PCIE_4L_DBI>, <&cru PCLK_PCIE_4L>,
> > +                <&cru CLK_PCIE_AUX0>, <&cru CLK_PCIE4L_PIPE>,
> > +                <&pcie30_port0_refclk>;
> > +       clock-names = "aclk_mst", "aclk_slv",
> > +                     "aclk_dbi", "pclk",
> > +                     "aux", "pipe",
> > +                     "ref";
> >         num-lanes = <2>;
> >         pinctrl-names = "default";
> >         pinctrl-0 = <&pcie30x4_perstn_m1_l>;
> > --
> > 2.39.2