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.
