| Message ID | 20181221072716.29017-8-andrew.smirnov@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | i.MX6, DesignWare PCI improvements | expand |
Hi, On 21/12/2018 07:27, Andrey Smirnov wrote: > Make the intent a bit more clear as well as get rid of explicit > arithmetic by using IS_ALIGNED() to determine if "addr" is aligned to > "size". 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/pcie-designware.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c > index 93ef8c31fb39..67236379c61a 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.c > +++ b/drivers/pci/controller/dwc/pcie-designware.c > @@ -22,7 +22,7 @@ > > int dw_pcie_read(void __iomem *addr, int size, u32 *val) > { > - if ((uintptr_t)addr & (size - 1)) { > + if (!IS_ALIGNED((uintptr_t)addr, size)) { > *val = 0; > return PCIBIOS_BAD_REGISTER_NUMBER; > } > @@ -43,7 +43,7 @@ int dw_pcie_read(void __iomem *addr, int size, u32 *val) > > int dw_pcie_write(void __iomem *addr, int size, u32 val) > { > - if ((uintptr_t)addr & (size - 1)) > + if (!IS_ALIGNED((uintptr_t)addr, size)) > return PCIBIOS_BAD_REGISTER_NUMBER; > > if (size == 4) > Sounds good. Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
On Wed, 2019-01-02 at 09:33 +0000, Gustavo Pimentel wrote: > On 21/12/2018 07:27, Andrey Smirnov wrote: > > Make the intent a bit more clear as well as get rid of explicit > > arithmetic by using IS_ALIGNED() to determine if "addr" is aligned to > > "size". No functional change intended. [] > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c [] > > @@ -22,7 +22,7 @@ > > > > int dw_pcie_read(void __iomem *addr, int size, u32 *val) > > { > > - if ((uintptr_t)addr & (size - 1)) { > > + if (!IS_ALIGNED((uintptr_t)addr, size)) { The (uintptr_t) cast could probably be removed as well. > > @@ -43,7 +43,7 @@ int dw_pcie_read(void __iomem *addr, int size, u32 *val) > > > > int dw_pcie_write(void __iomem *addr, int size, u32 val) > > { > > - if ((uintptr_t)addr & (size - 1)) > > + if (!IS_ALIGNED((uintptr_t)addr, size)) > > return PCIBIOS_BAD_REGISTER_NUMBER; here too
On Fri, Jan 4, 2019 at 10:38 AM Joe Perches <joe@perches.com> wrote: > > On Wed, 2019-01-02 at 09:33 +0000, Gustavo Pimentel wrote: > > On 21/12/2018 07:27, Andrey Smirnov wrote: > > > Make the intent a bit more clear as well as get rid of explicit > > > arithmetic by using IS_ALIGNED() to determine if "addr" is aligned to > > > "size". No functional change intended. > [] > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c > [] > > > @@ -22,7 +22,7 @@ > > > > > > int dw_pcie_read(void __iomem *addr, int size, u32 *val) > > > { > > > - if ((uintptr_t)addr & (size - 1)) { > > > + if (!IS_ALIGNED((uintptr_t)addr, size)) { > > The (uintptr_t) cast could probably be removed as well. > Unfortunately no. IS_ALIGNED(x, a) is going to expand (((x) & ((typeof(x))(a) - 1)) == 0), so we'll end up with (((addr) & ((void *)(size) - 1)) which has two problems: 1. It tries to use & operator on two void * pointers 2. On 64-bit platforms (e.g. AArch64) it will try to cast an "int" to a pointer, resulting in "warning: cast to pointer from integer of different size" When working on it, I initially wrote the code without the cast, but was quickly reprimanded by GCC and had to add it back in. Thanks, Andrey Smirnov
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c index 93ef8c31fb39..67236379c61a 100644 --- a/drivers/pci/controller/dwc/pcie-designware.c +++ b/drivers/pci/controller/dwc/pcie-designware.c @@ -22,7 +22,7 @@ int dw_pcie_read(void __iomem *addr, int size, u32 *val) { - if ((uintptr_t)addr & (size - 1)) { + if (!IS_ALIGNED((uintptr_t)addr, size)) { *val = 0; return PCIBIOS_BAD_REGISTER_NUMBER; } @@ -43,7 +43,7 @@ int dw_pcie_read(void __iomem *addr, int size, u32 *val) int dw_pcie_write(void __iomem *addr, int size, u32 val) { - if ((uintptr_t)addr & (size - 1)) + if (!IS_ALIGNED((uintptr_t)addr, size)) return PCIBIOS_BAD_REGISTER_NUMBER; if (size == 4)
Make the intent a bit more clear as well as get rid of explicit arithmetic by using IS_ALIGNED() to determine if "addr" is aligned to "size". 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/pcie-designware.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)