| Message ID | 1646644054-24421-2-git-send-email-hongxing.zhu@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add the iMX8MP PCIe support | expand |
Hi Richard, On Mo, 2022-03-07 at 17:07 +0800, Richard Zhu wrote: > Add the i.MX8MP PCIe PHY PERST support. > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> > --- > drivers/reset/reset-imx7.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c > index 185a333df66c..d2408725eb2c 100644 > --- a/drivers/reset/reset-imx7.c > +++ b/drivers/reset/reset-imx7.c > @@ -329,6 +329,7 @@ static int imx8mp_reset_set(struct > reset_controller_dev *rcdev, > break; > > case IMX8MP_RESET_PCIE_CTRL_APPS_EN: > + case IMX8MP_RESET_PCIEPHY_PERST: > value = assert ? 0 : bit; > break; > } This doesn't do what the commit description says. The PCIEPHY_PERST bit is already supported by the driver (albeit incorrectly?) - this patch just inverts the bit. Since this bit is not inverted on the other platforms, and the i.MX8MP reference manual says nothing about this, please explicitly state why this needs to be inverted and call it a fix in the commit description. regards Philipp
Am Montag, dem 07.03.2022 um 17:07 +0800 schrieb Richard Zhu: > Add the i.MX8MP PCIe PHY PERST support. As Philipp said: please add some more description on why this is necessary. As far as I can see the reset is already present on 8MQ, and is low-active, like this patch claims. We just didn't handle this reset at all on other SoCs as the power on de-asserted state was okay to get things working. Regards, Lucas > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> > --- > drivers/reset/reset-imx7.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c > index 185a333df66c..d2408725eb2c 100644 > --- a/drivers/reset/reset-imx7.c > +++ b/drivers/reset/reset-imx7.c > @@ -329,6 +329,7 @@ static int imx8mp_reset_set(struct reset_controller_dev *rcdev, > break; > > case IMX8MP_RESET_PCIE_CTRL_APPS_EN: > + case IMX8MP_RESET_PCIEPHY_PERST: > value = assert ? 0 : bit; > break; > }
Hi Philipp: > -----Original Message----- > From: Philipp Zabel <p.zabel@pengutronix.de> > Sent: 2022年4月4日 17:34 > To: Hongxing Zhu <hongxing.zhu@nxp.com>; l.stach@pengutronix.de; > bhelgaas@google.com; lorenzo.pieralisi@arm.com; robh@kernel.org; > shawnguo@kernel.org; vkoul@kernel.org; alexander.stein@ew.tq-group.com > Cc: linux-phy@lists.infradead.org; devicetree@vger.kernel.org; > linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > linux-kernel@vger.kernel.org; kernel@pengutronix.de; dl-linux-imx > <linux-imx@nxp.com> > Subject: Re: [PATCH v2 1/7] reset: imx7: Add the iMX8MP PCIe PHY PERST > support > > Hi Richard, > > On Mo, 2022-03-07 at 17:07 +0800, Richard Zhu wrote: > > Add the i.MX8MP PCIe PHY PERST support. > > > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> > > --- > > drivers/reset/reset-imx7.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c > > index 185a333df66c..d2408725eb2c 100644 > > --- a/drivers/reset/reset-imx7.c > > +++ b/drivers/reset/reset-imx7.c > > @@ -329,6 +329,7 @@ static int imx8mp_reset_set(struct > > reset_controller_dev *rcdev, > > break; > > > > case IMX8MP_RESET_PCIE_CTRL_APPS_EN: > > + case IMX8MP_RESET_PCIEPHY_PERST: > > value = assert ? 0 : bit; > > break; > > } > > This doesn't do what the commit description says. > > The PCIEPHY_PERST bit is already supported by the driver (albeit > incorrectly?) - this patch just inverts the bit. > > Since this bit is not inverted on the other platforms, and the i.MX8MP > reference manual says nothing about this, please explicitly state why this > needs to be inverted and call it a fix in the commit description. Thanks for your comments, and sorry for replying late. I didn't get more details about this bit difference between i.MX8MP and others. Let me consult with design team again, and back to you later. Best Regards Richard Zhu > > regards > Philipp
> -----Original Message----- > From: Lucas Stach <l.stach@pengutronix.de> > Sent: 2022年4月15日 4:48 > To: Hongxing Zhu <hongxing.zhu@nxp.com>; p.zabel@pengutronix.de; > bhelgaas@google.com; lorenzo.pieralisi@arm.com; robh@kernel.org; > shawnguo@kernel.org; vkoul@kernel.org; alexander.stein@ew.tq-group.com > Cc: linux-phy@lists.infradead.org; devicetree@vger.kernel.org; > linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > linux-kernel@vger.kernel.org; kernel@pengutronix.de; dl-linux-imx > <linux-imx@nxp.com> > Subject: Re: [PATCH v2 1/7] reset: imx7: Add the iMX8MP PCIe PHY PERST > support > > Am Montag, dem 07.03.2022 um 17:07 +0800 schrieb Richard Zhu: > > Add the i.MX8MP PCIe PHY PERST support. > > As Philipp said: please add some more description on why this is necessary. As > far as I can see the reset is already present on 8MQ, and is low-active, like this > patch claims. We just didn't handle this reset at all on other SoCs as the power > on de-asserted state was okay to get things working. > Yes, it is. I had asking the details from design team. Would update the description after I get the more information. Best Regards Richard Zhu > Regards, > Lucas > > > > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> > > --- > > drivers/reset/reset-imx7.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c > > index 185a333df66c..d2408725eb2c 100644 > > --- a/drivers/reset/reset-imx7.c > > +++ b/drivers/reset/reset-imx7.c > > @@ -329,6 +329,7 @@ static int imx8mp_reset_set(struct > reset_controller_dev *rcdev, > > break; > > > > case IMX8MP_RESET_PCIE_CTRL_APPS_EN: > > + case IMX8MP_RESET_PCIEPHY_PERST: > > value = assert ? 0 : bit; > > break; > > } >
Hi Philipp: > -----Original Message----- > From: Hongxing Zhu > Sent: 2022年4月15日 15:33 > To: Philipp Zabel <p.zabel@pengutronix.de>; l.stach@pengutronix.de; > bhelgaas@google.com; lorenzo.pieralisi@arm.com; robh@kernel.org; > shawnguo@kernel.org; vkoul@kernel.org; alexander.stein@ew.tq-group.com > Cc: linux-phy@lists.infradead.org; devicetree@vger.kernel.org; > linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > linux-kernel@vger.kernel.org; kernel@pengutronix.de; dl-linux-imx > <linux-imx@nxp.com> > Subject: RE: [PATCH v2 1/7] reset: imx7: Add the iMX8MP PCIe PHY PERST > support > > Hi Philipp: > > > -----Original Message----- > > From: Philipp Zabel <p.zabel@pengutronix.de> > > Sent: 2022年4月4日 17:34 > > To: Hongxing Zhu <hongxing.zhu@nxp.com>; l.stach@pengutronix.de; > > bhelgaas@google.com; lorenzo.pieralisi@arm.com; robh@kernel.org; > > shawnguo@kernel.org; vkoul@kernel.org; alexander.stein@ew.tq-group.com > > Cc: linux-phy@lists.infradead.org; devicetree@vger.kernel.org; > > linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > > linux-kernel@vger.kernel.org; kernel@pengutronix.de; dl-linux-imx > > <linux-imx@nxp.com> > > Subject: Re: [PATCH v2 1/7] reset: imx7: Add the iMX8MP PCIe PHY PERST > > support > > > > Hi Richard, > > > > On Mo, 2022-03-07 at 17:07 +0800, Richard Zhu wrote: > > > Add the i.MX8MP PCIe PHY PERST support. > > > > > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> > > > --- > > > drivers/reset/reset-imx7.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c > > > index 185a333df66c..d2408725eb2c 100644 > > > --- a/drivers/reset/reset-imx7.c > > > +++ b/drivers/reset/reset-imx7.c > > > @@ -329,6 +329,7 @@ static int imx8mp_reset_set(struct > > > reset_controller_dev *rcdev, > > > break; > > > > > > case IMX8MP_RESET_PCIE_CTRL_APPS_EN: > > > + case IMX8MP_RESET_PCIEPHY_PERST: > > > value = assert ? 0 : bit; > > > break; > > > } > > > > This doesn't do what the commit description says. > > > > The PCIEPHY_PERST bit is already supported by the driver (albeit > > incorrectly?) - this patch just inverts the bit. > > > > Since this bit is not inverted on the other platforms, and the i.MX8MP > > reference manual says nothing about this, please explicitly state why > > this needs to be inverted and call it a fix in the commit description. > Thanks for your comments, and sorry for replying late. > I didn't get more details about this bit difference between i.MX8MP and > others. > Let me consult with design team again, and back to you later. I got some details of this PERST bit(BIT3) of SRC_PCIEPHY_RCR. The initialized default value of this bit is 1b'1 on i.MX7/iMX8MM/iMX8MQ. But unfortunately, the i.MX8MP has one inverted default value 1b'0 of this bit. And this bit should be kept 1b'1 after power and clocks are stable. So, I assert/de-assert this bit on i.MX8MP only. Best Regards Richard Zhu > > Best Regards > Richard Zhu > > > > > regards > > Philipp
diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c index 185a333df66c..d2408725eb2c 100644 --- a/drivers/reset/reset-imx7.c +++ b/drivers/reset/reset-imx7.c @@ -329,6 +329,7 @@ static int imx8mp_reset_set(struct reset_controller_dev *rcdev, break; case IMX8MP_RESET_PCIE_CTRL_APPS_EN: + case IMX8MP_RESET_PCIEPHY_PERST: value = assert ? 0 : bit; break; }
Add the i.MX8MP PCIe PHY PERST support. Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com> --- drivers/reset/reset-imx7.c | 1 + 1 file changed, 1 insertion(+)