* [PATCH 02/28] [v2] mtd: mtk: avoid warning in mtk_ecc_encode
[not found] <20161017220342.1627073-1-arnd@arndb.de>
@ 2016-10-17 22:05 ` Arnd Bergmann
2016-10-18 5:19 ` Boris Brezillon
2016-10-17 22:16 ` [PATCH 20/28] net: bcm63xx: avoid referencing uninitialized variable Arnd Bergmann
1 sibling, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:05 UTC (permalink / raw)
To: linux-arm-kernel
When building with -Wmaybe-uninitialized, gcc produces a silly false positive
warning for the mtk_ecc_encode function:
drivers/mtd/nand/mtk_ecc.c: In function 'mtk_ecc_encode':
drivers/mtd/nand/mtk_ecc.c:402:15: error: 'val' may be used uninitialized in this function [-Werror=maybe-uninitialized]
The function for some reason contains a double byte swap on big-endian
builds to get the OOB data into the correct order again, and is written
in a slightly confusing way.
Using a simple memcpy32_fromio() to read the data simplifies it a lot
so it becomes more readable and produces no warning. However, the
output might not have 32-bit alignment, so we have to use another
memcpy to avoid taking alignment faults or writing beyond the end
of the array.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v2: move temporary buffer into struct mtk_ecc instead of having it
on the stack, as suggested by Boris Brezillon
---
drivers/mtd/nand/mtk_ecc.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/mtd/nand/mtk_ecc.c b/drivers/mtd/nand/mtk_ecc.c
index d54f666..dbf2562 100644
--- a/drivers/mtd/nand/mtk_ecc.c
+++ b/drivers/mtd/nand/mtk_ecc.c
@@ -86,6 +86,8 @@ struct mtk_ecc {
struct completion done;
struct mutex lock;
u32 sectors;
+
+ u8 eccdata[112];
};
static inline void mtk_ecc_wait_idle(struct mtk_ecc *ecc,
@@ -366,9 +368,8 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
u8 *data, u32 bytes)
{
dma_addr_t addr;
- u8 *p;
- u32 len, i, val;
- int ret = 0;
+ u32 len;
+ int ret;
addr = dma_map_single(ecc->dev, data, bytes, DMA_TO_DEVICE);
ret = dma_mapping_error(ecc->dev, addr);
@@ -393,14 +394,12 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
/* Program ECC bytes to OOB: per sector oob = FDM + ECC + SPARE */
len = (config->strength * ECC_PARITY_BITS + 7) >> 3;
- p = data + bytes;
- /* write the parity bytes generated by the ECC back to the OOB region */
- for (i = 0; i < len; i++) {
- if ((i % 4) == 0)
- val = readl(ecc->regs + ECC_ENCPAR(i / 4));
- p[i] = (val >> ((i % 4) * 8)) & 0xff;
- }
+ /* write the parity bytes generated by the ECC back to temp buffer */
+ __ioread32_copy(ecc->eccdata, ecc->regs + ECC_ENCPAR(0), round_up(len, 4));
+
+ /* copy into possibly unaligned OOB region with actual length */
+ memcpy(data + bytes, ecc->eccdata, len);
timeout:
dma_unmap_single(ecc->dev, addr, bytes, DMA_TO_DEVICE);
--
2.9.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 20/28] net: bcm63xx: avoid referencing uninitialized variable
[not found] <20161017220342.1627073-1-arnd@arndb.de>
2016-10-17 22:05 ` [PATCH 02/28] [v2] mtd: mtk: avoid warning in mtk_ecc_encode Arnd Bergmann
@ 2016-10-17 22:16 ` Arnd Bergmann
2016-10-18 18:21 ` David Miller
1 sibling, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:16 UTC (permalink / raw)
To: linux-arm-kernel
gcc found a reference to an uninitialized variable in the error handling
of bcm_enet_open, introduced by a recent cleanup:
drivers/net/ethernet/broadcom/bcm63xx_enet.c: In function 'bcm_enet_open'
drivers/net/ethernet/broadcom/bcm63xx_enet.c:1129:2: warning: 'phydev' may be used uninitialized in this function [-Wmaybe-uninitialized]
This makes the use of that variable conditional, so we only reference it
here after it has been used before. Unlike my normal patches, I have not
build-tested this one, as I don't currently have mips test in my
randconfig setup.
Fixes: 625eb8667d6f ("net: ethernet: broadcom: bcm63xx: use phydev from struct net_device")
Cc: Philippe Reynes <tremyfr@gmail.com>
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/ethernet/broadcom/bcm63xx_enet.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bcm63xx_enet.c b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
index ae364c7..5370909 100644
--- a/drivers/net/ethernet/broadcom/bcm63xx_enet.c
+++ b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
@@ -1126,7 +1126,8 @@ static int bcm_enet_open(struct net_device *dev)
free_irq(dev->irq, dev);
out_phy_disconnect:
- phy_disconnect(phydev);
+ if (priv->has_phy)
+ phy_disconnect(phydev);
return ret;
}
--
2.9.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 02/28] [v2] mtd: mtk: avoid warning in mtk_ecc_encode
2016-10-17 22:05 ` [PATCH 02/28] [v2] mtd: mtk: avoid warning in mtk_ecc_encode Arnd Bergmann
@ 2016-10-18 5:19 ` Boris Brezillon
2016-10-18 10:12 ` RogerCC.Lin
0 siblings, 1 reply; 6+ messages in thread
From: Boris Brezillon @ 2016-10-18 5:19 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 18 Oct 2016 00:05:31 +0200
Arnd Bergmann <arnd@arndb.de> wrote:
> When building with -Wmaybe-uninitialized, gcc produces a silly false positive
> warning for the mtk_ecc_encode function:
>
> drivers/mtd/nand/mtk_ecc.c: In function 'mtk_ecc_encode':
> drivers/mtd/nand/mtk_ecc.c:402:15: error: 'val' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> The function for some reason contains a double byte swap on big-endian
> builds to get the OOB data into the correct order again, and is written
> in a slightly confusing way.
>
> Using a simple memcpy32_fromio() to read the data simplifies it a lot
> so it becomes more readable and produces no warning. However, the
> output might not have 32-bit alignment, so we have to use another
> memcpy to avoid taking alignment faults or writing beyond the end
> of the array.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Jorge, RogerCC, can I have an Acked-by and/or Tested-by for this patch?
> ---
> v2: move temporary buffer into struct mtk_ecc instead of having it
> on the stack, as suggested by Boris Brezillon
> ---
> drivers/mtd/nand/mtk_ecc.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/mtd/nand/mtk_ecc.c b/drivers/mtd/nand/mtk_ecc.c
> index d54f666..dbf2562 100644
> --- a/drivers/mtd/nand/mtk_ecc.c
> +++ b/drivers/mtd/nand/mtk_ecc.c
> @@ -86,6 +86,8 @@ struct mtk_ecc {
> struct completion done;
> struct mutex lock;
> u32 sectors;
> +
> + u8 eccdata[112];
> };
>
> static inline void mtk_ecc_wait_idle(struct mtk_ecc *ecc,
> @@ -366,9 +368,8 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
> u8 *data, u32 bytes)
> {
> dma_addr_t addr;
> - u8 *p;
> - u32 len, i, val;
> - int ret = 0;
> + u32 len;
> + int ret;
>
> addr = dma_map_single(ecc->dev, data, bytes, DMA_TO_DEVICE);
> ret = dma_mapping_error(ecc->dev, addr);
> @@ -393,14 +394,12 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
>
> /* Program ECC bytes to OOB: per sector oob = FDM + ECC + SPARE */
> len = (config->strength * ECC_PARITY_BITS + 7) >> 3;
> - p = data + bytes;
>
> - /* write the parity bytes generated by the ECC back to the OOB region */
> - for (i = 0; i < len; i++) {
> - if ((i % 4) == 0)
> - val = readl(ecc->regs + ECC_ENCPAR(i / 4));
> - p[i] = (val >> ((i % 4) * 8)) & 0xff;
> - }
> + /* write the parity bytes generated by the ECC back to temp buffer */
> + __ioread32_copy(ecc->eccdata, ecc->regs + ECC_ENCPAR(0), round_up(len, 4));
> +
> + /* copy into possibly unaligned OOB region with actual length */
> + memcpy(data + bytes, ecc->eccdata, len);
> timeout:
>
> dma_unmap_single(ecc->dev, addr, bytes, DMA_TO_DEVICE);
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 02/28] [v2] mtd: mtk: avoid warning in mtk_ecc_encode
2016-10-18 5:19 ` Boris Brezillon
@ 2016-10-18 10:12 ` RogerCC.Lin
2016-10-18 19:45 ` Boris Brezillon
0 siblings, 1 reply; 6+ messages in thread
From: RogerCC.Lin @ 2016-10-18 10:12 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 2016-10-18 at 07:19 +0200, Boris Brezillon wrote:
> On Tue, 18 Oct 2016 00:05:31 +0200
> Arnd Bergmann <arnd@arndb.de> wrote:
>
> > When building with -Wmaybe-uninitialized, gcc produces a silly false positive
> > warning for the mtk_ecc_encode function:
> >
> > drivers/mtd/nand/mtk_ecc.c: In function 'mtk_ecc_encode':
> > drivers/mtd/nand/mtk_ecc.c:402:15: error: 'val' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> >
> > The function for some reason contains a double byte swap on big-endian
> > builds to get the OOB data into the correct order again, and is written
> > in a slightly confusing way.
> >
> > Using a simple memcpy32_fromio() to read the data simplifies it a lot
> > so it becomes more readable and produces no warning. However, the
> > output might not have 32-bit alignment, so we have to use another
> > memcpy to avoid taking alignment faults or writing beyond the end
> > of the array.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> Jorge, RogerCC, can I have an Acked-by and/or Tested-by for this patch?
Tested, this patch is OK,
Tested-by: RogerCC Lin <rogercc.lin@mediatek.com>
>
> > ---
> > v2: move temporary buffer into struct mtk_ecc instead of having it
> > on the stack, as suggested by Boris Brezillon
> > ---
> > drivers/mtd/nand/mtk_ecc.c | 19 +++++++++----------
> > 1 file changed, 9 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/mtk_ecc.c b/drivers/mtd/nand/mtk_ecc.c
> > index d54f666..dbf2562 100644
> > --- a/drivers/mtd/nand/mtk_ecc.c
> > +++ b/drivers/mtd/nand/mtk_ecc.c
> > @@ -86,6 +86,8 @@ struct mtk_ecc {
> > struct completion done;
> > struct mutex lock;
> > u32 sectors;
> > +
> > + u8 eccdata[112];
> > };
> >
> > static inline void mtk_ecc_wait_idle(struct mtk_ecc *ecc,
> > @@ -366,9 +368,8 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
> > u8 *data, u32 bytes)
> > {
> > dma_addr_t addr;
> > - u8 *p;
> > - u32 len, i, val;
> > - int ret = 0;
> > + u32 len;
> > + int ret;
> >
> > addr = dma_map_single(ecc->dev, data, bytes, DMA_TO_DEVICE);
> > ret = dma_mapping_error(ecc->dev, addr);
> > @@ -393,14 +394,12 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
> >
> > /* Program ECC bytes to OOB: per sector oob = FDM + ECC + SPARE */
> > len = (config->strength * ECC_PARITY_BITS + 7) >> 3;
> > - p = data + bytes;
> >
> > - /* write the parity bytes generated by the ECC back to the OOB region */
> > - for (i = 0; i < len; i++) {
> > - if ((i % 4) == 0)
> > - val = readl(ecc->regs + ECC_ENCPAR(i / 4));
> > - p[i] = (val >> ((i % 4) * 8)) & 0xff;
> > - }
> > + /* write the parity bytes generated by the ECC back to temp buffer */
> > + __ioread32_copy(ecc->eccdata, ecc->regs + ECC_ENCPAR(0), round_up(len, 4));
> > +
> > + /* copy into possibly unaligned OOB region with actual length */
> > + memcpy(data + bytes, ecc->eccdata, len);
> > timeout:
> >
> > dma_unmap_single(ecc->dev, addr, bytes, DMA_TO_DEVICE);
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 20/28] net: bcm63xx: avoid referencing uninitialized variable
2016-10-17 22:16 ` [PATCH 20/28] net: bcm63xx: avoid referencing uninitialized variable Arnd Bergmann
@ 2016-10-18 18:21 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2016-10-18 18:21 UTC (permalink / raw)
To: linux-arm-kernel
From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 18 Oct 2016 00:16:08 +0200
> gcc found a reference to an uninitialized variable in the error handling
> of bcm_enet_open, introduced by a recent cleanup:
>
> drivers/net/ethernet/broadcom/bcm63xx_enet.c: In function 'bcm_enet_open'
> drivers/net/ethernet/broadcom/bcm63xx_enet.c:1129:2: warning: 'phydev' may be used uninitialized in this function [-Wmaybe-uninitialized]
>
> This makes the use of that variable conditional, so we only reference it
> here after it has been used before. Unlike my normal patches, I have not
> build-tested this one, as I don't currently have mips test in my
> randconfig setup.
>
> Fixes: 625eb8667d6f ("net: ethernet: broadcom: bcm63xx: use phydev from struct net_device")
> Cc: Philippe Reynes <tremyfr@gmail.com>
> Reported-by: kbuild test robot <fengguang.wu@intel.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 02/28] [v2] mtd: mtk: avoid warning in mtk_ecc_encode
2016-10-18 10:12 ` RogerCC.Lin
@ 2016-10-18 19:45 ` Boris Brezillon
0 siblings, 0 replies; 6+ messages in thread
From: Boris Brezillon @ 2016-10-18 19:45 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 18 Oct 2016 18:12:32 +0800
RogerCC.Lin <rogercc.lin@mediatek.com> wrote:
> On Tue, 2016-10-18 at 07:19 +0200, Boris Brezillon wrote:
> > On Tue, 18 Oct 2016 00:05:31 +0200
> > Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > > When building with -Wmaybe-uninitialized, gcc produces a silly false positive
> > > warning for the mtk_ecc_encode function:
> > >
> > > drivers/mtd/nand/mtk_ecc.c: In function 'mtk_ecc_encode':
> > > drivers/mtd/nand/mtk_ecc.c:402:15: error: 'val' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > >
> > > The function for some reason contains a double byte swap on big-endian
> > > builds to get the OOB data into the correct order again, and is written
> > > in a slightly confusing way.
> > >
> > > Using a simple memcpy32_fromio() to read the data simplifies it a lot
> > > so it becomes more readable and produces no warning. However, the
> > > output might not have 32-bit alignment, so we have to use another
> > > memcpy to avoid taking alignment faults or writing beyond the end
> > > of the array.
> > >
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> >
> > Jorge, RogerCC, can I have an Acked-by and/or Tested-by for this patch?
> Tested, this patch is OK,
> Tested-by: RogerCC Lin <rogercc.lin@mediatek.com>
Acked-by: Boris Brezillon <boris.brezillon@free-electrons.com>
Brian, can you take this patch for the next -rc?
>
> >
> > > ---
> > > v2: move temporary buffer into struct mtk_ecc instead of having it
> > > on the stack, as suggested by Boris Brezillon
> > > ---
> > > drivers/mtd/nand/mtk_ecc.c | 19 +++++++++----------
> > > 1 file changed, 9 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/mtd/nand/mtk_ecc.c b/drivers/mtd/nand/mtk_ecc.c
> > > index d54f666..dbf2562 100644
> > > --- a/drivers/mtd/nand/mtk_ecc.c
> > > +++ b/drivers/mtd/nand/mtk_ecc.c
> > > @@ -86,6 +86,8 @@ struct mtk_ecc {
> > > struct completion done;
> > > struct mutex lock;
> > > u32 sectors;
> > > +
> > > + u8 eccdata[112];
> > > };
> > >
> > > static inline void mtk_ecc_wait_idle(struct mtk_ecc *ecc,
> > > @@ -366,9 +368,8 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
> > > u8 *data, u32 bytes)
> > > {
> > > dma_addr_t addr;
> > > - u8 *p;
> > > - u32 len, i, val;
> > > - int ret = 0;
> > > + u32 len;
> > > + int ret;
> > >
> > > addr = dma_map_single(ecc->dev, data, bytes, DMA_TO_DEVICE);
> > > ret = dma_mapping_error(ecc->dev, addr);
> > > @@ -393,14 +394,12 @@ int mtk_ecc_encode(struct mtk_ecc *ecc, struct mtk_ecc_config *config,
> > >
> > > /* Program ECC bytes to OOB: per sector oob = FDM + ECC + SPARE */
> > > len = (config->strength * ECC_PARITY_BITS + 7) >> 3;
> > > - p = data + bytes;
> > >
> > > - /* write the parity bytes generated by the ECC back to the OOB region */
> > > - for (i = 0; i < len; i++) {
> > > - if ((i % 4) == 0)
> > > - val = readl(ecc->regs + ECC_ENCPAR(i / 4));
> > > - p[i] = (val >> ((i % 4) * 8)) & 0xff;
> > > - }
> > > + /* write the parity bytes generated by the ECC back to temp buffer */
> > > + __ioread32_copy(ecc->eccdata, ecc->regs + ECC_ENCPAR(0), round_up(len, 4));
> > > +
> > > + /* copy into possibly unaligned OOB region with actual length */
> > > + memcpy(data + bytes, ecc->eccdata, len);
> > > timeout:
> > >
> > > dma_unmap_single(ecc->dev, addr, bytes, DMA_TO_DEVICE);
> >
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-10-18 19:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20161017220342.1627073-1-arnd@arndb.de>
2016-10-17 22:05 ` [PATCH 02/28] [v2] mtd: mtk: avoid warning in mtk_ecc_encode Arnd Bergmann
2016-10-18 5:19 ` Boris Brezillon
2016-10-18 10:12 ` RogerCC.Lin
2016-10-18 19:45 ` Boris Brezillon
2016-10-17 22:16 ` [PATCH 20/28] net: bcm63xx: avoid referencing uninitialized variable Arnd Bergmann
2016-10-18 18:21 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).