linux-kernel - Re: [PATCH v6 4/6] mtd: rawnand: add NVIDIA Tegra NAND Flash controller driver

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] [day] [month] [year] [list]

Date:   Fri, 22 Jun 2018 00:01:28 +0200
From:   Boris Brezillon <boris.brezillon@...tlin.com>
To:     Stefan Agner <stefan@...er.ch>
Cc:     dwmw2@...radead.org, computersforpeace@...il.com,
        marek.vasut@...il.com, robh+dt@...nel.org, mark.rutland@....com,
        thierry.reding@...il.com, dev@...xeye.de,
        miquel.raynal@...tlin.com, richard@....at, marcel@...wiler.com,
        krzk@...nel.org, digetx@...il.com, benjamin.lindqvist@...ian.se,
        jonathanh@...dia.com, pdeschrijver@...dia.com, pgaikwad@...dia.com,
        mirza.krak@...il.com, gaireg@...reg.de,
        linux-mtd@...ts.infradead.org, linux-tegra@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 4/6] mtd: rawnand: add NVIDIA Tegra NAND Flash
 controller driver

Hi Stefan,

On Thu, 21 Jun 2018 22:31:43 +0200
Stefan Agner <stefan@...er.ch> wrote:

> Add support for the NAND flash controller found on NVIDIA
> Tegra 2 SoCs. This implementation does not make use of the
> command queue feature. Regular operations using ->exec_op()
> use PIO mode for data transfers. Raw, ECC and OOB read/writes
> make use of the DMA mode for data transfer.
> 
> Signed-off-by: Lucas Stach <dev@...xeye.de>
> Signed-off-by: Stefan Agner <stefan@...er.ch>
> Reviewed-by: Dmitry Osipenko <digetx@...il.com>
> ---
>  MAINTAINERS                       |    7 +
>  drivers/mtd/nand/raw/Kconfig      |    6 +
>  drivers/mtd/nand/raw/Makefile     |    1 +
>  drivers/mtd/nand/raw/tegra_nand.c | 1225 +++++++++++++++++++++++++++++
>  4 files changed, 1239 insertions(+)
>  create mode 100644 drivers/mtd/nand/raw/tegra_nand.c

I think we're almost good. No big issues spotted in this version.
Can you fix the CHECKs/WARNINGs reported by checkpatch --strict (most
of them are valid)?

> +
> +static int tegra_nand_remove(struct platform_device *pdev)
> +{
> +	struct tegra_nand_controller *ctrl = platform_get_drvdata(pdev);
> +
> +	nand_release(nand_to_mtd(ctrl->chip));

Use mtd_device_unregister() + nand_cleanup() instead of nand_release().
nand_release() is broken in that it does not check the return code of
mtd_device_unregister() before calling nand_cleanup(), and does not
propagate mtd_device_unregister() ret code to its caller. 

> +
> +	clk_disable_unprepare(ctrl->clk);
> +
> +	return 0;
> +}

Thanks for the great work you've done! Yay, another driver implementing
->exec_op()!

Boris