[BUG] pxa3xx: wait time out when scanning for bb

Ezequiel Garcia ezequiel at vanguardiasur.com.ar
Mon Dec 11 08:24:56 PST 2017


Greg,

On 11 December 2017 at 10:18, Greg Cook <greg at morpheus.ws> wrote:
> Sean,
>
> I am not completely up-to-date on this, but everything in your traces reads
> like the same issue I was having on bringup for Armada 385 nand (under 4.9).
> I've been stuck on another project, so I haven't had time to follow up
> further, but I just diffed against linux-stable v4.12 pxa3xx_nand.c and it
> looks like the problem is still there.
>
> As far as I can see, the driver is broken for OOB reads when BCH is enabled
> because the setup in prepare_set_command() results in drain_fifo() not
> reading enough words from the read fifo in the nfc2 IP block.
>

Yes, this sounds just like the bug I was expecting for OOB reads.

> The patch we are using is below. I have the following in my DTS.
> nand-keep-config is commented out because I was having some issues with
> u-boot at the time and it may no longer be relevant:

Probably.

> flash at d0000 {
> status = "okay";
> num-cs = <1>;
> //marvell,nand-keep-config;
> marvell,nand-enable-arbiter;
> nand-on-flash-bbt;
> nand-ecc-strength = <4>;
> nand-ecc-step-size = <512>;
> };
>
> --- /home/user/build/linux-stable/drivers/mtd/nand/pxa3xx_nand.c
> +++
> /home/user/build/beam/openwrt/build_dir/target-arm_cortex-a9+vfpv3_musl_eabi/linux-mvebu/linux-4.9.34/drivers/mtd/nand/pxa3xx_nand.c
> @@ -668,7 +669,7 @@
>
>  static void drain_fifo(struct pxa3xx_nand_info *info, void *data, int len)
>  {
> - if (info->ecc_bch) {
> + if (info->use_ecc && info->ecc_bch) {
>   u32 val;
>   int ret;
>
> @@ -1012,7 +1014,11 @@
>
>   if (info->cur_chunk < info->nfullchunks) {
>   info->step_chunk_size = info->chunk_size;
> - info->step_spare_size = info->spare_size;
> + if (info->use_ecc) {
> + info->step_spare_size = info->spare_size;
> + } else {
> + info->step_spare_size = info->spare_size + info->ecc_size;
> + }
>   } else {
>   info->step_chunk_size = info->last_chunk_size;
>   info->step_spare_size = info->last_spare_size;
>

Looks like a decent change.

Boris: do you think this is stable material worth backporting?
In other words, does it make sense to fix it, given the reworked driver?
-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar



More information about the linux-mtd mailing list