| Message ID | 20190401042547.14067-9-andrew.smirnov@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | i.MX6, DesignWare PCI improvements | expand |
Am Sonntag, den 31.03.2019, 21:25 -0700 schrieb Andrey Smirnov: > Simplify pcie_phy_poll_ack() by incorporating shifting into constant > definition and convert the code to use 'bool'. No functional change > intended. > > > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > Cc: Bjorn Helgaas <bhelgaas@google.com> > > Cc: Fabio Estevam <fabio.estevam@nxp.com> > > Cc: Chris Healy <cphealy@gmail.com> > > Cc: Lucas Stach <l.stach@pengutronix.de> > > Cc: Leonard Crestez <leonard.crestez@nxp.com> > > Cc: "A.s. Dong" <aisheng.dong@nxp.com> > > Cc: Richard Zhu <hongxing.zhu@nxp.com> > Cc: linux-imx@nxp.com > Cc: linux-arm-kernel@lists.infradead.org > Cc: linux-kernel@vger.kernel.org > Cc: linux-pci@vger.kernel.org > > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com> > --- > drivers/pci/controller/dwc/pci-imx6.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > index a49e5e491e12..7c3ffb751002 100644 > --- a/drivers/pci/controller/dwc/pci-imx6.c > +++ b/drivers/pci/controller/dwc/pci-imx6.c > @@ -112,7 +112,7 @@ struct imx6_pcie { > > #define PCIE_PHY_CTRL_RD BIT(19) > > #define PCIE_PHY_STAT (PL_OFFSET + 0x110) > -#define PCIE_PHY_STAT_ACK_LOC 16 > > +#define PCIE_PHY_STAT_ACK BIT(16) > > > #define PCIE_LINK_WIDTH_SPEED_CONTROL 0x80C > > @@ -151,16 +151,16 @@ struct imx6_pcie { > > #define PHY_RX_OVRD_IN_LO_RX_DATA_EN BIT(5) > > #define PHY_RX_OVRD_IN_LO_RX_PLL_EN BIT(3) > > -static int pcie_phy_poll_ack(struct imx6_pcie *imx6_pcie, int exp_val) > +static int pcie_phy_poll_ack(struct imx6_pcie *imx6_pcie, bool exp_val) This looks like only going half the way to me. The parameter type is changed to bool, but the callers still use 0 and 1 directly. If we use bool here the callers should be changed to true/false. > { > struct dw_pcie *pci = imx6_pcie->pci; > - u32 val; > + bool val; > u32 max_iterations = 10; > u32 wait_counter = 0; > > do { > - val = dw_pcie_readl_dbi(pci, PCIE_PHY_STAT); > - val = (val >> PCIE_PHY_STAT_ACK_LOC) & 0x1; > + val = dw_pcie_readl_dbi(pci, PCIE_PHY_STAT) & > + PCIE_PHY_STAT_ACK; I think this needs to be val = !!(dw_pcie_readl_dbi(pci, PCIE_PHY_STAT) & PCIE_PHY_STAT_ACK); as the ACK bit isn't shifted down into bit position 0 anymore. > wait_counter++; > > > if (val == exp_val)
On Fri, Apr 12, 2019 at 9:12 AM Lucas Stach <l.stach@pengutronix.de> wrote: > > Am Sonntag, den 31.03.2019, 21:25 -0700 schrieb Andrey Smirnov: > > Simplify pcie_phy_poll_ack() by incorporating shifting into constant > > definition and convert the code to use 'bool'. No functional change > > intended. > > > > > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > > Cc: Bjorn Helgaas <bhelgaas@google.com> > > > Cc: Fabio Estevam <fabio.estevam@nxp.com> > > > Cc: Chris Healy <cphealy@gmail.com> > > > Cc: Lucas Stach <l.stach@pengutronix.de> > > > Cc: Leonard Crestez <leonard.crestez@nxp.com> > > > Cc: "A.s. Dong" <aisheng.dong@nxp.com> > > > Cc: Richard Zhu <hongxing.zhu@nxp.com> > > Cc: linux-imx@nxp.com > > Cc: linux-arm-kernel@lists.infradead.org > > Cc: linux-kernel@vger.kernel.org > > Cc: linux-pci@vger.kernel.org > > > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com> > > --- > > drivers/pci/controller/dwc/pci-imx6.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > > index a49e5e491e12..7c3ffb751002 100644 > > --- a/drivers/pci/controller/dwc/pci-imx6.c > > +++ b/drivers/pci/controller/dwc/pci-imx6.c > > @@ -112,7 +112,7 @@ struct imx6_pcie { > > > #define PCIE_PHY_CTRL_RD BIT(19) > > > > #define PCIE_PHY_STAT (PL_OFFSET + 0x110) > > -#define PCIE_PHY_STAT_ACK_LOC 16 > > > +#define PCIE_PHY_STAT_ACK BIT(16) > > > > > #define PCIE_LINK_WIDTH_SPEED_CONTROL 0x80C > > > > @@ -151,16 +151,16 @@ struct imx6_pcie { > > > #define PHY_RX_OVRD_IN_LO_RX_DATA_EN BIT(5) > > > #define PHY_RX_OVRD_IN_LO_RX_PLL_EN BIT(3) > > > > -static int pcie_phy_poll_ack(struct imx6_pcie *imx6_pcie, int exp_val) > > +static int pcie_phy_poll_ack(struct imx6_pcie *imx6_pcie, bool exp_val) > > This looks like only going half the way to me. The parameter type is > changed to bool, but the callers still use 0 and 1 directly. If we use > bool here the callers should be changed to true/false. > OK, will change in next version. > > { > > struct dw_pcie *pci = imx6_pcie->pci; > > - u32 val; > > + bool val; > > u32 max_iterations = 10; > > u32 wait_counter = 0; > > > > do { > > - val = dw_pcie_readl_dbi(pci, PCIE_PHY_STAT); > > - val = (val >> PCIE_PHY_STAT_ACK_LOC) & 0x1; > > + val = dw_pcie_readl_dbi(pci, PCIE_PHY_STAT) & > > + PCIE_PHY_STAT_ACK; > > I think this needs to be > val = !!(dw_pcie_readl_dbi(pci, PCIE_PHY_STAT) & PCIE_PHY_STAT_ACK); > as the ACK bit isn't shifted down into bit position 0 anymore. > That should already be taken care of by conversion to bool/_Bool(see "Using bool" in Documentation/process/coding-style.rst) and was one of the reasons I converted "val" to that type. Thanks, Andrey Smirnov
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c index a49e5e491e12..7c3ffb751002 100644 --- a/drivers/pci/controller/dwc/pci-imx6.c +++ b/drivers/pci/controller/dwc/pci-imx6.c @@ -112,7 +112,7 @@ struct imx6_pcie { #define PCIE_PHY_CTRL_RD BIT(19) #define PCIE_PHY_STAT (PL_OFFSET + 0x110) -#define PCIE_PHY_STAT_ACK_LOC 16 +#define PCIE_PHY_STAT_ACK BIT(16) #define PCIE_LINK_WIDTH_SPEED_CONTROL 0x80C @@ -151,16 +151,16 @@ struct imx6_pcie { #define PHY_RX_OVRD_IN_LO_RX_DATA_EN BIT(5) #define PHY_RX_OVRD_IN_LO_RX_PLL_EN BIT(3) -static int pcie_phy_poll_ack(struct imx6_pcie *imx6_pcie, int exp_val) +static int pcie_phy_poll_ack(struct imx6_pcie *imx6_pcie, bool exp_val) { struct dw_pcie *pci = imx6_pcie->pci; - u32 val; + bool val; u32 max_iterations = 10; u32 wait_counter = 0; do { - val = dw_pcie_readl_dbi(pci, PCIE_PHY_STAT); - val = (val >> PCIE_PHY_STAT_ACK_LOC) & 0x1; + val = dw_pcie_readl_dbi(pci, PCIE_PHY_STAT) & + PCIE_PHY_STAT_ACK; wait_counter++; if (val == exp_val)
Simplify pcie_phy_poll_ack() by incorporating shifting into constant definition and convert the code to use 'bool'. No functional change intended. Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: Fabio Estevam <fabio.estevam@nxp.com> Cc: Chris Healy <cphealy@gmail.com> Cc: Lucas Stach <l.stach@pengutronix.de> Cc: Leonard Crestez <leonard.crestez@nxp.com> Cc: "A.s. Dong" <aisheng.dong@nxp.com> Cc: Richard Zhu <hongxing.zhu@nxp.com> Cc: linux-imx@nxp.com Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Cc: linux-pci@vger.kernel.org Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com> --- drivers/pci/controller/dwc/pci-imx6.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)