Re: [PATCH v2 3/3] PCI: imx: Add workaround for e10728, IMX7d PCIe PLL failure

[Trent Piepho <tpiepho@impinj.com>]

On Fri, 2019-02-08 at 10:18 +0000, Lorenzo Pieralisi wrote:
> On Thu, Feb 07, 2019 at 06:15:52PM +0000, Trent Piepho wrote:
> > On Thu, 2019-02-07 at 12:31 +0000, Lorenzo Pieralisi wrote:
> > > +			/* De-assert DCC_FB_EN */
> > > > +			writel(PCIE_PHY_CMN_REG4_DCC_FB_EN,
> > > > +			       imx6_pcie->phy_base + PCIE_PHY_CMN_REG4);
> > > > +			/* Assert RX_EQS and RX_EQS_SEL */
> > > > +			writel(PCIE_PHY_CMN_REG24_RX_EQ_SEL
> > > > +				| PCIE_PHY_CMN_REG24_RX_EQ,
> > > > +			       imx6_pcie->phy_base + PCIE_PHY_CMN_REG24);
> > > > +			/* Assert ATT_MODE */
> > > > +			writel(PCIE_PHY_CMN_REG26_ATT_MODE,
> > > > +			       imx6_pcie->phy_base + PCIE_PHY_CMN_REG26);
> > > > +		} else {
> > > > +			dev_warn(dev, "DT lacks imx7d-pcie-phy, unable to apply ERR010728 workaround\n");
> > > 
> > > It is a nit but the warning log is not necessarily true ie you may not
> > > have a mapped address for other reasons as well. It is up to you but I
> > > would change the log message.
> > 
> > True, but I hardly want to enumerate every possible failure point in a
> > log message.  How about:
> > 
> > Unable to apply ERR010728 workaround.  DT lacks imx7d-pcie-phy?
> 
> I would just log "Unable to apply ERR010728 workaround" but it is up
> to you. I can make the change myself with no need for reposting.

I was following Lucas's comment on the v1 version:

   the user. It would be better to have something like "No access to
   PHY registers, can't apply workaround for ERR010728." Maybe even
   with a hint to update the DT.

I think the pretty much the only way any of the errors conditions will
get hit, is when someone with an old DT runs a new kernel.  So the hint
about the DT change might be actually useful to some users.

But it's not a big deal so just change it to whatever when applying the
patch.