[PATCH] nand: gpmc: Handle bitflips in erased pages when using BCH ECC engine
From: David Rivshin
In the case of an erased (sub)page both the data and ECC are all 0xFF
bytes. This fails the normal ECC verification, as the computed ECC of
all-0xFF is not also 0xFF. The GPMC NAND driver attempted to detect
erased pages by checking that the ECC bytes are all-0xFF, but this had
two problems:
1) bitflips in the data were not corrected, so the data looked not-erased
2) bitflips in the ECC bytes were reported as uncorrectable ECC errors
The equivalent Linux driver [1] correctly handles this by counting the
number of 0-bits in the combination of data and ECC bytes. If the number
of 0-bits is less than the amount of bits correctable by the selected
ECC algorithm, then it is treated as an erased page with correctable
bitflips.
Implement similar, though simplified, logic in omap_correct_data_bch().
[1] see omap_elm_correct_data() in omap2.c
Signed-off-by: David Rivshin
---
drivers/mtd/nand/raw/omap_gpmc.c | 45 +---
1 file changed, 36 insertions(+), 9 deletions(-)
diff --git a/drivers/mtd/nand/raw/omap_gpmc.c b/drivers/mtd/nand/raw/omap_gpmc.c
index 97fd5690f5..5cfabe5c3c 100644
--- a/drivers/mtd/nand/raw/omap_gpmc.c
+++ b/drivers/mtd/nand/raw/omap_gpmc.c
@@ -506,19 +506,45 @@ static int omap_correct_data_bch(struct mtd_info *mtd,
uint8_t *dat,
/* check calculated ecc */
for (i = 0; i < ecc->bytes && !ecc_flag; i++) {
if (calc_ecc[i] != 0x00)
- ecc_flag = 1;
+ goto not_ecc_match;
}
- if (!ecc_flag)
- return 0;
+ return 0;
+not_ecc_match:
- /* check for whether its a erased-page */
- ecc_flag = 0;
- for (i = 0; i < ecc->bytes && !ecc_flag; i++) {
+ /* check for whether it's an erased-page */
+ for (i = 0; i < ecc->bytes; i++) {
if (read_ecc[i] != 0xff)
- ecc_flag = 1;
+ goto not_erased;
+ }
+ for (i = 0; i < SECTOR_BYTES; i++) {
+ if (dat[i] != 0xff)
+ goto not_erased;
+ }
+ return 0;
+not_erased:
+
+ /*
+* Check for whether it's an erased page with a correctable
+* number of bitflips. Erased pages have all 1's in the data,
+* so we just compute the number of 0 bits in the data and
+* see if it's under the correction threshold.
+*
+* NOTE: The check for a perfect erased page above is faster for
+* the more common case, even though it's logically redundant.
+*/
+ for (i = 0; i < ecc->bytes; i++)
+ error_count += hweight8(~read_ecc[i]);
+
+ for (i = 0; i < SECTOR_BYTES; i++)
+ error_count += hweight8(~dat[i]);
+
+ if (error_count <= ecc->strength) {
+ memset(read_ecc, 0xFF, ecc->bytes);
+ memset(dat, 0xFF, SECTOR_BYTES);
+ debug("nand: %u bit-flip(s) corrected in erased page\n",
+ error_count);
+ return error_count;
}
- if (!ecc_flag)
- return 0;
/*
* while reading ECC result we read it in big endian.
@@ -538,6 +564,7 @@ static int omap_correct_data_bch(struct mtd_info *mtd,
uint8_t *dat,
}
/* use elm module to check for errors */
elm_config(bch_type);
+ error_count = 0;
err = elm_check_error(calc_ecc, bch_type, &error_count, error_loc);
if (err)
return err;
base-commit: d80bb749fab53da72c4a0e09b8c2d2aaa3103c91
--
2.31.1
[PATCH] net: Do not respond to ICMP_ECHO_REQUEST if we do not have an IP address
From: David Rivshin While doing DHCP the interface IP is set to 0.0.0.0. This causes the check in net.c on dst_ip to be effectively skipped, and all IP datagrams are accepted up the IP stack. In the case of an ICMP_ECHO_REQUEST for the matching MAC address (regardless of destination IP), the result is that an ICMP_ECHO_REPLY is sent. The source address of the ICMP_ECHO_REPLY is 0.0.0.0, which is an illegal source address. This can happen in common practice with the following sequence: DHCP (U-Boot or OS) acquires IP address 10.0.0.1 System reboots U-Boot starts DHCP and send DHCP DISCOVER DHCP server decides to OFFER 10.0.0.1 again (perhaps because of existing lease or manual configuration) DHCP server tries to PING 10.0.0.1 to see if anyone is squatting on it DHCP server still has our MAC address in its ARP table for 10.0.0.1 U-Boot receives PING, and responds with an illegal source address This may further result in a the DHCP server seeing the response as confirmation that someone is squatting on 10.0.0.1, and picking a new IP address from the pool to try again Signed-off-by: David Rivshin --- net/ping.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/net/ping.c b/net/ping.c index 0e33660f6c..075df3663f 100644 --- a/net/ping.c +++ b/net/ping.c @@ -90,6 +90,9 @@ void ping_receive(struct ethernet_hdr *et, struct ip_udp_hdr *ip, int len) net_set_state(NETLOOP_SUCCESS); return; case ICMP_ECHO_REQUEST: + if (net_ip.s_addr == 0) + return; + eth_hdr_size = net_update_ether(et, et->et_src, PROT_IP); debug_cond(DEBUG_DEV_PKT, base-commit: 050acee119b3757fee3bd128f55d720fdd9bb890 -- 2.26.2
Re: [U-Boot] [PATCH] am335x-fb: don't override lcd_get_size()
On Mon, 25 Feb 2019 12:54:57 +0100
Hannes Schmelzer wrote:
> On 2/21/19 5:05 PM, David Rivshin wrote:
> > From: David Rivshin
> >
> > am335x-fb overrides the default lcd_get_size() to add an extra 32 bytes
> > compared to the normal calculation. While the gd->fb_base is an extra 32
> > bytes larger than the logical framebuffer size, the first 32 bytes is
> > always skipped. Adding that extra 32 bytes in lcd_get_size() can cause
> > check_cache_range() (called from lcd_sync(), via flush_dcache_range()) to
> > output a warning because the end address is not cacheline aligned, even
> > if the start and real framebuffer size are.
> >
> > When removing that "+ 0x20" the am335x-fb lcd_get_size() is identical
> > to the default one, so just delete it altogether.
> >
> > Signed-off-by: David Rivshin
> > ---
> > drivers/video/am335x-fb.c | 6 --
> > 1 file changed, 6 deletions(-)
> >
> > diff --git a/drivers/video/am335x-fb.c b/drivers/video/am335x-fb.c
> > index 51c1af587f..3a510cf91c 100644
> > --- a/drivers/video/am335x-fb.c
> > +++ b/drivers/video/am335x-fb.c
> > @@ -103,12 +103,6 @@ static struct am335x_lcdhw *lcdhw = (void
> > *)LCD_CNTL_BASE;
> >
> > DECLARE_GLOBAL_DATA_PTR;
> >
> > -int lcd_get_size(int *line_length)
> > -{
> > - *line_length = (panel_info.vl_col * NBITS(panel_info.vl_bpix)) / 8;
> > - return *line_length * panel_info.vl_row + 0x20;
> > -}
> > -
> > int am335xfb_init(struct am335x_lcdpanel *panel)
> > {
> > u32 raster_ctrl = 0;
> >
> > base-commit: d3689267f92c5956e09cc7d1baa4700141662bff
> I'll look into.
> But it's not easy as it looks like.
>
> Just downloaded latest TRM, looks like documentation had become better.
Hi Hannes,
Have you had a chance to look further? Just for more explanation, my
understanding is that lcd_get_size() should return the size of the pixel
array, starting at gd->fb_base. While gd->fb_base is allocated with an
extra 32 bytes at the start, it is updated in am335xfb_init:
gd->fb_base += 0x20
to move past that. After that point, gd->fb_base has an effective size
of just the usual (bytes_per_pixel * pixels_per_line * num_lines).
Specifically, my issue was with lcd_sync(), which does:
flush_dcache_range((ulong)lcd_base,
(ulong)(lcd_base + lcd_get_size(&line_length)));
and was flushing 32 bytes too much.
I do see that lcd_setmem() is also using lcd_get_size(), and if I'm
reading correctly it is reserving memory for a buffer of (at least)
that size. So perhaps in that place it would need the extra 32 bytes,
whereas everything after am335xfb_init() is called would ignore the
extra 32 bytes.
If that's the case, then perhaps we need two functions, one which
returns the amount of memory that needs to be reserved (with the
32bytes extra) and another which returns the size of the actual
framebuffer?
>
> cheers,
> Hannes7
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH] am335x-fb: don't override lcd_get_size()
From: David Rivshin
am335x-fb overrides the default lcd_get_size() to add an extra 32 bytes
compared to the normal calculation. While the gd->fb_base is an extra 32
bytes larger than the logical framebuffer size, the first 32 bytes is
always skipped. Adding that extra 32 bytes in lcd_get_size() can cause
check_cache_range() (called from lcd_sync(), via flush_dcache_range()) to
output a warning because the end address is not cacheline aligned, even
if the start and real framebuffer size are.
When removing that "+ 0x20" the am335x-fb lcd_get_size() is identical
to the default one, so just delete it altogether.
Signed-off-by: David Rivshin
---
drivers/video/am335x-fb.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/drivers/video/am335x-fb.c b/drivers/video/am335x-fb.c
index 51c1af587f..3a510cf91c 100644
--- a/drivers/video/am335x-fb.c
+++ b/drivers/video/am335x-fb.c
@@ -103,12 +103,6 @@ static struct am335x_lcdhw *lcdhw = (void *)LCD_CNTL_BASE;
DECLARE_GLOBAL_DATA_PTR;
-int lcd_get_size(int *line_length)
-{
- *line_length = (panel_info.vl_col * NBITS(panel_info.vl_bpix)) / 8;
- return *line_length * panel_info.vl_row + 0x20;
-}
-
int am335xfb_init(struct am335x_lcdpanel *panel)
{
u32 raster_ctrl = 0;
base-commit: d3689267f92c5956e09cc7d1baa4700141662bff
--
2.20.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH] spi: omap3: fix set_wordlen() reading from incorrect address for CHCONF
From: David Rivshin
_omap3_spi_set_wordlen() indexed the regs->channel[] array with the
old wordlen (instead of the chipselect number) when reading the current
CHCONF register value. This meant it read from the wrong memory location,
modified that value, and then wrote it back to the correct CHCONF
register. The end result is that most slave configuration settings would
be lost, such as clock divisor, clock/chipselect polarities, etc.
Fixes: 77b8d04854f4 ("spi: omap3: Convert to driver model")
Signed-off-by: David Rivshin
---
drivers/spi/omap3_spi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/spi/omap3_spi.c b/drivers/spi/omap3_spi.c
index c7fcf050a5..ff4c700645 100644
--- a/drivers/spi/omap3_spi.c
+++ b/drivers/spi/omap3_spi.c
@@ -415,7 +415,7 @@ static void _omap3_spi_set_wordlen(struct omap3_spi_priv
*priv)
unsigned int confr;
/* McSPI individual channel configuration */
- confr = readl(&priv->regs->channel[priv->wordlen].chconf);
+ confr = readl(&priv->regs->channel[priv->cs].chconf);
/* wordlength */
confr &= ~OMAP3_MCSPI_CHCONF_WL_MASK;
base-commit: d3689267f92c5956e09cc7d1baa4700141662bff
--
2.20.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
