Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <CANAwSgSf=JZ_JiK1Gh5EYR30KQ5Yh-w3GsxzcFVweKHPi7KuLg@mail.gmail.com>
 <20220930152531.5910-1-amadeus@jmu.edu.cn>
In-Reply-To: <20220930152531.5910-1-amadeus@jmu.edu.cn>
From:   Anand Moon <linux.amoon@gmail.com>
Date:   Sun, 2 Oct 2022 21:16:52 +0530
Message-ID: <CANAwSgQp-kJxC2iP-abQuzGwDyz-NE8pLBT-3eE9F48t6JgaFg@mail.gmail.com>
Subject: Re: [PATCH 0/1] Re: [PATCH-next v1] arm64: dts: rockchip: Enable NVM
 Express PCIe controller on rock3a
To:     Chukun Pan <amadeus@jmu.edu.cn>
Cc:     heiko@sntech.de, robh+dt@kernel.org, michael.riesch@wolfvision.net,
        krzysztof.kozlowski+dt@linaro.org, linux-kernel@vger.kernel.org,
        linux-rockchip@lists.infradead.org,
        linux-arm-kernel@lists.infradead.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Chukun,

On Fri, 30 Sept 2022 at 20:55, Chukun Pan <amadeus@jmu.edu.cn> wrote:
>
> Hi Anand,
>
> > No, It's not working on my board.
> > If you check the driver code it requires *data-line* and *num-lanes
> > from the dts.
>
> I have successfully tested NVME hard drives on my board. See
> below for the patch, looking forward to your testing.
>
> root@rock3a:~# lspci
> 0000:00:00.0 PCI bridge: Rockchip Electronics Co., Ltd \
>              RK3568 Remote Signal Processor (rev 01)
> 0000:01:00.0 Network controller: MEDIATEK Corp. MT7921 \
>              802.11ax PCI Express Wireless Network Adapter
> 0002:00:00.0 PCI bridge: Rockchip Electronics Co., Ltd \
>              RK3568 Remote Signal Processor (rev 01)
> 0002:01:00.0 Non-Volatile memory controller: Sandisk Corp \
>              PC SN520 NVMe SSD (rev 01)
>
> Patch:
>

Thanks this patch works for me.

I would prefer the regulators would get enabled/disabled by the core drivers
like it's done in rk3399 PCIe drivers.

$ lspci
0000:00:00.0 PCI bridge: Rockchip Electronics Co., Ltd RK3568 Remote
Signal Processor (rev 01)
0000:01:00.0 Network controller: Intel Corporation Wireless 8265 / 8275 (rev 78)
0002:00:00.0 PCI bridge: Rockchip Electronics Co., Ltd RK3568 Remote
Signal Processor (rev 01)
0002:01:00.0 Non-Volatile memory controller: Micron/Crucial Technology
P2 NVMe PCIe SSD (rev 01)

> --- a/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-rock-3a.dts
> @@ -67,6 +67,37 @@ vcc12v_dcin: vcc12v-dcin-regulator {
>                 regulator-boot-on;
>         };
>
> +       pcie30_avdd0v9: pcie30-avdd0v9-regulator {
> +               compatible = "regulator-fixed";
> +               regulator-name = "pcie30_avdd0v9";
> +               regulator-always-on;
> +               regulator-boot-on;
> +               regulator-min-microvolt = <900000>;
> +               regulator-max-microvolt = <900000>;
> +               vin-supply = <&vcc3v3_sys>;
> +       };
> +
> +       pcie30_avdd1v8: pcie30-avdd1v8-regulator {
> +               compatible = "regulator-fixed";
> +               regulator-name = "pcie30_avdd1v8";
> +               regulator-always-on;
> +               regulator-boot-on;
> +               regulator-min-microvolt = <1800000>;
> +               regulator-max-microvolt = <1800000>;
> +               vin-supply = <&vcc3v3_sys>;
> +       };
> +
> +       /* pi6c pcie clock generator */
> +       vcc3v3_pi6c_03: vcc3v3-pi6c-03-regulator {
> +               compatible = "regulator-fixed";
> +               regulator-name = "vcc3v3_pi6c_03";
> +               regulator-always-on;
> +               regulator-boot-on;
> +               regulator-min-microvolt = <3300000>;
> +               regulator-max-microvolt = <3300000>;
> +               vin-supply = <&vcc5v0_sys>;
> +       };
> +
>         vcc3v3_pcie: vcc3v3-pcie-regulator {
>                 compatible = "regulator-fixed";
>                 enable-active-high;
> @@ -546,6 +577,19 @@ &pcie2x1 {
>         status = "okay";
>  };
>
> +&pcie30phy {
Please add the following field as per the device tree binding
 # Documentation/devicetree/bindings/phy/rockchip,pcie3-phy.yaml
            data-lanes = <0 1>;
> +       phy-supply = <&vcc3v3_pi6c_03>;
> +       status = "okay";
> +};
> +
> +&pcie3x2 {
Please add the following field as per the device tree binding
# Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
            num-lanes = <2>;
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&pcie30x2m1_pins>;
> +       reset-gpios = <&gpio2 RK_PD6 GPIO_ACTIVE_HIGH>;
> +       vpcie3v3-supply = <&vcc3v3_pcie>;
> +       status = "okay";
> +};
> +
>  &pinctrl {
>         cam {
>                 vcc_cam_en: vcc_cam_en {
> --
> 2.25.1
>
> Thanks,
> Chukun

Thanks
-Anand