[PATCH v2 5/8] phy: phy-imx8m-pcie: Add support for i.MX8M{M/P} PCIe PHY
Sumit Garg
sumit.garg at linaro.org
Thu Mar 7 13:10:40 CET 2024
On Thu, 7 Mar 2024 at 17:12, Adam Ford <aford173 at gmail.com> wrote:
>
> On Thu, Mar 7, 2024 at 12:35 AM Sumit Garg <sumit.garg at linaro.org> wrote:
> >
> > On Wed, 6 Mar 2024 at 22:56, Tim Harvey <tharvey at gateworks.com> wrote:
> > >
> > > On Tue, Mar 5, 2024 at 4:47 PM Adam Ford <aford173 at gmail.com> wrote:
> > > >
> > > > On Tue, Mar 5, 2024 at 7:51 AM Adam Ford <aford173 at gmail.com> wrote:
> > > > >
> > > > > On Tue, Mar 5, 2024 at 7:48 AM Sumit Garg <sumit.garg at linaro.org> wrote:
> > > > > >
> > > > > > Hi Adam,
> > > > > >
> > > > > > On Wed, 28 Feb 2024 at 12:29, Sumit Garg <sumit.garg at linaro.org> wrote:
> > > > > > >
> > > > > > > Hi Adam,
> > > > > > >
> > > > > > > On Wed, 28 Feb 2024 at 04:42, Adam Ford <aford173 at gmail.com> wrote:
> > > > > > > >
> > > > > > > > On Mon, Feb 26, 2024 at 2:05 AM Sumit Garg <sumit.garg at 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 at 32f00000: PHY: Failed to power on
> > > > > > > > pcie-phy at 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 <aford173 at gmail.com> #imx8mp-beacon-kit
>
Thanks for taking time to test this series.
-Sumit
> adam
>
>
> > >
> > > tested-by: Tim Harvey <tharvey at gateworks.com> : imx8mp-venice*
> > >
> >
> > Thanks again.
> >
> > -Sumit
> >
> > > Best regards,
> > >
> > > Tim
More information about the U-Boot
mailing list