| Message ID | a0dc1cda0b534e7a3e477c4b8d9805e4cb4e7211.1437426110.git.marcel.ziswiler@toradex.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Warren |
| Headers | show |
On Tue, 2015-07-21 at 00:35 +0200, Marcel Ziswiler wrote: > From: Marcel Ziswiler <marcel.ziswiler@toradex.com> > > Fix PIO read_byte() implementation not only used for the legacy READ ID > but also the PARAM command now required for proper ONFI detection. > > This fix is inspired by Lucas Stach's Linux Tegra NAND driver of late. Could you explain a bit more how this fixes issues with READ PARAM? > While at it also disable subpage writes. Why are these two changes combined? -Scott
On 27 July 2015 22:00:15 CEST, Scott Wood <scottwood@freescale.com> wrote: >On Tue, 2015-07-21 at 00:35 +0200, Marcel Ziswiler wrote: >> From: Marcel Ziswiler <marcel.ziswiler@toradex.com> >> >> Fix PIO read_byte() implementation not only used for the legacy READ >ID >> but also the PARAM command now required for proper ONFI detection. >> >> This fix is inspired by Lucas Stach's Linux Tegra NAND driver of >late. > >Could you explain a bit more how this fixes issues with READ PARAM? I vaguely remember that those commands are special on 16-bit bus NAND (e.g. always return 8-bit data regardless) and later Linux MTD fixed/changed the way this is handled which in turn broke once U-Boot pulled that in. Which as explained in my commit message this fixes by doing what Lucas does in Linux (not mainline yet but getting there soon I hope). >> While at it also disable subpage writes. > >Why are these two changes combined? Well just because it's a one line fix but if required I can split that in a separate patch as well.
On Tue, 2015-07-28 at 04:05 +0200, Marcel Ziswiler wrote: > On 27 July 2015 22:00:15 CEST, Scott Wood <scottwood@freescale.com> wrote: > > On Tue, 2015-07-21 at 00:35 +0200, Marcel Ziswiler wrote: > > > From: Marcel Ziswiler <marcel.ziswiler@toradex.com> > > > > > > Fix PIO read_byte() implementation not only used for the legacy READ > > ID > > > but also the PARAM command now required for proper ONFI detection. > > > > > > This fix is inspired by Lucas Stach's Linux Tegra NAND driver of > > late. > > > > Could you explain a bit more how this fixes issues with READ PARAM? > > I vaguely remember that those commands are special on 16-bit bus NAND (e.g. > always return 8-bit data regardless) and later Linux MTD fixed/changed the > way this is handled which in turn broke once U-Boot pulled that in. Which > as explained in my commit message this fixes by doing what Lucas does in > Linux (not mainline yet but getting there soon I hope). The more detail about this you can put in the changelog (describing what it is that Lucas does and how it helps), the better. > > > While at it also disable subpage writes. > > > > Why are these two changes combined? > > Well just because it's a one line fix but if required I can split that in a > separate patch as well. I think it should be separate, at least so it gets a separate changelog explaining why. -Scott
diff --git a/drivers/mtd/nand/tegra_nand.c b/drivers/mtd/nand/tegra_nand.c index b660f3b..9c90634 100644 --- a/drivers/mtd/nand/tegra_nand.c +++ b/drivers/mtd/nand/tegra_nand.c @@ -86,16 +86,6 @@ struct fdt_nand { struct nand_drv { struct nand_ctlr *reg; - - /* - * When running in PIO mode to get READ ID bytes from register - * RESP_0, we need this variable as an index to know which byte in - * register RESP_0 should be read. - * Because common code in nand_base.c invokes read_byte function two - * times for NAND_CMD_READID. - * And our controller returns 4 bytes at once in register RESP_0. - */ - int pio_byte_index; struct fdt_nand config; }; @@ -181,25 +171,16 @@ static int nand_waitfor_cmd_completion(struct nand_ctlr *reg) static uint8_t read_byte(struct mtd_info *mtd) { struct nand_chip *chip = mtd->priv; - u32 dword_read; struct nand_drv *info; info = (struct nand_drv *)chip->priv; - /* In PIO mode, only 4 bytes can be transferred with single CMD_GO. */ - if (info->pio_byte_index > 3) { - info->pio_byte_index = 0; - writel(CMD_GO | CMD_PIO - | CMD_RX | CMD_CE0, - &info->reg->command); - if (!nand_waitfor_cmd_completion(info->reg)) - printf("Command timeout\n"); - } + writel(CMD_GO | CMD_PIO | CMD_RX | CMD_CE0 | CMD_A_VALID, + &info->reg->command); + if (!nand_waitfor_cmd_completion(info->reg)) + printf("Command timeout\n"); - dword_read = readl(&info->reg->resp); - dword_read = dword_read >> (8 * info->pio_byte_index); - info->pio_byte_index++; - return (uint8_t)dword_read; + return (uint8_t)readl(&info->reg->resp); } /** @@ -314,6 +295,9 @@ static void nand_command(struct mtd_info *mtd, unsigned int command, if (column != -1 && (chip->options & NAND_BUSWIDTH_16)) column >>= 1; + /* Disable subpage writes as we do not provide ecc->hwctl */ + chip->options |= NAND_NO_SUBPAGE_WRITE; + nand_clear_interrupt_status(info->reg); /* Stop DMA engine, clear DMA completion status */ @@ -330,12 +314,8 @@ static void nand_command(struct mtd_info *mtd, unsigned int command, case NAND_CMD_READID: writel(NAND_CMD_READID, &info->reg->cmd_reg1); writel(column & 0xFF, &info->reg->addr_reg1); - writel(CMD_GO | CMD_CLE | CMD_ALE | CMD_PIO - | CMD_RX | - ((4 - 1) << CMD_TRANS_SIZE_SHIFT) - | CMD_CE0, - &info->reg->command); + writel(CMD_GO | CMD_CLE | CMD_ALE | CMD_CE0, + &info->reg->command); - info->pio_byte_index = 0; break; case NAND_CMD_PARAM: writel(NAND_CMD_PARAM, &info->reg->cmd_reg1); @@ -376,7 +356,6 @@ static void nand_command(struct mtd_info *mtd, unsigned int command, | ((1 - 0) << CMD_TRANS_SIZE_SHIFT) | CMD_CE0, &info->reg->command); - info->pio_byte_index = 0; break; case NAND_CMD_RESET: writel(NAND_CMD_RESET, &info->reg->cmd_reg1);