On Thu, 7 Mar 2024 at 17:12, Adam Ford <aford...@gmail.com> wrote: > > On Thu, Mar 7, 2024 at 12:35 AM Sumit Garg <sumit.g...@linaro.org> wrote: > > > > On Wed, 6 Mar 2024 at 22:56, Tim Harvey <thar...@gateworks.com> wrote: > > > > > > On Tue, Mar 5, 2024 at 4:47 PM Adam Ford <aford...@gmail.com> wrote: > > > > > > > > On Tue, Mar 5, 2024 at 7:51 AM Adam Ford <aford...@gmail.com> wrote: > > > > > > > > > > On Tue, Mar 5, 2024 at 7:48 AM Sumit Garg <sumit.g...@linaro.org> > > > > > wrote: > > > > > > > > > > > > Hi Adam, > > > > > > > > > > > > On Wed, 28 Feb 2024 at 12:29, Sumit Garg <sumit.g...@linaro.org> > > > > > > wrote: > > > > > > > > > > > > > > Hi Adam, > > > > > > > > > > > > > > On Wed, 28 Feb 2024 at 04:42, Adam Ford <aford...@gmail.com> > > > > > > > wrote: > > > > > > > > > > > > > > > > On Mon, Feb 26, 2024 at 2:05 AM Sumit Garg > > > > > > > > <sumit.g...@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/arch/arm64?id=63c46b51c7995d8aeb4b44493633f4ce1dcf62bc > > > > 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. >
No worries. > > 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 <aford...@gmail.com> #imx8mp-beacon-kit > Thanks for taking time to test this series. -Sumit > adam > > > > > > > > tested-by: Tim Harvey <thar...@gateworks.com> : imx8mp-venice* > > > > > > > Thanks again. > > > > -Sumit > > > > > Best regards, > > > > > > Tim