Re: [PATCH 2/3] pci: dw_imx: add support for IMX8MM

[Tim Harvey]

On Wed, Apr 17, 2024 at 8:04 PM Marek Vasut  wrote:
>
> On 4/17/24 10:09 PM, Tim Harvey wrote:
> > Add support for the IMX8MM SoC by adding driver data with the compatible
> > string of the GPR controller.
> >
> > Signed-off-by: Tim Harvey 
> > ---
> >   drivers/pci/pcie_dw_imx.c | 20 ++--
> >   1 file changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/pcie_dw_imx.c b/drivers/pci/pcie_dw_imx.c
> > index a2ee228224b5..10d926c30645 100644
> > --- a/drivers/pci/pcie_dw_imx.c
> > +++ b/drivers/pci/pcie_dw_imx.c
> > @@ -45,6 +45,10 @@
> >   #define IMX8M_GPR_PCIE_CLK_REQ_OVERRIDE_EN  BIT(10)
> >   #define IMX8M_GPR_PCIE_CLK_REQ_OVERRIDE BIT(11)
> >
> > +struct pcie_chip_info {
> > + const char *gpr;
> > +};
> > +
> >   struct pcie_dw_imx {
> >   /* Must be first member of the struct */
> >   struct pcie_dw  dw;
> > @@ -54,6 +58,15 @@ struct pcie_dw_imx {
> >   struct reset_ctlapps_reset;
> >   struct phy  phy;
> >   struct udevice  *vpcie;
> > + struct pcie_chip_info   *info;
> > +};
> > +
> > +static const struct pcie_chip_info imx8mm_chip_info = {
> > + .gpr = "fsl,imx8mm-iomuxc-gpr",
> > +};
> > +
> > +static const struct pcie_chip_info imx8mp_chip_info = {
> > + .gpr = "fsl,imx8mp-iomuxc-gpr",
> >   };
> >
> >   static void pcie_dw_configure(struct pcie_dw_imx *priv, u32 cap_speed)
> > @@ -246,6 +259,8 @@ static int pcie_dw_imx_of_to_plat(struct udevice *dev)
> >   ofnode gpr;
> >   int ret;
> >
> > + priv->info = (void *)dev_get_driver_data(dev);
> > +
>
> Does this really have to be cached in priv ?
>
> The priv->info seems used only in this function.
>

Hi Marek,

Good point. I will remove that. Thanks for the review!

Best Regards,

Tim

Re: [PATCH 2/3] pci: dw_imx: add support for IMX8MM

[Marek Vasut]

On 4/17/24 10:09 PM, Tim Harvey wrote:

Add support for the IMX8MM SoC by adding driver data with the compatible
string of the GPR controller.

Signed-off-by: Tim Harvey 
---
  drivers/pci/pcie_dw_imx.c | 20 ++--
  1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pcie_dw_imx.c b/drivers/pci/pcie_dw_imx.c
index a2ee228224b5..10d926c30645 100644
--- a/drivers/pci/pcie_dw_imx.c
+++ b/drivers/pci/pcie_dw_imx.c
@@ -45,6 +45,10 @@
  #define IMX8M_GPR_PCIE_CLK_REQ_OVERRIDE_ENBIT(10)
  #define IMX8M_GPR_PCIE_CLK_REQ_OVERRIDE   BIT(11)
  
+struct pcie_chip_info {

+   const char *gpr;
+};
+
  struct pcie_dw_imx {
/* Must be first member of the struct */
struct pcie_dw  dw;
@@ -54,6 +58,15 @@ struct pcie_dw_imx {
struct reset_ctlapps_reset;
struct phy  phy;
struct udevice  *vpcie;
+   struct pcie_chip_info   *info;
+};
+
+static const struct pcie_chip_info imx8mm_chip_info = {
+   .gpr = "fsl,imx8mm-iomuxc-gpr",
+};
+
+static const struct pcie_chip_info imx8mp_chip_info = {
+   .gpr = "fsl,imx8mp-iomuxc-gpr",
  };
  
  static void pcie_dw_configure(struct pcie_dw_imx *priv, u32 cap_speed)

@@ -246,6 +259,8 @@ static int pcie_dw_imx_of_to_plat(struct udevice *dev)
ofnode gpr;
int ret;
  
+	priv->info = (void *)dev_get_driver_data(dev);

+


Does this really have to be cached in priv ?

The priv->info seems used only in this function.

[...]

[PATCH 2/3] pci: dw_imx: add support for IMX8MM

[Tim Harvey]

Add support for the IMX8MM SoC by adding driver data with the compatible
string of the GPR controller.

Signed-off-by: Tim Harvey 
---
 drivers/pci/pcie_dw_imx.c | 20 ++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pcie_dw_imx.c b/drivers/pci/pcie_dw_imx.c
index a2ee228224b5..10d926c30645 100644
--- a/drivers/pci/pcie_dw_imx.c
+++ b/drivers/pci/pcie_dw_imx.c
@@ -45,6 +45,10 @@
 #define IMX8M_GPR_PCIE_CLK_REQ_OVERRIDE_EN BIT(10)
 #define IMX8M_GPR_PCIE_CLK_REQ_OVERRIDEBIT(11)
 
+struct pcie_chip_info {
+   const char *gpr;
+};
+
 struct pcie_dw_imx {
/* Must be first member of the struct */
struct pcie_dw  dw;
@@ -54,6 +58,15 @@ struct pcie_dw_imx {
struct reset_ctlapps_reset;
struct phy  phy;
struct udevice  *vpcie;
+   struct pcie_chip_info   *info;
+};
+
+static const struct pcie_chip_info imx8mm_chip_info = {
+   .gpr = "fsl,imx8mm-iomuxc-gpr",
+};
+
+static const struct pcie_chip_info imx8mp_chip_info = {
+   .gpr = "fsl,imx8mp-iomuxc-gpr",
 };
 
 static void pcie_dw_configure(struct pcie_dw_imx *priv, u32 cap_speed)
@@ -246,6 +259,8 @@ static int pcie_dw_imx_of_to_plat(struct udevice *dev)
ofnode gpr;
int ret;
 
+   priv->info = (void *)dev_get_driver_data(dev);
+
/* Get the controller base address */
priv->dw.dbi_base = (void *)dev_read_addr_name(dev, "dbi");
if ((fdt_addr_t)priv->dw.dbi_base == FDT_ADDR_T_NONE) {
@@ -287,7 +302,7 @@ static int pcie_dw_imx_of_to_plat(struct udevice *dev)
goto err_phy;
}
 
-   gpr = ofnode_by_compatible(ofnode_null(), "fsl,imx8mp-iomuxc-gpr");
+   gpr = ofnode_by_compatible(ofnode_null(), priv->info->gpr);
if (ofnode_equal(gpr, ofnode_null())) {
dev_err(dev, "unable to find GPR node\n");
ret = -ENODEV;
@@ -322,7 +337,8 @@ static const struct dm_pci_ops pcie_dw_imx_ops = {
 };
 
 static const struct udevice_id pcie_dw_imx_ids[] = {
-   { .compatible = "fsl,imx8mp-pcie" },
+   { .compatible = "fsl,imx8mm-pcie", .data = (ulong)_chip_info, },
+   { .compatible = "fsl,imx8mp-pcie", .data = (ulong)_chip_info, },
{ }
 };
 
-- 
2.25.1