On Thu, Mar 7, 2024 at 12:35 AM Sumit Garg sumit.garg@linaro.org wrote:
On Wed, 6 Mar 2024 at 22:56, Tim Harvey tharvey@gateworks.com wrote:
On Tue, Mar 5, 2024 at 4:47 PM Adam Ford aford173@gmail.com wrote:
On Tue, Mar 5, 2024 at 7:51 AM Adam Ford aford173@gmail.com wrote:
On Tue, Mar 5, 2024 at 7:48 AM Sumit Garg sumit.garg@linaro.org wrote:
Hi Adam,
On Wed, 28 Feb 2024 at 12:29, Sumit Garg sumit.garg@linaro.org wrote:
Hi Adam,
On Wed, 28 Feb 2024 at 04:42, Adam Ford aford173@gmail.com wrote: > > On Mon, Feb 26, 2024 at 2:05 AM Sumit Garg sumit.garg@linaro.org wrote: > > > > Add initial support for i.MX8M{M/P} PCIe PHY. On i.MX8M{M/P} SoCs PCIe > > PHY initialization moved to this standalone PHY driver. > > > > Inspired from counterpart Linux kernel v6.8-rc3 driver: > > drivers/phy/freescale/phy-fsl-imx8m-pcie.c. > > I have a PCIe device that works just fine in Linux. I have applied > your series an enabled the same config options as you did along with > some others to resolve some build issues. > > Any ideas how to address: > > nxp_imx8_pcie_phy pcie-phy@32f00000: PHY: Failed to power on > pcie-phy@32f00000: -110. > > My PHY node looks like this > > &pcie_phy { > fsl,refclk-pad-mode = <IMX8_PCIE_REFCLK_PAD_INPUT>; > clocks = <&pcie0_refclk>; > clock-names = "ref"; > status = "okay"; > }; > > The pcie_refcllk is defined as a 100MHz fixed clock in U-Boot. clk > dump shows it to be: > > 100000000 0 |-- clock-pcie >
I suppose that's an EVK board, try enabling MPCIE_3V3 regulator required for EVK board via following patch:
diff --git a/drivers/pci/pcie_dw_imx.c b/drivers/pci/pcie_dw_imx.c index 7856823c9188..32fd2cb05d4b 100644 --- a/drivers/pci/pcie_dw_imx.c +++ b/drivers/pci/pcie_dw_imx.c @@ -17,6 +17,7 @@ #include <linux/iopoll.h> #include <log.h> #include <pci.h> +#include <power/regulator.h> #include <regmap.h> #include <reset.h> #include <syscon.h> @@ -158,6 +159,8 @@ static int pcie_dw_imx_probe(struct udevice *dev) struct pci_controller *hose = dev_get_uclass_priv(ctlr); int ret;
regulator_autoset_by_name("MPCIE_3V3", NULL);I think you should search the device tree for "vpcie-supply" and enable the corresponding regulator, because is more flexible than hard-coding regulator names. This is also more of a standard practice.
ret = imx_pcie_assert_core_reset(priv); if (ret) { dev_err(dev, "failed to assert core reset\n");Were you able to give a retry with the above diff?
Not yet, but I'll try to do it tonight.
That didn't work for me. I am using a Beacon Embedded kit which does not use a regulator, so this had no impact, but I think having the vpcie-supply regulator is a good idea.
I'll investigate a bit more and let you know if I make any progress.
Adam and Sumit,
In case it helps this series works for PCI bus scanning for the imx8mp-venice-* boards with the following patch:
Thanks Tim for taking time to test this series. I hope you are fine with me taking your below patch with you being the author.
diff --git a/configs/imx8mp_venice_defconfig b/configs/imx8mp_venice_defconfig index 11b356b77856..ff7ea9811ed6 100644 --- a/configs/imx8mp_venice_defconfig +++ b/configs/imx8mp_venice_defconfig @@ -12,6 +12,7 @@ CONFIG_DEFAULT_DEVICE_TREE="imx8mp-venice" CONFIG_SPL_TEXT_BASE=0x920000 CONFIG_TARGET_IMX8MP_VENICE=y CONFIG_OF_LIBFDT_OVERLAY=y +CONFIG_DM_RESET=y CONFIG_SYS_MONITOR_LEN=524288 CONFIG_SPL_MMC=y CONFIG_SPL_SERIAL=y @@ -21,6 +22,7 @@ CONFIG_SPL=y CONFIG_ENV_OFFSET_REDUND=0x3f8000 CONFIG_SPL_IMX_ROMAPI_LOADADDR=0x48000000 CONFIG_SYS_LOAD_ADDR=0x40480000 +CONFIG_PCI=y CONFIG_SYS_MEMTEST_START=0x40000000 CONFIG_SYS_MEMTEST_END=0x80000000 CONFIG_LTO=y @@ -65,6 +67,7 @@ CONFIG_CMD_FUSE=y CONFIG_CMD_GPIO=y CONFIG_CMD_I2C=y CONFIG_CMD_MMC=y +CONFIG_CMD_PCI=y CONFIG_CMD_USB=y CONFIG_SYS_DISABLE_AUTOLOAD=y CONFIG_CMD_CACHE=y @@ -86,6 +89,8 @@ CONFIG_NET_RANDOM_ETHADDR=y CONFIG_IP_DEFRAG=y CONFIG_TFTP_BLOCKSIZE=4096 CONFIG_SPL_DM=y +CONFIG_REGMAP=y +CONFIG_SYSCON=y CONFIG_CLK_COMPOSITE_CCF=y CONFIG_CLK_IMX8MP=y CONFIG_GPIO_HOG=y @@ -114,7 +119,10 @@ CONFIG_FEC_MXC=y CONFIG_KSZ9477=y CONFIG_RGMII=y CONFIG_MII=y +CONFIG_NVME_PCI=y +CONFIG_PCIE_DW_IMX=y CONFIG_PHY_IMX8MQ_USB=y +CONFIG_PHY_IMX8M_PCIE=y CONFIG_PINCTRL=y CONFIG_SPL_PINCTRL=y CONFIG_PINCTRL_IMX8M=y
on an imx8mp-venice-gw71xx-2x (imx8mp with miniPCIe socket using a miniPCIe to NVMe adapter) u-boot=> pci enum PCIE-0: Link up (Gen1-x1, Bus0) u-boot=> pci BusDevFun VendorId DeviceId Device Class Sub-Class _____________________________________________________________ 00.00.00 0x16c3 0xabcd Bridge device 0x04 01.00.00 0x10ec 0x5765 Mass storage controller 0x08 u-boot=> nvme scan u-boot=> nvme info Device 0: Vendor: 0x10ec Rev: 0426-D00 Prod: 60272-0005 Type: Hard Disk Capacity: 57241.8 MB = 55.9 GB (117231408 x 512)
I also verified this works imx8mp-venice-gw74xx (imx8mp with PCIe switch to multiple miniPCIe sockets using a miniPCIe to NVMe adapter) u-boot=> pci enum PCIE-0: Link up (Gen1-x1, Bus0) u-boot=> pci BusDevFun VendorId DeviceId Device Class Sub-Class _____________________________________________________________ 00.00.00 0x16c3 0xabcd Bridge device 0x04 01.00.00 0x12d8 0x2608 Bridge device 0x04 02.01.00 0x12d8 0x2608 Bridge device 0x04 02.02.00 0x12d8 0x2608 Bridge device 0x04 02.03.00 0x12d8 0x2608 Bridge device 0x04 02.04.00 0x12d8 0x2608 Bridge device 0x04 05.00.00 0x10ec 0x5765 Mass storage controller 0x08 u-boot=> nvme scan u-boot=> nvme info Device 0: Vendor: 0x10ec Rev: 0426-D00 Prod: 60272-0005 Type: Hard Disk Capacity: 57241.8 MB = 55.9 GB (117231408 x 512)
It looks like the imx8mp-beacon-kit has almost the same setup as the imx8mp-venice-gw74xx with the exception of you having a PCIe_nDIS gpio defined in pinctrl (used nowhere) and the gw74xx does not support CLKREQ so has 'fsl,clkreq-unsupported'.
Ah, I see. So it turns out that it's Adam Linux kernel DTS patch [1] which is missing for U-Boot DTS. This is one of the reasons why we should switch to OF_UPSTREAM to start regular sync with Linux kernel DTS.
Adam,
Can you make corresponding changes to U-Boot DTS for imx8mp-beacon-kit? Or better just switch to OF_UPSTREAM instead?
I have it working now.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/ar...
This is one of the first things I did when I tried the patch, but the renesas clock driver isn't available in U-Boot. With and without that above patch, the Linux driver enumerates, but the real issue appears to be something related to ATF (or lack thereof). I had experimented with a series of patches which bypassed ATF, and pushed them upstream a while ago. Unfortunately, it appears to have caused more issues. When I revert those it all works just fine. Sorry for the noise.
u-boot=> pci enum PCIE-0: Link up (Gen1-x1, Bus0) u-boot=> nvme scan u-boot=> nvme info Device 0: Vendor: 0x15b7 Rev: 20120022 Prod: 184960441105 Type: Hard Disk Capacity: 122104.3 MB = 119.2 GB (250069680 x 512) u-boot=>
WIth my suggested regulator and build-dependency fixes added you can add
Tested-by: Adam Ford aford173@gmail.com #imx8mp-beacon-kit
adam
tested-by: Tim Harvey tharvey@gateworks.com : imx8mp-venice*
Thanks again.
-Sumit
Best regards,
Tim