linux-kernel - Re: [PATCH 3/3] mtd: nand: add NAND driver for Broadcom STB NAND controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 7 Mar 2015 23:15:28 +0100
From:	Rafał Miłecki <zajec5@...il.com>
To:	Brian Norris <computersforpeace@...il.com>
Cc:	"linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
	Dmitry Torokhov <dtor@...gle.com>,
	Anatol Pomazao <anatol@...gle.com>,
	Ray Jui <rjui@...adcom.com>,
	Corneliu Doban <cdoban@...adcom.com>,
	Jonathan Richardson <jonathar@...adcom.com>,
	Florian Fainelli <f.fainelli@...il.com>,
	bcm-kernel-feedback-list@...adcom.com,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Kevin Cernekee <cernekee@...il.com>
Subject: Re: [PATCH 3/3] mtd: nand: add NAND driver for Broadcom STB NAND controller

On 7 March 2015 at 02:18, Brian Norris <computersforpeace@...il.com> wrote:
> +static int brcmnand_waitfunc(struct mtd_info *mtd, struct nand_chip *this)
> +{
> +       struct nand_chip *chip = mtd->priv;
> +       struct brcmnand_host *host = chip->priv;
> +       struct brcmnand_controller *ctrl = host->ctrl;
> +       unsigned long timeo = msecs_to_jiffies(100);
> +
> +       dev_dbg(ctrl->dev, "wait on native cmd %d\n", ctrl->cmd_pending);
> +       if (ctrl->cmd_pending &&
> +                       wait_for_completion_timeout(&ctrl->done, timeo) <= 0) {
> +               unsigned long cmd = brcmnand_read_reg(ctrl, BRCMNAND_CMD_START)
> +                                       >> brcmnand_cmd_shift(ctrl);
> +
> +               dev_err_ratelimited(ctrl->dev,
> +                       "timeout waiting for command %u (%ld)\n",
> +                       host->last_cmd, cmd);

I think that using host->last_cmd in the above message is really
misleading. Please consider following:

[    2.061139] [brcmnand_cmdfunc:1151] command:0x90 native_cmd:0x07
[    2.067123] brcmstb_nand 18028000.nand: send native cmd 7 addr_lo 0x40
[    2.168395] brcmstb_nand 18028000.nand: timeout waiting for command 144 (7)
[    2.175321] brcmstb_nand 18028000.nand: intfc status f0000000
[    2.181051] nand: device found, Manufacturer ID: 0x92, Chip ID: 0xf1
[    2.187370] nand: Eon NAND 128MiB 3,3V 8-bit
[    2.191632] nand: 128 MiB, SLC, erase size: 128 KiB, page size:
2048, OOB size: 64
[    2.199192] brcmstb_nand 18028000.nand: detected 128MiB total,
128KiB blocks, 2KiB pages, 16B OOB, 8-bit, BCH-8
[    2.199192]
[    2.210724] Scanning device for bad blocks
[    2.214812] brcmstb_nand 18028000.nand: send native cmd 1 addr_lo 0x0
[    2.318394] brcmstb_nand 18028000.nand: timeout waiting for command 144 (1)

As you can see, first there was a brcmnand_cmdfunc call with 0x90
(NAND_CMD_READID). It was translated into internal command 0x07
(CMD_DEVICE_ID_READ).

Then there was a call to brcmnand_read_by_pio which resulted in
sending internal command 0x01 (CMD_PAGE_READ). This one overwrote
previous command. So the timeout wasn't about 144 (0x90) anymore.

I suggest replacing that with something like
"timeout waiting for internal command 0x%02x\n", cmd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/